Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 6 additions & 0 deletions port/posix/posix_transport_shm.c
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,9 @@ int posixTransportShm_ClientStaticMemDmaCallback(
else if (oper == WH_DMA_OPER_CLIENT_READ_POST) {
if (isInDma == 0) {
uint8_t* ptr = (uint8_t*)dmaPtr + (uintptr_t)*xformedCliAddr;
/* Scrub key material before freeing. len is bounded by the temp
* buffer's XMALLOC, well within uint32_t for this transport. */
wh_Utils_ForceZero(ptr, (uint32_t)len);
XFREE(ptr, heap, DYNAMIC_TYPE_TMP_BUFFER);
}
}
Expand All @@ -611,6 +614,9 @@ int posixTransportShm_ClientStaticMemDmaCallback(
uint8_t* ptr = (uint8_t*)dmaPtr + (uintptr_t)*xformedCliAddr;
memcpy((void*)clientAddr, ptr,
len); /* copy results of what server wrote */
/* Scrub key material before freeing. len is bounded by the temp
* buffer's XMALLOC, well within uint32_t for this transport. */
wh_Utils_ForceZero(ptr, (uint32_t)len);
XFREE(ptr, heap, DYNAMIC_TYPE_TMP_BUFFER);
}
}
Expand Down
43 changes: 33 additions & 10 deletions src/wh_auth_base.c
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,29 @@ static int wh_Auth_BaseHashPin(const void* pin, uint16_t pin_len,
#endif /* WOLFHSM_CFG_NO_CRYPTO */
}

/* Constant-time credential compare: the byte compare and length check combine
* without an early-out on mismatch, so timing can't reveal the stored length.
* (The oversized-length guard below returns early, but only on the supplied
* length, never the stored one.) Returns 0 on match. */
static int wh_Auth_BaseCredsCompare(const unsigned char* stored,
uint16_t stored_len, const void* supplied,
uint16_t supplied_len)
{
int diff;

/* Oversized supplied length can't match and must not read past the
* buffer. */
if (supplied_len > WH_AUTH_BASE_MAX_CREDENTIALS_LEN) {
return 1;
}

diff = wh_Utils_ConstantCompare(stored, (const uint8_t*)supplied,
supplied_len);
/* Fold in the length-equality check without branching. */
diff |= (int)((unsigned int)stored_len ^ (unsigned int)supplied_len);
return diff;
}

static whAuthBase_User* wh_Auth_BaseCheckPin(const char* username,
const void* auth_data,
uint16_t auth_data_len)
Expand All @@ -133,8 +156,8 @@ static whAuthBase_User* wh_Auth_BaseCheckPin(const char* username,

found_user = wh_Auth_BaseFindUser(username);
if (found_user != NULL && found_user->method == WH_AUTH_METHOD_PIN &&
found_user->credentials_len == authCheck_len &&
wh_Utils_ConstantCompare(found_user->credentials, authCheck,
wh_Auth_BaseCredsCompare(found_user->credentials,
found_user->credentials_len, authCheck,
authCheck_len) == 0) {
ret = found_user;
}
Expand Down Expand Up @@ -518,19 +541,19 @@ int wh_Auth_BaseUserSetCredentials(void* context, uint16_t current_user_id,
}
wh_Utils_ForceZero(hash, sizeof(hash));
#else
/* When crypto is disabled, compare PINs directly */
if (user->credentials_len != current_credentials_len ||
wh_Utils_ConstantCompare(user->credentials, current_credentials,
current_credentials_len) != 0) {
/* When crypto is disabled, compare PINs directly (constant time) */
if (wh_Auth_BaseCredsCompare(
user->credentials, user->credentials_len,
current_credentials, current_credentials_len) != 0) {
return WH_ERROR_ACCESS;
}
#endif /* WOLFHSM_CFG_NO_CRYPTO */
}
else {
/* For non-PIN methods, compare as-is */
if (user->credentials_len != current_credentials_len ||
wh_Utils_ConstantCompare(user->credentials, current_credentials,
current_credentials_len) != 0) {
/* For non-PIN methods, compare as-is (constant time) */
if (wh_Auth_BaseCredsCompare(
user->credentials, user->credentials_len,
current_credentials, current_credentials_len) != 0) {
return WH_ERROR_ACCESS;
}
}
Expand Down
7 changes: 4 additions & 3 deletions src/wh_client_she.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,10 @@ int wh_Client_ShePreProgramKey(whClientContext* c, whNvmId keyId,

/* Create a key with 0 counter */
wh_She_Meta2Label(0, flags, label);
ret = wh_Client_NvmAddObject(c,
WH_MAKE_KEYID(WH_KEYTYPE_SHE, c->comm->client_id, keyId),
0, 0, sizeof(label), label, keySz, key, (int32_t*)&outRc);
ret = wh_Client_NvmAddObject(
c, WH_MAKE_KEYID(WH_KEYTYPE_SHE, c->comm->client_id, keyId),
WH_NVM_ACCESS_ANY, 0, sizeof(label), label, keySz, key,
(int32_t*)&outRc);
if (ret == 0)
ret = outRc;
return ret;
Expand Down
3 changes: 2 additions & 1 deletion src/wh_server_counter.c
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,8 @@ int wh_Server_HandleCounter(whServerContext* server, uint16_t magic,
} break;

default:
ret = WH_ERROR_BADARGS;
*out_resp_size = 0;
ret = WH_ERROR_BADARGS;
break;
}

Expand Down
26 changes: 15 additions & 11 deletions src/wh_server_she.c
Original file line number Diff line number Diff line change
Expand Up @@ -606,6 +606,7 @@ static int _LoadKey(whServerContext* server, uint16_t magic, uint16_t req_size,
if (ret == 0) {
meta->id = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
_PopId(req.messageOne));
meta->access = WH_NVM_ACCESS_ANY;
she_meta_flags = _PopFlags(req.messageTwo);
she_meta_count = wh_Utils_ntohl(msg_counter_val) >> 4;
/* Update the meta label with new values */
Expand Down Expand Up @@ -725,9 +726,10 @@ static int _LoadPlainKey(whServerContext* server, uint16_t magic,
&req);
}

meta->id = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
WH_SHE_RAM_KEY_ID);
meta->len = WH_SHE_KEY_SZ;
meta->id = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
WH_SHE_RAM_KEY_ID);
meta->access = WH_NVM_ACCESS_ANY;
meta->len = WH_SHE_KEY_SZ;

/* cache if ram key, overwrite otherwise */
if (ret == 0) {
Expand Down Expand Up @@ -984,10 +986,11 @@ static int _InitRnd(whServerContext* server, uint16_t magic, uint16_t req_size,
wc_AesFree(server->she->sheAes);
/* save PRNG_SEED, i */
if (ret == 0) {
meta->id = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
WH_SHE_PRNG_SEED_ID);
meta->len = WH_SHE_KEY_SZ;
ret = wh_Nvm_AddObject(server->nvm, meta, meta->len, cmacOutput);
meta->id = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
WH_SHE_PRNG_SEED_ID);
meta->access = WH_NVM_ACCESS_ANY;
meta->len = WH_SHE_KEY_SZ;
ret = wh_Nvm_AddObject(server->nvm, meta, meta->len, cmacOutput);
if (ret != 0) {
ret = WH_SHE_ERC_KEY_UPDATE_ERROR;
}
Expand Down Expand Up @@ -1121,10 +1124,11 @@ static int _ExtendSeed(whServerContext* server, uint16_t magic,
}
/* save PRNG_SEED */
if (ret == 0) {
meta->id = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
WH_SHE_PRNG_SEED_ID);
meta->len = WH_SHE_KEY_SZ;
ret = wh_Nvm_AddObject(server->nvm, meta, meta->len, kdfInput);
meta->id = WH_MAKE_KEYID(WH_KEYTYPE_SHE, server->comm->client_id,
WH_SHE_PRNG_SEED_ID);
meta->access = WH_NVM_ACCESS_ANY;
meta->len = WH_SHE_KEY_SZ;
ret = wh_Nvm_AddObject(server->nvm, meta, meta->len, kdfInput);
if (ret != 0) {
ret = WH_SHE_ERC_KEY_UPDATE_ERROR;
}
Expand Down
47 changes: 47 additions & 0 deletions test/wh_test_auth.c
Original file line number Diff line number Diff line change
Expand Up @@ -637,6 +637,24 @@ int whTest_AuthLogin(whClientContext* client)
server_rc != WH_ERROR_OK);
WH_TEST_ASSERT_RETURN(user_id == WH_USER_ID_INVALID);

/* Wrong-length prefixes of the admin PIN "1234" must be rejected
* (report 5458): "123" is a shorter prefix, "12345" a longer one. */
WH_TEST_PRINT(" Test: Login rejects wrong-length credential\n");
server_rc = 0;
user_id = WH_USER_ID_INVALID;
WH_TEST_RETURN_ON_FAIL(_whTest_Auth_LoginOp(client, WH_AUTH_METHOD_PIN,
TEST_ADMIN_USERNAME, "123", 3,
&server_rc, &user_id));
WH_TEST_ASSERT_RETURN(server_rc == WH_AUTH_LOGIN_FAILED);
WH_TEST_ASSERT_RETURN(user_id == WH_USER_ID_INVALID);
server_rc = 0;
user_id = WH_USER_ID_INVALID;
WH_TEST_RETURN_ON_FAIL(_whTest_Auth_LoginOp(client, WH_AUTH_METHOD_PIN,
TEST_ADMIN_USERNAME, "12345", 5,
&server_rc, &user_id));
WH_TEST_ASSERT_RETURN(server_rc == WH_AUTH_LOGIN_FAILED);
WH_TEST_ASSERT_RETURN(user_id == WH_USER_ID_INVALID);

/* Test 2: Login with valid credentials - use blocking version */
WH_TEST_PRINT(" Test: Login with valid credentials\n");
server_rc = 0;
Expand Down Expand Up @@ -1412,6 +1430,35 @@ int whTest_AuthSetCredentials(whClientContext* client)
_whTest_Auth_DeleteUserByName(client, "victimcreduser");
}

/* Wrong-length current credential must be rejected like a wrong-value one
* (report 5458). testuser4's PIN is "newpass" (7); we are admin. */
WH_TEST_PRINT(" Test: Set-credentials rejects wrong current credential\n");
/* Wrong length (shorter) */
server_rc = 0;
WH_TEST_RETURN_ON_FAIL(
_whTest_Auth_UserSetCredsOp(client, user_id, WH_AUTH_METHOD_PIN,
"newpas", 6, "changed", 7, &server_rc));
WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_ACCESS);
/* Wrong length (longer, correct value as a prefix) */
server_rc = 0;
WH_TEST_RETURN_ON_FAIL(
_whTest_Auth_UserSetCredsOp(client, user_id, WH_AUTH_METHOD_PIN,
"newpassX", 8, "changed", 7, &server_rc));
WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_ACCESS);
/* Correct length, wrong value */
server_rc = 0;
WH_TEST_RETURN_ON_FAIL(
_whTest_Auth_UserSetCredsOp(client, user_id, WH_AUTH_METHOD_PIN,
"wrongpw", 7, "changed", 7, &server_rc));
WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_ACCESS);
/* Correct current credential still succeeds (behavior preserved); keep the
* PIN as "newpass" so the verification below continues to hold. */
server_rc = 0;
WH_TEST_RETURN_ON_FAIL(
_whTest_Auth_UserSetCredsOp(client, user_id, WH_AUTH_METHOD_PIN,
"newpass", 7, "newpass", 7, &server_rc));
WH_TEST_ASSERT_RETURN(server_rc == WH_ERROR_OK);

/* Verify new credentials work */
_whTest_Auth_LogoutOp(client, admin_id, &server_rc);
memset(&admin_perms, 0, sizeof(admin_perms));
Expand Down
20 changes: 20 additions & 0 deletions test/wh_test_clientserver.c
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,26 @@ static int _testClientCounter(whClientContext* client)
wh_Client_CounterRead(client, (whNvmId)i, &counter));
}

/* Report 2001: an invalid counter action hits the handler's default case,
* which must report a 0-length response instead of echoing the stale
* request-sized buffer back to the client. */
{
uint8_t reqbuf[8] = {0};
uint8_t respbuf[64] = {0};
uint16_t respGroup = 0;
uint16_t respAction = 0;
uint16_t respSz = 0xFFFF;

WH_TEST_RETURN_ON_FAIL(wh_Client_SendRequest(
client, WH_MESSAGE_GROUP_COUNTER, 0x7F, sizeof(reqbuf), reqbuf));
do {
rc = wh_Client_RecvResponse(client, &respGroup, &respAction,
&respSz, respbuf);
} while (rc == WH_ERROR_NOTREADY);
WH_TEST_ASSERT_RETURN(rc == WH_ERROR_OK);
WH_TEST_ASSERT_RETURN(respSz == 0);
}

/* Ensure NVM is empty */
WH_TEST_RETURN_ON_FAIL(rc = wh_Client_NvmGetAvailable(
client, &server_rc, &avail_size, &avail_objects,
Expand Down
Loading
Loading