openpgp: Pass a timestamp to the KeyIter instead of each filter.

  - KeyIter::revoked and KeyIter::key_flags (and its variants) didn't
    take a time stamp so they could only be used for filtering keys
    based on their current state, not their state at some time in the
    past.  Adding a time stamp to each of the filters would have fixed
    the problem, but it would have made the interface ugly: callers
    always want the same time stamp for all filters.

  - Split KeyIter into two structures: a KeyIter and a ValidKeyIter.

  - Add KeyIter::policy.  It takes a time stamp, which is then used
    for filters like `alive` and `revoked`, and it returns a
    ValidKeyIter, which exposes filters that require a time stamp.

2 files changed, 196 insertions, 66 deletions

diff --git a/openpgp-ffi/src/cert.rs b/openpgp-ffi/src/cert.rs
index 0c91dd88..90aac1ea 100644
--- a/openpgp-ffi/src/cert.rs
+++ b/openpgp-ffi/src/cert.rs
@@ -26,6 +26,7 @@ use self::openpgp::{
         CertRevocationBuilder,
         UserIDBinding,
         UserIDBindingIter,
+        ValidKeyIter,
     },
 };
 
@@ -459,17 +460,11 @@ pub extern "C" fn pgp_cert_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 `for_signing`
-/// function, the *union* of the values is used.  That is, the
-/// iterator will return keys that are certification capable *or*
-/// signing capable.
+/// Changes the iterator to only return keys that have secret keys.
 ///
 /// Note: you may not call this function after starting to iterate.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle]
-pub extern "C" fn pgp_cert_key_iter_for_certification<'a>(
+pub extern "C" fn pgp_cert_key_iter_secret<'a>(
     iter_wrapper: *mut KeyIterWrapper<'a>)
 {
     let iter_wrapper = ffi_param_ref_mut!(iter_wrapper);
@@ -479,20 +474,15 @@ pub extern "C" fn pgp_cert_key_iter_for_certification<'a>(
 
     use std::mem;
     let tmp = mem::replace(&mut iter_wrapper.iter, unsafe { mem::zeroed() });
-    iter_wrapper.iter = tmp.for_certification();
+    iter_wrapper.iter = unsafe { std::mem::transmute(tmp.secret()) };
 }
 
-/// Changes the iterator to only return keys that are certification
-/// capable.
-///
-/// If you call this function and, e.g., the `for_signing`
-/// function, the *union* of the values is used.  That is, the
-/// iterator will return keys that are certification capable *or*
-/// signing capable.
+/// Changes the iterator to only return keys that have unencrypted
+/// secret keys.
 ///
 /// Note: you may not call this function after starting to iterate.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle]
-pub extern "C" fn pgp_cert_key_iter_for_signing<'a>(
+pub extern "C" fn pgp_cert_key_iter_unencrypted_secret<'a>(
     iter_wrapper: *mut KeyIterWrapper<'a>)
 {
     let iter_wrapper = ffi_param_ref_mut!(iter_wrapper);
@@ -502,21 +492,19 @@ pub extern "C" fn pgp_cert_key_iter_for_signing<'a>(
 
     use std::mem;
     let tmp = mem::replace(&mut iter_wrapper.iter, unsafe { mem::zeroed() });
-    iter_wrapper.iter = tmp.for_signing();
+    iter_wrapper.iter =
+        unsafe { std::mem::transmute(tmp.unencrypted_secret()) };
 }
 
-/// Changes the iterator to only return keys that are capable of
-/// encrypting data at rest.
-///
-/// If you call this function and, e.g., the `for_signing`
-/// function, the *union* of the values is used.  That is, the
-/// iterator will return keys that are certification capable *or*
-/// signing capable.
+/// Changes the iterator to only return keys that are valid at time
+/// `t`.
 ///
 /// Note: you may not call this function after starting to iterate.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle]
-pub extern "C" fn pgp_cert_key_iter_for_storage_encryption<'a>(
-    iter_wrapper: *mut KeyIterWrapper<'a>)
+pub extern "C" fn pgp_cert_key_iter_policy<'a>(
+    iter_wrapper: *mut KeyIterWrapper<'a>,
+    when: time_t)
+    -> *mut ValidKeyIterWrapper<'static>
 {
     let iter_wrapper = ffi_param_ref_mut!(iter_wrapper);
     if iter_wrapper.next_called {
@@ -525,11 +513,111 @@ pub extern "C" fn pgp_cert_key_iter_for_storage_encryption<'a>(
 
     use std::mem;
     let tmp = mem::replace(&mut iter_wrapper.iter, unsafe { mem::zeroed() });
-    iter_wrapper.iter = tmp.for_storage_encryption();
+
+    box_raw!(ValidKeyIterWrapper {
+        iter: unsafe { std::mem::transmute(tmp.policy(maybe_time(when))) },
+        next_called: false,
+    })
 }
 
-/// Changes the iterator to only return keys that are capable of
-/// encrypting data for transport.
+/// 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
+/// *sigo.  (Note: subkeys always have signatures, but a primary key
+/// may not have a direct signature, and there might not be any user
+/// ids.)
+///
+/// If rso is not NULL, this stores the key's revocation status in
+/// *rso.
+#[::sequoia_ffi_macros::extern_fn] #[no_mangle]
+pub extern "C" fn pgp_cert_key_iter_next<'a>(
+    iter_wrapper: *mut KeyIterWrapper<'a>)
+    -> Maybe<Key>
+{
+    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())
+            .move_into_raw()
+    } else {
+        None
+    }
+}
+
+/// 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>,
+    // Whether next has been called.
+    next_called: bool,
+}
+
+/// Returns an iterator over all valid `Key`s in a Cert.
+///
+/// That is, this returns an iterator over the primary key and any
+/// subkeys that are valid (i.e., have a self-signature at time
+/// `when`).
+#[::sequoia_ffi_macros::extern_fn] #[no_mangle]
+pub extern "C" fn pgp_cert_valid_key_iter(cert: *const Cert, when: time_t)
+    -> *mut ValidKeyIterWrapper<'static>
+{
+    let cert = cert.ref_raw();
+    let iter = box_raw!(KeyIterWrapper {
+        iter: cert.keys(),
+        next_called: false,
+    });
+
+    pgp_cert_key_iter_policy(iter, when)
+}
+
+/// Frees a pgp_cert_key_iter_t.
+#[::sequoia_ffi_macros::extern_fn] #[no_mangle]
+pub extern "C" fn pgp_cert_valid_key_iter_free(
+    iter: Option<&mut ValidKeyIterWrapper>)
+{
+    ffi_free!(iter)
+}
+
+/// Changes the iterator to only return keys that have secret keys.
+///
+/// Note: you may not call this function after starting to iterate.
+#[::sequoia_ffi_macros::extern_fn] #[no_mangle]
+pub extern "C" fn pgp_cert_valid_key_iter_secret<'a>(
+    iter_wrapper: *mut ValidKeyIterWrapper<'a>)
+{
+    let iter_wrapper = ffi_param_ref_mut!(iter_wrapper);
+    if iter_wrapper.next_called {
+        panic!("Can't change ValidKeyIter filter after iterating.");
+    }
+
+    use std::mem;
+    let tmp = mem::replace(&mut iter_wrapper.iter, unsafe { mem::zeroed() });
+    iter_wrapper.iter = unsafe { std::mem::transmute(tmp.secret()) };
+}
+
+/// Changes the iterator to only return keys that have unencrypted
+/// secret keys.
+///
+/// Note: you may not call this function after starting to iterate.
+#[::sequoia_ffi_macros::extern_fn] #[no_mangle]
+pub extern "C" fn pgp_cert_valid_key_iter_unencrypted_secret<'a>(
+    iter_wrapper: *mut ValidKeyIterWrapper<'a>)
+{
+    let iter_wrapper = ffi_param_ref_mut!(iter_wrapper);
+    if iter_wrapper.next_called {
+        panic!("Can't change ValidKeyIter filter after iterating.");
+    }
+
+    use std::mem;
+    let tmp = mem::replace(&mut iter_wrapper.iter, unsafe { mem::zeroed() });
+    iter_wrapper.iter =
+        unsafe { std::mem::transmute(tmp.unencrypted_secret()) };
+}
+
+/// Changes the iterator to only return keys that are certification
+/// capable.
 ///
 /// If you call this function and, e.g., the `for_signing`
 /// function, the *union* of the values is used.  That is, the
@@ -538,8 +626,8 @@ pub extern "C" fn pgp_cert_key_iter_for_storage_encryption<'a>(
 ///
 /// Note: you may not call this function after starting to iterate.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle]
-pub extern "C" fn pgp_cert_key_iter_for_transport_encryption<'a>(
-    iter_wrapper: *mut KeyIterWrapper<'a>)
+pub extern "C" fn pgp_cert_valid_key_iter_for_certification<'a>(
+    iter_wrapper: *mut ValidKeyIterWrapper<'a>)
 {
     let iter_wrapper = ffi_param_ref_mut!(iter_wrapper);
     if iter_wrapper.next_called {
@@ -548,18 +636,21 @@ pub extern "C" fn pgp_cert_key_iter_for_transport_encryption<'a>(
 
     use std::mem;
     let tmp = mem::replace(&mut iter_wrapper.iter, unsafe { mem::zeroed() });
-    iter_wrapper.iter = tmp.for_transport_encryption();
+    iter_wrapper.iter = tmp.for_certification();
 }
 
-/// Changes the iterator to only return keys that are alive.
+/// Changes the iterator to only return keys that are certification
+/// capable.
 ///
-/// If you call this function (or `pgp_cert_key_iter_alive_at`), only
-/// the last value is used.
+/// If you call this function and, e.g., the `for_signing`
+/// 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 "C" fn pgp_cert_key_iter_alive<'a>(
-    iter_wrapper: *mut KeyIterWrapper<'a>)
+pub extern "C" fn pgp_cert_valid_key_iter_for_signing<'a>(
+    iter_wrapper: *mut ValidKeyIterWrapper<'a>)
 {
     let iter_wrapper = ffi_param_ref_mut!(iter_wrapper);
     if iter_wrapper.next_called {
@@ -568,20 +659,21 @@ pub extern "C" fn pgp_cert_key_iter_alive<'a>(
 
     use std::mem;
     let tmp = mem::replace(&mut iter_wrapper.iter, unsafe { mem::zeroed() });
-    iter_wrapper.iter = tmp.alive();
+    iter_wrapper.iter = tmp.for_signing();
 }
 
-/// Changes the iterator to only return keys that are alive at the
-/// specified time.
+/// Changes the iterator to only return keys that are capable of
+/// encrypting data at rest.
 ///
-/// If you call this function (or `pgp_cert_key_iter_alive`), only the
-/// last value is used.
+/// If you call this function and, e.g., the `for_signing`
+/// 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 "C" fn pgp_cert_key_iter_alive_at<'a>(
-    iter_wrapper: *mut KeyIterWrapper<'a>,
-    when: time_t)
+pub extern "C" fn pgp_cert_valid_key_iter_for_storage_encryption<'a>(
+    iter_wrapper: *mut ValidKeyIterWrapper<'a>)
 {
     let iter_wrapper = ffi_param_ref_mut!(iter_wrapper);
     if iter_wrapper.next_called {
@@ -590,18 +682,21 @@ pub extern "C" fn pgp_cert_key_iter_alive_at<'a>(
 
     use std::mem;
     let tmp = mem::replace(&mut iter_wrapper.iter, unsafe { mem::zeroed() });
-    iter_wrapper.iter =
-        tmp.alive_at(maybe_time(when).unwrap_or(std::time::UNIX_EPOCH));
+    iter_wrapper.iter = tmp.for_storage_encryption();
 }
 
-/// Changes the iterator to only return keys whose revocation status
-/// matches `revoked`.
+/// Changes the iterator to only return keys that are capable of
+/// encrypting data for transport.
+///
+/// If you call this function and, e.g., the `for_signing`
+/// 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 "C" fn pgp_cert_key_iter_revoked<'a>(
-    iter_wrapper: *mut KeyIterWrapper<'a>,
-    revoked: bool)
+pub extern "C" fn pgp_cert_valid_key_iter_for_transport_encryption<'a>(
+    iter_wrapper: *mut ValidKeyIterWrapper<'a>)
 {
     let iter_wrapper = ffi_param_ref_mut!(iter_wrapper);
     if iter_wrapper.next_called {
@@ -610,15 +705,18 @@ pub extern "C" fn pgp_cert_key_iter_revoked<'a>(
 
     use std::mem;
     let tmp = mem::replace(&mut iter_wrapper.iter, unsafe { mem::zeroed() });
-    iter_wrapper.iter = tmp.revoked(Some(revoked));
+    iter_wrapper.iter = tmp.for_transport_encryption();
 }
 
-/// Changes the iterator to only return keys that have secret keys.
+/// Changes the iterator to only return keys that are alive.
+///
+/// If you call this function (or `pgp_cert_valid_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 "C" fn pgp_cert_key_iter_secret<'a>(
-    iter_wrapper: *mut KeyIterWrapper<'a>)
+pub extern "C" fn pgp_cert_valid_key_iter_alive<'a>(
+    iter_wrapper: *mut ValidKeyIterWrapper<'a>)
 {
     let iter_wrapper = ffi_param_ref_mut!(iter_wrapper);
     if iter_wrapper.next_called {
@@ -627,16 +725,17 @@ pub extern "C" fn pgp_cert_key_iter_secret<'a>(
 
     use std::mem;
     let tmp = mem::replace(&mut iter_wrapper.iter, unsafe { mem::zeroed() });
-    iter_wrapper.iter = unsafe { std::mem::transmute(tmp.secret()) };
+    iter_wrapper.iter = tmp.alive();
 }
 
-/// Changes the iterator to only return keys that have unencrypted
-/// secret keys.
+/// 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 "C" fn pgp_cert_key_iter_unencrypted_secret<'a>(
-    iter_wrapper: *mut KeyIterWrapper<'a>)
+pub extern "C" fn pgp_cert_valid_key_iter_revoked<'a>(
+    iter_wrapper: *mut ValidKeyIterWrapper<'a>,
+    revoked: bool)
 {
     let iter_wrapper = ffi_param_ref_mut!(iter_wrapper);
     if iter_wrapper.next_called {
@@ -645,8 +744,7 @@ pub extern "C" fn pgp_cert_key_iter_unencrypted_secret<'a>(
 
     use std::mem;
     let tmp = mem::replace(&mut iter_wrapper.iter, unsafe { mem::zeroed() });
-    iter_wrapper.iter =
-        unsafe { std::mem::transmute(tmp.unencrypted_secret()) };
+    iter_wrapper.iter = tmp.revoked(Some(revoked));
 }
 
 /// Returns the next key.  Returns NULL if there are no more elements.
@@ -659,8 +757,8 @@ pub extern "C" fn pgp_cert_key_iter_unencrypted_secret<'a>(
 /// If rso is not NULL, this stores the key's revocation status in
 /// *rso.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle]
-pub extern "C" fn pgp_cert_key_iter_next<'a>(
-    iter_wrapper: *mut KeyIterWrapper<'a>,
+pub extern "C" fn pgp_cert_valid_key_iter_next<'a>(
+    iter_wrapper: *mut ValidKeyIterWrapper<'a>,
     sigo: Option<&mut Maybe<Signature>>,
     rso: Option<&mut *mut RevocationStatus<'a>>)
     -> Maybe<Key>
diff --git a/openpgp-ffi/src/serialize.rs b/openpgp-ffi/src/serialize.rs
index 0a6435a0..d6a7695b 100644
--- a/openpgp-ffi/src/serialize.rs
+++ b/openpgp-ffi/src/serialize.rs
@@ -38,6 +38,7 @@ use self::openpgp::serialize::{
 use super::keyid::KeyID;
 use super::packet::key::Key;
 use super::cert::KeyIterWrapper;
+use super::cert::ValidKeyIterWrapper;
 
 /// Streams an OpenPGP message.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle]
@@ -282,6 +283,37 @@ 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())
+        .collect::<Vec<openpgp::serialize::stream::Recipient>>();
+
+    let result = unsafe {
+        libc::calloc(recipients.len(), std::mem::size_of::<* mut Recipient>())
+            as *mut *mut Recipient
+    };
+    let r = unsafe {
+        slice::from_raw_parts_mut(result,
+                                  recipients.len())
+    };
+    *result_len = recipients.len();
+    r.iter_mut().zip(recipients.into_iter())
+        .for_each(|(r, recipient)| *r = recipient.move_into_raw());
+    result
+}
+
+/// Collects recipients from a `pgp_cert_valid_key_iter_t`.
+///
+/// Consumes the iterator.  The returned buffer must be freed using
+/// libc's allocator.
+#[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "C"
+fn pgp_recipients_from_valid_key_iter<'a>(
+    iter_wrapper: *mut ValidKeyIterWrapper<'a>,
+    result_len: *mut size_t)
+    -> *mut *mut Recipient<'a>
+{
+    let iter_wrapper = ffi_param_move!(iter_wrapper);
+    let result_len = ffi_param_ref_mut!(result_len);
+    let recipients =
+        iter_wrapper.iter
         .map(|ka| ka.key().into())
         .collect::<Vec<openpgp::serialize::stream::Recipient>>();