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.

6 files changed, 214 insertions, 110 deletions

diff --git a/net/src/wkd.rs b/net/src/wkd.rs
index c2a47d71..50f4d25e 100644
--- a/net/src/wkd.rs
+++ b/net/src/wkd.rs
@@ -385,7 +385,7 @@ impl KeyRing {
     fn insert(&mut self, cert: Cert) -> Result<()> {
         let fp = cert.fingerprint();
         if let Some(existing) = self.0.get_mut(&fp) {
-            *existing = existing.clone().merge(cert)?;
+            *existing = existing.clone().merge_public(cert)?;
         } else {
             self.0.insert(fp, cert);
         }
diff --git a/openpgp-ffi/include/sequoia/openpgp.h b/openpgp-ffi/include/sequoia/openpgp.h
index 104dede6..cb6bc31d 100644
--- a/openpgp-ffi/include/sequoia/openpgp.h
+++ b/openpgp-ffi/include/sequoia/openpgp.h
@@ -888,14 +888,26 @@ pgp_status_t pgp_cert_serialize (pgp_error_t *errp,
                               pgp_writer_t writer);
 
 /*/
-/// Merges `other` into `cert`.
+/// Merges `other` into `cert`, ignoring secret key material in `other`.
 ///
 /// If `other` is a different key, then nothing is merged into
 /// `cert`, but `cert` is still canonicalized.
 ///
 /// Consumes `cert` and `other`.
 /*/
-pgp_cert_t pgp_cert_merge (pgp_error_t *errp,
+pgp_cert_t pgp_cert_merge_public (pgp_error_t *errp,
+                       pgp_cert_t cert,
+                       pgp_cert_t other);
+
+/*/
+/// Merges `other` into `cert`, including secret key material in `other`.
+///
+/// If `other` is a different key, then nothing is merged into
+/// `cert`, but `cert` is still canonicalized.
+///
+/// Consumes `cert` and `other`.
+/*/
+pgp_cert_t pgp_cert_merge_public_and_secret (pgp_error_t *errp,
                        pgp_cert_t cert,
                        pgp_cert_t other);
 
diff --git a/openpgp-ffi/src/cert.rs b/openpgp-ffi/src/cert.rs
index 6eecd3d3..37b17472 100644
--- a/openpgp-ffi/src/cert.rs
+++ b/openpgp-ffi/src/cert.rs
@@ -85,20 +85,36 @@ fn pgp_cert_from_packet_parser(errp: Option<&mut *mut crate::error::Error>,
     openpgp::Cert::try_from(*ppr).move_into_raw(errp)
 }
 
-/// Merges `other` into `cert`.
+/// Merges `other` into `cert`, ignoring secret key material in `other`.
 ///
 /// If `other` is a different key, then nothing is merged into
 /// `cert`, but `cert` is still canonicalized.
 ///
 /// Consumes `cert` and `other`.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "C"
-fn pgp_cert_merge(errp: Option<&mut *mut crate::error::Error>,
+fn pgp_cert_merge_public(errp: Option<&mut *mut crate::error::Error>,
                  cert: *mut Cert,
                  other: *mut Cert)
                  -> Maybe<Cert> {
     let cert = cert.move_from_raw();
     let other = other.move_from_raw();
-    cert.merge(other).move_into_raw(errp)
+    cert.merge_public(other).move_into_raw(errp)
+}
+
+/// Merges `other` into `cert`, including secret key material in `other`.
+///
+/// If `other` is a different key, then nothing is merged into
+/// `cert`, but `cert` is still canonicalized.
+///
+/// Consumes `cert` and `other`.
+#[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "C"
+fn pgp_cert_merge_public_and_secret(errp: Option<&mut *mut crate::error::Error>,
+                 cert: *mut Cert,
+                 other: *mut Cert)
+                 -> Maybe<Cert> {
+    let cert = cert.move_from_raw();
+    let other = other.move_from_raw();
+    cert.merge_public_and_secret(other).move_into_raw(errp)
 }
 
 /// Adds packets to the Cert.
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<Self> {
+    pub fn merge_public(self, other: Cert) -> Result<Self> {
+        // 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<Self> {
         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);
 
         // N