Skip to content

Adds NVM for storing users with authentication feature - #290

Merged
bigbrett merged 1 commit into
wolfSSL:mainfrom
JacobBarthelmeh:auth_nvm
Aug 7, 2026
Merged

Adds NVM for storing users with authentication feature#290
bigbrett merged 1 commit into
wolfSSL:mainfrom
JacobBarthelmeh:auth_nvm

Conversation

@JacobBarthelmeh

Copy link
Copy Markdown
Contributor

Builds on top of (#270) with adding NVM storage for users.

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

Adds an Authentication Manager feature with a default backend that can persist the user database into NVM, and wires auth into the client/server request path (plus tests, docs, and CI knobs).

Changes:

  • Introduces Auth Manager public API, message group/actions, and client/server handlers for login/logout/user management.
  • Adds a base auth backend (wh_auth_base) with optional NVM-backed persistence of the user DB.
  • Updates tests, examples, documentation, and CI/Makefiles to support AUTH=1 builds and auth-enabled runs.

Reviewed changes

Copilot reviewed 41 out of 43 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
wolfhsm/wh_utils.h Declares secure zero + constant-time compare helpers used by auth.
wolfhsm/wh_server_auth.h New server-side auth request handler API header.
wolfhsm/wh_server.h Adds auth context pointer to server config/context.
wolfhsm/wh_message_auth.h New auth message definitions + permissions flattening API.
wolfhsm/wh_message.h Adds AUTH message group and auth action IDs; defines WH_NUMBER_OF_GROUPS.
wolfhsm/wh_error.h Adds auth-specific error codes.
wolfhsm/wh_client.h Adds client-side auth API declarations.
wolfhsm/wh_auth_base.h Declares default auth backend with optional NVM persistence.
wolfhsm/wh_auth.h Adds core Auth Manager API/types + permissions macros.
test/wh_test_she.c Logs in as admin for auth-enabled test runs.
test/wh_test_posix_threadsafe_stress.c Skips stress test on macOS due to missing barriers.
test/wh_test_keywrap.c Logs in as admin for auth-enabled keywrap tests.
test/wh_test_crypto.c Logs in as admin for auth-enabled crypto tests; minor formatting tweaks.
test/wh_test_common.h Adds WH_TEST_SKIP and allows skip in WH_TEST_RETURN_ON_FAIL.
test/wh_test_clientserver.c Logs in as admin for auth-enabled runs; explicitly disables auth in some tests.
test/wh_test_auth.h Declares auth test entry points.
test/wh_test_auth.c Implements auth unit tests and a memory-transport auth harness.
test/wh_test.c Hooks auth tests into unit and TCP test flows.
test/Makefile Adds AUTH=1 build option; tweaks coverage gcovr behavior.
src/wh_utils.c Implements wh_Utils_ForceZero and constant-time compare.
src/wh_server_she.c Minor formatting change.
src/wh_server_auth.c Implements server-side auth request dispatch + zeroization of credentials.
src/wh_server.c Enforces auth authorization checks for requests; adds auth group handling and error formatting helper.
src/wh_message_auth.c Implements auth message translation + permissions flatten/unflatten.
src/wh_client_auth.c Implements client auth request/response helpers and blocking wrappers.
src/wh_client.c Minor formatting fix.
src/wh_auth_base.c Implements default auth backend, including NVM persistence of the user DB.
src/wh_auth.c Implements core Auth Manager wrapper logic, locking, and authorization checks.
port/posix/posix_transport_tls.c Minor formatting + comment tweaks.
examples/posix/wh_posix_server/wh_posix_server_cfg.h Adds auth config function declaration.
examples/posix/wh_posix_server/wh_posix_server_cfg.c Adds default auth configuration (NVM-backed) and seeds admin user.
examples/posix/wh_posix_server/wh_posix_server.c Initializes Auth Manager in the POSIX server example when enabled.
examples/posix/wh_posix_server/Makefile Adds coverage flags and AUTH=1 option.
examples/posix/wh_posix_client/Makefile Adds AUTH=1 option.
examples/demo/client/wh_demo_client_auth.h Declares auth demo entry point.
examples/demo/client/wh_demo_client_auth.c Adds a full auth demo (PIN + cert) and persistence check.
examples/demo/client/wh_demo_client_all.c Runs auth demo and logs in as admin before other demos.
docs/src/chapter09.md New documentation chapter for Authentication Manager.
Makefile Exports AUTH to sub-makes.
.github/workflows/code-coverage.yml Adds gcovr ignore-parse-errors option for negative hits.
.github/workflows/build-and-test.yml Adds AUTH build/test permutations (ASAN/THREADSAFE/NOCRYPTO).
.github/workflows/build-and-test-clientonly.yml Adds client-only AUTH testing against auth+non-auth servers.
.github/workflows/build-and-run-examples.yml Adds matrix option to build/run examples with AUTH=1.

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

Comment thread src/wh_auth_base.c Outdated
Comment thread src/wh_auth_base.c Outdated
Comment thread src/wh_server.c Outdated
Comment thread src/wh_utils.c

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

Copilot reviewed 4 out of 4 changed files in this pull request and generated 11 comments.

Comment thread src/wh_auth_base.c Outdated
Comment thread src/wh_auth_base.c Outdated
Comment thread src/wh_auth_base.c Outdated
Comment thread src/wh_auth_base.c Outdated
Comment thread src/wh_auth_base.c Outdated
Comment thread src/wh_auth_base.c
Comment thread src/wh_auth_base.c
Comment thread examples/posix/wh_posix_server/wh_posix_server_cfg.c Outdated
Comment thread examples/demo/client/wh_demo_client_auth.c
Comment thread src/wh_auth_base.c

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

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comment thread examples/demo/client/wh_demo_client_auth.c

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

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comment thread src/wh_auth_base.c Outdated
Comment thread examples/posix/wh_posix_server/wh_posix_server_cfg.c
Comment thread src/wh_auth_base.c

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

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread src/wh_auth_base.c Outdated

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #290

Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src

No new issues found in the changed files. ✅

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

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread src/wh_auth_base.c Outdated
Comment thread src/wh_auth_base.c

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

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comment thread wolfhsm/wh_auth_base.h Outdated
Comment thread docs/draft/auth.md Outdated
Comment thread docs/draft/auth.md Outdated

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #290

Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

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

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread src/wh_auth_base.c Outdated

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fenrir Automated Review — PR #290

Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread src/wh_auth_base.c

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

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Comment thread src/wh_auth_base.c
Comment thread src/wh_auth_base.c Outdated
Comment thread src/wh_auth_base.c Outdated

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

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Comment thread src/wh_auth_base.c
Comment thread wolfhsm/wh_auth_base.h
@JacobBarthelmeh
JacobBarthelmeh marked this pull request as ready for review July 16, 2026 19:33
@JacobBarthelmeh

Copy link
Copy Markdown
Contributor Author

@bigbrett revived this and got it to the point where I think it's ready for review. It adds in a default NVM option for user storage, and a nice addition of separating user meta data from user credentials stored.

@bigbrett bigbrett 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.

Overall, this looks good. I found a few cases where a client could still erase or replace credentials through another path, especially the certificate API. That API intentionally allows some protected objects to be updated so trust anchors can be rotated.

This branch has fallen behind main, which now includes changes that can help close these gaps. I recommend the following as a follow up PR

Since this is an "experimental" feature I'm going to approve/merge now, but leaving this for follow up:

  1. Rebase onto main.

    This will bring in the WH_NVM_FLAGS_SERVER_ONLY mask. Client-provided metadata is already stripped of these flags by wh_Nvm_AddObjectChecked, and wh_Nvm_CheckPolicy rejects them for add, destroy, and read operations.

  2. Add a new WH_NVM_FLAGS_SERVER_INTERNAL flag using bit 13, and include it in WH_NVM_FLAGS_SERVER_ONLY.

    Also update these checks that this issue made me realize I missed:

    • In wh_Nvm_CheckPolicy, check the full SERVER_ONLY mask instead of only TRUSTED. The "server only" bits being set should indicate that a client can never modify the object
    • Make the same change to the keystore freeze check at wh_server_keystore.c:215.
    • Keep the key-encryption-key check at wh_server_keystore.c:1484 limited to TRUSTED.

    The other checks already use the full mask, so they should handle the new flag automatically.

  3. Mark auth objects created by the server as SERVER_INTERNAL.

    Add the flag where these objects are created in wh_auth_base.c:154 and wh_auth_base.c:206.

    This prevents client-facing APIs, including the certificate API, from modifying or replacing these objects through another object type.

    It should not interfere with the server’s own work because its writes and deletes use the unchecked backend functions: wh_Nvm_AddObjectWithReclaim and wh_Nvm_DestroyObjects.

  4. Require SERVER_INTERNAL when loading credentials.

    Add this check to both wh_Auth_BaseLoadCredHeader and wh_Auth_BaseLoadFromNvm, alongside the existing length and magic-value checks.

    This is important because the flags only protect objects that already exist. If an object does not exist, wh_Nvm_CheckPolicy returns after GetMetadata reports NOTFOUND.

    A client could therefore create an object using an unused reserved ID, such as 0xFE04 when only three users exist, or use the slot of a user that was created without credentials.

    On the next boot, the loader could treat that object as a valid credential because it currently reads the method from meta.label[23] and the length from meta.len without checking where the object came from.

    Clients cannot set SERVER_INTERNAL, so requiring that flag during loading confirms that the credential was created by the server.

Comment thread docs/draft/auth.md

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.

please update the actual docs now. There is a whole section on auth in 5-Features.md. I should probably remove docs/draft.md

Comment thread docs/draft/auth.md
- NVM not configured (`cfg.nvm == NULL`, or a NULL config): the backend runs RAM-only. `s_auth_base_nvm` stays NULL, no NVM operations are performed (nothing read at init, nothing written on change), and credentials live in the `s_auth_base_ram_cred` fallback table. All state is lost on restart.
- NVM configured (`cfg.nvm != NULL`): the index is loaded at init; the index and the affected credential object are rewritten on every change.

> Note on `WOLFHSM_CFG_AUTH_BASE_NVM_ONLY` (RAM savings): the RAM-only mode requires a static fallback table sized to hold every user's full credential blob at once, `s_auth_base_ram_cred[WH_AUTH_BASE_MAX_USERS][WH_AUTH_BASE_MAX_CREDENTIALS_LEN]`, i.e. `5 x 2048 = 10 KiB` at the defaults. On a memory-constrained target that always configures an NVM backend, define `WOLFHSM_CFG_AUTH_BASE_NVM_ONLY` to compile that table out entirely (and drop support for the `nvm == NULL` configuration). Credentials then only ever occupy the single shared `s_auth_base_cred_buf` scratch buffer (`WH_AUTH_BASE_MAX_CREDENTIALS_LEN` = 2 KiB, force-zeroed after each use) while one credential is being read or written, rather than a per-user resident copy. That reclaims roughly `(MAX_USERS - 1) x MAX_CREDENTIALS_LEN` of RAM.

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.

Is there any advantage to having RAM + NVM? Or even the pure RAM version at all? I def want to keep it flexible, so perhaps no action required, but just thinking out loud.

RAM only is probably good to keep around only because we do have some platforms without dedicated HSM NVM object storage, however we would want the capability for the DB objects to be "wrapped" for it to work in this case, so they can be stored client-side encrypted. But this is WAY beyond the scope of this PR so dont' worry about that for now.

However do we think that if you have NVM backing, is there a reason you would ever need the fallback table at all? Might as well save space if possible?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No advantage to both on at the same time, could be just one or the other. I like keeping the RAM around though for its simplicity and possibly trust zone use or in simple cases where no NVM is available. Wrapping the RAM would be neat, could do something similar to the PKCS11 token wrapping here (wolfSSL/wolfPKCS11#159) where a devices unique key is used to encrypt the RAM when "saved" and "restored".

@bigbrett
bigbrett merged commit 34f699c into wolfSSL:main Aug 7, 2026
108 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.

4 participants