openpgp: Rework unencrypted secret key handling.

  - Instead of giving out references to the stored secret, use a new
    function Unencrypted::map that maps a given function over the
    secret.  This allows us to encrypt the secret, and decrypt it on
    demand.

4 files changed, 30 insertions, 26 deletions

diff --git a/openpgp/src/crypto/asymmetric.rs b/openpgp/src/crypto/asymmetric.rs
index 60b7d6b0..49b95c0b 100644
--- a/openpgp/src/crypto/asymmetric.rs
+++ b/openpgp/src/crypto/asymmetric.rs
@@ -88,8 +88,9 @@ impl Signer for KeyPair {
 
         let mut rng = Yarrow::default();
 
-        #[allow(deprecated)]
-        match (self.public.pk_algo(), self.public.mpis(), &self.secret.mpis())
+        self.secret.map(|secret| {
+            #[allow(deprecated)]
+            match (self.public.pk_algo(), self.public.mpis(), secret)
         {
             (RSASign,
              &PublicKey::RSA { ref e, ref n },
@@ -198,7 +199,7 @@ impl Signer for KeyPair {
                 "unsupported combination of algorithm {:?}, key {:?}, \
                  and secret key {:?}",
                 pk_algo, self.public, self.secret)).into()),
-        }
+        }})
     }
 }
 
@@ -215,7 +216,8 @@ impl Decryptor for KeyPair {
         use crate::crypto::mpis::PublicKey;
         use nettle::rsa;
 
-        Ok(match (self.public.mpis(), &self.secret.mpis(), ciphertext)
+        self.secret.map(
+            |secret| Ok(match (self.public.mpis(), secret, ciphertext)
         {
             (PublicKey::RSA{ ref e, ref n },
              mpis::SecretKey::RSA{ ref p, ref q, ref d, .. },
@@ -237,15 +239,14 @@ impl Decryptor for KeyPair {
             (PublicKey::ECDH{ .. },
              mpis::SecretKey::ECDH { .. },
              mpis::Ciphertext::ECDH { .. }) =>
-                crate::crypto::ecdh::decrypt(&self.public, &self.secret.mpis(),
-                                        ciphertext)?,
+                crate::crypto::ecdh::decrypt(&self.public, secret, ciphertext)?,
 
             (public, secret, ciphertext) =>
                 return Err(Error::InvalidOperation(format!(
                     "unsupported combination of key pair {:?}/{:?} \
                      and ciphertext {:?}",
                     public, secret, ciphertext)).into()),
-        })
+        }))
     }
 }
 
diff --git a/openpgp/src/packet/key.rs b/openpgp/src/packet/key.rs
index 86ff1e30..960c0574 100644
--- a/openpgp/src/packet/key.rs
+++ b/openpgp/src/packet/key.rs
@@ -650,9 +650,11 @@ impl From<mpis::SecretKey> for Unencrypted {
 }
 
 impl Unencrypted {
-    /// Returns a reference to the secret key.
-    pub fn mpis(&self) -> &mpis::SecretKey {
-        &self.mpis
+    /// Maps the given function over the secret.
+    pub fn map<F, T>(&self, mut fun: F) -> T
+        where F: FnMut(&mpis::SecretKey) -> T
+    {
+        fun(&self.mpis)
     }
 
     /// Encrypts this secret key using `password`.
@@ -673,7 +675,7 @@ impl Unencrypted {
         {
             let mut encryptor = Encryptor::new(algo, &key, &mut esk)?;
             encryptor.write_all(&trash)?;
-            self.mpis.serialize_chksumd(&mut encryptor)?;
+            self.map(|mpis| mpis.serialize_chksumd(&mut encryptor))?;
         }
 
         Ok(Encrypted { s2k, algo, ciphertext: esk.into_boxed_slice() })
@@ -759,10 +761,10 @@ mod tests {
         assert!(!secret.is_encrypted());
 
         match secret {
-            SecretKey::Unencrypted(ref u) => match u.mpis() {
+            SecretKey::Unencrypted(ref u) => u.map(|mpis| match mpis {
                 mpis::SecretKey::RSA { .. } => (),
                 _ => panic!(),
-            },
+            }),
             _ => panic!(),
         }
     }
@@ -952,14 +954,14 @@ mod tests {
 
         // Session key
         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 sec = match key.secret() {
-            Some(SecretKey::Unencrypted(ref u)) => u.mpis(),
+
+        let got_dek = match key.secret() {
+            Some(SecretKey::Unencrypted(ref u)) => u.map(|mpis| {
+                ecdh::decrypt(&key, mpis, &ciphertext).unwrap()
+            }),
             _ => unreachable!(),
         };
 
-        // Expected
-        let got_dek = ecdh::decrypt(&key, sec, &ciphertext).unwrap();
-
         assert_eq!(&dek[..], &got_dek[..]);
     }
 
diff --git a/openpgp/src/serialize/mod.rs b/openpgp/src/serialize/mod.rs
index f79e2a26..dd9007a5 100644
--- a/openpgp/src/serialize/mod.rs
+++ b/openpgp/src/serialize/mod.rs
@@ -1210,14 +1210,12 @@ impl Key4 {
 
         if have_secret_key {
             match self.secret().unwrap() {
-                SecretKey::Unencrypted(ref u) => {
-                    let mpis = u.mpis();
-
+                SecretKey::Unencrypted(ref u) => u.map(|mpis| -> Result<()> {
                     // S2K usage.
                     write_byte(o, 0)?;
 
                     // To compute the checksum, serialize to a buffer first.
-                    let mut buf = Vec::new();
+                    let mut buf = Vec::new(); // XXX: Protect this vec.
                     mpis.serialize(&mut buf)?;
                     let checksum: usize = buf.iter().map(|x| *x as usize)
                         .sum();
@@ -1225,7 +1223,8 @@ impl Key4 {
                     // Then, just write out the buffer.
                     o.write_all(&buf)?;
                     write_be_u16(o, checksum as u16)?;
-                },
+                    Ok(())
+                })?,
                 SecretKey::Encrypted(ref e) => {
                     // S2K usage.
                     write_byte(o, 254)?;
@@ -1249,7 +1248,7 @@ impl Key4 {
             + if have_secret_key {
                 1 + match self.secret().as_ref().unwrap() {
                     SecretKey::Unencrypted(ref u) =>
-                        u.mpis().serialized_len()
+                        u.map(|mpis| mpis.serialized_len())
                         + 2, // Two octet checksum.
                     SecretKey::Encrypted(ref e) =>
                         1 + e.s2k().serialized_len() + e.ciphertext().len(),
diff --git a/tool/src/commands/dump.rs b/tool/src/commands/dump.rs
index 4af64e0b..3cdd7c0a 100644
--- a/tool/src/commands/dump.rs
+++ b/tool/src/commands/dump.rs
@@ -409,7 +409,9 @@ impl PacketDumper {
 
                         let ii = format!("{}    ", i);
                         match secrets {
-                            SecretKey::Unencrypted(ref u) => match u.mpis()
+                            SecretKey::Unencrypted(ref u) => u.map(
+                                |mpis| -> Result<()> {
+                                    match mpis
                             {
                                 mpis::SecretKey::RSA { d, p, q, u } =>
                                     self.dump_mpis(output, &ii,
@@ -450,7 +452,7 @@ impl PacketDumper {
                                     self.dump_mpis(output, &ii, &[rest],
                                                    &["rest"])?;
                                 },
-                            },
+                            } Ok(()) })?,
                             SecretKey::Encrypted(ref e) => {
                                 writeln!(output, "{}", i)?;
                                 write!(output, "{}  S2K: ", ii)?;