[twcook86]

This adds a new device attestation example config (lpc55s69-tz-psa.config) to wolfBoot, to demonstrate and test the new lpc55s69 hw puf support added in wolfSSL/wolfssl#11700

[dgarske]

I see this enabling the wolfCrypt HW PUF options, but I don't see anywhere it actually gets used in wolfBoot? Or even called in wolfCrypt. How's this supposed to work end to end?

[danielinux]

This adds the useful build plumbing for LPC55 PUF support, but it does not yet connect the HW PUF to the attestation identity.

What we actually need here is the HW PUF to become the UID/UDS source for the boot attestation demo. Today the attestation stack gets its identity through hal_uds_derive_key() / hal_attestation_get_ueid() in dice.c.

This PR still needs:

• a new LPC55 attestation config enabling PSA attestation demo options
• LPC55 hal support that rebuilds identity from PUF exposing it via hal_uds_derive_key / get_ueid. Check existing generic software managed internal flash PUF support in STM32H5 existing support for HW PUF in mcxn.c via those API for how to implement
• The test app showing that the PSA attestation token is actually produced from the PUF-derived identity wired inside wolfBoot.

[twcook86]

I see this enabling the wolfCrypt HW PUF options, but I don't see anywhere it actually gets used in wolfBoot? Or even called in wolfCrypt. How's this supposed to work end to end?

lpc55s69-hwpuf.config will compile in wolfssl test.c, which tests the hwpuf. But, you have to turn on HWPUF=1 in that config file, and point your wolfssl to wolfSSL/wolfssl#10584

[danielinux]

Depends on wolfSSL/wolfssl#10584

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #787

Scan targets checked: wolfboot-src, wolfboot-bugs

Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

[danielinux]

This is now properly demonstrating HW PUF on LPC55S69 in DICE.

LGTM after wolfSSL/wolfssl#10584 is merged.

[dgarske]

TustZone build is failing: https://github.com/wolfSSL/wolfBoot/actions/runs/35663229749/job/106543075792?pr=787

[danielinux]

Verified support for hw puf by adding tests including TZEN and attestation on this target. Off to @dgarske for final review / merge.

[aidangarske]

Skoll Multi-Scan Review

Modes: review + review-security

Overall recommendation: REQUEST_CHANGES
Findings: 16 total — 16 posted, 0 skipped
16 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [High] [review+review-security] A cut-short PUF provisioning write is accepted as valid, so attestation can be broken for good — hal/lpc55s69.c:115-133
• [High] [review] PUF peripheral clock is never enabled although driver clock control is disabled — hal/lpc55s69.c:468-470
• [Medium] [review+review-security] Under TZEN, the non-secure world can reach the HW PUF, which is now the root of attestation trust — hal/lpc55s69.c:411
• [Medium] [review] HWPUF emulator test runs a single boot, so the stored-record path and identity persistence are never exercised — .github/workflows/trustzone-emulator-tests.yml:194-211
• [Medium] [review] lpc55s69-hwpuf.config ships with HWPUF disabled and, even when enabled, does not exercise the PUF — config/examples/lpc55s69-hwpuf.config:34-44
• [Medium] [review] hal_attestation_get_ueid duplicates dice.c, adds an extra PUF cycle per token, and can mismatch the UDS in FORTEST builds — hal/lpc55s69.c:230-260
• [Medium] [review+review-security] Hard-coded PUF record address 0x80000 is not checked against the partition layout — hal/lpc55s69.c:95-96
• [Low] [review-security] With HWPUF=1, FORTEST still wins: UDS comes from the public UID while UEID comes from the PUF — hal/lpc55s69.c:364-375
• [Low] [review-security] hal_attestation_get_ueid leaves the UDS-derived secret in the SHA-256 state on the stack — hal/lpc55s69.c:232-259
• [Low] [review+review-security] Static PUF activation/key code buffer is not cleared on several return paths — hal/lpc55s69.c:146-149
• [Low] [review+review-security] uds_from_uid leaves the derived UDS digest on the stack on success — hal/lpc55s69.c:349
• [Low] [review] Comments describe a UDS fallback that does not exist — hal/lpc55s69.c:363
• [Info] [review] cryptocb.o linked and cryptocb.h included with no WOLF_CRYPTO_CB user — arch.mk:2156-2162
• [Info] [review] AHBSC programmed through magic offsets on the non-secure alias — hal/lpc55s69.c:384-392
• [Info] [review] wolfCrypt_Init() now runs in hal_init for every lpc55s69 wolfBoot build, return value ignored — hal/lpc55s69.c:482-484
• [Info] [review] Unrelated whitespace change in update_flash.c — src/update_flash.c:1783

Review generated by Skoll

[aidangarske]

Still some things to address

[aidangarske]

A few suggestions since this overlaps with work I already did. we can merge as is and I can take it from here also.

[aidangarske]

Skoll Multi-Scan Review

Modes: review + review-security

Overall recommendation: COMMENT
Findings: 4 total — 3 posted, 1 skipped
3 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [Medium] [review-security] PUF provisioning record lacks required word alignment — hal/lpc55s69.c:145
• [Medium] [review+review-security] PUF persisted-record path and identity stability are not tested — .github/workflows/trustzone-emulator-tests.yml:207-221
• [Medium] [review] Build the new non-TrustZone HWPUF branch in CI — .github/workflows/test-configs.yml:181-193

Skipped findings

• [Low] Benchmark build lost the declaration for CLOCK_GetCoreSysClkFreq

Review generated by Skoll

stale