openpgp: Drop hash algorithm parameter from signing functions.

  - This adds and promotes the use of
    signature::Builder::set_hash_algo instead.  Adapt callsites.

  - In particular, document why we use SHA512 when creating signatures
    in the TPK builder.

7 files changed, 89 insertions, 87 deletions

diff --git a/openpgp/src/packet/signature/mod.rs b/openpgp/src/packet/signature/mod.rs
index e5c5e21f..fb1a1224 100644
--- a/openpgp/src/packet/signature/mod.rs
+++ b/openpgp/src/packet/signature/mod.rs
@@ -75,7 +75,7 @@ impl Builder {
             version: 4,
             typ: typ,
             pk_algo: PublicKeyAlgorithm::Unknown(0),
-            hash_algo: HashAlgorithm::Unknown(0),
+            hash_algo: HashAlgorithm::default(),
             subpackets: SubpacketAreas::empty(),
         }
     }
@@ -106,26 +106,28 @@ impl Builder {
         self.hash_algo
     }
 
+    /// Sets the hash algorithm.
+    pub fn set_hash_algo(mut self, h: HashAlgorithm) -> Self {
+        self.hash_algo = h;
+        self
+    }
+
     /// Creates a standalone signature.
-    pub fn sign_standalone<R>(mut self, signer: &mut dyn Signer<R>,
-                              algo: HashAlgorithm)
+    pub fn sign_standalone<R>(mut self, signer: &mut dyn Signer<R>)
                               -> Result<Signature>
         where R: key::KeyRole
     {
         self.pk_algo = signer.public().pk_algo();
-        self.hash_algo = algo;
         let digest = Signature::standalone_hash(&self)?;
         self.sign(signer, digest)
     }
 
     /// Creates a timestamp signature.
-    pub fn sign_timestamp<R>(mut self, signer: &mut dyn Signer<R>,
-                              algo: HashAlgorithm)
+    pub fn sign_timestamp<R>(mut self, signer: &mut dyn Signer<R>)
                               -> Result<Signature>
         where R: key::KeyRole
     {
         self.pk_algo = signer.public().pk_algo();
-        self.hash_algo = algo;
         let digest = Signature::timestamp_hash(&self)?;
         self.sign(signer, digest)
     }
@@ -135,13 +137,11 @@ impl Builder {
     /// The Signature's public-key algorithm field is set to the
     /// algorithm used by `signer`, the hash-algorithm field is set to
     /// `hash_algo`.
-    pub fn sign_primary_key_binding<R>(mut self, signer: &mut dyn Signer<R>,
-                                       algo: HashAlgorithm)
+    pub fn sign_primary_key_binding<R>(mut self, signer: &mut dyn Signer<R>)
         -> Result<Signature>
         where R: key::KeyRole
     {
         self.pk_algo = signer.public().pk_algo();
-        self.hash_algo = algo;
         let digest =
             Signature::primary_key_binding_hash(&self,
                                                 signer.public()
@@ -157,13 +157,11 @@ impl Builder {
     /// `hash_algo`.
     pub fn sign_userid_binding<R>(mut self, signer: &mut dyn Signer<R>,
                                  key: &key::PublicKey,
-                                 userid: &UserID,
-                                 algo: HashAlgorithm)
+                                 userid: &UserID)
         -> Result<Signature>
         where R: key::KeyRole
     {
         self.pk_algo = signer.public().pk_algo();
-        self.hash_algo = algo;
         let digest = Signature::userid_binding_hash(&self, key, userid)?;
 
         self.sign(signer, digest)
@@ -176,13 +174,11 @@ impl Builder {
     /// `hash_algo`.
     pub fn sign_subkey_binding<R>(mut self, signer: &mut dyn Signer<R>,
                                   primary: &key::PublicKey,
-                                  subkey: &key::PublicSubkey,
-                                  algo: HashAlgorithm)
+                                  subkey: &key::PublicSubkey)
         -> Result<Signature>
         where R: key::KeyRole
     {
         self.pk_algo = signer.public().pk_algo();
-        self.hash_algo = algo;
         let digest = Signature::subkey_binding_hash(&self, primary, subkey)?;
 
         self.sign(signer, digest)
@@ -195,13 +191,11 @@ impl Builder {
     /// `hash_algo`.
     pub fn sign_user_attribute_binding<R>(mut self, signer: &mut dyn Signer<R>,
                                           key: &key::PublicKey,
-                                          ua: &UserAttribute,
-                                          algo: HashAlgorithm)
+                                          ua: &UserAttribute)
         -> Result<Signature>
         where R: key::KeyRole
     {
         self.pk_algo = signer.public().pk_algo();
-        self.hash_algo = algo;
         let digest =
             Signature::user_attribute_binding_hash(&self, key, ua)?;
 
@@ -235,18 +229,16 @@ impl Builder {
     /// The Signature's public-key algorithm field is set to the
     /// algorithm used by `signer`, the hash-algorithm field is set to
     /// `hash_algo`.
-    pub fn sign_message<R>(mut self, signer: &mut dyn Signer<R>,
-                           hash_algo: HashAlgorithm, msg: &[u8])
+    pub fn sign_message<R>(mut self, signer: &mut dyn Signer<R>, msg: &[u8])
         -> Result<Signature>
         where R: key::KeyRole
     {
         // Hash the message
-        let mut hash = hash_algo.context()?;
+        let mut hash = self.hash_algo.context()?;
         hash.update(msg);
 
         // Fill out some fields, then hash the packet.
         self.pk_algo = signer.public().pk_algo();
-        self.hash_algo = hash_algo;
         self.hash(&mut hash);
 
         // Compute the digest.
@@ -1304,7 +1296,7 @@ mod test {
             .set_signature_creation_time(time::now()).unwrap()
             .set_issuer_fingerprint(pair.public().fingerprint()).unwrap()
             .set_issuer(pair.public().keyid()).unwrap()
-            .sign_message(&mut pair, HashAlgorithm::SHA512, msg).unwrap();
+            .sign_message(&mut pair, msg).unwrap();
 
         assert!(sig.verify_message(pair.public(), msg).unwrap());
     }
@@ -1452,7 +1444,7 @@ mod test {
             .set_signature_creation_time(time::now()).unwrap()
             .set_issuer_fingerprint(pair.public().fingerprint()).unwrap()
             .set_issuer(pair.public().keyid()).unwrap()
-            .sign_standalone(&mut pair, HashAlgorithm::SHA256)
+            .sign_standalone(&mut pair)
             .unwrap();
 
         assert!(sig.verify_standalone(pair.public()).unwrap());
@@ -1483,7 +1475,7 @@ mod test {
             .set_signature_creation_time(time::now()).unwrap()
             .set_issuer_fingerprint(pair.public().fingerprint()).unwrap()
             .set_issuer(pair.public().keyid()).unwrap()
-            .sign_timestamp(&mut pair, HashAlgorithm::SHA256)
+            .sign_timestamp(&mut pair)
             .unwrap();
 
         assert!(sig.verify_timestamp(pair.public()).unwrap());
diff --git a/openpgp/src/serialize/mod.rs b/openpgp/src/serialize/mod.rs
index 9a810c42..d37c0c7e 100644
--- a/openpgp/src/serialize/mod.rs
+++ b/openpgp/src/serialize/mod.rs
@@ -2945,7 +2945,7 @@ mod test {
         let sig = uid.bind(
             &mut keypair, &tpk,
             signature::Builder::new(SignatureType::GenericCertificate),
-            None, None).unwrap();
+            None).unwrap();
 
         // The signature is exportable.  Try to export it in
         // various ways.
@@ -2970,7 +2970,7 @@ mod test {
             &mut keypair, &tpk,
             signature::Builder::new(SignatureType::GenericCertificate)
                 .set_exportable_certification(true).unwrap(),
-            None, None).unwrap();
+            None).unwrap();
 
         // The signature is exportable.  Try to export it in
         // various ways.
@@ -2995,7 +2995,7 @@ mod test {
             &mut keypair, &tpk,
             signature::Builder::new(SignatureType::GenericCertificate)
                 .set_exportable_certification(false).unwrap(),
-            None, None).unwrap();
+            None).unwrap();
 
         // The signature is not exportable.  Try to export it in
         // various ways.
diff --git a/openpgp/src/serialize/tpk.rs b/openpgp/src/serialize/tpk.rs
index 520cb684..bd655559 100644
--- a/openpgp/src/serialize/tpk.rs
+++ b/openpgp/src/serialize/tpk.rs
@@ -739,7 +739,7 @@ mod test {
                     &KeyFlags::default().set_encrypt_for_transport(true))
                 .unwrap()
                 .set_exportable_certification(false).unwrap(),
-            None, None).unwrap();
+            None).unwrap();
 
         let uid = UserID::from("foo");
         let uid_binding = uid.bind(
@@ -748,7 +748,7 @@ mod test {
                 tpk.primary_key_signature(None).unwrap().clone())
                 .set_type(SignatureType::PositiveCertificate)
                 .set_exportable_certification(false).unwrap(),
-            None, None).unwrap();
+            None).unwrap();
 
         let ua = UserAttribute::new(&[
             Subpacket::Unknown(2, b"foo".to_vec().into_boxed_slice()),
@@ -759,7 +759,7 @@ mod test {
                 tpk.primary_key_signature(None).unwrap().clone())
                 .set_type(SignatureType::PositiveCertificate)
                 .set_exportable_certification(false).unwrap(),
-            None, None).unwrap();
+            None).unwrap();
 
         let tpk = tpk.merge_packets(vec![
             Packet::SecretSubkey(key), key_binding.into(),
diff --git a/openpgp/src/tpk/bindings.rs b/openpgp/src/tpk/bindings.rs
index b192d37f..0de11543 100644
--- a/openpgp/src/tpk/bindings.rs
+++ b/openpgp/src/tpk/bindings.rs
@@ -41,7 +41,7 @@ impl Key<key::PublicParts, key::SubordinateRole> {
     ///     = Key::V4(Key4::generate_ecc(false, Curve::Cv25519)?);
     /// let builder = signature::Builder::new(SignatureType::SubkeyBinding)
     ///     .set_key_flags(&flags)?;
-    /// let binding = subkey.bind(&mut keypair, &tpk, builder, None, None)?;
+    /// let binding = subkey.bind(&mut keypair, &tpk, builder, None)?;
     ///
     /// // Now merge the key and binding signature into the TPK.
     /// let tpk = tpk.merge_packets(vec![subkey.into(),
@@ -50,12 +50,11 @@ impl Key<key::PublicParts, key::SubordinateRole> {
     /// // Check that we have an encryption subkey.
     /// assert_eq!(tpk.keys_valid().key_flags(flags).count(), 1);
     /// # Ok(()) }
-    pub fn bind<H, T, R>(&self, signer: &mut dyn Signer<R>, tpk: &TPK,
-                         signature: signature::Builder,
-                         hash_algo: H, creation_time: T)
+    pub fn bind<T, R>(&self, signer: &mut dyn Signer<R>, tpk: &TPK,
+                      signature: signature::Builder,
+                      creation_time: T)
         -> Result<Signature>
-        where H: Into<Option<HashAlgorithm>>,
-              T: Into<Option<time::Tm>>,
+        where T: Into<Option<time::Tm>>,
               R: key::KeyRole
     {
         signature
@@ -63,9 +62,7 @@ impl Key<key::PublicParts, key::SubordinateRole> {
                 creation_time.into().unwrap_or_else(time::now_utc))?
             .set_issuer_fingerprint(signer.public().fingerprint())?
             .set_issuer(signer.public().keyid())?
-            .sign_subkey_binding(
-                signer, tpk.primary(), self,
-                hash_algo.into().unwrap_or(HashAlgorithm::SHA512))
+            .sign_subkey_binding(signer, tpk.primary(), self)
     }
 }
 
@@ -101,7 +98,7 @@ impl UserID {
     /// let userid = UserID::from("test@example.org");
     /// let builder =
     ///     signature::Builder::new(SignatureType::PositiveCertificate);
-    /// let binding = userid.bind(&mut keypair, &tpk, builder, None, None)?;
+    /// let binding = userid.bind(&mut keypair, &tpk, builder, None)?;
     ///
     /// // Now merge the userid and binding signature into the TPK.
     /// let tpk = tpk.merge_packets(vec![userid.into(), binding.into()])?;
@@ -109,12 +106,11 @@ impl UserID {
     /// // Check that we have a userid.
     /// assert_eq!(tpk.userids().len(), 1);
     /// # Ok(()) }
-    pub fn bind<H, T, R>(&self, signer: &mut dyn Signer<R>, tpk: &TPK,
+    pub fn bind<T, R>(&self, signer: &mut dyn Signer<R>, tpk: &TPK,
                       signature: signature::Builder,
-                      hash_algo: H, creation_time: T)
+                      creation_time: T)
                       -> Result<Signature>
-        where H: Into<Option<HashAlgorithm>>,
-              T: Into<Option<time::Tm>>,
+        where T: Into<Option<time::Tm>>,
               R: key::KeyRole
     {
         signature
@@ -123,8 +119,7 @@ impl UserID {
             .set_issuer_fingerprint(signer.public().fingerprint())?
             .set_issuer(signer.public().keyid())?
             .sign_userid_binding(
-                signer, tpk.primary(), self,
-                hash_algo.into().unwrap_or(HashAlgorithm::SHA512))
+                signer, tpk.primary(), self)
     }
 
     /// Returns a certificate for the user id.
@@ -198,10 +193,13 @@ impl UserID {
                 format!("Invalid signature type: {}", t)).into()),
             None => SignatureType::GenericCertificate,
         };
-        self.bind(signer, tpk, signature::Builder::new(typ),
+        let mut sig = signature::Builder::new(typ);
+        if let Some(algo) = hash_algo.into() {
+            sig = sig.set_hash_algo(algo);
+        }
+        self.bind(signer, tpk, sig,
                   // Unwrap arguments to prevent further
                   // monomorphization of bind().
-                  hash_algo.into().unwrap_or(HashAlgorithm::SHA512),
                   creation_time.into().unwrap_or_else(time::now_utc))
     }
 }
@@ -243,7 +241,7 @@ impl UserAttribute {
     /// ])?;
     /// let builder =
     ///     signature::Builder::new(SignatureType::PositiveCertificate);
-    /// let binding = user_attr.bind(&mut keypair, &tpk, builder, None, None)?;
+    /// let binding = user_attr.bind(&mut keypair, &tpk, builder, None)?;
     ///
     /// // Now merge the user attribute and binding signature into the TPK.
     /// let tpk = tpk.merge_packets(vec![user_attr.into(), binding.into()])?;
@@ -251,12 +249,11 @@ impl UserAttribute {
     /// // Check that we have a user attribute.
     /// assert_eq!(tpk.user_attributes().len(), 1);
     /// # Ok(()) }
-    pub fn bind<H, T, R>(&self, signer: &mut dyn Signer<R>, tpk: &TPK,
-                         signature: signature::Builder,
-                         hash_algo: H, creation_time: T)
+    pub fn bind<T, R>(&self, signer: &mut dyn Signer<R>, tpk: &TPK,
+                      signature: signature::Builder,
+                      creation_time: T)
         -> Result<Signature>
-        where H: Into<Option<HashAlgorithm>>,
-              T: Into<Option<time::Tm>>,
+        where T: Into<Option<time::Tm>>,
               R: key::KeyRole
     {
         signature
@@ -264,9 +261,7 @@ impl UserAttribute {
                 creation_time.into().unwrap_or_else(time::now_utc))?
             .set_issuer_fingerprint(signer.public().fingerprint())?
             .set_issuer(signer.public().keyid())?
-            .sign_user_attribute_binding(
-                signer, tpk.primary(), self,
-                hash_algo.into().unwrap_or(HashAlgorithm::SHA512))
+            .sign_user_attribute_binding(signer, tpk.primary(), self)
     }
 
     /// Returns a certificate for the user attribute.
@@ -345,10 +340,13 @@ impl UserAttribute {
                 format!("Invalid signature type: {}", t)).into()),
             None => SignatureType::GenericCertificate,
         };
-        self.bind(signer, tpk, signature::Builder::new(typ),
+        let mut sig = signature::Builder::new(typ);
+        if let Some(algo) = hash_algo.into() {
+            sig = sig.set_hash_algo(algo);
+        }
+        self.bind(signer, tpk, sig,
                   // Unwrap arguments to prevent further
                   // monomorphization of bind().
-                  hash_algo.into().unwrap_or(HashAlgorithm::SHA512),
                   creation_time.into().unwrap_or_else(time::now_utc))
     }
 }
diff --git a/openpgp/src/tpk/builder.rs b/openpgp/src/tpk/builder.rs
index 1a52c1fa..704ca67d 100644
--- a/openpgp/src/tpk/builder.rs
+++ b/openpgp/src/tpk/builder.rs
@@ -321,16 +321,20 @@ impl TPKBuilder {
         // Sign UserIDs.
         for uid in self.userids.into_iter() {
             let builder = signature::Builder::from(sig.clone())
-                .set_type(SignatureType::PositiveCertificate);
-            let signature = uid.bind(&mut signer, &tpk, builder, None, None)?;
+                .set_type(SignatureType::PositiveCertificate)
+                // GnuPG wants at least a 512-bit hash for P521 keys.
+                .set_hash_algo(HashAlgorithm::SHA512);
+            let signature = uid.bind(&mut signer, &tpk, builder, None)?;
             tpk = tpk.merge_packets(vec![uid.into(), signature.into()])?;
         }
 
         // Sign UserAttributes.
         for ua in self.user_attributes.into_iter() {
             let builder = signature::Builder::from(sig.clone())
-                .set_type(SignatureType::PositiveCertificate);
-            let signature = ua.bind(&mut signer, &tpk, builder, None, None)?;
+                .set_type(SignatureType::PositiveCertificate)
+            // GnuPG wants at least a 512-bit hash for P521 keys.
+                .set_hash_algo(HashAlgorithm::SHA512);
+            let signature = ua.bind(&mut signer, &tpk, builder, None)?;
             tpk = tpk.merge_packets(vec![ua.into(), signature.into()])?;
         }
 
@@ -341,6 +345,8 @@ impl TPKBuilder {
 
             let mut builder =
                 signature::Builder::new(SignatureType::SubkeyBinding)
+                // GnuPG wants at least a 512-bit hash for P521 keys.
+                .set_hash_algo(HashAlgorithm::SHA512)
                 .set_features(&Features::sequoia())?
                 .set_key_flags(flags)?
                 .set_key_expiration_time(self.expiration)?;
@@ -361,17 +367,18 @@ impl TPKBuilder {
                 let mut subkey_signer = subkey.clone().into_keypair().unwrap();
                 let backsig =
                     signature::Builder::new(SignatureType::PrimaryKeyBinding)
+                    // GnuPG wants at least a 512-bit hash for P521 keys.
+                    .set_hash_algo(HashAlgorithm::SHA512)
                     .set_signature_creation_time(time::now().canonicalize())?
                     .set_issuer_fingerprint(subkey.fingerprint())?
                     .set_issuer(subkey.keyid())?
                     .sign_subkey_binding(&mut subkey_signer, &primary,
-                                         &subkey.mark_parts_public_ref(),
-                                         HashAlgorithm::SHA512)?;
+                                         &subkey.mark_parts_public_ref())?;
                 builder = builder.set_embedded_signature(backsig)?;
             }
 
             let signature = subkey.mark_parts_public_ref()
-                .bind(&mut signer, &tpk, builder, None, None)?;
+                .bind(&mut signer, &tpk, builder, None)?;
 
             if let Some(ref password) = self.password {
                 subkey.secret_mut().unwrap().encrypt_in_place(password)?;
@@ -398,6 +405,8 @@ impl TPKBuilder {
         let key = self.ciphersuite.generate_key(
             &KeyFlags::default().set_certify(true))?;
         let sig = signature::Builder::new(SignatureType::DirectKey)
+            // GnuPG wants at least a 512-bit hash for P521 keys.
+            .set_hash_algo(HashAlgorithm::SHA512)
             .set_features(&Features::sequoia())?
             .set_key_flags(&self.primary.flags)?
             .set_signature_creation_time(time::now().canonicalize())?
@@ -408,8 +417,7 @@ impl TPKBuilder {
 
         let mut signer = key.clone().into_keypair()
             .expect("key generated above has a secret");
-        let sig = sig.sign_primary_key_binding(&mut signer,
-                                               HashAlgorithm::SHA512)?;
+        let sig = sig.sign_primary_key_binding(&mut signer)?;
 
         Ok((key.mark_parts_public(), sig.into()))
     }
diff --git a/openpgp/src/tpk/mod.rs b/openpgp/src/tpk/mod.rs
index bc6e8ed0..8e263da7 100644
--- a/openpgp/src/tpk/mod.rs
+++ b/openpgp/src/tpk/mod.rs
@@ -2382,8 +2382,7 @@ mod test {
                 .set_issuer_fingerprint(key.fingerprint()).unwrap()
                 .set_issuer(key.keyid()).unwrap()
                 .set_preferred_hash_algorithms(vec![HashAlgorithm::SHA512]).unwrap()
-                .sign_primary_key_binding(&mut pair,
-                                          HashAlgorithm::SHA512).unwrap();
+                .sign_primary_key_binding(&mut pair).unwrap();
 
             let rev1 = signature::Builder::new(SignatureType::KeyRevocation)
                 .set_signature_creation_time(t2).unwrap()
@@ -2391,8 +2390,7 @@ mod test {
                                            &b""[..]).unwrap()
                 .set_issuer_fingerprint(key.fingerprint()).unwrap()
                 .set_issuer(key.keyid()).unwrap()
-                .sign_primary_key_binding(&mut pair,
-                                          HashAlgorithm::SHA512).unwrap();
+                .sign_primary_key_binding(&mut pair).unwrap();
 
             let bind2 = signature::Builder::new(SignatureType::DirectKey)
                 .set_features(&Features::sequoia()).unwrap()
@@ -2402,8 +2400,7 @@ mod test {
                 .set_issuer_fingerprint(key.fingerprint()).unwrap()
                 .set_issuer(key.keyid()).unwrap()
                 .set_preferred_hash_algorithms(vec![HashAlgorithm::SHA512]).unwrap()
-                .sign_primary_key_binding(&mut pair,
-                                          HashAlgorithm::SHA512).unwrap();
+                .sign_primary_key_binding(&mut pair).unwrap();
 
             let rev2 = signature::Builder::new(SignatureType::KeyRevocation)
                 .set_signature_creation_time(t4).unwrap()
@@ -2411,8 +2408,7 @@ mod test {
                                            &b""[..]).unwrap()
                 .set_issuer_fingerprint(key.fingerprint()).unwrap()
                 .set_issuer(key.keyid()).unwrap()
-                .sign_primary_key_binding(&mut pair,
-                                          HashAlgorithm::SHA512).unwrap();
+                .sign_primary_key_binding(&mut pair).unwrap();
 
             (bind1, rev1, bind2, rev2)
         };
@@ -3012,8 +3008,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g=
                     .set_issuer(key.keyid()).unwrap()
                     .set_preferred_hash_algorithms(vec![HashAlgorithm::SHA512]).unwrap()
                     .set_signature_creation_time(*t).unwrap()
-                    .sign_primary_key_binding(&mut pair,
-                                              HashAlgorithm::SHA512).unwrap();
+                    .sign_primary_key_binding(&mut pair).unwrap();
 
                 let binding : Packet = binding.into();
 
@@ -3074,7 +3069,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g=
                         .into_keypair().unwrap(),
                     &bob,
                     sig_template,
-                    None, None).unwrap();
+                    None).unwrap();
 
             let bob
                 = bob.merge_packets(vec![ alice_certifies_bob.clone().into() ])
diff --git a/openpgp/src/tpk/revoke.rs b/openpgp/src/tpk/revoke.rs
index 2d2b622e..9b4cbb01 100644
--- a/openpgp/src/tpk/revoke.rs
+++ b/openpgp/src/tpk/revoke.rs
@@ -219,7 +219,7 @@ impl SubkeyRevocationBuilder {
 
     /// Returns a revocation certificate for the tpk `TPK` signed by
     /// `signer`.
-    pub fn build<H, R>(self, signer: &mut dyn Signer<R>,
+    pub fn build<H, R>(mut self, signer: &mut dyn Signer<R>,
                        tpk: &TPK, key: &key::PublicSubkey,
                        hash_algo: H)
         -> Result<Signature>
@@ -230,7 +230,10 @@ impl SubkeyRevocationBuilder {
         let creation_time
             = self.signature_creation_time().unwrap_or_else(time::now);
 
-        key.bind(signer, tpk, self.builder, hash_algo, creation_time)
+        if let Some(algo) = hash_algo.into() {
+            self.builder = self.builder.set_hash_algo(algo);
+        }
+        key.bind(signer, tpk, self.builder, creation_time)
     }
 }
 
@@ -331,7 +334,7 @@ impl UserIDRevocationBuilder {
 
     /// Returns a revocation certificate for the tpk `TPK` signed by
     /// `signer`.
-    pub fn build<H, R>(self, signer: &mut dyn Signer<R>,
+    pub fn build<H, R>(mut self, signer: &mut dyn Signer<R>,
                        tpk: &TPK, userid: &UserID,
                        hash_algo: H)
         -> Result<Signature>
@@ -342,7 +345,10 @@ impl UserIDRevocationBuilder {
         let creation_time
             = self.signature_creation_time().unwrap_or_else(time::now);
 
-        userid.bind(signer, tpk, self.builder, hash_algo, creation_time)
+        if let Some(algo) = hash_algo.into() {
+            self.builder = self.builder.set_hash_algo(algo);
+        }
+        userid.bind(signer, tpk, self.builder, creation_time)
     }
 }
 
@@ -446,7 +452,7 @@ impl UserAttributeRevocationBuilder {
 
     /// Returns a revocation certificate for the tpk `TPK` signed by
     /// `signer`.
-    pub fn build<H, R>(self, signer: &mut dyn Signer<R>,
+    pub fn build<H, R>(mut self, signer: &mut dyn Signer<R>,
                        tpk: &TPK, ua: &UserAttribute,
                        hash_algo: H)
         -> Result<Signature>
@@ -457,7 +463,10 @@ impl UserAttributeRevocationBuilder {
         let creation_time
             = self.signature_creation_time().unwrap_or_else(time::now);
 
-        ua.bind(signer, tpk, self.builder, hash_algo, creation_time)
+        if let Some(algo) = hash_algo.into() {
+            self.builder = self.builder.set_hash_algo(algo);
+        }
+        ua.bind(signer, tpk, self.builder, creation_time)
     }
 }