Skip to content

Commit d7b21c6

Browse files
authored
reverseproxy: fix tls dialing w/ proxy protocol (#7508)
1 parent 6610e2f commit d7b21c6

File tree

2 files changed

+91
-4
lines changed

2 files changed

+91
-4
lines changed

modules/caddyhttp/reverseproxy/httptransport.go

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -412,8 +412,13 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e
412412
return nil, fmt.Errorf("making TLS client config: %v", err)
413413
}
414414

415-
// servername has a placeholder, so we need to replace it
416-
if strings.Contains(h.TLS.ServerName, "{") {
415+
serverNameHasPlaceholder := strings.Contains(h.TLS.ServerName, "{")
416+
417+
// We need to use custom DialTLSContext if:
418+
// 1. ServerName has a placeholder that needs to be replaced at request-time, OR
419+
// 2. ProxyProtocol is enabled, because req.URL.Host is modified to include
420+
// client address info with "->" separator which breaks Go's address parsing
421+
if serverNameHasPlaceholder || h.ProxyProtocol != "" {
417422
rt.DialTLSContext = func(ctx context.Context, network, addr string) (net.Conn, error) {
418423
// reuses the dialer from above to establish a plaintext connection
419424
conn, err := dialContext(ctx, network, addr)
@@ -422,9 +427,11 @@ func (h *HTTPTransport) NewTransport(caddyCtx caddy.Context) (*http.Transport, e
422427
}
423428

424429
// but add our own handshake logic
425-
repl := ctx.Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
426430
tlsConfig := rt.TLSClientConfig.Clone()
427-
tlsConfig.ServerName = repl.ReplaceAll(tlsConfig.ServerName, "")
431+
if serverNameHasPlaceholder {
432+
repl := ctx.Value(caddy.ReplacerCtxKey).(*caddy.Replacer)
433+
tlsConfig.ServerName = repl.ReplaceAll(tlsConfig.ServerName, "")
434+
}
428435

429436
// h1 only
430437
if caddyhttp.GetVar(ctx, tlsH1OnlyVarKey) == true {

modules/caddyhttp/reverseproxy/httptransport_test.go

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package reverseproxy
22

33
import (
4+
"context"
45
"encoding/json"
56
"fmt"
67
"reflect"
78
"testing"
89

10+
"github.com/caddyserver/caddy/v2"
911
"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
1012
)
1113

@@ -115,3 +117,81 @@ func TestHTTPTransport_RequestHeaderOps_TLS(t *testing.T) {
115117
t.Fatalf("unexpected Host value; want placeholder, got: %s", got)
116118
}
117119
}
120+
121+
// TestHTTPTransport_DialTLSContext_ProxyProtocol verifies that when TLS and
122+
// ProxyProtocol are both enabled, DialTLSContext is set. This is critical because
123+
// ProxyProtocol modifies req.URL.Host to include client info with "->" separator
124+
// (e.g., "[2001:db8::1]:12345->127.0.0.1:443"), which breaks Go's address parsing.
125+
// Without a custom DialTLSContext, Go's HTTP library would fail with
126+
// "too many colons in address" when trying to parse the mangled host.
127+
func TestHTTPTransport_DialTLSContext_ProxyProtocol(t *testing.T) {
128+
ctx, cancel := caddy.NewContext(caddy.Context{Context: context.Background()})
129+
defer cancel()
130+
131+
tests := []struct {
132+
name string
133+
tls *TLSConfig
134+
proxyProtocol string
135+
serverNameHasPlaceholder bool
136+
expectDialTLSContext bool
137+
}{
138+
{
139+
name: "no TLS, no proxy protocol",
140+
tls: nil,
141+
proxyProtocol: "",
142+
expectDialTLSContext: false,
143+
},
144+
{
145+
name: "TLS without proxy protocol",
146+
tls: &TLSConfig{},
147+
proxyProtocol: "",
148+
expectDialTLSContext: false,
149+
},
150+
{
151+
name: "TLS with proxy protocol v1",
152+
tls: &TLSConfig{},
153+
proxyProtocol: "v1",
154+
expectDialTLSContext: true,
155+
},
156+
{
157+
name: "TLS with proxy protocol v2",
158+
tls: &TLSConfig{},
159+
proxyProtocol: "v2",
160+
expectDialTLSContext: true,
161+
},
162+
{
163+
name: "TLS with placeholder ServerName",
164+
tls: &TLSConfig{ServerName: "{http.request.host}"},
165+
proxyProtocol: "",
166+
serverNameHasPlaceholder: true,
167+
expectDialTLSContext: true,
168+
},
169+
{
170+
name: "TLS with placeholder ServerName and proxy protocol",
171+
tls: &TLSConfig{ServerName: "{http.request.host}"},
172+
proxyProtocol: "v2",
173+
serverNameHasPlaceholder: true,
174+
expectDialTLSContext: true,
175+
},
176+
}
177+
178+
for _, tt := range tests {
179+
t.Run(tt.name, func(t *testing.T) {
180+
ht := &HTTPTransport{
181+
TLS: tt.tls,
182+
ProxyProtocol: tt.proxyProtocol,
183+
}
184+
185+
rt, err := ht.NewTransport(ctx)
186+
if err != nil {
187+
t.Fatalf("NewTransport() error = %v", err)
188+
}
189+
190+
hasDialTLSContext := rt.DialTLSContext != nil
191+
if hasDialTLSContext != tt.expectDialTLSContext {
192+
t.Errorf("DialTLSContext set = %v, want %v", hasDialTLSContext, tt.expectDialTLSContext)
193+
}
194+
})
195+
}
196+
}
197+

0 commit comments

Comments
 (0)