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

Rewrite rust key handling

7 files changed, 231 insertions, 200 deletions

diff --git a/Cargo.lock b/Cargo.lock
index 22ac0e3b..11b3bd5f 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -86,8 +86,11 @@ dependencies = [
  "libc",
  "log",
  "rpassword",
+ "strum",
+ "strum_macros",
  "udev",
  "uuid",
+ "zeroize",
 ]
 
 [[package]]
@@ -517,6 +520,12 @@ dependencies = [
 ]
 
 [[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"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -529,6 +538,28 @@ 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"
 source = "registry+https://github.com/rust-lang/crates.io-index"
@@ -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",
+]
diff --git a/Cargo.toml b/Cargo.toml
index 853123ee..9709e478 100644
--- a/Cargo.toml
+++ b/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"] }
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 474f58ff..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<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() {
-        Err(anyhow::anyhow!("No device found from specified parameters"))?;
-    }
-
-    let key_name = CString::new(format!(
-        "bcachefs:{}",
-        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_and_locked(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)?;
-        };
+    ensure!(!sbs.is_empty(), "No device(s) to mount specified");
+
+    let first_sb = sbs[0];
+    let uuid = first_sb.sb().uuid();
+
+    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 b8ddcd02..b4735d66 100644
--- a/src/key.rs
+++ b/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;
-use bch_bindgen::bcachefs::bch_sb_handle;
-use clap::builder::PossibleValue;
+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, ErrnoError};
+
+const BCH_KEY_MAGIC: &str = "bch**key";
 
-#[derive(Clone, Debug)]
+#[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<Self> {
-        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 UnlockPolicy {
+    pub fn apply(&self, sb: &bch_sb_handle) -> Result<KeyHandle> {
+        let uuid = sb.sb().uuid();
 
-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<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"),
-        })
-    }
-}
+        info!(
+            "Attempting to unlock filesystem {} with unlock policy '{}'",
+            uuid, self
+        );
 
-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"),
+            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)),
         }
     }
 }
 
-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()),
-        }
+impl Default for UnlockPolicy {
+    fn default() -> Self {
+        Self::Ask
     }
 }
 
-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(());
-        }
+/// 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,
+}
 
-        std::thread::sleep(std::time::Duration::from_secs(1));
+impl KeyHandle {
+    pub fn format_key_name(uuid: &Uuid) -> CString {
+        CString::new(format!("bcachefs:{}", uuid)).unwrap()
     }
-}
 
-// 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)
-}
+    pub fn new(sb: &bch_sb_handle, passphrase: &Passphrase) -> Result<Self> {
+        let bch_key_magic = BCH_KEY_MAGIC.as_bytes().read_u64::<LittleEndian>().unwrap();
 
-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(());
-    }
+        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 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 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>(),
-        )
-    };
-    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(
+            bch2_chacha_encrypt_key(
+                &mut output as *mut _,
+                sb.sb().nonce(),
+                &mut key as *mut _ as *mut _,
+                mem::size_of_val(&key),
+            )
+        };
+
+        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.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 {
+
+        if key_id > 0 {
+            info!("Found key in keyring");
+            Ok(KeyHandle {
+                _uuid: sb.sb().uuid(),
+                _id:   key_id as i64,
+            })
+        } else {
             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 {
-            Ok(())
+            Err(ErrnoError(errno::errno()).into())
         }
     }
-}
 
-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)
+    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 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),
-        _ => Err(anyhow!("no unlock policy specified for locked filesystem")),
+#[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 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())?))
     }
 }