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.

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::<Vec<openpgp::serialize::stream::Recipient>>();
 
     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<KeyHandle>,
 
     _p: std::marker::PhantomData<P>,
-    _r: std::marker::PhantomData<R>,
 }
 
-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<SecretParts, _> 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<key::PublicParts,
-                                                   key::UnspecifiedRole>>
+        impl<'a> Iterator for KeyIter<'a, $parts>
         {
-            type Item = &'a Key<$parts, R>;
+            type Item = KeyAmalgamation<'a, $parts>;
 
             fn next(&mut self) -> Option<Self::Item> {
                 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<key::SecretParts, R>: From<&'a Key<key::SecretParts,
-                                                     key::UnspecifiedRole>>
-{
-    type Item = &'a Key<key::SecretParts, key::UnspecifiedRole>;
+impl<'a> Iterator for KeyIter<'a, key::SecretParts> {
+    type Item = KeyAmalgamation<'a, key::SecretParts>;
 
     fn next(&mut self) -> Option<Self::Item> {
         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<key::PublicParts,
-                                                key::UnspecifiedRole>>
+impl<'a, P: 'a + key::KeyParts> KeyIter<'a, P> {
+    fn next_common(&mut self) -> Option<KeyAmalgamation<'a, key::PublicParts>>
     {
         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<key::PublicParts, key::UnspecifiedRole>
+            let ka : KeyAmalgamation<key::PublicParts>
                 = 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<T>(self, time: T) -> ValidKeyIter<'a, P, R>
+    pub fn policy<T>(self, time: T) -> ValidKeyIter<'a, P>
         where T: Into<Option<SystemTime>>
     {
         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<bool>,
 
     _p: std::marker::PhantomData<P>,
-    _r: std::marker::PhantomData<R>,
 }
 
-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<SecretParts, _> 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<key::PublicParts,
-                                                   key::UnspecifiedRole>>
+        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<key::SecretParts, R>: From<&'a Key<key::SecretParts,
-                                                     key::UnspecifiedRole>>
+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<ValidKeyAmalgamation<'a, key::PublicParts>>
     {
         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::<Vec<_>>();
+        let keyids = cert.keys().map(|ka| ka.key().keyid()).collect::<Vec<_>>();
 
         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::<Vec<KeyHandle>>(),
                     &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<key::PublicParts, key::UnspecifiedRole>
+    pub fn keys(&self) -> KeyIter<key::PublicParts>
     {
         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.