openpgp: Change KeyIter to return a struct instead of a tuple.

  - A tuple is just an unnamed, inflexible struct.  Use a struct
    instead.

  - Fixes #400.

17 files changed, 251 insertions, 63 deletions

diff --git a/openpgp/examples/decrypt-with.rs b/openpgp/examples/decrypt-with.rs
index 224a8726..7c924cc9 100644
--- a/openpgp/examples/decrypt-with.rs
+++ b/openpgp/examples/decrypt-with.rs
@@ -58,16 +58,17 @@ impl Helper {
         // Map (sub)KeyIDs to secrets.
         let mut keys = HashMap::new();
         for cert in certs {
-            for (sig, _, key) in cert.keys_all() {
-                if sig.map(|s| (s.key_flags().for_storage_encryption()
-                                || s.key_flags().for_transport_encryption()))
+            for ka in cert.keys_all() {
+                if ka.binding_signature(None)
+                    .map(|s| (s.key_flags().for_storage_encryption()
+                              || s.key_flags().for_transport_encryption()))
                     .unwrap_or(false)
                 {
                     // This only works for unencrypted secret keys.
                     if let Ok(keypair) =
-                        key.clone().mark_parts_secret().unwrap().into_keypair()
+                        ka.key().clone().mark_parts_secret().unwrap().into_keypair()
                     {
-                        keys.insert(key.keyid(), keypair);
+                        keys.insert(ka.key().keyid(), keypair);
                     }
                 }
             }
diff --git a/openpgp/examples/encrypt-for.rs b/openpgp/examples/encrypt-for.rs
index f3004532..eceec5b3 100644
--- a/openpgp/examples/encrypt-for.rs
+++ b/openpgp/examples/encrypt-for.rs
@@ -38,7 +38,7 @@ fn main() {
     let mut recipients =
         certs.iter()
         .flat_map(|cert| cert.keys_valid().key_flags(mode.clone()))
-        .map(|(_, _, key)| key.into())
+        .map(|ka| ka.key().into())
         .collect::<Vec<_>>();
 
     // Compose a writer stack corresponding to the output format and
diff --git a/openpgp/examples/generate-encrypt-decrypt.rs b/openpgp/examples/generate-encrypt-decrypt.rs
index 55ea89d6..01782b47 100644
--- a/openpgp/examples/generate-encrypt-decrypt.rs
+++ b/openpgp/examples/generate-encrypt-decrypt.rs
@@ -44,7 +44,7 @@ fn encrypt(sink: &mut dyn Write, plaintext: &str, recipient: &openpgp::Cert)
     let mut recipients =
         recipient.keys_valid()
         .for_transport_encryption()
-        .map(|(_, _, key)| key.into())
+        .map(|ka| ka.key().into())
         .collect::<Vec<_>>();
 
     // Start streaming an OpenPGP message.
diff --git a/openpgp/examples/generate-sign-verify.rs b/openpgp/examples/generate-sign-verify.rs
index 6e3d8084..1cccac44 100644
--- a/openpgp/examples/generate-sign-verify.rs
+++ b/openpgp/examples/generate-sign-verify.rs
@@ -40,8 +40,8 @@ fn generate() -> openpgp::Result<openpgp::Cert> {
 fn sign(sink: &mut dyn Write, plaintext: &str, tsk: &openpgp::Cert)
            -> openpgp::Result<()> {
     // Get the keypair to do the signing from the Cert.
-    let keypair = tsk.keys_valid().for_signing().nth(0).unwrap().2
-        .clone().mark_parts_secret().unwrap().into_keypair()?;
+    let keypair = tsk.keys_valid().for_signing().nth(0).unwrap()
+        .key().clone().mark_parts_secret().unwrap().into_keypair()?;
 
     // Start streaming an OpenPGP message.
     let message = Message::new(sink);
diff --git a/openpgp/examples/notarize.rs b/openpgp/examples/notarize.rs
index ce007968..f52466f4 100644
--- a/openpgp/examples/notarize.rs
+++ b/openpgp/examples/notarize.rs
@@ -28,7 +28,7 @@ fn main() {
             .expect("Failed to read key");
         let mut n = 0;
 
-        for (_, _, key) in tsk.keys_valid().for_signing().secret() {
+        for key in tsk.keys_valid().for_signing().secret().map(|ka| ka.key()) {
             keys.push({
                 let mut key = key.clone();
                 if key.secret().expect("filtered").is_encrypted() {
diff --git a/openpgp/examples/pad.rs b/openpgp/examples/pad.rs
index a13e53d5..32583a42 100644
--- a/openpgp/examples/pad.rs
+++ b/openpgp/examples/pad.rs
@@ -40,7 +40,7 @@ fn main() {
     let mut recipients =
         certs.iter()
         .flat_map(|cert| cert.keys_valid().key_flags(mode.clone()))
-        .map(|(_, _, key)| Recipient::new(KeyID::wildcard(), key))
+        .map(|ka| Recipient::new(KeyID::wildcard(), ka.key()))
         .collect::<Vec<_>>();
 
     // Compose a writer stack corresponding to the output format and
diff --git a/openpgp/examples/sign-detached.rs b/openpgp/examples/sign-detached.rs
index 0fee0e36..20d4150b 100644
--- a/openpgp/examples/sign-detached.rs
+++ b/openpgp/examples/sign-detached.rs
@@ -24,7 +24,7 @@ fn main() {
             .expect("Failed to read key");
         let mut n = 0;
 
-        for (_, _, key) in tsk.keys_valid().for_signing().secret() {
+        for key in tsk.keys_valid().for_signing().secret().map(|ka| ka.key()) {
             keys.push({
                 let mut key = key.clone();
                 if key.secret().expect("filtered").is_encrypted() {
diff --git a/openpgp/examples/sign.rs b/openpgp/examples/sign.rs
index 195ac508..7fae29ef 100644
--- a/openpgp/examples/sign.rs
+++ b/openpgp/examples/sign.rs
@@ -23,7 +23,7 @@ fn main() {
             .expect("Failed to read key");
         let mut n = 0;
 
-        for (_, _, key) in tsk.keys_valid().for_signing().secret() {
+        for key in tsk.keys_valid().for_signing().secret().map(|ka| ka.key()) {
             keys.push({
                 let mut key = key.clone();
                 if key.secret().expect("filtered").is_encrypted() {
diff --git a/openpgp/src/cert/builder.rs b/openpgp/src/cert/builder.rs
index ce1a0a6c..24bc5fe7 100644
--- a/openpgp/src/cert/builder.rs
+++ b/openpgp/src/cert/builder.rs
@@ -656,13 +656,17 @@ mod tests {
         assert!(! sig.key_alive(key, now + 610 * s).is_ok());
 
         let (sig, key) = cert.keys_valid().for_signing()
-            .nth(0).map(|(s, _, k)| (s.unwrap(), k)).unwrap();
+            .nth(0).map(|ka| {
+                (ka.binding_signature(None).unwrap(), ka.key())
+            }).unwrap();
         assert!(sig.key_alive(key, now).is_ok());
         assert!(sig.key_alive(key, now + 290 * s).is_ok());
         assert!(! sig.key_alive(key, now + 310 * s).is_ok());
 
         let (sig, key) = cert.keys_valid().for_authentication()
-            .nth(0).map(|(s, _, k)| (s.unwrap(), k)).unwrap();
+            .nth(0).map(|ka| {
+                (ka.binding_signature(None).unwrap(), ka.key())
+            }).unwrap();
         assert!(sig.key_alive(key, now).is_ok());
         assert!(sig.key_alive(key, now + 590 * s).is_ok());
         assert!(! sig.key_alive(key, now + 610 * s).is_ok());
diff --git a/openpgp/src/cert/key_amalgamation.rs b/openpgp/src/cert/key_amalgamation.rs
new file mode 100644
index 00000000..af7f7a7e
--- /dev/null
+++ b/openpgp/src/cert/key_amalgamation.rs
@@ -0,0 +1,183 @@
+use std::time;
+use std::convert::TryInto;
+use std::convert::TryFrom;
+
+use crate::{
+    Cert,
+    cert::KeyBinding,
+    packet::key,
+    packet::Key,
+    packet::Signature,
+    Result,
+    RevocationStatus,
+};
+
+/// A variant of `KeyAmalgamation` for primary keys.
+#[derive(Debug)]
+struct PrimaryKeyAmalgamation<'a, P: key::KeyParts> {
+    cert: &'a Cert,
+    binding: &'a KeyBinding<P, key::PrimaryRole>,
+}
+
+/// A variant of `KeyAmalgamation` for subkeys.
+#[derive(Debug)]
+struct SubordinateKeyAmalgamation<'a, P: key::KeyParts> {
+    cert: &'a Cert,
+    binding: &'a KeyBinding<P, key::SubordinateRole>,
+}
+
+/// The underlying `KeyAmalgamation` type.
+///
+/// We don't make this type public, because an enum's variant types
+/// must also all be public, and we don't want that here.  Wrapping
+/// this in a struct means that we can hide that.
+#[derive(Debug)]
+enum KeyAmalgamation0<'a, P: key::KeyParts> {
+    Primary(PrimaryKeyAmalgamation<'a, P>),
+    Subordinate(SubordinateKeyAmalgamation<'a, P>),
+}
+
+/// A `Key` and its associated data.
+#[derive(Debug)]
+pub struct KeyAmalgamation<'a, P: key::KeyParts>(KeyAmalgamation0<'a, P>);
+
+impl<'a, P> From<(&'a Cert, &'a KeyBinding<P, key::PrimaryRole>)>
+    for KeyAmalgamation<'a, P>
+    where P: key::KeyParts
+{
+    fn from(x: (&'a Cert, &'a KeyBinding<P, key::PrimaryRole>)) -> Self {
+        KeyAmalgamation(KeyAmalgamation0::Primary(PrimaryKeyAmalgamation {
+            cert: x.0,
+            binding: x.1,
+        }))
+    }
+}
+
+impl<'a, P> From<(&'a Cert, &'a KeyBinding<P, key::SubordinateRole>)>
+    for KeyAmalgamation<'a, P>
+    where P: key::KeyParts
+{
+    fn from(x: (&'a Cert, &'a KeyBinding<P, key::SubordinateRole>)) -> Self {
+        KeyAmalgamation(KeyAmalgamation0::Subordinate(SubordinateKeyAmalgamation {
+            cert: x.0,
+            binding: x.1,
+        }))
+    }
+}
+
+// We can't make the key parts generic, because then the impl would
+// conflict with 'impl<T> std::convert::From<T> for T'.
+impl<'a> From<KeyAmalgamation<'a, key::PublicParts>>
+    for KeyAmalgamation<'a, key::UnspecifiedParts>
+{
+    fn from(ka: KeyAmalgamation<'a, key::PublicParts>) -> Self {
+        match ka {
+            KeyAmalgamation(KeyAmalgamation0::Primary(ka)) => {
+                KeyAmalgamation(KeyAmalgamation0::Primary(
+                    PrimaryKeyAmalgamation {
+                        cert: ka.cert,
+                        binding: ka.binding.into(),
+                    })
+                )
+            }
+            KeyAmalgamation(KeyAmalgamation0::Subordinate(ka)) => {
+                KeyAmalgamation(KeyAmalgamation0::Subordinate(
+                    SubordinateKeyAmalgamation {
+                        cert: ka.cert,
+                        binding: ka.binding.into(),
+                    })
+                )
+            }
+        }
+    }
+}
+
+impl<'a> From<KeyAmalgamation<'a, key::SecretParts>>
+    for KeyAmalgamation<'a, key::PublicParts>
+{
+    fn from(ka: KeyAmalgamation<'a, key::SecretParts>) -> Self {
+        match ka {
+            KeyAmalgamation(KeyAmalgamation0::Primary(ka)) => {
+                KeyAmalgamation(KeyAmalgamation0::Primary(
+                    PrimaryKeyAmalgamation {
+                        cert: ka.cert,
+                        binding: ka.binding.into(),
+                    })
+                )
+            }
+            KeyAmalgamation(KeyAmalgamation0::Subordinate(ka)) => {
+                KeyAmalgamation(KeyAmalgamation0::Subordinate(
+                    SubordinateKeyAmalgamation {
+                        cert: ka.cert,
+                        binding: ka.binding.into(),
+                    })
+                )
+            }
+        }
+    }
+}
+
+impl<'a> TryFrom<KeyAmalgamation<'a, key::PublicParts>>
+    for KeyAmalgamation<'a, key::SecretParts>
+{
+    type Error = failure::Error;
+
+    fn try_from(ka: KeyAmalgamation<'a, key::PublicParts>) -> Result<Self> {
+        Ok(match ka {
+            KeyAmalgamation(KeyAmalgamation0::Primary(ka)) => {
+                KeyAmalgamation(KeyAmalgamation0::Primary(
+                    PrimaryKeyAmalgamation {
+                        cert: ka.cert,
+                        binding: ka.binding.try_into()?,
+                    })
+                )
+            }
+            KeyAmalgamation(KeyAmalgamation0::Subordinate(ka)) => {
+                KeyAmalgamation(KeyAmalgamation0::Subordinate(
+                    SubordinateKeyAmalgamation {
+                        cert: ka.cert,
+                        binding: ka.binding.try_into()?,
+                    })
+                )
+            }
+        })
+    }
+}
+
+impl<'a, P: 'a + key::KeyParts> KeyAmalgamation<'a, P> {
+    /// Returns the key.
+    pub fn key(&self) -> &'a Key<P, key::UnspecifiedRole>
+        where &'a Key<P, key::UnspecifiedRole>: From<&'a key::PublicKey>
+    {
+        match self {
+            KeyAmalgamation(KeyAmalgamation0::Primary(ref h)) =>
+                h.cert.primary.key().into(),
+            KeyAmalgamation(KeyAmalgamation0::Subordinate(ref h)) =>
+                h.binding.key().into(),
+        }
+    }
+
+    /// Returns the key's binding signature at time `t`, if any.
+    pub fn binding_signature<T>(&self, t: T) -> Option<&'a Signature>
+        where T: Into<Option<time::SystemTime>>
+    {
+        match self {
+            KeyAmalgamation(KeyAmalgamation0::Primary(ref h)) =>
+                h.cert.primary_key_signature(t),
+            KeyAmalgamation(KeyAmalgamation0::Subordinate(ref h)) =>
+                h.binding.binding_signature(t),
+        }
+    }
+
+    /// Returns the key's revocation status at time `t`.
+    pub fn revoked<T>(&self, t: T) -> RevocationStatus<'a>
+        where T: Into<Option<time::SystemTime>>
+    {
+        match self {
+            KeyAmalgamation(KeyAmalgamation0::Primary(ref h)) =>
+                h.cert.revoked(t),
+            KeyAmalgamation(KeyAmalgamation0::Subordinate(ref h)) =>
+                h.binding.revoked(t),
+        }
+    }
+}
diff --git a/openpgp/src/cert/keyiter.rs b/openpgp/src/cert/keyiter.rs
index 565e4611..8c948f50 100644
--- a/openpgp/src/cert/keyiter.rs
+++ b/openpgp/src/cert/keyiter.rs
@@ -1,4 +1,5 @@
 use std::fmt;
+use std::convert::TryInto;
 
 use crate::{
     RevocationStatus,
@@ -6,9 +7,9 @@ use crate::{
     packet::Key,
     packet::key::SecretKeyMaterial,
     types::KeyFlags,
-    packet::Signature,
     Cert,
     cert::KeyBindingIter,
+    cert::KeyAmalgamation,
 };
 
 /// An iterator over all `Key`s (both the primary key and any subkeys)
@@ -76,11 +77,10 @@ macro_rules! impl_iterator {
             where &'a Key<$parts, R>: From<&'a Key<key::PublicParts,
                                                    key::UnspecifiedRole>>
         {
-            type Item = (Option<&'a Signature>, RevocationStatus<'a>,
-                         &'a Key<$parts, R>);
+            type Item = KeyAmalgamation<'a, $parts>;
 
             fn next(&mut self) -> Option<Self::Item> {
-                self.next_common().map(|(s, r, k)| (s, r, k.into()))
+                self.next_common().map(|ka| ka.into())
             }
         }
     }
@@ -92,22 +92,15 @@ 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 = (Option<&'a Signature>, RevocationStatus<'a>,
-                 &'a Key<key::SecretParts, R>);
+    type Item = KeyAmalgamation<'a, key::SecretParts>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        self.next_common()
-            .map(|(s, r, k)|
-                 (s, r,
-                  k.mark_parts_secret_ref().expect("has secret parts").into()))
+        self.next_common().map(|ka| ka.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<(Option<&'a Signature>,
-                              RevocationStatus<'a>,
-                              &'a Key<key::PublicParts, key::UnspecifiedRole>)>
+    fn next_common(&mut self) -> Option<KeyAmalgamation<'a, key::PublicParts>>
     {
         tracer!(false, "KeyIter::next", 0);
         t!("KeyIter: {:?}", self);
@@ -126,24 +119,20 @@ impl <'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> {
         }
 
         loop {
-            let (sigo, revoked, key) : (_, _, &key::UnspecifiedPublic)
-                = if ! self.primary {
-                    self.primary = true;
+            let ka : KeyAmalgamation<'a, key::PublicParts> = if ! self.primary {
+                self.primary = true;
+                (cert, &cert.primary).into()
+            } else {
+                (cert, self.subkey_iter.next()?).into()
+            };
 
-                    (cert.primary_key_signature(None),
-                     cert.revoked(None),
-                     cert.primary().into())
-                } else {
-                    self.subkey_iter.next()
-                        .map(|sk_binding| (sk_binding.binding_signature(None),
-                                           sk_binding.revoked(None),
-                                           sk_binding.key().into(),))?
-                };
+            let key = ka.key();
 
-            t!("Considering key: {:?}", key);
+            t!("Considering key: {:?}", ka);
 
             if let Some(flags) = self.flags.as_ref() {
-                if let Some(sig) = sigo {
+                // XXX: Shouldn't assume the current time.
+                if let Some(sig) = ka.binding_signature(None) {
                     if (&sig.key_flags() & &flags).is_empty() {
                         t!("Have flags: {:?}, want flags: {:?}... skipping.",
                            sig.key_flags(), flags);
@@ -157,7 +146,8 @@ impl <'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> {
             }
 
             if let Some(alive_at) = self.alive_at {
-                if let Some(sig) = sigo {
+                // XXX: Shouldn't assume the current time.
+                if let Some(sig) = ka.binding_signature(None) {
                     if ! sig.key_alive(key, alive_at).is_ok() {
                         t!("Key not alive... skipping.");
                         continue;
@@ -170,7 +160,8 @@ impl <'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> {
             }
 
             if let Some(want_revoked) = self.revoked {
-                if let RevocationStatus::Revoked(_) = revoked {
+                // XXX: Shouldn't assume the current time.
+                if let RevocationStatus::Revoked(_) = ka.revoked(None) {
                     // The key is definitely revoked.
                     if ! want_revoked {
                         t!("Key revoked... skipping.");
@@ -220,7 +211,7 @@ impl <'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyIter<'a, P, R> {
                 }
             }
 
-            return Some((sigo, revoked, key));
+            return Some(ka.into());
         }
     }
 }
diff --git a/openpgp/src/cert/mod.rs b/openpgp/src/cert/mod.rs
index 3f7c4cf3..45e22271 100644
--- a/openpgp/src/cert/mod.rs
+++ b/openpgp/src/cert/mod.rs
@@ -39,12 +39,14 @@ use crate::types::{
 mod builder;
 mod bindings;
 mod keyiter;
+mod key_amalgamation;
 mod parser;
 mod revoke;
 
 pub use self::builder::{CertBuilder, CipherSuite};
 
 pub use keyiter::KeyIter;
+pub use key_amalgamation::KeyAmalgamation;
 
 pub use parser::{
     KeyringValidity,
diff --git a/openpgp/src/crypto/keygrip.rs b/openpgp/src/crypto/keygrip.rs
index adb05ed0..ecc1d4e7 100644
--- a/openpgp/src/crypto/keygrip.rs
+++ b/openpgp/src/crypto/keygrip.rs
@@ -340,10 +340,10 @@ mod tests {
             .iter().map(|n| (n, crate::Cert::from_bytes(crate::tests::key(n)).unwrap()))
         {
             eprintln!("{}", name);
-            for key in cert.keys_all() {
-                let fp = key.2.fingerprint();
+            for key in cert.keys_all().map(|ka| ka.key()) {
+                let fp = key.fingerprint();
                 eprintln!("(sub)key: {}", fp);
-                assert_eq!(&key.2.mpis().keygrip().unwrap(),
+                assert_eq!(&key.mpis().keygrip().unwrap(),
                            keygrips.get(&fp).unwrap());
             }
         }
diff --git a/openpgp/src/crypto/mpis.rs b/openpgp/src/crypto/mpis.rs
index 7dd731f5..2c62a82b 100644
--- a/openpgp/src/crypto/mpis.rs
+++ b/openpgp/src/crypto/mpis.rs
@@ -1047,7 +1047,7 @@ 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_all().nth(*key_no).unwrap().2;
+            let key = cert.keys_all().nth(*key_no).unwrap().key();
             assert_eq!(key.mpis().bits().unwrap(), *bits,
                        "Cert {}, key no {}", name, *key_no);
         }
diff --git a/openpgp/src/packet/signature/mod.rs b/openpgp/src/packet/signature/mod.rs
index 2b711b87..02b256c1 100644
--- a/openpgp/src/packet/signature/mod.rs
+++ b/openpgp/src/packet/signature/mod.rs
@@ -1486,7 +1486,7 @@ mod test {
         let cert_key1 = test1.keys_all()
             .for_certification()
             .nth(0)
-            .map(|x| x.2)
+            .map(|ka| ka.key())
             .unwrap();
         let test2 = Cert::from_bytes(
             crate::tests::key("test2-signed-by-test1.pgp")).unwrap();
diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs
index c4b16049..fe0d7dd5 100644
--- a/openpgp/src/parse/stream.rs
+++ b/openpgp/src/parse/stream.rs
@@ -698,8 +698,12 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> {
                         for issuer in sig.get_issuers() {
                             if let Some((i, j)) = self.keys.get(&issuer) {
                                 let cert = &self.certs[*i];
-                                let (binding, revoked, key)
-                                    = cert.keys_all().nth(*j).unwrap();
+
+                                let ka = cert.keys_all().nth(*j).unwrap();
+                                let binding = ka.binding_signature(self.time);
+                                let revoked = ka.revoked(self.time);
+                                let key = ka.key();
+