Merge pull request #281 from tmuehlbacher/rewrite-rust-key-handling

Rewrite rust key handling

Cargo.lock

@@ -86,8 +86,11 @@ dependencies = [
  "libc",
  "log",
  "rpassword",
+ "strum",
+ "strum_macros",
  "udev",
  "uuid",
+ "zeroize",
 ]
 
 [[package]]
@@ -516,6 +519,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 +537,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"
@@ -743,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",
+]

Cargo.toml

@@ -23,3 +23,6 @@ either = "1.5"
 rpassword = "7"
 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"] }

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)

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;
 

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 *);

src/commands/mount.rs

@@ -1,15 +1,19 @@
-use crate::key;
+use std::{
-use crate::key::UnlockPolicy;
+    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<std::path::Path>,
@@ -231,7 +235,8 @@ pub struct Cli {
     #[arg(
         short = 'k',
         long = "key_location",
-        default_value = "ask",
+        value_enum,
+        default_value_t,
         verbatim_doc_comment
     )]
     unlock_policy: UnlockPolicy,
@@ -303,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=") {
@@ -332,44 +337,26 @@ fn cmd_mount_inner(opt: Cli) -> anyhow::Result<()> {
         }
     };
 
-    if block_devices_to_mount.is_empty() {
+    ensure!(!sbs.is_empty(), "No device(s) to mount specified");
-        Err(anyhow::anyhow!("No device found from specified parameters"))?;
-    }
 
-    let key_name = CString::new(format!(
+    let first_sb = sbs[0];
-        "bcachefs:{}",
+    let uuid = first_sb.sb().uuid();
-        block_devices_to_mount[0].sb().uuid()
-    ))
-    .unwrap();
 
-    // Check if the filesystem's master key is encrypted and we don't have a key
+    if unsafe { bcachefs::bch2_sb_is_encrypted(first_sb.sb) } {
-    if unsafe { bcachefs::bch2_sb_is_encrypted_and_locked(block_devices_to_mount[0].sb) }
+        let _key_handle = KeyHandle::new_from_search(&uuid).or_else(|_| {
-        && !key::check_for_key(&key_name)?
+            opt.passphrase_file
-    {
+                .map(|path| {
-        // First by password_file, if available
+                    Passphrase::new_from_file(&first_sb, path)
-        let fallback_to_unlock_policy = if let Some(passphrase_file) = &opt.passphrase_file {
+                        .inspect_err(|e| {
-            match key::read_from_passphrase_file(
+                            error!(
-                &block_devices_to_mount[0],
+                                "Failed to read passphrase from file, falling back to prompt: {}",
-                passphrase_file.as_path(),
+                                e
-            ) {
+                            )
-                Ok(()) => {
+                        })
-                    // Decryption succeeded
+                        .and_then(|p| KeyHandle::new(&first_sb, &p))
-                    false
+                })
-                }
+                .unwrap_or_else(|| opt.unlock_policy.apply(&first_sb))
-                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 let Some(mountpoint) = opt.mountpoint {

src/key.rs

@@ -1,190 +1,180 @@
 use std::{
-    fmt, fs,
+    ffi::{CStr, CString},
+    fs,
     io::{stdin, IsTerminal},
+    mem,
+    path::Path,
+    thread,
+    time::Duration,
 };
 
-use crate::c_str;
+use anyhow::{anyhow, ensure, Result};
-use anyhow::anyhow;
+use bch_bindgen::{
-use bch_bindgen::bcachefs::bch_sb_handle;
+    bcachefs::{self, bch_key, bch_sb_handle},
-use clap::builder::PossibleValue;
+    c::bch2_chacha_encrypt_key,
+    keyutils::{self, keyctl_search},
+};
+use byteorder::{LittleEndian, ReadBytesExt};
 use log::info;
+use uuid::Uuid;
+use zeroize::{ZeroizeOnDrop, Zeroizing};
 
-#[derive(Clone, Debug)]
+use crate::{c_str, ErrnoError};
+
+const BCH_KEY_MAGIC: &str = "bch**key";
+
+#[derive(Clone, Debug, clap::ValueEnum, strum::Display)]
 pub enum UnlockPolicy {
-    None,
     Fail,
     Wait,
     Ask,
 }
 
-impl std::str::FromStr for UnlockPolicy {
+impl UnlockPolicy {
-    type Err = anyhow::Error;
+    pub fn apply(&self, sb: &bch_sb_handle) -> Result<KeyHandle> {
-    fn from_str(s: &str) -> anyhow::Result<Self> {
+        let uuid = sb.sb().uuid();
-        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 {
+        info!(
-    fn value_variants<'a>() -> &'a [Self] {
+            "Attempting to unlock filesystem {} with unlock policy '{}'",
-        &[
+            uuid, self
-            UnlockPolicy::None,
+        );
-            UnlockPolicy::Fail,
-            UnlockPolicy::Wait,
-            UnlockPolicy::Ask,
-        ]
-    }
 
-    fn to_possible_value(&self) -> Option<PossibleValue> {
-        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"),
+            Self::Fail => Err(anyhow!("no passphrase available")),
-            UnlockPolicy::Fail => write!(f, "Fail"),
+            Self::Wait => Ok(KeyHandle::wait_for_unlock(&uuid)?),
-            UnlockPolicy::Wait => write!(f, "Wait"),
+            Self::Ask => Passphrase::new_from_prompt().and_then(|p| KeyHandle::new(sb, &p)),
-            UnlockPolicy::Ask => write!(f, "Ask"),
         }
     }
 }
 
-pub fn check_for_key(key_name: &std::ffi::CStr) -> anyhow::Result<bool> {
+impl Default for UnlockPolicy {
-    use bch_bindgen::keyutils::{self, keyctl_search};
+    fn default() -> Self {
-    let key_name = key_name.to_bytes_with_nul().as_ptr() as *const _;
+        Self::Ask
-    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()),
-        }
-    }
-}
-
-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
+/// A handle to an existing bcachefs key in the kernel keyring
-fn ask_for_passphrase(sb: &bch_sb_handle) -> anyhow::Result<()> {
+pub struct KeyHandle {
-    let passphrase = if stdin().is_terminal() {
+    // FIXME: Either these come in useful for something or we remove them
-        rpassword::prompt_password("Enter passphrase: ")?
+    _uuid: Uuid,
-    } else {
+    _id:   i64,
-        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";
+impl KeyHandle {
-fn unlock_master_key(sb: &bch_sb_handle, passphrase: &str) -> anyhow::Result<()> {
+    pub fn format_key_name(uuid: &Uuid) -> CString {
-    use bch_bindgen::bcachefs::{self, bch2_chacha_encrypt_key, bch_encrypted_key, bch_key};
+        CString::new(format!("bcachefs:{}", uuid)).unwrap()
-    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(());
     }
 
+    pub fn new(sb: &bch_sb_handle, passphrase: &Passphrase) -> Result<Self> {
         let bch_key_magic = BCH_KEY_MAGIC.as_bytes().read_u64::<LittleEndian>().unwrap();
+
         let crypt = sb.sb().crypt().unwrap();
-    let passphrase = std::ffi::CString::new(passphrase.trim_end())?; // bind to keep the CString alive
+        let crypt_ptr = crypt as *const _ as *mut _;
-    let mut output: bch_key = unsafe {
+
-        bcachefs::derive_passphrase(
+        let mut output: bch_key =
-            crypt as *const _ as *mut _,
+            unsafe { bcachefs::derive_passphrase(crypt_ptr, passphrase.get().as_ptr()) };
-            passphrase.as_c_str().to_bytes_with_nul().as_ptr() as *const _,
-        )
-    };
 
         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::<bch_encrypted_key>(),
+                mem::size_of_val(&key),
             )
         };
-    if ret != 0 {
+
-        Err(anyhow!("chacha decryption failure"))
+        ensure!(ret == 0, "chacha decryption failure");
-    } else if key.magic != bch_key_magic {
+        ensure!(key.magic == bch_key_magic, "failed to verify passphrase");
-        Err(anyhow!("failed to verify the password"))
+
-    } else {
+        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 ret = unsafe {
+
-            bch_bindgen::keyutils::add_key(
+        let key_id = unsafe {
+            keyutils::add_key(
                 key_type,
-                key_name.as_c_str().to_bytes_with_nul() as *const _ as *const c_char,
+                key_name,
                 &output as *const _ as *const _,
-                std::mem::size_of::<bch_key>(),
+                mem::size_of_val(&output),
-                bch_bindgen::keyutils::KEY_SPEC_USER_KEYRING,
+                keyutils::KEY_SPEC_USER_KEYRING,
             )
         };
-        if ret == -1 {
+
-            Err(anyhow!("failed to add key to keyring: {}", errno::errno()))
+        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<Self> {
+        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<Self> {
+        loop {
+            match Self::new_from_search(uuid) {
+                Err(_) => thread::sleep(Duration::from_secs(1)),
+                r => break r,
+            }
         }
     }
 }
 
-pub fn read_from_passphrase_file(
+#[derive(ZeroizeOnDrop)]
-    block_device: &bch_sb_handle,
+pub struct Passphrase(CString);
-    passphrase_file: &std::path::Path,
+
-) -> anyhow::Result<()> {
+impl Passphrase {
-    // Attempts to unlock the master key by password_file
+    fn get(&self) -> &CStr {
-    // Return true if unlock was successful, false otherwise
+        &self.0
+    }
+
+    // blocks indefinitely if no input is available on stdin
+    fn new_from_prompt() -> Result<Self> {
+        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<Path>) -> Result<Self> {
+        let passphrase_file = passphrase_file.as_ref();
+
         info!(
-        "Attempting to unlock master key for filesystem {}, using password from file {}",
+            "Attempting to unlock key for filesystem {} with passphrase from file {}",
-        block_device.sb().uuid(),
+            sb.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)
-}
 
-pub fn apply_key_unlocking_policy(
+        let passphrase = Zeroizing::new(fs::read_to_string(passphrase_file)?);
-    block_device: &bch_sb_handle,
+
-    unlock_policy: UnlockPolicy,
+        Ok(Self(CString::new(passphrase.as_str())?))
-) -> 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),
-        _ => Err(anyhow!("no unlock policy specified for locked filesystem")),
     }
 }