Skip to content

Commit d6d8311

Browse files
rizlikrizlik
committed
keystore: improvements
* use whKeyId instead of whNvmId in wh_Client_KeyRevoke API * propagate WH_ERROR_NOT_FOUND in check policy functions * simplify check policies functions
1 parent 46fcb7a commit d6d8311

File tree

5 files changed

+90
-135
lines changed

5 files changed

+90
-135
lines changed

‎src/wh_client.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1114,7 +1114,7 @@ int wh_Client_KeyRevokeResponse(whClientContext* c)
11141114
return ret;
11151115
}
11161116

1117-
int wh_Client_KeyRevoke(whClientContext* c, whNvmId keyId)
1117+
int wh_Client_KeyRevoke(whClientContext* c, whKeyId keyId)
11181118
{
11191119
int ret;
11201120
ret = wh_Client_KeyRevokeRequest(c, keyId);

‎src/wh_nvm.c‎

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -52,38 +52,29 @@ static int wh_Nvm_CheckPolicy(whNvmContext* context, whNvmOp op, whNvmId id,
5252
}
5353

5454
ret = wh_Nvm_GetMetadata(context, id, &meta);
55-
if (existing_meta != NULL && ret == WH_ERROR_OK) {
55+
if (ret != WH_ERROR_OK) {
56+
return ret;
57+
}
58+
59+
if (existing_meta != NULL) {
5660
*existing_meta = meta;
5761
}
5862

5963
switch (op) {
6064
case WH_NVM_OP_ADD:
61-
if (ret == WH_ERROR_OK) {
62-
if (meta.flags & WH_NVM_FLAGS_NONMODIFIABLE) {
63-
return WH_ERROR_ACCESS;
64-
}
65-
}
66-
else if (ret != WH_ERROR_NOTFOUND) {
67-
return ret;
65+
if (meta.flags & WH_NVM_FLAGS_NONMODIFIABLE) {
66+
return WH_ERROR_ACCESS;
6867
}
6968
break;
7069

7170
case WH_NVM_OP_DESTROY:
72-
if (ret == WH_ERROR_OK) {
73-
if (meta.flags & (WH_NVM_FLAGS_NONMODIFIABLE |
74-
WH_NVM_FLAGS_NONDESTROYABLE)) {
75-
return WH_ERROR_ACCESS;
76-
}
77-
}
78-
else if (ret != WH_ERROR_NOTFOUND) {
79-
return ret;
71+
if (meta.flags &
72+
(WH_NVM_FLAGS_NONMODIFIABLE | WH_NVM_FLAGS_NONDESTROYABLE)) {
73+
return WH_ERROR_ACCESS;
8074
}
8175
break;
8276

8377
case WH_NVM_OP_READ:
84-
if (ret != WH_ERROR_OK) {
85-
return ret;
86-
}
8778
if (meta.flags & WH_NVM_FLAGS_NONEXPORTABLE) {
8879
return WH_ERROR_ACCESS;
8980
}
@@ -97,16 +88,15 @@ static int wh_Nvm_CheckPolicy(whNvmContext* context, whNvmOp op, whNvmId id,
9788
}
9889

9990

100-
int wh_Nvm_Init(whNvmContext* context, const whNvmConfig *config)
91+
int wh_Nvm_Init(whNvmContext* context, const whNvmConfig* config)
10192
{
10293
int rc = 0;
10394

104-
if ( (context == NULL) ||
105-
(config == NULL) ) {
95+
if ((context == NULL) || (config == NULL)) {
10696
return WH_ERROR_BADARGS;
10797
}
10898

109-
context->cb = config->cb;
99+
context->cb = config->cb;
110100
context->context = config->context;
111101

112102
#if !defined(WOLFHSM_CFG_NO_CRYPTO) && defined(WOLFHSM_CFG_GLOBAL_KEYS)
@@ -221,7 +211,7 @@ int wh_Nvm_AddObjectChecked(whNvmContext* context, whNvmMetadata* meta,
221211
int ret;
222212

223213
ret = wh_Nvm_CheckPolicy(context, WH_NVM_OP_ADD, meta->id, NULL);
224-
if (ret != WH_ERROR_OK) {
214+
if (ret != WH_ERROR_OK && ret != WH_ERROR_NOTFOUND) {
225215
return ret;
226216
}
227217

‎src/wh_server_keystore.c‎

Lines changed: 72 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -114,116 +114,82 @@ static int _KeyIsCommitted(whServerContext* server, whKeyId keyId)
114114
/* Centralized cache/NVM policy: enforce NONMODIFIABLE/NONEXPORTABLE at the
115115
* keystore layer. Usage enforcement remains separate. */
116116
static int _KeystoreCheckPolicy(whServerContext* server, whKsOp op,
117-
whKeyId keyId, const whNvmMetadata* metaOpt)
117+
whKeyId keyId)
118118
{
119-
whNvmMetadata* cacheMeta = NULL;
120-
whNvmMetadata nvmMeta;
121-
const whNvmMetadata* meta = metaOpt;
122-
int ret;
119+
whNvmMetadata* cacheMeta = NULL;
120+
whNvmMetadata nvmMeta;
121+
whNvmFlags flags;
122+
int ret;
123+
int foundInCache = 0;
124+
int foundInNvm = 0;
123125

124126
if ((server == NULL) || WH_KEYID_ISERASED(keyId)) {
125127
return WH_ERROR_BADARGS;
126128
}
127129

128-
/* See if the key is already in cache to use its flags */
130+
/* Check cache first */
129131
ret = _FindInCache(server, keyId, NULL, NULL, NULL, &cacheMeta);
130-
if (ret == WH_ERROR_OK) {
131-
if (cacheMeta != NULL) {
132-
switch (op) {
133-
case WH_KS_OP_CACHE:
134-
if (cacheMeta->flags & WH_NVM_FLAGS_NONMODIFIABLE) {
135-
return WH_ERROR_ACCESS;
136-
}
137-
else {
138-
return WH_ERROR_OK;
139-
}
140-
case WH_KS_OP_ERASE:
141-
if (cacheMeta->flags & (WH_NVM_FLAGS_NONMODIFIABLE |
142-
WH_NVM_FLAGS_NONDESTROYABLE)) {
143-
return WH_ERROR_ACCESS;
144-
}
145-
else {
146-
return WH_ERROR_OK;
147-
}
148-
break;
149-
case WH_KS_OP_EVICT:
150-
/* committed key can be evicted */
151-
if (_KeyIsCommitted(server, keyId)) {
152-
return WH_ERROR_OK;
153-
}
154-
if (cacheMeta->flags & WH_NVM_FLAGS_NONMODIFIABLE) {
155-
return WH_ERROR_ACCESS;
156-
}
157-
else {
158-
return WH_ERROR_OK;
159-
}
160-
break;
161-
case WH_KS_OP_COMMIT:
162-
return WH_ERROR_OK;
163-
case WH_KS_OP_EXPORT:
164-
if (cacheMeta->flags & WH_NVM_FLAGS_NONEXPORTABLE) {
165-
return WH_ERROR_ACCESS;
166-
}
167-
else {
168-
return WH_ERROR_OK;
169-
}
170-
break;
171-
case WH_KS_OP_REVOKE:
172-
/* revoke is allowed even if already NONMODIFIABLE */
173-
return WH_ERROR_OK;
174-
}
175-
meta = cacheMeta;
176-
}
132+
if (ret == WH_ERROR_OK && cacheMeta != NULL) {
133+
foundInCache = 1;
177134
}
178-
else if (ret != WH_ERROR_NOTFOUND) {
135+
else if (ret != WH_ERROR_OK && ret != WH_ERROR_NOTFOUND) {
179136
return ret;
180137
}
181138

182-
/* Get NVM metadata if not found in cache */
183-
ret = wh_Nvm_GetMetadata(server->nvm, keyId, &nvmMeta);
184-
if (ret == WH_ERROR_OK) {
185-
switch (op) {
186-
/* should never happen */
187-
case WH_KS_OP_COMMIT:
188-
return WH_ERROR_ABORTED;
189-
case WH_KS_OP_CACHE:
190-
if (nvmMeta.flags & WH_NVM_FLAGS_NONMODIFIABLE) {
191-
return WH_ERROR_ACCESS;
192-
}
193-
else {
194-
return WH_ERROR_OK;
195-
}
196-
break;
197-
case WH_KS_OP_ERASE:
198-
if (nvmMeta.flags & (WH_NVM_FLAGS_NONMODIFIABLE |
199-
WH_NVM_FLAGS_NONDESTROYABLE)) {
200-
return WH_ERROR_ACCESS;
201-
}
202-
else {
203-
return WH_ERROR_OK;
204-
}
205-
break;
206-
case WH_KS_OP_EXPORT:
207-
if (nvmMeta.flags & WH_NVM_FLAGS_NONEXPORTABLE) {
208-
return WH_ERROR_ACCESS;
209-
}
210-
else {
211-
return WH_ERROR_OK;
212-
}
213-
break;
214-
case WH_KS_OP_REVOKE:
215-
/* allow revoke even if already NONMODIFIABLE */
216-
return WH_ERROR_OK;
217-
case WH_KS_OP_EVICT:
218-
/* no-op */
219-
return WH_ERROR_OK;
139+
/* Check NVM if not in cache */
140+
if (!foundInCache) {
141+
ret = wh_Nvm_GetMetadata(server->nvm, keyId, &nvmMeta);
142+
if (ret == WH_ERROR_OK) {
143+
foundInNvm = 1;
220144
}
221-
if (meta == NULL) {
222-
meta = &nvmMeta;
145+
else if (ret != WH_ERROR_NOTFOUND) {
146+
return ret;
223147
}
224148
}
225-
else if (ret != WH_ERROR_NOTFOUND) {
226-
return ret;
149+
150+
/* Key not found */
151+
if (!foundInCache && !foundInNvm) {
152+
return WH_ERROR_NOTFOUND;
153+
}
154+
155+
/* Get flags from the appropriate source */
156+
flags = (foundInCache) ? cacheMeta->flags : nvmMeta.flags;
157+
158+
switch (op) {
159+
case WH_KS_OP_CACHE:
160+
if (flags & WH_NVM_FLAGS_NONMODIFIABLE) {
161+
return WH_ERROR_ACCESS;
162+
}
163+
break;
164+
165+
case WH_KS_OP_EVICT:
166+
if (_KeyIsCommitted(server, keyId)) {
167+
/* Committed keys can always be evicted */
168+
break;
169+
}
170+
if (flags &
171+
(WH_NVM_FLAGS_NONMODIFIABLE | WH_NVM_FLAGS_NONDESTROYABLE)) {
172+
return WH_ERROR_ACCESS;
173+
}
174+
break;
175+
176+
case WH_KS_OP_ERASE:
177+
if (flags &
178+
(WH_NVM_FLAGS_NONMODIFIABLE | WH_NVM_FLAGS_NONDESTROYABLE)) {
179+
return WH_ERROR_ACCESS;
180+
}
181+
break;
182+
183+
case WH_KS_OP_EXPORT:
184+
if (flags & WH_NVM_FLAGS_NONEXPORTABLE) {
185+
return WH_ERROR_ACCESS;
186+
}
187+
break;
188+
189+
case WH_KS_OP_COMMIT:
190+
case WH_KS_OP_REVOKE:
191+
/* Always allowed */
192+
break;
227193
}
228194

229195
return WH_ERROR_OK;
@@ -518,8 +484,8 @@ int wh_Server_KeystoreGetCacheSlotChecked(whServerContext* server,
518484
whNvmMetadata** outMeta)
519485
{
520486
int ret;
521-
ret = _KeystoreCheckPolicy(server, WH_KS_OP_CACHE, keyId, NULL);
522-
if (ret != WH_ERROR_OK) {
487+
ret = _KeystoreCheckPolicy(server, WH_KS_OP_CACHE, keyId);
488+
if (ret != WH_ERROR_OK && ret != WH_ERROR_NOTFOUND) {
523489
return ret;
524490
}
525491
return wh_Server_KeystoreGetCacheSlot(server, keyId, keySz, outBuf,
@@ -651,15 +617,14 @@ int wh_Server_KeystoreFreshenKey(whServerContext* server, whKeyId keyId,
651617
cacheBufOut, cacheMetaOut);
652618
if (ret == WH_ERROR_OK) {
653619
/* Read the key from NVM into the cache slot */
654-
ret = wh_Nvm_Read(server->nvm, keyId, 0, tmpMeta->len,
655-
*cacheBufOut);
620+
ret =
621+
wh_Nvm_Read(server->nvm, keyId, 0, tmpMeta->len, *cacheBufOut);
656622
if (ret == WH_ERROR_OK) {
657623
/* Copy the metadata to the cache slot if key read is
658624
* successful*/
659625
memcpy((uint8_t*)*cacheMetaOut, (uint8_t*)tmpMeta,
660626
sizeof(whNvmMetadata));
661-
_MarkKeyCommitted(_GetCacheContext(server, keyId), keyId,
662-
1);
627+
_MarkKeyCommitted(_GetCacheContext(server, keyId), keyId, 1);
663628
}
664629
}
665630
}
@@ -749,7 +714,7 @@ int wh_Server_KeystoreReadKeyChecked(whServerContext* server, whKeyId keyId,
749714
{
750715
int ret;
751716

752-
ret = _KeystoreCheckPolicy(server, WH_KS_OP_EXPORT, keyId, NULL);
717+
ret = _KeystoreCheckPolicy(server, WH_KS_OP_EXPORT, keyId);
753718
if (ret != WH_ERROR_OK) {
754719
return ret;
755720
}
@@ -783,7 +748,7 @@ int wh_Server_KeystoreEvictKeyChecked(whServerContext* server, whNvmId keyId)
783748
{
784749
int ret;
785750

786-
ret = _KeystoreCheckPolicy(server, WH_KS_OP_EVICT, keyId, NULL);
751+
ret = _KeystoreCheckPolicy(server, WH_KS_OP_EVICT, keyId);
787752
if (ret != WH_ERROR_OK) {
788753
return ret;
789754
}
@@ -826,7 +791,7 @@ int wh_Server_KeystoreCommitKeyChecked(whServerContext* server, whNvmId keyId)
826791
{
827792
int ret;
828793

829-
ret = _KeystoreCheckPolicy(server, WH_KS_OP_COMMIT, keyId, NULL);
794+
ret = _KeystoreCheckPolicy(server, WH_KS_OP_COMMIT, keyId);
830795
if (ret != WH_ERROR_OK) {
831796
return ret;
832797
}
@@ -896,7 +861,7 @@ int wh_Server_KeystoreRevokeKey(whServerContext* server, whNvmId keyId)
896861
return WH_ERROR_BADARGS;
897862
}
898863

899-
ret = _KeystoreCheckPolicy(server, WH_KS_OP_REVOKE, keyId, NULL);
864+
ret = _KeystoreCheckPolicy(server, WH_KS_OP_REVOKE, keyId);
900865
if (ret != WH_ERROR_OK) {
901866
return ret;
902867
}
@@ -2210,7 +2175,7 @@ int wh_Server_KeystoreExportKeyDmaChecked(whServerContext* server,
22102175
{
22112176
int ret;
22122177

2213-
ret = _KeystoreCheckPolicy(server, WH_KS_OP_EXPORT, keyId, NULL);
2178+
ret = _KeystoreCheckPolicy(server, WH_KS_OP_EXPORT, keyId);
22142179
if (ret != WH_ERROR_OK) {
22152180
return ret;
22162181
}

‎wolfhsm/wh_client.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -773,7 +773,7 @@ int wh_Client_KeyErase(whClientContext* c, whNvmId keyId);
773773
* @param[in] keyId Key ID to be revoked.
774774
* @return int Returns 0 on success, or a negative error code on failure.
775775
*/
776-
int wh_Client_KeyRevokeRequest(whClientContext* c, whNvmId keyId);
776+
int wh_Client_KeyRevokeRequest(whClientContext* c, whKeyId keyId);
777777

778778
/**
779779
* @brief Receives a key revoke response from the server.
@@ -800,7 +800,7 @@ int wh_Client_KeyRevokeResponse(whClientContext* c);
800800
* @param[in] keyId Key ID to be revoked.
801801
* @return int Returns 0 on success, or a negative error code on failure.
802802
*/
803-
int wh_Client_KeyRevoke(whClientContext* c, whNvmId keyId);
803+
int wh_Client_KeyRevoke(whClientContext* c, whKeyId keyId);
804804

805805
#ifdef WOLFHSM_CFG_DMA
806806

‎wolfhsm/wh_server_keystore.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ int wh_Server_KeystoreEraseKeyChecked(whServerContext* server, whNvmId keyId);
189189
*
190190
* Placeholder implementation for key revocation.
191191
*/
192-
int wh_Server_KeystoreRevokeKey(whServerContext* server, whNvmId keyId);
192+
int wh_Server_KeystoreRevokeKey(whServerContext* server, whKeyId keyId);
193193

194194
/**
195195
* @brief Handle key management requests from clients

0 commit comments

Comments
 (0)