Develop

[abhishek9686]

No description provided.

[tenki-reviewer]

Review complete. 4 potential issues to review.

Files Reviewed: 26
Findings: 4

By Severity:

• 🟠 High: 1
• 🟡 Medium: 3

---

The PR introduces an L7 HTTP CONNECT egress proxy and uplink wire protocol with significant issues: a critical X25519 zero-key vulnerability, TOCTOU DNS rebinding, port-agnostic allowlist bypass, stale session replacement race, and missing freshness checks enabling replay attacks.

Files Reviewed (26 files)

.github/dependabot.yml
README.md
doc.go
docs/PROXY_L7_EGRESS.md
docs/PROXY_PHASE1_ARCHITECTURE.md
l7/connect.go
l7/doc.go
l7/policy.go
l7/server.go
l7/server_test.go
l7/types.go
uplink/client.go
uplink/doc.go
uplink/errors.go
uplink/example_test.go
uplink/frame.go
uplink/frame_test.go
uplink/hellomac.go
uplink/hellomac_test.go
uplink/integration_test.go
uplink/interfaces.go
uplink/noop.go
uplink/protocol.go
uplink/registry.go
uplink/server.go
uplink/types.go

[tenki-reviewer]

Review complete. 4 potential issues to review.

---

Overview

This PR adds two major new packages: l7/ (HTTP CONNECT egress proxy) and uplink/ (wire protocol with X25519/HMAC handshake). The changes span 26 files with 1205 lines of diff.

Critical: X25519 Zero-Key Vulnerability

In uplink/hellomac.go, ComputeHelloProof accepts a zero-valued X25519 private key without validation, producing an all-zero shared secret and trivially forgeable HMAC proof. This completely breaks the authentication handshake.

High Severity

• Stale session race in uplink/server.go — a deferred Detach for a replaced session removes the winning replacement from the registry, causing connection blackholing after reconnect.
• Port-agnostic allowlist bypass in l7/policy.go — the Allowlist check validates domain only, permitting any TCP port on allowed domains and defeating egress port restrictions.

Medium Severity

• DNS rebinding TOCTOU in l7/server.go — hostname ACL check and DNS resolution are unpinned, enabling SSRF via DNS rebinding.
• Goroutine leak on shutdown in l7/server.go — active tunnels are not interrupted by context cancellation during graceful shutdown.
• Write deadline bypass in uplink/server.go — HelloAck write skips connection write deadline, risking indefinite server goroutine blocking.
• No timestamp freshness in uplink/hellomac.go — VerifyHelloProof lacks replay protection, enabling ClientHello replay attacks.
• IPv6 address bug in l7/types.go — HostPort() produces invalid addresses for IPv6 literal targets due to missing bracket wrapping.

Low Severity

• Stale proxy: error message prefix in uplink/errors.go left from package rename.

[tenki-reviewer]

🟡 HostPort() produces invalid address for IPv6 literal targets — missing brackets after SplitHostPort (bug)

parseConnect in l7/connect.go calls net.SplitHostPort(hostPort) which strips square brackets from IPv6 addresses (e.g., [::1]:443 → host=::1, port=443). ConnectTarget.HostPort() in l7/types.go reconstructs the address with naive string concatenation: host + ':' + port, producing ::1:443. Go's net.Dial requires IPv6 addresses to be bracketed, so any CONNECT to an IPv6 literal fails with 'too many colons in address', returning 502 Bad Gateway.

💡 Suggestion: Replace the manual concatenation in HostPort() with net.JoinHostPort(t.Host, t.Port), which correctly wraps IPv6 addresses in brackets.

📋 Prompt for AI Agents

In l7/types.go, change the HostPort() method to use net.JoinHostPort instead of manual string concatenation. Replace lines 22-27 with:

func (t ConnectTarget) HostPort() string {
if t.Port == "" {
return t.Host
}
return net.JoinHostPort(t.Host, t.Port)
}

net.JoinHostPort correctly wraps IPv6 addresses in brackets (e.g., [::1]:443). Ensure "net" is imported (already present).