openpgp: Return Result<()> from Signature::verify*.

7 files changed, 65 insertions, 84 deletions

diff --git a/openpgp/src/cert/mod.rs b/openpgp/src/cert/mod.rs
index 8109a401..c52c346f 100644
--- a/openpgp/src/cert/mod.rs
+++ b/openpgp/src/cert/mod.rs
@@ -1282,9 +1282,9 @@ impl Cert {
                 for sig in mem::replace(&mut $binding.$sigs, Vec::new())
                     .into_iter()
                 {
-                    if let Ok(true) = sig.$verify_method(self.primary.key(),
-                                                         self.primary.key(),
-                                                         $($verify_args),*) {
+                    if let Ok(()) = sig.$verify_method(self.primary.key(),
+                                                       self.primary.key(),
+                                                       $($verify_args),*) {
                         $binding.$sigs.push(sig);
                     } else {
                         t!("Sig {:02X}{:02X}, type = {} doesn't belong to {}",
@@ -1327,7 +1327,7 @@ impl Cert {
                             // See if we can get the key for a
                             // positive verification.
                             if let Some(key) = $lookup_fn(&sig) {
-                                if let Ok(true) = sig.$verify_method(
+                                if let Ok(()) = sig.$verify_method(
                                     &key, self.primary.key(), $($verify_args),*)
                                 {
                                     $binding.$sigs.push(sig);
@@ -1459,7 +1459,7 @@ impl Cert {
                      t!("check_one!({}, {:?}, {:?}, {}, ...)",
                         $desc, $sigs, $sig,
                         stringify!($verify_method));
-                     if let Ok(true)
+                     if let Ok(())
                          = $sig.$verify_method(self.primary.key(),
                                                self.primary.key(),
                                                $($verify_args),*)
@@ -1497,9 +1497,9 @@ impl Cert {
                        $desc, stringify!($sigs), $sig,
                        stringify!($verify_method), stringify!($hash_method));
                     if let Some(key) = $lookup_fn(&sig) {
-                        if let Ok(true) = sig.$verify_method(&key,
-                                                             self.primary.key(),
-                                                             $($verify_args),*)
+                        if let Ok(()) = sig.$verify_method(&key,
+                                                           self.primary.key(),
+                                                           $($verify_args),*)
                         {
                             t!("Sig {:02X}{:02X}, {:?} \
                                 was out of place.  Belongs to {}.",
@@ -3427,13 +3427,10 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g=
                        &[ alice_certifies_bob.clone() ]);
 
             // Make sure the certification is correct.
-            assert_eq!(
-                alice_certifies_bob
-                    .verify_userid_binding(&alice.primary().clone(),
-                                           &bob.primary().clone(),
-                                           bob_userid_binding.userid())
-                    .unwrap(),
-                true);
+            alice_certifies_bob
+                .verify_userid_binding(&alice.primary().clone(),
+                                       &bob.primary().clone(),
+                                       bob_userid_binding.userid()).unwrap();
         }
    }
 
diff --git a/openpgp/src/crypto/asymmetric.rs b/openpgp/src/crypto/asymmetric.rs
index 76729ddb..32787db6 100644
--- a/openpgp/src/crypto/asymmetric.rs
+++ b/openpgp/src/crypto/asymmetric.rs
@@ -306,13 +306,13 @@ impl<P: key::KeyParts, R: key::KeyRole> Key<P, R> {
     }
 
     /// Verifies the given signature.
-    pub fn verify(&self, sig: &packet::Signature, digest: &[u8]) -> Result<bool>
+    pub fn verify(&self, sig: &packet::Signature, digest: &[u8]) -> Result<()>
     {
         use crate::PublicKeyAlgorithm::*;
         use crate::crypto::mpis::{PublicKey, Signature};
 
         #[allow(deprecated)]
-        match (sig.pk_algo(), self.mpis(), sig.mpis()) {
+        let ok = match (sig.pk_algo(), self.mpis(), sig.mpis()) {
             (RSASign,        PublicKey::RSA { e, n }, Signature::RSA { s }) |
             (RSAEncryptSign, PublicKey::RSA { e, n }, Signature::RSA { s }) => {
                 let key = rsa::PublicKey::new(n.value(), e.value())?;
@@ -394,6 +394,12 @@ impl<P: key::KeyParts, R: key::KeyRole> Key<P, R> {
                 "unsupported combination of algorithm {}, key {} and \
                  signature {:?}.",
                 sig.pk_algo(), self.pk_algo(), sig.mpis())).into()),
+        }?;
+
+        if ok {
+            Ok(())
+        } else {
+            Err(Error::ManipulatedMessage.into())
         }
     }
 }
diff --git a/openpgp/src/packet/key/mod.rs b/openpgp/src/packet/key/mod.rs
index c334ab4a..152459e7 100644
--- a/openpgp/src/packet/key/mod.rs
+++ b/openpgp/src/packet/key/mod.rs
@@ -1623,7 +1623,7 @@ mod tests {
                                       r: mpis::MPI::new(r), s: mpis::MPI::new(s)
                                   });
         let sig: Signature = sig.into();
-        assert_eq!(sig.verify_message(&key, b"Hello, World\n").unwrap(), true);
+        sig.verify_message(&key, b"Hello, World\n").unwrap();
     }
 
     #[test]
diff --git a/openpgp/src/packet/signature/mod.rs b/openpgp/src/packet/signature/mod.rs
index 3424c85b..78e98347 100644
--- a/openpgp/src/packet/signature/mod.rs
+++ b/openpgp/src/packet/signature/mod.rs
@@ -599,7 +599,7 @@ impl crate::packet::Signature {
     /// subkey binding signature (if appropriate), has the signing
     /// capability, etc.
     pub fn verify_digest<P, R, D>(&self, key: &Key<P, R>, digest: D)
-        -> Result<bool>
+        -> Result<()>
         where P: key::KeyParts,
               R: key::KeyRole,
               D: AsRef<[u8]>,
@@ -632,7 +632,7 @@ impl crate::packet::Signature {
     /// is not revoked, not expired, has a valid self-signature, has a
     /// subkey binding signature (if appropriate), has the signing
     /// capability, etc.
-    pub fn verify<P, R>(&self, key: &Key<P, R>) -> Result<bool>
+    pub fn verify<P, R>(&self, key: &Key<P, R>) -> Result<()>
         where P: key::KeyParts,
               R: key::KeyRole,
     {
@@ -661,7 +661,7 @@ impl crate::packet::Signature {
     /// is not revoked, not expired, has a valid self-signature, has a
     /// subkey binding signature (if appropriate), has the signing
     /// capability, etc.
-    pub fn verify_standalone<P, R>(&self, key: &Key<P, R>) -> Result<bool>
+    pub fn verify_standalone<P, R>(&self, key: &Key<P, R>) -> Result<()>
         where P: key::KeyParts,
               R: key::KeyRole,
     {
@@ -688,7 +688,7 @@ impl crate::packet::Signature {
     /// is not revoked, not expired, has a valid self-signature, has a
     /// subkey binding signature (if appropriate), has the signing
     /// capability, etc.
-    pub fn verify_timestamp<P, R>(&self, key: &Key<P, R>) -> Result<bool>
+    pub fn verify_timestamp<P, R>(&self, key: &Key<P, R>) -> Result<()>
         where P: key::KeyParts,
               R: key::KeyRole,
     {
@@ -724,7 +724,7 @@ impl crate::packet::Signature {
     pub fn verify_direct_key<P, Q, R>(&self,
                                       signer: &Key<P, R>,
                                       pk: &Key<Q, key::PrimaryRole>)
-        -> Result<bool>
+        -> Result<()>
         where P: key::KeyParts,
               Q: key::KeyParts,
               R: key::KeyRole,
@@ -759,7 +759,7 @@ impl crate::packet::Signature {
     pub fn verify_primary_key_revocation<P, Q, R>(&self,
                                                   signer: &Key<P, R>,
                                                   pk: &Key<Q, key::PrimaryRole>)
-        -> Result<bool>
+        -> Result<()>
         where P: key::KeyParts,
               Q: key::KeyParts,
               R: key::KeyRole,
@@ -801,7 +801,7 @@ impl crate::packet::Signature {
         signer: &Key<P, R>,
         pk: &Key<Q, key::PrimaryRole>,
         subkey: &Key<S, key::SubordinateRole>)
-        -> Result<bool>
+        -> Result<()>
         where P: key::KeyParts,
               Q: key::KeyParts,
               R: key::KeyRole,
@@ -812,23 +812,20 @@ impl crate::packet::Signature {
         }
 
         let hash = Signature::hash_subkey_binding(self, pk, subkey)?;
-        if self.verify_digest(signer, &hash[..])? {
-            // The signature is good, but we may still need to verify
-            // the back sig.
+        self.verify_digest(signer, &hash[..])?;
+
+        // The signature is good, but we may still need to verify the
+        // back sig.
+        if self.key_flags().for_signing() {
+            if let Some(backsig) = self.embedded_signature() {
+                backsig.verify_primary_key_binding(pk, subkey)
+            } else {
+                Err(Error::BadSignature(
+                    "Primary key binding signature missing".into()).into())
+            }
         } else {
-            return Ok(false);
-        }
-
-        if ! self.key_flags().for_signing() {
             // No backsig required.
-            return Ok(true)
-        }
-
-        if let Some(backsig) = self.embedded_signature() {
-            backsig.verify_primary_key_binding(pk, subkey)
-        } else {
-            Err(Error::BadSignature(
-                "Primary key binding signature missing".into()).into())
+            Ok(())
         }
     }
 
@@ -852,7 +849,7 @@ impl crate::packet::Signature {
         &self,
         pk: &Key<P, key::PrimaryRole>,
         subkey: &Key<Q, key::SubordinateRole>)
-        -> Result<bool>
+        -> Result<()>
         where P: key::KeyParts,
               Q: key::KeyParts,
     {
@@ -888,7 +885,7 @@ impl crate::packet::Signature {
         signer: &Key<P, R>,
         pk: &Key<Q, key::PrimaryRole>,
         subkey: &Key<S, key::SubordinateRole>)
-        -> Result<bool>
+        -> Result<()>
         where P: key::KeyParts,
               Q: key::KeyParts,
               R: key::KeyRole,
@@ -925,7 +922,7 @@ impl crate::packet::Signature {
                                           signer: &Key<P, R>,
                                           pk: &Key<Q, key::PrimaryRole>,
                                           userid: &UserID)
-        -> Result<bool>
+        -> Result<()>
         where P: key::KeyParts,
               Q: key::KeyParts,
               R: key::KeyRole,
@@ -964,7 +961,7 @@ impl crate::packet::Signature {
                                              signer: &Key<P, R>,
                                              pk: &Key<Q, key::PrimaryRole>,
                                              userid: &UserID)
-        -> Result<bool>
+        -> Result<()>
         where P: key::KeyParts,
               Q: key::KeyParts,
               R: key::KeyRole,
@@ -1000,7 +997,7 @@ impl crate::packet::Signature {
                                                   signer: &Key<P, R>,
                                                   pk: &Key<Q, key::PrimaryRole>,
                                                   ua: &UserAttribute)
-        -> Result<bool>
+        -> Result<()>
         where P: key::KeyParts,
               Q: key::KeyParts,
               R: key::KeyRole,
@@ -1040,7 +1037,7 @@ impl crate::packet::Signature {
         signer: &Key<P, R>,
         pk: &Key<Q, key::PrimaryRole>,
         ua: &UserAttribute)
-        -> Result<bool>
+        -> Result<()>
         where P: key::KeyParts,
               Q: key::KeyParts,
               R: key::KeyRole,
@@ -1074,7 +1071,7 @@ impl crate::packet::Signature {
     /// signing capability, etc.
     pub fn verify_message<M, P, R>(&self, signer: &Key<P, R>,
                                    msg: M)
-        -> Result<bool>
+        -> Result<()>
         where M: AsRef<[u8]>,
               P: key::KeyParts,
               R: key::KeyRole,
@@ -1215,7 +1212,8 @@ mod test {
                 crate::tests::message(test.data)).unwrap();
             while let PacketParserResult::Some(pp) = ppr {
                 if let Packet::Signature(ref sig) = pp.packet {
-                    let result = sig.verify(cert.primary()).unwrap_or(false);
+                    let result = sig.verify(cert.primary())
+                        .map(|_| true).unwrap_or(false);
                     eprintln!("  Primary {:?}: {:?}",
                               cert.primary().fingerprint(), result);
                     if result {
@@ -1223,7 +1221,8 @@ mod test {
                     }
 
                     for sk in cert.subkeys() {
-                        let result = sig.verify(sk.key()).unwrap_or(false);
+                        let result = sig.verify(sk.key())
+                            .map(|_| true).unwrap_or(false);
                         eprintln!("   Subkey {:?}: {:?}",
                                   sk.key().fingerprint(), result);
                         if result {
@@ -1291,11 +1290,11 @@ mod test {
             sig.hash(&mut hash);
             let mut digest = vec![0u8; hash.digest_size()];
             hash.digest(&mut digest);
-            assert!(sig.verify_digest(pair.public(), &digest[..]).unwrap());
+            sig.verify_digest(pair.public(), &digest[..]).unwrap();
 
             // Bad signature.
             digest[0] ^= 0xff;
-            assert!(! sig.verify_digest(pair.public(), &digest[..]).unwrap());
+            sig.verify_digest(pair.public(), &digest[..]).unwrap_err();
         }
     }
 
@@ -1315,7 +1314,7 @@ mod test {
             .set_issuer(pair.public().keyid()).unwrap()
             .sign_message(&mut pair, msg).unwrap();
 
-        assert!(sig.verify_message(pair.public(), msg).unwrap());
+        sig.verify_message(pair.public(), msg).unwrap();
     }
 
     #[test]
@@ -1332,7 +1331,7 @@ mod test {
             panic!("Expected a Signature, got: {:?}", p);
         };
 
-        assert!(sig.verify_message(cert.primary(), &msg[..]).unwrap());
+        sig.verify_message(cert.primary(), &msg[..]).unwrap();
     }
 
     #[test]
@@ -1389,10 +1388,9 @@ mod test {
             .unwrap().1.unwrap().0;
         let cert = &uid_binding.certifications()[0];
 
-        assert_eq!(cert.verify_userid_binding(cert_key1,
-                                              test2.primary(),
-                                              uid_binding.userid()).ok(),
-                   Some(true));
+        cert.verify_userid_binding(cert_key1,
+                                   test2.primary(),
+                                   uid_binding.userid()).unwrap();
     }
 
     #[test]
@@ -1463,7 +1461,7 @@ mod test {
             .sign_standalone(&mut pair)
             .unwrap();
 
-        assert!(sig.verify_standalone(pair.public()).unwrap());
+        sig.verify_standalone(pair.public()).unwrap();
     }
 
     #[test]
@@ -1475,7 +1473,7 @@ mod test {
         if let Packet::Signature(sig) = p {
             let digest = Signature::hash_standalone(&sig).unwrap();
             eprintln!("{}", crate::fmt::hex::encode(&digest));
-            assert!(sig.verify_timestamp(alpha.primary()).unwrap());
+            sig.verify_timestamp(alpha.primary()).unwrap();
         } else {
             panic!("expected a signature packet");
         }
@@ -1495,6 +1493,6 @@ mod test {
             .sign_timestamp(&mut pair)
             .unwrap();
 
-        assert!(sig.verify_timestamp(pair.public()).unwrap());
+        sig.verify_timestamp(pair.public()).unwrap();
     }
 }
diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs
index ec53635c..8599af93 100644
--- a/openpgp/src/parse/stream.rs
+++ b/openpgp/src/parse/stream.rs
@@ -733,7 +733,7 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> {
                                 }
                             } else {
                                 match sig.verify(ka.key()) {
-                                    Ok(true) => {
+                                    Ok(()) => {
                                         results.push_verification_result(
                                             VerificationResult::GoodChecksum {
                                                 sig: sig,
@@ -743,13 +743,6 @@ impl<'a, H: VerificationHelper> Verifier<'a, H> {
                                         // Continue to the next sig.
                                         continue 'sigs;
                                     }
-                                    Ok(false) => {
-                                        VerificationResult::Error {
-                                            sig: sig.clone(),
-                                            error:
-                                            Error::ManipulatedMessage.into(),
-                                        }
-                                    }
                                     Err(err) => {
                                         VerificationResult::Error {
                                             sig: sig.clone(),
@@ -1726,7 +1719,7 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
                                 }
                             } else {
                                 match sig.verify(ka.key()) {
-                                    Ok(true) => {
+                                    Ok(()) => {
                                         results.push_verification_result(
                                             VerificationResult::GoodChecksum {
                                                 sig: sig,
@@ -1736,13 +1729,6 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
                                         // Continue to the next sig.
                                         continue 'sigs;
                                     }
-                                    Ok(false) => {
-                                        VerificationResult::Error {
-                                            sig: sig.clone(),
-                                            error:
-                                            Error::ManipulatedMessage.into(),
-                                        }
-                                    }
                                     Err(err) => {
                                         VerificationResult::Error {
                                             sig: sig.clone(),
diff --git a/openpgp/src/serialize/stream.rs b/openpgp/src/serialize/stream.rs
index 6fb4a241..eaeee97e 100644
--- a/openpgp/src/serialize/stream.rs
+++ b/openpgp/src/serialize/stream.rs
@@ -1502,8 +1502,7 @@ mod test {
             if let Packet::Signature(ref sig) = pp.packet {
                 let key = keys.get(&sig.issuer_fingerprint().unwrap())
                     .unwrap();
-                let result = sig.verify(key).unwrap();
-                assert!(result);
+                sig.verify(key).unwrap();
                 good += 1;
             }
 
diff --git a/sqv/src/sqv.rs b/sqv/src/sqv.rs
index 380b52f4..afa27833 100644
--- a/sqv/src/sqv.rs
+++ b/sqv/src/sqv.rs
@@ -262,7 +262,7 @@ fn real_main() -> Result<(), failure::Error> {
                     let mut digest = vec![0u8; hash.digest_size()];
                     hash.digest(&mut digest);
                     match sig.verify_digest(key, &digest[..]) {
-                        Ok(true) => {
+                        Ok(()) => {
                             if let Some(t) = sig.signature_creation_time() {
                                 if let Some(not_before) = not_before {
                                     if t < not_before {
@@ -336,11 +336,6 @@ fn real_main() -> Result<(), failure::Error> {
                             println!("{}", cert.primary().fingerprint());
                             good += 1;
                         },
-                        Ok(false) => {
-                            if trace {
-                                eprintln!("Signature by {} is bad.", issuer);
-                            }
-                        },
                         Err(err) => {
                             if trace {
                                 eprintln!("Verifying signature: {}.", err);