Skip to content

fix(eval): classify CONNECT hosts and kill raw TCP tunnels - #3008

Merged
liugddx merged 1 commit into
apache:mainfrom
1625567290:fix/eval-egress-connect-tunnels
Aug 14, 2026
Merged

fix(eval): classify CONNECT hosts and kill raw TCP tunnels#3008
liugddx merged 1 commit into
apache:mainfrom
1625567290:fix/eval-egress-connect-tunnels

Conversation

@1625567290

Copy link
Copy Markdown
Contributor

Summary

egress_filter.py only implemented request, so a CONNECT never hit the contamination rules. A tunnel whose first bytes were neither TLS nor HTTP became raw TCP with no audit record. CONNECT tbench.ai:443 returned 200; CONNECT ssh.github.com:443 forwarded a real SSH banner.

  • http_connect runs the existing host rules and returns 451 for blocklisted CONNECT targets before the tunnel opens.
  • next_layer replaces a raw TCPLayer, tcp_message clears already-buffered payloads, and tcp_start records raw_tunnel. flow.kill() alone does not stop mitmproxy 12.2.3's TCP relay.
  • Unrelated HTTPS through the proxy is unchanged.

Fixes #2977

Verification

  • python3.13 packages/eval/harbor/test_egress_filter.py — 7 pass
  • Live maka-eval-egress-proxy:12.2.3 built from this branch:
    • CONNECT tbench.ai:443451 tbench_domain
    • https://example.com/ via the proxy → 200
    • CONNECT ssh.github.com:443 → 200 then no SSH-2.0 banner; hits.jsonl has raw_tunnel / ssh.github.com

Not run: full maka eval run cohort.

Checklist

  • Tests cover the change and fail without it
  • Focused lint/typecheck and the affected suites pass locally
  • Full workspace lint/format/typecheck

Does this PR entail a change in behavior?

  • Yes — blocklisted CONNECT hosts are refused, and raw TCP tunnels are killed and audited

Refuse blocklisted CONNECT targets before the tunnel opens. Raw TCP
fallbacks — including SSH over 443 — are audited as raw_tunnel, stripped
of payload, and closed. HTTPS interception for unrelated hosts is
unchanged.

Fixes apache#2977

Generated-by: Grok

@liugddx liugddx left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Verdict: merge recommended (non-blocking). The core direction is sound: classify CONNECT before tunnel establishment, then identify, audit, and close raw fallback before relay. Setting rawtcp=false alone would be shorter, but it loses precise raw_tunnel auditing and is therefore not equivalent.

Please consider the inline hardening items before merge or immediately afterward: make the closing-layer contract explicit and add a test that actually verifies CloseConnection commands. The tcp_start/tcp_message + flow.kill path is removable fallback complexity; its mocks prove method invocation, not socket closure. No correctness blocker found.

中文审查结论

结论:建议合并(非阻塞)。核心方向正确:CONNECT 在建立隧道前复用污染规则;raw fallback 在 relay 前被识别、审计并关闭。单独设置 rawtcp=false 虽然更短,但会失去精确的 raw_tunnel 审计,因此不是完整等价方案。

建议在合并前或紧随其后完成行内加固:让关闭层契约更明确,并补上真正验证 CloseConnection 的测试。tcp_start/tcp_message + flow.kill 属于可以删除的冗余兜底;它们的 mock 测试只证明方法被调用,不能证明连接已经关闭。未发现正确性阻塞问题。

Comment thread packages/eval/harbor/test_egress_filter.py
Comment thread packages/eval/harbor/egress_filter.py
Comment thread packages/eval/harbor/egress_filter.py
@liugddx
liugddx merged commit 2632b85 into apache:main Aug 14, 2026
10 checks passed
@1625567290

Copy link
Copy Markdown
Contributor Author

Follow-up for the inline hardening items is #3017.

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.

Raw CONNECT tunnels bypass the Eval contamination filter and the egress audit log

2 participants