feat: rewrite key.rs

- 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 <tmuehlbacher@posteo.net>

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",
+]

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"] }

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<std::path::Path>,
@@ -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 {

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<KeyHandle> {
+        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<bool> {
-    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()
     }
 
+    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 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 _,
-        )
-    };
+        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 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"))
-    } else if key.magic != bch_key_magic {
-        Err(anyhow!("failed to verify the password"))
-    } else {
+
+        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 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>(),
-                bch_bindgen::keyutils::KEY_SPEC_USER_KEYRING,
+                mem::size_of_val(&output),
+                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(
-    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
+#[derive(ZeroizeOnDrop)]
+pub struct Passphrase(CString);
+
+impl Passphrase {
+    fn get(&self) -> &CStr {
+        &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 {}",
-        block_device.sb().uuid(),
+            "Attempting to unlock key for filesystem {} with passphrase from file {}",
+            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(
-    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),
+        let passphrase = Zeroizing::new(fs::read_to_string(passphrase_file)?);
+
+        Ok(Self(CString::new(passphrase.as_str())?))
     }
 }