openpgp: Align equality, serialization of Key packets.

  - Previously, serializing Packet::PublicKey(k) would not serialize
    any secret key material on k, but when comparing
    Packet::PublicKey(k) with Packet::PublicKey(l), the secret key
    material would be significant.  This is in conflict with our
    definition of equality, which states that two objects are
    considered equal if their canonical serialized form is equal.

  - Closely related, secret key material was considered significant
    when comparing Key<_, _> objects, and secret key material was
    emitted when they were serialized, even for objects of type
    Key<PublicParts, _>.

  - Align equality, serialization of Key<_, _> objects by ignoring any
    secret key material when comparing and serializing objects of type
    Key<PublicParts, _>.

  - Fixes #632 and #633.

3 files changed, 43 insertions, 14 deletions

diff --git a/openpgp/src/packet/key.rs b/openpgp/src/packet/key.rs
index 83e94f81..40f22a23 100644
--- a/openpgp/src/packet/key.rs
+++ b/openpgp/src/packet/key.rs
@@ -315,6 +315,10 @@ pub trait KeyParts: fmt::Debug + seal::Sealed {
         ka: &'a ComponentAmalgamation<'a, Key<UnspecifiedParts, R>>)
         -> Result<&'a ComponentAmalgamation<'a, Key<Self, R>>>
         where Self: Sized;
+
+    /// Indicates that secret key material should be considered when
+    /// comparing or hashing this key.
+    fn significant_secrets() -> bool;
 }
 
 /// A marker trait that captures a `Key`'s role.
@@ -477,6 +481,10 @@ impl KeyParts for PublicParts {
         -> Result<&'a ComponentAmalgamation<'a, Key<Self, R>>> {
         Ok(ka.into())
     }
+
+    fn significant_secrets() -> bool {
+        false
+    }
 }
 
 /// A marker that indicates that a `Key` should be treated like a
@@ -530,6 +538,10 @@ impl KeyParts for SecretParts {
         -> Result<&'a ComponentAmalgamation<'a, Key<Self, R>>> {
         ka.try_into()
     }
+
+    fn significant_secrets() -> bool {
+        true
+    }
 }
 
 /// A marker that indicates that a `Key`'s parts are unspecified.
@@ -591,6 +603,10 @@ impl KeyParts for UnspecifiedParts {
         -> Result<&'a ComponentAmalgamation<'a, Key<Self, R>>> {
         Ok(ka.into())
     }
+
+    fn significant_secrets() -> bool {
+        true
+    }
 }
 
 /// A marker that indicates the `Key` should be treated like a primary key.
@@ -790,7 +806,7 @@ impl<P: KeyParts, R: KeyRole> PartialEq for Key4<P, R> {
         self.creation_time == other.creation_time
             && self.pk_algo == other.pk_algo
             && self.mpis == other.mpis
-            && self.secret == other.secret
+            && (! P::significant_secrets() || self.secret == other.secret)
     }
 }
 
@@ -801,7 +817,9 @@ impl<P: KeyParts, R: KeyRole> std::hash::Hash for Key4<P, R> {
         std::hash::Hash::hash(&self.creation_time, state);
         std::hash::Hash::hash(&self.pk_algo, state);
         std::hash::Hash::hash(&self.mpis, state);
-        std::hash::Hash::hash(&self.secret, state);
+        if P::significant_secrets() {
+            std::hash::Hash::hash(&self.secret, state);
+        }
     }
 }
 
diff --git a/openpgp/src/packet/mod.rs b/openpgp/src/packet/mod.rs
index 3b544de2..e87e7b16 100644
--- a/openpgp/src/packet/mod.rs
+++ b/openpgp/src/packet/mod.rs
@@ -1379,10 +1379,12 @@ impl From<SKESK> for Packet {
 /// # A note on equality
 ///
 /// The implementation of `Eq` for `Key` compares the serialized form
-/// of `Key`s.  Notably this includes the secret key material, but
-/// excludes the `KeyParts` and `KeyRole` marker traits.
-/// To exclude the secret key material from the comparison, use
-/// [`Key::public_cmp`] or [`Key::public_eq`].
+/// of `Key`s.  Comparing or serializing values of `Key<PublicParts,
+/// _>` ignore secret key material, whereas the secret key material is
+/// considered and serialized for `Key<SecretParts, _>`, and for
+/// `Key<UnspecifiedParts, _>` if present.  To explicitly exclude the
+/// secret key material from the comparison, use [`Key::public_cmp`]
+/// or [`Key::public_eq`].
 ///
 /// When merging in secret key material from untrusted sources, you
 /// need to be very careful: secret key material is not
@@ -1404,26 +1406,35 @@ impl From<SKESK> for Packet {
 /// use sequoia_openpgp as openpgp;
 /// use openpgp::cert::prelude::*;
 /// use openpgp::packet::prelude::*;
+/// use openpgp::packet::key::*;
 ///
 /// # fn main() -> openpgp::Result<()> {
 /// // Generate a new certificate.  It has secret key material.
 /// let (cert, _) = CertBuilder::new()
 ///     .generate()?;
 ///
-/// let sk = cert.primary_key().key();
+/// let sk: &Key<PublicParts, _> = cert.primary_key().key();
 /// assert!(sk.has_secret());
 ///
 /// // Strip the secret key material.
 /// let cert = cert.clone().strip_secret_key_material();
-/// let pk = cert.primary_key().key();
+/// let pk: &Key<PublicParts, _> = cert.primary_key().key();
 /// assert!(! pk.has_secret());
 ///
-/// // Eq compares both the public and the secret bits, so it
-/// // considers pk and sk to be different.
+/// // Eq on Key<PublicParts, _> compares only the public bits, so it
+/// // considers pk and sk to be equal.
+/// assert_eq!(pk, sk);
+///
+/// // Convert to Key<UnspecifiedParts, _>.
+/// let sk: &Key<UnspecifiedParts, _> = sk.parts_as_unspecified();
+/// let pk: &Key<UnspecifiedParts, _> = pk.parts_as_unspecified();
+///
+/// // Eq on Key<UnspecifiedParts, _> compares both the public and the
+/// // secret bits, so it considers pk and sk to be different.
 /// assert_ne!(pk, sk);
 ///
-/// // Key::public_eq only compares the public bits, so it considers
-/// // them to be equal.
+/// // In any case, Key::public_eq only compares the public bits,
+/// // so it considers them to be equal.
 /// assert!(Key::public_eq(pk, sk));
 /// # Ok(())
 /// # }
diff --git a/openpgp/src/serialize.rs b/openpgp/src/serialize.rs
index b604d9d0..0e862692 100644
--- a/openpgp/src/serialize.rs
+++ b/openpgp/src/serialize.rs
@@ -1812,7 +1812,7 @@ impl<P, R> Marshal for Key4<P, R>
           R: key::KeyRole,
 {
     fn serialize(&self, o: &mut dyn io::Write) -> Result<()> {
-        self.serialize_key(o, true)
+        self.serialize_key(o, P::significant_secrets())
     }
 }
 
@@ -1883,7 +1883,7 @@ impl<P, R> MarshalInto for Key4<P, R>
           R: key::KeyRole,
 {
     fn serialized_len(&self) -> usize {
-        self.net_len_key(true)
+        self.net_len_key(P::significant_secrets())
     }
 
     fn serialize_into(&self, buf: &mut [u8]) -> Result<usize> {