Skip to content

fix(proxy): extract SNI before dial for SNI-deferred connections#14

Merged
nnemirovsky merged 1 commit intomainfrom
fix/sni-defer-dial-ordering
Apr 9, 2026
Merged

fix(proxy): extract SNI before dial for SNI-deferred connections#14
nnemirovsky merged 1 commit intomainfrom
fix/sni-defer-dial-ordering

Conversation

@nnemirovsky
Copy link
Copy Markdown
Owner

@nnemirovsky nnemirovsky commented Apr 9, 2026

Summary

Fixes the recurring bug where SNI-deferred IP connections fail with x509: cannot validate certificate for <IP> because it doesn't contain any IP SANs. This broke OpenClaw's Telegram connection whenever the DNS reverse cache expired.

Root cause: handleConnect called s.dial() before extracting SNI, so the MITM proxy used the raw IP for the upstream TLS ServerName. The real server's cert has DNS SANs only.

Fix: Restructure handleConnect for SNI-deferred connections to extract SNI BEFORE dialing. Also makes SNI the happy path for ALL TLS connections with IP-only CONNECT (no DNS cache dependency for TLS).

Also fixes peekSNI to handle multi-read TLS ClientHello records (some records span multiple TCP segments).

Test plan

  • All unit tests pass
  • Lint passes
  • Build succeeds
  • Deployed to knuth
  • SNI extraction works: [SNI] 149.154.166.110 -> api.telegram.org:443
  • Policy evaluation with hostname: [SNI->ALLOW] hostname matched allow rule
  • Credential injection works: [INJECT] injected credential "telegram_bot_token"
  • No IP SANs errors
  • OpenClaw responds in Telegram
  • SNI-deferred MITM cert plan moved to completed

@nnemirovsky nnemirovsky force-pushed the fix/sni-defer-dial-ordering branch from 41ecfd3 to 2a9c4f7 Compare April 9, 2026 04:18
Restructure handleConnect so that SNI-deferred connections extract the
TLS ClientHello SNI BEFORE dialing through the MITM proxy. Previously,
dial was called first with the raw IP as the CONNECT target, causing
goproxy to use the IP for the upstream TLS ServerName. The real server's
cert has DNS SANs (e.g. *.telegram.org), not IP SANs, so TLS verification
failed with "cannot validate certificate for <IP>".

New flow for SNI-deferred connections:
1. Send SOCKS5 CONNECT success (so client starts TLS handshake)
2. Peek ClientHello to extract SNI hostname
3. Update context FQDN with recovered hostname
4. Evaluate policy with hostname
5. Dial through MITM with hostname (not IP)

Also: refactor relay logic into reusable relayData method, extract
sniSaveRule helper to reduce duplication.

Also: revised ECH DNS hostname recovery plan from review feedback.
@nnemirovsky nnemirovsky force-pushed the fix/sni-defer-dial-ordering branch from 2a9c4f7 to 76dfe9b Compare April 9, 2026 04:21
@nnemirovsky nnemirovsky merged commit b643af1 into main Apr 9, 2026
5 of 6 checks passed
@nnemirovsky nnemirovsky deleted the fix/sni-defer-dial-ordering branch April 9, 2026 04:24
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