Skip to content

[BREAKING] Fix AES-CCM nonce reuse (direction bit) + exact-counter replay (MJ-12)#84

Draft
balloob wants to merge 1 commit into
OpenDisplay:mainfrom
balloob:fix/aes-ccm-nonce-direction-and-replay
Draft

[BREAKING] Fix AES-CCM nonce reuse (direction bit) + exact-counter replay (MJ-12)#84
balloob wants to merge 1 commit into
OpenDisplay:mainfrom
balloob:fix/aes-ccm-nonce-direction-and-replay

Conversation

@balloob

@balloob balloob commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

⚠️ BREAKING — coordinated protocol change. DO NOT MERGE standalone.

This fixes finding MJ-12 in src/encryption.cpp. It has two parts: one safe replay-guard fix, and one breaking change to the on-wire AES-CCM nonce.

Part A — safe replay-guard fix (non-breaking, internal state only)

verifyNonceReplay() skipped the replay-window scan whenever the incoming counter exactly equaled last_seen_counter — the && counter_diff != 0 guard on the window check. That let the most-recent command (and the counter-0 command) be replayed and re-executed, since an exact-counter match bypassed the "already seen" scan entirely.

Fix: remove the && counter_diff != 0 guard so an exact-counter match is scanned against the replay window like any other non-new-maximum counter. A legitimate packet always carries a strictly-increasing counter, so this rejects only replays.

One supporting change is required to keep it correct: the client's first command uses counter 0 (py-opendisplay _nonce_counter starts at 0 and increments after encrypting), and the replay window was zero-initialised. Scanning counter 0 against an all-zero window would false-match and reject the genuine first command. So the replay window is now initialised to a 0xFF sentinel ("no counter recorded") instead of 0, which is a real counter value. This is internal RAM state only — no wire/format change.

Part B — nonce direction bit (⚠️ BREAKING wire crypto change)

The 16-byte full nonce was session_id(8) || counter(8, big-endian) for both directions. Both the inbound command counter and the outbound response counter reset to 0 at authentication, so command #0 and response #0 encrypt under an identical (key, nonce) pair. AES-CCM nonce reuse under the same key is fatal: it leaks the keystream (XOR of the two plaintexts) and enables tag forgery.

Fix: reserve the most-significant bit of the counter field — bit 0x80 of nonce[8], which sits inside the CCM nonce slice nonce_full[3:16] — as a direction flag:

  • inbound commands (client → server): bit MUST be clear; an inbound command with the bit set is now rejected.
  • outbound responses (server → client): bit is SET.

This guarantees the two directions can never collide on a nonce. The counter never grows large enough to reach this bit in practice.

This changes the on-wire crypto. Updated firmware will not interoperate with an un-updated client, and vice-versa.

Rollout / lockstep required

Part B must land together with matching nonce-construction changes in all peers, or encrypted comms break for un-updated devices/clients:

  • py-opendisplaysrc/opendisplay/crypto.py (get_nonce() / encrypt_command() / decrypt_response()): as the client it must keep the direction bit clear on the command nonces it sends, and set/expect it set on the response nonces it decrypts.
  • Firmware_NRFencryption.c (nonce construction + inbound direction-bit check).
  • Firmware_Silabs — equivalent nonce construction + inbound check.

None of those repos are touched by this PR.

Suggestion: Part A (the replay-guard fix + sentinel init) is fully backward-compatible and could be split into its own non-breaking PR if the team wants the replay fix shipped ahead of the coordinated Part B rollout.

Testing

Firmware builds clean with the changes:

  • pio run -e nrf52840customSUCCESS (encryption.cpp recompiled + linked)
  • pio run -e esp32-c3-N4SUCCESS (encryption.cpp recompiled + linked)

No on-device / interop testing performed; Part B interop requires the coordinated peer changes above.

🤖 Generated with Claude Code

https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR

…AKING]

MJ-12 has two parts.

Part A (safe, non-breaking): verifyNonceReplay() skipped the replay-window
scan whenever the incoming counter exactly equaled last_seen_counter (the
`&& counter_diff != 0` guard), so the most-recent command (and the counter-0
command) could be replayed. Remove the guard so an exact-counter match is
checked against the window. Because 0 is a valid counter and the window was
zero-initialised, also initialise the replay window to a 0xFF sentinel so the
genuine first counter-0 command is not mistaken for a replay. Internal state
only -- no wire change.

Part B (BREAKING -- wire crypto change): the 16-byte nonce was
session_id(8) || counter(8 BE) for BOTH directions, and both the inbound
command counter and the outbound response counter start at 0 after auth, so
command #0 and response #0 encrypted under an identical (key, nonce) pair
(AES-CCM nonce reuse). Reserve the top bit of the counter field (0x80 of
nonce[8], which is inside the CCM nonce slice nonce_full[3:16]) as a
direction flag: set for server->client responses, required clear on inbound
commands, and reject any inbound command whose direction bit is set.

This changes the on-wire crypto and must ship in lockstep with matching
changes in py-opendisplay (crypto.py), Firmware_NRF (encryption.c) and
Firmware_Silabs, or encrypted comms break with un-updated peers.

Builds: nrf52840custom SUCCESS, esp32-c3-N4 SUCCESS.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012g2e8mr132vcizx92WsgiR
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.

1 participant