openpgp: Make conversions into Key<SecretParts, _> fallible.

  - Fixes #380.

13 files changed, 102 insertions, 43 deletions

diff --git a/openpgp/src/crypto/asymmetric.rs b/openpgp/src/crypto/asymmetric.rs
index 144bcc4c..41d61014 100644
--- a/openpgp/src/crypto/asymmetric.rs
+++ b/openpgp/src/crypto/asymmetric.rs
@@ -280,6 +280,6 @@ impl<R> From<KeyPair<R>> for Key<key::SecretParts, R>
     fn from(p: KeyPair<R>) -> Self {
         let (mut key, secret) = (p.public, p.secret);
         key.set_secret(Some(secret.into()));
-        key.mark_parts_secret()
+        key.mark_parts_secret().expect("XXX")
     }
 }
diff --git a/openpgp/src/packet/key/mod.rs b/openpgp/src/packet/key/mod.rs
index b6337ce0..389d16f5 100644
--- a/openpgp/src/packet/key/mod.rs
+++ b/openpgp/src/packet/key/mod.rs
@@ -42,16 +42,18 @@
 //! #         .generate()?;
 //! // Get a handle to the TPK's primary key that allows using the
 //! // secret key material.
-//! let sk : &key::SecretKey = tpk.primary().into();
+//! use std::convert::TryInto;
+//! let sk : &key::SecretKey = tpk.primary().try_into()?;
 //!
 //! // Make the conversion explicit.
-//! let sk : &key::SecretKey = tpk.primary().mark_parts_secret_ref();
+//! let sk : &key::SecretKey = tpk.primary().mark_parts_secret_ref()?;
 //! #     Ok(())
 //! # }
 //! ```
 
 use std::fmt;
 use std::cmp::Ordering;
+use std::convert::TryFrom;
 use std::time;
 
 use crate::Error;
@@ -249,14 +251,43 @@ macro_rules! create_conversions {
             }
         }
 
-        p!(<PublicParts> -> <SecretParts>);
+        // Likewise, but using TryFrom.
+        macro_rules! p_try {
+            ( <$from_parts:ty> -> <$to_parts:ty>) => {
+                impl<R> TryFrom<$Key<$from_parts, R>> for $Key<$to_parts, R>
+                    where R: KeyRole
+                {
+                    type Error = failure::Error;
+                    fn try_from(p: $Key<$from_parts, R>) -> Result<Self> {
+                        p.mark_parts_secret()
+                    }
+                }
+
+                impl<R> TryFrom<&$Key<$from_parts, R>> for &$Key<$to_parts, R>
+                    where R: KeyRole
+                {
+                    type Error = failure::Error;
+                    fn try_from(p: &$Key<$from_parts, R>) -> Result<Self> {
+                        if p.secret().is_some() {
+                            Ok(convert_ref!(p))
+                        } else {
+                            Err(Error::InvalidArgument("No secret key".into())
+                                .into())
+                        }
+                    }
+                }
+            }
+        }
+
+
+        p_try!(<PublicParts> -> <SecretParts>);
         p!(<PublicParts> -> <UnspecifiedParts>);
 
         p!(<SecretParts> -> <PublicParts>);
         p!(<SecretParts> -> <UnspecifiedParts>);
 
         p!(<UnspecifiedParts> -> <PublicParts>);
-        p!(<UnspecifiedParts> -> <SecretParts>);
+        p_try!(<UnspecifiedParts> -> <SecretParts>);
 
         // Convert between two KeyRoles for a constant KeyParts.  See
         // the comment for the p macro above.
@@ -438,13 +469,22 @@ macro_rules! create_conversions {
             }
 
             /// Changes the key's parts tag to `SecretParts`.
-            pub fn mark_parts_secret(self) -> $Key<SecretParts, R> {
-                convert!(self)
+            pub fn mark_parts_secret(self) -> Result<$Key<SecretParts, R>> {
+                if self.secret().is_some() {
+                    Ok(convert!(self))
+                } else {
+                    Err(Error::InvalidArgument("No secret key".into()).into())
+                }
             }
 
             /// Changes the key's parts tag to `SecretParts`.
-            pub fn mark_parts_secret_ref(&self) -> &$Key<SecretParts, R> {
-                convert_ref!(self)
+            pub fn mark_parts_secret_ref(&self) -> Result<&$Key<SecretParts, R>>
+            {
+                if self.secret().is_some() {
+                    Ok(convert_ref!(self))
+                } else {
+                    Err(Error::InvalidArgument("No secret key".into()).into())
+                }
             }
 
             /// Changes the key's parts tag to `UnspecifiedParts`.
@@ -1370,7 +1410,8 @@ mod tests {
         for key in keys.into_iter() {
             let key : key::PublicKey = key.into();
             let mut keypair
-                = key.clone().mark_parts_secret().into_keypair().unwrap();
+                = key.clone().mark_parts_secret().unwrap()
+                .into_keypair().unwrap();
             let cipher = SymmetricAlgorithm::AES256;
             let sk = SessionKey::new(cipher.key_size().unwrap());
 
diff --git a/openpgp/src/packet/pkesk.rs b/openpgp/src/packet/pkesk.rs
index b2e9a0f4..29fd715f 100644
--- a/openpgp/src/packet/pkesk.rs
+++ b/openpgp/src/packet/pkesk.rs
@@ -229,7 +229,7 @@ mod tests {
             crate::tests::message("encrypted-to-testy.gpg")).unwrap();
         let mut keypair =
             tpk.subkeys().next().unwrap()
-            .key().clone().mark_parts_secret().into_keypair().unwrap();
+            .key().clone().mark_parts_secret().unwrap().into_keypair().unwrap();
 
         let pkg = pile.descendants().skip(0).next().clone();
 
@@ -250,7 +250,7 @@ mod tests {
             crate::tests::message("encrypted-to-testy-new.pgp")).unwrap();
         let mut keypair =
             tpk.subkeys().next().unwrap()
-            .key().clone().mark_parts_secret().into_keypair().unwrap();
+            .key().clone().mark_parts_secret().unwrap().into_keypair().unwrap();
 
         let pkg = pile.descendants().skip(0).next().clone();
 
@@ -271,7 +271,7 @@ mod tests {
             crate::tests::message("encrypted-to-testy-nistp256.pgp")).unwrap();
         let mut keypair =
             tpk.subkeys().next().unwrap()
-            .key().clone().mark_parts_secret().into_keypair().unwrap();
+            .key().clone().mark_parts_secret().unwrap().into_keypair().unwrap();
 
         let pkg = pile.descendants().skip(0).next().clone();
 
@@ -292,7 +292,7 @@ mod tests {
             crate::tests::message("encrypted-to-testy-nistp384.pgp")).unwrap();
         let mut keypair =
             tpk.subkeys().next().unwrap()
-            .key().clone().mark_parts_secret().into_keypair().unwrap();
+            .key().clone().mark_parts_secret().unwrap().into_keypair().unwrap();
 
         let pkg = pile.descendants().skip(0).next().clone();
 
@@ -313,7 +313,7 @@ mod tests {
             crate::tests::message("encrypted-to-testy-nistp521.pgp")).unwrap();
         let mut keypair =
             tpk.subkeys().next().unwrap()
-            .key().clone().mark_parts_secret().into_keypair().unwrap();
+            .key().clone().mark_parts_secret().unwrap().into_keypair().unwrap();
 
         let pkg = pile.descendants().skip(0).next().clone();
 
@@ -370,7 +370,8 @@ mod tests {
         let sess_key = SessionKey::new(32);
         let pkesk = PKESK3::for_recipient(SymmetricAlgorithm::AES256, &sess_key,
                                           &key).unwrap();
-        let mut keypair = key.mark_parts_secret().into_keypair().unwrap();
+        let mut keypair =
+            key.mark_parts_secret().unwrap().into_keypair().unwrap();
         pkesk.decrypt(&mut keypair).unwrap();
     }
 }
diff --git a/openpgp/src/packet/signature/mod.rs b/openpgp/src/packet/signature/mod.rs
index ab7762b9..211a4447 100644
--- a/openpgp/src/packet/signature/mod.rs
+++ b/openpgp/src/packet/signature/mod.rs
@@ -1260,7 +1260,7 @@ mod test {
         ] {
             let tpk = TPK::from_bytes(crate::tests::key(key)).unwrap();
             let mut pair = tpk.primary().clone()
-                .mark_parts_secret()
+                .mark_parts_secret().unwrap()
                 .into_keypair()
                 .expect("secret key is encrypted/missing");
 
diff --git a/openpgp/src/packet/signature/subpacket.rs b/openpgp/src/packet/signature/subpacket.rs
index b801eb6c..2a83920b 100644
--- a/openpgp/src/packet/signature/subpacket.rs
+++ b/openpgp/src/packet/signature/subpacket.rs
@@ -2735,7 +2735,8 @@ fn accessors() {
     let mut sig = signature::Builder::new(crate::constants::SignatureType::Binary);
     let mut key: crate::packet::key::PublicKey =
         crate::packet::key::Key4::generate_ecc(true, Curve::Ed25519).unwrap().into();
-    let mut keypair = key.clone().mark_parts_secret().into_keypair().unwrap();
+    let mut keypair = key.clone().mark_parts_secret().unwrap()
+        .into_keypair().unwrap();
 
     // Cook up a timestamp without ns resolution.
     let now = time::SystemTime::now().canonicalize();
diff --git a/openpgp/src/parse/stream.rs b/openpgp/src/parse/stream.rs
index bab21e4c..48c1fd2a 100644
--- a/openpgp/src/parse/stream.rs
+++ b/openpgp/src/parse/stream.rs
@@ -1911,7 +1911,8 @@ mod test {
         {
             let key = tpk.keys_all().signing_capable().nth(0).unwrap().2;
             let keypair =
-                key.clone().mark_parts_secret().into_keypair().unwrap();
+                key.clone().mark_parts_secret().unwrap()
+                .into_keypair().unwrap();
 
             let m = Message::new(&mut buf);
             let signer = Signer::new(m, keypair).build().unwrap();
diff --git a/openpgp/src/serialize/mod.rs b/openpgp/src/serialize/mod.rs
index d37c0c7e..325b181f 100644
--- a/openpgp/src/serialize/mod.rs
+++ b/openpgp/src/serialize/mod.rs
@@ -2938,7 +2938,7 @@ mod test {
 
         let (tpk, _) = TPKBuilder::new().generate().unwrap();
         let mut keypair = tpk.primary().clone().mark_parts_secret()
-            .into_keypair().unwrap();
+            .unwrap().into_keypair().unwrap();
         let uid = UserID::from("foo");
 
         // Make a signature w/o an exportable certification subpacket.
diff --git a/openpgp/src/serialize/stream.rs b/openpgp/src/serialize/stream.rs
index 4b1e0d88..5af424d3 100644
--- a/openpgp/src/serialize/stream.rs
+++ b/openpgp/src/serialize/stream.rs
@@ -230,7 +230,7 @@ impl<'a> Signer<'a> {
     /// #     "../../tests/data/keys/testy-new-private.pgp")[..])
     /// #     .unwrap();
     /// # let keypair = tsk.keys_valid().signing_capable().nth(0).unwrap().2
-    /// #     .clone().mark_parts_secret().into_keypair().unwrap();
+    /// #     .clone().mark_parts_secret().unwrap().into_keypair().unwrap();
     /// # f(tsk, keypair).unwrap();
     /// # fn f(tpk: TPK, mut signing_keypair: KeyPair<key::UnspecifiedRole>)
     /// #      -> Result<()> {
@@ -333,7 +333,7 @@ impl<'a> Signer<'a> {
     /// #     "../../tests/data/keys/testy-new-private.pgp")[..])
     /// #     .unwrap();
     /// # let keypair = tsk.keys_valid().signing_capable().nth(0).unwrap().2
-    /// #     .clone().mark_parts_secret().into_keypair().unwrap();
+    /// #     .clone().mark_parts_secret().unwrap().into_keypair().unwrap();
     /// # f(tsk, keypair).unwrap();
     /// # fn f(tpk: TPK, mut signing_keypair: KeyPair<key::UnspecifiedRole>)
     /// #      -> Result<()> {
@@ -1478,7 +1478,7 @@ mod test {
         let mut o = vec![];
         {
             let mut signers = keys.iter().map(|(_, key)| {
-                key.clone().mark_parts_secret().into_keypair()
+                key.clone().mark_parts_secret().unwrap().into_keypair()
                     .expect("expected unencrypted secret key")
             }).collect::<Vec<KeyPair<_>>>();
 
@@ -1678,7 +1678,8 @@ mod test {
                         KeyFlags::default()
                             .set_encrypt_for_transport(true))
                     .map(|(_, _, key)| key).next().unwrap()
-                    .clone().mark_parts_secret().into_keypair().unwrap();
+                    .clone().mark_parts_secret().unwrap()
+                    .into_keypair().unwrap();
                 pkesks[0].decrypt(&mut keypair)
                     .and_then(|(algo, session_key)| decrypt(algo, &session_key))
                     .map(|_| None)
diff --git a/openpgp/src/serialize/tpk.rs b/openpgp/src/serialize/tpk.rs
index bd655559..b8ecfdc1 100644
--- a/openpgp/src/serialize/tpk.rs
+++ b/openpgp/src/serialize/tpk.rs
@@ -323,7 +323,8 @@ impl<'a> TSK<'a> {
     /// tpk.as_tsk()
     ///     .set_filter(
     ///         |k| k == tpk.primary()
-    ///                 .mark_parts_secret_ref().mark_role_unspecified_ref())
+    ///                  .mark_parts_secret_ref().unwrap()
+    ///                  .mark_role_unspecified_ref())
     ///     .serialize(&mut buf)?;
     ///
     /// let tpk_ = TPK::from_bytes(&buf)?;
@@ -728,7 +729,7 @@ mod test {
 
         let (tpk, _) = TPKBuilder::new().generate().unwrap();
         let mut keypair = tpk.primary().clone().mark_parts_secret()
-            .into_keypair().unwrap();
+            .unwrap().into_keypair().unwrap();
 
         let key: key::SecretSubkey =
             Key4::generate_ecc(false, Curve::Cv25519).unwrap().into();
diff --git a/openpgp/src/tpk/bindings.rs b/openpgp/src/tpk/bindings.rs
index a9606cf1..d3e5ee29 100644
--- a/openpgp/src/tpk/bindings.rs
+++ b/openpgp/src/tpk/bindings.rs
@@ -33,7 +33,7 @@ impl Key<key::PublicParts, key::SubordinateRole> {
     /// // Generate a TPK, and create a keypair from the primary key.
     /// let (tpk, _) = TPKBuilder::new().generate()?;
     /// let mut keypair = tpk.primary().clone()
-    ///     .mark_parts_secret().into_keypair()?;
+    ///     .mark_parts_secret()?.into_keypair()?;
     ///
     /// // Let's add an encryption subkey.
     /// let flags = KeyFlags::default().set_encrypt_at_rest(true);
@@ -96,7 +96,7 @@ impl UserID {
     /// // Generate a TPK, and create a keypair from the primary key.
     /// let (tpk, _) = TPKBuilder::new().generate()?;
     /// let mut keypair = tpk.primary().clone()
-    ///     .mark_parts_secret().into_keypair()?;
+    ///     .mark_parts_secret()?.into_keypair()?;
     /// assert_eq!(tpk.userids().len(), 0);
     ///
     /// // Generate a userid and a binding signature.
@@ -161,7 +161,7 @@ impl UserID {
     ///     .add_userid("alice@example.org")
     ///     .generate()?;
     /// let mut keypair = alice.primary().clone()
-    ///     .mark_parts_secret().into_keypair()?;
+    ///     .mark_parts_secret()?.into_keypair()?;
     ///
     /// // Generate a TPK for Bob.
     /// let (bob, _) = TPKBuilder::new()
@@ -240,7 +240,7 @@ impl UserAttribute {
     /// let (tpk, _) = TPKBuilder::new()
     ///     .generate()?;
     /// let mut keypair = tpk.primary().clone()
-    ///     .mark_parts_secret().into_keypair()?;
+    ///     .mark_parts_secret()?.into_keypair()?;
     /// assert_eq!(tpk.userids().len(), 0);
     ///
     /// // Generate a user attribute and a binding signature.
@@ -307,7 +307,7 @@ impl UserAttribute {
     ///     .add_userid("alice@example.org")
     ///     .generate()?;
     /// let mut keypair = alice.primary().clone()
-    ///     .mark_parts_secret().into_keypair()?;
+    ///     .mark_parts_secret()?.into_keypair()?;
     ///
     /// // Generate a TPK for Bob.
     /// let user_attr = UserAttribute::new(&[
diff --git a/openpgp/src/tpk/builder.rs b/openpgp/src/tpk/builder.rs
index 6649c602..a7b52571 100644
--- a/openpgp/src/tpk/builder.rs
+++ b/openpgp/src/tpk/builder.rs
@@ -306,7 +306,8 @@ impl TPKBuilder {
 
         // Generate & and self-sign primary key.
         let (primary, sig) = self.primary_key()?;
-        let mut signer = primary.clone().mark_parts_secret().into_keypair().unwrap();
+        let mut signer = primary.clone().mark_parts_secret().unwrap()
+            .into_keypair().unwrap();
 
         packets.push(Packet::PublicKey({
             let mut primary = primary.clone();
diff --git a/openpgp/src/tpk/mod.rs b/openpgp/src/tpk/mod.rs
index e4ef8664..4d0c7d0c 100644
--- a/openpgp/src/tpk/mod.rs
+++ b/openpgp/src/tpk/mod.rs
@@ -91,6 +91,18 @@ fn sig_cmp(a: &Signature, b: &Signature) -> Ordering {
 /// signatures.
 pub type KeyBinding<KeyPart, KeyRole> = ComponentBinding<Key<KeyPart, KeyRole>>;
 
+impl<K: key::KeyParts, R: key::KeyRole> KeyBinding<K, R>
+{
+    /// Gets the key packet's `SecretKeyMaterial`.
+    ///
+    /// Note: The key module installs conversion functions on
+    /// KeyBinding.  They need to access the key's secret.
+    pub(crate) fn secret(&self)
+                         -> Option<&crate::packet::key::SecretKeyMaterial> {
+        self.key().secret()
+    }
+}
+
 /// A primary key and any associated signatures.
 pub type PrimaryKeyBinding<KeyPart> = KeyBinding<KeyPart, key::PrimaryRole>;
 
@@ -987,7 +999,7 @@ impl TPK {
     ///            tpk.revoked(None));
     ///
     /// let mut keypair = tpk.primary().clone()
-    ///     .mark_parts_secret().into_keypair()?;
+    ///     .mark_parts_secret()?.into_keypair()?;
     /// let tpk = tpk.revoke_in_place(&mut keypair,
     ///                               ReasonForRevocation::KeyCompromised,
     ///                               b"It was the maid :/")?;
@@ -2076,7 +2088,7 @@ mod test {
             .expect("Keys expire by default.");
 
         let mut keypair = tpk.primary().clone().mark_parts_secret()
-            .into_keypair().unwrap();
+            .unwrap().into_keypair().unwrap();
 
         // Clear the expiration.
         let as_of1 = now + time::Duration::new(10, 0);
@@ -2249,7 +2261,7 @@ mod test {
                    tpk.revoked(None));
 
         let mut keypair = tpk.primary().clone().mark_parts_secret()
-            .into_keypair().unwrap();
+            .unwrap().into_keypair().unwrap();
 
         let sig = TPKRevocationBuilder::new()
             .set_reason_for_revocation(
@@ -2271,7 +2283,7 @@ mod test {
             .generate().unwrap();
 
         let mut keypair = other.primary().clone().mark_parts_secret()
-            .into_keypair().unwrap();
+            .unwrap().into_keypair().unwrap();
 
         let sig = TPKRevocationBuilder::new()
             .set_reason_for_revocation(
@@ -2300,7 +2312,7 @@ mod test {
             assert_eq!(RevocationStatus::NotAsFarAsWeKnow, subkey.revoked(None));
 
             let mut keypair = tpk.primary().clone().mark_parts_secret()
-                .into_keypair().unwrap();
+                .unwrap().into_keypair().unwrap();
             SubkeyRevocationBuilder::new()
                 .set_reason_for_revocation(
                     ReasonForRevocation::UIDRetired,
@@ -2332,7 +2344,7 @@ mod test {
             assert_eq!(RevocationStatus::NotAsFarAsWeKnow, uid.revoked(None));
 
             let mut keypair = tpk.primary().clone().mark_parts_secret()
-                .into_keypair().unwrap();
+                .unwrap().into_keypair().unwrap();
             UserIDRevocationBuilder::new()
                 .set_reason_for_revocation(
                     ReasonForRevocation::UIDRetired,
@@ -3073,7 +3085,7 @@ Pu1xwz57O4zo1VYf6TqHJzVC3OMvMUM2hhdecMUe5x6GorNaj6g=
             let alice_certifies_bob
                 = bob_userid_binding.userid().bind(
                     &mut alice.primary().clone().mark_parts_secret()
-                        .into_keypair().unwrap(),
+                        .unwrap().into_keypair().unwrap(),
                     &bob,
                     sig_template,
                     None).unwrap();
diff --git a/openpgp/src/tpk/revoke.rs b/openpgp/src/tpk/revoke.rs
index dd7264bf..a230d184 100644
--- a/openpgp/src/tpk/revoke.rs
+++ b/openpgp/src/tpk/revoke.rs
@@ -55,7 +55,7 @@ use crate::tpk::TPK;
 ///            tpk.revoked(None));
 ///
 /// let mut signer = tpk.primary().clone()
-///     .mark_parts_secret().into_keypair()?;
+///     .mark_parts_secret()?.into_keypair()?;
 /// let sig = TPKRevocationBuilder::new()
 ///     .set_reason_for_revocation(ReasonForRevocation::KeyCompromised,
 ///                                b"It was the maid :/")?
@@ -163,7 +163,7 @@ impl Deref for TPKRevocationBuilder {
 ///     .add_encryption_subkey()
 ///     .generate()?;
 /// let mut keypair = tpk.primary().clone()
-///     .mark_parts_secret().into_keypair()?;
+///     .mark_parts_secret()?.into_keypair()?;
 /// let subkey = tpk.subkeys().nth(0).unwrap();
 ///
 /// // Generate the revocation for the first and only Subkey.
@@ -279,7 +279,7 @@ impl Deref for SubkeyRevocationBuilder {
 ///     .add_userid("some@example.org")
 ///     .generate()?;
 /// let mut keypair = tpk.primary().clone()
-///     .mark_parts_secret().into_keypair()?;
+///     .mark_parts_secret()?.into_keypair()?;
 /// let userid = tpk.userids().nth(0).unwrap();
 ///
 /// // Generate the revocation for the first and only UserID.
@@ -398,7 +398,7 @@ impl Deref for UserIDRevocationBuilder {
 ///     .add_user_attribute(some_user_attribute)
 ///     .generate()?;
 /// let mut keypair = tpk.primary().clone()
-///     .mark_parts_secret().into_keypair()?;
+///     .mark_parts_secret()?.into_keypair()?;
 /// let ua = tpk.user_attributes().nth(0).unwrap();
 ///
 /// // Generate the revocation for the first and only UserAttribute.