openpgp: Correctly handle keys where a primary key is also a subkey.

  - It is possible for a primary key to also be a subkey.

  - Correctly handle that case.

  - In particular, don't merge Public Key packets with Public Subkey
    packets, etc.

2 files changed, 115 insertions, 19 deletions

diff --git a/openpgp/src/cert.rs b/openpgp/src/cert.rs
index bc32f0b6..54720fcd 100644
--- a/openpgp/src/cert.rs
+++ b/openpgp/src/cert.rs
@@ -2338,30 +2338,45 @@ impl Cert {
     {
         let mut combined = self.into_packets().collect::<Vec<_>>();
 
-        fn replace_or_push<P, R>(acc: &mut Vec<Packet>, k: Key<P, R>)
-            where P: key::KeyParts,
-                  R: key::KeyRole,
-                  Packet: From<packet::Key<P, R>>,
+        fn replace_or_push(acc: &mut Vec<Packet>, p: Packet)
         {
+            match p {
+                Packet::PublicKey(_) => (),
+                Packet::SecretKey(_) => (),
+                Packet::PublicSubkey(_) => (),
+                Packet::SecretSubkey(_) => (),
+                _ => unreachable!(),
+            }
+
             for q in acc.iter_mut() {
-                let replace = match q {
-                    Packet::PublicKey(k_) =>
-                        k_.public_cmp(&k) == Ordering::Equal,
-                    Packet::SecretKey(k_) =>
-                        k_.public_cmp(&k) == Ordering::Equal,
-                    Packet::PublicSubkey(k_) =>
-                        k_.public_cmp(&k) == Ordering::Equal,
-                    Packet::SecretSubkey(k_) =>
-                        k_.public_cmp(&k) == Ordering::Equal,
+                let replace = match (&p, &q) {
+                    (Packet::PublicKey(a), Packet::PublicKey(b)) =>
+                        a.public_cmp(&b) == Ordering::Equal,
+                    (Packet::SecretKey(a), Packet::SecretKey(b)) =>
+                        a.public_cmp(&b) == Ordering::Equal,
+                    (Packet::PublicKey(a), Packet::SecretKey(b)) =>
+                        a.public_cmp(&b) == Ordering::Equal,
+                    (Packet::SecretKey(a), Packet::PublicKey(b)) =>
+                        a.public_cmp(&b) == Ordering::Equal,
+
+                    (Packet::PublicSubkey(a), Packet::PublicSubkey(b)) =>
+                        a.public_cmp(&b) == Ordering::Equal,
+                    (Packet::SecretSubkey(a), Packet::SecretSubkey(b)) =>
+                        a.public_cmp(&b) == Ordering::Equal,
+                    (Packet::PublicSubkey(a), Packet::SecretSubkey(b)) =>
+                        a.public_cmp(&b) == Ordering::Equal,
+                    (Packet::SecretSubkey(a), Packet::PublicSubkey(b)) =>
+                        a.public_cmp(&b) == Ordering::Equal,
+
                     _ => false,
                 };
 
                 if replace {
-                    *q = k.into();
+                    *q = p;
                     return;
                 }
             }
-            acc.push(k.into());
+            acc.push(p);
         };
 
         /// Replaces or pushes a signature.
@@ -2389,10 +2404,10 @@ impl Cert {
             let p = p.into();
             Cert::valid_packet(&p)?;
             match p {
-                Packet::PublicKey(k) => replace_or_push(&mut combined, k),
-                Packet::SecretKey(k) => replace_or_push(&mut combined, k),
-                Packet::PublicSubkey(k) => replace_or_push(&mut combined, k),
-                Packet::SecretSubkey(k) => replace_or_push(&mut combined, k),
+                p @ Packet::PublicKey(_) => replace_or_push(&mut combined, p),
+                p @ Packet::SecretKey(_) => replace_or_push(&mut combined, p),
+                p @ Packet::PublicSubkey(_) => replace_or_push(&mut combined, p),
+                p @ Packet::SecretSubkey(_) => replace_or_push(&mut combined, p),
                 Packet::Signature(sig) => rop_sig(&mut combined, sig),
                 p => combined.push(p),
             }
@@ -5907,4 +5922,56 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g=
 
         Ok(())
     }
+
+    /// Make sure we can parse a key where the primary key is its own
+    /// subkeys.
+    #[test]
+    fn primary_key_is_subkey() -> Result<()> {
+        let p = &crate::policy::StandardPolicy::new();
+
+        let cert =
+            Cert::from_bytes(crate::tests::key("primary-key-is-also-subkey.pgp"))?;
+
+        // There should be three keys:
+        //
+        //     Fingerprint: 8E8C 33FA 4626 3379 76D9  7978 069C 0C34 8DD8 2C19
+        // Public-key algo: EdDSA Edwards-curve Digital Signature Algorithm
+        // Public-key size: 256 bits
+        //      Secret key: Unencrypted
+        //   Creation time: 2018-06-11 14:12:09 UTC
+        //       Key flags: certification, signing
+        //
+        //          Subkey: 8E8C 33FA 4626 3379 76D9  7978 069C 0C34 8DD8 2C19
+        // Public-key algo: EdDSA Edwards-curve Digital Signature Algorithm
+        // Public-key size: 256 bits
+        //      Secret key: Unencrypted
+        //   Creation time: 2018-06-11 14:12:09 UTC
+        //       Key flags: certification, signing
+        //
+        //          Subkey: 061C 3CA4 4AFF 0EC5 8DC6  6E95 22E3 FAFE 96B5 6C32
+        // Public-key algo: EdDSA Edwards-curve Digital Signature Algorithm
+        // Public-key size: 256 bits
+        //      Secret key: Unencrypted
+        //   Creation time: 2018-08-27 10:55:43 UTC
+        //       Key flags: signing
+        //
+        //          UserID: Emmelie Dorothea Dina Samantha Awina Ed25519
+        assert_eq!(cert.keys().count(), 3);
+
+        // Make sure there is a subkey with the same fingerprint as
+        // the primary key.
+        assert!(cert.keys().subkeys().any(|k| {
+            k.fingerprint() == cert.primary_key().fingerprint()
+        }));
+
+        // Make sure the self sig is valid, too.
+        assert_eq!(cert.keys().count(), 3);
+
+        let vc = cert.with_policy(p, None)?;
+        assert!(vc.keys().subkeys().any(|k| {
+            k.fingerprint() == vc.primary_key().fingerprint()
+        }));
+
+        Ok(())
+    }
 }
diff --git a/openpgp/tests/data/keys/primary-key-is-also-subkey.pgp b/openpgp/tests/data/keys/primary-key-is-also-subkey.pgp
new file mode 100644
index 00000000..f5975998
--- /dev/null
+++ b/openpgp/tests/data/keys/primary-key-is-also-subkey.pgp
@@ -0,0 +1,29 @@
+-----BEGIN PGP PUBLIC KEY BLOCK-----
+Comment: 8E8C 33FA 4626 3379 76D9  7978 069C 0C34 8DD8 2C19
+Comment: Emmelie Dorothea Dina Samantha Awina Ed25519
+
+xVgEWx6DORYJKwYBBAHaRw8BAQdABJa6xH6/nQoBQtVuqaenNLrKvkJ5gniGtBH3
+tsK+ckkAAP9uxXBqYoH/Kh+rjNMKRO6pgdkoYTYvMh5TVcQHR6LzoA+tzSxFbW1l
+bGllIERvcm90aGVhIERpbmEgU2FtYW50aGEgQXdpbmEgRWQyNTUxOcKQBBMWCAA4
+FiEEjowz+kYmM3l22Xl4BpwMNI3YLBkFAlsegzkCGwMFCwkIBwIGFQoJCAsCBBYC
+AwECHgECF4AACgkQBpwMNI3YLBlo5wD7B2CyTh/hEQOaZV56TqRpabY+zpCs2cTX
+7IjZnkEi5OAA/0WxAICvyJBkKIittgbnyQXml1UysgZ/Vv0dzNb+UgsPx1gEWx6D
+ORYJKwYBBAHaRw8BAQdABJa6xH6/nQoBQtVuqaenNLrKvkJ5gniGtBH3tsK+ckkA
+AP9uxXBqYoH/Kh+rjNMKRO6pgdkoYTYvMh5TVcQHR6LzoA+twsC/BBgWCgExBYJf
+4cXhCRAGnAw0jdgsGUcUAAAAAAAeACBzYWx0QG5vdGF0aW9ucy5zZXF1b2lhLXBn
+cC5vcme+n7H0EBLbDFbpvvx4eHLYbFeWacbznzObBC6WrksVTQKbA76gBBkWCgBv
+BYJf4cXhCRAGnAw0jdgsGUcUAAAAAAAeACBzYWx0QG5vdGF0aW9ucy5zZXF1b2lh
+LXBncC5vcmdRks7edU9bjyvlBTyKARbFpoiol8MYKqpt9A0Tim/rBxYhBI6MM/pG
+JjN5dtl5eAacDDSN2CwZAACzxgD8DfXRYsqoY0mHSoxr4uNgp5OdBzYy9/XHRLxu
+QAopyfkBAK31K6LNxHb2yopFvE/HI+p0csRTfoWLoMVRZBGg3x4LFiEEjowz+kYm
+M3l22Xl4BpwMNI3YLBkAANBLAQDHyejnvMLSfNveAesrKn624vMz5rs7nR7gnbC1
+WiFuXQEA8NenjjR3JBhUCdIjpybOX4kX/597P3rT/vtJRWanJQ3HWARbg9ivFgkr
+BgEEAdpHDwEBB0DQvCiWhC75FONjJ0v7ZCn92aZjXd8VQbva0TDclOwmvwABAIZC
+rDr8zd249g2I3GjcPXDZdbWPUgk+bf5nFvVvEMIeEMzCwC8EGBYIACAWIQSOjDP6
+RiYzeXbZeXgGnAw0jdgsGQUCW4PYrwIbAgCBCRAGnAw0jdgsGXYgBBkWCAAdFiEE
+Bhw8pEr/DsWNxm6VIuP6/pa1bDIFAluD2K8ACgkQIuP6/pa1bDJoTQD/cYH2EFRB
+ljjnT6DiPJYEJRoz5IAXgnKaOntXPA/9uCYBAN8po38vE9auBLpOM8QKNVISCGG3
+Y2bOe2BIQ8K25bkKJ4ABAN1KMV+Lb5Bdgh1xMvjGILyT+aVH3dIppj/mBlnHO3mr
+AP9RgDT1iuvJlwIaML8Hq/uaG1Ryd9rwfAt0tfqj0dY1Cw==
+=zKol
+-----END PGP PUBLIC KEY BLOCK-----