openpgp: Rework secret key handling.

  - Introduce two new types, `Encrypted` and `Unencrypted`, to make
    the fields of enum `SecretKey` private.  Add accessors, implement
    From<..> to make the new types ergonomic to use, update callsites.

11 files changed, 199 insertions, 174 deletions

diff --git a/openpgp-ffi/include/sequoia/openpgp/crypto.h b/openpgp-ffi/include/sequoia/openpgp/crypto.h
index 44e035dc..9a3c3964 100644
--- a/openpgp-ffi/include/sequoia/openpgp/crypto.h
+++ b/openpgp-ffi/include/sequoia/openpgp/crypto.h
@@ -61,7 +61,7 @@ pgp_password_t pgp_password_clone (pgp_password_t password);
 /*/
 int pgp_password_equal (const pgp_password_t a, const pgp_password_t b);
 
-typedef struct pgp_mpis_secret_key *pgp_mpis_secret_key_t;
+typedef struct pgp_key_unencrypted *pgp_key_unencrypted_t;
 
 /*/
 /// Creates a signature.
@@ -89,7 +89,7 @@ typedef struct pgp_key_pair *pgp_key_pair_t;
 /*/
 /// Creates a new key pair.
 /*/
-void pgp_key_pair_new (pgp_key_t pub, pgp_mpis_secret_key_t secret);
+void pgp_key_pair_new (pgp_key_t pub, pgp_key_unencrypted_t secret);
 
 /*/
 /// Frees a key pair.
diff --git a/openpgp-ffi/src/crypto.rs b/openpgp-ffi/src/crypto.rs
index 2e3ef661..94e1a81d 100644
--- a/openpgp-ffi/src/crypto.rs
+++ b/openpgp-ffi/src/crypto.rs
@@ -67,7 +67,8 @@ pub extern "C" fn pgp_signer_free
 /// Creates a new key pair.
 #[::sequoia_ffi_macros::extern_fn] #[no_mangle]
 pub extern "C" fn pgp_key_pair_new
-    (errp: Option<&mut *mut ::error::Error>, public: *mut Key, secret: *mut crypto::mpis::SecretKey)
+    (errp: Option<&mut *mut ::error::Error>, public: *mut Key,
+     secret: *mut openpgp::packet::key::Unencrypted)
      -> *mut crypto::KeyPair
 {
     ffi_make_fry_from_errp!(errp);
diff --git a/openpgp/src/crypto/asymmetric.rs b/openpgp/src/crypto/asymmetric.rs
index 5cb0221d..839f0eaa 100644
--- a/openpgp/src/crypto/asymmetric.rs
+++ b/openpgp/src/crypto/asymmetric.rs
@@ -51,12 +51,12 @@ pub trait Decryptor {
 #[derive(Clone)]
 pub struct KeyPair {
     public: Key,
-    secret: mpis::SecretKey,
+    secret: packet::key::Unencrypted,
 }
 
 impl KeyPair {
     /// Creates a new key pair.
-    pub fn new(public: Key, secret: mpis::SecretKey) -> Result<Self> {
+    pub fn new(public: Key, secret: packet::key::Unencrypted) -> Result<Self> {
         Ok(Self {
             public: public,
             secret: secret,
@@ -69,7 +69,7 @@ impl KeyPair {
     }
 
     /// Returns a reference to the secret key.
-    pub fn secret(&self) -> &mpis::SecretKey {
+    pub fn secret(&self) -> &packet::key::Unencrypted {
         &self.secret
     }
 }
@@ -89,7 +89,7 @@ impl Signer for KeyPair {
         let mut rng = Yarrow::default();
 
         #[allow(deprecated)]
-        match (self.public.pk_algo(), self.public.mpis(), &self.secret)
+        match (self.public.pk_algo(), self.public.mpis(), &self.secret.mpis())
         {
             (RSASign,
              &PublicKey::RSA { ref e, ref n },
@@ -215,7 +215,7 @@ impl Decryptor for KeyPair {
         use crypto::mpis::PublicKey;
         use nettle::rsa;
 
-        Ok(match (self.public.mpis(), &self.secret, ciphertext)
+        Ok(match (self.public.mpis(), &self.secret.mpis(), ciphertext)
         {
             (PublicKey::RSA{ ref e, ref n },
              mpis::SecretKey::RSA{ ref p, ref q, ref d, .. },
@@ -237,7 +237,7 @@ impl Decryptor for KeyPair {
             (PublicKey::ECDH{ .. },
              mpis::SecretKey::ECDH { .. },
              mpis::Ciphertext::ECDH { .. }) =>
-                ::crypto::ecdh::decrypt(&self.public, &self.secret,
+                ::crypto::ecdh::decrypt(&self.public, &self.secret.mpis(),
                                         ciphertext)?,
 
             (public, secret, ciphertext) =>
@@ -252,9 +252,7 @@ impl Decryptor for KeyPair {
 impl From<KeyPair> for packet::Key {
     fn from(p: KeyPair) -> Self {
         let (mut key, secret) = (p.public, p.secret);
-        key.set_secret(Some(packet::key::SecretKey::Unencrypted {
-            mpis: secret,
-        }));
+        key.set_secret(Some(secret.into()));
         key
     }
 
diff --git a/openpgp/src/packet/key.rs b/openpgp/src/packet/key.rs
index 9cac1012..bf687b81 100644
--- a/openpgp/src/packet/key.rs
+++ b/openpgp/src/packet/key.rs
@@ -158,11 +158,9 @@ impl Key4 {
                 sym: sym.into().unwrap_or(SymmetricAlgorithm::AES256),
                 q: mpis::MPI::new(&public_key),
             },
-            secret: Some(SecretKey::Unencrypted{
-                mpis: mpis::SecretKey::ECDH{
-                    scalar: mpis::MPI::new(&private_key)
-                }
-            }),
+            secret: Some(mpis::SecretKey::ECDH {
+                scalar: mpis::MPI::new(&private_key)
+            }.into()),
         })
     }
 
@@ -212,11 +210,9 @@ impl Key4 {
                 curve: Curve::Ed25519,
                 q: mpis::MPI::new(&public_key),
             },
-            secret: Some(SecretKey::Unencrypted{
-                mpis: mpis::SecretKey::EdDSA{
-                    scalar: mpis::MPI::new(&private_key)
-                }
-            }),
+            secret: Some(mpis::SecretKey::EdDSA {
+                scalar: mpis::MPI::new(&private_key)
+            }.into()),
         })
     }
 
@@ -262,14 +258,12 @@ impl Key4 {
                 e: mpis::MPI::new(&key.e()[..]),
                 n: mpis::MPI::new(&key.n()[..]),
             },
-            secret: Some(SecretKey::Unencrypted{
-                mpis: mpis::SecretKey::RSA{
+            secret: Some(mpis::SecretKey::RSA {
                     d: mpis::MPI::new(d),
                     p: mpis::MPI::new(&a[..]),
                     q: mpis::MPI::new(&b[..]),
                     u: mpis::MPI::new(&c[..]),
-                }
-            })
+                }.into()),
         })
     }
 
@@ -291,9 +285,7 @@ impl Key4 {
             q: MPI::new(&*q),
             u: MPI::new(&*u),
         };
-        let sec = Some(SecretKey::Unencrypted{
-            mpis: private_mpis
-        });
+        let sec = Some(private_mpis.into());
 
         Ok(Key4 {
             common: Default::default(),
@@ -340,9 +332,7 @@ impl Key4 {
                 let private_mpis = mpis::SecretKey::EdDSA {
                     scalar: MPI::new(&private),
                 };
-                let sec = Some(SecretKey::Unencrypted{
-                    mpis: private_mpis,
-                });
+                let sec = Some(private_mpis.into());
 
                 (public_mpis, sec, EdDSA)
             }
@@ -368,9 +358,7 @@ impl Key4 {
                 let private_mpis = mpis::SecretKey::ECDH {
                     scalar: MPI::new(&private),
                 };
-                let sec = Some(SecretKey::Unencrypted{
-                    mpis: private_mpis,
-                });
+                let sec = Some(private_mpis.into());
 
                 (public_mpis, sec, ECDH)
             }
@@ -403,9 +391,7 @@ impl Key4 {
                 let private_mpis = mpis::SecretKey::ECDSA{
                     scalar: MPI::new(&private.as_bytes()),
                 };
-                let sec = Some(SecretKey::Unencrypted{
-                    mpis:  private_mpis
-                });
+                let sec = Some(private_mpis.into());
 
                 (public_mpis, sec, ECDSA)
             }
@@ -444,9 +430,7 @@ impl Key4 {
                     let private_mpis = mpis::SecretKey::ECDH{
                         scalar: MPI::new(&private.as_bytes()),
                     };
-                    let sec = Some(SecretKey::Unencrypted{
-                        mpis:  private_mpis
-                    });
+                    let sec = Some(private_mpis.into());
 
                     (public_mpis, sec, ECDH)
                 }
@@ -560,8 +544,8 @@ impl Key4 {
     pub fn into_keypair(mut self) -> Result<KeyPair> {
         use packet::key::SecretKey;
         let secret = match self.set_secret(None) {
-            Some(SecretKey::Unencrypted { mpis }) => mpis,
-            Some(SecretKey::Encrypted { .. }) =>
+            Some(SecretKey::Unencrypted(secret)) => secret,
+            Some(SecretKey::Encrypted(_)) =>
                 return Err(Error::InvalidArgument(
                     "secret key is encrypted".into()).into()),
             None =>
@@ -585,47 +569,30 @@ impl From<Key4> for super::Key {
 #[derive(PartialEq, Eq, Hash, Clone, Debug)]
 pub enum SecretKey {
     /// Unencrypted secret key. Can be used as-is.
-    Unencrypted {
-        /// MPIs of the secret key.
-        mpis: mpis::SecretKey,
-    },
+    Unencrypted(Unencrypted),
     /// The secret key is encrypted with a password.
-    Encrypted {
-        /// Key derivation mechanism to use.
-        s2k: S2K,
-        /// Symmetric algorithm used for encryption the secret key.
-        algorithm: SymmetricAlgorithm,
-        /// Encrypted MPIs prefixed with the IV.
-        ciphertext: Box<[u8]>,
-    },
+    Encrypted(Encrypted),
 }
 
-impl SecretKey {
-    /// Decrypts this secret key using `password`.
-    ///
-    /// The SecretKey type does not know what kind of key it is, so
-    /// `pk_algo` is needed to parse the correct number of MPIs.
-    pub fn decrypt(&self, pk_algo: PublicKeyAlgorithm, password: &Password)
-                   -> Result<mpis::SecretKey> {
-        use std::io::{Cursor, Read};
-        use crypto::symmetric::Decryptor;
+impl From<mpis::SecretKey> for SecretKey {
+    fn from(mpis: mpis::SecretKey) -> Self {
+        SecretKey::Unencrypted(mpis.into())
+    }
+}
 
-        match self {
-            &SecretKey::Unencrypted { .. } =>
-                Err(Error::InvalidOperation("Key is not encrypted".into())
-                    .into()),
-            &SecretKey::Encrypted { ref s2k, algorithm, ref ciphertext } => {
-                let key = s2k.derive_key(password, algorithm.key_size()?)?;
-                let mut cur = Cursor::new(ciphertext);
-                let mut dec = Decryptor::new(algorithm, &key, cur)?;
-                let mut trash = vec![0u8; algorithm.block_size()?];
-
-                dec.read_exact(&mut trash)?;
-                mpis::SecretKey::parse_chksumd(pk_algo, &mut dec)
-            }
-        }
+impl From<Unencrypted> for SecretKey {
+    fn from(key: Unencrypted) -> Self {
+        SecretKey::Unencrypted(key)
     }
+}
 
+impl From<Encrypted> for SecretKey {
+    fn from(key: Encrypted) -> Self {
+        SecretKey::Encrypted(key)
+    }
+}
+
+impl SecretKey {
     /// Decrypts this secret key using `password`.
     ///
     /// The SecretKey type does not know what kind of key it is, so
@@ -633,65 +600,140 @@ impl SecretKey {
     pub fn decrypt_in_place(&mut self, pk_algo: PublicKeyAlgorithm,
                             password: &Password)
                             -> Result<()> {
-        if self.is_encrypted() {
-            *self = SecretKey::Unencrypted {
-                mpis: self.decrypt(pk_algo, password)?,
-            };
+        let new = match self {
+            SecretKey::Encrypted(ref e) =>
+                Some(e.decrypt(pk_algo, password)?.into()),
+            SecretKey::Unencrypted(_) => None,
+        };
+
+        if let Some(v) = new {
+            *self = v;
+        }
+
+        Ok(())
+    }
+
+    /// Encrypts this secret key using `password`.
+    pub fn encrypt_in_place(&mut self, password: &Password) -> Result<()> {
+        let new = match self {
+            SecretKey::Unencrypted(ref u) =>
+                Some(u.encrypt(password)?.into()),
+            SecretKey::Encrypted(_) => None,
+        };
+
+        if let Some(v) = new {
+            *self = v;
         }
 
         Ok(())
     }
 
+    /// Returns true if this secret key is encrypted.
+    pub fn is_encrypted(&self) -> bool {
+        match self {
+            SecretKey::Encrypted(_) => true,
+            SecretKey::Unencrypted(_) => false,
+        }
+    }
+}
+
+/// Unencrypted secret key. Can be used as-is.
+#[derive(PartialEq, Eq, Hash, Clone, Debug)]
+pub struct Unencrypted {
+    /// MPIs of the secret key.
+    mpis: mpis::SecretKey,
+}
+
+impl From<mpis::SecretKey> for Unencrypted {
+    fn from(mpis: mpis::SecretKey) -> Self {
+        Unencrypted { mpis }
+    }
+}
+
+impl Unencrypted {
+    /// Returns a reference to the secret key.
+    pub fn mpis(&self) -> &mpis::SecretKey {
+        &self.mpis
+    }
+
     /// Encrypts this secret key using `password`.
     pub fn encrypt(&self, password: &Password)
-                   -> Result<(S2K, SymmetricAlgorithm, Box<[u8]>)> {
+                   -> Result<Encrypted> {
         use std::io::Write;
         use crypto::symmetric::Encryptor;
         use nettle::{Random, Yarrow};
 
-        match self {
-            &SecretKey::Encrypted { .. } =>
-                Err(Error::InvalidOperation("Key is already encrypted".into())
-                    .into()),
-            &SecretKey::Unencrypted { ref mpis } => {
-                let s2k = S2K::default();
-                let cipher = SymmetricAlgorithm::AES256;
-                let key = s2k.derive_key(password, cipher.key_size()?)?;
-
-                // Ciphertext is preceded by a random block.
-                let mut trash = vec![0u8; cipher.block_size()?];
-                Yarrow::default().random(&mut trash);
-
-                let mut esk = Vec::new();
-                {
-                    let mut encryptor = Encryptor::new(cipher, &key, &mut esk)?;
-                    encryptor.write_all(&trash)?;
-                    mpis.serialize_chksumd(&mut encryptor)?;
-                }
+        let s2k = S2K::default();
+        let algo = SymmetricAlgorithm::AES256;
+        let key = s2k.derive_key(password, algo.key_size()?)?;
 
-                Ok((s2k, cipher, esk.into_boxed_slice()))
-            },
+        // Ciphertext is preceded by a random block.
+        let mut trash = vec![0u8; algo.block_size()?];
+        Yarrow::default().random(&mut trash);
+
+        let mut esk = Vec::new();
+        {
+            let mut encryptor = Encryptor::new(algo, &key, &mut esk)?;
+            encryptor.write_all(&trash)?;
+            self.mpis.serialize_chksumd(&mut encryptor)?;
         }
+
+        Ok(Encrypted { s2k, algo, ciphertext: esk.into_boxed_slice() })
     }
+}
 
-    /// Encrypts this secret key using `password`.
-    pub fn encrypt_in_place(&mut self, password: &Password) -> Result<()> {
-        let (s2k, cipher, esk) = self.encrypt(password)?;
-        *self = SecretKey::Encrypted {
-            s2k: s2k,
-            algorithm: cipher,
-            ciphertext: esk,
-        };
+/// The secret key is encrypted with a password.
+#[derive(PartialEq, Eq, Hash, Clone, Debug)]
+pub struct Encrypted {
+    /// Key derivation mechanism to use.
+    s2k: S2K,
+    /// Symmetric algorithm used for encryption the secret key.
+    algo: SymmetricAlgorithm,
+    /// Encrypted MPIs prefixed with the IV.
+    ciphertext: Box<[u8]>,
+}
 
-        Ok(())
+impl Encrypted {
+    /// Creates a new encrypted key object.
+    pub fn new(s2k: S2K, algo: SymmetricAlgorithm, ciphertext: Box<[u8]>)
+               -> Self {
+        Encrypted { s2k, algo, ciphertext }
     }
 
-    /// Returns true if this secret key is encrypted.
-    pub fn is_encrypted(&self) -> bool {
-        match self {
-            &SecretKey::Encrypted { .. } => true,
-            &SecretKey::Unencrypted { .. } => false,
-        }
+    /// Returns the key derivation mechanism.
+    pub fn s2k(&self) -> &S2K {
+        &self.s2k
+    }
+
+    /// Returns the symmetric algorithm used for encryption the secret
+    /// key.
+    pub fn algo(&self) -> SymmetricAlgorithm {
+        self.algo
+    }
+
+    /// Returns the key derivation mechanism.
+    pub fn ciphertext(&self) -> &[u8] {
+        &self.ciphertext
+    }
+
+    /// Decrypts this secret key using `password`.
+    ///
+    /// The `Encrypted` key does not know what kind of key it is, so
+    /// `pk_algo` is needed to parse the correct number of MPIs.
+    pub fn decrypt(&self, pk_algo: PublicKeyAlgorithm, password: &Password)
+                   -> Result<Unencrypted> {
+        use std::io::{Cursor, Read};
+        use crypto::symmetric::Decryptor;
+
+        let key = self.s2k.derive_key(password, self.algo.key_size()?)?;
+        let cur = Cursor::new(&self.ciphertext);
+        let mut dec = Decryptor::new(self.algo, &key, cur)?;
+
+        // Consume the first block.
+        let mut trash = vec![0u8; self.algo.block_size()?];
+        dec.read_exact(&mut trash)?;
+
+        mpis::SecretKey::parse_chksumd(pk_algo, &mut dec).map(|m| m.into())
     }
 }
 
@@ -719,9 +761,11 @@ mod tests {
         assert!(!secret.is_encrypted());
 
         match secret {
-            &mut SecretKey::Unencrypted { mpis: mpis::SecretKey::RSA { .. } } =>
-                {}
-            _ => { unreachable!() }
+            SecretKey::Unencrypted(ref u) => match u.mpis() {
+                mpis::SecretKey::RSA { .. } => (),
+                _ => panic!(),
+            },
+            _ => panic!(),
         }
     }
 
@@ -910,11 +954,11 @@ 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(),
+            _ => unreachable!(),
+        };
 
-            let sec = match key.secret() {
-                Some(SecretKey::Unencrypted{ ref mpis }) => mpis,
-                _ => unreachable!(),
-            };
         // Expected
         let got_dek = ecdh::decrypt(&key, sec, &ciphertext).unwrap();
 
diff --git a/openpgp/src/packet/pkesk.rs b/openpgp/src/packet/pkesk.rs
index 3a5188a1..1088d848 100644
--- a/openpgp/src/packet/pkesk.rs
+++ b/openpgp/src/packet/pkesk.rs
@@ -204,7 +204,6 @@ mod tests {
     use super::*;
     use TPK;
     use PacketPile;
-    use packet::key::SecretKey;
     use Packet;
     use parse::Parse;
     use serialize::SerializeInto;
@@ -362,9 +361,7 @@ mod tests {
                                      PublicKeyAlgorithm::ECDH,
                                      public_mpis, None)
             .unwrap().into();
-        key.set_secret(Some(SecretKey::Unencrypted {
-            mpis: private_mpis,
-        }));
+        key.set_secret(Some(private_mpis.into()));
         let mut rng = Yarrow::default();
         let sess_key = SessionKey::new(&mut rng, 32);
         let pkesk = PKESK3::for_recipient(SymmetricAlgorithm::AES256, &sess_key,
diff --git a/openpgp/src/packet/signature/mod.rs b/openpgp/src/packet/signature/mod.rs
index c67b57c7..c08046f4 100644
--- a/openpgp/src/packet/signature/mod.rs
+++ b/openpgp/src/packet/signature/mod.rs
@@ -1150,7 +1150,6 @@ mod test {
 
     #[test]
     fn sign_with_short_ed25519_secret_key() {
-        use packet::key::SecretKey;
         use conversions::Time;
         use nettle;
         use time;
@@ -1174,9 +1173,7 @@ mod test {
         };
         let key = Key4::new(time::now().canonicalize(),
                             PublicKeyAlgorithm::EdDSA,
-                            public_mpis, Some(SecretKey::Unencrypted {
-                                mpis: private_mpis,
-                            }))
+                            public_mpis, Some(private_mpis.into()))
             .unwrap();
         let mut pair = key.into_keypair().unwrap();
         let msg = b"Hello, World";
diff --git a/openpgp/src/parse/parse.rs b/openpgp/src/parse/parse.rs
index 9d05bc66..8df103e8 100644
--- a/openpgp/src/parse/parse.rs
+++ b/openpgp/src/parse/parse.rs
@@ -26,7 +26,6 @@ use {
     packet::prelude::*,
     Packet,
     KeyID,
-    packet::key::SecretKey,
     crypto::SessionKey,
 };
 use constants::{
@@ -1354,7 +1353,7 @@ impl Key4 {
                         return php.fail("wrong secret key checksum");
                     }
 
-                    SecretKey::Unencrypted{ mpis: sec }
+                    sec.into()
                 }
                 // Encrypted & MD5 for key derivation: unsupported
                 1...253 => {
@@ -1366,11 +1365,8 @@ impl Key4 {
                     let s2k = php_try!(S2K::parse(&mut php));
                     let mut cipher = php_try!(php.parse_bytes_eof("encrypted_mpis"));
 
-                    SecretKey::Encrypted{
-                        s2k: s2k,
-                        algorithm: sk,
-                        ciphertext: cipher.into_boxed_slice(),
-                    }
+                    ::packet::key::Encrypted::new(
+                        s2k, sk, cipher.into_boxed_slice()).into()
                 }
                 // Encrypted, S2K & mod 65536 checksum: unsupported
                 255 => {
diff --git a/openpgp/src/serialize/mod.rs b/openpgp/src/serialize/mod.rs
index 5a7995f3..66a2017d 100644
--- a/openpgp/src/serialize/mod.rs
+++ b/openpgp/src/serialize/mod.rs
@@ -1195,7 +1195,9 @@ impl Key4 {
 
         if have_secret_key {
             match self.secret().unwrap() {
-                &SecretKey::Unencrypted { ref mpis } => {
+                SecretKey::Unencrypted(ref u) => {
+                    let mpis = u.mpis();
+
                     // S2K usage.
                     write_byte(o, 0)?;
 
@@ -1209,16 +1211,12 @@ impl Key4 {
                     o.write_all(&buf)?;
                     write_be_u16(o, checksum as u16)?;
                 },
-                &SecretKey::Encrypted {
-                    ref s2k,
-                    algorithm,
-                    ref ciphertext,
-                } => {
+                SecretKey::Encrypted(ref e) => {
                     // S2K usage.
                     write_byte(o, 254)?;
-                    write_byte(o, algorithm.into())?;
-                    s2k.serialize(o)?;
-                    o.write_all(ciphertext)?;
+                    write_byte(o, e.algo().into())?;
+                    e.s2k().serialize(o)?;
+                    o.write_all(e.ciphertext())?;
                 },
             }
         }
@@ -1235,14 +1233,11 @@ impl Key4 {
             + self.mpis().serialized_len()
             + if have_secret_key {
                 1 + match self.secret().as_ref().unwrap() {
-                    &SecretKey::Unencrypted { ref mpis } =>
-                        mpis.serialized_len()
+                    SecretKey::Unencrypted(ref u) =>
+                        u.mpis().serialized_len()
                         + 2, // Two octet checksum.
-                    &SecretKey::Encrypted {
-                        ref s2k,
-                        ref ciphertext,
-                        ..
-                    } => 1 + s2k.serialized_len() + ciphertext.len(),
+                    SecretKey::Encrypted(ref e) =>
+                        1 + e.s2k().serialized_len() + e.ciphertext().len(),
                 }
             } else {
                 0
diff --git a/tool/src/commands/dump.rs b/tool/src/commands/dump.rs
index 39b37cee..57ee8deb 100644
--- a/tool/src/commands/dump.rs
+++ b/tool/src/commands/dump.rs
@@ -364,15 +364,15 @@ impl PacketDumper {
                     }
 
                     if let Some(secrets) = k.secret() {
+                        use openpgp::packet::key::SecretKey;
                         use openpgp::crypto::mpis::SecretKey::*;
                         writeln!(output, "{}", i)?;
                         writeln!(output, "{}  Secret Key:", i)?;
 
                         let ii = format!("{}    ", i);
                         match secrets {
-                            openpgp::packet::key::SecretKey::Unencrypted {
-                                mpis,
-                            } => match mpis {
+                            SecretKey::Unencrypted(ref u) => match u.mpis()
+                            {
                                 RSA { d, p, q, u } =>
                                     self.dump_mpis(output, &ii,
                                                    &[&d.value, &p.value, &q.value,
@@ -410,15 +410,13 @@ impl PacketDumper {
                                                    &["rest"])?;
                                 },
                             },
-                            openpgp::packet::key::SecretKey::Encrypted {
-                                s2k, algorithm, ciphertext,
-                            } => {
+                            SecretKey::Encrypted(ref e) => {
                                 writeln!(output, "{}", i)?;
                                 write!(output, "{}  S2K: ", ii)?;
-                                self.dump_s2k(output, &ii, s2k)?;
+                                self.dump_s2k(output, &ii, e.s2k())?;
                                 writeln!(output, "{}  Sym. algo: {}", ii,
-                                         algorithm)?;
-                                self.dump_mpis(output, &ii, &[&ciphertext[..]],
+                                         e.algo())?;
+                                self.dump_mpis(output, &ii, &[e.ciphertext()],
                                                &["ciphertext"])?;
                             },
                         }
diff --git a/tool/src/commands/mod.rs b/tool/src/commands/mod.rs
index 0f729968..298295f2 100644
--- a/