openpgp: unify revocation handling for all TPK elements.

Use active_revokation for TPK, SubkeyBinding and UserAttributeBinding.

Fixes #137

1 files changed, 61 insertions, 41 deletions

diff --git a/openpgp/src/tpk/mod.rs b/openpgp/src/tpk/mod.rs
index db3db1c2..a1d3b33e 100644
--- a/openpgp/src/tpk/mod.rs
+++ b/openpgp/src/tpk/mod.rs
@@ -301,6 +301,36 @@ impl TPKValidator {
 
 const TRACE : bool = false;
 
+/// Returns true if latest revocation signature in `revs` is newer than the
+/// latest self signature in sigs.
+///
+/// Signatures are expected to have the right signature types and be
+/// cryptographically sound.
+fn active_revocation(mut sigs: Vec<Signature>, mut revs: Vec<Signature>)
+    -> bool
+{
+    let cmp = |a: &Signature, b: &Signature| {
+        match (a.signature_creation_time(),b.signature_creation_time()) {
+            (None, None) => Ordering::Equal,
+            (None, Some(_)) => Ordering::Greater,
+            (Some(_), None) => Ordering::Less,
+            (Some(ref a), Some(ref b)) => a.cmp(b),
+        }
+    };
+
+    sigs.sort_by(&cmp);
+    revs.sort_by(&cmp);
+
+    match (sigs.last(), revs.last()) {
+        (None, Some(_)) => true,
+        (Some(_), None) => false,
+        (None, None) => false,
+        (Some(ref sig), Some(ref revc)) => {
+            cmp(sig, revc) != Ordering::Greater
+        }
+    }
+}
+
 /// A subkey and any associated signatures.
 #[derive(Debug, Clone, PartialEq)]
 pub struct SubkeyBinding {
@@ -534,16 +564,16 @@ impl UserIDBinding {
     /// then you need to query them separately.
     pub fn revoked(&self) -> RevocationStatus {
         let has_self_revs =
-            Self::active_revocation(self.selfsigs.clone(),
-                                    self.self_revocations.clone());
+            active_revocation(self.selfsigs.clone(),
+                              self.self_revocations.clone());
 
         if has_self_revs {
             return RevocationStatus::Revoked(&self.self_revocations[..]);
         }
 
         let has_other_revs =
-            Self::active_revocation(self.selfsigs.clone(),
-                                    self.other_revocations.clone());
+            active_revocation(self.selfsigs.clone(),
+                              self.other_revocations.clone());
 
         if has_other_revs {
             RevocationStatus::CouldBe(&self.other_revocations[..])
@@ -551,36 +581,6 @@ impl UserIDBinding {
             RevocationStatus::NotAsFarAsWeKnow
         }
     }
-
-    /// Returns true if latest revocation signature in `revs` is newer than the
-    /// latest self signature in sigs.
-    ///
-    /// Signatures are expected to have the right signature types and be
-    /// cryptographically sound.
-    fn active_revocation(mut sigs: Vec<Signature>, mut revs: Vec<Signature>)
-    -> bool
-    {
-        let cmp = |a: &Signature, b: &Signature| {
-            match (a.signature_creation_time(),b.signature_creation_time()) {
-                (None, None) => Ordering::Equal,
-                (None, Some(_)) => Ordering::Greater,
-                (Some(_), None) => Ordering::Less,
-                (Some(ref a), Some(ref b)) => a.cmp(b),
-            }
-        };
-
-        sigs.sort_by(&cmp);
-        revs.sort_by(&cmp);
-
-        match (sigs.last(), revs.last()) {
-            (None, Some(_)) => true,
-            (Some(_), None) => false,
-            (None, None) => false,
-            (Some(ref sig), Some(ref revc)) => {
-                cmp(sig, revc) != Ordering::Greater
-            }
-        }
-    }
 }
 
 /// A User Attribute and any associated signatures.
@@ -654,9 +654,19 @@ impl UserAttributeBinding {
     /// If you want to know whether the key, subkey, etc., is revoked,
     /// then you need to query them separately.
     pub fn revoked(&self) -> RevocationStatus {
-        if self.self_revocations.len() > 0 {
-            RevocationStatus::Revoked(&self.self_revocations[..])
-        } else if self.other_revocations.len() > 0 {
+        let has_self_revs =
+            active_revocation(self.selfsigs.clone(),
+                              self.self_revocations.clone());
+
+        if has_self_revs {
+            return RevocationStatus::Revoked(&self.self_revocations[..]);
+        }
+
+        let has_other_revs =
+            active_revocation(self.selfsigs.clone(),
+                              self.other_revocations.clone());
+
+        if has_other_revs {
             RevocationStatus::CouldBe(&self.other_revocations[..])
         } else {
             RevocationStatus::NotAsFarAsWeKnow
@@ -1230,13 +1240,23 @@ impl TPK {
 
     /// Returns the TPK's revocation status.
     ///
-    /// Note: this only returns whether the TPK is revoked.  If you
+    /// Note: this only returns whether the primary key is revoked.  If you
     /// want to know whether a subkey, user id, etc., is revoked, then
     /// you need to query them separately.
     pub fn revoked(&self) -> RevocationStatus {
-        if self.primary_self_revocations.len() > 0 {
-            RevocationStatus::Revoked(&self.primary_self_revocations[..])
-        } else if self.primary_other_revocations.len() > 0 {
+        let has_self_revs =
+            active_revocation(self.primary_selfsigs.clone(),
+            self.primary_self_revocations.clone());
+
+        if has_self_revs {
+            return RevocationStatus::Revoked(&self.primary_self_revocations[..]);
+        }
+
+        let has_other_revs =
+            active_revocation(self.primary_selfsigs.clone(),
+            self.primary_other_revocations.clone());
+
+        if has_other_revs {
             RevocationStatus::CouldBe(&self.primary_other_revocations[..])
         } else {
             RevocationStatus::NotAsFarAsWeKnow