diff options
author | Justus Winter <justus@sequoia-pgp.org> | 2020-05-25 13:20:15 +0200 |
---|---|---|
committer | Justus Winter <justus@sequoia-pgp.org> | 2020-05-28 11:52:26 +0200 |
commit | 271280e62d1e0ee64a8f4cbb5766b17e3edf947d (patch) | |
tree | d30a6172c9626e6fb36db62f336bd7d80abce819 /openpgp | |
parent | 94dcb41c69c4e16f1f491a9b27148e90a0d713e7 (diff) |
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.
Diffstat (limited to 'openpgp')
-rw-r--r-- | openpgp/examples/decrypt-with.rs | 8 | ||||
-rw-r--r-- | openpgp/examples/generate-encrypt-decrypt.rs | 4 | ||||
-rw-r--r-- | openpgp/src/parse/stream.rs | 37 | ||||
-rw-r--r-- | openpgp/src/policy.rs | 11 | ||||
-rw-r--r-- | openpgp/src/serialize/stream.rs | 5 |
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) } } |