From b36219a18a1dab941bf48aeea508bae4d4858aad Mon Sep 17 00:00:00 2001 From: "Neal H. Walfield" Date: Tue, 21 Jan 2020 09:28:55 +0100 Subject: openpgp: Enforce that KeyAmalgamations have a binding signature. - ValidKeyIter enforces that KeyAmalgamations have a valid binding signature. - Enforce this constraint when constructing a KeyAmalgamation and when modifying a KeyAmalgamation's policy. --- openpgp/src/cert/builder.rs | 10 ++-- openpgp/src/cert/key_amalgamation.rs | 89 ++++++++++++++++++++++++------------ openpgp/src/cert/keyiter.rs | 28 ++++++++---- 3 files changed, 84 insertions(+), 43 deletions(-) (limited to 'openpgp/src') diff --git a/openpgp/src/cert/builder.rs b/openpgp/src/cert/builder.rs index 24172b0b..c5adc427 100644 --- a/openpgp/src/cert/builder.rs +++ b/openpgp/src/cert/builder.rs @@ -678,15 +678,15 @@ mod tests { .for_signing() .nth(0).unwrap(); assert!(ka.alive().is_ok()); - assert!(ka.clone().set_time(now + 290 * s).alive().is_ok()); - assert!(! ka.clone().set_time(now + 310 * s).alive().is_ok()); + assert!(ka.clone().set_time(now + 290 * s).unwrap().alive().is_ok()); + assert!(! ka.clone().set_time(now + 310 * s).unwrap().alive().is_ok()); let ka = cert.keys().policy(now).alive().revoked(false) .for_authentication() .nth(0).unwrap(); assert!(ka.alive().is_ok()); - assert!(ka.clone().set_time(now + 590 * s).alive().is_ok()); - assert!(! ka.clone().set_time(now + 610 * s).alive().is_ok()); + assert!(ka.clone().set_time(now + 590 * s).unwrap().alive().is_ok()); + assert!(! ka.clone().set_time(now + 610 * s).unwrap().alive().is_ok()); } #[test] @@ -708,7 +708,7 @@ mod tests { assert_eq!(cert.keys().policy(None).count(), 2); for ka in cert.keys().policy(None) { assert_eq!(ka.key().creation_time(), UNIX_EPOCH); - assert_eq!(ka.binding_signature().unwrap() + assert_eq!(ka.binding_signature() .signature_creation_time().unwrap(), UNIX_EPOCH); } diff --git a/openpgp/src/cert/key_amalgamation.rs b/openpgp/src/cert/key_amalgamation.rs index 85695f2e..8c510a18 100644 --- a/openpgp/src/cert/key_amalgamation.rs +++ b/openpgp/src/cert/key_amalgamation.rs @@ -32,33 +32,49 @@ enum KeyAmalgamationBinding<'a, P: key::KeyParts> { #[derive(Debug, Clone)] pub struct KeyAmalgamation<'a, P: key::KeyParts> { cert: &'a Cert, - time: SystemTime, binding: KeyAmalgamationBinding<'a, P>, + + // The reference time. + time: SystemTime, + // The binding siganture at time `time`. (This is just a cache.) + binding_signature: &'a Signature, } -impl<'a, P> From<(&'a Cert, SystemTime)> +impl<'a, P> TryFrom<(&'a Cert, SystemTime)> for KeyAmalgamation<'a, P> where P: key::KeyParts { - fn from(x: (&'a Cert, SystemTime)) -> Self { - KeyAmalgamation { + type Error = failure::Error; + + fn try_from(x: (&'a Cert, SystemTime)) -> Result { + Ok(KeyAmalgamation { cert: x.0, binding: KeyAmalgamationBinding::Primary(), time: x.1, - } + binding_signature: + x.0.primary_key_signature(x.1) + .ok_or_else(|| Error::NoBindingSignature(x.1))?, + }) } } -impl<'a, P> From<(&'a Cert, &'a KeyBinding, SystemTime)> +impl<'a, P> TryFrom<(&'a Cert, &'a KeyBinding, SystemTime)> for KeyAmalgamation<'a, P> where P: key::KeyParts { - fn from(x: (&'a Cert, &'a KeyBinding, SystemTime)) -> Self { - KeyAmalgamation { + type Error = failure::Error; + + fn try_from(x: (&'a Cert, &'a KeyBinding, SystemTime)) + -> Result + { + Ok(KeyAmalgamation { cert: x.0, binding: KeyAmalgamationBinding::Subordinate(x.1), time: x.2, - } + binding_signature: + x.1.binding_signature(x.2) + .ok_or_else(|| Error::NoBindingSignature(x.2))?, + }) } } @@ -73,21 +89,25 @@ impl<'a> From> cert, binding: KeyAmalgamationBinding::Primary(), time, + binding_signature, } => KeyAmalgamation { cert, binding: KeyAmalgamationBinding::Primary(), time, + binding_signature, }, KeyAmalgamation { cert, binding: KeyAmalgamationBinding::Subordinate(binding), time, + binding_signature, } => KeyAmalgamation { cert, binding: KeyAmalgamationBinding::Subordinate(binding.into()), time, + binding_signature, }, } } @@ -101,22 +121,26 @@ impl<'a> From> KeyAmalgamation { cert, binding: KeyAmalgamationBinding::Primary(), - time + time, + binding_signature, } => KeyAmalgamation { cert, binding: KeyAmalgamationBinding::Primary(), time, + binding_signature, }, KeyAmalgamation { cert, binding: KeyAmalgamationBinding::Subordinate(binding), - time + time, + binding_signature, } => KeyAmalgamation { cert, binding: KeyAmalgamationBinding::Subordinate(binding.into()), time, + binding_signature, }, } } @@ -132,7 +156,8 @@ impl<'a> TryFrom> KeyAmalgamation { cert, binding: KeyAmalgamationBinding::Primary(), - time + time, + binding_signature, } => { // Error out if the primary key does not have secret // key material. @@ -142,17 +167,20 @@ impl<'a> TryFrom> cert, binding: KeyAmalgamationBinding::Primary(), time, + binding_signature, } } KeyAmalgamation { cert, binding: KeyAmalgamationBinding::Subordinate(binding), - time + time, + binding_signature, } => KeyAmalgamation { cert, binding: KeyAmalgamationBinding::Subordinate(binding.try_into()?), time, + binding_signature, }, }) } @@ -202,23 +230,32 @@ impl<'a, P: 'a + key::KeyParts> KeyAmalgamation<'a, P> { /// Changes the amalgamation's reference time. /// /// If `time` is `None`, the current time is used. - pub fn set_time(mut self, time: T) -> Self + pub fn set_time(mut self, time: T) -> Result where T: Into> { self.time = time.into().unwrap_or_else(SystemTime::now); - self + self.binding_signature = match self { + KeyAmalgamation { + binding: KeyAmalgamationBinding::Primary(), + .. + } => + self.cert.primary_key_signature(self.time) + .ok_or_else(|| Error::NoBindingSignature(self.time))?, + KeyAmalgamation { + binding: KeyAmalgamationBinding::Subordinate(binding), + .. + } => + binding.binding_signature(self.time) + .ok_or_else(|| Error::NoBindingSignature(self.time))?, + }; + Ok(self) } /// Returns the key's binding signature as of the reference time, /// if any. - pub fn binding_signature(&self) -> Option<&'a Signature> + pub fn binding_signature(&self) -> &'a Signature { - match self { - KeyAmalgamation { binding: KeyAmalgamationBinding::Primary(), .. } => - self.cert.primary_key_signature(self.time()), - KeyAmalgamation { binding: KeyAmalgamationBinding::Subordinate(ref binding), .. } => - binding.binding_signature(self.time()), - } + self.binding_signature } /// Returns the key's revocation status as of the amalgamation's @@ -246,7 +283,7 @@ impl<'a, P: 'a + key::KeyParts> KeyAmalgamation<'a, P> { /// reference time. pub fn key_flags(&self) -> Option { - self.binding_signature().map(|sig| sig.key_flags()) + Some(self.binding_signature.key_flags()) } /// Returns whether the key has at least one of the specified key @@ -308,11 +345,7 @@ impl<'a, P: 'a + key::KeyParts> KeyAmalgamation<'a, P> { /// Note: this does not return whether the certificate is valid. pub fn alive(&self) -> Result<()> { - if let Some(sig) = self.binding_signature() { - sig.key_alive(self.generic_key(), self.time()) - } else { - Err(Error::NoBindingSignature(self.time()).into()) - } + self.binding_signature.key_alive(self.generic_key(), self.time()) } /// Returns whether the key contains secret key material. diff --git a/openpgp/src/cert/keyiter.rs b/openpgp/src/cert/keyiter.rs index a9a689a8..c40658eb 100644 --- a/openpgp/src/cert/keyiter.rs +++ b/openpgp/src/cert/keyiter.rs @@ -303,7 +303,7 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> { if let Some(cert) = self.cert.as_ref() { let time = time.into().unwrap_or_else(std::time::SystemTime::now); - let ka: KeyAmalgamation<'a, P> = (*cert, time).into(); + let ka: KeyAmalgamation<'a, P> = (*cert, time).try_into()?; ka.alive()?; Ok(ka) } else { @@ -474,9 +474,23 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> ValidKeyIter<'a, P, R> { loop { let ka : KeyAmalgamation<'a, key::PublicParts> = if ! self.primary { self.primary = true; - (cert, self.time).into() + match (cert, self.time).try_into() { + Ok(ka) => ka, + Err(err) => { + // The primary key is bad. Abort. + t!("Getting primary key: {:?}", err); + return None; + } + } } else { - (cert, self.subkey_iter.next()?, self.time).into() + match (cert, self.subkey_iter.next()?, self.time).try_into() { + Ok(ka) => ka, + Err(err) => { + // The subkey is bad, abort. + t!("Getting subkey: {:?}", err); + continue; + } + } }; let key = ka.key(); @@ -493,13 +507,7 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> ValidKeyIter<'a, P, R> { } } - let binding_signature - = if let Some(binding_signature) = ka.binding_signature() { - binding_signature - } else { - t!("No self-signature at time {:?}", self.time); - continue - }; + let binding_signature = ka.binding_signature(); if let Some(flags) = self.flags.as_ref() { if !ka.has_any_key_flag(flags) { -- cgit v1.2.3