From dc50161c51d5b479a54d3dc912574f9ed208892a Mon Sep 17 00:00:00 2001 From: Justus Winter Date: Fri, 27 Nov 2020 13:22:37 +0100 Subject: openpgp: Add Cert::merge_public and Cert::merge_public_and_secret. - Secret key material is not authenticated by OpenPGP, so care must be taken when merging certificates. - Rename Cert::merge to Cert::merge_public_and_secret. - Add new function Cert::merge_public. This function can be used to merge certificates from untrusted sources as it ignores secret key material that cannot be authenticated by OpenPGP. - Fixes #584. --- openpgp/src/cert.rs | 278 +++++++++++++++++++++++++++++++++------------------- 1 file changed, 177 insertions(+), 101 deletions(-) (limited to 'openpgp') diff --git a/openpgp/src/cert.rs b/openpgp/src/cert.rs index 3c4a34ee..40b439ba 100644 --- a/openpgp/src/cert.rs +++ b/openpgp/src/cert.rs @@ -36,7 +36,7 @@ //! - *Using a certificate*: See the [`Cert`] and [`ValidCert`] data structures. //! - *Revoking a certificate*: See the [`CertRevocationBuilder`] data structure. //! - *Merging packets*: See the [`Cert::insert_packets`] method. -//! - *Merging certificates*: See the [`Cert::merge`] method. +//! - *Merging certificates*: See the [`Cert::merge_public`] method. //! - *Creating third-party certifications*: See the [`UserID::certify`] //! and [`UserAttribute::certify`] methods. //! - *Using User IDs and User Attributes*: See the [`ComponentAmalgamation`] module. @@ -119,7 +119,7 @@ //! [`ValidCert`]: struct.ValidCert.html //! [`CertRevocationBuilder`]: struct.CertRevocationBuilder.html //! [`Cert::insert_packets`]: struct.Cert.html#method.insert_packets -//! [`Cert::merge`]: struct.Cert.html#method.merge +//! [`Cert::merge_public`]: struct.Cert.html#method.merge_public //! [`UserID::certify`]: ../packet/struct.UserID.html#method.certify //! [`UserAttribute::certify`]: ../packet/user_attribute/struct.UserAttribute.html#method.certify //! [`ComponentAmalgamation`]: amalgamation/index.html @@ -2016,7 +2016,8 @@ impl Cert { self.primary.key().keyid() } - /// Merges `other` into `self`. + /// Merges `other` into `self`, ignoring secret key material in + /// `other`. /// /// If `other` is a different certificate, then an error is /// returned. @@ -2027,27 +2028,78 @@ impl Cert { /// /// [`Cert::insert_packets`]: #method.insert_packets /// + /// This function is appropriate to merge certificate material + /// from untrusted sources like keyservers. If `other` contains + /// secret key material, it is ignored. See + /// [`Cert::merge_public_and_secret`] on how to merge certificates + /// containing secret key material from trusted sources. + /// + /// [`Cert::merge_public_and_secret`]: #method.merge_public_and_secret + /// /// # Examples /// /// ``` - /// use sequoia_openpgp as openpgp; + /// # use sequoia_openpgp as openpgp; /// # use openpgp::cert::prelude::*; - /// + /// # /// # fn main() -> openpgp::Result<()> { /// # let (local, _) = /// # CertBuilder::general_purpose(None, Some("alice@example.org")) /// # .generate()?; /// # let keyserver = local.clone(); - /// // Merge the "local" version with the version from the "key server". - /// let cert = if local.fingerprint() == keyserver.fingerprint() { - /// local.merge(keyserver)?; - /// } else { - /// // Error, the key server returned a different certificate. - /// }; - /// # Ok(()) - /// # } + /// // Merge the local version with the version from the keyserver. + /// let cert = local.merge_public(keyserver)?; + /// # let _ = cert; + /// # Ok(()) } /// ``` - pub fn merge(mut self, mut other: Cert) -> Result { + pub fn merge_public(self, other: Cert) -> Result { + // Strip all secrets from `other`. + let other_public = Cert::from_packets( + other.into_packets().map(|p| match p { + Packet::PublicKey(k) => k.take_secret().0.into(), + Packet::SecretKey(k) => k.take_secret().0.into(), + Packet::PublicSubkey(k) => k.take_secret().0.into(), + Packet::SecretSubkey(k) => k.take_secret().0.into(), + p => p, + }))?; + // Then merge it. + self.merge_public_and_secret(other_public) + } + + /// Merges `other` into `self`, including secret key material. + /// + /// If `other` is a different certificate, then an error is + /// returned. + /// + /// This routine merges duplicate packets. This is different from + /// [`Cert::insert_packets`], which prefers keys in the packets that + /// are being merged into the certificate. + /// + /// [`Cert::insert_packets`]: #method.insert_packets + /// + /// It is important to only merge key material from trusted + /// sources using this function, because it may be used to import + /// secret key material. Secret key material is not authenticated + /// by OpenPGP, and there are plausible attack scenarios where a + /// malicious actor injects secret key material. + /// + /// # Examples + /// + /// ``` + /// # use sequoia_openpgp as openpgp; + /// # use openpgp::cert::prelude::*; + /// # + /// # fn main() -> openpgp::Result<()> { + /// # let (local, _) = + /// # CertBuilder::general_purpose(None, Some("alice@example.org")) + /// # .generate()?; + /// # let other_device = local.clone(); + /// // Merge the local version with the version from your other device. + /// let cert = local.merge_public_and_secret(other_device)?; + /// # let _ = cert; + /// # Ok(()) } + /// ``` + pub fn merge_public_and_secret(mut self, mut other: Cert) -> Result { if self.fingerprint() != other.fingerprint() { // The primary key is not the same. There is nothing to // do. @@ -3534,7 +3586,7 @@ mod test { // When we merge it with itself, we should get the exact same // thing. - let merged = cert_base.clone().merge(cert_base.clone()).unwrap(); + let merged = cert_base.clone().merge_public_and_secret(cert_base.clone()).unwrap(); assert_eq!(cert_base, merged); let cert_add_uid_1 @@ -3556,16 +3608,16 @@ mod test { assert_eq!(cert_all_uids.userids.len(), 3); // Merge in order. - let merged = cert_base.clone().merge(cert_add_uid_1.clone()).unwrap() - .merge(cert_add_uid_2.clone()).unwrap() - .merge(cert_add_uid_3.clone()).unwrap(); + let merged = cert_base.clone().merge_public_and_secret(cert_add_uid_1.clone()).unwrap() + .merge_public_and_secret(cert_add_uid_2.clone()).unwrap() + .merge_public_and_secret(cert_add_uid_3.clone()).unwrap(); assert_eq!(cert_all_uids, merged); // Merge in reverse order. let merged = cert_base.clone() - .merge(cert_add_uid_3.clone()).unwrap() - .merge(cert_add_uid_2.clone()).unwrap() - .merge(cert_add_uid_1.clone()).unwrap(); + .merge_public_and_secret(cert_add_uid_3.clone()).unwrap() + .merge_public_and_secret(cert_add_uid_2.clone()).unwrap() + .merge_public_and_secret(cert_add_uid_1.clone()).unwrap(); assert_eq!(cert_all_uids, merged); let cert_add_subkey_1 @@ -3579,28 +3631,28 @@ mod test { = Cert::from_bytes(key("bannon-all-subkeys.gpg")).unwrap(); // Merge the first user, then the second, then the third. - let merged = cert_base.clone().merge(cert_add_subkey_1.clone()).unwrap() - .merge(cert_add_subkey_2.clone()).unwrap() - .merge(cert_add_subkey_3.clone()).unwrap(); + let merged = cert_base.clone().merge_public_and_secret(cert_add_subkey_1.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_2.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_3.clone()).unwrap(); assert_eq!(cert_all_subkeys, merged); // Merge the third user, then the second, then the first. - let merged = cert_base.clone().merge(cert_add_subkey_3.clone()).unwrap() - .merge(cert_add_subkey_2.clone()).unwrap() - .merge(cert_add_subkey_1.clone()).unwrap(); + let merged = cert_base.clone().merge_public_and_secret(cert_add_subkey_3.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_2.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_1.clone()).unwrap(); assert_eq!(cert_all_subkeys, merged); // Merge a lot. let merged = cert_base.clone() - .merge(cert_add_subkey_1.clone()).unwrap() - .merge(cert_add_subkey_1.clone()).unwrap() - .merge(cert_add_subkey_3.clone()).unwrap() - .merge(cert_add_subkey_1.clone()).unwrap() - .merge(cert_add_subkey_2.clone()).unwrap() - .merge(cert_add_subkey_3.clone()).unwrap() - .merge(cert_add_subkey_3.clone()).unwrap() - .merge(cert_add_subkey_1.clone()).unwrap() - .merge(cert_add_subkey_2.clone()).unwrap(); + .merge_public_and_secret(cert_add_subkey_1.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_1.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_3.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_1.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_2.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_3.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_3.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_1.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_2.clone()).unwrap(); assert_eq!(cert_all_subkeys, merged); let cert_all @@ -3609,25 +3661,25 @@ mod test { // Merge all the subkeys with all the uids. let merged = cert_all_subkeys.clone() - .merge(cert_all_uids.clone()).unwrap(); + .merge_public_and_secret(cert_all_uids.clone()).unwrap(); assert_eq!(cert_all, merged); // Merge all uids with all the subkeys. let merged = cert_all_uids.clone() - .merge(cert_all_subkeys.clone()).unwrap(); + .merge_public_and_secret(cert_all_subkeys.clone()).unwrap(); assert_eq!(cert_all, merged); // All the subkeys and the uids in a mixed up order. let merged = cert_base.clone() - .merge(cert_add_subkey_1.clone()).unwrap() - .merge(cert_add_uid_2.clone()).unwrap() - .merge(cert_add_uid_1.clone()).unwrap() - .merge(cert_add_subkey_3.clone()).unwrap() - .merge(cert_add_subkey_1.clone()).unwrap() - .merge(cert_add_uid_3.clone()).unwrap() - .merge(cert_add_subkey_2.clone()).unwrap() - .merge(cert_add_subkey_1.clone()).unwrap() - .merge(cert_add_uid_2.clone()).unwrap(); + .merge_public_and_secret(cert_add_subkey_1.clone()).unwrap() + .merge_public_and_secret(cert_add_uid_2.clone()).unwrap() + .merge_public_and_secret(cert_add_uid_1.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_3.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_1.clone()).unwrap() + .merge_public_and_secret(cert_add_uid_3.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_2.clone()).unwrap() + .merge_public_and_secret(cert_add_subkey_1.clone()).unwrap() + .merge_public_and_secret(cert_add_uid_2.clone()).unwrap(); assert_eq!(cert_all, merged); // Certifications. @@ -3650,13 +3702,13 @@ mod test { assert!(cert_donald_signs_base.userids[0].certifications.len() == 1); let merged = cert_donald_signs_base.clone() - .merge(cert_ivanka_signs_base.clone()).unwrap(); + .merge_public_and_secret(cert_ivanka_signs_base.clone()).unwrap(); assert!(merged.userids.len() == 1); assert!(merged.userids[0].self_signatures.len() == 1); assert!(merged.userids[0].certifications.len() == 2); let merged = cert_donald_signs_base.clone() - .merge(cert_donald_signs_all.clone()).unwrap(); + .merge_public_and_secret(cert_donald_signs_all.clone()).unwrap(); assert!(merged.userids.len() == 3); assert!(merged.userids[0].self_signatures.len() == 1); // There should be two certifications from the Donald on the @@ -3666,9 +3718,9 @@ mod test { assert!(merged.userids[2].certifications.len() == 1); let merged = cert_donald_signs_base.clone() - .merge(cert_donald_signs_all.clone()).unwrap() - .merge(cert_ivanka_signs_base.clone()).unwrap() - .merge(cert_ivanka_signs_all.clone()).unwrap(); + .merge_public_and_secret(cert_donald_signs_all.clone()).unwrap() + .merge_public_and_secret(cert_ivanka_signs_base.clone()).unwrap() + .merge_public_and_secret(cert_ivanka_signs_all.clone()).unwrap(); assert!(merged.userids.len() == 3); assert!(merged.userids[0].self_signatures.len() == 1); // There should be two certifications from each of the Donald @@ -3679,14 +3731,14 @@ mod test { // Same as above, but redundant. let merged = cert_donald_signs_base.clone() - .merge(cert_ivanka_signs_base.clone()).unwrap() - .merge(cert_donald_signs_all.clone()).unwrap() - .merge(cert_donald_signs_all.clone()).unwrap() - .merge(cert_ivanka_signs_all.clone()).unwrap() - .merge(cert_ivanka_signs_base.clone()).unwrap() - .merge(cert_donald_signs_all.clone()).unwrap() - .merge(cert_donald_signs_all.clone()).unwrap() - .merge(cert_ivanka_signs_all.clone()).unwrap(); + .merge_public_and_secret(cert_ivanka_signs_base.clone()).unwrap() + .merge_public_and_secret(cert_donald_signs_all.clone()).unwrap() + .merge_public_and_secret(cert_donald_signs_all.clone()).unwrap() + .merge_public_and_secret(cert_ivanka_signs_all.clone()).unwrap() + .merge_public_and_secret(cert_ivanka_signs_base.clone()).unwrap() + .merge_public_and_secret(cert_donald_signs_all.clone()).unwrap() + .merge_public_and_secret(cert_donald_signs_all.clone()).unwrap() + .merge_public_and_secret(cert_ivanka_signs_all.clone()).unwrap(); assert!(merged.userids.len() == 3); assert!(merged.userids[0].self_signatures.len() == 1); // There should be two certifications from each of the Donald @@ -3902,7 +3954,7 @@ mod test { let update = Cert::from_bytes(crate::tests::key("about-to-expire.update-no-uid.pgp")) .unwrap(); - let cert = cert.merge(update).unwrap(); + let cert = cert.merge_public_and_secret(update).unwrap(); cert.primary_key().with_policy(p, None).unwrap().alive().unwrap(); } @@ -4192,40 +4244,40 @@ mod test { crate::tests::key("already-revoked-direct-revocation.pgp")).unwrap(); check(&d, true, false, false); - check(&cert.clone().merge(d.clone()).unwrap(), true, false, false); + check(&cert.clone().merge_public_and_secret(d.clone()).unwrap(), true, false, false); // Make sure the merge order does not matter. - check(&d.clone().merge(cert.clone()).unwrap(), true, false, false); + check(&d.clone().merge_public_and_secret(cert.clone()).unwrap(), true, false, false); let u = Cert::from_bytes( crate::tests::key("already-revoked-userid-revocation.pgp")).unwrap(); check(&u, false, true, false); - check(&cert.clone().merge(u.clone()).unwrap(), false, true, false); - check(&u.clone().merge(cert.clone()).unwrap(), false, true, false); + check(&cert.clone().merge_public_and_secret(u.clone()).unwrap(), false, true, false); + check(&u.clone().merge_public_and_secret(cert.clone()).unwrap(), false, true, false); let k = Cert::from_bytes( crate::tests::key("already-revoked-subkey-revocation.pgp")).unwrap(); check(&k, false, false, true); - check(&cert.clone().merge(k.clone()).unwrap(), false, false, true); - check(&k.clone().merge(cert.clone()).unwrap(), false, false, true); + check(&cert.clone().merge_public_and_secret(k.clone()).unwrap(), false, false, true); + check(&k.clone().merge_public_and_secret(cert.clone()).unwrap(), false, false, true); // direct and user id revocation. - check(&d.clone().merge(u.clone()).unwrap(), true, true, false); - check(&u.clone().merge(d.clone()).unwrap(), true, true, false); + check(&d.clone().merge_public_and_secret(u.clone()).unwrap(), true, true, false); + check(&u.clone().merge_public_and_secret(d.clone()).unwrap(), true, true, false); // direct and subkey revocation. - check(&d.clone().merge(k.clone()).unwrap(), true, false, true); - check(&k.clone().merge(d.clone()).unwrap(), true, false, true); + check(&d.clone().merge_public_and_secret(k.clone()).unwrap(), true, false, true); + check(&k.clone().merge_public_and_secret(d.clone()).unwrap(), true, false, true); // user id and subkey revocation. - check(&u.clone().merge(k.clone()).unwrap(), false, true, true); - check(&k.clone().merge(u.clone()).unwrap(), false, true, true); + check(&u.clone().merge_public_and_secret(k.clone()).unwrap(), false, true, true); + check(&k.clone().merge_public_and_secret(u.clone()).unwrap(), false, true, true); // direct, user id and subkey revocation. - check(&d.clone().merge(u.clone().merge(k.clone()).unwrap()).unwrap(), + check(&d.clone().merge_public_and_secret(u.clone().merge_public_and_secret(k.clone()).unwrap()).unwrap(), true, true, true); - check(&d.clone().merge(k.clone().merge(u.clone()).unwrap()).unwrap(), + check(&d.clone().merge_public_and_secret(k.clone().merge_public_and_secret(u.clone()).unwrap()).unwrap(), true, true, true); } @@ -4479,7 +4531,7 @@ mod test { assert!(!revoked(p, &cert, None)); t!("Soft revocation"); - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key( &format!("really-revoked-{}-1-soft-revocation.pgp", f)) @@ -4490,7 +4542,7 @@ mod test { assert!(revoked(p, &cert, None)); t!("New self signature"); - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key( &format!("really-revoked-{}-2-new-self-sig.pgp", f)) @@ -4500,7 +4552,7 @@ mod test { assert!(!revoked(p, &cert, None)); t!("Hard revocation"); - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key( &format!("really-revoked-{}-3-hard-revocation.pgp", f)) @@ -4510,7 +4562,7 @@ mod test { assert!(revoked(p, &cert, None)); t!("New self signature"); - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key( &format!("really-revoked-{}-4-new-self-sig.pgp", f)) @@ -4600,7 +4652,7 @@ mod test { check(p, &cert, false, now); // A soft-revocation. - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key( &format!("really-revoked-{}-1-soft-revocation.pgp", f)) @@ -4610,7 +4662,7 @@ mod test { check(p, &cert, true, now); // A new self signature. This should override the soft-revocation. - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key( &format!("really-revoked-{}-2-new-self-sig.pgp", f)) @@ -4621,7 +4673,7 @@ mod test { // A hard revocation. Unlike for Certs, this does NOT trumps // everything. - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key( &format!("really-revoked-{}-3-hard-revocation.pgp", f)) @@ -4631,7 +4683,7 @@ mod test { check(p, &cert, true, now); // A newer self signature. - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key( &format!("really-revoked-{}-4-new-self-sig.pgp", f)) @@ -4700,13 +4752,13 @@ mod test { assert!(!cert.is_tsk()); assert!(cert.subkeys().all(|k| ! k.key().has_secret())); - let merge1 = cert.clone().merge(tsk.clone()).unwrap(); + let merge1 = cert.clone().merge_public_and_secret(tsk.clone()).unwrap(); assert!(merge1.is_tsk()); assert!(merge1.primary.key().has_secret()); assert_eq!(merge1.subkeys().len(), subkey_count); assert!(merge1.subkeys().all(|k| k.key().has_secret())); - let merge2 = tsk.clone().merge(cert.clone()).unwrap(); + let merge2 = tsk.clone().merge_public_and_secret(cert.clone()).unwrap(); assert!(merge2.is_tsk()); assert!(merge2.primary.key().has_secret()); assert_eq!(merge2.subkeys().len(), subkey_count); @@ -4861,7 +4913,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= b"Eminem"); // A soft-revocation for "Slim Shady". - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key("really-revoked-userid-1-soft-revocation.pgp") ).unwrap()).unwrap(); @@ -4875,7 +4927,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= // A new self signature for "Slim Shady". This should // override the soft-revocation. - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key("really-revoked-userid-2-new-self-sig.pgp") ).unwrap()).unwrap(); @@ -4888,7 +4940,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= b"Slim Shady"); // A hard revocation for "Slim Shady". - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key("really-revoked-userid-3-hard-revocation.pgp") ).unwrap()).unwrap(); @@ -4902,7 +4954,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= // A newer self signature for "Slim Shady". Unlike for Certs, this // does NOT trump everything. - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key("really-revoked-userid-4-new-self-sig.pgp") ).unwrap()).unwrap(); @@ -4935,7 +4987,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= // Add a second user id. Since neither is marked primary, the // newer one should be considered primary. - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key("primary-key-1-add-userid-bbbbb.pgp") ).unwrap()).unwrap(); @@ -4948,7 +5000,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= b"bbbbb"); // Mark aaaaa as primary. It is now primary and the newest one. - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key("primary-key-2-make-aaaaa-primary.pgp") ).unwrap()).unwrap(); @@ -4962,7 +5014,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= // Update the preferences on bbbbb. It is now the newest, but // it is not marked as primary. - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key("primary-key-3-make-bbbbb-new-self-sig.pgp") ).unwrap()).unwrap(); @@ -4976,7 +5028,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= // Mark bbbbb as primary. It is now the newest and marked as // primary. - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key("primary-key-4-make-bbbbb-primary.pgp") ).unwrap()).unwrap(); @@ -4991,7 +5043,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= // Update the preferences on aaaaa. It is now has the newest // self sig, but that self sig does not say that it is // primary. - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key("primary-key-5-make-aaaaa-self-sig.pgp") ).unwrap()).unwrap(); @@ -5005,7 +5057,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= // Hard revoke aaaaa. Unlike with Certs, a hard revocation is // not treated specially. - let cert = cert.merge( + let cert = cert.merge_public_and_secret( Cert::from_bytes( crate::tests::key("primary-key-6-revoked-aaaaa.pgp") ).unwrap()).unwrap(); @@ -5299,14 +5351,14 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= Cert::try_from(vec![primary_pub.clone().into()])?; let cert_s = Cert::try_from(vec![primary_sec.clone().into()])?; - let cert = cert_p.merge(cert_s)?; + let cert = cert_p.merge_public_and_secret(cert_s)?; assert!(cert.primary_key().has_secret()); let cert_p = Cert::try_from(vec![primary_pub.clone().into()])?; let cert_s = Cert::try_from(vec![primary_sec.clone().into()])?; - let cert = cert_s.merge(cert_p)?; + let cert = cert_s.merge_public_and_secret(cert_p)?; assert!(cert.primary_key().has_secret()); Ok(()) } @@ -5639,7 +5691,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= } } let cert_b = Cert::from_packets(packets.into_iter())?; - let cert = cert.merge(cert_b)?; + let cert = cert.merge_public_and_secret(cert_b)?; assert_eq!(cert.userids().count(), 1); assert_eq!(cert.subkeys().count(), 1); assert_eq!(cert.unknowns().count(), 0); @@ -5755,7 +5807,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= assert_eq!(malicious_cert.bad_signatures().count(), 0); // Now try to merge it in. - let merged = cert.clone().merge(malicious_cert.clone())?; + let merged = cert.clone().merge_public_and_secret(malicious_cert.clone())?; // The subkey binding signature should still be fine. assert_eq!(merged.with_policy(p, None)?.keys().subkeys() .for_signing().count(), 1); @@ -5764,7 +5816,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= assert_eq!(sig.embedded_signatures().count(), 2); // Now the other way around. - let merged = malicious_cert.clone().merge(cert.clone())?; + let merged = malicious_cert.clone().merge_public_and_secret(cert.clone())?; // The subkey binding signature should still be fine. assert_eq!(merged.with_policy(p, None)?.keys().subkeys() .for_signing().count(), 1); @@ -5796,7 +5848,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= } let cert = Cert::try_from(pp)?; - assert_eq!(cert.clone().merge(cert.clone())?, cert); + assert_eq!(cert.clone().merge_public_and_secret(cert.clone())?, cert); // Specifically, the issuer information should have been added // back by the canonicalization. @@ -5810,4 +5862,28 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g= 1); Ok(()) } + + /// Checks that Cert::merge_public ignores secret key material. + #[test] + fn merge_public() -> Result<()> { + let cert = + Cert::from_bytes(crate::tests::key("testy-new.pgp"))?; + let key = + Cert::from_bytes(crate::tests::key("testy-new-private.pgp"))?; + + assert!(! cert.is_tsk()); + assert!(key.is_tsk()); + + // Secrets are ignored in `other`. + let merged = cert.clone().merge_public(key.clone())?; + assert!(! merged.is_tsk()); + assert_eq!(merged, cert); + + // Secrets are retained in `self`. + let merged = key.clone().merge_public(cert.clone())?; + assert!(merged.is_tsk()); + assert_eq!(merged, key); + + Ok(()) + } } -- cgit v1.2.3