Skip to content

Commit

Permalink
Merge pull request #3700 from mulkieran/issue_stratisd_3699
Browse files Browse the repository at this point in the history
Accept "adv" key as well as "thp" key
  • Loading branch information
mulkieran authored Oct 17, 2024
2 parents 48cd9d1 + 3335f2c commit 07d780d
Show file tree
Hide file tree
Showing 4 changed files with 101 additions and 44 deletions.
31 changes: 12 additions & 19 deletions src/engine/strat_engine/backstore/backstore/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@ use crate::{
devices::UnownedDevices,
shared::BlockSizeSummary,
},
crypt::{
back_up_luks_header, handle::v1::CryptHandle, interpret_clevis_config,
restore_luks_header,
},
crypt::{back_up_luks_header, handle::v1::CryptHandle, restore_luks_header},
dm::{get_dm, list_of_backstore_devices, remove_optional_devices},
metadata::{MDADataSize, BDA},
names::{format_backstore_ids, CacheRole},
Expand Down Expand Up @@ -689,20 +686,8 @@ impl Backstore {
}
};

let mut parsed_config = clevis_info.clone();
let yes = interpret_clevis_config(pin, &mut parsed_config)?;

if let Some((ref existing_pin, ref existing_info)) = encryption_info.clevis_info() {
// Ignore thumbprint if stratis:tang:trust_url is set in the clevis_info
// config.
let mut config_to_check = existing_info.clone();
if yes {
if let Value::Object(ref mut ei) = config_to_check {
ei.remove("thp");
}
}

if (existing_pin.as_str(), &config_to_check) == (pin, &parsed_config)
if existing_pin.as_str() == pin
&& CryptHandle::can_unlock(
self.blockdevs()
.first()
Expand All @@ -716,8 +701,8 @@ impl Backstore {
Ok(false)
} else {
Err(StratisError::Msg(format!(
"Block devices have already been bound with pin {existing_pin} and config {config_to_check}; \
requested pin {pin} and config {parsed_config} can't be applied"
"Block devices have already been bound with pin {existing_pin} and config {existing_info}; \
requested pin {pin} and config {clevis_info} can't be applied"
)))
}
} else {
Expand Down Expand Up @@ -1333,6 +1318,14 @@ mod tests {
.unwrap();
cmd::udev_settle().unwrap();

assert_matches!(
backstore.bind_clevis(
"tang",
&json!({"url": env::var("TANG_URL").expect("TANG_URL env var required")})
),
Ok(false)
);

assert_matches!(
backstore.bind_clevis(
"tang",
Expand Down
26 changes: 11 additions & 15 deletions src/engine/strat_engine/backstore/backstore/v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::{
blockdevmgr::BlockDevMgr, cache_tier::CacheTier, data_tier::DataTier,
devices::UnownedDevices, shared::BlockSizeSummary,
},
crypt::{crypt_metadata_size, handle::v2::CryptHandle, interpret_clevis_config},
crypt::{crypt_metadata_size, handle::v2::CryptHandle},
dm::{get_dm, list_of_backstore_devices, remove_optional_devices, DEVICEMAPPER_PATH},
metadata::{MDADataSize, BDA},
names::{format_backstore_ids, CacheRole},
Expand Down Expand Up @@ -931,26 +931,14 @@ impl Backstore {
StratisError::Msg("No space has been allocated from the backstore".to_string())
})?;

let mut parsed_config = clevis_info.clone();
let yes = interpret_clevis_config(pin, &mut parsed_config)?;

if let Some((ref existing_pin, ref existing_info)) = handle.encryption_info().clevis_info()
{
// Ignore thumbprint if stratis:tang:trust_url is set in the clevis_info
// config.
let mut config_to_check = existing_info.clone();
if yes {
if let Value::Object(ref mut ei) = config_to_check {
ei.remove("thp");
}
}

if (existing_pin.as_str(), &config_to_check) == (pin, &parsed_config) {
if existing_pin.as_str() == pin {
Ok(false)
} else {
Err(StratisError::Msg(format!(
"Block devices have already been bound with pin {existing_pin} and config {existing_info}; \
requested pin {pin} and config {parsed_config} can't be applied"
requested pin {pin} and config {clevis_info} can't be applied"
)))
}
} else {
Expand Down Expand Up @@ -1421,6 +1409,14 @@ mod tests {
backstore.alloc(pool_uuid, &[Sectors(512)]).unwrap();
cmd::udev_settle().unwrap();

assert_matches!(
backstore.bind_clevis(
"tang",
&json!({"url": env::var("TANG_URL").expect("TANG_URL env var required")})
),
Ok(false)
);

assert_matches!(
backstore.bind_clevis(
"tang",
Expand Down
4 changes: 2 additions & 2 deletions src/engine/strat_engine/crypt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ mod shared;
pub use self::{
consts::CLEVIS_TANG_TRUST_URL,
shared::{
back_up_luks_header, crypt_metadata_size, interpret_clevis_config, register_clevis_token,
restore_luks_header, set_up_crypt_logging,
back_up_luks_header, crypt_metadata_size, register_clevis_token, restore_luks_header,
set_up_crypt_logging,
},
};
84 changes: 76 additions & 8 deletions src/engine/strat_engine/crypt/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,10 @@ pub fn clevis_info_from_metadata(
pin_dispatch(&subjson, CLEVIS_RECURSION_LIMIT).map(Some)
}

/// Returns true if the Tang config has a thumbprint or all Tang configs in the nested sss config
/// have thumbprints.
fn all_tang_configs_have_thp(
/// Returns true if the Tang config has a thumbprint or an advertisement
/// or all Tang configs in the nested sss config have thumbprints or
/// advertisements.
fn all_tang_configs_have_url_trust_info(
pin: &str,
clevis_config: &Value,
recursion_limit: u64,
Expand All @@ -222,6 +223,10 @@ fn all_tang_configs_have_thp(
Ok(obj
.get("thp")
.map(|val| val.as_str().is_some())
.or_else(|| {
obj.get("adv")
.map(|val| val.as_str().is_some() || val.as_object().is_some())
})
.unwrap_or(false))
} else {
Err(StratisError::Msg(format!(
Expand All @@ -232,7 +237,13 @@ fn all_tang_configs_have_thp(
if let Some(obj) = clevis_config.as_object() {
if let Some(obj) = obj.get("pins").and_then(|val| val.as_object()) {
obj.iter().try_fold(true, |b, (pin, config)| {
Ok(b && all_tang_configs_have_thp(pin, config, recursion_limit - 1)?)
Ok(
b && all_tang_configs_have_url_trust_info(
pin,
config,
recursion_limit - 1,
)?,
)
})
} else {
Err(StratisError::Msg(
Expand All @@ -256,8 +267,9 @@ fn all_tang_configs_have_thp(
/// from the configuration.
/// The only value to be returned is whether or not the bind command should be
/// passed the argument yes.
pub fn interpret_clevis_config(pin: &str, clevis_config: &mut Value) -> StratisResult<bool> {
let all_tang_has_thp = all_tang_configs_have_thp(pin, clevis_config, CLEVIS_RECURSION_LIMIT)?;
pub(super) fn interpret_clevis_config(pin: &str, clevis_config: &mut Value) -> StratisResult<bool> {
let all_tang_has_trust_info =
all_tang_configs_have_url_trust_info(pin, clevis_config, CLEVIS_RECURSION_LIMIT)?;
let yes = if pin == "tang" || pin == "sss" {
if let Some(map) = clevis_config.as_object_mut() {
map.remove(CLEVIS_TANG_TRUST_URL)
Expand All @@ -272,9 +284,9 @@ pub fn interpret_clevis_config(pin: &str, clevis_config: &mut Value) -> StratisR
false
};

if !all_tang_has_thp && !yes {
if !all_tang_has_trust_info && !yes {
return Err(StratisError::Msg(
"Either thumbprints for all Tang servers or a directive to trust all Tang servers is required".to_string()
"A thumbprint or advertisement must be provided for every Tang server or a directive to trust all Tang servers is required".to_string()
));
}

Expand Down Expand Up @@ -857,3 +869,59 @@ pub fn register_clevis_token() -> StratisResult<()> {
)?;
Ok(())
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_missing_thp_1() {
assert_matches!(
all_tang_configs_have_url_trust_info("tang", &json!({"url": "https://aurl"}), 12),
Ok(false)
)
}

#[test]
fn test_missing_thp_2() {
assert_matches!(
all_tang_configs_have_url_trust_info(
"sss",
&json!({"t": 1, "pins": {"tang": {"url": "https://myurl"}, "tpm2": {}}}),
12
),
Ok(false)
)
}

#[test]
fn test_present_adv_1() {
assert_matches!(
all_tang_configs_have_url_trust_info(
"sss",
&json!({"t": 1, "pins": {"tang": {"url": "https://myurl", "adv": "file"}, "tang": {"url": "https://junk", "thp": "abc"}}}),
12
),
Ok(true)
)
}

#[test]
fn test_present_no_check() {
assert_matches!(
all_tang_configs_have_url_trust_info(
"sss",
&json!({
"t": 1,
"stratis:tang:trust_url": true,
"pins": {
"tang": {"url": "https://myurl"},
"tpm2": {}
}
}),
12
),
Ok(false)
)
}
}

0 comments on commit 07d780d

Please sign in to comment.