#161: make three VICE suites measure what they claim (mutation-proven)

src/tls_cert.s

@@ -77,11 +77,14 @@
 ; tls_hs_ptr_reset - point tls_hs_ptr at tls_rec_buf
 ;
 ; The pre-deframer contract: every handshake message starts at
-; tls_rec_buf[0].  Handlers call this at entry so that direct callers
-; (tools/test_tls_p384_negotiation.py, tools/test_finished_verify.py —
-; both drive handlers over DMA with no dispatcher in the loop) keep
-; working unchanged.  Lives in CRYPTO_CODE, not TLS_CODE: the NET_CODE
-; region that hosts TLS_CODE under UCI has ~8 bytes of slack.
+; tls_rec_buf[0].  Handlers call this at entry ONLY on a non-streaming
+; build (.ifndef TLS_STREAM_DEFRAME); under BACKEND=uci the deframer owns
+; tls_hs_ptr and the resets are compiled out.  A direct caller that drives
+; a handler over DMA with no dispatcher in the loop must therefore call
+; this itself rather than assume the handler did — see issue #161, and
+; tools/test_finished_verify.py's carry stub, which jsr's here first.
+; Lives in CRYPTO_CODE, not TLS_CODE: the NET_CODE region that hosts
+; TLS_CODE under UCI has ~8 bytes of slack.
 ; Clobbers: A
 ; =============================================================================
         .segment "CRYPTO_CODE"

src/tls_keyschedule.s

@@ -710,10 +710,24 @@ tls_compute_finished:
 ; Input: tls_s_hs_secret = server handshake traffic secret
 ;        tls_transcript = transcript hash up to (but not including) Finished
 ;        (tls_hs_ptr)+4 = received verify_data (32 bytes, after 4-byte HS
-;        header).  The pointer is reset to tls_rec_buf at entry (see
-;        tls_hs_ptr_reset) so direct callers — tools/test_finished_verify.py
-;        drives this routine over DMA with no dispatcher in the loop —
-;        keep the historical tls_rec_buf+4 contract unchanged.
+;        header).
+;
+;        WHO SETS tls_hs_ptr, and why it is not always us (issue #161).
+;        On a NON-streaming build this routine calls tls_hs_ptr_reset at
+;        entry, so the historical "message starts at tls_rec_buf" contract
+;        holds for free.  Under TLS_STREAM_DEFRAME (BACKEND=uci — the
+;        backend that ships to hardware) that reset is compiled out: the
+;        deframer owns tls_hs_ptr and points it at the message before
+;        dispatch, in-place or into df_carry_buf, and re-resetting it here
+;        would read the wrong 32 bytes.
+;
+;        A DIRECT caller with no deframer in the loop must therefore
+;        establish tls_hs_ptr ITSELF — jsr tls_hs_ptr_reset first is
+;        enough, and is what tools/test_finished_verify.py's carry stub
+;        does.  It is not optional under uci: without it the compare runs
+;        against whatever the pointer last held, every vector "fails to
+;        match", and a negative-case suite goes green while measuring
+;        nothing (the 16-vacuous-negatives half of #161).
 ; Output: C=0 if match (server Finished is valid)
 ;         C=1 if mismatch (verification failed)
 ;

tools/https_e2e/chain_certs.py

@@ -88,11 +88,21 @@ def _gen_certs():
     return gen_certs
 
 
-def build_padded_intermediate(cn: str, pad_bytes: int) -> bytes:
+def build_padded_intermediate(cn: str, pad_bytes: int,
+                              sans: list[str] | None = None) -> bytes:
     """Mint one self-signed throwaway P-256 cert of ~(420 + pad_bytes) B.
 
     Returns the DER. The key is generated and discarded — these certs are
     padding, not signers of anything.
+
+    *sans* adds a subjectAltName extension carrying those dNSNames. The
+    padding intermediates of ``ensure_chain_certs`` want none (nothing
+    validates a name on them, and the C64 discards every entry after the
+    leaf), so it defaults to None and their bytes are unchanged. It exists
+    for callers that mint a cert to be used AS A LEAF against a build with
+    X509_VERIFY_NAME — ``tools/test_tls_deframer.py``'s wiki-sized leaf.
+    Such a cert needs a SAN or src/x509_name.s rejects it outright, exactly
+    as a browser rejects a SAN-less certificate (issue #161).
     """
     g = _gen_certs()
     crv = g.CURVES["p256"]
@@ -113,10 +123,18 @@ def build_padded_intermediate(cn: str, pad_bytes: int) -> bytes:
         g._seq(g._oid(g.OID_EC_PUBLIC_KEY), g._oid(g.OID_PRIME256V1)),
         g._bitstring(pub_point),
     )
-    # One non-critical private-OID extension carrying opaque padding.
-    extensions = g._explicit(3, g._seq(
-        g._seq(g._oid(_PAD_OID), g._octetstring(b"\x5a" * pad_bytes)),
-    ))
+    # One non-critical private-OID extension carrying opaque padding,
+    # optionally preceded by a subjectAltName (see the docstring).
+    ext_list = []
+    if sans:
+        # GeneralNames: dNSName is [2] IMPLICIT IA5String, i.e. tag 0x82 —
+        # same encoding gen_certs uses for the listener leaf.
+        san_value = g._seq(*[g._tlv(0x82, h.encode("ascii")) for h in sans])
+        ext_list.append(
+            g._seq(g._oid(g.OID_SUBJECT_ALT_NAME), g._octetstring(san_value)))
+    ext_list.append(
+        g._seq(g._oid(_PAD_OID), g._octetstring(b"\x5a" * pad_bytes)))
+    extensions = g._explicit(3, g._seq(*ext_list))
     tbs = g._seq(
         g._explicit(0, g._int(2)),
         g._int(secrets.randbits(159) | 1),

tools/test_finished_verify.py

@@ -94,6 +94,7 @@
 
 REQUIRED_LABELS = [
     "tls_verify_finished",
+    "tls_hs_ptr_reset",
     "tls_verify_data",
     "tls_s_hs_secret",
     "tls_transcript",
@@ -102,10 +103,11 @@
 
 # Cassette buffer. $033C-$03FB is free once BASIC has booted. The harness's
 # own jsr() trampoline lives at $0334 (5 bytes) and run_subroutine's U64
-# trampoline at $0360 (14 bytes) with flags at $03F0/$03F1 — $0340 and $034C
-# collide with none of them.
+# trampoline at $0360 (14 bytes) with flags at $03F0/$03F1 — $0340 and $0350
+# collide with none of them. The stub is 13 bytes ($0340-$034C), so the
+# latch sits at $0350.
 CARRY_STUB_ADDR = 0x0340
-CARRY_RESULT_ADDR = 0x034C
+CARRY_RESULT_ADDR = 0x0350
 
 
 # ---------------------------------------------------------------------------
@@ -197,22 +199,51 @@ def build_cases(secret: bytes, transcript: bytes):
 # C64 plumbing
 # ---------------------------------------------------------------------------
 
-def install_carry_stub(transport, target_addr: int) -> None:
+def install_carry_stub(transport, target_addr: int,
+                       hs_ptr_reset_addr: int) -> None:
     """Install a stub that calls *target_addr* and latches the carry flag.
 
-        JSR target      20 lo hi
-        LDA #$00        A9 00
-        ROL A           2A        ; carry -> bit 0
-        STA result      8D lo hi
-        RTS             60
+        JSR tls_hs_ptr_reset    20 lo hi  ; establish the (tls_hs_ptr)+4 input
+        JSR target              20 lo hi
+        LDA #$00                A9 00
+        ROL A                   2A        ; carry -> bit 0
+        STA result              8D lo hi
+        RTS                     60
 
     Reading the P register back over the monitor is unreliable across
     backends; latching the flag into RAM from 6502 code is not. The stub is
     written once and reused for every case.
+
+    The leading ``jsr tls_hs_ptr_reset`` is load-bearing, not decoration
+    (issue #161). ``tls_verify_finished`` reads the received verify_data
+    through ``(tls_hs_ptr)+4``, and it resets that pointer itself only on
+    a NON-streaming build::
+
+        src/tls_keyschedule.s   tls_verify_finished:
+                                .ifndef TLS_STREAM_DEFRAME
+                                        jsr tls_hs_ptr_reset
+                                .endif
+
+    Under ``TLS_STREAM_DEFRAME`` (BACKEND=uci — the backend that ships to
+    hardware) the deframer owns ``tls_hs_ptr`` and sets it per message
+    before dispatch, so the reset is compiled out. This rig has no
+    deframer in the loop: without the call below the routine compares 32
+    bytes at whatever address the pointer happened to hold, which is not
+    where the rig wrote its vector. Every negative case then "rejects"
+    for a reason that has nothing to do with the vector — 16/18 green
+    while measuring nothing, and a mutation that compares only byte 0 of
+    the verify_data passes unnoticed (issue #161, mutation M-C).
+
+    Calling the repo's own ``tls_hs_ptr_reset`` rather than poking $3E/$3F
+    from Python keeps one copy of the fact: if the base ever moves, the
+    rig follows it.
     """
     lo, hi = target_addr & 0xFF, (target_addr >> 8) & 0xFF
+    plo, phi = hs_ptr_reset_addr & 0xFF, (hs_ptr_reset_addr >> 8) & 0xFF
     rlo, rhi = CARRY_RESULT_ADDR & 0xFF, (CARRY_RESULT_ADDR >> 8) & 0xFF
-    stub = bytes([0x20, lo, hi, 0xA9, 0x00, 0x2A, 0x8D, rlo, rhi, 0x60])
+    stub = bytes([0x20, plo, phi,
+                  0x20, lo, hi,
+                  0xA9, 0x00, 0x2A, 0x8D, rlo, rhi, 0x60])
     write_bytes(transport, CARRY_STUB_ADDR, stub)
     readback = read_bytes(transport, CARRY_STUB_ADDR, len(stub))
     if readback != stub:
@@ -253,7 +284,8 @@ def call_verify_finished(transport, labels, secret: bytes, transcript: bytes,
 def run_tests(transport, labels) -> tuple[int, int]:
     passed = failed = 0
 
-    install_carry_stub(transport, labels["tls_verify_finished"])
+    install_carry_stub(transport, labels["tls_verify_finished"],
+                       labels["tls_hs_ptr_reset"])
 
     for set_name, secret, transcript in VECTOR_SETS:
         print(f"\n--- Vector set {set_name} ---")

tools/test_tls_deframe.py

@@ -77,8 +77,21 @@
     "cert_buf", "cert_buf_len", "cert_buf_size",
     "ecdsa_pubkey_x", "ecdsa_pubkey_y", "ecdsa_curve_id",
     "tls_state",
+    "tls_hostname", "tls_hostname_len",
 ]
 
+# The name every Certificate this rig mints is issued FOR, and the name the
+# rig tells the client it asked for. ONE constant on purpose (issue #161):
+# it is written into the certificate's SAN and into tls_hostname, so the two
+# cannot drift into "the cert says one thing, the client wants another" —
+# which is indistinguishable, at the DF_ERR_CERT_FMT the deframer returns,
+# from the certificate being malformed.
+#
+# `.invalid` is reserved by RFC 2606 s2 / RFC 6761 s6.4 and can never
+# resolve; see tools/test_reserved_test_host.py for why every name this
+# project writes down lives under a reserved TLD.
+CERT_HOST = "deframe.foo.invalid"
+
 # Handshake message types
 HS_EE = 8
 HS_CERT = 11
@@ -158,10 +171,19 @@ def chunks(data: bytes, n: int):
 
 
 def generate_p256_cert():
-    """Self-signed ECDSA P-256 cert (same recipe as test_x509.py)."""
+    """Self-signed ECDSA P-256 cert with a SAN naming CERT_HOST.
+
+    The SAN is not cosmetic. Under BACKEND=uci (X509_VERIFY_NAME) the
+    public-key extraction tail-calls x509_verify_hostname, whose carry IS
+    the Certificate handler's result, and that routine rejects a leaf with
+    no subjectAltName exactly as a browser does. A CN-only fixture — what
+    this helper minted before issue #161 — is refused by name validation
+    long before any deframer behaviour is observed, and the refusal arrives
+    as the same DF_ERR_CERT_FMT a malformed message would produce.
+    """
     key = ec.generate_private_key(ec.SECP256R1())
     subject = x509.Name([
-        x509.NameAttribute(NameOID.COMMON_NAME, "test.example.com"),
+        x509.NameAttribute(NameOID.COMMON_NAME, CERT_HOST),
     ])
     cert = (x509.CertificateBuilder()
             .subject_name(subject)
@@ -171,6 +193,9 @@ def generate_p256_cert():
             .not_valid_before(datetime.datetime.now(datetime.UTC))
             .not_valid_after(datetime.datetime.now(datetime.UTC)
                              + datetime.timedelta(days=365))
+            .add_extension(
+                x509.SubjectAlternativeName([x509.DNSName(CERT_HOST)]),
+                critical=False)
             .sign(key, hashes.SHA256()))
     der = cert.public_bytes(serialization.Encoding.DER)
     nums = key.public_key().public_numbers()
@@ -298,9 +323,42 @@ def hs_ptr(self) -> int:
 # Cases
 # ---------------------------------------------------------------------------
 
+def install_hostname(transport, labels, host: str = CERT_HOST) -> None:
+    """Populate tls_hostname / tls_hostname_len — a precondition, not a knob.
+
+    http_get writes these at runtime for both the menu path and the DMA
+    trampoline the hardware rigs use; this rig calls neither, so before
+    issue #161 they were zero. src/x509_name.s opens with::
+
+        lda tls_hostname_len            ; nothing to validate against
+        bne :+
+        sec
+        rts
+
+    so every Certificate was rejected before a single SAN byte was read.
+    The W2 acceptance cases then failed for a reason that has nothing to do
+    with the deframer, and — worse — the W2 *rejection* cases passed
+    without their guards ever running: deleting the
+    certificate_request_context zero-check from src/tls_deframe.s left
+    "non-zero request context rejected (streamed)" green (issue #161,
+    mutation M-B).
+
+    64 bytes is the tls_hostname buffer (src/tls_handshake.s); the whole
+    buffer is written so a longer name from an earlier run cannot leave a
+    tail behind.
+    """
+    raw = host.encode("ascii")
+    assert len(raw) < 64, "tls_hostname is 64 bytes (src/tls_handshake.s)"
+    write_bytes(transport, labels["tls_hostname"], raw.ljust(64, b"\x00"))
+    write_bytes(transport, labels["tls_hostname_len"], bytes([len(raw)]))
+
+
 def run_cases(transport, labels):
     rig = Rig(transport, labels)
     passed = failed = 0
+
+    # Precondition, established once for every case below (issue #161).
+    install_hostname(transport, labels)
     fed_ok = []                 # messages whose bytes should be in transcript
 
     def check(name, cond, detail=""):