Key usage policies #233

bigbrett · 2025-11-05T20:11:39Z

This PR adds usage policy flags to cached keys that are set by the client and enforced by the server. Clients can now restrict usage of keys to specific operations (encrypt, decrypt, sign, verify, wrap, derive).

Usage of the wolfCrypt API with local keys (e.g. not referring to keys via ID) will automatically set the appropriate flags under the hood.

Copilot

Pull Request Overview

This PR implements key usage policy enforcement for cryptographic operations in the wolfHSM system. Keys can now be tagged with specific usage flags (ENCRYPT, DECRYPT, SIGN, VERIFY, WRAP, DERIVE) that control which operations are permitted. The server validates these policies before allowing key operations to proceed.

Key changes:

Added new key usage policy flags and enforcement functions to validate operations against key metadata
Updated all crypto operation handlers to check usage permissions before executing
Modified test code and examples to specify appropriate usage flags when caching/importing keys

Reviewed Changes

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

Show a summary per file

File	Description
wolfhsm/wh_common.h	Added 6 new usage policy flag definitions and WH_NVM_FLAGS_USAGE_ANY macro
wolfhsm/wh_error.h	Added WH_ERROR_USAGE error code for policy violations
wolfhsm/wh_server_keystore.h	Added function declarations for usage policy enforcement
src/wh_server_keystore.c	Implemented enforcement functions and updated key management logic
src/wh_server_crypto.c	Added policy checks to all crypto operation handlers
src/wh_client_crypto.c	Updated temporary key imports to include appropriate usage flags
src/wh_client_keywrap.c	Removed wrappedKeySz validation that could fail on error responses
test/wh_test_crypto.c	Added comprehensive usage policy test suite; updated existing tests with flags
test/wh_test_keywrap.c	Updated KEK and test key flags
test/wh_test_multiclient.c	Updated wrapping key cache calls with WRAP flag
examples/demo/client/*.c	Updated all key cache/import operations with appropriate flags
benchmark/bench_modules/*.c	Updated benchmark key operations to use WH_NVM_FLAGS_USAGE_ANY

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

wolfhsm/wh_common.h

wolfhsm/wh_server_keystore.h

test/wh_test_crypto.c

wolfhsm/wh_common.h

output arguments

billphipps

Looks great! A few naming questions and some design questions, but these can be punted as we further refine the design. We may want to isolate all of the checks/usage code into a new file and support callback overrides for the logic.

billphipps · 2025-11-07T13:26:27Z

wolfhsm/wh_common.h

+ * operations.
+ */
+/* Key can be used for encryption */
+#define WH_NVM_FLAGS_USAGE_ENCRYPT ((whNvmFlags)1 << 5)


Consider starting usage flags at bit 8 (high byte), since these are specific to keys.
Consider naming these WH_NVM_FLAGS_KEYUSAGE_xxx to indicate these are for keys and not common to all NVM types. Other types would then use the high byte for type-specific interpretation.

billphipps · 2025-11-07T13:31:46Z

wolfhsm/wh_common.h

-#define WH_NVM_FLAGS_LOCAL          ((whNvmFlags)1 << 3) /* Was generated locally */
-#define WH_NVM_FLAGS_EPHEMERAL      ((whNvmFlags)1 << 4) /* Cannot be cached nor committed */
+
+/* Generic NVM flags */


Common? Global? "Generic" seems to be wrong description since this group of flags is applicable to all types of objects with associated metadata.

Seems to me that "SENSITIVE" and "LOCAL" are attributes that won't have any server-side logic, but may be interesting to the client. The other flags have strong server-side logic to dictate how the data can be used. Consider reordering to put all the server-side attributes in the least-significant nibble.

billphipps · 2025-11-07T13:39:35Z

wolfhsm/wh_error.h

    WH_ERROR_NOHANDLER = -2007,   /* No customcb handler registered */
    WH_ERROR_NOTIMPL   = -2008,   /* Functionality not implemented given the
                                      compile-time configuration */
+    WH_ERROR_USAGE =


Contrast this with WH_ERROR_ACCESS? Consider WH_ERROR_KEYUSAGE.

billphipps · 2025-11-07T13:50:33Z

wolfhsm/wh_server_keystore.h

+ * @return WH_ERROR_USAGE if the key does not have the required flags
+ * @return WH_ERROR_BADARGS if meta is NULL
+ */
+int wh_Server_KeystoreEnforceKeyUsage(const whNvmMetadata* meta,


Consider moving this function out of keystore (or at least renaming to wh_Metadata_CheckKeyUsage) since this has nothing to do with the server nor keystore itself since it uses only metadata. The keystore variant below (named xxxFindxxx) could then simply be named wh_Server_KeystoreCheckKeyUsage and would use the metadata checkkeyusage once it found the metadata.

Good point on the metadata-only path. For the other function, I wanted to keep "Find" in the name to indicate that it does do an internal lookup (potentially even from NVM). There are lots of code paths that first do a lookup and have access to the metadata already, so calling this out in the name prevents accidental redundant use.

billphipps · 2025-11-07T13:55:05Z

wolfhsm/wh_common.h

+/* Key can be used for encryption */
+#define WH_NVM_FLAGS_USAGE_ENCRYPT ((whNvmFlags)1 << 5)
+/* Key can be used for decryption */
+#define WH_NVM_FLAGS_USAGE_DECRYPT ((whNvmFlags)1 << 6)


Consider removing this since it overlaps with ENCRYPT. For symmetric algos, knowledge of the key is sufficient for any operation. Most streaming modes (GCM, CTR, etc) only use the ENCRYPT direction and the encrypt and decrypt operations are the same.

Funny, I actually thought about this but kept it for 2 reasons -

These attributes are explicitly called out it a customer SoW

PKCS11 keeps them separate

I think this is enough justification for keeping them separate? We could always make a helper WH_NVM_FLAGS_USAGE_CRYPT that OR's them together to make it easier on users?

billphipps · 2025-11-07T14:01:28Z

test/wh_test_keywrap.c

            .label = "AES Key Label",
            .len   = WH_TEST_AES_KEYSIZE,
-            .flags = WH_NVM_FLAGS_NONE,
+            .flags = WH_NVM_FLAGS_USAGE_ANY,


I know this is only a test, but consider a more limited key usage flag, since signing/verifying is does not make sense:

Suggested change

.flags = WH_NVM_FLAGS_USAGE_ANY,

.flags = WH_NVM_FLAGS_USAGE_ENCRYPT|WH_NVM_FLAGS_USAGE_DECRYPT,

the key isn't actually used for anything so I just used that as a placeholder. Can fix if you care.

billphipps · 2025-11-07T14:07:09Z

src/wh_server_keystore.c

+    if ((actualFlags & requiredUsage) == requiredUsage) {
+        return WH_ERROR_OK;
+    }
+
+    /* Key does not have ALL the required usage flags */
+    return WH_ERROR_USAGE;


Only a style nit to try to keep the linear path all positive.

Suggested change

if ((actualFlags & requiredUsage) == requiredUsage) {

return WH_ERROR_OK;

}

/* Key does not have ALL the required usage flags */

return WH_ERROR_USAGE;

if ((actualFlags & requiredUsage) != requiredUsage) {

return WH_ERROR_USAGE;

}

/* Key has ALL the required usage flags */

return WH_ERROR_OK;

bigbrett · 2025-11-07T15:08:09Z

I will address feedback in a subsequent PR

bigbrett requested a review from Copilot November 5, 2025 20:11

Copilot AI reviewed Nov 5, 2025

View reviewed changes

bigbrett added 2 commits November 5, 2025 13:22

Add key usage policies and enforcement for cryptographic operations

0f0072a

relax NULL checks in freshen key and helper function to support optional

264d9a8

output arguments

bigbrett force-pushed the key-usage-policies branch from 784cf00 to 264d9a8 Compare November 5, 2025 20:23

bigbrett requested review from AlexLanzano and billphipps November 5, 2025 20:26

bigbrett assigned billphipps Nov 5, 2025

bigbrett requested a review from JacobBarthelmeh November 5, 2025 20:27

billphipps approved these changes Nov 7, 2025

View reviewed changes

billphipps merged commit 04b199f into wolfSSL:main Nov 7, 2025
15 checks passed

	.flags = WH_NVM_FLAGS_USAGE_ANY,
	.flags = WH_NVM_FLAGS_USAGE_ENCRYPT\|WH_NVM_FLAGS_USAGE_DECRYPT,

Key usage policies #233

Key usage policies #233

Uh oh!

Conversation

bigbrett commented Nov 5, 2025

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!

billphipps 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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bigbrett Nov 7, 2025 • 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!

Uh oh!

bigbrett commented Nov 7, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

bigbrett Nov 7, 2025 •

edited

Loading