fix(sub): stop external-proxy dest from clobbering TLS SNI

externalProxySNI fell back to ep["dest"] when an external-proxy row had
no SNI of its own, silently overwriting the upstream tlsSettings
serverName already written into the share-link params. Operators using
forceTls=same with a CDN edge IP got SNI=<edge-ip> in the link instead
of the real cert hostname, breaking TLS handshakes.

The fallback is dropped: an explicit ep["sni"] still overrides, but a
blank entry now leaves the upstream SNI in place. Tests updated.
This commit is contained in:
MHSanaei 2026-05-28 21:26:24 +02:00
parent 798e18b6ee
commit cda7f2ac17
No known key found for this signature in database
GPG key ID: 7E4060F2FBE5AB7A
2 changed files with 31 additions and 13 deletions

View file

@ -990,9 +990,6 @@ func externalProxySNI(ep map[string]any) (string, bool) {
if sni, ok := ep["sni"].(string); ok && sni != "" { if sni, ok := ep["sni"].(string); ok && sni != "" {
return sni, true return sni, true
} }
if dest, ok := ep["dest"].(string); ok && dest != "" {
return dest, true
}
return "", false return "", false
} }

View file

@ -484,24 +484,42 @@ func TestApplyExternalProxyTLSParams_UsesProxyDomainAndOverrides(t *testing.T) {
} }
} }
func TestApplyExternalProxyTLSParams_FallsBackToDestSNI(t *testing.T) { func TestApplyExternalProxyTLSParams_PreservesUpstreamSNI(t *testing.T) {
params := map[string]string{"security": "tls"} // External-proxy entry has no SNI of its own; its dest must not
// clobber the upstream tlsSettings.serverName already written into
// params. Regression: the dest fallback used to overwrite "222" with
// "111" whenever an operator set forceTls=same and left the proxy's
// SNI field blank.
params := map[string]string{"security": "tls", "sni": "real.example.com"}
ep := map[string]any{"dest": "proxy.example.com"} ep := map[string]any{"dest": "proxy.example.com"}
applyExternalProxyTLSParams(ep, params, "tls") applyExternalProxyTLSParams(ep, params, "tls")
if params["sni"] != "proxy.example.com" { if params["sni"] != "real.example.com" {
t.Fatalf("sni = %q, want proxy.example.com", params["sni"]) t.Fatalf("sni = %q, want upstream sni preserved (real.example.com)", params["sni"])
}
}
func TestApplyExternalProxyTLSParams_ExplicitSNIOverridesUpstream(t *testing.T) {
params := map[string]string{"security": "tls", "sni": "real.example.com"}
ep := map[string]any{"dest": "proxy.example.com", "sni": "edge.example.com"}
applyExternalProxyTLSParams(ep, params, "tls")
if params["sni"] != "edge.example.com" {
t.Fatalf("sni = %q, want edge.example.com", params["sni"])
} }
} }
func TestApplyExternalProxyTLSToStream_DoesNotLeakAcrossProxies(t *testing.T) { func TestApplyExternalProxyTLSToStream_DoesNotLeakAcrossProxies(t *testing.T) {
stream := map[string]any{ stream := map[string]any{
"security": "tls", "security": "tls",
"tlsSettings": map[string]any{}, "tlsSettings": map[string]any{
"serverName": "upstream.example.com",
},
} }
proxies := []map[string]any{ proxies := []map[string]any{
{"dest": "a.example.com", "fingerprint": "chrome", "alpn": []any{"h3"}}, {"dest": "a.example.com", "sni": "a-sni.example.com", "fingerprint": "chrome", "alpn": []any{"h3"}},
{"dest": "b.example.com"}, {"dest": "b.example.com"},
} }
@ -518,11 +536,14 @@ func TestApplyExternalProxyTLSToStream_DoesNotLeakAcrossProxies(t *testing.T) {
results = append(results, snapshot) results = append(results, snapshot)
} }
if results[0]["serverName"] != "a.example.com" || results[0]["fingerprint"] != "chrome" { if results[0]["serverName"] != "a-sni.example.com" || results[0]["fingerprint"] != "chrome" {
t.Fatalf("proxy A snapshot = %v", results[0]) t.Fatalf("proxy A snapshot = %v", results[0])
} }
if results[1]["serverName"] != "b.example.com" { // Proxy B has no SNI of its own — the upstream tlsSettings serverName
t.Fatalf("proxy B serverName = %v, want b.example.com", results[1]["serverName"]) // must remain in place (no dest fallback) and no fingerprint/alpn
// must leak from proxy A.
if results[1]["serverName"] != "upstream.example.com" {
t.Fatalf("proxy B serverName = %v, want upstream.example.com preserved", results[1]["serverName"])
} }
if results[1]["fingerprint"] != nil { if results[1]["fingerprint"] != nil {
t.Fatalf("proxy B should inherit no fingerprint, got %v (leaked from A)", results[1]["fingerprint"]) t.Fatalf("proxy B should inherit no fingerprint, got %v (leaked from A)", results[1]["fingerprint"])