Skip to content

ip65_hw_checks dropped five defences from the c64-wireguard reference — one carries an explicit upstream warning against losing it #202

Description

@JC-000

tools/ip65_hw_checks.py (#200) is adapted from c64-wireguard's module of the same name. Five defences did not come across. Recording the reasoning so each is a decision rather than an omission, and flagging the one where the upstream author wrote a warning specifically against losing it.

1. PETSCII encoding forms — flagged, not shrugged

Upstream has petscii_form() and petscii_shifted_form(), so a plaintext search covers the bytes a C64 would actually put on the wire, not just ASCII. Their petscii_shifted_form docstring (tools/ip65_hw_checks.py:425) says in as many words:

"Our payload alphabet is UPPERCASE, so petscii_form … returns the needle unchanged and its search branch is dead on this rig — a leak that left the machine in the shifted block would not be found by any of the other three forms … it must not be lost when that tool moves onto this library."

It was lost. And the condition it names holds here exactly: tests/rig_ip65_rrnet_hw.py's RESPONSE_BODY = "TLS13 OK OVER REAL RRNET" is uppercase, digits and spaces — the alphabet that docstring is about.

Mitigating, and the reason it did not block #200: a leak would have to involve the C64 re-encoding the body before transmitting it, and this topology has the client only receiving that body and rendering it locally, so there is no obvious path that produces shifted-block bytes on the wire. That is an argument about this rig's shape, not about the search being complete — and the upstream author anticipated exactly this reasoning and wrote against relying on it.

Recommendation: port both forms. They are pure functions over bytes, they cost nothing at runtime, and each needs one red case.

2. _longest_run / Finding — partial-match reporting

Upstream reports a partial leak: the longest run of the secret found in one datagram, above a floor (PARTIAL_RUN_MIN = 8). Ours is a whole-needle in test, so a 23-of-24-byte leak reads as absent. That is a false negative in the one check whose entire job is an absence claim.

Recommendation: port it, with the same floor and the same rationale (below ~8 bytes the report is noise from short common substrings).

3. check_net_counters — ip65 send-attempt / recv-dropped counters

Upstream reads ip65_send_attempts / ip65_recv_dropped and reasons about them. Ours reads net_last_error only. On a first-silicon run these counters are precisely the signal you want: they distinguish "the driver never tried" from "it tried and the wire ate it", which is the ambiguity a cartridge run is most likely to hit.

Note this needs the symbols to exist in our build — they are c64-wireguard exports and did not resolve in our build/labels.txt when #200's resolve_symbols ran, so porting the check means adding the counters to our ip65 adapter first.

4. check_ip65_config_written — netmask, gateway, cfg_mac

Upstream asserts that none of ip65's four config fields still holds its build-time constant. Ours checks the IP alone (check_dhcp_lease). A cfg_mac still reading 00:80:10:00:51:00 means eth_init never ran — the absence of a MAC rather than a MAC — and we would currently not notice.

Partly mitigated: our check_mac_on_wire rejects that same default by value. But it is fed a constant from the rig script rather than a value read from the device, so today it cannot actually observe the condition (see its docstring, and #200's note that it contributes no independent information as called).

5. check_cs8900a_identified — the chip's product ID

Deliberately not ported, and this one I would leave out. Upstream's own module is clear that the only valid presence check runs on the 6510, and reading the product ID from the host over DMA does not do that — c64-wireguard measured $DE00 from the host as open bus: seven observations across three observers, no two agreeing, not reflecting its own writes. A working DHCP exchange over the cable is a far stronger statement that the chip is present and addressable, and #200's run has one.

Priority

1 and 2 are the ones with teeth, and 1 has an upstream warning attached. 3 and 4 need adapter work first. 5 should be recorded as declined.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions