diff options
author | Neal H. Walfield <neal@pep.foundation> | 2019-11-11 19:51:04 +0100 |
---|---|---|
committer | Neal H. Walfield <neal@pep.foundation> | 2019-11-19 11:04:05 +0100 |
commit | ec03e1614a48fbe30f1200cb18bb00c7135f5242 (patch) | |
tree | 363aa6030de4fcb00d727830acbc308703a20468 | |
parent | a5fade1d635d75e474294a06870251d8f617db08 (diff) |
openpgp: Be tolerant when deciding wheter a signature is alive.
- Consider the following scenario: computer A's clock says 9:00.00
and signs and sends a message to computer B. Computer B's clock
says 8:59.59, it receives the message and tries to verify it.
From Computer B's perspective, the signature is not valid, because
it was generated in the future.
- This situation occured, because the two clocks were not completely
synchronized. Unfortunately, a few seconds of clock skew are not
unusual, particularly when dealing with VMs.
- Since it is almost always better to consider such messages as
valid, be tolerant when deciding whether a signature is alive.
-rw-r--r-- | openpgp-ffi/include/sequoia/openpgp.h | 103 | ||||
-rw-r--r-- | openpgp-ffi/src/packet/signature.rs | 121 | ||||
-rw-r--r-- | openpgp/src/packet/signature/subpacket.rs | 120 | ||||
-rw-r--r-- | openpgp/src/parse/stream.rs | 8 | ||||
-rw-r--r-- | openpgp/src/serialize/tpk_armored.rs | 4 | ||||
-rw-r--r-- | openpgp/src/tpk/mod.rs | 8 | ||||
-rw-r--r-- | tool/src/commands/inspect.rs | 2 |
7 files changed, 327 insertions, 39 deletions
diff --git a/openpgp-ffi/include/sequoia/openpgp.h b/openpgp-ffi/include/sequoia/openpgp.h index 0f41d6ed..6aeca8e9 100644 --- a/openpgp-ffi/include/sequoia/openpgp.h +++ b/openpgp-ffi/include/sequoia/openpgp.h @@ -381,14 +381,109 @@ bool pgp_signature_is_group_key(pgp_signature_t signature); /*/ /// Returns whether the signature is alive at the specified time. /// -/// If `when` is 0, then the current time is used. -/// -/// A signature is alive if the creation date is in the past, and the -/// signature has not expired at the specified time. +/// A signature is considered to be alive if `creation time - +/// tolerance <= time` and `time <= expiration time`. +/// +/// If `time` is 0, uses the current time. +/// +/// This function uses the default tolerance. If you want to specify +/// a different tolerance (or no tolerance), then use +/// `pgp_signature_alive_with_tolerance`. +/// +/// Some tolerance for clock skew is sometimes necessary, because +/// although most computers synchronize their clock with a time +/// server, up to a few seconds of clock skew are not unusual in +/// practice. And, even worse, several minutes of clock skew appear +/// to be not uncommon on virtual machines. +/// +/// Not accounting for clock skew can result in signatures being +/// unexpectedly considered invalid. Consider: computer A sends a +/// message to computer B at 9:00, but computer B, whose clock says +/// the current time is 8:59, rejects it, because the signature +/// appears to have been made in the future. This is particularly +/// problematic for low-latency protocols built on top of OpenPGP, +/// e.g., state synchronization between two MUAs via a shared IMAP +/// folder. +/// +/// Being tolerant to potential clock skew is not always appropriate. +/// For instance, when determining a User ID's current self signature +/// at time `t`, we don't ever want to consider a self-signature made +/// after `t` to be valid, even if it was made just a few moments +/// after `t`. This goes doubly so for soft revocation certificates: +/// the user might send a message that she is retiring, and then +/// immediately create a soft revocation. The soft revocation should +/// not invalidate the message. +/// +/// Unfortunately, in many cases, whether we should account for clock +/// skew or not depends on application-specific context. As a rule of +/// thumb, if the time and the timestamp come from different sources, +/// you probably want to account for clock skew. +/// +/// Note that [Section 5.2.3.4 of RFC 4880] states that "[[A Signature +/// Creation Time subpacket]] MUST be present in the hashed area." +/// Consequently, if such a packet does not exist, but a "Signature +/// Expiration Time" subpacket exists, we conservatively treat the +/// signature as expired, because there is no way to evaluate the +/// expiration time. +/// +/// [Section 5.2.3.4 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.2.3.4 /*/ bool pgp_signature_alive(pgp_signature_t signature, time_t when); /*/ +/// Returns whether the signature is alive at the specified time. +/// +/// A signature is considered to be alive if `creation time - +/// tolerance <= time` and `time <= expiration time`. +/// +/// If `time` is 0, uses the current time. +/// +/// If `tolerance` is 0, uses no tolerance. To ensure consistency +/// across callers, you should use the default tolerance (i.e., use +/// `pgp_signature_alive`). +/// +/// Some tolerance for clock skew is sometimes necessary, because +/// although most computers synchronize their clock with a time +/// server, up to a few seconds of clock skew are not unusual in +/// practice. And, even worse, several minutes of clock skew appear +/// to be not uncommon on virtual machines. +/// +/// Not accounting for clock skew can result in signatures being +/// unexpectedly considered invalid. Consider: computer A sends a +/// message to computer B at 9:00, but computer B, whose clock says +/// the current time is 8:59, rejects it, because the signature +/// appears to have been made in the future. This is particularly +/// problematic for low-latency protocols built on top of OpenPGP, +/// e.g., state synchronization between two MUAs via a shared IMAP +/// folder. +/// +/// Being tolerant to potential clock skew is not always appropriate. +/// For instance, when determining a User ID's current self signature +/// at time `t`, we don't ever want to consider a self-signature made +/// after `t` to be valid, even if it was made just a few moments +/// after `t`. This goes doubly so for soft revocation certificates: +/// the user might send a message that she is retiring, and then +/// immediately create a soft revocation. The soft revocation should +/// not invalidate the message. +/// +/// Unfortunately, in many cases, whether we should account for clock +/// skew or not depends on application-specific context. As a rule of +/// thumb, if the time and the timestamp come from different sources, +/// you probably want to account for clock skew. +/// +/// Note that [Section 5.2.3.4 of RFC 4880] states that "[[A Signature +/// Creation Time subpacket]] MUST be present in the hashed area." +/// Consequently, if such a packet does not exist, but a "Signature +/// Expiration Time" subpacket exists, we conservatively treat the +/// signature as expired, because there is no way to evaluate the +/// expiration time. +/// +/// [Section 5.2.3.4 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.2.3.4 +/*/ +bool pgp_signature_alive_with_tolerance(pgp_signature_t signature, + time_t time, unsigned int tolerance); + +/*/ /// Returns whether the signature is expired at the specified time. /// /// If `when` is 0, then the current time is used. diff --git a/openpgp-ffi/src/packet/signature.rs b/openpgp-ffi/src/packet/signature.rs index 636318b3..2b1b301e 100644 --- a/openpgp-ffi/src/packet/signature.rs +++ b/openpgp-ffi/src/packet/signature.rs @@ -8,6 +8,7 @@ //! [Section 5.2 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.2 use libc::time_t; +use libc::c_uint; extern crate sequoia_openpgp as openpgp; use super::Packet; @@ -118,18 +119,124 @@ fn pgp_signature_is_group_key(sig: *const Signature) -> bool { /// Returns whether the signature is alive at the specified time. /// -/// If `when` is 0, then the current time is used. +/// A signature is considered to be alive if `creation time - +/// tolerance <= time` and `time <= expiration time`. /// -/// A signature is alive if the creation date is in the past, and the -/// signature has not expired at the specified time. +/// If `time` is 0, uses the current time. +/// +/// This function uses the default tolerance. If you want to specify +/// a different tolerance (or no tolerance), then use +/// `pgp_signature_alive_with_tolerance`. +/// +/// Some tolerance for clock skew is sometimes necessary, because +/// although most computers synchronize their clock with a time +/// server, up to a few seconds of clock skew are not unusual in +/// practice. And, even worse, several minutes of clock skew appear +/// to be not uncommon on virtual machines. +/// +/// Not accounting for clock skew can result in signatures being +/// unexpectedly considered invalid. Consider: computer A sends a +/// message to computer B at 9:00, but computer B, whose clock says +/// the current time is 8:59, rejects it, because the signature +/// appears to have been made in the future. This is particularly +/// problematic for low-latency protocols built on top of OpenPGP, +/// e.g., state synchronization between two MUAs via a shared IMAP +/// folder. +/// +/// Being tolerant to potential clock skew is not always appropriate. +/// For instance, when determining a User ID's current self signature +/// at time `t`, we don't ever want to consider a self-signature made +/// after `t` to be valid, even if it was made just a few moments +/// after `t`. This goes doubly so for soft revocation certificates: +/// the user might send a message that she is retiring, and then +/// immediately create a soft revocation. The soft revocation should +/// not invalidate the message. +/// +/// Unfortunately, in many cases, whether we should account for clock +/// skew or not depends on application-specific context. As a rule of +/// thumb, if the time and the timestamp come from different sources, +/// you probably want to account for clock skew. +/// +/// Note that [Section 5.2.3.4 of RFC 4880] states that "[[A Signature +/// Creation Time subpacket]] MUST be present in the hashed area." +/// Consequently, if such a packet does not exist, but a "Signature +/// Expiration Time" subpacket exists, we conservatively treat the +/// signature as expired, because there is no way to evaluate the +/// expiration time. +/// +/// [Section 5.2.3.4 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.2.3.4 #[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "C" -fn pgp_signature_alive(sig: *const Signature, when: time_t) -> bool { - let t = if when == 0 { +fn pgp_signature_alive(sig: *const Signature, time: time_t) + -> bool +{ + let time = if time == 0 { None } else { - Some(time::at(time::Timespec::new(when as i64, 0))) + Some(time::at(time::Timespec::new(time as i64, 0))) + }; + sig.ref_raw().signature_alive(time, None) +} + +/// Returns whether the signature is alive at the specified time. +/// +/// A signature is considered to be alive if `creation time - +/// tolerance <= time` and `time <= expiration time`. +/// +/// If `time` is 0, uses the current time. +/// +/// If `tolerance` is 0, uses no tolerance. To ensure consistency +/// across callers, you should use the default tolerance (i.e., use +/// `pgp_signature_alive`). +/// +/// Some tolerance for clock skew is sometimes necessary, because +/// although most computers synchronize their clock with a time +/// server, up to a few seconds of clock skew are not unusual in +/// practice. And, even worse, several minutes of clock skew appear +/// to be not uncommon on virtual machines. +/// +/// Not accounting for clock skew can result in signatures being +/// unexpectedly considered invalid. Consider: computer A sends a +/// message to computer B at 9:00, but computer B, whose clock says +/// the current time is 8:59, rejects it, because the signature +/// appears to have been made in the future. This is particularly +/// problematic for low-latency protocols built on top of OpenPGP, +/// e.g., state synchronization between two MUAs via a shared IMAP +/// folder. +/// +/// Being tolerant to potential clock skew is not always appropriate. +/// For instance, when determining a User ID's current self signature +/// at time `t`, we don't ever want to consider a self-signature made +/// after `t` to be valid, even if it was made just a few moments +/// after `t`. This goes doubly so for soft revocation certificates: +/// the user might send a message that she is retiring, and then +/// immediately create a soft revocation. The soft revocation should +/// not invalidate the message. +/// +/// Unfortunately, in many cases, whether we should account for clock +/// skew or not depends on application-specific context. As a rule of +/// thumb, if the time and the timestamp come from different sources, +/// you probably want to account for clock skew. +/// +/// Note that [Section 5.2.3.4 of RFC 4880] states that "[[A Signature +/// Creation Time subpacket]] MUST be present in the hashed area." +/// Consequently, if such a packet does not exist, but a "Signature +/// Expiration Time" subpacket exists, we conservatively treat the +/// signature as expired, because there is no way to evaluate the +/// expiration time. +/// +/// [Section 5.2.3.4 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.2.3.4 +#[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "C" +fn pgp_signature_alive_with_tolerance(sig: *const Signature, + time: time_t, tolerance: c_uint) + -> bool +{ + let time = if time == 0 { + None + } else { + Some(time::at(time::Timespec::new(time as i64, 0))) }; - sig.ref_raw().signature_alive(t) + let tolerance = time::Duration::seconds(tolerance as i64); + sig.ref_raw().signature_alive(time, Some(tolerance)) } /// Returns whether the signature is expired at the specified time. diff --git a/openpgp/src/packet/signature/subpacket.rs b/openpgp/src/packet/signature/subpacket.rs index 4b74b3b3..219251f3 100644 --- a/openpgp/src/packet/signature/subpacket.rs +++ b/openpgp/src/packet/signature/subpacket.rs @@ -62,6 +62,7 @@ use std::sync::Mutex; use std::ops::{Deref, DerefMut}; use std::fmt; use std::io; +use std::cmp; use time; use quickcheck::{Arbitrary, Gen}; @@ -96,6 +97,39 @@ use crate::conversions::{ Duration, }; +lazy_static!{ + /// The default amount of tolerance to use when comparing + /// some timestamps. + /// + /// Used by `Subpacket::signature_alive`. + /// + /// When determining whether a timestamp generated on another + /// machine is valid *now*, we need to account for clock skew. + /// (Note: you don't normally need to consider clock skew when + /// evaluating a signature's validity at some time in the past.) + /// + /// We tolerate half an hour of skew based on the following + /// anecdote: In 2019, a developer using Sequoia in a Windows VM + /// running inside of Virtual Box on Mac OS X reported that he + /// typically observed a few minutes of clock skew and + /// occasionally saw over 20 minutes of clock skew. + /// + /// Note: when new messages override older messages, and their + /// signatures are evaluated at some arbitrary point in time, an + /// application may not see a consistent state if it uses a + /// tolerance. Consider an application that has two messages and + /// wants to get the current message at time te: + /// + /// - t0: message 0 + /// - te: "get current message" + /// - t1: message 1 + /// + /// If te is close to t1, then t1 may be considered valid, which + /// is probably not what you want. + pub static ref CLOCK_SKEW_TOLERANCE: time::Duration + = time::Duration::seconds(30 * 60); +} + /// The subpacket types specified by [Section 5.2.3.1 of RFC 4880]. /// /// [Section 5.2.3.1 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.2.3.1 @@ -2075,14 +2109,48 @@ impl SubpacketAreas { } } - /// Returns whether or not the signature is alive at the given + /// Returns whether or not the signature is alive at the specified /// time. /// - /// A signature is considered to be alive if `creation time <= t` - /// and `t <= expiration time`. - /// - /// If `t` is None, uses the current time. - /// + /// A signature is considered to be alive if `creation time - + /// tolerance <= time` and `time <= expiration time`. + /// + /// If `time` is None, uses the current time. + /// + /// If `time` is None, and `clock_skew_tolerance` is None, then + /// uses `CLOCK_SKEW_TOLERANCE`. If `time` is not None, but + /// `clock_skew_tolerance` is None, uses no tolerance. + /// + /// Some tolerance for clock skew is sometimes necessary, because + /// although most computers synchronize their clock with a time + /// server, up to a few seconds of clock skew are not unusual in + /// practice. And, even worse, several minutes of clock skew + /// appear to be not uncommon on virtual machines. + /// + /// Not accounting for clock skew can result in signatures being + /// unexpectedly considered invalid. Consider: computer A sends a + /// message to computer B at 9:00, but computer B, whose clock + /// says the current time is 8:59, rejects it, because the + /// signature appears to have been made in the future. This is + /// particularly problematic for low-latency protocols built on + /// top of OpenPGP, e.g., state synchronization between two MUAs + /// via a shared IMAP folder. + /// + /// Being tolerant to potential clock skew is not always + /// appropriate. For instance, when determining a User ID's + /// current self signature at time `t`, we don't ever want to + /// consider a self-signature made after `t` to be valid, even if + /// it was made just a few moments after `t`. This goes doubly so + /// for soft revocation certificates: the user might send a + /// message that she is retiring, and then immediately create a + /// soft revocation. The soft revocation should not invalidate + /// the message. + /// + /// Unfortunately, in many cases, whether we should account for + /// clock skew or not depends on application-specific context. As + /// a rule of thumb, if the time and the timestamp come from + /// different sources, you probably want to account for clock + /// skew. /// /// Note that [Section 5.2.3.4 of RFC 4880] states that "[[A /// Signature Creation Time subpacket]] MUST be present in the @@ -2092,12 +2160,28 @@ impl SubpacketAreas { /// is no way to evaluate the expiration time. /// /// [Section 5.2.3.4 of RFC 4880]: https://tools.ietf.org/html/rfc4880#section-5.2.3.4 - pub fn signature_alive<T>(&self, t: T) -> bool - where T: Into<Option<time::Tm>> + pub fn signature_alive<T, U>(&self, time: T, clock_skew_tolerance: U) + -> bool + where T: Into<Option<time::Tm>>, + U: Into<Option<time::Duration>> { - let t = t.into().unwrap_or_else(time::now_utc); + let (time, tolerance) + = match (time.into(), clock_skew_tolerance.into()) { + (None, None) => + (time::now_utc(), *CLOCK_SKEW_TOLERANCE), + (None, Some(tolerance)) => + (time::now_utc(), tolerance), + (Some(time), None) => + (time, time::Duration::seconds(0)), + (Some(time), Some(tolerance)) => + (time, tolerance) + }; + let time_zero = || time::at_utc(time::Timespec::new(0, 0)); + if let Some(creation_time) = self.signature_creation_time() { - creation_time <= t && ! self.signature_expired(t) + // Be careful to avoid underflow. + cmp::max(creation_time, time_zero() + tolerance) - tolerance <= time + && ! self.signature_expired(time) } else { false } @@ -2666,10 +2750,10 @@ fn accessors() { assert!(!sig_.signature_expired(now)); assert!(sig_.signature_expired(now + ten_minutes)); - assert!(sig_.signature_alive(None)); - assert!(sig_.signature_alive(now)); - assert!(!sig_.signature_alive(now - five_minutes)); - assert!(!sig_.signature_alive(now + ten_minutes)); + assert!(sig_.signature_alive(None, time::Duration::seconds(0))); + assert!(sig_.signature_alive(now, time::Duration::seconds(0))); + assert!(!sig_.signature_alive(now - five_minutes, time::Duration::seconds(0))); + assert!(!sig_.signature_alive(now + ten_minutes, time::Duration::seconds(0))); sig = sig.set_signature_expiration_time(None).unwrap(); let sig_ = @@ -2679,10 +2763,10 @@ fn accessors() { assert!(!sig_.signature_expired(now)); assert!(!sig_.signature_expired(now + ten_minutes)); - assert!(sig_.signature_alive(None)); - assert!(sig_.signature_alive(now)); - assert!(!sig_.signature_alive(now - five_minutes)); - assert!(sig_.signature_alive(now + ten_minutes)); + assert!(sig_.signature_alive(None, time::Duration::seconds(0))); + assert!(sig_.signature_alive(now, time::Duration::seconds(0))); + assert!(!sig_.signature_alive(now - five_minutes, time::Duration::seconds(0))); + assert!(sig_.signature_alive(now + ten_minutes, time::Duration::seconds(0))); sig = sig.set_exportable_certification(true).unwrap(); let sig_ = diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs index ff381ea7..3f33e833 100644 --- a/openpgp/src/parse/stream.rs +++ b/openpgp/src/parse/stream.rs @@ -486,7 +486,7 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { if let Some(sig) = sig { sig.key_flags().can_sign() // Check expiry. - && sig.signature_alive(t) + && sig.signature_alive(t, None) && sig.key_alive(key, t) } else { false @@ -612,7 +612,7 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { let (binding, revocation, key) = tpk.keys_all().nth(*j).unwrap(); if sig.verify(key).unwrap_or(false) { - if sig.signature_alive(self.time) { + if sig.signature_alive(self.time, None) { VerificationResult::GoodChecksum (sig, tpk, key, binding, revocation) @@ -1316,7 +1316,7 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> { if let Some(sig) = sig { sig.key_flags().can_sign() // Check expiry. - && sig.signature_alive(t) + && sig.signature_alive(t, None) && sig.key_alive(key, t) } else { false @@ -1463,7 +1463,7 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> { let (binding, revocation, key) = tpk.keys_all().nth(*j).unwrap(); if sig.verify(key).unwrap_or(false) && - sig.signature_alive(self.time) + sig.signature_alive(self.time, None) { // Check intended recipients. if let Some(identity) = diff --git a/openpgp/src/serialize/tpk_armored.rs b/openpgp/src/serialize/tpk_armored.rs index 47940af7..96681781 100644 --- a/openpgp/src/serialize/tpk_armored.rs +++ b/openpgp/src/serialize/tpk_armored.rs @@ -39,7 +39,9 @@ impl TPK { } // Ignore userids not "alive". }).filter_map(|uidb| { - if uidb.binding_signature(None)?.signature_alive(None) { + if uidb.binding_signature(None)? + .signature_alive(None, time::Duration::seconds(0)) + { Some(uidb) } else { None diff --git a/openpgp/src/tpk/mod.rs b/openpgp/src/tpk/mod.rs index c73a27fe..bc6e8ed0 100644 --- a/openpgp/src/tpk/mod.rs +++ b/openpgp/src/tpk/mod.rs @@ -213,7 +213,7 @@ impl<C> ComponentBinding<C> { }; self.self_signatures[i..].iter().filter(|s| { - s.signature_alive(t) + s.signature_alive(t, time::Duration::seconds(0)) }).nth(0) } @@ -279,7 +279,7 @@ impl<C> ComponentBinding<C> { selfsig_creation_time.rfc822(), t.rfc822()); if let Some(selfsig) = selfsig { - assert!(selfsig.signature_alive(t)); + assert!(selfsig.signature_alive(t, time::Duration::seconds(0))); } macro_rules! check { @@ -309,7 +309,7 @@ impl<C> ComponentBinding<C> { rev.signature_creation_time() .unwrap_or_else(time_zero).rfc822()); None - } else if !rev.signature_alive(t) { + } else if !rev.signature_alive(t, time::Duration::seconds(0)) { t!(" ignoring revocation that is not alive ({} - {})", rev.signature_creation_time() .unwrap_or_else(time_zero).rfc822(), @@ -797,7 +797,7 @@ impl TPK { // No binding signature at time `t` => not alive. let selfsig = b.binding_signature(t)?; - if !selfsig.signature_alive(t) { + if !selfsig.signature_alive(t, time::Duration::seconds(0)) { return None; } diff --git a/tool/src/commands/inspect.rs b/tool/src/commands/inspect.rs index ee915c54..23ebe8a5 100644 --- a/tool/src/commands/inspect.rs +++ b/tool/src/commands/inspect.rs @@ -151,7 +151,7 @@ fn inspect_tpk(output: &mut dyn io::Write, tpk: &openpgp::TPK, if let Some(sig) = uidb.binding_signature(None) { if sig.signature_expired(None) { writeln!(output, " Expired")?; - } else if ! sig.signature_alive(None) { + } else if ! sig.signature_alive(None, time::Duration::seconds(0)) { writeln!(output, " Not yet valid")?; } } |