openpgp: Make the various keyflags() methods return an Option.

  - This signals the absence of a subpacket.

  - Likewise for features(), key_server_preferences().

6 files changed, 35 insertions, 36 deletions

diff --git a/openpgp-ffi/src/packet/signature.rs b/openpgp-ffi/src/packet/signature.rs
index 9b41282a..60903fc5 100644
--- a/openpgp-ffi/src/packet/signature.rs
+++ b/openpgp-ffi/src/packet/signature.rs
@@ -71,14 +71,14 @@ fn pgp_signature_issuer_fingerprint(sig: *const Signature)
 /// make certifications.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "C"
 fn pgp_signature_for_certification(sig: *const Signature) -> bool {
-    sig.ref_raw().key_flags().for_certification()
+    sig.ref_raw().key_flags().unwrap_or_default().for_certification()
 }
 
 /// Returns whether the KeyFlags indicates that the key can be used to
 /// make signatures.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "C"
 fn pgp_signature_for_signing(sig: *const Signature) -> bool {
-    sig.ref_raw().key_flags().for_signing()
+    sig.ref_raw().key_flags().unwrap_or_default().for_signing()
 }
 
 /// Returns whether the KeyFlags indicates that the key can be used to
@@ -86,35 +86,35 @@ fn pgp_signature_for_signing(sig: *const Signature) -> bool {
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "C"
 fn pgp_signature_for_transport_encryption(sig: *const Signature)
                                            -> bool {
-    sig.ref_raw().key_flags().for_transport_encryption()
+    sig.ref_raw().key_flags().unwrap_or_default().for_transport_encryption()
 }
 
 /// Returns whether the KeyFlags indicates that the key can be used to
 /// encrypt data at rest.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "C"
 fn pgp_signature_for_storage_encryption(sig: *const Signature) -> bool {
-    sig.ref_raw().key_flags().for_storage_encryption()
+    sig.ref_raw().key_flags().unwrap_or_default().for_storage_encryption()
 }
 
 /// Returns whether the KeyFlags indicates that the key can be used
 /// for authentication.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "C"
 fn pgp_signature_for_authentication(sig: *const Signature) -> bool {
-    sig.ref_raw().key_flags().for_authentication()
+    sig.ref_raw().key_flags().unwrap_or_default().for_authentication()
 }
 
 /// Returns whether the KeyFlags indicates that the key is a split
 /// key.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "C"
 fn pgp_signature_is_split_key(sig: *const Signature) -> bool {
-    sig.ref_raw().key_flags().is_split_key()
+    sig.ref_raw().key_flags().unwrap_or_default().is_split_key()
 }
 
 /// Returns whether the KeyFlags indicates that the key is a group
 /// key.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle] pub extern "C"
 fn pgp_signature_is_group_key(sig: *const Signature) -> bool {
-    sig.ref_raw().key_flags().is_group_key()
+    sig.ref_raw().key_flags().unwrap_or_default().is_group_key()
 }
 
 
diff --git a/openpgp/src/cert/builder.rs b/openpgp/src/cert/builder.rs
index e5144ef2..ef5d5b62 100644
--- a/openpgp/src/cert/builder.rs
+++ b/openpgp/src/cert/builder.rs
@@ -503,7 +503,7 @@ mod tests {
         let sig =
             cert.keys().primary(None).unwrap().binding_signature();
         assert_eq!(sig.typ(), crate::types::SignatureType::DirectKey);
-        assert!(sig.features().supports_mdc());
+        assert!(sig.features().unwrap().supports_mdc());
     }
 
     #[test]
@@ -535,7 +535,7 @@ mod tests {
                    PublicKeyAlgorithm::EdDSA);
         assert!(cert1.subkeys().next().is_none());
         if let Some(sig) = cert1.primary_key_signature(None) {
-            assert!(sig.features().supports_mdc());
+            assert!(sig.features().unwrap().supports_mdc());
         } else {
             panic!();
         }
diff --git a/openpgp/src/cert/key_amalgamation.rs b/openpgp/src/cert/key_amalgamation.rs
index cbdd9ee0..0e57c234 100644
--- a/openpgp/src/cert/key_amalgamation.rs
+++ b/openpgp/src/cert/key_amalgamation.rs
@@ -281,8 +281,7 @@ impl<'a, P: 'a + key::KeyParts> KeyAmalgamation<'a, P> {
 
     /// Returns the key's key flags as of the amalgamtion's
     /// reference time.
-    pub fn key_flags(&self) -> KeyFlags
-    {
+    pub fn key_flags(&self) -> Option<KeyFlags> {
         self.binding_signature.key_flags()
     }
 
@@ -291,7 +290,7 @@ impl<'a, P: 'a + key::KeyParts> KeyAmalgamation<'a, P> {
     pub fn has_any_key_flag<F>(&self, flags: F) -> bool
         where F: Borrow<KeyFlags>
     {
-        let our_flags = self.key_flags();
+        let our_flags = self.key_flags().unwrap_or_default();
         !(&our_flags & flags.borrow()).is_empty()
     }
 
diff --git a/openpgp/src/packet/signature/mod.rs b/openpgp/src/packet/signature/mod.rs
index c7eb2e7d..50709276 100644
--- a/openpgp/src/packet/signature/mod.rs
+++ b/openpgp/src/packet/signature/mod.rs
@@ -816,7 +816,7 @@ impl crate::packet::Signature {
 
         // The signature is good, but we may still need to verify the
         // back sig.
-        if self.key_flags().for_signing() {
+        if self.key_flags().map(|kf| kf.for_signing()).unwrap_or(false) {
             if let Some(backsig) = self.embedded_signature() {
                 backsig.verify_primary_key_binding(pk, subkey)
             } else {
diff --git a/openpgp/src/packet/signature/subpacket.rs b/openpgp/src/packet/signature/subpacket.rs
index 7010a6f3..1e19e724 100644
--- a/openpgp/src/packet/signature/subpacket.rs
+++ b/openpgp/src/packet/signature/subpacket.rs
@@ -1293,16 +1293,16 @@ impl SubpacketArea {
     ///
     /// Note: if the signature contains multiple instances of this
     /// subpacket, only the last one is considered.
-    pub fn key_server_preferences(&self) -> KeyServerPreferences {
+    pub fn key_server_preferences(&self) -> Option<KeyServerPreferences> {
         // N octets of flags
         if let Some(sb) = self.subpacket(SubpacketTag::KeyServerPreferences) {
             if let SubpacketValue::KeyServerPreferences(v) = &sb.value {
-                v.clone()
+                Some(v.clone())
             } else {
-                KeyServerPreferences::default()
+                None
             }
         } else {
-            KeyServerPreferences::default()
+            None
         }
     }
 
@@ -1380,20 +1380,20 @@ impl SubpacketArea {
     /// used (certification, signing, encryption, authentication), and
     /// how it is stored (split, held by multiple people).
     ///
-    /// If the subpacket is not present, this returns the empty set.
+    /// If the subpacket is not present, this returns `None`.
     ///
     /// Note: if the signature contains multiple instances of this
     /// subpacket, only the last one is considered.
-    pub fn key_flags(&self) -> KeyFlags {
+    pub fn key_flags(&self) -> Option<KeyFlags> {
         // N octets of flags
         if let Some(sb) = self.subpacket(SubpacketTag::KeyFlags) {
             if let SubpacketValue::KeyFlags(v) = &sb.value {
-                v.clone()
+                Some(v.clone())
             } else {
-                KeyFlags::default()
+                None
             }
         } else {
-            KeyFlags::default()
+            None
         }
     }
 
@@ -1447,21 +1447,20 @@ impl SubpacketArea {
     /// list of features that the user's OpenPGP implementation
     /// supports.
     ///
-    /// If the subpacket is not present or malformed, this returns
-    /// the default value.
+    /// If the subpacket is not present, this returns `None`.
     ///
     /// Note: if the signature contains multiple instances of this
     /// subpacket, only the last one is considered.
-    pub fn features(&self) -> Features {
+    pub fn features(&self) -> Option<Features> {
         // N octets of flags
         if let Some(sb) = self.subpacket(SubpacketTag::Features) {
             if let SubpacketValue::Features(v) = &sb.value {
-                v.clone()
+                Some(v.clone())
             } else {
-                Features::default()
+                None
             }
         } else {
-            Features::default()
+            None
         }
     }
 
@@ -2432,7 +2431,7 @@ fn accessors() {
     sig = sig.set_key_server_preferences(pref.clone()).unwrap();
     let sig_ =
         sig.clone().sign_hash(&mut keypair, hash.clone()).unwrap();
-    assert_eq!(sig_.key_server_preferences(), pref);
+    assert_eq!(sig_.key_server_preferences().unwrap(), pref);
 
     sig = sig.set_primary_userid(true).unwrap();
     let sig_ =
@@ -2454,7 +2453,7 @@ fn accessors() {
     sig = sig.set_key_flags(&key_flags).unwrap();
     let sig_ =
         sig.clone().sign_hash(&mut keypair, hash.clone()).unwrap();
-    assert_eq!(sig_.key_flags(), key_flags);
+    assert_eq!(sig_.key_flags().unwrap(), key_flags);
 
     sig = sig.set_signers_user_id(b"foobar").unwrap();
     let sig_ =
@@ -2472,13 +2471,13 @@ fn accessors() {
     sig = sig.set_features(&feats).unwrap();
     let sig_ =
         sig.clone().sign_hash(&mut keypair, hash.clone()).unwrap();
-    assert_eq!(sig_.features(), feats);
+    assert_eq!(sig_.features().unwrap(), feats);
 
     let feats = Features::default().set_aead(true);
     sig = sig.set_features(&feats).unwrap();
     let sig_ =
         sig.clone().sign_hash(&mut keypair, hash.clone()).unwrap();
-    assert_eq!(sig_.features(), feats);
+    assert_eq!(sig_.features().unwrap(), feats);
 
     let digest = vec![0; hash_algo.context().unwrap().digest_size()];
     sig = sig.set_signature_target(pk_algo, hash_algo, &digest).unwrap();
@@ -2730,7 +2729,7 @@ fn subpacket_test_2() {
                                 CompressionAlgorithm::Zip]
                        )}));
 
-        assert_eq!(sig.key_server_preferences(),
+        assert_eq!(sig.key_server_preferences().unwrap(),
                    KeyServerPreferences::default().set_no_modify(true));
         assert_eq!(sig.subpacket(SubpacketTag::KeyServerPreferences),
                    Some(&Subpacket {
@@ -2740,7 +2739,8 @@ fn subpacket_test_2() {
                            KeyServerPreferences::default().set_no_modify(true)),
                    }));
 
-        assert!(sig.key_flags().for_certification() && sig.key_flags().for_signing());
+        assert!(sig.key_flags().unwrap().for_certification());
+        assert!(sig.key_flags().unwrap().for_signing());
         assert_eq!(sig.subpacket(SubpacketTag::KeyFlags),
                    Some(&Subpacket {
                        length: 2.into(),
@@ -2749,7 +2749,7 @@ fn subpacket_test_2() {
                            KeyFlags::default().set_certification(true).set_signing(true))
                    }));
 
-        assert_eq!(sig.features(), Features::default().set_mdc(true));
+        assert_eq!(sig.features().unwrap(), Features::default().set_mdc(true));
         assert_eq!(sig.subpacket(SubpacketTag::Features),
                    Some(&Subpacket {
                        length: 2.into(),
diff --git a/tool/src/commands/inspect.rs b/tool/src/commands/inspect.rs
index 6cba96a1..fa16be32 100644
--- a/tool/src/commands/inspect.rs
+++ b/tool/src/commands/inspect.rs
@@ -202,7 +202,7 @@ fn inspect_key<P, R>(output: &mut dyn io::Write,
                      expires.convert())?;
         }
 
-        if let Some(flags) = inspect_key_flags(sig.key_flags()) {
+        if let Some(flags) = sig.key_flags().and_then(inspect_key_flags) {
             writeln!(output, "{}       Key flags: {}", indent, flags)?;
         }
     }