openpgp: Replace TPK::select_keys with an iterator.

  - TPK::select_keys mixes iterating and filtering.

  - Make KeyIter an implicit builder, which supports convenient
    filtering.

  - Provide a convenience function to key an iterator with a
    reasonable filter default.

11 files changed, 386 insertions, 130 deletions

diff --git a/openpgp/examples/decrypt-with.rs b/openpgp/examples/decrypt-with.rs
index 176e03de..a5b26baa 100644
--- a/openpgp/examples/decrypt-with.rs
+++ b/openpgp/examples/decrypt-with.rs
@@ -57,7 +57,7 @@ impl Helper {
         // Map (sub)KeyIDs to secrets.
         let mut keys = HashMap::new();
         for tpk in tpks {
-            for (sig, _, key) in tpk.keys() {
+            for (sig, _, key) in tpk.keys_all() {
                 if sig.map(|s| (s.key_flags().can_encrypt_at_rest()
                                 || s.key_flags().can_encrypt_for_transport()))
                     .unwrap_or(false)
diff --git a/openpgp/examples/notarize.rs b/openpgp/examples/notarize.rs
index 5214ad9e..1b849376 100644
--- a/openpgp/examples/notarize.rs
+++ b/openpgp/examples/notarize.rs
@@ -30,7 +30,7 @@ fn main() {
         let tsk = openpgp::TPK::from_file(filename)
             .expect("Failed to read key");
 
-        for key in tsk.select_signing_keys(None) {
+        for key in tsk.keys_valid().signing_capable().map(|k| k.2) {
             if let Some(mut secret) = key.secret() {
                 let secret_mpis = match secret {
                     SecretKey::Encrypted { .. } => {
diff --git a/openpgp/examples/sign-detached.rs b/openpgp/examples/sign-detached.rs
index 19dc771f..e558aa5e 100644
--- a/openpgp/examples/sign-detached.rs
+++ b/openpgp/examples/sign-detached.rs
@@ -25,7 +25,7 @@ fn main() {
         let tsk = openpgp::TPK::from_file(filename)
             .expect("Failed to read key");
 
-        for key in tsk.select_signing_keys(None) {
+        for key in tsk.keys_valid().signing_capable().map(|k| k.2) {
             if let Some(mut secret) = key.secret() {
                 let secret_mpis = match secret {
                     SecretKey::Encrypted { .. } => {
diff --git a/openpgp/examples/sign.rs b/openpgp/examples/sign.rs
index c3ee0728..33e7625e 100644
--- a/openpgp/examples/sign.rs
+++ b/openpgp/examples/sign.rs
@@ -25,7 +25,7 @@ fn main() {
         let tsk = openpgp::TPK::from_file(filename)
             .expect("Failed to read key");
 
-        for key in tsk.select_signing_keys(None) {
+        for key in tsk.keys_valid().signing_capable().map(|k| k.2) {
             if let Some(mut secret) = key.secret() {
                 let secret_mpis = match secret {
                     SecretKey::Encrypted { .. } => {
diff --git a/openpgp/src/crypto/keygrip.rs b/openpgp/src/crypto/keygrip.rs
index 50a2448c..3d1a84bd 100644
--- a/openpgp/src/crypto/keygrip.rs
+++ b/openpgp/src/crypto/keygrip.rs
@@ -340,7 +340,7 @@ mod tests {
             .iter().map(|n| (n, ::TPK::from_file(path_to(n)).unwrap()))
         {
             eprintln!("{}", name);
-            for key in tpk.keys() {
+            for key in tpk.keys_all() {
                 let fp = key.2.fingerprint();
                 eprintln!("(sub)key: {}", fp);
                 assert_eq!(&key.2.mpis().keygrip().unwrap(),
diff --git a/openpgp/src/packet/key_flags.rs b/openpgp/src/packet/key_flags.rs
index 92dac82f..51f9e00d 100644
--- a/openpgp/src/packet/key_flags.rs
+++ b/openpgp/src/packet/key_flags.rs
@@ -156,6 +156,11 @@ impl KeyFlags {
         }
     }
 
+    /// Returns a new `KeyFlags` with all capabilities disabled.
+    pub fn empty() -> Self {
+        KeyFlags::default()
+    }
+
     /// Returns a slice referencing the raw values.
     pub(crate) fn as_vec(&self) -> Vec<u8> {
         let mut ret = if self.unknown.is_empty() {
@@ -249,6 +254,11 @@ impl KeyFlags {
         self.is_group_key = v;
         self
     }
+
+    /// Returns whether no flags are set.
+    pub fn is_empty(&self) -> bool {
+        self.as_vec().into_iter().all(|b| b == 0)
+    }
 }
 
 // Numeric key capability flags.
diff --git a/openpgp/src/packet/signature/mod.rs b/openpgp/src/packet/signature/mod.rs
index 2d877c56..8097d149 100644
--- a/openpgp/src/packet/signature/mod.rs
+++ b/openpgp/src/packet/signature/mod.rs
@@ -1094,12 +1094,15 @@ mod test {
 
     #[test]
     fn verify_gpg_3rd_party_cert() {
-        use {packet::KeyFlags, TPK};
+        use TPK;
 
-        let cert_kf = KeyFlags::default().set_certify(true);
         let test1 = TPK::from_file(
             path_to("keys/test1-certification-key.pgp")).unwrap();
-        let cert_key1 = test1.select_keys(cert_kf, None)[0];
+        let cert_key1 = test1.keys_all()
+            .certification_capable()
+            .nth(0)
+            .map(|x| x.2)
+            .unwrap();
         let test2 = TPK::from_file(
             path_to("keys/test2-signed-by-test1.pgp")).unwrap();
         let uid_binding = &test2.primary_key_signature_full().unwrap().0.unwrap();
diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs
index a89d61a3..fcea6038 100644
--- a/openpgp/src/parse/stream.rs
+++ b/openpgp/src/parse/stream.rs
@@ -118,7 +118,7 @@ const BUFFER_SIZE: usize = 25 * 1024 * 1024;
 pub struct Verifier<'a, H: VerificationHelper> {
     helper: H,
     tpks: Vec<TPK>,
-    /// Maps KeyID to tpks[i].keys().nth(j).
+    /// Maps KeyID to tpks[i].keys_all().nth(j).
     keys: HashMap<KeyID, (usize, usize)>,
     oppr: Option<PacketParserResult<'a>>,
     sigs: Vec<Vec<VerificationResult>>,
@@ -350,7 +350,8 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> {
 
                 if let Some(issuer) = sig.get_issuer() {
                     if let Some((i, j)) = self.keys.get(&issuer) {
-                        let (_, _, key) = self.tpks[*i].keys().nth(*j).unwrap();
+                        let (_, _, key)
+                            = self.tpks[*i].keys_all().nth(*j).unwrap();
                         if sig.verify(key).unwrap_or(false) {
                             self.sigs.iter_mut().last()
                                 .expect("sigs is never empty").push(
@@ -808,7 +809,7 @@ impl DetachedVerifier {
 pub struct Decryptor<'a, H: VerificationHelper + DecryptionHelper> {
     helper: H,
     tpks: Vec<TPK>,
-    /// Maps KeyID to tpks[i].keys().nth(j).
+    /// Maps KeyID to tpks[i].keys_all().nth(j).
     keys: HashMap<KeyID, (usize, usize)>,
     oppr: Option<PacketParserResult<'a>>,
     identity: Option<Fingerprint>,
@@ -1167,7 +1168,8 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
 
                 if let Some(issuer) = sig.get_issuer() {
                     if let Some((i, j)) = self.keys.get(&issuer) {
-                        let (_, _, key) = self.tpks[*i].keys().nth(*j).unwrap();
+                        let (_, _, key)
+                            = self.tpks[*i].keys_all().nth(*j).unwrap();
                         if sig.verify(key).unwrap_or(false) {
                             self.sigs.iter_mut().last()
                                 .expect("sigs is never empty").push(
@@ -1488,7 +1490,7 @@ mod test {
         // sign 30MiB message
         let mut buf = vec![];
         {
-            let key = tpk.select_signing_keys(None)[0];
+            let key = tpk.keys_all().signing_capable().nth(0).unwrap().2;
             let sec = match key.secret() {
                 Some(SecretKey::Unencrypted { ref mpis }) => mpis,
                 _ => unreachable!(),
diff --git a/openpgp/src/serialize/stream.rs b/openpgp/src/serialize/stream.rs
index 5e88db65..1d3c79e1 100644
--- a/openpgp/src/serialize/stream.rs
+++ b/openpgp/src/serialize/stream.rs
@@ -228,7 +228,7 @@ impl<'a> Signer<'a> {
     /// # let tsk = TPK::from_bytes(include_bytes!(
     /// #     "../../tests/data/keys/testy-new-private.pgp"))
     /// #     .unwrap();
-    /// # let key = tsk.select_signing_keys(None)[0];
+    /// # let key = tsk.keys_valid().signing_capable().nth(0).unwrap().2;
     /// # let sec = match key.secret() {
     /// #     Some(SecretKey::Unencrypted { ref mpis }) => mpis,
     /// #     _ => unreachable!(),
@@ -284,7 +284,7 @@ impl<'a> Signer<'a> {
     /// # let tsk = TPK::from_bytes(include_bytes!(
     /// #     "../../tests/data/keys/testy-new-private.pgp"))
     /// #     .unwrap();
-    /// # let key = tsk.select_signing_keys(None)[0];
+    /// # let key = tsk.keys_valid().signing_capable().nth(0).unwrap().2;
     /// # let sec = match key.secret() {
     /// #     Some(SecretKey::Unencrypted { ref mpis }) => mpis,
     /// #     _ => unreachable!(),
@@ -1282,7 +1282,6 @@ mod test {
     #[test]
     fn signature() {
         use crypto::KeyPair;
-        use packet::KeyFlags;
         use packet::key::SecretKey;
         use std::collections::HashMap;
         use Fingerprint;
@@ -1292,8 +1291,7 @@ mod test {
             TPK::from_bytes(bytes!("keys/testy-private.pgp")).unwrap(),
             TPK::from_bytes(bytes!("keys/testy-new-private.pgp")).unwrap(),
         ] {
-            for key in tsk.select_keys(
-                KeyFlags::default().set_sign(true), None)
+            for key in tsk.keys_all().signing_capable().map(|x| x.2)
             {
                 keys.insert(key.fingerprint(), key.clone());
             }
diff --git a/openpgp/src/tpk/mod.rs b/openpgp/src/tpk/mod.rs
index eca1448b..394f9922 100644
--- a/openpgp/src/tpk/mod.rs
+++ b/openpgp/src/tpk/mod.rs
@@ -20,6 +20,7 @@ use {
     packet::Tag,
     packet::signature::{self, Signature},
     packet::Key,
+    packet::key::SecretKey,
     packet::UserID,
     packet::UserAttribute,
     packet::Unknown,
@@ -861,32 +862,312 @@ pub struct UnknownBinding {
 /// in a TPK.
 ///
 /// Returned by TPK::keys().
+///
+/// `KeyIter` follows the builder pattern.  There is no need to
+/// explicitly finalize it, however: it already implements the
+/// `Iterator` interface.
+///
+/// By default, `KeyIter` will only return live, non-revoked keys.  It
+/// is possible to control how `KeyIter` filters using, for instance,
+/// `KeyIter::flags` to only return keys with particular flags set.
 pub struct KeyIter<'a> {
-    tpk: &'a TPK,
+    // This is an option to make it easier to create an empty KeyIter.
+    tpk: Option<&'a TPK>,
     primary: bool,
     subkey_iter: SubkeyBindingIter<'a>,
+
+    // If not None, only returns keys with the specified flags.
+    flags: Option<KeyFlags>,
+
+    // If not None, only returns keys that are live at the specified
+    // time.
+    alive_at: Option<time::Tm>,
+
+    // If not None, filters by revocation status.
+    revoked: Option<bool>,
+
+    // If not None, filters by whether a key has a secret.
+    secret: Option<bool>,
+
+    // If not None, filters by whether a key has an unencrypted
+    // secret.
+    unencrypted_secret: Option<bool>,
+}
+
+impl<'a> fmt::Debug for KeyIter<'a> {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_struct("KeyIter")
+            .field("flags", &self.flags)
+            .field("alive_at", &self.alive_at)
+            .field("revoked", &self.revoked)
+            .field("secret", &self.secret)
+            .field("unencrypted_secret", &self.unencrypted_secret)
+            .finish()
+    }
 }
 
 impl<'a> Iterator for KeyIter<'a> {
     type Item = (Option<&'a Signature>, RevocationStatus<'a>, &'a Key);
 
     fn next(&mut self) -> Option<Self::Item> {
-        if ! self.primary {
-            self.primary = true;
-            Some((self.tpk.primary_key_signature(),
-                  self.tpk.revoked(None),
-                  self.tpk.primary()))
-        } else {
-            self.subkey_iter.next()
-                .map(|sk_binding| (sk_binding.binding_signature(),
-                                   sk_binding.revoked(None),
-                                   &sk_binding.subkey,))
+        tracer!(false, "KeyIter::next", 0);
+        t!("KeyIter: {:?}", self);
+
+        if self.tpk.is_none() {
+            return None;
+        }
+        let tpk = self.tpk.unwrap();
+
+        if let Some(flags) = self.flags.as_ref() {
+            if flags.is_empty() {
+                // Nothing to do.
+                t!("short circuiting: flags is empty");
+                return None;
+            }
+        }
+
+        loop {
+            let (sigo, revoked, key) = if ! self.primary {
+                self.primary = true;
+
+                (tpk.primary_key_signature(),
+                 tpk.revoked(None),
+                 tpk.primary())
+            } else {
+                self.subkey_iter.next()
+                    .map(|sk_binding| (sk_binding.binding_signature(),
+                                       sk_binding.revoked(None),
+                                       &sk_binding.subkey,))?
+            };
+
+            t!("Considering key: {:?}", key);
+
+            if let Some(flags) = self.flags.as_ref() {
+                if let Some(sig) = sigo {
+                    if (&sig.key_flags() & &flags).is_empty() {
+                        t!("Have flags: {:?}, want flags: {:?}... skipping.",
+                           sig.key_flags(), flags);
+                        continue;
+                    }
+                } else {
+                    // No self-signature, skip it.
+                    t!("No self-signature... skipping.");
+                    continue;
+                }
+            }
+
+            if let Some(alive_at) = self.alive_at {
+                if let Some(sig) = sigo {
+                    if ! sig.key_alive_at(key, alive_at) {
+                        t!("Key not alive... skipping.");
+                        continue;
+                    }
+                } else {
+                    // No self-signature, skip it.
+                    t!("No self-signature... skipping.");
+                    continue;
+                }
+            }
+
+            if let Some(want_revoked) = self.revoked {
+                if let RevocationStatus::Revoked(_) = revoked {
+                    // The key is definitely revoked.
+                    if ! want_revoked {
+                        t!("Key revoked... skipping.");
+                        continue;
+                    }
+                } else {
+                    // The key is probably not revoked.
+                    if want_revoked {
+                        t!("Key not revoked... skipping.");
+                        continue;
+                    }
+                }
+            }
+
+            if let Some(want_secret) = self.secret {
+                if key.secret().is_some() {
+                    // We have a secret.
+                    if ! want_secret {
+                        t!("Have a secret... skipping.");
+                        continue;
+                    }
+                } else {
+                    if want_secret {
+                        t!("No secret... skipping.");
+                        continue;
+                    }
+                }
+            }
+
+            if let Some(want_unencrypted_secret) = self.unencrypted_secret {
+                if let Some(secret) = key.secret() {
+                    if let SecretKey::Unencrypted { .. } = secret {
+                        if ! want_unencrypted_secret {
+                            t!("Unencrypted secret... skipping.");
+                            continue;
+                        }
+                    } else {
+                        if want_unencrypted_secret {
+                            t!("Encrypted secret... skipping.");
+                            continue;
+                        }
+                    }
+                } else {
+                    // No secret.
+                    t!("No secret... skipping.");
+                    continue;
+                }
+            }
+
+            return Some((sigo, revoked, key));
         }
     }
 }
 
-impl<'a> ExactSizeIterator for KeyIter<'a> {
-    fn len(&self) -> usize { 1 + self.subkey_iter.len() }
+impl<'a> KeyIter<'a> {
+    /// Returns a new `KeyIter` instance with no filters enabled.
+    fn new(tpk: &'a TPK) -> Self where Self: 'a {
+        KeyIter {
+            tpk: Some(tpk),
+            primary: false,
+            subkey_iter: tpk.subkeys(),
+
+            // The filters.
+            flags: None,
+            alive_at: None,
+            revoked: None,
+            secret: None,
+            unencrypted_secret: None,
+        }
+    }
+
+    /// Clears all filters.
+    ///
+    /// This causes the `KeyIter` to return all keys in the TPK.
+    pub fn unfiltered(self) -> Self {
+        KeyIter::new(self.tpk.unwrap())
+    }
+
+    /// Returns an empty KeyIter.
+    pub fn empty() -> Self {
+        KeyIter {
+            tpk: None,
+            primary: false,
+            subkey_iter: SubkeyBindingIter { iter: None },
+
+            // The filters.
+            flags: None,
+            alive_at: None,
+            revoked: None,
+            secret: None,
+            unencrypted_secret: None,
+        }
+    }
+
+    /// Returns keys that have the at least one of the flags specified
+    /// in `flags`.
+    ///
+    /// If you call this function (or one of `certification_capable`
+    /// or `signing_capable` functions) multiple times, the *union* of
+    /// the values is used.  Thus,
+    /// `tpk.flags().certification_capable().signing_capable()` will
+    /// return keys that are certification capable or signing capable.
+    ///
+    /// If you need more complex filtering, e.g., you want a key that
+    /// is both certification and signing capable, then just use a
+    /// normal [`Iterator::filter`].
+    ///
+    ///   [`Iterator::filter`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.filter
+    pub fn key_flags(mut self, flags: KeyFlags) -> Self {
+        if let Some(flags_old) = self.flags {
+            self.flags = Some(&flags | &flags_old);
+        } else {
+            self.flags = Some(flags);
+        }
+        self
+    }
+
+    /// Returns keys that are certification capable.
+    ///
+    /// See `key_flags` for caveats.
+    pub fn certification_capable(self) -> Self {
+        self.key_flags(KeyFlags::default().set_certify(true))
+    }
+
+    /// Returns keys that are signing capable.
+    ///
+    /// See `key_flags` for caveats.
+    pub fn signing_capable(self) -> Self {
+        self.key_flags(KeyFlags::default().set_sign(true))
+    }
+
+    /// Only returns keys that are live as of `now`.
+    ///
+    /// If `now` is none, then all keys are returned whether they are
+    /// live or not.
+    ///
+    /// A value of None disables this filter, which is set by default
+    /// to only return live keys at the current time.
+    ///
+    /// If you call this function (or `alive`) multiple times, only
+    /// the last value is used.
+    pub fn alive_at<T>(mut self, alive_at: T) -> Self
+        where T: Into<Option<time::Tm>>
+    {
+        self.alive_at = alive_at.into();
+        self
+    }
+
+    /// Only returns keys that are live right now.
+    ///
+    /// If you call this function (or `alive_at`) multiple times, only
+    /// the last value is used.
+    pub fn alive(mut self) -> Self
+    {
+        self.alive_at = Some(time::now());
+        self
+    }
+
+    /// If not None, filters by whether a key is definitely revoked.
+    ///
+    /// That is, whether it's revocation status is
+    /// `RevocationStatus::Revoked`.
+    ///
+    /// A value of None disables this filter, which is set by default
+    /// to not return revoked keys.
+    ///
+    /// If you call this function multiple times, only the last value
+    /// is used.
+    pub fn revoked<T>(mut self, revoked: T) -> Self
+        where T: Into<Option<bool>>
+    {
+        self.revoked = revoked.into();
+        self
+    }
+
+    /// If not None, filters by whether a key has a secret.
+    ///
+    /// If you call this function multiple times, only the last value
+    /// is used.
+    pub fn secret<T>(mut self, secret: T) -> Self
+        where T: Into<Option<bool>>
+    {
+        self.secret = secret.into();
+        self
+    }
+
+    /// If not None, filters by whether a key has an unencrypted
+    /// secret.
+    ///
+    /// If you call this function multiple times, only the last value
+    /// is used.
+    pub fn unencrypted_secret<T>(mut self, unencrypted_secret: T) -> Self
+        where T: Into<Option<bool>>
+    {
+        self.unencrypted_secret = unencrypted_secret.into();
+        self
+    }
 }
 
 // A TPKParser can read packets from either an Iterator or a
@@ -1343,19 +1624,27 @@ impl<'a> ExactSizeIterator for UserAttributeBindingIter<'a> {
 
 /// An iterator over `SubkeyBinding`s.
 pub struct SubkeyBindingIter<'a> {
-    iter: slice::Iter<'a, SubkeyBinding>,
+    iter: Option<slice::Iter<'a, SubkeyBinding>>,
 }
 
 impl<'a> Iterator for SubkeyBindingIter<'a> {
     type Item = &'a SubkeyBinding;
 
     fn next(&mut self) -> Option<Self::Item> {
-        self.iter.next()
+        match self.iter {
+            Some(ref mut iter) => iter.next(),
+            None => None,
+        }
     }
 }
 
 impl<'a> ExactSizeIterator for SubkeyBindingIter<'a> {
-    fn len(&self) -> usize { self.iter.len() }
+    fn len(&self) -> usize {
+        match self.iter {
+            Some(ref iter) => iter.len(),
+            None => 0,
+        }
+    }
 }
 
 
@@ -1695,10 +1984,11 @@ impl TPK {
     ///
     /// A valid `SubkeyBinding` has at least one good self-signature.
     pub fn subkeys(&self) -> SubkeyBindingIter {
-        SubkeyBindingIter { iter: self.subkeys.iter() }
+        SubkeyBindingIter { iter: Some(self.subkeys.iter()) }
     }
 
-    /// Returns an iterator over all of the TPK's valid keys.
+    /// Returns an iterator over the TPK's valid keys (live and
+    /// not-revoked).
     ///
     /// That is, this returns an iterator over the primary key and any
     /// subkeys, along with the corresponding signatures.
@@ -1712,79 +2002,20 @@ impl TPK {
     /// `None` for the primary key's signature.
     ///
     /// A valid `Key` has at least one good self-signature.
-    pub fn keys(&self) -> KeyIter {
-        KeyIter {
-            tpk: self,
-            primary: false,
-            subkey_iter: self.subkeys()
-        }
-    }
-
-    /// Returns all unrevoked (sub)keys that have all capabilities in `cap` and
-    /// are valid `now`. If `now` is `None` the current time is used.
     ///
-    /// Keys are sorted by creation time (newest first). If the primary key
-    /// qualifies it will always be last. Using the first key should suffice
-    /// for most use cases.
-    pub fn select_keys<'a, T>(&'a self, cap: KeyFlags, now: T)
-        -> Vec<&'a Key>
-        where T: Into<Option<time::Tm>> {
-            use std::slice;
-            use std::iter;
-            use std::borrow::Borrow;
-
-        fn is_usable<S>(key: &Key, mut bindings: slice::Iter<S>, now: time::Tm,
-                        cap: &KeyFlags) -> bool where S: Borrow<Signature> {
-            let key_now_valid = *key.creation_time() < now;
-            let sig_now_valid = bindings.clone().any(|sig| {
-                sig.borrow().signature_alive_at(now)
-            });
-            let right_caps = bindings.any(|sig| {
-                *cap <= sig.borrow().key_flags()
-            });
-
-            key_now_valid && sig_now_valid && right_caps
-        }
-        let now = now.into().unwrap_or_else(time::now);
-
-        if self.revoked(now) == RevocationStatus::NotAsFarAsWeKnow {
-            let prim_sigs = match self.primary_key_signature_full() {
-                None => Vec::default(),
-                Some((None, sig)) => vec![sig],
-                Some((Some(uid), sig)) =>
-                    uid.selfsigs().iter().chain(iter::once(sig)).collect(),
-            };
-            let prim = Some(&self.primary)
-                .filter(|k| is_usable(k, prim_sigs.iter(), now, &cap));
-            let mut ret = self.subkeys
-                .iter()
-                .filter(|sb| {
-                    sb.revoked(None) == RevocationStatus::NotAsFarAsWeKnow &&
-                        is_usable(sb.subkey(), sb.selfsigs().iter(), now, &cap)
-                })
-                .map(|sb| sb.subkey())
-                .collect::<Vec<_>>();
-
-            ret.sort_by(|a,b| b.creation_time().cmp(a.creation_time()));
-            ret.extend(prim);
-
-            ret
-        } else {
-            Vec::default()
-        }
+    /// To return all keys, do `keys().unfiltered()`.  See the
+    /// documentation of `keys` for how to control what keys are
+    /// returned.
+    pub fn keys_valid(&self) -> KeyIter {
+        KeyIter::new(self).alive().revoked(false)
     }
 
-    /// Returns all unrevoked (sub)keys that have the signing
-    /// capability and are valid `now`. If `now` is `None` the current
-    /// time is used.
+    /// Returns an iterator over the TPK's keys.
     ///
-    /// Keys are sorted by creation time (newest first). If the
-    /// primary key qualifies it will always be last. Using the fir