keyfork-prompt: make clippy happy

keyfork-derive-openpgp/src/lib.rs

@@ -45,7 +45,7 @@ pub enum Error {
 
 pub type Result<T, E = Error> = std::result::Result<T, E>;
 
-pub fn derive(data: DerivationResponse, keys: &[KeyFlags], userid: UserID) -> Result<Cert> {
+pub fn derive(data: DerivationResponse, keys: &[KeyFlags], userid: &UserID) -> Result<Cert> {
     let primary_key_flags = match keys.get(0) {
         Some(kf) if kf.for_certification() => kf,
         _ => return Err(Error::NotCert),
@@ -96,16 +96,14 @@ pub fn derive(data: DerivationResponse, keys: &[KeyFlags], userid: UserID) -> Re
                     None,
                     None,
                     epoch,
-                )
+                )?
-                .expect("keyforkd gave invalid cv25519 key"),
             )
         } else {
             Key::from(
                 Key4::<_, SubordinateRole>::import_secret_ed25519(
                     &PrivateKey::to_bytes(derived_key.private_key()),
                     epoch,
-                )
+                )?
-                .expect("keyforkd gave invalid ed25519 key"),
             )
         };
 

keyfork-derive-openpgp/src/main.rs

@@ -5,7 +5,7 @@ use keyfork_derive_util::{
     DerivationIndex, DerivationPath,
 };
 use keyforkd_client::Client;
-use sequoia_openpgp::{packet::UserID, types::KeyFlags};
+use sequoia_openpgp::{packet::UserID, types::KeyFlags, armor::{Kind, Writer}, serialize::Marshal};
 
 #[derive(Debug, thiserror::Error)]
 enum Error {
@@ -15,41 +15,41 @@ enum Error {
 
 #[derive(Debug)]
 struct KeyType {
-    _inner: KeyFlags,
+    inner: KeyFlags,
 }
 
 impl Default for KeyType {
     fn default() -> Self {
         Self {
-            _inner: KeyFlags::empty(),
+            inner: KeyFlags::empty(),
         }
     }
 }
 
 impl KeyType {
     fn certify(mut self) -> Self {
-        self._inner = self._inner.set_certification();
+        self.inner = self.inner.set_certification();
         self
     }
 
     fn sign(mut self) -> Self {
-        self._inner = self._inner.set_signing();
+        self.inner = self.inner.set_signing();
         self
     }
 
     fn encrypt(mut self) -> Self {
-        self._inner = self._inner.set_transport_encryption();
+        self.inner = self.inner.set_transport_encryption();
-        self._inner = self._inner.set_storage_encryption();
+        self.inner = self.inner.set_storage_encryption();
         self
     }
 
     fn authenticate(mut self) -> Self {
-        self._inner = self._inner.set_authentication();
+        self.inner = self.inner.set_authentication();
         self
     }
 
     fn inner(&self) -> &KeyFlags {
-        &self._inner
+        &self.inner
     }
 }
 
@@ -115,12 +115,7 @@ fn run() -> Result<(), Box<dyn std::error::Error>> {
         .map(|kt| kt.inner().clone())
         .collect::<Vec<_>>();
 
-    let cert = keyfork_derive_openpgp::derive(derived_data, subkeys.as_slice(), default_userid)?;
+    let cert = keyfork_derive_openpgp::derive(derived_data, subkeys.as_slice(), &default_userid)?;
-
-    use sequoia_openpgp::{
-        armor::{Kind, Writer},
-        serialize::Marshal,
-    };
 
     let mut w = Writer::new(std::io::stdout(), Kind::SecretKey)?;
 

keyfork-derive-path-data/src/lib.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::unreadable_literal)]
+
 use keyfork_derive_util::{DerivationIndex, DerivationPath};
 
 pub static OPENPGP: DerivationIndex = DerivationIndex::new_unchecked(7366512, true);
@@ -19,7 +21,7 @@ impl std::fmt::Display for Target {
 /// Determine the closest [`Target`] for the given path. This method is intended to be used by
 /// `keyforkd` to provide an optional textual prompt to what a client is attempting to derive.
 pub fn guess_target(path: &DerivationPath) -> Option<Target> {
-    Some(match Vec::from_iter(path.iter())[..] {
+    Some(match path.iter().collect::<Vec<_>>()[..] {
         [t, index] if t == &OPENPGP => Target::OpenPGP(index.clone()),
         _ => return None,
     })

keyfork-derive-util/src/index.rs

@@ -90,7 +90,7 @@ mod tests {
     #[test]
     #[should_panic]
     fn fails_on_high_index() {
-        DerivationIndex::new(0x80000001, false).unwrap();
+        DerivationIndex::new(0x8000_0001, false).unwrap();
     }
 
     #[test]
@@ -100,19 +100,19 @@ mod tests {
 
     #[test]
     fn misc_values() -> Result<()> {
-        assert_eq!(DerivationIndex::new(0x80000000 - 1, true)?.0, u32::MAX);
+        assert_eq!(DerivationIndex::new(0x8000_0000 - 1, true)?.0, u32::MAX);
         assert_eq!(DerivationIndex::new(0x2, false)?.0, 2);
-        assert_eq!(DerivationIndex::new(0x00ABCDEF, true)?.0, 0x80ABCDEF);
+        assert_eq!(DerivationIndex::new(0x00AB_CDEF, true)?.0, 0x80AB_CDEF);
-        assert_eq!(DerivationIndex::new(0x00ABCDEF, false)?.0, 0x00ABCDEF);
+        assert_eq!(DerivationIndex::new(0x00AB_CDEF, false)?.0, 0x00AB_CDEF);
         Ok(())
     }
 
     #[test]
     fn from_str() -> Result<()> {
-        assert_eq!(DerivationIndex::from_str("100000")?.0, 100000);
+        assert_eq!(DerivationIndex::from_str("100000")?.0, 100_000);
         assert_eq!(
             DerivationIndex::from_str("100000'")?.0,
-            (0b1 << 31) + 100000
+            (0b1 << 31) + 100_000
         );
         Ok(())
     }
@@ -136,6 +136,6 @@ mod tests {
     #[test]
     #[should_panic]
     fn from_str_fails_on_high_index() {
-        DerivationIndex::from_str(&0x80000001u32.to_string()).unwrap();
+        DerivationIndex::from_str(&0x8000_0001u32.to_string()).unwrap();
     }
 }

keyfork-derive-util/src/request.rs

@@ -1,3 +1,6 @@
+// Because all algorithms make use of wildcard matching
+#![allow(clippy::match_wildcard_for_single_variants)]
+
 use crate::{
     extended_key::private_key::Error as XPrvError, DerivationPath, ExtendedPrivateKey, PrivateKey,
 };

keyfork-derive-util/src/tests.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::implicit_clone)]
+
 use crate::{request::*, *};
 use hex_literal::hex;
 use std::str::FromStr;
@@ -48,7 +50,7 @@ fn secp256k1() {
 
             // Tests for DerivationRequest
             let request = DerivationRequest::new(DerivationAlgorithm::Secp256k1, &chain);
-            let response = request.derive_with_master_seed(seed.to_vec()).unwrap();
+            let response = request.derive_with_master_seed(seed.clone()).unwrap();
             assert_eq!(&response.data, private_key, "test: {chain}");
         }
     }
@@ -119,7 +121,7 @@ fn panics_at_depth() {
 
     let seed = hex!("000102030405060708090a0b0c0d0e0f");
     let mut xkey = ExtendedPrivateKey::<SigningKey>::new(seed).unwrap();
-    for i in 0..u32::from(u8::MAX) + 1 {
+    for i in 0..=u32::from(u8::MAX) {
         xkey = xkey
             .derive_child(&DerivationIndex::new(i, true).unwrap())
             .unwrap();

keyfork-mnemonic-util/src/lib.rs

@@ -126,7 +126,7 @@ impl Display for Mnemonic {
             .filter_map(|word| self.wordlist.get_word(word))
             .peekable();
         while let Some(word) = iter.next() {
-            f.write_str(&word)?;
+            f.write_str(word)?;
             if iter.peek().is_some() {
                 f.write_str(" ")?;
             }
@@ -180,7 +180,7 @@ impl FromStr for Mnemonic {
             let word = wordlist
                 .0
                 .iter()
-                .position(|w| &w == word)
+                .position(|w| w == word)
                 .ok_or(MnemonicFromStrError::InvalidWord(index))?;
             usize_words.push(word);
             for bit in 0..11 {
@@ -211,7 +211,7 @@ impl FromStr for Mnemonic {
         let hash = hasher.finalize().to_vec();
 
         for (i, bit) in checksum_bits.iter().enumerate() {
-            if !hash[i / 8] & (1 << (7 - (i % 8))) == *bit as u8 {
+            if !hash[i / 8] & (1 << (7 - (i % 8))) == u8::from(*bit) {
                 return Err(MnemonicFromStrError::InvalidChecksum);
             }
         }
@@ -246,6 +246,11 @@ impl Mnemonic {
         Ok(unsafe { Self::from_raw_entropy(bytes, wordlist) })
     }
 
+    /// # Safety
+    ///
+    /// This function can potentially produce mnemonics that are not BIP-0039 compliant or can't
+    /// properly be encoded as a mnemonic. It is assumed the caller asserts the byte count is `% 4
+    /// == 0`.
     pub unsafe fn from_raw_entropy(bytes: &[u8], wordlist: Arc<Wordlist>) -> Mnemonic {
         Mnemonic {
             entropy: bytes.to_vec(),

keyfork-prompt/Cargo.toml

@@ -6,8 +6,9 @@ edition = "2021"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [features]
-default = ["mnemonic"]
+default = ["mnemonic", "qrencode"]
 mnemonic = ["keyfork-mnemonic-util"]
+qrencode = []
 
 [dependencies]
 crossterm = { version = "0.27.0", default-features = false, features = ["use-dev-tty", "events", "bracketed-paste"] }

keyfork-prompt/src/bin/test-basic-prompt.rs

@@ -1,14 +1,19 @@
-use std::{io::{stdin, stdout}, str::FromStr};
+use std::{
+    io::{stdin, stdout},
+    str::FromStr,
+};
 
-use keyfork_prompt::*;
 use keyfork_mnemonic_util::Mnemonic;
+use keyfork_prompt::{qrencode, Message, PromptManager};
 
-pub fn main() -> Result<()> {
+fn main() -> Result<(), Box<dyn std::error::Error>> {
     let mut mgr = PromptManager::new(stdin(), stdout())?;
     mgr.prompt_passphrase("Passphrase: ")?;
     let string = mgr.prompt_wordlist("Mnemonic: ", &Default::default())?;
     let mnemonic = Mnemonic::from_str(&string).unwrap();
     let entropy = mnemonic.entropy();
-    mgr.prompt_message(Message::Text(format!("Your entropy is: {entropy:X?}")))?;
+    mgr.prompt_message(&Message::Text(format!("Your entropy is: {entropy:X?}")))?;
+    let qrcode = qrencode::qrencode(&string)?;
+    mgr.prompt_message(&Message::Data(qrcode))?;
     Ok(())
 }

keyfork-prompt/src/lib.rs

@@ -17,8 +17,11 @@ use crossterm::{
 
 mod alternate_screen;
 mod raw_mode;
-use alternate_screen::*;
+use alternate_screen::AlternateScreen;
-use raw_mode::*;
+use raw_mode::RawMode;
+
+#[cfg(feature = "qrencode")]
+pub mod qrencode;
 
 #[derive(thiserror::Error, Debug)]
 pub enum Error {
@@ -79,6 +82,7 @@ where
 
     // TODO: create a wrapper for bracketed paste similar to RawMode
     #[cfg(feature = "mnemonic")]
+    #[allow(clippy::too_many_lines)]
     pub fn prompt_wordlist(&mut self, prompt: &str, wordlist: &Wordlist) -> Result<String> {
         let mut terminal = AlternateScreen::new(&mut self.write)?;
         let mut terminal = RawMode::new(&mut terminal)?;
@@ -134,7 +138,7 @@ where
                         }
                     }
                     KeyCode::Char(' ') => {
-                        if !input.chars().rev().next().is_some_and(char::is_whitespace) {
+                        if !input.chars().next_back().is_some_and(char::is_whitespace) {
                             input.push(' ');
                         }
                     }
@@ -148,21 +152,27 @@ where
 
             let usable_space = cols as usize - prefix_length - 1;
 
+            #[allow(clippy::cast_possible_truncation)]
             terminal
-                .queue(cursor::MoveToColumn(prefix_length as u16))?
+                .queue(cursor::MoveToColumn(
+                    std::cmp::min(u16::MAX as usize, prefix_length) as u16,
+                ))?
                 .queue(terminal::Clear(terminal::ClearType::UntilNewLine))?
                 .flush()?;
 
             let printable_input_start = if input.len() > usable_space {
                 let start_index = input.len() - usable_space;
-                input
+                // Find a word boundary, otherwise slice the current word in half
+                if let Some((index, _)) = input
                     .chars()
                     .enumerate()
                     .skip(start_index)
-                    .skip_while(|(_, ch)| !ch.is_whitespace())
+                    .find(|(_, ch)| ch.is_whitespace())
-                    .next()
+                {
-                    .expect("any printable character")
+                    index
-                    .0
+                } else {
+                    start_index
+                }
             } else {
                 0
             };
@@ -179,8 +189,7 @@ where
                 if iter.peek().is_some()
                     || printable_input
                         .chars()
-                        .rev()
+                        .next_back()
-                        .next()
                         .is_some_and(char::is_whitespace)
                 {
                     terminal.queue(Print(" "))?;
@@ -256,24 +265,24 @@ where
         Ok(passphrase)
     }
 
-    pub fn prompt_message(&mut self, prompt: Message) -> Result<()> {
+    pub fn prompt_message(&mut self, prompt: &Message) -> Result<()> {
         let mut terminal = AlternateScreen::new(&mut self.write)?;
         let mut terminal = RawMode::new(&mut terminal)?;
 
         loop {
-            let (cols, _) = terminal::size()?;
+            let (cols, rows) = terminal::size()?;
 
             terminal
                 .queue(terminal::Clear(terminal::ClearType::All))?
                 .queue(cursor::MoveTo(0, 0))?;
 
-            use Message::*;
             match &prompt {
-                Text(text) => {
+                Message::Text(text) => {
                     for line in text.lines() {
                         let mut written_chars = 0;
                         for word in line.split_whitespace() {
-                            let len = word.len() as u16;
+                            #[allow(clippy::cast_possible_truncation)]
+                            let len = std::cmp::min(u16::MAX as usize, word.len()) as u16;
                             written_chars += len + 1;
                             if written_chars > cols {
                                 terminal
@@ -288,12 +297,25 @@ where
                             .queue(cursor::MoveToColumn(0))?;
                     }
                 }
-                Data(data) => {
+                Message::Data(data) => {
-                    for line in data.lines() {
+                    let count = data.lines().count();
+                    // NOTE: GE to allow a MoveDown(1)
+                    if count >= rows as usize {
+                        let msg = format!(
+                            "{} {count} {} {rows} {}",
+                            "Could not print data", "lines long (screen is", "lines long)"
+                        );
                         terminal
-                            .queue(Print(line))?
+                            .queue(Print(msg))?
                             .queue(cursor::MoveDown(1))?
                             .queue(cursor::MoveToColumn(0))?;
+                    } else {
+                        for line in data.lines() {
+                            terminal
+                                .queue(Print(line))?
+                                .queue(cursor::MoveDown(1))?
+                                .queue(cursor::MoveToColumn(0))?;
+                        }
                     }
                 }
             }
@@ -302,9 +324,10 @@ where
                 .queue(PrintStyledContent(" OK ".negative()))?
                 .flush()?;
 
+            #[allow(clippy::single_match)]
             match read()? {
                 Event::Key(k) => match k.code {
-                    KeyCode::Enter | KeyCode::Char(' ') | KeyCode::Char('q') => break,
+                    KeyCode::Enter | KeyCode::Char(' ' | 'q') => break,
                     _ => (),
                 },
                 _ => (),

keyfork-prompt/src/qrencode.rs

@@ -0,0 +1,31 @@
+use std::{
+    io::Write,
+    process::{Command, Stdio},
+};
+
+#[derive(thiserror::Error, Debug)]
+pub enum QrGenerationError {
+    #[error("{0}")]
+    Io(#[from] std::io::Error),
+
+    #[error("{0}")]
+    StringParse(#[from] std::string::FromUtf8Error)
+}
+
+/// Generate a terminal-printable QR code for a given string. Uses the `qrencode` CLI utility.
+pub fn qrencode(text: &str) -> Result<String, QrGenerationError> {
+    let mut qrencode = Command::new("qrencode")
+        .arg("-t")
+        .arg("ansiutf8")
+        .arg("-m")
+        .arg("2")
+        .stdin(Stdio::piped())
+        .stdout(Stdio::piped())
+        .spawn()?;
+    if let Some(stdin) = qrencode.stdin.as_mut() {
+        stdin.write_all(text.as_bytes())?;
+    }
+    let output = qrencode.wait_with_output()?;
+    let result = String::from_utf8(output.stdout)?;
+    Ok(result)
+}

keyfork-shard/src/bin/keyfork-shard-combine-openpgp.rs

@@ -42,7 +42,7 @@ fn run() -> Result<()> {
 
     combine(
         cert_list,
-        encrypted_metadata,
+        &encrypted_metadata,
         encrypted_messages.into(),
         stdout(),
     )?;

keyfork-shard/src/bin/keyfork-shard-decrypt-openpgp.rs

@@ -43,7 +43,7 @@ fn run() -> Result<()> {
 
     decrypt(
         &cert_list,
-        encrypted_metadata,
+        &encrypted_metadata,
         encrypted_messages.make_contiguous(),
     )?;
 

keyfork-shard/src/lib.rs

@@ -8,7 +8,7 @@ use aes_gcm::{
     Aes256Gcm, KeyInit,
 };
 use keyfork_mnemonic_util::{Mnemonic, Wordlist};
-use keyfork_prompt::{Message as PromptMessage, PromptManager};
+use keyfork_prompt::{qrencode, Message as PromptMessage, PromptManager};
 use sharks::{Share, Sharks};
 use x25519_dalek::{EphemeralSecret, PublicKey};
 
@@ -24,6 +24,10 @@ pub enum SharksError {
     CombineShare(String),
 }
 
+#[derive(thiserror::Error, Debug)]
+#[error("Mnemonic did not store enough data")]
+pub struct InvalidMnemonicData;
+
 /// Decrypt hunk version 1:
 /// 1 byte: Version
 /// 1 byte: Threshold
@@ -31,6 +35,10 @@ pub enum SharksError {
 pub(crate) const HUNK_VERSION: u8 = 1;
 pub(crate) const HUNK_OFFSET: usize = 2;
 
+/// # Panics
+///
+/// The function may panic if it is given payloads generated using a version of Keyfork that is
+/// incompatible with the currently running version.
 pub fn remote_decrypt() -> Result<(), Box<dyn std::error::Error>> {
     let mut pm = PromptManager::new(stdin(), stdout())?;
     let wordlist = Wordlist::default();
@@ -47,10 +55,15 @@ pub fn remote_decrypt() -> Result<(), Box<dyn std::error::Error>> {
         let our_key = EphemeralSecret::random();
         let key_mnemonic =
             Mnemonic::from_entropy(PublicKey::from(&our_key).as_bytes(), Default::default())?;
-        pm.prompt_message(PromptMessage::Text(format!(
+        let combined_mnemonic = format!("{nonce_mnemonic} {key_mnemonic}");
-            "Our words: {nonce_mnemonic} {key_mnemonic}"
+        pm.prompt_message(&PromptMessage::Text(format!(
+            "Our words: {combined_mnemonic}"
         )))?;
 
+        if let Ok(qrcode) = qrencode::qrencode(&combined_mnemonic) {
+            pm.prompt_message(&PromptMessage::Data(qrcode))?;
+        }
+
         let their_words = pm.prompt_wordlist("Their words: ", &wordlist)?;
 
         let mut pubkey_words = their_words.split_whitespace().take(24).peekable();
@@ -71,13 +84,13 @@ pub fn remote_decrypt() -> Result<(), Box<dyn std::error::Error>> {
         }
 
         let their_key = Mnemonic::from_str(&pubkey_mnemonic)?.entropy();
-        let their_key: [u8; 32] = their_key.try_into().expect("24 words");
+        let their_key: [u8; 32] = their_key.try_into().map_err(|_| InvalidMnemonicData)?;
 
         let shared_secret = our_key
             .diffie_hellman(&PublicKey::from(their_key))
             .to_bytes();
         let shared_key =
-            Aes256Gcm::new_from_slice(&shared_secret).expect("Invalid length of constant key size");
+            Aes256Gcm::new_from_slice(&shared_secret)?;
 
         let payload = Mnemonic::from_str(&payload_mnemonic)?.entropy();
         let payload =

keyfork-shard/src/openpgp.rs

@@ -7,6 +7,7 @@ use std::{
 
 use aes_gcm::{
     aead::{consts::U12, Aead},
+    aes::cipher::InvalidLength,
     Aes256Gcm, Error as AesError, KeyInit, Nonce,
 };
 use keyfork_derive_openpgp::derive_util::{
@@ -14,7 +15,7 @@ use keyfork_derive_openpgp::derive_util::{
     DerivationPath,
 };
 use keyfork_mnemonic_util::{Mnemonic, MnemonicFromStrError, MnemonicGenerationError, Wordlist};
-use keyfork_prompt::{Error as PromptError, Message as PromptMessage, PromptManager};
+use keyfork_prompt::{qrencode, Error as PromptError, Message as PromptMessage, PromptManager};
 use openpgp::{
     armor::{Kind, Writer},
     cert::{Cert, CertParser, ValidCert},
@@ -44,11 +45,11 @@ use smartcard::SmartcardManager;
 /// Shard metadata verson 1:
 /// 1 byte: Version
 /// 1 byte: Threshold
-/// OpenPGP Packet Pile of Certs
+/// Packet Pile of Certs
 const SHARD_METADATA_VERSION: u8 = 1;
 const SHARD_METADATA_OFFSET: usize = 2;
 
-use super::{HUNK_VERSION, SharksError};
+use super::{SharksError, InvalidMnemonicData, HUNK_VERSION};
 
 // 256 bit share is 49 bytes + some amount of hunk bytes, gives us reasonable padding
 const ENC_LEN: u8 = 4 * 16;
@@ -61,6 +62,9 @@ pub enum Error {
     #[error("Error decrypting share: {0}")]
     SymDecryptShare(#[from] AesError),
 
+    #[error("Invalid length of AES key: {0}")]
+    AesLength(#[from] InvalidLength),
+
     #[error("Derived secret hash {0} != expected {1}")]
     InvalidSecret(Fingerprint, Fingerprint),
 
@@ -85,6 +89,9 @@ pub enum Error {
     #[error("Mnemonic parse error: {0}")]
     MnemonicFromStr(#[from] MnemonicFromStrError),
 
+    #[error("{0}")]
+    InvalidMnemonicData(#[from] InvalidMnemonicData),
+
     #[error("IO error: {0}")]
     Io(#[source] std::io::Error),
 
@@ -173,6 +180,10 @@ pub fn discover_certs(path: impl AsRef<Path>) -> Result<Vec<Cert>> {
     }
 }
 
+/// # Panics
+///
+/// When given packets that are not a list of PKESK packets and SEIP packets, the function panics.
+/// The `split` utility should never give packets that are not in this format.
 pub fn parse_messages(reader: impl Read + Send + Sync) -> Result<VecDeque<EncryptedMessage>> {
     let mut pkesks = Vec::new();
     let mut encrypted_messages = VecDeque::new();
@@ -255,7 +266,7 @@ fn decrypt_with_manager(
     threshold: u8,
     messages: &mut HashMap<KeyID, EncryptedMessage>,
     certs: &[Cert],
-    policy: NullPolicy,
+    policy: &dyn Policy,
     manager: &mut SmartcardManager,
 ) -> Result<HashMap<KeyID, Vec<u8>>> {
     let mut decrypted_messages = HashMap::new();
@@ -267,7 +278,7 @@ fn decrypt_with_manager(
         for valid_cert in certs
             .iter()
             .filter(|cert| !decrypted_messages.contains_key(&cert.keyid()))
-            .map(|cert| cert.with_policy(&policy, None))
+            .map(|cert| cert.with_policy(policy, None))
         {
             let valid_cert = valid_cert.map_err(Error::Sequoia)?;
             let fp = valid_cert
@@ -285,7 +296,7 @@ fn decrypt_with_manager(
         if let Some(fp) = manager.load_any_fingerprint(unused_fingerprints)? {
             let cert_keyid = cert_by_fingerprint.get(&fp).unwrap().clone();
             if let Some(message) = messages.remove(&cert_keyid) {
-                let message = message.decrypt_with(&policy, &mut *manager)?;
+                let message = message.decrypt_with(policy, &mut *manager)?;
                 decrypted_messages.insert(cert_keyid, message);
             }
         }
@@ -347,14 +358,14 @@ fn decrypt_metadata(
 fn decrypt_one(
     messages: Vec<EncryptedMessage>,
     certs: &[Cert],
-    metadata: EncryptedMessage,
+    metadata: &EncryptedMessage,
 ) -> Result<(Vec<u8>, u8, Cert)> {
     let policy = NullPolicy::new();
 
     let mut keyring = Keyring::new(certs)?;
     let mut manager = SmartcardManager::new()?;
 
-    let content = decrypt_metadata(&metadata, &policy, &mut keyring, &mut manager)?;
+    let content = decrypt_metadata(metadata, &policy, &mut keyring, &mut manager)?;
 
     let (threshold, root_cert, certs) = decode_metadata_v1(&content)?;
 
@@ -362,7 +373,7 @@ fn decrypt_one(
     manager.set_root_cert(root_cert.clone());
 
     let mut messages: HashMap<KeyID, EncryptedMessage> =
-        HashMap::from_iter(certs.iter().map(|c| c.keyid()).zip(messages));
+        certs.iter().map(Cert::keyid).zip(messages).collect();
 
     let decrypted_messages = decrypt_with_keyring(&mut messages, &certs, &policy, &mut keyring)?;
 
@@ -370,7 +381,7 @@ fn decrypt_one(
         return Ok((message, threshold, root_cert));
     }
 
-    let decrypted_messages = decrypt_with_manager(1, &mut messages, &certs, policy, &mut manager)?;
+    let decrypted_messages = decrypt_with_manager(1, &mut messages, &certs, &policy, &mut manager)?;
 
     if let Some(message) = decrypted_messages.into_values().next() {
         return Ok((message, threshold, root_cert));
@@ -379,9 +390,14 @@ fn decrypt_one(
     unreachable!("smartcard manager should always decrypt")
 }
 
+/// # Panics
+///
+/// The function may panic if a share is decrypted but has a length larger than 256 bits. This is
+/// atypical usage and should not be encountered in normal usage, unless something that is not a
+/// Keyfork seed has been fed into [`split`].
 pub fn decrypt(
     certs: &[Cert],
-    metadata: EncryptedMessage,
+    metadata: &EncryptedMessage,
     encrypted_messages: &[EncryptedMessage],
 ) -> Result<()> {
     let mut pm = PromptManager::new(stdin(), stdout())?;
@@ -404,7 +420,9 @@ pub fn decrypt(
         }
     }
     let their_key = Mnemonic::from_str(&pubkey_mnemonic)?.entropy();
-    let their_key: [u8; 32] = their_key.try_into().expect("24 words");
+    let their_key: [u8; 32] = their_key
+        .try_into()
+        .map_err(|_| InvalidMnemonicData)?;
     let their_nonce = Mnemonic::from_str(&nonce_mnemonic)?.entropy();
     let their_nonce = Nonce::<U12>::from_slice(&their_nonce);
 
@@ -416,7 +434,7 @@ pub fn decrypt(
         .diffie_hellman(&PublicKey::from(their_key))
         .to_bytes();
 
-    let (mut share, threshold, ..) = decrypt_one(encrypted_messages.to_vec(), &certs, metadata)?;
+    let (mut share, threshold, ..) = decrypt_one(encrypted_messages.to_vec(), certs, metadata)?;
     share.insert(0, HUNK_VERSION);
     share.insert(1, threshold);
     assert!(
@@ -424,10 +442,8 @@ pub fn decrypt(
         "invalid share length (too long, max {ENC_LEN} bytes)"
     );
 
-    let shared_key =
+    let shared_key = Aes256Gcm::new_from_slice(&shared_secret)?;
-        Aes256Gcm::new_from_slice(&shared_secret).expect("Invalid length of constant key size");
     let bytes = shared_key.encrypt(their_nonce, share.as_slice())?;
-    dbg!(bytes.len());
     shared_key.decrypt(their_nonce, &bytes[..])?;
 
     // NOTE: Padding length is less than u8::MAX because ENC_LEN < u8::MAX
@@ -435,15 +451,20 @@ pub fn decrypt(
     // difficulty when entering in prompts manually, as one's place could be lost due to repeated
     // keywords. This is done below by having sequentially increasing numbers up to but not
     // including the last byte.
-    assert!(ENC_LEN < u8::MAX, "padding byte can be u8");
+    #[allow(clippy::assertions_on_constants)]
+    {
+        assert!(ENC_LEN < u8::MAX, "padding byte can be u8");
+    }
+    #[allow(clippy::cast_possible_truncation)]
     let mut out_bytes = [bytes.len() as u8; ENC_LEN as usize];
     assert!(
         bytes.len() < out_bytes.len(),
         "encrypted payload larger than acceptable limit"
     );
     out_bytes[..bytes.len()].clone_from_slice(&bytes);
-    for (i, byte) in (&mut out_bytes[bytes.len()..(ENC_LEN as usize - 1)])
+    #[allow(clippy::cast_possible_truncation)]
-        .into_iter()
+    for (i, byte) in (out_bytes[bytes.len()..(ENC_LEN as usize - 1)])
+        .iter_mut()
         .enumerate()
     {
         *byte = (i % u8::MAX as usize) as u8;
@@ -451,17 +472,22 @@ pub fn decrypt(
 
     // safety: size of out_bytes is constant and always % 4 == 0
     let mnemonic = unsafe { Mnemonic::from_raw_entropy(&out_bytes, Default::default()) };
+    let combined_mnemonic = format!("{our_mnemonic} {mnemonic}");
 
-    pm.prompt_message(PromptMessage::Text(format!(
+    pm.prompt_message(&PromptMessage::Text(format!(
-        "Our words: {our_mnemonic} {mnemonic}"
+        "Our words: {combined_mnemonic}"
     )))?;
 
+    if let Ok(qrcode) = qrencode::qrencode(&combined_mnemonic) {
+        pm.prompt_message(&PromptMessage::Data(qrcode))?;
+    }
+
     Ok(())
 }
 
 pub fn combine(
     certs: Vec<Cert>,
-    metadata: EncryptedMessage,
+    metadata: &EncryptedMessage,
     messages: Vec<EncryptedMessage>,
     mut output: impl Write,
 ) -> Result<()> {
@@ -471,7 +497,7 @@ pub fn combine(
 
     let mut keyring = Keyring::new(certs)?;
     let mut manager = SmartcardManager::new()?;
-    let content = decrypt_metadata(&metadata, &policy, &mut keyring, &mut manager)?;
+    let content = decrypt_metadata(metadata, &policy, &mut keyring, &mut manager)?;
 
     let (threshold, root_cert, certs) = decode_metadata_v1(&content)?;
 
@@ -482,7 +508,7 @@ pub fn combine(
     // because we control the order packets are encrypted and certificates are stored.
 
     let mut messages: HashMap<KeyID, EncryptedMessage> =
-        HashMap::from_iter(certs.iter().map(|c| c.keyid()).zip(messages));
+        certs.iter().map(Cert::keyid).zip(messages).collect();
 
     let mut decrypted_messages =
         decrypt_with_keyring(&mut messages, &certs, &policy, &mut keyring)?;
@@ -492,14 +518,15 @@ pub fn combine(
 
     let left_from_threshold = threshold as usize - decrypted_messages.len();
     if left_from_threshold > 0 {
+        #[allow(clippy::cast_possible_truncation)]
         let new_messages = decrypt_with_manager(
             left_from_threshold as u8,
             &mut messages,
             &certs,
-            policy,
+            &policy,
             &mut manager,
         )?;
-        decrypted_messages.extend(new_messages.into_iter());
+        decrypted_messages.extend(new_messages);
     }
 
     let shares = decrypted_messages
@@ -517,11 +544,11 @@ pub fn combine(
         DerivationAlgorithm::Ed25519,
         &DerivationPath::from_str("m/7366512'/0'")?,
     )
-    .derive_with_master_seed(secret.to_vec())?;
+    .derive_with_master_seed(secret.clone())?;
     let derived_cert = keyfork_derive_openpgp::derive(
         kdr,
         &[KeyFlags::empty().set_certification().set_signing()],
-        userid,
+        &userid,
     )?;
 
     // NOTE: Signatures on certs will be different. Compare fingerprints instead.
@@ -538,6 +565,10 @@ pub fn combine(
     Ok(())
 }
 
+/// # Panics
+///
+/// The function may panic if the metadata can't properly store the certificates used to generate
+/// the encrypted shares.
 pub fn split(threshold: u8, certs: Vec<Cert>, secret: &[u8], output: impl Write) -> Result<()> {
     // build cert to sign encrypted shares
     let userid = UserID::from("keyfork-sss");
@@ -549,7 +580,7 @@ pub fn split(threshold: u8, certs: Vec<Cert>, secret: &[u8], output: impl Write)
     let derived_cert = keyfork_derive_openpgp::derive(
         kdr,
         &[KeyFlags::empty().set_certification().set_signing()],
-        userid,
+        &userid,
     )?;
     let signing_key = derived_cert
         .primary_key()
@@ -562,14 +593,14 @@ pub fn split(threshold: u8, certs: Vec<Cert>, secret: &[u8], output: impl Write)
 
     let sharks = Sharks(threshold);
     let dealer = sharks.dealer(secret);
-    let shares = dealer.map(|s| Vec::from(&s)).collect::<Vec<_>>();
+    let generated_shares = dealer.map(|s| Vec::from(&s)).collect::<Vec<_>>();
     let policy = StandardPolicy::new();
     let mut writer = Writer::new(output, Kind::Message).map_err(Error::SequoiaIo)?;
 
     let mut total_recipients = vec![];
     let mut messages = vec![];
 
-    for (share, cert) in shares.iter().zip(certs) {
+    for (share, cert) in generated_shares.iter().zip(certs) {
         total_recipients.push(cert.clone());
         let valid_cert = cert.with_policy(&policy, None).map_err(Error::Sequoia)?;
         let encryption_keys = get_encryption_keys(&valid_cert).collect::<Vec<_>>();
@@ -609,12 +640,12 @@ pub fn split(threshold: u8, certs: Vec<Cert>, secret: &[u8], output: impl Write)
         .skip(1)
         .zip(total_recipients.iter())
     {
-        if packet_cert.map_err(Error::Sequoia)? != *cert {
+        assert_eq!(
-            panic!(
+            &packet_cert.map_err(Error::Sequoia)?,
-                "packet pile could not recreate cert: {}",
+            cert,
-                cert.fingerprint()
+            "packet pile could not recreate cert: {}",
-            );
+            cert.fingerprint()
-        }
+        );
     }
 
     let valid_certs = total_recipients

keyfork-shard/src/openpgp/keyring.rs

@@ -70,7 +70,7 @@ impl VerificationHelper for &mut Keyring {
             .collect())
     }
     fn check(&mut self, structure: MessageStructure) -> openpgp::Result<()> {
-        for layer in structure.into_iter() {
+        for layer in structure {
             #[allow(unused_variables)]
             match layer {
                 MessageLayer::Compression { algo } => {}
@@ -149,8 +149,7 @@ impl DecryptionHelper for &mut Keyring {
                     };
                     if pkesk
                         .decrypt(&mut keypair, sym_algo)
-                        .map(|(algo, sk)| decrypt(algo, &sk))
+                        .is_some_and(|(algo, sk)| decrypt(algo, &sk))
-                        .unwrap_or(false)
                     {
                         return Ok(Some(key.fingerprint()));
                     }

keyfork-shard/src/openpgp/smartcard.rs

@@ -61,6 +61,7 @@ fn format_name(input: impl AsRef<str>) -> String {
     n.join(" ")
 }
 
+#[allow(clippy::module_name_repetitions)]
 pub struct SmartcardManager {
     current_card: Option<Card<Open>>,
     root: Option<Cert>,
@@ -146,7 +147,7 @@ impl SmartcardManager {
             }
 
             #[rustfmt::skip]
-            self.pm.prompt_message(Message::Text("Please plug in a smart card and press enter".to_string()))?;
+            self.pm.prompt_message(&Message::Text("Please plug in a smart card and press enter".to_string()))?;
         }
 
         Ok(None)
@@ -155,6 +156,7 @@ impl SmartcardManager {
 
 impl VerificationHelper for &mut SmartcardManager {
     fn get_certs(&mut self, ids: &[openpgp::KeyHandle]) -> openpgp::Result<Vec<Cert>> {
+        #[allow(clippy::flat_map_option)]
         Ok(ids
             .iter()
             .flat_map(|kh| self.root.as_ref().filter(|cert| cert.key_handle() == *kh))
@@ -163,7 +165,7 @@ impl VerificationHelper for &mut SmartcardManager {
     }
 
     fn check(&mut self, structure: MessageStructure) -> openpgp::Result<()> {
-        for layer in structure.into_iter() {
+        for layer in structure {
             #[allow(unused_variables)]
             match layer {
                 MessageLayer::Compression { algo } => {}
@@ -241,12 +243,13 @@ impl DecryptionHelper for &mut SmartcardManager {
             let temp_pin = self.pm.prompt_passphrase(&message)?;
             let verification_status = transaction.verify_user_pin(temp_pin.as_str().trim());
             match verification_status {
+                #[allow(clippy::ignored_unit_patterns)]
                 Ok(_) => {
                     self.pin_cache.insert(fp.clone(), temp_pin.clone());
                     pin.replace(temp_pin);
                 }
                 Err(CardError::CardStatus(StatusBytes::IncorrectParametersCommandDataField)) => {
-                    self.pm.prompt_message(Message::Text("Invalid PIN length entered.".to_string()))?;
+                    self.pm.prompt_message(&Message::Text("Invalid PIN length entered.".to_string()))?;
                 }
                 Err(_) => {}
             }
@@ -261,8 +264,7 @@ impl DecryptionHelper for &mut SmartcardManager {
         for pkesk in pkesks {
             if pkesk
                 .decrypt(&mut decryptor, sym_algo)
-                .map(|(algo, sk)| decrypt(algo, &sk))
+                .is_some_and(|(algo, sk)| decrypt(algo, &sk))
-                .unwrap_or(false)
             {
                 return Ok(Some(fp));
             }

keyfork/src/cli/shard.rs

@@ -88,7 +88,7 @@ impl ShardExec for OpenPGP {
 
         keyfork_shard::openpgp::combine(
             certs,
-            encrypted_metadata,
+            &encrypted_metadata,
             encrypted_messages.into(),
             output,
         )?;

keyforkd/src/lib.rs

@@ -35,6 +35,7 @@ pub async fn start_and_run_server_on(
         }
     };
     match server.run(service).await {
+        #[allow(clippy::ignored_unit_patterns)]
         Ok(_) => (),
         Err(e) => {
             #[cfg(feature = "tracing")]

keyforkd/src/service.rs

@@ -1,3 +1,5 @@
+#![allow(clippy::implicit_clone)]
+
 use std::{future::Future, pin::Pin, sync::Arc, task::Poll};
 
 use keyfork_derive_path_data::guess_target;
@@ -156,7 +158,7 @@ mod tests {
                 .unwrap()
                 .try_into()
                 .unwrap();
-            assert_eq!(response.data, private_key)
+            assert_eq!(response.data, private_key);
         }
     }
 
@@ -182,7 +184,7 @@ mod tests {
                 .unwrap()
                 .try_into()
                 .unwrap();
-            assert_eq!(response.data, private_key)
+            assert_eq!(response.data, private_key);
         }
     }
 }