From 51348b2f5b10a2600b279f971fe874ac1b46b9c0 Mon Sep 17 00:00:00 2001 From: Ionut Mihalcea Date: Fri, 2 Sep 2022 23:44:06 +0100 Subject: [PATCH 1/2] Make separate constructors for RO/RW sessions The previous constructor took a boolean argument which made it difficult to understand what the user code would be doing. That constructor is now two separate functions, one for RO sessions, one for RW sessions. The `_no_callback` part of the method name was also removed and added to the documentation instead. Signed-off-by: Ionut Mihalcea --- cryptoki/src/context/mod.rs | 17 ++++++-- cryptoki/src/session/mod.rs | 2 +- cryptoki/tests/basic.rs | 82 +++++++++++++++++++++++++++++++------ cryptoki/tests/common.rs | 2 +- 4 files changed, 86 insertions(+), 17 deletions(-) diff --git a/cryptoki/src/context/mod.rs b/cryptoki/src/context/mod.rs index 3407a673..8bb7c268 100644 --- a/cryptoki/src/context/mod.rs +++ b/cryptoki/src/context/mod.rs @@ -168,9 +168,20 @@ impl Pkcs11 { slot_token_management::get_mechanism_info(self, slot, type_) } - /// Open a new session with no callback set - pub fn open_session_no_callback(&self, slot_id: Slot, read_write: bool) -> Result { - session_management::open_session_no_callback(self, slot_id, read_write) + /// Open a new Read-Only session + /// + /// For a Read-Write session, use `open_rw_session` + /// + /// Note: No callback is set when opening the session. + pub fn open_ro_session(&self, slot_id: Slot) -> Result { + session_management::open_session_no_callback(self, slot_id, false) + } + + /// Open a new Read/Write session + /// + /// Note: No callback is set when opening the session. + pub fn open_rw_session(&self, slot_id: Slot) -> Result { + session_management::open_session_no_callback(self, slot_id, true) } /// Check whether a given PKCS11 spec-defined function is supported by this implementation diff --git a/cryptoki/src/session/mod.rs b/cryptoki/src/session/mod.rs index 5ef34b55..245c461e 100644 --- a/cryptoki/src/session/mod.rs +++ b/cryptoki/src/session/mod.rs @@ -175,7 +175,7 @@ impl Session { /// pkcs11.initialize(CInitializeArgs::OsThreads).unwrap(); /// let slot = pkcs11.get_slots_with_token().unwrap().remove(0); /// - /// let session = pkcs11.open_session_no_callback(slot, true).unwrap(); + /// let session = pkcs11.open_ro_session(slot).unwrap(); /// session.login(UserType::User, Some("fedcba")); /// /// let empty_attrib= vec![]; diff --git a/cryptoki/tests/basic.rs b/cryptoki/tests/basic.rs index 6e08c93a..ecb42666 100644 --- a/cryptoki/tests/basic.rs +++ b/cryptoki/tests/basic.rs @@ -29,7 +29,7 @@ fn sign_verify() -> Result<()> { let (pkcs11, slot) = init_pins(); // open a session - let session = pkcs11.open_session_no_callback(slot, true)?; + let session = pkcs11.open_rw_session(slot)?; // log in the session session.login(UserType::User, Some(USER_PIN))?; @@ -77,7 +77,7 @@ fn encrypt_decrypt() -> Result<()> { let (pkcs11, slot) = init_pins(); // open a session - let session = pkcs11.open_session_no_callback(slot, true)?; + let session = pkcs11.open_rw_session(slot)?; // log in the session session.login(UserType::User, Some(USER_PIN))?; @@ -129,7 +129,7 @@ fn derive_key() -> Result<()> { let (pkcs11, slot) = init_pins(); // open a session - let session = pkcs11.open_session_no_callback(slot, true)?; + let session = pkcs11.open_rw_session(slot)?; // log in the session session.login(UserType::User, Some(USER_PIN))?; @@ -220,7 +220,7 @@ fn import_export() -> Result<()> { let (pkcs11, slot) = init_pins(); // open a session - let session = pkcs11.open_session_no_callback(slot, true)?; + let session = pkcs11.open_rw_session(slot)?; // log in the session session.login(UserType::User, Some(USER_PIN))?; @@ -286,7 +286,7 @@ fn get_token_info() -> Result<()> { fn wrap_and_unwrap_key() { let (pkcs11, slot) = init_pins(); // open a session - let session = pkcs11.open_session_no_callback(slot, true).unwrap(); + let session = pkcs11.open_rw_session(slot).unwrap(); // log in the session session.login(UserType::User, Some(USER_PIN)).unwrap(); @@ -375,7 +375,7 @@ fn login_feast() { for _ in 0..SESSIONS { let pkcs11 = pkcs11.clone(); threads.push(thread::spawn(move || { - let session = pkcs11.open_session_no_callback(slot, true).unwrap(); + let session = pkcs11.open_rw_session(slot).unwrap(); match session.login(UserType::User, Some(USER_PIN)) { Ok(_) | Err(Error::Pkcs11(RvError::UserAlreadyLoggedIn)) => {} Err(e) => panic!("Bad error response: {}", e), @@ -437,7 +437,7 @@ fn get_slot_info_test() -> Result<()> { fn get_session_info_test() -> Result<()> { let (pkcs11, slot) = init_pins(); { - let session = pkcs11.open_session_no_callback(slot, false)?; + let session = pkcs11.open_ro_session(slot)?; let session_info = session.get_session_info()?; assert!(!session_info.read_write()); assert_eq!(session_info.slot_id(), slot); @@ -461,7 +461,7 @@ fn get_session_info_test() -> Result<()> { } } - let session = pkcs11.open_session_no_callback(slot, true)?; + let session = pkcs11.open_rw_session(slot)?; let session_info = session.get_session_info()?; assert!(session_info.read_write()); assert_eq!(session_info.slot_id(), slot); @@ -493,7 +493,7 @@ fn get_session_info_test() -> Result<()> { fn generate_random_test() -> Result<()> { let (pkcs11, slot) = init_pins(); - let session = pkcs11.open_session_no_callback(slot, false)?; + let session = pkcs11.open_ro_session(slot)?; let poor_seed: [u8; 32] = [0; 32]; session.seed_random(&poor_seed)?; @@ -517,7 +517,7 @@ fn set_pin_test() -> Result<()> { let new_user_pin = "123456"; let (pkcs11, slot) = init_pins(); - let session = pkcs11.open_session_no_callback(slot, true)?; + let session = pkcs11.open_rw_session(slot)?; session.login(UserType::User, Some(USER_PIN))?; session.set_pin(USER_PIN, new_user_pin)?; @@ -533,7 +533,7 @@ fn get_attribute_info_test() -> Result<()> { let (pkcs11, slot) = init_pins(); // open a session - let session = pkcs11.open_session_no_callback(slot, true)?; + let session = pkcs11.open_rw_session(slot)?; // log in the session session.login(UserType::User, Some(USER_PIN))?; @@ -684,7 +684,7 @@ fn aes_key_attributes_test() -> Result<()> { let (pkcs11, slot) = init_pins(); // open a session - let session = pkcs11.open_session_no_callback(slot, true)?; + let session = pkcs11.open_rw_session(slot)?; // log in the session session.login(UserType::User, Some(USER_PIN))?; @@ -723,3 +723,61 @@ fn aes_key_attributes_test() -> Result<()> { Ok(()) } + +#[test] +#[serial] +fn ro_rw_session_test() -> Result<()> { + let public_exponent: Vec = vec![0x01, 0x00, 0x01]; + let modulus = vec![0xFF; 1024]; + + let template = vec![ + Attribute::Token(true), + Attribute::Private(false), + Attribute::PublicExponent(public_exponent), + Attribute::Modulus(modulus), + Attribute::Class(ObjectClass::PUBLIC_KEY), + Attribute::KeyType(KeyType::RSA), + Attribute::Verify(true), + ]; + + let (pkcs11, slot) = init_pins(); + + // Try out Read-Only session + { + // open a session + let ro_session = pkcs11.open_ro_session(slot)?; + + // log in the session + ro_session.login(UserType::User, Some(USER_PIN))?; + + // generate a key pair + // This should NOT work using the Read/Write session + let e = ro_session.create_object(&template).unwrap_err(); + + if let Error::Pkcs11(RvError::SessionReadOnly) = e { + // as expected + } else { + panic!("Got wrong error code (expecting SessionReadOnly): {}", e); + } + ro_session.logout()?; + } + + // Try out Read/Write session + { + // open a session + let rw_session = pkcs11.open_rw_session(slot)?; + + // log in the session + rw_session.login(UserType::User, Some(USER_PIN))?; + + // generate a key pair + // This should work using the Read/Write session + let object = rw_session.create_object(&template)?; + + // delete keys + rw_session.destroy_object(object)?; + rw_session.logout()?; + } + + Ok(()) +} diff --git a/cryptoki/tests/common.rs b/cryptoki/tests/common.rs index f6f2883a..247e5f29 100644 --- a/cryptoki/tests/common.rs +++ b/cryptoki/tests/common.rs @@ -27,7 +27,7 @@ pub fn init_pins() -> (Pkcs11, Slot) { { // open a session - let session = pkcs11.open_session_no_callback(slot, true).unwrap(); + 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(); From 38d5538c4b23fbf0abc84b5e5f4e3dad32ac18e7 Mon Sep 17 00:00:00 2001 From: Ionut Mihalcea Date: Tue, 6 Sep 2022 12:06:50 +0100 Subject: [PATCH 2/2] PR feedback Signed-off-by: Ionut Mihalcea --- cryptoki/src/context/mod.rs | 4 ++-- cryptoki/src/context/session_management.rs | 6 +----- cryptoki/tests/basic.rs | 2 +- 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/cryptoki/src/context/mod.rs b/cryptoki/src/context/mod.rs index 8bb7c268..c6f25e3a 100644 --- a/cryptoki/src/context/mod.rs +++ b/cryptoki/src/context/mod.rs @@ -174,14 +174,14 @@ impl Pkcs11 { /// /// Note: No callback is set when opening the session. pub fn open_ro_session(&self, slot_id: Slot) -> Result { - session_management::open_session_no_callback(self, slot_id, false) + session_management::open_session(self, slot_id, false) } /// Open a new Read/Write session /// /// Note: No callback is set when opening the session. pub fn open_rw_session(&self, slot_id: Slot) -> Result { - session_management::open_session_no_callback(self, slot_id, true) + session_management::open_session(self, slot_id, true) } /// Check whether a given PKCS11 spec-defined function is supported by this implementation diff --git a/cryptoki/src/context/session_management.rs b/cryptoki/src/context/session_management.rs index bf628312..dbe52e16 100644 --- a/cryptoki/src/context/session_management.rs +++ b/cryptoki/src/context/session_management.rs @@ -11,11 +11,7 @@ use crate::slot::Slot; use std::convert::TryInto; // See public docs on stub in parent mod.rs #[inline(always)] -pub(super) fn open_session_no_callback( - ctx: &Pkcs11, - slot_id: Slot, - read_write: bool, -) -> Result { +pub(super) fn open_session(ctx: &Pkcs11, slot_id: Slot, read_write: bool) -> Result { let mut session_handle = 0; let flags = if read_write { diff --git a/cryptoki/tests/basic.rs b/cryptoki/tests/basic.rs index ecb42666..b798c5db 100644 --- a/cryptoki/tests/basic.rs +++ b/cryptoki/tests/basic.rs @@ -751,7 +751,7 @@ fn ro_rw_session_test() -> Result<()> { ro_session.login(UserType::User, Some(USER_PIN))?; // generate a key pair - // This should NOT work using the Read/Write session + // This should NOT work using the Read-Only session let e = ro_session.create_object(&template).unwrap_err(); if let Error::Pkcs11(RvError::SessionReadOnly) = e {