use boolean array for group permissions, fix permissions bitmask, remove unused enum

JacobBarthelmeh · JacobBarthelmeh · commit 31b7a9005366 · 2026-04-13T10:48:28.000-06:00
diff --git a/examples/demo/client/wh_demo_client_auth.c b/examples/demo/client/wh_demo_client_auth.c
@@ -377,13 +377,14 @@ static int wh_DemoClient_AuthUserSetPermissions(whClientContext* clientContext)
 
     /* Enable CRYPTO group and all CRYPTO actions */
     memset(&permissions, 0, sizeof(permissions));
-    permissions.groupPermissions |= WH_MESSAGE_GROUP_CRYPTO;
-    
+
     /* Enable all CRYPTO actions by setting all bits in all words, an example of
      * a CRYPTO action is WC_ALGO_TYPE_CIPHER or WC_ALGO_TYPE_PK */
     {
         int groupIndex = (WH_MESSAGE_GROUP_CRYPTO >> 8) & 0xFF;
         int wordIndex;
+        /* Enable access to CRYPTO group */
+        permissions.groupPermissions[groupIndex] = 1;
         /* Set all action bits for CRYPTO group (allows all actions) */
         for (wordIndex = 0; wordIndex < WH_AUTH_ACTION_WORDS; wordIndex++) {
             permissions.actionPermissions[groupIndex][wordIndex] = 0xFFFFFFFF;
diff --git a/port/posix/posix_auth.c b/port/posix/posix_auth.c
@@ -181,7 +181,7 @@ static whAuthBase_User* posixAuth_CheckCertificate(const char* username,
 
 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,
+                      uint16_t auth_data_len, whUserId* out_user_id,
                       whAuthPermissions* out_permissions, int* loggedIn)
 {
     whAuthBase_User* current_user = NULL;
@@ -277,7 +277,7 @@ int posixAuth_CheckKeyAuthorization(void* context, int err, uint16_t user_id,
 
 
 int posixAuth_UserAdd(void* context, const char* username,
-                        uint16_t* out_user_id, whAuthPermissions permissions,
+                        whUserId* out_user_id, whAuthPermissions permissions,
                         whAuthMethod method, const void* credentials,
                         uint16_t credentials_len)
 {
@@ -420,7 +420,7 @@ int posixAuth_UserSetPermissions(void* context, uint16_t current_user_id,
 
 
 int posixAuth_UserGet(void* context, const char* username,
-                        uint16_t*          out_user_id,
+                        whUserId*          out_user_id,
                         whAuthPermissions* out_permissions)
 {
     whAuthBase_User* user = posixAuth_FindUser(username);
diff --git a/port/posix/posix_auth.h b/port/posix/posix_auth.h
@@ -66,7 +66,7 @@ int posixAuth_Cleanup(void* context);
  */
 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,
+                      uint16_t auth_data_len, whUserId* out_user_id,
                       whAuthPermissions* out_permissions, int* loggedIn);
 
 /**
@@ -111,7 +111,7 @@ int posixAuth_CheckKeyAuthorization(void* context, int err, uint16_t user_id,
  * @return int Returns 0 on success, or a negative error code on failure.
  */
 int posixAuth_UserAdd(void* context, const char* username,
-                        uint16_t* out_user_id, whAuthPermissions permissions,
+                        whUserId* out_user_id, whAuthPermissions permissions,
                         whAuthMethod method, const void* credentials,
                         uint16_t credentials_len);
 
@@ -149,7 +149,7 @@ int posixAuth_UserSetPermissions(void* context, uint16_t current_user_id,
  * @return int Returns 0 on success, or a negative error code on failure.
  */
 int posixAuth_UserGet(void* context, const char* username,
-                        uint16_t*          out_user_id,
+                        whUserId*          out_user_id,
                         whAuthPermissions* out_permissions);
 
 /**
@@ -171,4 +171,4 @@ int posixAuth_UserSetCredentials(void* context, uint16_t user_id,
                                    const void*  new_credentials,
                                    uint16_t     new_credentials_len);
 
-#endif /* PORT_POSIX_POSIX_AUTH_H_ */
+#endif /* PORT_POSIX_POSIX_AUTH_H_ */
diff --git a/src/wh_auth.c b/src/wh_auth.c
@@ -189,13 +189,15 @@ int wh_Auth_CheckRequestAuthorization(whAuthContext* context, uint16_t group,
             rc = WH_ERROR_OK;
         }
         else {
-            if (user->permissions.groupPermissions & group) {
+            if (user->permissions.groupPermissions[groupIndex]) {
                 /* Check if action is within supported range */
                 if (action < WH_AUTH_ACTIONS_PER_GROUP) {
                     /* 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);
+                    uint32_t wordIndex;
+                    uint32_t bitmask;
+
+                    WH_AUTH_ACTION_TO_WORD_AND_BITMASK(action, wordIndex,
+                        bitmask);
 
                     if (wordIndex < WH_AUTH_ACTION_WORDS &&
                         (user->permissions.actionPermissions[groupIndex]
@@ -255,9 +257,6 @@ int wh_Auth_CheckKeyAuthorization(whAuthContext* context, uint32_t key_id,
         }
     }
 
-    (void)context;
-    (void)action; /* Action could be used for future fine-grained key access
-                     control */
     if (context->cb->CheckKeyAuthorization != NULL) {
         rc = context->cb->CheckKeyAuthorization(context->context, rc,
             user_id, key_id, action);
diff --git a/src/wh_message_auth.c b/src/wh_message_auth.c
@@ -126,19 +126,21 @@ int wh_MessageAuth_FlattenPermissions(whAuthPermissions* permissions,
         return WH_ERROR_BADARGS;
     }
 
-    /* Serialize groupPermissions (2 bytes) */
-    buffer[idx++] = (uint8_t)(permissions->groupPermissions & 0xFF);
-    buffer[idx++] = (uint8_t)((permissions->groupPermissions >> 8) & 0xFF);
+    /* Serialize groupPermissions array (WH_NUMBER_OF_GROUPS bytes) */
+    for (i = 0; i < WH_NUMBER_OF_GROUPS; i++) {
+        buffer[idx++] = permissions->groupPermissions[i];
+    }
 
-    /* Serialize actionPermissions array (4*WH_NUMBER_OF_GROUPS*WH_AUTH_ACTION_WORDS bytes) */
+    /* Serialize actionPermissions array
+     * (4*WH_NUMBER_OF_GROUPS*WH_AUTH_ACTION_WORDS bytes) */
     for (i = 0; i < WH_NUMBER_OF_GROUPS; i++) {
         int j;
         for (j = 0; j < WH_AUTH_ACTION_WORDS; j++) {
             uint32_t actionPerm = permissions->actionPermissions[i][j];
-            buffer[idx++] = (uint8_t)(actionPerm & 0xFF);
-            buffer[idx++] = (uint8_t)((actionPerm >> 8) & 0xFF);
-            buffer[idx++] = (uint8_t)((actionPerm >> 16) & 0xFF);
-            buffer[idx++] = (uint8_t)((actionPerm >> 24) & 0xFF);
+            buffer[idx++]       = (uint8_t)(actionPerm & 0xFF);
+            buffer[idx++]       = (uint8_t)((actionPerm >> 8) & 0xFF);
+            buffer[idx++]       = (uint8_t)((actionPerm >> 16) & 0xFF);
+            buffer[idx++]       = (uint8_t)((actionPerm >> 24) & 0xFF);
         }
     }
 
@@ -179,25 +181,25 @@ int wh_MessageAuth_UnflattenPermissions(uint8_t* buffer, uint16_t buffer_len,
         return WH_ERROR_BADARGS;
     }
 
-    /* Deserialize groupPermissions (2 bytes) */
-    permissions->groupPermissions = buffer[idx] | (buffer[idx + 1] << 8);
-    idx += 2;
+    /* Deserialize groupPermissions array (WH_NUMBER_OF_GROUPS bytes) */
+    for (i = 0; i < WH_NUMBER_OF_GROUPS; i++) {
+        permissions->groupPermissions[i] = buffer[idx++];
+    }
 
-    /* Deserialize actionPermissions array (4*WH_NUMBER_OF_GROUPS*WH_AUTH_ACTION_WORDS bytes) */
+    /* Deserialize actionPermissions array
+     * (4*WH_NUMBER_OF_GROUPS*WH_AUTH_ACTION_WORDS bytes) */
     for (i = 0; i < WH_NUMBER_OF_GROUPS; i++) {
         int j;
         for (j = 0; j < WH_AUTH_ACTION_WORDS; j++) {
             permissions->actionPermissions[i][j] =
-                buffer[idx] |
-                (buffer[idx + 1] << 8) |
-                (buffer[idx + 2] << 16) |
-                (buffer[idx + 3] << 24);
+                (uint32_t)(buffer[idx] | (buffer[idx + 1] << 8) |
+                           (buffer[idx + 2] << 16) | (buffer[idx + 3] << 24));
             idx += 4;
         }
     }
 
     /* Deserialize keyIdCount (2 bytes) */
-    keyIdCount = buffer[idx] | (buffer[idx + 1] << 8);
+    keyIdCount = (uint16_t)(buffer[idx] | (buffer[idx + 1] << 8));
     idx += 2;
     if (keyIdCount > WH_AUTH_MAX_KEY_IDS) {
         keyIdCount = WH_AUTH_MAX_KEY_IDS;
@@ -206,10 +208,8 @@ int wh_MessageAuth_UnflattenPermissions(uint8_t* buffer, uint16_t buffer_len,
 
     /* Deserialize keyIds array (4*WH_AUTH_MAX_KEY_IDS bytes) */
     for (i = 0; i < WH_AUTH_MAX_KEY_IDS; i++) {
-        keyId = buffer[idx] |
-                (buffer[idx + 1] << 8) |
-                (buffer[idx + 2] << 16) |
-                (buffer[idx + 3] << 24);
+        keyId = (uint32_t)(buffer[idx] | (buffer[idx + 1] << 8) |
+                           (buffer[idx + 2] << 16) | (buffer[idx + 3] << 24));
         permissions->keyIds[i] = keyId;
         idx += 4;
     }
diff --git a/src/wh_server.c b/src/wh_server.c
@@ -315,7 +315,7 @@ static int _wh_Server_HandlePkcs11Request(whServerContext* server,
 /* 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,
+static uint16_t _FormatAuthErrorResponse(uint16_t magic,
                                                    uint16_t group,
                                                    uint16_t action,
                                                    int32_t  error_code,
@@ -524,7 +524,7 @@ int wh_Server_HandleRequestMessage(whServerContext* server)
                 /* Authorization failed - format and send error response to
                  * client */
                 int32_t  error_code = (int32_t)WH_AUTH_PERMISSION_ERROR;
-                uint16_t resp_size  = _wh_Server_FormatAuthErrorResponse(
+                uint16_t resp_size  = _FormatAuthErrorResponse(
                     magic, group, action, error_code, data);
 
                 /* Send error response to client */
diff --git a/test/wh_test_auth.c b/test/wh_test_auth.c
@@ -856,13 +856,14 @@ int whTest_AuthSetPermissions(whClientContext* client)
     /* Test 2b: Set user permissions success path  */
     WH_TEST_PRINT("  Test: Set user permissions success\n");
     memset(&new_perms, 0, sizeof(new_perms));
-    new_perms.groupPermissions = WH_MESSAGE_GROUP_AUTH;
     /* Convert action enum value to bitmask: action 0x04 -> word 0, bit 4 -> 0x10 */
     {
-        int groupIndex = (WH_MESSAGE_GROUP_AUTH >> 8) & 0xFF;
-        uint32_t wordAndBit = WH_AUTH_ACTION_TO_WORD_AND_BIT(WH_MESSAGE_AUTH_ACTION_USER_ADD);
-        uint32_t wordIndex = WH_AUTH_ACTION_WORD(wordAndBit);
-        uint32_t bitmask = WH_AUTH_ACTION_BIT(wordAndBit);
+        int      groupIndex = (WH_MESSAGE_GROUP_AUTH >> 8) & 0xFF;
+        uint32_t wordIndex;
+        uint32_t bitmask;
+        WH_AUTH_ACTION_TO_WORD_AND_BITMASK(WH_MESSAGE_AUTH_ACTION_USER_ADD,
+                                           wordIndex, bitmask);
+        new_perms.groupPermissions[groupIndex]             = 1;
         new_perms.actionPermissions[groupIndex][wordIndex] = bitmask;
     }
     server_rc = 0;
@@ -878,13 +879,14 @@ int whTest_AuthSetPermissions(whClientContext* client)
         client, "testuser3", &get_rc, &fetched_user_id, &fetched_perms));
     WH_TEST_ASSERT_RETURN(get_rc == WH_ERROR_OK);
     WH_TEST_ASSERT_RETURN(fetched_user_id == user_id);
-    WH_TEST_ASSERT_RETURN(fetched_perms.groupPermissions ==
-                          new_perms.groupPermissions);
     {
-        /* Compare all action permission words for this group */
-        int groupIndex = (WH_MESSAGE_GROUP_AUTH >> 8) & 0xFF;
+        /* Compare group permission and all action permission words */
+        int groupIndex        = (WH_MESSAGE_GROUP_AUTH >> 8) & 0xFF;
         int j;
         int permissions_match = 1;
+        /* Verify groupPermissions for this group */
+        WH_TEST_ASSERT_RETURN(fetched_perms.groupPermissions[groupIndex] ==
+                              new_perms.groupPermissions[groupIndex]);
         for (j = 0; j < WH_AUTH_ACTION_WORDS; j++) {
             if (fetched_perms.actionPermissions[groupIndex][j] !=
                 new_perms.actionPermissions[groupIndex][j]) {
@@ -1105,13 +1107,14 @@ int whTest_AuthRequestAuthorization(whClientContext* client)
     WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);
 
     memset(&perms, 0, sizeof(perms));
-    perms.groupPermissions = WH_MESSAGE_GROUP_AUTH;
     /* Convert action enum value to bitmask: action 0x04 -> word 0, bit 4 -> 0x10 */
     {
-        int groupIndex = (WH_MESSAGE_GROUP_AUTH >> 8) & 0xFF;
-        uint32_t wordAndBit = WH_AUTH_ACTION_TO_WORD_AND_BIT(WH_MESSAGE_AUTH_ACTION_USER_ADD);
-        uint32_t wordIndex = WH_AUTH_ACTION_WORD(wordAndBit);
-        uint32_t bitmask = WH_AUTH_ACTION_BIT(wordAndBit);
+        int      groupIndex = (WH_MESSAGE_GROUP_AUTH >> 8) & 0xFF;
+        uint32_t wordIndex;
+        uint32_t bitmask;
+        WH_AUTH_ACTION_TO_WORD_AND_BITMASK(WH_MESSAGE_AUTH_ACTION_USER_ADD,
+                                           wordIndex, bitmask);
+        perms.groupPermissions[groupIndex]             = 1;
         perms.actionPermissions[groupIndex][wordIndex] = bitmask;
     }
     WH_TEST_RETURN_ON_FAIL(
diff --git a/wolfhsm/wh_auth.h b/wolfhsm/wh_auth.h
@@ -40,6 +40,7 @@
 #include <stdbool.h>
 
 #include "wolfhsm/wh_common.h"
+#include "wolfhsm/wh_message.h" /* for WH_NUMBER_OF_GROUPS */
 
 /** Auth Manager Types */
 
@@ -54,27 +55,23 @@ typedef enum {
     WH_AUTH_METHOD_CERTIFICATE,
 } whAuthMethod;
 
-#define WH_NUMBER_OF_GROUPS 14
 #define WH_AUTH_MAX_KEY_IDS \
     2 /* Maximum number of key IDs a user can have access to */
 #define WH_AUTH_ACTIONS_PER_GROUP 256 /* Support up to 256 actions (0-255) */
 #define WH_AUTH_ACTION_WORDS \
     ((WH_AUTH_ACTIONS_PER_GROUP + 31) / 32) /* 8 uint32_t words for 256 bits */
 
-/* Convert action enum value (0-255) to bitmask and word index.
- * Returns the word index in the upper 16 bits and bitmask in lower 32 bits.
- * Use WH_AUTH_ACTION_WORD() and WH_AUTH_ACTION_BIT() to extract. */
-#define WH_AUTH_ACTION_TO_WORD_AND_BIT(_action) \
-    ((((_action) / 32) << 16) | (1UL << ((_action) % 32)))
-#define WH_AUTH_ACTION_WORD(_word_and_bit) (((_word_and_bit) >> 16) & 0xFF)
-#define WH_AUTH_ACTION_BIT(_word_and_bit) ((_word_and_bit) & 0xFFFFFFFFUL)
-
-/* Legacy macro for backward compatibility - only works for actions < 32 */
-#define WH_AUTH_ACTION_TO_BITMASK(_action) \
-    (((_action) < 32) ? (1UL << (_action)) : 0)
+/* Convert action enum value (0-255) to word index and bitmask.
+ * Sets wordIdx to the array index (0-7) and bitMask to the bit position. */
+#define WH_AUTH_ACTION_TO_WORD_AND_BITMASK(action, wordIdx, bitMask) \
+    do {                                                             \
+        (wordIdx) = ((action) / 32);                                 \
+        (bitMask) = (1UL << ((action) % 32));                        \
+    } while (0)
 
 typedef struct {
-    uint16_t groupPermissions; /* bit mask of if allowed for use in group */
+    uint8_t groupPermissions[WH_NUMBER_OF_GROUPS]; /* boolean array of if group
+                                                       is allowed */
     uint32_t actionPermissions[WH_NUMBER_OF_GROUPS]
                               [WH_AUTH_ACTION_WORDS]; /* multi-word bit array
                                                           for action permissions
@@ -155,8 +152,6 @@ typedef struct whAuthContext_t {
     void*      context;
 } whAuthContext;
 
-#define WOLFHSM_MAX_CERTIFICATE_LEN 2048
-
 /* Simple helper configuration structure associated with an Auth Manager
  * instance */
 typedef struct whAuthConfig_t {
diff --git a/wolfhsm/wh_message.h b/wolfhsm/wh_message.h
@@ -52,6 +52,7 @@ enum WH_MESSAGE_ENUM {
     WH_MESSAGE_ACTION_MASK = 0x00FF, /* 255 subtypes per group*/
     WH_MESSAGE_ACTION_NONE = 0x0000, /* No action. Invalid. */
 };
+#define WH_NUMBER_OF_GROUPS 14
 
 /* keystore actions */
 enum WH_KEY_ENUM {
@@ -101,12 +102,13 @@ enum {
 
 /* auth actions */
 enum {
-    WH_AUTH_ACTION_LOGIN,
-    WH_AUTH_ACTION_LOGOUT,
-    WH_AUTH_ACTION_USER_ADD,
-    WH_AUTH_ACTION_USER_DELETE,
-    WH_AUTH_ACTION_USER_MODIFY,
-    WH_AUTH_ACTION_PERMISSION_SET,
+    WH_MESSAGE_AUTH_ACTION_LOGIN,
+    WH_MESSAGE_AUTH_ACTION_LOGOUT,
+    WH_MESSAGE_AUTH_ACTION_USER_ADD,
+    WH_MESSAGE_AUTH_ACTION_USER_DELETE,
+    WH_MESSAGE_AUTH_ACTION_USER_GET,
+    WH_MESSAGE_AUTH_ACTION_USER_SET_PERMISSIONS,
+    WH_MESSAGE_AUTH_ACTION_USER_SET_CREDENTIALS,
 };
 
 /* Construct the message kind based on group and action */
diff --git a/wolfhsm/wh_message_auth.h b/wolfhsm/wh_message_auth.h
@@ -35,25 +35,12 @@
 #include "wolfhsm/wh_message.h"
 #include "wolfhsm/wh_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,
-};
-
-enum WH_MESSAGE_AUTH_MAX_ENUM {
-    WH_MESSAGE_AUTH_MAX_USERNAME_LEN = 32,
-    /* Reserve space for UserAddRequest fixed fields:
-     * username (32) + permissions (WH_FLAT_PERMISSIONS_LEN) + method (2) +
-     * credentials_len (2) + overhead (2) = 32 + 460 + 2 + 2 + 2 = 498 bytes */
-    WH_MESSAGE_AUTH_MAX_CREDENTIALS_LEN = WOLFHSM_CFG_COMM_DATA_LEN - 498,
-    WH_MESSAGE_AUTH_MAX_SESSIONS        = 16,
-};
+#define WH_MESSAGE_AUTH_MAX_USERNAME_LEN 32
+/* Reserve space for UserAddRequest fixed fields:
+ * username (32) + permissions (WH_FLAT_PERMISSIONS_LEN) + method (2) +
+ * credentials_len (2) + overhead (2) = 32 + 460 + 2 + 2 + 2 = 498 bytes */
+#define WH_MESSAGE_AUTH_MAX_CREDENTIALS_LEN (WOLFHSM_CFG_COMM_DATA_LEN - 498)
+#define WH_MESSAGE_AUTH_MAX_SESSIONS 16
 
 /* Simple reusable response message */
 typedef struct {
@@ -133,11 +120,12 @@ int wh_MessageAuth_TranslateLogoutRequest(
 /** Logout Response (SimpleResponse) */
 
 /* whAuthPermissions struct
- * uint16_t (groupPermissions) + uint32_t[WH_NUMBER_OF_GROUPS][WH_AUTH_ACTION_WORDS]
- * (actionPermissions) + uint16_t (keyIdCount) + uint32_t[WH_AUTH_MAX_KEY_IDS]
- * (keyIds) */
-#define WH_FLAT_PERMISSIONS_LEN \
-    (2 + (4 * WH_NUMBER_OF_GROUPS * WH_AUTH_ACTION_WORDS) + 2 + \
+ * uint8_t[WH_NUMBER_OF_GROUPS] (groupPermissions) +
+ * uint32_t[WH_NUMBER_OF_GROUPS][WH_AUTH_ACTION_WORDS] (actionPermissions) +
+ * uint16_t (keyIdCount) + uint32_t[WH_AUTH_MAX_KEY_IDS] (keyIds) */
+#define WH_FLAT_PERMISSIONS_LEN                               \
+    (WH_NUMBER_OF_GROUPS +                                    \
+     (4 * WH_NUMBER_OF_GROUPS * WH_AUTH_ACTION_WORDS) + 2 +   \
      (4 * WH_AUTH_MAX_KEY_IDS))
 
 /**
