From 21b1111b59fe9935d12e2e23abd5261458c4d77e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BChlbacher?= Date: Thu, 30 May 2024 18:31:10 +0200 Subject: [PATCH 1/3] refactor: reduce `UnlockPolicy` boilerplate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Thomas Mühlbacher --- Cargo.lock | 30 +++++++++++++++++++++++ Cargo.toml | 2 ++ src/commands/mount.rs | 3 ++- src/key.rs | 55 +++++++------------------------------------ 4 files changed, 42 insertions(+), 48 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 22ac0e3b..61e55c83 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -86,6 +86,8 @@ dependencies = [ "libc", "log", "rpassword", + "strum", + "strum_macros", "udev", "uuid", ] @@ -516,6 +518,12 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "rustversion" +version = "1.0.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "955d28af4278de8121b7ebeb796b6a45735dc01436d898801014aced2773a3d6" + [[package]] name = "shlex" version = "1.3.0" @@ -528,6 +536,28 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" +[[package]] +name = "strum" +version = "0.26.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5d8cec3501a5194c432b2b7976db6b7d10ec95c253208b45f83f7136aa985e29" +dependencies = [ + "strum_macros", +] + +[[package]] +name = "strum_macros" +version = "0.26.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c6cf59daf282c0a494ba14fd21610a0325f9f90ec9d1231dea26bcb1d696c946" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "rustversion", + "syn", +] + [[package]] name = "syn" version = "2.0.48" diff --git a/Cargo.toml b/Cargo.toml index 853123ee..8e762952 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -23,3 +23,5 @@ either = "1.5" rpassword = "7" bch_bindgen = { path = "bch_bindgen" } byteorder = "1.3" +strum = { version = "0.26", features = ["derive"] } +strum_macros = "0.26" diff --git a/src/commands/mount.rs b/src/commands/mount.rs index 474f58ff..7f5937d0 100644 --- a/src/commands/mount.rs +++ b/src/commands/mount.rs @@ -231,7 +231,8 @@ pub struct Cli { #[arg( short = 'k', long = "key_location", - default_value = "ask", + value_enum, + default_value_t, verbatim_doc_comment )] unlock_policy: UnlockPolicy, diff --git a/src/key.rs b/src/key.rs index b8ddcd02..284d1844 100644 --- a/src/key.rs +++ b/src/key.rs @@ -1,63 +1,25 @@ use std::{ - fmt, fs, + fmt::Debug, + fs, io::{stdin, IsTerminal}, }; -use crate::c_str; use anyhow::anyhow; use bch_bindgen::bcachefs::bch_sb_handle; -use clap::builder::PossibleValue; use log::info; -#[derive(Clone, Debug)] +use crate::c_str; + +#[derive(Clone, Debug, clap::ValueEnum, strum::Display)] pub enum UnlockPolicy { - None, Fail, Wait, Ask, } -impl std::str::FromStr for UnlockPolicy { - type Err = anyhow::Error; - fn from_str(s: &str) -> anyhow::Result { - match s { - "" | "none" => Ok(UnlockPolicy::None), - "fail" => Ok(UnlockPolicy::Fail), - "wait" => Ok(UnlockPolicy::Wait), - "ask" => Ok(UnlockPolicy::Ask), - _ => Err(anyhow!("Invalid unlock policy provided")), - } - } -} - -impl clap::ValueEnum for UnlockPolicy { - fn value_variants<'a>() -> &'a [Self] { - &[ - UnlockPolicy::None, - UnlockPolicy::Fail, - UnlockPolicy::Wait, - UnlockPolicy::Ask, - ] - } - - fn to_possible_value(&self) -> Option { - Some(match self { - Self::None => PossibleValue::new("none").alias(""), - Self::Fail => PossibleValue::new("fail"), - Self::Wait => PossibleValue::new("wait"), - Self::Ask => PossibleValue::new("ask"), - }) - } -} - -impl fmt::Display for UnlockPolicy { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - UnlockPolicy::None => write!(f, "None"), - UnlockPolicy::Fail => write!(f, "Fail"), - UnlockPolicy::Wait => write!(f, "Wait"), - UnlockPolicy::Ask => write!(f, "Ask"), - } +impl Default for UnlockPolicy { + fn default() -> Self { + Self::Ask } } @@ -185,6 +147,5 @@ pub fn apply_key_unlocking_policy( UnlockPolicy::Fail => Err(anyhow!("no passphrase available")), UnlockPolicy::Wait => Ok(wait_for_unlock(&block_device.sb().uuid())?), UnlockPolicy::Ask => ask_for_passphrase(block_device), - _ => Err(anyhow!("no unlock policy specified for locked filesystem")), } } From 25bce91b4bdcc37d063ba22081007c1ef76355d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BChlbacher?= Date: Thu, 30 May 2024 20:50:00 +0200 Subject: [PATCH 2/3] refactor: rename function again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bch2_sb_is_encrypted_and_locked()` simply does not check if the fs is locked. The name is misleading. Signed-off-by: Thomas Mühlbacher --- c_src/cmd_key.c | 2 +- c_src/crypto.c | 2 +- c_src/crypto.h | 2 +- src/commands/mount.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/c_src/cmd_key.c b/c_src/cmd_key.c index 552aa867..adb0ac8d 100644 --- a/c_src/cmd_key.c +++ b/c_src/cmd_key.c @@ -66,7 +66,7 @@ int cmd_unlock(int argc, char *argv[]) if (ret) die("Error opening %s: %s", dev, bch2_err_str(ret)); - if (!bch2_sb_is_encrypted_and_locked(sb.sb)) + if (!bch2_sb_is_encrypted(sb.sb)) die("%s is not encrypted", dev); if (check) diff --git a/c_src/crypto.c b/c_src/crypto.c index bc4a9aee..32671bd8 100644 --- a/c_src/crypto.c +++ b/c_src/crypto.c @@ -101,7 +101,7 @@ struct bch_key derive_passphrase(struct bch_sb_field_crypt *crypt, return key; } -bool bch2_sb_is_encrypted_and_locked(struct bch_sb *sb) +bool bch2_sb_is_encrypted(struct bch_sb *sb) { struct bch_sb_field_crypt *crypt; diff --git a/c_src/crypto.h b/c_src/crypto.h index b93b1b08..baea6d86 100644 --- a/c_src/crypto.h +++ b/c_src/crypto.h @@ -12,7 +12,7 @@ char *read_passphrase(const char *); char *read_passphrase_twice(const char *); struct bch_key derive_passphrase(struct bch_sb_field_crypt *, const char *); -bool bch2_sb_is_encrypted_and_locked(struct bch_sb *); +bool bch2_sb_is_encrypted(struct bch_sb *); void bch2_passphrase_check(struct bch_sb *, const char *, struct bch_key *, struct bch_encrypted_key *); void bch2_add_key(struct bch_sb *, const char *, const char *, const char *); diff --git a/src/commands/mount.rs b/src/commands/mount.rs index 7f5937d0..f6254366 100644 --- a/src/commands/mount.rs +++ b/src/commands/mount.rs @@ -344,7 +344,7 @@ fn cmd_mount_inner(opt: Cli) -> anyhow::Result<()> { .unwrap(); // Check if the filesystem's master key is encrypted and we don't have a key - if unsafe { bcachefs::bch2_sb_is_encrypted_and_locked(block_devices_to_mount[0].sb) } + if unsafe { bcachefs::bch2_sb_is_encrypted(block_devices_to_mount[0].sb) } && !key::check_for_key(&key_name)? { // First by password_file, if available From 22495e0d31e5a440fe58039941a2f916aee6c89f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20M=C3=BChlbacher?= Date: Thu, 30 May 2024 21:42:38 +0200 Subject: [PATCH 3/3] feat: rewrite key.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Introduce `KeyHandle` and `Passphrase` types - Refactor the functions into associated functions - Add `zeroizing` crate to handle passphrase memory safely Signed-off-by: Thomas Mühlbacher --- Cargo.lock | 21 ++++ Cargo.toml | 1 + src/commands/mount.rs | 76 +++++-------- src/key.rs | 257 +++++++++++++++++++++++------------------- 4 files changed, 196 insertions(+), 159 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 61e55c83..11b3bd5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -90,6 +90,7 @@ dependencies = [ "strum_macros", "udev", "uuid", + "zeroize", ] [[package]] @@ -773,3 +774,23 @@ name = "windows_x86_64_msvc" version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "dff9641d1cd4be8d1a070daf9e3773c5f67e78b4d9d42263020c057706765c04" + +[[package]] +name = "zeroize" +version = "1.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ced3678a2879b30306d323f4542626697a464a97c0a07c9aebf7ebca65cd4dde" +dependencies = [ + "zeroize_derive", +] + +[[package]] +name = "zeroize_derive" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce36e65b0d2999d2aafac989fb249189a141aee1f53c612c1f37d72631959f69" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] diff --git a/Cargo.toml b/Cargo.toml index 8e762952..9709e478 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -25,3 +25,4 @@ bch_bindgen = { path = "bch_bindgen" } byteorder = "1.3" strum = { version = "0.26", features = ["derive"] } strum_macros = "0.26" +zeroize = { version = "1", features = ["std", "zeroize_derive"] } diff --git a/src/commands/mount.rs b/src/commands/mount.rs index f6254366..49849aae 100644 --- a/src/commands/mount.rs +++ b/src/commands/mount.rs @@ -1,15 +1,19 @@ -use crate::key; -use crate::key::UnlockPolicy; +use std::{ + collections::HashMap, + ffi::{c_char, c_void, CString}, + io::{stdout, IsTerminal}, + path::{Path, PathBuf}, + {env, fs, str}, +}; + +use anyhow::{ensure, Result}; use bch_bindgen::{bcachefs, bcachefs::bch_sb_handle, opt_set, path_to_cstr}; use clap::Parser; use log::{debug, error, info, LevelFilter}; -use std::collections::HashMap; -use std::ffi::{c_char, c_void, CString}; -use std::io::{stdout, IsTerminal}; -use std::path::{Path, PathBuf}; -use std::{env, fs, str}; use uuid::Uuid; +use crate::key::{KeyHandle, Passphrase, UnlockPolicy}; + fn mount_inner( src: String, target: impl AsRef, @@ -304,11 +308,11 @@ fn devs_str_sbs_from_device( } } -fn cmd_mount_inner(opt: Cli) -> anyhow::Result<()> { +fn cmd_mount_inner(opt: Cli) -> Result<()> { // Grab the udev information once let udev_info = udev_bcachefs_info()?; - let (devices, block_devices_to_mount) = if opt.dev.starts_with("UUID=") { + let (devices, sbs) = if opt.dev.starts_with("UUID=") { let uuid = opt.dev.replacen("UUID=", "", 1); devs_str_sbs_from_uuid(&udev_info, uuid)? } else if opt.dev.starts_with("OLD_BLKID_UUID=") { @@ -333,44 +337,26 @@ fn cmd_mount_inner(opt: Cli) -> anyhow::Result<()> { } }; - if block_devices_to_mount.is_empty() { - Err(anyhow::anyhow!("No device found from specified parameters"))?; - } + ensure!(!sbs.is_empty(), "No device(s) to mount specified"); - let key_name = CString::new(format!( - "bcachefs:{}", - block_devices_to_mount[0].sb().uuid() - )) - .unwrap(); + let first_sb = sbs[0]; + let uuid = first_sb.sb().uuid(); - // Check if the filesystem's master key is encrypted and we don't have a key - if unsafe { bcachefs::bch2_sb_is_encrypted(block_devices_to_mount[0].sb) } - && !key::check_for_key(&key_name)? - { - // First by password_file, if available - let fallback_to_unlock_policy = if let Some(passphrase_file) = &opt.passphrase_file { - match key::read_from_passphrase_file( - &block_devices_to_mount[0], - passphrase_file.as_path(), - ) { - Ok(()) => { - // Decryption succeeded - false - } - Err(err) => { - // Decryption failed, fall back to unlock_policy - error!("Failed to decrypt using passphrase_file: {}", err); - true - } - } - } else { - // No passphrase_file specified, fall back to unlock_policy - true - }; - // If decryption by key_file was unsuccesful, prompt for passphrase (or follow key_policy) - if fallback_to_unlock_policy { - key::apply_key_unlocking_policy(&block_devices_to_mount[0], opt.unlock_policy)?; - }; + if unsafe { bcachefs::bch2_sb_is_encrypted(first_sb.sb) } { + let _key_handle = KeyHandle::new_from_search(&uuid).or_else(|_| { + opt.passphrase_file + .map(|path| { + Passphrase::new_from_file(&first_sb, path) + .inspect_err(|e| { + error!( + "Failed to read passphrase from file, falling back to prompt: {}", + e + ) + }) + .and_then(|p| KeyHandle::new(&first_sb, &p)) + }) + .unwrap_or_else(|| opt.unlock_policy.apply(&first_sb)) + }); } if let Some(mountpoint) = opt.mountpoint { diff --git a/src/key.rs b/src/key.rs index 284d1844..b4735d66 100644 --- a/src/key.rs +++ b/src/key.rs @@ -1,14 +1,27 @@ use std::{ - fmt::Debug, + ffi::{CStr, CString}, fs, io::{stdin, IsTerminal}, + mem, + path::Path, + thread, + time::Duration, }; -use anyhow::anyhow; -use bch_bindgen::bcachefs::bch_sb_handle; +use anyhow::{anyhow, ensure, Result}; +use bch_bindgen::{ + bcachefs::{self, bch_key, bch_sb_handle}, + c::bch2_chacha_encrypt_key, + keyutils::{self, keyctl_search}, +}; +use byteorder::{LittleEndian, ReadBytesExt}; use log::info; +use uuid::Uuid; +use zeroize::{ZeroizeOnDrop, Zeroizing}; -use crate::c_str; +use crate::{c_str, ErrnoError}; + +const BCH_KEY_MAGIC: &str = "bch**key"; #[derive(Clone, Debug, clap::ValueEnum, strum::Display)] pub enum UnlockPolicy { @@ -17,135 +30,151 @@ pub enum UnlockPolicy { Ask, } +impl UnlockPolicy { + pub fn apply(&self, sb: &bch_sb_handle) -> Result { + let uuid = sb.sb().uuid(); + + info!( + "Attempting to unlock filesystem {} with unlock policy '{}'", + uuid, self + ); + + match self { + Self::Fail => Err(anyhow!("no passphrase available")), + Self::Wait => Ok(KeyHandle::wait_for_unlock(&uuid)?), + Self::Ask => Passphrase::new_from_prompt().and_then(|p| KeyHandle::new(sb, &p)), + } + } +} + impl Default for UnlockPolicy { fn default() -> Self { Self::Ask } } -pub fn check_for_key(key_name: &std::ffi::CStr) -> anyhow::Result { - use bch_bindgen::keyutils::{self, keyctl_search}; - let key_name = key_name.to_bytes_with_nul().as_ptr() as *const _; - let key_type = c_str!("user"); - - let key_id = unsafe { keyctl_search(keyutils::KEY_SPEC_USER_KEYRING, key_type, key_name, 0) }; - if key_id > 0 { - info!("Key has become available"); - Ok(true) - } else { - match errno::errno().0 { - libc::ENOKEY | libc::EKEYREVOKED => Ok(false), - _ => Err(crate::ErrnoError(errno::errno()).into()), - } - } +/// A handle to an existing bcachefs key in the kernel keyring +pub struct KeyHandle { + // FIXME: Either these come in useful for something or we remove them + _uuid: Uuid, + _id: i64, } -fn wait_for_unlock(uuid: &uuid::Uuid) -> anyhow::Result<()> { - let key_name = std::ffi::CString::new(format!("bcachefs:{}", uuid)).unwrap(); - loop { - if check_for_key(&key_name)? { - break Ok(()); - } - - std::thread::sleep(std::time::Duration::from_secs(1)); - } -} - -// blocks indefinitely if no input is available on stdin -fn ask_for_passphrase(sb: &bch_sb_handle) -> anyhow::Result<()> { - let passphrase = if stdin().is_terminal() { - rpassword::prompt_password("Enter passphrase: ")? - } else { - info!("Trying to read passphrase from stdin..."); - let mut line = String::new(); - stdin().read_line(&mut line)?; - line - }; - unlock_master_key(sb, &passphrase) -} - -const BCH_KEY_MAGIC: &str = "bch**key"; -fn unlock_master_key(sb: &bch_sb_handle, passphrase: &str) -> anyhow::Result<()> { - use bch_bindgen::bcachefs::{self, bch2_chacha_encrypt_key, bch_encrypted_key, bch_key}; - use byteorder::{LittleEndian, ReadBytesExt}; - use std::os::raw::c_char; - - let key_name = std::ffi::CString::new(format!("bcachefs:{}", sb.sb().uuid())).unwrap(); - if check_for_key(&key_name)? { - return Ok(()); +impl KeyHandle { + pub fn format_key_name(uuid: &Uuid) -> CString { + CString::new(format!("bcachefs:{}", uuid)).unwrap() } - let bch_key_magic = BCH_KEY_MAGIC.as_bytes().read_u64::().unwrap(); - let crypt = sb.sb().crypt().unwrap(); - let passphrase = std::ffi::CString::new(passphrase.trim_end())?; // bind to keep the CString alive - let mut output: bch_key = unsafe { - bcachefs::derive_passphrase( - crypt as *const _ as *mut _, - passphrase.as_c_str().to_bytes_with_nul().as_ptr() as *const _, - ) - }; + pub fn new(sb: &bch_sb_handle, passphrase: &Passphrase) -> Result { + let bch_key_magic = BCH_KEY_MAGIC.as_bytes().read_u64::().unwrap(); + + let crypt = sb.sb().crypt().unwrap(); + let crypt_ptr = crypt as *const _ as *mut _; + + let mut output: bch_key = + unsafe { bcachefs::derive_passphrase(crypt_ptr, passphrase.get().as_ptr()) }; + + let mut key = *crypt.key(); - let mut key = *crypt.key(); - let ret = unsafe { - bch2_chacha_encrypt_key( - &mut output as *mut _, - sb.sb().nonce(), - &mut key as *mut _ as *mut _, - std::mem::size_of::(), - ) - }; - if ret != 0 { - Err(anyhow!("chacha decryption failure")) - } else if key.magic != bch_key_magic { - Err(anyhow!("failed to verify the password")) - } else { - let key_type = c_str!("user"); let ret = unsafe { - bch_bindgen::keyutils::add_key( - key_type, - key_name.as_c_str().to_bytes_with_nul() as *const _ as *const c_char, - &output as *const _ as *const _, - std::mem::size_of::(), - bch_bindgen::keyutils::KEY_SPEC_USER_KEYRING, + bch2_chacha_encrypt_key( + &mut output as *mut _, + sb.sb().nonce(), + &mut key as *mut _ as *mut _, + mem::size_of_val(&key), ) }; - if ret == -1 { - Err(anyhow!("failed to add key to keyring: {}", errno::errno())) + + ensure!(ret == 0, "chacha decryption failure"); + ensure!(key.magic == bch_key_magic, "failed to verify passphrase"); + + let key_name = Self::format_key_name(&sb.sb().uuid()); + let key_name = CStr::as_ptr(&key_name); + let key_type = c_str!("user"); + + let key_id = unsafe { + keyutils::add_key( + key_type, + key_name, + &output as *const _ as *const _, + mem::size_of_val(&output), + keyutils::KEY_SPEC_USER_KEYRING, + ) + }; + + if key_id > 0 { + info!("Found key in keyring"); + Ok(KeyHandle { + _uuid: sb.sb().uuid(), + _id: key_id as i64, + }) } else { - Ok(()) + Err(anyhow!("failed to add key to keyring: {}", errno::errno())) + } + } + + pub fn new_from_search(uuid: &Uuid) -> Result { + let key_name = Self::format_key_name(uuid); + let key_name = CStr::as_ptr(&key_name); + let key_type = c_str!("user"); + + let key_id = + unsafe { keyctl_search(keyutils::KEY_SPEC_USER_KEYRING, key_type, key_name, 0) }; + + if key_id > 0 { + info!("Found key in keyring"); + Ok(Self { + _uuid: *uuid, + _id: key_id, + }) + } else { + Err(ErrnoError(errno::errno()).into()) + } + } + + fn wait_for_unlock(uuid: &Uuid) -> Result { + loop { + match Self::new_from_search(uuid) { + Err(_) => thread::sleep(Duration::from_secs(1)), + r => break r, + } } } } -pub fn read_from_passphrase_file( - block_device: &bch_sb_handle, - passphrase_file: &std::path::Path, -) -> anyhow::Result<()> { - // Attempts to unlock the master key by password_file - // Return true if unlock was successful, false otherwise - info!( - "Attempting to unlock master key for filesystem {}, using password from file {}", - block_device.sb().uuid(), - passphrase_file.display() - ); - // Read the contents of the password_file into a string - let passphrase = fs::read_to_string(passphrase_file)?; - // Call decrypt_master_key with the read string - unlock_master_key(block_device, &passphrase) -} +#[derive(ZeroizeOnDrop)] +pub struct Passphrase(CString); -pub fn apply_key_unlocking_policy( - block_device: &bch_sb_handle, - unlock_policy: UnlockPolicy, -) -> anyhow::Result<()> { - info!( - "Attempting to unlock master key for filesystem {}, using unlock policy {}", - block_device.sb().uuid(), - unlock_policy - ); - match unlock_policy { - UnlockPolicy::Fail => Err(anyhow!("no passphrase available")), - UnlockPolicy::Wait => Ok(wait_for_unlock(&block_device.sb().uuid())?), - UnlockPolicy::Ask => ask_for_passphrase(block_device), +impl Passphrase { + fn get(&self) -> &CStr { + &self.0 + } + + // blocks indefinitely if no input is available on stdin + fn new_from_prompt() -> Result { + let passphrase = if stdin().is_terminal() { + Zeroizing::new(rpassword::prompt_password("Enter passphrase: ")?) + } else { + info!("Trying to read passphrase from stdin..."); + let mut line = Zeroizing::new(String::new()); + stdin().read_line(&mut line)?; + line + }; + + Ok(Self(CString::new(passphrase.as_str())?)) + } + + pub fn new_from_file(sb: &bch_sb_handle, passphrase_file: impl AsRef) -> Result { + let passphrase_file = passphrase_file.as_ref(); + + info!( + "Attempting to unlock key for filesystem {} with passphrase from file {}", + sb.sb().uuid(), + passphrase_file.display() + ); + + let passphrase = Zeroizing::new(fs::read_to_string(passphrase_file)?); + + Ok(Self(CString::new(passphrase.as_str())?)) } }