Repository navigation
Conversation
… the command path - An R-block with an error indication (the host did not get the last block, e.g. it gave up waiting) now gets the last single-frame response again. Before it got no answer, so the host, and any bridge relaying frames, waited forever. - With persistence enabled, the store was rewritten synchronously on every change, several times per command; on a busy disk that took up to a second, longer than NXP's T=1 host waits before asking for a retransmission. A background thread now writes the latest snapshot (temporary file, then rename). Unit tests keep the synchronous write. Unit test for the retransmission. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The persistence changes can lose updates across stores, reload stale security state after reopening, and weaken file permissions, exposing private and SCP03 keys to other users.
4 open issues
What changed in this PR
This PR improves SE050 simulator reliability with NXP Plug & Trust hosts by adding response retransmission and moving persistence writes off the command path.
Changes:
- Retransmits the last single-frame response when an R-block reports an error.
- Adds a background persistence writer using temporary files and rename, while retaining synchronous writes for unit tests.
- Adds a regression test for retransmission and error-free R-block handling.
| File | Description |
|---|---|
SE050Sim/se050-sim/src/t1.rs |
Caches single-frame responses, handles retransmission requests, and adds a test. |
SE050Sim/se050-sim/src/object_store/mod.rs |
Queues persistence snapshots for background file writes. |
Review effort: Balanced
| let tmp = p.with_extension("json.tmp"); | ||
| if std::fs::write(&tmp, &j).is_ok() { | ||
| let _ = std::fs::rename(&tmp, &p); |
There was a problem hiding this comment.
Fixed in 4e076b0. The temporary file is now created with create_new and mode 0600, so it is never readable by others while being written, and it takes the mode of the file it replaces when there is one (a store chmodded to 0600, or opened up on purpose, keeps its mode). Covered by test_store_file_is_private_and_keeps_its_mode.
| while let Ok((mut p, mut j)) = rx.recv() { | ||
| while let Ok((p2, j2)) = rx.try_recv() { | ||
| p = p2; | ||
| j = j2; | ||
| } | ||
| let tmp = p.with_extension("json.tmp"); | ||
| if std::fs::write(&tmp, &j).is_ok() { | ||
| let _ = std::fs::rename(&tmp, &p); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fixed in 4e076b0. Pending snapshots are now a map keyed by destination path, so each store's latest snapshot is written. Covered by test_stores_on_different_paths_keep_their_latest_snapshot (two stores, interleaved updates).
| if let Ok(tx) = tx.lock() { | ||
| let _ = tx.send((path.to_path_buf(), json)); | ||
| } |
There was a problem hiding this comment.
Good catch: tests/scp03.rs set_platform_scp_request_flow did fail on this branch because of it. Fixed in 4e076b0 with a per-path flush barrier: loading a store file waits for its pending write, and dropping a persistent store waits for its own. APDU handling still never waits for the disk. Covered by that test and by test_reopen_sees_updates_of_a_store_still_open. One limit, stated plainly: the TCP server has no signal handling, so a process killed by a signal can still lose a write queued a few milliseconds earlier; I did not add a dependency for that.
| // last block (e.g. it gave up waiting) and asks for it again. | ||
| // Without an answer the host and a bridge in between would | ||
| // wait forever. | ||
| self.last_response.clone() |
There was a problem hiding this comment.
Fixed in 4e076b0: both InterfaceSoftReset and resync clear the cached response. Covered by test_reset_and_resync_forget_last_response (an error R-block after either reset gets no replay).
…1 reset Fixes from review feedback on the T=1 retransmission and background store writes. The background writer kept only the last queued snapshot, whatever its destination, so two persistent stores in one process could lose each other's updates. Snapshots are now coalesced per path. Writes returned before reaching the disk and nothing waited for them, so a store dropped and reopened on the same file could load stale state: tests/scp03.rs set_platform_scp_request_flow failed on the reload of scp_required. There is now a flush barrier per path: loading a store file waits for its pending write, and dropping a persistent store waits for its own. APDU handling still never waits for the disk. The replacement file was created with the umask, so a store chmodded to 0600 became world-readable on its next update, and the temporary file was readable while being written. The store holds private keys and SCP03 keys: the temporary file is now created with mode 0600 and takes the mode of the file it replaces, if any. The cached response for an error R-block survived InterfaceSoftReset and resync, so a host could get a response from the previous link replayed. Both resets now clear it. Tests: per-path coalescing with two stores, reopening a store while the first is still open, file modes (new file 0600, existing mode kept), and an error R-block after each reset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ttested reads in sessions Fixes from review feedback specific to the UserID / AESKey sessions (the ones shared with the ECKey sessions and the store writer are in the two previous commits, also applied to wolfSSL#23 and wolfSSL#24). The attempt limit was checked at INITIALIZE UPDATE only. With a maximum of one, a host could start two AESKey handshakes, fail the first to block the object, then complete the second, which also reset the counter. A pending handshake now rechecks the block at EXTERNAL AUTHENTICATE and is refused without touching the counter. AES encryption and decryption ignored object policies, so a new AESKey Authentication Object without a policy (read only by default) still encrypted and decrypted in the default session. One-shot and CipherInit now require POLICY_OBJ_ALLOW_ENC / POLICY_OBJ_ALLOW_DEC for the current session owner; update and final use the key bound at init. The SE's attestation key pair (0xF0000012) is now applet-reserved like the ECKey session key: readable, but not usable for generic signing, nor replaceable or deletable. An attested ReadObject inside an authenticated session went to plain dispatch, which masks INS_ATTEST, and came back without attestation fields. It now goes through the attestation handler, with the session's Authentication Object as the policy owner. Updating an existing AESKey Authentication Object with a 24 or 32 byte key succeeded and left it unusable for sessions. Authentication Objects stay AES-128: the length is checked before anything is stored, on creation and on update. TAG_MAX_ATTEMPTS is parsed by one helper for all three types: a malformed or out-of-range value is refused with 0x6A80 instead of meaning "unlimited" (UserID: at most 255). Tests: two pending AESKey handshakes against a one-attempt object, cipher policies (one-shot and init, both directions), AES-128 on update, malformed maximum for AESKey and UserID, attested read in a UserID session, and the reserved attestation key. The NXP Plug & Trust SDK test passes 32/32 in all seven session modes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ttested reads in sessions Fixes from review feedback specific to the UserID / AESKey sessions (the ones shared with the ECKey sessions and the store writer are in the two previous commits, also applied to wolfSSL#23 and wolfSSL#24). The attempt limit was checked at INITIALIZE UPDATE only. With a maximum of one, a host could start two AESKey handshakes, fail the first to block the object, then complete the second, which also reset the counter. A pending handshake now rechecks the block at EXTERNAL AUTHENTICATE and is refused without touching the counter. AES encryption and decryption ignored object policies, so a new AESKey Authentication Object without a policy (read only by default) still encrypted and decrypted in the default session. One-shot and CipherInit now require POLICY_OBJ_ALLOW_ENC / POLICY_OBJ_ALLOW_DEC for the current session owner; update and final use the key bound at init. The SE's attestation key pair (0xF0000012) is now applet-reserved like the ECKey session key: readable, but not usable for generic signing, nor replaceable or deletable. An attested ReadObject inside an authenticated session went to plain dispatch, which masks INS_ATTEST, and came back without attestation fields. It now goes through the attestation handler, with the session's Authentication Object as the policy owner. Updating an existing AESKey Authentication Object with a 24 or 32 byte key succeeded and left it unusable for sessions. Authentication Objects stay AES-128: the length is checked before anything is stored, on creation and on update. TAG_MAX_ATTEMPTS is parsed by one helper for all three types: a malformed or out-of-range value is refused with 0x6A80 instead of meaning "unlimited" (UserID: at most 255). Tests: two pending AESKey handshakes against a one-attempt object, cipher policies (one-shot and init, both directions), AES-128 on update, malformed maximum for AESKey and UserID, attested read in a UserID session, and the reserved attestation key. The NXP Plug & Trust SDK test passes 32/32 in all seven session modes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the review. All four points are addressed in 4e076b0, with a test for each:
Verified locally: |


Two robustness fixes found while driving the simulator from NXP Plug & Trust on a microcontroller (real T=1 over I2C,
relayed to the TCP server):
T=1 stack sends an R-block asking for it again. The simulator ignored every R-block, so host and relay waited forever.
It now resends its last single-frame response; R-blocks without error bits (chaining acknowledgements) are still
ignored, as before.
every change, several times per command (e.g. creating an object with a policy). On a busy disk this took up to a
second, longer than the host waits, which then triggered the case above. A background thread now writes the latest
snapshot (temporary file, then rename). Unit tests keep the synchronous write.
Unit test included for the retransmission. All existing tests pass.
🤖 Generated with Claude Code