Skip to content

fix(test): AEAD tamper oracle must fail closed (audit F1) - #78

Merged
JC-000 merged 1 commit into
docs/benchmark-refreshfrom
fix/audit-f1-aead-oracle
Aug 13, 2026
Merged

fix(test): AEAD tamper oracle must fail closed (audit F1)#78
JC-000 merged 1 commit into
docs/benchmark-refreshfrom
fix/audit-f1-aead-oracle

Conversation

@JC-000

Copy link
Copy Markdown
Owner

F1 — the AEAD tamper test could not fail

Defect.tools/test_tls_record.py test 4b feeds tls_record_decrypt a
tampered record and is supposed to assert C=1 (rejected); it read the carry
only if regs and "P" in regs, but VICE's binary monitor names the 6502 status
register FL (harness backends/vice_binary.py:1325:
regs.get("FL", regs.get("FLAGS", regs.get("SR", 0)))), so that branch was dead
on every VICE run and 4b always fell through to a fallback that asserted only
that the computed tag differed from the record's tag — true of any tampered
input whether or not aead_decrypt rejected it.

Fix.carry_from_regs() looks the status register up under FL/P/FLAGS/SR
so the real oracle fires, and the fallback pass path is gone: not being able to
read the carry means the rejection could not be evaluated, which is a FAIL. The
tag comparison survives only as diagnostic text on a C=0 failure. Also switched
the VICE launch from a bare ViceConfig to
_vice_helpers.default_vice_config(), matching every other in-tree VICE test
(same file; no other file touched).

Mutant effectiveness — proven in the built binary, not assumed

Mutation: src/crypto/aead.s, the bne @auth_fail after
jsr aead_verify_tag, replaced with nop/nop. Byte-diff of the linked PRG
against the pristine one:

$ cmp -l laneA_pristine.prg build/c64-https.prg
24491 320 352
24492 61 352

Exactly two bytes, D0 31EA EA (octal 320/61 → 352). PRG offset 24491
(1-based) maps to $0801 + 24489 - 2 = $67A9, and build/labels.txt has
al C:67A0 .aead_decrypt — i.e. aead_decrypt+9, the branch itself. Mutation
confirmed present and in the right place. The mutation lived in the working tree
only and was reverted before the commit; the branch contains just the test fix.

Acceptance A/B (VICE, ip65 default profile, --seed 20260813)

PRISTINE ⇒ 17/17, exit 0

 [4a] Decrypt: Python-encrypted record
PASS: plaintext recovered, type=23
[4b] Decrypt: tampered ciphertext (tag verify fail)
PASS: decrypt returned C=1 (tag mismatch)
[4c] Decrypt: application keys (state=CONNECTED)
PASS: plaintext recovered (40 bytes), type=23
OK: 3/3 passed
...
RESULTS
============================================================
Passed: 17/17
Failed: 0/17
[+] TLS RECORD LAYER: ALL 17 TESTS PASSED

(EXIT=0)

MUTANT ⇒ 4b FAILS, exit 1

 [4a] Decrypt: Python-encrypted record
PASS: plaintext recovered, type=23
[4b] Decrypt: tampered ciphertext (tag verify fail)
FAIL: decrypt returned C=0 (should be C=1 for tampered data)
(computed tag != record tag, so the tamper was detectable but not rejected)
[4c] Decrypt: application keys (state=CONNECTED)
PASS: plaintext recovered (40 bytes), type=23
FAIL: 2/3 passed
...
RESULTS
============================================================
Passed: 16/17
Failed: 1/17
[-] TLS RECORD LAYER: 1 TEST(S) FAILED

(EXIT=1)

The mutant's diagnostic line is the defect stated out loud: the tags did
differ, which is exactly the condition the old fallback scored as a PASS.

Blast radius

  • 4b now takes the carry path on VICE for the first time, and the pristine run
    confirms tls_record_decrypt really does return C=1 on tag mismatch — the
    previously-unexercised assertion is correct.
  • default_vice_config() adds -reu -reusize 512 to this suite's VICE launch;
    17/17 with it, so no behavioural change beyond matching house style.
  • Environment note for anyone reproducing in a fresh worktree: submodules are
    not populated and ip65-build/ip65-c64.bin is gitignored, so a first run needs
    git submodule update --init --recursive then make ip65-libs && make ip65-blob
    (the blob produced is byte-identical to the main clone's).

tools/test_tls_record.py 4b asserted the carry flag only when the
register dict contained a key named "P". VICE's binary monitor names the
6502 status register "FL", so that branch was dead on every VICE run and
4b always fell through to the fallback oracle, which asserted merely that
the computed tag differed from the record's tag. Tags always differ on
tampered input, so 4b passed whether or not aead_decrypt rejected the
record: with the `bne @auth_fail` tag-rejection branch removed from
src/crypto/aead.s the suite still reported 17/17.
Fix:
- carry_from_regs() looks the status register up under FL/P/FLAGS/SR,
so the primary (and only sound) oracle actually fires.
- The fallback pass path is gone. Not being able to read the carry
means the tamper rejection could not be evaluated, which is a FAIL,
not a pass.
- The tag comparison survives only as diagnostic text printed on a C=0
failure -- it distinguishes "detectable but not rejected" from "tag
never recomputed".
- Launch VICE through _vice_helpers.default_vice_config() like every
other in-tree VICE test, instead of a bare ViceConfig.
A/B (VICE, ip65 default profile, --seed 20260813):
pristine 17/17 pass, exit 0, 4b "PASS: decrypt
returned C=1 (tag mismatch)"
mutant (bne @auth_fail -> nop nop, verified in the linked PRG as the
only two changed bytes, D0 31 -> EA EA at $67A9 = aead_decrypt+9)
16/17, 4b FAIL, exit 1
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JC-000

Copy link
Copy Markdown
OwnerAuthor

Independently reproduced by the supervising session — not accepted on report alone.

Verified in a clean copy of the repo at f0127a0 with only tools/test_tls_record.py taken from this branch:

Pristine sources + fixed testPassed: 17/17, exit 0, and 4b now exercises the real oracle:

[4b] Decrypt: tampered ciphertext (tag verify fail)
PASS: decrypt returned C=1 (tag mismatch)

MUTANT (bne @auth_failnop/nop in aead_decrypt) + fixed testPassed: 16/17, exit 1:

[4b] Decrypt: tampered ciphertext (tag verify fail)
FAIL: decrypt returned C=0 (should be C=1 for tampered data)
(computed tag != record tag, so the tamper was detectable but not rejected)

Mutation confirmed live in the built binary before trusting the run — cmp -l against the pristine PRG shows exactly two changed bytes at offset 24491: D0 31 (bne) → EA EA (nop nop).

The root cause identified here is worth emphasising beyond "the fallback was weak": the primary assertion was gated on if regs and "P" in regs, but VICE's binary monitor names the status register FL (c64-test-harness backends/vice_binary.py:1325 resolves FL/FLAGS/SR). The real oracle was therefore dead code on every VICE run, and 4b fell through to the tag-difference fallback unconditionally. That is the full mechanism behind the mutant's original 17/17.

Confirms audit finding F1.

Sign up for freeto 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

@JC-000