Skip to content

fix(doh): use per-connection local address for PROXY protocol#8005

Merged
yongtang merged 1 commit into
coredns:masterfrom
zongqi-wang:fix/doh-proxyproto-laddr
Apr 4, 2026
Merged

fix(doh): use per-connection local address for PROXY protocol#8005
yongtang merged 1 commit into
coredns:masterfrom
zongqi-wang:fix/doh-proxyproto-laddr

Conversation

@zongqi-wang

@zongqi-wang zongqi-wang commented Apr 2, 2026

Copy link
Copy Markdown
Contributor

Summary

PROXY protocol support was introduced in CoreDNS 1.24.1 in #7738. But when PROXY protocol is enabled on the DoH server, DoHWriter.laddr always used the static listener address (s.listenAddr) instead of the per-connection destination address from the PROXY protocol header. This is incorrect when the server is behind a load balancer with VIPs, and defeats the purpose of PROXY protocol.

  • Add http.Server.ConnContext to capture c.LocalAddr() per connection
  • Add localAddr() helper that reads from context with s.listenAddr as fallback
  • Replace s.listenAddr with s.localAddr(r) in ServeHTTP

Note: r.RemoteAddr (used for raddr) is already correct for TCP — Go's net/http reads it from proxyproto.Conn.RemoteAddr() through the TLS chain.

Test plan

  • TestDoHWriterLaddrFromConnContext — verifies laddr reflects PP destination
  • TestDoHWriterLaddrFallback — verifies fallback to listenAddr without PP
  • Full go test ./core/dnsserver/ passes

Signed-off-by: Cedric Wang <cedwang@amazon.com>
@zongqi-wang

Copy link
Copy Markdown
Contributor Author

This extends the PR #7738 to fix the gap on DoH with PPv2 enabled. if you could give it a look I would appreciate it @yongtang @Adphi

@yongtang yongtang merged commit 03d0863 into coredns:master Apr 4, 2026
13 checks passed
Filippo125 pushed a commit to Filippo125/coredns that referenced this pull request Apr 10, 2026
…s#8005)

Signed-off-by: Filippo <filippo.ferrazini@gmail.com>
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