Skip to content

Commit 02aa2c4

Browse files
miyazakhmiyazakh
committed
addressed code review
1 parent 4653cf5 commit 02aa2c4

13 files changed

Lines changed: 403 additions & 317 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
@@ -1523,4 +1529,104 @@ int wh_Client_KeyExportDma(whClientContext* c, uint16_t keyId,
15231529
}
15241530
#endif /* WOLFHSM_CFG_DMA */
15251531

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

‎src/wh_client_crypto.c‎

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

255255
/* Send request and get response */
256256
ret = wh_Client_SendRequest(ctx, group, action, req_len, dataPtr);
257-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
257+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT)
258258
if (ret == WH_ERROR_OK) {
259-
ret = wh_CommClient_InitCryptTimeout(ctx->comm);
259+
ret = wh_Client_InitCryptTimeout(ctx);
260260
}
261261
#endif
262262
if (ret == 0) {
@@ -446,9 +446,9 @@ int wh_Client_AesCtr(whClientContext* ctx, Aes* aes, int enc, const uint8_t* in,
446446
wh_Utils_Hexdump("[client] req packet: \n", (uint8_t*)req, req_len);
447447
#endif
448448
ret = wh_Client_SendRequest(ctx, group, action, req_len, dataPtr);
449-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
449+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT)
450450
if (ret == WH_ERROR_OK) {
451-
ret = wh_CommClient_InitCryptTimeout(ctx->comm);
451+
ret = wh_Client_InitCryptTimeout(ctx);
452452
}
453453
#endif
454454
/* read response */
@@ -575,9 +575,9 @@ int wh_Client_AesEcb(whClientContext* ctx, Aes* aes, int enc, const uint8_t* in,
575575
wh_Utils_Hexdump("[client] req packet: \n", (uint8_t*)req, req_len);
576576
#endif
577577
ret = wh_Client_SendRequest(ctx, group, action, req_len, dataPtr);
578-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
578+
#if defined(WOLFHSM_CFG_ENABLE_CWOLFHSM_CFG_CLIENT_TIMEOUTLIENT_TIMEOUT)
579579
if (ret == WH_ERROR_OK) {
580-
ret = wh_CommClient_InitCryptTimeout(ctx->comm);
580+
ret = wh_Client_InitCryptTimeout(ctx);
581581
}
582582
#endif
583583
/* read response */
@@ -701,9 +701,9 @@ int wh_Client_AesCbc(whClientContext* ctx, Aes* aes, int enc, const uint8_t* in,
701701
wh_Utils_Hexdump("[client] req packet: \n", (uint8_t*)req, req_len);
702702
#endif
703703
ret = wh_Client_SendRequest(ctx, group, action, req_len, dataPtr);
704-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
704+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT)
705705
if (ret == WH_ERROR_OK) {
706-
ret = wh_CommClient_InitCryptTimeout(ctx->comm);
706+
ret = wh_Client_InitCryptTimeout(ctx);
707707
}
708708
#endif
709709
/* read response */
@@ -842,9 +842,9 @@ int wh_Client_AesGcm(whClientContext* ctx, Aes* aes, int enc, const uint8_t* in,
842842

843843
/* Send request and receive response */
844844
ret = wh_Client_SendRequest(ctx, group, action, req_len, dataPtr);
845-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
845+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT)
846846
if (ret == WH_ERROR_OK) {
847-
ret = wh_CommClient_InitCryptTimeout(ctx->comm);
847+
ret = wh_Client_InitCryptTimeout(ctx);
848848
}
849849
#endif
850850
if (ret == 0) {
@@ -1058,9 +1058,9 @@ int wh_Client_AesGcmDma(whClientContext* ctx, Aes* aes, int enc,
10581058
if (ret == WH_ERROR_OK) {
10591059
ret = wh_Client_SendRequest(ctx, group, action, reqLen, dataPtr);
10601060
}
1061-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT)
1061+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT)
10621062
if (ret == WH_ERROR_OK) {
1063-
ret = wh_CommClient_InitCryptTimeout(ctx->comm);
1063+
ret = wh_Client_InitCryptTimeout(ctx);
10641064
}
10651065
#endif
10661066
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
@@ -136,7 +136,7 @@ else
136136
endif
137137

138138
ifeq ($(CRYPTIMEOUT),1)
139-
DEF += -DWOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT
139+
DEF += -DWOLFHSM_CFG_CLIENT_TIMEOUT
140140
endif
141141

142142
## Source files

‎test/config/wolfhsm_cfg.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,10 @@
6262
#define WOLFHSM_CFG_SERVER_NVM_FLASH_LOG
6363

6464
/* Enable client crypto timeout feature for testing */
65-
#if defined(WOLFHSM_CFG_ENABLE_CLIENT_CRYPTIMEOUT) && \
65+
#if defined(WOLFHSM_CFG_CLIENT_TIMEOUT) && \
6666
defined(WOLFHSM_CFG_TEST_POSIX)
67-
#define WOLFHSM_CFG_CLIENT_CRYPTIMEOUT_SEC (2)
68-
#define WOLFHSM_CFG_TEST_CLIENT_CRYPTIMEOUT
69-
#endif /* WOLFHSM_CFG_TEST_CLIENT_CRYPTIMEOUT */
67+
#define WOLFHSM_CFG_CLIENT_TIMEOUT_USEC (500000) /* 500ms */
68+
#define WOLFHSM_CFG_TEST_CLIENT_TIMEOUT
69+
#endif /* WOLFHSM_CFG_TEST_CLIENT_TIMEOUT */
7070

7171
#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
@@ -140,12 +140,12 @@ int whTest_NvmCfgBackend(whTestNvmBackendType type,
140140
whFlashRamsimCfg* fCfg, whFlashRamsimCtx* fCtx,
141141
const whFlashCb* fCb);
142142
uint64_t whTest_GetCurrentTime(int reset);
143-
int whTest_CheckCryptoTimeout(uint64_t* start_time, uint64_t timeout_ms);
143+
int whTest_CheckTimeout(uint64_t* start_time, uint64_t timeout_val);
144144

145-
#define WH_CLIENT_CRYPTO_TIMEOUT_CB \
145+
#define WH_CLIENT_TIMEOUT_CB \
146146
{ \
147147
.GetCurrentTime = whTest_GetCurrentTime, \
148-
.CheckTimeout = whTest_CheckCryptoTimeout, \
148+
.CheckTimeout = whTest_CheckTimeout, \
149149
}
150150

151151
#endif /* WH_TEST_COMMON_H_ */

0 commit comments

Comments
 (0)