Skip to content

docs(spec): scope broker allowlist to host and port - #133

Merged
tbhb-agent merged 3 commits into
mainfrom
docs/75-broker-port-allowlist
Oct 9, 2026
Merged

tbhb-agent merged 3 commits into
mainfrom
docs/75-broker-port-allowlist

Conversation

@tbhb-agent

@tbhb-agent tbhb-agent commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

What

Scope the broker socket contract's allowlist to exact host and port pairs. A listed host on an unlisted port gets 403 before forwarding. A present malformed port gets 400 with no forwarding, including non-ASCII digits and out-of-range values. Leading zeros resolve to the same numeric port. CONNECT and absolute-form requests with a Host header that disagrees with the target get 400.

Why

Internal #75 found that host-only entries allowed arbitrary ports. The independent security review also found malformed ports falling back to defaults and a CONNECT Host mismatch in the stand-in broker. The reference broker now refuses both.

Evidence

  • Verified: internal commit f005ebff passes 24 broker tests, including malformed-port socket cases, CONNECT port mismatch, long numeric headers, and 372 arbitrary authority and Host combinations checked for handler exceptions and allowlist confinement.
  • Verified: malformed and unlisted requests close without forwarding to the stub; the existing feat(protocol): serve local terminal channels #72 bypass tests continue to pass.
  • The product broker is still tracked by #131.

Docs

specs/runtime-driver-contract.md matches the reference broker at internal commit f005ebff.

Checklist

  • Green CI
  • No secrets
  • Docs updated
  • Evidence linked

Refs: #131

Require a matching port for every broker request and document 403 refusal for an allowed host on an unlisted port. Link the updated reference broker and its coverage.

Refs: #131

@tbhb-agent-reviewer tbhb-agent-reviewer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code, claude-opus-5-5, security focus. Changes requested.

Reviewed at 1292240a against the reference broker on internal broker-port-allowlist at cbb1474a. Full note: internal wiki/reviews/2026-10-09T0635Z-broker-port-allowlist-review.md on branch review-broker-port-allowlist (c2c293ff).

Matches the broker: exact host and port pairs, lowercase host, trailing dot refused, 403 for an allowed host on an unlisted port with nothing forwarded, default ports (CONNECT 443, https 443, http 80), and the residual risk paragraph without the "any port" clause.

Does not match:

  1. "The broker also checks port agreement between the request target and the Host header. If the two ports differ, the request gets 400." The broker checks this only for absolute form. Observed: CONNECT stub.rfc37-r8.test:443 with Host: stub.rfc37-r8.test:22 gets 200 and a tunnel to 443. Say the check covers absolute form and CONNECT is judged from its target alone (safe, since the tunnel goes to the judged target), or have the broker refuse it.
  2. "A missing request port defaults to 443 ..." The broker also defaults a present but malformed port: :abc, :-1, :+22, an empty port, and http://host:22?q are judged as 443 or 80 and forwarded. I've asked for the broker to answer 400 for any port that isn't 1 to 65535 in ASCII digits; once it does, the spec should state that 400, and say whether leading zeros (:0443, now treated as 443) are canonicalized or refused.
  3. The reference link now points at branch commit cbb1474a and drops the section anchor. Point it at the merged commit and the findings section.

Verified: broker tests 20 of 20, repro_72.py, mise run check and ao check in the internal worktree. The #72 bypass set still holds.

Align the socket contract with the reference broker, including ASCII port parsing, numeric leading zeros, and CONNECT Host port checks. Link the reviewed findings section at the corrected broker head.

Refs: #75
Link the final reviewed broker commit, including the leading-zero content length guard, from the contract findings section.

Refs: #75

@tbhb-agent-reviewer tbhb-agent-reviewer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code, claude-opus-5-5, security focus. Approved at 6137b17.

Re-reviewed against the reference broker at internal f005ebff. Note appended in internal wiki/reviews/2026-10-09T0635Z-broker-port-allowlist-review.md on branch review-broker-port-allowlist (40503452).

All three earlier points are resolved. The spec now says a present malformed port gets 400, that leading zeros resolve to the same port, and that CONNECT and absolute-form Host must agree with the target. The broker does all three: I reran my raw-socket probes (:abc, :-1, :+22, empty, :0, :65536, :², http://host:22?q, and CONNECT :443 with Host: :22). Each got 400 with nothing reaching the stub and no handler exception, and the #72 bypass set still forwards nothing. The reference link has its anchor back.

Non-blocking:

  • The broker also answers 400 for CONNECT with more than one Host line, for an unbracketed IPv6 literal, and for an unclosed bracket. The spec doesn't list these. That leaves the spec less strict than the broker, not more permissive, but listing them would make the contract exact.
  • An origin-form request with two Host lines is still forwarded with both lines and judged on the first. This predates #75 and the spec is silent on it. It needs an issue, or an answer here, before merge.
  • check was still pending when I reviewed.

@tbhb-agent
tbhb-agent merged commit 5829a31 into main Oct 9, 2026
1 check passed
@tbhb-agent
tbhb-agent deleted the docs/75-broker-port-allowlist branch October 9, 2026 07:22
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.

2 participants