FIDO2: five host-side bugs from the transit v2 hardware brief - #7
Merged
Merged
Conversation
apps.crp.to and apps.onlykey.io will both serve this app in production, so it should say OnlyKey rather than OnlyAgent, and it should look like OnlyKey: the wordmark is flat black type with a keypad glyph for the O, not a teal-to-blue gradient. Text: page titles, the nav brand, the footer credit, and the store link, which now points at onlykey.io rather than onlyagent.us. Theme: - Palette is greys on near-black. One accent, white, carries links, focus rings, primary buttons and emphasis. - --grad was a teal-to-blue gradient and is now a flat white. Kept as a variable because a dozen rules reference it; a primary button now reads as the wordmark inverted rather than as a second brand. - The brand chip was a gradient square. It is now a white tile carrying the keypad glyph that stands in for the O in the wordmark, as an inline data URI - no new asset, no change to the webpack copy step. - The hero's gradient-clipped text would have clipped to a flat white and looked like a bug, so the emphasis is carried by weight instead: the lead-in sits back in --muted, the phrase that matters is full white. - Prose links get an underline. They used to be told apart from body text by hue; with both white they would simply vanish. Scoped so nav items, tool cards and buttons keep their own affordances. - Nav icons were accent-coloured, which with a white accent made them louder than the label beside them. They inherit now. - Alerts: success and warning no longer differ by hue, so they differ by the left rule - solid white against dashed grey. Danger keeps red. A failed decrypt or a bad signature is the one state a user must not misread, and it is the one place a hue earns its keep. - Primary buttons dim on hover instead of brightening, because white cannot get brighter. Rendered and checked with bootstrap and font-awesome loaded, as the shells load them: nav, hero, tool cards, the full form (inputs, readonly, radio pills), all four button styles, all three alerts and a code block. NOT renamed: onlyagent-theme.css, onlyagent-logo.svg and the .oa-* class prefixes. Both HTML shells, the webpack asset copy and the plugins reference them, and renaming buys nothing a comment at the top of the file cannot. NOT touched: plugins/vault/vault.js. Its "onlyagent" strings are not cosmetic - "onlyagent-vault-v1" is an HKDF info string and "onlyagent-vault" is the IndexedDB name. Changing either would make every existing vault undecryptable or orphan its data.
I drew the keypad glyph from memory as a 3x3 grid on a square tile. The real one is 3x2 - six keys - in a frame that is wider than it is tall. Measured off OnlyKey_Logo_360x.png rather than eyeballed a second time. The glyph is 51x41 units, the frame ring is 6 thick, and the keys are 7x7 on a grid at x 11/22/33 and y 12/22. The chip goes from 22x22 to 26x21 to match, and the geometry in the data URI is those numbers directly. Rendered against the real glyph at 8x to confirm the structure lines up before committing this time.
All five are in the WebAuthn/FIDO2 layer and none needs a firmware
change. Found against v3.0.5-test hardware, 2026-09-16/17.
1. Transit framing state was per-instance, not per-device.
onlykey.extra.js is a factory: onlykey-api.js requires it twice and
onlykey-pgp.js and onlykey-3rd-party.js once each, so `transit`
existed as four independent objects. transit_select() flipped v2 on
one of them while the derive/composite path went on framing v1 at a
v2 device - transit_seal() falling through to aesgcm_encrypt(),
transit_framed() understating every expected length by 20 bytes -
all while the connect banner reported "Transit framing: v2" from the
other copy. TRANSIT_V2_MIN and `transit` are hoisted to module scope
so the four copies share one object.
2. The composite chunk size was not a multiple of 57.
ok_extension.cpp re-chunks each arriving keyhandle into 57-byte
device packets and can write a true short length only when opt2
marks the FINAL host chunk; every other short tail goes out as 0xFF
and is counted as a full 57. At 224 (57*3 + 53) the device advanced
four bytes per non-final chunk past what the host had sent, so a
derived X-Wing [label32 | ct1120] overshot by 20 and was refused
with "Error derived decaps payload size"; RSA-4096 overshot by 8 the
same way. 224 -> 171 (57*3, + 20-byte frame = 191 <= 245) in both
onlykey-3rd-party.js and onlykey-pgp.js. Chunk counts go up and
priming is slower; that is the price of the framing.
3. A failed chunk was stepped over instead of ending the send.
Neither prime_composite() nor u2fSignBuffer() looked at what came
back from the ceremony, so a cancelled prompt, a tab that refused to
run WebAuthn, or a key pulled mid-send left the device reassembling
a payload short by however many chunks were lost - and blaming its
size. Both now stop and name the chunk that did not land. packetnum
is deliberately not reset on abort: the device keeps an opt3
high-water mark across operations and drops anything not strictly
greater.
4. The derived-decap challenge code was missing.
Field 30 (web_agent_derive_mode) = 0 makes the key wait for three
specific buttons, and the host is the only thing that can say which.
The earlier precomputation hashed [keytype | label32 | ct_X32],
which stopped being what the firmware hashes, and had been removed
rather than corrected - leaving "press these in order:" above an
empty box and, on a press, "Error incorrect challenge was entered".
emit_derive_challenge() restores it over the preimage the firmware
actually uses: SHA256(SHA256(label32 || ct1120)), bytes 0/15/31,
%6+1 - two hashes, not one. Emitted before the chunks go out and
cleared when the request ends, so stale digits never carry into the
next prompt. It is unconditional: onlykeyApi.OKversion cannot gate
it, because UNLOCKED reports "UNLOCKEDv3.0.5-test" - exactly 19
characters - so the byte it tests is a NUL terminator and every
Color calls itself a Go. That detection wants fixing at the protocol
level, not working around here.
5. aesgcm_encrypt/decrypt threw on an empty payload.
forge returns '' for a zero-length input and ''.match(/.{2}/g) is
null, so poll_for_response()'s sealed empty OKPING produced "Cannot
read properties of null (reading 'map')" on every poll of a v1
session - a message that points nowhere near framing. Both paths now
use bytesFromHex(), which guards it.
docs/ is the onlyagent.app deployment, so this is the build the hardware tests run against. Picks up the five FIDO2 fixes in the previous commit and the monochrome OnlyKey theme and shell copy from 2489083/d0fd23d, whose source changes had never been rebuilt.
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.
Five host-side bugs found running the transit v2 hardware test brief against v3.0.5-test on 2026-09-16/17. All five are in the WebAuthn/FIDO2 layer; none needs a firmware change.
1. Transit framing state was per-instance, not per-device
onlykey.extra.jsis a factory.onlykey-api.jsrequires it twice,onlykey-pgp.jsandonlykey-3rd-party.jsonce each — four independent copies oftransit.transit_select()flippedv2on one of them while the derive/composite path went on framing v1 at a v2 device:transit_seal()fell through toaesgcm_encrypt(),transit_open()toaesgcm_decrypt(), andtransit_framed()understated every expected length by 20 bytes — all while the connect banner reportedTransit framing: v2from a different copy.TRANSIT_V2_MINandtransitare hoisted to module scope so all four share one object.2. The composite chunk size was not a multiple of 57
ok_extension.cppre-chunks each arriving keyhandle into 57-byte device packets:0xFFmeans "a full 57, more coming", and the true length is written only whenopt2marks the final host chunk. There is no way to say "n bytes, more coming" — so on every chunk but the last, a tail shorter than 57 still counts as 57 and the device advances past bytes the host never sent.At 224 (57×3 + 53) each non-final chunk overshot by 4. A derived X-Wing
[label32 | ct1120]= 1152 sends five non-final chunks, so the device reached 1140 where the host had sent 1120, and the 32-byte tail took it to 1172 against a 1152 total — 20 over, past the 16-byte padding tolerance, refused withError derived decaps payload size. RSA-4096 (512 B, two non-final chunks) overshot by 8 the same way.Sizing: a credential id is 255 bytes with a 10-byte header → 245 per assertion; the transit frame costs 20 (4-byte counter + 16-byte tag) → 225 of plaintext; the largest multiple of 57 at or below 225 is 171. Changed in both
onlykey-3rd-party.js(COMPOSITE_MAX_PACKET) andonlykey-pgp.js(maxPacketSize).Chunk counts go up — ML-KEM-768 5→7, derived X-Wing 6→7, RSA-4096 3→3 — and each chunk is a full WebAuthn ceremony, so priming is slower. That is the price of the framing.
3. A failed chunk was stepped over instead of ending the send
Neither
prime_composite()noru2fSignBuffer()looked at what came back from the ceremony. A cancelled prompt, a tab that refused to run WebAuthn, or a key pulled mid-send simply did not reach the device — and the loop went on and eventually setopt2, so the device reassembled a payload short by however many chunks were lost and refused it for its size. That blames the payload for a transport failure; it cost a long detour into chunk arithmetic that was correct.Both paths now stop and name the chunk that did not land.
packetnumis deliberately not reset on abort: the device keeps anopt3high-water mark across operations and drops anything not strictly greater, so restarting at 1 would make the next operation's first chunk vanish silently.This one proved itself during testing — a browser-extension failure on an unrelated run surfaced as
packet 1 did not reach the deviceinstead of a silent size error.4. The derived-decap challenge code was missing
Field 30 (
web_agent_derive_mode) = 0 makes the key blink and wait for three specific buttons, and the host is the only thing that can say which. The earlier precomputation hashed[keytype | label32 | ct_X32], which stopped being what the firmware hashes, and had been removed rather than corrected — leaving "press these in order:" above an empty box, and on a press:emit_derive_challenge()restores it over the preimage the firmware actually uses, fromokcrypto.cppandokcore_prime_user_confirmation():Two hashes, not one — easy to get wrong reading only the call site. Emitted before the chunks go out and cleared when the request ends, so stale digits never carry into the next prompt. Verified against
python-onlykey'schallenge_code()and the liveageCLI before shipping.It is unconditional (six buttons, the same default
python-onlykeyships) becauseonlykeyApi.OKversioncannot gate it — see below.5.
aesgcm_encrypt/aesgcm_decryptthrew on an empty payloadforge returns
''for a zero-length input and''.match(/.{2}/g)isnull, sopoll_for_response()'s sealed emptyOKPINGproducedCannot read properties of null (reading 'map')on every poll of a v1 session — a message that points nowhere near framing, and the first symptom of bug 1. Both paths now usebytesFromHex(), which has the guard.Separate finding, not fixed here
onlykeyApi.OKversionis derived asresponse[32+19] == 99 ? 'Color' : 'Go'.UNLOCKEDreportsUNLOCKEDv3.0.5-test— exactly 19 characters — so index 19 of that field is the NUL terminator, not a hardware byte, and every Color reports itself as a Go. Gating the challenge box on that string left it empty on the very hardware it is for. Wants fixing at the protocol level rather than worked around in the library; flagged for a separate change.Build
docs/is theonlyagent.appdeployment, so the second commit rebuilds it. It picks up these five fixes and also the monochrome OnlyKey theme and shell copy from 2489083 / d0fd23d, whose source changes had never been rebuilt.Test status
Transit v2 brief: tests 1, 3, 4, 5, 6 and 8 pass; test 2 passes on lengths and status; test 7 decrypt passes, sign was not run (the browser extension dropped the message channel twice, not a device failure).