Skip to content

usb: ncry v3 - #759

Open
scgbckbone wants to merge 2 commits into
Coldcard:masterfrom
scgbckbone:ncry3-usb-auth
Open

usb: ncry v3#759
scgbckbone wants to merge 2 commits into
Coldcard:masterfrom
scgbckbone:ncry3-usb-auth

Conversation

@scgbckbone

Copy link
Copy Markdown
Collaborator

Implements opt-in USB ncry v3 authenticated encryption. V3 derives independent AES-CTR and HMAC-SHA256 keys for each direction and authenticates every frame over its direction, sequence number, length, and ciphertext. This rejects tampering, replay, reordering, and cross-direction reflection; authentication or framing failures terminate the session.

The default remains ncry v1, with v1/v2 behavior covered by regression tests. V3 protects the encrypted channel but does not authenticate the COLDCARD endpoint, so clients requiring endpoint authentication must use check_mitm() with a previously trusted xpub.

Includes matching ckcc-protocol 1.6.0 support, fixed test vectors, and protocol documentation. First supported in Mk4/Mk5 firmware 5.6.1 and Q firmware 1.5.1Q.

@scgbckbone

Copy link
Copy Markdown
Collaborator Author

client part Coldcard/ckcc-protocol#63

@scgbckbone

scgbckbone commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator Author
  • requires ckcc submodule bump after client PR 63 is merged

@scgbckbone

Copy link
Copy Markdown
Collaborator Author

Adversarial cross-repository review: ncry v3 (firmware #759 × ckcc-protocol #63)

Reviewed heads (fetched live):

  • Firmware: 1fcfa2a1e69977f2ce7075d4ba829cf0478e7215 (merge-base 1deeac4c)
  • Client: 20f35918040401b3f19dea26c01a44c17ac48768 (merge-base 3d1dfa85)
  • Submodule pointer in this PR = 20f35918… — exactly the client PR head ✓

Method: both full diffs read line-by-line; an independent reference implementation written from the documented construction only (stdlib hashlib/hmac + cryptography's AES — not pyaes, not the firmware ARM-asm module); the PR firmware built from a clean worktree and exercised over the simulator both by the stock client and by a third, hand-rolled v3 client for adversarial frames.

Findings (severity-ordered)

Medium — v3 client does not go terminal on transport failures; stale authentic responses get misattributed to later commands (also flagged by another reviewer; reproduced and confirmed here)

  • ckcc/client.py:146 encrypts the request (advancing tx_seq + CTR state), but a failed HID write (client.py:172) or a response timeout (client.py:185) raises a plain AssertionError, so _v3_failed is never set (only CCFramingError paths latch it).
  • Reproduced live against the PR simulator: send_recv(slow_cmd, timeout=100) times out → device still processes it and queues a properly tagged, sequence-correct response → caller reuses the object → the next command receives the stale response, which passes tag/direction/sequence checks because both sides' counters are honestly in sync. The off-by-one then persists for every later command.
  • No forgery is possible (data is authentic), but in v3's target use case (HSM automation) a traffic-delaying attacker — or a short timeout on a flaky link — makes the client accept a stale okay/result as the answer to a different command, contradicting the documented "any failure is terminal; the client rejects further use of the connection."
  • Fix: in send_recv, latch _v3_failed on any exception raised after encrypt_request ran (write loop and read loop), without latching pre-encryption failures. Regression test included in a follow-up client branch.

Low 1 — TX-sequence exhaustion kills the firmware USB task via unhandled exceptionshared/usb.py:367-371 raises FramingError('seq') inside send_response; the except FramingError block then raises again via framing_error(), and the task dies with a traceback instead of the clean v3_failed path (verified in simulator log). Only reachable after 2³² responses in one session. Suggest dropping the response and setting v3_failed instead of raising.

Low 2 — Doc example import path doesn't exist in the published packagedocs/usb-ncry-v3.md:129-131 uses from ckcc_protocol.client import …, but the package installs only ckcc (setup.py: packages=['ckcc']); the example fails for end users (ckcc_protocol exists only as a testing/ convenience copy inside the firmware repo).

Low 3 — Ambiguous HMAC argument-order notationdocs/usb-ncry-v3.md:30,43 and PROTOCOL.md:63: HMAC-SHA256(transcript, session_key) and HMAC-SHA256(direction, sequence, length, ciphertext) don't say which argument is the key nor the exact encoding. Both implementations agree and the normative vectors pin it down, but the notation should state key=/message= and direction‖LE32(seq)‖LE32(len)‖ct explicitly.

Low 4 — Client has no cap on accumulated HID response lengthckcc/client.py:179-187: a malfunctioning/malicious device can stream non-final packets indefinitely; the client buffers unboundedly (pre-existing for v1/v2; v3 raises the legitimate max to 2076). Suggest raising CCFramingError past USB_V3_MAX_WIRE_MSG_LEN/MAX_MSG_LEN.

Cross-repo byte-level agreement (all verified by execution)

USB_NCRY_V3=0x03; ncry request <4sI64s; reply mypb‖64B pk‖LE32 xfp‖LE32 len‖xpub; 64-byte raw x‖y pubkeys in wire and transcript; SHA256("ccncry3"‖LE32(3)‖host_pk‖dev_pk); prk=HMAC(key=transcript, msg=session_key); RFC5869 HKDF-Expand(info="ccncry3", L=128); key order h2d-enc, h2d-mac, d2h-enc, d2h-mac; AES-256-CTR from counter 0, stream continuing per direction; C2D\0/D2C\0; MAC over dir‖LE32(seq)‖LE32(len)‖ct, truncated to 16 bytes; seq from 0, increment after successful verify/decrypt, 0xffffffff usable once then terminal; 0x40 flag on final HID fragment only; 2060-byte plaintext / 2076-byte wire maximum; terminal-on-failure both sides. Every row matches. Documented fixed vectors independently recomputed and matched on all three implementations (firmware asm AES, pyaes, cryptography).

Compatibility matrix (each cell executed)

  • Default (v1) client ↔ old firmware: works. Default (v1) ↔ new firmware: works.
  • Explicit v1/v2 ↔ new firmware: works; v1 re-key still allowed; v2 cleartext rejection remains non-terminal (unchanged); 2060-byte v1 message OK.
  • Explicit v3 ↔ old firmware: fram bad ncry version, device not wedged (v1 still served after); client must close the attempt.
  • Second ncry: allowed for v1, rejected for v2/v3 (terminal for v3). No negotiation, no silent downgrade, no auto-fallback.

Cryptographic assessment

Per-session/direction keys make counter-0 CTR reset safe; fresh ECDH both sides gives session-key uniqueness; transcript binding blocks UKS/reflection/cross-version confusion; C2D/D2C blocks cross-direction reflection (verified live); 128-bit tag appropriate; verify-before-decrypt with no CTR advancement on failure (verified); constant-time compares on both sides; no plaintext/key leakage via errors; firmware receive buffer fixed at 2076 with checks before each write.

Endpoint-authentication boundary

Docs are accurate: v3 does not authenticate the endpoint, and check_mitm(expected_xpub=trusted_xpub) genuinely binds a pre-trusted xpub to the current session (signature over the ECDH session key — verified pass with session xpub, fail with a wrong xpub). A MITM cannot relay a signature across its two distinct sessions; the docs correctly warn that an xpub learned from the same first-contact connection is insufficient.

Tests run

  • PR firmware sim suite pytest ncry_tests.py --headless against a simulator built from 1fcfa2a1: 12/12 pass.
  • PR client unit tests: 7/7 pass (4 deselected require physical HID hardware).
  • Independent reference vs client: 41/41 checks (vectors, 60 random sessions, seq boundaries 0/1/0xfffffffe/0xffffffff/exhaustion, tamper/replay/reorder/reflection/truncation/tag-only/oversize, v1-format preservation).
  • Adversarial live-firmware scenarios via a third hand-rolled client: 29/29 (firmware KDF == reference on random inputs, reflection, replay, mixed HID flags, cleartext-in-bound-mode, 2077-byte wire, rx/tx seq exhaustion, second ncry, MITM check, legacy cells).

Verdict

Compatible; merge after fixing the Medium transport-failure latching bug (small client-side change + regression test; follow-up branch on ckcc-protocol), and consider the four Low items. The core protocol — KDF, key mapping, wire format, sequence semantics, terminal behavior on auth/framing failures, cross-repo byte agreement, and v1/v2 compatibility — is fully verified and sound.

@doc-hex

doc-hex commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Codex: I reviewed this specifically for compatibility with deployed protocol clients, including older ckcc-protocol, Electrum, HWI, Sparrow/Lark, ckbunker, rust-ckcc-protocol, and bhwi.

Compatibility verdict

I found no backward-compatibility break for deployed ncry v1/v2 clients.

The legacy AES-CTR wire format, 2060-byte message limit, v1 mixed plaintext/encrypted traffic, v1 rekeying, and v2 bound behavior remain intact. I also built the PR simulator and directly tested the exact pre-PR ckcc 1.5.0 client:

  • v1 encrypted traffic: pass
  • v1 plaintext/encrypted mixing: pass
  • v1 rekeying: pass
  • v1 maximum-size request/response: pass
  • v2 encrypted maximum-size request/response: pass
  • ckcc 1.6.0 v3 authenticated maximum-size exchange: pass

Released-client audit:

  • Electrum 4.8.1 pins ckcc 1.5.0 in deterministic builds and does not specify ncry_ver, so it uses v1.
  • HWI 3.2.0 vendors a v1/v2 implementation defaulting to v1.
  • Sparrow 2.5.3/Lark explicitly sends version 1.
  • ckbunker, rust-ckcc-protocol, and bhwi also default or hardcode v1, with optional v2 in some cases.

Findings to address

1. V3 client state is not poisoned after transport failures

send_recv() advances AES-CTR and tx_seq before HID writes and reads. A write error, disconnect, or response timeout escapes without setting _v3_failed.

I reproduced this with a simulated HID write failure: tx_seq advanced to 1 while _v3_failed remained false. A retry is then allowed even though client/device stream state is indeterminate.

For v3, mark the session failed after any transport/framing exception occurring after request encryption, until a complete authenticated response has been received. Application-level CCProtoError responses should not poison the session because those leave the streams synchronized.

2. Update the ckcc-protocol submodule pointer

Firmware currently pins 20f35918, the pre-squash head of ckcc-protocol PR #63. That PR was squash-merged as a6891e3; the trees are identical, but the firmware should point to the merged commit or the eventual 1.6.0 release-tag commit rather than a PR-only commit.

3. Legacy CI does not use a legacy client

The new v1/v2 tests instantiate the updated ckcc client. They test legacy mode in new code, but do not lock compatibility with the released ckcc 1.5.0 implementation.

Please add an exact 1.5.0 fixture or frozen interoperability coverage for v1, v2, v1 plaintext mixing, v1 rekeying, and the 2060-byte boundary.

4. Documentation imports the wrong package name

The new example imports ckcc_protocol.*. The installed package exposes ckcc.*; ckcc_protocol works in firmware tests only because of the local symlink.

The public example should use:

from ckcc.client import ColdcardDevice
from ckcc.constants import USB_NCRY_V3
from ckcc.protocol import CCProtocolPacker

Overall: the legacy compatibility story looks sound, but I recommend resolving the v3 transport-failure handling and submodule pin before merge.

— Codex

@scgbckbone

Copy link
Copy Markdown
Collaborator Author

Review feedback implemented. All findings from the two reviews above (cross-repo review, Codex compatibility review) are now addressed, split across the two repos as they belong:

This PR — commit 334265ec:

  • TX-sequence exhaustion no longer kills the USB task with an unhandled FramingError; the undeliverable error report is dropped and the session stays terminal via v3_failed. Verified on the simulator: no response at exhaustion, session terminal, no traceback.
  • docs/usb-ncry-v3.md: explicit HMAC key/message argument notation and wire format; example imports fixed to the installed package name (ckcc, not ckcc_protocol).
  • Submodule re-pinned from the PR-only head 20f35918 to merged master a6891e3 (identical tree).
  • Firmware ncry_tests.py re-run after the change: 12/12.

Client sideckcc-protocol PR #64:

  • v3 sessions now latch _v3_failed on any post-encryption transport failure (write error, disconnect, response timeout), closing the stale-response misattribution hole. Application-level error replies deliberately do not poison the session. Verified live against this PR's simulator.
  • Accumulated HID responses are capped at USB_V3_MAX_WIRE_MSG_LEN / MAX_MSG_LEN.
  • 6 new regression tests; 13/13 unit, 12/12 against this PR's simulator suite, 41/41 against an independent reference implementation.

Not addressed, deliberately: the suggestion to add a frozen ckcc 1.5.0 client fixture to CI — hardening rather than a defect, given the real 1.5.0 client was already validated against this PR's simulator manually. Happy to add it if you'd like it locked in.

One follow-up after merge order: once ckcc-protocol#64 lands, I'll bump the submodule pin here once more so it carries those client fixes (one-line change).

@scgbckbone

scgbckbone commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator Author

Submodule now points to c39db297, including the merged v3-only response-limit fix from client PR #65.

@scgbckbone

Copy link
Copy Markdown
Collaborator Author

@doc-hex doc-hex added enhancement New feature or request help wanted Extra attention is needed labels Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request help wanted Extra attention is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants