From 5c0cd1775fadc46e68a24a5af3d1f06803d69c4b Mon Sep 17 00:00:00 2001 From: Azul Date: Sat, 28 Nov 2020 10:20:37 +0100 Subject: openpgp: take ownership of KeyFlags bitfield - Make `generate_key` polymorphic over `AsRef`. - Since `set_key_flags` requires ownership of the key flags, it should take ownership rather than borrowing and cloning the them. See https://rust-lang.github.io/api-guidelines/flexibility.html#caller-decides-where-to-copy-and-place-data-c-caller-control . - See #616. --- openpgp/src/cert.rs | 8 ++++---- openpgp/src/cert/bindings.rs | 2 +- openpgp/src/cert/builder.rs | 12 +++++++----- openpgp/src/packet/signature.rs | 4 ++-- openpgp/src/packet/signature/subpacket.rs | 10 +++++----- openpgp/src/policy.rs | 4 ++-- openpgp/src/serialize/cert.rs | 2 +- openpgp/src/types/key_flags.rs | 6 ++++++ sqv/tests/revoked-key.rs | 8 ++++---- 9 files changed, 32 insertions(+), 24 deletions(-) diff --git a/openpgp/src/cert.rs b/openpgp/src/cert.rs index 1202ae5f..2e25ad62 100644 --- a/openpgp/src/cert.rs +++ b/openpgp/src/cert.rs @@ -4434,7 +4434,7 @@ mod test { let (bind1, rev1, bind2, rev2) = { let bind1 = signature::SignatureBuilder::new(SignatureType::DirectKey) .set_features(&Features::sequoia()).unwrap() - .set_key_flags(&KeyFlags::empty()).unwrap() + .set_key_flags(KeyFlags::empty()).unwrap() .set_signature_creation_time(t1).unwrap() .set_key_validity_period(Some(time::Duration::new(10 * 52 * 7 * 24 * 60 * 60, 0))).unwrap() .set_preferred_hash_algorithms(vec![HashAlgorithm::SHA512]).unwrap() @@ -4448,7 +4448,7 @@ mod test { let bind2 = signature::SignatureBuilder::new(SignatureType::DirectKey) .set_features(&Features::sequoia()).unwrap() - .set_key_flags(&KeyFlags::empty()).unwrap() + .set_key_flags(KeyFlags::empty()).unwrap() .set_signature_creation_time(t3).unwrap() .set_key_validity_period(Some(time::Duration::new(10 * 52 * 7 * 24 * 60 * 60, 0))).unwrap() .set_preferred_hash_algorithms(vec![HashAlgorithm::SHA512]).unwrap() @@ -5117,7 +5117,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= for i in 0..N { let binding = signature::SignatureBuilder::new(SignatureType::DirectKey) .set_features(&Features::sequoia()).unwrap() - .set_key_flags(&KeyFlags::empty()).unwrap() + .set_key_flags(KeyFlags::empty()).unwrap() .set_signature_creation_time(t1).unwrap() // Vary this... .set_key_validity_period(Some( @@ -5383,7 +5383,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= key::Key4::generate_ecc(false, Curve::Cv25519)?.into(); let subkey_pub = subkey_sec.clone().take_secret().0; let builder = signature::SignatureBuilder::new(SignatureType::SubkeyBinding) - .set_key_flags(&KeyFlags::empty() + .set_key_flags(KeyFlags::empty() .set_transport_encryption())?; let binding = subkey_sec.bind(&mut primary_pair, &cert, builder)?; diff --git a/openpgp/src/cert/bindings.rs b/openpgp/src/cert/bindings.rs index 5da9360d..34b99d63 100644 --- a/openpgp/src/cert/bindings.rs +++ b/openpgp/src/cert/bindings.rs @@ -58,7 +58,7 @@ impl Key { /// Key4::generate_ecc(false, Curve::Cv25519)? /// .into(); /// let builder = signature::SignatureBuilder::new(SignatureType::SubkeyBinding) - /// .set_key_flags(&flags)?; + /// .set_key_flags(flags.clone())?; /// let binding = subkey.bind(&mut keypair, &cert, builder)?; /// /// // Now merge the key and binding signature into the Cert. diff --git a/openpgp/src/cert/builder.rs b/openpgp/src/cert/builder.rs index d30cee59..6d79a910 100644 --- a/openpgp/src/cert/builder.rs +++ b/openpgp/src/cert/builder.rs @@ -73,9 +73,10 @@ impl Default for CipherSuite { } impl CipherSuite { - fn generate_key(self, flags: &KeyFlags) + fn generate_key(self, flags: K) -> Result> - where R: key::KeyRole + where R: key::KeyRole, + K: AsRef, { use crate::types::Curve; @@ -88,6 +89,7 @@ impl CipherSuite { Key4::generate_rsa(4096), CipherSuite::Cv25519 | CipherSuite::P256 | CipherSuite::P384 | CipherSuite::P521 => { + let flags = flags.as_ref(); let sign = flags.for_certification() || flags.for_signing() || flags.for_authentication(); let encrypt = flags.for_transport_encryption() @@ -1038,7 +1040,7 @@ impl CertBuilder<'_> { // GnuPG wants at least a 512-bit hash for P521 keys. .set_hash_algo(HashAlgorithm::SHA512) .set_features(&Features::sequoia())? - .set_key_flags(flags)? + .set_key_flags(flags.clone())? .set_key_validity_period(blueprint.validity.or(self.primary.validity))?; if flags.for_certification() || flags.for_signing() { @@ -1081,13 +1083,13 @@ impl CertBuilder<'_> { { let mut key = self.primary.ciphersuite .unwrap_or(self.ciphersuite) - .generate_key(&KeyFlags::empty().set_certification())?; + .generate_key(KeyFlags::empty().set_certification())?; key.set_creation_time(creation_time)?; let mut sig = signature::SignatureBuilder::new(SignatureType::DirectKey) // GnuPG wants at least a 512-bit hash for P521 keys. .set_hash_algo(HashAlgorithm::SHA512) .set_features(&Features::sequoia())? - .set_key_flags(&self.primary.flags)? + .set_key_flags(self.primary.flags.clone())? .set_signature_creation_time(creation_time)? .set_key_validity_period(self.primary.validity)? .set_preferred_hash_algorithms(vec![ diff --git a/openpgp/src/packet/signature.rs b/openpgp/src/packet/signature.rs index ddc80214..3819bb18 100644 --- a/openpgp/src/packet/signature.rs +++ b/openpgp/src/packet/signature.rs @@ -1067,7 +1067,7 @@ impl SignatureBuilder { /// let mut sk_signer = subkey.clone().into_keypair()?; /// /// let sig = SignatureBuilder::new(SignatureType::SubkeyBinding) - /// .set_key_flags(&KeyFlags::empty().set_transport_encryption())? + /// .set_key_flags(KeyFlags::empty().set_transport_encryption())? /// .sign_subkey_binding(&mut pk_signer, None, &subkey)?; /// /// let cert = cert.insert_packets(vec![Packet::SecretSubkey(subkey), @@ -1211,7 +1211,7 @@ impl SignatureBuilder { /// let mut sk_signer = subkey.clone().into_keypair()?; /// /// let sig = SignatureBuilder::new(SignatureType::SubkeyBinding) - /// .set_key_flags(&KeyFlags::empty().set_signing())? + /// .set_key_flags(KeyFlags::empty().set_signing())? /// // The backsig. This is essential for subkeys that create signatures! /// .set_embedded_signature( /// SignatureBuilder::new(SignatureType::PrimaryKeyBinding) diff --git a/openpgp/src/packet/signature/subpacket.rs b/openpgp/src/packet/signature/subpacket.rs index ae5ccf1d..2d2c5506 100644 --- a/openpgp/src/packet/signature/subpacket.rs +++ b/openpgp/src/packet/signature/subpacket.rs @@ -6020,7 +6020,7 @@ impl signature::SignatureBuilder { /// = Key4::generate_ecc(false, Curve::Cv25519)? /// .into(); /// let builder = signature::SignatureBuilder::new(SignatureType::SubkeyBinding) - /// .set_key_flags(&KeyFlags::empty().set_storage_encryption())?; + /// .set_key_flags(KeyFlags::empty().set_storage_encryption())?; /// let binding = subkey.bind(&mut signer, &cert, builder)?; /// /// // Now merge the key and binding signature into the Cert. @@ -6032,9 +6032,9 @@ impl signature::SignatureBuilder { /// # 1); /// # Ok(()) } /// ``` - pub fn set_key_flags(mut self, flags: &KeyFlags) -> Result { + pub fn set_key_flags(mut self, flags: KeyFlags) -> Result { self.hashed_area.replace(Subpacket::new( - SubpacketValue::KeyFlags(flags.clone()), + SubpacketValue::KeyFlags(flags), true)?)?; Ok(self) @@ -6348,7 +6348,7 @@ impl signature::SignatureBuilder { /// /// // Create the binding signature. /// let sig = SignatureBuilder::new(SignatureType::SubkeyBinding) - /// .set_key_flags(&KeyFlags::empty().set_signing())? + /// .set_key_flags(KeyFlags::empty().set_signing())? /// // And, the backsig. This is essential for subkeys that create signatures! /// .set_embedded_signature( /// SignatureBuilder::new(SignatureType::PrimaryKeyBinding) @@ -6982,7 +6982,7 @@ fn accessors() { let key_flags = KeyFlags::empty() .set_certification() .set_signing(); - sig = sig.set_key_flags(&key_flags).unwrap(); + sig = sig.set_key_flags(key_flags.clone()).unwrap(); let sig_ = sig.clone().sign_hash(&mut keypair, hash.clone()).unwrap(); assert_eq!(sig_.key_flags().unwrap(), key_flags); diff --git a/openpgp/src/policy.rs b/openpgp/src/policy.rs index 8a33a8c2..cc268d17 100644 --- a/openpgp/src/policy.rs +++ b/openpgp/src/policy.rs @@ -1511,7 +1511,7 @@ mod test { let subkey: key::SecretSubkey = Key4::generate_rsa(4096)?.into(); let binding = signature::SignatureBuilder::new(SignatureType::SubkeyBinding) - .set_key_flags(&KeyFlags::empty().set_transport_encryption())? + .set_key_flags(KeyFlags::empty().set_transport_encryption())? .sign_subkey_binding(&mut pk.clone().into_keypair()?, pk.parts_as_public(), &subkey)?; @@ -1534,7 +1534,7 @@ mod test { let subkey: key::SecretSubkey = key::Key4::generate_ecc(true, Curve::Ed25519)?.into(); let binding = signature::SignatureBuilder::new(SignatureType::SubkeyBinding) - .set_key_flags(&KeyFlags::empty().set_transport_encryption())? + .set_key_flags(KeyFlags::empty().set_transport_encryption())? .sign_subkey_binding(&mut pk.clone().into_keypair()?, pk.parts_as_public(), &subkey)?; diff --git a/openpgp/src/serialize/cert.rs b/openpgp/src/serialize/cert.rs index 32f8433b..fca26277 100644 --- a/openpgp/src/serialize/cert.rs +++ b/openpgp/src/serialize/cert.rs @@ -901,7 +901,7 @@ mod test { &mut keypair, &cert, signature::SignatureBuilder::new(SignatureType::SubkeyBinding) .set_key_flags( - &KeyFlags::empty().set_transport_encryption()) + KeyFlags::empty().set_transport_encryption()) .unwrap() .set_exportable_certification(false).unwrap()).unwrap(); diff --git a/openpgp/src/types/key_flags.rs b/openpgp/src/types/key_flags.rs index 3909dbfe..cca9ac92 100644 --- a/openpgp/src/types/key_flags.rs +++ b/openpgp/src/types/key_flags.rs @@ -143,6 +143,12 @@ impl BitOr for &KeyFlags { } } +impl AsRef for KeyFlags { + fn as_ref(&self) -> &KeyFlags { + self + } +} + impl KeyFlags { /// Creates a new instance from `bits`. pub fn new>(bits: B) -> Self { diff --git a/sqv/tests/revoked-key.rs b/sqv/tests/revoked-key.rs index 404092e1..fcc35f4f 100644 --- a/sqv/tests/revoked-key.rs +++ b/sqv/tests/revoked-key.rs @@ -315,7 +315,7 @@ fn create_key() { // 1st direct key signature valid from t1 on let mut b = signature::SignatureBuilder::new(SignatureType::DirectKey) .set_features(&Features::sequoia()).unwrap() - .set_key_flags(&KeyFlags::empty() + .set_key_flags(KeyFlags::empty() .set_signing().set_certification()).unwrap() .set_signature_creation_time(t1).unwrap() .set_preferred_hash_algorithms(vec![HashAlgorithm::SHA512]) @@ -324,7 +324,7 @@ fn create_key() { // 1st subkey binding signature valid from t_sk_binding on b = signature::SignatureBuilder::new(SignatureType::SubkeyBinding) - .set_key_flags(&KeyFlags::empty().set_signing()).unwrap() + .set_key_flags(KeyFlags::empty().set_signing()).unwrap() .set_signature_creation_time(t_sk_binding).unwrap() .set_embedded_signature( signature::SignatureBuilder::new(SignatureType::PrimaryKeyBinding) @@ -336,7 +336,7 @@ fn create_key() { // 2nd direct key signature valid from t3 on b = signature::SignatureBuilder::new(SignatureType::DirectKey) .set_features(&Features::sequoia()).unwrap() - .set_key_flags(&KeyFlags::empty() + .set_key_flags(KeyFlags::empty() .set_signing().set_certification()).unwrap() .set_signature_creation_time(t3).unwrap() .set_preferred_hash_algorithms(vec![HashAlgorithm::SHA512]) @@ -345,7 +345,7 @@ fn create_key() { // 2nd subkey binding signature valid from t3 on let mut b = signature::SignatureBuilder::new(SignatureType::SubkeyBinding) - .set_key_flags(&KeyFlags::empty().set_signing()).unwrap() + .set_key_flags(KeyFlags::empty().set_signing()).unwrap() .set_signature_creation_time(t3).unwrap() .set_embedded_signature( signature::SignatureBuilder::new(SignatureType::PrimaryKeyBinding) -- cgit v1.2.3