Repository navigation
Conversation
ctap_process_apdu_cbor_message used to read uint8_t cmd = *req before checking current_req_src. In the FIDO PKE-backed APDU path the caller passes capdu->data, which still points at shared_io_buffer even though the real payload is in PKE. The byte we read was discarded by the following source-read branch but the deref itself is misleading. Mirror the structure already used in ctap_process_cbor: only fall back to *req when no source is bound. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
applet_session_scratch_t aliases ctap_ga / ctap_mldsa / buffer through a union. ctap_process_cbor uses buffer[APPLET_SHARED_BUFFER_LENGTH] as the encoder output, while CTAP_get_assertion fields like ext_hmac_secret_salt_auth and pin_uv_auth_param are still read from ga after the encoder has begun emitting bytes. The flow is only safe because those fields physically sit past byte 544 in CTAP_get_assertion. Add static_asserts so a future field reorder or size change breaks the build instead of silently letting the encoder overwrite parsed input, and document the contract next to the union. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three DBG_MSG lines (apdu_output stream sent/total/sw, GET RESPONSE state dump, GET RESPONSE rejected dump) were added during streaming bring-up but the values they print are reproducible from the failing SW alone. Drop the per-chunk happy-path traces, keep the read-failure condition as a single ERR_MSG. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two places release DEVICE_APPLET_SESSION_CTAPHID for the same flow: CTAPHID_TxReset on stream tear-down, and the call sites in CTAPHID_Execute_Msg / CTAPHID_Execute_Cbor right after they finish. device_applet_session_release is idempotent so the redundant call is harmless, but the contract was opaque. Add comments at TxReset and at the post-send guard explaining that the explicit release covers only the inline (non-streaming) path; the stream path is already covered by TxReset. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ctap_process_cbor_stream_source_with_src and its ctap_process_cbor_stream_with_src wrapper only return 1 (success) or -1 (failure). The CTAPHID dispatch had a separate stream_ret < 0 branch followed by an unreachable fall-through that did the same thing. Document the contract on the prototypes and merge the failure paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three small mismatches accumulated after the streaming rework: - Repository Layout listed fido2-tests/ as if it were tracked, but it is gitignored and only checked out by CI. Move it out of the tree diagram and add a note pointing to where CI fetches it. - The rand.h porting contract listed random_buffer as optional, but ctap_install / piv_install and virt-card fabrication call it directly, so a port that omits it will not boot. - The Streaming / scratch-space policy did not call out two real contracts the code now relies on: per-applet *_acquire helpers are bookkeeping flags rather than synchronization primitives, and the applet_session_scratch_t union aliases ctap_ga/ctap_mldsa with the encoder buffer (now guarded by static asserts). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The OpenPGP import paths (ck_parse_openpgp and the streaming ck_parse_openpgp_stream_update) copied the wire-format X25519 private key directly into ecc.pri. Wire format is little-endian per RFC 7748, but ecc.pri is read big-endian by mbedtls_mpi_read_binary inside K__x25519, so the derived public key was computed against a different scalar value than the host actually sent. The PIV import paths already swapped LE→BE but never clamped the scalar. mbedtls_ecp_mul calls mbedtls_ecp_check_privkey before the Curve25519 ladder and rejects unclamped scalars; for unclamped inputs ecc_complete_key returned 0 with a never-written all-zero pub. The existing test_x25519_public_key_encoding case happened to anchor on that buggy output. Apply the swap on the OpenPGP paths to match PIV, and apply the RFC 7748 §5 scalar clamp on all four (PIV streaming + non-streaming, OpenPGP streaming + non-streaming) so the key is always well-formed at the point ecc_complete_key runs. Tests: - New RFC 7748 §6.1 round-trip test for both OpenPGP paths in test_key.c — fails before the fix (pub stays zero). - Update test_x25519_public_key_encoding's expected public to the canonical X25519(scalar, 9) value computed offline against the same imported private. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Core CCID defines bulkin_data and bulkout_data as scalars, but virt-card/ifdhandler.c declared them extern as [2] arrays and indexed each access by Lun. Lun==0 happens to land on the real scalar so the suite worked, but any Lun!=0 would read/write neighbouring globals. The compile-time array bound also disagreed with the actual storage. Switch the externs to match the scalar definitions and pass Lun only where it is meaningful (the bSlot field stamping); add an explicit (void)Lun in the receive path so the parameter is still acknowledged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ctap_get_assertion declares a stack ecc_key_t key (with a TODO comment on cleanup) and only memzeros it on the MLDSA path before returning. Every other return — error returns from CHECK_PARSER_RET / CHECK_CBOR_RET, missing-credential paths, hmac-secret failures, the main signing path — leaves up to ~200 B of private key material on the stack for the next caller's frame to inherit. Add a tiny ecc_key_cleanup helper and tag the local with __attribute__((cleanup(...))) so the zeroing happens unconditionally on scope exit, then drop the now-redundant manual memzero in the MLDSA branch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
oath_poweroff cleared oath_remaining_type and is_validated but left record_idx untouched. The CALCULATE ALL handler treats record_idx==0 as the entry-point case where it parses the host challenge, then increments record_idx through the records. If applet preemption (or any other poweroff) hits mid-iteration the leftover non-zero record_idx makes the next fresh CALCULATE ALL skip challenge parsing and resume from somewhere in the middle of the list. Reset record_idx alongside the other per-session state. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ndef_create_init_ndef compared write_file/truncate_file return values against -1 with `<`, which only catches values strictly less than -1. LFS error codes happen to all be <=-2 today so the check fires in practice, but the intent is "any error", and the surrounding code uses < 0 consistently. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Inside the rx-queue drain in CTAPHID_Loop, two identical CANCEL detectors fired on the same five conditions (TYPE_INIT, cmd==CANCEL, channel.executing, cid match, MSG_LEN==0). The first one runs before the cid-mismatch ERR_CHANNEL_BUSY check at line 808; if its cid match fails and state is BUSY (which it always is when executing is set in this codebase), the cid-mismatch path consumes the frame. The second detector therefore could only fire in an "executing=1 but state!=BUSY" configuration, which the code path never produces. Replace it with a comment that points back to the top-of-loop check so future readers do not re-add the same defensive duplicate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
When device_applet_session_expire fires mid-chain it called applets_poweroff (which releases the PKE buffer via ctap_poweroff) but never told src/apdu.c about it. The static fido_capdu_chaining flags (in_chaining, uses_pke, pke_owner) stayed set, so the next chained FIDO APDU would skip pke_buffer_acquire, see uses_pke==1, and call pke_buffer_write on a buffer that no one currently owns — silently corrupting whichever applet had since taken over PKE. Expose fido_capdu_reset as apdu_fido_chain_reset and invoke it from device_applet_session_expire, after applets_poweroff drops PKE. test_core_helpers stubs the new symbol alongside the other transport stubs so the helper unit test still links. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five small omissions surfaced in PR review: - APDU_INCOMING_DATA_SIZE (288) is the actual chaining bound; the doc only mentioned APDU_BUFFER_SIZE (256) and could mislead readers into using the wrong limit in applet bounds checks. - The apdu_response_source_set / _clear / _active streaming helper API was load-bearing but undocumented; describe the contract since this PR's PIV / OpenPGP / FIDO large-response work all hangs off it. - ctap_req_src_t.cancelled is a real second member used for cooperative CTAPHID CANCEL but the lifecycle prose only mentioned read. - common.h defines htobe16 too; mention it and call out that no 16-bit LE helper exists. - testmode_set_initial_ticks and testmode_err_triggered are part of the TEST surface but were missing from the listing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
e67c2e6 added a swap+clamp to ck_parse_openpgp / ck_parse_openpgp_stream_update on the assumption that OpenPGP X25519 wire is little-endian like PIV. That was wrong: OpenPGP keytocard sends the scalar as a big-endian MPI per RFC 4880 (gpg / libgcrypt store all keys BE), pre-clamped host-side. The existing OpenPGP path stored the bytes verbatim and was correct; my swap permuted them into a non-clamped form that mbedtls_ecp_check_privkey rejects, leaving the derived public key all-zero. CI's GPG CV25519 phase happened to keep working before the bad commit because gpg ships clamped BE bytes; the new RFC 7748 OpenPGP test added in e67c2e6 happened to "validate" the broken behaviour because its input was fortuitously clamped under both interpretations (1/32 chance per random scalar, but hand-picked to satisfy both). PIV's wire is in fact little-endian (NIST SP 800-78-5), so the existing PIV swap is correct; the speculative clamp I added isn't needed because yubico-piv-tool clamps host-side and CI was already exercising it. Restore both OpenPGP entry points and the PIV entry points to their pre-e67c2e6 logic. Replace the over-eager LE-input regression test with: - test_parse_openpgp_x25519_rfc7748 — uses the RFC 7748 §6.1 Alice scalar in clamped BE wire form (what gpg actually sends), asserts ecc.pri stores it verbatim and the derived pub matches the canonical X25519(scalar, 9) value. - test_parse_openpgp_x25519_streaming_rfc7748 — same vector, fed to ck_parse_openpgp_stream_update in 7-byte chunks. - test_parse_piv_x25519_rfc7748 — same Alice scalar in clamped LE wire form (what yubico-piv-tool sends) through ck_parse_piv, expecting the same canonical pub. Also restore the test_x25519_public_key_encoding expected pub: that value is the canonical X25519 result for the test's BE wire scalar; my "corrected" value was wrong. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
cmocka coverage of src/apdu.c was 69.8%, with the entire apdu_response_source_set + apdu_output streaming branch (lines 287-310, 327-336 of apdu.c) untested. Add focused tests: test/test_apdu.c: - test_response_source_multi_chunk_get_response: drives a 600-byte source through 3 GET RESPONSE rounds, asserts chunk size, SW chain (0x61FF / 0x6164 / 0x9000), payload bytes, close-callback bookkeeping, and the post-stream "no pending response" rejection. - test_response_source_tail_restore_on_shared_buffer: stages payload in shared_io_buffer with a source that reads from the same buffer; after chunk 1 we deliberately stamp a fake SW trailer at the boundary, and verify that chunk 2 returns the original bytes (the tail-restore copy must have replayed them). - test_response_source_read_failure_clears_state: a source whose read() returns -1 must yield SW_UNABLE_TO_PROCESS and still call close(). - test_response_source_clear_calls_close: explicit clear() calls close exactly once, idempotent on subsequent calls. - test_apdu_output_chaining_aliased_buffer: rapdu.data == sh->data path with a 280-byte response in the 288-byte shared buffer, exercising the tail-save branch at apdu.c:327-336 that is critical for CCID in-place chunking. - test_fido_apdu_chain_overflow_returns_wrong_length: pumps maximum- sized chained FIDO APDUs until the PKE-staged accumulator overflows; asserts SW_WRONG_LENGTH and that fido_capdu_reset released PKE. test/test_piv.c: - test_piv_cert_chained_read: writes a 600-byte cardholder authentication cert via 3 chained PUT DATA APDUs, then drains it via GET DATA + GET RESPONSE through piv_process_apdu_message, asserting byte-exact reassembly. Covers the PIV state-machine GET DATA / GET RESPONSE transitions and the apdu_output non-source chaining path. test/test_openpgp.c: - test_openpgp_cert_chained_read: same shape for OpenPGP, using TAG_CARDHOLDER_CERTIFICATE PUT DATA + GET DATA, exercising openpgp_process_apdu_message and the file-backed cert source. lcov uplift (cmake unit tests only): - src/apdu.c 69.8% → 78.0% - applets/openpgp/... 41.7% → 45.2% - applets/piv/piv.c 27.9% → 34.6% - total 40.4% → 42.0% Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The smoking step manually listed only test_apdu, test_openpgp, test_oath, test_piv. test_core_helpers and test_key were built but never run, so the cmocka coverage they produce never landed in the coveralls report. That alone explains src/common.c dropping from 91.3% to 60.9% (the 0x82 TLV length path now lives only in test_key) and src/device.c dropping similarly (test_core_helpers covers session APIs and TLV helpers). Switch to `ctest --output-on-failure`. add_cmocka_test already calls add_test(), so any future add_mocked_test(...) is picked up automatically and we stop relying on the smoking-test list staying in sync with test/CMakeLists.txt. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ck_parse_openpgp and ck_parse_piv were the original buffer-mode
parsers for `7F48` (OpenPGP) and `06/07/08`-tag (PIV) key import.
The streaming variants ck_parse_*_stream_init/update have replaced
them everywhere on the production path (applets/openpgp/openpgp.c
and applets/piv/piv.c only invoke the stream APIs). The non-streaming
versions only stayed alive because test/test_key.c still called them.
Delete the two functions (~190 lines) along with their declarations
in include/key.h. The TLV layout block above the OpenPGP variant is
still useful as documentation for the streaming parser, so move/keep
it. Tests are folded onto the streaming API:
- test_parse_openpgp_x25519: removed; the RFC 7748 variant below
asserts a stronger property on the same input shape.
- test_parse_openpgp_x25519_rfc7748: removed; redundant with the
streaming variant.
- test_parse_piv_x25519_rfc7748: rewritten to feed bytes through
ck_parse_piv_stream_update in 5-byte chunks.
Update AGENTS.md to describe the streaming-only contract.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two cmocka tests targeting code that the Python/PCSC integration suite does not exercise: - test_algorithm_information walks the response of GET DATA P2=0xFA, the only callsite for add_all_algorithm_info / add_algo_info (~50 lines in applets/openpgp/openpgp.c). It checks the SIG/DEC/AUT per-tag entry counts and verifies each entry is well-formed (tag + length + algo-id-prefixed OID). - test_piv_get_metadata_extended_algo_ids drops a freshly generated asymmetric key of each extended type (ED25519, X25519, SECP256K1, SM2) into the PIV AUTH slot and reads metadata back, asserting the algorithm byte matches the alg_ext_cfg defaults populated by piv_install. This covers the X25519/SECP256K1/SM2/ED25519 arms of applets/piv/piv.c::key_type_to_algo_id, which integration only reaches for RSA2048 / SECP256R1 / SECP384R1. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The CTAPHID applet step runs fido-hid-over-udp in the background and ends with `kill "$pid"`, which sends SIGTERM. SIGTERM's default action is "terminate" — atexit handlers do NOT run, so the gcov runtime never gets to call __gcov_dump() and the in-memory line counters are discarded. As a result, every code path the simulator exercised during test_mldsa65 (ML-DSA make_credential branch in ctap.c, the streaming callbacks, get_assertion ML-DSA branch) showed up as 0 hits in coveralls, even though the test ran and passed. Verified locally: without the handler, SIGTERM yields zero .gcda files; with it, 107 files (2840 bytes for ctap.c) are written. Install handlers for SIGTERM and SIGINT that call exit(0). The C runtime then runs atexit hooks, including the gcov dump, before process teardown. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
exit(0) makes a SIGTERM-killed simulator look like a clean voluntary return, so callers cannot distinguish "ran to completion" from "got killed". Switch to the POSIX-conventional 128+signo (143 for SIGTERM, 130 for SIGINT). Functionally identical for our gcov flush — exit() with any code triggers atexit hooks — but keeps the signal-termination signal observable to anyone who cares. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Ctaphid, OpenPGP, PIV streaming
Keep YubiKey-compatible HMAC-SHA1 commands in the OATH applet over APDU/CCID and preserve the existing KBDHID eject path.
Add a vendor PIV extension to read/set a UTF-16LE name (at most 78 bytes) per asymmetric key slot, stored as a LittleFS attribute. Replacement clears the name before writing the new key; move carries it; reset deletes it for ordinary slots. Also includes internal refactors: deduplicated fs close/error paths, unified EC public-key length encoding, length-driven PIV policy parsing, mask-based admin config flag updates, and pointer-based admin config access.
Add write_file_attrs() and set_attrs_commit() so callers can commit file data and/or several user attributes in a single atomic commit through lfs_file_opencfg, reusing the shared file buffer. write_file_attrs() opens with LFS_O_WRONLY | LFS_O_CREAT (optionally LFS_O_TRUNC), writes from offset 0 when len > 0, and commits the listed attributes together with the data. set_attrs_commit() opens an existing file with LFS_O_WRONLY only, writes nothing, and commits the attribute batch on close; a missing file fails and is never created. Both helpers validate arguments before open (file/attr size limits, attr_count >= 0, non-NULL attrs/buffers for non-zero sizes) and return LFS_ERR_INVAL without touching the storage on violation. Error handling follows close_file_result(): open failure returns immediately, every post-open path closes, a prior write error is preserved, otherwise the close error is returned, and success is never reported when the closing commit failed. Existing attributes not listed are preserved, and zero-length attributes are legal. On storage error the outcome is uncertain: after a remount the file is either the old version or the complete new version, never a mix. The new tests in test_key.c cover batched commits, pre-open validation, error propagation, and block-device fault injection (prog/erase/sync, including ops that execute but report failure) with fresh-remount recovery checks for existing and newly created files.
feat(fs): one-shot attr-batched write helpers
ck_write_key() used two commits per write: one for the key material and one for the KEY_META_ATTR attribute. Add ck_write_key_attrs(), which merges the material, the metadata attr, and optional caller attributes into a single write_file_attrs() commit; ck_write_key() becomes its NULL wrapper and existing callers are unchanged. piv_replace_asymmetric_key() now commits the new key together with a zero-length PIV_CONTAINER_NAME_ATTR instead of removing the name in a separate commit first, so replacing an existing key no longer pays an extra clear-name commit. The zero-length name attr is written on every replacement deliberately: a newly installed key can never inherit the old name, and the attr then exists permanently with length 0 (slightly larger directory entry, intentional). Error contract: on storage error the outcome is uncertain; after a remount the slot holds either the old key with its old name or the complete new key with no name, never a mix.
pin_create() now commits the PIN data together with RETRY_ATTR and DEFAULT_RETRY_ATTR in a single write_file_attrs() commit (was three commits on an existing file). pin_set_retries() commits both counters in one set_attrs_commit(); a missing file still fails with PIN_IO_FAIL and is never created. pin_update() and pin_clear() read the default retry counter first (read failure still returns PIN_IO_FAIL) and then commit the data update and the RETRY_ATTR reset together. pin_verify() keeps the comparison and failure-decrement flow unchanged. On success it reads DEFAULT_RETRY_ATTR into a separate variable and only writes RETRY_ATTR back when the current counter differs, so a successful verify at default retries performs no flash writes at all. is_validated is now set only after verification and any needed retry persistence succeed: an error return no longer leaves the PIN validated. All exits holding pin_buf still memzero it. The new test_pin_batched_retry_updates asserts via block-device counters that a default-retry successful verify performs no prog/erase, that the restore after a failed verify does commit, and that write failures, blocked/missing PINs, and missing attrs preserve the existing error semantics without setting is_validated.
openpgp_get_data() read all three key metadata attributes before the tag switch, but only TAG_APPLICATION_RELATED_DATA and TAG_KEY_INFO consume them. Move the reads into a small helper invoked at the top of those two cases (the switch makes them mutually exclusive, so a single command still reads at most once); every other GET DATA tag now does no key-metadata reads. Error handling is unchanged: the first failed read still aborts with -1.
Add fs_reader_t, a scoped read-only file reader for synchronous record scans: fs_reader_open/size/read_at/close, with read_at returning the actual byte count so callers can check complete records. The reader reuses the long-lived file_config/file_buffer shared by all wrappers, so it owns that cache from open to close; close is idempotent and resets the reader to its zero state. To make the 'no nested file ops while a reader is open' rule executable, TEST builds track shared-cache ownership: every fs.c entry that uses file_buffer (read_file, write_file/append_file/truncate_file via write_file_at, get_file_size, write_file_attrs/set_attrs_commit via opencfg_attrs_close) plus fs_format/fs_mount borrows on entry and releases on every exit; a nested borrow fails with LFS_ERR_INVAL and sets a conflict flag queryable via fs_cache_conflict(). The reader holds ownership from a successful open until close; size/read_at verify ownership without re-acquiring. Without TEST the checks compile out and behavior is unchanged. test_key.c gains reader lifecycle coverage (offsets, short EOF reads, double/zero-init close), rejection of nested cache users and of a second reader while one is open, cache release after a failed open, and cache release on wrapper error exits (injected write error, seek error).
fs.c gains a mutation generation counter, advanced at the entry of
every public operation that may modify the filesystem (data writes,
attr write/remove, file removal, rename, format, mount), success or
failure — a failed write may still have compacted. Read-only
operations and the fs_reader API do not advance it. Compared for
equality only; wraparound is harmless. Internal helpers
(write_file_at, opencfg_attrs_close) are reached via exactly one
public entry and do not double-count.
ctap_capacity_remaining_new_credentials() previously ran a full
lfs_fs_size scan on every authenticatorGetInfo. It now caches
{value, generation, valid} and recomputes only when fs_generation()
changed; a failed computation (get_fs_free_bytes error) is not cached,
so the next call retries. TEST builds expose
ctap_test_capacity_compute_count() to observe recomputations.
test_key.c asserts the generation advances exactly once per mutating
entry (including injected-failure and mid-commit-failure writes),
stays put on reads and reader operations, and advances on
format/mount. test_apdu.c drives the real capacity path: read-only
queries compute once, an unrelated write invalidates, an injected
failed write invalidates, a remount invalidates, and a read-failed
computation is retried without an intervening write. Both test mains
now hand littlefs static work buffers so mid-suite remounts do not
leak.
ndef_toggle_read_only() updated current_cc in RAM before writing the CC file, so a failed write left the cache disagreeing with the disk. On storage error the commit outcome is uncertain: build the candidate CC, write first, update current_cc only on success, and invalidate the cache on failure instead of assuming the old state survived. Add a validity flag and a single load entry ndef_cc_ensure(): only a complete CC_LENGTH read establishes a valid cache, a failed load stays invalid and never caches an error, and every consumer of current_cc (READ CC, the NDEF read/write permission checks, ndef_is_read_only()) goes through it. When permission data is unavailable, reads and writes fail with an error and ndef_is_read_only() conservatively reports read-only; no stale permission is ever served. ndef_install()/reset invalidate the cache first and re-establish it only after a complete load or a successful write. Tests live in a new test_ndef target (the suite had none): toggle and read-back, a commit that lands on disk but reports an error (final prog executed, then LFS_ERR_IO) followed by reload observing the applied write, reload failure rejecting permission-dependent operations without serving stale state, recovery on a later successful reload with no write, and install re-establishing the cache after the CC file went missing.
READ BINARY on the CC file is served from current_cc via ndef_cc_ensure(): a cache hit is a plain memcpy with zero flash access, and the existing offset/length checks are unchanged. An invalid cache falls through to one flash reload that re-populates it. NDEF data file reads are unchanged.
Consolidate the fs write paths (write_file/append_file/write_file_attrs/ set_attrs_commit) onto one write_attrs_at helper and share seek_and_read between read_file and fs_reader_t; merge pin_get_retries/ pin_get_default_retries into pin_get_counter; simplify the NDEF CC toggle to invalidate-on-failure without a candidate copy and restructure ndef_read_binary around the cached-CC early return. Partially offsets the NFCC flash cost of the merged-commit series.
The dispatch read the terminated attribute from flash on every APDU. The flag is only written by install and terminate, so keep a RAM copy: loaded on first use, written through on success, and invalidated on a failed write because the commit outcome is uncertain. Saves one attribute read per OpenPGP command.
- drop __packed from config_page_t: fields already follow their natural alignment (static asserts pin the on-flash layout), so Thumb-1 helpers use word accesses instead of unpacking bytes - check SN write-once against the validated snapshot in config_update instead of rereading the page - fold the flag readers into config_read_flag; drop unused config_valid
The key-handle wrapping key was re-read from flash for every allow/exclude-list entry verification and every key handle generation. Cache it in RAM: invalidated at the top of ctap_install (its sole production writer) so resets and reconnects always reload the committed key, loaded lazily on first use, and a failed load leaves the cache invalid so the next use retries. Also reject truncated or oversized KH_KEY attributes with LFS_ERR_CORRUPT instead of silently using a partially filled key.
perf(ctap): cache KH_KEY in RAM
SP 800-73-4 Part 2 §3.1.1 requires that re-selecting the PIV Card Application (full or right-truncated AID) leaves all security status indicators unchanged; they may only be cleared when the selected application changes. piv_select() previously reset the PIN, PUK and management-key authentication on every SELECT. Drop the resets from piv_select(); the applet-switch path already clears everything via piv_poweroff(), which now also resets the management-key mutual-auth challenge (previously only cleared by piv_select) to keep the switch-away semantics unchanged. Streaming and GA stream state were already reset by the INS checks in piv_process_apdu(). Add test_piv_reselect_preserves_security_status covering re-select with the full and the RID-only AID, and the cross-applet switch that must still invalidate PIN verification.
The release-version centralization made piv_get_version report the build-configured CANOKEY_PIV_VERSION bytes (0.0.0 in development builds) instead of a hardcoded 6.0.0. Compare against the generated firmware-version.h macros; the SW-trailer restoration regression this test targets does not depend on the specific version value.
Acquire the applet session before staging large HID requests in PKE and retain ownership through fragmented reception. Release receive ownership on abort, timeout, resynchronization and PING completion; hand MSG/CBOR ownership to their command handlers. Share CCID response-header construction and HID setup handling without changing compiler flags. Add wire-response and request-lifetime regressions. Validation: 94 test_apdu tests pass; DevKit and NFCC Release builds pass. NFCC Flash is 163728 bytes, saving 108 bytes against the session-fix baseline. The session fix was also verified on the flashed DevKit with CCID/HID interleaving.
Apply the optional restriction to GetInfo, registration, and existing credential authentication while preserving credential management. Cover advertised algorithms, registration fallback, and assertions in both build modes.
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
2 participants
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
Promote the accumulated
devbranch tomaster.This range brings together the history-preserving development layers merged in #169 through #173, including:
GET RANDOMHistory policy
devis 419 commits ahead ofmasterat the time this PR is opened.Validation
The source layers were validated with the native unit suites, focused protocol tests, workflow/shell checks, and APDU replay smoke tests documented in #169, #170, #171, #172, and #173. This integration PR adds no commits beyond
devate1ee3710d97f2d6350d67fa0937a7ee2974a3e9c.