Add secrecy/zeroize wrapper for new Pin type

Related to parallaxsecond#126 Signed-off-by: Simon Bihel <[email protected]>
chrisFrodo · Feb 16, 2023 · 3281f81 · 3281f81
1 parent cd497d6
commit 3281f81
Show file tree

Hide file tree

Showing 8 changed files with 73 additions and 42 deletions.
diff --git a/cryptoki/Cargo.toml b/cryptoki/Cargo.toml
@@ -19,6 +19,7 @@ derivative = "2.2.0"
 psa-crypto = { version = "0.9.0", default-features = false, optional = true }
 cryptoki-sys = { path = "../cryptoki-sys", version = "0.1.4" }
 paste = "1.0.6"
+secrecy = "0.8.0"
 
 [dev-dependencies]
 num-traits = "0.2.14"
@@ -29,3 +30,4 @@ testresult = "0.2.0"
 [features]
 psa-crypto-conversions = ["psa-crypto"]
 generate-bindings = ["cryptoki-sys/generate-bindings"]
+serde = ["secrecy/serde"]
diff --git a/cryptoki/src/context/slot_token_management.rs b/cryptoki/src/context/slot_token_management.rs
@@ -7,7 +7,9 @@ use crate::error::{Result, Rv};
 use crate::label_from_str;
 use crate::mechanism::{MechanismInfo, MechanismType};
 use crate::slot::{Slot, SlotInfo, TokenInfo};
+use crate::types::Pin;
 use cryptoki_sys::{CK_BBOOL, CK_FALSE, CK_MECHANISM_INFO, CK_SLOT_INFO, CK_TOKEN_INFO, CK_TRUE};
+use secrecy::ExposeSecret;
 use std::convert::{TryFrom, TryInto};
 
 use crate::error::RvError::BufferTooSmall;
@@ -75,13 +77,13 @@ impl Pkcs11 {
     /// Initialize a token
     ///
     /// Currently will use an empty label for all tokens.
-    pub fn init_token(&self, slot: Slot, pin: &str, label: &str) -> Result<()> {
+    pub fn init_token(&self, slot: Slot, pin: &Pin, label: &str) -> Result<()> {
         let label = label_from_str(label);
         unsafe {
             Rv::from(get_pkcs11!(self, C_InitToken)(
                 slot.try_into()?,
-                pin.as_ptr() as *mut u8,
-                pin.len().try_into()?,
+                pin.expose_secret().as_ptr() as *mut u8,
+                pin.expose_secret().len().try_into()?,
                 label.as_ptr() as *mut u8,
             ))
             .into_result()

diff --git a/cryptoki/src/session/object_management.rs b/cryptoki/src/session/object_management.rs
@@ -117,6 +117,7 @@ impl Session {
     /// use cryptoki::context::CInitializeArgs;
     /// use cryptoki::object::AttributeType;
     /// use cryptoki::session::UserType;
+    /// use cryptoki::types::Pin;
     /// use std::collections::HashMap;
     /// use std::env;
     ///
@@ -130,7 +131,7 @@ impl Session {
     /// let slot = pkcs11.get_slots_with_token().unwrap().remove(0);
     ///
     /// let session = pkcs11.open_ro_session(slot).unwrap();
-    /// session.login(UserType::User, Some("fedcba"));
+    /// session.login(UserType::User, Some(&Pin::new("fedcba".into())));
     ///
     /// let empty_attrib= vec![];
     /// if let Some(object) = session.find_objects(&empty_attrib).unwrap().get(0) {

diff --git a/cryptoki/src/session/session_management.rs b/cryptoki/src/session/session_management.rs
@@ -4,11 +4,13 @@
 
 use crate::error::{Result, Rv};
 use crate::session::{Session, SessionInfo, UserType};
+use crate::types::{Pin, RawPin};
 
 #[cfg(doc)]
 use cryptoki_sys::CKF_PROTECTED_AUTHENTICATION_PATH;
 use cryptoki_sys::CK_SESSION_INFO;
 use log::error;
+use secrecy::ExposeSecret;
 use std::convert::{TryFrom, TryInto};
 
 impl Drop for Session {
@@ -39,9 +41,12 @@ impl Session {
     ///
     /// _NOTE: By passing `None` into `login`, you must ensure that the
     /// [CKF_PROTECTED_AUTHENTICATION_PATH] flag is set in the `TokenFlags`._
-    pub fn login(&self, user_type: UserType, pin: Option<&str>) -> Result<()> {
+    pub fn login(&self, user_type: UserType, pin: Option<&Pin>) -> Result<()> {
         let (pin, pin_len) = match pin {
-            Some(pin) => (pin.as_ptr() as *mut u8, pin.len()),
+            Some(pin) => (
+                pin.expose_secret().as_ptr() as *mut u8,
+                pin.expose_secret().len(),
+            ),
             None => (std::ptr::null_mut(), 0),
         };
         unsafe {
@@ -67,13 +72,13 @@ impl Session {
     ///
     /// _NOTE: By passing `None` into `login`, you must ensure that the
     /// [CKF_PROTECTED_AUTHENTICATION_PATH] flag is set in the `TokenFlags`._
-    pub fn login_with_raw(&self, user_type: UserType, pin: &[u8]) -> Result<()> {
+    pub fn login_with_raw(&self, user_type: UserType, pin: &RawPin) -> Result<()> {
         unsafe {
             Rv::from(get_pkcs11!(self.client(), C_Login)(
                 self.handle(),
                 user_type.into(),
-                pin.as_ptr() as *mut u8,
-                pin.len().try_into()?,
+                pin.expose_secret().as_ptr() as *mut u8,
+                pin.expose_secret().len().try_into()?,
             ))
             .into_result()
         }

diff --git a/cryptoki/src/session/slot_token_management.rs b/cryptoki/src/session/slot_token_management.rs
@@ -4,31 +4,33 @@
 
 use crate::error::{Result, Rv};
 use crate::session::Session;
+use crate::types::Pin;
+use secrecy::ExposeSecret;
 use std::convert::TryInto;
 
 impl Session {
     /// Initialize the normal user's pin for a token
-    pub fn init_pin(&self, pin: &str) -> Result<()> {
+    pub fn init_pin(&self, pin: &Pin) -> Result<()> {
         unsafe {
             Rv::from(get_pkcs11!(self.client(), C_InitPIN)(
                 self.handle(),
-                pin.as_ptr() as *mut u8,
-                pin.len().try_into()?,
+                pin.expose_secret().as_ptr() as *mut u8,
+                pin.expose_secret().len().try_into()?,
             ))
             .into_result()
         }
     }
 
     /// Changes the PIN of either the currently logged in user or of the `CKU_USER` if no user is
     /// logged in.
-    pub fn set_pin(&self, old_pin: &str, new_pin: &str) -> Result<()> {
+    pub fn set_pin(&self, old_pin: &Pin, new_pin: &Pin) -> Result<()> {
         unsafe {
             Rv::from(get_pkcs11!(self.client(), C_SetPIN)(
                 self.handle(),
-                old_pin.as_ptr() as *mut u8,
-                old_pin.len().try_into()?,
-                new_pin.as_ptr() as *mut u8,
-                new_pin.len().try_into()?,
+                old_pin.expose_secret().as_ptr() as *mut u8,
+                old_pin.expose_secret().len().try_into()?,
+                new_pin.expose_secret().as_ptr() as *mut u8,
+                new_pin.expose_secret().len().try_into()?,
             ))
             .into_result()
         }

diff --git a/cryptoki/src/types.rs b/cryptoki/src/types.rs
@@ -4,6 +4,8 @@
 
 use crate::error::{Error, Result};
 use cryptoki_sys::*;
+use secrecy::SecretString;
+use secrecy::SecretVec;
 use std::convert::TryFrom;
 use std::convert::TryInto;
 use std::fmt::Formatter;
@@ -271,6 +273,16 @@ pub(crate) fn convert_utc_time(orig: [u8; 16]) -> Result<UtcTime> {
     })
 }
 
+/// Secret wrapper for a Pin
+///
+/// Enable the `serde` feature to add support for Deserialize
+pub type Pin = SecretString;
+
+/// Secret wrapper for a raw non UTF-8 Pin
+///
+/// Enable the `serde` feature to add support for Deserialize
+pub type RawPin = SecretVec<u8>;
+
 #[cfg(test)]
 mod test {
 

diff --git a/cryptoki/tests/basic.rs b/cryptoki/tests/basic.rs
@@ -8,6 +8,7 @@ use cryptoki::error::{Error, RvError};
 use cryptoki::mechanism::Mechanism;
 use cryptoki::object::{Attribute, AttributeInfo, AttributeType, KeyType, ObjectClass};
 use cryptoki::session::{SessionState, UserType};
+use cryptoki::types::Pin;
 use serial_test::serial;
 use std::collections::HashMap;
 use std::thread;
@@ -23,7 +24,7 @@ fn sign_verify() -> TestResult {
     let session = pkcs11.open_rw_session(slot)?;
 
     // log in the session
-    session.login(UserType::User, Some(USER_PIN))?;
+    session.login(UserType::User, Some(&Pin::new(USER_PIN.into())))?;
 
     // get mechanism
     let mechanism = Mechanism::RsaPkcsKeyPairGen;
@@ -71,7 +72,7 @@ fn encrypt_decrypt() -> TestResult {
     let session = pkcs11.open_rw_session(slot)?;
 
     // log in the session
-    session.login(UserType::User, Some(USER_PIN))?;
+    session.login(UserType::User, Some(&Pin::new(USER_PIN.into())))?;
 
     // get mechanism
     let mechanism = Mechanism::RsaPkcsKeyPairGen;
@@ -123,7 +124,7 @@ fn derive_key() -> TestResult {
     let session = pkcs11.open_rw_session(slot)?;
 
     // log in the session
-    session.login(UserType::User, Some(USER_PIN))?;
+    session.login(UserType::User, Some(&Pin::new(USER_PIN.into())))?;
 
     // get mechanism
     let mechanism = Mechanism::EccKeyPairGen;
@@ -207,7 +208,7 @@ fn import_export() -> TestResult {
     let session = pkcs11.open_rw_session(slot)?;
 
     // log in the session
-    session.login(UserType::User, Some(USER_PIN))?;
+    session.login(UserType::User, Some(&Pin::new(USER_PIN.into())))?;
 
     let public_exponent: Vec<u8> = vec![0x01, 0x00, 0x01];
     let modulus = vec![0xFF; 1024];
@@ -273,7 +274,9 @@ fn wrap_and_unwrap_key() {
     let session = pkcs11.open_rw_session(slot).unwrap();
 
     // log in the session
-    session.login(UserType::User, Some(USER_PIN)).unwrap();
+    session
+        .login(UserType::User, Some(&Pin::new(USER_PIN.into())))
+        .unwrap();
 
     let key_to_be_wrapped_template = vec![
         Attribute::Token(true),
@@ -360,15 +363,15 @@ fn login_feast() {
         let pkcs11 = pkcs11.clone();
         threads.push(thread::spawn(move || {
             let session = pkcs11.open_rw_session(slot).unwrap();
-            match session.login(UserType::User, Some(USER_PIN)) {
+            match session.login(UserType::User, Some(&Pin::new(USER_PIN.into()))) {
                 Ok(_) | Err(Error::Pkcs11(RvError::UserAlreadyLoggedIn)) => {}
                 Err(e) => panic!("Bad error response: {}", e),
             }
-            match session.login(UserType::User, Some(USER_PIN)) {
+            match session.login(UserType::User, Some(&Pin::new(USER_PIN.into()))) {
                 Ok(_) | Err(Error::Pkcs11(RvError::UserAlreadyLoggedIn)) => {}
                 Err(e) => panic!("Bad error response: {}", e),
             }
-            match session.login(UserType::User, Some(USER_PIN)) {
+            match session.login(UserType::User, Some(&Pin::new(USER_PIN.into()))) {
                 Ok(_) | Err(Error::Pkcs11(RvError::UserAlreadyLoggedIn)) => {}
                 Err(e) => panic!("Bad error response: {}", e),
             }
@@ -430,14 +433,14 @@ fn get_session_info_test() -> TestResult {
             SessionState::RoPublic
         ));
 
-        session.login(UserType::User, Some(USER_PIN))?;
+        session.login(UserType::User, Some(&Pin::new(USER_PIN.into())))?;
         let session_info = session.get_session_info()?;
         assert!(!session_info.read_write());
         assert_eq!(session_info.slot_id(), slot);
         assert!(matches!(session_info.session_state(), SessionState::RoUser));
         session.logout()?;
         if let Err(cryptoki::error::Error::Pkcs11(rv_error)) =
-            session.login(UserType::So, Some(SO_PIN))
+            session.login(UserType::So, Some(&Pin::new(SO_PIN.into())))
         {
             assert_eq!(rv_error, RvError::SessionReadOnlyExists)
         } else {
@@ -454,13 +457,13 @@ fn get_session_info_test() -> TestResult {
         SessionState::RwPublic
     ));
 
-    session.login(UserType::User, Some(USER_PIN))?;
+    session.login(UserType::User, Some(&Pin::new(USER_PIN.into())))?;
     let session_info = session.get_session_info()?;
     assert!(session_info.read_write());
     assert_eq!(session_info.slot_id(), slot);
     assert!(matches!(session_info.session_state(), SessionState::RwUser,));
     session.logout()?;
-    session.login(UserType::So, Some(SO_PIN))?;
+    session.login(UserType::So, Some(&Pin::new(SO_PIN.into())))?;
     let session_info = session.get_session_info()?;
     assert!(session_info.read_write());
     assert_eq!(session_info.slot_id(), slot);
@@ -502,11 +505,13 @@ fn set_pin_test() -> TestResult {
     let (pkcs11, slot) = init_pins();
 
     let session = pkcs11.open_rw_session(slot)?;
+    let user_pin = Pin::new(USER_PIN.into());
+    let new_user_pin = Pin::new(new_user_pin.into());
 
-    session.login(UserType::User, Some(USER_PIN))?;
-    session.set_pin(USER_PIN, new_user_pin)?;
+    session.login(UserType::User, Some(&user_pin))?;
+    session.set_pin(&user_pin, &new_user_pin)?;
     session.logout()?;
-    session.login(UserType::User, Some(new_user_pin))?;
+    session.login(UserType::User, Some(&new_user_pin))?;
 
     Ok(())
 }
@@ -520,7 +525,7 @@ fn get_attribute_info_test() -> TestResult {
     let session = pkcs11.open_rw_session(slot)?;
 
     // log in the session
-    session.login(UserType::User, Some(USER_PIN))?;
+    session.login(UserType::User, Some(&Pin::new(USER_PIN.into())))?;
 
     // get mechanism
     let mechanism = Mechanism::RsaPkcsKeyPairGen;
@@ -671,7 +676,7 @@ fn aes_key_attributes_test() -> TestResult {
     let session = pkcs11.open_rw_session(slot)?;
 
     // log in the session
-    session.login(UserType::User, Some(USER_PIN))?;
+    session.login(UserType::User, Some(&Pin::new(USER_PIN.into())))?;
 
     // get mechanism
     let mechanism = Mechanism::AesKeyGen;
@@ -732,7 +737,7 @@ fn ro_rw_session_test() -> TestResult {
         let ro_session = pkcs11.open_ro_session(slot)?;
 
         // log in the session
-        ro_session.login(UserType::User, Some(USER_PIN))?;
+        ro_session.login(UserType::User, Some(&Pin::new(USER_PIN.into())))?;
 
         // generate a key pair
         // This should NOT work using the Read-Only session
@@ -752,7 +757,7 @@ fn ro_rw_session_test() -> TestResult {
         let rw_session = pkcs11.open_rw_session(slot)?;
 
         // log in the session
-        rw_session.login(UserType::User, Some(USER_PIN))?;
+        rw_session.login(UserType::User, Some(&Pin::new(USER_PIN.into())))?;
 
         // generate a key pair
         // This should work using the Read/Write session
@@ -781,7 +786,7 @@ fn aes_cbc_encrypt() -> TestResult {
 
     let (pkcs11, slot) = init_pins();
     let session = pkcs11.open_rw_session(slot)?;
-    session.login(UserType::User, Some(USER_PIN))?;
+    session.login(UserType::User, Some(&Pin::new(USER_PIN.into())))?;
 
     let template = [
         Attribute::Class(ObjectClass::SECRET_KEY),
@@ -812,7 +817,7 @@ fn aes_cbc_pad_encrypt() -> TestResult {
 
     let (pkcs11, slot) = init_pins();
     let session = pkcs11.open_rw_session(slot)?;
-    session.login(UserType::User, Some(USER_PIN))?;
+    session.login(UserType::User, Some(&Pin::new(USER_PIN.into())))?;
 
     let template = [
         Attribute::Class(ObjectClass::SECRET_KEY),
@@ -835,7 +840,7 @@ fn update_attributes_key() -> TestResult {
     let session = pkcs11.open_rw_session(slot)?;
 
     // log in the session
-    session.login(UserType::User, Some(USER_PIN))?;
+    session.login(UserType::User, Some(&Pin::new(USER_PIN.into())))?;
 
     // pub key template
     let pub_key_template = vec![

diff --git a/cryptoki/tests/common.rs b/cryptoki/tests/common.rs
@@ -3,6 +3,7 @@
 use cryptoki::context::{CInitializeArgs, Pkcs11};
 use cryptoki::session::UserType;
 use cryptoki::slot::Slot;
+use cryptoki::types::Pin;
 use std::env;
 
 // The default user pin
@@ -23,14 +24,15 @@ pub fn init_pins() -> (Pkcs11, Slot) {
     // find a slot, get the first one
     let slot = pkcs11.get_slots_with_token().unwrap().remove(0);
 
-    pkcs11.init_token(slot, SO_PIN, "Test Token").unwrap();
+    let so_pin = Pin::new(SO_PIN.into());
+    pkcs11.init_token(slot, &so_pin, "Test Token").unwrap();
 
     {
         // open a session
         let session = pkcs11.open_rw_session(slot).unwrap();
         // log in the session
-        session.login(UserType::So, Some(SO_PIN)).unwrap();
-        session.init_pin(USER_PIN).unwrap();
+        session.login(UserType::So, Some(&so_pin)).unwrap();
+        session.init_pin(&Pin::new(USER_PIN.into())).unwrap();
     }
 
     (pkcs11, slot)