Skip to content

Commit

Permalink
remove dedicated maps for SNI match
Browse files Browse the repository at this point in the history 
SNI maps were incorrectly used to match requests on ancient versions of
HAProxy Ingress - v0.4 or so. A separated group of match files were
being used since then on TLS based authentication configurations. We
don't need it anymore, since all the mTLS configurations don't depend
on the maps, so we're now dropping its support.

There is one behavior change with this update: a missing or
misconfigured host header, for an ingress with mTLS, with optional
certificate, without sending a certificate, would fallback to SNI in
order to try a match. Now, since only the host header is the source of
truth, a non matching host header with a distinct SNI will 404 despite
of its mTLS configuration.
  • Loading branch information
@jcmoraisjr
jcmoraisjr committed Jun 14, 2024
1 parent 914b581 commit 1515040
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 74 deletions.
10 changes: 2 additions & 8 deletions pkg/haproxy/config.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ func (c *config) WriteFrontendMaps() error {
fmaps := &hatypes.FrontendMaps{
HTTPHostMap: mapBuilder.AddMap(mapsDir + "/_front_http_host.map"),
HTTPSHostMap: mapBuilder.AddMap(mapsDir + "/_front_https_host.map"),
HTTPSSNIMap: mapBuilder.AddMap(mapsDir + "/_front_https_sni.map"),
//
RedirFromRootMap: mapBuilder.AddMap(mapsDir + "/_front_redir_fromroot.map"),
RedirFromMap: mapBuilder.AddMap(mapsDir + "/_front_redir_from.map"),
Expand Down Expand Up @@ -223,13 +222,8 @@ func (c *config) WriteFrontendMaps() error {
}
} else if host.HasTLS() {
// ssl offload in place
if host.HasTLSAuth() {
fmaps.HTTPSSNIMap.AddHostnamePathMapping(host.Hostname, path, backendID)
fmaps.HTTPSSNIMap.AddAliasPathMapping(host.Alias, path, backendID)
} else {
fmaps.HTTPSHostMap.AddHostnamePathMapping(host.Hostname, path, backendID)
fmaps.HTTPSHostMap.AddAliasPathMapping(host.Alias, path, backendID)
}
fmaps.HTTPSHostMap.AddHostnamePathMapping(host.Hostname, path, backendID)
fmaps.HTTPSHostMap.AddAliasPathMapping(host.Alias, path, backendID)
}
fmaps.HTTPHostMap.AddHostnamePathMapping(host.Hostname, path, backendID)
fmaps.HTTPHostMap.AddAliasPathMapping(host.Alias, path, backendID)
Expand Down
62 changes: 15 additions & 47 deletions pkg/haproxy/instance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1685,17 +1685,9 @@ func TestInstanceFrontingProxy(t *testing.T) {
http-request set-header X-SSL-Client-SHA1 %{+Q}[ssl_c_sha1,hex]
http-response set-header Strict-Transport-Security "max-age=15768000; includeSubDomains; preload"`
setvarBegin = `
http-request set-var(req.snibase) ssl_fc_sni,lower,concat(\#,req.path)
http-request set-var(req.snibackend) var(req.snibase),lower,map_beg(/etc/haproxy/maps/_front_https_sni__begin.map)
http-request set-var(req.snibackend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_https_sni__begin.map) if !{ var(req.snibackend) -m found } !tls-has-crt !tls-host-need-crt
http-request set-var(req.tls_nocrt_redir) str(_internal) if !tls-has-crt tls-need-crt
http-request set-var(req.tls_invalidcrt_redir) str(_internal) if tls-has-invalid-crt tls-check-crt`
http-request set-var(req.hostbackend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_https_host__begin.map)`
setvarRegex = `
http-request set-var(req.snibase) ssl_fc_sni,lower,concat(\#,req.path)
http-request set-var(req.snibackend) var(req.snibase),map_reg(/etc/haproxy/maps/_front_https_sni__regex.map)
http-request set-var(req.snibackend) var(req.base),map_reg(/etc/haproxy/maps/_front_https_sni__regex.map) if !{ var(req.snibackend) -m found } !tls-has-crt !tls-host-need-crt
http-request set-var(req.tls_nocrt_redir) str(_internal) if !tls-has-crt tls-need-crt
http-request set-var(req.tls_invalidcrt_redir) str(_internal) if tls-has-invalid-crt tls-check-crt`
http-request set-var(req.hostbackend) var(req.base),map_reg(/etc/haproxy/maps/_front_https_host__regex.map)`
)
testCases := []struct {
frontingBind string
Expand Down Expand Up @@ -1943,13 +1935,15 @@ frontend _front_http` + test.expectedFront + `
frontend _front_https
mode http
bind :443 ssl alpn h2,http/1.1 crt-list /etc/haproxy/maps/_front_bind_crt.list ca-ignore-err all crt-ignore-err all
<<set-req-base>>
<<set-req-base>>` + test.expectedSetvar + `
http-request set-header X-Forwarded-Proto https
http-request del-header X-SSL-Client-CN
http-request del-header X-SSL-Client-DN
http-request del-header X-SSL-Client-SHA1
http-request del-header X-SSL-Client-SHA2
http-request del-header X-SSL-Client-Cert` + test.expectedACLFront + test.expectedSetvar + `
http-request del-header X-SSL-Client-Cert` + test.expectedACLFront + `
http-request set-var(req.tls_nocrt_redir) str(_internal) if !tls-has-crt tls-need-crt
http-request set-var(req.tls_invalidcrt_redir) str(_internal) if tls-has-invalid-crt tls-check-crt
http-request use-service lua.send-421 if tls-has-crt { ssl_fc_has_sni } !{ ssl_fc_sni,strcmp(req.host) eq 0 }
http-request use-service lua.send-496 if { var(req.tls_nocrt_redir) -m str _internal }
http-request use-service lua.send-421 if !tls-has-crt tls-host-need-crt
Expand Down Expand Up @@ -2918,13 +2912,6 @@ frontend _front_https
acl tls-host-need-crt var(req.host) -i -m str -f /etc/haproxy/maps/_front_tls_needcrt__exact.list
acl tls-has-invalid-crt ssl_c_verify gt 0
acl tls-check-crt ssl_fc_sni -i -m str -f /etc/haproxy/maps/_front_tls_auth__exact.list
http-request set-var(req.snibase) ssl_fc_sni,lower,concat(\#,req.path)
http-request set-var(req.snibackend) var(req.snibase),lower,map_beg(/etc/haproxy/maps/_front_https_sni__begin_01.map) if { hdr(x-user) -- 'id' } { hdr(x-version) -m reg -- '^[Ss]taging$' }
http-request set-var(req.snibackend) var(req.snibase),lower,map_beg(/etc/haproxy/maps/_front_https_sni__begin_02.map) if !{ var(req.snibackend) -m found } { hdr(x-version) -m reg -- '^[Tt]est$' }
http-request set-var(req.snibackend) var(req.snibase),lower,map_beg(/etc/haproxy/maps/_front_https_sni__begin.map) if !{ var(req.snibackend) -m found }
http-request set-var(req.snibackend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_https_sni__begin_01.map) if !{ var(req.snibackend) -m found } !tls-has-crt !tls-host-need-crt { hdr(x-user) -- 'id' } { hdr(x-version) -m reg -- '^[Ss]taging$' }
http-request set-var(req.snibackend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_https_sni__begin_02.map) if !{ var(req.snibackend) -m found } !tls-has-crt !tls-host-need-crt { hdr(x-version) -m reg -- '^[Tt]est$' }
http-request set-var(req.snibackend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_https_sni__begin.map) if !{ var(req.snibackend) -m found } !tls-has-crt !tls-host-need-crt
http-request set-var(req.tls_nocrt_redir) str(_internal) if !tls-has-crt tls-need-crt
http-request set-var(req.tls_invalidcrt_redir) str(_internal) if tls-has-invalid-crt tls-check-crt
http-request use-service lua.send-421 if tls-has-crt { ssl_fc_has_sni } !{ ssl_fc_sni,strcmp(req.host) eq 0 }
Expand Down Expand Up @@ -2963,25 +2950,19 @@ h5.local#/ b1_app_8080
`)
c.checkMap("_front_https_host__begin_01.map", `
h2.local#/ b1_app_8080
h4.local#/ b1_app_8080
h5.local#/ b1_app_8080
`)
c.checkMap("_front_https_host__begin_02.map", `
h2.local#/app2 b21_app_8080
h4.local#/app2 b21_app_8080
h5.local#/app2 b21_app_8080
`)
c.checkMap("_front_https_host__begin.map", `
h2.local#/ b1_app_8080
h3.local#/ b1_app_8080
h5.local#/ b1_app_8080
`)
c.checkMap("_front_https_sni__begin_01.map", `
h4.local#/ b1_app_8080
`)
c.checkMap("_front_https_sni__begin_02.map", `
h4.local#/app2 b21_app_8080
`)
c.checkMap("_front_https_sni__begin.map", `
h4.local#/ b1_app_8080
h5.local#/ b1_app_8080
`)
c.checkMap("_front_namespace__begin_01.map", `
h2.local#/ -
Expand Down Expand Up @@ -3113,6 +3094,7 @@ frontend _front_https
bind :443 ssl alpn h2,http/1.1 crt-list /etc/haproxy/maps/_front_bind_crt.list ca-ignore-err all crt-ignore-err all
<<set-req-base>>
http-request set-var(req.hostbackend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_https_host__begin.map)
http-request set-var(req.hostbackend) var(req.base),map_reg(/etc/haproxy/maps/_front_https_host__regex.map) if !{ var(req.hostbackend) -m found }
<<https-headers>>
acl tls-has-crt ssl_c_used
acl tls-need-crt ssl_fc_sni -i -m str -f /etc/haproxy/maps/_front_tls_needcrt__exact.list
Expand All @@ -3122,11 +3104,6 @@ frontend _front_https
acl tls-has-invalid-crt ssl_c_verify gt 0
acl tls-check-crt ssl_fc_sni -i -m str -f /etc/haproxy/maps/_front_tls_auth__exact.list
acl tls-check-crt ssl_fc_sni -i -m reg -f /etc/haproxy/maps/_front_tls_auth__regex.list
http-request set-var(req.snibase) ssl_fc_sni,lower,concat(\#,req.path)
http-request set-var(req.snibackend) var(req.snibase),lower,map_beg(/etc/haproxy/maps/_front_https_sni__begin.map)
http-request set-var(req.snibackend) var(req.snibase),map_reg(/etc/haproxy/maps/_front_https_sni__regex.map) if !{ var(req.snibackend) -m found }
http-request set-var(req.snibackend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_https_sni__begin.map) if !{ var(req.snibackend) -m found } !tls-has-crt !tls-host-need-crt
http-request set-var(req.snibackend) var(req.base),map_reg(/etc/haproxy/maps/_front_https_sni__regex.map) if !{ var(req.snibackend) -m found } !tls-has-crt !tls-host-need-crt
http-request set-var(req.tls_nocrt_redir) ssl_fc_sni,lower,map_str(/etc/haproxy/maps/_front_tls_missingcrt_pages__exact.map,_internal) if !tls-has-crt tls-need-crt
http-request set-var(req.tls_nocrt_redir) ssl_fc_sni,lower,map_reg(/etc/haproxy/maps/_front_tls_missingcrt_pages__regex.map,_internal) if { var(req.tls_nocrt_redir) -m str _internal }
http-request set-var(req.tls_invalidcrt_redir) ssl_fc_sni,lower,map_str(/etc/haproxy/maps/_front_tls_invalidcrt_pages__exact.map,_internal) if tls-has-invalid-crt tls-check-crt
Expand Down Expand Up @@ -3163,15 +3140,13 @@ d6.local#/ d_app_8080
/var/haproxy/ssl/certs/default.pem [ssl-min-ver TLSv1.0 ssl-max-ver TLSv1.2] d6.local
`)
c.checkMap("_front_https_host__begin.map", `
d2.local#/ d_app_8080
d3.local#/ d_app_8080
d4.local#/ d_app_8080
d5.local#/ d_app_8080
d6.local#/ d_app_8080
`)
c.checkMap("_front_https_sni__begin.map", `
d2.local#/ d_app_8080
`)
c.checkMap("_front_https_sni__regex.map", `
c.checkMap("_front_https_host__regex.map", `
^[^.]+\.d1\.local#/ d_app_8080
`)
c.checkMap("_front_tls_needcrt__exact.list", `
Expand Down Expand Up @@ -5261,9 +5236,6 @@ frontend _front_https
acl tls-has-crt ssl_c_used
acl tls-has-invalid-crt ssl_c_verify gt 0
acl tls-check-crt ssl_fc_sni -i -m reg -f /etc/haproxy/maps/_front_tls_auth__regex.list
http-request set-var(req.snibase) ssl_fc_sni,lower,concat(\#,req.path)
http-request set-var(req.snibackend) var(req.snibase),map_reg(/etc/haproxy/maps/_front_https_sni__regex.map)
http-request set-var(req.snibackend) var(req.base),map_reg(/etc/haproxy/maps/_front_https_sni__regex.map) if !{ var(req.snibackend) -m found } !tls-has-crt
http-request set-var(req.tls_invalidcrt_redir) ssl_fc_sni,lower,map_reg(/etc/haproxy/maps/_front_tls_invalidcrt_pages__regex.map,_internal) if tls-has-invalid-crt tls-check-crt
http-request redirect location %[var(req.tls_invalidcrt_redir)] code 303 if { var(req.tls_invalidcrt_redir) -m found } !{ var(req.tls_invalidcrt_redir) -m str _internal }
http-request use-service lua.send-421 if tls-has-crt { ssl_fc_has_sni } !{ ssl_fc_sni,strcmp(req.host) eq 0 }
Expand All @@ -5290,13 +5262,11 @@ d1.local#/ d1_app_8080
`)
c.checkMap("_front_https_host__regex.map", `
^[^.]+\.app\.d1\.local#/ d1_app_8080
^[^.]+\.sub\.d1\.local#/ d1_app_8080
^[^.]+\.d2\.local#/ d2_app_8080
`)
c.checkMap("_front_redir_fromroot__regex.map", `
^[^.]+\.d2\.local$ /app
`)
c.checkMap("_front_https_sni__regex.map", `
^[^.]+\.sub\.d1\.local#/ d1_app_8080
`)
c.checkMap("_front_tls_auth__regex.list", `
^[^.]+\.sub\.d1\.local$
Expand Down Expand Up @@ -5349,12 +5319,10 @@ frontend _front_https
mode http
bind :443 ssl alpn h2,http/1.1 crt-list /etc/haproxy/maps/_front_bind_crt.list ca-ignore-err all crt-ignore-err all
<<set-req-base>>
http-request set-var(req.hostbackend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_https_host__begin.map)
<<https-headers>>
acl tls-has-crt ssl_c_used
acl tls-has-invalid-crt ssl_c_verify gt 0
http-request set-var(req.snibase) ssl_fc_sni,lower,concat(\#,req.path)
http-request set-var(req.snibackend) var(req.snibase),lower,map_beg(/etc/haproxy/maps/_front_https_sni__begin.map)
http-request set-var(req.snibackend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_https_sni__begin.map) if !{ var(req.snibackend) -m found } !tls-has-crt
http-request use-service lua.send-421 if tls-has-crt { ssl_fc_has_sni } !{ ssl_fc_sni,strcmp(req.host) eq 0 }
use_backend %[var(req.hostbackend)] if { var(req.hostbackend) -m found }
use_backend %[var(req.snibackend)] if { var(req.snibackend) -m found }
Expand All @@ -5369,7 +5337,7 @@ d1.local#/ d1_app_8080
/var/haproxy/ssl/certs/default.pem !*
/var/haproxy/ssl/certs/default.pem [ca-file /var/haproxy/ssl/ca/d1.local.pem verify optional] d1.local
`)
c.checkMap("_front_https_sni__begin.map", `
c.checkMap("_front_https_host__begin.map", `
d1.local#/ d1_app_8080
`)

Expand Down
1 change: 0 additions & 1 deletion pkg/haproxy/types/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,6 @@ type HostsMaps struct {
type FrontendMaps struct {
HTTPHostMap *HostsMap
HTTPSHostMap *HostsMap
HTTPSSNIMap *HostsMap
//
RedirFromRootMap *HostsMap
RedirRootSSLMap *HostsMap
Expand Down
18 changes: 0 additions & 18 deletions rootfs/etc/templates/haproxy/haproxy.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1372,24 +1372,6 @@ frontend {{ $frontend.Name }}
acl tls-check-crt ssl_fc_sni -i -m {{ $match.Method }} -f {{ $match.Filename }}
{{- end }}

{{- if $fmaps.HTTPSSNIMap.HasHost }}
http-request set-var(req.snibase) ssl_fc_sni,lower,concat(\#,req.path)
{{- range $match := $fmaps.HTTPSSNIMap.MatchFiles }}
http-request set-var(req.snibackend) var(req.snibase)
{{- if $match.Lower }},lower{{ end }}
{{- "" }},map_{{ $match.Method }}({{ $match.Filename }})
{{- template "httpFilters" map $match "req.snibackend" 1 }}
{{- end }}
{{- range $match := $fmaps.HTTPSSNIMap.MatchFiles }}
http-request set-var(req.snibackend) var(req.base)
{{- if $match.Lower }},lower{{ end }}
{{- "" }},map_{{ $match.Method }}({{ $match.Filename }})
{{- "" }} if !{ var(req.snibackend) -m found } !tls-has-crt
{{- if $fmaps.TLSNeedCrtList.HasHost }} !tls-host-need-crt{{ end }}
{{- template "httpFilters" map $match "" 0 }}
{{- end }}
{{- end }}

{{- if $mandatory }}
{{- if not $fmaps.TLSMissingCrtPagesMap.HasHost }}
http-request set-var(req.tls_nocrt_redir) str(_internal) if !tls-has-crt tls-need-crt
Expand Down
58 changes: 58 additions & 0 deletions tests/integration/integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,64 @@ func TestIntegrationIngress(t *testing.T) {
assert.Equal(t, reqHeaders, res.EchoResponse.ReqHeaders)
})

// should match wildcard host
// should match domain conflicting with wildcard host

t.Run("should give priority on specific domains over wildcard", func(t *testing.T) {
t.Parallel()
hostname := framework.RandomHostName()
hostSubdomain := "sub." + hostname
hostWildcard := "*." + hostname

backend1 := f.CreateHTTPServer(ctx, t, "backend1")
svc1 := f.CreateService(ctx, t, backend1)
backend2 := f.CreateHTTPServer(ctx, t, "backend2")
svc2 := f.CreateService(ctx, t, backend2)

_, _ = f.CreateIngress(ctx, t, svc1,
options.DefaultTLS(),
options.CustomHostName(hostSubdomain),
options.AddConfigKeyAnnotation(ingtypes.HostAuthTLSSecret, secretCA.Name),
)
_, _ = f.CreateIngress(ctx, t, svc2,
options.DefaultTLS(),
options.CustomHostName(hostWildcard),
)

res := f.Request(ctx, t, http.MethodGet, hostSubdomain, "/",
options.HTTPSRequest(),
options.TLSSkipVerify(),
options.SNI(hostSubdomain),
options.ExpectResponseCode(496),
)
assert.False(t, res.EchoResponse.Parsed)

res = f.Request(ctx, t, http.MethodGet, hostSubdomain, "/",
options.HTTPSRequest(),
options.TLSSkipVerify(),
options.SNI(hostSubdomain),
options.ClientCertificateKeyPEM(crtValid, keyValid),
options.ExpectResponseCode(200),
)
assert.True(t, res.EchoResponse.Parsed)
assert.Equal(t, "backend1", res.EchoResponse.ServerName)

res = f.Request(ctx, t, http.MethodGet, "another."+hostname, "/",
options.HTTPSRequest(),
options.TLSSkipVerify(),
options.ExpectResponseCode(200),
)
assert.True(t, res.EchoResponse.Parsed)
assert.Equal(t, "backend2", res.EchoResponse.ServerName)

res = f.Request(ctx, t, http.MethodGet, hostname, "/",
options.HTTPSRequest(),
options.TLSSkipVerify(),
options.ExpectResponseCode(404),
)
assert.False(t, res.EchoResponse.Parsed)
})

t.Run("should take leader", func(t *testing.T) {
t.Parallel()
assert.EventuallyWithT(t, func(collect *assert.CollectT) {
Expand Down

0 comments on commit 1515040

Please sign in to comment.