diff options
author | Neal H. Walfield <neal@pep.foundation> | 2019-12-04 11:48:33 +0100 |
---|---|---|
committer | Neal H. Walfield <neal@pep.foundation> | 2019-12-04 11:54:00 +0100 |
commit | daee0e230c36cec119d523ca33407789b6fd109f (patch) | |
tree | c3540b91ab411ea9ca7ca371ca8a3979ac5b7037 | |
parent | 5000e6e522f206f35a9544b36f3dc2e74828c4a3 (diff) |
openpgp: Account for clock skew when using the streaming verifiers.
- Modify the streaming verifiers to account for clock skew when
using the current time.
-rw-r--r-- | openpgp/src/parse/stream.rs | 127 |
1 files changed, 89 insertions, 38 deletions
diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs index d1bf9229..9b202a5f 100644 --- a/openpgp/src/parse/stream.rs +++ b/openpgp/src/parse/stream.rs @@ -131,8 +131,29 @@ pub struct Verifier<'a, H: VerificationHelper> { // The reserve data. reserve: Option<Vec<u8>>, - /// Signature verification relative to this time. + /// Perform the signature verification relative to this time. + /// + /// This is needed for checking the signature's liveness. + /// + /// We want the same semantics as `Subpacket::signature_alive`. + /// Specifically, when using the current time, we want to tolerate + /// some clock skew, but when using some specific time, we don't. + /// (See `Subpacket::signature_alive` for an explanation.) + /// + /// These semantics can be realized by making `time` an + /// `Option<time::SystemTime>` and passing that as is to + /// `Subpacket::signature_alive`. But that approach has two new + /// problems. First, if we are told to use the current time, then + /// we want to use the time at which the Verifier was + /// instantiated, not the time at which we call + /// `Subpacket::signature_alive`. Second, if we call + /// `Subpacket::signature_alive` multiple times, they should all + /// use the same time. To work around these issues, when a + /// Verifier is instantiated, we evaluate `time` and we record how + /// much we want to tolerate clock skew in the same way as + /// `Subpacket::signature_alive`. time: time::SystemTime, + clock_skew_tolerance: time::Duration, } /// Contains the result of a signature verification. @@ -459,8 +480,8 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { -> Result<Verifier<'a, H>> where R: io::Read + 'a, T: Into<Option<time::SystemTime>> { - let t = t.into() - .unwrap_or_else(|| time::SystemTime::now()); + // Do not eagerly map `t` to the current time. + let t = t.into(); Verifier::from_buffered_reader( Box::new(buffered_reader::Generic::with_cookie(reader, None, Default::default())), @@ -475,8 +496,8 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { where P: AsRef<Path>, T: Into<Option<time::SystemTime>> { - let t = t.into() - .unwrap_or_else(|| time::SystemTime::now()); + // Do not eagerly map `t` to the current time. + let t = t.into(); Verifier::from_buffered_reader( Box::new(buffered_reader::File::with_cookie(path, Default::default())?), @@ -491,7 +512,8 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { -> Result<Verifier<'a, H>> where T: Into<Option<time::SystemTime>> { - let t = t.into().unwrap_or_else(|| time::SystemTime::now()); + // Do not eagerly map `t` to the current time. + let t = t.into(); Verifier::from_buffered_reader( Box::new(buffered_reader::Memory::with_cookie(bytes, Default::default())), @@ -525,20 +547,29 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { /// /// Signature verifications are done relative to time `t`, or the /// current time, if `t` is `None`. - pub(crate) fn from_buffered_reader(bio: Box<dyn BufferedReader<Cookie> + 'a>, - helper: H, t: time::SystemTime) - -> Result<Verifier<'a, H>> + pub(crate) fn from_buffered_reader<T>(bio: Box<dyn BufferedReader<Cookie> + 'a>, + helper: H, time: T) + -> Result<Verifier<'a, H>> + where T: Into<Option<time::SystemTime>> { + let time = time.into(); + let tolerance = time + .map(|_| time::Duration::new(0, 0)) + .unwrap_or( + *crate::packet::signature::subpacket::CLOCK_SKEW_TOLERANCE); + let time = time + .unwrap_or_else(|| time::SystemTime::now()); + fn can_sign<P, R>(key: &Key<P, R>, sig: Option<&Signature>, - t: time::SystemTime) + time: time::SystemTime, tolerance: time::Duration) -> bool where P: key::KeyParts, R: key::KeyRole { if let Some(sig) = sig { sig.key_flags().can_sign() // Check expiry. - && sig.signature_alive(t, None) - && sig.key_alive(key, t) + && sig.signature_alive(time, tolerance) + && sig.key_alive(key, time) } else { false } @@ -553,7 +584,8 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { oppr: None, structure: IMessageStructure::new(), reserve: None, - time: t, + time: time, + clock_skew_tolerance: tolerance, }; let mut issuers = Vec::new(); @@ -577,14 +609,16 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { for (i, cert) in v.certs.iter().enumerate() { if can_sign(cert.primary(), - cert.primary_key_signature(None), t) { + cert.primary_key_signature(None), + time, tolerance) { v.keys.insert(cert.fingerprint().into(), (i, 0)); v.keys.insert(cert.keyid().into(), (i, 0)); } for (j, skb) in cert.subkeys().enumerate() { let key = skb.key(); - if can_sign(key, skb.binding_signature(None), t) { + if can_sign(key, skb.binding_signature(None), + time, tolerance) { v.keys.insert(key.fingerprint().into(), (i, j + 1)); v.keys.insert(key.keyid().into(), @@ -668,7 +702,8 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { = cert.keys_all().nth(*j).unwrap(); results.push_verification_result( if sig.verify(key).unwrap_or(false) { - if sig.signature_alive(self.time, None) + if sig.signature_alive( + self.time, self.clock_skew_tolerance) { VerificationResult::GoodChecksum { sig: sig.clone(), @@ -1047,8 +1082,8 @@ impl DetachedVerifier { where R: io::Read + 'a, S: io::Read + 's, H: VerificationHelper, T: Into<Option<time::SystemTime>> { - let t = t.into() - .unwrap_or_else(|| time::SystemTime::now()); + // Do not eagerly map `t` to the current time. + let t = t.into(); Self::from_buffered_reader( Box::new(buffered_reader::Generic::with_cookie(signature_reader, None, Default::default())), @@ -1066,8 +1101,8 @@ impl DetachedVerifier { where P: AsRef<Path>, S: AsRef<Path>, H: VerificationHelper, T: Into<Option<time::SystemTime>> { - let t = t.into() - .unwrap_or_else(|| time::SystemTime::now()); + // Do not eagerly map `t` to the current time. + let t = t.into(); Self::from_buffered_reader( Box::new(buffered_reader::File::with_cookie(signature_path, Default::default())?), @@ -1081,10 +1116,11 @@ impl DetachedVerifier { /// current time, if `t` is `None`. pub fn from_bytes<'a, 's, H, T>(signature_bytes: &'s [u8], bytes: &'a [u8], helper: H, t: T) - -> Result<Verifier<'a, H>> + -> Result<Verifier<'a, H>> where H: VerificationHelper, T: Into<Option<time::SystemTime>> { - let t = t.into().unwrap_or_else(|| time::SystemTime::now()); + // Do not eagerly map `t` to the current time. + let t = t.into(); Self::from_buffered_reader( Box::new(buffered_reader::Memory::with_cookie(signature_bytes, Default::default())), @@ -1096,13 +1132,16 @@ impl DetachedVerifier { /// /// Signature verifications are done relative to time `t`, or the /// current time, if `t` is `None`. - pub(crate) fn from_buffered_reader<'a, 's, H> + pub(crate) fn from_buffered_reader<'a, 's, H, T> (signature_bio: Box<dyn BufferedReader<Cookie> + 's>, reader: Box<dyn BufferedReader<()> + 'a>, - helper: H, t: time::SystemTime) + helper: H, t: T) -> Result<Verifier<'a, H>> - where H: VerificationHelper + where H: VerificationHelper, + T: Into<Option<time::SystemTime>> { + // Do not eagerly map `t` to the current time. + let t = t.into(); Verifier::from_buffered_reader( Box::new(buffered_reader::Generic::with_cookie( Transformer::new(signature_bio, reader)?, @@ -1194,6 +1233,7 @@ pub struct Decryptor<'a, H: VerificationHelper + DecryptionHelper> { /// Signature verification relative to this time. time: time::SystemTime, + clock_skew_tolerance: time::Duration, } /// Helper for decrypting messages. @@ -1239,8 +1279,8 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> { -> Result<Decryptor<'a, H>> where R: io::Read + 'a, T: Into<Option<time::SystemTime>> { - let t = t.into() - .unwrap_or_else(|| time::SystemTime::now()); + // Do not eagerly map `t` to the current time. + let t = t.into(); Decryptor::from_buffered_reader( Box::new(buffered_reader::Generic::with_cookie(reader, None, Default::default())), @@ -1255,8 +1295,8 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> { where P: AsRef<Path>, T: Into<Option<time::SystemTime>> { - let t = t.into() - .unwrap_or_else(|| time::SystemTime::now()); + // Do not eagerly map `t` to the current time. + let t = t.into(); Decryptor::from_buffered_reader( Box::new(buffered_reader::File::with_cookie(path, Default::default())?), @@ -1271,8 +1311,8 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> { -> Result<Decryptor<'a, H>> where T: Into<Option<time::SystemTime>> { - let t = t.into() - .unwrap_or_else(|| time::SystemTime::now()); + // Do not eagerly map `t` to the current time. + let t = t.into(); Decryptor::from_buffered_reader( Box::new(buffered_reader::Memory::with_cookie(bytes, Default::default())), @@ -1303,12 +1343,21 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> { } /// Creates the `Decryptor`, and buffers the data up to `BUFFER_SIZE`. - pub(crate) fn from_buffered_reader(bio: Box<dyn BufferedReader<Cookie> + 'a>, - helper: H, t: time::SystemTime) - -> Result<Decryptor<'a, H>> + pub(crate) fn from_buffered_reader<T>(bio: Box<dyn BufferedReader<Cookie> + 'a>, + helper: H, time: T) + -> Result<Decryptor<'a, H>> + where T: Into<Option<time::SystemTime>> { tracer!(TRACE, "Decryptor::from_buffered_reader", 0); + let time = time.into(); + let tolerance = time + .map(|_| time::Duration::new(0, 0)) + .unwrap_or( + *crate::packet::signature::subpacket::CLOCK_SKEW_TOLERANCE); + let time = time + .unwrap_or_else(|| time::SystemTime::now()); + let mut ppr = PacketParserBuilder::from_buffered_reader(bio)? .map(helper.mapping()).finalize()?; @@ -1320,7 +1369,8 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> { identity: None, structure: IMessageStructure::new(), reserve: None, - time: t, + time: time, + clock_skew_tolerance: tolerance, }; let mut issuers = Vec::new(); @@ -1389,8 +1439,8 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> { if let Some(sig) = sig { sig.key_flags().can_sign() // Check expiry. - && sig.signature_alive(t, None) - && sig.key_alive(key, t) + && sig.signature_alive(time, tolerance) + && sig.key_alive(key, time) } else { false } @@ -1543,7 +1593,8 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> { = cert.keys_all().nth(*j).unwrap(); results.push_verification_result( if sig.verify(key).unwrap_or(false) && - sig.signature_alive(self.time, None) + sig.signature_alive( + self.time, self.clock_skew_tolerance) { // Check intended recipients. if let Some(identity) = |