diff --git a/examples/posix/wh_posix_server/wh_posix_server_cfg.c b/examples/posix/wh_posix_server/wh_posix_server_cfg.c index c826438f0..17a671f94 100644 --- a/examples/posix/wh_posix_server/wh_posix_server_cfg.c +++ b/examples/posix/wh_posix_server/wh_posix_server_cfg.c @@ -486,12 +486,19 @@ static void parseNvmInitFile(const char* filePath) /* Parse the file path */ token = strtok(NULL, " "); - if (!token || sscanf(token, "%s", filePath) != 1) { + if (!token) { WOLFHSM_CFG_PRINTF("Error on line %d: Malformed entry - missing file path\n", lineNumber); fclose(file); exit(EXIT_FAILURE); } + if (snprintf(filePath, sizeof(filePath), "%s", token) >= + (int)sizeof(filePath)) { + WOLFHSM_CFG_PRINTF("Error on line %d: File path too long\n", + lineNumber); + fclose(file); + exit(EXIT_FAILURE); + } /* Add the parsed entry to the linked list */ appendEntry(&entryHead, (uint8_t)clientId, (uint16_t)id, diff --git a/port/posix/posix_transport_shm.c b/port/posix/posix_transport_shm.c index a5ba81ba9..504e6d4eb 100644 --- a/port/posix/posix_transport_shm.c +++ b/port/posix/posix_transport_shm.c @@ -208,8 +208,8 @@ static int posixTransportShm_CreateMap(char* name, uint16_t req_size, /* Attempt to remove any existing object. */ (void)shm_unlink(name); - /* Create shared memory object and set the size */ - fd = shm_open(name, O_CREAT | O_RDWR, PTSHM_CREATEMODE); + /* O_EXCL fails the open if a racing process re-created the name */ + fd = shm_open(name, O_CREAT | O_EXCL | O_RDWR, PTSHM_CREATEMODE); if (fd >= 0) { /* Set the size of the shared memory object. * Note this is the minimum size, as the OS may make it larger. */ diff --git a/src/wh_auth_base.c b/src/wh_auth_base.c index 9b49f7001..fda3547a7 100644 --- a/src/wh_auth_base.c +++ b/src/wh_auth_base.c @@ -479,6 +479,11 @@ int wh_Auth_BaseUserSetCredentials(void* context, uint16_t current_user_id, return WH_ERROR_ACCESS; } + /* A positive new-credential length requires a valid buffer */ + if (new_credentials == NULL && new_credentials_len > 0) { + return WH_ERROR_BADARGS; + } + /* Validate method is supported */ if (method != WH_AUTH_METHOD_PIN #if defined(WOLFHSM_CFG_CERTIFICATE_MANAGER) && !defined(WOLFHSM_CFG_NO_CRYPTO) @@ -543,7 +548,8 @@ int wh_Auth_BaseUserSetCredentials(void* context, uint16_t current_user_id, } } - /* Set new credentials */ + /* Set new credentials. The prior credential is zeroized only at the commit + * point so a failed update leaves the existing credential intact. */ if (new_credentials_len > 0) { if (method == WH_AUTH_METHOD_PIN) { #ifndef WOLFHSM_CFG_NO_CRYPTO @@ -555,6 +561,7 @@ int wh_Auth_BaseUserSetCredentials(void* context, uint16_t current_user_id, wh_Utils_ForceZero(hash, sizeof(hash)); return rc; } + wh_Utils_ForceZero(user->credentials, sizeof(user->credentials)); memcpy(user->credentials, hash, WC_SHA256_DIGEST_SIZE); user->credentials_len = WC_SHA256_DIGEST_SIZE; wh_Utils_ForceZero(hash, sizeof(hash)); @@ -562,6 +569,7 @@ int wh_Auth_BaseUserSetCredentials(void* context, uint16_t current_user_id, if (new_credentials_len > WH_AUTH_BASE_MAX_CREDENTIALS_LEN) { return WH_ERROR_BUFFER_SIZE; } + wh_Utils_ForceZero(user->credentials, sizeof(user->credentials)); memcpy(user->credentials, new_credentials, new_credentials_len); user->credentials_len = new_credentials_len; #endif /* WOLFHSM_CFG_NO_CRYPTO */ @@ -570,12 +578,14 @@ int wh_Auth_BaseUserSetCredentials(void* context, uint16_t current_user_id, if (new_credentials_len > WH_AUTH_BASE_MAX_CREDENTIALS_LEN) { return WH_ERROR_BUFFER_SIZE; } + wh_Utils_ForceZero(user->credentials, sizeof(user->credentials)); memcpy(user->credentials, new_credentials, new_credentials_len); user->credentials_len = new_credentials_len; } } else { /* Allow clearing credentials by setting length to 0 */ + wh_Utils_ForceZero(user->credentials, sizeof(user->credentials)); user->credentials_len = 0; } user->method = method; diff --git a/src/wh_client.c b/src/wh_client.c index 688c42143..97562a78c 100644 --- a/src/wh_client.c +++ b/src/wh_client.c @@ -37,6 +37,7 @@ /* Components */ #include "wolfhsm/wh_comm.h" +#include "wolfhsm/wh_utils.h" #ifndef WOLFHSM_CFG_NO_CRYPTO #include "wolfssl/wolfcrypt/settings.h" @@ -811,6 +812,7 @@ int wh_Client_KeyCacheRequest_ex(whClientContext* c, uint32_t flags, whMessageKeystore_CacheRequest* req = NULL; uint8_t* packIn; uint16_t capSz; + int ret; if (c == NULL || in == NULL || inSz == 0 || sizeof(*req) + inSz > WOLFHSM_CFG_COMM_DATA_LEN) { @@ -841,8 +843,10 @@ int wh_Client_KeyCacheRequest_ex(whClientContext* c, uint32_t flags, memcpy(packIn, in, inSz); /* write request */ - return wh_Client_SendRequest(c, WH_MESSAGE_GROUP_KEY, WH_KEY_CACHE, - sizeof(*req) + inSz, (uint8_t*)req); + ret = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_KEY, WH_KEY_CACHE, + sizeof(*req) + inSz, (uint8_t*)req); + wh_Utils_ForceZero(packIn, inSz); + return ret; } int wh_Client_KeyCacheRequest(whClientContext* c, uint32_t flags, @@ -1001,6 +1005,10 @@ int wh_Client_KeyExportResponse(whClientContext* c, uint8_t* label, if (resp->rc != 0) { ret = resp->rc; } + else if (size < sizeof(*resp) || resp->len > (size - sizeof(*resp))) { + /* Response frame does not contain the claimed key length */ + ret = WH_ERROR_ABORTED; + } else { if (out == NULL) { *outSz = resp->len; @@ -1019,6 +1027,7 @@ int wh_Client_KeyExportResponse(whClientContext* c, uint8_t* label, else memcpy(label, resp->label, labelSz); } + wh_Utils_ForceZero(packOut, resp->len); } } return ret; diff --git a/src/wh_client_keywrap.c b/src/wh_client_keywrap.c index 0bb1e15f0..763892c23 100644 --- a/src/wh_client_keywrap.c +++ b/src/wh_client_keywrap.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -17,6 +18,7 @@ int wh_Client_KeyWrapRequest(whClientContext* ctx, uint16_t serverKeyId, void* key, uint16_t keySz, whNvmMetadata* metadata) { + int ret; uint16_t group = WH_MESSAGE_GROUP_KEY; uint16_t action = WH_KEY_KEYWRAP; whMessageKeystore_KeyWrapRequest* req = NULL; @@ -30,6 +32,10 @@ int wh_Client_KeyWrapRequest(whClientContext* ctx, return WH_ERROR_BADARGS; } + if (sizeof(*req) + sizeof(*metadata) + keySz > WOLFHSM_CFG_COMM_DATA_LEN) { + return WH_ERROR_BADARGS; + } + /* Set the request pointer to the shared comm data memory region */ req = (whMessageKeystore_KeyWrapRequest*)wh_CommClient_GetDataPtr(ctx->comm); @@ -47,9 +53,11 @@ int wh_Client_KeyWrapRequest(whClientContext* ctx, memcpy(reqData, metadata, sizeof(*metadata)); memcpy(reqData + sizeof(*metadata), key, keySz); - return wh_Client_SendRequest(ctx, group, action, - sizeof(*req) + sizeof(*metadata) + keySz, - (uint8_t*)req); + ret = wh_Client_SendRequest(ctx, group, action, + sizeof(*req) + sizeof(*metadata) + keySz, + (uint8_t*)req); + wh_Utils_ForceZero(reqData + sizeof(*metadata), keySz); + return ret; } int wh_Client_KeyWrapResponse(whClientContext* ctx, @@ -220,6 +228,7 @@ int wh_Client_KeyUnwrapAndExportResponse(whClientContext* ctx, memcpy(keyOut, respData + sizeof(*metadataOut), resp->keySz); *keyInOutSz = resp->keySz; + wh_Utils_ForceZero(respData + sizeof(*metadataOut), resp->keySz); return WH_ERROR_OK; } diff --git a/src/wh_client_she.c b/src/wh_client_she.c index 716934f83..4d772f222 100644 --- a/src/wh_client_she.c +++ b/src/wh_client_she.c @@ -46,6 +46,7 @@ #include "wolfhsm/wh_client.h" #include "wolfhsm/wh_client_she.h" +#include "wolfhsm/wh_utils.h" int wh_Client_ShePreProgramKey(whClientContext* c, whNvmId keyId, whNvmFlags flags, uint8_t* key, whNvmSize keySz) @@ -336,6 +337,7 @@ int wh_Client_SheLoadKey(whClientContext* c, uint8_t* messageOne, int wh_Client_SheLoadPlainKeyRequest(whClientContext* c, uint8_t* key, uint32_t keySz) { + int ret; whMessageShe_LoadPlainKeyRequest* req = NULL; if (c == NULL || key == NULL || keySz < WH_SHE_KEY_SZ) { @@ -346,8 +348,10 @@ int wh_Client_SheLoadPlainKeyRequest(whClientContext* c, uint8_t* key, memcpy(req->key, key, WH_SHE_KEY_SZ); - return wh_Client_SendRequest(c, WH_MESSAGE_GROUP_SHE, WH_SHE_LOAD_PLAIN_KEY, - sizeof(*req), (uint8_t*)req); + ret = wh_Client_SendRequest(c, WH_MESSAGE_GROUP_SHE, WH_SHE_LOAD_PLAIN_KEY, + sizeof(*req), (uint8_t*)req); + wh_Utils_ForceZero(req->key, WH_SHE_KEY_SZ); + return ret; } int wh_Client_SheLoadPlainKeyResponse(whClientContext* c) diff --git a/src/wh_server_cert.c b/src/wh_server_cert.c index db0871605..5de6ebeeb 100644 --- a/src/wh_server_cert.c +++ b/src/wh_server_cert.c @@ -1515,15 +1515,15 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic, WH_DMA_OPER_CLIENT_READ_POST, (whServerDmaFlags){0}); } + /* Propagate any server-side error before serializing the response */ + if (rc != WH_ERROR_OK) { + resp.rc = rc; + } + /* Convert the response struct */ wh_MessageCert_TranslateSimpleResponse( magic, &resp, (whMessageCert_SimpleResponse*)resp_packet); *out_resp_size = sizeof(resp); - - /* If there was an error, return it in the response */ - if (rc != WH_ERROR_OK) { - resp.rc = rc; - } } break; #endif /* WOLFHSM_CFG_DMA */ #endif /* WOLFHSM_CFG_CERTIFICATE_MANAGER_ACERT */ diff --git a/src/wh_server_crypto.c b/src/wh_server_crypto.c index 788f91f53..81966e3e4 100644 --- a/src/wh_server_crypto.c +++ b/src/wh_server_crypto.c @@ -1533,8 +1533,10 @@ static int _HandleHkdf(whServerContext* ctx, uint16_t magic, int devId, */ uint8_t* out = (uint8_t*)cryptoDataOut + sizeof(whMessageCrypto_HkdfResponse); - uint16_t max_size = (uint16_t)(WOLFHSM_CFG_COMM_DATA_LEN - - ((uint8_t*)out - (uint8_t*)cryptoDataOut)); + uint16_t max_size = + (uint16_t)(WOLFHSM_CFG_COMM_DATA_LEN - + sizeof(whMessageCrypto_GenericResponseHeader) - + sizeof(whMessageCrypto_HkdfResponse)); /* Check if output size is valid */ if (outSz > max_size) { @@ -1694,8 +1696,10 @@ static int _HandleCmacKdf(whServerContext* ctx, uint16_t magic, int devId, uint8_t* out = (uint8_t*)cryptoDataOut + sizeof(whMessageCrypto_CmacKdfResponse); - uint16_t max_size = (uint16_t)(WOLFHSM_CFG_COMM_DATA_LEN - - ((uint8_t*)out - (uint8_t*)cryptoDataOut)); + uint16_t max_size = + (uint16_t)(WOLFHSM_CFG_COMM_DATA_LEN - + sizeof(whMessageCrypto_GenericResponseHeader) - + sizeof(whMessageCrypto_CmacKdfResponse)); if (outSz > max_size) { return WH_ERROR_BADARGS; @@ -3365,9 +3369,10 @@ static int _HandleAesGcm(whServerContext* ctx, uint16_t magic, int devId, uint32_t tag_len = req.authTagSz; whKeyId key_id = wh_KeyId_TranslateFromClient( WH_KEYTYPE_CRYPTO, ctx->comm->client_id, req.keyId); - uint64_t needed_size = sizeof(whMessageCrypto_AesGcmRequest) + len + - key_len + iv_len + authin_len + - ((enc == 0) ? tag_len : 0); + uint64_t needed_size = (uint64_t)sizeof(whMessageCrypto_AesGcmRequest) + + (uint64_t)len + (uint64_t)key_len + + (uint64_t)iv_len + (uint64_t)authin_len + + ((enc == 0) ? (uint64_t)tag_len : 0); if (needed_size != inSize) { return WH_ERROR_BADARGS; } @@ -3391,6 +3396,12 @@ static int _HandleAesGcm(whServerContext* ctx, uint16_t magic, int devId, uint32_t res_len = sizeof(whMessageCrypto_AesGcmResponse) + len + ((enc == 0) ? 0 : tag_len); + /* Ensure the response output and tag fit within the comm data buffer */ + if (res_len > (WOLFHSM_CFG_COMM_DATA_LEN - + sizeof(whMessageCrypto_GenericResponseHeader))) { + return WH_ERROR_BADARGS; + } + WH_DEBUG_SERVER_VERBOSE("AESGCM: enc:%d keylen:%d ivsz:%d insz:%d authinsz:%d " "authtagsz:%d reqsz:%u ressz:%u\n", enc, key_len, iv_len, len, authin_len, tag_len, (uint32_t)needed_size, diff --git a/tools/whnvmtool/whnvmtool.c b/tools/whnvmtool/whnvmtool.c index ec8012dbf..ffc46d0b1 100644 --- a/tools/whnvmtool/whnvmtool.c +++ b/tools/whnvmtool/whnvmtool.c @@ -384,13 +384,20 @@ void parseConfigFile(const char* filePath) /* Parse the file path */ token = strtok(NULL, " "); - if (!token || sscanf(token, "%s", filePath) != 1) { + if (!token) { fprintf(stderr, "Error on line %d: Malformed entry - missing file path\n", lineNumber); fclose(file); exit(EXIT_FAILURE); } + if (snprintf(filePath, sizeof(filePath), "%s", token) >= + (int)sizeof(filePath)) { + fprintf(stderr, "Error on line %d: File path too long\n", + lineNumber); + fclose(file); + exit(EXIT_FAILURE); + } /* Add the parsed entry to the linked list */ appendEntry(&entryHead, (uint8_t)clientId, (uint16_t)id,