openpgp: Improve tracking of secret keys.

  - We use marker traits to track with the type system if a Key has
    secret key material attached.  Previously, it was possible to
    subvert that by taking the secret key material using
    Key4::set_secret, creating a Key4<SecretParts, ..> without any
    secrets.

  - Related, the accessor functions returned an
    Option<SecretKeyMaterial> even for Key4<SecretParts, ..>.

  - Replace set_secret by add_secret and take_secret that also change
    the Key's type accordingly.  Make the accessors infallible if we
    know we have a secret key, rename Key4<P, R>::secret to
    Key4<P, R>::optional_secret to make the distinction clear.

  - Fixes #435.

15 files changed, 186 insertions, 89 deletions

diff --git a/openpgp/examples/notarize.rs b/openpgp/examples/notarize.rs
index 83e34cee..9c8605ea 100644
--- a/openpgp/examples/notarize.rs
+++ b/openpgp/examples/notarize.rs
@@ -36,12 +36,12 @@ fn main() {
         {
             keys.push({
                 let mut key = key.clone();
-                if key.secret().expect("filtered").is_encrypted() {
+                if key.secret().is_encrypted() {
                     let password = rpassword::read_password_from_tty(
                         Some(&format!("Please enter password to decrypt \
                                        {}/{}: ",tsk, key))).unwrap();
                     let algo = key.pk_algo();
-                    key.secret_mut().expect("filtered")
+                    key.secret_mut()
                         .decrypt_in_place(algo, &password.into())
                         .expect("decryption failed");
                 }
diff --git a/openpgp/examples/sign-detached.rs b/openpgp/examples/sign-detached.rs
index da5cd776..2d576808 100644
--- a/openpgp/examples/sign-detached.rs
+++ b/openpgp/examples/sign-detached.rs
@@ -32,12 +32,12 @@ fn main() {
         {
             keys.push({
                 let mut key = key.clone();
-                if key.secret().expect("filtered").is_encrypted() {
+                if key.secret().is_encrypted() {
                     let password = rpassword::read_password_from_tty(
                         Some(&format!("Please enter password to decrypt \
                                        {}/{}: ",tsk, key))).unwrap();
                     let algo = key.pk_algo();
-                    key.secret_mut().expect("filtered")
+                    key.secret_mut()
                         .decrypt_in_place(algo, &password.into())
                         .expect("decryption failed");
                 }
diff --git a/openpgp/examples/sign.rs b/openpgp/examples/sign.rs
index 95afee20..1a0f61dd 100644
--- a/openpgp/examples/sign.rs
+++ b/openpgp/examples/sign.rs
@@ -31,12 +31,12 @@ fn main() {
         {
             keys.push({
                 let mut key = key.clone();
-                if key.secret().expect("filtered").is_encrypted() {
+                if key.secret().is_encrypted() {
                     let password = rpassword::read_password_from_tty(
                         Some(&format!("Please enter password to decrypt \
                                        {}/{}: ",tsk, key))).unwrap();
                     let algo = key.pk_algo();
-                    key.secret_mut().expect("filtered")
+                    key.secret_mut()
                         .decrypt_in_place(algo, &password.into())
                         .expect("decryption failed");
                 }
diff --git a/openpgp/src/cert/builder.rs b/openpgp/src/cert/builder.rs
index d0f81918..7c4d063d 100644
--- a/openpgp/src/cert/builder.rs
+++ b/openpgp/src/cert/builder.rs
@@ -301,7 +301,7 @@ impl CertBuilder {
         packets.push(Packet::SecretKey({
             let mut primary = primary.clone();
             if let Some(ref password) = self.password {
-                primary.secret_mut().unwrap().encrypt_in_place(password)?;
+                primary.secret_mut().encrypt_in_place(password)?;
             }
             primary
         }));
@@ -380,7 +380,7 @@ impl CertBuilder {
             let signature = subkey.bind(&mut signer, &cert, builder)?;
 
             if let Some(ref password) = self.password {
-                subkey.secret_mut().unwrap().encrypt_in_place(password)?;
+                subkey.secret_mut().encrypt_in_place(password)?;
             }
             cert = cert.merge_packets(vec![Packet::SecretSubkey(subkey),
                                          signature.into()])?;
@@ -575,7 +575,7 @@ mod tests {
             .set_cipher_suite(CipherSuite::Cv25519)
             .set_password(Some(String::from("streng geheim").into()))
             .generate().unwrap();
-        assert!(cert.primary_key().secret().unwrap().is_encrypted());
+        assert!(cert.primary_key().optional_secret().unwrap().is_encrypted());
     }
 
     #[test]
diff --git a/openpgp/src/cert/keyiter.rs b/openpgp/src/cert/keyiter.rs
index d3e94bac..d8b7b9b5 100644
--- a/openpgp/src/cert/keyiter.rs
+++ b/openpgp/src/cert/keyiter.rs
@@ -146,7 +146,7 @@ impl<'a, P: 'a + key::KeyParts> KeyIter<'a, P> {
             }
 
             if let Some(want_unencrypted_secret) = self.unencrypted_secret {
-                if let Some(secret) = ka.key().secret() {
+                if let Some(secret) = ka.key().optional_secret() {
                     if let SecretKeyMaterial::Unencrypted { .. } = secret {
                         if ! want_unencrypted_secret {
                             t!("Unencrypted secret... skipping.");
@@ -651,7 +651,7 @@ impl<'a, P: 'a + key::KeyParts> ValidKeyIter<'a, P> {
             }
 
             if let Some(want_unencrypted_secret) = self.unencrypted_secret {
-                if let Some(secret) = key.secret() {
+                if let Some(secret) = key.optional_secret() {
                     if let SecretKeyMaterial::Unencrypted { .. } = secret {
                         if ! want_unencrypted_secret {
                             t!("Unencrypted secret... skipping.");
@@ -1007,7 +1007,7 @@ impl<'a, P: 'a + key::KeyParts, R: 'a + key::KeyRole> KeyBundleIter<'a, P, R>
             }
 
             if let Some(want_unencrypted_secret) = self.unencrypted_secret {
-                if let Some(secret) = key.secret() {
+                if let Some(secret) = key.optional_secret() {
                     if let SecretKeyMaterial::Unencrypted { .. } = secret {
                         if ! want_unencrypted_secret {
                             t!("Unencrypted secret... skipping.");
diff --git a/openpgp/src/cert/mod.rs b/openpgp/src/cert/mod.rs
index 31fe1dfd..2efafef9 100644
--- a/openpgp/src/cert/mod.rs
+++ b/openpgp/src/cert/mod.rs
@@ -3094,9 +3094,10 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g=
         assert_eq!(cert.keys().secret().count(), 2);
         assert_eq!(cert.keys().unencrypted_secret().count(), 0);
 
-        let mut primary = cert.primary_key().key().clone();
+        let mut primary = cert.primary_key().key().clone()
+            .mark_parts_secret().unwrap();
         let algo = primary.pk_algo();
-        primary.secret_mut().unwrap()
+        primary.secret_mut()
             .decrypt_in_place(algo, &"streng geheim".into()).unwrap();
         let cert = cert.merge_packets(vec![
             primary.mark_parts_secret().unwrap().mark_role_primary().into()
@@ -3220,11 +3221,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g=
     fn merge_keeps_secrets() -> Result<()> {
         let primary_sec: Key<_, key::PrimaryRole> =
             key::Key4::generate_ecc(true, Curve::Ed25519)?.into();
-        let primary_pub: Key<key::PublicParts, key::PrimaryRole> = {
-            let mut k = primary_sec.clone();
-            k.set_secret(None);
-            k.into()
-        };
+        let primary_pub = primary_sec.clone().take_secret().0;
 
         let cert_p =
             Cert::from_packet_pile(vec![primary_pub.clone().into()].into())?;
@@ -3252,11 +3249,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g=
 
         let subkey_sec: Key<_, key::SubordinateRole> =
             key::Key4::generate_ecc(false, Curve::Cv25519)?.into();
-        let subkey_pub: Key<key::PublicParts, key::SubordinateRole> = {
-            let mut k = subkey_sec.clone();
-            k.set_secret(None);
-            k.into()
-        };
+        let subkey_pub = subkey_sec.clone().take_secret().0;
         let builder = signature::Builder::new(SignatureType::SubkeyBinding)
             .set_key_flags(&KeyFlags::default()
                            .set_transport_encryption(true))?;
diff --git a/openpgp/src/crypto/asymmetric.rs b/openpgp/src/crypto/asymmetric.rs
index 555132d6..73d9f4df 100644
--- a/openpgp/src/crypto/asymmetric.rs
+++ b/openpgp/src/crypto/asymmetric.rs
@@ -274,9 +274,8 @@ impl Decryptor for KeyPair {
 
 impl From<KeyPair> for Key<key::SecretParts, key::UnspecifiedRole> {
     fn from(p: KeyPair) -> Self {
-        let (mut key, secret) = (p.public, p.secret);
-        key.set_secret(Some(secret.into()));
-        key.mark_parts_secret().expect("XXX")
+        let (key, secret) = (p.public, p.secret);
+        key.add_secret(secret.into()).0
     }
 }
 
diff --git a/openpgp/src/packet/key/mod.rs b/openpgp/src/packet/key/mod.rs
index ac088594..6db85271 100644
--- a/openpgp/src/packet/key/mod.rs
+++ b/openpgp/src/packet/key/mod.rs
@@ -1334,25 +1334,11 @@ impl<P, R> Key4<P, R>
         }
     }
 
-    /// Gets the key packet's `SecretKeyMaterial`.
-    pub fn secret(&self) -> Option<&SecretKeyMaterial> {
+    /// Gets the key packet's `SecretKeyMaterial`, if any.
+    pub fn optional_secret(&self) -> Option<&SecretKeyMaterial> {
         self.secret.as_ref()
     }
 
-    /// Gets a mutable reference to the key packet's `SecretKeyMaterial`.
-    pub fn secret_mut(&mut self) -> Option<&mut SecretKeyMaterial> {
-        self.secret.as_mut()
-    }
-
-    /// Sets the key packet's `SecretKeyMaterial`.
-    ///
-    /// Returns the old value.
-    pub fn set_secret(&mut self, secret: Option<SecretKeyMaterial>)
-        -> Option<SecretKeyMaterial>
-    {
-        std::mem::replace(&mut self.secret, secret)
-    }
-
     /// Computes and returns the key's fingerprint as per Section 12.2
     /// of RFC 4880 as returns it as a KeyHandle.
     pub fn key_handle(&self) -> KeyHandle {
@@ -1378,6 +1364,69 @@ impl<P, R> Key4<P, R>
     }
 }
 
+macro_rules! impl_common_secret_functions {
+    ($t: ident) => {
+        /// Secret key handling.
+        impl<R> Key4<$t, R>
+            where R: key::KeyRole,
+        {
+            /// Takes the key packet's `SecretKeyMaterial`, if any.
+            pub fn take_secret(mut self)
+                               -> (Key4<PublicParts, R>, Option<SecretKeyMaterial>)
+            {
+                let old = std::mem::replace(&mut self.secret, None);
+                (self.mark_parts_public(), old)
+            }
+
+            /// Adds `SecretKeyMaterial` to the packet, returning the old if
+            /// any.
+            pub fn add_secret(mut self, secret: SecretKeyMaterial)
+                              -> (Key4<SecretParts, R>, Option<SecretKeyMaterial>)
+            {
+                let old = std::mem::replace(&mut self.secret, Some(secret));
+                (self.mark_parts_secret().expect("secret just set"), old)
+            }
+        }
+    }
+}
+impl_common_secret_functions!(PublicParts);
+impl_common_secret_functions!(UnspecifiedParts);
+
+/// Secret key handling.
+impl<R> Key4<SecretParts, R>
+    where R: key::KeyRole,
+{
+    /// Gets the key packet's `SecretKeyMaterial`.
+    pub fn secret(&self) -> &SecretKeyMaterial {
+        self.secret.as_ref().expect("has secret")
+    }
+
+    /// Gets a mutable reference to the key packet's
+    /// `SecretKeyMaterial`.
+    pub fn secret_mut(&mut self) -> &mut SecretKeyMaterial {
+        self.secret.as_mut().expect("has secret")
+    }
+
+    /// Takes the key packet's `SecretKeyMaterial`.
+    pub fn take_secret(mut self)
+                       -> (Key4<PublicParts, R>, SecretKeyMaterial)
+    {
+        let old = std::mem::replace(&mut self.secret, None);
+        (self.mark_parts_public(),
+         old.expect("Key<SecretParts, _> has a secret key material"))
+    }
+
+    /// Adds `SecretKeyMaterial` to the packet, returning the old if
+    /// any.
+    pub fn add_secret(mut self, secret: SecretKeyMaterial)
+                      -> (Key4<SecretParts, R>, SecretKeyMaterial)
+    {
+        let old = std::mem::replace(&mut self.secret, Some(secret));
+        (self.mark_parts_secret().expect("secret just set"),
+         old.expect("Key<SecretParts, _> has a secret key material"))
+    }
+}
+
 impl<P, R> From<Key4<P, R>> for super::Key<P, R>
     where P: key::KeyParts,
           R: key::KeyRole,
@@ -1652,7 +1701,7 @@ mod tests {
             Key4::generate_rsa(b).unwrap()
         }));
 
-        for mut key in keys {
+        for key in keys {
             let mut b = Vec::new();
             Packet::SecretKey(key.clone().into()).serialize(&mut b).unwrap();
 
@@ -1680,8 +1729,8 @@ mod tests {
             {
                 assert!(! parsed_key.has_secret());
 
-                key.set_secret(None);
-                assert_eq!(&key.mark_parts_public(), parsed_key);
+                let key = key.take_secret().0;
+                assert_eq!(&key, parsed_key);
             } else {
                 panic!("bad packet: {:?}", pp.path_ref(&[0]));
             }
@@ -1732,18 +1781,17 @@ mod tests {
         }));
 
         for key in keys {
-            assert!(! key.secret().unwrap().is_encrypted());
+            assert!(! key.secret().is_encrypted());
 
             let password = Password::from("foobarbaz");
             let mut encrypted_key = key.clone();
 
-            encrypted_key.secret_mut().unwrap()
-                .encrypt_in_place(&password).unwrap();
-            assert!(encrypted_key.secret().unwrap().is_encrypted());
+            encrypted_key.secret_mut().encrypt_in_place(&password).unwrap();
+            assert!(encrypted_key.secret().is_encrypted());
 
-            encrypted_key.secret_mut().unwrap()
+            encrypted_key.secret_mut()
                 .decrypt_in_place(key.pk_algo, &password).unwrap();
-            assert!(! key.secret().unwrap().is_encrypted());
+            assert!(! key.secret().is_encrypted());
             assert_eq!(key, encrypted_key);
             assert_eq!(key.secret(), encrypted_key.secret());
         }
@@ -1816,7 +1864,7 @@ mod tests {
         let dek = b"\x09\x0D\xDC\x40\xC5\x71\x51\x88\xAC\xBD\x45\x56\xD4\x2A\xDF\x77\xCD\xF4\x82\xA2\x1B\x8F\x2E\x48\x3B\xCA\xBF\xD3\xE8\x6D\x0A\x7C\xDF\x10\xe6";
 
         let key = key.mark_parts_public();
-        let got_dek = match key.secret() {
+        let got_dek = match key.optional_secret() {
             Some(SecretKeyMaterial::Unencrypted(ref u)) => u.map(|mpis| {
                 ecdh::decrypt(&key, mpis, &ciphertext)
                     .unwrap()
diff --git a/openpgp/src/packet/mod.rs b/openpgp/src/packet/mod.rs
index 82b05bcd..2bf6c102 100644
--- a/openpgp/src/packet/mod.rs
+++ b/openpgp/src/packet/mod.rs
@@ -598,19 +598,17 @@ impl<R: key::KeyRole> Key<key::SecretParts, R> {
     /// # Errors
     ///
     /// Fails if the secret key is missing, or encrypted.
-    pub fn into_keypair(mut self) -> Result<KeyPair> {
+    pub fn into_keypair(self) -> Result<KeyPair> {
         use crate::packet::key::SecretKeyMaterial;
-        let secret = match self.set_secret(None) {
-            Some(SecretKeyMaterial::Unencrypted(secret)) => secret,
-            Some(SecretKeyMaterial::Encrypted(_)) =>
+        let (key, secret) = self.take_secret();
+        let secret = match secret {
+            SecretKeyMaterial::Unencrypted(secret) => secret,
+            SecretKeyMaterial::Encrypted(_) =>
                 return Err(Error::InvalidArgument(
                     "secret key is encrypted".into()).into()),
-            None =>
-                return Err(Error::InvalidArgument(
-                    "no secret key".into()).into()),
         };
 
-        KeyPair::new(self.mark_role_unspecified().into(), secret)
+        KeyPair::new(key.mark_role_unspecified(), secret)
     }
 }
 
@@ -621,22 +619,87 @@ impl<R: key::KeyRole> key::Key4<key::SecretParts, R> {
     /// # Errors
     ///
     /// Fails if the secret key is missing, or encrypted.
-    pub fn into_keypair(mut self) -> Result<KeyPair> {
+    pub fn into_keypair(self) -> Result<KeyPair> {
         use crate::packet::key::SecretKeyMaterial;
-        let secret = match self.set_secret(None) {
-            Some(SecretKeyMaterial::Unencrypted(secret)) => secret,
-            Some(SecretKeyMaterial::Encrypted(_)) =>
+        let (key, secret) = self.take_secret();
+        let secret = match secret {
+            SecretKeyMaterial::Unencrypted(secret) => secret,
+            SecretKeyMaterial::Encrypted(_) =>
                 return Err(Error::InvalidArgument(
                     "secret key is encrypted".into()).into()),
-            None =>
-                return Err(Error::InvalidArgument(
-                    "no secret key".into()).into()),
         };
 
-        KeyPair::new(self.mark_role_unspecified().mark_parts_public().into(),
-                     secret)
+        KeyPair::new(key.mark_role_unspecified().into(), secret)
+    }
+}
+
+macro_rules! impl_common_secret_functions {
+    ($t: path) => {
+        /// Secret key handling.
+        impl<R: key::KeyRole> Key<$t, R> {
+            /// Takes the key packet's `SecretKeyMaterial`, if any.
+            pub fn take_secret(self)
+                               -> (Key<key::PublicParts, R>,
+                                   Option<key::SecretKeyMaterial>)
+            {
+                match self {
+                    Key::V4(k) => {
+                        let (k, s) = k.take_secret();
+                        (k.into(), s)
+                    },
+                    Key::__Nonexhaustive => unreachable!(),
+                }
+            }
+
+            /// Adds `SecretKeyMaterial` to the packet, returning the old if
+            /// any.
+            pub fn add_secret(self, secret: key::SecretKeyMaterial)
+                              -> (Key<key::SecretParts, R>,
+                                  Option<key::SecretKeyMaterial>)
+            {
+                match self {
+                    Key::V4(k) => {
+                        let (k, s) = k.add_secret(secret);
+                        (k.into(), s)
+                    },
+                    Key::__Nonexhaustive => unreachable!(),
+                }
+            }
+        }
     }
 }
+impl_common_secret_functions!(key::PublicParts);
+impl_common_secret_functions!(key::UnspecifiedParts);
+
+/// Secret key handling.
+impl<R: key::KeyRole> Key<key::SecretParts, R> {
+    /// Takes the key packet's `SecretKeyMaterial`.
+    pub fn take_secret(self)
+                       -> (Key<key::PublicParts, R>, key::SecretKeyMaterial)
+    {
+        match self {
+            Key::V4(k) => {
+                let (k, s) = k.take_secret();
+                (k.into(), s)
+            },
+            Key::__Nonexhaustive => unreachable!(),
+        }
+    }
+
+    /// Adds `SecretKeyMaterial` to the packet, returning the old.
+    pub fn add_secret(self, secret: key::SecretKeyMaterial)
+                      -> (Key<key::SecretParts, R>, key::SecretKeyMaterial)
+    {
+        match self {
+            Key::V4(k) => {
+                let (k, s) = k.add_secret(secret);
+                (k.into(), s)
+            },
+            Key::__Nonexhaustive => unreachable!(),
+        }
+    }
+}
+
 
 // Trivial forwarder for singleton enum.
 impl<P: key::KeyParts, R: key::KeyRole> Deref for Key<P, R> {
diff --git a/openpgp/src/packet/pkesk.rs b/openpgp/src/packet/pkesk.rs
index 0a39cd60..4ab19c8d 100644
--- a/openpgp/src/packet/pkesk.rs
+++ b/openpgp/src/packet/pkesk.rs
@@ -376,12 +376,12 @@ mod tests {
         let private_mpis = mpis::SecretKeyMaterial::ECDH {
             scalar: MPI::new(&sec[..]).into(),
         };
-        let mut key: key::UnspecifiedPublic
+        let key: key::UnspecifiedPublic
             = Key4::new(std::time::SystemTime::now(),
                         PublicKeyAlgorithm::ECDH,
                         public_mpis)
                 .unwrap().into();
-        key.set_secret(Some(private_mpis.into()));
+        let key = key.add_secret(private_mpis.into()).0;
         let sess_key = SessionKey::new(32);
         let pkesk = PKESK3::for_recipient(SymmetricAlgorithm::AES256, &sess_key,
                                           &key).unwrap();
diff --git a/openpgp/src/serialize/mod.rs b/openpgp/src/serialize/mod.rs
index 172c4d55..69b3fe31 100644
--- a/openpgp/src/serialize/mod.rs
+++ b/openpgp/src/serialize/mod.rs
@@ -1406,7 +1406,7 @@ impl<P, R> Key4<P, R>
         self.mpis().serialize(o)?;
 
         if have_secret_key {
-            match self.secret().unwrap() {
+            match self.optional_secret().unwrap() {
                 SecretKeyMaterial::Unencrypted(ref u) => u.map(|mpis| -> Result<()> {
                     // S2K usage.
                     write_byte(o, 0)?;
@@ -1443,7 +1443,7 @@ impl<P, R> Key4<P, R>
             + 1 // PK algo.
             + self.mpis().serialized_len()
             + if have_secret_key {
-                1 + match self.secret().as_ref().unwrap() {
+                1 + match self.optional_secret().unwrap() {
                     SecretKeyMaterial::Unencrypted(ref u) =>
                         u.map(|mpis| mpis.serialized_len())
                         + 2, // Two octet checksum.
diff --git a/tool/src/commands/decrypt.rs b/tool/src/commands/decrypt.rs
index 6c459967..65b9d579 100644
--- a/tool/src/commands/decrypt.rs
+++ b/tool/src/commands/decrypt.rs
@@ -147,7 +147,7 @@ impl<'a> DecryptionHelper for Helper<'a> {
         for pkesk in pkesks {
             let keyid = pkesk.recipient();
             if let Some(key) = self.secret_keys.get(&keyid) {
-                if key.secret().map(|s| ! s.is_encrypted()).unwrap_or(false) {
+                if ! key.secret().is_encrypted() {
                     if let Ok(fp) = key.clone().into_keypair()
                         .and_then(|mut k|
                                   self.try_decrypt(pkesk, sym_algo, &mut k, &mut decrypt))
@@ -169,8 +169,7 @@ impl<'a> DecryptionHelper for Helper<'a> {
             let keyid = pkesk.recipient();
             if let Some(key) = self.secret_keys.get_mut(&keyid) {
                 let mut keypair = loop {
-                    if key.secret().map(|s| ! s.is_encrypted()).unwrap_or(false)
-                    {
+                    if ! key.secret().is_encrypted() {
                         break key.clone().into_keypair().unwrap();
                     }
 
@@ -181,9 +180,7 @@ impl<'a> DecryptionHelper for Helper<'a> {
 
                     let algo = key.pk_algo();
                     if let Some(()) =
-                        key.secret_mut()
-                        .and_then(|s| s.decrypt_in_place(algo, &p).ok())
-                    {
+                        key.secret_mut().decrypt_in_place(algo, &p).ok() {
                         break key.clone().into_keypair().unwrap()
                     } else {
                         eprintln!("Bad password.");
@@ -202,7 +199,7 @@ impl<'a> DecryptionHelper for Helper<'a> {
         // prompting for a password.
         for pkesk in pkesks.iter().filter(|p| p.recipient().is_wildcard()) {
             for key in self.secret_keys.values() {
-                if key.secret().map(|s| ! s.is_encrypted()).unwrap_or(false) {
+                if ! key.secret().is_encrypted() {
                     if let Ok(fp) = key.clone().into_keypair()
                         .and_then(|mut k|
                                   self.try_decrypt(pkesk, sym_algo, &mut k, &mut decrypt))
@@ -229,8 +226,7 @@ impl<'a> DecryptionHelper for Helper<'a> {
                 let mut keypair = loop {
                     let key = self.secret_keys.get_mut(&keyid).unwrap(); // Yuck
 
-                    if key.secret().map(|s| ! s.is_encrypted()).unwrap_or(false)
-                    {
+                    if ! key.secret().is_encrypted() {
                         break key.clone().into_keypair().unwrap();
                     }
 
@@ -241,9 +237,7 @@ impl<'a> DecryptionHelper for Helper<'a> {
 
                     let algo = key.pk_algo();
                     if let Some(()) =
-                        key.secret_mut()
-                        .and_then(|s| s.decrypt_in_place(algo, &p).ok())
-                    {
+