refactor: move subkey binding creation to packet

hko-s · hko-s · commit 4834bde20181 · 2025-05-26T14:17:13.000+02:00
diff --git a/src/composed/key/public.rs b/src/composed/key/public.rs
@@ -1,17 +1,16 @@
 use std::ops::Deref;
 
-use chrono::SubsecRound;
 use rand::{CryptoRng, Rng};
 
 use crate::{
     composed::{KeyDetails, SignedPublicKey, SignedPublicSubKey},
     crypto::{hash::HashAlgorithm, public_key::PublicKeyAlgorithm},
-    errors::{unsupported_err, Result},
-    packet::{self, KeyFlags, SignatureConfig, SignatureType, Subpacket, SubpacketData},
+    errors::Result,
+    packet::{self, KeyFlags},
     ser::Serialize,
     types::{
-        EskType, Fingerprint, KeyId, KeyVersion, Password, PkeskBytes, PublicKeyTrait,
-        PublicParams, SecretKeyTrait, SignatureBytes,
+        EskType, Fingerprint, KeyId, Password, PkeskBytes, PublicKeyTrait, PublicParams,
+        SecretKeyTrait, SignatureBytes,
     },
 };
 
@@ -106,43 +105,14 @@ impl PublicSubkey {
         P: PublicKeyTrait + Serialize,
     {
         let key = self.key;
-        let hashed_subpackets = vec![
-            Subpacket::regular(SubpacketData::SignatureCreationTime(
-                chrono::Utc::now().trunc_subsecs(0),
-            ))?,
-            Subpacket::regular(SubpacketData::KeyFlags(self.keyflags))?,
-            Subpacket::regular(SubpacketData::IssuerFingerprint(
-                primary_sec_key.fingerprint(),
-            ))?,
-        ];
-
-        let mut config = match primary_sec_key.version() {
-            KeyVersion::V4 => SignatureConfig::v4(
-                SignatureType::SubkeyBinding,
-                primary_sec_key.algorithm(),
-                primary_sec_key.hash_alg(),
-            ),
-            KeyVersion::V6 => SignatureConfig::v6(
-                &mut rng,
-                SignatureType::SubkeyBinding,
-                primary_sec_key.algorithm(),
-                primary_sec_key.hash_alg(),
-            )?,
-            v => unsupported_err!("unsupported key version: {:?}", v),
-        };
-
-        config.hashed_subpackets = hashed_subpackets;
-
-        // If the version of the issuer is greater than 4, this subpacket MUST NOT be included in
-        // the signature.
-        if primary_sec_key.version() <= KeyVersion::V4 {
-            config.unhashed_subpackets = vec![Subpacket::regular(SubpacketData::Issuer(
-                primary_sec_key.key_id(),
-            ))?];
-        }
 
-        let signatures =
-            vec![config.sign_subkey_binding(primary_sec_key, primary_pub_key, key_pw, &key)?];
+        let signatures = vec![key.sign(
+            &mut rng,
+            primary_sec_key,
+            primary_pub_key,
+            key_pw,
+            self.keyflags,
+        )?];
 
         Ok(SignedPublicSubKey { key, signatures })
     }
diff --git a/src/composed/key/secret.rs b/src/composed/key/secret.rs
@@ -1,13 +1,12 @@
 use aes_gcm::aead::rand_core::CryptoRng;
-use chrono::SubsecRound;
 use rand::Rng;
 
 use crate::{
     composed::{KeyDetails, PublicSubkey, SignedSecretKey, SignedSecretSubKey},
-    errors::{unsupported_err, Result},
-    packet::{self, KeyFlags, SignatureConfig, SignatureType, Subpacket, SubpacketData},
+    errors::Result,
+    packet::{self, KeyFlags},
     ser::Serialize,
-    types::{KeyVersion, Password, PublicKeyTrait, SecretKeyTrait},
+    types::{Password, PublicKeyTrait, SecretKeyTrait},
 };
 
 /// User facing interface to work with the components of a "Transferable Secret Key (TSK)"
@@ -90,44 +89,12 @@ impl SecretSubkey {
     {
         let key = self.key;
 
-        let mut config = match primary_sec_key.version() {
-            KeyVersion::V4 => SignatureConfig::v4(
-                SignatureType::SubkeyBinding,
-                primary_sec_key.algorithm(),
-                primary_sec_key.hash_alg(),
-            ),
-            KeyVersion::V6 => SignatureConfig::v6(
-                &mut rng,
-                SignatureType::SubkeyBinding,
-                primary_sec_key.algorithm(),
-                primary_sec_key.hash_alg(),
-            )?,
-            v => unsupported_err!("unsupported key version: {:?}", v),
-        };
-
-        config.hashed_subpackets = vec![
-            Subpacket::regular(SubpacketData::SignatureCreationTime(
-                chrono::Utc::now().trunc_subsecs(0),
-            ))?,
-            Subpacket::regular(SubpacketData::KeyFlags(self.keyflags))?,
-            Subpacket::regular(SubpacketData::IssuerFingerprint(
-                primary_sec_key.fingerprint(),
-            ))?,
-        ];
-
-        // If the version of the issuer is greater than 4, this subpacket MUST NOT be included in
-        // the signature.
-        if primary_sec_key.version() <= KeyVersion::V4 {
-            config.unhashed_subpackets = vec![Subpacket::regular(SubpacketData::Issuer(
-                primary_sec_key.key_id(),
-            ))?];
-        }
-
-        let signatures = vec![config.sign_subkey_binding(
+        let signatures = vec![key.sign(
+            &mut rng,
             primary_sec_key,
             primary_pub_key,
             key_pw,
-            key.public_key(),
+            self.keyflags,
         )?];
 
         Ok(SignedSecretSubKey { key, signatures })
diff --git a/src/packet/key/public.rs b/src/packet/key/public.rs
@@ -1,6 +1,7 @@
 use std::io::BufRead;
 
 use byteorder::{BigEndian, ByteOrder, WriteBytesExt};
+use chrono::SubsecRound;
 use digest::generic_array::GenericArray;
 use md5::Md5;
 use rand::{CryptoRng, Rng};
@@ -15,7 +16,9 @@ use crate::{
         public_key::PublicKeyAlgorithm,
     },
     errors::{bail, ensure, ensure_eq, unimplemented_err, unsupported_err, Result},
-    packet::{PacketHeader, Signature, SignatureConfig, SignatureType, Subpacket, SubpacketData},
+    packet::{
+        KeyFlags, PacketHeader, Signature, SignatureConfig, SignatureType, Subpacket, SubpacketData,
+    },
     ser::Serialize,
     types::{
         EcdhPublicParams, EddsaLegacyPublicParams, EskType, Fingerprint, Imprint, KeyDetails,
@@ -91,18 +94,6 @@ impl PublicKey {
         })
     }
 
-    pub fn sign<R: CryptoRng + Rng, K>(
-        &self,
-        rng: R,
-        key: &K,
-        key_pw: &Password,
-    ) -> Result<Signature>
-    where
-        K: SecretKeyTrait + Serialize,
-    {
-        self.inner.sign(rng, key, key_pw, SignatureType::KeyBinding)
-    }
-
     pub fn encrypt<R: rand::CryptoRng + rand::Rng>(
         &self,
         rng: R,
@@ -167,17 +158,53 @@ impl PublicSubkey {
         })
     }
 
-    pub fn sign<R: CryptoRng + Rng, K>(
+    /// Produce a Subkey Binding Signature (Type ID 0x18), to bind this subkey to a primary key
+    pub fn sign<R: CryptoRng + Rng, K, P>(
         &self,
-        rng: R,
-        key: &K,
+        mut rng: R,
+        primary_sec_key: &K,
+        primary_pub_key: &P,
         key_pw: &Password,
+        keyflags: KeyFlags,
     ) -> Result<Signature>
     where
-        K: SecretKeyTrait + Serialize,
+        K: SecretKeyTrait,
+        P: PublicKeyTrait + Serialize,
     {
-        self.inner
-            .sign(rng, key, key_pw, SignatureType::SubkeyBinding)
+        let mut config = match primary_sec_key.version() {
+            KeyVersion::V4 => SignatureConfig::v4(
+                SignatureType::SubkeyBinding,
+                primary_sec_key.algorithm(),
+                primary_sec_key.hash_alg(),
+            ),
+            KeyVersion::V6 => SignatureConfig::v6(
+                &mut rng,
+                SignatureType::SubkeyBinding,
+                primary_sec_key.algorithm(),
+                primary_sec_key.hash_alg(),
+            )?,
+            v => unsupported_err!("unsupported key version: {:?}", v),
+        };
+
+        config.hashed_subpackets = vec![
+            Subpacket::regular(SubpacketData::SignatureCreationTime(
+                chrono::Utc::now().trunc_subsecs(0),
+            ))?,
+            Subpacket::regular(SubpacketData::KeyFlags(keyflags))?,
+            Subpacket::regular(SubpacketData::IssuerFingerprint(
+                primary_sec_key.fingerprint(),
+            ))?,
+        ];
+
+        // If the version of the issuer is greater than 4, this subpacket MUST NOT be included in
+        // the signature.
+        if primary_sec_key.version() <= KeyVersion::V4 {
+            config.unhashed_subpackets = vec![Subpacket::regular(SubpacketData::Issuer(
+                primary_sec_key.key_id(),
+            ))?];
+        }
+
+        config.sign_subkey_binding(primary_sec_key, primary_pub_key, key_pw, &self)
     }
 
     pub fn encrypt<R: rand::CryptoRng + rand::Rng>(
@@ -364,6 +391,9 @@ impl PubKeyInner {
         sum
     }
 
+    /// Signs a direct key signature or a revocation.
+    #[allow(dead_code)]
+    // TODO: Expose in public API
     fn sign<R: CryptoRng + Rng, K>(
         &self,
         mut rng: R,
diff --git a/src/packet/key/secret.rs b/src/packet/key/secret.rs
@@ -13,7 +13,7 @@ use crate::{
     },
     errors::{bail, ensure_eq, unsupported_err, Result},
     packet::{
-        PacketHeader, PacketTrait, Signature, SignatureConfig, SignatureType, Subpacket,
+        KeyFlags, PacketHeader, PacketTrait, Signature, SignatureConfig, SignatureType, Subpacket,
         SubpacketData,
     },
     ser::Serialize,
@@ -89,20 +89,6 @@ impl SecretKey {
         self.secret_params.has_sha1_checksum()
     }
 
-    pub fn sign<R: CryptoRng + Rng, K, P>(
-        &self,
-        rng: R,
-        key: &K,
-        pub_key: &P,
-        key_pw: &Password,
-    ) -> Result<Signature>
-    where
-        K: SecretKeyTrait,
-        P: PublicKeyTrait + Serialize,
-    {
-        sign(rng, key, key_pw, SignatureType::KeyBinding, pub_key)
-    }
-
     pub fn unlock<G, T>(&self, pw: &Password, work: G) -> Result<Result<T>>
     where
         G: FnOnce(&PublicParams, &PlainSecretParams) -> Result<T>,
@@ -168,20 +154,6 @@ impl SecretSubkey {
         self.secret_params.has_sha1_checksum()
     }
 
-    pub fn sign<R: CryptoRng + Rng, K, P>(
-        &self,
-        rng: R,
-        key: &K,
-        pub_key: &P,
-        key_pw: &Password,
-    ) -> Result<Signature>
-    where
-        K: SecretKeyTrait,
-        P: PublicKeyTrait + Serialize,
-    {
-        sign(rng, key, key_pw, SignatureType::SubkeyBinding, pub_key)
-    }
-
     /// Produce a Primary Key Binding Signature over the primary `pub_key`.
     ///
     /// This signature is used in an embedded signature subpacket to show that the subkey is
@@ -503,6 +475,23 @@ impl SecretSubkey {
         Ok(())
     }
 
+    /// Produce a Subkey Binding Signature (Type ID 0x18), to bind this subkey to a primary key
+    pub fn sign<R: CryptoRng + Rng, K, P>(
+        &self,
+        mut rng: R,
+        primary_sec_key: &K,
+        primary_pub_key: &P,
+        key_pw: &Password,
+        keyflags: KeyFlags,
+    ) -> Result<Signature>
+    where
+        K: SecretKeyTrait,
+        P: PublicKeyTrait + Serialize,
+    {
+        self.details
+            .sign(&mut rng, primary_sec_key, primary_pub_key, key_pw, keyflags)
+    }
+
     pub fn encrypt<R: rand::Rng + rand::CryptoRng>(
         &self,
         rng: R,
@@ -628,6 +617,9 @@ fn create_signature(
     }
 }
 
+/// Signs a direct key signature or a revocation.
+#[allow(dead_code)]
+// TODO: Expose in public API
 fn sign<R: CryptoRng + Rng, K, P>(
     mut rng: R,
     key: &K,
