From d69d370b47262473a41c0b57b478f587a132a800 Mon Sep 17 00:00:00 2001 From: John Baublitz Date: Tue, 7 Jan 2025 19:38:47 -0500 Subject: [PATCH] Give better error message on Clevis unlock failure This commit fixes a bug in our clevis_decrypt function where it would report success even on failure. It also provides a better error message on failure to the user. --- src/engine/strat_engine/cmd.rs | 20 ++++- src/engine/strat_engine/crypt/handle/v1.rs | 12 +-- src/engine/strat_engine/crypt/handle/v2.rs | 15 ++-- src/engine/strat_engine/crypt/shared.rs | 89 ++++++++++++++-------- 4 files changed, 88 insertions(+), 48 deletions(-) diff --git a/src/engine/strat_engine/cmd.rs b/src/engine/strat_engine/cmd.rs index 7dc6fe2204..e02e8c7efc 100644 --- a/src/engine/strat_engine/cmd.rs +++ b/src/engine/strat_engine/cmd.rs @@ -461,6 +461,7 @@ pub fn clevis_decrypt(jwe: &Value) -> StratisResult { .arg("decrypt") .stdin(Stdio::piped()) .stdout(Stdio::piped()) + .stderr(Stdio::piped()) .spawn()?; let mut clevis_stdin = clevis_child.stdin.take().ok_or_else(|| { StratisError::Msg( @@ -472,7 +473,24 @@ pub fn clevis_decrypt(jwe: &Value) -> StratisResult { clevis_stdin.write_all(jose_output.as_bytes())?; drop(clevis_stdin); - clevis_child.wait()?; + let result = clevis_child.wait()?; + if result.code() != Some(0) { + match clevis_child.stderr { + Some(mut stderr) => { + let mut msg = String::new(); + stderr.read_to_string(&mut msg)?; + return Err(StratisError::Chained( + "Failed to retrieve Clevis passphrase".to_string(), + Box::new(StratisError::Msg(msg.trim().to_string())), + )); + } + None => { + return Err(StratisError::Msg( + "Failed to retrieve Clevis passphrase".to_string(), + )); + } + } + } let mut mem = SafeMemHandle::alloc(MAX_STRATIS_PASS_SIZE)?; let bytes_read = clevis_child diff --git a/src/engine/strat_engine/crypt/handle/v1.rs b/src/engine/strat_engine/crypt/handle/v1.rs index 7365f7582a..7e2625fd21 100644 --- a/src/engine/strat_engine/crypt/handle/v1.rs +++ b/src/engine/strat_engine/crypt/handle/v1.rs @@ -41,9 +41,9 @@ use crate::{ TOKEN_TYPE_KEY, }, shared::{ - acquire_crypt_device, activate, add_keyring_keyslot, check_luks2_token, - clevis_info_from_metadata, device_from_physical_path, ensure_inactive, - ensure_wiped, get_keyslot_number, interpret_clevis_config, + acquire_crypt_device, activate, activate_by_token, add_keyring_keyslot, + check_luks2_token, clevis_info_from_metadata, device_from_physical_path, + ensure_inactive, ensure_wiped, get_keyslot_number, interpret_clevis_config, key_desc_from_metadata, luks2_token_type_is_valid, read_key, wipe_fallback, }, }, @@ -520,11 +520,11 @@ impl CryptHandle { } if try_unlock_clevis { log_on_failure!( - device.token_handle().activate_by_token::<()>( + activate_by_token( + &mut device, None, Some(CLEVIS_LUKS_TOKEN_ID), - None, - CryptActivate::empty(), + CryptActivate::empty() ), "libcryptsetup reported that the decrypted Clevis passphrase \ is unable to open the encrypted device" diff --git a/src/engine/strat_engine/crypt/handle/v2.rs b/src/engine/strat_engine/crypt/handle/v2.rs index e3b5a885ac..d514de406e 100644 --- a/src/engine/strat_engine/crypt/handle/v2.rs +++ b/src/engine/strat_engine/crypt/handle/v2.rs @@ -37,10 +37,10 @@ use crate::{ LUKS2_TOKEN_ID, STRATIS_MEK_SIZE, }, shared::{ - acquire_crypt_device, activate, add_keyring_keyslot, clevis_info_from_metadata, - device_from_physical_path, ensure_wiped, get_keyslot_number, - interpret_clevis_config, key_desc_from_metadata, luks2_token_type_is_valid, - wipe_fallback, + acquire_crypt_device, activate, activate_by_token, add_keyring_keyslot, + clevis_info_from_metadata, device_from_physical_path, ensure_wiped, + get_keyslot_number, interpret_clevis_config, key_desc_from_metadata, + luks2_token_type_is_valid, wipe_fallback, }, }, device::blkdev_size, @@ -757,12 +757,7 @@ impl CryptHandle { None => 0, }; let mut crypt = self.acquire_crypt_device()?; - crypt.token_handle().activate_by_token::<()>( - None, - None, - None, - CryptActivate::KEYRING_KEY, - )?; + activate_by_token(&mut crypt, None, None, CryptActivate::KEYRING_KEY)?; crypt .context_handle() .resize(&self.activation_name().to_string(), processed_size) diff --git a/src/engine/strat_engine/crypt/shared.rs b/src/engine/strat_engine/crypt/shared.rs index b1db0db0ef..b4aae190d0 100644 --- a/src/engine/strat_engine/crypt/shared.rs +++ b/src/engine/strat_engine/crypt/shared.rs @@ -4,10 +4,11 @@ use std::{ fs::OpenOptions, - io::{ErrorKind, Write}, + io::Write, mem::forget, path::{Path, PathBuf}, slice::from_raw_parts_mut, + sync::{Mutex, OnceLock}, }; use data_encoding::BASE64URL_NOPAD; @@ -25,7 +26,7 @@ use libcryptsetup_rs::{ CryptDebugLevel, CryptLogLevel, CryptStatusInfo, CryptWipePattern, EncryptionFormat, }, }, - register, set_debug_level, set_log_callback, CryptDevice, CryptInit, LibcryptErr, + register, set_debug_level, set_log_callback, CryptDevice, CryptInit, }; use crate::{ @@ -46,6 +47,8 @@ use crate::{ stratis::{StratisError, StratisResult}, }; +static CLEVIS_ERROR: OnceLock>> = OnceLock::new(); + /// Set up crypt logging to log cryptsetup debug information at the trace level. pub fn set_up_crypt_logging() { fn logging_callback(level: CryptLogLevel, msg: &str, _: Option<&mut ()>) { @@ -523,29 +526,18 @@ pub fn activate( ))); } } - device - .token_handle() - .activate_by_token::<()>( - Some(&name.to_string()), - if unlock_method == UnlockMethod::Keyring { - Some(LUKS2_TOKEN_ID) - } else if unlock_method == UnlockMethod::Clevis { - Some(CLEVIS_LUKS_TOKEN_ID) - } else { - None - }, - None, - CryptActivate::empty(), - ) - .map_err(|e| match e { - LibcryptErr::IOError(e) => match e.kind() { - ErrorKind::NotFound => StratisError::Msg(format!( - "Token slot for unlock method {unlock_method:?} was empty" - )), - _ => StratisError::from(e), - }, - _ => StratisError::from(e), - })?; + activate_by_token( + device, + Some(&name.to_string()), + if unlock_method == UnlockMethod::Keyring { + Some(LUKS2_TOKEN_ID) + } else if unlock_method == UnlockMethod::Clevis { + Some(CLEVIS_LUKS_TOKEN_ID) + } else { + None + }, + CryptActivate::empty(), + )?; } // Check activation status. @@ -733,12 +725,7 @@ pub fn ensure_wiped( /// No activation will actually occur, only validation. pub fn check_luks2_token(device: &mut CryptDevice) -> StratisResult<()> { log_on_failure!( - device.token_handle().activate_by_token::<()>( - None, - Some(LUKS2_TOKEN_ID), - None, - CryptActivate::empty(), - ), + activate_by_token(device, None, Some(LUKS2_TOKEN_ID), CryptActivate::empty()), "libcryptsetup reported that the LUKS2 token is unable to \ open the encrypted device; this could be due to a malformed \ LUKS2 keyring token on the device or a missing or inaccessible \ @@ -844,6 +831,13 @@ unsafe extern "C" fn open( } Err(e) => { error!("{}", e.to_string()); + let guard = CLEVIS_ERROR + .get() + .expect("Must have been initialized") + .lock(); + if let Ok(mut g) = guard { + *g = Some(e); + } -1 } } @@ -867,9 +861,42 @@ pub fn register_clevis_token() -> StratisResult<()> { Some(validate), Some(dump), )?; + CLEVIS_ERROR + .set(Mutex::new(None)) + .expect("Only initialized here"); Ok(()) } +pub fn activate_by_token( + device: &mut CryptDevice, + device_name: Option<&str>, + token_slot: Option, + activate_flags: CryptActivate, +) -> StratisResult<()> { + let res = device.token_handle().activate_by_token::<()>( + device_name, + token_slot, + None, + activate_flags, + ); + let error = CLEVIS_ERROR + .get() + .expect("Must be initialized") + .lock() + .map(|mut guard| guard.take()); + match (error, res) { + (Ok(Some(e)), Err(_)) => Err(StratisError::Chained( + "Failed to unlock using token".to_string(), + Box::new(e), + )), + (Ok(Some(_)), _) => { + unreachable!("failure in callback fails function") + } + (_, Err(e)) => Err(StratisError::from(e)), + (_, Ok(_)) => Ok(()), + } +} + #[cfg(test)] mod tests { use super::*;