authentication manager feature addition by JacobBarthelmeh · Pull Request #270 · wolfSSL/wolfHSM

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

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.

src/wh_auth_base.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

src/wh_auth_base.c

…port auth

…ouch up for test cases

bigbrett

Found a bug and have a few suggestions.

bigbrett · 2026-02-05T16:46:21Z

src/wh_auth.c

+            rc = WH_ERROR_OK;
+        }
+        else {
+            if (user->permissions.groupPermissions & group) {


This is wrong / a bug. As written, it would grant users access to groups they don't belong to.

This check treats permissions.groupPermissions as a bitmask but compares it directly against group identifier values like WH_MESSAGE_GROUP_AUTH (0x0D00), which are not intended as bitmasks (e.g. not limited to powers of two). This means a permission set intended to allow only AUTH (0x0D00) also unintentionally authorizes COMM (0x0100), CRYPTO (0x0400), and COUNTER (0x0800) because those bits overlap.

You need to reconsolidate this logic to clearly define what is a identifier, what is a bit position index, and what is a bitmask. Pls make sure they are clearly identified and not easily mixed up when reading the code. Recommend helper macros to easily convert between the two representations (logical group ID and the actual group bit permission flag)

bigbrett · 2026-02-05T16:58:03Z

port/posix/posix_auth.h

+ */
+int posixAuth_Login(void* context, uint8_t client_id, whAuthMethod method,
+                      const char* username, const void* auth_data,
+                      uint16_t auth_data_len, uint16_t* out_user_id,


user ID here (and elsewhere in this file) should be whUserId not uint16_t right?

bigbrett · 2026-02-05T17:01:57Z

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


FYI #275 merged. Want to take a crack at adding the lock protection? Can also defer to a subsequent PR too.

bigbrett · 2026-02-05T17:09:53Z

src/wh_auth_base.c

I'm wondering if this shouldn't be scoped to the POSIX port and called "posix auth" since there is nothing POSIX-specific about it. You could theoretically use this on any of our ports. Perhaps we can include this as a built-in auth mechanism that could be used by any port, similar to our shared memory transport impl (wh_transport_mem.{c,h}). Any objection to this? Could call it wh_auth_basic or something like that?

Did you have a scheme to persist the auth database in mind? Here it seems like it is intended for the server to have a hardcoded list and just iterate over each element and "add user" on startup before processing requests. Is that the intent? Could we add an InitFromNvm function that takes an object ID as an argument, where the auth list could be pulled out of NVM and configured out-of-band (e.g. provisioned into NVM at factory)? I know there will be the typical issues of serializing a raw struct, compat across build config updates, but perhaps this is ok for now?

Yeah it could be moved to a non posix location, at one point considered placing it in the examples directory. Showing it as a simplified example of user database. I'll look at moving it there or in demo's, I think in a different location than port makes sense for it.

NVM had crossed my mind and it would be great for persistence. Wanted to keep this initial users setup as simple as possible though while ironing out the authentication part of the feature in wolfHSM itself.

bigbrett

(part 2, sorry terminated my last review early)

bigbrett · 2026-02-05T17:38:30Z

wolfhsm/wh_message_auth.h

+enum WH_MESSAGE_AUTH_ACTION_ENUM {
+    WH_MESSAGE_AUTH_ACTION_AUTHENTICATE         = 0x01,
+    WH_MESSAGE_AUTH_ACTION_LOGIN                = 0x02,
+    WH_MESSAGE_AUTH_ACTION_LOGOUT               = 0x03,
+    WH_MESSAGE_AUTH_ACTION_USER_ADD             = 0x04,
+    WH_MESSAGE_AUTH_ACTION_USER_DELETE          = 0x05,
+    WH_MESSAGE_AUTH_ACTION_USER_GET             = 0x06,
+    WH_MESSAGE_AUTH_ACTION_USER_SET_PERMISSIONS = 0x07,
+    WH_MESSAGE_AUTH_ACTION_USER_SET_CREDENTIALS = 0x08,
+};


these are duplicated in wh_message.h

bigbrett · 2026-02-05T17:39:20Z

wolfhsm/wh_message.h

+    WH_AUTH_ACTION_USER_DELETE,
+    WH_AUTH_ACTION_USER_MODIFY,
+    WH_AUTH_ACTION_PERMISSION_SET,
+};


duplicated in wh_message_auth.h. Imagine these ones can be deleted?

bigbrett · 2026-02-05T17:41:33Z

src/wh_server.c

+/* Helper to format an authorization error response for any group/action.
+ * All response structures have int32_t rc as the first field.
+ * Returns the response size to send. */
+static uint16_t _wh_Server_FormatAuthErrorResponse(uint16_t magic,


Suggested change

static uint16_t _wh_Server_FormatAuthErrorResponse(uint16_t magic,

static uint16_t _FormatAuthErrorResponse(uint16_t magic,

bigbrett · 2026-02-05T17:45:29Z

wolfhsm/wh_auth.h

+    WH_AUTH_METHOD_CERTIFICATE,
+} whAuthMethod;
+
+#define WH_NUMBER_OF_GROUPS 14


This shouldn't be hardcoded here. It should ideally be defined with the actual groups in wh_message.h. Perhaps can be auto-computed based on the new group ID fields you may need to create to fix the bug in the previous review?

bigbrett · 2026-02-05T17:52:25Z

src/wh_auth.c

+                    /* Get word index and bitmask for this action */
+                    uint32_t wordAndBit = WH_AUTH_ACTION_TO_WORD_AND_BIT(action);
+                    uint32_t wordIndex   = WH_AUTH_ACTION_WORD(wordAndBit);
+                    uint32_t bitmask     = WH_AUTH_ACTION_BIT(wordAndBit);
+
+                    if (wordIndex < WH_AUTH_ACTION_WORDS &&
+                        (user->permissions.actionPermissions[groupIndex]
+                                                                [wordIndex] &
+                            bitmask)) {
+                        rc = WH_ERROR_OK;


Think there is another bug here: The packing scheme here has a collision problem. The macros pack two values into overlapping bit ranges. The word index is stored in bits 16-23, but the bitmask occupies bits 0-31 - so when action % 32 > 16, the bitmask bleeds into the word index bits.

For example, action 49 gives word=1 and bit position=17. The packed value becomes (1 << 16) | (1 << 17) = 0x30000. When we extract the word index with (0x30000 >> 16) & 0xFF, we get 3 instead of 1. The bitmask extraction is also wrong - we get 0x30000 instead of 0x20000

This affects actions 48-63, 80-95, 112-127, 144-159, 176-191, 208-223, and 240-255 (about half of all possible actions). Authorization checks for these actions will look at the wrong word in the permissions array and check the wrong bits.

Suggest switching to a uint64_t return type with the word index in the upper 32 bits to avoid any overlap.

bigbrett · 2026-02-05T17:53:58Z

wolfhsm/wh_auth.h

+    void*      context;
+} whAuthContext;
+
+#define WOLFHSM_MAX_CERTIFICATE_LEN 2048


this should not be defined here. Don't we already have a macro for this? It should also be WOFHSM_CFG_XXX if you are adding a new config value.

bigbrett · 2026-02-05T17:54:53Z

src/wh_auth.c

+    (void)context;
+    (void)action; /* Action could be used for future fine-grained key access


why are we void-casting these?

bigbrett · 2026-02-05T17:56:08Z

wolfhsm/wh_message_auth.h

+#include "wolfhsm/wh_auth.h"
+
+enum WH_MESSAGE_AUTH_ACTION_ENUM {
+    WH_MESSAGE_AUTH_ACTION_AUTHENTICATE         = 0x01,


This isn't ever used. Is there a future intent here?

…ove unused enum

…d test that authorization callback override is being called when set

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 36 out of 37 changed files in this pull request and generated 13 comments.

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

src/wh_auth.c

src/wh_message_auth.c

src/wh_server_auth.c

src/wh_client_auth.c

src/wh_server.c

src/wh_auth.c

src/wh_message_auth.c

Copilot

Pull request overview

Copilot reviewed 39 out of 41 changed files in this pull request and generated 6 comments.

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

src/wh_message_auth.c

src/wh_client_auth.c

src/wh_auth.c

src/wh_auth_base.c

wolfhsm/wh_message_auth.h

…ique max credentials, add force zero to utils

Copilot

Pull request overview

Copilot reviewed 39 out of 41 changed files in this pull request and generated 7 comments.

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

wolfhsm/wh_message_auth.h

examples/demo/client/wh_demo_client_auth.c

src/wh_server_auth.c

src/wh_auth_base.c

src/wh_client_auth.c

port/posix/posix_transport_tls.c

wolfhsm/wh_message.h

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 Outdated Show resolved Hide resolved

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

bigbrett assigned JacobBarthelmeh Jan 22, 2026

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

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

JacobBarthelmeh added 6 commits February 2, 2026 16:16

add server simple response back of auth not enabled

d97f88c

add WH_TEST_SKIP and authentication skipping when server does not sup…

f66c4ab

…port auth

move most of authentication logic into wolfHSM rather than in port, t…

c6b9a31

…ouch up for test cases

fix for scan-build warning

37c9263

spelling fix, additional sanity checks

131812c

add flag to avoid gcc coverage bug 68080

4f5b452

JacobBarthelmeh force-pushed the auth_manager branch from 9c1c042 to 4f5b452 Compare February 2, 2026 23:16

bigbrett requested changes Feb 5, 2026

View reviewed changes

bigbrett assigned JacobBarthelmeh and unassigned bigbrett and billphipps Feb 5, 2026

bigbrett requested changes Feb 5, 2026

View reviewed changes

JacobBarthelmeh added 3 commits February 5, 2026 22:00

use boolean array for group permissions, fix permissions bitmask, rem…

acec44c

…ove unused enum

add locks around user login / modify sections of code for thread safety

a12f422

move base implementation from port/posix to src/wh_auth_base.c and ad…

8f73877

…d test that authorization callback override is being called when set

JacobBarthelmeh requested a review from Copilot February 6, 2026 17:10

Copilot started reviewing on behalf of JacobBarthelmeh February 6, 2026 17:20 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

JacobBarthelmeh requested a review from Copilot February 6, 2026 17:47

Copilot started reviewing on behalf of JacobBarthelmeh February 6, 2026 17:48 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

git-clang-format, add additional sanity checks, add lock for auth check

ba82b7c

JacobBarthelmeh requested a review from Copilot February 6, 2026 20:56

Copilot started reviewing on behalf of JacobBarthelmeh February 6, 2026 20:57 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

src/wh_message_auth.c Show resolved Hide resolved

src/wh_client_auth.c Outdated Show resolved Hide resolved

src/wh_auth.c Show resolved Hide resolved

src/wh_auth.c Show resolved Hide resolved

src/wh_auth_base.c Show resolved Hide resolved

wolfhsm/wh_message_auth.h Show resolved Hide resolved

add admin boolean to permissions, aditional delete user test case, un…

b57aceb

…ique max credentials, add force zero to utils

JacobBarthelmeh requested a review from Copilot February 6, 2026 22:19

Copilot started reviewing on behalf of JacobBarthelmeh February 6, 2026 22:20 View session

Copilot AI reviewed Feb 6, 2026

View reviewed changes

add a WH_MESSAGE_GROUP_MAX and use it to get the number of groups

2e66b0e

	static uint16_t _wh_Server_FormatAuthErrorResponse(uint16_t magic,
	static uint16_t _FormatAuthErrorResponse(uint16_t magic,

		(void)context;
		(void)action; /* Action could be used for future fine-grained key access

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!

bigbrett left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

JacobBarthelmeh commented Jan 16, 2026 •

edited

Loading

bigbrett left a comment •

edited

Loading