From 8e89a8f63a5aa6a0f51ed595a34ee20cc2f8bcfa Mon Sep 17 00:00:00 2001 From: "Neal H. Walfield" Date: Mon, 13 Jan 2020 22:36:54 +0100 Subject: openpgp: Rework stream verification logic. - Select keys only when verifying the signatures: the relevant keys depend on the timestamp in the signature, and different signatures may have different time stamps. - If the signature doens't have a Signature Creation Time stamp, return that the signature is invalid. --- ffi/src/error.rs | 2 + openpgp-ffi/src/error.rs | 6 +++ openpgp/src/lib.rs | 4 ++ openpgp/src/parse/stream.rs | 127 +++++++++++++++++++++----------------------- 4 files changed, 74 insertions(+), 65 deletions(-) diff --git a/ffi/src/error.rs b/ffi/src/error.rs index 5b1c6ea0..5c58f0b0 100644 --- a/ffi/src/error.rs +++ b/ffi/src/error.rs @@ -76,6 +76,8 @@ impl<'a> FromSequoiaError<'a> for Status { Status::NotYetLive, &openpgp::Error::NoBindingSignature(_) => Status::NoBindingSignature, + &openpgp::Error::InvalidKey(_) => + Status::InvalidKey, openpgp::Error::__Nonexhaustive => unreachable!(), } } diff --git a/openpgp-ffi/src/error.rs b/openpgp-ffi/src/error.rs index 43b2b63d..b26610a5 100644 --- a/openpgp-ffi/src/error.rs +++ b/openpgp-ffi/src/error.rs @@ -156,6 +156,9 @@ pub enum Status { /// No binding signature. NoBindingSignature = -32, + + /// Invalid key. + InvalidKey = -33, } /// Returns the error message. @@ -199,6 +202,7 @@ pub extern "C" fn pgp_status_to_string(status: Status) -> *const c_char { Expired => "Expired\x00", NotYetLive => "Not yet live\x00", NoBindingSignature => "No binding signature\x00", + InvalidKey => "Invalid key\x00", }.as_bytes().as_ptr() as *const c_char } @@ -256,6 +260,8 @@ impl<'a> From<&'a failure::Error> for Status { Status::NotYetLive, &openpgp::Error::NoBindingSignature(_) => Status::NoBindingSignature, + &openpgp::Error::InvalidKey(_) => + Status::InvalidKey, openpgp::Error::__Nonexhaustive => unreachable!(), } } diff --git a/openpgp/src/lib.rs b/openpgp/src/lib.rs index c436d3b5..f1d1506e 100644 --- a/openpgp/src/lib.rs +++ b/openpgp/src/lib.rs @@ -276,6 +276,10 @@ pub enum Error { #[fail(display = "No binding signature at time {:?}", _0)] NoBindingSignature(std::time::SystemTime), + /// Invalid key. + #[fail(display = "Invalid key: {:?}", _0)] + InvalidKey(String), + /// This marks this enum as non-exhaustive. Do not use this /// variant. #[doc(hidden)] #[fail(display = "__Nonexhaustive")] __Nonexhaustive, diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs index b4a1dfb8..e53f08f1 100644 --- a/openpgp/src/parse/stream.rs +++ b/openpgp/src/parse/stream.rs @@ -29,7 +29,6 @@ use crate::{ header::BodyLength, header::CTB, key, - Key, Literal, OnePassSig, one_pass_sig::OnePassSig3, @@ -122,8 +121,6 @@ const BUFFER_SIZE: usize = 25 * 1024 * 1024; pub struct Verifier<'a, H: VerificationHelper> { helper: H, certs: Vec, - /// Maps KeyID to certs[i].keys_all().nth(j). - keys: Vec<(crate::KeyHandle, (usize, usize))>, oppr: Option>, structure: IMessageStructure, @@ -565,27 +562,11 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { let time = time .unwrap_or_else(|| time::SystemTime::now()); - fn for_signing(key: &Key, sig: Option<&Signature>, - time: time::SystemTime, tolerance: time::Duration) - -> bool - where P: key::KeyParts, R: key::KeyRole - { - if let Some(sig) = sig { - sig.key_flags().for_signing() - // Check expiry. - && sig.signature_alive(time, tolerance).is_ok() - && sig.key_alive(key, time).is_ok() - } else { - false - } - } - let mut ppr = PacketParser::from_buffered_reader(bio)?; let mut v = Verifier { helper: helper, certs: Vec::new(), - keys: Vec::new(), oppr: None, structure: IMessageStructure::new(), reserve: None, @@ -609,27 +590,7 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { }, Packet::Literal(_) => { v.structure.insert_missing_signature_group(); - // Query keys. v.certs = v.helper.get_public_keys(&issuers)?; - - for (i, cert) in v.certs.iter().enumerate() { - if for_signing(cert.primary(), - cert.primary_key_signature(None), - time, tolerance) { - v.keys.push((cert.fingerprint().into(), - (i, 0))); - } - - for (j, skb) in cert.subkeys().enumerate() { - let key = skb.key(); - if for_signing(key, skb.binding_signature(None), - time, tolerance) { - v.keys.push((key.fingerprint().into(), - (i, j + 1))); - } - } - } - v.oppr = Some(PacketParserResult::Some(pp)); v.finish_maybe()?; return Ok(v); @@ -698,38 +659,74 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { IMessageLayer::SignatureGroup { sigs, .. } => { results.new_signature_group(); 'sigs: for sig in sigs.into_iter() { - for issuer in sig.get_issuers() { - if let Some((_, (i, j))) = self.keys.iter().find(|(h, _)| { - h.aliases(&issuer) - }) { - let cert = &self.certs[*i]; - let ka = cert.keys().policy(self.time).nth(*j).unwrap(); - results.push_verification_result( - if sig.verify(ka.key()).unwrap_or(false) { - if sig.signature_alive( - self.time, self.clock_skew_tolerance) - .is_ok() - { - VerificationResult::GoodChecksum { - sig: sig.clone(), - cert, ka, - } - } else { - VerificationResult::NotAlive { - sig: sig.clone(), - } + let sig_time = if let Some(t) = sig.signature_creation_time() { + t + } else { + // Invalid signature. + results.push_verification_result( + VerificationResult::Error { + sig, + error: Error::MalformedPacket( + "missing a Signature Creation Time subpacket" + .into()).into() + }); + continue; + }; + + let issuers = sig.get_issuers(); + + for ka in self.certs.iter() + .flat_map(|cert| { + cert.keys().policy(sig_time).filter_map(|ka| { + if issuers.iter().any(|i| { + i.aliases(ka.key().key_handle()) + }) { + Some(ka) + } else { + None + } + }) + }) + { + results.push_verification_result( + if sig.verify(ka.key()).unwrap_or(false) { + if let Err(err) = ka.alive() { + VerificationResult::Error { + sig, + error: err, + } + } else if ! ka.for_signing() { + VerificationResult::Error { + sig, + error: Error::InvalidKey( + "key is not signing capable".into()) + .into(), + } + } else if sig.signature_alive( + self.time, self.clock_skew_tolerance) + .is_ok() + { + VerificationResult::GoodChecksum { + sig: sig.clone(), + cert: ka.cert(), + ka, } } else { - VerificationResult::BadChecksum { + VerificationResult::NotAlive { sig: sig.clone(), - cert, ka, } } - ); + } else { + VerificationResult::BadChecksum { + sig: sig.clone(), + cert: ka.cert(), + ka, + } + } + ); - // We found a key, continue to next sig. - continue 'sigs; - } + // We found a key, continue to next sig. + continue 'sigs; } results.push_verification_result( -- cgit v1.2.3