Skip to content

Stop READTRUSTED transmitting unwritten response bytes #459

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yosuke-wolfssl wants to merge 1 commit into
base: main
Choose a base branch
from
+262 −8
Open

Stop READTRUSTED transmitting unwritten response bytes #459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions src/wh_server_cert.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -688,29 +688,31 @@ int wh_Server_CertReadTrusted(whServerContext* server, whNvmId id,
uint8_t* cert, uint32_t* inout_cert_len)
{
int rc;
uint32_t buf_len;
whNvmMetadata meta;

if ((server == NULL) || (cert == NULL) || (inout_cert_len == NULL) ||
(*inout_cert_len > WOLFHSM_CFG_MAX_CERT_SIZE)) {
return WH_ERROR_BADARGS;
}

buf_len = *inout_cert_len;

/* Get metadata to check the certificate size */
rc = wh_Nvm_GetMetadata(server->nvm, id, &meta);
if (rc != 0) {
return rc;
}

/* Report the actual length even when the buffer is too small, so the
* caller learns the size it could not receive */
*inout_cert_len = meta.len;

/* Check if the provided buffer is large enough */
if (meta.len > *inout_cert_len) {
if (meta.len > buf_len) {
return WH_ERROR_BUFFER_SIZE;
}

/* Clamp the input length to the actual length of the certificate. This will
* be reflected back to the user on length mismatch failure */
*inout_cert_len = meta.len;

return wh_Nvm_Read(server->nvm, id, 0, meta.len, cert);
}

Expand Down Expand Up @@ -1007,7 +1009,13 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
else {
rc = wh_Server_CertReadTrusted(server, req.id,
cert_data, &cert_len);
resp.cert_len = cert_len;
/* These are the only outcomes that resolve a length.
* Any other error staged no certificate, so there is
* no length to describe it with */
if ((rc == WH_ERROR_OK) ||
(rc == WH_ERROR_BUFFER_SIZE)) {
resp.cert_len = cert_len;
}
}
}

Expand All @@ -1018,7 +1026,12 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
/* Convert the response struct */
wh_MessageCert_TranslateReadTrustedResponse(
magic, &resp, (whMessageCert_ReadTrustedResponse*)resp_packet);
*out_resp_size = sizeof(resp) + resp.cert_len;
/* Certificate data is only staged on success. Sizing the response
* to cert_len otherwise would transmit unwritten packet bytes */
*out_resp_size = sizeof(resp);
if (rc == WH_ERROR_OK) {
*out_resp_size += resp.cert_len;
}
}; break;

case WH_MESSAGE_CERT_ACTION_VERIFY: {
Expand Down Expand Up @@ -1294,7 +1307,9 @@ int wh_Server_HandleCertRequest(whServerContext* server, uint16_t magic,
resp.rc = WH_ERROR_ACCESS;
}
else {
/* Clamp cert_len to actual stored length */
/* The callee reports the stored length back into
* cert_len, but SimpleResponse has no field to
* return it, so the client cannot learn it here */
cert_len = req.cert_len;
resp.rc = wh_Server_CertReadTrusted(
server, req.id, cert_data, &cert_len);
Expand Down
67 changes: 67 additions & 0 deletions test-refactor/client-server/wh_test_client_certs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,11 @@
&& !defined(WOLFHSM_CFG_NO_CRYPTO)

#include <stdint.h>
#include <string.h>

#include "wolfhsm/wh_error.h"
#include "wolfhsm/wh_client.h"
#include "wolfhsm/wh_message_cert.h"

#include "wh_test_common.h"
#include "wh_test_list.h"
Expand Down Expand Up @@ -83,9 +85,74 @@ static int _whTest_CertReadTrustedSmallBuffer(whClientContext* ctx)
}


/* Only with DMA does the certificate limit drop below the comm buffer, which
* is what makes a stored-but-unreadable certificate possible */
#if defined(WOLFHSM_CFG_DMA)

/* Mirrors the staging clamp in the non-DMA READTRUSTED handler */
#define WH_TEST_CERT_TRANSPORT_LEN \
(WOLFHSM_CFG_COMM_DATA_LEN - sizeof(whMessageCert_ReadTrustedResponse))
#define WH_TEST_CERT_STAGED_LEN \
((WOLFHSM_CFG_MAX_CERT_SIZE > WH_TEST_CERT_TRANSPORT_LEN) \
? WH_TEST_CERT_TRANSPORT_LEN \
: WOLFHSM_CFG_MAX_CERT_SIZE)

/* The add paths and the read clamp have different bounds, so a certificate
* can be stored that a non-DMA read cannot return */
static int _whTest_CertReadTrustedOverClamp(whClientContext* ctx)
{
static uint8_t big_cert[WH_TEST_CERT_STAGED_LEN + 1];
static uint8_t read_buf[WOLFHSM_CFG_COMM_DATA_LEN];
const whNvmId cert_id = 104;
const uint32_t stored_len = (uint32_t)WH_TEST_CERT_STAGED_LEN + 1;
const uint32_t inline_max =
WOLFHSM_CFG_COMM_DATA_LEN - sizeof(whMessageCert_AddTrustedRequest);
uint32_t cert_len = 0;
int32_t out_rc = 0;

/* One of the two add paths must be able to hold it, or there is no window
* to probe and the config needs a different size than STAGED_LEN + 1 */
WH_TEST_ASSERT_RETURN((stored_len <= inline_max) ||
(stored_len <= WOLFHSM_CFG_MAX_CERT_SIZE));

memset(big_cert, 0x5A, sizeof(big_cert));

/* A large comm buffer lets the inline add outrun the clamp; otherwise the
* certificate limit is the larger bound and only DMA can */
if (stored_len <= inline_max) {
WH_TEST_RETURN_ON_FAIL(wh_Client_CertAddTrusted(
ctx, cert_id, WH_NVM_ACCESS_ANY, WH_NVM_FLAGS_NONE, NULL, 0,
big_cert, stored_len, &out_rc));
}
else {
WH_TEST_RETURN_ON_FAIL(wh_Client_CertAddTrustedDma(
ctx, cert_id, WH_NVM_ACCESS_ANY, WH_NVM_FLAGS_NONE, NULL, 0,
big_cert, stored_len, &out_rc));
}
WH_TEST_ASSERT_RETURN(out_rc == WH_ERROR_OK);

cert_len = sizeof(read_buf);
WH_TEST_RETURN_ON_FAIL(wh_Client_CertReadTrusted(
ctx, cert_id, read_buf, &cert_len, &out_rc));
WH_TEST_ASSERT_RETURN(out_rc == WH_ERROR_BUFFER_SIZE);
/* The stored size, not the clamp the server staged with */
WH_TEST_ASSERT_RETURN(cert_len == stored_len);

WH_TEST_RETURN_ON_FAIL(wh_Client_CertEraseTrusted(ctx, cert_id, &out_rc));
WH_TEST_ASSERT_RETURN(out_rc == WH_ERROR_OK);

return WH_ERROR_OK;
}

#endif /* WOLFHSM_CFG_DMA */


int whTest_ClientCerts(whClientContext* ctx)
{
WH_TEST_RETURN_ON_FAIL(_whTest_CertReadTrustedSmallBuffer(ctx));
#if defined(WOLFHSM_CFG_DMA)
WH_TEST_RETURN_ON_FAIL(_whTest_CertReadTrustedOverClamp(ctx));
#endif

return WH_ERROR_OK;
}
Expand Down
170 changes: 170 additions & 0 deletions test-refactor/server/wh_test_cert_readtrusted.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
/*
* Copyright (C) 2026 wolfSSL Inc.
*
* This file is part of wolfHSM.
*
* wolfHSM is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 3 of the License, or
* (at your option) any later version.
*
* wolfHSM is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with wolfHSM. If not, see <http://www.gnu.org/licenses/>.
*/
/*
* test-refactor/server/wh_test_cert_readtrusted.c
*
* READTRUSTED response sizing over the comm buffer. Drives the
* request handler directly against the shared server context.
*/

#include "wolfhsm/wh_settings.h"

#if defined(WOLFHSM_CFG_CERTIFICATE_MANAGER) \
&& !defined(WOLFHSM_CFG_NO_CRYPTO)

#include <stdint.h>
#include <string.h>

#include "wolfhsm/wh_error.h"
#include "wolfhsm/wh_comm.h"
#include "wolfhsm/wh_message_cert.h"
#include "wolfhsm/wh_server.h"
#include "wolfhsm/wh_server_cert.h"

#include "wh_test_common.h"
#include "wh_test_list.h"

/* wh_test_cert_data.h defines rather than declares, so it can only be
* included once across the suite */
extern const unsigned char ROOT_A_CERT[];
extern const size_t ROOT_A_CERT_len;

/* Mirrors the staging clamp in the non-DMA READTRUSTED handler. Reading at
* both STAGED_LEN and STAGED_LEN + 1 pins the clamp from either side */
#define WH_TEST_CERT_TRANSPORT_LEN \
(WOLFHSM_CFG_COMM_DATA_LEN - sizeof(whMessageCert_ReadTrustedResponse))
#define WH_TEST_CERT_STAGED_LEN \
((WOLFHSM_CFG_MAX_CERT_SIZE > WH_TEST_CERT_TRANSPORT_LEN) \
? WH_TEST_CERT_TRANSPORT_LEN \
: WOLFHSM_CFG_MAX_CERT_SIZE)

/* An unstageable certificate must report the stored size and no payload */
static int _whTest_CertReadTrustedOversized(whServerContext* server)
{
whMessageCert_ReadTrustedRequest req[1] = {{0}};
union {
whMessageCert_ReadTrustedResponse resp;
uint8_t bytes[WOLFHSM_CFG_COMM_DATA_LEN];
} respPkt;
/* Static filler: an automatic copy would double this stack frame on the
* embedded targets these suites also run on */
static uint8_t oversized_cert[WH_TEST_CERT_STAGED_LEN + 1];
const whNvmId certId = 20;
const uint32_t oversized_len = (uint32_t)WH_TEST_CERT_STAGED_LEN + 1;
uint16_t resp_size = 0;
int handler_rc;
uint32_t i;

/* One byte too large to stage */
memset(oversized_cert, 0x5A, sizeof(oversized_cert));
WH_TEST_RETURN_ON_FAIL(wh_Server_CertAddTrusted(
server, certId, WH_NVM_ACCESS_ANY, WH_NVM_FLAGS_NONE, NULL, 0,
oversized_cert, oversized_len));

/* Poison makes bytes the handler never staged detectable */
memset(respPkt.bytes, 0xA5, sizeof(respPkt.bytes));
req->id = certId;

/* The server transmits regardless of this return, so out_resp_size is
* what actually reaches the client */
handler_rc = wh_Server_HandleCertRequest(
server, WH_COMM_MAGIC_NATIVE, WH_MESSAGE_CERT_ACTION_READTRUSTED, 0,
(uint16_t)sizeof(*req), req, &resp_size, respPkt.bytes);

WH_TEST_ASSERT_RETURN(handler_rc == WH_ERROR_BUFFER_SIZE);
WH_TEST_ASSERT_RETURN(respPkt.resp.rc == WH_ERROR_BUFFER_SIZE);
/* Reports the true size, which no non-DMA response can carry */
WH_TEST_ASSERT_RETURN(respPkt.resp.cert_len == oversized_len);
WH_TEST_ASSERT_RETURN(resp_size == sizeof(respPkt.resp));

for (i = sizeof(respPkt.resp); i < sizeof(respPkt.bytes); i++) {
WH_TEST_ASSERT_RETURN(respPkt.bytes[i] == 0xA5);
}

WH_TEST_RETURN_ON_FAIL(wh_Server_CertEraseTrusted(server, certId));

/* The largest stageable certificate must still read back whole */
WH_TEST_RETURN_ON_FAIL(wh_Server_CertAddTrusted(
server, certId, WH_NVM_ACCESS_ANY, WH_NVM_FLAGS_NONE, NULL, 0,
oversized_cert, oversized_len - 1));

memset(respPkt.bytes, 0xA5, sizeof(respPkt.bytes));
req->id = certId;

WH_TEST_RETURN_ON_FAIL(wh_Server_HandleCertRequest(
server, WH_COMM_MAGIC_NATIVE, WH_MESSAGE_CERT_ACTION_READTRUSTED, 0,
(uint16_t)sizeof(*req), req, &resp_size, respPkt.bytes));

WH_TEST_ASSERT_RETURN(respPkt.resp.rc == WH_ERROR_OK);
WH_TEST_ASSERT_RETURN(respPkt.resp.cert_len == oversized_len - 1);
WH_TEST_ASSERT_RETURN(resp_size == sizeof(respPkt.resp) + oversized_len - 1);

for (i = 0; i < oversized_len - 1; i++) {
WH_TEST_ASSERT_RETURN(respPkt.bytes[sizeof(respPkt.resp) + i] == 0x5A);
}

return wh_Server_CertEraseTrusted(server, certId);
}

/* A rejected read stages no certificate, so the response must carry no length
* and no payload */
static int _whTest_CertReadTrustedDenied(whServerContext* server)
{
whMessageCert_ReadTrustedRequest req[1] = {{0}};
union {
whMessageCert_ReadTrustedResponse resp;
uint8_t bytes[WOLFHSM_CFG_COMM_DATA_LEN];
} respPkt;
const whNvmId certId = 21;
uint16_t resp_size = 0;
int handler_rc;

WH_TEST_RETURN_ON_FAIL(wh_Server_CertAddTrusted(
server, certId, WH_NVM_ACCESS_ANY, WH_NVM_FLAGS_NONEXPORTABLE, NULL, 0,
ROOT_A_CERT, ROOT_A_CERT_len));

memset(respPkt.bytes, 0xA5, sizeof(respPkt.bytes));
req->id = certId;

handler_rc = wh_Server_HandleCertRequest(
server, WH_COMM_MAGIC_NATIVE, WH_MESSAGE_CERT_ACTION_READTRUSTED, 0,
(uint16_t)sizeof(*req), req, &resp_size, respPkt.bytes);

WH_TEST_ASSERT_RETURN(handler_rc == WH_ERROR_ACCESS);
WH_TEST_ASSERT_RETURN(respPkt.resp.rc == WH_ERROR_ACCESS);
WH_TEST_ASSERT_RETURN(respPkt.resp.cert_len == 0);
WH_TEST_ASSERT_RETURN(resp_size == sizeof(respPkt.resp));

return wh_Server_CertEraseTrusted(server, certId);
}

/* READTRUSTED response sizing over the comm buffer */
int whTest_CertReadTrusted(whServerContext* ctx)
{
whServerContext* server = (whServerContext*)ctx;

WH_TEST_RETURN_ON_FAIL(wh_Server_CertInit(server));
WH_TEST_RETURN_ON_FAIL(_whTest_CertReadTrustedOversized(server));
WH_TEST_RETURN_ON_FAIL(_whTest_CertReadTrustedDenied(server));

return 0;
}


#endif /* WOLFHSM_CFG_CERTIFICATE_MANAGER && !WOLFHSM_CFG_NO_CRYPTO */
2 changes: 2 additions & 0 deletions test-refactor/wh_test_list.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ WH_TEST_DECL(whTest_Lock);
WH_TEST_DECL(whTest_CertVerify);
WH_TEST_DECL(whTest_CertNvmPolicy);
WH_TEST_DECL(whTest_CertReadRejectsServerOnly);
WH_TEST_DECL(whTest_CertReadTrusted);
WH_TEST_DECL(whTest_HwKeystoreServer);
WH_TEST_DECL(whTest_ServerImgMgr);
WH_TEST_DECL(whTest_NvmOptional);
Expand Down Expand Up @@ -115,6 +116,7 @@ const whTestCase whTestsServer[] = {
{ "whTest_CertNvmPolicy", whTest_CertNvmPolicy },
{ "whTest_CertReadRejectsServerOnly", whTest_CertReadRejectsServerOnly },
{ "whTest_ServerImgMgr", whTest_ServerImgMgr },
{ "whTest_CertReadTrusted", whTest_CertReadTrusted },
{ "whTest_NvmOptional", whTest_NvmOptional },
{ "whTest_SheMasterEcuKeyFallback", whTest_SheMasterEcuKeyFallback },
{ "whTest_SheReqSizeChecking", whTest_SheReqSizeChecking },
Expand Down
Loading