Skip to content

Commit

Permalink
Review feedback:
Browse files Browse the repository at this point in the history 
- Renamed custom structs and functions to customcb
- renamed files
- added blocking client helper
- changed request/response id from uint16_t to uint32_t
  • Loading branch information
@bigbrett
bigbrett committed Apr 4, 2024
1 parent 5909758 commit 1029307
Show file tree
Hide file tree
Showing 11 changed files with 180 additions and 158 deletions.
52 changes: 37 additions & 15 deletions src/wh_client.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
/* Message definitions */
#include "wolfhsm/wh_message.h"
#include "wolfhsm/wh_message_comm.h"
#include "wolfhsm/wh_message_custom.h"
#include "wolfhsm/wh_message_customcb.h"

#include "wolfhsm/wh_client.h"

Expand Down Expand Up @@ -202,20 +202,20 @@ int wh_Client_Echo(whClientContext* c, uint16_t snd_len, const void* snd_data,
return rc;
}

int wh_Client_CustomRequest(whClientContext* c, const whMessageCustom_Request* req)
int wh_Client_CustomCbRequest(whClientContext* c, const whMessageCustomCb_Request* req)
{
if (NULL == c || req == NULL || req->id >= WH_CUSTOM_RQST_NUM_CALLBACKS) {
if (NULL == c || req == NULL || req->id >= WH_CUSTOM_CB_NUM_CALLBACKS) {
return WH_ERROR_BADARGS;
}

return wh_Client_SendRequest(c, WH_MESSAGE_GROUP_CUSTOM, req->id,
sizeof(*req), req);
}

int wh_Client_CustomResponse(whClientContext* c,
whMessageCustom_Response* outResp)
int wh_Client_CustomCbResponse(whClientContext* c,
whMessageCustomCb_Response* outResp)
{
whMessageCustom_Response resp;
whMessageCustomCb_Response resp;
uint16_t resp_group = 0;
uint16_t resp_action = 0;
uint16_t resp_size = 0;
Expand All @@ -232,7 +232,7 @@ int wh_Client_CustomResponse(whClientContext* c,
}

if (resp_size != sizeof(resp) || resp_group != WH_MESSAGE_GROUP_CUSTOM ||
resp_action >= WH_CUSTOM_RQST_NUM_CALLBACKS) {
resp_action >= WH_CUSTOM_CB_NUM_CALLBACKS) {
/* message invalid */
return WH_ERROR_ABORTED;
}
Expand All @@ -242,37 +242,37 @@ int wh_Client_CustomResponse(whClientContext* c,
return WH_ERROR_OK;
}

int wh_Client_CustomRequestCheckRegistered(whClientContext* c, uint32_t id)
int wh_Client_CustomCheckRegisteredRequest(whClientContext* c, uint32_t id)
{
whMessageCustom_Request req = {0};
whMessageCustomCb_Request req = {0};

if (c == NULL || id >= WH_CUSTOM_RQST_NUM_CALLBACKS) {
if (c == NULL || id >= WH_CUSTOM_CB_NUM_CALLBACKS) {
return WH_ERROR_BADARGS;
}

req.id = id;
req.type = WH_MESSAGE_CUSTOM_TYPE_QUERY;
req.type = WH_MESSAGE_CUSTOM_CB_TYPE_QUERY;

return wh_Client_SendRequest(c, WH_MESSAGE_GROUP_CUSTOM, req.id,
sizeof(req), &req);
}


int wh_Client_CustomResponseCheckRegistered(whClientContext* c, uint16_t* outId, int* responseError)
int wh_Client_CustomCbCheckRegisteredResponse(whClientContext* c, uint16_t* outId, int* responseError)
{
int rc = 0;
whMessageCustom_Response resp = {0};
whMessageCustomCb_Response resp = {0};

if (c == NULL || outId == NULL || responseError == NULL) {
return WH_ERROR_BADARGS;
}

rc = wh_Client_CustomResponse(c, &resp);
rc = wh_Client_CustomCbResponse(c, &resp);
if (rc != WH_ERROR_OK) {
return rc;
}

if (resp.type != WH_MESSAGE_CUSTOM_TYPE_QUERY) {
if (resp.type != WH_MESSAGE_CUSTOM_CB_TYPE_QUERY) {
/* message invalid */
return WH_ERROR_ABORTED;
}
Expand All @@ -286,4 +286,26 @@ int wh_Client_CustomResponseCheckRegistered(whClientContext* c, uint16_t* outId,
*responseError = resp.err;

return WH_ERROR_OK;
}


int wh_Client_CustomCbCheckRegistered(whClientContext* c, uint16_t id, int* responseError)
{
int rc = 0;

if (NULL == c || NULL == responseError || id >= WH_CUSTOM_CB_NUM_CALLBACKS) {
return WH_ERROR_BADARGS;
}

do {
rc = wh_Client_CustomCheckRegisteredRequest(c, id);
} while (rc == WH_ERROR_NOTREADY);

if (rc == WH_ERROR_OK) {
do {
rc = wh_Client_CustomCbCheckRegisteredResponse(c, &id, responseError);
} while (rc == WH_ERROR_NOTREADY);
}

return rc;
}
33 changes: 16 additions & 17 deletions src/wh_message_custom.c → src/wh_message_customcb.c
View file
Original file line number Diff line number Diff line change
@@ -1,21 +1,21 @@
#include <stddef.h>
#include <string.h>

#include "wolfhsm/wh_message_custom.h"
#include "wolfhsm/wh_message_customcb.h"
#include "wolfhsm/wh_error.h"
#include "wolfhsm/wh_comm.h"


static void _translateCustomData(uint16_t magic, uint32_t translatedType,
const whMessageCustom_Data* src,
whMessageCustom_Data* dst)
const whMessageCustomCb_Data* src,
whMessageCustomCb_Data* dst)
{
if (translatedType < WH_MESSAGE_CUSTOM_TYPE_USER_DEFINED_START) {
if (translatedType < WH_MESSAGE_CUSTOM_CB_TYPE_USER_DEFINED_START) {
switch (translatedType) {
case WH_MESSAGE_CUSTOM_TYPE_QUERY: {
case WH_MESSAGE_CUSTOM_CB_TYPE_QUERY: {
/* right now, no further translations required */
} break;
case WH_MESSAGE_CUSTOM_TYPE_DMA32: {
case WH_MESSAGE_CUSTOM_CB_TYPE_DMA32: {
dst->dma32.client_addr =
wh_Translate32(magic, src->dma32.client_addr);
dst->dma32.client_sz =
Expand All @@ -25,7 +25,7 @@ static void _translateCustomData(uint16_t magic, uint32_t translatedType,
dst->dma32.server_sz =
wh_Translate32(magic, src->dma32.server_sz);
} break;
case WH_MESSAGE_CUSTOM_TYPE_DMA64: {
case WH_MESSAGE_CUSTOM_CB_TYPE_DMA64: {
dst->dma64.client_addr =
wh_Translate64(magic, src->dma64.client_addr);
dst->dma64.client_sz =
Expand All @@ -42,31 +42,30 @@ static void _translateCustomData(uint16_t magic, uint32_t translatedType,
}
else {
/* use memmove in case data is translated "in place" */
memmove(dst->buffer.data, src->buffer.data,
sizeof(dst->buffer.data));
memmove(dst->buffer.data, src->buffer.data, sizeof(dst->buffer.data));
}
}


int wh_MessageCustom_TranslateRequest(uint16_t magic,
const whMessageCustom_Request* src,
whMessageCustom_Request* dst)
int wh_MessageCustomCb_TranslateRequest(uint16_t magic,
const whMessageCustomCb_Request* src,
whMessageCustomCb_Request* dst)
{
if ((src == NULL) || (dst == NULL)) {
return WH_ERROR_BADARGS;
}

dst->id = wh_Translate16(magic, src->id);
dst->id = wh_Translate32(magic, src->id);
dst->type = wh_Translate32(magic, src->type);
_translateCustomData(magic, dst->type, &src->data, &dst->data);

return WH_ERROR_OK;
}


int wh_MessageCustom_TranslateResponse(uint16_t magic,
const whMessageCustom_Response* src,
whMessageCustom_Response* dst)
int wh_MessageCustomCb_TranslateResponse(uint16_t magic,
const whMessageCustomCb_Response* src,
whMessageCustomCb_Response* dst)
{
if ((src == NULL) || (dst == NULL)) {
return WH_ERROR_BADARGS;
Expand All @@ -77,7 +76,7 @@ int wh_MessageCustom_TranslateResponse(uint16_t magic,

/* TODO: should we continue to translate responses for err != 0?
* Probably still should...*/
dst->id = wh_Translate16(magic, src->id);
dst->id = wh_Translate32(magic, src->id);
dst->type = wh_Translate32(magic, src->type);
_translateCustomData(magic, dst->type, &src->data, &dst->data);

Expand Down
2 changes: 1 addition & 1 deletion src/wh_server.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ int wh_Server_HandleRequestMessage(whServerContext* server)
#endif

case WH_MESSAGE_GROUP_CUSTOM:
rc = wh_Server_HandleCustomRequest(server, magic, action, seq,
rc = wh_Server_HandleCustomCbRequest(server, magic, action, seq,
size, data, &size, data);
break;

Expand Down
33 changes: 17 additions & 16 deletions src/wh_server_custom.c → src/wh_server_customcb.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@
#include "wolfhsm/wh_common.h"
#include "wolfhsm/wh_error.h"
#include "wolfhsm/wh_message.h"
#include "wolfhsm/wh_message_custom.h"
#include "wolfhsm/wh_message_customcb.h"


int wh_Server_RegisterCustomCb(whServerContext* server, uint16_t action, whServerCustomCb handler)
int wh_Server_RegisterCustomCb(whServerContext* server, uint16_t action,
whServerCustomCb handler)
{
if (NULL == server || NULL == handler ||
action >= WH_CUSTOM_RQST_NUM_CALLBACKS) {
action >= WH_CUSTOM_CB_NUM_CALLBACKS) {
return WH_ERROR_BADARGS;
}

Expand All @@ -18,41 +19,41 @@ int wh_Server_RegisterCustomCb(whServerContext* server, uint16_t action, whServe
}


int wh_Server_HandleCustomRequest(whServerContext* server, uint16_t magic,
uint16_t action, uint16_t seq,
uint16_t req_size, const void* req_packet,
uint16_t* out_resp_size, void* resp_packet)
int wh_Server_HandleCustomCbRequest(whServerContext* server, uint16_t magic,
uint16_t action, uint16_t seq,
uint16_t req_size, const void* req_packet,
uint16_t* out_resp_size, void* resp_packet)
{
int rc = 0;
whMessageCustom_Request req = {0};
whMessageCustom_Response resp = {0};
int rc = 0;
whMessageCustomCb_Request req = {0};
whMessageCustomCb_Response resp = {0};

if (NULL == server || NULL == req_packet || NULL == resp_packet ||
out_resp_size == NULL) {
return WH_ERROR_BADARGS;
}

if (action >= WH_CUSTOM_RQST_NUM_CALLBACKS) {
if (action >= WH_CUSTOM_CB_NUM_CALLBACKS) {
/* Invalid callback index */
/* TODO: is this the appropriate error to return? */
return WH_ERROR_BADARGS;
}

if (req_size != sizeof(whMessageCustom_Request)) {
if (req_size != sizeof(whMessageCustomCb_Request)) {
/* Request is malformed */
return WH_ERROR_ABORTED;
}

/* Translate the request */
if ((rc = wh_MessageCustom_TranslateRequest(magic, req_packet, &req)) !=
if ((rc = wh_MessageCustomCb_TranslateRequest(magic, req_packet, &req)) !=
WH_ERROR_OK) {
return rc;
}

if (server->customHandlerTable[action] != NULL) {
/* If this isn't a query to check if the callback exists, invoke the
* registered callback, storing the return value in the reponse */
if (req.type != WH_MESSAGE_CUSTOM_TYPE_QUERY) {
if (req.type != WH_MESSAGE_CUSTOM_CB_TYPE_QUERY) {
resp.rc = server->customHandlerTable[action](server, &req, &resp);
}
/* TODO: propagate other wolfHSM error codes (requires modifiying caller
Expand All @@ -69,8 +70,8 @@ int wh_Server_HandleCustomRequest(whServerContext* server, uint16_t magic,
resp.id = req.id;

/* Translate the response */
if ((rc = wh_MessageCustom_TranslateResponse(magic, &resp, resp_packet)) !=
WH_ERROR_OK) {
if ((rc = wh_MessageCustomCb_TranslateResponse(
magic, &resp, resp_packet)) != WH_ERROR_OK) {
return rc;
}

Expand Down
4 changes: 2 additions & 2 deletions test/Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,13 @@ SRC_C += \
$(WOLFHSM_DIR)/src/wh_flash_unit.c \
$(WOLFHSM_DIR)/src/wh_message_comm.c \
$(WOLFHSM_DIR)/src/wh_message_nvm.c \
$(WOLFHSM_DIR)/src/wh_message_custom.c \
$(WOLFHSM_DIR)/src/wh_message_customcb.c \
$(WOLFHSM_DIR)/src/wh_nvm.c \
$(WOLFHSM_DIR)/src/wh_nvm_flash.c \
$(WOLFHSM_DIR)/src/wh_server.c \
$(WOLFHSM_DIR)/src/wh_server_nvm.c \
$(WOLFHSM_DIR)/src/wh_server_crypto.c \
$(WOLFHSM_DIR)/src/wh_server_custom.c \
$(WOLFHSM_DIR)/src/wh_server_customcb.c \
$(WOLFHSM_DIR)/src/wh_client_cryptocb.c \
$(WOLFHSM_DIR)/src/wh_transport_mem.c \
$(WOLFHSM_DIR)/src/wh_flash_ramsim.c
Expand Down
Loading

0 comments on commit 1029307

Please sign in to comment.