From 1515040ae687d68988591a9dc96a377b0959d704 Mon Sep 17 00:00:00 2001 From: Joao Morais Date: Thu, 6 Jun 2024 11:31:21 -0300 Subject: [PATCH] remove dedicated maps for SNI match 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. --- pkg/haproxy/config.go | 10 +--- pkg/haproxy/instance_test.go | 62 ++++++----------------- pkg/haproxy/types/types.go | 1 - rootfs/etc/templates/haproxy/haproxy.tmpl | 18 ------- tests/integration/integration_test.go | 58 +++++++++++++++++++++ 5 files changed, 75 insertions(+), 74 deletions(-) diff --git a/pkg/haproxy/config.go b/pkg/haproxy/config.go index 7fd21c0d..6d96e5ba 100644 --- a/pkg/haproxy/config.go +++ b/pkg/haproxy/config.go @@ -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"), @@ -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) diff --git a/pkg/haproxy/instance_test.go b/pkg/haproxy/instance_test.go index 7a4d57ea..6a3d36d2 100644 --- a/pkg/haproxy/instance_test.go +++ b/pkg/haproxy/instance_test.go @@ -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 @@ -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 - <> + <>` + 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 @@ -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 } @@ -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#/ - @@ -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 <> 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 } <> 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 @@ -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 @@ -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", ` @@ -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 } @@ -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$ @@ -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 <> + http-request set-var(req.hostbackend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_https_host__begin.map) <> 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 } @@ -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 `) diff --git a/pkg/haproxy/types/types.go b/pkg/haproxy/types/types.go index 30641b7c..bce07e6d 100644 --- a/pkg/haproxy/types/types.go +++ b/pkg/haproxy/types/types.go @@ -404,7 +404,6 @@ type HostsMaps struct { type FrontendMaps struct { HTTPHostMap *HostsMap HTTPSHostMap *HostsMap - HTTPSSNIMap *HostsMap // RedirFromRootMap *HostsMap RedirRootSSLMap *HostsMap diff --git a/rootfs/etc/templates/haproxy/haproxy.tmpl b/rootfs/etc/templates/haproxy/haproxy.tmpl index 782cc7aa..d14fbf06 100644 --- a/rootfs/etc/templates/haproxy/haproxy.tmpl +++ b/rootfs/etc/templates/haproxy/haproxy.tmpl @@ -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 diff --git a/tests/integration/integration_test.go b/tests/integration/integration_test.go index 36ef206c..dae8af52 100644 --- a/tests/integration/integration_test.go +++ b/tests/integration/integration_test.go @@ -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) {