feat: require key to be at least 128 bits

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"
@@ -1497,9 +1503,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",
@@ -1572,15 +1578,14 @@ dependencies = [
 
 [[package]]
 name = "insta"
-version = "1.34.0"
+version = "1.38.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "5d64600be34b2fcfc267740a243fa7744441bb4947a619ac4e5bb6507f35fbfc"
+checksum = "3eab73f58e59ca6526037208f0e98851159ec1633cf17b6cd2e1f2c3fd5d53cc"
 dependencies = [
  "console",
  "lazy_static",
  "linked-hash-map",
  "similar",
- "yaml-rust",
 ]
 
 [[package]]
@@ -1758,7 +1763,7 @@ dependencies = [
 
 [[package]]
 name = "keyfork-derive-util"
-version = "0.1.0"
+version = "0.1.1"
 dependencies = [
  "digest",
  "ed25519-dalek",
@@ -1776,7 +1781,7 @@ dependencies = [
 
 [[package]]
 name = "keyfork-entropy"
-version = "0.1.0"
+version = "0.1.1"
 dependencies = [
  "keyfork-bug",
  "smex",
@@ -1818,7 +1823,7 @@ dependencies = [
 
 [[package]]
 name = "keyfork-qrcode"
-version = "0.1.0"
+version = "0.1.1"
 dependencies = [
  "image",
  "keyfork-bug",
@@ -1830,10 +1835,11 @@ dependencies = [
 
 [[package]]
 name = "keyfork-shard"
-version = "0.1.0"
+version = "0.2.0"
 dependencies = [
  "aes-gcm",
  "anyhow",
+ "base64 0.22.0",
  "card-backend",
  "card-backend-pcsc",
  "hkdf",
@@ -1879,7 +1885,7 @@ dependencies = [
 
 [[package]]
 name = "keyforkd"
-version = "0.1.0"
+version = "0.1.1"
 dependencies = [
  "bincode",
  "hex-literal",
@@ -2103,9 +2109,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",
@@ -2886,7 +2892,7 @@ dependencies = [
  "aes",
  "aes-gcm",
  "anyhow",
- "base64",
+ "base64 0.21.7",
  "block-padding",
  "blowfish",
  "buffered-reader",
@@ -3811,15 +3817,6 @@ version = "0.8.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "53be06678ed9e83edb1745eb72efc0bbcd7b5c3c35711a860906aed827a13d61"
 
-[[package]]
-name = "yaml-rust"
-version = "0.4.5"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "56c1936c4cc7a1c9ab21a1ebb602eb942ba868cbd44a99cb7cdc5892335e1c85"
-dependencies = [
- "linked-hash-map",
-]
-
 [[package]]
 name = "zerocopy"
 version = "0.7.32"

crates/daemon/keyforkd-client/src/lib.rs

@@ -234,7 +234,7 @@ impl Client {
                 }
 
                 let depth = path.len() as u8;
-                Ok(ExtendedPrivateKey::new_from_parts(
+                Ok(ExtendedPrivateKey::from_parts(
                     &d.data,
                     depth,
                     d.chain_code,

crates/daemon/keyforkd-client/src/tests.rs

@@ -25,6 +25,9 @@ fn secp256k1_test_suite() {
                 if chain_len < 2 {
                     continue;
                 }
+                if chain.iter().take(2).any(|index| !index.is_hardened()) {
+                    continue;
+                }
                 // Consistency check: ensure the server and the client can each derive the same
                 // key using an XPrv, for all but the last XPrv, which is verified after this
                 for i in 2..chain_len {

crates/daemon/keyforkd-models/src/lib.rs

@@ -43,6 +43,10 @@ pub enum DerivationError {
     #[error("Invalid derivation length: Expected at least 2, actual: {0}")]
     InvalidDerivationLength(usize),
 
+    /// The derivation request did not use hardened derivation on the 2 highest indexes.
+    #[error("Invalid derivation paths: expected index #{0} (1) to be hardened")]
+    InvalidDerivationPath(usize, u32),
+
     /// An error occurred while deriving data.
     #[error("Derivation error: {0}")]
     Derivation(String),

crates/daemon/keyforkd/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "keyforkd"
-version = "0.1.0"
+version = "0.1.1"
 edition = "2021"
 license = "AGPL-3.0-only"
 

crates/daemon/keyforkd/src/service.rs

@@ -69,6 +69,18 @@ impl Service<Request> for Keyforkd {
                     return Err(DerivationError::InvalidDerivationLength(len).into());
                 }
 
+                if let Some((i, unhardened_index)) = req
+                    .path()
+                    .iter()
+                    .take(2)
+                    .enumerate()
+                    .find(|(_, index)| {
+                        !index.is_hardened()
+                    })
+                {
+                    return Err(DerivationError::InvalidDerivationPath(i, unhardened_index.inner()).into())
+                }
+
                 #[cfg(feature = "tracing")]
                 if let Some(target) = guess_target(req.path()) {
                     info!("Deriving path: {target}");
@@ -110,6 +122,9 @@ mod tests {
                 if chain.len() < 2 {
                     continue;
                 }
+                if chain.iter().take(2).any(|index| !index.is_hardened()) {
+                    continue;
+                }
                 let req = DerivationRequest::new(DerivationAlgorithm::Secp256k1, &chain);
                 let response: DerivationResponse = keyforkd
                     .ready()

crates/daemon/keyforkd/src/test_util.rs

@@ -61,7 +61,7 @@ where
         ));
     let socket_dir = tempfile::tempdir().expect(bug!("can't create tempdir"));
     let socket_path = socket_dir.path().join("keyforkd.sock");
-    rt.block_on(async move {
+    let result = 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();
@@ -87,8 +87,13 @@ where
         let result = test_handle.await;
         server_handle.abort();
         result
-    })
-    .expect(bug!("runtime could not join all threads"))
+    });
+    if let Err(e) = result {
+        if e.is_panic() {
+            std::panic::resume_unwind(e.into_panic());
+        }
+    }
+    Ok(())
 }
 
 #[cfg(test)]

crates/derive/keyfork-derive-openpgp/src/lib.rs

@@ -59,13 +59,17 @@ pub enum Error {
 #[allow(missing_docs)]
 pub type Result<T, E = Error> = std::result::Result<T, E>;
 
-/// Create an OpenPGP Cert with derived keys from the given derivation response, keys, and User
-/// ID.
+/// Create an OpenPGP Cert with private key data, with derived keys from the given derivation
+/// response, keys, and User ID.
+///
+/// Certificates are created with a default expiration of one day, but may be configured to expire
+/// later using the `KEYFORK_OPENPGP_EXPIRE` environment variable using values such as "15d" (15
+/// days), "1m" (one month), or "2y" (two years).
 ///
 /// # Errors
 /// The function may error for any condition mentioned in [`Error`].
 pub fn derive(xprv: XPrv, keys: &[KeyFlags], userid: &UserID) -> Result<Cert> {
-    let primary_key_flags = match keys.get(0) {
+    let primary_key_flags = match keys.first() {
         Some(kf) if kf.for_certification() => kf,
         _ => return Err(Error::NotCert),
     };
@@ -109,7 +113,7 @@ pub fn derive(xprv: XPrv, keys: &[KeyFlags], userid: &UserID) -> Result<Cert> {
     let cert = cert.insert_packets(vec![Packet::from(userid.clone()), binding.into()])?;
     let policy = sequoia_openpgp::policy::StandardPolicy::new();
 
-    // Set certificate expiration to one day
+    // Set certificate expiration to configured expiration or (default) one day
     let mut keypair = primary_key.clone().into_keypair()?;
     let signatures =
         cert.set_expiration_time(&policy, None, &mut keypair, Some(expiration_date))?;

crates/derive/keyfork-derive-util/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "keyfork-derive-util"
-version = "0.1.0"
+version = "0.1.1"
 edition = "2021"
 license = "MIT"
 

crates/derive/keyfork-derive-util/src/extended_key/private_key.rs

@@ -124,9 +124,9 @@ mod serde_with {
         K: PrivateKey + Clone,
     {
         let variable_len_bytes = <&[u8]>::deserialize(deserializer)?;
-        let bytes: [u8; 32] = variable_len_bytes
-            .try_into()
-            .expect(bug!("unable to parse serialized private key; no support for static len"));
+        let bytes: [u8; 32] = variable_len_bytes.try_into().expect(bug!(
+            "unable to parse serialized private key; no support for static len"
+        ));
         Ok(K::from_bytes(&bytes))
     }
 }
@@ -179,13 +179,20 @@ where
             .into_bytes();
         let (private_key, chain_code) = hash.split_at(KEY_SIZE / 8);
 
-        Self::new_from_parts(
+        assert!(
+            !private_key.iter().all(|byte| *byte == 0),
+            bug!("hmac function returned all-zero master key")
+        );
+
+        Self::from_parts(
             private_key
                 .try_into()
                 .expect(bug!("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(bug!("Invalid chain code length")),
+            chain_code
+                .try_into()
+                .expect(bug!("Invalid chain code length")),
         )
     }
 
@@ -205,9 +212,9 @@ where
     /// #   b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
     /// let chain_code: &[u8; 32] = //
     /// #   b"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB";
-    /// let xprv = ExtendedPrivateKey::<PrivateKey>::new_from_parts(key, 4, *chain_code);
+    /// let xprv = ExtendedPrivateKey::<PrivateKey>::from_parts(key, 4, *chain_code);
     /// ```
-    pub fn new_from_parts(key: &[u8; 32], depth: u8, chain_code: [u8; 32]) -> Self {
+    pub fn from_parts(key: &[u8; 32], depth: u8, chain_code: [u8; 32]) -> Self {
         Self {
             private_key: K::from_bytes(key),
             depth,
@@ -229,7 +236,7 @@ where
     /// #   b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
     /// let chain_code: &[u8; 32] = //
     /// #   b"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB";
-    /// let xprv = ExtendedPrivateKey::<PrivateKey>::new_from_parts(key, 4, *chain_code);
+    /// let xprv = ExtendedPrivateKey::<PrivateKey>::from_parts(key, 4, *chain_code);
     /// assert_eq!(xprv.private_key(), &PrivateKey::from_bytes(key));
     /// ```
     pub fn private_key(&self) -> &K {
@@ -262,7 +269,7 @@ where
     /// # }
     /// ```
     pub fn extended_public_key(&self) -> ExtendedPublicKey<K::PublicKey> {
-        ExtendedPublicKey::new_from_parts(self.public_key(), self.depth, self.chain_code)
+        ExtendedPublicKey::from_parts(self.public_key(), self.depth, self.chain_code)
     }
 
     /// Return a public key for the current [`PrivateKey`].
@@ -301,7 +308,7 @@ where
     /// #   b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
     /// let chain_code: &[u8; 32] = //
     /// #   b"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB";
-    /// let xprv = ExtendedPrivateKey::<PrivateKey>::new_from_parts(key, 4, *chain_code);
+    /// let xprv = ExtendedPrivateKey::<PrivateKey>::from_parts(key, 4, *chain_code);
     /// assert_eq!(xprv.depth(), 4);
     /// ```
     pub fn depth(&self) -> u8 {
@@ -321,7 +328,7 @@ where
     /// #   b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
     /// let chain_code: &[u8; 32] = //
     /// #   b"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB";
-    /// let xprv = ExtendedPrivateKey::<PrivateKey>::new_from_parts(key, 4, *chain_code);
+    /// let xprv = ExtendedPrivateKey::<PrivateKey>::from_parts(key, 4, *chain_code);
     /// assert_eq!(chain_code, &xprv.chain_code());
     /// ```
     pub fn chain_code(&self) -> [u8; 32] {

crates/derive/keyfork-derive-util/src/extended_key/public_key.rs

@@ -11,8 +11,8 @@ const KEY_SIZE: usize = 256;
 /// Errors associated with creating or deriving Extended Public Keys.
 #[derive(Error, Clone, Debug)]
 pub enum Error {
-    /// BIP-0032 does not support deriving public keys from hardened private keys.
-    #[error("Public keys may not be derived when hardened")]
+    /// BIP-0032 does not support hardened public key derivation from parent public keys.
+    #[error("Hardened child public keys may not be derived from parent public keys")]
     HardenedIndex,
 
     /// The maximum depth for key derivation has been reached. The supported maximum depth is 255.
@@ -60,11 +60,11 @@ where
     /// let chain_code: &[u8; 32] = //
     /// #   b"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB";
     /// let pubkey = PublicKey::from_bytes(key);
-    /// let xpub = ExtendedPublicKey::<PublicKey>::new_from_parts(pubkey, 0, *chain_code);
+    /// let xpub = ExtendedPublicKey::<PublicKey>::from_parts(pubkey, 0, *chain_code);
     /// # Ok(())
     /// # }
     /// ```
-    pub fn new_from_parts(public_key: K, depth: u8, chain_code: ChainCode) -> Self {
+    pub fn from_parts(public_key: K, depth: u8, chain_code: ChainCode) -> Self {
         Self {
             public_key,
             depth,
@@ -86,7 +86,7 @@ where
     /// # let chain_code: &[u8; 32] = b"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB";
     /// # let pubkey = PublicKey::from_bytes(key);
     /// let xpub = //
-    /// # ExtendedPublicKey::<PublicKey>::new_from_parts(pubkey, 0, *chain_code);
+    /// # ExtendedPublicKey::<PublicKey>::from_parts(pubkey, 0, *chain_code);
     /// let pubkey = xpub.public_key();
     /// # Ok(())
     /// # }
@@ -121,7 +121,7 @@ where
     /// # let chain_code: &[u8; 32] = b"BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB";
     /// # let pubkey = PublicKey::from_bytes(key);
     /// let xpub = //
-    /// # ExtendedPublicKey::<PublicKey>::new_from_parts(pubkey, 0, *chain_code);
+    /// # ExtendedPublicKey::<PublicKey>::from_parts(pubkey, 0, *chain_code);
     /// let index = DerivationIndex::new(0, false)?;
     /// let child = xpub.derive_child(&index)?;
     /// # Ok(())

crates/derive/keyfork-derive-util/src/private_key.rs

@@ -85,7 +85,7 @@ pub trait PrivateKey: Sized {
     /// # Errors
     ///
     /// An error may be returned if:
-    /// * A nonzero `other` is provided.
+    /// * An all-zero `other` is provided.
     /// * An error specific to the given algorithm was encountered.
     fn derive_child(&self, other: &PrivateKeyBytes) -> Result<Self, Self::Err>;
 
@@ -102,6 +102,10 @@ pub enum PrivateKeyError {
     /// For the given algorithm, the private key must be nonzero.
     #[error("The provided private key must be nonzero, but is not")]
     NonZero,
+
+    /// A scalar could not be constructed for the given algorithm.
+    #[error("A scalar could not be constructed for the given algorithm")]
+    InvalidScalar,
 }
 
 #[cfg(feature = "secp256k1")]
@@ -130,20 +134,19 @@ impl PrivateKey for k256::SecretKey {
     }
 
     fn derive_child(&self, other: &PrivateKeyBytes) -> Result<Self, Self::Err> {
-        if other.iter().all(|n| n == &0) {
-            return Err(PrivateKeyError::NonZero);
-        }
-        let other = *other;
-        // Checked: See above nonzero check
-        let scalar = Option::<NonZeroScalar>::from(NonZeroScalar::from_repr(other.into()))
-            .expect(bug!("Should have been able to get a NonZeroScalar"));
+        use k256::elliptic_curve::ScalarPrimitive;
+        use k256::{Scalar, Secp256k1};
+
+        // Construct a scalar from bytes
+        let scalar = ScalarPrimitive::<Secp256k1>::from_bytes(other.into());
+        let scalar = Option::<ScalarPrimitive<Secp256k1>>::from(scalar);
+        let scalar = scalar.ok_or(PrivateKeyError::InvalidScalar)?;
+        let scalar = Scalar::from(scalar);
 
         let derived_scalar = self.to_nonzero_scalar().as_ref() + scalar.as_ref();
-        Ok(
-            Option::<NonZeroScalar>::from(NonZeroScalar::new(derived_scalar))
-                .map(Into::into)
-                .expect(bug!("Should be able to make Key")),
-        )
+        let nonzero_scalar = Option::<NonZeroScalar>::from(NonZeroScalar::new(derived_scalar))
+            .ok_or(PrivateKeyError::NonZero)?;
+        Ok(Self::from(nonzero_scalar))
     }
 }
 
@@ -180,7 +183,8 @@ impl PrivateKey for ed25519_dalek::SigningKey {
 
 use crate::public_key::TestPublicKey;
 
-#[doc(hidden)]
+/// A private key that can be used for testing purposes. Does not utilize any significant
+/// cryptographic operations.
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub struct TestPrivateKey {
     key: [u8; 32],
@@ -201,9 +205,7 @@ impl PrivateKey for TestPrivateKey {
     type Err = PrivateKeyError;
 
     fn from_bytes(b: &PrivateKeyBytes) -> Self {
-        Self {
-            key: *b
-        }
+        Self { key: *b }
     }
 
     fn to_bytes(&self) -> PrivateKeyBytes {

crates/derive/keyfork-derive-util/src/public_key.rs

@@ -42,7 +42,7 @@ pub trait PublicKey: Sized {
     /// # Errors
     ///
     /// An error may be returned if:
-    /// * A nonzero `other` is provided.
+    /// * An all-zero `other` is provided.
     /// * An error specific to the given algorithm was encountered.
     fn derive_child(&self, other: PrivateKeyBytes) -> Result<Self, Self::Err>;
 
@@ -77,6 +77,10 @@ pub enum PublicKeyError {
     #[error("The provided public key must be nonzero, but is not")]
     NonZero,
 
+    /// A scalar could not be constructed for the given algorithm.
+    #[error("A scalar could not be constructed for the given algorithm")]
+    InvalidScalar,
+
     /// Public key derivation is unsupported for this algorithm.
     #[error("Public key derivation is unsupported for this algorithm")]
     DerivationUnsupported,
@@ -85,7 +89,7 @@ pub enum PublicKeyError {
 #[cfg(feature = "secp256k1")]
 use k256::{
     elliptic_curve::{group::prime::PrimeCurveAffine, sec1::ToEncodedPoint},
-    AffinePoint, NonZeroScalar,
+    AffinePoint,
 };
 
 #[cfg(feature = "secp256k1")]
@@ -105,14 +109,16 @@ impl PublicKey for k256::PublicKey {
     }
 
     fn derive_child(&self, other: PrivateKeyBytes) -> Result<Self, Self::Err> {
-        if other.iter().all(|n| n == &0) {
-            return Err(PublicKeyError::NonZero);
-        }
-        // Checked: See above
-        let scalar = Option::<NonZeroScalar>::from(NonZeroScalar::from_repr(other.into()))
-            .expect(bug!("Should have been able to get a NonZeroScalar"));
+        use k256::elliptic_curve::ScalarPrimitive;
+        use k256::{Secp256k1, Scalar};
 
-        let point = self.to_projective() + (AffinePoint::generator() * *scalar);
+        // Construct a scalar from bytes
+        let scalar = ScalarPrimitive::<Secp256k1>::from_bytes(&other.into());
+        let scalar = Option::<ScalarPrimitive<Secp256k1>>::from(scalar);
+        let scalar = scalar.ok_or(PublicKeyError::InvalidScalar)?;
+        let scalar = Scalar::from(scalar);
+
+        let point = self.to_projective() + (AffinePoint::generator() * scalar);
         Ok(Self::from_affine(point.into())
             .expect(bug!("Could not from_affine after scalar arithmetic")))
     }
@@ -142,14 +148,15 @@ impl PublicKey for VerifyingKey {
     }
 }
 
-#[doc(hidden)]
+/// A public key that can be used for testing purposes. Does not utilize any significant
+/// cryptographic operations.
 #[derive(Clone)]
 pub struct TestPublicKey {
     pub(crate) key: [u8; 33],
 }
 
 impl TestPublicKey {
-    #[doc(hidden)]
+    /// Create a new TestPublicKey from the given bytes.
     #[allow(dead_code)]
     pub fn from_bytes(b: &[u8]) -> Self {
         Self {

crates/derive/keyfork-derive-util/src/request.rs

@@ -57,7 +57,7 @@ pub enum DerivationAlgorithm {
     #[allow(missing_docs)]
     Secp256k1,
     #[doc(hidden)]
-    Internal,
+    TestAlgorithm,
 }
 
 impl DerivationAlgorithm {
@@ -86,7 +86,7 @@ impl DerivationAlgorithm {
                     &derived_key,
                 ))
             }
-            Self::Internal => {
+            Self::TestAlgorithm => {
                 let key = ExtendedPrivateKey::<TestPrivateKey>::new(seed);
                 let derived_key = key.derive_path(path)?;
                 Ok(DerivationResponse::with_algo_and_xprv(
@@ -120,7 +120,7 @@ pub trait AsAlgorithm: PrivateKey {
 
 impl AsAlgorithm for TestPrivateKey {
     fn as_algorithm() -> DerivationAlgorithm {
-        DerivationAlgorithm::Internal
+        DerivationAlgorithm::TestAlgorithm
     }
 }
 
@@ -144,7 +144,7 @@ impl DerivationRequest {
     /// # };
     /// # fn main() -> Result<(), Box<dyn std::error::Error>> {
     /// let algo: DerivationAlgorithm = //
-    /// #   DerivationAlgorithm::Internal;
+    /// #   DerivationAlgorithm::TestAlgorithm;
     /// let path: DerivationPath = //
     /// #   DerivationPath::default();
     /// let request = DerivationRequest::new(algo, &path);
@@ -169,7 +169,7 @@ impl DerivationRequest {
     /// # };
     /// # fn main() -> Result<(), Box<dyn std::error::Error>> {
     /// let algo: DerivationAlgorithm = //
-    /// #   DerivationAlgorithm::Internal;
+    /// #   DerivationAlgorithm::TestAlgorithm;
     /// let path: DerivationPath = //
     /// #   DerivationPath::default();
     /// let request = DerivationRequest::new(algo, &path);
@@ -199,7 +199,7 @@ impl DerivationRequest {
     /// #   b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
     /// #   )?;
     /// let algo: DerivationAlgorithm = //
-    /// #   DerivationAlgorithm::Internal;
+    /// #   DerivationAlgorithm::TestAlgorithm;
     /// let path: DerivationPath = //
     /// #   DerivationPath::default();
     /// let request = DerivationRequest::new(algo, &path);
@@ -228,7 +228,7 @@ impl DerivationRequest {
     /// let seed: &[u8; 64] = //
     /// #   b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
     /// let algo: DerivationAlgorithm = //
-    /// #   DerivationAlgorithm::Internal;
+    /// #   DerivationAlgorithm::TestAlgorithm;
     /// let path: DerivationPath = //
     /// #   DerivationPath::default();
     /// let request = DerivationRequest::new(algo, &path);
@@ -300,7 +300,7 @@ mod secp256k1 {
 
         fn try_from(value: &DerivationResponse) -> Result<Self, Self::Error> {
             match value.algorithm {
-                DerivationAlgorithm::Secp256k1 => Ok(Self::new_from_parts(
+                DerivationAlgorithm::Secp256k1 => Ok(Self::from_parts(
                     &value.data,
                     value.depth,
                     value.chain_code,
@@ -335,7 +335,7 @@ mod ed25519 {
 
         fn try_from(value: &DerivationResponse) -> Result<Self, Self::Error> {
             match value.algorithm {
-                DerivationAlgorithm::Ed25519 => Ok(Self::new_from_parts(
+                DerivationAlgorithm::Ed25519 => Ok(Self::from_parts(
                     &value.data,
                     value.depth,
                     value.chain_code,

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"
 
@@ -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"

crates/keyfork-shard/src/lib.rs

@@ -7,22 +7,28 @@ use std::{
 };
 
 use aes_gcm::{
-    aead::{consts::U12, Aead, AeadCore, OsRng},
+    aead::{consts::U12, Aead},
     Aes256Gcm, KeyInit, Nonce,
 };
+use base64::prelude::{Engine, BASE64_STANDARD};
 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;
 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;
@@ -194,7 +200,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
@@ -204,12 +209,13 @@ 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)?;
-                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 = BASE64_STANDARD
+                    .decode(qrcode_content)
+                    .expect(bug!("qrcode should contain base64 encoded data"));
+                pubkey_data = Some(decoded_data.try_into().map_err(|_| InvalidData)?)
             } else {
                 prompt
                     .lock()
@@ -219,43 +225,43 @@ 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::<English, _>(
                         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)?
             }
         };
 
         // create our shared key
         let our_key = EphemeralSecret::random();
         let our_pubkey_mnemonic = Mnemonic::from_bytes(PublicKey::from(&our_key).as_bytes())?;
-        let shared_secret = our_key
-            .diffie_hellman(&PublicKey::from(their_pubkey))
-            .to_bytes();
-        let hkdf = Hkdf::<Sha256>::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 shared_secret = our_key.diffie_hellman(&PublicKey::from(their_pubkey));
+        assert!(
+            shared_secret.was_contributory(),
+            bug!("shared secret might be insecure")
+        );
+        let hkdf = Hkdf::<Sha256>::new(None, shared_secret.as_bytes());
+
+        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::<U12>::from_slice(&nonce_data);
 
         // decrypt a single shard and create the payload
         let (share, threshold) =
@@ -264,49 +270,47 @@ 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 nonce = Nonce::<U12>::from_slice(&nonce);
-        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");
         }
+
+        // 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)]
-        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"
+        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,
+            bug!("encrypted bytes size != expected len"),
         );
-        out_bytes[..payload_bytes.len()].clone_from_slice(&payload_bytes);
+        let mut mnemonic_bytes = [0u8; ENCRYPTED_LENGTH as usize];
+        mnemonic_bytes.copy_from_slice(&encrypted_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.
-        #[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;
-        }
-
-        // safety: size of out_bytes is constant and always % 4 == 0
-        let payload_mnemonic = unsafe { Mnemonic::from_raw_bytes(&out_bytes) };
+        let payload_mnemonic = Mnemonic::from_nonstandard_bytes(mnemonic_bytes);
 
         #[cfg(feature = "qrcode")]
         {
             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))
@@ -401,7 +405,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.";
@@ -432,22 +436,22 @@ pub fn remote_decrypt(w: &mut impl Write) -> Result<(), Box<dyn std::error::Erro
 
     while iter_count.is_none() || iter_count.is_some_and(|i| i > 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());
-            if let Ok(qrcode) = qrencode(&smex::encode(&qrcode_data), ErrorCorrection::Highest) {
+            let qrcode_data = key_mnemonic.to_bytes();
+            if let Ok(qrcode) = qrencode(
+                &BASE64_STANDARD.encode(qrcode_data),
+                ErrorCorrection::Highest,
+            ) {
                 pm.prompt_message(PromptMessage::Text(format!(
                     concat!(
-                        "A QR code will be displayed after this prompt. ",
-                        "Send the QR code to only shardholder {iter}. ",
-                        "Nobody else should scan this QR code."
+                        "QR code #{iter} will be displayed after this prompt. ",
+                        "Send the QR code to the next shardholder. ",
+                        "Only the next shardholder should scan the QR code."
                     ),
                     iter = iter
                 )))?;
@@ -457,11 +461,9 @@ pub fn remote_decrypt(w: &mut impl Write) -> Result<(), Box<dyn std::error::Erro
 
         pm.prompt_message(PromptMessage::Text(format!(
             concat!(
-                "Upon request, these words should be sent to shardholder {iter}: ",
-                "{nonce_mnemonic} {key_mnemonic}"
+                "Upon request, these words should be sent to the shardholder: ",
+                "{key_mnemonic}"
             ),
-            iter = iter,
-            nonce_mnemonic = nonce_mnemonic,
             key_mnemonic = key_mnemonic,
         )))?;
 
@@ -471,10 +473,17 @@ pub fn remote_decrypt(w: &mut impl Write) -> Result<(), Box<dyn std::error::Erro
         #[cfg(feature = "qrcode")]
         {
             pm.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(QRCODE_TIMEOUT), 0)
             {
-                let decoded_data = smex::decode(&hex)?;
+                let decoded_data = BASE64_STANDARD
+                    .decode(qrcode_content)
+                    .expect(bug!("qrcode should contain base64 encoded data"));
+                assert_eq!(
+                    decoded_data.len(),
+                    ENCRYPTED_LENGTH as usize,
+                    bug!("invalid payload data")
+                );
                 let _ = pubkey_data.insert(decoded_data[..32].try_into().map_err(|_| InvalidData)?);
                 let _ = payload_data.insert(decoded_data[32..].to_vec());
             } else {
@@ -486,7 +495,7 @@ pub fn remote_decrypt(w: &mut impl Write) -> Result<(), Box<dyn std::error::Erro
             (Some(pubkey), Some(payload)) => (pubkey, payload),
             _ => {
                 let validator = MnemonicSetValidator {
-                    word_lengths: [24, 48],
+                    word_lengths: [24, 39],
                 };
 
                 let [pubkey_mnemonic, payload_mnemonic] = pm
@@ -504,14 +513,28 @@ pub fn remote_decrypt(w: &mut impl Write) -> Result<(), Box<dyn std::error::Erro
             }
         };
 
-        let shared_secret = our_key.diffie_hellman(&PublicKey::from(pubkey)).to_bytes();
-        let hkdf = Hkdf::<Sha256>::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)?;
+        assert_eq!(
+            payload.len(),
+            ENCRYPTED_LENGTH as usize,
+            bug!("invalid payload data")
+        );
 
-        let payload =
-            shared_key.decrypt(&nonce, &payload[..payload[payload.len() - 1] as usize])?;
+        let shared_secret = our_key.diffie_hellman(&PublicKey::from(pubkey));
+        assert!(
+            shared_secret.was_contributory(),
+            bug!("shared secret might be insecure")
+        );
+        let hkdf = Hkdf::<Sha256>::new(None, shared_secret.as_bytes());
+
+        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::<U12>::from_slice(&nonce_data);
+
+        let payload = shared_key.decrypt(nonce, payload.as_slice())?;
         assert_eq!(HUNK_VERSION, payload[0], "Incompatible hunk version");
 
         match &mut iter_count {
@@ -526,7 +549,8 @@ pub fn remote_decrypt(w: &mut impl Write) -> Result<(), Box<dyn std::error::Erro
             }
         }
 
-        shares.push(payload[HUNK_OFFSET..].to_vec());
+        let payload_len = payload.last().expect(bug!("payload should not be empty"));
+        shares.push(payload[HUNK_OFFSET..usize::from(*payload_len)].to_vec());
     }
 
     let shares = shares

crates/keyfork-shard/src/openpgp.rs

@@ -194,33 +194,52 @@ impl<P: PromptHandler> OpenPGP<P> {
 }
 
 impl<P: PromptHandler> OpenPGP<P> {
-    /// Read all OpenPGP certificates in a path and return a [`Vec`] of them. Certificates are read
-    /// from a file, or from files one level deep in a directory.
+    /// Read all OpenPGP certificates in a path and return a [`Vec`] of them.
+    ///
+    /// Certificates are read from a file, or from files one level deep in a directory.
+    /// Certificates with duplicated fingerprints will be discarded.
     ///
     /// # Errors
-    /// The function may return an error if it is unable to read the directory or if Sequoia is unable
-    /// to load certificates from the file.
+    /// The function may return an error if it is unable to read the directory or if Sequoia is
+    /// unable to load certificates from the file.
     pub fn discover_certs(path: impl AsRef<Path>) -> Result<Vec<Cert>> {
         let path = path.as_ref();
 
+        let mut pubkeys = std::collections::HashSet::new();
+        let mut certs = HashMap::new();
         if path.is_file() {
-            let mut vec = vec![];
-            for cert in CertParser::from_file(path).map_err(Error::Sequoia)? {
-                vec.push(cert.map_err(Error::Sequoia)?);
+            for maybe_cert in CertParser::from_file(path).map_err(Error::Sequoia)? {
+                let cert = maybe_cert.map_err(Error::Sequoia)?;
+                let certfp = cert.fingerprint();
+                for key in cert.keys() {
+                    let fp = key.fingerprint();
+                    if pubkeys.contains(&fp) {
+                        eprintln!("Received duplicate key: {fp} in public key: {certfp}");
+                    }
+                    pubkeys.insert(fp);
+                }
+                certs.insert(certfp, cert);
             }
-            Ok(vec)
         } else {
-            let mut vec = vec![];
             for entry in path
                 .read_dir()
                 .map_err(Error::Io)?
                 .filter_map(Result::ok)
                 .filter(|p| p.path().is_file())
             {
-                vec.push(Cert::from_file(entry.path()).map_err(Error::Sequoia)?);
+                let cert = Cert::from_file(entry.path()).map_err(Error::Sequoia)?;
+                let certfp = cert.fingerprint();
+                for key in cert.keys() {
+                    let fp = key.fingerprint();
+                    if pubkeys.contains(&fp) {
+                        eprintln!("Received duplicate key: {fp} in public key: {certfp}");
                     }
-            Ok(vec)
+                    pubkeys.insert(fp);
                 }
+                certs.insert(certfp, cert);
+            }
+        }
+        Ok(certs.into_values().collect())
     }
 }
 

crates/keyfork-shard/src/openpgp/keyring.rs

@@ -84,13 +84,23 @@ impl<P: PromptHandler> VerificationHelper for &mut Keyring<P> {
                     aead_algo,
                 } => {}
                 MessageLayer::SignatureGroup { results } => {
-                    for result in results {
-                        if let Err(e) = result {
+                    match &results[..] {
+                        [Ok(_)] => {
+                            return Ok(());
+                        }
+                        _ => {
                             // FIXME: anyhow leak: VerificationError impl std::error::Error
                             // return Err(e.context("Invalid signature"));
+                            return Err(anyhow::anyhow!("Error validating signature; either multiple signatures were passed or the single signature was not valid"));
+                        }
+                    }
+                    /*
+                    for result in results {
+                        if let Err(e) = result {
                             return Err(anyhow::anyhow!("Invalid signature: {e}"));
                         }
                     }
+                    */
                 }
             }
         }

crates/keyfork-shard/src/openpgp/smartcard.rs

@@ -193,12 +193,23 @@ impl<P: PromptHandler> VerificationHelper for &mut SmartcardManager<P> {
                     aead_algo,
                 } => {}
                 MessageLayer::SignatureGroup { results } => {
+                    match &results[..] {
+                        [Ok(_)] => {
+                            return Ok(());
+                        }
+                        _ => {
+                            // FIXME: anyhow leak: VerificationError impl std::error::Error
+                            // return Err(e.context("Invalid signature"));
+                            return Err(anyhow::anyhow!("Error validating signature; either multiple signatures were passed or the single signature was not valid"));
+                        }
+                    }
+                    /*
                     for result in results {
                         if let Err(e) = result {
-                            // FIXME: anyhow leak
-                            return Err(anyhow::anyhow!("Verification error: {}", e.to_string()));
+                            return Err(anyhow::anyhow!("Invalid signature: {e}"));
                         }
                     }
+                    */
                 }
             }
         }
@@ -264,8 +275,8 @@ impl<P: PromptHandler> DecryptionHelper for &mut SmartcardManager<P> {
             } else {
                 format!("Unlock card {card_id} ({cardholder_name})\n{rpea}: {attempts}\n\nPIN: ")
             };
-            let temp_pin =
-                self.pm
+            let temp_pin = self
+                .pm
                 .lock()
                 .expect(bug!(POISONED_MUTEX))
                 .prompt_validated_passphrase(&message, 3, &pin_validator)?;

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"] }

crates/keyfork/src/cli/derive.rs

@@ -20,8 +20,12 @@ pub enum DeriveSubcommands {
     /// Derive an OpenPGP Transferable Secret Key (private key). The key is encoded using OpenPGP
     /// ASCII Armor, a format usable by most programs using OpenPGP.
     ///
-    /// The key is generated with a 24-hour expiration time. The operation to set the expiration
-    /// time to a higher value is left to the user to ensure the key is usable by the user.
+    /// Certificates are created with a default expiration of one day, but may be configured to
+    /// expire later using the `KEYFORK_OPENPGP_EXPIRE` environment variable using values such as
+    /// "15d" (15 days), "1m" (one month), or "2y" (two years).
+    ///
+    /// It is recommended to use the default expiration of one day and to change the expiration
+    /// using an external utility, to ensure the Certify key is usable.
     #[command(name = "openpgp")]
     OpenPGP {
         /// Default User ID for the certificate, using the OpenPGP User ID format.

crates/keyfork/src/cli/wizard.rs

@@ -11,7 +11,7 @@ use keyfork_derive_openpgp::{
 };
 use keyfork_derive_util::{DerivationIndex, DerivationPath};
 use keyfork_prompt::{
-    validators::{PinValidator, Validator},
+    validators::{SecurePinValidator, Validator},
     Message, PromptHandler, DefaultTerminal, default_terminal
 };
 
@@ -38,7 +38,7 @@ fn derive_key(seed: [u8; 32], index: u8) -> Result<Cert> {
     let chain = DerivationIndex::new(u32::from_be_bytes(pgp_u32), true)?;
     let mut shrd_u32 = [0u8; 4];
     shrd_u32[..].copy_from_slice(&"shrd".bytes().collect::<Vec<u8>>());
-    let account = DerivationIndex::new(u32::from_be_bytes(pgp_u32), true)?;
+    let account = DerivationIndex::new(u32::from_be_bytes(shrd_u32), true)?;
     let subkey = DerivationIndex::new(u32::from(index), true)?;
     let path = DerivationPath::default()
         .chain_push(chain)
@@ -116,12 +116,12 @@ fn generate_shard_secret(
         );
     }
 
-    let user_pin_validator = PinValidator {
+    let user_pin_validator = SecurePinValidator {
         min_length: Some(6),
         ..Default::default()
     }
     .to_fn();
-    let admin_pin_validator = PinValidator {
+    let admin_pin_validator = SecurePinValidator {
         min_length: Some(8),
         ..Default::default()
     }
@@ -132,8 +132,8 @@ fn generate_shard_secret(
         for i in 0..keys_per_shard {
             pm.prompt_message(Message::Text(format!(
                 "Please remove all keys and insert key #{} for user #{}",
-                i + 1,
-                index + 1,
+                (i as u16) + 1,
+                (index as u16) + 1,
             )))?;
             let card_backend = loop {
                 if let Some(c) = PcscBackend::cards(None)?.next().transpose()? {

crates/qrcode/keyfork-qrcode/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "keyfork-qrcode"
-version = "0.1.0"
+version = "0.1.1"
 repository = "https://git.distrust.co/public/keyfork"
 edition = "2021"
 license = "MIT"

crates/qrcode/keyfork-qrcode/src/lib.rs

@@ -5,7 +5,7 @@ use keyfork_bug as bug;
 use image::io::Reader as ImageReader;
 use std::{
     io::{Cursor, Write},
-    time::{Duration, SystemTime},
+    time::{Duration, Instant},
     process::{Command, Stdio},
 };
 use v4l::{
@@ -110,11 +110,10 @@ pub fn scan_camera(timeout: Duration, index: usize) -> Result<Option<String>, QR
     fmt.fourcc = FourCC::new(b"MPG1");
     device.set_format(&fmt)?;
     let mut stream = Stream::with_buffers(&device, Type::VideoCapture, 4)?;
-    let start = SystemTime::now();
+    let start = Instant::now();
 
-    while SystemTime::now()
+    while Instant::now()
         .duration_since(start)
-        .unwrap_or(Duration::from_secs(0))
         < timeout
     {
         let (buffer, _) = stream.next()?;
@@ -141,12 +140,11 @@ pub fn scan_camera(timeout: Duration, index: usize) -> Result<Option<String>, QR
     fmt.fourcc = FourCC::new(b"MPG1");
     device.set_format(&fmt)?;
     let mut stream = Stream::with_buffers(&device, Type::VideoCapture, 4)?;
-    let start = SystemTime::now();
+    let start = Instant::now();
     let mut scanner = keyfork_zbar::image_scanner::ImageScanner::new();
 
-    while SystemTime::now()
+    while Instant::now()
         .duration_since(start)
-        .unwrap_or(Duration::from_secs(0))
         < timeout
     {
         let (buffer, _) = stream.next()?;

crates/util/keyfork-entropy/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "keyfork-entropy"
-version = "0.1.0"
+version = "0.1.1"
 edition = "2021"
 license = "MIT"
 

crates/util/keyfork-entropy/src/main.rs

@@ -10,14 +10,16 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
         bit_size % 8 == 0,
         "Bit size must be divisible by 8, got: {bit_size}"
     );
-    assert!(
-        bit_size <= 256,
-        "Maximum supported bit size is 256, got: {bit_size}"
-    );
     assert!(
         bit_size >= 128,
         "Minimum supported bit size is 128, got {bit_size}"
     );
+    match bit_size {
+        128 | 256 | 512 => {}
+        _ => {
+            eprintln!("reading entropy of uncommon size: {bit_size}");
+        }
+    }
 
     let entropy = keyfork_entropy::generate_entropy_of_size(bit_size / 8)?;
     println!("{}", smex::encode(entropy));

crates/util/keyfork-mnemonic-util/src/bin/keyfork-mnemonic-from-seed.rs

@@ -8,7 +8,7 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
     input.read_line(&mut line)?;
     let decoded = smex::decode(line.trim())?;
 
-    let mnemonic = unsafe { Mnemonic::from_raw_bytes(&decoded) };
+    let mnemonic = Mnemonic::from_raw_bytes(&decoded) ;
 
     println!("{mnemonic}");
 

crates/util/keyfork-mnemonic-util/src/lib.rs

@@ -125,6 +125,13 @@ impl Wordlist for English {
     }
 }
 
+struct AssertValidMnemonicSize<const N: usize>;
+
+impl<const N: usize> AssertValidMnemonicSize<N> {
+    const OK_CHUNKS: () = assert!(N % 4 == 0, "bytes must be a length divisible by 4");
+    const OK_SIZE: () = assert!(N <= 1024, "bytes must be less-or-equal 1024");
+}
+
 /// A BIP-0039 mnemonic with reference to a [`Wordlist`].
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct MnemonicBase<W: Wordlist> {
@@ -276,7 +283,36 @@ where
             return Err(MnemonicGenerationError::InvalidByteLength(bit_count));
         }
 
-        Ok(unsafe { Self::from_raw_bytes(bytes) })
+        Ok( Self::from_raw_bytes(bytes) )
+    }
+
+    /// Generate a [`Mnemonic`] from the provided data and [`Wordlist`]. The data may be of a size
+    /// of a factor of 4, up to 1024 bytes.
+    ///
+    /// ```rust
+    /// use keyfork_mnemonic_util::Mnemonic;
+    /// let data = b"hello world!";
+    /// let mnemonic = Mnemonic::from_nonstandard_bytes(*data);
+    /// ```
+    ///
+    /// If an invalid size is requested, the code will fail to compile:
+    ///
+    /// ```rust,compile_fail
+    /// use keyfork_mnemonic_util::Mnemonic;
+    /// let mnemonic = Mnemonic::from_nonstandard_bytes([0u8; 53]);
+    /// ```
+    ///
+    /// ```rust,compile_fail
+    /// use keyfork_mnemonic_util::Mnemonic;
+    /// let mnemonic = Mnemonic::from_nonstandard_bytes([0u8; 1024 + 4]);
+    /// ```
+    pub fn from_nonstandard_bytes<const N: usize>(bytes: [u8; N]) -> MnemonicBase<W> {
+        #[allow(clippy::let_unit_value)]
+        {
+            let () = AssertValidMnemonicSize::<N>::OK_CHUNKS;
+            let () = AssertValidMnemonicSize::<N>::OK_SIZE;
+        }
+        Self::from_raw_bytes(&bytes)
     }
 
     /// Generate a [`Mnemonic`] from the provided data and [`Wordlist`]. The data is expected to be
@@ -292,11 +328,12 @@ where
     /// Create a Mnemonic using an arbitrary length of given data. The length does not need to
     /// conform to BIP-0039 standards, but should be a multiple of 32 bits or 4 bytes.
     ///
-    /// # Safety
-    ///
+    /// # Panics
     /// 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`. If the assumption is incorrect, code may panic.
+    /// == 0`. If the assumption is incorrect, code may panic. The
+    /// [`MnemonicBase::from_nonstandard_bytes`] function may be used to generate entropy if the
+    /// length of the data is known at compile-time.
     ///
     /// # Examples
     /// ```rust
@@ -315,11 +352,10 @@ where
     /// // NOTE: Data is of invalid length, 31
     /// let data = b"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
     /// let mnemonic = unsafe { Mnemonic::from_raw_bytes(data.as_slice()) };
-    /// let mnemonic_text = mnemonic.to_string();
-    /// // NOTE: panic happens here
-    /// let new_mnemonic = Mnemonic::from_str(&mnemonic_text).unwrap();
     /// ```
-    pub unsafe fn from_raw_bytes(bytes: &[u8]) -> MnemonicBase<W> {
+    pub fn from_raw_bytes(bytes: &[u8]) -> MnemonicBase<W> {
+        assert!(bytes.len() % 4 == 0);
+        assert!(bytes.len() <= 1024);
         MnemonicBase {
             data: bytes.to_vec(),
             marker: PhantomData,
@@ -520,12 +556,30 @@ mod tests {
     }
 
     #[test]
-    fn can_do_up_to_1024_bits() {
-        let entropy = &mut [0u8; 128];
+    fn can_do_up_to_8192_bits() {
+        let mut entropy = [0u8; 1024];
         let mut random = std::fs::File::open("/dev/urandom").unwrap();
         random.read_exact(&mut entropy[..]).unwrap();
-        let mnemonic = unsafe { Mnemonic::from_raw_bytes(&entropy[..]) };
+        let mnemonic = Mnemonic::from_nonstandard_bytes(entropy);
         let words = mnemonic.words();
-        assert!(words.len() == 96);
+        assert_eq!(words.len(), 768);
+    }
+
+    #[test]
+    #[should_panic]
+    fn fails_over_8192_bits() {
+        let entropy = &mut [0u8; 1024 + 4];
+        let mut random = std::fs::File::open("/dev/urandom").unwrap();
+        random.read_exact(&mut entropy[..]).unwrap();
+        let _mnemonic = Mnemonic::from_raw_bytes(&entropy[..]);
+    }
+
+    #[test]
+    #[should_panic]
+    fn fails_over_invalid_size() {
+        let entropy = &mut [0u8; 255];
+        let mut random = std::fs::File::open("/dev/urandom").unwrap();
+        random.read_exact(&mut entropy[..]).unwrap();
+        let _mnemonic = Mnemonic::from_raw_bytes(&entropy[..]);
     }
 }

crates/util/keyfork-prompt/src/validators.rs

@@ -29,6 +29,84 @@ pub enum PinError {
     /// The PIN contained invalid characters.
     #[error("PIN contained invalid characters (found {0} at position {1})")]
     InvalidCharacters(char, usize),
+
+    /// The provided PIN had either too many repeated characters or too many sequential characters.
+    #[error("PIN contained too many repeated or sequential characters")]
+    InsecurePIN,
+}
+
+/// Validate that a PIN is of a certain length, matches a range of characters, and does not use
+/// incrementing or decrementing sequences of characters.
+///
+/// The validator determines a score for a passphrase and, if the score is high enough, returns an
+/// error.
+///
+/// Score is calculated based on:
+/// * how many sequential characters are in the passphrase (ascending or descending)
+/// * how many repeated characters are in the passphrase
+#[derive(Default, Clone)]
+pub struct SecurePinValidator {
+    /// The minimum length of provided PINs.
+    pub min_length: Option<usize>,
+
+    /// The maximum length of provided PINs.
+    pub max_length: Option<usize>,
+
+    /// The characters allowed by the PIN parser.
+    pub range: Option<RangeInclusive<char>>,
+
+    /// Whether repeated characters count against the PIN.
+    pub ignore_repeated_characters: bool,
+
+    /// Whether sequential characters count against the PIN.
+    pub ignore_sequential_characters: bool,
+}
+
+impl Validator for SecurePinValidator {
+    type Output = String;
+    type Error = PinError;
+
+    fn to_fn(&self) -> Box<dyn Fn(String) -> Result<String, Box<dyn std::error::Error>>> {
+        let min_len = self.min_length.unwrap_or(usize::MIN);
+        let max_len = self.max_length.unwrap_or(usize::MAX);
+        let range = self.range.clone().unwrap_or('0'..='9');
+        let ignore_repeated_characters = self.ignore_repeated_characters;
+        let ignore_sequential_characters = self.ignore_sequential_characters;
+        Box::new(move |mut s: String| {
+            s.truncate(s.trim_end().len());
+            let len = s.len();
+            if len < min_len {
+                return Err(Box::new(PinError::TooShort(len, min_len)));
+            }
+            if len > max_len {
+                return Err(Box::new(PinError::TooLong(len, max_len)));
+            }
+            let mut last_char = 0;
+            let mut score = 0;
+            for (index, ch) in s.chars().enumerate() {
+                if !range.contains(&ch) {
+                    return Err(Box::new(PinError::InvalidCharacters(ch, index)));
+                }
+                if [-1, 1].contains(&(ch as i32 - last_char))
+                    && !ignore_sequential_characters
+                {
+                    score += 1;
+                }
+                last_char = ch as i32;
+            }
+            let mut chars = s.chars().collect::<Vec<_>>();
+            chars.sort();
+            chars.dedup();
+            if !ignore_repeated_characters {
+                // SAFETY: the amount of characters can't have _increased_ since deduping
+                score += s.chars().count() - chars.len();
+            }
+            if score * 2 > s.chars().count() {
+                return Err(Box::new(PinError::InsecurePIN))
+            }
+            Ok(s)
+        })
+    }
 }
 
 /// Validate that a PIN is of a certain length and matches a range of characters.
@@ -79,8 +157,8 @@ pub mod mnemonic {
 
     use super::Validator;
 
-    use keyfork_mnemonic_util::{Mnemonic, MnemonicFromStrError};
     use keyfork_bug::bug;
+    use keyfork_mnemonic_util::{Mnemonic, MnemonicFromStrError};
 
     /// A mnemonic could not be validated from the given input.
     #[derive(thiserror::Error, Debug)]

docs/src/SUMMARY.md

@@ -33,3 +33,4 @@
   - [Provisioners](./dev-guide/provisioners.md)
 - [Auditing Dependencies](./dev-guide/auditing.md)
 - [Entropy Guide](./dev-guide/entropy.md)
+- [The Shard Protocol](./dev-guide/shard-protocol.md)

docs/src/dev-guide/shard-protocol.md

@@ -0,0 +1,39 @@
+# The Shard Protocol
+
+Keyfork Shard uses a single-handshake protocol to transfer encrypted shards.
+The initial payload is generated by the program combining the shards, while the
+response is generated by the program transport-encrypting the shards.
+
+## Combiner Payload
+
+The combiner payload consists of a 12-byte nonce and a 32-byte x25519 public
+key. The payload is then either encoded to hex and displayed as a QR code, and
+encoded as a mnemonic and printed on-screen.
+
+```
+[12-byte nonce | 32-byte public key]
+```
+
+The transporter receives the 12-byte nonce and 32-byte x25519 key and generates
+their own x25519 key. Using HKDF-Sha256 with no salt on the resulting key
+generates the AES-256-GCM key used to encrypt the now-decrypted shard, along
+with the received nonce.
+
+## Transporter Payload
+
+The transporter payload consists of a 32-byte x25519 public key and a
+64-byte-padded encrypted "hunk". The hunk contains a version byte, a threshold
+byte, and the encrypted shard. The last byte of the 64-byte sequence is the
+total length of the encrypted hunk.
+
+```
+Handshake:
+[32-byte public key | 63-byte-padded encrypted hunk | 1-byte hunk length ]
+
+Hunk:
+[1-byte version | 1-byte threshold | variable-length shard ]
+```
+
+The combiner receives the 32-byte x25519 key and the 64-byte hunk, and uses the
+same key derivation scheme as above to generate the decryption key. The
+threshold byte is used to determine how many shares (in total) are needed.