From 1434843469109127f41c5dbd8aacf5400c537759 Mon Sep 17 00:00:00 2001 From: "Neal H. Walfield" Date: Thu, 14 Mar 2019 09:20:10 +0100 Subject: 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. --- openpgp-ffi/include/sequoia/openpgp.h | 103 ++++++++- openpgp-ffi/src/tpk.rs | 186 ++++++++++++++- openpgp/examples/decrypt-with.rs | 2 +- openpgp/examples/notarize.rs | 2 +- openpgp/examples/sign-detached.rs | 2 +- openpgp/examples/sign.rs | 2 +- openpgp/src/crypto/keygrip.rs | 2 +- openpgp/src/packet/key_flags.rs | 10 + openpgp/src/packet/signature/mod.rs | 9 +- openpgp/src/parse/stream.rs | 12 +- openpgp/src/serialize/stream.rs | 8 +- openpgp/src/tpk/mod.rs | 417 ++++++++++++++++++++++++++-------- openpgp/src/tsk.rs | 50 ++-- sqv/src/sqv.rs | 16 +- store/src/backend/mod.rs | 2 +- tool/src/commands/mod.rs | 9 +- tool/tests/sq-sign.rs | 2 +- 17 files changed, 682 insertions(+), 152 deletions(-) diff --git a/openpgp-ffi/include/sequoia/openpgp.h b/openpgp-ffi/include/sequoia/openpgp.h index 1efd7d8a..b4115388 100644 --- a/openpgp-ffi/include/sequoia/openpgp.h +++ b/openpgp-ffi/include/sequoia/openpgp.h @@ -503,11 +503,85 @@ pgp_signature_t pgp_user_id_binding_selfsig(pgp_user_id_binding_t binding); /*/ pgp_user_id_binding_t pgp_user_id_binding_iter_next (pgp_user_id_binding_iter_t iter); +/*/ /// Frees an pgp_user_id_binding_iter_t. +/*/ void pgp_user_id_binding_iter_free (pgp_user_id_binding_iter_t iter); /* openpgp::tpk::KeyIter. */ +/*/ +/// Changes the iterator to only return keys that are certification +/// capable. +/// +/// If you call this function and, e.g., the `signing_capable` +/// function, the *union* of the values is used. That is, the +/// iterator will return keys that are certification capable *or* +/// signing capable. +/// +/// Note: you may not call this function after starting to iterate. +/*/ +void pgp_tpk_key_iter_certification_capable (pgp_tpk_key_iter_t iter); + +/*/ +/// Changes the iterator to only return keys that are certification +/// capable. +/// +/// If you call this function and, e.g., the `signing_capable` +/// function, the *union* of the values is used. That is, the +/// iterator will return keys that are certification capable *or* +/// signing capable. +/// +/// Note: you may not call this function after starting to iterate. +/*/ +void pgp_tpk_key_iter_signing_capable (pgp_tpk_key_iter_t iter); + +/*/ +/// Changes the iterator to only return keys that are alive. +/// +/// If you call this function (or `pgp_tpk_key_iter_alive_at`), only +/// the last value is used. +/// +/// Note: you may not call this function after starting to iterate. +/*/ +void pgp_tpk_key_iter_alive (pgp_tpk_key_iter_t iter); + +/*/ +/// Changes the iterator to only return keys that are alive at the +/// specified time. +/// +/// If you call this function (or `pgp_tpk_key_iter_alive`), only the +/// last value is used. +/// +/// Note: you may not call this function after starting to iterate. +/*/ +void pgp_tpk_key_iter_alive_at (pgp_tpk_key_iter_t iter, time_t when); + +/*/ +/// Changes the iterator to only return keys whose revocation status +/// matches `revoked`. +/// +/// Note: you may not call this function after starting to iterate. +/*/ +void pgp_tpk_key_iter_revoked (pgp_tpk_key_iter_t iter, bool revoked); + +/*/ +/// Changes the iterator to only return keys that have secret keys (or +/// not). +/// +/// Note: you may not call this function after starting to iterate. +/*/ +void pgp_tpk_key_iter_secret (pgp_tpk_key_iter_t iter, bool secret); + +/*/ +/// Changes the iterator to only return keys that have unencrypted +/// secret keys (or not). +/// +/// Note: you may not call this function after starting to iterate. +/*/ +void pgp_tpk_key_iter_unencrypted_secret (pgp_tpk_key_iter_t iter, + bool unencrypted_secret); + /*/ /// Returns the next key. Returns NULL if there are no more elements. /// @@ -718,10 +792,33 @@ int pgp_tpk_is_tsk(pgp_tpk_t tpk); pgp_user_id_binding_iter_t pgp_tpk_user_id_binding_iter (pgp_tpk_t tpk); /*/ -/// Returns an iterator over all `Key`s (both the primary key and any -/// subkeys) in a TPK. +/// Returns an iterator over the live and unrevoked `Key`s in a TPK. +/// +/// Compare with `pgp_tpk_keys_iter_valid`, which doesn'filters out +/// expired and revoked keys by default. +/*/ +pgp_tpk_key_iter_t pgp_tpk_keys_iter_all (pgp_tpk_t tpk); + +/*/ +/// Returns an iterator over all `Key`s in a TPK. +/// +/// That is, this returns an iterator over the primary key and any +/// subkeys, along with the corresponding signatures. +/// +/// Note: since a primary key is different from a subkey, the iterator +/// is over `Key`s and not `SubkeyBindings`. Since the primary key +/// has no binding signature, the signature carrying the primary key's +/// key flags is returned (either a direct key signature, or the +/// self-signature on the primary User ID). There are corner cases +/// where no such signature exists (e.g. partial TPKs), therefore this +/// iterator may return `None` for the primary key's signature. +/// +/// A valid `Key` has at least one good self-signature. +/// +/// Compare with `pgp_tpk_keys_iter_all`, which doesn't filter out +/// expired and revoked keys. /*/ -pgp_tpk_key_iter_t pgp_tpk_key_iter (pgp_tpk_t tpk); +pgp_tpk_key_iter_t pgp_tpk_keys_iter_valid (pgp_tpk_t tpk); /*/ /// Returns the TPK's primary user id (if any). diff --git a/openpgp-ffi/src/tpk.rs b/openpgp-ffi/src/tpk.rs index 1483c71c..39315565 100644 --- a/openpgp-ffi/src/tpk.rs +++ b/openpgp-ffi/src/tpk.rs @@ -437,19 +437,51 @@ pub extern "system" fn pgp_user_id_binding_iter_next<'a>( pub struct KeyIterWrapper<'a> { iter: KeyIter<'a>, rso: Option>, + // Whether next has been called. + next_called: bool, } -/// Returns an iterator over the TPK's keys. +/// Returns an iterator over the TPK's live, non-revoked keys. /// -/// This iterates over both the primary key and any subkeys. +/// That is, this returns an iterator over the primary key and any +/// subkeys, along with the corresponding signatures. +/// +/// Note: since a primary key is different from a subkey, the iterator +/// is over `Key`s and not `SubkeyBindings`. Since the primary key +/// has no binding signature, the signature carrying the primary key's +/// key flags is returned (either a direct key signature, or the +/// self-signature on the primary User ID). There are corner cases +/// where no such signature exists (e.g. partial TPKs), therefore this +/// iterator may return `None` for the primary key's signature. +/// +/// A valid `Key` has at least one good self-signature. +/// +/// To return all keys, use `pgp_tpk_keys_all_iter()`. +#[::sequoia_ffi_macros::extern_fn] #[no_mangle] +pub extern "system" fn pgp_tpk_keys_iter_valid(tpk: *const TPK) + -> *mut KeyIterWrapper<'static> +{ + let tpk = tpk.ref_raw(); + box_raw!(KeyIterWrapper { + iter: tpk.keys_valid(), + rso: None, + next_called: false, + }) +} + +/// Returns an iterator over all `Key`s in a TPK. +/// +/// Compare with `pgp_tpk_keys_iter_all`, which doesn't filter out +/// expired and revoked keys by default. #[::sequoia_ffi_macros::extern_fn] #[no_mangle] -pub extern "system" fn pgp_tpk_key_iter(tpk: *const TPK) +pub extern "system" fn pgp_tpk_keys_iter_all(tpk: *const TPK) -> *mut KeyIterWrapper<'static> { let tpk = tpk.ref_raw(); box_raw!(KeyIterWrapper { - iter: tpk.keys(), + iter: tpk.keys_all(), rso: None, + next_called: false, }) } @@ -461,6 +493,151 @@ pub extern "system" fn pgp_tpk_key_iter_free( ffi_free!(iter) } +/// Changes the iterator to only return keys that are certification +/// capable. +/// +/// If you call this function and, e.g., the `signing_capable` +/// function, the *union* of the values is used. That is, the +/// iterator will return keys that are certification capable *or* +/// signing capable. +/// +/// Note: you may not call this function after starting to iterate. +#[::sequoia_ffi_macros::extern_fn] #[no_mangle] +pub extern "system" fn pgp_tpk_key_iter_certification_capable<'a>( + iter_wrapper: *mut KeyIterWrapper<'a>) +{ + let iter_wrapper = ffi_param_ref_mut!(iter_wrapper); + if iter_wrapper.next_called { + panic!("Can't change KeyIter filter after iterating."); + } + + use std::mem; + let tmp = mem::replace(&mut iter_wrapper.iter, KeyIter::empty()); + iter_wrapper.iter = tmp.certification_capable(); +} + +/// Changes the iterator to only return keys that are certification +/// capable. +/// +/// If you call this function and, e.g., the `signing_capable` +/// function, the *union* of the values is used. That is, the +/// iterator will return keys that are certification capable *or* +/// signing capable. +/// +/// Note: you may not call this function after starting to iterate. +#[::sequoia_ffi_macros::extern_fn] #[no_mangle] +pub extern "system" fn pgp_tpk_key_iter_signing_capable<'a>( + iter_wrapper: *mut KeyIterWrapper<'a>) +{ + let iter_wrapper = ffi_param_ref_mut!(iter_wrapper); + if iter_wrapper.next_called { + panic!("Can't change KeyIter filter after iterating."); + } + + use std::mem; + let tmp = mem::replace(&mut iter_wrapper.iter, KeyIter::empty()); + iter_wrapper.iter = tmp.signing_capable(); +} + +/// Changes the iterator to only return keys that are alive. +/// +/// If you call this function (or `pgp_tpk_key_iter_alive_at`), only +/// the last value is used. +/// +/// Note: you may not call this function after starting to iterate. +#[::sequoia_ffi_macros::extern_fn] #[no_mangle] +pub extern "system" fn pgp_tpk_key_iter_alive<'a>( + iter_wrapper: *mut KeyIterWrapper<'a>) +{ + let iter_wrapper = ffi_param_ref_mut!(iter_wrapper); + if iter_wrapper.next_called { + panic!("Can't change KeyIter filter after iterating."); + } + + use std::mem; + let tmp = mem::replace(&mut iter_wrapper.iter, KeyIter::empty()); + iter_wrapper.iter = tmp.alive(); +} + +/// Changes the iterator to only return keys that are alive at the +/// specified time. +/// +/// If you call this function (or `pgp_tpk_key_iter_alive`), only the +/// last value is used. +/// +/// Note: you may not call this function after starting to iterate. +#[::sequoia_ffi_macros::extern_fn] #[no_mangle] +pub extern "system" fn pgp_tpk_key_iter_alive_at<'a>( + iter_wrapper: *mut KeyIterWrapper<'a>, + when: time_t) +{ + let iter_wrapper = ffi_param_ref_mut!(iter_wrapper); + if iter_wrapper.next_called { + panic!("Can't change KeyIter filter after iterating."); + } + + use std::mem; + let tmp = mem::replace(&mut iter_wrapper.iter, KeyIter::empty()); + iter_wrapper.iter = tmp.alive_at(time::at(time::Timespec::new(when as i64, 0))); +} + +/// Changes the iterator to only return keys whose revocation status +/// matches `revoked`. +/// +/// Note: you may not call this function after starting to iterate. +#[::sequoia_ffi_macros::extern_fn] #[no_mangle] +pub extern "system" fn pgp_tpk_key_iter_revoked<'a>( + iter_wrapper: *mut KeyIterWrapper<'a>, + revoked: bool) +{ + let iter_wrapper = ffi_param_ref_mut!(iter_wrapper); + if iter_wrapper.next_called { + panic!("Can't change KeyIter filter after iterating."); + } + + use std::mem; + let tmp = mem::replace(&mut iter_wrapper.iter, KeyIter::empty()); + iter_wrapper.iter = tmp.revoked(Some(revoked)); +} + +/// Changes the iterator to only return keys that have secret keys (or +/// not). +/// +/// Note: you may not call this function after starting to iterate. +#[::sequoia_ffi_macros::extern_fn] #[no_mangle] +pub extern "system" fn pgp_tpk_key_iter_secret<'a>( + iter_wrapper: *mut KeyIterWrapper<'a>, + secret: bool) +{ + let iter_wrapper = ffi_param_ref_mut!(iter_wrapper); + if iter_wrapper.next_called { + panic!("Can't change KeyIter filter after iterating."); + } + + use std::mem; + let tmp = mem::replace(&mut iter_wrapper.iter, KeyIter::empty()); + iter_wrapper.iter = tmp.secret(Some(secret)); +} + +/// Changes the iterator to only return keys that have unencrypted +/// secret keys (or not). +/// +/// Note: you may not call this function after starting to iterate. +#[::sequoia_ffi_macros::extern_fn] #[no_mangle] +pub extern "system" fn pgp_tpk_key_iter_unencrypted_secret<'a>( + iter_wrapper: *mut KeyIterWrapper<'a>, + unencrypted_secret: bool) +{ + let iter_wrapper = ffi_param_ref_mut!(iter_wrapper); + if iter_wrapper.next_called { + panic!("Can't change KeyIter filter after iterating."); + } + + use std::mem; + let tmp = mem::replace(&mut iter_wrapper.iter, KeyIter::empty()); + iter_wrapper.iter = tmp.unencrypted_secret(Some(unencrypted_secret)); +} + /// Returns the next key. Returns NULL if there are no more elements. /// /// If sigo is not NULL, stores the current self-signature (if any) in @@ -479,6 +656,7 @@ pub extern "system" fn pgp_tpk_key_iter_next<'a>( { let iter_wrapper = ffi_param_ref_mut!(iter_wrapper); iter_wrapper.rso = None; + iter_wrapper.next_called = true; if let Some((sig, rs, key)) = iter_wrapper.iter.next() { if let Some(ptr) = sigo { 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 { 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, - /// Maps KeyID to tpks[i].keys().nth(j). + /// Maps KeyID to tpks[i].keys_all().nth(j). keys: HashMap, oppr: Option>, sigs: Vec>, @@ -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, - /// Maps KeyID to tpks[i].keys().nth(j). + /// Maps KeyID to tpks[i].keys_all().nth(j). keys: HashMap, oppr: Option>, identity: Option, @@ -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, + + // If not None, only returns keys that are live at the specified + // time. + alive_at: Option, + + // If not None, filters by revocation status. + revoked: Option, + + // If not None, filters by whether a key has a secret. + secret: Option, + + // If not None, filters by whether a key has an unencrypted + // secret. + unencrypted_secret: Option, +} + +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 { - 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(mut self, alive_at: T) -> Self + where T: Into> + { + 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(mut self, revoked: T) -> Self + where T: Into> + { + 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(mut self, secret: T) -> Self + where T: Into> + { + 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(mut self, unencrypted_secret: T) -> Self + where T: Into> + { + 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>, } impl<'a> Iterator for SubkeyBindingIter<'a> { type Item = &'a SubkeyBinding; fn next(&mut self) -> Option { - 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> { - use std::slice; - use std::iter; - use std::borrow::Borrow; - - fn is_usable(key: &Key, mut bindings: slice::Iter, now: time::Tm, - cap: &KeyFlags) -> bool where S: Borrow { - 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::>(); - - 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 first - /// key should suffice for most use cases. - pub fn select_signing_keys<'a, T>(&'a self, now: T) -> Vec<&'a Key> - where T: Into> - { - self.select_keys(KeyFlags::default().set_sign(true), now) + /// Unlike `TPK::keys_valid()`, this iterator also returns expired + /// and revoked keys. + pub fn keys_all(&self) -> KeyIter { + KeyIter::new(self) } /// Returns the first TPK found in the packet stream. @@ -3111,7 +3342,7 @@ mod test { fn key_iter_test() { let key = TPK::from_bytes(bytes!("neal.pgp")).unwrap(); assert_eq!(1 + key.subkeys().count(), - key.keys().count()); + key.keys_all().count()); } #[test] @@ -3702,7 +3933,7 @@ mod test { .generate().unwrap(); let flags = KeyFlags::default().set_encrypt_for_transport(true); - assert!(tpk.select_keys(flags, None).is_empty()); + assert_eq!(tpk.keys_all().key_flags(flags).count(), 0); } #[test] @@ -3712,7 +3943,7 @@ mod test { .generate().unwrap(); let flags = KeyFlags::default().set_encrypt_for_transport(true); - assert_eq!(tpk.select_keys(flags, None).len(), 1); + assert_eq!(tpk.keys_all().key_flags(flags).count(), 1); } #[test] @@ -3723,7 +3954,7 @@ mod test { .generate().unwrap(); let flags = KeyFlags::default().set_encrypt_for_transport(true); - assert_eq!(tpk.select_keys(flags, None).len(), 1); + assert_eq!(tpk.keys_all().key_flags(flags).count(), 1); } #[test] @@ -3735,7 +3966,7 @@ mod test { let flags = KeyFlags::default().set_encrypt_for_transport(true); now.tm_year -= 1; - assert_eq!(tpk.select_keys(flags, now).len(), 0); + assert_eq!(tpk.keys_all().key_flags(flags).alive_at(now).count(), 0); } #[test] @@ -3745,7 +3976,7 @@ mod test { .generate().unwrap(); let flags = KeyFlags::default().set_certify(true); - assert_eq!(tpk.select_keys(flags, None).len(), 2); + assert_eq!(tpk.keys_all().key_flags(flags).count(), 2); } // Make sure that when merging two TPKs, the primary key and diff --git a/openpgp/src/tsk.rs b/openpgp/src/tsk.rs index 7d7ea17a..080a9431 100644 --- a/openpgp/src/tsk.rs +++ b/openpgp/src/tsk.rs @@ -104,13 +104,22 @@ impl TSK { /// Signs `key` and `userid` with a 3rd party certification. pub fn certify_userid(&self, key: &Key, userid: &UserID) -> Result { - use packet::{KeyFlags, signature, key::SecretKey}; + use packet::{signature, key::SecretKey}; use constants::{HashAlgorithm, SignatureType}; - let caps = KeyFlags::default().set_certify(true); - let keys = self.key.select_keys(caps, None); - - match keys.first() { + // We're willing to use an expired certification key here, + // because otherwise it is impossible to extend the expiration + // of an expired TPK. + // + // XXX: If there are multiple certification keys, then we + // should prefer a non-expired one. + let certification_key = self.key.keys_all() + .certification_capable() + .unencrypted_secret(true) + .nth(0) + .map(|x| x.2); + + match certification_key { Some(my_key) => { match my_key.secret() { Some(&SecretKey::Unencrypted{ ref mpis }) => { @@ -142,12 +151,9 @@ impl TSK { /// Signs `userid` with this TSK. pub fn sign_userid(&self, userid: &UserID) -> Result { - use packet::{KeyFlags, signature, key::SecretKey}; + use packet::{signature, key::SecretKey}; use constants::{HashAlgorithm, SignatureType}; - let caps = KeyFlags::default().set_certify(true); - let keys = self.key.select_keys(caps, None); - let builder = if let Some(sig) = self.primary_key_signature() { signature::Builder::from(sig.clone()) @@ -157,7 +163,13 @@ impl TSK { } .set_signature_creation_time(time::now())?; - match keys.first() { + let certification_key = self.key.keys_valid() + .certification_capable() + .unencrypted_secret(true) + .nth(0) + .map(|x| x.2); + + match certification_key { Some(my_key) => { match my_key.secret() { Some(&SecretKey::Unencrypted{ ref mpis }) => { @@ -180,13 +192,16 @@ impl TSK { /// Signs `userattr` with a the primary key. pub fn sign_user_attribute(&self, userattr: &UserAttribute) -> Result { - use packet::{KeyFlags, signature, key::SecretKey}; + use packet::{signature, key::SecretKey}; use constants::{HashAlgorithm, SignatureType}; - let caps = KeyFlags::default().set_certify(true); - let keys = self.key.select_keys(caps, None); + let certification_key = self.key.keys_valid() + .certification_capable() + .unencrypted_secret(true) + .nth(0) + .map(|x| x.2); - match keys.first() { + match certification_key { Some(my_key) => { match my_key.secret() { Some(&SecretKey::Unencrypted{ ref mpis }) => { @@ -441,8 +456,6 @@ mod tests { #[test] fn certification_user_id() { - use packet::KeyFlags; - let (tpk1, _) = TPKBuilder::default() .add_certification_subkey() .generate().unwrap(); @@ -453,9 +466,8 @@ mod tests { .generate().unwrap(); let sig = tsk.certify_key(&tpk2).unwrap(); - let key = tsk.tpk().select_keys( - KeyFlags::default().set_certify(true), - None)[0]; + let key = tsk.tpk().keys_all() + .certification_capable().nth(0).unwrap().2; assert_eq!( sig.verify_userid_binding( diff --git a/sqv/src/sqv.rs b/sqv/src/sqv.rs index c9cc65c6..1ab6568d 100644 --- a/sqv/src/sqv.rs +++ b/sqv/src/sqv.rs @@ -145,15 +145,9 @@ fn real_main() -> Result<(), failure::Error> { .into_iter().collect(); fn tpk_has_key(tpk: &TPK, keyid: &KeyID) -> bool { - if *keyid == tpk.primary().keyid() { - return true; - } - for binding in tpk.subkeys() { - if *keyid == binding.subkey().keyid() { - return true; - } - } - false + // Even if a key is revoked or expired, we can still use it to + // verify a message. + tpk.keys_all().any(|(_, _, k)| *keyid == k.keyid()) } // Find the keys. @@ -215,7 +209,7 @@ fn real_main() -> Result<(), failure::Error> { if let Some(ref tpk) = tpko { // Find the right key. - for (maybe_binding, _, key) in tpk.keys() { + for (maybe_binding, _, key) in tpk.keys_all() { let binding = match maybe_binding { Some(b) => b, None => continue, @@ -223,7 +217,7 @@ fn real_main() -> Result<(), failure::Error> { if issuer == key.keyid() { if !binding.key_flags().can_sign() { - eprintln!("Cannot check signature, key has no siginig \ + eprintln!("Cannot check signature, key has no signing \ capability"); continue 'sig_loop; } diff --git a/store/src/backend/mod.rs b/store/src/backend/mod.rs index 39db1279..8a488028 100644 --- a/store/src/backend/mod.rs +++ b/store/src/backend/mod.rs @@ -794,7 +794,7 @@ impl KeyServer { /// Keeps the mapping of (sub)KeyIDs to keys up-to-date. fn reindex_subkeys(c: &Connection, key_id: ID, tpk: &TPK) -> Result<()> { - for (_, _, key) in tpk.keys() { + for (_, _, key) in tpk.keys_all() { let keyid = key.fingerprint().to_keyid().as_u64() .expect("computed keyid is valid"); diff --git a/tool/src/commands/mod.rs b/tool/src/commands/mod.rs index db713caa..076bafbc 100644 --- a/tool/src/commands/mod.rs +++ b/tool/src/commands/mod.rs @@ -44,7 +44,10 @@ fn tm2str(t: &time::Tm) -> String { fn get_signing_keys(tpks: &[openpgp::TPK]) -> Result> { let mut keys = Vec::new(); 'next_tpk: for tsk in tpks { - 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 { .. } => { @@ -191,7 +194,9 @@ impl<'a> VerificationHelper for VHelper<'a> { let mut tpks = self.tpks.take().unwrap(); let seen: HashSet<_> = tpks.iter() .flat_map(|tpk| { - tpk.keys().map(|(_, _, key)| key.fingerprint().to_keyid()) + // Even if a key is revoked or expired, we can still + // use it to verify a message. + tpk.keys_all().map(|(_, _, key)| key.fingerprint().to_keyid()) }).collect(); // Explicitly provided keys are trusted. diff --git a/tool/tests/sq-sign.rs b/tool/tests/sq-sign.rs index 90da8d8a..8a891f65 100644 --- a/tool/tests/sq-sign.rs +++ b/tool/tests/sq-sign.rs @@ -208,7 +208,7 @@ fn sq_sign_append_on_compress_then_sign() { // message by foot. let tsk = TPK::from_file(&p("keys/dennis-simon-anton-private.pgp")) .unwrap(); - let key = tsk.select_signing_keys(None)[0]; + let key = tsk.keys_all().signing_capable().nth(0).unwrap().2; let sec = match key.secret() { Some(SecretKey::Unencrypted { ref mpis }) => mpis, _ => unreachable!(), -- cgit v1.2.3