From f2f1db13b429de78745108a63f1ff7f21a4a3552 Mon Sep 17 00:00:00 2001 From: "Neal H. Walfield" Date: Tue, 21 Jan 2020 12:36:21 +0100 Subject: openpgp: Change KeyIter to return KeyAmalgamations. - Change KeyIter to return KeyAmalgamations instead of Keys. - Given a `KeyAmalgamation`, it is possible to turn it into a `ValidKeyAmalgamation`. This is not possible with a `Key`. - With a `KeyAmalgamation`, it is still possible to query things about the certificate. --- net/src/lib.rs | 4 +-- openpgp-ffi/src/cert.rs | 10 +++--- openpgp-ffi/src/serialize.rs | 2 +- openpgp/src/cert/keyiter.rs | 84 ++++++++++++++++--------------------------- openpgp/src/cert/mod.rs | 2 +- openpgp/src/crypto/keygrip.rs | 2 +- openpgp/src/crypto/mpis.rs | 4 +-- openpgp/src/parse/stream.rs | 8 ++--- store/src/backend/mod.rs | 4 +-- tool/src/commands/mod.rs | 2 +- 10 files changed, 49 insertions(+), 73 deletions(-) diff --git a/net/src/lib.rs b/net/src/lib.rs index 048031fe..f7add939 100644 --- a/net/src/lib.rs +++ b/net/src/lib.rs @@ -182,8 +182,8 @@ impl KeyServer { Some(armor::Kind::PublicKey))); match Cert::from_reader(r) { Ok(cert) => { - if cert.keys().any(|key| { - KeyID::from(key.fingerprint()) + if cert.keys().any(|ka| { + KeyID::from(ka.key().fingerprint()) == keyid_want }) { future::done(Ok(cert)) diff --git a/openpgp-ffi/src/cert.rs b/openpgp-ffi/src/cert.rs index 241453c9..7dadadf0 100644 --- a/openpgp-ffi/src/cert.rs +++ b/openpgp-ffi/src/cert.rs @@ -433,8 +433,7 @@ pub extern "C" fn pgp_user_id_binding_iter_next<'a>( /// Wraps a KeyIter for export via the FFI. pub struct KeyIterWrapper<'a> { pub(crate) // For serialize.rs. - iter: KeyIter<'a, openpgp::packet::key::PublicParts, - openpgp::packet::key::UnspecifiedRole>, + iter: KeyIter<'a, openpgp::packet::key::PublicParts>, // Whether next has been called. next_called: bool, } @@ -539,8 +538,8 @@ pub extern "C" fn pgp_cert_key_iter_next<'a>( let iter_wrapper = ffi_param_ref_mut!(iter_wrapper); iter_wrapper.next_called = true; - if let Some(key) = iter_wrapper.iter.next() { - Some(key.mark_parts_unspecified_ref().mark_role_unspecified_ref()) + if let Some(ka) = iter_wrapper.iter.next() { + Some(ka.key().mark_parts_unspecified_ref().mark_role_unspecified_ref()) .move_into_raw() } else { None @@ -550,8 +549,7 @@ pub extern "C" fn pgp_cert_key_iter_next<'a>( /// Wraps a ValidKeyIter for export via the FFI. pub struct ValidKeyIterWrapper<'a> { pub(crate) // For serialize.rs. - iter: ValidKeyIter<'a, openpgp::packet::key::PublicParts, - openpgp::packet::key::UnspecifiedRole>, + iter: ValidKeyIter<'a, openpgp::packet::key::PublicParts>, // Whether next has been called. next_called: bool, } diff --git a/openpgp-ffi/src/serialize.rs b/openpgp-ffi/src/serialize.rs index db9e708a..265ba9ca 100644 --- a/openpgp-ffi/src/serialize.rs +++ b/openpgp-ffi/src/serialize.rs @@ -283,7 +283,7 @@ fn pgp_recipients_from_key_iter<'a>( let result_len = ffi_param_ref_mut!(result_len); let recipients = iter_wrapper.iter - .map(|key| key.into()) + .map(|ka| ka.key().into()) .collect::>(); let result = unsafe { diff --git a/openpgp/src/cert/keyiter.rs b/openpgp/src/cert/keyiter.rs index 831f15b9..48276d97 100644 --- a/openpgp/src/cert/keyiter.rs +++ b/openpgp/src/cert/keyiter.rs @@ -7,7 +7,6 @@ use crate::{ KeyHandle, RevocationStatus, packet::key, - packet::Key, packet::key::SecretKeyMaterial, types::KeyFlags, cert::{ @@ -38,7 +37,7 @@ use crate::{ /// include secret key material. Of course, since `KeyIter` /// implements `Iterator`, it is possible to use `Iterator::filter` to /// implement custom filters. -pub struct KeyIter<'a, P: key::KeyParts, R: key::KeyRole> { +pub struct KeyIter<'a, P: key::KeyParts> { // This is an option to make it easier to create an empty KeyIter. cert: Option<&'a Cert>, primary: bool, @@ -57,11 +56,9 @@ pub struct KeyIter<'a, P: key::KeyParts, R: key::KeyRole> { key_handles: Vec, _p: std::marker::PhantomData

, - _r: std::marker::PhantomData, } -impl<'a, P: key::KeyParts, R: key::KeyRole> fmt::Debug - for KeyIter<'a, P, R> +impl<'a, P: key::KeyParts> fmt::Debug for KeyIter<'a, P> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.debug_struct("KeyIter") @@ -78,11 +75,9 @@ impl<'a, P: key::KeyParts, R: key::KeyRole> fmt::Debug // implementation for Key below. macro_rules! impl_iterator { ($parts:path) => { - impl<'a, R: 'a + key::KeyRole> Iterator for KeyIter<'a, $parts, R> - where &'a Key<$parts, R>: From<&'a Key> + impl<'a> Iterator for KeyIter<'a, $parts> { - type Item = &'a Key<$parts, R>; + type Item = KeyAmalgamation<'a, $parts>; fn next(&mut self) -> Option { self.next_common().map(|k| k.into()) @@ -93,21 +88,16 @@ macro_rules! impl_iterator { impl_iterator!(key::PublicParts); impl_iterator!(key::UnspecifiedParts); -impl<'a, R: 'a + key::KeyRole> Iterator for KeyIter<'a, key::SecretParts, R> - where &'a Key: From<&'a Key> -{ - type Item = &'a Key; +impl<'a> Iterator for KeyIter<'a, key::SecretParts> { + type Item = KeyAmalgamation<'a, key::SecretParts>; fn next(&mut self) -> Option { self.next_common().map(|k| k.try_into().expect("has secret parts")) } } -impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> -{ - fn next_common(&mut self) -> Option<&'a Key> +impl<'a, P: 'a + key::KeyParts> KeyIter<'a, P> { + fn next_common(&mut self) -> Option> { tracer!(false, "KeyIter::next", 0); t!("KeyIter: {:?}", self); @@ -118,29 +108,29 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> let cert = self.cert.unwrap(); loop { - let key : &Key + let ka : KeyAmalgamation = if ! self.primary { self.primary = true; - cert.primary.key().into() + cert.into() } else { - self.subkey_iter.next()?.key().into() + (cert, self.subkey_iter.next()?).into() }; - t!("Considering key: {:?}", key); + t!("Considering key: {:?}", ka.key()); if self.key_handles.len() > 0 { if !self.key_handles .iter() - .any(|h| h.aliases(key.key_handle())) + .any(|h| h.aliases(ka.key().key_handle())) { t!("{} is not one of the keys that we are looking for ({:?})", - key.fingerprint(), self.key_handles); + ka.key().fingerprint(), self.key_handles); continue; } } if let Some(want_secret) = self.secret { - if key.secret().is_some() { + if ka.key().secret().is_some() { // We have a secret. if ! want_secret { t!("Have a secret... skipping."); @@ -155,7 +145,7 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> } if let Some(want_unencrypted_secret) = self.unencrypted_secret { - if let Some(secret) = key.secret() { + if let Some(secret) = ka.key().secret() { if let SecretKeyMaterial::Unencrypted { .. } = secret { if ! want_unencrypted_secret { t!("Unencrypted secret... skipping."); @@ -174,12 +164,12 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> } } - return Some(key); + return Some(ka); } } } -impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> +impl<'a, P: 'a + key::KeyParts> KeyIter<'a, P> { /// Returns a new `KeyIter` instance. pub(crate) fn new(cert: &'a Cert) -> Self where Self: 'a { @@ -194,12 +184,11 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> key_handles: Vec::with_capacity(0), _p: std::marker::PhantomData, - _r: std::marker::PhantomData, } } /// Changes the filter to only return keys with secret key material. - pub fn secret(self) -> KeyIter<'a, key::SecretParts, R> { + pub fn secret(self) -> KeyIter<'a, key::SecretParts> { KeyIter { cert: self.cert, primary: self.primary, @@ -211,13 +200,12 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> key_handles: self.key_handles, _p: std::marker::PhantomData, - _r: std::marker::PhantomData, } } /// Changes the filter to only return keys with unencrypted secret /// key material. - pub fn unencrypted_secret(self) -> KeyIter<'a, key::SecretParts, R> { + pub fn unencrypted_secret(self) -> KeyIter<'a, key::SecretParts> { KeyIter { cert: self.cert, primary: self.primary, @@ -229,7 +217,6 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> key_handles: self.key_handles, _p: std::marker::PhantomData, - _r: std::marker::PhantomData, } } @@ -270,7 +257,7 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> /// message, you want keys that were valid when the message was /// encrypted, because these are the only keys that the encryptor /// could have used. The same holds when verifying a message. - pub fn policy(self, time: T) -> ValidKeyIter<'a, P, R> + pub fn policy(self, time: T) -> ValidKeyIter<'a, P> where T: Into> { ValidKeyIter { @@ -288,7 +275,6 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> revoked: None, _p: self._p, - _r: self._r, } } @@ -371,7 +357,7 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> /// `ValidKeyIter` follows the builder pattern. There is no need to /// explicitly finalize it, however: it already implements the /// `Iterator` trait. -pub struct ValidKeyIter<'a, P: key::KeyParts, R: key::KeyRole> { +pub struct ValidKeyIter<'a, P: key::KeyParts> { // This is an option to make it easier to create an empty ValidKeyIter. cert: Option<&'a Cert>, primary: bool, @@ -403,11 +389,9 @@ pub struct ValidKeyIter<'a, P: key::KeyParts, R: key::KeyRole> { revoked: Option, _p: std::marker::PhantomData

, - _r: std::marker::PhantomData, } -impl<'a, P: key::KeyParts, R: key::KeyRole> fmt::Debug - for ValidKeyIter<'a, P, R> +impl<'a, P: key::KeyParts> fmt::Debug for ValidKeyIter<'a, P> { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.debug_struct("ValidKeyIter") @@ -428,9 +412,7 @@ impl<'a, P: key::KeyParts, R: key::KeyRole> fmt::Debug // implementation for Key below. macro_rules! impl_valid_key_iterator { ($parts:path) => { - impl<'a, R: 'a + key::KeyRole> Iterator for ValidKeyIter<'a, $parts, R> - where &'a Key<$parts, R>: From<&'a Key> + impl<'a> Iterator for ValidKeyIter<'a, $parts> { type Item = ValidKeyAmalgamation<'a, $parts>; @@ -443,9 +425,7 @@ macro_rules! impl_valid_key_iterator { impl_valid_key_iterator!(key::PublicParts); impl_valid_key_iterator!(key::UnspecifiedParts); -impl<'a, R: 'a + key::KeyRole> Iterator for ValidKeyIter<'a, key::SecretParts, R> - where &'a Key: From<&'a Key> +impl<'a> Iterator for ValidKeyIter<'a, key::SecretParts> { type Item = ValidKeyAmalgamation<'a, key::SecretParts>; @@ -454,7 +434,7 @@ impl<'a, R: 'a + key::KeyRole> Iterator for ValidKeyIter<'a, key::SecretParts, R } } -impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> ValidKeyIter<'a, P, R> { +impl<'a, P: 'a + key::KeyParts> ValidKeyIter<'a, P> { fn next_common(&mut self) -> Option> { tracer!(false, "ValidKeyIter::next", 0); @@ -585,7 +565,7 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> ValidKeyIter<'a, P, R> { } } -impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> ValidKeyIter<'a, P, R> +impl<'a, P: 'a + key::KeyParts> ValidKeyIter<'a, P> { /// Returns keys that have the at least one of the flags specified /// in `flags`. @@ -723,7 +703,7 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> ValidKeyIter<'a, P, R> } /// Changes the filter to only return keys with secret key material. - pub fn secret(self) -> ValidKeyIter<'a, key::SecretParts, R> { + pub fn secret(self) -> ValidKeyIter<'a, key::SecretParts> { ValidKeyIter { cert: self.cert, primary: self.primary, @@ -740,13 +720,12 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> ValidKeyIter<'a, P, R> revoked: self.revoked, _p: std::marker::PhantomData, - _r: std::marker::PhantomData, } } /// Changes the filter to only return keys with unencrypted secret /// key material. - pub fn unencrypted_secret(self) -> ValidKeyIter<'a, key::SecretParts, R> { + pub fn unencrypted_secret(self) -> ValidKeyIter<'a, key::SecretParts> { ValidKeyIter { cert: self.cert, primary: self.primary, @@ -763,7 +742,6 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> ValidKeyIter<'a, P, R> revoked: self.revoked, _p: std::marker::PhantomData, - _r: std::marker::PhantomData, } } @@ -1045,7 +1023,7 @@ mod test { let keys = cert.keys().count(); assert_eq!(keys, 6); - let keyids = cert.keys().map(|key| key.keyid()).collect::>(); + let keyids = cert.keys().map(|ka| ka.key().keyid()).collect::>(); fn check(got: &[KeyHandle], expected: &[KeyHandle]) { if expected.len() != got.len() { @@ -1069,7 +1047,7 @@ mod test { check( &cert.keys().key_handles(keyids.iter()) - .map(|ka| ka.key_handle()) + .map(|ka| ka.key().key_handle()) .collect::>(), &keyids); check( diff --git a/openpgp/src/cert/mod.rs b/openpgp/src/cert/mod.rs index b6011200..47d458ca 100644 --- a/openpgp/src/cert/mod.rs +++ b/openpgp/src/cert/mod.rs @@ -704,7 +704,7 @@ impl Cert { /// /// That is, this returns an iterator over the primary key and any /// subkeys. - pub fn keys(&self) -> KeyIter + pub fn keys(&self) -> KeyIter { KeyIter::new(self) } diff --git a/openpgp/src/crypto/keygrip.rs b/openpgp/src/crypto/keygrip.rs index a6095ef4..d1b8148e 100644 --- a/openpgp/src/crypto/keygrip.rs +++ b/openpgp/src/crypto/keygrip.rs @@ -340,7 +340,7 @@ mod tests { .iter().map(|n| (n, crate::Cert::from_bytes(crate::tests::key(n)).unwrap())) { eprintln!("{}", name); - for key in cert.keys() { + for key in cert.keys().map(|a| a.key()) { let fp = key.fingerprint(); eprintln!("(sub)key: {}", fp); assert_eq!(&key.mpis().keygrip().unwrap(), diff --git a/openpgp/src/crypto/mpis.rs b/openpgp/src/crypto/mpis.rs index 84aa21f5..894c7679 100644 --- a/openpgp/src/crypto/mpis.rs +++ b/openpgp/src/crypto/mpis.rs @@ -1061,8 +1061,8 @@ mod tests { ("erika-corinna-daniela-simone-antonia-nistp521.pgp", 0, 521), ] { let cert = crate::Cert::from_bytes(crate::tests::key(name)).unwrap(); - let key = cert.keys().nth(*key_no).unwrap(); - assert_eq!(key.mpis().bits().unwrap(), *bits, + let ka = cert.keys().nth(*key_no).unwrap(); + assert_eq!(ka.key().mpis().bits().unwrap(), *bits, "Cert {}, key no {}", name, *key_no); } } diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs index f8b77b28..79b4c7e8 100644 --- a/openpgp/src/parse/stream.rs +++ b/openpgp/src/parse/stream.rs @@ -817,7 +817,7 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { // less misleading) error message than // `VerificationResult::MissingKey`. if let VerificationResult::MissingKey { .. } = err { - if let Some(key) = self.certs.iter() + if let Some(ka) = self.certs.iter() .flat_map(|cert| { cert.keys().key_handles(issuers.iter()) }) @@ -829,7 +829,7 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> { format!( "Signing key ({}) not valid \ when signature was created", - key.fingerprint())) + ka.key().fingerprint())) .into(), } } @@ -1769,7 +1769,7 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> { // less misleading) error message than // `VerificationResult::MissingKey`. if let VerificationResult::MissingKey { .. } = err { - if let Some(key) = self.certs.iter() + if let Some(ka) = self.certs.iter() .flat_map(|cert| { cert.keys().key_handles(issuers.iter()) }) @@ -1781,7 +1781,7 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> { format!( "Signing key ({}) not valid \ when signature was created", - key.fingerprint())) + ka.key().fingerprint())) .into(), } } diff --git a/store/src/backend/mod.rs b/store/src/backend/mod.rs index b9fc1a9a..412011b1 100644 --- a/store/src/backend/mod.rs +++ b/store/src/backend/mod.rs @@ -806,8 +806,8 @@ impl KeyServer { /// Keeps the mapping of (sub)KeyIDs to keys up-to-date. fn reindex_subkeys(c: &Connection, key_id: ID, cert: &Cert) -> Result<()> { - for key in cert.keys() { - let keyid = key.keyid().as_u64() + for ka in cert.keys() { + let keyid = ka.key().keyid().as_u64() .expect("computed keyid is valid"); let r = c.execute( diff --git a/tool/src/commands/mod.rs b/tool/src/commands/mod.rs index 7dd6869a..24ce3337 100644 --- a/tool/src/commands/mod.rs +++ b/tool/src/commands/mod.rs @@ -322,7 +322,7 @@ impl<'a> VerificationHelper for VHelper<'a> { // Get all keys. let seen: HashSet<_> = certs.iter() .flat_map(|cert| { - cert.keys().map(|key| key.fingerprint().into()) + cert.keys().map(|ka| ka.key().fingerprint().into()) }).collect(); // Explicitly provided keys are trusted. -- cgit v1.2.3