Skip to content

Fenrir#439

Open
jackctj117 wants to merge 7 commits into
mainfrom
Fenrir
Open

Fenrir#439
jackctj117 wants to merge 7 commits into
mainfrom
Fenrir

Conversation

@jackctj117

Copy link
Copy Markdown
Contributor

This pull request introduces several important security and correctness improvements across authentication, memory handling, and key usage policy enforcement, along with new and enhanced test coverage. The most significant changes are the implementation of constant-time credential comparison to prevent timing attacks, stricter memory scrubbing for sensitive data, and improved enforcement and testing of key access and usage policies.

Authentication Security Improvements:

  • Added a new wh_Auth_BaseCredsCompare function for constant-time credential comparison, ensuring both value and length are checked without leaking timing information. Updated all PIN and credential checks to use this function instead of direct comparison, preventing timing attacks and enforcing exact-length credential matching. [1] [2] [3]
  • Extended authentication tests to verify that credentials with incorrect lengths (shorter or longer than the correct PIN) are rejected, and that only exact matches succeed. [1] [2]

Sensitive Data Handling:

  • Ensured that key material is securely scrubbed (zeroed out) in memory before freeing temporary buffers in the DMA callback, reducing the risk of sensitive data leakage. [1] [2]

Key Access and Usage Policy Enforcement:

  • Enforced that NVM objects with WH_NVM_ACCESS_NONE deny read, overwrite, and destroy operations, ensuring a fail-closed security posture for no-access objects.
  • Updated key loading and initialization routines to explicitly set meta->access = WH_NVM_ACCESS_ANY for SHe keys and PRNG seeds, clarifying and standardizing key access defaults. [1] [2] [3] [4]
  • Modified wh_Client_ShePreProgramKey to use WH_NVM_ACCESS_ANY when adding new objects, aligning with the new access policy.

Test Coverage Enhancements:

  • Added tests to verify that objects with WH_NVM_ACCESS_NONE cannot be read, overwritten, or destroyed, and that these restrictions are enforced even without additional flags. [1] [2] [3]
  • Introduced a test to confirm that X25519 key derivation fails if the private key lacks the DERIVE usage flag, ensuring proper enforcement of key usage policies.
  • Added a test confirming that a corrupted M3 value in the SHE LOAD KEY operation is correctly rejected, and that reloading with the correct M3 at the same counter succeeds, verifying slot integrity.

These changes collectively improve the security, correctness, and testability of authentication, key management, and memory handling in the codebase.

@jackctj117 jackctj117 self-assigned this Jul 9, 2026
Copilot AI review requested due to automatic review settings July 9, 2026 19:29
@jackctj117 jackctj117 marked this pull request as draft July 9, 2026 19:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR strengthens wolfHSM’s security posture across authentication and key/NVM handling by introducing constant-time credential checks, stricter sensitive-memory scrubbing, and more explicit enforcement/testing of NVM access/usage policies.

Changes:

  • Add constant-time credential comparison helper and update authentication checks to enforce exact-length matches.
  • Enforce fail-closed behavior for WH_NVM_ACCESS_NONE objects and standardize default access for several SHE-related keys/objects.
  • Expand tests for SHE LOAD KEY integrity, NVM access policy enforcement, and Curve25519 usage-flag enforcement.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/wh_auth_base.c Introduces constant-time credential comparison helper and updates credential checks.
test/wh_test_auth.c Adds regression tests ensuring wrong-length credentials are rejected.
src/wh_nvm.c Adds fail-closed policy enforcement for WH_NVM_ACCESS_NONE.
test/wh_test_nvmflags.c Adds tests verifying WH_NVM_ACCESS_NONE denies read/overwrite/destroy.
src/wh_server_she.c Standardizes SHE object metadata defaults (notably meta->access).
src/wh_client_she.c Aligns SHE pre-program key creation with new NVM access policy expectations.
test/wh_test_she.c Adds test ensuring corrupted M3 is rejected without mutating slot state.
test/wh_test_crypto.c Adds Curve25519/X25519 test verifying DERIVE usage enforcement.
port/posix/posix_transport_shm.c Adds explicit scrubbing of temporary DMA buffers before free.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread port/posix/posix_transport_shm.c
Comment thread port/posix/posix_transport_shm.c Outdated
Comment thread test/wh_test_auth.c Outdated
Comment thread test/wh_test_auth.c Outdated
Comment thread src/wh_auth_base.c Outdated
@jackctj117 jackctj117 marked this pull request as ready for review July 9, 2026 19:44
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.

2 participants