Composite PQC: make sign/decrypt usable, conform to draft-10, and stop reporting false success - #2
Open
bmatusiak wants to merge 7 commits into
Open
Composite PQC: make sign/decrypt usable, conform to draft-10, and stop reporting false success#2bmatusiak wants to merge 7 commits into
bmatusiak wants to merge 7 commits into
Conversation
The except-handler bailed out as soon as any data had been received, on any read exception - including an ordinary 2s per-read timeout mid-stream. That truncated real multi-packet responses (e.g. the 1216-byte X-Wing pubkey) whenever one read happened to time out before the next packet arrived, well before the caller's actual deadline. Now it keeps polling until the real deadline regardless. Found while debugging onlykey-testing's TC-04 keygen test intermittently getting back short reads (e.g. 1024/1152 bytes instead of 1216).
…all data is read before processing errors
…e and update related functions in cli.py and derived_xwing.py to use the new implementation
…load
Three defects in the composite PGP-PQC path, all of which left the CLI
unable to complete the flow it advertises.
pqc.sign()/pqc.decrypt() could not return their own output. Both ended in
read_string()[:N], and read_string() is
''.join(chr(item) for item in read_bytes(...) if item != 0)
which drops every zero byte and returns str. Underneath it read_bytes() is
a single self._hid.read(n) of ONE 64-byte report with no reassembly, so
read_string(...)[:3309] for an ML-DSA-65 signature was impossible by
construction rather than merely unreliable - 64 bytes is the most it could
ever have returned. Both now use a new pqc.read_exact(), which reassembles
consecutive reports the way the device actually sends them
(send_transport_response() emits ceil(len/64) back-to-back reports), skips
the status broadcasts that otherwise land where a signature belongs, and
returns bytes.
read_string() itself is untouched. 37 subcommands share it and the test kit
pins its current behaviour; this is a binary path alongside it, modelled on
the age plugin's OnlyKeyPQ._read_response(), which already solved the same
problem for the 1216-byte X-Wing pubkey.
Nothing exposed those functions. setpqc/loadpqc could load a composite key
and no command could then use it, so "load a PQC key, decrypt something,
sign something" had no command-line route past step one. Adds:
onlykey-cli signpqc [RSA1-RSA4] [ecc|pqc] [digest hex | file]
onlykey-cli decryptpqc [RSA1-RSA4] [hex | file]
Both are device primitives, deliberately: a composite OpenPGP signature is
the two halves concatenated and a composite session key needs the KMAC
combine plus an RFC 3394 unwrap, and that framing belongs to the caller.
Operands may be given as a path because an ML-KEM ciphertext is 2176 hex
characters, past what several shells accept in one argument.
setpqc reported success for a load the device refused. Outside config mode
the device answers each of the three chunks with "Error not in config
mode"; those replies were never read, so setpqc printed "Loaded composite
PQC PGP key (160 bytes) into RSA1" and exited 0 having stored nothing. This
is unusually bad here because there is no readback - okcrypto_getpubkey()
has no KEYTYPE_PQC_PGP branch - so a caller had no second way to find out.
load_composite_key() now waits for the device's acknowledgement and raises
on a refusal, and setpqc/loadpqc exit non-zero. The wider exit-code problem
across the other subcommands is left alone; it is its own change.
Also drops pqc.py's "UNTESTED against hardware" docstring, which stopped
being true on 2026-08-01.
Verified against the emulator (onlykey-testing 02-cli/17-composite-cli-ops,
8/8): the refusal is reported, both signature halves verify against keys
derived independently from the loaded blob - including the 3309-byte
ML-DSA-65 one - and both decrypt halves match host-computed secrets.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…copy)
The Node/OpenPGP.js bridge behind `loadpqc` vendors the PQC fork, and it is
required to stay BYTE-IDENTICAL to the copy in onlykey.github.io. This is
that copy, resynced after the conformance change there (md5 verified).
The fork implemented an earlier revision of the draft in three places, so
nothing outside our own code could read what it produced:
- codepoints: pqc_mldsa_ed25519 107 -> 30, pqc_mlkem_x25519 105 -> 35
(IANA assigns 30/35; 105/107 are private/experimental)
- ECC key share: the raw X25519 shared secret, not SHA3-256(ss||ct||pub)
- key combiner: SHA3-256(... || algId || domSep || len(domSep)), not
KMAC256 over data that also carried mlkemCipherText and mlkemPublicKey
All three alter derived session keys, so they land together - one flag day
rather than three. Nothing on the device changes; the firmware never sees an
OpenPGP algorithm ID and every hash and combine is host-side.
For this repo specifically, the bridge is used by `loadpqc` to PARSE a
composite private key file and extract the 160-byte blob. That path is
unaffected by the KEM changes - the blob layout is untouched - and
02-cli/16-cli-key-files still passes. The codepoints do matter here, because
a key file written by a conformant tool would otherwise not be recognised as
composite at all.
Verified end to end against rpgp 0.20 (Rust, independent implementation):
all four directions pass, where before it could not parse our keys.
Composite kit suites: 30/30.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
No behaviour change. Comment and docstring wording only, plus the resynced vendored bridge copy. - pqc.py: the module docstring cited a test-kit run and specific test files by path. Those live in a separate repo and cannot be resolved from here, so the claim is now just that the path has been exercised against hardware. - pqc.py: _await_load_reply() and read_exact() described what the code used to do rather than what it does. Rewritten to explain the current behaviour. - cli.py: same for the setpqc comment. - openpgp_bridge/openpgp.js: resynced with the copy in onlykey.github.io, which had the same comment tidy-up. The two must stay byte-identical.
Comments only; no code changed. Found by auditing every factual claim in the diff against the firmware sources, RFC/IANA, and draft-ietf-openpgp-pqc-10. pqc.py, decrypt(): said the caller does "the KMAC combine". It does not - the combiner is SHA3-256 (draft section 4.2.1). This was left behind when the KDF was corrected and is the same class of error as the codepoints: a confident, wrong crypto claim. Same wording fixed in cli.py decryptpqc, which additionally named KMAC256 with the domain separator as though that were the construction. pqc.py module docstring: claimed the load path AND both operations had been exercised end to end against hardware. Only the load path has. read_exact, and therefore sign() and decrypt(), are new and have run against an emulated device only. The docstring now separates the two. pqc.py read_exact(): "64 bytes is the most it can ever return" is true only where MAX_INPUT_REPORT_SIZE is 64; it is 65 on Windows. Restated in terms of one report. pqc.py _op_timeout(): dropped an unsourced "a few hundred ms on the M4" timing figure. The point it supports - that the budget is sized for a human, not the device - stands without it. derived_xwing.py and tests/test_derived_xwing.py: both cited a test case in a separate test-kit repo as evidence. The observations are real; the citation cannot be resolved from this repo, so it is dropped and the claim kept. openpgp_bridge/openpgp.js: resynced with the copy in onlykey.github.io, which had the same comment corrections. The two must stay byte-identical. Verified after: py_compile clean, composite suites 30/30 unchanged.
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
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.
Brings the composite PGP-PQC path in
onlykey-clito a working, standards-conformant state.Before this, the flow it advertises — load a PQC key, decrypt something, sign something — could
not be completed from the command line at all.
The composite functions could not return their own output
pqc.sign()andpqc.decrypt()both ended inread_string()[:N].read_string()drops everyzero byte and returns
str, andread_bytes()underneath it does a single HID read of onereport with no reassembly. So a 3309-byte ML-DSA-65 signature could never be returned — one
report's worth is the ceiling. The 32- and 64-byte cases were corrupted whenever a zero byte
appeared in the output.
Both now use a new
pqc.read_exact(), which reassembles consecutive reports, skips statusbroadcasts, and returns
bytes.read_string()itself is untouched — 37 subcommands share it,so this is a binary path alongside it rather than a change beneath them.
Nothing exposed those functions
setpqc/loadpqccould load a composite key and no command could then use it. Adds:Both are deliberately device primitives: composing the two signature halves, or doing the key
combine and RFC 3394 unwrap, belongs to the caller. Operands may be given as a file path because
an ML-KEM ciphertext is 2176 hex characters, past what some shells accept in one argument.
setpqcreported success for loads the device refusedOutside config mode the device answers each of the three chunks with "Error not in config mode".
Those replies were never read, so
setpqcprinted "Loaded composite PQC PGP key (160 bytes) intoRSA1" and exited 0 having stored nothing. That is worse here than it sounds, because there is no
readback —
okcrypto_getpubkey()has no PQC branch — so a caller had no second way to find out.load_composite_key()now waits for the device's acknowledgement and raises on refusal, and bothsubcommands exit non-zero.
Conformance to draft-ietf-openpgp-pqc-10
The vendored OpenPGP.js bridge implemented an older revision of the draft in three places:
SHA3-256(ss ‖ ct ‖ pub)KMAC256(…)All three are corrected, and the copy in the web app is kept byte-identical.
This is a flag-day change: all three inputs feed the derived key, so any composite key or message
created before it cannot be read after. That is why it belongs before release rather than after —
afterwards it would mean orphaning real user keys.
Also included, from earlier work: two fixes to
_read_response()truncating multi-packet replies,and a refactor moving Bech32 encoding for derived identities into its own module.
Evidence
02-cli/17-composite-cli-ops— 8/8. The refusal is reported; both signature halves verifyagainst keys derived independently from the loaded blob, including the 3309-byte ML-DSA-65 one;
both decrypt halves match host-computed secrets.
OpenPGP.js: before the conformance fix it could not parse our keys at all; after, all four
directions pass — it parses the key, verifies our signatures, decrypts what we encrypt, and we
decrypt what it encrypts.
Not verified
Everything above ran against an emulated device, not a Teensy. The load path last ran on physical
hardware on 2026-08-01; the new binary read path has never run on hardware. Linux only — the HID
report size is platform-conditional, so Windows and macOS are untested.
The wider exit-code problem is deliberately out of scope: roughly 25 of 37 subcommands never map
failure onto the exit code, because they all bottom out in
setslot/setkey. That is its ownchange and its own PR.