Skip to content

Commit 3d95567

Browse files
committed
Report 4324
1 parent 619688f commit 3d95567

3 files changed

Lines changed: 225 additions & 18 deletions

File tree

src/wh_server_img_mgr.c

Lines changed: 59 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,55 @@ int wh_Server_ImgMgrInit(whServerImgMgrContext* context,
8080
return ret;
8181
}
8282

83+
/* Copy key material out of the keystore cache while holding the NVM lock, so
84+
* the caller works from a private snapshot instead of a live cache slot that a
85+
* concurrent evict or cache request could rewrite. */
86+
static int _ImgMgrCopyKeyFromKeystore(whServerContext* server, whKeyId keyId,
87+
uint8_t* dst, size_t dstMax,
88+
size_t* outLen)
89+
{
90+
int ret;
91+
uint8_t* keyBuf = NULL;
92+
whNvmMetadata* keyMeta = NULL;
93+
94+
if ((server == NULL) || (dst == NULL) || (outLen == NULL) ||
95+
(dstMax == 0)) {
96+
return WH_ERROR_BADARGS;
97+
}
98+
99+
ret = WH_SERVER_NVM_LOCK(server);
100+
if (ret != WH_ERROR_OK) {
101+
return ret;
102+
}
103+
104+
ret = wh_Server_KeystoreFreshenKey(server, keyId, &keyBuf, &keyMeta);
105+
if (ret == WH_ERROR_OK) {
106+
if ((keyBuf == NULL) || (keyMeta == NULL)) {
107+
ret = WH_ERROR_ABORTED;
108+
}
109+
else if ((size_t)keyMeta->len > dstMax) {
110+
ret = WH_ERROR_BUFFER_SIZE;
111+
}
112+
else {
113+
memcpy(dst, keyBuf, keyMeta->len);
114+
*outLen = (size_t)keyMeta->len;
115+
}
116+
}
117+
118+
(void)WH_SERVER_NVM_UNLOCK(server);
119+
120+
return ret;
121+
}
122+
83123
int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext* context,
84124
const whServerImgMgrImg* img,
85125
whServerImgMgrVerifyResult* result)
86126
{
87-
int ret = WH_ERROR_OK;
88-
whServerContext* server = NULL;
89-
uint8_t* keyBuf = NULL;
90-
whNvmMetadata* keyMeta = NULL;
91-
size_t keySz = 0;
127+
int ret = WH_ERROR_OK;
128+
whServerContext* server = NULL;
129+
uint8_t keyBuf[WOLFHSM_CFG_SERVER_IMG_MGR_MAX_KEY_SIZE];
130+
const uint8_t* keyPtr = NULL; /* stays NULL for paths with no key */
131+
size_t keySz = 0;
92132
uint8_t sigBuf[WOLFHSM_CFG_SERVER_IMG_MGR_MAX_SIG_SIZE]; /* Buffer for
93133
signature */
94134
whNvmMetadata sigMeta = {0};
@@ -112,12 +152,12 @@ int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext* context,
112152
switch (img->imgType) {
113153
case WH_IMG_MGR_IMG_TYPE_WOLFBOOT:
114154
/* Load key from keystore, skip sig loading (sig is in header) */
115-
ret = wh_Server_KeystoreFreshenKey(server, img->keyId, &keyBuf,
116-
&keyMeta);
155+
ret = _ImgMgrCopyKeyFromKeystore(server, img->keyId, keyBuf,
156+
sizeof(keyBuf), &keySz);
117157
if (ret != WH_ERROR_OK) {
118158
return ret;
119159
}
120-
keySz = keyMeta->len;
160+
keyPtr = keyBuf;
121161
/* sig/sigSz passed as NULL/0 to callback */
122162
break;
123163

@@ -127,15 +167,8 @@ int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext* context,
127167
break;
128168

129169
case WH_IMG_MGR_IMG_TYPE_RAW:
130-
/* Existing behavior: load key from keystore + sig from NVM */
131-
ret = wh_Server_KeystoreFreshenKey(server, img->keyId, &keyBuf,
132-
&keyMeta);
133-
if (ret != WH_ERROR_OK) {
134-
return ret;
135-
}
136-
keySz = keyMeta->len;
137-
138-
/* Load the signature from NVM */
170+
/* Load the signature from NVM first so the key snapshot is the
171+
* last thing taken before verification */
139172
ret = wh_Nvm_GetMetadata(server->nvm, img->sigNvmId, &sigMeta);
140173
if (ret != WH_ERROR_OK) {
141174
return ret;
@@ -153,6 +186,14 @@ int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext* context,
153186
}
154187
actualSigSize = sigMeta.len;
155188
sigPtr = sigBuf;
189+
190+
/* Load key from keystore */
191+
ret = _ImgMgrCopyKeyFromKeystore(server, img->keyId, keyBuf,
192+
sizeof(keyBuf), &keySz);
193+
if (ret != WH_ERROR_OK) {
194+
return ret;
195+
}
196+
keyPtr = keyBuf;
156197
break;
157198

158199
default:
@@ -162,7 +203,7 @@ int wh_Server_ImgMgrVerifyImg(whServerImgMgrContext* context,
162203
/* Invoke verify method callback */
163204
if (img->verifyMethod != NULL) {
164205
result->verifyMethodResult = img->verifyMethod(
165-
context, img, keyBuf, keySz, sigPtr, actualSigSize);
206+
context, img, keyPtr, keySz, sigPtr, actualSigSize);
166207
}
167208
else {
168209
result->verifyMethodResult = WH_ERROR_NOHANDLER;

test/wh_test_server_img_mgr.c

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1430,6 +1430,155 @@ whTest_ServerImgMgrServerCfgWolfBootCertChainRsa4096(whServerConfig* serverCfg)
14301430
#endif /* WOLFHSM_CFG_CERTIFICATE_MANAGER */
14311431
#endif /* !NO_RSA */
14321432

1433+
/* Key used to prove the verify callback gets a private copy */
1434+
static const uint8_t testSnapshotKey[16] = {0x00, 0x11, 0x22, 0x33, 0x44, 0x55,
1435+
0x66, 0x77, 0x88, 0x99, 0xAA, 0xBB,
1436+
0xCC, 0xDD, 0xEE, 0xFF};
1437+
1438+
/* Verify method that evicts and re-caches the same keyId the way a concurrent
1439+
* request would. The key passed in must be a private snapshot, so it stays
1440+
* intact for the rest of the callback. */
1441+
static int _ImgMgrSnapshotVerifyMethod(whServerImgMgrContext* context,
1442+
const whServerImgMgrImg* img,
1443+
const uint8_t* key, size_t keySz,
1444+
const uint8_t* sig, size_t sigSz)
1445+
{
1446+
int ret;
1447+
whNvmMetadata meta = {0};
1448+
uint8_t attackerKey[sizeof(testSnapshotKey)];
1449+
1450+
(void)sig;
1451+
(void)sigSz;
1452+
1453+
if (context == NULL || context->server == NULL || img == NULL ||
1454+
key == NULL || keySz != sizeof(testSnapshotKey)) {
1455+
return WH_ERROR_BADARGS;
1456+
}
1457+
1458+
memset(attackerKey, 0xAA, sizeof(attackerKey));
1459+
1460+
ret = wh_Server_KeystoreEvictKey(context->server, img->keyId);
1461+
if (ret != WH_ERROR_OK) {
1462+
WH_ERROR_PRINT("Failed to evict key during verify: %d\n", ret);
1463+
return ret;
1464+
}
1465+
1466+
meta.id = img->keyId;
1467+
meta.access = WH_NVM_ACCESS_ANY;
1468+
meta.flags = WH_NVM_FLAGS_NONE;
1469+
meta.len = sizeof(attackerKey);
1470+
ret = wh_Server_KeystoreCacheKey(context->server, &meta, attackerKey);
1471+
if (ret != WH_ERROR_OK) {
1472+
WH_ERROR_PRINT("Failed to cache substitute key during verify: %d\n",
1473+
ret);
1474+
return ret;
1475+
}
1476+
1477+
if (memcmp(key, testSnapshotKey, keySz) != 0) {
1478+
WH_ERROR_PRINT("Verify key was substituted mid verification\n");
1479+
return WH_ERROR_ABORTED;
1480+
}
1481+
1482+
return WH_ERROR_OK;
1483+
}
1484+
1485+
static int whTest_ServerImgMgrKeySnapshot(whServerConfig* serverCfg)
1486+
{
1487+
int ret = 0;
1488+
whServerContext server[1] = {0};
1489+
whServerImgMgrConfig imgMgrConfig = {0};
1490+
whServerImgMgrContext imgMgr = {0};
1491+
whServerImgMgrImg testImage = {0};
1492+
whServerImgMgrVerifyResult result = {0};
1493+
whNvmMetadata keyMeta = {0};
1494+
whNvmMetadata sigMeta = {0};
1495+
const whNvmId testKeyId = 1;
1496+
const whNvmId testSigNvmId = 2;
1497+
const uint8_t dummySig[4] = {0};
1498+
1499+
/* The callback ignores the signature, but the RAW path requires one */
1500+
sigMeta.id = testSigNvmId;
1501+
sigMeta.access = WH_NVM_ACCESS_ANY;
1502+
sigMeta.flags = WH_NVM_FLAGS_NONE;
1503+
sigMeta.len = sizeof(dummySig);
1504+
snprintf((char*)sigMeta.label, WH_NVM_LABEL_LEN, "TestSnapshotSig");
1505+
1506+
ret =
1507+
wh_Nvm_AddObject(serverCfg->nvm, &sigMeta, sizeof(dummySig), dummySig);
1508+
if (ret != WH_ERROR_OK) {
1509+
WH_ERROR_PRINT("Failed to add snapshot signature to NVM: %d\n", ret);
1510+
return ret;
1511+
}
1512+
1513+
testImage.imgType = WH_IMG_MGR_IMG_TYPE_RAW;
1514+
testImage.addr = (uintptr_t)testData;
1515+
testImage.size = sizeof(testData);
1516+
testImage.keyId = testKeyId;
1517+
testImage.sigNvmId = testSigNvmId;
1518+
testImage.verifyMethod = _ImgMgrSnapshotVerifyMethod;
1519+
testImage.verifyAction = wh_Server_ImgMgrVerifyActionDefault;
1520+
1521+
imgMgrConfig.images = &testImage;
1522+
imgMgrConfig.imageCount = 1;
1523+
imgMgrConfig.server = server;
1524+
1525+
ret = wh_Server_Init(server, serverCfg);
1526+
if (ret != WH_ERROR_OK) {
1527+
WH_ERROR_PRINT("Failed to initialize server: %d\n", ret);
1528+
return ret;
1529+
}
1530+
1531+
ret = wh_Server_ImgMgrInit(&imgMgr, &imgMgrConfig);
1532+
if (ret != WH_ERROR_OK) {
1533+
WH_ERROR_PRINT("Failed to initialize image manager: %d\n", ret);
1534+
wh_Server_Cleanup(server);
1535+
return ret;
1536+
}
1537+
1538+
keyMeta.id = testKeyId;
1539+
keyMeta.access = WH_NVM_ACCESS_ANY;
1540+
keyMeta.flags = WH_NVM_FLAGS_NONE;
1541+
keyMeta.len = sizeof(testSnapshotKey);
1542+
snprintf((char*)keyMeta.label, WH_NVM_LABEL_LEN, "TestSnapshotKey");
1543+
1544+
ret =
1545+
wh_Server_KeystoreCacheKey(server, &keyMeta, (uint8_t*)testSnapshotKey);
1546+
if (ret != WH_ERROR_OK) {
1547+
WH_ERROR_PRINT("Failed to cache snapshot key: %d\n", ret);
1548+
wh_Server_Cleanup(server);
1549+
return ret;
1550+
}
1551+
1552+
ret = wh_Server_ImgMgrVerifyImg(&imgMgr, &testImage, &result);
1553+
if (ret != WH_ERROR_OK) {
1554+
WH_ERROR_PRINT("Snapshot image verification failed: %d\n", ret);
1555+
wh_Server_Cleanup(server);
1556+
return ret;
1557+
}
1558+
1559+
if (result.verifyMethodResult != WH_ERROR_OK) {
1560+
WH_ERROR_PRINT("Snapshot verify method failed: %d\n",
1561+
result.verifyMethodResult);
1562+
wh_Server_Cleanup(server);
1563+
return result.verifyMethodResult;
1564+
}
1565+
1566+
/* Drop the substitute key the callback left behind */
1567+
(void)wh_Server_KeystoreEvictKey(server, testKeyId);
1568+
1569+
ret = wh_Nvm_DestroyObjects(serverCfg->nvm, 1, &sigMeta.id);
1570+
if (ret != WH_ERROR_OK) {
1571+
WH_ERROR_PRINT("Failed to delete snapshot signature object: %d\n", ret);
1572+
wh_Server_Cleanup(server);
1573+
return ret;
1574+
}
1575+
1576+
wh_Server_Cleanup(server);
1577+
1578+
WH_TEST_PRINT("IMG_MGR key snapshot Test completed successfully!\n");
1579+
return 0;
1580+
}
1581+
14331582
int whTest_ServerImgMgr(whTestNvmBackendType nvmType)
14341583
{
14351584
int rc = 0;
@@ -1489,6 +1638,14 @@ int whTest_ServerImgMgr(whTestNvmBackendType nvmType)
14891638
return rc;
14901639
}
14911640

1641+
/* Verify key material is snapshotted before the verify callback runs */
1642+
rc = whTest_ServerImgMgrKeySnapshot(s_conf);
1643+
if (rc != 0) {
1644+
WH_ERROR_PRINT("Image manager key snapshot test failed: %d\n", rc);
1645+
wh_Nvm_Cleanup(nvm);
1646+
return rc;
1647+
}
1648+
14921649
/* Run image manager server config tests for each built-in verify method */
14931650

14941651
#ifdef HAVE_ECC

wolfhsm/wh_settings.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,9 @@
9696
* WOLFHSM_CFG_SERVER_IMG_MGR_MAX_SIG_SIZE - Maximum signature size for image
9797
* verification Default: 512 bytes (RSA4096)
9898
*
99+
* WOLFHSM_CFG_SERVER_IMG_MGR_MAX_KEY_SIZE - Maximum verification key size for
100+
* image verification Default: 1200 bytes (ASN.1 RSA4096 public key)
101+
*
99102
* WOLFHSM_CFG_DMA_CUSTOM_CLIENT_COPY - if defined, allows to setup a custom
100103
* callback to handle client to server and/or server to client memory copy
101104
* operation in DMA requests.
@@ -289,6 +292,12 @@
289292
#define WOLFHSM_CFG_SERVER_IMG_MGR_MAX_SIG_SIZE 512
290293
#endif
291294

295+
/* Image manager maximum verification key size. Sized to hold an ASN.1 RSA4096
296+
* public key. Raise it for larger keys such as ML-DSA. */
297+
#ifndef WOLFHSM_CFG_SERVER_IMG_MGR_MAX_KEY_SIZE
298+
#define WOLFHSM_CFG_SERVER_IMG_MGR_MAX_KEY_SIZE 1200
299+
#endif
300+
292301
/* WOLFHSM_CFG_CUSTOMCB_LEN - Maximum size of a customcb message.
293302
* Default: 256 */
294303
#ifndef WOLFHSM_CFG_CUSTOMCB_LEN

0 commit comments

Comments
 (0)