Skip to content

Commit b3f2f60

Browse files
miyazakhmiyazakh
committed
addressed code review
1 parent cdb0cfa commit b3f2f60

13 files changed

Lines changed: 403 additions & 314 deletions

‎src/wh_client.c‎

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,13 @@ int wh_Client_Init(whClientContext* c, const whClientConfig* config)
8080
/* register the cancel callback */
8181
c->cancelCb = config->cancelCb;
8282
#endif
83-
83+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT)
84+
if (NULL != config->timeoutConfig) {
85+
c->timeout.timeout_val = config->timeoutConfig->timeout_val;
86+
c->timeout.timeout_enabled = config->timeoutConfig->timeout_enabled;
87+
c->timeout.start_time = 0;
88+
}
89+
#endif
8490
rc = wh_CommClient_Init(c->comm, config->comm);
8591

8692
#ifndef WOLFHSM_CFG_NO_CRYPTO
@@ -1519,4 +1525,104 @@ int wh_Client_KeyExportDma(whClientContext* c, uint16_t keyId,
15191525
}
15201526
#endif /* WOLFHSM_CFG_DMA */
15211527

1528+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT)
1529+
static uint64_t wh_timeval_to_64(const wh_timeval* tv)
1530+
{
1531+
if (tv == NULL) return 0;
1532+
return (uint64_t)tv->tv_sec * WH_BASE_TIMEOUT_UNIT
1533+
+ (uint64_t)((tv->tv_usec) / WH_BASE_TIMEOUT_UNIT);
1534+
}
1535+
/* Set Time Out if needed */
1536+
int wh_Client_InitCryptTimeout(whClientContext* c)
1537+
{
1538+
if (c == NULL) {
1539+
return WH_ERROR_BADARGS;
1540+
}
1541+
1542+
/* if feature not enabled, nothing to do */
1543+
if (c->timeout.timeout_enabled != 1) {
1544+
return WH_ERROR_OK;
1545+
}
1546+
if (c->timeout.cb.GetCurrentTime == NULL) {
1547+
return WH_ERROR_BADARGS;
1548+
}
1549+
/* initialize start time */
1550+
c->timeout.start_time = c->timeout.cb.GetCurrentTime(1);
1551+
1552+
return WH_ERROR_OK;
1553+
}
1554+
1555+
/* Check Crypto Timeout */
1556+
int wh_Client_CheckTimeout(whClientContext* c)
1557+
{
1558+
uint64_t current_ = 0;
1559+
uint64_t elapsed_ = 0;
1560+
uint64_t timeout_ = 0;
1561+
1562+
if (c == NULL) {
1563+
return WH_ERROR_BADARGS;
1564+
}
1565+
1566+
if (c->timeout.timeout_enabled != 1) {
1567+
return WH_ERROR_OK;
1568+
}
1569+
1570+
if (c->timeout.cb.GetCurrentTime == NULL) {
1571+
return WH_ERROR_BADARGS;
1572+
}
1573+
1574+
timeout_ = wh_timeval_to_64(&c->timeout.timeout_val);
1575+
if (timeout_ == 0) {
1576+
return WH_ERROR_OK;
1577+
}
1578+
1579+
/* check timeout by user cb if defined */
1580+
if (c->timeout.cb.CheckTimeout != NULL) {
1581+
return c->timeout.cb.CheckTimeout(
1582+
&c->timeout.start_time, timeout_);
1583+
}
1584+
1585+
/* Otherwise compute elapsed using user-provided GetCurrentTime */
1586+
current_ = c->timeout.cb.GetCurrentTime(0);
1587+
elapsed_ = current_ - c->timeout.start_time;
1588+
if (elapsed_ > timeout_) {
1589+
return WH_ERROR_TIMEOUT;
1590+
}
1591+
1592+
return WH_ERROR_OK;
1593+
}
1594+
1595+
int wh_Client_timeoutRegisterCb(whClientContext* client,
1596+
whClientTimeOutCb* cb)
1597+
{
1598+
/* No NULL check for cb, since it is optional and always NULL checked before
1599+
* it is called */
1600+
if (NULL == client) {
1601+
return WH_ERROR_BADARGS;
1602+
}
1603+
1604+
client->timeout.cb.GetCurrentTime = cb->GetCurrentTime;
1605+
client->timeout.cb.CheckTimeout = cb->CheckTimeout;
1606+
1607+
return WH_ERROR_OK;
1608+
}
1609+
1610+
int wh_Client_timeoutEnable(whClientContext* client,
1611+
wh_timeval* timeout_val)
1612+
{
1613+
if (NULL == client) {
1614+
return WH_ERROR_BADARGS;
1615+
}
1616+
1617+
if (timeout_val != NULL) {
1618+
client->timeout.timeout_enabled = 1;
1619+
memcpy(&client->timeout.timeout_val, timeout_val,
1620+
sizeof(wh_timeval));
1621+
} else {
1622+
client->timeout.timeout_enabled = 0;
1623+
memset(&client->timeout.timeout_val, 0, sizeof(wh_timeval));
1624+
}
1625+
return WH_ERROR_OK;
1626+
}
1627+
#endif /* WOLFHSM_CFG_CLIENT_TIMEOUT */
15221628
#endif /* WOLFHSM_CFG_ENABLE_CLIENT */

‎src/wh_client_crypto.c‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -188,12 +188,12 @@ static int _wait_response_with_crypttimeout(whClientContext *ctx,
188188
int ret = WH_ERROR_OK;
189189
do {
190190
ret = wh_Client_RecvResponse(ctx, out_group, out_action, out_size, data);
191-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
191+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT)
192192
if (ret == WH_ERROR_NOTREADY) {
193193
/* Check for crypto timeout */
194-
int chk = wh_CommClient_CheckTimeout(ctx->comm);
195-
if (chk == WH_ERROR_CRYPTIMEOUT) {
196-
return WH_ERROR_CRYPTIMEOUT;
194+
int chk = wh_Client_CheckTimeout(ctx);
195+
if (chk == WH_ERROR_TIMEOUT) {
196+
return WH_ERROR_TIMEOUT;
197197
} else if (chk < 0 && chk != WH_ERROR_OK) {
198198
return chk;
199199
}
@@ -253,9 +253,9 @@ int wh_Client_RngGenerate(whClientContext* ctx, uint8_t* out, uint32_t size)
253253

254254
/* Send request and get response */
255255
ret = wh_Client_SendRequest(ctx, group, action, req_len, dataPtr);
256-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
256+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT)
257257
if (ret == WH_ERROR_OK) {
258-
ret = wh_CommClient_InitCryptTimeout(ctx->comm);
258+
ret = wh_Client_InitCryptTimeout(ctx);
259259
}
260260
#endif
261261
if (ret == 0) {
@@ -436,9 +436,9 @@ int wh_Client_AesCtr(whClientContext* ctx, Aes* aes, int enc, const uint8_t* in,
436436
WH_DEBUG_VERBOSE_HEXDUMP("[client] tmp: \n", req_tmp, AES_BLOCK_SIZE);
437437
WH_DEBUG_VERBOSE_HEXDUMP("[client] req packet: \n", (uint8_t*)req, req_len);
438438
ret = wh_Client_SendRequest(ctx, group, action, req_len, dataPtr);
439-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
439+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT)
440440
if (ret == WH_ERROR_OK) {
441-
ret = wh_CommClient_InitCryptTimeout(ctx->comm);
441+
ret = wh_Client_InitCryptTimeout(ctx);
442442
}
443443
#endif
444444
/* read response */
@@ -555,9 +555,9 @@ int wh_Client_AesEcb(whClientContext* ctx, Aes* aes, int enc, const uint8_t* in,
555555
WH_DEBUG_VERBOSE_HEXDUMP("[client] iv: \n", req_iv, iv_len);
556556
WH_DEBUG_VERBOSE_HEXDUMP("[client] req packet: \n", (uint8_t*)req, req_len);
557557
ret = wh_Client_SendRequest(ctx, group, action, req_len, dataPtr);
558-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
558+
#if defined(WOLFHSM_CFG_ENABLE_CWOLFHSM_CFG_CLIENT_TIMEOUTLIENT_TIMEOUT)
559559
if (ret == WH_ERROR_OK) {
560-
ret = wh_CommClient_InitCryptTimeout(ctx->comm);
560+
ret = wh_Client_InitCryptTimeout(ctx);
561561
}
562562
#endif
563563
/* read response */
@@ -671,9 +671,9 @@ int wh_Client_AesCbc(whClientContext* ctx, Aes* aes, int enc, const uint8_t* in,
671671
WH_DEBUG_VERBOSE_HEXDUMP("[client] iv: \n", req_iv, iv_len);
672672
WH_DEBUG_VERBOSE_HEXDUMP("[client] req packet: \n", (uint8_t*)req, req_len);
673673
ret = wh_Client_SendRequest(ctx, group, action, req_len, dataPtr);
674-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
674+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT)
675675
if (ret == WH_ERROR_OK) {
676-
ret = wh_CommClient_InitCryptTimeout(ctx->comm);
676+
ret = wh_Client_InitCryptTimeout(ctx);
677677
}
678678
#endif
679679
/* read response */
@@ -801,9 +801,9 @@ int wh_Client_AesGcm(whClientContext* ctx, Aes* aes, int enc, const uint8_t* in,
801801

802802
/* Send request and receive response */
803803
ret = wh_Client_SendRequest(ctx, group, action, req_len, dataPtr);
804-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
804+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT)
805805
if (ret == WH_ERROR_OK) {
806-
ret = wh_CommClient_InitCryptTimeout(ctx->comm);
806+
ret = wh_Client_InitCryptTimeout(ctx);
807807
}
808808
#endif
809809
if (ret == 0) {
@@ -1007,9 +1007,9 @@ int wh_Client_AesGcmDma(whClientContext* ctx, Aes* aes, int enc,
10071007
if (ret == WH_ERROR_OK) {
10081008
ret = wh_Client_SendRequest(ctx, group, action, reqLen, dataPtr);
10091009
}
1010-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
1010+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT)
10111011
if (ret == WH_ERROR_OK) {
1012-
ret = wh_CommClient_InitCryptTimeout(ctx->comm);
1012+
ret = wh_Client_InitCryptTimeout(ctx);
10131013
}
10141014
#endif
10151015
if (ret == 0) {

‎src/wh_comm.c‎

Lines changed: 1 addition & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -74,11 +74,7 @@ int wh_CommClient_Init(whCommClient* context, const whCommClientConfig* config)
7474
context->transport_context = config->transport_context;
7575
context->client_id = config->client_id;
7676
context->connect_cb = config->connect_cb;
77-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
78-
context->crypt_timeout_cb = config->crypt_timeout_cb;
79-
context->crypt_timeout_enabled = config->crypt_timeout_enabled;
80-
context->crypt_timeout = config->crypt_timeout;
81-
#endif
77+
8278
if (context->transport_cb->Init != NULL) {
8379
rc = context->transport_cb->Init(context->transport_context,
8480
config->transport_config, NULL, NULL);
@@ -215,69 +211,7 @@ int wh_CommClient_Cleanup(whCommClient* context)
215211
return rc;
216212
}
217213

218-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
219-
static uint64_t wh_timeval_to_ms64(const WOLFHSM_TIMEVAL* tv)
220-
{
221-
if (tv == NULL) return 0;
222-
return (uint64_t)tv->tv_sec * 1000ULL + (uint64_t)((tv->tv_usec) / 1000ULL);
223-
}
224-
/* Set Crypto Time Out if needed */
225-
int wh_CommClient_InitCryptTimeout(whCommClient* context)
226-
{
227-
if (context == NULL)
228-
return WH_ERROR_BADARGS;
229-
/* if feature not enabled, nothing to do */
230-
if (context->crypt_timeout_enabled != 1)
231-
return WH_ERROR_OK;
232-
if (context->crypt_timeout_cb == NULL ||
233-
context->crypt_timeout_cb->GetCurrentTime == NULL)
234-
return WH_ERROR_BADARGS;
235-
236-
/* cache conversion of crypt_timeout to milliseconds */
237-
context->crypt_timeout_ms = wh_timeval_to_ms64(&context->crypt_timeout);
238-
/* initialize start time */
239-
context->crypt_start_time =
240-
context->crypt_timeout_cb->GetCurrentTime(1);
241-
242-
return WH_ERROR_OK;
243-
}
244-
245-
/* Check Crypto Timeout */
246-
int wh_CommClient_CheckTimeout(whCommClient* context)
247-
{
248-
uint64_t current_ms = 0;
249-
uint64_t elapsed_ms = 0;
250-
uint64_t timeout_ms = 0;
251214

252-
if (context == NULL) return WH_ERROR_BADARGS;
253-
254-
if (context->crypt_timeout_enabled != 1)
255-
return WH_ERROR_OK;
256-
257-
if (context->crypt_timeout_cb == NULL ||
258-
context->crypt_timeout_cb->GetCurrentTime == NULL)
259-
return WH_ERROR_BADARGS;
260-
261-
timeout_ms = context->crypt_timeout_ms;
262-
if (timeout_ms == 0)
263-
return WH_ERROR_OK;
264-
265-
/* check timeout by user cb if defined */
266-
if (context->crypt_timeout_cb->CheckTimeout != NULL) {
267-
return context->crypt_timeout_cb->CheckTimeout(
268-
&context->crypt_start_time, timeout_ms);
269-
}
270-
271-
/* Otherwise compute elapsed using user-provided GetCurrentTime */
272-
current_ms = context->crypt_timeout_cb->GetCurrentTime(0);
273-
elapsed_ms = current_ms - context->crypt_start_time;
274-
if (elapsed_ms > timeout_ms) {
275-
return WH_ERROR_CRYPTIMEOUT;
276-
}
277-
278-
return WH_ERROR_OK;
279-
}
280-
#endif /* WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT */
281215
#endif /* WOLFHSM_CFG_ENABLE_CLIENT */
282216

283217
/** Server Functions */

‎test/Makefile‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ else
137137
endif
138138

139139
ifeq ($(CRYPTIMEOUT),1)
140-
DEF += -DWOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT
140+
DEF += -DWOLFHSM_CFG_CLIENT_TIMEOUT
141141
endif
142142

143143
## Source files

‎test/config/wolfhsm_cfg.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,10 +69,10 @@
6969
#define WOLFHSM_CFG_SERVER_NVM_FLASH_LOG
7070

7171
/* Enable client crypto timeout feature for testing */
72-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT) && \
72+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT) && \
7373
defined(WOLFHSM_CFG_TEST_POSIX)
74-
#define WOLFHSM_CFG_CLIENT_CRYPTIMEOUT_SEC (2)
75-
#define WOLFHSM_CFG_TEST_CLIENT_CRYPTIMEOUT
76-
#endif /* WOLFHSM_CFG_TEST_CLIENT_CRYPTIMEOUT */
74+
#define WOLFHSM_CFG_CLIENT_TIMEOUT_USEC (500000) /* 500ms */
75+
#define WOLFHSM_CFG_TEST_CLIENT_TIMEOUT
76+
#endif /* WOLFHSM_CFG_TEST_CLIENT_TIMEOUT */
7777

7878
#endif /* WOLFHSM_CFG_H_ */

‎test/wh_test_common.c‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
#include <wolfhsm/wh_error.h>
2727

2828
#include "wh_test_common.h"
29-
#if defined(WOLFHSM_CFG_TEST_CLIENT_CRYPTIMEOUT)
29+
#if defined(WOLFHSM_CFG_TEST_CLIENT_TIMEOUT)
3030
#include <sys/time.h> /* For gettimeofday */
3131
#endif
3232

@@ -93,7 +93,7 @@ int whTest_NvmCfgBackend(whTestNvmBackendType type,
9393
return 0;
9494
}
9595

96-
#if defined(WOLFHSM_CFG_TEST_CLIENT_CRYPTIMEOUT)
96+
#if defined(WOLFHSM_CFG_TEST_CLIENT_TIMEOUT)
9797
#include <time.h>
9898
#include <sys/time.h> /* For gettimeofday */
9999

@@ -120,24 +120,29 @@ uint64_t whTest_GetCurrentTime(int reset)
120120
/* start_time stores the time (in milliseconds) returned by the GetCurrentTime()
121121
* callback when the operation started.
122122
* The actual unit depends on the GetCurrentTime() implementation.
123-
* timeout_ms represents the timeout in milliseconds, which is derived from
124-
* the crypt_timeout value in whCommClientConfig.
123+
* timeout_val represents the timeout in milliseconds(default),
124+
* which is derived from the timeout value in whCommClientConfig.
125125
*/
126-
int whTest_CheckCryptoTimeout(uint64_t* start_time, uint64_t timeout_ms)
126+
int whTest_CheckTimeout(uint64_t* start_time, uint64_t timeout_val)
127127
{
128128
uint64_t current_time;
129129
uint64_t elapsed_time;
130130

131-
if (start_time == NULL) return WH_ERROR_BADARGS;
132-
if (timeout_ms == 0) return WH_ERROR_OK;
131+
if (start_time == NULL) {
132+
return WH_ERROR_BADARGS;
133+
}
134+
135+
if (timeout_val == 0) {
136+
return WH_ERROR_OK;
137+
}
133138

134139
current_time = whTest_GetCurrentTime(0);
135140
elapsed_time = current_time - *start_time;
136141

137-
if (elapsed_time > timeout_ms) {
138-
return WH_ERROR_CRYPTIMEOUT;
142+
if (elapsed_time > timeout_val) {
143+
return WH_ERROR_TIMEOUT;
139144
}
140145

141146
return WH_ERROR_OK;
142147
}
143-
#endif /* WOLFHSM_CFG_TEST_CLIENT_CRYPTIMEOUT */
148+
#endif /* WOLFHSM_CFG_TEST_CLIENT_TIMEOUT */

‎test/wh_test_common.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -145,12 +145,12 @@ int whTest_NvmCfgBackend(whTestNvmBackendType type,
145145
whFlashRamsimCfg* fCfg, whFlashRamsimCtx* fCtx,
146146
const whFlashCb* fCb);
147147
uint64_t whTest_GetCurrentTime(int reset);
148-
int whTest_CheckCryptoTimeout(uint64_t* start_time, uint64_t timeout_ms);
148+
int whTest_CheckTimeout(uint64_t* start_time, uint64_t timeout_val);
149149

150-
#define WH_CLIENT_CRYPTO_TIMEOUT_CB \
150+
#define WH_CLIENT_TIMEOUT_CB \
151151
{ \
152152
.GetCurrentTime = whTest_GetCurrentTime, \
153-
.CheckTimeout = whTest_CheckCryptoTimeout, \
153+
.CheckTimeout = whTest_CheckTimeout, \
154154
}
155155

156156
#endif /* WH_TEST_COMMON_H_ */

0 commit comments

Comments
 (0)