Skip to content

fix(test): ServerHello oracle — dead carry gate + stub skip-as-pass (audit F10) - #79

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

fix(test): ServerHello oracle — dead carry gate + stub skip-as-pass (audit F10)#79
JC-000 merged 1 commit into
docs/benchmark-refreshfrom
fix/audit-f10-handshake-oracle

Conversation

@JC-000

Copy link
Copy Markdown
Owner

F10 — the ServerHello oracle: a dead carry gate hiding a skip-as-pass

Two defects in tools/test_tls_handshake.py, fixed together because neither is
visible while the other stands.

1. The carry gate was dead. Test 3a read
carry = 0; if regs and "P" in regs: carry = regs["P"] & 0x01. VICE's binary
monitor names the 6502 status register FL (harness
backends/vice_binary.py:1325), so the lookup never matched, carry stayed
pinned at 0, and the else branch — "parser returned C=1 for a valid
ServerHello" — was unreachable. Same root cause as F1 in test_tls_record.py.
3a now reads the carry through this file's own jsr_check_carry() trampoline
(LDA #0 / ROL A / STA $033F), which captures C on the 6502 itself and has no
dependency on register naming.

2. Behind it, a skip-as-pass. Both clc; rts stub checks answered a
detected stub with return 0, 0, zeroing the group's passed and failed
counters, so the group left the denominator entirely. report_if_stub() now
prints a loud STUB: diagnostic naming the routine and the tests run anyway,
failing on their own merits.

Why one PR. Fixing (1) alone makes the else branch reachable for the first
time, at which point a stub lands in (2) and silently vanishes instead of
failing — a dead assertion traded for a silent skip.

Also in scope: the identical stub-skip on tls_derive_handshake_keys (same
file, same shape, mechanical), and the VICE launch moved to
_vice_helpers.default_vice_config() like the other in-tree VICE tests.

Stub effectiveness — proven in the built binary

tls_parse_server_hello (src/tls_handshake.s:338) prefixed with clc; rts
exactly what the deleted skip branch was written to detect. build/labels.txt
puts the label at $1491; reading the built PRG at that address:

label tls_parse_server_hello = $1491
pristine bytes at label: a000ad00 <- ldy #0 ...
stub bytes at label: 1860a000 <- clc; rts
stub is clc;rts (18 60): True

The suite then ran with C64_SKIP_BUILD=1, so the binary under test is the one
proven above. The mutation lived in the working tree only and was reverted before
the commit.

Acceptance A/B (VICE, ip65 default profile)

PRISTINE ⇒ 21/21, exit 0

 [3a] ServerHello: valid parse (x25519, cipher 0x1303)
PASS: server_random and pubkey extracted correctly
[3b] ServerHello: wrong cipher suite -> error
PASS: returned C=1 for wrong cipher suite
[3c] ServerHello: missing key_share -> error
PASS: returned C=1 for missing key_share
OK: 3/3 passed
...
RESULTS
============================================================
Passed: 21/21
Failed: 0/21
[+] TLS HANDSHAKE: ALL 21 TESTS PASSED

(EXIT=0)

STUB ⇒ group 0/3, exit 1, stub named

 STUB: tls_parse_server_hello is a `clc; rts` stub -- unimplemented. The tests below still run and are expected to FAIL; a stub is never a skip and never a pass.
[3a] ServerHello: valid parse (x25519, cipher 0x1303)
FAIL: server_random mismatch
expected: 6515429011859779...
got: 0000000000000000...
FAIL: server_pubkey mismatch
expected: e588c73b81890d13...
got: 87678fed52472af5...
[3b] ServerHello: wrong cipher suite -> error
FAIL: returned C=0, expected C=1 for wrong cipher
[3c] ServerHello: missing key_share -> error
FAIL: returned C=0, expected C=1 for missing key_share
FAIL: 0/3 passed
...
RESULTS
============================================================
Passed: 18/21
Failed: 3/21
[-] TLS HANDSHAKE: 3 TEST(S) FAILED

(EXIT=1)

The defect, demonstrated on the same binary

The pre-fix test code (git show origin/docs/benchmark-refresh:tools/test_tls_handshake.py)
run against the identical stub PRG, C64_SKIP_BUILD=1:

 [3a] ServerHello: valid parse (x25519, cipher 0x1303)
FAIL: server_random mismatch
expected: 694017f2688548b4...
got: 0000000000000000...
FAIL: server_pubkey mismatch
expected: 554c1c275147ea63...
got: ccb8b8cd701fc1e8...
SKIP: remaining ServerHello tests (stub implementation)
...
RESULTS
============================================================
Passed: 18/18
Failed: 0/18
[+] TLS HANDSHAKE: ALL 18 TESTS PASSED

(EXIT=0)

It printed two FAIL lines and then reported a clean pass: the return 0, 0
discarded 3a's failures along with the group, and the denominator silently shrank
21 → 18. That is the whole finding in one screen.

Does the newly-reachable error branch expose anything?

  • In the parser, no. Pristine 3a passes with C=0, so the real
    tls_parse_server_hello satisfies the assertion that was never being made. No
    latent bug was being masked by the dead gate.
  • In the test, yes — the skip-as-pass in (2), which had never executed
    before, since nothing could reach it.
  • Worth noting what 3a was actually testing: only the buffer comparison. With
    carry pinned to 0 it always took the success path, so "parser reported
    success" was assumed, not checked. It is checked now.
  • Unrelated observation, not fixed here:test_key_schedule (the RFC 8448
    tls_derive_handshake_keys group, 5 tests) is defined but never registered in
    test_groups — dead code that has never run. I repaired its stub-skip for
    consistency, but someone should decide whether to wire it up or delete it.
  • The remaining skip-as-pass surface in this file is the missing-label kind
    (check_labels(...) → return 0, 0), which is F3's shape. Left alone
    deliberately so it follows whatever convention Lane C lands for F3 rather than
    inventing a second one.

…-as-pass (audit F10)
tools/test_tls_handshake.py had two interlocking defects in the
ServerHello group, and neither is visible while the other stands.
1. Test 3a read the carry as `carry = 0; if regs and "P" in regs: carry
= regs["P"] & 0x01`. VICE's binary monitor names the 6502 status
register "FL", so the lookup never matched, `carry` stayed pinned at
0, and the C=1 error branch was unreachable. (Same root cause as F1
in test_tls_record.py.) 3a is now read through the file's own
jsr_check_carry() trampoline (LDA #0 / ROL A / STA), which captures C
on the 6502 itself and does not depend on register naming at all.
2. Behind that, both `clc; rts` stub checks answered a detected stub
with `return 0, 0`, zeroing the group's passed AND failed counters so
it vanished from the denominator — a routine that unconditionally
claims success was reported as "ALL 18 TESTS PASSED", exit 0. Fixing
(1) alone would have made that path reachable for the first time and
traded a dead assertion for a silent skip, so both are fixed
together: report_if_stub() prints a loud STUB: diagnostic naming the
routine and the tests run anyway, failing on their own merits.
The identical stub-skip on tls_derive_handshake_keys gets the same
treatment, and the VICE launch now goes through
_vice_helpers.default_vice_config() like the other in-tree VICE tests.
A/B, `tls_parse_server_hello` replaced by a `clc; rts` stub (verified in
the built PRG: bytes at $1491, the label address, read 18 60 where
pristine reads A0 00):
pristine 21/21 pass, exit 0
stub, fixed test 3a/3b/3c all FAIL, group 0/3, 18/21, exit 1,
"STUB: tls_parse_server_hello is a `clc; rts`
stub -- unimplemented"
stub, baseline test (same binary) prints two FAIL lines, then
"SKIP: remaining ServerHello tests" and reports
"ALL 18 TESTS PASSED", exit 0
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@JC-000

Copy link
Copy Markdown
OwnerAuthor

Independently reproduced by the supervising session.

Built a stubbed binary from a clean copy at f0127a0 (tls_parse_server_hello replaced with clc; rts), confirmed the stub is live — bytes at $1491 read 18 60 — then ran both versions of the test file against that same binary:

Pre-fix (origin/docs/benchmark-refresh), exit 0:

 FAIL: server_random mismatch
FAIL: server_pubkey mismatch
SKIP: remaining ServerHello tests (stub implementation)
Passed: 18/18
[+] TLS HANDSHAKE: ALL 18 TESTS PASSED

Post-fix (this branch), same binary, exit 1:

STUB: tls_parse_server_hello is a `clc; rts` stub -- unimplemented. The tests below
still run and are expected to FAIL; a stub is never a skip and never a pass.
Passed: 18/21
Failed: 3/21

The old path printed two genuine failures and then discarded them along with the group; the denominator shrank 21 → 18 and the suite reported success. That is a stronger demonstration than the original finding claimed.

Using the file's own jsr_check_carry() trampoline instead of a register-name list is the better fix — it removes the dependency on harness register naming entirely rather than tracking it, and matches the pattern test_x509.py and test_ecdsa_kat_oracle.py already use.

Two follow-ups raised here and deliberately left alone are worth tracking separately: test_key_schedule is defined but never registered in test_groups (dead code that has never run), and the missing-label skips are F3's shape, left to follow whatever convention #81 lands.

Confirms audit finding F10.

@JC-000
JC-000 merged commit 29dcbcc into docs/benchmark-refreshAug 13, 2026
@JC-000
JC-000 deleted the fix/audit-f10-handshake-oracle branch August 13, 2026 13:30
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