openpgp: Change how SignatureBuilder emits the sig's creation time.

  - Currently, `SignatureBuilder` sets the `Signature Creation Time`
    subpacket when it is instantiated, or uses any existing `Signature
    Creation Time` when it is converted using `From`.

  - This can be problematic: usually, we want the current time.

  - Change the behavior to emit a `Signature Creation Time` subpakcet
    when the signature is generated, unless this behavior is
    explicitly overridden.

4 files changed, 66 insertions, 6 deletions

diff --git a/openpgp/src/cert/builder.rs b/openpgp/src/cert/builder.rs
index 4243cb2f..23c84792 100644
--- a/openpgp/src/cert/builder.rs
+++ b/openpgp/src/cert/builder.rs
@@ -923,7 +923,7 @@ impl CertBuilder {
             self.primary.flags = self.primary.flags.set_certification(true);
         }
 
-        // Generate & and self-sign primary key.
+        // Generate & self-sign primary key.
         let (primary, sig) = self.primary_key(creation_time)?;
         let mut signer = primary.clone().into_keypair().unwrap();
 
@@ -936,7 +936,8 @@ impl CertBuilder {
         }));
         packets.push(sig.clone().into());
 
-        let sig = signature::SignatureBuilder::from(sig.clone());
+        let sig = signature::SignatureBuilder::from(sig.clone())
+            .set_signature_creation_time(creation_time)?;
 
         // Remove subpackets that needn't be copied into the binding
         // signatures.
diff --git a/openpgp/src/packet/signature/mod.rs b/openpgp/src/packet/signature/mod.rs
index 2e144947..45d61dc3 100644
--- a/openpgp/src/packet/signature/mod.rs
+++ b/openpgp/src/packet/signature/mod.rs
@@ -172,6 +172,7 @@ impl SignatureFields {
 // IMPORTANT: implement PartialEq, Eq, and Hash.
 #[derive(Clone, Hash, PartialEq, Eq)]
 pub struct SignatureBuilder {
+    overrode_creation_time: bool,
     fields: SignatureFields,
 }
 
@@ -193,6 +194,7 @@ impl SignatureBuilder {
     /// Returns a new `SignatureBuilder` object.
     pub fn new(typ: SignatureType) ->  Self {
         SignatureBuilder {
+            overrode_creation_time: false,
             fields: SignatureFields {
                 version: 4,
                 typ,
@@ -201,10 +203,6 @@ impl SignatureBuilder {
                 subpackets: SubpacketAreas::default(),
             }
         }
-        .set_signature_creation_time(
-            std::time::SystemTime::now())
-            .expect("area is empty, insertion cannot fail; \
-                     time is representable for the foreseeable future")
     }
 
     /// Sets the signature type.
@@ -400,6 +398,13 @@ impl SignatureBuilder {
     fn pre_sign(mut self, signer: &dyn Signer) -> Result<Self> {
         self.pk_algo = signer.public().pk_algo();
 
+        // Set the creation time.
+        if ! self.overrode_creation_time {
+            self = self.set_signature_creation_time(
+                std::time::SystemTime::now())?;
+        }
+
+        // Make sure we have an issuer packet.
         if self.issuer().is_none() && self.issuer_fingerprint().is_none() {
             self = self.set_issuer(signer.public().keyid())?
                 .set_issuer_fingerprint(signer.public().fingerprint())?;
@@ -438,6 +443,7 @@ impl From<Signature> for SignatureBuilder {
 impl From<Signature4> for SignatureBuilder {
     fn from(sig: Signature4) -> Self {
         SignatureBuilder {
+            overrode_creation_time: false,
             fields: sig.fields,
         }
     }
diff --git a/openpgp/src/packet/signature/subpacket.rs b/openpgp/src/packet/signature/subpacket.rs
index 84d32c9d..0a2410aa 100644
--- a/openpgp/src/packet/signature/subpacket.rs
+++ b/openpgp/src/packet/signature/subpacket.rs
@@ -2006,6 +2006,8 @@ impl signature::SignatureBuilder {
                                           -> Result<Self>
         where T: Into<time::SystemTime>
     {
+        self.overrode_creation_time = true;
+
         self.hashed_area.replace(Subpacket::new(
             SubpacketValue::SignatureCreationTime(
                 creation_time.into().try_into()?),
@@ -2014,6 +2016,55 @@ impl signature::SignatureBuilder {
         Ok(self)
     }
 
+    /// Causes the builder to use an existing signature creation time
+    /// subpacket.
+    ///
+    /// Unless `SignatureBuilder::set_signature_creation_time` has
+    /// been called, `SignatureBuilder` sets the
+    /// `SignatureCreationTime` subpacket when the signature is
+    /// generated.  Calling this function causes the signature
+    /// generation code to use the existing `Signature Creation Time`
+    /// subpacket.
+    ///
+    /// This function returns an error if there is no `Signature
+    /// Creation Time` subpacket in the hashed area.
+    pub fn preserve_signature_creation_time(mut self)
+        -> Result<Self>
+    {
+        self.overrode_creation_time = true;
+
+        if self.hashed_area.lookup(SubpacketTag::SignatureCreationTime).is_none() {
+            Err(Error::InvalidOperation(
+                "Signature does not contain a Signature Creation Time subpacket".into())
+                .into())
+        } else {
+            Ok(self)
+        }
+    }
+
+    /// Causes the builder to not output a signature creation time
+    /// subpacket.
+    ///
+    /// [Section 5.2.3.4 of RFC 4880] says that the `Signature
+    /// Creation Time` subpacket must be present in the hashed area.
+    /// This function clears any `Signature Creation Time` subpackets
+    /// from both the hashed area and the unhashed are, and causes the
+    /// various `SignatureBuilder` finalizers to not emit a `Signature
+    /// Creation Time` subpacket.  This function should only be used
+    /// for testing purposes.
+    ///
+    /// [Section 5.2.3.4 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.2.3.4
+    pub fn suppress_signature_creation_time(mut self)
+        -> Result<Self>
+    {
+        self.overrode_creation_time = true;
+
+        self.hashed_area.remove_all(SubpacketTag::SignatureCreationTime);
+        self.unhashed_area.remove_all(SubpacketTag::SignatureCreationTime);
+
+        Ok(self)
+    }
+
     /// Sets the value of the Signature Expiration Time subpacket.
     ///
     /// If `None` is given, any expiration subpacket is removed.
diff --git a/openpgp/src/serialize/cert.rs b/openpgp/src/serialize/cert.rs
index 9e6bace9..fb436217 100644
--- a/openpgp/src/serialize/cert.rs
+++ b/openpgp/src/serialize/cert.rs
@@ -782,6 +782,7 @@ mod test {
                 cert.primary_key().with_policy(p, None).unwrap()
                     .direct_key_signature().unwrap().clone())
                     .set_type(SignatureType::PositiveCertification)
+                    .preserve_signature_creation_time().unwrap()
                     .set_exportable_certification(false).unwrap()).unwrap();
 
         let ua = UserAttribute::new(&[
@@ -793,6 +794,7 @@ mod test {
                 cert.primary_key().with_policy(p, None).unwrap()
                     .direct_key_signature().unwrap().clone())
                 .set_type(SignatureType::PositiveCertification)
+                .preserve_signature_creation_time().unwrap()
                 .set_exportable_certification(false).unwrap()).unwrap();
 
         let cert = cert.merge_packets(vec![