openpgp: Change the `decrypt` proxy in the decryption helper.

  - Returning rich errors from this function may compromise secret key
    material due to Bleichenbacher-style attacks.  Change the API to
    prevent this.

  - Hat tip to Hanno Böck.

  - Fixes #507.

5 files changed, 33 insertions, 32 deletions

diff --git a/openpgp/examples/decrypt-with.rs b/openpgp/examples/decrypt-with.rs
index 3a5723bc..f50d2102 100644
--- a/openpgp/examples/decrypt-with.rs
+++ b/openpgp/examples/decrypt-with.rs
@@ -84,14 +84,14 @@ impl DecryptionHelper for Helper {
                   sym_algo: Option<SymmetricAlgorithm>,
                   mut decrypt: D)
                   -> openpgp::Result<Option<openpgp::Fingerprint>>
-        where D: FnMut(SymmetricAlgorithm, &SessionKey) -> openpgp::Result<()>
+        where D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool
     {
         // Try each PKESK until we succeed.
         for pkesk in pkesks {
             if let Some(pair) = self.keys.get_mut(pkesk.recipient()) {
-                if let Some(_) = pkesk.decrypt(pair, sym_algo)
-                    .and_then(|(algo, session_key)| decrypt(algo, &session_key)
-                              .ok())
+                if pkesk.decrypt(pair, sym_algo)
+                    .map(|(algo, session_key)| decrypt(algo, &session_key))
+                    .unwrap_or(false)
                 {
                     break;
                 }
diff --git a/openpgp/examples/generate-encrypt-decrypt.rs b/openpgp/examples/generate-encrypt-decrypt.rs
index c7e2e38d..3a02f264 100644
--- a/openpgp/examples/generate-encrypt-decrypt.rs
+++ b/openpgp/examples/generate-encrypt-decrypt.rs
@@ -118,7 +118,7 @@ impl<'a> DecryptionHelper for Helper<'a> {
                   sym_algo: Option<SymmetricAlgorithm>,
                   mut decrypt: D)
                   -> openpgp::Result<Option<openpgp::Fingerprint>>
-        where D: FnMut(SymmetricAlgorithm, &SessionKey) -> openpgp::Result<()>
+        where D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool
     {
         let key = self.secret.keys().unencrypted_secret()
             .with_policy(self.policy, None)
@@ -128,7 +128,7 @@ impl<'a> DecryptionHelper for Helper<'a> {
         let mut pair = key.into_keypair().unwrap();
 
         pkesks[0].decrypt(&mut pair, sym_algo)
-            .and_then(|(algo, session_key)| decrypt(algo, &session_key).ok());
+            .map(|(algo, session_key)| decrypt(algo, &session_key));
 
         // XXX: In production code, return the Fingerprint of the
         // recipient's Cert here
diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs
index 69513432..bb242291 100644
--- a/openpgp/src/parse/stream.rs
+++ b/openpgp/src/parse/stream.rs
@@ -667,7 +667,7 @@ impl<V: VerificationHelper> DecryptionHelper for NoDecryptionHelper<V> {
     fn decrypt<D>(&mut self, _: &[PKESK], _: &[SKESK],
                   _: Option<SymmetricAlgorithm>,
                   _: D) -> Result<Option<Fingerprint>>
-        where D: FnMut(SymmetricAlgorithm, &SessionKey) -> Result<()>
+        where D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool
     {
         unreachable!("This is not used for verifications")
     }
@@ -1135,11 +1135,11 @@ enum Mode {
 ///     fn decrypt<D>(&mut self, _: &[PKESK], skesks: &[SKESK],
 ///                   _sym_algo: Option<SymmetricAlgorithm>,
 ///                   mut decrypt: D) -> Result<Option<openpgp::Fingerprint>>
-///         where D: FnMut(SymmetricAlgorithm, &SessionKey) -> Result<()>
+///         where D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool
 ///     {
 ///         skesks[0].decrypt(&"streng geheim".into())
-///             .and_then(|(algo, session_key)| decrypt(algo, &session_key))
-///             .map(|_| None)
+///             .map(|(algo, session_key)| decrypt(algo, &session_key));
+///         Ok(None)
 ///     }
 /// }
 ///
@@ -1318,7 +1318,8 @@ pub trait DecryptionHelper {
     /// the symmetric algorithm and session key from one of the
     /// [`PKESK`] packets, the [`SKESK`] packets, or retrieve it from
     /// a cache, and then call `decrypt` with the symmetric algorithm
-    /// and session key.
+    /// and session key.  `decrypt` returns `true` if the decryption
+    /// was successful.
     ///
     ///   [`PKESK`]: ../../packet/enum.PKESK.html
     ///   [`SKESK`]: ../../packet/enum.SKESK.html
@@ -1371,14 +1372,14 @@ pub trait DecryptionHelper {
     ///     fn decrypt<D>(&mut self, pkesks: &[PKESK], skesks: &[SKESK],
     ///                   sym_algo: Option<SymmetricAlgorithm>,
     ///                   mut decrypt: D) -> Result<Option<Fingerprint>>
-    ///         where D: FnMut(SymmetricAlgorithm, &SessionKey) -> Result<()>
+    ///         where D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool
     ///     {
     ///         // Try to decrypt, from the most convenient method to the
     ///         // least convenient one.
     ///
     ///         // First, see if it is in the cache.
     ///         if let Some((fp, algo, sk)) = lookup_cache(pkesks, skesks) {
-    ///             if decrypt(algo, &sk).is_ok() {
+    ///             if decrypt(algo, &sk) {
     ///                 return Ok(fp);
     ///             }
     ///         }
@@ -1390,8 +1391,8 @@ pub trait DecryptionHelper {
     ///                 if ! key.secret().is_encrypted() {
     ///                     let mut keypair = key.clone().into_keypair()?;
     ///                     if pkesk.decrypt(&mut keypair, sym_algo)
-    ///                         .and_then(|(algo, sk)| decrypt(algo, &sk).ok())
-    ///                         .is_some()
+    ///                         .map(|(algo, sk)| decrypt(algo, &sk))
+    ///                         .unwrap_or(false)
     ///                     {
     ///                         return Ok(Some(fp));
     ///                     }
@@ -1407,8 +1408,8 @@ pub trait DecryptionHelper {
     ///                 if ! key.secret().is_encrypted() {
     ///                     let mut keypair = key.clone().into_keypair()?;
     ///                     if pkesk.decrypt(&mut keypair, sym_algo)
-    ///                         .and_then(|(algo, sk)| decrypt(algo, &sk).ok())
-    ///                         .is_some()
+    ///                         .map(|(algo, sk)| decrypt(algo, &sk))
+    ///                         .unwrap_or(false)
     ///                     {
     ///                         return Ok(Some(fp));
     ///                     }
@@ -1438,8 +1439,8 @@ pub trait DecryptionHelper {
     ///
     ///             for skesk in skesks {
     ///                 if skesk.decrypt(&password)
-    ///                     .and_then(|(algo, sk)| decrypt(algo, &sk))
-    ///                     .is_ok()
+    ///                     .map(|(algo, sk)| decrypt(algo, &sk))
+    ///                     .unwrap_or(false)
     ///                 {
     ///                     return Ok(None);
     ///                 }
@@ -1453,7 +1454,7 @@ pub trait DecryptionHelper {
     fn decrypt<D>(&mut self, pkesks: &[PKESK], skesks: &[SKESK],
                   sym_algo: Option<SymmetricAlgorithm>,
                   decrypt: D) -> Result<Option<Fingerprint>>
-        where D: FnMut(SymmetricAlgorithm, &SessionKey) -> Result<()>;
+        where D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool;
 }
 
 impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
@@ -1558,8 +1559,10 @@ impl<'a, H: VerificationHelper + DecryptionHelper> Decryptor<'a, H> {
                             let result = pp.decrypt(algo, secret);
                             if let Ok(_) = result {
                                 sym_algo = Some(algo);
+                                true
+                            } else {
+                                false
                             }
-                            result
                         };
 
                         v.identity =
@@ -2081,7 +2084,7 @@ mod test {
         fn decrypt<D>(&mut self, _: &[PKESK], _: &[SKESK],
                       _: Option<SymmetricAlgorithm>, _: D)
                       -> Result<Option<Fingerprint>>
-            where D: FnMut(SymmetricAlgorithm, &SessionKey) -> Result<()>
+            where D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool
         {
             unreachable!();
         }
@@ -2212,7 +2215,7 @@ mod test {
             fn decrypt<D>(&mut self, _: &[PKESK], _: &[SKESK],
                           _: Option<SymmetricAlgorithm>, _: D)
                           -> Result<Option<Fingerprint>>
-                where D: FnMut(SymmetricAlgorithm, &SessionKey) -> Result<()>
+                where D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool
             {
                 unreachable!();
             }
diff --git a/openpgp/src/policy.rs b/openpgp/src/policy.rs
index fe266b79..a4635201 100644
--- a/openpgp/src/policy.rs
+++ b/openpgp/src/policy.rs
@@ -1201,7 +1201,7 @@ mod test {
             fn decrypt<D>(&mut self, _: &[PKESK], _: &[SKESK],
                           _: Option<SymmetricAlgorithm>,_: D)
                           -> Result<Option<Fingerprint>>
-                where D: FnMut(SymmetricAlgorithm, &SessionKey) -> Result<()>
+                where D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool
             {
                 unreachable!();
             }
@@ -1640,7 +1640,7 @@ mod test {
             fn decrypt<D>(&mut self, _: &[PKESK], _: &[SKESK],
                           _: Option<SymmetricAlgorithm>,_: D)
                           -> Result<Option<Fingerprint>>
-                where D: FnMut(SymmetricAlgorithm, &SessionKey) -> Result<()>
+                where D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool
             {
                 unreachable!();
             }
@@ -1763,7 +1763,7 @@ mod test {
             fn decrypt<D>(&mut self, _: &[PKESK], _: &[SKESK],
                           _: Option<SymmetricAlgorithm>, _: D)
                           -> Result<Option<Fingerprint>>
-                where D: FnMut(SymmetricAlgorithm, &SessionKey) -> Result<()> {
+                where D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool {
                 Ok(None)
             }
         }
@@ -1810,7 +1810,7 @@ mod test {
             fn decrypt<D>(&mut self, pkesks: &[PKESK], _: &[SKESK],
                           algo: Option<SymmetricAlgorithm>, mut decrypt: D)
                           -> Result<Option<Fingerprint>>
-                where D: FnMut(SymmetricAlgorithm, &SessionKey) -> Result<()>
+                where D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool
             {
                 let p = &P::new();
                 let mut pair = Cert::from_bytes(
@@ -1819,8 +1819,7 @@ mod test {
                     .for_transport_encryption().secret().nth(0).unwrap()
                     .key().clone().into_keypair()?;
                 pkesks[0].decrypt(&mut pair, algo)
-                    .and_then(|(algo, session_key)|
-                              decrypt(algo, &session_key).ok());
+                    .map(|(algo, session_key)| decrypt(algo, &session_key));
                 Ok(None)
             }
         }
diff --git a/openpgp/src/serialize/stream.rs b/openpgp/src/serialize/stream.rs
index 7228bcd5..36c3358f 100644
--- a/openpgp/src/serialize/stream.rs
+++ b/openpgp/src/serialize/stream.rs
@@ -3057,7 +3057,7 @@ mod test {
             fn decrypt<D>(&mut self, pkesks: &[PKESK], _skesks: &[SKESK],
                           sym_algo: Option<SymmetricAlgorithm>,
                           mut decrypt: D) -> Result<Option<crate::Fingerprint>>
-                where D: FnMut(SymmetricAlgorithm, &SessionKey) -> Result<()>
+                where D: FnMut(SymmetricAlgorithm, &SessionKey) -> bool
             {
                 let mut keypair = self.tsk.keys().with_policy(self.policy, None)
                     .for_transport_encryption()
@@ -3065,8 +3065,7 @@ mod test {
                     .clone().parts_into_secret().unwrap()
                     .into_keypair().unwrap();
                 pkesks[0].decrypt(&mut keypair, sym_algo)
-                    .and_then(|(algo, session_key)|
-                              decrypt(algo, &session_key).ok());
+                    .map(|(algo, session_key)| decrypt(algo, &session_key));
                 Ok(None)
             }
         }