Skip to content

derp/derphttp: honor DERPNode.DERPPort in proxied CONNECT dial#120

Merged
deansheather merged 1 commit into
coder:mainfrom
mzihlmann:fix-derp-proxy-port-coder
May 19, 2026
Merged

derp/derphttp: honor DERPNode.DERPPort in proxied CONNECT dial#120
deansheather merged 1 commit into
coder:mainfrom
mzihlmann:fix-derp-proxy-port-coder

Conversation

@mzihlmann
Copy link
Copy Markdown

@mzihlmann mzihlmann commented May 18, 2026

Cherry-pick of tailscale#19752 (fixes tailscale#19748) onto coder's fork.

When derphttp.Client routes a DERP connection through an HTTPS forward proxy (HTTPS_PROXY), the CONNECT request always targets port 443, even when the DERP map specifies a custom DERPPort.

The non-proxy path, dialNode, derp/derphttp/derphttp_client.go#L909-L913, correctly honors n.DERPPort:

port := "443"
if n.DERPPort != 0 {
    port = fmt.Sprint(n.DERPPort)
}

The proxy path, dialNodeUsingProxy, same file L993, hardcodes "443" and never consults n.DERPPort:

target := net.JoinHostPort(n.HostName, "443")

A custom DERP server on a non-443 port is therefore unreachable through HTTPS_PROXY: the CONNECT line is sent to host:443, the proxy faithfully tunnels to whatever sits on :443 at that hostname/IP, and TLS either fails on cert validation (if a different service runs on :443) or talks to the wrong service.

This PR mirrors dialNode's port selection in dialNodeUsingProxy. The end-to-end test from the upstream PR doesn't port cleanly (this fork lacks feature.HookProxyFromEnvironment, renamed/narrowed types) and the unit-test variant was dropped on the upstream PR (commit bb8b711), so this PR is fix-only.

dialNode picks the destination port from n.DERPPort when non-zero,
falling back to 443 (or 3340 when useHTTPS is false). The proxy path,
dialNodeUsingProxy, hardcoded "443" in the CONNECT target, so a DERP
server reachable only on a custom port was unreachable through
HTTPS_PROXY: the proxy would faithfully tunnel to :443 at the DERP
hostname, and TLS would either fail cert validation or talk to the
wrong service.

Mirror dialNode's port selection so both paths behave the same.

Fixes tailscale#19748

Signed-off-by: Martin Zihlmann <martizih@outlook.com>
@mzihlmann
Copy link
Copy Markdown
Author

I think failure is unrelated, no?

@mzihlmann
Copy link
Copy Markdown
Author

CI failures here are pre-existing on main (staticcheck + the cross (...) build-tests jobs all fail with the same compile errors in github.com/aws/aws-sdk-go-v2/feature/s3/manager@v1.11.64 against the newer service/s3 pulled in by #111). Fix is in #121 — once that lands and this branch is rebased, CI should go green.

@deansheather deansheather merged commit f4d8b89 into coder:main May 19, 2026
15 of 26 checks passed
@deansheather
Copy link
Copy Markdown
Member

Yeah CI is broken on our fork, the main thing we care about is tests on Linux. Thanks for the contribution!

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.

derp: dialNodeUsingProxy hardcodes :443 in CONNECT, ignoring DERPNode.DERPPort

2 participants