Adds NVM for storing users with authentication feature#290
Draft
JacobBarthelmeh wants to merge 1 commit into
Draft
Adds NVM for storing users with authentication feature#290JacobBarthelmeh wants to merge 1 commit into
JacobBarthelmeh wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
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=1builds 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.
JacobBarthelmeh
force-pushed
the
auth_nvm
branch
from
85b6f97 to
ea93c13
Compare
Comment on lines
+86
to
+93
| /* Serialize: magic + version + users (clear is_active before storing) */ | ||
| ((uint32_t*)buf)[0] = WH_AUTH_BASE_NVM_MAGIC; | ||
| ((uint16_t*)(buf + 4))[0] = WH_AUTH_BASE_NVM_VERSION; | ||
| memcpy(buf + WH_AUTH_BASE_NVM_HEADER_SIZE, users, sizeof(users)); | ||
| /* Clear is_active in serialized copy - session state is not persisted */ | ||
| for (i = 0; i < WH_AUTH_BASE_MAX_USERS; i++) { | ||
| ((whAuthBase_User*)(buf + WH_AUTH_BASE_NVM_HEADER_SIZE))[i].user.is_active = false; | ||
| } |
Comment on lines
+133
to
+136
| if (((uint32_t*)buf)[0] != WH_AUTH_BASE_NVM_MAGIC || | ||
| ((uint16_t*)(buf + 4))[0] != WH_AUTH_BASE_NVM_VERSION) { | ||
| return WH_ERROR_OK; /* Unknown format, start fresh */ | ||
| } |
Comment on lines
+116
to
+131
| rc = wh_Nvm_GetMetadata(s_auth_base_nvm, WH_NVM_ID_AUTH_USER_DB, &meta); | ||
| if (rc == WH_ERROR_NOTFOUND) { | ||
| return WH_ERROR_OK; /* No stored data, keep empty */ | ||
| } | ||
| if (rc != WH_ERROR_OK) { | ||
| return rc; | ||
| } | ||
| if (meta.len != WH_AUTH_BASE_NVM_DATA_SIZE) { | ||
| return WH_ERROR_OK; /* Version mismatch, ignore */ | ||
| } | ||
|
|
||
| rc = wh_Nvm_Read(s_auth_base_nvm, WH_NVM_ID_AUTH_USER_DB, 0, | ||
| WH_AUTH_BASE_NVM_DATA_SIZE, buf); | ||
| if (rc != WH_ERROR_OK) { | ||
| return rc; | ||
| } |
Comment on lines
+460
to
+462
| rc = wh_Auth_BasePersistToNvm(); | ||
| (void)auth_context; | ||
| return WH_ERROR_OK; | ||
| return rc; |
Comment on lines
489
to
+491
| wh_Utils_ForceZero(user, sizeof(whAuthBase_User)); | ||
| (void)context; | ||
| return WH_ERROR_OK; | ||
| return wh_Auth_BasePersistToNvm(); |
Comment on lines
+679
to
+681
| if (rc == WH_ERROR_OK) { | ||
| rc = wh_Auth_BasePersistToNvm(); | ||
| } |
Comment on lines
+679
to
+681
| if (rc == WH_ERROR_OK) { | ||
| rc = wh_Auth_BasePersistToNvm(); | ||
| } |
Comment on lines
718
to
+719
| WOLFHSM_CFG_PRINTF( | ||
| "Default auth context configured (stub implementation)\n"); | ||
|
|
||
| /* Add an admin user with permissions for everything */ | ||
| memset(&permissions, 0xFF, sizeof(whAuthPermissions)); | ||
| permissions.keyIdCount = 0; | ||
| for (i = 0; i < WH_AUTH_MAX_KEY_IDS; i++) { | ||
| permissions.keyIds[i] = 0; | ||
| } | ||
| rc = wh_Auth_BaseUserAdd(&auth_ctx, "admin", &out_user_id, permissions, | ||
| WH_AUTH_METHOD_PIN, "1234", 4); | ||
| if (rc != WH_ERROR_OK) { | ||
| WOLFHSM_CFG_PRINTF("Failed to add admin user: %d\n", rc); | ||
| return rc; | ||
| "Default auth context configured (NVM-backed user database)\n"); |
Comment on lines
+395
to
+406
| /* Try to log in as demo user - if successful, user was persisted from a | ||
| * previous server run (NVM-backed auth) */ | ||
| int rc = wh_Client_AuthLogin(clientContext, WH_AUTH_METHOD_PIN, "demo", | ||
| "1234", 4, &serverRc, &userId); | ||
| if (serverRc == WH_AUTH_NOT_ENABLED) { | ||
| return WH_ERROR_OK; /* Auth not enabled, skip */ | ||
| } | ||
| if (rc == 0 && serverRc == 0) { | ||
| printf("[AUTH-DEMO] NVM persistence verified: demo user found from " | ||
| "previous server run\n"); | ||
| (void)wh_Client_AuthLogout(clientContext, userId, &serverRc); | ||
| } |
Comment on lines
+156
to
+161
| if (config != NULL) { | ||
| const whAuthBaseConfig* base_config = (const whAuthBaseConfig*)config; | ||
| if (base_config->nvm != NULL) { | ||
| s_auth_base_nvm = (whNvmContext*)base_config->nvm; | ||
| return wh_Auth_BaseLoadFromNvm(); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on top of (#270) with adding NVM storage for users.