From ebf7765dddceacf79dbe32c1048022c392e4b02d Mon Sep 17 00:00:00 2001 From: Wiktor Kwapisiewicz Date: Thu, 23 Oct 2025 11:29:32 +0200 Subject: [PATCH 1/5] Make `Session` take a reference to `Pkcs11` and remove inner `Arc` Signed-off-by: Wiktor Kwapisiewicz --- cryptoki/src/context/mod.rs | 37 +++++++------------ cryptoki/src/context/session_management.rs | 8 ++-- cryptoki/src/session/decryption.rs | 2 +- cryptoki/src/session/digesting.rs | 2 +- cryptoki/src/session/encapsulation.rs | 2 +- cryptoki/src/session/encryption.rs | 2 +- cryptoki/src/session/key_management.rs | 2 +- cryptoki/src/session/message_decryption.rs | 2 +- cryptoki/src/session/message_encryption.rs | 2 +- cryptoki/src/session/mod.rs | 20 ++++------ cryptoki/src/session/object_management.rs | 4 +- cryptoki/src/session/random.rs | 2 +- cryptoki/src/session/session_management.rs | 4 +- cryptoki/src/session/signing_macing.rs | 2 +- cryptoki/src/session/slot_token_management.rs | 2 +- cryptoki/src/session/validation.rs | 2 +- cryptoki/tests/basic.rs | 4 +- 17 files changed, 44 insertions(+), 55 deletions(-) diff --git a/cryptoki/src/context/mod.rs b/cryptoki/src/context/mod.rs index 388cc4b7..3504b5d8 100644 --- a/cryptoki/src/context/mod.rs +++ b/cryptoki/src/context/mod.rs @@ -34,7 +34,6 @@ use log::error; use std::fmt; use std::path::Path; use std::ptr; -use std::sync::Arc; use std::sync::RwLock; /// Enum for various function lists @@ -100,10 +99,10 @@ impl Drop for Pkcs11Impl { } /// Main PKCS11 context. Should usually be unique per application. -#[derive(Clone, Debug)] +#[derive(Debug)] pub struct Pkcs11 { - pub(crate) impl_: Arc, - initialized: Arc>, + pub(crate) impl_: Pkcs11Impl, + initialized: RwLock, } impl Pkcs11 { @@ -154,21 +153,21 @@ impl Pkcs11 { let list32_ptr: *mut cryptoki_sys::CK_FUNCTION_LIST_3_2 = ifce.pFunctionList as *mut cryptoki_sys::CK_FUNCTION_LIST_3_2; return Ok(Pkcs11 { - impl_: Arc::new(Pkcs11Impl { + impl_: Pkcs11Impl { _pkcs11_lib: pkcs11_lib, function_list: FunctionList::V3_2(*list32_ptr), - }), - initialized: Arc::new(RwLock::new(false)), + }, + initialized: RwLock::new(false), }); } let list30_ptr: *mut cryptoki_sys::CK_FUNCTION_LIST_3_0 = ifce.pFunctionList as *mut cryptoki_sys::CK_FUNCTION_LIST_3_0; return Ok(Pkcs11 { - impl_: Arc::new(Pkcs11Impl { + impl_: Pkcs11Impl { _pkcs11_lib: pkcs11_lib, function_list: FunctionList::V3_0(v30tov32(*list30_ptr)), - }), - initialized: Arc::new(RwLock::new(false)), + }, + initialized: RwLock::new(false), }); } /* fall back to the 2.* API */ @@ -180,21 +179,17 @@ impl Pkcs11 { .into_result(Function::GetFunctionList)?; Ok(Pkcs11 { - impl_: Arc::new(Pkcs11Impl { + impl_: Pkcs11Impl { _pkcs11_lib: pkcs11_lib, function_list: FunctionList::V2(v2tov3(*list_ptr)), - }), - initialized: Arc::new(RwLock::new(false)), + }, + initialized: RwLock::new(false), }) } /// Initialize the PKCS11 library pub fn initialize(&self, init_args: CInitializeArgs) -> Result<()> { - let mut init_lock = self - .initialized - .as_ref() - .write() - .expect("lock not to be poisoned"); + let mut init_lock = self.initialized.write().expect("lock not to be poisoned"); if *init_lock { Err(Error::AlreadyInitialized)? } @@ -203,11 +198,7 @@ impl Pkcs11 { /// Check whether the PKCS11 library has been initialized pub fn is_initialized(&self) -> bool { - *self - .initialized - .as_ref() - .read() - .expect("lock not to be poisoned") + *self.initialized.read().expect("lock not to be poisoned") } /// Finalize the PKCS11 library. Indicates that the application no longer needs to use PKCS11. diff --git a/cryptoki/src/context/session_management.rs b/cryptoki/src/context/session_management.rs index 382c45da..3edc85e1 100644 --- a/cryptoki/src/context/session_management.rs +++ b/cryptoki/src/context/session_management.rs @@ -13,7 +13,7 @@ use super::Function; impl Pkcs11 { #[inline(always)] - fn open_session(&self, slot_id: Slot, read_write: bool) -> Result { + fn open_session(&self, slot_id: Slot, read_write: bool) -> Result> { let mut session_handle = 0; let flags = if read_write { @@ -33,7 +33,7 @@ impl Pkcs11 { .into_result(Function::OpenSession)?; } - Ok(Session::new(session_handle, self.clone())) + Ok(Session::new(session_handle, self)) } /// Open a new Read-Only session @@ -60,14 +60,14 @@ impl Pkcs11 { /// let session = client.open_ro_session(slot)?; /// # let _ = session; Ok(()) } /// ``` - pub fn open_ro_session(&self, slot_id: Slot) -> Result { + pub fn open_ro_session(&self, slot_id: Slot) -> Result> { self.open_session(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 { + pub fn open_rw_session(&self, slot_id: Slot) -> Result> { self.open_session(slot_id, true) } } diff --git a/cryptoki/src/session/decryption.rs b/cryptoki/src/session/decryption.rs index 4017091b..8d4f2514 100644 --- a/cryptoki/src/session/decryption.rs +++ b/cryptoki/src/session/decryption.rs @@ -10,7 +10,7 @@ use crate::session::Session; use cryptoki_sys::*; use std::convert::TryInto; -impl Session { +impl Session<'_> { /// Single-part decryption operation pub fn decrypt( &self, diff --git a/cryptoki/src/session/digesting.rs b/cryptoki/src/session/digesting.rs index 615ec89d..2079d6e5 100644 --- a/cryptoki/src/session/digesting.rs +++ b/cryptoki/src/session/digesting.rs @@ -10,7 +10,7 @@ use crate::session::Session; use cryptoki_sys::*; use std::convert::TryInto; -impl Session { +impl Session<'_> { /// Single-part digesting operation pub fn digest(&self, m: &Mechanism, data: &[u8]) -> Result> { let mut mechanism: CK_MECHANISM = m.into(); diff --git a/cryptoki/src/session/encapsulation.rs b/cryptoki/src/session/encapsulation.rs index 842fa1af..1610b30c 100644 --- a/cryptoki/src/session/encapsulation.rs +++ b/cryptoki/src/session/encapsulation.rs @@ -10,7 +10,7 @@ use crate::session::Session; use cryptoki_sys::*; use std::convert::TryInto; -impl Session { +impl Session<'_> { /// Encapsulate key pub fn encapsulate_key( &self, diff --git a/cryptoki/src/session/encryption.rs b/cryptoki/src/session/encryption.rs index 7aabb1e9..66cff3fd 100644 --- a/cryptoki/src/session/encryption.rs +++ b/cryptoki/src/session/encryption.rs @@ -10,7 +10,7 @@ use crate::session::Session; use cryptoki_sys::*; use std::convert::TryInto; -impl Session { +impl Session<'_> { /// Single-part encryption operation pub fn encrypt( &self, diff --git a/cryptoki/src/session/key_management.rs b/cryptoki/src/session/key_management.rs index 0d5dfd13..689076b4 100644 --- a/cryptoki/src/session/key_management.rs +++ b/cryptoki/src/session/key_management.rs @@ -10,7 +10,7 @@ use crate::session::Session; use cryptoki_sys::{CK_ATTRIBUTE, CK_MECHANISM, CK_MECHANISM_PTR}; use std::convert::TryInto; -impl Session { +impl Session<'_> { /// Generate a secret key pub fn generate_key( &self, diff --git a/cryptoki/src/session/message_decryption.rs b/cryptoki/src/session/message_decryption.rs index cd86db2f..4d71de88 100644 --- a/cryptoki/src/session/message_decryption.rs +++ b/cryptoki/src/session/message_decryption.rs @@ -10,7 +10,7 @@ use crate::session::Session; use cryptoki_sys::*; use std::convert::TryInto; -impl Session { +impl Session<'_> { /// Prepare a session for one or more Message-based decryption using the same mechanism and key pub fn message_decrypt_init(&self, mechanism: &Mechanism, key: ObjectHandle) -> Result<()> { let mut mechanism: CK_MECHANISM = mechanism.into(); diff --git a/cryptoki/src/session/message_encryption.rs b/cryptoki/src/session/message_encryption.rs index 1808f8e0..70638039 100644 --- a/cryptoki/src/session/message_encryption.rs +++ b/cryptoki/src/session/message_encryption.rs @@ -10,7 +10,7 @@ use crate::session::Session; use cryptoki_sys::*; use std::convert::TryInto; -impl Session { +impl Session<'_> { /// Prepare a session for one or more Message-based encryption using the same mechanism and key pub fn message_encrypt_init(&self, mechanism: &Mechanism, key: ObjectHandle) -> Result<()> { let mut mechanism: CK_MECHANISM = mechanism.into(); diff --git a/cryptoki/src/session/mod.rs b/cryptoki/src/session/mod.rs index d71dab05..786e8622 100644 --- a/cryptoki/src/session/mod.rs +++ b/cryptoki/src/session/mod.rs @@ -34,37 +34,33 @@ pub use validation::ValidationFlagsType; /// threads. A Session needs to be created in its own thread or to be passed by ownership to /// another thread. #[derive(Debug)] -pub struct Session { +pub struct Session<'a> { handle: CK_SESSION_HANDLE, - client: Pkcs11, + client: &'a Pkcs11, // This is not used but to prevent Session to automatically implement Send and Sync _guard: PhantomData<*mut u32>, } -impl std::fmt::Display for Session { +impl<'a> std::fmt::Display for Session<'a> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!(f, "{}", self.handle) } } -impl std::fmt::LowerHex for Session { +impl<'a> std::fmt::LowerHex for Session<'a> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!(f, "{:08x}", self.handle) } } -impl std::fmt::UpperHex for Session { +impl<'a> std::fmt::UpperHex for Session<'a> { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!(f, "{:08X}", self.handle) } } -// Session does not implement Sync to prevent the same Session instance to be used from multiple -// threads. -unsafe impl Send for Session {} - -impl Session { - pub(crate) fn new(handle: CK_SESSION_HANDLE, client: Pkcs11) -> Self { +impl<'a> Session<'a> { + pub(crate) fn new(handle: CK_SESSION_HANDLE, client: &'a Pkcs11) -> Self { Session { handle, client, @@ -73,7 +69,7 @@ impl Session { } } -impl Session { +impl<'a> Session<'a> { /// Close a session /// This will be called on drop as well. pub fn close(self) {} diff --git a/cryptoki/src/session/object_management.rs b/cryptoki/src/session/object_management.rs index 13034867..bf31d676 100644 --- a/cryptoki/src/session/object_management.rs +++ b/cryptoki/src/session/object_management.rs @@ -78,7 +78,7 @@ const MAX_OBJECT_COUNT: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(10) /// ``` #[derive(Debug)] pub struct ObjectHandleIterator<'a> { - session: &'a Session, + session: &'a Session<'a>, object_count: usize, index: usize, cache: Vec, @@ -207,7 +207,7 @@ impl Drop for ObjectHandleIterator<'_> { } } -impl Session { +impl Session<'_> { /// Iterate over session objects matching a template. /// /// # Arguments diff --git a/cryptoki/src/session/random.rs b/cryptoki/src/session/random.rs index 4926933b..2269fcbb 100644 --- a/cryptoki/src/session/random.rs +++ b/cryptoki/src/session/random.rs @@ -7,7 +7,7 @@ use crate::error::{Result, Rv}; use crate::session::Session; use std::convert::TryInto; -impl Session { +impl Session<'_> { /// Generates a random number and sticks it in a slice /// /// # Arguments diff --git a/cryptoki/src/session/session_management.rs b/cryptoki/src/session/session_management.rs index 40839ad7..f8624bfa 100644 --- a/cryptoki/src/session/session_management.rs +++ b/cryptoki/src/session/session_management.rs @@ -14,7 +14,7 @@ use log::error; use secrecy::ExposeSecret; use std::convert::{TryFrom, TryInto}; -impl Drop for Session { +impl Drop for Session<'_> { fn drop(&mut self) { #[inline(always)] fn close(session: &Session) -> Result<()> { @@ -32,7 +32,7 @@ impl Drop for Session { } } -impl Session { +impl Session<'_> { /// Log a session in. /// /// # Arguments diff --git a/cryptoki/src/session/signing_macing.rs b/cryptoki/src/session/signing_macing.rs index 0927c5c4..d4ea1157 100644 --- a/cryptoki/src/session/signing_macing.rs +++ b/cryptoki/src/session/signing_macing.rs @@ -10,7 +10,7 @@ use crate::session::Session; use cryptoki_sys::*; use std::convert::TryInto; -impl Session { +impl Session<'_> { /// Sign data in single-part pub fn sign(&self, mechanism: &Mechanism, key: ObjectHandle, data: &[u8]) -> Result> { let mut mechanism: CK_MECHANISM = mechanism.into(); diff --git a/cryptoki/src/session/slot_token_management.rs b/cryptoki/src/session/slot_token_management.rs index 4dde294b..4983fac1 100644 --- a/cryptoki/src/session/slot_token_management.rs +++ b/cryptoki/src/session/slot_token_management.rs @@ -9,7 +9,7 @@ use crate::types::AuthPin; use secrecy::ExposeSecret; use std::convert::TryInto; -impl Session { +impl Session<'_> { /// Initialize the normal user's pin for a token pub fn init_pin(&self, pin: &AuthPin) -> Result<()> { unsafe { diff --git a/cryptoki/src/session/validation.rs b/cryptoki/src/session/validation.rs index 69db3984..74f324e6 100644 --- a/cryptoki/src/session/validation.rs +++ b/cryptoki/src/session/validation.rs @@ -46,7 +46,7 @@ impl From for CK_SESSION_VALIDATION_FLAGS_TYPE { } } -impl Session { +impl Session<'_> { /// Get requested validation flags from the session /// /// The only supported flag as for PKCS#11 3.2 is `ValidationFlagsType::VALIDATION_OK` diff --git a/cryptoki/tests/basic.rs b/cryptoki/tests/basic.rs index f6bbde4f..ebcb631d 100644 --- a/cryptoki/tests/basic.rs +++ b/cryptoki/tests/basic.rs @@ -25,6 +25,7 @@ use cryptoki::types::{AuthPin, Ulong}; use serial_test::serial; use std::collections::HashMap; use std::num::NonZeroUsize; +use std::sync::Arc; use std::thread; use cryptoki::mechanism::ekdf::AesCbcDeriveParams; @@ -1466,6 +1467,7 @@ fn login_feast() { const SESSIONS: usize = 100; let (pkcs11, slot) = init_pins(); + let pkcs11 = Arc::new(pkcs11); let mut threads = Vec::new(); for _ in 0..SESSIONS { @@ -1802,7 +1804,7 @@ fn is_initialized_test() { fn test_clone_initialize() { use cryptoki::context::CInitializeArgs; - let pkcs11 = get_pkcs11(); + let pkcs11 = Arc::new(get_pkcs11()); let clone = pkcs11.clone(); assert!( From 796cc050e59cae95edeec3d5726cbe4a23d68cb3 Mon Sep 17 00:00:00 2001 From: Hugues de Valon Date: Mon, 24 Nov 2025 13:48:03 +0100 Subject: [PATCH 2/5] Remove finalizing Pkcs11 on Drop As explained in #208, this might not be wanted in contexts where the Pkcs11 structure is initialised/finalised outside of the Rust program. Signed-off-by: Hugues de Valon --- cryptoki/src/context/general_purpose.rs | 9 ++++++++ cryptoki/src/context/mod.rs | 29 +++---------------------- 2 files changed, 12 insertions(+), 26 deletions(-) diff --git a/cryptoki/src/context/general_purpose.rs b/cryptoki/src/context/general_purpose.rs index 8c7eee8e..ff511bb1 100644 --- a/cryptoki/src/context/general_purpose.rs +++ b/cryptoki/src/context/general_purpose.rs @@ -8,6 +8,7 @@ use cryptoki_sys::{CK_C_INITIALIZE_ARGS, CK_INFO}; use paste::paste; use std::convert::TryFrom; use std::fmt::Display; +use std::ptr; // See public docs on stub in parent mod.rs #[inline(always)] @@ -23,6 +24,14 @@ pub(super) fn initialize(ctx: &Pkcs11, init_args: CInitializeArgs) -> Result<()> } } +// See public docs on stub in parent mod.rs +#[inline(always)] +pub(super) fn finalize(ctx: Pkcs11) -> Result<()> { + unsafe { + Rv::from(get_pkcs11!(ctx, C_Finalize)(ptr::null_mut())).into_result(Function::Finalize) + } +} + // See public docs on stub in parent mod.rs #[inline(always)] pub(super) fn get_library_info(ctx: &Pkcs11) -> Result { diff --git a/cryptoki/src/context/mod.rs b/cryptoki/src/context/mod.rs index 3504b5d8..cb2b25b8 100644 --- a/cryptoki/src/context/mod.rs +++ b/cryptoki/src/context/mod.rs @@ -30,11 +30,9 @@ pub use locking::*; use crate::error::{Error, Result, Rv}; -use log::error; use std::fmt; use std::path::Path; use std::ptr; -use std::sync::RwLock; /// Enum for various function lists /// Each following is super-set of the previous one with overlapping start so we store them @@ -74,28 +72,6 @@ impl Pkcs11Impl { FunctionList::V3_2(l) => l, } } - - // Private finalize call - #[inline(always)] - fn finalize(&self) -> Result<()> { - unsafe { - Rv::from(self - .get_function_list() - .C_Finalize - .ok_or(Error::NullFunctionPointer)?( - ptr::null_mut() - )) - .into_result(Function::Finalize) - } - } -} - -impl Drop for Pkcs11Impl { - fn drop(&mut self) { - if let Err(err) = self.finalize() { - error!("Failed to finalize: {err}"); - } - } } /// Main PKCS11 context. Should usually be unique per application. @@ -202,8 +178,9 @@ impl Pkcs11 { } /// Finalize the PKCS11 library. Indicates that the application no longer needs to use PKCS11. - /// The library is also automatically finalized on drop. - pub fn finalize(self) {} + pub fn finalize(self) -> Result<()> { + finalize(self) + } /// Returns the information about the library pub fn get_library_info(&self) -> Result { From cc5308f45d61c6df669eb27af5c608f5caf84b11 Mon Sep 17 00:00:00 2001 From: Hugues de Valon Date: Mon, 24 Nov 2025 13:49:47 +0100 Subject: [PATCH 3/5] Remove the initialize check Callers can directly use the initialize function and check the return code to see if the initialization already happened. Signed-off-by: Hugues de Valon --- cryptoki/src/context/mod.rs | 15 +-------------- cryptoki/src/error/mod.rs | 7 +------ 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/cryptoki/src/context/mod.rs b/cryptoki/src/context/mod.rs index cb2b25b8..4a18bea0 100644 --- a/cryptoki/src/context/mod.rs +++ b/cryptoki/src/context/mod.rs @@ -78,7 +78,6 @@ impl Pkcs11Impl { #[derive(Debug)] pub struct Pkcs11 { pub(crate) impl_: Pkcs11Impl, - initialized: RwLock, } impl Pkcs11 { @@ -133,7 +132,6 @@ impl Pkcs11 { _pkcs11_lib: pkcs11_lib, function_list: FunctionList::V3_2(*list32_ptr), }, - initialized: RwLock::new(false), }); } let list30_ptr: *mut cryptoki_sys::CK_FUNCTION_LIST_3_0 = @@ -143,7 +141,6 @@ impl Pkcs11 { _pkcs11_lib: pkcs11_lib, function_list: FunctionList::V3_0(v30tov32(*list30_ptr)), }, - initialized: RwLock::new(false), }); } /* fall back to the 2.* API */ @@ -159,22 +156,12 @@ impl Pkcs11 { _pkcs11_lib: pkcs11_lib, function_list: FunctionList::V2(v2tov3(*list_ptr)), }, - initialized: RwLock::new(false), }) } /// Initialize the PKCS11 library pub fn initialize(&self, init_args: CInitializeArgs) -> Result<()> { - let mut init_lock = self.initialized.write().expect("lock not to be poisoned"); - if *init_lock { - Err(Error::AlreadyInitialized)? - } - initialize(self, init_args).map(|_| *init_lock = true) - } - - /// Check whether the PKCS11 library has been initialized - pub fn is_initialized(&self) -> bool { - *self.initialized.read().expect("lock not to be poisoned") + initialize(self, init_args) } /// Finalize the PKCS11 library. Indicates that the application no longer needs to use PKCS11. diff --git a/cryptoki/src/error/mod.rs b/cryptoki/src/error/mod.rs index be2c647d..353076ca 100644 --- a/cryptoki/src/error/mod.rs +++ b/cryptoki/src/error/mod.rs @@ -48,9 +48,6 @@ pub enum Error { /// The PIN was not set before logging in. PinNotSet, - - /// The PKCS11 library has already been initialized - AlreadyInitialized, } impl fmt::Display for Error { @@ -67,7 +64,6 @@ impl fmt::Display for Error { Error::NullFunctionPointer => write!(f, "Calling a NULL function pointer"), Error::InvalidValue => write!(f, "The value is not one of the expected options"), Error::PinNotSet => write!(f, "Pin has not been set before trying to log in"), - Error::AlreadyInitialized => write!(f, "PKCS11 library has already been initialized"), } } } @@ -85,8 +81,7 @@ impl std::error::Error for Error { | Error::NotSupported | Error::NullFunctionPointer | Error::PinNotSet - | Error::InvalidValue - | Error::AlreadyInitialized => None, + | Error::InvalidValue => None, } } } From 2afc201e79e9fd11bcd3550da307fec83d44d078 Mon Sep 17 00:00:00 2001 From: Hugues de Valon Date: Mon, 24 Nov 2025 13:50:39 +0100 Subject: [PATCH 4/5] Allow closing a session manually Currently a session can only be closed on drop. Signed-off-by: Hugues de Valon --- cryptoki/src/session/mod.rs | 7 ++++-- cryptoki/src/session/session_management.rs | 27 ++++++++++++---------- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/cryptoki/src/session/mod.rs b/cryptoki/src/session/mod.rs index 786e8622..e4dcb767 100644 --- a/cryptoki/src/session/mod.rs +++ b/cryptoki/src/session/mod.rs @@ -4,6 +4,7 @@ use crate::context::Pkcs11; +use crate::error::Result; use cryptoki_sys::*; use std::fmt::Formatter; use std::marker::PhantomData; @@ -72,7 +73,9 @@ impl<'a> Session<'a> { impl<'a> Session<'a> { /// Close a session /// This will be called on drop as well. - pub fn close(self) {} + pub fn close(self) -> Result<()> { + self.close_inner() + } /// Get the raw handle of the session. pub fn handle(&self) -> CK_SESSION_HANDLE { @@ -80,7 +83,7 @@ impl<'a> Session<'a> { } pub(crate) fn client(&self) -> &Pkcs11 { - &self.client + self.client } } diff --git a/cryptoki/src/session/session_management.rs b/cryptoki/src/session/session_management.rs index f8624bfa..9aa1ee01 100644 --- a/cryptoki/src/session/session_management.rs +++ b/cryptoki/src/session/session_management.rs @@ -3,7 +3,7 @@ //! Session management functions use crate::context::Function; -use crate::error::{Result, Rv}; +use crate::error::{Error, Result, Rv, RvError}; use crate::session::{Session, SessionInfo, UserType}; use crate::types::{AuthPin, RawAuthPin}; @@ -16,19 +16,13 @@ use std::convert::{TryFrom, TryInto}; impl Drop for Session<'_> { fn drop(&mut self) { - #[inline(always)] - fn close(session: &Session) -> Result<()> { - unsafe { - Rv::from(get_pkcs11!(session.client(), C_CloseSession)( - session.handle(), - )) - .into_result(Function::CloseSession) + match self.close_inner() { + Err(Error::Pkcs11(RvError::SessionClosed, Function::CloseSession)) => (), // the session has already been closed: ignore. + Ok(()) => (), + Err(err) => { + error!("Failed to close session: {err}"); } } - - if let Err(err) = close(self) { - error!("Failed to close session: {err}"); - } } } @@ -105,4 +99,13 @@ impl Session<'_> { SessionInfo::try_from(session_info) } } + + // Helper function to be able to close a session only taking a reference. + // This is used in the Drop trait function which only takes a reference as input. + pub(super) fn close_inner(&self) -> Result<()> { + unsafe { + Rv::from(get_pkcs11!(self.client(), C_CloseSession)(self.handle())) + .into_result(Function::CloseSession) + } + } } From f3581ac935899be2245fb99f3ae4f7eb5a6c18c5 Mon Sep 17 00:00:00 2001 From: Hugues de Valon Date: Mon, 24 Nov 2025 13:50:59 +0100 Subject: [PATCH 5/5] Finalize Pkcs11 and sessions in tests Signed-off-by: Hugues de Valon --- cryptoki/tests/basic.rs | 270 ++++++++++++++++++++++++++++++++------ cryptoki/tests/ml_dsa.rs | 12 ++ cryptoki/tests/ml_kem.rs | 3 + cryptoki/tests/slh_dsa.rs | 12 ++ 4 files changed, 258 insertions(+), 39 deletions(-) diff --git a/cryptoki/tests/basic.rs b/cryptoki/tests/basic.rs index ebcb631d..e55dcf42 100644 --- a/cryptoki/tests/basic.rs +++ b/cryptoki/tests/basic.rs @@ -80,6 +80,9 @@ fn sign_verify() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -123,6 +126,9 @@ fn sign_verify_eddsa() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -173,6 +179,9 @@ fn sign_verify_eddsa_with_ed25519_schemes() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -221,6 +230,9 @@ fn sign_verify_eddsa_with_ed448_schemes() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -274,6 +286,9 @@ fn sign_verify_multipart() -> TestResult { session.destroy_object(pub_key)?; session.destroy_object(priv_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -330,6 +345,9 @@ fn sign_verify_multipart_not_initialized() -> TestResult { Error::Pkcs11(_, Function::VerifyFinal) )); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -391,6 +409,9 @@ fn sign_verify_multipart_already_initialized() -> TestResult { session.destroy_object(pub_key)?; session.destroy_object(priv_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -448,6 +469,9 @@ fn encrypt_decrypt() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -501,6 +525,9 @@ fn encrypt_decrypt_multipart() -> TestResult { // Delete key session.destroy_object(key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -559,6 +586,9 @@ fn encrypt_decrypt_multipart_not_initialized() -> TestResult { Error::Pkcs11(_, Function::DecryptFinal) )); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -609,6 +639,9 @@ fn encrypt_decrypt_multipart_already_initialized() -> TestResult { // Delete key session.destroy_object(key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -693,6 +726,9 @@ fn derive_key() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -778,6 +814,9 @@ fn derive_key_sp800() -> TestResult { // delete keys session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -853,6 +892,9 @@ fn derive_key_concatenation_two_keys() -> TestResult { session.destroy_object(key2)?; session.destroy_object(derived_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -933,6 +975,9 @@ fn derive_key_concatenation_key_and_data() -> TestResult { session.destroy_object(derived_key1)?; session.destroy_object(derived_key2)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -992,6 +1037,9 @@ fn derive_key_xor_key_and_data() -> TestResult { session.destroy_object(key)?; session.destroy_object(derived_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -1060,6 +1108,9 @@ fn derive_key_extract_from_key() -> TestResult { session.destroy_object(key)?; session.destroy_object(derived_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -1117,6 +1168,9 @@ fn import_export() -> TestResult { // delete key session.destroy_object(is_it_the_public_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -1127,6 +1181,8 @@ fn get_token_info() -> TestResult { let info = pkcs11.get_token_info(slot)?; assert_ne!("", info.manufacturer_id()); + pkcs11.finalize()?; + Ok(()) } @@ -1182,6 +1238,9 @@ fn session_find_objects() -> testresult::TestResult { session.destroy_object(found_keys.pop().unwrap())?; let found_keys = session.find_objects(&key_search_template)?; assert_eq!(found_keys.len(), 9); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -1266,14 +1325,19 @@ fn session_objecthandle_iterator() -> testresult::TestResult { assert_eq!(found_keys, 9); // test interleaved iterators - the second iterator should fail - let iter = session.iter_objects(&key_search_template); - let iter2 = session.iter_objects(&key_search_template); + { + let iter = session.iter_objects(&key_search_template); + let iter2 = session.iter_objects(&key_search_template); + + assert!(iter.is_ok()); + assert!(matches!( + iter2, + Err(Error::Pkcs11(RvError::OperationActive, _)) + )); + } + session.close()?; + pkcs11.finalize()?; - assert!(iter.is_ok()); - assert!(matches!( - iter2, - Err(Error::Pkcs11(RvError::OperationActive, _)) - )); Ok(()) } @@ -1524,6 +1588,8 @@ fn get_info_test() -> TestResult { "Only 3.0 and 3.2 versions are expected but got 3.{minor}" ); } + pkcs11.finalize()?; + Ok(()) } @@ -1536,6 +1602,8 @@ fn get_slot_info_test() -> TestResult { assert!(!slot_info.hardware_slot()); assert!(!slot_info.removable_device()); assert_ne!("", slot_info.manufacturer_id()); + pkcs11.finalize()?; + Ok(()) } @@ -1592,6 +1660,9 @@ fn get_session_info_test() -> TestResult { SessionState::RwSecurityOfficer )); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -1615,6 +1686,9 @@ fn generate_random_test() -> TestResult { assert_eq!(random_vec.len(), 32); assert!(!random_vec.iter().all(|&x| x == 0)); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -1633,6 +1707,9 @@ fn set_pin_test() -> TestResult { session.logout()?; session.login(UserType::User, Some(&new_user_pin))?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -1735,6 +1812,9 @@ fn get_attribute_info_test() -> TestResult { _ => panic!("Private Exponent of RSA private key should be sensitive"), } + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -1773,57 +1853,54 @@ fn is_fn_supported_test() { #[test] #[serial] -fn is_initialized_test() { +fn is_initialized_test() -> TestResult { use cryptoki::context::CInitializeArgs; let pkcs11 = get_pkcs11(); - assert!( - !pkcs11.is_initialized(), - "Context created with initialized flag on" - ); - // initialize the library - pkcs11.initialize(CInitializeArgs::OsThreads).unwrap(); - - assert!( - pkcs11.is_initialized(), - "Context was not marked as initialized" - ); + pkcs11.initialize(CInitializeArgs::OsThreads)?; match pkcs11.initialize(CInitializeArgs::OsThreads) { - Err(Error::AlreadyInitialized) => (), + Err(Error::Pkcs11(RvError::CryptokiAlreadyInitialized, Function::Initialize)) => (), // expected Err(e) => panic!("Got unexpected error when initializing: {e}"), Ok(()) => panic!("Initializing twice should not have been allowed"), } + + pkcs11.finalize()?; + + Ok(()) } #[test] #[serial] #[allow(clippy::redundant_clone)] -fn test_clone_initialize() { +fn test_clone_initialize() -> TestResult { use cryptoki::context::CInitializeArgs; let pkcs11 = Arc::new(get_pkcs11()); - let clone = pkcs11.clone(); - assert!( - !pkcs11.is_initialized(), - "Before initialize() it should not be initialized" - ); - assert!( - !clone.is_initialized(), - "Before initialize() the clone should not be initialized" - ); - pkcs11.initialize(CInitializeArgs::OsThreads).unwrap(); - assert!( - pkcs11.is_initialized(), - "After initialize() it should be initialized" - ); - assert!( - clone.is_initialized(), - "After initialize() the clone should be initialized" - ); + { + let clone = pkcs11.clone(); + + pkcs11.initialize(CInitializeArgs::OsThreads)?; + + match clone.initialize(CInitializeArgs::OsThreads) { + Err(Error::Pkcs11(RvError::CryptokiAlreadyInitialized, Function::Initialize)) => (), // expected + Err(e) => panic!("Got unexpected error when initializing: {e}"), + Ok(()) => panic!("Initializing twice should not have been allowed"), + } + + match pkcs11.initialize(CInitializeArgs::OsThreads) { + Err(Error::Pkcs11(RvError::CryptokiAlreadyInitialized, Function::Initialize)) => (), // expected + Err(e) => panic!("Got unexpected error when initializing: {e}"), + Ok(()) => panic!("Initializing twice should not have been allowed"), + } + } + + Arc::into_inner(pkcs11).unwrap().finalize()?; + + Ok(()) } #[test] @@ -1869,6 +1946,9 @@ fn aes_key_attributes_test() -> TestResult { panic!("First attribute was not an end date"); } + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -1927,6 +2007,8 @@ fn ro_rw_session_test() -> TestResult { rw_session.logout()?; } + pkcs11.finalize()?; + Ok(()) } @@ -1977,6 +2059,9 @@ fn session_copy_object() -> TestResult { rw_session.destroy_object(object)?; rw_session.logout()?; + rw_session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2007,6 +2092,9 @@ fn aes_cbc_encrypt() -> TestResult { let mechanism = Mechanism::AesCbc(iv); let cipher = session.encrypt(&mechanism, key_handle, &plain)?; assert_eq!(expected_cipher[..], cipher[..]); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2038,6 +2126,9 @@ fn aes_cbc_pad_encrypt() -> TestResult { let mechanism = Mechanism::AesCbcPad(iv); let cipher = session.encrypt(&mechanism, key_handle, &plain)?; assert_eq!(expected_cipher[..], cipher[..]); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2081,6 +2172,9 @@ fn update_attributes_key() -> TestResult { panic!("Last attribute was not extractable"); } + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2106,6 +2200,9 @@ fn sha256_digest() -> TestResult { let have = session.digest(&Mechanism::Sha256, &data)?; assert_eq!(want[..], have[..]); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2138,6 +2235,9 @@ fn sha256_digest_multipart() -> TestResult { assert_eq!(have, want); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2187,6 +2287,9 @@ fn sha256_digest_multipart_with_key() -> TestResult { // Delete key session.destroy_object(key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2222,6 +2325,9 @@ fn sha256_digest_multipart_not_initialized() -> TestResult { Error::Pkcs11(_, Function::DigestFinal) )); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2244,6 +2350,9 @@ fn sha256_digest_multipart_already_initialized() -> TestResult { Error::Pkcs11(RvError::OperationActive, Function::DigestInit) )); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2295,6 +2404,9 @@ fn aes_gcm_no_aad() -> TestResult { let mechanism = Mechanism::AesGcm(GcmParams::new(&mut iv, &aad, 96.into())?); let cipher_and_tag = session.encrypt(&mechanism, key_handle, &plain)?; assert_eq!(expected_cipher_and_tag[..], cipher_and_tag[..]); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2327,6 +2439,9 @@ fn aes_gcm_with_aad() -> TestResult { let mechanism = Mechanism::AesGcm(gcm_params); let cipher_and_tag = session.encrypt(&mechanism, key_handle, &plain)?; assert_eq!(expected_cipher_and_tag[..], cipher_and_tag[..]); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2387,6 +2502,9 @@ fn encrypt_decrypt_gcm_message_no_aad() -> TestResult { let plain_decrypted = session.decrypt_message(¶m2, &aad, &cipher)?; assert_eq!(plain_decrypted[..], plain[..]); session.message_decrypt_final()?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2446,6 +2564,9 @@ fn encrypt_decrypt_gcm_message_with_aad() -> TestResult { let plain_decrypted = session.decrypt_message(¶m2, &aad, &cipher)?; assert_eq!(plain_decrypted[..], plain[..]); session.message_decrypt_final()?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2479,6 +2600,9 @@ fn rsa_pkcs_oaep_empty() -> TestResult { let decrypted = String::from_utf8(decrypted_data)?; assert_eq!("Hello", decrypted); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2517,6 +2641,9 @@ fn rsa_pkcs_oaep_with_data() -> TestResult { let decrypted = String::from_utf8(decrypted_data)?; assert_eq!("Hello", decrypted); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2533,6 +2660,8 @@ fn get_slot_event() -> TestResult { // Not implemented in Kryoptic pkcs11.get_slot_event().unwrap_err(); } + pkcs11.finalize()?; + Ok(()) } @@ -2578,6 +2707,9 @@ fn generate_generic_secret_key() -> TestResult { let attributes_result = session.find_objects(&[key_label])?.remove(0); assert_eq!(key, attributes_result); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2634,6 +2766,9 @@ fn ekdf_aes_cbc_encrypt_data() -> TestResult { session.find_objects(&[derived_key_label])?.remove(0) ); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2727,6 +2862,9 @@ fn kbkdf_counter_mode() -> TestResult { session.destroy_object(derived_key)?; session.destroy_object(base_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2844,6 +2982,9 @@ fn kbkdf_feedback_mode() -> TestResult { } session.destroy_object(base_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -2929,6 +3070,9 @@ fn kbkdf_double_pipeline_mode() -> TestResult { session.destroy_object(derived_key)?; session.destroy_object(base_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -3085,6 +3229,9 @@ fn kbkdf_additional_keys_counter_mode() -> TestResult { } session.destroy_object(base_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -3275,6 +3422,9 @@ fn kbkdf_additional_keys_feedback_mode() -> TestResult { } session.destroy_object(base_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -3423,6 +3573,9 @@ fn kbkdf_additional_keys_double_pipeline_mode() -> TestResult { } session.destroy_object(base_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -3572,6 +3725,9 @@ fn kbkdf_invalid_data_params_counter_mode() -> TestResult { // Delete base key session.destroy_object(base_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -3695,6 +3851,9 @@ fn kbkdf_invalid_data_params_feedback_mode() -> TestResult { // Delete base key session.destroy_object(base_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -3818,6 +3977,9 @@ fn kbkdf_invalid_data_params_double_pipeline_mode() -> TestResult { // Delete base key session.destroy_object(base_key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -3854,6 +4016,9 @@ fn sign_verify_sha1_hmac() -> TestResult { session.verify(&Mechanism::Sha1Hmac, private, &data, &signature)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -3890,6 +4055,9 @@ fn sign_verify_sha224_hmac() -> TestResult { session.verify(&Mechanism::Sha224Hmac, private, &data, &signature)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -3926,6 +4094,9 @@ fn sign_verify_sha256_hmac() -> TestResult { session.verify(&Mechanism::Sha256Hmac, private, &data, &signature)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -3962,6 +4133,9 @@ fn sign_verify_sha384_hmac() -> TestResult { session.verify(&Mechanism::Sha384Hmac, private, &data, &signature)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -3998,6 +4172,9 @@ fn sign_verify_sha512_hmac() -> TestResult { session.verify(&Mechanism::Sha512Hmac, private, &data, &signature)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -4071,6 +4248,9 @@ fn aes_cmac_sign_impl(key: [u8; 16], message: &[u8], expected_mac: [u8; 16]) -> let signature = session.sign(&Mechanism::AesCMac, key, message)?; assert_eq!(expected_mac.as_slice(), signature.as_slice()); + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -4142,6 +4322,9 @@ fn aes_cmac_verify_impl(key: [u8; 16], message: &[u8], expected_mac: [u8; 16]) - ]; let key = session.create_object(&key_template)?; session.verify(&Mechanism::AesCMac, key, message, &expected_mac)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -4217,6 +4400,9 @@ fn unique_id() -> TestResult { session.destroy_object(key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -4296,6 +4482,9 @@ fn validation() -> TestResult { session.destroy_object(key)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -4348,5 +4537,8 @@ fn object_handle_new_from_raw() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } diff --git a/cryptoki/tests/ml_dsa.rs b/cryptoki/tests/ml_dsa.rs index 1fcea1ca..6abcc0b8 100644 --- a/cryptoki/tests/ml_dsa.rs +++ b/cryptoki/tests/ml_dsa.rs @@ -115,6 +115,9 @@ fn ml_dsa() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -185,6 +188,9 @@ fn ml_dsa_multipart() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -280,6 +286,9 @@ fn ml_dsa_hash() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -360,5 +369,8 @@ fn ml_dsa_hashes() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } diff --git a/cryptoki/tests/ml_kem.rs b/cryptoki/tests/ml_kem.rs index 2733ac01..72b5828c 100644 --- a/cryptoki/tests/ml_kem.rs +++ b/cryptoki/tests/ml_kem.rs @@ -112,5 +112,8 @@ fn ml_kem() -> TestResult { session.destroy_object(secret)?; session.destroy_object(secret2)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } diff --git a/cryptoki/tests/slh_dsa.rs b/cryptoki/tests/slh_dsa.rs index 43117486..a06e9c43 100644 --- a/cryptoki/tests/slh_dsa.rs +++ b/cryptoki/tests/slh_dsa.rs @@ -109,6 +109,9 @@ fn slh_dsa() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -174,6 +177,9 @@ fn slh_dsa_multipart() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -264,6 +270,9 @@ fn slh_dsa_hash() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) } @@ -339,5 +348,8 @@ fn slh_dsa_hashes() -> TestResult { session.destroy_object(public)?; session.destroy_object(private)?; + session.close()?; + pkcs11.finalize()?; + Ok(()) }