Skip to content

Review follow-ups: IPv6 peers, constant-time admin key, magnet hash validation#6

Merged
iksnerd merged 1 commit into
mainfrom
fix/review-followups
Jun 13, 2026
Merged

Review follow-ups: IPv6 peers, constant-time admin key, magnet hash validation#6
iksnerd merged 1 commit into
mainfrom
fix/review-followups

Conversation

@iksnerd

@iksnerd iksnerd commented Jun 13, 2026

Copy link
Copy Markdown
Owner

The lower-priority findings from the 2026-06-13 review (follow-up to #5).

Changes

IPv6 peer supportinternal/client/tracker.go
unpackPeers only handled the 6-byte IPv4 compact form and the tracker response struct never decoded the peers6 key, so IPv6 swarms were silently dropped. Now decodes BEP 7 peers6 and parses both the 6-byte (IPv4) and 18-byte (IPv6) compact forms through a length-parameterized unpacker. IPv6 test added.

Constant-time admin-key compareinternal/tracker/registry.go
The registry POST/DELETE X-Weightless-Key check used a plain != string compare (timing side-channel). Switched to subtle.ConstantTimeCompare, matching the HMAC announce path.

Magnet v1-hash validationcmd/wl/main.go
wl get discarded the hex.DecodeString error for the v1 info hash, handing a truncated/garbage hash to the downloader. Now rejects a malformed value at the boundary; an empty v1 hash (v2-only magnet) is still accepted.

Dropped (false positive)

The review's wire.go "payload cap comment mismatch" — 2<<20 is exactly 2 MiB, which matches the "2MB max payload" comment. No change.

Verification

  • go build ./..., go vet ./..., gofmt clean
  • go test ./... — all pass, including the new TestUnpackPeersIPv6

🤖 Generated with Claude Code

…alidation

Three lower-priority findings from the 2026-06-13 review:

- internal/client: unpackPeers only handled the 6-byte IPv4 compact form and
  the tracker response never decoded peers6, so IPv6 swarms were silently
  dropped. Decode the BEP 7 peers6 key and parse both forms (4- and 16-byte
  addresses) via a length-parameterized unpacker. Adds an IPv6 test.

- internal/tracker: registry POST/DELETE admin-key check used a plain string
  compare (timing side-channel); switch to subtle.ConstantTimeCompare for
  parity with the HMAC announce path.

- cmd/wl: 'wl get' discarded the hex.DecodeString error for the v1 info hash,
  handing a truncated hash to the downloader. Reject a malformed value at the
  boundary (empty v1 hash for v2-only magnets is still fine).

The wire.go payload-cap 'comment mismatch' from the review was a false
positive: 2<<20 is exactly 2 MiB, matching the comment.
@iksnerd iksnerd merged commit db7045c into main Jun 13, 2026
1 check passed
@iksnerd iksnerd deleted the fix/review-followups branch June 13, 2026 11:05
iksnerd added a commit that referenced this pull request Jun 21, 2026
Review follow-ups: IPv6 peers, constant-time admin key, magnet hash validation
Sign up for free to 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