From 5b427516c635251bbbb4b5dd9d07be2061e0e0ee Mon Sep 17 00:00:00 2001 From: ryan Date: Fri, 3 Nov 2023 20:42:33 -0500 Subject: [PATCH] keyfork-shard: enable step 1 decoding with openpgp-card, fix bug with multiple smartcards when decrypting --- .../src/bin/keyfork-shard-combine-openpgp.rs | 4 +- keyfork-shard/src/openpgp.rs | 53 ++++++++++-------- keyfork-shard/src/openpgp/keyring.rs | 4 ++ keyfork-shard/src/openpgp/smartcard.rs | 56 +++++++++++++------ 4 files changed, 76 insertions(+), 41 deletions(-) diff --git a/keyfork-shard/src/bin/keyfork-shard-combine-openpgp.rs b/keyfork-shard/src/bin/keyfork-shard-combine-openpgp.rs index 0a41e71..cead655 100644 --- a/keyfork-shard/src/bin/keyfork-shard-combine-openpgp.rs +++ b/keyfork-shard/src/bin/keyfork-shard-combine-openpgp.rs @@ -25,8 +25,8 @@ fn validate( // Load certs from path let certs = discover_certs(key_discovery)?; - let recovery_file = PathBuf::from(if recovery_file == "=" { - eprintln!("loading certs from stdin; note that prompting smartcard devices will not work"); + let recovery_file = PathBuf::from(if recovery_file == "-" { + eprintln!("loading certs from stdin; note that prompting smartcard PINs will not work"); "/dev/stdin" } else { recovery_file diff --git a/keyfork-shard/src/openpgp.rs b/keyfork-shard/src/openpgp.rs index e4ee90f..43e83b7 100644 --- a/keyfork-shard/src/openpgp.rs +++ b/keyfork-shard/src/openpgp.rs @@ -177,7 +177,14 @@ pub fn combine( let policy = NullPolicy::new(); let mut keyring = Keyring::new(certs); - let content = metadata.decrypt_with(&policy, &mut keyring)?; + let mut manager = SmartcardManager::new(); + let content = if keyring.is_empty() { + let card_fp = manager.load_any_card()?; + eprintln!("key discovery is empty, using hardware smartcard: {card_fp}"); + metadata.decrypt_with(&policy, &mut manager)? + } else { + metadata.decrypt_with(&policy, &mut keyring)? + }; let mut cert_parser = CertParser::from_bytes(&content)?; let root_cert = match cert_parser.next() { @@ -186,7 +193,8 @@ pub fn combine( None => panic!("No certs found in cert parser"), }; let certs = cert_parser.collect::>>()?; - keyring.set_root_cert(root_cert); + keyring.set_root_cert(root_cert.clone()); + manager.set_root_cert(root_cert); let mut messages: HashMap = HashMap::from_iter(certs.iter().map(|c| c.keyid()).zip(messages)); let mut decrypted_messages: HashMap> = HashMap::new(); @@ -231,9 +239,6 @@ pub fn combine( let left_from_threshold = threshold as usize - decrypted_messages.len(); if left_from_threshold > 0 { eprintln!("remaining keys: {left_from_threshold}, prompting yubikeys"); - // TODO: allow decrypt metadata with Yubikey, avoid require stage 1 - let mut manager = - SmartcardManager::new(keyring.root_cert().expect("stage 1 decrypt").clone()); let mut remaining_usable_certs = certs .iter() .filter(|cert| messages.contains_key(&cert.keyid())) @@ -241,29 +246,33 @@ pub fn combine( while threshold as usize - decrypted_messages.len() > 0 { remaining_usable_certs.retain(|cert| messages.contains_key(&cert.keyid())); - let mut fingerprints = HashMap::new(); + let mut key_by_fingerprints = HashMap::new(); + let mut total_fingerprints = vec![]; for valid_cert in remaining_usable_certs .iter() .map(|cert| cert.with_policy(&policy, None)) { let valid_cert = valid_cert?; - fingerprints.insert( - valid_cert.keyid(), - valid_cert - .keys() - .for_storage_encryption() - .map(|k| k.fingerprint()) - .collect::>(), - ); + let fp = valid_cert + .keys() + .for_storage_encryption() + .map(|k| k.fingerprint()) + .collect::>(); + for fp in &fp { + key_by_fingerprints.insert(fp.clone(), valid_cert.keyid()); + } + total_fingerprints.extend(fp.iter().cloned()); } - for (cert_id, fingerprints) in fingerprints { - if manager.load_any_fingerprint(fingerprints)?.is_some() { - // manager is loaded with a Card, utilize in tx - let message = messages.remove(&cert_id); - if let Some(message) = message { - let message = message.decrypt_with(&policy, &mut manager)?; - decrypted_messages.insert(cert_id, message); - } + + // Iterate over all fingerprints and use key_by_fingerprints to assoc with Enc. Message + if let Some(fp) = manager.load_any_fingerprint(total_fingerprints)? { + // soundness: `key_by_fingerprints` is extended by the same fps that are then + // inserted into `total_fingerprints` + let cert_keyid = key_by_fingerprints.get(&fp).unwrap().clone(); + let message = messages.remove(&cert_keyid); + if let Some(message) = message { + let message = message.decrypt_with(&policy, &mut manager)?; + decrypted_messages.insert(cert_keyid, message); } } } diff --git a/keyfork-shard/src/openpgp/keyring.rs b/keyfork-shard/src/openpgp/keyring.rs index 1b3f829..4bd0149 100644 --- a/keyfork-shard/src/openpgp/keyring.rs +++ b/keyfork-shard/src/openpgp/keyring.rs @@ -40,6 +40,10 @@ impl Keyring { } } + pub fn is_empty(&self) -> bool { + self.full_certs.is_empty() + } + // Sets the root cert, returning the old cert pub fn set_root_cert(&mut self, cert: impl Into>) -> Option { let mut cert = cert.into(); diff --git a/keyfork-shard/src/openpgp/smartcard.rs b/keyfork-shard/src/openpgp/smartcard.rs index a26aeff..83e87e8 100644 --- a/keyfork-shard/src/openpgp/smartcard.rs +++ b/keyfork-shard/src/openpgp/smartcard.rs @@ -13,34 +13,37 @@ use openpgp_card_sequoia::{state::Open, Card}; #[derive(Clone, Debug)] pub enum SmartcardFailure { - #[allow(dead_code)] - SmartCardPromptFailed, - SmartCardNotFound, + SmartCardHasNoDecrypt, } impl std::fmt::Display for SmartcardFailure { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Self::SmartCardPromptFailed => f.write_str("Attempt to prompt for smart card failed"), Self::SmartCardNotFound => f.write_str("No smart card backend was stored"), + Self::SmartCardHasNoDecrypt => f.write_str("Selected smart card has no decrypt key"), } } } impl std::error::Error for SmartcardFailure {} +#[derive(Default)] pub struct SmartcardManager { current_card: Option>, - root: Cert, + root: Option, } impl SmartcardManager { - pub fn new(root: Cert) -> Self { - Self { - current_card: None, - root, - } + pub fn new() -> Self { + Default::default() + } + + // Sets the root cert, returning the old cert + pub fn set_root_cert(&mut self, cert: impl Into>) -> Option { + let mut cert = cert.into(); + std::mem::swap(&mut self.root, &mut cert); + cert } /// Utility function to prompt for a newline from standard input. @@ -57,6 +60,7 @@ impl SmartcardManager { Ok(output) } + /* /// Return all [`Fingerprint`] for the currently accessible backends. /// /// NOTE: Only implemented for decryption keys. @@ -74,6 +78,29 @@ impl SmartcardManager { }) }) } + */ + + /// Load any backend. + pub fn load_any_card(&mut self) -> Result> { + PcscBackend::cards(None)? + .next() + .transpose()? + .ok_or(SmartcardFailure::SmartCardNotFound.into()) + .and_then( + |backend| -> Result> { + let mut card = Card::::new(backend)?; + let transaction = card.transaction()?; + let fingerprint = transaction + .fingerprints()? + .decryption() + .map(|fp| Fingerprint::from_bytes(fp.as_bytes())) + .ok_or(SmartcardFailure::SmartCardHasNoDecrypt)?; + drop(transaction); + self.current_card.replace(card); + Ok(fingerprint) + }, + ) + } /// Load a backend if any [`Fingerprint`] has been matched by a currently active card. /// @@ -125,13 +152,8 @@ impl VerificationHelper for &mut SmartcardManager { fn get_certs(&mut self, ids: &[openpgp::KeyHandle]) -> openpgp::Result> { Ok(ids .iter() - .flat_map(|kh| { - if &self.root.key_handle() == kh { - Some(self.root.clone()) - } else { - None - } - }) + .flat_map(|kh| self.root.as_ref().filter(|cert| cert.key_handle() == *kh)) + .cloned() .collect()) }