Add optional NXP SE050 secure-element PKI key agreement - #11259
Open
cvaldess wants to merge 4 commits into
Open
Conversation
…03/X25519) Meshtastic already detects an NXP SE050 over I2C (ScanI2C::DeviceType::NXP_SE050), but nothing in the tree talks to it beyond that probe - no driver, no APDUs, no T=1oI2C past the interface reset. This adds the transport and everything Meshtastic's PKI path needs on top of it, gated entirely behind -D HAS_SE050 so it compiles to nothing on every board that doesn't define it. Covers, in order: block framing and CRC-16 for T=1oI2C with the interface reset that returns the ATR; APDU exchange (SELECT, GetVersion, GetRandom) with WTX handling; a PlatformSCP03 secure channel (the chip refuses key agreement outside an authenticated channel); and an X25519 identity, either generated in the chip (identityEnsure, private half never leaves it) or imported to mirror a key the caller already holds (identityImport), backing one ECDH key agreement (identityEcdh) at a time. Two things worth calling out for review: - Transaction buffers (SCP03 session state included) are object members, not stack locals. A single key agreement nests identityEcdh -> sessionApdu -> secureApdu -> transceive -> xfer, and as locals those buffers came to ~2.5 KB in one call chain - fine entered from setup()'s shallow frame, not fine entered from deep inside a packet handler on a board with a small task stack. reentered() (checked at every layer) keeps this safe against re-entry: a second operation half-way through the first would otherwise advance the SCP03 counter and overwrite buffers already in flight. - The probe sequence hunts an existing chip's saved identity before creating one: identityImport is idempotent (a matching key already in the object is a no-op, so the one NVM write happens once in the life of the node, not once per boot) and refuses to silently overwrite an object holding a different key - that has to be explicit (replaceStale), since deleting an identity is not something to do by accident. Validated on hardware against an SE050E2 (applet 7.2.0): ATR/SELECT/GetVersion/ GetRandom, SCP03 card-cryptogram match + EXTERNAL AUTHENTICATE, and ECDH producing the identical shared secret to software Curve25519, byte for byte.
Wires the driver added in the previous commit into Meshtastic's PKI path and enables it on pico2_w5500_e22, the one board in the tree that carries the chip. SE050CryptoEngine overrides only CryptoEngine::setDHPublicKey - the single method both encryptCurve25519 and decryptCurve25519 route through - so it's the one place that needs to know the agreement can run in hardware. Nothing else changes: the node's identity stays exactly where it is today (config.security.private_key, exportable/backup-able as always), mirrored into the chip on first use via identityImport. If the mirror import fails for any reason (chip absent, NVM write refused), setDHPublicKey falls straight back to CryptoEngine's own software Curve25519 - a node is never left unable to complete a key agreement because a secure element didn't cooperate. selfTest() (called from main.cpp, once, after the network is up) exists because a test that can't fail isn't a test: it explicitly checks the agreement actually ran against the chip (not just "software agreed with itself twice") before comparing anything, and reports FAILED rather than a false OK if the mirror never got established. -D SE050_REPLACE_MIRROR (commented out, meant to be flashed once and removed) is the escape hatch for an object in the chip holding a stale key from a prior identity - the driver won't overwrite it silently, and this is the explicit way to do it on purpose. main.cpp wiring: an ENA power-on-reset pulse before the I2C scan (the SE050 has no reset line and comes up however the last power cycle left it), probing + constructing the driver right after the scan if it found one, and the self-test call deliberately placed after Ethernet is up rather than next to NodeDB where the identity actually loads - if this ever hangs, the board still answers over the network instead of needing a bootloader pull. Verified on hardware: self-test passes with the agreement genuinely running in the chip, a real PKI direct message encrypted, sent, and received without resetting the node (previously one message was enough to reboot it - see the buffer-placement note in the driver commit), and the shared secret matches software bit for bit. Measured cost: ~64ms per agreement against the chip (SCP03-wrapped: AES-CMAC + AES-CBC on both command and response, plus a nested UserID session, over 100kHz I2C) vs ~27ms for the bundled software Curve25519 on the same RP2350 - slower, but a LoRa packet's own airtime dwarfs the difference, so it isn't the bottleneck it might look like on paper. Not in this change: the mirrored key stays exportable and backup-able exactly as it is today for every other board - nothing about identity semantics changes. A mode where the key is generated in the chip and never mirrored to config at all is a real possibility (identityEnsure() already supports it) but is a bigger product conversation - what happens to remote key rotation, to local identity backup, to a lost/replaced chip - not something to decide inside a driver PR.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughAdds conditional SE050 support with I2C transport, SCP03 secure-channel handling, X25519 identity and ECDH operations, boot-time probing, and integration with the custom crypto engine and self-test path. ChangesSE050 secure-element integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant setup
participant I2C
participant SE050
participant SE050CryptoEngine
setup->>I2C: scan and detect SE050
setup->>SE050: construct and probe
SE050->>SE050: open SCP03 and ensure identity
setup->>SE050CryptoEngine: run self-test
SE050CryptoEngine->>SE050: perform ECDH
SE050-->>SE050CryptoEngine: return shared secret
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
src/security/SE050CryptoEngine.cpp (2)
188-193: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUnchecked
static_caston the globalcrypto.If anything later reassigns
crypto(some platforms do swap engines), this cast is undefined behaviour. Keep a typed pointer to the instance instead.♻️ Proposed change
-CryptoEngine *crypto = new SE050CryptoEngine(); +static SE050CryptoEngine *se050Crypto = new SE050CryptoEngine(); +CryptoEngine *crypto = se050Crypto; void se050CryptoSelfTest() { - static_cast<SE050CryptoEngine *>(crypto)->selfTest(); + se050Crypto->selfTest(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/SE050CryptoEngine.cpp` around lines 188 - 193, Update se050CryptoSelfTest to use a dedicated typed SE050CryptoEngine pointer for the SE050CryptoEngine instance instead of static_casting the global crypto pointer; ensure the typed pointer remains valid if crypto is later reassigned or swapped.
97-129: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
mirrorReady()runs a multi-second import on the first PKI packet.The first
setDHPublicKeyafter boot pays session setup plus a potential NVM write inside packet handling. The self-test at Line 1205 ofsrc/main.cppwarms this up in practice, but only when both flags are set and only if it runs before the first PKI packet arrives. Worth making the warm-up explicit rather than incidental.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/SE050CryptoEngine.cpp` around lines 97 - 129, Add an explicit startup warm-up that invokes the SE050 mirror initialization before PKI packet handling, reusing mirrorReady() and guarding it with the same availability/configuration conditions used by the engine. Ensure the warm-up occurs after the private key is available and does not change mirrorReady()’s existing fallback behavior when initialization cannot proceed.src/security/SE050.h (2)
21-28: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider deleting copy/move to prevent accidental duplication.
The object carries ~2.2 KB of buffers plus live SCP03 session state; an accidental copy would both blow RAM and clone a session that only one instance may drive.
♻️ Proposed change
SE050(TwoWire &bus, uint8_t address = DEFAULT_ADDRESS) : bus(bus), address(address) {} + SE050(const SE050 &) = delete; + SE050 &operator=(const SE050 &) = delete;Also applies to: 147-168
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/SE050.h` around lines 21 - 28, Make SE050 non-copyable and non-movable by explicitly deleting its copy constructor, copy assignment operator, move constructor, and move assignment operator near the existing constructor. Preserve normal construction with the shared TwoWire reference while preventing duplication of its buffers and SCP03 session state.
3-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTrim the explanatory comment blocks.
Several multi-paragraph blocks here (header preamble,
identityImportrationale, the buffer-sizing story) read as design notes rather than code comments. Consider moving the background into a doc/ADR and keeping one or two lines at each site.As per coding guidelines: "Keep code comments minimal—one or two lines maximum—and comment only when the reason is not obvious; do not restate straightforward code or add multi-paragraph explanatory blocks."
Also applies to: 61-70, 135-146
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/SE050.h` around lines 3 - 11, Trim the multi-paragraph explanatory comments in SE050.h, including the file header, the identityImport rationale, and the buffer-sizing discussion near the referenced sections, to one or two lines each. Retain only non-obvious rationale; remove design background and details that can be moved to external documentation.Source: Coding guidelines
src/security/SE050.cpp (3)
703-715: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReturn values of these idempotent setup APDUs are discarded entirely.
secureApduresults andsware ignored for both the curve creation and the authenticator write. A genuine failure (not the expected 6985) surfaces only later as a confusingCreateSessionerror. Consider logging whenswis neither0x9000nor0x6985.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/SE050.cpp` around lines 703 - 715, Update the idempotent setup APDU blocks for curve creation and the UserID authenticator to inspect the return value from secureApdu and the resulting sw. Log the setup operation when sw is neither 0x9000 nor 0x6985, while preserving the existing flow for successful and expected idempotent responses.
769-769: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
0x%08xfor the 32-bit object IDs and0x%xfor the address.
objId %08lxand0x%02xdon't match the project's logging conventions for IDs and addresses.As per coding guidelines: "Format 32-bit node and packet IDs as
0x%08x; format one-byte values, flags, addresses, and reason codes as0x%xinstead."Also applies to: 785-785, 833-833, 840-841, 849-849, 866-866, 965-965
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/SE050.cpp` at line 769, Update the LOG_INFO calls around the SE050 identity-generation and related object-ID/address messages to follow project formatting conventions: use 0x%08x for 32-bit object IDs and 0x%x for one-byte addresses or other byte-sized values, removing mismatched length modifiers and padding while preserving the existing arguments and messages.Source: Coding guidelines
1064-1096: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueBenchmark loop in
probe()costs five extra ECDH rounds at every boot.Useful during bring-up, less so in shipped firmware. Consider gating it behind a debug define.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/SE050.cpp` around lines 1064 - 1096, Gate the ECDH benchmark loop and its related timing/logging code in probe() behind an appropriate debug compile-time define, so shipped firmware performs no extra rounds while debug builds retain the existing benchmark behavior.src/main.cpp (1)
621-641: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value~255 ms of blocking boot delay; confirm this is acceptable on this variant.
Fine functionally, but it lands unconditionally in
setup()before the scan. If the 250 ms settle is empirical, consider polling the address until it acks with a bounded timeout instead of a fixed worst-case wait.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main.cpp` around lines 621 - 641, Replace the unconditional 250 ms delay in the SE050_ENA_PIN setup block with bounded readiness polling of the SE050 I2C address before the scan, returning as soon as the device acknowledges. Preserve the existing ENA low/high pulse and enforce a timeout so boot cannot block indefinitely.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main.cpp`:
- Around line 696-700: Update the SE050 allocation flow around the se050 pointer
so probe() is called only when new SE050 succeeds; retain the existing cleanup
and nullptr assignment when probing an allocated device fails, and handle
allocation failure without dereferencing the null pointer.
In `@src/security/SE050.cpp`:
- Around line 249-256: Update the SE050 authentication logging near the “chip
authenticated” message to describe successful secure-channel establishment
rather than chip trust or identity authentication, reflecting the public default
SCP03 keys in SCP_KEY_ENC and SCP_KEY_MAC. Do not present this transport
handshake as a security feature; record key provisioning and rotation as a
follow-up separately.
- Around line 384-386: Replace the deterministic random() loop initializing
hostChallenge in the SCP03 setup with the chip TRNG via GetRandom (or the
established platform RNG), ensuring all 8 bytes are filled from a
cryptographically suitable source while preserving the existing hostChallenge
size and flow.
- Around line 485-538: Update the failure paths in the secureApdu exchange,
including the transceive failure and R-MAC validation failure, to mark both
scp.open and sessionActive false after host SCP03 state may have advanced
without card confirmation. Preserve the existing status/error returns while
ensuring the next call re-establishes the secure channel.
- Around line 107-119: Update the body read in the response handling flow around
body and bus.requestFrom so it passes the full size_t body value without
narrowing to uint8_t, using the appropriate size_t/int overload. Preserve the
existing capacity check and failed-read handling, ensuring LEN values up to the
supported frame size are not wrapped or silently capped.
- Around line 509-527: Guard APDU lengths before narrowing them to a byte:
validate encLen + 8 in the secure-command construction before assigning Lc, and
validate apduLen in transceive before writing frame[2]. Reject or fail the
operation when either length exceeds 255, while preserving existing behavior for
valid lengths.
---
Nitpick comments:
In `@src/main.cpp`:
- Around line 621-641: Replace the unconditional 250 ms delay in the
SE050_ENA_PIN setup block with bounded readiness polling of the SE050 I2C
address before the scan, returning as soon as the device acknowledges. Preserve
the existing ENA low/high pulse and enforce a timeout so boot cannot block
indefinitely.
In `@src/security/SE050.cpp`:
- Around line 703-715: Update the idempotent setup APDU blocks for curve
creation and the UserID authenticator to inspect the return value from
secureApdu and the resulting sw. Log the setup operation when sw is neither
0x9000 nor 0x6985, while preserving the existing flow for successful and
expected idempotent responses.
- Line 769: Update the LOG_INFO calls around the SE050 identity-generation and
related object-ID/address messages to follow project formatting conventions: use
0x%08x for 32-bit object IDs and 0x%x for one-byte addresses or other byte-sized
values, removing mismatched length modifiers and padding while preserving the
existing arguments and messages.
- Around line 1064-1096: Gate the ECDH benchmark loop and its related
timing/logging code in probe() behind an appropriate debug compile-time define,
so shipped firmware performs no extra rounds while debug builds retain the
existing benchmark behavior.
In `@src/security/SE050.h`:
- Around line 21-28: Make SE050 non-copyable and non-movable by explicitly
deleting its copy constructor, copy assignment operator, move constructor, and
move assignment operator near the existing constructor. Preserve normal
construction with the shared TwoWire reference while preventing duplication of
its buffers and SCP03 session state.
- Around line 3-11: Trim the multi-paragraph explanatory comments in SE050.h,
including the file header, the identityImport rationale, and the buffer-sizing
discussion near the referenced sections, to one or two lines each. Retain only
non-obvious rationale; remove design background and details that can be moved to
external documentation.
In `@src/security/SE050CryptoEngine.cpp`:
- Around line 188-193: Update se050CryptoSelfTest to use a dedicated typed
SE050CryptoEngine pointer for the SE050CryptoEngine instance instead of
static_casting the global crypto pointer; ensure the typed pointer remains valid
if crypto is later reassigned or swapped.
- Around line 97-129: Add an explicit startup warm-up that invokes the SE050
mirror initialization before PKI packet handling, reusing mirrorReady() and
guarding it with the same availability/configuration conditions used by the
engine. Ensure the warm-up occurs after the private key is available and does
not change mirrorReady()’s existing fallback behavior when initialization cannot
proceed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a7b43c43-8c91-462f-8e83-7654457ca195
📒 Files selected for processing (5)
src/main.cppsrc/security/SE050.cppsrc/security/SE050.hsrc/security/SE050CryptoEngine.cppvariants/rp2350/diy/pico2_w5500_e22/platformio.ini
Contributor
⚡ Try this PR in the Web FlasherNote Building this pull request… the flash button, badges and supported-board |
Six actionable findings from the first review pass, all real: - main.cpp: new SE050(...) could return null under memory pressure before probe() is called on it; guard added. - SE050.cpp openSecureChannel(): the SCP03 host challenge was pulled from random(), a non-cryptographic PRNG, instead of the platform's actual entropy source. Swapped to HardwareRNG::fill(), which on RP2350 is the ROSC-backed hardware RNG (rp2040.hwrand32()) - the same facility CryptoEngine already trusts for XEdDSA/Curve25519 keygen. - SE050.cpp secureApdu(): every early-return after scp.counter++ / chainedCmac() left the host's SCP03 counter and MAC-chaining value advanced for a command the card may never have processed - transceive failure, a short/unmac'd error response, a response too large to verify, and R-MAC verification failure all leave the two sides' ICVs out of step, so the *next* call would build a C-MAC the card rejects, forever, with no indication why. Every such exit now also closes scp.open/sessionActive so the failure is loud and immediate instead of a silent, permanent wedge. (SE050CryptoEngine::setDHPublicKey already falls back to software on any agreement failure, so this doesn't change the node's actual PKI reliability - it changes a wedged secure channel from "keeps trying and keeps failing the same way" to "fails once, loudly, and the fallback takes over" every time from there.) - SE050.cpp xfer(): bus.requestFrom() takes size_t on this platform (Wire.h), so the (uint8_t) casts around it were pure narrowing with no purpose - and a real one: a LEN of 253-255 computes a body of 255-257, which wraps through uint8_t into a request for far fewer bytes than the frame actually needs. Dropped the casts. - SE050.cpp secureApdu()/transceive(): two more truncation points - the Lc byte (encLen + 8) and the LEN byte (apduLen) are both ISO7816/UM11225 single-byte fields, tighter than what the surrounding buffer-capacity checks alone enforce. A payload in roughly the 240-283 byte range would pass those checks and then silently wrap into a wrong length byte instead of being rejected. Both now explicitly bounded to 255 first. Plus the cheap nitpicks worth taking: SE050 is now non-copyable (it holds live SCP03 session state and ~2 KB of transaction buffers - a copy would both blow RAM and clone a session only one instance may drive); the global crypto pointer is no longer static_cast to call selfTest(), a second typed pointer avoids relying on nothing else ever reassigning crypto; the two idempotent applet-setup APDUs (curve creation, UserID authenticator write) now log when the status word is neither success nor "already exists" instead of discarding it silently; object-ID/address log lines switched to the project's 0x%08x/0x%x convention; and the 5-round ECDH timing benchmark in probe() - useful during bring-up, not something a shipped device needs to redo every boot - moved behind a new opt-in -D SE050_BENCHMARK, off by default. Also reworded the SCP03 log line from "chip authenticated" to "secure channel open": SCP_KEY_ENC/MAC are NXP's public factory defaults (documented a few lines above that log line), so a successful handshake confirms a chip that speaks PlatformSCP03 correctly, not one trusted more than an attacker with physical I2C access could be. Rotating to per-device keys - noted in-thread as a real prerequisite for that stronger claim - stays out of scope here, same as the rest of Fase 2. Not changed, with reasoning left as a reply on the review thread rather than a diff: the fixed 250ms ENA settle delay (the chip acks its address well before it can answer T=1oI2C, so address-ACK polling would just move the same race earlier rather than remove it - and reset()/xfer() already retry bounded on top of this floor); the multi-paragraph comments (this is the one file in the tree explaining a chip and a protocol nobody else here has touched, and every block earns its length under "why is this necessary" - CodeRabbit flagged all of these as its own lowest tier, trivial/low-value); and the mirrorReady() warm-up suggestion, which the self-test call in main.cpp - already placed before any real packet could plausibly arrive - already provides. Re-verified on hardware after these changes (.154, pico2_w5500_e22): clean build, and a second real PKI DM sent from a peer and decrypted through the chip without incident, confirming the modified secureApdu() success path (unchanged logic, only the failure exits gained the state reset) still works end to end.
Contributor
Author
|
Thanks for the review — went through all 6 actionable findings against the actual code (not just the diff) and fixed everything that was real. Pushed as a new commit rather than folding into the originals so the fix is easy to diff against the review that prompted it. Fixed (all 6 actionable):
Nitpicks taken: deleted copy/move on Also reworded "chip authenticated" → "secure channel open": the SCP03 keys are NXP's public factory defaults (documented right above that line), so a successful handshake confirms a chip that speaks PlatformSCP03 correctly, not one trusted more than an attacker with physical I2C access could be. Rotating to per-device keys is real follow-up work, out of scope here (see "Not in this PR" in the description). Not changed, with reasoning:
Re-verified on hardware after all of the above: clean build, and a second real PKI DM sent from a peer node and decrypted through the chip without incident — confirms |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an optional NXP SE050 secure-element backend for the existing PKI key-agreement path (X25519/Curve25519 ECDH). When present and enabled, the SE050 performs the ECDH for a node's PKI direct messages; when absent, the node behaves exactly as it does today.
This is deliberately scoped to the part that doesn't change any product semantics: the node's identity stays exactly where it lives today (
config.security.private_key), exportable and backup-able as always. A copy of that key is mirrored into the chip so it can run the agreement; nothing about identity backup, remote key rotation, or NodeNum derivation changes. See "Not in this PR" below for the bigger, separate conversation this deliberately doesn't touch.Everything is gated behind two build flags (
HAS_SE050,HAS_CUSTOM_CRYPTO_ENGINE), both off by default and only defined forpico2_w5500_e22in this PR (the one board here that carries the chip). Every other board's binary is byte-for-byte unaffected — the new.cppfiles compile to nothing without those flags.What's added
src/security/SE050.{h,cpp}— the chip driver: T=1oI2C transport (block framing, CRC-16, ATR), APDU exchange with WTX handling, a PlatformSCP03 secure channel (the SE050 refuses key agreement outside an authenticated channel), and an X25519 identity that's either generated in the chip (identityEnsure, private half never leaves it) or imported to mirror a key the caller already holds (identityImport), backing one ECDH agreement at a time (identityEcdh).src/security/SE050CryptoEngine.cpp— aCryptoEnginesubclass overriding onlysetDHPublicKey(the single method bothencryptCurve25519/decryptCurve25519route through), so it's the one place that needs to know the agreement can run in hardware. Falls back toCryptoEngine's own software Curve25519 if the mirror import ever fails for any reason — a node is never left unable to complete a key agreement because a secure element didn't cooperate.src/main.cpp— an ENA power-on-reset pulse before the I2C scan (the SE050 has no reset line and comes up however the last power cycle left it), probing + constructing the driver right after the scan if it found one, and a self-test call placed deliberately after the network is up rather than next to NodeDB where the identity actually loads — if it ever hangs, the board still answers over the network instead of needing a bootloader pull.variants/rp2350/diy/pico2_w5500_e22/platformio.ini— the 3 lines enabling it on this board.Two implementation details worth flagging for review:
identityEcdh -> sessionApdu -> secureApdu -> transceive -> xfer, and as locals those buffers came to ~2.5 KB in one call chain — fine entered fromsetup()'s shallow frame, not fine entered from deep inside a packet handler on a board with a 4 KB FreeRTOS task stack (RP2350 targets build with-D__FREERTOS=1;setup()/loop()run in a1024-word CORE0 task). Moving them into the object is what makes this safe from a real packet-processing context, not just from a boot-time self-test.reentered()(checked at every layer) refuses to start new work on the chip while a transaction is parked waiting for a response — a second operation half-way through the first would otherwise advance the SCP03 counter and overwrite buffers still in flight.Not in this PR
The mirrored key stays exportable and backup-able exactly as it is today. A mode where the key is generated in the chip and never mirrored to config at all is a real possibility (
identityEnsure()already supports it, unused by anything in this PR), but it's a bigger product conversation — what happens to remote key rotation (an admin client can send its own chosen private key today; the chip can't be told "use exactly this value"), to local identity backup/export (today a locally-connected client can read its own private key back for backup purposes), to a lost or replaced chip. Not something to fold into a driver PR.On-hardware verification
Tested against a real SE050E2 (applet 7.2.0) on
pico2_w5500_e22, built from this branch (fresh offdevelop, not a byte-copy of a prior working tree):pio run -e pico2_w5500_e22— RAM 18.0%, Flash 25.4%.SE050: node key already mirrored→SE050: PKI key agreement now runs in hardware→SE050: self-test OK - PKI agreement through the chip matches software.Attempt PKI decryption→SE050: key agreement leaves 1240 bytes of the 4 KB CORE0 task stack→PKI Decryption worked!→ message delivered intact. No reset, node kept operating normally on real mesh/MQTT traffic afterward.🤝 Attestations
HAS_SE050/HAS_CUSTOM_CRYPTO_ENGINE, both undefined everywhere exceptpico2_w5500_e22in this PR, so every other board's build is byte-for-byte unaffected.pico2_w5500_e22(see on-hardware verification above)Summary by CodeRabbit