From 1b1cb57b40b4ab03eb9fb1075ae06e4ee803648f Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Mon, 16 Dec 2024 17:15:28 -0500 Subject: [PATCH 1/7] Fix log saving with v4 of the artifact action Signed-off-by: Simo Sorce --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b570aa2..b3ee523 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -104,7 +104,7 @@ jobs: - uses: actions/upload-artifact@v4 if: failure() with: - name: Build logs ${{ matrix.name }} + name: Build logs ${{ matrix.db }} ${{ matrix.name }} path: | target/debug/build/*/output From e6ef09d3a69f9c2f328ce495026e90e584641f5f Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Mon, 16 Dec 2024 16:58:05 -0500 Subject: [PATCH 2/7] Confine macros to the kryoptic crate With #[macro_export] macros are declared public crate interfaces, but that is not what we really want to do with these macros. We just want to make them available to all modules within the crate. Use the correct method to do that. Signed-off-by: Simo Sorce --- src/aes.rs | 2 +- src/attribute.rs | 2 +- src/ec/ecdh.rs | 3 +-- src/ec/ecdsa.rs | 1 - src/ec/eddsa.rs | 1 - src/ec/montgomery.rs | 1 - src/error.rs | 10 +++++++++- src/fips/indicators.rs | 3 +-- src/hmac.rs | 2 +- src/lib.rs | 2 ++ src/misc.rs | 19 ++++++------------- src/native/sp800_108.rs | 4 ++-- src/native/sshkdf.rs | 10 ++++++---- src/object.rs | 4 ++-- src/ossl/aes.rs | 4 ++-- src/ossl/common.rs | 2 +- src/ossl/ecdh.rs | 2 +- src/ossl/ecdsa.rs | 3 +-- src/ossl/eddsa.rs | 4 ++-- src/ossl/hkdf.rs | 12 +++--------- src/ossl/kbkdf.rs | 5 ++--- src/ossl/rsa.rs | 3 ++- src/pbkdf2.rs | 3 +-- src/rsa.rs | 1 - src/sp800_108.rs | 6 ++---- src/storage/aci.rs | 2 +- src/storage/nssdb/ci.rs | 2 +- src/tests/mod.rs | 1 + src/tests/util.rs | 1 - src/tlskdf.rs | 3 +-- 30 files changed, 53 insertions(+), 65 deletions(-) diff --git a/src/aes.rs b/src/aes.rs index c507f82..b334beb 100644 --- a/src/aes.rs +++ b/src/aes.rs @@ -7,9 +7,9 @@ use crate::attribute::Attribute; use crate::error::Result; use crate::interface::*; use crate::mechanism::*; +use crate::misc::cast_params; use crate::object::*; use crate::ossl::aes::*; -use crate::{attr_element, cast_params}; use once_cell::sync::Lazy; diff --git a/src/attribute.rs b/src/attribute.rs index 99ed485..8bc8fcf 100644 --- a/src/attribute.rs +++ b/src/attribute.rs @@ -5,7 +5,7 @@ use std::borrow::Cow; use crate::error::{Error, Result}; use crate::interface::*; -use crate::{bytes_to_vec, sizeof, void_ptr}; +use crate::misc::{bytes_to_vec, sizeof, void_ptr}; use zeroize::Zeroize; diff --git a/src/ec/ecdh.rs b/src/ec/ecdh.rs index 3980afe..0c2476a 100644 --- a/src/ec/ecdh.rs +++ b/src/ec/ecdh.rs @@ -7,11 +7,10 @@ use crate::ec::ecdsa::{MAX_EC_SIZE_BITS, MIN_EC_SIZE_BITS}; use crate::error::Result; use crate::interface::*; use crate::mechanism::{Mechanism, Mechanisms, Operation}; +use crate::misc::cast_params; use crate::object::ObjectFactories; use crate::ossl::ecdh::ECDHOperation; -use crate::cast_params; - pub fn register(mechs: &mut Mechanisms, _: &mut ObjectFactories) { ECDHMechanism::register_mechanisms(mechs); } diff --git a/src/ec/ecdsa.rs b/src/ec/ecdsa.rs index 86c82ce..692878d 100644 --- a/src/ec/ecdsa.rs +++ b/src/ec/ecdsa.rs @@ -3,7 +3,6 @@ use std::fmt::Debug; -use crate::attr_element; use crate::attribute::Attribute; use crate::ec::*; use crate::error::{general_error, Error, Result}; diff --git a/src/ec/eddsa.rs b/src/ec/eddsa.rs index ee13a41..3e02c6e 100644 --- a/src/ec/eddsa.rs +++ b/src/ec/eddsa.rs @@ -3,7 +3,6 @@ use std::fmt::Debug; -use crate::attr_element; use crate::attribute::Attribute; use crate::ec::*; use crate::error::{general_error, Error, Result}; diff --git a/src/ec/montgomery.rs b/src/ec/montgomery.rs index 4bcb276..d4f65e4 100644 --- a/src/ec/montgomery.rs +++ b/src/ec/montgomery.rs @@ -3,7 +3,6 @@ use std::fmt::Debug; -use crate::attr_element; use crate::attribute::Attribute; use crate::ec::montgomery::montgomery::ECMontgomeryOperation; use crate::ec::*; diff --git a/src/error.rs b/src/error.rs index 49353cb..6bcf5f4 100644 --- a/src/error.rs +++ b/src/error.rs @@ -211,7 +211,6 @@ impl From for Error { } } -#[macro_export] macro_rules! some_or_err { ($action:expr) => { if let Some(ref x) = $action { @@ -228,6 +227,7 @@ macro_rules! some_or_err { } }; } +pub(crate) use some_or_err; pub fn general_error(error: E) -> Error where @@ -242,3 +242,11 @@ where { Error::ck_rv_from_error(interface::CKR_DEVICE_ERROR, error) } + +macro_rules! map_err { + ($map:expr, $err:tt) => {{ + use crate::error::Error; + $map.map_err(|e| Error::ck_rv_from_error($err, e)) + }}; +} +pub(crate) use map_err; diff --git a/src/fips/indicators.rs b/src/fips/indicators.rs index 2fe6f63..03e4b45 100644 --- a/src/fips/indicators.rs +++ b/src/fips/indicators.rs @@ -1,12 +1,11 @@ // Copyright 2024 Simo Sorce // See LICENSE.txt file for terms -use crate::attr_element; use crate::attribute::Attribute; use crate::ec::{get_oid_from_obj, oid_to_bits}; use crate::error::Result; use crate::interface::*; -use crate::object::{OAFlags, Object, ObjectAttr, ObjectFactory}; +use crate::object::{attr_element, OAFlags, Object, ObjectAttr, ObjectFactory}; use crate::Token; use once_cell::sync::Lazy; diff --git a/src/hmac.rs b/src/hmac.rs index 408698a..be548f7 100644 --- a/src/hmac.rs +++ b/src/hmac.rs @@ -7,8 +7,8 @@ use crate::error::{Error, Result}; use crate::hash; use crate::interface::*; use crate::mechanism::*; +use crate::misc::sizeof; use crate::object::*; -use crate::sizeof; use once_cell::sync::Lazy; use zeroize::Zeroize; diff --git a/src/lib.rs b/src/lib.rs index c9d1b4d..c8e4edf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -49,6 +49,8 @@ include!("enabled.rs"); mod kasn1; mod misc; +use crate::misc::{bytes_to_slice, bytes_to_vec, cast_params}; + macro_rules! ret_to_rv { ($ret:expr) => { match $ret { diff --git a/src/misc.rs b/src/misc.rs index b83c99b..9aa0cda 100644 --- a/src/misc.rs +++ b/src/misc.rs @@ -9,14 +9,6 @@ use crate::object::{Object, ObjectFactories, ObjectType}; pub const CK_ULONG_SIZE: usize = std::mem::size_of::(); -#[macro_export] -macro_rules! map_err { - ($map:expr, $err:tt) => {{ - $map.map_err(|e| error::Error::ck_rv_from_error($err, e)) - }}; -} - -#[macro_export] macro_rules! bytes_to_vec { ($ptr:expr, $len:expr) => {{ let ptr = $ptr as *const u8; @@ -33,22 +25,22 @@ macro_rules! bytes_to_vec { } }}; } +pub(crate) use bytes_to_vec; -#[macro_export] macro_rules! void_ptr { ($ptr:expr) => { $ptr as *const _ as CK_VOID_PTR }; } +pub(crate) use void_ptr; -#[macro_export] macro_rules! byte_ptr { ($ptr:expr) => { $ptr as *const _ as CK_BYTE_PTR }; } +pub(crate) use byte_ptr; -#[macro_export] macro_rules! cast_params { ($mech:expr, $params:ty) => {{ let Ok(len) = usize::try_from($mech.ulParameterLen) else { @@ -80,15 +72,15 @@ macro_rules! cast_params { unsafe { *($param as *const $params) } }}; } +pub(crate) use cast_params; -#[macro_export] macro_rules! sizeof { ($type:ty) => { CK_ULONG::try_from(std::mem::size_of::<$type>()).unwrap() }; } +pub(crate) use sizeof; -#[macro_export] macro_rules! bytes_to_slice { ($ptr: expr, $len:expr, $typ:ty) => { if $len > 0 { @@ -116,6 +108,7 @@ macro_rules! bytes_to_slice { } }; } +pub(crate) use bytes_to_slice; #[allow(dead_code)] pub fn common_derive_data_object( diff --git a/src/native/sp800_108.rs b/src/native/sp800_108.rs index 80936ea..18559eb 100644 --- a/src/native/sp800_108.rs +++ b/src/native/sp800_108.rs @@ -3,12 +3,12 @@ use crate::attribute::Attribute; use crate::error; -use crate::error::Result; +use crate::error::{map_err, Result}; use crate::interface::*; use crate::mechanism::{Derive, Mac, MechOperation, Mechanisms}; +use crate::misc::{bytes_to_slice, bytes_to_vec}; use crate::object::{Object, ObjectFactories}; use crate::sp800_108::*; -use crate::{bytes_to_slice, bytes_to_vec, map_err}; macro_rules! maxsize { ($size: expr) => { diff --git a/src/native/sshkdf.rs b/src/native/sshkdf.rs index dfeb23a..af102f6 100644 --- a/src/native/sshkdf.rs +++ b/src/native/sshkdf.rs @@ -10,7 +10,6 @@ use crate::interface::*; use crate::mechanism::{Derive, MechOperation, Mechanisms}; use crate::misc; use crate::object::{Object, ObjectFactories}; -use crate::{bytes_to_vec, cast_params}; #[derive(Debug)] pub struct SSHKDFOperation { @@ -25,7 +24,7 @@ pub struct SSHKDFOperation { impl SSHKDFOperation { pub fn new(mech: &CK_MECHANISM) -> Result { - let params = cast_params!(mech, KR_SSHKDF_PARAMS); + let params = misc::cast_params!(mech, KR_SSHKDF_PARAMS); if !hash::is_valid_hash(params.prfHashMechanism) { return Err(CKR_MECHANISM_PARAM_INVALID)?; @@ -46,11 +45,14 @@ impl SSHKDFOperation { finalized: false, prf: params.prfHashMechanism, key_type: params.derivedKeyType, - exchange_hash: bytes_to_vec!( + exchange_hash: misc::bytes_to_vec!( params.pExchangeHash, params.ulExchangeHashLen ), - session_id: bytes_to_vec!(params.pSessionId, params.ulSessionIdLen), + session_id: misc::bytes_to_vec!( + params.pSessionId, + params.ulSessionIdLen + ), is_data: is_data, }) } diff --git a/src/object.rs b/src/object.rs index 8060219..b68ab33 100644 --- a/src/object.rs +++ b/src/object.rs @@ -296,14 +296,13 @@ impl ObjectAttr { } } -#[macro_export] macro_rules! attr_element { ($id:expr; $flags:expr; $from_type:expr; val $defval:expr) => { ObjectAttr::new($from_type($id, $defval), $flags) }; } +pub(crate) use attr_element; -#[macro_export] macro_rules! bytes_attr_not_empty { ($obj:expr; $id:expr) => { match $obj.get_attr_as_bytes($id) { @@ -322,6 +321,7 @@ macro_rules! bytes_attr_not_empty { } }; } +pub(crate) use bytes_attr_not_empty; pub trait ObjectFactory: Debug + Send + Sync { fn create(&self, _template: &[CK_ATTRIBUTE]) -> Result { diff --git a/src/ossl/aes.rs b/src/ossl/aes.rs index a8b2778..38f25d4 100644 --- a/src/ossl/aes.rs +++ b/src/ossl/aes.rs @@ -5,15 +5,15 @@ use std::ffi::{c_char, c_int, c_void}; use crate::aes::*; use crate::error; -use crate::error::Result; +use crate::error::{map_err, Result}; use crate::interface::*; use crate::mechanism::*; +use crate::misc::{bytes_to_slice, bytes_to_vec, cast_params, void_ptr}; use crate::object::Object; use crate::ossl::bindings::*; use crate::ossl::common::*; #[cfg(feature = "fips")] use crate::ossl::fips::*; -use crate::{bytes_to_slice, bytes_to_vec, cast_params, map_err, void_ptr}; use crate::get_random_data; diff --git a/src/ossl/common.rs b/src/ossl/common.rs index 1b7a7a5..1c59e53 100644 --- a/src/ossl/common.rs +++ b/src/ossl/common.rs @@ -7,10 +7,10 @@ use std::ffi::{c_char, c_int, c_uint, c_void}; use crate::error::Result; use crate::interface::*; use crate::kasn1::oid; +use crate::misc::{byte_ptr, void_ptr}; use crate::object::Object; use crate::ossl::bindings::*; use crate::ossl::get_libctx; -use crate::{byte_ptr, void_ptr}; #[cfg(feature = "ecc")] use crate::ec::get_oid_from_obj; diff --git a/src/ossl/ecdh.rs b/src/ossl/ecdh.rs index 17cc94e..a4f8cee 100644 --- a/src/ossl/ecdh.rs +++ b/src/ossl/ecdh.rs @@ -5,10 +5,10 @@ use core::ffi::{c_char, c_int, c_uint}; use std::borrow::Cow; use crate::attribute::CkAttrs; -use crate::bytes_to_vec; use crate::error::Result; use crate::interface::*; use crate::mechanism::*; +use crate::misc::bytes_to_vec; use crate::object::{default_key_attributes, Object, ObjectFactories}; use crate::ossl::bindings::*; use crate::ossl::common::*; diff --git a/src/ossl/ecdsa.rs b/src/ossl/ecdsa.rs index 5e2a985..afff972 100644 --- a/src/ossl/ecdsa.rs +++ b/src/ossl/ecdsa.rs @@ -6,14 +6,13 @@ use core::ffi::{c_char, c_int}; use crate::attribute::Attribute; use crate::ec::ecdsa::*; use crate::ec::get_ec_point_from_obj; -use crate::error::Result; +use crate::error::{some_or_err, Result}; use crate::interface::*; use crate::kasn1::DerEncBigUint; use crate::mechanism::*; use crate::object::Object; use crate::ossl::bindings::*; use crate::ossl::common::*; -use crate::some_or_err; #[cfg(feature = "fips")] use crate::ossl::fips::*; diff --git a/src/ossl/eddsa.rs b/src/ossl/eddsa.rs index c1f74b4..4ba7c92 100644 --- a/src/ossl/eddsa.rs +++ b/src/ossl/eddsa.rs @@ -5,13 +5,13 @@ use std::ffi::{c_char, c_int}; use crate::attribute::Attribute; use crate::ec::get_ec_point_from_obj; -use crate::error::Result; +use crate::error::{some_or_err, Result}; use crate::interface::*; use crate::mechanism::*; +use crate::misc::{bytes_to_vec, cast_params}; use crate::object::Object; use crate::ossl::bindings::*; use crate::ossl::common::*; -use crate::{bytes_to_vec, cast_params, some_or_err}; #[cfg(feature = "fips")] use crate::ossl::fips::*; diff --git a/src/ossl/hkdf.rs b/src/ossl/hkdf.rs index 1340b07..c6f3d16 100644 --- a/src/ossl/hkdf.rs +++ b/src/ossl/hkdf.rs @@ -10,13 +10,12 @@ use crate::hash::INVALID_HASH_SIZE; use crate::hmac::hmac_size; use crate::interface::*; use crate::mechanism::{Derive, MechOperation, Mechanisms}; -use crate::misc; +use crate::misc::*; use crate::object::{Object, ObjectFactories}; use crate::ossl::bindings::*; use crate::ossl::common::*; #[cfg(feature = "fips")] use crate::ossl::fips::*; -use crate::{bytes_to_slice, cast_params}; #[derive(Debug)] pub struct HKDFOperation { @@ -212,14 +211,9 @@ impl Derive for HKDFOperation { } let (mut obj, keysize) = if self.emit_data_obj { - misc::common_derive_data_object(template, objfactories, self.prflen) + common_derive_data_object(template, objfactories, self.prflen) } else { - misc::common_derive_key_object( - key, - template, - objfactories, - self.prflen, - ) + common_derive_key_object(key, template, objfactories, self.prflen) }?; if !self.expand && keysize != self.prflen { diff --git a/src/ossl/kbkdf.rs b/src/ossl/kbkdf.rs index 0f6ad16..fbcae1c 100644 --- a/src/ossl/kbkdf.rs +++ b/src/ossl/kbkdf.rs @@ -4,16 +4,15 @@ use std::ffi::c_int; use crate::attribute::Attribute; -use crate::error; -use crate::error::Result; +use crate::error::{map_err, Result}; use crate::interface::*; use crate::mechanism::{Derive, MechOperation, Mechanisms}; +use crate::misc::{bytes_to_slice, bytes_to_vec}; use crate::object::{Object, ObjectFactories}; use crate::ossl::bindings::*; use crate::ossl::common::*; use crate::ossl::fips::*; use crate::sp800_108::*; -use crate::{bytes_to_slice, bytes_to_vec, map_err}; const SP800_MODE_COUNTER: &[u8; 8] = b"counter\0"; const SP800_MODE_FEEDBACK: &[u8; 9] = b"feedback\0"; diff --git a/src/ossl/rsa.rs b/src/ossl/rsa.rs index 26e0636..02bb5c9 100644 --- a/src/ossl/rsa.rs +++ b/src/ossl/rsa.rs @@ -4,14 +4,15 @@ use core::ffi::{c_char, c_int, c_uint}; use crate::attribute::Attribute; +use crate::error::some_or_err; use crate::error::{Error, Result}; use crate::hash::{hash_size, INVALID_HASH_SIZE}; use crate::interface::*; use crate::mechanism::*; +use crate::misc::{bytes_to_vec, cast_params}; use crate::object::Object; use crate::ossl::bindings::*; use crate::ossl::common::*; -use crate::{bytes_to_vec, cast_params, some_or_err}; #[cfg(not(feature = "fips"))] use crate::ossl::get_libctx; diff --git a/src/pbkdf2.rs b/src/pbkdf2.rs index 33e1d58..934d2e7 100644 --- a/src/pbkdf2.rs +++ b/src/pbkdf2.rs @@ -8,10 +8,9 @@ use crate::error::Result; use crate::hmac; use crate::interface::*; use crate::mechanism::{Mechanism, Mechanisms}; +use crate::misc::{bytes_to_vec, cast_params}; use crate::object::{default_key_attributes, Object, ObjectFactories}; -use crate::{bytes_to_vec, cast_params}; - #[cfg(not(feature = "fips"))] use crate::native::pbkdf2::pbkdf2_derive; diff --git a/src/rsa.rs b/src/rsa.rs index 4e667c3..9b79800 100644 --- a/src/rsa.rs +++ b/src/rsa.rs @@ -10,7 +10,6 @@ use crate::kasn1::{oid, DerEncBigUint, PrivateKeyInfo}; use crate::mechanism::*; use crate::object::*; use crate::ossl::rsa::*; -use crate::{attr_element, bytes_attr_not_empty}; use asn1; use once_cell::sync::Lazy; diff --git a/src/sp800_108.rs b/src/sp800_108.rs index 2d913dc..a7d0881 100644 --- a/src/sp800_108.rs +++ b/src/sp800_108.rs @@ -3,14 +3,12 @@ use std::fmt::Debug; -use crate::error; -use crate::error::Result; +use crate::error::{map_err, Result}; use crate::interface::*; use crate::mechanism::{Mechanism, Mechanisms, Operation}; +use crate::misc::{bytes_to_vec, cast_params, sizeof}; use crate::object::{Object, ObjectFactories}; -use crate::{bytes_to_vec, cast_params, map_err, sizeof}; - #[cfg(not(feature = "fips"))] use crate::native::sp800_108::*; diff --git a/src/storage/aci.rs b/src/storage/aci.rs index a242fa0..24e55e2 100644 --- a/src/storage/aci.rs +++ b/src/storage/aci.rs @@ -8,11 +8,11 @@ use crate::attribute::CkAttrs; use crate::error::Result; use crate::interface::*; use crate::kasn1::*; +use crate::misc::{byte_ptr, sizeof, void_ptr}; use crate::object::Object; use crate::token::TokenFacilities; use crate::Operation; use crate::CSPRNG; -use crate::{byte_ptr, sizeof, void_ptr}; use asn1; use zeroize::Zeroize; diff --git a/src/storage/nssdb/ci.rs b/src/storage/nssdb/ci.rs index 02930dd..6672d61 100644 --- a/src/storage/nssdb/ci.rs +++ b/src/storage/nssdb/ci.rs @@ -9,11 +9,11 @@ use crate::error::Result; use crate::interface::*; use crate::kasn1::oid::*; use crate::kasn1::pkcs::*; +use crate::misc::{sizeof, void_ptr}; use crate::object::Object; use crate::storage::aci::pbkdf2_derive; use crate::token::TokenFacilities; use crate::CSPRNG; -use crate::{sizeof, void_ptr}; use zeroize::Zeroize; diff --git a/src/tests/mod.rs b/src/tests/mod.rs index c5f25c7..e86585a 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -6,6 +6,7 @@ use std::fs::{create_dir_all, remove_dir_all, OpenOptions}; use std::io::Write; use std::sync::Once; +use crate::misc::*; use crate::storage::StorageDBInfo; use crate::*; diff --git a/src/tests/util.rs b/src/tests/util.rs index b7d0767..1082972 100644 --- a/src/tests/util.rs +++ b/src/tests/util.rs @@ -3,7 +3,6 @@ use crate::tests::*; -pub const CK_ULONG_SIZE: usize = std::mem::size_of::(); pub const CK_BBOOL_SIZE: usize = std::mem::size_of::(); macro_rules! make_attribute { diff --git a/src/tlskdf.rs b/src/tlskdf.rs index 3a7d635..429b70a 100644 --- a/src/tlskdf.rs +++ b/src/tlskdf.rs @@ -8,9 +8,8 @@ use crate::error::Result; use crate::hmac::{hash_to_hmac_mech, register_mechs_only}; use crate::interface::*; use crate::mechanism::*; -use crate::misc::CK_ULONG_SIZE; +use crate::misc::{bytes_to_slice, bytes_to_vec, cast_params, CK_ULONG_SIZE}; use crate::object::{Object, ObjectFactories}; -use crate::{bytes_to_slice, bytes_to_vec, cast_params}; use constant_time_eq::constant_time_eq; use once_cell::sync::Lazy; From 30f664a9939bd9fe8e8101e8390a6af57dbf3bc3 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Mon, 16 Dec 2024 17:00:17 -0500 Subject: [PATCH 3/7] Do not make public stuff that don't need to Some functions and variables were made public, but didn't need to, tidy up access. Signed-off-by: Simo Sorce --- src/enabled.rs | 2 +- src/lib.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/enabled.rs b/src/enabled.rs index 3242c6a..146d435 100644 --- a/src/enabled.rs +++ b/src/enabled.rs @@ -43,7 +43,7 @@ mod tlskdf; use mechanism::Mechanisms; use object::ObjectFactories; -pub fn register_all(mechs: &mut Mechanisms, ot: &mut ObjectFactories) { +fn register_all(mechs: &mut Mechanisms, ot: &mut ObjectFactories) { object::register(mechs, ot); #[cfg(feature = "aes")] diff --git a/src/lib.rs b/src/lib.rs index c8e4edf..efa841a 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -95,11 +95,11 @@ macro_rules! cast_or_ret { thread_local!(static CSPRNG: RefCell = RefCell::new(RNG::new("HMAC DRBG SHA256").unwrap())); -pub fn get_random_data(data: &mut [u8]) -> Result<()> { +fn get_random_data(data: &mut [u8]) -> Result<()> { CSPRNG.with(|rng| rng.borrow_mut().generate_random(data)) } -pub fn random_add_seed(data: &[u8]) -> Result<()> { +fn random_add_seed(data: &[u8]) -> Result<()> { CSPRNG.with(|rng| rng.borrow_mut().add_seed(data)) } @@ -2391,7 +2391,7 @@ extern "C" fn fn_wait_for_slot_event( CKR_FUNCTION_NOT_SUPPORTED } -pub static FNLIST_240: CK_FUNCTION_LIST = CK_FUNCTION_LIST { +static FNLIST_240: CK_FUNCTION_LIST = CK_FUNCTION_LIST { version: CK_VERSION { major: 2, minor: 40, @@ -3137,7 +3137,7 @@ extern "C" fn fn_message_verify_final(_session: CK_SESSION_HANDLE) -> CK_RV { CKR_FUNCTION_NOT_SUPPORTED } -pub static FNLIST_300: CK_FUNCTION_LIST_3_0 = CK_FUNCTION_LIST_3_0 { +static FNLIST_300: CK_FUNCTION_LIST_3_0 = CK_FUNCTION_LIST_3_0 { version: CK_VERSION { major: 3, minor: 0 }, C_Initialize: Some(fn_initialize), C_Finalize: Some(fn_finalize), From 7f27362033a6f7079b1c2d1ba085381a3b01ac56 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Mon, 16 Dec 2024 17:01:37 -0500 Subject: [PATCH 4/7] Add documentation for the public functions This crate exposes only three public functions which are the official entrypoints for the PKCS#11 interface. Document them with additional pointers to the spec. Signed-off-by: Simo Sorce --- src/lib.rs | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index efa841a..9ba6e2b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,6 +1,12 @@ // Copyright 2023 Simo Sorce // See LICENSE.txt file for terms +#![warn(missing_docs)] + +//! This is Kryoptic +//! +//! A cryptographic software token using the PKCS#11 standard API + use std::cell::RefCell; use std::collections::HashMap; use std::ffi::{c_char, CStr}; @@ -2554,6 +2560,21 @@ extern "C" fn fn_get_info(info: CK_INFO_PTR) -> CK_RV { CKR_OK } +/// Provides access to the functions defined in the API specification +/// +/// The vtable returned by this function includes a version specifier as +/// the first element of this table. This version number determines the +/// length and contents of the rest of the vtable. +/// +/// Often for backwards compatibility reasons the table returned by this +/// function is the table specified in PKCS#11 v2.40. +/// +/// While access to later versions of the table is deferred to the +/// `C_GeInterfaceList` function available starting with version 3.0 of the +/// specification. +/// +/// Version 3.1 Specification: [https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.1/os/pkcs11-spec-v3.1-os.html#_Toc111203258](https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.1/os/pkcs11-spec-v3.1-os.html#_Toc111203258) + #[no_mangle] pub extern "C" fn C_GetFunctionList(fnlist: CK_FUNCTION_LIST_PTR_PTR) -> CK_RV { unsafe { @@ -3278,6 +3299,18 @@ static INTERFACE_SET: Lazy> = Lazy::new(|| { v }); +/// Provides access to the list of interfaces defined by this implementation +/// +/// Starting with PKCS#11 version 3.0.0 tokens provide a list of interfaces +/// that can be obtained from the token. Each interface provides a name, and +/// a pointer to a vtable containing the functions defined for that interface. +/// Additionally flags are returned as well. +/// Custom interfaces can be defined by any vendor by specifying a custom +/// interface name. The name "PKCS 11" is reserved for official standard +/// interfaces. +/// +/// Version 3.1 Specification: [https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.1/os/pkcs11-spec-v3.1-os.html#_Toc111203259](https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.1/os/pkcs11-spec-v3.1-os.html#_Toc111203259) + #[no_mangle] pub extern "C" fn C_GetInterfaceList( interfaces_list: CK_INTERFACE_PTR, @@ -3313,6 +3346,14 @@ pub extern "C" fn C_GetInterfaceList( CKR_OK } +/// Returns a specific interface identified by name and version +/// +/// Applications that wants to immediately access a specific interface name, +/// optionally a speific version too. +/// The `interface` argument returns the pointer to the requested vtable +/// +/// Version 3.1 Specification: [https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.1/os/pkcs11-spec-v3.1-os.html#_Toc111203260](https://docs.oasis-open.org/pkcs11/pkcs11-spec/v3.1/os/pkcs11-spec-v3.1-os.html#_Toc111203260) + #[no_mangle] pub extern "C" fn C_GetInterface( interface_name: CK_UTF8CHAR_PTR, From e617b8d78cf7c1ccfc4470eb1705d4ff77c617c6 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Mon, 16 Dec 2024 17:02:49 -0500 Subject: [PATCH 5/7] Suppress warnings in some feature configuations Signed-off-by: Simo Sorce --- src/error.rs | 4 ++++ src/lib.rs | 2 ++ src/mechanism.rs | 3 +++ src/object.rs | 3 +++ src/ossl/common.rs | 33 ++++++++++++++++++++++++++++++++- 5 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/error.rs b/src/error.rs index 6bcf5f4..b4b5e2d 100644 --- a/src/error.rs +++ b/src/error.rs @@ -211,6 +211,7 @@ impl From for Error { } } +#[allow(unused_macros)] macro_rules! some_or_err { ($action:expr) => { if let Some(ref x) = $action { @@ -227,8 +228,10 @@ macro_rules! some_or_err { } }; } +#[allow(unused_imports)] pub(crate) use some_or_err; +#[allow(dead_code)] pub fn general_error(error: E) -> Error where E: Into>, @@ -236,6 +239,7 @@ where Error::ck_rv_from_error(interface::CKR_GENERAL_ERROR, error) } +#[allow(dead_code)] pub fn device_error(error: E) -> Error where E: Into>, diff --git a/src/lib.rs b/src/lib.rs index 9ba6e2b..de65ed3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -486,6 +486,7 @@ fn force_load_config() -> CK_RV { return CKR_OK; } +#[cfg(feature = "eddsa")] #[cfg(test)] fn get_ec_point_encoding(save: &mut config::EcPointEncoding) -> CK_RV { let gconf = global_rlock!(noinitcheck CONFIG); @@ -493,6 +494,7 @@ fn get_ec_point_encoding(save: &mut config::EcPointEncoding) -> CK_RV { CKR_OK } +#[cfg(feature = "eddsa")] #[cfg(test)] fn set_ec_point_encoding(val: config::EcPointEncoding) -> CK_RV { let mut gconf = global_wlock!(noinitcheck CONFIG); diff --git a/src/mechanism.rs b/src/mechanism.rs index 4b1474c..dd2ee26 100644 --- a/src/mechanism.rs +++ b/src/mechanism.rs @@ -247,12 +247,15 @@ pub trait Mac: MechOperation { fn mac(&mut self, _data: &[u8], _digest: &mut [u8]) -> Result<()> { Err(CKR_GENERAL_ERROR)? } + #[allow(dead_code)] fn mac_update(&mut self, _data: &[u8]) -> Result<()> { Err(CKR_GENERAL_ERROR)? } + #[allow(dead_code)] fn mac_final(&mut self, _digest: &mut [u8]) -> Result<()> { Err(CKR_GENERAL_ERROR)? } + #[allow(dead_code)] fn mac_len(&self) -> Result { Err(CKR_GENERAL_ERROR)? } diff --git a/src/object.rs b/src/object.rs index b68ab33..8a17d08 100644 --- a/src/object.rs +++ b/src/object.rs @@ -303,6 +303,7 @@ macro_rules! attr_element { } pub(crate) use attr_element; +#[allow(unused_macros)] macro_rules! bytes_attr_not_empty { ($obj:expr; $id:expr) => { match $obj.get_attr_as_bytes($id) { @@ -321,6 +322,7 @@ macro_rules! bytes_attr_not_empty { } }; } +#[allow(unused_imports)] pub(crate) use bytes_attr_not_empty; pub trait ObjectFactory: Debug + Send + Sync { @@ -502,6 +504,7 @@ pub trait ObjectFactory: Debug + Send + Sync { Ok(obj) } + #[allow(dead_code)] fn set_attribute_default( &self, attr: CK_ATTRIBUTE_TYPE, diff --git a/src/ossl/common.rs b/src/ossl/common.rs index 1c59e53..2dec2ab 100644 --- a/src/ossl/common.rs +++ b/src/ossl/common.rs @@ -154,11 +154,13 @@ ptr_wrapper!(ctx; CIPHER; Cipher); ptr_wrapper!(ctx_from_name; KDF; Kdf); ptr_wrapper!(ctx_from_name; MAC; Mac); +#[cfg(any(feature = "ecc", feature = "rsa"))] #[derive(Debug)] pub struct EvpPkeyCtx { ptr: *mut EVP_PKEY_CTX, } +#[cfg(any(feature = "ecc", feature = "rsa"))] impl EvpPkeyCtx { pub fn new(name: *const c_char) -> Result { let ptr = unsafe { @@ -187,6 +189,7 @@ impl EvpPkeyCtx { } } +#[cfg(any(feature = "ecc", feature = "rsa"))] impl Drop for EvpPkeyCtx { fn drop(&mut self) { unsafe { @@ -195,14 +198,18 @@ impl Drop for EvpPkeyCtx { } } +#[cfg(any(feature = "ecc", feature = "rsa"))] unsafe impl Send for EvpPkeyCtx {} +#[cfg(any(feature = "ecc", feature = "rsa"))] unsafe impl Sync for EvpPkeyCtx {} +#[cfg(any(feature = "ecc", feature = "rsa"))] #[derive(Debug)] pub struct EvpPkey { ptr: *mut EVP_PKEY, } +#[cfg(any(feature = "ecc", feature = "rsa"))] impl EvpPkey { pub fn fromdata( pkey_name: *const c_char, @@ -276,6 +283,7 @@ impl EvpPkey { self.ptr } + #[cfg(any(feature = "ecc", feature = "rsa"))] fn from_object(obj: &Object, class: CK_OBJECT_CLASS) -> Result { let key_class = match class { CKO_PUBLIC_KEY => EVP_PKEY_PUBLIC_KEY, @@ -297,10 +305,12 @@ impl EvpPkey { Self::fromdata(name, key_class, ¶ms) } + #[cfg(any(feature = "ecc", feature = "rsa"))] pub fn pubkey_from_object(obj: &Object) -> Result { Self::from_object(obj, CKO_PUBLIC_KEY) } + #[cfg(any(feature = "ecc", feature = "rsa"))] pub fn privkey_from_object(obj: &Object) -> Result { Self::from_object(obj, CKO_PRIVATE_KEY) } @@ -331,6 +341,7 @@ impl EvpPkey { } } +#[cfg(any(feature = "ecc", feature = "rsa"))] impl Drop for EvpPkey { fn drop(&mut self) { unsafe { @@ -339,7 +350,9 @@ impl Drop for EvpPkey { } } +#[cfg(any(feature = "ecc", feature = "rsa"))] unsafe impl Send for EvpPkey {} +#[cfg(any(feature = "ecc", feature = "rsa"))] unsafe impl Sync for EvpPkey {} pub const CIPHER_NAME_AES128: &[u8; 7] = b"AES128\0"; @@ -388,6 +401,7 @@ impl<'a> OsslParam<'a> { } } + #[allow(dead_code)] pub fn from_ptr(ptr: *mut OSSL_PARAM) -> Result> { if ptr.is_null() { return Err(CKR_DEVICE_ERROR)?; @@ -424,6 +438,7 @@ impl<'a> OsslParam<'a> { p } + #[allow(dead_code)] pub fn add_bn(&mut self, key: *const c_char, v: &Vec) -> Result<()> { if self.finalized { return Err(CKR_GENERAL_ERROR)?; @@ -523,6 +538,7 @@ impl<'a> OsslParam<'a> { Ok(()) } + #[allow(dead_code)] pub fn add_const_c_string( &mut self, key: *const c_char, @@ -543,6 +559,7 @@ impl<'a> OsslParam<'a> { Ok(()) } + #[allow(dead_code)] pub fn add_octet_string( &mut self, key: *const c_char, @@ -614,6 +631,7 @@ impl<'a> OsslParam<'a> { Ok(()) } + #[allow(dead_code)] pub fn add_uint( &mut self, key: *const c_char, @@ -634,6 +652,7 @@ impl<'a> OsslParam<'a> { Ok(()) } + #[allow(dead_code)] pub fn add_int( &mut self, key: *const c_char, @@ -712,6 +731,7 @@ impl<'a> OsslParam<'a> { } } + #[allow(dead_code)] pub fn as_ptr(&self) -> *const OSSL_PARAM { if !self.finalized { panic!("Unfinalized OsslParam"); @@ -719,6 +739,7 @@ impl<'a> OsslParam<'a> { self.p.as_ref().as_ptr() } + #[allow(dead_code)] pub fn as_mut_ptr(&mut self) -> *mut OSSL_PARAM { if !self.finalized { panic!("Unfinalized OsslParam"); @@ -745,6 +766,7 @@ impl<'a> OsslParam<'a> { Ok(val) } + #[allow(dead_code)] pub fn get_bn(&self, key: *const c_char) -> Result> { if !self.finalized { return Err(CKR_GENERAL_ERROR)?; @@ -777,6 +799,7 @@ impl<'a> OsslParam<'a> { Ok(vec) } + #[allow(dead_code)] pub fn get_octet_string(&self, key: *const c_char) -> Result<&'a [u8]> { if !self.finalized { return Err(CKR_GENERAL_ERROR)?; @@ -861,17 +884,25 @@ pub fn mech_type_to_digest_name(mech: CK_MECHANISM_TYPE) -> *const c_char { }) as *const c_char } +#[cfg(feature = "ecc")] pub static EC_NAME: &[u8; 3] = b"EC\0"; -#[cfg(feature = "fips")] +#[cfg(all(feature = "ecc", feature = "fips"))] pub static ECDSA_NAME: &[u8; 6] = b"ECDSA\0"; /* Curve names as used in OpenSSL */ +#[cfg(feature = "ecc")] const NAME_SECP256R1: &[u8] = b"prime256v1\0"; +#[cfg(feature = "ecc")] const NAME_SECP384R1: &[u8] = b"secp384r1\0"; +#[cfg(feature = "ecc")] const NAME_SECP521R1: &[u8] = b"secp521r1\0"; +#[cfg(feature = "ecc")] const NAME_ED25519: &[u8] = b"ED25519\0"; +#[cfg(feature = "ecc")] const NAME_ED448: &[u8] = b"ED448\0"; +#[cfg(feature = "ecc")] const NAME_X25519: &[u8] = b"X25519\0"; +#[cfg(feature = "ecc")] const NAME_X448: &[u8] = b"X448\0"; #[cfg(feature = "ecc")] From b843f5cf9d0e596280ccac4f95bd0f034cbf8240 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Mon, 16 Dec 2024 17:05:21 -0500 Subject: [PATCH 6/7] Add make target to build docs This target build both public and internal doc strings in the standard rustdoc documentation. Signed-off-by: Simo Sorce --- Makefile | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Makefile b/Makefile index 1e01a62..a96ab20 100644 --- a/Makefile +++ b/Makefile @@ -18,3 +18,6 @@ fix-format: check-spell: @.github/codespell.sh + +docs: + cargo doc --features standard,nssdb,jsondb,fips --examples --document-private-items From 751f912ffc6621e936ac0e49166efdc0a4a98898 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Mon, 16 Dec 2024 17:06:08 -0500 Subject: [PATCH 7/7] Document the config.rs file Signed-off-by: Simo Sorce --- src/config.rs | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/src/config.rs b/src/config.rs index a844c3c..f3ef463 100644 --- a/src/config.rs +++ b/src/config.rs @@ -26,6 +26,33 @@ const DEFAULT_CONF_DIR: &str = "test"; pub const DEFAULT_CONF_NAME: &str = "token.conf"; +/// Configuration for a slot +/// +/// The basic facility of a PKCS#11 is the slot. The slot represents an +/// idealized hardware slot where a token can be inserted at any time to +/// execute operations. +/// +/// In Kryoptic we use slots to allow to provide multiple independent +/// tokens with their own storage separate from any other slot. Slots +/// can't share the same storage. +/// +/// Each slot is identified by a slot number (a u32 quantity) and can +/// optionally have a customized description and manufacturer string. +/// If not description or manufacturer strings are provided then default +/// ones are set and returned to PKCS#11 applications. +/// +/// Finally the storage is defined by a pair of arguments: dbtype and dbargs +/// +/// This structure is generally sourced from a toml configuration file that +/// defines the all the slots to be exposed to the application. +/// +/// Example: +/// +/// \[\[slots\]\] +/// slot = 1 +/// dbtype = "sql" +/// dbargs = "/var/lib/kryoptic/token.sql" + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Slot { pub slot: u32, @@ -36,6 +63,9 @@ pub struct Slot { } impl Slot { + /// Creates a new empty slot with the slot number set to the special + /// indicator of u32::MAX, which will fault if encountered by the + /// configuration processing functions pub fn new() -> Slot { Slot { slot: u32::MAX, @@ -46,6 +76,9 @@ impl Slot { } } + /// Creates a new slot with a specific dbtype and db arguments set + /// The slot number is set to u32::MAX which indicates this slot still + /// needs to be assigned a specific number (tests will do that) #[cfg(test)] pub fn with_db(dbtype: &str, dbargs: Option) -> Slot { Slot { @@ -58,6 +91,15 @@ impl Slot { } } +/// For compatibility with applications that expect DER encoded EC Points +/// +/// Allows to set a global default encoding for CKA_EC_POINT attributes. +/// +/// Example: +/// +/// \[ec_point_encoding\] +/// encoding = "Bytes" + #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] #[serde(tag = "encoding")] pub enum EcPointEncoding { @@ -71,6 +113,11 @@ impl Default for EcPointEncoding { } } +/// Main configuration structure +/// +/// The main config structure is comprised of two elements, a general +/// EC Point Encoding indicator and a list of slots + #[derive(Debug, Serialize, Deserialize)] pub struct Config { #[serde(default)] @@ -83,6 +130,8 @@ fn config_error(error: E) -> Error { } impl Config { + /// Creates a new, empty, config structure, with the EC POINT Encoding + /// set to the default. pub fn new() -> Config { Config { ec_point_encoding: EcPointEncoding::default(), @@ -90,6 +139,9 @@ impl Config { } } + /// Allows to add a preconfigured slot structure. + /// Available only for tests. Ensures the slot number is set and that + /// there are no duplicates #[cfg(test)] pub fn add_slot(&mut self, slot: Slot) -> Result<()> { for s in &self.slots { @@ -101,6 +153,27 @@ impl Config { Ok(()) } + /// Find the applicable configuration for Kryoptic. + /// Kryoptic searches for a configuration file in multiple places + /// falling back from one to the next and stops once configuration file + /// is found. There is no config file merging/include support currently + /// + /// The first place where configuration is looked for is in the file + /// indicated by the `KRYOPTIC_CONF` environment variable. If this + /// variable is not set, then the code checks if the standard + /// `XDG_CONFIG_HOME` environment variable is available. + /// If this variable exists kryoptic assumes the config file is named: + /// `${XDG_CONFIG_HOME}/kryoptic/token.conf` + /// + /// Otherwise if the environment variable HOME is set the code assumes + /// the configuration file is named: + /// `${HOME}/.config/kryoptic/token.conf` + /// + /// Finally if nothing matches the code tries the relative path: + /// `test/kryoptic/token.conf` + /// + /// It is srongly advised to set the `KRYOPTIC_CONF` variable for most + /// use cases. fn find_conf() -> Result { /* First check for our own env var, * this has the highest precedence */ @@ -130,12 +203,19 @@ impl Config { } } + /// Generates a configuration structure from the named file which must + /// be a properly formatted confgiuation file in toml format. fn from_file(filename: &str) -> Result { let config_str = fs::read_to_string(filename)?; let conf: Config = toml::from_str(&config_str).map_err(config_error)?; Ok(conf) } + /// Generates a configuration structure from a legacy argument as passed + /// into the reserved argument of the `C_Initialize()` function. + /// + /// A valid argument is the path of a file for the sqlite storage driver + /// which must end with a .sql suffix fn from_legacy_conf_string(name: &str) -> Result { let mut conf = Config { ec_point_encoding: EcPointEncoding::default(), @@ -156,6 +236,9 @@ impl Config { Ok(conf) } + /// Ensure all slot numbers are consistents, and allocates new slot + /// numbers for slots that have the special invalid slow number of + /// u32::MAX fn fix_slot_numbers(&mut self) { let mut slotnum: u32 = 0; /* if there are any slot missing a valid slot number @@ -183,6 +266,8 @@ impl Config { } } + /// Generates the default configuration structure by searching the default + /// configuration file pub fn default_config() -> Result { let filename = Self::find_conf()?; @@ -201,6 +286,16 @@ impl Config { } } + /// Load environment variables overrides for configurations items. + /// + /// The only variable currently defined is `KRYOPTIC_EC_POINT_ENCODING` + /// Which can be used to override the encoding specified in the + /// configuration file. This is useful when multiple applications use + /// the same configuration file but expect different behavior from the + /// configure default: + /// + /// Example: + /// `export KRYOPTIC_EC_POINT_ENCODING="BYTES"` pub fn load_env_vars_overrides(&mut self) { match env::var("KRYOPTIC_EC_POINT_ENCODING") { Ok(var) => { @@ -218,6 +313,9 @@ impl Config { } } + /// Loads the NSS DB Storage configuration which is generally provided + /// as a complex formatted string as a reserved argument when calling + /// the `C_Intialize()` function. #[cfg(feature = "nssdb")] fn from_nss_init_args(args: &str) -> Result { let mut conf = Config { @@ -232,6 +330,8 @@ impl Config { Ok(conf) } + /// Calls the correct configuration parser based on the detected + /// database configuration string fn conf_from_args(&self, args: &str) -> Result { if args.starts_with("kryoptic_conf=") { let comps: Vec<&str> = args.splitn(2, '=').collect(); @@ -248,6 +348,8 @@ impl Config { Self::from_legacy_conf_string(args) } + /// Allows to specify the configuration file as a string provided as the + /// reserved argument of the `C_Initialize()` function. pub fn from_init_args(&mut self, args: &str) -> Result<()> { let conf = self.conf_from_args(args)?;