From 9394500f2f77eda33568d05e79e0ec10495146db Mon Sep 17 00:00:00 2001 From: ryan Date: Tue, 9 Apr 2024 19:46:37 -0400 Subject: [PATCH 1/4] keyfork-shard: generate nonce using hkdf --- Cargo.lock | 2 +- crates/keyfork-shard/Cargo.toml | 2 +- crates/keyfork-shard/src/lib.rs | 70 ++++++++++++++++----------------- crates/keyfork/Cargo.toml | 2 +- 4 files changed, 37 insertions(+), 39 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2e707bd..03f9ed1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1830,7 +1830,7 @@ dependencies = [ [[package]] name = "keyfork-shard" -version = "0.1.0" +version = "0.2.0" dependencies = [ "aes-gcm", "anyhow", diff --git a/crates/keyfork-shard/Cargo.toml b/crates/keyfork-shard/Cargo.toml index 6cdd0c0..8066277 100644 --- a/crates/keyfork-shard/Cargo.toml +++ b/crates/keyfork-shard/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "keyfork-shard" -version = "0.1.0" +version = "0.2.0" edition = "2021" license = "AGPL-3.0-only" diff --git a/crates/keyfork-shard/src/lib.rs b/crates/keyfork-shard/src/lib.rs index 7aeb2a3..338aaea 100644 --- a/crates/keyfork-shard/src/lib.rs +++ b/crates/keyfork-shard/src/lib.rs @@ -7,14 +7,17 @@ use std::{ }; use aes_gcm::{ - aead::{consts::U12, Aead, AeadCore, OsRng}, + aead::{consts::U12, Aead}, Aes256Gcm, KeyInit, Nonce, }; use hkdf::Hkdf; use keyfork_bug::{bug, POISONED_MUTEX}; use keyfork_mnemonic_util::{English, Mnemonic}; use keyfork_prompt::{ - validators::{mnemonic::MnemonicSetValidator, Validator}, + validators::{ + mnemonic::{MnemonicSetValidator, MnemonicValidator, WordLength}, + Validator, + }, Message as PromptMessage, PromptHandler, Terminal, }; use sha2::Sha256; @@ -194,7 +197,6 @@ pub trait Format { let encrypted_messages = self.parse_shard_file(reader)?; // establish AES-256-GCM key via ECDH - let mut nonce_data: Option<[u8; 12]> = None; let mut pubkey_data: Option<[u8; 32]> = None; // receive remote data via scanning QR code from camera @@ -207,9 +209,8 @@ pub trait Format { if let Ok(Some(hex)) = keyfork_qrcode::scan_camera(std::time::Duration::from_secs(30), 0) { - let decoded_data = smex::decode(&hex)?; - nonce_data = Some(decoded_data[..12].try_into().map_err(|_| InvalidData)?); - pubkey_data = Some(decoded_data[12..].try_into().map_err(|_| InvalidData)?) + let decoded_data = smex::decode(hex)?; + pubkey_data = Some(decoded_data.try_into().map_err(|_| InvalidData)?) } else { prompt .lock() @@ -219,30 +220,23 @@ pub trait Format { } // if QR code scanning failed or was unavailable, read from a set of mnemonics - let (nonce, their_pubkey) = match (nonce_data, pubkey_data) { - (Some(nonce), Some(pubkey)) => (nonce, pubkey), - _ => { - let validator = MnemonicSetValidator { - word_lengths: [9, 24], + let their_pubkey = match pubkey_data { + Some(pubkey) => pubkey, + None => { + let validator = MnemonicValidator { + word_length: Some(WordLength::Count(24)), }; - let [nonce_mnemonic, pubkey_mnemonic] = prompt + prompt .lock() .expect(bug!(POISONED_MUTEX)) .prompt_validated_wordlist::( QRCODE_COULDNT_READ, 3, validator.to_fn(), - )?; - - let nonce = nonce_mnemonic + )? .as_bytes() .try_into() - .map_err(|_| InvalidData)?; - let pubkey = pubkey_mnemonic - .as_bytes() - .try_into() - .map_err(|_| InvalidData)?; - (nonce, pubkey) + .map_err(|_| InvalidData)? } }; @@ -253,9 +247,14 @@ pub trait Format { .diffie_hellman(&PublicKey::from(their_pubkey)) .to_bytes(); let hkdf = Hkdf::::new(None, &shared_secret); - let mut hkdf_output = [0u8; 256 / 8]; - hkdf.expand(&[], &mut hkdf_output)?; - let shared_key = Aes256Gcm::new_from_slice(&hkdf_output)?; + + let mut shared_key_data = [0u8; 256 / 8]; + hkdf.expand(b"key", &mut shared_key_data)?; + let shared_key = Aes256Gcm::new_from_slice(&shared_key_data)?; + + let mut nonce_data = [0u8; 12]; + hkdf.expand(b"nonce", &mut nonce_data)?; + let nonce = Nonce::::from_slice(&nonce_data); // decrypt a single shard and create the payload let (share, threshold) = @@ -269,7 +268,6 @@ pub trait Format { ); // encrypt data - let nonce = Nonce::::from_slice(&nonce); let payload_bytes = shared_key.encrypt(nonce, payload.as_slice())?; // convert data to a static-size payload @@ -432,16 +430,13 @@ pub fn remote_decrypt(w: &mut impl Write) -> Result<(), Box 0) { iter += 1; - let nonce = Aes256Gcm::generate_nonce(&mut OsRng); - let nonce_mnemonic = unsafe { Mnemonic::from_raw_bytes(nonce.as_slice()) }; let our_key = EphemeralSecret::random(); let key_mnemonic = Mnemonic::from_bytes(PublicKey::from(&our_key).as_bytes())?; #[cfg(feature = "qrcode")] { use keyfork_qrcode::{qrencode, ErrorCorrection}; - let mut qrcode_data = nonce_mnemonic.to_bytes(); - qrcode_data.extend(key_mnemonic.as_bytes()); + let qrcode_data = key_mnemonic.to_bytes(); if let Ok(qrcode) = qrencode(&smex::encode(&qrcode_data), ErrorCorrection::Highest) { pm.prompt_message(PromptMessage::Text(format!( concat!( @@ -458,10 +453,9 @@ pub fn remote_decrypt(w: &mut impl Write) -> Result<(), Box Result<(), Box::new(None, &shared_secret); - let mut hkdf_output = [0u8; 256 / 8]; - hkdf.expand(&[], &mut hkdf_output)?; - let shared_key = Aes256Gcm::new_from_slice(&hkdf_output)?; - let payload = - shared_key.decrypt(&nonce, &payload[..payload[payload.len() - 1] as usize])?; + let mut shared_key_data = [0u8; 256 / 8]; + hkdf.expand(b"key", &mut shared_key_data)?; + let shared_key = Aes256Gcm::new_from_slice(&shared_key_data)?; + + let mut nonce_data = [0u8; 12]; + hkdf.expand(b"nonce", &mut nonce_data)?; + let nonce = Nonce::::from_slice(&nonce_data); + + let payload = shared_key.decrypt(nonce, &payload[..payload[payload.len() - 1] as usize])?; assert_eq!(HUNK_VERSION, payload[0], "Incompatible hunk version"); match &mut iter_count { diff --git a/crates/keyfork/Cargo.toml b/crates/keyfork/Cargo.toml index bc3bda2..d54db2e 100644 --- a/crates/keyfork/Cargo.toml +++ b/crates/keyfork/Cargo.toml @@ -32,7 +32,7 @@ keyfork-entropy = { version = "0.1.0", path = "../util/keyfork-entropy", registr keyfork-mnemonic-util = { version = "0.2.0", path = "../util/keyfork-mnemonic-util", registry = "distrust" } keyfork-prompt = { version = "0.1.0", path = "../util/keyfork-prompt", registry = "distrust" } keyfork-qrcode = { version = "0.1.0", path = "../qrcode/keyfork-qrcode", default-features = false, registry = "distrust" } -keyfork-shard = { version = "0.1.0", path = "../keyfork-shard", default-features = false, features = ["openpgp", "openpgp-card", "qrcode"], registry = "distrust" } +keyfork-shard = { version = "0.2.0", path = "../keyfork-shard", default-features = false, features = ["openpgp", "openpgp-card", "qrcode"], registry = "distrust" } smex = { version = "0.1.0", path = "../util/smex", registry = "distrust" } clap = { version = "4.4.2", features = ["derive", "env", "wrap_help"] } -- 2.40.1 From 68f07f6f0279676a2725e07a24e1946de5013b33 Mon Sep 17 00:00:00 2001 From: ryan Date: Tue, 9 Apr 2024 19:47:47 -0400 Subject: [PATCH 2/4] bump mio and iana-time-zone --- Cargo.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 03f9ed1..67dda37 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1497,9 +1497,9 @@ dependencies = [ [[package]] name = "iana-time-zone" -version = "0.1.59" +version = "0.1.60" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b6a67363e2aa4443928ce15e57ebae94fd8949958fd1223c4cfc0cd473ad7539" +checksum = "e7ffbb5a1b541ea2561f8c41c087286cc091e21e556a4f09a8f6cbf17b69b141" dependencies = [ "android_system_properties", "core-foundation-sys", @@ -2103,9 +2103,9 @@ dependencies = [ [[package]] name = "mio" -version = "0.8.10" +version = "0.8.11" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f3d0b296e374a4e6f3c7b0a1f5a51d748a0d34c85e7dc48fc3fa9a87657fe09" +checksum = "a4a650543ca06a924e8b371db273b2756685faae30f8487da1b56505a8f78b0c" dependencies = [ "libc", "log", -- 2.40.1 From 6fa434e89c4231d3ce209fefbdb8aeae0246b512 Mon Sep 17 00:00:00 2001 From: ryan Date: Sun, 14 Apr 2024 20:27:00 -0400 Subject: [PATCH 3/4] keyfork-shard: shorten length and pad inside encrypted block --- crates/keyfork-shard/src/lib.rs | 68 +++++++++++++++++---------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/crates/keyfork-shard/src/lib.rs b/crates/keyfork-shard/src/lib.rs index 338aaea..e81c57a 100644 --- a/crates/keyfork-shard/src/lib.rs +++ b/crates/keyfork-shard/src/lib.rs @@ -24,8 +24,10 @@ use sha2::Sha256; use sharks::{Share, Sharks}; use x25519_dalek::{EphemeralSecret, PublicKey}; -// 256 bit share encrypted is 49 bytes, couple more bytes before we reach max size -const ENC_LEN: u8 = 4 * 16; +// 32-byte share, 1-byte index, 1-byte threshold, 1-byte version == 36 bytes +// Encrypted, is 52 bytes +const PLAINTEXT_LENGTH: u8 = 36; +const ENCRYPTED_LENGTH: u8 = PLAINTEXT_LENGTH + 16; #[cfg(feature = "openpgp")] pub mod openpgp; @@ -263,41 +265,34 @@ pub trait Format { payload.insert(0, HUNK_VERSION); payload.insert(1, threshold); assert!( - payload.len() <= ENC_LEN as usize, - "invalid share length (too long, max {ENC_LEN} bytes)" + payload.len() < PLAINTEXT_LENGTH as usize, + "invalid share length (too long, max {PLAINTEXT_LENGTH} bytes)" ); - // encrypt data - let payload_bytes = shared_key.encrypt(nonce, payload.as_slice())?; - - // convert data to a static-size payload - // NOTE: Padding length is less than u8::MAX because ENC_LEN < u8::MAX + // convert plaintext to static-size payload #[allow(clippy::assertions_on_constants)] { - assert!(ENC_LEN < u8::MAX, "padding byte can be u8"); + assert!(PLAINTEXT_LENGTH < u8::MAX, "length byte can be u8"); } - #[allow(clippy::cast_possible_truncation)] - let mut out_bytes = [payload_bytes.len() as u8; ENC_LEN as usize]; - assert!( - payload_bytes.len() < out_bytes.len(), - "encrypted payload larger than acceptable limit" - ); - out_bytes[..payload_bytes.len()].clone_from_slice(&payload_bytes); - // NOTE: This previously used a single repeated value as the padding byte, but resulted in - // difficulty when entering in prompts manually, as one's place could be lost due to - // repeated keywords. This is resolved below by having sequentially increasing numbers up to - // but not including the last byte. + // NOTE: Previous versions of Keyfork Shard would modify the padding bytes to avoid + // duplicate mnemonic words. This version does not include that, and instead uses a + // repeated length byte. #[allow(clippy::cast_possible_truncation)] - for (i, byte) in (out_bytes[payload_bytes.len()..(ENC_LEN as usize - 1)]) - .iter_mut() - .enumerate() - { - *byte = (i % u8::MAX as usize) as u8; - } + let mut plaintext_bytes = [u8::try_from(payload.len()).expect(bug!( + "previously asserted length must be < {PLAINTEXT_LENGTH}", + PLAINTEXT_LENGTH = PLAINTEXT_LENGTH + )); PLAINTEXT_LENGTH as usize]; + plaintext_bytes[..payload.len()].clone_from_slice(&payload); + + // encrypt data + let encrypted_bytes = shared_key.encrypt(nonce, plaintext_bytes.as_slice())?; + + assert_eq!(encrypted_bytes.len(), ENCRYPTED_LENGTH as usize); // safety: size of out_bytes is constant and always % 4 == 0 - let payload_mnemonic = unsafe { Mnemonic::from_raw_bytes(&out_bytes) }; + let payload_mnemonic = unsafe { Mnemonic::from_raw_bytes(&encrypted_bytes) }; + dbg!(payload_mnemonic.words().len()); #[cfg(feature = "qrcode")] { @@ -399,7 +394,7 @@ pub struct InvalidData; /// 1 byte: Version /// 1 byte: Threshold /// Data: &[u8] -pub(crate) const HUNK_VERSION: u8 = 1; +pub(crate) const HUNK_VERSION: u8 = 2; pub(crate) const HUNK_OFFSET: usize = 2; const QRCODE_PROMPT: &str = "Press enter, then present QR code to camera."; @@ -468,7 +463,7 @@ pub fn remote_decrypt(w: &mut impl Write) -> Result<(), Box Result<(), Box (pubkey, payload), _ => { let validator = MnemonicSetValidator { - word_lengths: [24, 48], + word_lengths: [24, 39], }; let [pubkey_mnemonic, payload_mnemonic] = pm @@ -498,6 +493,12 @@ pub fn remote_decrypt(w: &mut impl Write) -> Result<(), Box::new(None, &shared_secret); @@ -509,7 +510,7 @@ pub fn remote_decrypt(w: &mut impl Write) -> Result<(), Box::from_slice(&nonce_data); - let payload = shared_key.decrypt(nonce, &payload[..payload[payload.len() - 1] as usize])?; + let payload = shared_key.decrypt(nonce, payload.as_slice())?; assert_eq!(HUNK_VERSION, payload[0], "Incompatible hunk version"); match &mut iter_count { @@ -524,7 +525,8 @@ pub fn remote_decrypt(w: &mut impl Write) -> Result<(), Box Date: Sun, 14 Apr 2024 21:19:06 -0400 Subject: [PATCH 4/4] keyfork-shard: base64 encode content instead of base16 --- Cargo.lock | 9 ++++++++- crates/keyfork-shard/Cargo.toml | 1 + crates/keyfork-shard/src/lib.rs | 13 +++++++------ 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 67dda37..b1f724a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -341,6 +341,12 @@ version = "0.21.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" +[[package]] +name = "base64" +version = "0.22.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9475866fec1451be56a3c2400fd081ff546538961565ccb5b7142cbd22bc7a51" + [[package]] name = "base64ct" version = "1.6.0" @@ -1834,6 +1840,7 @@ version = "0.2.0" dependencies = [ "aes-gcm", "anyhow", + "base64 0.22.0", "card-backend", "card-backend-pcsc", "hkdf", @@ -2886,7 +2893,7 @@ dependencies = [ "aes", "aes-gcm", "anyhow", - "base64", + "base64 0.21.7", "block-padding", "blowfish", "buffered-reader", diff --git a/crates/keyfork-shard/Cargo.toml b/crates/keyfork-shard/Cargo.toml index 8066277..4d0737f 100644 --- a/crates/keyfork-shard/Cargo.toml +++ b/crates/keyfork-shard/Cargo.toml @@ -37,3 +37,4 @@ card-backend-pcsc = { version = "0.5.0", optional = true } openpgp-card-sequoia = { version = "0.2.0", optional = true, default-features = false } openpgp-card = { version = "0.4.0", optional = true } sequoia-openpgp = { version = "1.17.0", optional = true, default-features = false } +base64 = "0.22.0" diff --git a/crates/keyfork-shard/src/lib.rs b/crates/keyfork-shard/src/lib.rs index e81c57a..cb5c669 100644 --- a/crates/keyfork-shard/src/lib.rs +++ b/crates/keyfork-shard/src/lib.rs @@ -23,6 +23,7 @@ use keyfork_prompt::{ use sha2::Sha256; use sharks::{Share, Sharks}; use x25519_dalek::{EphemeralSecret, PublicKey}; +use base64::prelude::{BASE64_STANDARD, Engine}; // 32-byte share, 1-byte index, 1-byte threshold, 1-byte version == 36 bytes // Encrypted, is 52 bytes @@ -208,10 +209,10 @@ pub trait Format { .lock() .expect(bug!(POISONED_MUTEX)) .prompt_message(PromptMessage::Text(QRCODE_PROMPT.to_string()))?; - if let Ok(Some(hex)) = + if let Ok(Some(qrcode_content)) = keyfork_qrcode::scan_camera(std::time::Duration::from_secs(30), 0) { - let decoded_data = smex::decode(hex)?; + let decoded_data = BASE64_STANDARD.decode(qrcode_content).unwrap(); pubkey_data = Some(decoded_data.try_into().map_err(|_| InvalidData)?) } else { prompt @@ -299,7 +300,7 @@ pub trait Format { use keyfork_qrcode::{qrencode, ErrorCorrection}; let mut qrcode_data = our_pubkey_mnemonic.to_bytes(); qrcode_data.extend(payload_mnemonic.as_bytes()); - if let Ok(qrcode) = qrencode(&smex::encode(&qrcode_data), ErrorCorrection::Highest) { + if let Ok(qrcode) = qrencode(&BASE64_STANDARD.encode(qrcode_data), ErrorCorrection::Highest) { prompt .lock() .expect(bug!(POISONED_MUTEX)) @@ -432,7 +433,7 @@ pub fn remote_decrypt(w: &mut impl Write) -> Result<(), Box Result<(), Box