From 8108f5e61a457da3b0950bafb549f98be74e0537 Mon Sep 17 00:00:00 2001 From: ryan Date: Sun, 11 Feb 2024 20:14:35 -0500 Subject: [PATCH 1/4] keyfork-derive-util, keyforkd-client: support fearless conversions --- crates/daemon/keyforkd-client/src/lib.rs | 45 +++++++- .../derive/keyfork-derive-openpgp/src/lib.rs | 39 +++---- .../derive/keyfork-derive-openpgp/src/main.rs | 21 ++-- .../src/extended_key/private_key.rs | 51 ++++++--- .../derive/keyfork-derive-util/src/request.rs | 100 ++++++++++++------ crates/keyfork-shard/src/openpgp.rs | 28 +++-- crates/keyfork/src/cli/derive.rs | 25 ++--- crates/keyfork/src/cli/wizard.rs | 13 ++- 8 files changed, 201 insertions(+), 121 deletions(-) diff --git a/crates/daemon/keyforkd-client/src/lib.rs b/crates/daemon/keyforkd-client/src/lib.rs index 2362895..8ba4e79 100644 --- a/crates/daemon/keyforkd-client/src/lib.rs +++ b/crates/daemon/keyforkd-client/src/lib.rs @@ -2,8 +2,13 @@ use std::{collections::HashMap, os::unix::net::UnixStream, path::PathBuf}; +use keyfork_derive_util::{ + request::{AsAlgorithm, DerivationRequest}, + DerivationPath, ExtendedPrivateKey, PrivateKey, +}; + use keyfork_frame::{try_decode_from, try_encode_to, DecodeError, EncodeError}; -use keyforkd_models::{Request, Response, Error as KeyforkdError}; +use keyforkd_models::{Error as KeyforkdError, Request, Response}; #[cfg(test)] mod tests; @@ -11,6 +16,10 @@ mod tests; /// An error occurred while interacting with Keyforkd. #[derive(Debug, thiserror::Error)] pub enum Error { + /// The response from the server did not match the request. + #[error("The response from the server did not match the request")] + InvalidResponse, + /// The environment variables used for determining a Keyforkd socket path were not set. #[error("Neither KEYFORK_SOCKET_PATH nor XDG_RUNTIME_DIR were set")] EnvVarsNotFound, @@ -37,7 +46,7 @@ pub enum Error { /// An error encountered in Keyforkd. #[error("Error in Keyforkd: {0}")] - Keyforkd(#[from] KeyforkdError) + Keyforkd(#[from] KeyforkdError), } #[allow(missing_docs)] @@ -95,6 +104,38 @@ impl Client { get_socket().map(|socket| Self { socket }) } + /// Request an [`ExtendedPrivateKey`] for a given [`DerivationPath`]. + /// + /// # Errors + /// An error may be returned if: + /// * Reading or writing from or to the socket encountered an error. + /// * Bincode could not serialize the request or deserialize the response. + /// * An error occurred in Keyforkd. + /// * Keyforkd returned invalid data. + pub fn request_xprv(&mut self, path: &DerivationPath) -> Result> + where + K: PrivateKey + Clone + AsAlgorithm, + { + let algo = K::as_algorithm(); + let request = Request::Derivation(DerivationRequest::new(algo.clone(), path)); + let response = self.request(&request)?; + match response { + Response::Derivation(d) => { + if d.algorithm != algo { + return Err(Error::InvalidResponse); + } + + let depth = path.len() as u8; + Ok(ExtendedPrivateKey::new_from_parts( + &d.data, + depth, + d.chain_code, + )) + } + _ => Err(Error::InvalidResponse), + } + } + /// Serialize and send a [`Request`] to the server, awaiting a [`Result`]. /// /// # Errors diff --git a/crates/derive/keyfork-derive-openpgp/src/lib.rs b/crates/derive/keyfork-derive-openpgp/src/lib.rs index 2055734..1292ca7 100644 --- a/crates/derive/keyfork-derive-openpgp/src/lib.rs +++ b/crates/derive/keyfork-derive-openpgp/src/lib.rs @@ -2,13 +2,10 @@ use std::time::{Duration, SystemTime, SystemTimeError}; -use derive_util::{ - request::{DerivationResponse, TryFromDerivationResponseError}, - DerivationIndex, ExtendedPrivateKey, PrivateKey, - IndexError, -}; +use derive_util::{DerivationIndex, ExtendedPrivateKey, IndexError, PrivateKey}; use ed25519_dalek::SigningKey; pub use keyfork_derive_util as derive_util; +pub use sequoia_openpgp as openpgp; use sequoia_openpgp::{ packet::{ key::{Key4, PrimaryRole, SubordinateRole}, @@ -18,7 +15,9 @@ use sequoia_openpgp::{ types::{KeyFlags, SignatureType}, Cert, Packet, }; -pub use sequoia_openpgp as openpgp; + +pub type XPrvKey = SigningKey; +pub type XPrv = ExtendedPrivateKey; /// An error occurred while creating an OpenPGP key. #[derive(Debug, thiserror::Error)] @@ -32,10 +31,6 @@ pub enum Error { #[error("Key configured with both encryption and non-encryption key flags: {0:?}")] InvalidKeyFlags(KeyFlags), - /// The derivation response contained incorrect data. - #[error("Incorrect derived data: {0}")] - IncorrectDerivedData(#[from] TryFromDerivationResponseError), - /// A derivation index could not be created from the given index. #[error("Could not create derivation index: {0}")] Index(#[from] IndexError), @@ -66,7 +61,7 @@ pub type Result = std::result::Result; /// /// # Errors /// The function may error for any condition mentioned in [`Error`]. -pub fn derive(data: DerivationResponse, keys: &[KeyFlags], userid: &UserID) -> Result { +pub fn derive(xprv: XPrv, keys: &[KeyFlags], userid: &UserID) -> Result { let primary_key_flags = match keys.get(0) { Some(kf) if kf.for_certification() => kf, _ => return Err(Error::NotCert), @@ -76,7 +71,6 @@ pub fn derive(data: DerivationResponse, keys: &[KeyFlags], userid: &UserID) -> R let one_day = SystemTime::now() + Duration::from_secs(60 * 60 * 24); // Create certificate with initial key and signature - let xprv = ExtendedPrivateKey::::try_from(data)?; let derived_primary_key = xprv.derive_child(&DerivationIndex::new(0, true)?)?; let primary_key = Key::from(Key4::<_, PrimaryRole>::import_secret_ed25519( &PrivateKey::to_bytes(derived_primary_key.private_key()), @@ -118,21 +112,14 @@ pub fn derive(data: DerivationResponse, keys: &[KeyFlags], userid: &UserID) -> R bytes[0] &= 0b1111_1000; bytes[31] &= !0b1000_0000; bytes[31] |= 0b0100_0000; - Key::from( - Key4::<_, SubordinateRole>::import_secret_cv25519( - &bytes, - None, - None, - epoch, - )? - ) + Key::from(Key4::<_, SubordinateRole>::import_secret_cv25519( + &bytes, None, None, epoch, + )?) } else { - Key::from( - Key4::<_, SubordinateRole>::import_secret_ed25519( - &PrivateKey::to_bytes(derived_key.private_key()), - epoch, - )? - ) + Key::from(Key4::<_, SubordinateRole>::import_secret_ed25519( + &PrivateKey::to_bytes(derived_key.private_key()), + epoch, + )?) }; // As per OpenPGP spec, signing keys must backsig the primary key diff --git a/crates/derive/keyfork-derive-openpgp/src/main.rs b/crates/derive/keyfork-derive-openpgp/src/main.rs index 3aec137..8431b17 100644 --- a/crates/derive/keyfork-derive-openpgp/src/main.rs +++ b/crates/derive/keyfork-derive-openpgp/src/main.rs @@ -2,12 +2,16 @@ use std::{env, process::ExitCode, str::FromStr}; -use keyfork_derive_util::{ - request::{DerivationAlgorithm, DerivationRequest, DerivationResponse}, - DerivationIndex, DerivationPath, -}; +use keyfork_derive_util::{DerivationIndex, DerivationPath}; use keyforkd_client::Client; -use sequoia_openpgp::{packet::UserID, types::KeyFlags, armor::{Kind, Writer}, serialize::Marshal}; + +use ed25519_dalek::SigningKey; +use sequoia_openpgp::{ + armor::{Kind, Writer}, + packet::UserID, + serialize::Marshal, + types::KeyFlags, +}; #[derive(Debug, thiserror::Error)] enum Error { @@ -108,16 +112,13 @@ fn run() -> Result<(), Box> { _ => panic!("Usage: {program_name} path subkey_format default_userid"), }; - let request = DerivationRequest::new(DerivationAlgorithm::Ed25519, &path); - let derived_data: DerivationResponse = Client::discover_socket()? - .request(&request.into())? - .try_into()?; + let derived_xprv = Client::discover_socket()?.request_xprv::(&path)?; let subkeys = subkey_format .iter() .map(|kt| kt.inner().clone()) .collect::>(); - let cert = keyfork_derive_openpgp::derive(derived_data, subkeys.as_slice(), &default_userid)?; + let cert = keyfork_derive_openpgp::derive(derived_xprv, subkeys.as_slice(), &default_userid)?; let mut w = Writer::new(std::io::stdout(), Kind::SecretKey)?; diff --git a/crates/derive/keyfork-derive-util/src/extended_key/private_key.rs b/crates/derive/keyfork-derive-util/src/extended_key/private_key.rs index 1bbf3ca..5921159 100644 --- a/crates/derive/keyfork-derive-util/src/extended_key/private_key.rs +++ b/crates/derive/keyfork-derive-util/src/extended_key/private_key.rs @@ -45,11 +45,36 @@ type HmacSha512 = Hmac; #[derive(Clone, Serialize, Deserialize)] pub struct ExtendedPrivateKey { /// The internal private key data. + #[serde(with = "serde_with")] private_key: K, depth: u8, chain_code: ChainCode, } +mod serde_with { + use super::*; + + pub(crate) fn serialize(value: &K, serializer: S) -> Result + where + S: serde::Serializer, + K: PrivateKey + Clone, + { + serializer.serialize_bytes(&value.to_bytes()) + } + + pub(crate) fn deserialize<'de, D, K>(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + K: PrivateKey + Clone, + { + let variable_len_bytes = <&[u8]>::deserialize(deserializer)?; + let bytes: [u8; 32] = variable_len_bytes + .try_into() + .expect("unable to parse serialized private key; no support for static len"); + Ok(K::from_bytes(&bytes)) + } +} + impl std::fmt::Debug for ExtendedPrivateKey { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { f.debug_struct("ExtendedPrivateKey") @@ -105,12 +130,14 @@ where .into_bytes(); let (private_key, chain_code) = hash.split_at(KEY_SIZE / 8); - Self::new_from_parts( - private_key, + Ok(Self::new_from_parts( + private_key + .try_into() + .expect("KEY_SIZE / 8 did not give a 32 byte slice"), 0, // Checked: chain_code is always the same length, hash is static size chain_code.try_into().expect("Invalid chain code length"), - ) + )) } /// Create an [`ExtendedPrivateKey`] from a given `seed`, `depth`, and `chain_code`. @@ -125,21 +152,18 @@ where /// # public_key::TestPublicKey as PublicKey, /// # private_key::TestPrivateKey as PrivateKey, /// # }; - /// # fn main() -> Result<(), Box> { /// let key: &[u8; 32] = // /// # b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; /// let chain_code: &[u8; 32] = // /// # b"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"; - /// let xprv = ExtendedPrivateKey::::new_from_parts(key, 4, *chain_code)?; - /// # Ok(()) - /// # } + /// let xprv = ExtendedPrivateKey::::new_from_parts(key, 4, *chain_code); /// ``` - pub fn new_from_parts(seed: &[u8], depth: u8, chain_code: [u8; 32]) -> Result { - Ok(Self { - private_key: K::from_bytes(seed.try_into()?), + pub fn new_from_parts(key: &[u8; 32], depth: u8, chain_code: [u8; 32]) -> Self { + Self { + private_key: K::from_bytes(&key), depth, chain_code, - }) + } } /// Returns a reference to the [`PrivateKey`]. @@ -152,15 +176,12 @@ where /// # public_key::TestPublicKey as PublicKey, /// # private_key::TestPrivateKey as PrivateKey, /// # }; - /// # fn main() -> Result<(), Box> { /// let key: &[u8; 32] = // /// # b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; /// let chain_code: &[u8; 32] = // /// # b"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"; - /// let xprv = ExtendedPrivateKey::::new_from_parts(key, 4, *chain_code)?; + /// let xprv = ExtendedPrivateKey::::new_from_parts(key, 4, *chain_code); /// assert_eq!(xprv.private_key(), &PrivateKey::from_bytes(key)); - /// # Ok(()) - /// # } /// ``` pub fn private_key(&self) -> &K { &self.private_key diff --git a/crates/derive/keyfork-derive-util/src/request.rs b/crates/derive/keyfork-derive-util/src/request.rs index 57fc810..c609adc 100644 --- a/crates/derive/keyfork-derive-util/src/request.rs +++ b/crates/derive/keyfork-derive-util/src/request.rs @@ -110,6 +110,18 @@ impl std::str::FromStr for DerivationAlgorithm { } } +/// Acquire the associated [`DerivationAlgorithm`] for a [`PrivateKey`]. +pub trait AsAlgorithm: PrivateKey { + /// Return the appropriate [`DerivationAlgorithm`]. + fn as_algorithm() -> DerivationAlgorithm; +} + +impl AsAlgorithm for TestPrivateKey { + fn as_algorithm() -> DerivationAlgorithm { + DerivationAlgorithm::Internal + } +} + /// A derivation request. #[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)] pub struct DerivationRequest { @@ -234,7 +246,7 @@ pub struct DerivationResponse { pub algorithm: DerivationAlgorithm, /// The derived private key. - pub data: Vec, + pub data: [u8; 32], /// The chain code, used for further derivation. pub chain_code: [u8; 32], @@ -251,7 +263,7 @@ impl DerivationResponse { ) -> Self { Self { algorithm, - data: PrivateKey::to_bytes(xprv.private_key()).to_vec(), + data: PrivateKey::to_bytes(xprv.private_key()), chain_code: xprv.chain_code(), depth: xprv.depth(), } @@ -272,47 +284,71 @@ pub enum TryFromDerivationResponseError { } #[cfg(feature = "secp256k1")] -impl TryFrom<&DerivationResponse> for ExtendedPrivateKey { - type Error = TryFromDerivationResponseError; +mod secp256k1 { + use super::*; + use k256::SecretKey; - fn try_from(value: &DerivationResponse) -> std::result::Result { - match value.algorithm { - DerivationAlgorithm::Secp256k1 => { - Self::new_from_parts(&value.data, value.depth, value.chain_code).map_err(From::from) - } - _ => Err(Self::Error::Algorithm), + impl AsAlgorithm for SecretKey { + fn as_algorithm() -> DerivationAlgorithm { + DerivationAlgorithm::Secp256k1 } } -} -#[cfg(feature = "secp256k1")] -impl TryFrom for ExtendedPrivateKey { - type Error = TryFromDerivationResponseError; + impl TryFrom<&DerivationResponse> for ExtendedPrivateKey { + type Error = TryFromDerivationResponseError; - fn try_from(value: DerivationResponse) -> std::result::Result { - ExtendedPrivateKey::::try_from(&value) - } -} - -#[cfg(feature = "ed25519")] -impl TryFrom<&DerivationResponse> for ExtendedPrivateKey { - type Error = TryFromDerivationResponseError; - - fn try_from(value: &DerivationResponse) -> std::result::Result { - match value.algorithm { - DerivationAlgorithm::Ed25519 => { - Self::new_from_parts(&value.data, value.depth, value.chain_code).map_err(From::from) + fn try_from(value: &DerivationResponse) -> Result { + match value.algorithm { + DerivationAlgorithm::Secp256k1 => Ok(Self::new_from_parts( + &value.data, + value.depth, + value.chain_code, + )), + _ => Err(Self::Error::Algorithm), } - _ => Err(Self::Error::Algorithm), + } + } + + impl TryFrom for ExtendedPrivateKey { + type Error = TryFromDerivationResponseError; + + fn try_from(value: DerivationResponse) -> Result { + ExtendedPrivateKey::::try_from(&value) } } } #[cfg(feature = "ed25519")] -impl TryFrom for ExtendedPrivateKey { - type Error = TryFromDerivationResponseError; +mod ed25519 { + use super::*; + use ed25519_dalek::SigningKey; - fn try_from(value: DerivationResponse) -> std::result::Result { - ExtendedPrivateKey::::try_from(&value) + impl AsAlgorithm for SigningKey { + fn as_algorithm() -> DerivationAlgorithm { + DerivationAlgorithm::Ed25519 + } + } + + impl TryFrom<&DerivationResponse> for ExtendedPrivateKey { + type Error = TryFromDerivationResponseError; + + fn try_from(value: &DerivationResponse) -> Result { + match value.algorithm { + DerivationAlgorithm::Ed25519 => Ok(Self::new_from_parts( + &value.data, + value.depth, + value.chain_code, + )), + _ => Err(Self::Error::Algorithm), + } + } + } + + impl TryFrom for ExtendedPrivateKey { + type Error = TryFromDerivationResponseError; + + fn try_from(value: DerivationResponse) -> Result { + ExtendedPrivateKey::::try_from(&value) + } } } diff --git a/crates/keyfork-shard/src/openpgp.rs b/crates/keyfork-shard/src/openpgp.rs index 36ab011..a081b51 100644 --- a/crates/keyfork-shard/src/openpgp.rs +++ b/crates/keyfork-shard/src/openpgp.rs @@ -13,9 +13,9 @@ use aes_gcm::{ Aes256Gcm, Error as AesError, KeyInit, Nonce, }; use hkdf::{Hkdf, InvalidLength as HkdfInvalidLength}; -use keyfork_derive_openpgp::derive_util::{ - request::{DerivationAlgorithm, DerivationRequest}, - DerivationPath, PathError, +use keyfork_derive_openpgp::{ + derive_util::{DerivationPath, PathError}, + XPrv, }; use keyfork_mnemonic_util::{Mnemonic, MnemonicFromStrError, MnemonicGenerationError, Wordlist}; use keyfork_prompt::{ @@ -123,6 +123,10 @@ pub enum Error { #[error("IO error: {0}")] Io(#[source] std::io::Error), + /// An error occurred while deriving data. + #[error("Derivation: {0}")] + Derivation(#[from] keyfork_derive_openpgp::derive_util::extended_key::private_key::Error), + /// An error occurred while parsing a derivation path. #[error("Derivation path: {0}")] DerivationPath(#[from] PathError), @@ -643,13 +647,10 @@ pub fn combine( // TODO: extract as function let userid = UserID::from("keyfork-sss"); - let kdr = DerivationRequest::new( - DerivationAlgorithm::Ed25519, - &DerivationPath::from_str("m/7366512'/0'")?, - ) - .derive_with_master_seed(secret.clone())?; + let path = DerivationPath::from_str("m/7366512'/0'")?; + let xprv = XPrv::new(&secret)?.derive_path(&path)?; let derived_cert = keyfork_derive_openpgp::derive( - kdr, + xprv, &[KeyFlags::empty().set_certification().set_signing()], &userid, )?; @@ -680,13 +681,10 @@ pub fn combine( pub fn split(threshold: u8, certs: Vec, secret: &[u8], output: impl Write) -> Result<()> { // build cert to sign encrypted shares let userid = UserID::from("keyfork-sss"); - let kdr = DerivationRequest::new( - DerivationAlgorithm::Ed25519, - &DerivationPath::from_str("m/7366512'/0'")?, - ) - .derive_with_master_seed(secret.to_vec())?; + let path = DerivationPath::from_str("m/7366512'/0'")?; + let xprv = XPrv::new(&secret)?.derive_path(&path)?; let derived_cert = keyfork_derive_openpgp::derive( - kdr, + xprv, &[KeyFlags::empty().set_certification().set_signing()], &userid, )?; diff --git a/crates/keyfork/src/cli/derive.rs b/crates/keyfork/src/cli/derive.rs index 7b2dc2f..27cd731 100644 --- a/crates/keyfork/src/cli/derive.rs +++ b/crates/keyfork/src/cli/derive.rs @@ -1,16 +1,16 @@ use super::Keyfork; use clap::{Parser, Subcommand}; -use keyfork_derive_openpgp::openpgp::{ - armor::{Kind, Writer}, - packet::UserID, - serialize::Marshal, - types::KeyFlags, -}; -use keyfork_derive_util::{ - request::{DerivationAlgorithm, DerivationRequest, DerivationResponse}, - DerivationIndex, DerivationPath, +use keyfork_derive_openpgp::{ + openpgp::{ + armor::{Kind, Writer}, + packet::UserID, + serialize::Marshal, + types::KeyFlags, + }, + XPrvKey, }; +use keyfork_derive_util::{DerivationIndex, DerivationPath}; use keyforkd_client::Client; type Result> = std::result::Result; @@ -48,12 +48,9 @@ impl DeriveSubcommands { .set_storage_encryption(), KeyFlags::empty().set_authentication(), ]; - let request = DerivationRequest::new(DerivationAlgorithm::Ed25519, &path); - let derived_data: DerivationResponse = Client::discover_socket()? - .request(&request.into())? - .try_into()?; + let xprv = Client::discover_socket()?.request_xprv::(&path)?; let default_userid = UserID::from(user_id.as_str()); - let cert = keyfork_derive_openpgp::derive(derived_data, &subkeys, &default_userid)?; + let cert = keyfork_derive_openpgp::derive(xprv, &subkeys, &default_userid)?; let mut w = Writer::new(std::io::stdout(), Kind::SecretKey)?; diff --git a/crates/keyfork/src/cli/wizard.rs b/crates/keyfork/src/cli/wizard.rs index 2793890..f3f9992 100644 --- a/crates/keyfork/src/cli/wizard.rs +++ b/crates/keyfork/src/cli/wizard.rs @@ -5,11 +5,11 @@ use std::{collections::HashSet, fs::File, io::IsTerminal, path::PathBuf}; use card_backend_pcsc::PcscBackend; use openpgp_card_sequoia::{state::Open, types::KeyType, Card}; -use keyfork_derive_openpgp::openpgp::{self, packet::UserID, types::KeyFlags, Cert}; -use keyfork_derive_util::{ - request::{DerivationAlgorithm, DerivationRequest}, - DerivationIndex, DerivationPath, +use keyfork_derive_openpgp::{ + openpgp::{self, packet::UserID, types::KeyFlags, Cert}, + XPrv, }; +use keyfork_derive_util::{DerivationIndex, DerivationPath}; use keyfork_prompt::{ validators::{PinValidator, Validator}, Message, PromptHandler, Terminal, @@ -42,10 +42,9 @@ fn derive_key(seed: &[u8], index: u8) -> Result { .chain_push(chain) .chain_push(account) .chain_push(subkey); - let request = DerivationRequest::new(DerivationAlgorithm::Ed25519, &path); - let response = request.derive_with_master_seed(seed.to_vec())?; + let xprv = XPrv::new(seed)?.derive_path(&path)?; let userid = UserID::from(format!("Keyfork Shard {index}")); - let cert = keyfork_derive_openpgp::derive(response, &subkeys, &userid)?; + let cert = keyfork_derive_openpgp::derive(xprv, &subkeys, &userid)?; Ok(cert) } -- 2.40.1 From 4354be4304f8573608793a1943619fed07317b38 Mon Sep 17 00:00:00 2001 From: ryan Date: Sun, 11 Feb 2024 20:35:26 -0500 Subject: [PATCH 2/4] keyfork-derive-util: add arbitrary length seeds, remove length-based errors --- .../src/extended_key/private_key.rs | 55 ++++++------------- .../derive/keyfork-derive-util/src/request.rs | 6 +- .../derive/keyfork-derive-util/src/tests.rs | 12 ++-- crates/keyfork-shard/src/openpgp.rs | 4 +- crates/keyfork/src/cli/wizard.rs | 2 +- 5 files changed, 29 insertions(+), 50 deletions(-) diff --git a/crates/derive/keyfork-derive-util/src/extended_key/private_key.rs b/crates/derive/keyfork-derive-util/src/extended_key/private_key.rs index 5921159..f63846e 100644 --- a/crates/derive/keyfork-derive-util/src/extended_key/private_key.rs +++ b/crates/derive/keyfork-derive-util/src/extended_key/private_key.rs @@ -10,18 +10,10 @@ const KEY_SIZE: usize = 256; /// Errors associated with creating or deriving Extended Private Keys. #[derive(Error, Clone, Debug)] pub enum Error { - /// The seed has an unsuitable length; supported lengths are 16 bytes, 32 bytes, or 64 bytes. - #[error("Seed had an unsuitable length: {0}")] - BadSeedLength(usize), - /// The maximum depth for key derivation has been reached. The supported maximum depth is 255. #[error("Reached maximum depth for key derivation")] Depth, - /// This should never happen. HMAC keys should be able to take any size input. - #[error("Invalid length for HMAC key while generating master key (report me!)")] - HmacInvalidLength(#[from] hmac::digest::InvalidLength), - /// An unknown error occurred while deriving a child key. #[error("Unknown error while deriving child key")] Derivation, @@ -98,7 +90,7 @@ where /// # Errors /// An error may be returned if: /// * The given seed had an incorrect length. - /// * A `HmacSha512` can't be constructed - this should be impossible. + /// * A `HmacSha512` can't be constructed. /// /// # Examples /// ```rust @@ -107,37 +99,30 @@ where /// # public_key::TestPublicKey as PublicKey, /// # private_key::TestPrivateKey as PrivateKey, /// # }; - /// # fn main() -> Result<(), Box> { /// let seed: &[u8; 64] = // /// # b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; - /// let xprv = ExtendedPrivateKey::::new(seed)?; - /// # Ok(()) - /// # } + /// let xprv = ExtendedPrivateKey::::new(seed); /// ``` - pub fn new(seed: impl AsRef<[u8]>) -> Result { + pub fn new(seed: impl AsRef<[u8]>) -> Self { Self::new_internal(seed.as_ref()) } - fn new_internal(seed: &[u8]) -> Result { - let len = seed.len(); - if ![16, 32, 64].contains(&len) { - return Err(Error::BadSeedLength(len)); - } - - let hash = HmacSha512::new_from_slice(&K::key().bytes().collect::>())? + fn new_internal(seed: &[u8]) -> Self { + let hash = HmacSha512::new_from_slice(&K::key().bytes().collect::>()) + .expect("HmacSha512 InvalidLength should be infallible") .chain_update(seed) .finalize() .into_bytes(); let (private_key, chain_code) = hash.split_at(KEY_SIZE / 8); - Ok(Self::new_from_parts( + Self::new_from_parts( private_key .try_into() .expect("KEY_SIZE / 8 did not give a 32 byte slice"), 0, // Checked: chain_code is always the same length, hash is static size chain_code.try_into().expect("Invalid chain code length"), - )) + ) } /// Create an [`ExtendedPrivateKey`] from a given `seed`, `depth`, and `chain_code`. @@ -160,7 +145,7 @@ where /// ``` pub fn new_from_parts(key: &[u8; 32], depth: u8, chain_code: [u8; 32]) -> Self { Self { - private_key: K::from_bytes(&key), + private_key: K::from_bytes(key), depth, chain_code, } @@ -206,7 +191,7 @@ where /// # 102, 201, 210, 159, 219, 222, 42, 201, 44, 196, 27, /// # 90, 221, 80, 85, 135, 79, 39, 253, 223, 35, 251 /// # ]; - /// let xprv = ExtendedPrivateKey::::new(seed)?; + /// let xprv = ExtendedPrivateKey::::new(seed); /// let xpub = xprv.extended_public_key(); /// assert_eq!(known_key, xpub.public_key().to_bytes()); /// # Ok(()) @@ -230,7 +215,7 @@ where /// # fn main() -> Result<(), Box> { /// let seed: &[u8; 64] = // /// # b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; - /// let xprv = ExtendedPrivateKey::::new(seed)?; + /// let xprv = ExtendedPrivateKey::::new(seed); /// let pubkey = xprv.public_key(); /// # Ok(()) /// # } @@ -248,15 +233,12 @@ where /// # public_key::TestPublicKey as PublicKey, /// # private_key::TestPrivateKey as PrivateKey, /// # }; - /// # fn main() -> Result<(), Box> { /// let key: &[u8; 32] = // /// # b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; /// let chain_code: &[u8; 32] = // /// # b"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"; - /// let xprv = ExtendedPrivateKey::::new_from_parts(key, 4, *chain_code)?; + /// let xprv = ExtendedPrivateKey::::new_from_parts(key, 4, *chain_code); /// assert_eq!(xprv.depth(), 4); - /// # Ok(()) - /// # } /// ``` pub fn depth(&self) -> u8 { self.depth @@ -271,15 +253,12 @@ where /// # public_key::TestPublicKey as PublicKey, /// # private_key::TestPrivateKey as PrivateKey, /// # }; - /// # fn main() -> Result<(), Box> { /// let key: &[u8; 32] = // /// # b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; /// let chain_code: &[u8; 32] = // /// # b"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"; - /// let xprv = ExtendedPrivateKey::::new_from_parts(key, 4, *chain_code)?; + /// let xprv = ExtendedPrivateKey::::new_from_parts(key, 4, *chain_code); /// assert_eq!(chain_code, &xprv.chain_code()); - /// # Ok(()) - /// # } /// ``` pub fn chain_code(&self) -> [u8; 32] { self.chain_code @@ -301,7 +280,7 @@ where /// # fn main() -> Result<(), Box> { /// let seed: &[u8; 64] = // /// # b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; - /// let root_xprv = ExtendedPrivateKey::::new(seed)?; + /// let root_xprv = ExtendedPrivateKey::::new(seed); /// let path = DerivationPath::default() /// .chain_push(DerivationIndex::new(44, true)?) /// .chain_push(DerivationIndex::new(0, true)?) @@ -347,7 +326,7 @@ where /// # fn main() -> Result<(), Box> { /// let seed: &[u8; 64] = // /// # b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; - /// let root_xprv = ExtendedPrivateKey::::new(seed)?; + /// let root_xprv = ExtendedPrivateKey::::new(seed); /// let bip44_wallet = DerivationPath::default() /// .chain_push(DerivationIndex::new(44, true)?) /// .chain_push(DerivationIndex::new(0, true)?) @@ -363,8 +342,8 @@ where pub fn derive_child(&self, index: &DerivationIndex) -> Result { let depth = self.depth.checked_add(1).ok_or(Error::Depth)?; - let mut hmac = - HmacSha512::new_from_slice(&self.chain_code).map_err(Error::HmacInvalidLength)?; + let mut hmac = HmacSha512::new_from_slice(&self.chain_code) + .expect("HmacSha512 InvalidLength should be infallible"); if index.is_hardened() { hmac.update(&[0]); hmac.update(&self.private_key.to_bytes()); diff --git a/crates/derive/keyfork-derive-util/src/request.rs b/crates/derive/keyfork-derive-util/src/request.rs index c609adc..cb5c63b 100644 --- a/crates/derive/keyfork-derive-util/src/request.rs +++ b/crates/derive/keyfork-derive-util/src/request.rs @@ -68,7 +68,7 @@ impl DerivationAlgorithm { match self { #[cfg(feature = "ed25519")] Self::Ed25519 => { - let key = ExtendedPrivateKey::::new(seed)?; + let key = ExtendedPrivateKey::::new(seed); let derived_key = key.derive_path(path)?; Ok(DerivationResponse::with_algo_and_xprv( self.clone(), @@ -77,7 +77,7 @@ impl DerivationAlgorithm { } #[cfg(feature = "secp256k1")] Self::Secp256k1 => { - let key = ExtendedPrivateKey::::new(seed)?; + let key = ExtendedPrivateKey::::new(seed); let derived_key = key.derive_path(path)?; Ok(DerivationResponse::with_algo_and_xprv( self.clone(), @@ -85,7 +85,7 @@ impl DerivationAlgorithm { )) } Self::Internal => { - let key = ExtendedPrivateKey::::new(seed)?; + let key = ExtendedPrivateKey::::new(seed); let derived_key = key.derive_path(path)?; Ok(DerivationResponse::with_algo_and_xprv( self.clone(), diff --git a/crates/derive/keyfork-derive-util/src/tests.rs b/crates/derive/keyfork-derive-util/src/tests.rs index f4238e3..eb9d6f4 100644 --- a/crates/derive/keyfork-derive-util/src/tests.rs +++ b/crates/derive/keyfork-derive-util/src/tests.rs @@ -30,7 +30,7 @@ fn secp256k1() { } = test; // Tests for ExtendedPrivateKey - let xkey = ExtendedPrivateKey::::new(seed).unwrap(); + let xkey = ExtendedPrivateKey::::new(seed); let derived_key = xkey.derive_path(&chain).unwrap(); assert_eq!( derived_key.chain_code().as_slice(), @@ -51,7 +51,7 @@ fn secp256k1() { // Tests for DerivationRequest let request = DerivationRequest::new(DerivationAlgorithm::Secp256k1, &chain); let response = request.derive_with_master_seed(seed.clone()).unwrap(); - assert_eq!(&response.data, private_key, "test: {chain}"); + assert_eq!(&response.data, private_key.as_slice(), "test: {chain}"); } } } @@ -75,7 +75,7 @@ fn ed25519() { } = test; // Tests for ExtendedPrivateKey - let xkey = ExtendedPrivateKey::::new(seed).unwrap(); + let xkey = ExtendedPrivateKey::::new(seed); let derived_key = xkey.derive_path(&chain).unwrap(); assert_eq!( derived_key.chain_code().as_slice(), @@ -96,7 +96,7 @@ fn ed25519() { // Tests for DerivationRequest let request = DerivationRequest::new(DerivationAlgorithm::Ed25519, &chain); let response = request.derive_with_master_seed(seed.to_vec()).unwrap(); - assert_eq!(&response.data, private_key, "test: {chain}"); + assert_eq!(&response.data, private_key.as_slice(), "test: {chain}"); } } } @@ -108,7 +108,7 @@ fn panics_with_unhardened_derivation() { use ed25519_dalek::SigningKey; let seed = hex!("000102030405060708090a0b0c0d0e0f"); - let xkey = ExtendedPrivateKey::::new(seed).unwrap(); + let xkey = ExtendedPrivateKey::::new(seed); xkey.derive_path(&DerivationPath::from_str("m/0").unwrap()) .unwrap(); } @@ -120,7 +120,7 @@ fn panics_at_depth() { use ed25519_dalek::SigningKey; let seed = hex!("000102030405060708090a0b0c0d0e0f"); - let mut xkey = ExtendedPrivateKey::::new(seed).unwrap(); + let mut xkey = ExtendedPrivateKey::::new(seed); for i in 0..=u32::from(u8::MAX) { xkey = xkey .derive_child(&DerivationIndex::new(i, true).unwrap()) diff --git a/crates/keyfork-shard/src/openpgp.rs b/crates/keyfork-shard/src/openpgp.rs index a081b51..c3c1aad 100644 --- a/crates/keyfork-shard/src/openpgp.rs +++ b/crates/keyfork-shard/src/openpgp.rs @@ -648,7 +648,7 @@ pub fn combine( // TODO: extract as function let userid = UserID::from("keyfork-sss"); let path = DerivationPath::from_str("m/7366512'/0'")?; - let xprv = XPrv::new(&secret)?.derive_path(&path)?; + let xprv = XPrv::new(&secret).derive_path(&path)?; let derived_cert = keyfork_derive_openpgp::derive( xprv, &[KeyFlags::empty().set_certification().set_signing()], @@ -682,7 +682,7 @@ pub fn split(threshold: u8, certs: Vec, secret: &[u8], output: impl Write) // build cert to sign encrypted shares let userid = UserID::from("keyfork-sss"); let path = DerivationPath::from_str("m/7366512'/0'")?; - let xprv = XPrv::new(&secret)?.derive_path(&path)?; + let xprv = XPrv::new(&secret).derive_path(&path)?; let derived_cert = keyfork_derive_openpgp::derive( xprv, &[KeyFlags::empty().set_certification().set_signing()], diff --git a/crates/keyfork/src/cli/wizard.rs b/crates/keyfork/src/cli/wizard.rs index f3f9992..fb52abe 100644 --- a/crates/keyfork/src/cli/wizard.rs +++ b/crates/keyfork/src/cli/wizard.rs @@ -42,7 +42,7 @@ fn derive_key(seed: &[u8], index: u8) -> Result { .chain_push(chain) .chain_push(account) .chain_push(subkey); - let xprv = XPrv::new(seed)?.derive_path(&path)?; + let xprv = XPrv::new(seed).derive_path(&path)?; let userid = UserID::from(format!("Keyfork Shard {index}")); let cert = keyfork_derive_openpgp::derive(xprv, &subkeys, &userid)?; Ok(cert) -- 2.40.1 From 053902bf4313322f6172a91033f215a69319fde9 Mon Sep 17 00:00:00 2001 From: ryan Date: Mon, 12 Feb 2024 00:02:59 -0500 Subject: [PATCH 3/4] keyfork-derive-util: make variable-length seeds opt-in --- crates/daemon/keyforkd-client/src/tests.rs | 4 +- crates/daemon/keyforkd/src/service.rs | 6 +- crates/derive/keyfork-derive-util/README.md | 2 +- .../src/extended_key/private_key.rs | 76 +++++++++++++++++-- crates/derive/keyfork-derive-util/src/lib.rs | 2 +- .../derive/keyfork-derive-util/src/request.rs | 12 +-- .../derive/keyfork-derive-util/src/tests.rs | 10 ++- crates/keyfork-shard/Cargo.toml | 3 +- crates/keyfork-shard/src/openpgp.rs | 8 +- crates/keyfork/src/cli/wizard.rs | 6 +- crates/util/keyfork-entropy/Cargo.toml | 2 +- crates/util/keyfork-entropy/src/lib.rs | 26 ++++++- crates/util/keyfork-mnemonic-util/Cargo.toml | 2 +- crates/util/keyfork-mnemonic-util/src/lib.rs | 6 +- .../examples/test-basic-prompt.rs | 8 +- 15 files changed, 132 insertions(+), 41 deletions(-) diff --git a/crates/daemon/keyforkd-client/src/tests.rs b/crates/daemon/keyforkd-client/src/tests.rs index a3aabfb..d12f528 100644 --- a/crates/daemon/keyforkd-client/src/tests.rs +++ b/crates/daemon/keyforkd-client/src/tests.rs @@ -47,7 +47,7 @@ fn secp256k1() { ); let response = DerivationResponse::try_from(client.request(&req.into()).unwrap()).unwrap(); - assert_eq!(response.data, test.private_key); + assert_eq!(&response.data, test.private_key.as_slice()); } handle.abort(); @@ -92,7 +92,7 @@ fn ed25519() { ); let response = DerivationResponse::try_from(client.request(&req.into()).unwrap()).unwrap(); - assert_eq!(response.data, test.private_key); + assert_eq!(&response.data, test.private_key.as_slice()); } handle.abort(); diff --git a/crates/daemon/keyforkd/src/service.rs b/crates/daemon/keyforkd/src/service.rs index 1b4058b..fee1cad 100644 --- a/crates/daemon/keyforkd/src/service.rs +++ b/crates/daemon/keyforkd/src/service.rs @@ -76,7 +76,7 @@ impl Service for Keyforkd { info!("Deriving path: {}", req.path()); } - req.derive_with_master_seed((*seed).clone()) + req.derive_with_master_seed(seed.as_ref()) .map(Response::Derivation) .map_err(|e| DerivationError::Derivation(e.to_string()).into()) }), @@ -120,7 +120,7 @@ mod tests { .unwrap() .try_into() .unwrap(); - assert_eq!(response.data, test.private_key); + assert_eq!(&response.data, test.private_key.as_slice()); assert_eq!(response.chain_code.as_slice(), test.chain_code); } } @@ -150,7 +150,7 @@ mod tests { .unwrap() .try_into() .unwrap(); - assert_eq!(response.data, test.private_key); + assert_eq!(&response.data, test.private_key.as_slice()); assert_eq!(response.chain_code.as_slice(), test.chain_code); } } diff --git a/crates/derive/keyfork-derive-util/README.md b/crates/derive/keyfork-derive-util/README.md index 20135ff..55fb1e1 100644 --- a/crates/derive/keyfork-derive-util/README.md +++ b/crates/derive/keyfork-derive-util/README.md @@ -40,7 +40,7 @@ fn main() -> Result<(), Box> { let key1 = request.derive_with_mnemonic(&mnemonic)?; let seed = mnemonic.seed(None)?; - let key2 = request.derive_with_master_seed(seed)?; + let key2 = request.derive_with_master_seed(&seed)?; assert_eq!(key1, key2); diff --git a/crates/derive/keyfork-derive-util/src/extended_key/private_key.rs b/crates/derive/keyfork-derive-util/src/extended_key/private_key.rs index f63846e..ad15994 100644 --- a/crates/derive/keyfork-derive-util/src/extended_key/private_key.rs +++ b/crates/derive/keyfork-derive-util/src/extended_key/private_key.rs @@ -31,6 +31,68 @@ type Result = std::result::Result; type ChainCode = [u8; 32]; type HmacSha512 = Hmac; +/// A reference to a variable-length seed. Keyfork automatically supports a seed of 128 bits, +/// 256 bits, or 512 bits, but because the master key is derived from a hashed seed, in theory +/// any amount of bytes could be used. It is not advised to use a variable-length seed longer +/// than 256 bits, as a brute-force attack on the master key could be performed in 2^256 +/// attempts. +/// +/// Mnemonics use a 512 bit seed, as knowledge of the mnemonics' words (such as through a side +/// channel attack) could leak which individual word is used, but not the order the words are +/// used in. Using a 512 bit hash to generate the seed results in a more computationally +/// expensive brute-force requirement. +pub struct VariableLengthSeed<'a> { + seed: &'a [u8], +} + +impl<'a> VariableLengthSeed<'a> { + /// Create a new VariableLengthSeed. + /// + /// # Examples + /// ```rust + /// use sha2::{Sha256, Digest}; + /// use keyfork_derive_util::VariableLengthSeed; + /// + /// let data = b"the missile is very eepy and wants to take a small sleeb"; + /// let seed = VariableLengthSeed::new(data); + /// ``` + pub fn new(seed: &'a [u8]) -> Self { + Self { seed } + } +} + +mod as_private_key { + use super::VariableLengthSeed; + + pub trait AsPrivateKey { + fn as_private_key(&self) -> &[u8]; + } + + impl AsPrivateKey for [u8; 16] { + fn as_private_key(&self) -> &[u8] { + self + } + } + + impl AsPrivateKey for [u8; 32] { + fn as_private_key(&self) -> &[u8] { + self + } + } + + impl AsPrivateKey for [u8; 64] { + fn as_private_key(&self) -> &[u8] { + self + } + } + + impl AsPrivateKey for VariableLengthSeed<'_> { + fn as_private_key(&self) -> &[u8] { + self.seed + } + } +} + /// Extended private keys derived using BIP-0032. /// /// Generic over types implementing [`PrivateKey`]. @@ -101,10 +163,10 @@ where /// # }; /// let seed: &[u8; 64] = // /// # b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; - /// let xprv = ExtendedPrivateKey::::new(seed); + /// let xprv = ExtendedPrivateKey::::new(*seed); /// ``` - pub fn new(seed: impl AsRef<[u8]>) -> Self { - Self::new_internal(seed.as_ref()) + pub fn new(seed: impl as_private_key::AsPrivateKey) -> Self { + Self::new_internal(seed.as_private_key()) } fn new_internal(seed: &[u8]) -> Self { @@ -191,7 +253,7 @@ where /// # 102, 201, 210, 159, 219, 222, 42, 201, 44, 196, 27, /// # 90, 221, 80, 85, 135, 79, 39, 253, 223, 35, 251 /// # ]; - /// let xprv = ExtendedPrivateKey::::new(seed); + /// let xprv = ExtendedPrivateKey::::new(*seed); /// let xpub = xprv.extended_public_key(); /// assert_eq!(known_key, xpub.public_key().to_bytes()); /// # Ok(()) @@ -215,7 +277,7 @@ where /// # fn main() -> Result<(), Box> { /// let seed: &[u8; 64] = // /// # b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; - /// let xprv = ExtendedPrivateKey::::new(seed); + /// let xprv = ExtendedPrivateKey::::new(*seed); /// let pubkey = xprv.public_key(); /// # Ok(()) /// # } @@ -280,7 +342,7 @@ where /// # fn main() -> Result<(), Box> { /// let seed: &[u8; 64] = // /// # b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; - /// let root_xprv = ExtendedPrivateKey::::new(seed); + /// let root_xprv = ExtendedPrivateKey::::new(*seed); /// let path = DerivationPath::default() /// .chain_push(DerivationIndex::new(44, true)?) /// .chain_push(DerivationIndex::new(0, true)?) @@ -326,7 +388,7 @@ where /// # fn main() -> Result<(), Box> { /// let seed: &[u8; 64] = // /// # b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; - /// let root_xprv = ExtendedPrivateKey::::new(seed); + /// let root_xprv = ExtendedPrivateKey::::new(*seed); /// let bip44_wallet = DerivationPath::default() /// .chain_push(DerivationIndex::new(44, true)?) /// .chain_push(DerivationIndex::new(0, true)?) diff --git a/crates/derive/keyfork-derive-util/src/lib.rs b/crates/derive/keyfork-derive-util/src/lib.rs index ba170d4..ce2c1a7 100644 --- a/crates/derive/keyfork-derive-util/src/lib.rs +++ b/crates/derive/keyfork-derive-util/src/lib.rs @@ -17,7 +17,7 @@ pub mod public_key; mod tests; #[doc(inline)] -pub use crate::extended_key::{private_key::ExtendedPrivateKey, public_key::ExtendedPublicKey}; +pub use crate::extended_key::{private_key::{ExtendedPrivateKey, Error as XPrvError, VariableLengthSeed}, public_key::{ExtendedPublicKey, Error as XPubError}}; pub use crate::{ index::{DerivationIndex, Error as IndexError}, diff --git a/crates/derive/keyfork-derive-util/src/request.rs b/crates/derive/keyfork-derive-util/src/request.rs index cb5c63b..d6f5830 100644 --- a/crates/derive/keyfork-derive-util/src/request.rs +++ b/crates/derive/keyfork-derive-util/src/request.rs @@ -19,10 +19,11 @@ //! ``` use crate::{ - extended_key::private_key::Error as XPrvError, + extended_key::private_key::{Error as XPrvError, VariableLengthSeed}, private_key::{PrivateKey, TestPrivateKey}, DerivationPath, ExtendedPrivateKey, }; + use keyfork_mnemonic_util::{Mnemonic, MnemonicGenerationError}; use serde::{Deserialize, Serialize}; @@ -64,7 +65,8 @@ impl DerivationAlgorithm { /// /// # Errors /// The method may error if the derivation fails or if the algorithm is not supported. - fn derive(&self, seed: Vec, path: &DerivationPath) -> Result { + fn derive(&self, seed: &[u8], path: &DerivationPath) -> Result { + let seed = VariableLengthSeed::new(seed); match self { #[cfg(feature = "ed25519")] Self::Ed25519 => { @@ -207,7 +209,7 @@ impl DerivationRequest { /// # } pub fn derive_with_mnemonic(&self, mnemonic: &Mnemonic) -> Result { // TODO: passphrase support and/or store passphrase within mnemonic - self.derive_with_master_seed(mnemonic.seed(None)?) + self.derive_with_master_seed(&mnemonic.seed(None)?) } /// Derive an [`ExtendedPrivateKey`] using the given seed. @@ -231,10 +233,10 @@ impl DerivationRequest { /// let path: DerivationPath = // /// # DerivationPath::default(); /// let request = DerivationRequest::new(algo, &path); - /// let response = request.derive_with_master_seed(seed.to_vec())?; + /// let response = request.derive_with_master_seed(seed)?; /// # Ok(()) /// # } - pub fn derive_with_master_seed(&self, seed: Vec) -> Result { + pub fn derive_with_master_seed(&self, seed: &[u8]) -> Result { self.algorithm.derive(seed, &self.path) } } diff --git a/crates/derive/keyfork-derive-util/src/tests.rs b/crates/derive/keyfork-derive-util/src/tests.rs index eb9d6f4..3f2eba3 100644 --- a/crates/derive/keyfork-derive-util/src/tests.rs +++ b/crates/derive/keyfork-derive-util/src/tests.rs @@ -30,7 +30,8 @@ fn secp256k1() { } = test; // Tests for ExtendedPrivateKey - let xkey = ExtendedPrivateKey::::new(seed); + let varlen_seed = VariableLengthSeed::new(&seed); + let xkey = ExtendedPrivateKey::::new(varlen_seed); let derived_key = xkey.derive_path(&chain).unwrap(); assert_eq!( derived_key.chain_code().as_slice(), @@ -50,7 +51,7 @@ fn secp256k1() { // Tests for DerivationRequest let request = DerivationRequest::new(DerivationAlgorithm::Secp256k1, &chain); - let response = request.derive_with_master_seed(seed.clone()).unwrap(); + let response = request.derive_with_master_seed(&seed).unwrap(); assert_eq!(&response.data, private_key.as_slice(), "test: {chain}"); } } @@ -75,7 +76,8 @@ fn ed25519() { } = test; // Tests for ExtendedPrivateKey - let xkey = ExtendedPrivateKey::::new(seed); + let varlen_seed = VariableLengthSeed::new(&seed); + let xkey = ExtendedPrivateKey::::new(varlen_seed); let derived_key = xkey.derive_path(&chain).unwrap(); assert_eq!( derived_key.chain_code().as_slice(), @@ -95,7 +97,7 @@ fn ed25519() { // Tests for DerivationRequest let request = DerivationRequest::new(DerivationAlgorithm::Ed25519, &chain); - let response = request.derive_with_master_seed(seed.to_vec()).unwrap(); + let response = request.derive_with_master_seed(&seed).unwrap(); assert_eq!(&response.data, private_key.as_slice(), "test: {chain}"); } } diff --git a/crates/keyfork-shard/Cargo.toml b/crates/keyfork-shard/Cargo.toml index f1ae88b..8c2d75a 100644 --- a/crates/keyfork-shard/Cargo.toml +++ b/crates/keyfork-shard/Cargo.toml @@ -7,11 +7,10 @@ license = "AGPL-3.0-only" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [features] -default = ["openpgp", "openpgp-card", "qrcode"] +default = ["openpgp", "openpgp-card", "qrcode", "sequoia-openpgp/crypto-nettle", "keyfork-qrcode/decode-backend-rqrr"] openpgp = ["sequoia-openpgp", "anyhow"] openpgp-card = ["openpgp-card-sequoia", "card-backend-pcsc", "card-backend", "dep:openpgp-card"] qrcode = ["keyfork-qrcode"] -bin = ["sequoia-openpgp/crypto-nettle", "keyfork-qrcode/decode-backend-rqrr"] [dependencies] keyfork-prompt = { version = "0.1.0", path = "../util/keyfork-prompt", default-features = false, features = ["mnemonic"] } diff --git a/crates/keyfork-shard/src/openpgp.rs b/crates/keyfork-shard/src/openpgp.rs index c3c1aad..c07396a 100644 --- a/crates/keyfork-shard/src/openpgp.rs +++ b/crates/keyfork-shard/src/openpgp.rs @@ -14,7 +14,7 @@ use aes_gcm::{ }; use hkdf::{Hkdf, InvalidLength as HkdfInvalidLength}; use keyfork_derive_openpgp::{ - derive_util::{DerivationPath, PathError}, + derive_util::{DerivationPath, PathError, VariableLengthSeed}, XPrv, }; use keyfork_mnemonic_util::{Mnemonic, MnemonicFromStrError, MnemonicGenerationError, Wordlist}; @@ -648,7 +648,8 @@ pub fn combine( // TODO: extract as function let userid = UserID::from("keyfork-sss"); let path = DerivationPath::from_str("m/7366512'/0'")?; - let xprv = XPrv::new(&secret).derive_path(&path)?; + let seed = VariableLengthSeed::new(&secret); + let xprv = XPrv::new(seed).derive_path(&path)?; let derived_cert = keyfork_derive_openpgp::derive( xprv, &[KeyFlags::empty().set_certification().set_signing()], @@ -679,10 +680,11 @@ pub fn combine( /// 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, secret: &[u8], output: impl Write) -> Result<()> { + let seed = VariableLengthSeed::new(secret); // build cert to sign encrypted shares let userid = UserID::from("keyfork-sss"); let path = DerivationPath::from_str("m/7366512'/0'")?; - let xprv = XPrv::new(&secret).derive_path(&path)?; + let xprv = XPrv::new(seed).derive_path(&path)?; let derived_cert = keyfork_derive_openpgp::derive( xprv, &[KeyFlags::empty().set_certification().set_signing()], diff --git a/crates/keyfork/src/cli/wizard.rs b/crates/keyfork/src/cli/wizard.rs index fb52abe..3c6e1db 100644 --- a/crates/keyfork/src/cli/wizard.rs +++ b/crates/keyfork/src/cli/wizard.rs @@ -21,7 +21,7 @@ pub struct PinLength(usize); type Result> = std::result::Result; -fn derive_key(seed: &[u8], index: u8) -> Result { +fn derive_key(seed: [u8; 32], index: u8) -> Result { let subkeys = vec![ KeyFlags::empty().set_certification(), KeyFlags::empty().set_signing(), @@ -102,7 +102,7 @@ fn generate_shard_secret( keys_per_shard: u8, output_file: &Option, ) -> Result<()> { - let seed = keyfork_entropy::generate_entropy_of_size(256 / 8)?; + let seed = keyfork_entropy::generate_entropy_of_const_size::<{256 / 8}>()?; let mut pm = Terminal::new(std::io::stdin(), std::io::stderr())?; let mut certs = vec![]; let mut seen_cards: HashSet = HashSet::new(); @@ -126,7 +126,7 @@ fn generate_shard_secret( .to_fn(); for index in 0..max { - let cert = derive_key(&seed, index)?; + let cert = derive_key(seed, index)?; for i in 0..keys_per_shard { pm.prompt_message(Message::Text(format!( "Please remove all keys and insert key #{} for user #{}", diff --git a/crates/util/keyfork-entropy/Cargo.toml b/crates/util/keyfork-entropy/Cargo.toml index 1d5a444..20a75c2 100644 --- a/crates/util/keyfork-entropy/Cargo.toml +++ b/crates/util/keyfork-entropy/Cargo.toml @@ -7,7 +7,7 @@ license = "MIT" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [features] -default = [] +default = ["bin"] bin = ["smex"] [dependencies] diff --git a/crates/util/keyfork-entropy/src/lib.rs b/crates/util/keyfork-entropy/src/lib.rs index d7512b5..8f1580f 100644 --- a/crates/util/keyfork-entropy/src/lib.rs +++ b/crates/util/keyfork-entropy/src/lib.rs @@ -1,6 +1,9 @@ //! Utilities for reading entropy from secure sources. -use std::{fs::{read_dir, read_to_string, File}, io::Read}; +use std::{ + fs::{read_dir, read_to_string, File}, + io::Read, +}; static WARNING_LINKS: [&str; 1] = ["https://lore.kernel.org/lkml/20211223141113.1240679-2-Jason@zx2c4.com/"]; @@ -84,3 +87,24 @@ pub fn generate_entropy_of_size(byte_count: usize) -> Result, std::io::E entropy_file.read_exact(&mut vec[..])?; Ok(vec) } + +/// Read system entropy of a constant size. +/// +/// # Errors +/// An error may be returned if an error occurred while reading from the random source. +/// +/// # Examples +/// ```rust,no_run +/// # fn main() -> Result<(), Box> { +/// # std::env::set_var("SHOOT_SELF_IN_FOOT", "1"); +/// let entropy = keyfork_entropy::generate_entropy_of_const_size::<64>()?; +/// assert_eq!(entropy.len(), 64); +/// # Ok(()) +/// # } +/// ``` +pub fn generate_entropy_of_const_size() -> Result<[u8; N], std::io::Error> { + let mut output = [0u8; N]; + let mut entropy_file = File::open("/dev/urandom")?; + entropy_file.read_exact(&mut output[..])?; + Ok(output) +} diff --git a/crates/util/keyfork-mnemonic-util/Cargo.toml b/crates/util/keyfork-mnemonic-util/Cargo.toml index 3707884..10c88dc 100644 --- a/crates/util/keyfork-mnemonic-util/Cargo.toml +++ b/crates/util/keyfork-mnemonic-util/Cargo.toml @@ -7,7 +7,7 @@ edition = "2021" license = "MIT" [features] -default = [] +default = ["bin"] bin = ["smex"] [dependencies] diff --git a/crates/util/keyfork-mnemonic-util/src/lib.rs b/crates/util/keyfork-mnemonic-util/src/lib.rs index c34e305..b1eb04b 100644 --- a/crates/util/keyfork-mnemonic-util/src/lib.rs +++ b/crates/util/keyfork-mnemonic-util/src/lib.rs @@ -276,7 +276,7 @@ impl Mnemonic { } /// Clone the existing entropy. - #[deprecated] + #[deprecated = "Use as_bytes(), to_bytes(), or into_bytes() instead"] pub fn entropy(&self) -> Vec { self.entropy.clone() } @@ -353,8 +353,8 @@ mod tests { random_handle.read_exact(&mut entropy[..]).unwrap(); let wordlist = Wordlist::default().arc(); let mnemonic = super::Mnemonic::from_entropy(&entropy[..256 / 8], wordlist).unwrap(); - let new_entropy = mnemonic.entropy(); - assert_eq!(&new_entropy, entropy); + let new_entropy = mnemonic.as_bytes(); + assert_eq!(new_entropy, entropy); } #[test] diff --git a/crates/util/keyfork-prompt/examples/test-basic-prompt.rs b/crates/util/keyfork-prompt/examples/test-basic-prompt.rs index fe4ad86..801c35c 100644 --- a/crates/util/keyfork-prompt/examples/test-basic-prompt.rs +++ b/crates/util/keyfork-prompt/examples/test-basic-prompt.rs @@ -22,8 +22,8 @@ fn main() -> Result<(), Box> { 3, transport_validator.to_fn(), )?; - assert_eq!(mnemonics[0].entropy().len(), 12); - assert_eq!(mnemonics[1].entropy().len(), 32); + assert_eq!(mnemonics[0].as_bytes().len(), 12); + assert_eq!(mnemonics[1].as_bytes().len(), 32); let mnemonics = mgr.prompt_validated_wordlist( "Enter a 24 and 48-word mnemonic: ", @@ -31,8 +31,8 @@ fn main() -> Result<(), Box> { 3, combine_validator.to_fn(), )?; - assert_eq!(mnemonics[0].entropy().len(), 32); - assert_eq!(mnemonics[1].entropy().len(), 64); + assert_eq!(mnemonics[0].as_bytes().len(), 32); + assert_eq!(mnemonics[1].as_bytes().len(), 64); Ok(()) } -- 2.40.1 From 12095495326ab637999674fb684a5c73e52eb974 Mon Sep 17 00:00:00 2001 From: ryan Date: Mon, 12 Feb 2024 01:28:04 -0500 Subject: [PATCH 4/4] keyforkd: impl test_util::run_test --- Cargo.lock | 22 ++-- crates/daemon/keyforkd-client/src/tests.rs | 125 ++++++++------------- crates/daemon/keyforkd/Cargo.toml | 1 + crates/daemon/keyforkd/src/lib.rs | 2 + crates/daemon/keyforkd/src/test_util.rs | 85 ++++++++++++++ 5 files changed, 144 insertions(+), 91 deletions(-) create mode 100644 crates/daemon/keyforkd/src/test_util.rs diff --git a/Cargo.lock b/Cargo.lock index f830002..66e9d0b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -244,7 +244,7 @@ dependencies = [ "futures-lite 2.2.0", "parking", "polling 3.3.2", - "rustix 0.38.30", + "rustix 0.38.31", "slab", "tracing", "windows-sys 0.52.0", @@ -1610,7 +1610,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0bad00257d07be169d870ab665980b06cdb366d792ad690bf2e76876dc503455" dependencies = [ "hermit-abi", - "rustix 0.38.30", + "rustix 0.38.31", "windows-sys 0.52.0", ] @@ -1870,6 +1870,7 @@ dependencies = [ "keyfork-slip10-test-data", "keyforkd-models", "serde", + "tempfile", "thiserror", "tokio", "tower", @@ -2533,7 +2534,7 @@ dependencies = [ "cfg-if", "concurrent-queue", "pin-project-lite", - "rustix 0.38.30", + "rustix 0.38.31", "tracing", "windows-sys 0.52.0", ] @@ -2806,9 +2807,9 @@ dependencies = [ [[package]] name = "rustix" -version = "0.38.30" +version = "0.38.31" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "322394588aaf33c24007e8bb3238ee3e4c5c09c084ab32bc73890b99ff326bca" +checksum = "6ea3e1a662af26cd7a3ba09c0297a31af215563ecf42817c98df621387f4e949" dependencies = [ "bitflags 2.4.2", "errno", @@ -3184,14 +3185,13 @@ dependencies = [ [[package]] name = "tempfile" -version = "3.9.0" +version = "3.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "01ce4141aa927a6d1bd34a041795abd0db1cccba5d5f24b009f694bdf3a1f3fa" +checksum = "a365e8cd18e44762ef95d87f284f4b5cd04107fec2ff3052bd6a3e6069669e67" dependencies = [ "cfg-if", "fastrand 2.0.1", - "redox_syscall", - "rustix 0.38.30", + "rustix 0.38.31", "windows-sys 0.52.0", ] @@ -3212,7 +3212,7 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "21bebf2b7c9e0a515f6e0f8c51dc0f8e4696391e6f1ff30379559f8365fb0df7" dependencies = [ - "rustix 0.38.30", + "rustix 0.38.31", "windows-sys 0.48.0", ] @@ -3606,7 +3606,7 @@ dependencies = [ "either", "home", "once_cell", - "rustix 0.38.30", + "rustix 0.38.31", ] [[package]] diff --git a/crates/daemon/keyforkd-client/src/tests.rs b/crates/daemon/keyforkd-client/src/tests.rs index d12f528..7dae50c 100644 --- a/crates/daemon/keyforkd-client/src/tests.rs +++ b/crates/daemon/keyforkd-client/src/tests.rs @@ -1,100 +1,65 @@ use crate::Client; +use keyforkd::test_util::{run_test, Infallible}; use keyfork_derive_util::{request::*, DerivationPath}; use keyfork_slip10_test_data::test_data; -use std::sync::mpsc::channel; use std::{os::unix::net::UnixStream, str::FromStr}; -use tokio::runtime::Builder; #[test] -fn secp256k1() { +fn secp256k1_test_suite() { let tests = test_data() .unwrap() .remove(&"secp256k1".to_string()) .unwrap(); - // note: since client is non async, can't be single threaded - let rt = Builder::new_multi_thread().enable_io().build().unwrap(); - let tempdir = tempfile::tempdir().unwrap(); - for (i, per_seed) in tests.into_iter().enumerate() { - let mut socket_name = i.to_string(); - socket_name.push_str("-keyforkd.sock"); - let socket_path = tempdir.path().join(socket_name); - let (tx, rx) = channel(); - let handle = rt.spawn({ - let socket_path = socket_path.clone(); - async move { - let seed = per_seed.seed.clone(); - let mut server = keyforkd::UnixServer::bind(&socket_path).unwrap(); - tx.send(()).unwrap(); - let service = keyforkd::ServiceBuilder::new() - .layer(keyforkd::middleware::BincodeLayer::new()) - .service(keyforkd::Keyforkd::new(seed)); - server.run(service).await.unwrap(); + for seed_test in tests { + let seed = seed_test.seed; + run_test(&seed, move |socket_path| { + for test in seed_test.tests { + let socket = UnixStream::connect(&socket_path).unwrap(); + let mut client = Client::new(socket); + let chain = DerivationPath::from_str(test.chain).unwrap(); + if chain.len() < 2 { + continue; + } + let req = DerivationRequest::new( + DerivationAlgorithm::Secp256k1, + &DerivationPath::from_str(test.chain).unwrap(), + ); + let response = + DerivationResponse::try_from(client.request(&req.into()).unwrap()).unwrap(); + assert_eq!(&response.data, test.private_key.as_slice()); } - }); - rx.recv().unwrap(); - - for test in &per_seed.tests { - let socket = UnixStream::connect(&socket_path).unwrap(); - let mut client = Client::new(socket); - let chain = DerivationPath::from_str(test.chain).unwrap(); - if chain.len() < 2 { - continue; - } - let req = DerivationRequest::new( - DerivationAlgorithm::Secp256k1, - &DerivationPath::from_str(test.chain).unwrap(), - ); - let response = - DerivationResponse::try_from(client.request(&req.into()).unwrap()).unwrap(); - assert_eq!(&response.data, test.private_key.as_slice()); - } - - handle.abort(); + Infallible::Ok(()) + }).unwrap(); } } #[test] -fn ed25519() { - let tests = test_data().unwrap().remove(&"ed25519".to_string()).unwrap(); +fn ed25519_test_suite() { + let tests = test_data() + .unwrap() + .remove(&"ed25519".to_string()) + .unwrap(); - let rt = Builder::new_multi_thread().enable_io().build().unwrap(); - let tempdir = tempfile::tempdir().unwrap(); - for (i, per_seed) in tests.into_iter().enumerate() { - let mut socket_name = i.to_string(); - socket_name.push_str("-keyforkd.sock"); - let socket_path = tempdir.path().join(socket_name); - let (tx, rx) = channel(); - let handle = rt.spawn({ - let socket_path = socket_path.clone(); - async move { - let seed = per_seed.seed.clone(); - let mut server = keyforkd::UnixServer::bind(&socket_path).unwrap(); - tx.send(()).unwrap(); - let service = keyforkd::ServiceBuilder::new() - .layer(keyforkd::middleware::BincodeLayer::new()) - .service(keyforkd::Keyforkd::new(seed)); - server.run(service).await.unwrap(); + for seed_test in tests { + let seed = seed_test.seed; + run_test(&seed, move |socket_path| { + for test in seed_test.tests { + let socket = UnixStream::connect(&socket_path).unwrap(); + let mut client = Client::new(socket); + let chain = DerivationPath::from_str(test.chain).unwrap(); + if chain.len() < 2 { + continue; + } + let req = DerivationRequest::new( + DerivationAlgorithm::Ed25519, + &DerivationPath::from_str(test.chain).unwrap(), + ); + let response = + DerivationResponse::try_from(client.request(&req.into()).unwrap()).unwrap(); + assert_eq!(&response.data, test.private_key.as_slice()); } - }); - rx.recv().unwrap(); - - for test in &per_seed.tests { - let socket = UnixStream::connect(&socket_path).unwrap(); - let mut client = Client::new(socket); - let chain = DerivationPath::from_str(test.chain).unwrap(); - if chain.len() < 2 { - continue; - } - let req = DerivationRequest::new( - DerivationAlgorithm::Ed25519, - &DerivationPath::from_str(test.chain).unwrap(), - ); - let response = - DerivationResponse::try_from(client.request(&req.into()).unwrap()).unwrap(); - assert_eq!(&response.data, test.private_key.as_slice()); - } - - handle.abort(); + Infallible::Ok(()) + }).unwrap(); } } diff --git a/crates/daemon/keyforkd/Cargo.toml b/crates/daemon/keyforkd/Cargo.toml index 158f810..7813fd0 100644 --- a/crates/daemon/keyforkd/Cargo.toml +++ b/crates/daemon/keyforkd/Cargo.toml @@ -31,6 +31,7 @@ tower = { version = "0.4.13", features = ["tokio", "util"] } # Personally audited thiserror = "1.0.47" serde = { version = "1.0.186", features = ["derive"] } +tempfile = { version = "3.10.0", default-features = false } [dev-dependencies] hex-literal = "0.4.1" diff --git a/crates/daemon/keyforkd/src/lib.rs b/crates/daemon/keyforkd/src/lib.rs index 4ea3a45..2fa0b20 100644 --- a/crates/daemon/keyforkd/src/lib.rs +++ b/crates/daemon/keyforkd/src/lib.rs @@ -30,6 +30,8 @@ pub use error::Keyforkd as KeyforkdError; pub use server::UnixServer; pub use service::Keyforkd; +pub mod test_util; + /// Set up a Tracing subscriber, defaulting to debug mode. #[cfg(feature = "tracing")] pub fn setup_registry() { diff --git a/crates/daemon/keyforkd/src/test_util.rs b/crates/daemon/keyforkd/src/test_util.rs new file mode 100644 index 0000000..5e7fdb7 --- /dev/null +++ b/crates/daemon/keyforkd/src/test_util.rs @@ -0,0 +1,85 @@ +//! # Keyforkd Test Utilities +//! +//! This module adds a helper to set up a Tokio runtime, start a Tokio runtime with a given seed, +//! start a Keyfork server on that runtime, and run a given test closure. + +use crate::{middleware, Keyforkd, ServiceBuilder, UnixServer}; + +use tokio::runtime::Builder; + +#[derive(Debug, thiserror::Error)] +#[error("This error can never be instantiated")] +#[doc(hidden)] +pub struct InfallibleError { + protected: (), +} + +/// An infallible result. This type can be used to represent a function that should never error. +/// +/// ```rust +/// use keyforkd::test_util::Infallible; +/// let closure = || { +/// Infallible::Ok(()) +/// }; +/// assert!(closure().is_ok()); +/// ``` +pub type Infallible = std::result::Result; + +/// Run a test making use of a Keyforkd server. The path to the socket of the Keyforkd server is +/// provided as the only argument to the closure. The closure is expected to return a Result; the +/// Error field of the Result may be an error returned by a test. +/// +/// # Panics +/// +/// The function is not expected to run in production; therefore, the function plays "fast and +/// loose" wih the usage of [`Result::expect`]. In normal usage, these should never be an issue. +#[allow(clippy::missing_errors_doc)] +pub fn run_test(seed: &[u8], closure: F) -> Result<(), E> +where + F: FnOnce(&std::path::Path) -> Result<(), E> + Send + 'static, + E: Send + 'static, +{ + let rt = Builder::new_multi_thread() + .worker_threads(2) + .enable_io() + .build() + .expect("tokio threaded IO runtime"); + let socket_dir = tempfile::tempdir().expect("can't create tempdir"); + let socket_path = socket_dir.path().join("keyforkd.sock"); + rt.block_on(async move { + let (tx, mut rx) = tokio::sync::mpsc::channel(1); + let server_handle = tokio::spawn({ + let socket_path = socket_path.clone(); + let seed = seed.to_vec(); + async move { + let mut server = UnixServer::bind(&socket_path).expect("can't bind unix socket"); + tx.send(()).await.expect("couldn't send server start signal"); + let service = ServiceBuilder::new() + .layer(middleware::BincodeLayer::new()) + .service(Keyforkd::new(seed.to_vec())); + server.run(service).await.unwrap(); + } + }); + + rx.recv() + .await + .expect("can't receive server start signal from channel"); + let test_handle = tokio::task::spawn_blocking(move || closure(&socket_path)); + + let result = test_handle.await; + server_handle.abort(); + result + }) + .expect("runtime could not join all threads") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_run_test() { + let seed = b"beefbeef"; + run_test(seed, |_path| Infallible::Ok(())).expect("infallible"); + } +} -- 2.40.1