Skip to content

Commit 8f9dbf9

Browse files
padelsbachpadelsbach
committed
Fix keywrap response size checks
1 parent d6d5080 commit 8f9dbf9

3 files changed

Lines changed: 238 additions & 4 deletions

File tree

‎src/wh_client_keywrap.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ int wh_Client_KeyWrapResponse(whClientContext* ctx,
8181
}
8282

8383
if (group != WH_MESSAGE_GROUP_KEY || action != WH_KEY_KEYWRAP ||
84-
size < sizeof(*resp) || size > sizeof(*resp) + resp->wrappedKeySz ||
84+
size < sizeof(*resp) || size < sizeof(*resp) + resp->wrappedKeySz ||
8585
resp->cipherType != cipherType) {
8686
return WH_ERROR_ABORTED;
8787
}
@@ -201,7 +201,7 @@ int wh_Client_KeyUnwrapAndExportResponse(whClientContext* ctx,
201201

202202
if (group != WH_MESSAGE_GROUP_KEY || action != WH_KEY_KEYUNWRAPEXPORT ||
203203
size < sizeof(*resp) ||
204-
size > sizeof(*resp) + sizeof(*metadataOut) + resp->keySz ||
204+
size < sizeof(*resp) + sizeof(*metadataOut) + resp->keySz ||
205205
resp->cipherType != cipherType) {
206206
return WH_ERROR_ABORTED;
207207
}
@@ -421,7 +421,7 @@ int wh_Client_DataWrapResponse(whClientContext* ctx,
421421
}
422422

423423
if (group != WH_MESSAGE_GROUP_KEY || action != WH_KEY_DATAWRAP ||
424-
size < sizeof(*resp) || size > sizeof(*resp) + resp->wrappedDataSz ||
424+
size < sizeof(*resp) || size < sizeof(*resp) + resp->wrappedDataSz ||
425425
resp->cipherType != cipherType) {
426426
return WH_ERROR_ABORTED;
427427
}
@@ -534,7 +534,7 @@ int wh_Client_DataUnwrapResponse(whClientContext* ctx,
534534
}
535535

536536
if (group != WH_MESSAGE_GROUP_KEY || action != WH_KEY_DATAUNWRAP ||
537-
size < sizeof(*resp) || size > sizeof(*resp) + resp->dataSz ||
537+
size < sizeof(*resp) || size < sizeof(*resp) + resp->dataSz ||
538538
resp->cipherType != cipherType) {
539539
return WH_ERROR_ABORTED;
540540
}

‎test-refactor/misc/wh_test_keywrap_respsize.c‎

Lines changed: 232 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,232 @@
1+
/*
2+
* Copyright (C) 2026 wolfSSL Inc.
3+
*
4+
* This file is part of wolfHSM.
5+
*
6+
* wolfHSM is free software; you can redistribute it and/or modify
7+
* it under the terms of the GNU General Public License as published by
8+
* the Free Software Foundation; either version 3 of the License, or
9+
* (at your option) any later version.
10+
*
11+
* wolfHSM is distributed in the hope that it will be useful,
12+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
* GNU General Public License for more details.
15+
*
16+
* You should have received a copy of the GNU General Public License
17+
* along with wolfHSM. If not, see <http://www.gnu.org/licenses/>.
18+
*/
19+
/*
20+
* test-refactor/misc/wh_test_keywrap_respsize.c
21+
*
22+
* Client-side response size validation for the keywrap family.
23+
*
24+
* Each of the four keywrap response handlers
25+
* (wh_Client_KeyWrapResponse, wh_Client_KeyUnwrapAndExportResponse,
26+
* wh_Client_DataWrapResponse, wh_Client_DataUnwrapResponse) parses a
27+
* response that carries a fixed header plus a variable-length payload
28+
* whose length is declared inside the header. A correct implementation
29+
* must verify the bytes actually received cover that declared length
30+
* before memcpy'ing the payload into the caller's output buffer.
31+
*
32+
* The test wires the client to a one-shot fake transport whose Recv
33+
* delivers a canned malformed response: the declared trailing length
34+
* is non-zero but no trailing bytes are actually delivered. Each
35+
* handler must return WH_ERROR_ABORTED rather than over-read the
36+
* comm buffer.
37+
*
38+
* Pure unit test of the client-side parser; no server, no fixture
39+
* context is consumed.
40+
*/
41+
42+
#include "wolfhsm/wh_settings.h"
43+
44+
#if defined(WOLFHSM_CFG_KEYWRAP) && defined(WOLFHSM_CFG_ENABLE_CLIENT) && \
45+
!defined(WOLFHSM_CFG_NO_CRYPTO)
46+
47+
#include <stdint.h>
48+
#include <string.h>
49+
50+
#include "wolfssl/wolfcrypt/types.h"
51+
52+
#include "wolfhsm/wh_error.h"
53+
#include "wolfhsm/wh_comm.h"
54+
#include "wolfhsm/wh_message.h"
55+
#include "wolfhsm/wh_message_keystore.h"
56+
#include "wolfhsm/wh_client.h"
57+
#include "wolfhsm/wh_nvm.h"
58+
59+
#include "wh_test_common.h"
60+
#include "wh_test_list.h"
61+
62+
#define TEST_CIPHER_TYPE WC_CIPHER_AES_GCM
63+
/* Trailing length the malformed response claims. Zero bytes are
64+
* actually delivered, so any positive value triggers the bug. */
65+
#define MALFORMED_TRAILING_SZ 64
66+
/* Caller output buffers sized large enough that the *Sz > outSz
67+
* short-circuit doesn't preempt the bounds check we want to hit. */
68+
#define OUT_BUF_SZ 1024
69+
/* Arbitrary non-zero seq the test pretends a request was sent with. */
70+
#define FAKE_SEQ 0x1234
71+
72+
/* Data wrap/unwrap response entry points are not in the public header
73+
* but have external linkage in wh_client_keywrap.c. */
74+
int wh_Client_DataWrapResponse(whClientContext* ctx,
75+
enum wc_CipherType cipherType,
76+
void* wrappedDataOut, uint32_t* wrappedDataSz);
77+
int wh_Client_DataUnwrapResponse(whClientContext* ctx,
78+
enum wc_CipherType cipherType, void* dataOut,
79+
uint32_t* dataSz);
80+
81+
/* One-shot fake transport: swallows Send, returns whatever the caller
82+
* has stashed in `pkt` from Recv. */
83+
typedef struct {
84+
uint8_t pkt[WH_COMM_MTU];
85+
uint16_t pkt_len;
86+
} FakeTransport;
87+
88+
static int Fake_Init(void* c, const void* cf, whCommSetConnectedCb connectcb,
89+
void* connectcb_arg)
90+
{
91+
(void)c; (void)cf; (void)connectcb; (void)connectcb_arg;
92+
return WH_ERROR_OK;
93+
}
94+
static int Fake_Cleanup(void* c) { (void)c; return WH_ERROR_OK; }
95+
static int Fake_Send(void* c, uint16_t size, const void* data)
96+
{
97+
(void)c; (void)size; (void)data;
98+
return WH_ERROR_OK;
99+
}
100+
static int Fake_Recv(void* c, uint16_t* out_size, void* data)
101+
{
102+
FakeTransport* t = (FakeTransport*)c;
103+
memcpy(data, t->pkt, t->pkt_len);
104+
*out_size = t->pkt_len;
105+
return WH_ERROR_OK;
106+
}
107+
108+
static const whTransportClientCb fakeCb = {
109+
Fake_Init, Fake_Send, Fake_Recv, Fake_Cleanup,
110+
};
111+
112+
/* Stand up a client wired to the fake transport, with state primed so
113+
* a single wh_Client_*Response call sees a pending request matching
114+
* `action`, and the staged comm buffer holds (hdr + resp_struct) with
115+
* resp_struct declaring trailing bytes that are not actually sent. */
116+
static int _RunCase(uint16_t action, const void* resp_struct,
117+
uint16_t resp_sz,
118+
int (*invokeResp)(whClientContext*))
119+
{
120+
FakeTransport transport;
121+
whCommClientConfig commCfg;
122+
whClientConfig clientCfg;
123+
whClientContext client;
124+
whCommHeader hdr;
125+
uint16_t kind = WH_MESSAGE_KIND(WH_MESSAGE_GROUP_KEY, action);
126+
int ret;
127+
128+
memset(&transport, 0, sizeof(transport));
129+
memset(&commCfg, 0, sizeof(commCfg));
130+
memset(&clientCfg, 0, sizeof(clientCfg));
131+
memset(&client, 0, sizeof(client));
132+
133+
commCfg.transport_cb = &fakeCb;
134+
commCfg.transport_context = &transport;
135+
commCfg.client_id = WH_TEST_DEFAULT_CLIENT_ID;
136+
clientCfg.comm = &commCfg;
137+
138+
WH_TEST_RETURN_ON_FAIL(wh_Client_Init(&client, &clientCfg));
139+
140+
/* Pretend a request was sent: pending + matching seq/kind so
141+
* RecvResponse won't bail early. */
142+
client.comm->pending = 1;
143+
client.comm->seq = FAKE_SEQ;
144+
client.last_req_id = FAKE_SEQ;
145+
client.last_req_kind = kind;
146+
147+
/* Pack hdr + malformed response into the fake transport buffer. */
148+
hdr.magic = WH_COMM_MAGIC_NATIVE;
149+
hdr.kind = kind;
150+
hdr.seq = FAKE_SEQ;
151+
hdr.aux = WH_COMM_AUX_RESP_OK;
152+
memcpy(transport.pkt, &hdr, sizeof(hdr));
153+
memcpy(transport.pkt + sizeof(hdr), resp_struct, resp_sz);
154+
transport.pkt_len = (uint16_t)(sizeof(hdr) + resp_sz);
155+
156+
ret = invokeResp(&client);
157+
(void)wh_Client_Cleanup(&client);
158+
159+
WH_TEST_ASSERT_RETURN(ret == WH_ERROR_ABORTED);
160+
return WH_ERROR_OK;
161+
}
162+
163+
/* Per-handler thunks: each wires up the output buffers the response
164+
* handler writes into. */
165+
static int _InvokeKeyWrap(whClientContext* c)
166+
{
167+
uint8_t out[OUT_BUF_SZ];
168+
uint16_t outSz = sizeof(out);
169+
return wh_Client_KeyWrapResponse(c, TEST_CIPHER_TYPE, out, &outSz);
170+
}
171+
static int _InvokeKeyUnwrapAndExport(whClientContext* c)
172+
{
173+
whNvmMetadata meta;
174+
uint8_t out[OUT_BUF_SZ];
175+
uint16_t outSz = sizeof(out);
176+
return wh_Client_KeyUnwrapAndExportResponse(c, TEST_CIPHER_TYPE, &meta,
177+
out, &outSz);
178+
}
179+
static int _InvokeDataWrap(whClientContext* c)
180+
{
181+
uint8_t out[OUT_BUF_SZ];
182+
uint32_t outSz = sizeof(out);
183+
return wh_Client_DataWrapResponse(c, TEST_CIPHER_TYPE, out, &outSz);
184+
}
185+
static int _InvokeDataUnwrap(whClientContext* c)
186+
{
187+
uint8_t out[OUT_BUF_SZ];
188+
uint32_t outSz = sizeof(out);
189+
return wh_Client_DataUnwrapResponse(c, TEST_CIPHER_TYPE, out, &outSz);
190+
}
191+
192+
int whTest_KeyWrapRespSize(void* ctx)
193+
{
194+
whMessageKeystore_KeyWrapResponse keyWrapResp;
195+
whMessageKeystore_KeyUnwrapAndExportResponse keyUnwrapResp;
196+
whMessageKeystore_DataWrapResponse dataWrapResp;
197+
whMessageKeystore_DataUnwrapResponse dataUnwrapResp;
198+
199+
(void)ctx;
200+
201+
memset(&keyWrapResp, 0, sizeof(keyWrapResp));
202+
keyWrapResp.cipherType = TEST_CIPHER_TYPE;
203+
keyWrapResp.wrappedKeySz = MALFORMED_TRAILING_SZ;
204+
205+
memset(&keyUnwrapResp, 0, sizeof(keyUnwrapResp));
206+
keyUnwrapResp.cipherType = TEST_CIPHER_TYPE;
207+
keyUnwrapResp.keySz = MALFORMED_TRAILING_SZ;
208+
209+
memset(&dataWrapResp, 0, sizeof(dataWrapResp));
210+
dataWrapResp.cipherType = TEST_CIPHER_TYPE;
211+
dataWrapResp.wrappedDataSz = MALFORMED_TRAILING_SZ;
212+
213+
memset(&dataUnwrapResp, 0, sizeof(dataUnwrapResp));
214+
dataUnwrapResp.cipherType = TEST_CIPHER_TYPE;
215+
dataUnwrapResp.dataSz = MALFORMED_TRAILING_SZ;
216+
217+
WH_TEST_RETURN_ON_FAIL(_RunCase(WH_KEY_KEYWRAP, &keyWrapResp,
218+
(uint16_t)sizeof(keyWrapResp),
219+
_InvokeKeyWrap));
220+
WH_TEST_RETURN_ON_FAIL(_RunCase(WH_KEY_KEYUNWRAPEXPORT, &keyUnwrapResp,
221+
(uint16_t)sizeof(keyUnwrapResp),
222+
_InvokeKeyUnwrapAndExport));
223+
WH_TEST_RETURN_ON_FAIL(_RunCase(WH_KEY_DATAWRAP, &dataWrapResp,
224+
(uint16_t)sizeof(dataWrapResp),
225+
_InvokeDataWrap));
226+
WH_TEST_RETURN_ON_FAIL(_RunCase(WH_KEY_DATAUNWRAP, &dataUnwrapResp,
227+
(uint16_t)sizeof(dataUnwrapResp),
228+
_InvokeDataUnwrap));
229+
return WH_ERROR_OK;
230+
}
231+
232+
#endif /* WOLFHSM_CFG_KEYWRAP && CLIENT && !NO_CRYPTO */

‎test-refactor/wh_test_list.c‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333

3434
/* Test declarations and weak skip implementations. */
3535
WH_TEST_DECL(whTest_Dma);
36+
WH_TEST_DECL(whTest_KeyWrapRespSize);
3637
WH_TEST_DECL(whTest_CertVerify);
3738
WH_TEST_DECL(whTest_CryptoAes);
3839
WH_TEST_DECL(whTest_CryptoEcc256);
@@ -43,6 +44,7 @@ WH_TEST_DECL(whTest_WolfCryptTest);
4344

4445
const whTestCase whTestsMisc[] = {
4546
{ "whTest_Dma", whTest_Dma },
47+
{ "whTest_KeyWrapRespSize", whTest_KeyWrapRespSize },
4648
};
4749
const size_t whTestsMiscCount = sizeof(whTestsMisc) / sizeof(whTestsMisc[0]);
4850

0 commit comments

Comments
 (0)