feat: implement TryFrom<PublicOrSecret> for Signed{Public,Secret}Key

link2xt · dignifiedquire · web-flow · commit f9e76946789a · 2024-12-17T19:35:26.000+01:00
This is less dangerous than checking
`is_public()` or `is_secret()`
and then using `into_secret()` and `into_public()`
that may panic.

Co-authored-by: Friedel Ziegelmayer &lt;me@dignifiedquire.com&gt;
diff --git a/src/composed/signed_key/shared.rs b/src/composed/signed_key/shared.rs
@@ -1,3 +1,4 @@
+use std::fmt;
 use std::io;
 
 use chrono::Duration;
@@ -230,20 +231,22 @@ impl PublicOrSecret {
         }
     }
 
+    /// Returns secret key.
+    ///
     /// Panics if not a secret key.
+    #[deprecated(note = "Can panic. Users should use TryFrom trait instead.")]
     pub fn into_secret(self) -> SignedSecretKey {
-        match self {
-            PublicOrSecret::Public(_) => panic!("Can not convert a public into a secret key"),
-            PublicOrSecret::Secret(k) => k,
-        }
+        self.try_into()
+            .expect("Can not convert a public into a secret key")
     }
 
+    /// Returns public key.
+    ///
     /// Panics if not a public key.
+    #[deprecated(note = "Can panic. Users should use TryFrom trait instead.")]
     pub fn into_public(self) -> SignedPublicKey {
-        match self {
-            PublicOrSecret::Secret(_) => panic!("Can not convert a secret into a public key"),
-            PublicOrSecret::Public(k) => k,
-        }
+        self.try_into()
+            .expect("Can not convert a secret into a public key")
     }
 
     pub fn is_public(&self) -> bool {
@@ -261,6 +264,39 @@ impl PublicOrSecret {
     }
 }
 
+/// Error returned when trying to convert `PublicOrSecret` key
+/// into the wrong type.
+#[derive(Debug, Copy, Clone, PartialEq, Eq, thiserror::Error)]
+pub struct TryFromPublicOrSecretError;
+
+impl fmt::Display for TryFromPublicOrSecretError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "Attempt to convert PublicOrSecret key to the wrong type")
+    }
+}
+
+impl TryFrom<PublicOrSecret> for SignedPublicKey {
+    type Error = TryFromPublicOrSecretError;
+
+    fn try_from(public_or_secret: PublicOrSecret) -> Result<Self, Self::Error> {
+        match public_or_secret {
+            PublicOrSecret::Public(k) => Ok(k),
+            PublicOrSecret::Secret(_) => Err(TryFromPublicOrSecretError),
+        }
+    }
+}
+
+impl TryFrom<PublicOrSecret> for SignedSecretKey {
+    type Error = TryFromPublicOrSecretError;
+
+    fn try_from(public_or_secret: PublicOrSecret) -> Result<Self, Self::Error> {
+        match public_or_secret {
+            PublicOrSecret::Public(_) => Err(TryFromPublicOrSecretError),
+            PublicOrSecret::Secret(k) => Ok(k),
+        }
+    }
+}
+
 impl Serialize for PublicOrSecret {
     fn to_writer<W: io::Write>(&self, writer: &mut W) -> Result<()> {
         match self {
diff --git a/tests/hsm-test.rs b/tests/hsm-test.rs
@@ -11,7 +11,7 @@ use pgp::packet::{PacketTrait, PublicKey, SignatureConfig};
 use pgp::types::{EcdhPublicParams, EskType, Fingerprint, PkeskBytes, SignatureBytes};
 use pgp::types::{KeyId, Mpi, PublicKeyTrait, PublicParams, SecretKeyTrait};
 use pgp::{packet, Deserializable, Esk};
-use pgp::{Message, SignedPublicKey};
+use pgp::{Message, SignedPublicKey, SignedSecretKey};
 use rand::{CryptoRng, Rng};
 
 #[derive(Debug, Clone)]
@@ -344,7 +344,7 @@ fn card_decrypt() {
 
         let key_file = File::open(keyfile).unwrap();
         let (mut x, _) = pgp::composed::signed_key::from_reader_many(key_file).unwrap();
-        let key = x.next().unwrap().unwrap().into_secret();
+        let key: SignedSecretKey = x.next().unwrap().unwrap().try_into().unwrap();
 
         let pubkey: SignedPublicKey = key.into();
         let enc_subkey = &pubkey.public_subkeys.first().unwrap().key;
@@ -470,7 +470,7 @@ fn card_sign() {
 
         let key_file = File::open(keyfile).unwrap();
         let (mut x, _) = pgp::composed::signed_key::from_reader_many(key_file).unwrap();
-        let key = x.next().unwrap().unwrap().into_secret();
+        let key: SignedSecretKey = x.next().unwrap().unwrap().try_into().unwrap();
 
         let pubkey: SignedPublicKey = key.into();
 
diff --git a/tests/key_test.rs b/tests/key_test.rs
@@ -1196,7 +1196,7 @@ fn test_parse_autocrypt_key(key: &str, unlock: bool) {
         parsed.verify().expect("invalid key");
 
         if unlock {
-            let sk = parsed.clone().into_secret();
+            let sk: SignedSecretKey = parsed.clone().try_into().unwrap();
             sk.unlock(|| "".to_string(), |_| Ok(()))
                 .expect("failed to unlock key");
 
diff --git a/tests/rfc9580.rs b/tests/rfc9580.rs
@@ -35,7 +35,7 @@ fn load_ssk(filename: &str) -> SignedSecretKey {
         pgp::composed::signed_key::from_reader_many(File::open(filename).unwrap()).expect("ok");
     let pos = iter.next().expect("some").expect("ok");
 
-    pos.into_secret()
+    pos.try_into().unwrap()
 }
 
 fn try_decrypt(keyfile: &str, msg_file: &str) {

Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ fn load_ssk(filename: &str) -> SignedSecretKey {`
`35`	`35`	`pgp::composed::signed_key::from_reader_many(File::open(filename).unwrap()).expect("ok");`
`36`	`36`	`let pos = iter.next().expect("some").expect("ok");`
`37`	`37`
`38`		`- pos.into_secret()`
	`38`	`+ pos.try_into().unwrap()`
`39`	`39`	`}`
`40`	`40`
`41`	`41`	`fn try_decrypt(keyfile: &str, msg_file: &str) {`