Skip to content

Commit

Permalink
Give better error message on Clevis unlock failure
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jbaublitz committed Jan 8, 2025
1 parent 96dd4b4 commit d69d370
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 48 deletions.
20 changes: 19 additions & 1 deletion src/engine/strat_engine/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,7 @@ pub fn clevis_decrypt(jwe: &Value) -> StratisResult<SizedKeyMemory> {
.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(
Expand All @@ -472,7 +473,24 @@ pub fn clevis_decrypt(jwe: &Value) -> StratisResult<SizedKeyMemory> {
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
Expand Down
12 changes: 6 additions & 6 deletions src/engine/strat_engine/crypt/handle/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
Expand Down Expand Up @@ -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"
Expand Down
15 changes: 5 additions & 10 deletions src/engine/strat_engine/crypt/handle/v2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
89 changes: 58 additions & 31 deletions src/engine/strat_engine/crypt/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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::{
Expand All @@ -46,6 +47,8 @@ use crate::{
stratis::{StratisError, StratisResult},
};

static CLEVIS_ERROR: OnceLock<Mutex<Option<StratisError>>> = 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 ()>) {
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 \
Expand Down Expand Up @@ -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
}
}
Expand All @@ -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<u32>,
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::*;
Expand Down

0 comments on commit d69d370

Please sign in to comment.