Enforce NVM objects' flags #263

rizlik · 2025-12-16T17:03:52Z

TODO:

consolidate internal key caching
avoid duplicates id in key cache
remove duplicated code in key cache
~~enforce flag in counters~~ NOT FOR THIS PR
add nondestroyable flag
fix she tests

bigbrett · 2025-12-17T19:02:58Z

@rizlik could you please squash the 25 commits before we start reviewing

Copilot

Pull request overview

This PR implements comprehensive flag enforcement for NVM objects, including the new WH_NVM_FLAGS_NONMODIFIABLE (renamed from WH_NVM_FLAGS_IMMUTABLE) and WH_NVM_FLAGS_NONDESTROYABLE flags. The implementation adds policy checking at both the NVM and keystore layers, consolidates internal key caching logic to avoid duplicates, and introduces a key revocation feature that marks keys as non-modifiable with no usage permissions.

Key changes:

Centralized policy enforcement through _KeystoreCheckPolicy() and wh_Nvm_CheckPolicy() functions
New "Checked" variants of API functions that enforce flag policies before operations
Key revocation functionality that prevents key usage while blocking erasure
Improved cache management that properly handles duplicate key IDs

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
wolfhsm/wh_common.h	Renamed `WH_NVM_FLAGS_IMMUTABLE` to `WH_NVM_FLAGS_NONMODIFIABLE` and added `WH_NVM_FLAGS_NONDESTROYABLE` flag
wolfhsm/wh_server_keystore.h	Added checked variants of keystore functions and revoke operation declarations
wolfhsm/wh_nvm.h	Added checked variants for NVM operations
wolfhsm/wh_message*.h	Added message structures for key revocation and SHE PreProgramKey
wolfhsm/wh_client*.h	Added client-side API for key revocation and SHE PreProgramKey
src/wh_nvm.c	Implemented centralized NVM policy checking for add/read/destroy operations
src/wh_server_keystore.c	Implemented keystore-level policy enforcement and key revocation logic
src/wh_server_nvm.c	Updated NVM request handlers to use checked variants and perform key ID translation
src/wh_server_she.c	Implemented PreProgramKey handler and updated to use checked variants
src/wh_server_crypto.c	Updated crypto functions to use checked cache slot allocation
src/wh_server_counter.c	Added key ID translation for counter operations
src/wh_server_cert.c	Updated to use checked cache slot functions
src/wh_client*.c	Implemented client-side request/response handlers for new operations
src/wh_message*.c	Implemented message translation functions for new message types
test/wh_test_nvmflags.*	Comprehensive test suite for flag enforcement on NVM objects, keys, and counters
test/wh_test_crypto.c	Added tests for duplicate key caching and key revocation
test/wh_test_cert.c	Updated flag usage from IMMUTABLE to NONMODIFIABLE
test/wh_test_clientserver.c	Fixed NvmList ID handling with proper key ID extraction
test/config/wolfhsm_cfg.h	Increased big cache count to 3 for test requirements
examples/posix/.../wolfhsm_cfg.h	Added big cache count configuration

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

src/wh_server_keystore.c

test/wh_test_crypto.c

src/wh_server_keystore.c

rizlik · 2025-12-17T20:19:56Z

@rizlik could you please squash the 25 commits before we start reviewing

reduced to 7 probably worth keeping separate

Copilot

Pull request overview

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

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

wolfhsm/wh_common.h

src/wh_server_keystore.c

src/wh_server_she.c

wolfhsm/wh_server_keystore.h

rizlik · 2025-12-17T23:34:06Z

fixed copilot comments

billphipps

review in progress

wolfhsm/wh_message.h

wolfhsm/wh_message_she.h

wolfhsm/wh_nvm.h

wolfhsm/wh_client.h

src/wh_server_she.c

bigbrett

As we discussed offline, no big architectural changes needed (yet), as those should be deployed across the codebase in one fell swoop. For now here are some items I noticed.

test/wh_test_nvmflags.c

test/wh_test_crypto.c

src/wh_nvm.c

src/wh_server_counter.c

src/wh_server_nvm.c

test/wh_test_clientserver.c

src/wh_server_keystore.c

This uniform the behaviour with `wh_Server_KeystoreExportKey`

* use whKeyId instead of whNvmId in wh_Client_KeyRevoke API * propagate WH_ERROR_NOT_FOUND in check policy functions * simplify check policies functions

.github/workflows/build-and-test.yml

docs/src/appendix01.md

test/wh_test_clientserver.c

feedback addressed

This reverts commit b7b7193.

billphipps

Looks like a great step in the right direction! Feel free to resolve all open comments and let me know when you are good to merge.

billphipps · 2025-12-23T18:29:38Z

src/wh_server_keystore.c

+        ret = wh_Nvm_AddObjectWithReclaim(server->nvm, cacheMeta,
+                                          cacheMeta->len, cacheBuf);


Maybe it makes sense to erase the content when revoked? Save some space?

rizlik requested a review from billphipps December 16, 2025 17:03

rizlik force-pushed the nvm_flags_enforce branch from c3a0df5 to ee40262 Compare December 17, 2025 17:51

rizlik marked this pull request as ready for review December 17, 2025 18:05

rizlik assigned billphipps Dec 17, 2025

rizlik changed the title ~~WIP: Enforce NVM objects' flags~~ Enforce NVM objects' flags Dec 17, 2025

bigbrett requested a review from Copilot December 17, 2025 19:53

Copilot started reviewing on behalf of bigbrett December 17, 2025 19:54 View session

Copilot AI reviewed Dec 17, 2025

View reviewed changes

src/wh_server_keystore.c Outdated Show resolved Hide resolved

test/wh_test_crypto.c Outdated Show resolved Hide resolved

src/wh_server_keystore.c Outdated Show resolved Hide resolved

src/wh_server_keystore.c Show resolved Hide resolved

rizlik force-pushed the nvm_flags_enforce branch from ef61652 to ea1ae4f Compare December 17, 2025 20:19

rizlik requested review from bigbrett and Copilot December 17, 2025 20:22

Copilot started reviewing on behalf of rizlik December 17, 2025 20:22 View session

Copilot AI reviewed Dec 17, 2025

View reviewed changes

billphipps previously requested changes Dec 18, 2025

View reviewed changes

wolfhsm/wh_message.h Outdated Show resolved Hide resolved

wolfhsm/wh_message_she.h Outdated Show resolved Hide resolved

wolfhsm/wh_nvm.h Show resolved Hide resolved

wolfhsm/wh_client.h Outdated Show resolved Hide resolved

src/wh_server_she.c Outdated Show resolved Hide resolved

bigbrett requested changes Dec 18, 2025

View reviewed changes

rizlik added 7 commits December 19, 2025 12:27

nvm: rename IMMUTABLE flag to NONMODIFIABLE

c3bc706

nvm, keystore: enforce NVM flags controls

d43a3f8

keystore: support key revocation

01361ff

keystore: use wh_Nvm_GetMetadata instead of wh_Nvm_List in GetUniqueId

c21082d

keystore: invert check and avoid a nested if level, typos

0cdd77e

keystore: bring the key in the cache in wh_Server_KeystoreExportKeyDma

46fcb7a

This uniform the behaviour with `wh_Server_KeystoreExportKey`

keystore: improvements

d6d8311

* use whKeyId instead of whNvmId in wh_Client_KeyRevoke API * propagate WH_ERROR_NOT_FOUND in check policy functions * simplify check policies functions

rizlik force-pushed the nvm_flags_enforce branch from c87d7e3 to 6c46788 Compare December 19, 2025 11:55

rizlik added 2 commits December 19, 2025 14:44

nvm: add revocation and nvm flag tests

3a91704

ci: temporarly fix wolfssl tag as master broke testing

b7b7193

rizlik force-pushed the nvm_flags_enforce branch from 6117274 to b7b7193 Compare December 19, 2025 13:44

docs: updates documentation with nvm enforcing and key revocation

edb28d0

bigbrett requested changes Dec 19, 2025

View reviewed changes

.github/workflows/build-and-test.yml Outdated Show resolved Hide resolved

docs/src/appendix01.md Show resolved Hide resolved

fixup! docs: updates documentation with nvm enforcing and key revocation

6cd6fd9

bigbrett reviewed Dec 19, 2025

View reviewed changes

test/wh_test_clientserver.c Outdated Show resolved Hide resolved

rizlik added 2 commits December 19, 2025 18:08

keystore: remove unused WH_KS_OP_ERASE operation

83a8a0a

test: move nvmflags test in its own file

d90ba22

rizlik requested a review from bigbrett December 19, 2025 17:25

test: nvmflags: add missing string.h header

6ee2489

bigbrett previously approved these changes Dec 19, 2025

View reviewed changes

Revert "ci: temporarly fix wolfssl tag as master broke testing"

490454e

This reverts commit b7b7193.

rizlik dismissed bigbrett’s stale review via 490454e December 22, 2025 15:46

billphipps approved these changes Dec 23, 2025

View reviewed changes

		ret = wh_Nvm_AddObjectWithReclaim(server->nvm, cacheMeta,
		cacheMeta->len, cacheBuf);

Enforce NVM objects' flags #263

Are you sure you want to change the base?

Enforce NVM objects' flags #263

Uh oh!

Conversation

rizlik commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bigbrett commented Dec 17, 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!

rizlik commented Dec 17, 2025

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!

rizlik commented Dec 17, 2025

Uh oh!

billphipps left a comment

Choose a reason for hiding this comment

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

billphipps Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rizlik commented Dec 16, 2025 •

edited

Loading