Merge pull request #63 from wiktor-k/wiktor/use-secrecy

Use `secrecy` crate to protect PINs from accidental leaks

Author: wiktor-k

Cargo.toml

@@ -32,6 +32,7 @@ thiserror = "1.0.58"
 #uuid = { version = "1.8.0", features = ["v4"] }
 subtle = { version = "2", default-features = false }
 signature = { version = "2.2.0", features = ["alloc"] }
+secrecy = "0.8.0"
 
 [features]
 default = ["agent"]

examples/openpgp-card-agent.rs

@@ -102,15 +102,15 @@ impl CardSession {
                         let mut tx = card.transaction()?;
                         let ident = tx.application_identifier()?.ident();
                         if ident == key.id {
-                            tx.verify_pw1_user(key.pin.as_bytes())?;
+                            tx.verify_pw1_user(key.pin.expose_secret().as_bytes())?;
                             Ok::<_, Box<dyn std::error::Error>>(true)
                         } else {
                             Ok(false)
                         }
                     })
                     .any(|x| x);
                 if card_pin_matches {
-                    self.pwds.insert(key.id, key.pin.into(), expiration).await;
+                    self.pwds.insert(key.id, key.pin, expiration).await;
                     Ok(())
                 } else {
                     Err(AgentError::IO(std::io::Error::other(

src/proto/message/add_remove.rs

@@ -5,6 +5,8 @@ mod credential;
 
 pub use constrained::*;
 pub use credential::*;
+use secrecy::ExposeSecret as _;
+use secrecy::SecretString;
 use ssh_encoding::{self, CheckedSum, Decode, Encode, Reader, Writer};
 use ssh_key::public::KeyData;
 
@@ -46,7 +48,7 @@ impl Encode for AddIdentity {
 /// This structure is sent in a [`Request::AddSmartcardKey`](super::Request::AddSmartcardKey) (`SSH_AGENTC_ADD_SMARTCARD_KEY`) message.
 ///
 /// Described in [draft-miller-ssh-agent-14 § 3.2](https://www.ietf.org/archive/id/draft-miller-ssh-agent-14.html#section-3.2)
-#[derive(Clone, PartialEq, Debug)]
+#[derive(Clone, Debug)]
 pub struct SmartcardKey {
     /// An opaque identifier for the hardware token
     ///
@@ -55,33 +57,43 @@ pub struct SmartcardKey {
     pub id: String,
 
     /// An optional password to unlock the key
-    pub pin: String,
+    pub pin: SecretString,
 }
 
 impl Decode for SmartcardKey {
     type Error = Error;
 
     fn decode(reader: &mut impl Reader) -> Result<Self> {
         let id = String::decode(reader)?;
-        let pin = String::decode(reader)?;
+        let pin = String::decode(reader)?.into();
 
         Ok(Self { id, pin })
     }
 }
 
 impl Encode for SmartcardKey {
     fn encoded_len(&self) -> ssh_encoding::Result<usize> {
-        [self.id.encoded_len()?, self.pin.encoded_len()?].checked_sum()
+        [
+            self.id.encoded_len()?,
+            self.pin.expose_secret().encoded_len()?,
+        ]
+        .checked_sum()
     }
 
     fn encode(&self, writer: &mut impl Writer) -> ssh_encoding::Result<()> {
         self.id.encode(writer)?;
-        self.pin.encode(writer)?;
+        self.pin.expose_secret().encode(writer)?;
 
         Ok(())
     }
 }
 
+impl PartialEq for SmartcardKey {
+    fn eq(&self, other: &Self) -> bool {
+        self.id == other.id && self.pin.expose_secret() == other.pin.expose_secret()
+    }
+}
+
 /// Remove a key from an agent.
 ///
 /// This structure is sent in a [`Request::RemoveIdentity`](super::Request::RemoveIdentity) (`SSH_AGENTC_REMOVE_IDENTITY`) message.