Skip to content

Refactor lib.rs and add PKCS#11 API-level logging#427

Merged
simo5 merged 18 commits intolatchset:mainfrom
simo5:oplog
Mar 6, 2026
Merged

Refactor lib.rs and add PKCS#11 API-level logging#427
simo5 merged 18 commits intolatchset:mainfrom
simo5:oplog

Conversation

@simo5
Copy link
Member

@simo5 simo5 commented Mar 4, 2026

Description

This patchset is mainly refactoring of the lib.rs with no change in behavior except for the added debug logging functionality.

It transitions the code from a monolithic lib.rs architecture into a modular design (src/fns/*) that improves code organization and maintainability.

Key changes include:

  1. Modularization: FFI entry points and logic are moved into specific modules (e.g., digest.rs, signing.rs, keymgmt.rs).
  2. Safety & Idioms: Custom error handling macros (map_err!, res_or_ret!) are replaced with standard Rust Result combinators (?, .map_err()).
  3. Traceability: A log_debug! macro is introduced and applied to all C-API entry points to trace input arguments and return codes.
  4. Refactoring: cast_params! and bytes_to_vec! macros are replaced with generic functions, enforcing stricter type checking and requiring explicit unsafe blocks at call sites.

The changes generally follow Rust best practices and improve the safety posture of the FFI boundary.

Fixes #425

Reviewer's checklist:

  • Any issues marked for closing are fully addressed
  • There is a test suite reasonably covering new functionality or modifications
  • This feature/change has adequate documentation added
  • A changelog entry is added if the change is significant
  • Code conform to coding style that today cannot yet be enforced via the check style test
  • Commits have short titles and sensible text
  • Doc string are properly updated

@simo5 simo5 changed the title Oplog Refactor lib.rs and add PKCS#11 API-level logging Mar 4, 2026
@simo5 simo5 force-pushed the oplog branch 2 times, most recently from 178e0c5 to 735f029 Compare March 5, 2026 00:08
@simo5 simo5 requested a review from Jakuje March 5, 2026 00:08
simo5 and others added 18 commits March 6, 2026 13:45
Extract general management functions (Initialize, Finalize, GetInfo,
GetFunctionList, etc.) from `src/lib.rs` into a new `src/fns`
module to improve code organization.

Update visibility of internal structures (State, Config) and global
statics to `pub(crate)` to share them with the new module. Move
global locking macros to the new module and update cdylib imports.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Move slot and token management functions (such as C_GetSlotList,
C_InitToken, etc.) from src/lib.rs into a new dedicated module at
src/fns/stmgmt.rs.

Additionally, move internal helper macros (res_or_ret, cast_or_ret,
etc.) to src/fns/mod.rs to facilitate code reuse across modules and
clean up the main library file.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Move session management functions (such as OpenSession, CloseSession, Login,
Logout, and SessionCancel) from lib.rs into a new dedicated module. This
improves code organization by reducing the size of the main library file and
grouping related functionality together.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Move the implementation of object management functions (create, copy,
destroy, get/set attributes, and find objects) from src/lib.rs to a
new module src/fns/objmgmt.rs. This improves code organization by
reducing the size of the main library file and grouping related logic.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
This refactors the encryption, decryption, and message encryption/decryption
implementations out of `src/lib.rs` and into a new `src/fns/encryption.rs`
module.

This reduces the size of the main library file and groups related
cryptographic operations together. Several helper functions (FIPS approval
and mechanism checks) are made crate-public to support this move.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Refactor the codebase by extracting all signing, verification, and
related signature handling functions (C_Sign*, C_Verify*) from
src/lib.rs into a new dedicated module src/fns/signing.rs. This
improves code organization and reduces the file size of the main
library entry point.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Refactor the implementation of key generation, derivation, wrapping,
and encapsulation functions out of `src/lib.rs` into a new dedicated
module `src/fns/keymgmt.rs`. This improves code organization and
reduces the size of the main library file.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Relocate the implementation of dual-function cryptographic operations
(DigestEncrypt, DecryptDigest, SignEncrypt, and DecryptVerify) from
lib.rs to a new src/fns/dual.rs module.

This improves code organization and aligns with the structure of other
cryptographic functions. Additionally, make internal_digest_update
public within the crate so it can be utilized by the new module.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Extract the message digest functions (DigestInit, Digest, DigestUpdate,
DigestKey, DigestFinal) from lib.rs into a new dedicated module at
src/fns/digest.rs.

This refactoring reduces the complexity of the main library file and
improves code organization by grouping digest operations together.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Move the implementations for random number generation,
legacy control functions, and async placeholders
from src/lib.rs to src/fns/mod.rs. This improves
code organization by grouping function definitions
and decluttering the main library file.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Reorder module declarations and static definitions in lib.rs for better
organization. Move the check_test_slot_busy helper function to the tests
module to reduce clutter in the main library entry point.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Remove `global_rlock!` and `global_wlock!` macros in favor of encapsulation
within `GlobalState` and `GlobalConfig` structs. These types now manage the
internal `RwLock`s and expose `rlock()` and `wlock()` methods to handle lock
acquisition and initialization checks, resulting in cleaner usage sites and
improved type safety.

Signed-off-by: Simo Sorce <simo@redhat.com>
Refactor C-API entry points to separate FFI wrappers from implementation
logic. Move the logic to inner functions returning `Result`, allowing
idiomatic error handling with the `?` operator.

Add debug logging to the wrappers to trace function arguments and return
codes. This improves the ability to debug the module at the boundary.

Remove obsolete macros `res_or_ret`, `ret_to_rv`, and `cast_or_ret`.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Refactor the `bytes_to_vec!` and `bytes_to_slice!` macros into proper generic
functions. This change improves type safety and requires explicit `unsafe`
blocks at call sites when creating slices from raw pointers, making the code's
safety requirements more visible.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Refactor the mechanism parameter casting logic by replacing the
`cast_params!` macro with a generic `cast_params` function in `src/misc.rs`.

This change improves code clarity by removing the macro and placing the
length validation and pointer casting logic into a single function. Call
sites have been updated to use the new function and explicitly wrap the
invocation in `unsafe` blocks, as the function involves dereferencing raw
pointers.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Remove the custom `map_err!` macro definition from `src/error.rs` and replace
all usages with the standard `Result::map_err` combinator. This simplifies the
code by removing unnecessary macro indirection and utilizing standard Rust
idioms for handling integer conversion errors.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Delete `general_error`, `device_error`, `arg_bad`, and the `some_or_err`
macro from the error module. Update call sites to use
`Error::ck_rv_from_error` directly or map errors to specific PKCS#11
return values, simplifying the error handling code.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
Dereferencing `CK_MECHANISM` pointers directly can lead to unaligned memory
access, causing undefined behavior or crashes on certain architectures.

This introduces `CK_MECHANISM::from_ptr` to safely read the struct by value
using `std::ptr::read_unaligned`. All cryptographic function initialization
routines have been updated to use this method to ensure memory safety.

Co-authored-by: Gemini <gemini@google.com>
Signed-off-by: Simo Sorce <simo@redhat.com>
@simo5
Copy link
Member Author

simo5 commented Mar 6, 2026

@Jakuje pleae re-check, I have:

  1. changed all log_debug calls on return as you pointed out
  2. turned cast_params() from a standalone function into a get_parameters() method of the CK_MECHANISM object (a copy of cast_params() remains in the aes.rs file for internal use)
  3. Add a safer from_ptr method in the newly created impl for CK_MECHANISM and plugged it into places where explicit casting from CK_MECHANSIM_PTR was happening
  4. fixed the empty lines issues

Copy link
Contributor

@Jakuje Jakuje left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good now! Thanks for addressing the reported issues!

@simo5
Copy link
Member Author

simo5 commented Mar 6, 2026

I think this looks good now! Thanks for addressing the reported issues!

No, thank you for patiently going through such a big PR and uncovering these issues!

@simo5 simo5 merged commit 04cc9f4 into latchset:main Mar 6, 2026
50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add configuration option for full operation logs

2 participants