authentication manager feature addition #270

JacobBarthelmeh · 2026-01-16T18:48:33Z

The authentication manager feature adds support for a user login and checking a users permissions for performing a group+action. The API was designed with PKCS11 in mind.

Some things of note:

I added a callback function framework for checking authorization of key use based on key ID and user permissions but did not tie in that check yet. I would like to tie that in later when/if needed. This currently checks for authorization of user for a group/action that they can do. Which ties a user ID to crypto actions done.
The user list in port/posix/posix_auth.c is a simple list not yet in NVM. This initial simplicity is deliberate.
There is a TODO listed for logging of authentication events. Login failures, success, crypto actions should have logging additions in the future.

Copilot

Pull request overview

This pull request adds a comprehensive authentication and authorization manager to wolfHSM, enabling user management, login/logout functionality, and permission-based access control for HSM operations.

Changes:

New authentication manager with PIN and certificate-based authentication support
Authorization system with group and action-level permission checks
User management APIs for adding, deleting, and modifying users and their credentials
Complete client and server implementation with message translation support

Reviewed changes

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

Show a summary per file

File	Description
wolfhsm/wh_auth.h	Core auth manager types, structures, and API definitions
wolfhsm/wh_message_auth.h	Message structures and translation functions for auth operations
wolfhsm/wh_server_auth.h	Server-side auth request handler declaration
wolfhsm/wh_client.h	Client-side auth API function declarations
wolfhsm/wh_server.h	Server context updated with auth context pointer
wolfhsm/wh_message.h	New auth message group and action enums
wolfhsm/wh_error.h	New auth-specific error codes
src/wh_auth.c	Core auth manager implementation with callback wrappers
src/wh_message_auth.c	Message translation implementations for auth messages
src/wh_server_auth.c	Server-side request handler for auth operations
src/wh_client_auth.c	Client-side auth API implementations
src/wh_server.c	Server integration with authorization checks
src/wh_client.c	Minor formatting fixes
port/posix/posix_auth.h	POSIX auth backend declarations
port/posix/posix_auth.c	POSIX auth backend implementation with in-memory user storage
test/wh_test_auth.h	Auth test suite declarations
test/wh_test_auth.c	Comprehensive auth test suite implementation
test/wh_test.c	Test integration for auth tests
examples/posix/wh_posix_server/*	Server configuration with auth context setup
examples/demo/client/wh_demo_client_all.c	Demo integration for auth

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

wolfhsm/wh_client.h

test/wh_test_auth.c

port/posix/posix_auth.c

wolfhsm/wh_message_auth.h

wolfhsm/wh_client.h

Copilot

Pull request overview

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

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

port/posix/posix_auth.c

src/wh_message_auth.c

port/posix/posix_auth.c

Copilot

Pull request overview

Copilot reviewed 29 out of 30 changed files in this pull request and generated no new comments.

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

Copilot

Pull request overview

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

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

examples/demo/client/wh_demo_client_auth.c

bigbrett · 2026-01-22T17:25:38Z

@JacobBarthelmeh merge conflicts

…nt user

…e, less verbose auth demos

JacobBarthelmeh · 2026-01-22T17:39:23Z

Force pushed to resolve merge conflict.

Copilot

Pull request overview

Copilot reviewed 28 out of 29 changed files in this pull request and generated 17 comments.

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

port/posix/posix_auth.c

src/wh_auth.c

src/wh_server_auth.c

test/wh_test_clientserver.c

src/wh_client_auth.c

src/wh_server_auth.c

src/wh_message_auth.c

src/wh_client_auth.c

port/posix/posix_auth.c

Copilot

Pull request overview

Copilot reviewed 28 out of 29 changed files in this pull request and generated 19 comments.

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

wolfhsm/wh_auth.h

test/wh_test_keywrap.c

test/wh_test_crypto.c

src/wh_server_auth.c

port/posix/posix_auth.c

test/wh_test_she.c

test/wh_test_clientserver.c

port/posix/posix_auth.c

examples/demo/client/wh_demo_client_auth.c

port/posix/posix_auth.c

bigbrett

Great work Jacob! Just did a first pass of this, and it mostly aligns with what I was envisioning, but with one caveat. I think that the actual authorization check and logic should be part of the generic "front end" and not delegated to the back-end. See my "architectural concern" comment below.

Also, a few requests right off the bat, before I get too deep into review:

Make the auth feature "opt-in" so it is off by default (e.g change to WOLFHSM_CFG_CLIENT_AUTH or something like that - I like calling out client auth specifically as a feature description). Protect all auth specific code (including the struct elements in the server context) with the feature macro. Reason being I think the bulk of our customers will not be using this feature, even though it is of critical importance for FIPS L2+ customers.
Since the feature should be opt-in, ensure that when the feature is turned on, the server is able to be initialized with a NULL auth config and handle this scenario safely. This should indicate that there is no auth needed. This facilitates backwards compatibility with tests (e.g. dont need to log in as admin in every test).
Try and use WH_ERROR_OK for success return. We were inconsistent about this in old code, but want to standardize around it going forward. Also helps distinguish wolfHSM returns from other lib returns like wolfCrypt/wolfSSL

Will keep going deeper into this but figured I'd speak up early so we can discuss. Let me know your thoughts.

bigbrett · 2026-01-23T18:41:47Z

src/wh_auth.c

+
+    if (context->cb != NULL && context->cb->Init != NULL) {
+        rc = context->cb->Init(context->context, config->config);
+        if (rc != 0) {


lets try to use WH_ERROR_OK unless we are explicitly dealing with return codes from other libs like wolfCrypt

bigbrett · 2026-01-23T18:54:35Z

wolfhsm/wh_client.h

+ * @param[in] c Pointer to the client context.
+ * @param[in] method The authentication method to use (e.g.,
+ * WH_AUTH_METHOD_PIN).
+ * @param[in] username The user name to login


Mention that this is a null terminated C string

bigbrett · 2026-01-23T19:00:17Z

src/wh_client_auth.c

+#include "wolfhsm/wh_client.h"
+#include "wolfhsm/wh_auth.h"
+
+static int _wh_Client_AuthUserNameSanityCheck(const char* username)


Nits:

no need to name static helpers with full module name (e.g. _wh_Client_AuthXXX), just public functions. Keeps things shorter and easier to read IMO. Consider just _UserNameSanityCheck()

That said...

"Sanity Check" doesn't quite make the return value obvious to the reader without looking at the internal implementation. Consider "_UserNameIsValid" or something like that, so you could write if (_UserNameIsValid()) and the boolean nature of the return value is made obvious to the reader

bigbrett · 2026-01-23T19:05:10Z

src/wh_client_auth.c

+    uint8_t                     buffer[WOLFHSM_CFG_COMM_DATA_LEN] = {0};
+    whMessageAuth_LoginRequest* msg = (whMessageAuth_LoginRequest*)buffer;
+    uint8_t*                    msg_auth_data = buffer + sizeof(*msg);
+    int                         msg_size;


consider size_t or a fixed with unsigned type (if required) for sizes

bigbrett · 2026-01-23T19:06:00Z

src/wh_client_auth.c

+                                        auth_data_len);
+    } while (rc == WH_ERROR_NOTREADY);
+
+    if (rc != 0) {


WH_ERROR_OK

bigbrett · 2026-01-23T19:12:55Z

src/wh_server_auth.c

+/* This function is responsible for handling all authentication and
+ * authorization requests from the client.
+ */
+int wh_Server_HandleAuthRequest(whServerContext* server, uint16_t magic,


missing feature flag macro protection

bigbrett · 2026-01-23T19:21:30Z

port/posix/posix_auth.c

+    unsigned char credentials[WH_AUTH_BASE_MAX_CREDENTIALS_LEN];
+    uint16_t      credentials_len;
+} whAuthBase_User;
+/* TODO: Thread safety - The global users array is not protected by any


#275 :)

No action needed, just FYI

bigbrett · 2026-01-23T20:30:29Z

test/wh_test_clientserver.c

    WH_TEST_ASSERT_RETURN(client_id == client->comm->client_id);

+    /* Attempt to log in as an admin user for the rest of the tests */
+    WH_TEST_RETURN_ON_FAIL(wh_Client_AuthLogin(client, WH_AUTH_METHOD_PIN,


should be opt-in and protected by feature flags. That said, I think it would it be better to have the server's just register a NULL (e.g. uninitialized) auth context and be able to handle that? So if a server's auth context/config is never initialized but you build with auth enabled, then the server just doesn't authenticate anything and you essentially run as you would have with the feature flag disabled?

bigbrett · 2026-01-23T20:46:52Z

src/wh_auth.c

+
+/* Check on request authorization and action permissions for current user
+ * logged in */
+int wh_Auth_CheckRequestAuthorization(whAuthContext* context, uint16_t group,


Architectural concern here.

TLDR shouldn't the actual auth check be generically expressed here, vs delegated to the auth backend/provider?

I find it odd that this would be entirely delegated to the "auth backend", since this implies that the actual authorization check itself is backend specific. This doesn't make sense to me, since this is specifically intended to authorize a user request message against a given group/action pair, all of which are wolfHSM core library concepts using definitions from the core library, not something specific to the back-end auth provider. Wouldn't every back-end implement this auth check in exactly the same way outside of actually retrieving the permissions for a user from some backend-specific store/encoding? Pretty sure the latter is all that is backend specific here? The way I see it, the auth back-end is really just an abstraction for the data store for user data and permissions. IMO the actual auth portion should be enforced consistently regardless of the back-end.

Put another way: shouldn't the backend just know how to retrieve permissions for a given user and serialize it into a whAuthPermissions struct such that we can generically check its permissions here against the requested action?

bigbrett · 2026-01-23T20:50:24Z

port/posix/posix_auth.c

+    whAuthBase_User* found_user;
+    found_user = posixAuth_FindUser(username);
+    if (found_user != NULL && found_user->credentials_len == auth_data_len &&
+        memcmp(found_user->credentials, auth_data, auth_data_len) == 0) {


I know it is just a reference implementation, but as a matter of example and good practice, perhaps we should store password hashes here instead of the passwords themselves? I know it might be overkill given the threat model and that it is just stored in data vs on disk, however I could see this being low hanging fruit for complaints or people trying to be clever in pointing this out.... just a thought

JacobBarthelmeh self-assigned this Jan 16, 2026

JacobBarthelmeh requested a review from Copilot January 16, 2026 18:48

Copilot started reviewing on behalf of JacobBarthelmeh January 16, 2026 18:49 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

JacobBarthelmeh requested a review from Copilot January 16, 2026 21:50

Copilot started reviewing on behalf of JacobBarthelmeh January 16, 2026 21:50 View session

Copilot AI reviewed Jan 16, 2026

View reviewed changes

port/posix/posix_auth.c Outdated Show resolved Hide resolved

src/wh_message_auth.c Show resolved Hide resolved

port/posix/posix_auth.c Outdated Show resolved Hide resolved

JacobBarthelmeh force-pushed the auth_manager branch 2 times, most recently from 6b32384 to 1bd722a Compare January 19, 2026 22:06

JacobBarthelmeh requested a review from Copilot January 19, 2026 22:17

Copilot started reviewing on behalf of JacobBarthelmeh January 19, 2026 22:18 View session

Copilot AI reviewed Jan 19, 2026

View reviewed changes

JacobBarthelmeh requested a review from Copilot January 20, 2026 17:27

JacobBarthelmeh marked this pull request as ready for review January 20, 2026 17:30

JacobBarthelmeh assigned bigbrett, wolfSSL-Bot and billphipps and unassigned JacobBarthelmeh Jan 20, 2026

Copilot started reviewing on behalf of JacobBarthelmeh January 20, 2026 17:37 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

examples/demo/client/wh_demo_client_auth.c Show resolved Hide resolved

examples/demo/client/wh_demo_client_auth.c Outdated Show resolved Hide resolved

bigbrett assigned JacobBarthelmeh Jan 22, 2026

JacobBarthelmeh added 8 commits January 22, 2026 10:38

Initial Auth Manager framework and user add message communication

c10bed8

set credentials messaging

4565e2c

login messaging and demo login

bc75ded

variable length for set credentials message and logout/login touch up

06a8117

demo using certificates for login credentials

8c68f5d

add delete user and get user implementations

a0573be

add demo for set permissions and update delete function to have curre…

5cf731b

…nt user

check in test cases planned so far

412e41e

JacobBarthelmeh added 11 commits January 22, 2026 10:38

run git-clang-format and checking format changes

62fe4eb

adding in more function comments

cd88f8c

move base example auth users to port/posix directory

bc144d9

spelling fixes, cast on sizeof return, macro guard for certificate us…

3962458

…e, less verbose auth demos

check in auth demo client files

11e59a1

update action permissions and method in message layer

e87b4aa

fix for bitmask of permissions and remove permissions return from login

26634d5

add auth login as admin during SHE tests

0b88d71

update posix client auth demo for new login function signature

b4245cd

touch up of comments and demo

37baeab

Fix typo and remove redundent return value check

4d0af48

JacobBarthelmeh force-pushed the auth_manager branch from 04bd058 to 4d0af48 Compare January 22, 2026 17:38

account for no WOLFHSM_CFG_ENABLE_SERVER build with test case

1fad291

JacobBarthelmeh requested a review from Copilot January 22, 2026 20:33

Copilot started reviewing on behalf of JacobBarthelmeh January 22, 2026 20:34 View session

Copilot AI reviewed Jan 22, 2026

View reviewed changes

JacobBarthelmeh unassigned bigbrett, wolfSSL-Bot and billphipps Jan 22, 2026

addressing some feedback about sanity checks and null string terminators

922e60d

JacobBarthelmeh requested a review from Copilot January 22, 2026 21:48

Copilot started reviewing on behalf of JacobBarthelmeh January 22, 2026 21:48 View session

Copilot AI reviewed Jan 22, 2026

View reviewed changes

update login comments and add defensive memset's

71c5708

JacobBarthelmeh assigned bigbrett, wolfSSL-Bot and billphipps and unassigned JacobBarthelmeh Jan 22, 2026

bigbrett requested changes Jan 23, 2026

View reviewed changes

authentication manager feature addition #270

Are you sure you want to change the base?

authentication manager feature addition #270

Conversation

JacobBarthelmeh commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

bigbrett commented Jan 22, 2026

Uh oh!

JacobBarthelmeh commented Jan 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bigbrett left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JacobBarthelmeh commented Jan 16, 2026 •

edited

Loading

bigbrett Jan 23, 2026 •

edited

Loading