From 4cb9d0035ab33323b5ec57049be3610bf5605bb7 Mon Sep 17 00:00:00 2001 From: Eric Devolder Date: Mon, 2 Sep 2024 21:44:29 +0200 Subject: [PATCH 1/8] implements session object handle iterator, with caching Signed-off-by: Eric Devolder --- cryptoki/src/context/mod.rs | 8 + cryptoki/src/session/mod.rs | 4 + cryptoki/src/session/object_management.rs | 216 +++++++++++++++++++++- cryptoki/tests/basic.rs | 98 +++++++++- 4 files changed, 324 insertions(+), 2 deletions(-) diff --git a/cryptoki/src/context/mod.rs b/cryptoki/src/context/mod.rs index 160b9cb..723078f 100644 --- a/cryptoki/src/context/mod.rs +++ b/cryptoki/src/context/mod.rs @@ -13,6 +13,14 @@ macro_rules! get_pkcs11 { }; } +/// Same as get_pkcs11! but does not attempt to apply '?' syntactic sugar. +/// Suitable only if the caller can't return a Result. +macro_rules! get_pkcs11_func { + ($pkcs11:expr, $func_name:ident) => { + ($pkcs11.impl_.function_list.$func_name) + }; +} + mod general_purpose; mod info; mod locking; diff --git a/cryptoki/src/session/mod.rs b/cryptoki/src/session/mod.rs index 77d9f79..83b7205 100644 --- a/cryptoki/src/session/mod.rs +++ b/cryptoki/src/session/mod.rs @@ -19,6 +19,7 @@ mod session_management; mod signing_macing; mod slot_token_management; +pub use object_management::ObjectHandleIterator; pub use session_info::{SessionInfo, SessionState}; /// Type that identifies a session @@ -31,6 +32,8 @@ pub use session_info::{SessionInfo, SessionState}; pub struct Session { handle: CK_SESSION_HANDLE, client: Pkcs11, + #[allow(dead_code)] + search_active: bool, // This is not used but to prevent Session to automatically implement Send and Sync _guard: PhantomData<*mut u32>, } @@ -62,6 +65,7 @@ impl Session { Session { handle, client, + search_active: false, _guard: PhantomData, } } diff --git a/cryptoki/src/session/object_management.rs b/cryptoki/src/session/object_management.rs index 350fce6..2bccf34 100644 --- a/cryptoki/src/session/object_management.rs +++ b/cryptoki/src/session/object_management.rs @@ -3,7 +3,7 @@ //! Object management functions use crate::context::Function; -use crate::error::{Result, Rv, RvError}; +use crate::error::{Error, Result, Rv, RvError}; use crate::object::{Attribute, AttributeInfo, AttributeType, ObjectHandle}; use crate::session::Session; use cryptoki_sys::*; @@ -13,7 +13,221 @@ use std::convert::TryInto; // Search 10 elements at a time const MAX_OBJECT_COUNT: usize = 10; +/// Iterator over object handles, in an active session. +/// +/// Used to iterate over the object handles returned by underlying calls to `C_FindObjects`. +/// The iterator is created by calling the `iter_objects` and `iter_objects_with_cache_size` methods on a `Session` object. +/// +/// # Example +/// +/// ```no_run +/// use cryptoki::context::CInitializeArgs; +/// use cryptoki::context::Pkcs11; +/// use cryptoki::error::Error; +/// use cryptoki::object::Attribute; +/// use cryptoki::object::AttributeType; +/// use cryptoki::session::UserType; +/// use cryptoki::types::AuthPin; +/// use std::env; +/// +/// fn test() -> Result<(), Error> { +/// let pkcs11 = Pkcs11::new( +/// env::var("PKCS11_SOFTHSM2_MODULE") +/// .unwrap_or_else(|_| "/usr/local/lib/libsofthsm2.so".to_string()), +/// )?; +/// +/// pkcs11.initialize(CInitializeArgs::OsThreads)?; +/// let slot = pkcs11.get_slots_with_token()?.remove(0); +/// +/// let session = pkcs11.open_ro_session(slot).unwrap(); +/// session.login(UserType::User, Some(&AuthPin::new("fedcba".into())))?; +/// +/// let token_object = vec![Attribute::Token(true)]; +/// let wanted_attr = vec![AttributeType::Label]; +/// +/// for (idx, obj) in session.iter_objects(&token_object)?.enumerate() { +/// let obj = obj?; // handle potential error condition +/// +/// let attributes = session.get_attributes(obj, &wanted_attr)?; +/// +/// match attributes.get(0) { +/// Some(Attribute::Label(l)) => { +/// println!( +/// "token object #{}: handle {}, label {}", +/// idx, +/// obj, +/// String::from_utf8(l.to_vec()) +/// .unwrap_or_else(|_| "*** not valid utf8 ***".to_string()) +/// ); +/// } +/// _ => { +/// println!("token object #{}: handle {}, label not found", idx, obj); +/// } +/// } +/// } +/// Ok(()) +/// } +/// +/// pub fn main() { +/// test().unwrap(); +/// } +/// ``` +#[derive(Debug)] +pub struct ObjectHandleIterator<'a> { + session: &'a Session, + object_count: usize, + index: usize, + cache: Vec, +} + +impl<'a> ObjectHandleIterator<'a> { + fn new( + session: &'a Session, + mut template: Vec, + cache_size: usize, + ) -> Result { + if cache_size == 0 { + return Err(Error::InvalidValue); + } + + unsafe { + Rv::from(get_pkcs11!(session.client(), C_FindObjectsInit)( + session.handle(), + template.as_mut_ptr(), + template.len().try_into()?, + )) + .into_result(Function::FindObjectsInit)?; + } + + let cache: Vec = vec![0; cache_size]; + Ok(ObjectHandleIterator { + session, + object_count: cache_size, + index: cache_size, + cache, + }) + } +} + +// In this implementation, we use object_count to keep track of the number of objects +// returned by the last C_FindObjects call; the index is used to keep track of +// the next object in the cache to be returned. The size of cache is never changed. +// In order to enter the loop for the first time, we set object_count to cache_size +// and index to cache_size. That allows to jump directly to the C_FindObjects call +// and start filling the cache. + +impl<'a> Iterator for ObjectHandleIterator<'a> { + type Item = Result; + + fn next(&mut self) -> Option { + // since the iterator is initialized with object_count and index both equal and > 0, + // we are guaranteed to enter the loop at least once + while self.object_count > 0 { + // if index { + p11rv = unsafe { + f( + self.session.handle(), + self.cache.as_mut_ptr(), + self.cache.len() as CK_ULONG, + &mut self.object_count as *mut usize as CK_ULONG_PTR, + ) + }; + } + None => { + // C_FindObjects() is not implemented on this implementation + // sort of unexpected. TODO: Consider panic!() instead? + log::error!("C_FindObjects() is not implemented on this library"); + return Some(Err(Error::NullFunctionPointer) as Result); + } + } + + if let Rv::Error(error) = Rv::from(p11rv) { + return Some( + Err(Error::Pkcs11(error, Function::FindObjects)) as Result + ); + } + } + None + } +} + +impl Drop for ObjectHandleIterator<'_> { + fn drop(&mut self) { + // silently pass if C_FindObjectsFinal() is not implemented on this implementation + // this is unexpected. TODO: Consider panic!() instead? + if let Some(f) = get_pkcs11_func!(self.session.client(), C_FindObjectsFinal) { + // swallow the return value, as we can't do anything about it + let _ = unsafe { f(self.session.handle()) }; + } + } +} + impl Session { + /// Iterate over session objects matching a template. + /// + /// # Arguments + /// * `template` - The template to match objects against + /// + /// # Returns + /// + /// This function will return a [`Result`] that can be used to iterate over the objects + /// matching the template. Note that the cache size is managed internally and set to a default value (10) + /// + /// # See also + /// * [`ObjectHandleIterator`] for more information on how to use the iterator + /// * [`Session::iter_objects_with_cache_size`] for a way to specify the cache size + #[inline(always)] + pub fn iter_objects(&self, template: &[Attribute]) -> Result { + self.iter_objects_with_cache_size(template, MAX_OBJECT_COUNT) + } + + /// Iterate over session objects matching a template, with cache size + /// + /// # Arguments + /// * `template` - The template to match objects against + /// * `cache_size` - The number of objects to cache. Note that 0 is an invalid value and will return an error. + /// + /// # Returns + /// + /// This function will return a [`Result`] that can be used to iterate over the objects + /// matching the template. The cache size corresponds to the size of the array provided to `C_FindObjects()`. + /// + /// # See also + /// * [`ObjectHandleIterator`] for more information on how to use the iterator + /// * [`Session::iter_objects`] for a simpler way to iterate over objects + pub fn iter_objects_with_cache_size( + &self, + template: &[Attribute], + cache_size: usize, + ) -> Result { + let template: Vec = template.iter().map(|attr| attr.into()).collect(); + ObjectHandleIterator::new(self, template, cache_size) + } + /// Search for session objects matching a template /// /// # Arguments diff --git a/cryptoki/tests/basic.rs b/cryptoki/tests/basic.rs index 02a83a6..11c0345 100644 --- a/cryptoki/tests/basic.rs +++ b/cryptoki/tests/basic.rs @@ -9,7 +9,9 @@ use cryptoki::error::{Error, RvError}; use cryptoki::mechanism::aead::GcmParams; use cryptoki::mechanism::rsa::{PkcsMgfType, PkcsOaepParams, PkcsOaepSource}; use cryptoki::mechanism::{Mechanism, MechanismType}; -use cryptoki::object::{Attribute, AttributeInfo, AttributeType, KeyType, ObjectClass}; +use cryptoki::object::{ + Attribute, AttributeInfo, AttributeType, KeyType, ObjectClass, ObjectHandle, +}; use cryptoki::session::{SessionState, UserType}; use cryptoki::types::AuthPin; use serial_test::serial; @@ -364,6 +366,100 @@ fn session_find_objects() { assert_eq!(found_keys.len(), 9); } +#[test] +#[serial] +fn session_objecthandle_iterator() { + let (pkcs11, slot) = init_pins(); + // open a session + let session = pkcs11.open_rw_session(slot).unwrap(); + + // log in the session + session + .login(UserType::User, Some(&AuthPin::new(USER_PIN.into()))) + .unwrap(); + + // we generate 11 keys with the same CKA_ID + + (1..=11).for_each(|i| { + let key_template = vec![ + Attribute::Token(true), + Attribute::Encrypt(true), + Attribute::Label(format!("key_{}", i).as_bytes().to_vec()), + Attribute::Id("12345678".as_bytes().to_vec()), // reusing the same CKA_ID + ]; + + // generate a secret key + let _key = session + .generate_key(&Mechanism::Des3KeyGen, &key_template) + .unwrap(); + }); + + // retrieve these keys using this template + let key_search_template = vec![ + Attribute::Token(true), + Attribute::Id("12345678".as_bytes().to_vec()), + Attribute::Class(ObjectClass::SECRET_KEY), + Attribute::KeyType(KeyType::DES3), + ]; + + // test iter_objects_with_cache_size() + // count keys with cache size of 20 + let found_keys = session + .iter_objects_with_cache_size(&key_search_template, 20) + .unwrap(); + let found_keys = found_keys.map_while(|key| key.ok()).count(); + assert_eq!(found_keys, 11); + + // count keys with cache size of 0 => should result in an error + let found_keys = session.iter_objects_with_cache_size(&key_search_template, 0); + assert!(found_keys.is_err()); + + // count keys with cache size of 1 + let found_keys = session + .iter_objects_with_cache_size(&key_search_template, 1) + .unwrap(); + let found_keys = found_keys.map_while(|key| key.ok()).count(); + assert_eq!(found_keys, 11); + + // count keys with cache size of 10 + let found_keys = session + .iter_objects_with_cache_size(&key_search_template, 10) + .unwrap(); + let found_keys = found_keys.map_while(|key| key.ok()).count(); + assert_eq!(found_keys, 11); + + // fetch keys into a vector + let found_keys: Vec = session + .iter_objects_with_cache_size(&key_search_template, 10) + .unwrap() + .map_while(|key| key.ok()) + .collect(); + assert_eq!(found_keys.len(), 11); + + let key0 = found_keys[0]; + let key1 = found_keys[1]; + + session.destroy_object(key0).unwrap(); + let found_keys = session + .iter_objects_with_cache_size(&key_search_template, 10) + .unwrap(); + let found_keys = found_keys.map_while(|key| key.ok()).count(); + assert_eq!(found_keys, 10); + + // destroy another key + session.destroy_object(key1).unwrap(); + let found_keys = session + .iter_objects_with_cache_size(&key_search_template, 10) + .unwrap(); + let found_keys = found_keys.map_while(|key| key.ok()).count(); + assert_eq!(found_keys, 9); + + // test iter_objects() + let found_keys = session.iter_objects(&key_search_template).unwrap(); + let found_keys = found_keys.map_while(|key| key.ok()).count(); + assert_eq!(found_keys, 9); +} + #[test] #[serial] fn wrap_and_unwrap_key() { From 1f72f1a8d056d29cfd1ff4ba583fea540584fcee Mon Sep 17 00:00:00 2001 From: Eric Devolder Date: Wed, 4 Sep 2024 17:53:07 +0200 Subject: [PATCH 2/8] adjustments required by clippy Signed-off-by: Eric Devolder --- cryptoki/src/session/object_management.rs | 24 ++++++++++------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/cryptoki/src/session/object_management.rs b/cryptoki/src/session/object_management.rs index 2bccf34..0459180 100644 --- a/cryptoki/src/session/object_management.rs +++ b/cryptoki/src/session/object_management.rs @@ -144,26 +144,22 @@ impl<'a> Iterator for ObjectHandleIterator<'a> { } } - let p11rv; - - match get_pkcs11_func!(self.session.client(), C_FindObjects) { - Some(f) => { - p11rv = unsafe { - f( - self.session.handle(), - self.cache.as_mut_ptr(), - self.cache.len() as CK_ULONG, - &mut self.object_count as *mut usize as CK_ULONG_PTR, - ) - }; - } + let p11rv = match get_pkcs11_func!(self.session.client(), C_FindObjects) { + Some(f) => unsafe { + f( + self.session.handle(), + self.cache.as_mut_ptr(), + self.cache.len() as CK_ULONG, + &mut self.object_count as *mut usize as CK_ULONG_PTR, + ) + }, None => { // C_FindObjects() is not implemented on this implementation // sort of unexpected. TODO: Consider panic!() instead? log::error!("C_FindObjects() is not implemented on this library"); return Some(Err(Error::NullFunctionPointer) as Result); } - } + }; if let Rv::Error(error) = Rv::from(p11rv) { return Some( From 737784415ba557e176dcd25b1e6e2313da9f659d Mon Sep 17 00:00:00 2001 From: Eric Devolder Date: Wed, 4 Sep 2024 17:58:00 +0200 Subject: [PATCH 3/8] removed dead code Signed-off-by: Eric Devolder --- cryptoki/src/session/mod.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/cryptoki/src/session/mod.rs b/cryptoki/src/session/mod.rs index 83b7205..7e3bbb8 100644 --- a/cryptoki/src/session/mod.rs +++ b/cryptoki/src/session/mod.rs @@ -32,8 +32,6 @@ pub use session_info::{SessionInfo, SessionState}; pub struct Session { handle: CK_SESSION_HANDLE, client: Pkcs11, - #[allow(dead_code)] - search_active: bool, // This is not used but to prevent Session to automatically implement Send and Sync _guard: PhantomData<*mut u32>, } @@ -65,7 +63,6 @@ impl Session { Session { handle, client, - search_active: false, _guard: PhantomData, } } From e42c41c2cc46a00f15ed91cd073ba663ce5c976a Mon Sep 17 00:00:00 2001 From: Eric Devolder Date: Fri, 6 Sep 2024 18:18:38 +0200 Subject: [PATCH 4/8] Refactor object handle iterator and find objects function - documentation enhanced - documentation example adjusted/simplified - tests simplifications - `find_objects()` method is now calling `iter_objects()` Signed-off-by: Eric Devolder --- cryptoki/src/session/object_management.rs | 140 +++++++++------------- cryptoki/tests/basic.rs | 67 +++++------ 2 files changed, 87 insertions(+), 120 deletions(-) diff --git a/cryptoki/src/session/object_management.rs b/cryptoki/src/session/object_management.rs index 0459180..847f7c1 100644 --- a/cryptoki/src/session/object_management.rs +++ b/cryptoki/src/session/object_management.rs @@ -18,6 +18,11 @@ const MAX_OBJECT_COUNT: usize = 10; /// Used to iterate over the object handles returned by underlying calls to `C_FindObjects`. /// The iterator is created by calling the `iter_objects` and `iter_objects_with_cache_size` methods on a `Session` object. /// +/// # Note +/// +/// The iterator `new()` method will call `C_FindObjectsInit`. It means that until the iterator is dropped, +/// creating another iterator will result in an error (typically `RvError::OperationActive` ). +/// /// # Example /// /// ```no_run @@ -30,47 +35,44 @@ const MAX_OBJECT_COUNT: usize = 10; /// use cryptoki::types::AuthPin; /// use std::env; /// -/// fn test() -> Result<(), Error> { -/// let pkcs11 = Pkcs11::new( -/// env::var("PKCS11_SOFTHSM2_MODULE") -/// .unwrap_or_else(|_| "/usr/local/lib/libsofthsm2.so".to_string()), -/// )?; +/// # fn main() -> testresult::TestResult { +/// let pkcs11 = Pkcs11::new( +/// env::var("PKCS11_SOFTHSM2_MODULE") +/// .unwrap_or_else(|_| "/usr/local/lib/libsofthsm2.so".to_string()), +/// )?; /// -/// pkcs11.initialize(CInitializeArgs::OsThreads)?; -/// let slot = pkcs11.get_slots_with_token()?.remove(0); +/// pkcs11.initialize(CInitializeArgs::OsThreads)?; +/// let slot = pkcs11.get_slots_with_token()?.remove(0); /// -/// let session = pkcs11.open_ro_session(slot).unwrap(); -/// session.login(UserType::User, Some(&AuthPin::new("fedcba".into())))?; +/// let session = pkcs11.open_ro_session(slot).unwrap(); +/// session.login(UserType::User, Some(&AuthPin::new("fedcba".into())))?; /// -/// let token_object = vec![Attribute::Token(true)]; -/// let wanted_attr = vec![AttributeType::Label]; +/// let token_object = vec![Attribute::Token(true)]; +/// let wanted_attr = vec![AttributeType::Label]; /// -/// for (idx, obj) in session.iter_objects(&token_object)?.enumerate() { -/// let obj = obj?; // handle potential error condition +/// for (idx, obj) in session.iter_objects(&token_object)?.enumerate() { +/// let obj = obj?; // handle potential error condition /// -/// let attributes = session.get_attributes(obj, &wanted_attr)?; +/// let attributes = session.get_attributes(obj, &wanted_attr)?; /// -/// match attributes.get(0) { -/// Some(Attribute::Label(l)) => { -/// println!( -/// "token object #{}: handle {}, label {}", -/// idx, -/// obj, -/// String::from_utf8(l.to_vec()) -/// .unwrap_or_else(|_| "*** not valid utf8 ***".to_string()) -/// ); -/// } -/// _ => { -/// println!("token object #{}: handle {}, label not found", idx, obj); -/// } +/// match attributes.get(0) { +/// Some(Attribute::Label(l)) => { +/// println!( +/// "token object #{}: handle {}, label {}", +/// idx, +/// obj, +/// String::from_utf8(l.to_vec()) +/// .unwrap_or_else(|_| "*** not valid utf8 ***".to_string()) +/// ); +/// } +/// _ => { +/// println!("token object #{}: handle {}, label not found", idx, obj); /// } /// } -/// Ok(()) /// } +/// # Ok(()) +/// # } /// -/// pub fn main() { -/// test().unwrap(); -/// } /// ``` #[derive(Debug)] pub struct ObjectHandleIterator<'a> { @@ -154,8 +156,7 @@ impl<'a> Iterator for ObjectHandleIterator<'a> { ) }, None => { - // C_FindObjects() is not implemented on this implementation - // sort of unexpected. TODO: Consider panic!() instead? + // C_FindObjects() is not implemented,, bark and return an error log::error!("C_FindObjects() is not implemented on this library"); return Some(Err(Error::NullFunctionPointer) as Result); } @@ -173,9 +174,9 @@ impl<'a> Iterator for ObjectHandleIterator<'a> { impl Drop for ObjectHandleIterator<'_> { fn drop(&mut self) { - // silently pass if C_FindObjectsFinal() is not implemented on this implementation - // this is unexpected. TODO: Consider panic!() instead? + // bark but pass if C_FindObjectsFinal() is not implemented if let Some(f) = get_pkcs11_func!(self.session.client(), C_FindObjectsFinal) { + log::error!("C_FindObjectsFinal() is not implemented on this library"); // swallow the return value, as we can't do anything about it let _ = unsafe { f(self.session.handle()) }; } @@ -220,17 +221,31 @@ impl Session { template: &[Attribute], cache_size: usize, ) -> Result { - let template: Vec = template.iter().map(|attr| attr.into()).collect(); + let template: Vec = template.iter().map(Into::into).collect(); ObjectHandleIterator::new(self, template, cache_size) } /// Search for session objects matching a template /// /// # Arguments + /// /// * `template` - A [Attribute] of search parameters that will be used /// to find objects. /// - /// # Examples + /// # Returns + /// + /// Upon success, a vector of [ObjectHandle] wrapped in a Result. + /// Upon failure, the first error encountered. + /// + /// # Note + /// + /// It is a convenience method that will call [`Session::iter_objects`] and collect the results. + /// + /// # See also + /// + /// * [`Session::iter_objects`] for a way to specify the cache size + + /// # Example /// /// ```rust /// # fn main() -> testresult::TestResult { @@ -260,54 +275,11 @@ impl Session { /// } /// # Ok(()) } /// ``` + /// + #[inline(always)] pub fn find_objects(&self, template: &[Attribute]) -> Result> { - let mut template: Vec = template.iter().map(|attr| attr.into()).collect(); - - unsafe { - Rv::from(get_pkcs11!(self.client(), C_FindObjectsInit)( - self.handle(), - template.as_mut_ptr(), - template.len().try_into()?, - )) - .into_result(Function::FindObjectsInit)?; - } - - let mut object_handles = [0; MAX_OBJECT_COUNT]; - let mut object_count = MAX_OBJECT_COUNT as CK_ULONG; // set to MAX_OBJECT_COUNT to enter loop - let mut objects = Vec::new(); - - // as long as the number of objects returned equals the maximum number - // of objects that can be returned, we keep calling C_FindObjects - while object_count == MAX_OBJECT_COUNT as CK_ULONG { - unsafe { - Rv::from(get_pkcs11!(self.client(), C_FindObjects)( - self.handle(), - object_handles.as_mut_ptr() as CK_OBJECT_HANDLE_PTR, - MAX_OBJECT_COUNT.try_into()?, - &mut object_count, - )) - .into_result(Function::FindObjects)?; - } - - // exit loop, no more objects to be returned, no need to extend the objects vector - if object_count == 0 { - break; - } - - // extend the objects vector with the new objects - objects.extend_from_slice(&object_handles[..object_count.try_into()?]); - } - - unsafe { - Rv::from(get_pkcs11!(self.client(), C_FindObjectsFinal)( - self.handle(), - )) - .into_result(Function::FindObjectsFinal)?; - } - - let objects = objects.into_iter().map(ObjectHandle::new).collect(); - - Ok(objects) + self.iter_objects(template)? + .collect::>>() } /// Create a new object diff --git a/cryptoki/tests/basic.rs b/cryptoki/tests/basic.rs index 11c0345..0793633 100644 --- a/cryptoki/tests/basic.rs +++ b/cryptoki/tests/basic.rs @@ -313,15 +313,13 @@ fn get_token_info() -> TestResult { #[test] #[serial] -fn session_find_objects() { +fn session_find_objects() -> testresult::TestResult { let (pkcs11, slot) = init_pins(); // open a session - let session = pkcs11.open_rw_session(slot).unwrap(); + let session = pkcs11.open_rw_session(slot)?; // log in the session - session - .login(UserType::User, Some(&AuthPin::new(USER_PIN.into()))) - .unwrap(); + session.login(UserType::User, Some(&AuthPin::new(USER_PIN.into())))?; // we generate 11 keys with the same CKA_ID // we will check 3 different use cases, this will cover all cases for Session.find_objects @@ -351,32 +349,31 @@ fn session_find_objects() { Attribute::KeyType(KeyType::DES3), ]; - let mut found_keys = session.find_objects(&key_search_template).unwrap(); + let mut found_keys = session.find_objects(&key_search_template)?; assert_eq!(found_keys.len(), 11); // destroy one key - session.destroy_object(found_keys.pop().unwrap()).unwrap(); + session.destroy_object(found_keys.pop().unwrap())?; - let mut found_keys = session.find_objects(&key_search_template).unwrap(); + let mut found_keys = session.find_objects(&key_search_template)?; assert_eq!(found_keys.len(), 10); // destroy another key - session.destroy_object(found_keys.pop().unwrap()).unwrap(); - let found_keys = session.find_objects(&key_search_template).unwrap(); + session.destroy_object(found_keys.pop().unwrap())?; + let found_keys = session.find_objects(&key_search_template)?; assert_eq!(found_keys.len(), 9); + Ok(()) } #[test] #[serial] -fn session_objecthandle_iterator() { +fn session_objecthandle_iterator() -> testresult::TestResult { let (pkcs11, slot) = init_pins(); // open a session - let session = pkcs11.open_rw_session(slot).unwrap(); + let session = pkcs11.open_rw_session(slot)?; // log in the session - session - .login(UserType::User, Some(&AuthPin::new(USER_PIN.into()))) - .unwrap(); + session.login(UserType::User, Some(&AuthPin::new(USER_PIN.into())))?; // we generate 11 keys with the same CKA_ID @@ -389,9 +386,7 @@ fn session_objecthandle_iterator() { ]; // generate a secret key - let _key = session - .generate_key(&Mechanism::Des3KeyGen, &key_template) - .unwrap(); + let _key = session.generate_key(&Mechanism::Des3KeyGen, &key_template); }); // retrieve these keys using this template @@ -404,9 +399,7 @@ fn session_objecthandle_iterator() { // test iter_objects_with_cache_size() // count keys with cache size of 20 - let found_keys = session - .iter_objects_with_cache_size(&key_search_template, 20) - .unwrap(); + let found_keys = session.iter_objects_with_cache_size(&key_search_template, 20)?; let found_keys = found_keys.map_while(|key| key.ok()).count(); assert_eq!(found_keys, 11); @@ -415,23 +408,18 @@ fn session_objecthandle_iterator() { assert!(found_keys.is_err()); // count keys with cache size of 1 - let found_keys = session - .iter_objects_with_cache_size(&key_search_template, 1) - .unwrap(); + let found_keys = session.iter_objects_with_cache_size(&key_search_template, 1)?; let found_keys = found_keys.map_while(|key| key.ok()).count(); assert_eq!(found_keys, 11); // count keys with cache size of 10 - let found_keys = session - .iter_objects_with_cache_size(&key_search_template, 10) - .unwrap(); + let found_keys = session.iter_objects_with_cache_size(&key_search_template, 10)?; let found_keys = found_keys.map_while(|key| key.ok()).count(); assert_eq!(found_keys, 11); // fetch keys into a vector let found_keys: Vec = session - .iter_objects_with_cache_size(&key_search_template, 10) - .unwrap() + .iter_objects_with_cache_size(&key_search_template, 10)? .map_while(|key| key.ok()) .collect(); assert_eq!(found_keys.len(), 11); @@ -440,24 +428,31 @@ fn session_objecthandle_iterator() { let key1 = found_keys[1]; session.destroy_object(key0).unwrap(); - let found_keys = session - .iter_objects_with_cache_size(&key_search_template, 10) - .unwrap(); + let found_keys = session.iter_objects_with_cache_size(&key_search_template, 10)?; let found_keys = found_keys.map_while(|key| key.ok()).count(); assert_eq!(found_keys, 10); // destroy another key session.destroy_object(key1).unwrap(); - let found_keys = session - .iter_objects_with_cache_size(&key_search_template, 10) - .unwrap(); + let found_keys = session.iter_objects_with_cache_size(&key_search_template, 10)?; let found_keys = found_keys.map_while(|key| key.ok()).count(); assert_eq!(found_keys, 9); // test iter_objects() - let found_keys = session.iter_objects(&key_search_template).unwrap(); + let found_keys = session.iter_objects(&key_search_template)?; let found_keys = found_keys.map_while(|key| key.ok()).count(); 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); + + assert!(matches!(iter, Ok(_))); + assert!(matches!( + iter2, + Err(Error::Pkcs11(RvError::OperationActive, _)) + )); + Ok(()) } #[test] From 0c33effdfa5d70e92de55ffc3cca7654bda463bb Mon Sep 17 00:00:00 2001 From: Eric Devolder Date: Fri, 6 Sep 2024 18:27:34 +0200 Subject: [PATCH 5/8] small refactor to satisfy clippy Signed-off-by: Eric Devolder --- cryptoki/tests/basic.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cryptoki/tests/basic.rs b/cryptoki/tests/basic.rs index 0793633..0ed6cb0 100644 --- a/cryptoki/tests/basic.rs +++ b/cryptoki/tests/basic.rs @@ -447,7 +447,7 @@ fn session_objecthandle_iterator() -> testresult::TestResult { let iter = session.iter_objects(&key_search_template); let iter2 = session.iter_objects(&key_search_template); - assert!(matches!(iter, Ok(_))); + assert!(iter.is_ok()); assert!(matches!( iter2, Err(Error::Pkcs11(RvError::OperationActive, _)) From fd10cc6b58dc7bf295776036ab1ed0489b00cdd9 Mon Sep 17 00:00:00 2001 From: Eric Devolder Date: Sat, 7 Sep 2024 03:09:54 +0200 Subject: [PATCH 6/8] use NonZeroUSize for capturing cache size Signed-off-by: Eric Devolder --- cryptoki/src/session/object_management.rs | 29 ++++++++++++----------- cryptoki/tests/basic.rs | 22 +++++++++-------- 2 files changed, 27 insertions(+), 24 deletions(-) diff --git a/cryptoki/src/session/object_management.rs b/cryptoki/src/session/object_management.rs index 847f7c1..cb16788 100644 --- a/cryptoki/src/session/object_management.rs +++ b/cryptoki/src/session/object_management.rs @@ -9,6 +9,7 @@ use crate::session::Session; use cryptoki_sys::*; use std::collections::HashMap; use std::convert::TryInto; +use std::num::NonZeroUsize; // Search 10 elements at a time const MAX_OBJECT_COUNT: usize = 10; @@ -86,12 +87,8 @@ impl<'a> ObjectHandleIterator<'a> { fn new( session: &'a Session, mut template: Vec, - cache_size: usize, + cache_size: NonZeroUsize, ) -> Result { - if cache_size == 0 { - return Err(Error::InvalidValue); - } - unsafe { Rv::from(get_pkcs11!(session.client(), C_FindObjectsInit)( session.handle(), @@ -101,11 +98,11 @@ impl<'a> ObjectHandleIterator<'a> { .into_result(Function::FindObjectsInit)?; } - let cache: Vec = vec![0; cache_size]; + let cache: Vec = vec![0; cache_size.get()]; Ok(ObjectHandleIterator { session, - object_count: cache_size, - index: cache_size, + object_count: cache_size.get(), + index: cache_size.get(), cache, }) } @@ -187,6 +184,7 @@ impl Session { /// Iterate over session objects matching a template. /// /// # Arguments + /// /// * `template` - The template to match objects against /// /// # Returns @@ -195,18 +193,20 @@ impl Session { /// matching the template. Note that the cache size is managed internally and set to a default value (10) /// /// # See also + /// /// * [`ObjectHandleIterator`] for more information on how to use the iterator /// * [`Session::iter_objects_with_cache_size`] for a way to specify the cache size #[inline(always)] pub fn iter_objects(&self, template: &[Attribute]) -> Result { - self.iter_objects_with_cache_size(template, MAX_OBJECT_COUNT) + self.iter_objects_with_cache_size(template, NonZeroUsize::new(MAX_OBJECT_COUNT).unwrap()) } /// Iterate over session objects matching a template, with cache size /// /// # Arguments + /// /// * `template` - The template to match objects against - /// * `cache_size` - The number of objects to cache. Note that 0 is an invalid value and will return an error. + /// * `cache_size` - The number of objects to cache (type is [`NonZeroUsize`]) /// /// # Returns /// @@ -214,12 +214,13 @@ impl Session { /// matching the template. The cache size corresponds to the size of the array provided to `C_FindObjects()`. /// /// # See also + /// /// * [`ObjectHandleIterator`] for more information on how to use the iterator /// * [`Session::iter_objects`] for a simpler way to iterate over objects pub fn iter_objects_with_cache_size( &self, template: &[Attribute], - cache_size: usize, + cache_size: NonZeroUsize, ) -> Result { let template: Vec = template.iter().map(Into::into).collect(); ObjectHandleIterator::new(self, template, cache_size) @@ -229,12 +230,12 @@ impl Session { /// /// # Arguments /// - /// * `template` - A [Attribute] of search parameters that will be used - /// to find objects. + /// * `template` - A reference to [Attribute] of search parameters that will be used + /// to find objects. /// /// # Returns /// - /// Upon success, a vector of [ObjectHandle] wrapped in a Result. + /// Upon success, a vector of [`ObjectHandle`] wrapped in a Result. /// Upon failure, the first error encountered. /// /// # Note diff --git a/cryptoki/tests/basic.rs b/cryptoki/tests/basic.rs index 0ed6cb0..bc9d505 100644 --- a/cryptoki/tests/basic.rs +++ b/cryptoki/tests/basic.rs @@ -16,6 +16,7 @@ use cryptoki::session::{SessionState, UserType}; use cryptoki::types::AuthPin; use serial_test::serial; use std::collections::HashMap; +use std::num::NonZeroUsize; use std::thread; use cryptoki::mechanism::ekdf::AesCbcDeriveParams; @@ -399,27 +400,26 @@ fn session_objecthandle_iterator() -> testresult::TestResult { // test iter_objects_with_cache_size() // count keys with cache size of 20 - let found_keys = session.iter_objects_with_cache_size(&key_search_template, 20)?; + let found_keys = session + .iter_objects_with_cache_size(&key_search_template, NonZeroUsize::new(20).unwrap())?; let found_keys = found_keys.map_while(|key| key.ok()).count(); assert_eq!(found_keys, 11); - // count keys with cache size of 0 => should result in an error - let found_keys = session.iter_objects_with_cache_size(&key_search_template, 0); - assert!(found_keys.is_err()); - // count keys with cache size of 1 - let found_keys = session.iter_objects_with_cache_size(&key_search_template, 1)?; + let found_keys = session + .iter_objects_with_cache_size(&key_search_template, NonZeroUsize::new(1).unwrap())?; let found_keys = found_keys.map_while(|key| key.ok()).count(); assert_eq!(found_keys, 11); // count keys with cache size of 10 - let found_keys = session.iter_objects_with_cache_size(&key_search_template, 10)?; + let found_keys = session + .iter_objects_with_cache_size(&key_search_template, NonZeroUsize::new(10).unwrap())?; let found_keys = found_keys.map_while(|key| key.ok()).count(); assert_eq!(found_keys, 11); // fetch keys into a vector let found_keys: Vec = session - .iter_objects_with_cache_size(&key_search_template, 10)? + .iter_objects_with_cache_size(&key_search_template, NonZeroUsize::new(10).unwrap())? .map_while(|key| key.ok()) .collect(); assert_eq!(found_keys.len(), 11); @@ -428,13 +428,15 @@ fn session_objecthandle_iterator() -> testresult::TestResult { let key1 = found_keys[1]; session.destroy_object(key0).unwrap(); - let found_keys = session.iter_objects_with_cache_size(&key_search_template, 10)?; + let found_keys = session + .iter_objects_with_cache_size(&key_search_template, NonZeroUsize::new(10).unwrap())?; let found_keys = found_keys.map_while(|key| key.ok()).count(); assert_eq!(found_keys, 10); // destroy another key session.destroy_object(key1).unwrap(); - let found_keys = session.iter_objects_with_cache_size(&key_search_template, 10)?; + let found_keys = session + .iter_objects_with_cache_size(&key_search_template, NonZeroUsize::new(10).unwrap())?; let found_keys = found_keys.map_while(|key| key.ok()).count(); assert_eq!(found_keys, 9); From 43a7edf1f8d0c035104be4bb246c145c5c5b2878 Mon Sep 17 00:00:00 2001 From: Eric Devolder Date: Sat, 7 Sep 2024 03:09:54 +0200 Subject: [PATCH 7/8] implementing and resolving comments from PR Signed-off-by: Eric Devolder --- cryptoki/src/session/object_management.rs | 60 ++++++++++++++++------- 1 file changed, 43 insertions(+), 17 deletions(-) diff --git a/cryptoki/src/session/object_management.rs b/cryptoki/src/session/object_management.rs index cb16788..9c56eca 100644 --- a/cryptoki/src/session/object_management.rs +++ b/cryptoki/src/session/object_management.rs @@ -12,7 +12,8 @@ use std::convert::TryInto; use std::num::NonZeroUsize; // Search 10 elements at a time -const MAX_OBJECT_COUNT: usize = 10; +// Safety: the value provided (10) must be non-zero +const MAX_OBJECT_COUNT: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(10) }; /// Iterator over object handles, in an active session. /// @@ -37,16 +38,16 @@ const MAX_OBJECT_COUNT: usize = 10; /// use std::env; /// /// # fn main() -> testresult::TestResult { -/// let pkcs11 = Pkcs11::new( -/// env::var("PKCS11_SOFTHSM2_MODULE") -/// .unwrap_or_else(|_| "/usr/local/lib/libsofthsm2.so".to_string()), -/// )?; -/// -/// pkcs11.initialize(CInitializeArgs::OsThreads)?; -/// let slot = pkcs11.get_slots_with_token()?.remove(0); -/// -/// let session = pkcs11.open_ro_session(slot).unwrap(); -/// session.login(UserType::User, Some(&AuthPin::new("fedcba".into())))?; +/// # let pkcs11 = Pkcs11::new( +/// # env::var("PKCS11_SOFTHSM2_MODULE") +/// # .unwrap_or_else(|_| "/usr/local/lib/libsofthsm2.so".to_string()), +/// # )?; +/// # +/// # pkcs11.initialize(CInitializeArgs::OsThreads)?; +/// # let slot = pkcs11.get_slots_with_token()?.remove(0); +/// # +/// # let session = pkcs11.open_ro_session(slot).unwrap(); +/// # session.login(UserType::User, Some(&AuthPin::new("fedcba".into())))?; /// /// let token_object = vec![Attribute::Token(true)]; /// let wanted_attr = vec![AttributeType::Label]; @@ -84,6 +85,28 @@ pub struct ObjectHandleIterator<'a> { } impl<'a> ObjectHandleIterator<'a> { + /// Create a new iterator over object handles. + /// + /// # Arguments + /// + /// * `session` - The session to iterate over + /// * `template` - The template to match objects against + /// * `cache_size` - The number of objects to cache (type is [`NonZeroUsize`]) + /// + /// # Returns + /// + /// This function will return a [`Result`] that can be used to iterate over the objects + /// matching the template. The cache size corresponds to the size of the array provided to `C_FindObjects()`. + /// + /// # Errors + /// + /// This function will return an error if the call to `C_FindObjectsInit` fails. + /// + /// # Note + /// + /// The iterator `new()` method will call `C_FindObjectsInit`. It means that until the iterator is dropped, + /// creating another iterator will result in an error (typically `RvError::OperationActive` ). + /// fn new( session: &'a Session, mut template: Vec, @@ -171,11 +194,15 @@ impl<'a> Iterator for ObjectHandleIterator<'a> { impl Drop for ObjectHandleIterator<'_> { fn drop(&mut self) { - // bark but pass if C_FindObjectsFinal() is not implemented if let Some(f) = get_pkcs11_func!(self.session.client(), C_FindObjectsFinal) { + // swallow the return value, as we can't do anything about it, + // but log the error + if let Rv::Error(error) = Rv::from(unsafe { f(self.session.handle()) }) { + log::error!("C_FindObjectsFinal() failed with error: {:?}", error); + } + } else { + // bark but pass if C_FindObjectsFinal() is not implemented log::error!("C_FindObjectsFinal() is not implemented on this library"); - // swallow the return value, as we can't do anything about it - let _ = unsafe { f(self.session.handle()) }; } } } @@ -198,7 +225,7 @@ impl Session { /// * [`Session::iter_objects_with_cache_size`] for a way to specify the cache size #[inline(always)] pub fn iter_objects(&self, template: &[Attribute]) -> Result { - self.iter_objects_with_cache_size(template, NonZeroUsize::new(MAX_OBJECT_COUNT).unwrap()) + self.iter_objects_with_cache_size(template, MAX_OBJECT_COUNT) } /// Iterate over session objects matching a template, with cache size @@ -279,8 +306,7 @@ impl Session { /// #[inline(always)] pub fn find_objects(&self, template: &[Attribute]) -> Result> { - self.iter_objects(template)? - .collect::>>() + self.iter_objects(template)?.collect() } /// Create a new object From de706ce0581bc76c2f7f98194db082480c56c301 Mon Sep 17 00:00:00 2001 From: Eric Devolder Date: Sat, 7 Sep 2024 03:09:54 +0200 Subject: [PATCH 8/8] minor changes - get_pkcs11! macro rewrite - test code refactoring Signed-off-by: Eric Devolder --- cryptoki/src/context/mod.rs | 7 ++----- cryptoki/tests/basic.rs | 6 +++--- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/cryptoki/src/context/mod.rs b/cryptoki/src/context/mod.rs index 723078f..cfa965e 100644 --- a/cryptoki/src/context/mod.rs +++ b/cryptoki/src/context/mod.rs @@ -3,13 +3,10 @@ //! Pkcs11 context and initialization types /// Directly get the PKCS #11 operation from the context structure and check for null pointers. +/// Note that this macro depends on the get_pkcs11_func! macro. macro_rules! get_pkcs11 { ($pkcs11:expr, $func_name:ident) => { - ($pkcs11 - .impl_ - .function_list - .$func_name - .ok_or(crate::error::Error::NullFunctionPointer)?) + (get_pkcs11_func!($pkcs11, $func_name).ok_or(crate::error::Error::NullFunctionPointer)?) }; } diff --git a/cryptoki/tests/basic.rs b/cryptoki/tests/basic.rs index bc9d505..8368a54 100644 --- a/cryptoki/tests/basic.rs +++ b/cryptoki/tests/basic.rs @@ -378,7 +378,7 @@ fn session_objecthandle_iterator() -> testresult::TestResult { // we generate 11 keys with the same CKA_ID - (1..=11).for_each(|i| { + for i in 1..=11 { let key_template = vec![ Attribute::Token(true), Attribute::Encrypt(true), @@ -387,8 +387,8 @@ fn session_objecthandle_iterator() -> testresult::TestResult { ]; // generate a secret key - let _key = session.generate_key(&Mechanism::Des3KeyGen, &key_template); - }); + session.generate_key(&Mechanism::Des3KeyGen, &key_template)?; + } // retrieve these keys using this template let key_search_template = vec![