From f0bcf8ada4e012125b510b9d5898116423c63803 Mon Sep 17 00:00:00 2001 From: JC-000 <3798556+JC-000@users.noreply.github.com> Date: Thu, 13 Aug 2026 06:11:47 -0500 Subject: [PATCH] fix(test): AEAD tamper oracle in test 4b must fail closed (audit F1) 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) --- tools/test_tls_record.py | 74 ++++++++++++++++++++++++++++++---------- 1 file changed, 56 insertions(+), 18 deletions(-) diff --git a/tools/test_tls_record.py b/tools/test_tls_record.py index df8b527..8cf2419 100644 --- a/tools/test_tls_record.py +++ b/tools/test_tls_record.py @@ -21,7 +21,6 @@ from c64_test_harness import ( Labels, - ViceConfig, ViceInstanceManager, read_bytes, write_bytes, @@ -33,6 +32,8 @@ wait_for_text, ) +from _vice_helpers import default_vice_config + # --------------------------------------------------------------------------- # Constants # --------------------------------------------------------------------------- @@ -75,6 +76,32 @@ "tls_seq_increment", ] +# Names the 6502 processor-status register can appear under in a +# harness register dict. VICE's binary monitor calls it "FL"; other +# backends use "P" / "FLAGS" / "SR". The original code only looked for +# "P", so on VICE the lookup never matched and test 4b silently fell +# through to a weaker oracle. +STATUS_REG_NAMES = ("FL", "P", "FLAGS", "SR") + + +def carry_from_regs(regs): + """Return the carry flag (0/1) from a harness register dict. + + Returns ``None`` when no processor-status register is present. A + caller that cannot read the carry has *not* observed the routine's + accept/reject decision, so ``None`` must be treated as a failed + test, never as a pass: the tag-comparison fallback this replaces + asserted only that the computed tag differed from the record's tag, + which is true of any tampered input whether or not ``aead_decrypt`` + rejected it (audit finding F1). + """ + if not regs: + return None + for name in STATUS_REG_NAMES: + if name in regs: + return regs[name] & 0x01 + return None + # --------------------------------------------------------------------------- # Python reference implementations @@ -487,27 +514,35 @@ def test_record_decrypt(transport, labels, rng): regs = jsr(transport, labels["tls_record_decrypt"], timeout=120.0) - # Expect carry flag set (C=1) indicating AEAD failure - # The carry flag is bit 0 of the status register (P) - if regs and "P" in regs: - carry = regs["P"] & 0x01 - if carry: - passed += 1 - print(" PASS: decrypt returned C=1 (tag mismatch)") - else: - failed += 1 - print(" FAIL: decrypt returned C=0 (should be C=1 " - "for tampered data)") + # The ONLY sound oracle here is the carry flag returned by + # tls_record_decrypt: C=1 means the record was rejected. There is + # deliberately no fallback oracle -- see carry_from_regs() and the + # note above it. + carry = carry_from_regs(regs) + if carry is None: + failed += 1 + print(" FAIL: could not read the 6502 status register " + f"(register names seen: {sorted(regs) if regs else 'none'}; " + f"looked for {'/'.join(STATUS_REG_NAMES)}). The tamper " + "rejection could not be evaluated, which is not a pass.") + elif carry: + passed += 1 + print(" PASS: decrypt returned C=1 (tag mismatch)") else: - # If we can't read P, check if tag comparison area differs + failed += 1 + print(" FAIL: decrypt returned C=0 (should be C=1 " + "for tampered data)") + # Diagnostic only -- never a pass criterion. Differing tags + # say the tamper was *detectable*, not that decrypt rejected + # the record. c64_tag = read_bytes(transport, labels["poly1305_tag"], 16) aead_tag = read_bytes(transport, labels["aead_tag"], 16) if c64_tag != aead_tag: - passed += 1 - print(" PASS: tags differ (tamper detected)") + print(" (computed tag != record tag, so the " + "tamper was detectable but not rejected)") else: - failed += 1 - print(" FAIL: tags match despite tampered ciphertext") + print(" (computed tag == record tag: the tag " + "was never recomputed over the tampered ciphertext)") except Exception as e: failed += 1 print(f" FAIL: {e}") @@ -765,7 +800,10 @@ def main(): print(f" Labels loaded: {len(REQUIRED_LABELS)} required labels verified") # Launch VICE - config = ViceConfig(prg_path=PRG_PATH, warp=True, ntsc=True, sound=False) + # default_vice_config() applies the mandatory -reu/-reusize=512 flags; + # see tools/_vice_helpers.py for the rationale. + config = default_vice_config(prg_path=PRG_PATH, warp=True, ntsc=True, + sound=False) print(f"\n=== Starting VICE ===") with ViceInstanceManager(config=config) as mgr: