openpgp: Change general purpose keys to have a signing subkey.

  - Certificates with a primary key that is not signing capable, and a
    subkey that is, are strictly more secure than ones that combine
    signing and certification capabilities in the primary key.

  - If the owner of a certificate with a signing-capable primary key
    can be tricked into creating a binary signature over carefully
    chosen attacker-controlled data, this signature can be repurposed
    to bind arbitrary attacker-controlled components to the
    certificate using a chosen-prefix collision attack on the hash
    function (see e.g.  "SHA-1 is a Shambles" for a similar attack).

  - Having a separate signing-subkey mitigates the attack, because
    signatures by the signing subkey cannot bind components to the
    certificate.

4 files changed, 28 insertions, 11 deletions

diff --git a/openpgp/src/cert.rs b/openpgp/src/cert.rs
index fc548cd2..648be37a 100644
--- a/openpgp/src/cert.rs
+++ b/openpgp/src/cert.rs
@@ -4034,6 +4034,8 @@ mod test {
                    + 1 // binding signature
                    + 1 // subkey
                    + 1 // binding signature
+                   + 1 // subkey
+                   + 1 // binding signature
         );
         let cert = check_set_validity_period(p, cert);
         assert_eq!(cert.clone().into_packet_pile().children().count(),
@@ -4045,6 +4047,8 @@ mod test {
                    + 2 // two new binding signatures
                    + 1 // subkey
                    + 1 // binding signature
+                   + 1 // subkey
+                   + 1 // binding signature
         );
     }
 
@@ -4075,6 +4079,8 @@ mod test {
                    + 1 // binding signature
                    + 1 // subkey
                    + 1 // binding signature
+                   + 1 // subkey
+                   + 1 // binding signature
         );
         let cert = check_set_validity_period(p, cert);
         assert_eq!(cert.clone().into_packet_pile().children().count(),
@@ -4089,6 +4095,8 @@ mod test {
                    + 2 // two new binding signatures
                    + 1 // subkey
                    + 1 // binding signature
+                   + 1 // subkey
+                   + 1 // binding signature
         );
         assert_eq!(&primary_uid, cert.with_policy(p, None)?.primary_userid()?.userid());
         Ok(())
@@ -5674,11 +5682,12 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g=
         let (cert, _) = CertBuilder::general_purpose(
             None, Some("alice@example.org")).generate().unwrap();
         assert_eq!(cert.userids().count(), 1);
-        assert_eq!(cert.subkeys().count(), 1);
+        assert_eq!(cert.subkeys().count(), 2);
         assert_eq!(cert.unknowns().count(), 0);
         assert_eq!(cert.bad_signatures().count(), 0);
         assert_eq!(cert.userids().nth(0).unwrap().self_signatures().len(), 1);
         assert_eq!(cert.subkeys().nth(0).unwrap().self_signatures().len(), 1);
+        assert_eq!(cert.subkeys().nth(1).unwrap().self_signatures().len(), 1);
 
         // Create a variant of cert where the signatures have
         // additional information in the unhashed area.
@@ -5698,11 +5707,12 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g=
         let cert_b = Cert::from_packets(packets.into_iter())?;
         let cert = cert.merge_public_and_secret(cert_b)?;
         assert_eq!(cert.userids().count(), 1);
-        assert_eq!(cert.subkeys().count(), 1);
+        assert_eq!(cert.subkeys().count(), 2);
         assert_eq!(cert.unknowns().count(), 0);
         assert_eq!(cert.bad_signatures().count(), 0);
         assert_eq!(cert.userids().nth(0).unwrap().self_signatures().len(), 1);
         assert_eq!(cert.subkeys().nth(0).unwrap().self_signatures().len(), 1);
+        assert_eq!(cert.subkeys().nth(1).unwrap().self_signatures().len(), 1);
 
         Ok(())
     }
diff --git a/openpgp/src/cert/amalgamation/key.rs b/openpgp/src/cert/amalgamation/key.rs
index f8fbaa01..5962ea4b 100644
--- a/openpgp/src/cert/amalgamation/key.rs
+++ b/openpgp/src/cert/amalgamation/key.rs
@@ -154,7 +154,8 @@
 //! #         CertBuilder::general_purpose(None, Some("alice@example.org"))
 //! #         .generate()?;
 //! #     let timestamp = None;
-//! #     let issuer = cert.fingerprint();
+//! #     let issuer = cert.with_policy(p, None)?.keys()
+//! #         .for_signing().nth(0).unwrap().fingerprint();
 //! #     let mut i = 0;
 //! let cert = cert.with_policy(p, timestamp)?;
 //! if let RevocationStatus::Revoked(_) = cert.revocation_status() {
diff --git a/openpgp/src/cert/amalgamation/key/iter.rs b/openpgp/src/cert/amalgamation/key/iter.rs
index 0601c2f0..53a72f79 100644
--- a/openpgp/src/cert/amalgamation/key/iter.rs
+++ b/openpgp/src/cert/amalgamation/key/iter.rs
@@ -434,7 +434,7 @@ impl<'a, P, R> KeyAmalgamationIter<'a, P, R>
     ///     // Use it.
     /// #   i += 1;
     /// }
-    /// # assert_eq!(i, 2);
+    /// # assert_eq!(i, 3);
     /// # Ok(()) }
     /// ```
     pub fn supported(mut self) -> Self {
@@ -1553,7 +1553,7 @@ impl<'a, P, R> ValidKeyAmalgamationIter<'a, P, R>
     ///     // Use it.
     /// #   i += 1;
     /// }
-    /// # assert_eq!(i, 2);
+    /// # assert_eq!(i, 3);
     /// # Ok(()) }
     /// ```
     pub fn supported(mut self) -> Self {
diff --git a/openpgp/src/cert/builder.rs b/openpgp/src/cert/builder.rs
index 15d0a350..627bd4f0 100644
--- a/openpgp/src/cert/builder.rs
+++ b/openpgp/src/cert/builder.rs
@@ -233,9 +233,10 @@ impl CertBuilder<'_> {
     /// Generates a general-purpose certificate.
     ///
     /// The returned builder is set to generate a certificate with a
-    /// certification- and signature-capable primary key, and an
-    /// encryption-capable subkey.  The subkey is marked as being
-    /// appropriate for both data in transit and data at rest.
+    /// certification-capable primary key, a signing-capable subkye,
+    /// and an encryption-capable subkey.  The encryption subkey is
+    /// marked as being appropriate for both data in transit and data
+    /// at rest.
     ///
     /// # Examples
     ///
@@ -248,7 +249,7 @@ impl CertBuilder<'_> {
     ///     CertBuilder::general_purpose(None,
     ///                                  Some("Alice Lovelace <alice@example.org>"))
     ///         .generate()?;
-    /// # assert_eq!(cert.keys().count(), 2);
+    /// # assert_eq!(cert.keys().count(), 3);
     /// # assert_eq!(cert.userids().count(), 1);
     /// # Ok(())
     /// # }
@@ -262,14 +263,19 @@ impl CertBuilder<'_> {
             ciphersuite: ciphersuite.into().unwrap_or(Default::default()),
             primary: KeyBlueprint {
                 flags: KeyFlags::empty()
-                    .set_certification()
-                    .set_signing(),
+                    .set_certification(),
                 validity: Some(time::Duration::new(3 * 52 * 7 * 24 * 60 * 60, 0)),
                 ciphersuite: None,
             },
             subkeys: vec![
                 KeyBlueprint {
                     flags: KeyFlags::empty()
+                        .set_signing(),
+                    validity: None,
+                    ciphersuite: None,
+                },
+                KeyBlueprint {
+                    flags: KeyFlags::empty()
                         .set_transport_encryption()
                         .set_storage_encryption(),
                     validity: None,