Implement SHE GET_ID command with client side request, server side handler, and tests

[AlexLanzano]

No description provided.

[Copilot]

Pull request overview

This PR adds support for the AUTOSAR SHE CMD_GET_ID command across the wolfHSM client/server messaging layer, exposing a client API to request module identity and implementing the server-side handler to return UID, status (SREG), and an identity CMAC.

Changes:

• Added WH_SHE_GET_ID to the SHE action enum and introduced GET_ID request/response message types plus translation helpers.
• Implemented client APIs (wh_Client_SheGetId*) and server handler (_GetId) including SREG composition refactoring via _BuildSreg.
• Added end-to-end and server-direct tests covering MAC verification, truncated request handling, empty MASTER_ECU_KEY fallback-to-zero-key behavior, and pre-secure-boot allowance; updated docs accordingly.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
wolfhsm/wh_message.h	Adds WH_SHE_GET_ID action identifier.
wolfhsm/wh_message_she.h	Defines GET_ID request/response structs and translation function prototypes.
wolfhsm/wh_client_she.h	Adds public client API declarations and documentation for CMD_GET_ID.
src/wh_server_she.c	Implements server-side GET_ID handler, refactors SREG building into _BuildSreg, and updates state-gating/dispatch.
src/wh_message_she.c	Implements GET_ID request/response translation functions.
src/wh_client_she.c	Implements client request/response helpers and blocking wh_Client_SheGetId.
test/wh_test_she.c	Adds end-to-end GET_ID validation and server-direct GET_ID cases (empty key + pre-boot) plus req-size truncation test.
test/wh_test_check_struct_padding.c	Includes new GET_ID message structs in padding/struct compilation coverage.
test-refactor/server/wh_test_she_server.c	Adds GET_ID truncated request test in refactor suite.
test-refactor/misc/wh_test_check_struct_padding.c	Includes new GET_ID message structs in refactor padding coverage.
test-refactor/client-server/wh_test_she.c	Adds end-to-end GET_ID identity+MAC verification in refactor client-server test.
docs/src/5-Features.md	Documents wh_Client_SheGetId in the SHE client API feature list.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.

Suppressed comments (3)

test-refactor/misc/wh_test_she_uid_cb.c:148

• Casting the byte buffer resp_packet to int32_t* for writes/reads can cause unaligned accesses (UB) on architectures that require 32-bit alignment. Use memcpy to/from a local int32_t instead of pointer casts when seeding/reading the response rc.

    *((int32_t*)resp_packet) = WH_SHE_ERC_NO_ERROR;
    ret = wh_Server_HandleSheRequest(server, WH_COMM_MAGIC_NATIVE, action,
                                     req_size, req_packet, &resp_size,
                                     resp_packet);
    if (ret != 0 || resp_size < sizeof(int32_t)) {
        return WH_SHE_ERC_GENERAL_ERROR;
    }
    return *((const int32_t*)resp_packet);

wolfhsm/wh_server_she.h:47

• The getUidCb comment notes it is called on every gated SHE request, but it doesn’t mention that this happens inside the state gate (before any NVM lock is taken). Callers implementing a backend in NVM/flash may need this detail to avoid unsafe assumptions about locking/thread-safety.

/* Reads WH_SHE_UID_SZ bytes into outUid. Returns 0, WH_ERROR_NOTFOUND if no UID
 * is provisioned, or another wolfHSM error. Called on every gated SHE request,
 * so it must be cheap and idempotent. */

docs/src/5-Features.md:766

• The secure-boot gate description just above still says only CMD_GET_STATUS and CMD_SET_UID are permitted before secure boot succeeds, but the server gate now also whitelists CMD_GET_ID (see wh_server_she.c). The docs should reflect that GET_ID is allowed in all states.

The same gate also enforces that a UID has been provisioned. When [UID storage callbacks](#she-uid-storage) are installed, that check queries the integrator's store, and a store that reports a failure causes every command except `CMD_GET_STATUS` to return `WH_SHE_ERC_MEMORY_FAILURE`. Status stays readable in every case, per the spec.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #450

Scan targets checked: wolfhsm-core-bugs, wolfhsm-crypto-bugs, wolfhsm-src
Findings: 1

Medium (1)

SHE UID callback invoked without NVM lock for SET_UID and the pre-dispatch state gate

File: src/wh_server_she.c:2057
Function: wh_Server_HandleSheRequest
Category: NV storage vulnerabilities

_ReportInvalidSheState calls _UidIsProvisioned/getUidCb for nearly every action before any lock is taken, and the WH_SHE_SET_UID case runs _SetUid (which can call setUidCb) with no WH_SERVER_NVM_LOCK, unlike every other handler that touches the keystore/NVM. Since docs explicitly endorse NVM-backed UID callbacks, this permits concurrent unsynchronized NVM access in WOLFHSM_CFG_THREADSAFE builds.

Recommendation: Wrap the _ReportInvalidSheState call and the WH_SHE_SET_UID case with WH_SERVER_NVM_LOCK/WH_SERVER_NVM_UNLOCK like the other handlers.

Referenced code: src/wh_server_she.c:2057-2063 (7 lines)

---

This review was generated automatically by Fenrir. Findings are non-blocking.

[bigbrett]

Run git-clang-format main to reformat the diff plz

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/wh_server_she.c:189

• _GetUid() will NULL-dereference outUid if it is ever called with outUid == NULL and the UID callback reports any error (ret != 0), because it unconditionally does memset(outUid, ...). Since the callback contract explicitly supports NULL outUid probes, it’s easy for a future refactor (or a mistaken internal caller) to trigger a crash here.

/* Reads the UID into outUid. Returns WH_ERROR_NOTFOUND if unprovisioned. */
static int _GetUid(whServerContext* server, uint8_t* outUid)
{
    whServerSheContext* she = server->she;
    int                 ret;

    if (she->getUidCb != NULL) {
        ret = she->getUidCb(she->uidCtx, outUid);
        if (ret != 0) {
            memset(outUid, 0, WH_SHE_UID_SZ);
        }
        return ret;

src/wh_server_she.c:2211

• wh_Server_SheSetUidCb() accepts setCb != NULL with getCb == NULL, but that configuration can never be used: _StoreUid() only consults setUidCb when getUidCb is non-NULL. This can silently drop persistence (UID appears set but won’t survive reset), which is a sharp edge for integrators wiring in UID storage.

int wh_Server_SheSetUidCb(whServerContext* server, whServerSheGetUidCb getCb,
                          whServerSheSetUidCb setCb, void* ctx)
{
    /* No NULL check on the callbacks, since both are optional and always NULL
     * checked before they are called */
    if ((server == NULL) || (server->she == NULL)) {
        return WH_ERROR_BADARGS;
    }

    server->she->getUidCb = getCb;
    server->she->setUidCb = setCb;
    server->she->uidCtx   = ctx;

src/wh_server.c:106

• wh_Server_Init() copies sheConfig->setUidCb/uidCtx even when sheConfig->getUidCb is NULL. Since the server ignores setUidCb unless getUidCb is installed (_StoreUid() checks getUidCb first), this allows an easy-to-misconfigure state where a caller thinks UID persistence is enabled but it will never be used.

    if (server->she != NULL) {
        if (config->sheConfig != NULL) {
            server->she->getUidCb = config->sheConfig->getUidCb;
            server->she->setUidCb = config->sheConfig->setUidCb;
            server->she->uidCtx   = config->sheConfig->uidCtx;
        }
        else {
            server->she->getUidCb = NULL;
            server->she->setUidCb = NULL;
            server->she->uidCtx   = NULL;
        }