Skip to content

SE050Sim: T=1 retransmission on R-blocks with error; store writes off the command path - #23

Open
mcamposv wants to merge 2 commits into
wolfSSL:mainfrom
shosoin:t1-retransmit-async-persist
Open

mcamposv wants to merge 2 commits into
wolfSSL:mainfrom
shosoin:t1-retransmit-async-persist

Conversation

@mcamposv

@mcamposv mcamposv commented Nov 1, 2026

Copy link
Copy Markdown

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):

  • R-block with an error indication. When the host does not get the last block (typically it gave up waiting), NXP's
    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.
  • Persistence off the command path. With persistence enabled, the whole JSON store was rewritten synchronously on
    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

… 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>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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

Comment on lines +112 to +114
let tmp = p.with_extension("json.tmp");
if std::fs::write(&tmp, &j).is_ok() {
let _ = std::fs::rename(&tmp, &p);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +107 to +116
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);
}
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Comment on lines +120 to +122
if let Ok(tx) = tx.lock() {
let _ = tx.send((path.to_path_buf(), json));
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
mcamposv added a commit to shosoin/simulators that referenced this pull request Nov 2, 2026
…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>
mcamposv added a commit to shosoin/simulators that referenced this pull request Nov 2, 2026
…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>
@mcamposv

mcamposv commented Nov 2, 2026

Copy link
Copy Markdown
Author

Thanks for the review. All four points are addressed in 4e076b0, with a test for each:

  • the background writer coalesces per path and has a flush barrier on load and drop (tests/scp03.rs reload test passes again)
  • the store file is created 0600 or keeps the replaced file's mode
  • the cached R-block response is cleared on soft reset and resync

Verified locally: cargo test (unit, integration and SCP03 suites), the NXP Plug & Trust SDK test 32/32 in all seven session modes (None, UserID, AESKey, ECKey, and the three with Platform SCP03), and the wolfCrypt test, which gives the same result as on main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants