diff options
author | Justus Winter <justus@sequoia-pgp.org> | 2023-09-27 11:45:59 +0200 |
---|---|---|
committer | Justus Winter <justus@sequoia-pgp.org> | 2023-09-27 12:37:36 +0200 |
commit | 06b13acc10164f5bd651e4eb00facce4fa15beb2 (patch) | |
tree | acc4645b5fcf95f3298145f3b4c228813db69b40 /openpgp/src | |
parent | 80964e359cab4f8af96f7e3246a27bb95d27b424 (diff) |
openpgp: Fix SignatureBuilder::signature_expiration_time.
- SignatureBuilder::signature_expiration_time is broken. This is
because SignatureBuilder doesn't actually implement
signature_expiration_time. Instead, it is resolved via a Deref to
the SubpacketAreas::signature_expiration_time. That function
returns: creation_time subpacket + expiration_time subpacket, but
the actual creation time in a SignatureBuilder may not yet have
propagated to the subpacket area!
- Fixes #998.
Diffstat (limited to 'openpgp/src')
-rw-r--r-- | openpgp/src/packet/signature.rs | 22 | ||||
-rw-r--r-- | openpgp/src/packet/signature/subpacket.rs | 76 |
2 files changed, 98 insertions, 0 deletions
diff --git a/openpgp/src/packet/signature.rs b/openpgp/src/packet/signature.rs index 45dd53ac..f35ca04f 100644 --- a/openpgp/src/packet/signature.rs +++ b/openpgp/src/packet/signature.rs @@ -4290,4 +4290,26 @@ mod test { Ok(()) } + + #[test] + fn issue_998() -> Result<()> { + let now_t = Timestamp::try_from(crate::now())?; + let now = SystemTime::from(now_t); + let hour = std::time::Duration::new(3600, 0); + let hour_t = crate::types::Duration::from(3600); + let past = now - 2 * hour; + + let sig = SignatureBuilder::new(SignatureType::PositiveCertification) + .modify_hashed_area(|mut a| { + a.add(Subpacket::new( + SubpacketValue::SignatureCreationTime(now_t), true)?)?; + a.add(Subpacket::new( + SubpacketValue::SignatureExpirationTime(hour_t), true)?)?; + Ok(a) + })?; + assert_eq!(sig.signature_expiration_time(), Some(now + hour)); + let sig = sig.set_reference_time(past); + assert_eq!(sig.signature_expiration_time(), Some(now - hour)); + Ok(()) + } } diff --git a/openpgp/src/packet/signature/subpacket.rs b/openpgp/src/packet/signature/subpacket.rs index d6b47ea2..760adc14 100644 --- a/openpgp/src/packet/signature/subpacket.rs +++ b/openpgp/src/packet/signature/subpacket.rs @@ -2472,6 +2472,8 @@ impl SubpacketAreas { /// Note: if the signature contains multiple instances of this /// subpacket in the hashed subpacket area, the last one is /// returned. + // Note: If you update this function, also update + // SignatureBuilder::signature_expiration_time. pub fn signature_expiration_time(&self) -> Option<time::SystemTime> { match (self.signature_creation_time(), self.signature_validity_period()) { @@ -4395,6 +4397,80 @@ impl signature::SignatureBuilder { Ok(self) } + /// Returns the value of the Signature Expiration Time subpacket + /// as an absolute time. + /// + /// A [Signature Expiration Time subpacket] specifies when the + /// signature expires. The value stored is not an absolute time, + /// but a duration, which is relative to the Signature's creation + /// time. To better reflect the subpacket's name, this method + /// returns the absolute expiry time, and the + /// [`SubpacketAreas::signature_validity_period`] method returns + /// the subpacket's raw value. + /// + /// [Signature Expiration Time subpacket]: https://tools.ietf.org/html/rfc4880#section-5.2.3.10 + /// [`SubpacketAreas::signature_validity_period`]: SubpacketAreas::signature_validity_period() + /// + /// The Signature Expiration Time subpacket is different from the + /// [Key Expiration Time subpacket], which is accessed using + /// [`SubpacketAreas::key_validity_period`], and used specifies + /// when an associated key expires. The difference is that in the + /// former case, the signature itself expires, but in the latter + /// case, only the associated key expires. This difference is + /// critical: if a binding signature expires, then an OpenPGP + /// implementation will still consider the associated key to be + /// valid if there is another valid binding signature, even if it + /// is older than the expired signature; if the active binding + /// signature indicates that the key has expired, then OpenPGP + /// implementations will not fallback to an older binding + /// signature. + /// + /// [Key Expiration Time subpacket]: https://tools.ietf.org/html/rfc4880#section-5.2.3.6 + /// [`SubpacketAreas::key_validity_period`]: SubpacketAreas::key_validity_period() + /// + /// There are several cases where having a signature expire is + /// useful. Say Alice certifies Bob's certificate for + /// `bob@example.org`. She can limit the lifetime of the + /// certification to force her to reevaluate the certification + /// shortly before it expires. For instance, is Bob still + /// associated with `example.org`? Does she have reason to + /// believe that his key has been compromised? Using an + /// expiration is common in the X.509 ecosystem. For instance, + /// [Let's Encrypt] issues certificates with 90-day lifetimes. + /// + /// [Let's Encrypt]: https://letsencrypt.org/2015/11/09/why-90-days.html + /// + /// Having signatures expire can also be useful when deploying + /// software. For instance, you might have a service that + /// installs an update if it has been signed by a trusted + /// certificate. To prevent an adversary from coercing the + /// service to install an older version, you could limit the + /// signature's lifetime to just a few minutes. + /// + /// If the subpacket is not present in the hashed subpacket area, + /// this returns `None`. If this function returns `None`, the + /// signature does not expire. + /// + /// Note: if the signature contains multiple instances of this + /// subpacket in the hashed subpacket area, the last one is + /// returned. + // Note: This shadows SubpacketAreas::signature_expiration_time + // (SignatureBuilder derefs to SubpacketAreas), because we need to + // take SignatureBuilder::reference_time into account. + // + // If you update this function, also update + // SubpacketAreas::signature_expiration_time. + pub fn signature_expiration_time(&self) -> Option<time::SystemTime> { + match (self.effective_signature_creation_time(), + // This ^ is the difference to + // SubpacketAreas::signature_expiration_time. + self.signature_validity_period()) + { + (Ok(Some(ct)), Some(vp)) if vp.as_secs() > 0 => Some(ct + vp), + _ => None, + } + } + /// Sets the Signature Expiration Time subpacket. /// /// Adds a [Signature Expiration Time subpacket] to the hashed |