skip acme-challenge path on to/from redirects

Acme challenge path is used to validate domains. This should have priority when compared with redirects. TLS redirect already have a global config key that adds `/.well-known/acme-challenge` by default. We need this same configuration for redirect-from/to family as well.
jcmoraisjr · Mar 6, 2023 · ee2014a · ee2014a
1 parent 3c0182f
commit ee2014a
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 85 deletions.
diff --git a/docs/content/en/docs/configuration/keys.md b/docs/content/en/docs/configuration/keys.md
@@ -416,6 +416,7 @@ The table below describes all supported configuration keys.
 | [`modsecurity-use-coraza`](#modsecurity)             | [true\|false]                           | Global  | `false`               |
 | [`nbproc-ssl`](#nbproc)                              | number of process                       | Global  | `0`                |
 | [`nbthread`](#nbthread)                              | number of threads                       | Global  |                    |
+| [`no-redirect-locations`](#redirect)                 | comma-separated list of URIs            | Global  | `/.well-known/acme-challenge` |
 | [`no-tls-redirect-locations`](#ssl-redirect)         | comma-separated list of URIs            | Global  | `/.well-known/acme-challenge` |
 | [`oauth`](#oauth)                                    | "oauth2_proxy"                          | Path    |                    |
 | [`oauth-headers`](#oauth)                            | `<header>:<var>,...`                    | Path    |                    |
@@ -2166,13 +2167,14 @@ See also:
 
 ## Redirect
 
-| Configuration key     | Scope    | Default | Since |
-|-----------------------|----------|---------|-------|
-| `redirect-from`       | `Host`   |         | v0.13 |
-| `redirect-from-code`  | `Global` | `302`   | v0.13 |
-| `redirect-from-regex` | `Host`   |         | v0.13 |
-| `redirect-to`         | `Path`   |         | v0.13 |
-| `redirect-to-code`    | `Global` | `302`   | v0.13 |
+| Configuration key       | Scope    | Default                       | Since   |
+|-------------------------|----------|-------------------------------|---------|
+| `no-redirect-locations` | `Global` | `/.well-known/acme-challenge` | v0.14.3 |
+| `redirect-from`         | `Host`   |                               | v0.13   |
+| `redirect-from-code`    | `Global` | `302`                         | v0.13   |
+| `redirect-from-regex`   | `Host`   |                               | v0.13   |
+| `redirect-to`           | `Path`   |                               | v0.13   |
+| `redirect-to-code`      | `Global` | `302`                         | v0.13   |
 
 Configures HTTP redirect. Redirect *from* matches source hostnames that should be redirected
 to the hostname declared in the ingress spec. Redirect *to* uses the hostname declared in the
@@ -2184,6 +2186,7 @@ examples below.
 * `redirect-from-code`: Which HTTP status code should be used in the redirect from. A `302` response is used by default if not configured.
 * `redirect-to`: Defines the destination URL to redirect the incoming request. The declared hostname and path are used only to match the request, the backend will not be used and it's only needed to be declared to satisfy ingress spec validation.
 * `redirect-to-code`: Which HTTP status code should be used in the redirect to. A `302` response is used by default if not configured.
+* `no-redirect-locations`: Defines a comma-separated list of paths that should be ignored by all the redirects. Default value is `/.well-known/acme-challenge`, used by ACME protocol. Configure as an empty string to make the redirect happen on all paths, including the ACME challenge.
 
 **Using redirect-from**
 

diff --git a/pkg/converters/ingress/annotations/updater.go b/pkg/converters/ingress/annotations/updater.go
@@ -153,6 +153,7 @@ func (c *updater) UpdateGlobalConfig(haproxyConfig haproxy.Config, mapper *Mappe
 	d.global.MaxConn = mapper.Get(ingtypes.GlobalMaxConnections).Int()
 	d.global.DefaultBackendRedir = mapper.Get(ingtypes.GlobalDefaultBackendRedirect).String()
 	d.global.DefaultBackendRedirCode = mapper.Get(ingtypes.GlobalDefaultBackendRedirectCode).Int()
+	d.global.NoRedirects = utils.Split(mapper.Get(ingtypes.GlobalNoRedirectLocations).String(), ",")
 	d.global.DrainSupport.Drain = mapper.Get(ingtypes.GlobalDrainSupport).Bool()
 	d.global.DrainSupport.Redispatch = mapper.Get(ingtypes.GlobalDrainSupportRedispatch).Bool()
 	d.global.Cookie.Key = mapper.Get(ingtypes.GlobalCookieKey).Value

diff --git a/pkg/converters/ingress/defaults.go b/pkg/converters/ingress/defaults.go
@@ -97,6 +97,7 @@ func createDefaults() map[string]string {
 		types.GlobalModsecurityTimeoutProcessing: "1s",
 		types.GlobalModsecurityTimeoutServer:     "5s",
 		types.GlobalNbprocBalance:                "1",
+		types.GlobalNoRedirectLocations:          "/.well-known/acme-challenge",
 		types.GlobalNoTLSRedirectLocations:       "/.well-known/acme-challenge",
 		types.GlobalPathTypeOrder:                "exact,prefix,begin,regex",
 		types.GlobalRedirectFromCode:             "302",

diff --git a/pkg/converters/ingress/types/global.go b/pkg/converters/ingress/types/global.go
@@ -103,6 +103,7 @@ const (
 	GlobalNbprocBalance                = "nbproc-balance"
 	GlobalNbprocSSL                    = "nbproc-ssl"
 	GlobalNbthread                     = "nbthread"
+	GlobalNoRedirectLocations          = "no-redirect-locations"
 	GlobalNoTLSRedirectLocations       = "no-tls-redirect-locations"
 	GlobalPathTypeOrder                = "path-type-order"
 	GlobalUsername                     = "username"

diff --git a/pkg/haproxy/instance_test.go b/pkg/haproxy/instance_test.go
@@ -3720,7 +3720,7 @@ func TestInstanceRedirectFrom(t *testing.T) {
 	testCases := []struct {
 		data     [3]hatypes.HostRedirectConfig
 		code     int
-		acme     bool
+		noredirs []string
 		expHTTP  string
 		expHTTPS string
 		expMaps  map[string]string
@@ -3774,14 +3774,14 @@ sub.d2.local d2.local
 			data: [3]hatypes.HostRedirectConfig{
 				{RedirectHost: "*.d1.local"},
 			},
-			code: 301,
-			acme: true,
+			code:     301,
+			noredirs: []string{"/.well-known/acme-challenge"},
 			expHTTP: `
-    http-request set-var(req.redirdest) var(req.host),map_reg(/etc/haproxy/maps/_front_redir_from__regex.map) if !acme-challenge !{ var(req.backend) -m found }
-    http-request redirect prefix //%[var(req.redirdest)] code 301 if !acme-challenge { var(req.redirdest) -m found }`,
+    http-request set-var(req.redirdest) var(req.host),map_reg(/etc/haproxy/maps/_front_redir_from__regex.map) if !{ path_beg "/.well-known/acme-challenge" } !{ var(req.backend) -m found }
+    http-request redirect prefix //%[var(req.redirdest)] code 301 if !{ path_beg "/.well-known/acme-challenge" } { var(req.redirdest) -m found }`,
 			expHTTPS: `
-    http-request set-var(req.redirdest) var(req.host),map_reg(/etc/haproxy/maps/_front_redir_from__regex.map) if !{ var(req.hostbackend) -m found }
-    http-request redirect prefix //%[var(req.redirdest)] code 301 if { var(req.redirdest) -m found }`,
+    http-request set-var(req.redirdest) var(req.host),map_reg(/etc/haproxy/maps/_front_redir_from__regex.map) if !{ path_beg "/.well-known/acme-challenge" } !{ var(req.hostbackend) -m found }
+    http-request redirect prefix //%[var(req.redirdest)] code 301 if !{ path_beg "/.well-known/acme-challenge" } { var(req.redirdest) -m found }`,
 			expMaps: map[string]string{
 				"_front_redir_from__regex.map": `
 ^[^.]+\.d1\.local$ d1.local
@@ -3794,6 +3794,8 @@ sub.d2.local d2.local
 		c := setup(t)
 		defer c.teardown()
 
+		c.config.global.NoRedirects = test.noredirs
+
 		var h *hatypes.Host
 		var b = c.config.Backends().AcquireBackend("d1", "app", "8080")
 		b.Endpoints = []*hatypes.Endpoint{endpointS1}
@@ -3819,20 +3821,6 @@ sub.d2.local d2.local
 			c.config.frontend.RedirectFromCode = 302
 		}
 
-		var acmeBackend, acmeACL, acmeUseBackend string
-		if test.acme {
-			acmeBackend = `
-backend _acme_challenge
-    mode http
-    server _acme_server unix@local`
-			acmeACL = `
-    acl acme-challenge path_beg `
-			acmeUseBackend = `
-    use_backend _acme_challenge if acme-challenge`
-			c.config.global.Acme.Socket = "local"
-			c.config.global.Acme.Enabled = true
-		}
-
 		c.Update()
 		c.checkConfig(`
 <<global>>
@@ -3845,11 +3833,11 @@ backend d2_app_8080
     server s21 172.17.0.121:8080 weight 100
 backend d3_app_8080
     mode http
-    server s31 172.17.0.131:8080 weight 100` + acmeBackend + `
+    server s31 172.17.0.131:8080 weight 100
 <<backends-default>>
 frontend _front_http
     mode http
-    bind :80` + acmeACL + `
+    bind :80
     http-request set-var(req.path) path
     http-request set-var(req.host) hdr(host),field(1,:),lower
     http-request set-var(req.base) var(req.host),concat(\#,req.path)
@@ -3859,7 +3847,7 @@ frontend _front_http
     http-request del-header X-SSL-Client-SHA1
     http-request del-header X-SSL-Client-SHA2
     http-request del-header X-SSL-Client-Cert
-    http-request set-var(req.backend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_http_host__begin.map)` + test.expHTTP + acmeUseBackend + `
+    http-request set-var(req.backend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_http_host__begin.map)` + test.expHTTP + `
     use_backend %[var(req.backend)] if { var(req.backend) -m found }
     default_backend _error404
 frontend _front_https
@@ -3890,9 +3878,8 @@ func TestInstanceRedirectTo(t *testing.T) {
 	testCases := []struct {
 		to       [3]string
 		code     int
-		acme     bool
+		noredirs []string
 		expected string
-		expHTTPS string
 		expMaps  map[string]string
 	}{
 		// 0
@@ -3950,13 +3937,10 @@ d1.local#/app2 https://app.local/app2
 			to: [3]string{
 				"https://app.local",
 			},
-			acme: true,
+			noredirs: []string{"/.well-known/acme-challenge"},
 			expected: `
-    http-request set-var(req.redirto) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_redir_to__begin.map) if !acme-challenge
-    http-request redirect location %[var(req.redirto)] code 302 if !acme-challenge { var(req.redirto) -m found }`,
-			expHTTPS: `
-    http-request set-var(req.redirto) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_redir_to__begin.map)
-    http-request redirect location %[var(req.redirto)] code 302 if { var(req.redirto) -m found }`,
+    http-request set-var(req.redirto) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_redir_to__begin.map) if !{ path_beg "/.well-known/acme-challenge" }
+    http-request redirect location %[var(req.redirto)] code 302 if !{ path_beg "/.well-known/acme-challenge" } { var(req.redirto) -m found }`,
 			expMaps: map[string]string{
 				"_front_redir_to__begin.map": `
 d1.local#/ https://app.local
@@ -3969,6 +3953,8 @@ d1.local#/ https://app.local
 		c := setup(t)
 		defer c.teardown()
 
+		c.config.global.NoRedirects = test.noredirs
+
 		var h = c.config.Hosts().AcquireHost("d1.local")
 		var b = c.config.Backends().AcquireBackend("d1", "app", "8080")
 		b.Endpoints = []*hatypes.Endpoint{endpointS1}
@@ -3994,30 +3980,12 @@ d1.local#/ https://app.local
 			h.AddPath(b, "/app3", hatypes.MatchBegin)
 		}
 
-		if test.expHTTPS == "" {
-			test.expHTTPS = test.expected
-		}
-
 		if test.code != 0 {
 			c.config.frontend.RedirectToCode = test.code
 		} else {
 			c.config.frontend.RedirectToCode = 302
 		}
 
-		var acmeBackend, acmeACL, acmeUseBackend string
-		if test.acme {
-			acmeBackend = `
-backend _acme_challenge
-    mode http
-    server _acme_server unix@local`
-			acmeACL = `
-    acl acme-challenge path_beg `
-			acmeUseBackend = `
-    use_backend _acme_challenge if acme-challenge`
-			c.config.global.Acme.Socket = "local"
-			c.config.global.Acme.Enabled = true
-		}
-
 		c.Update()
 		c.checkConfig(`
 <<global>>
@@ -4030,11 +3998,11 @@ backend d2_app_8080
     server s21 172.17.0.121:8080 weight 100
 backend d3_app_8080
     mode http
-    server s31 172.17.0.131:8080 weight 100` + acmeBackend + `
+    server s31 172.17.0.131:8080 weight 100
 <<backends-default>>
 frontend _front_http
     mode http
-    bind :80` + acmeACL + `
+    bind :80
     http-request set-var(req.path) path
     http-request set-var(req.host) hdr(host),field(1,:),lower
     http-request set-var(req.base) var(req.host),concat(\#,req.path)
@@ -4044,15 +4012,15 @@ frontend _front_http
     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.expected + `
-    http-request set-var(req.backend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_http_host__begin.map)` + acmeUseBackend + `
+    http-request set-var(req.backend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_http_host__begin.map)
     use_backend %[var(req.backend)] if { var(req.backend) -m found }
     default_backend _error404
 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.path) path
     http-request set-var(req.host) hdr(host),field(1,:),lower
-    http-request set-var(req.base) var(req.host),concat(\#,req.path)` + test.expHTTPS + `
+    http-request set-var(req.base) var(req.host),concat(\#,req.path)` + test.expected + `
     http-request set-var(req.hostbackend) var(req.base),lower,map_beg(/etc/haproxy/maps/_front_https_host__begin.map)
     http-request set-header X-Forwarded-Proto https
     http-request del-header X-SSL-Client-CN

diff --git a/pkg/haproxy/types/types.go b/pkg/haproxy/types/types.go
@@ -80,6 +80,7 @@ type Global struct {
 	UseHTX                  bool
 	DefaultBackendRedir     string
 	DefaultBackendRedirCode int
+	NoRedirects             []string
 	CustomConfig            []string
 	CustomDefaults          []string
 	CustomFrontendEarly     []string

diff --git a/rootfs/etc/templates/haproxy/haproxy.tmpl b/rootfs/etc/templates/haproxy/haproxy.tmpl
@@ -1221,11 +1221,7 @@ frontend {{ $proxy__front_http }}
 {{- end }}
 
 {{- /*------------------------------------*/}}
-{{- if $global.Acme.Enabled }}
-    {{- template "redirectTo" map $frontend $fmaps "!acme-challenge" }}
-{{- else }}
-    {{- template "redirectTo" map $frontend $fmaps "" }}
-{{- end }}
+{{- template "redirectTo" map $global $frontend $fmaps }}
 
 {{- /*------------------------------------*/}}
 {{- range $match := $fmaps.HTTPHostMap.MatchFiles }}
@@ -1243,11 +1239,7 @@ frontend {{ $proxy__front_http }}
 {{- end }}
 
 {{- /*------------------------------------*/}}
-{{- if $global.Acme.Enabled }}
-    {{- template "redirectFrom" map $frontend $fmaps "req.backend" "!acme-challenge" }}
-{{- else }}
-    {{- template "redirectFrom" map $frontend $fmaps "req.backend" "" }}
-{{- end }}
+{{- template "redirectFrom" map $global $frontend $fmaps "req.backend" }}
 
 {{- /*------------------------------------*/}}
 {{- range $snippet := $global.CustomFrontendLate }}
@@ -1310,7 +1302,7 @@ frontend {{ $frontend.Name }}
     http-request set-var(req.base) var(req.host),concat(\#,req.path)
 
 {{- /*------------------------------------*/}}
-{{- template "redirectTo" map $frontend $fmaps "" }}
+{{- template "redirectTo" map $global $frontend $fmaps }}
 
 {{- /*------------------------------------*/}}
 {{- range $match := $fmaps.HTTPSHostMap.MatchFiles }}
@@ -1328,7 +1320,7 @@ frontend {{ $frontend.Name }}
 {{- end }}
 
 {{- /*------------------------------------*/}}
-{{- template "redirectFrom" map $frontend $fmaps "req.hostbackend" "" }}
+{{- template "redirectFrom" map $global $frontend $fmaps "req.hostbackend" }}
 
 {{- /*------------------------------------*/}}
 {{- if $fmaps.RedirFromRootMap.HasHost }}
@@ -1472,45 +1464,45 @@ frontend {{ $frontend.Name }}
 {{- /*------------------------------------*/}}
 {{- /*------------------------------------*/}}
 {{- define "redirectFrom" }}
-{{- $frontend := .p1 }}
-{{- $fmaps := .p2 }}
-{{- $varbe := .p3 }}
-{{- $acls := .p4 }}
+{{- $global := .p1 }}
+{{- $frontend := .p2 }}
+{{- $fmaps := .p3 }}
+{{- $varbe := .p4 }}
 {{- if $fmaps.RedirFromMap.MatchFiles }}
 {{- range $match := $fmaps.RedirFromMap.MatchFiles }}
     http-request set-var(req.redirdest) var(req.host)
         {{- if $match.Lower }},lower{{ end }}
         {{- "" }},map_{{ $match.Method }}({{ $match.Filename }})
         {{- "" }} if
-        {{- if $acls }} {{ $acls }}{{ end }}
+        {{- if $global.NoRedirects }} !{ path_beg{{ range $global.NoRedirects }} "{{ . }}"{{ end }} }{{ end }}
         {{- "" }} !{ var({{ $varbe }}) -m found }
         {{- if not $match.First }} !{ var(req.redirdest) -m found }{{ end }}
 {{- end }}
     http-request redirect prefix //%[var(req.redirdest)]
         {{- "" }} code {{ $frontend.RedirectFromCode }}
         {{- "" }} if
-        {{- if $acls }} {{ $acls }}{{ end }}
+        {{- if $global.NoRedirects }} !{ path_beg{{ range $global.NoRedirects }} "{{ . }}"{{ end }} }{{ end }}
         {{- "" }} { var(req.redirdest) -m found }
 {{- end }}
 {{- end }}
 
 {{- define "redirectTo" }}
-{{- $frontend := .p1 }}
-{{- $fmaps := .p2 }}
-{{- $acls := .p3 }}
+{{- $global := .p1 }}
+{{- $frontend := .p2 }}
+{{- $fmaps := .p3 }}
 {{- if $fmaps.RedirToMap.MatchFiles }}
 {{- range $match := $fmaps.RedirToMap.MatchFiles }}
     http-request set-var(req.redirto) var(req.base)
         {{- if $match.Lower }},lower{{ end }}
         {{- "" }},map_{{ $match.Method }}({{ $match.Filename }})
-        {{- if or $acls (not $match.First) }} if{{ end }}
-        {{- if $acls }} {{ $acls }}{{ end }}
+        {{- if or $global.NoRedirects (not $match.First) }} if{{ end }}
+        {{- if $global.NoRedirects }} !{ path_beg{{ range $global.NoRedirects }} "{{ . }}"{{ end }} }{{ end }}
         {{- if not $match.First }} !{ var(req.redirto) -m found }{{ end }}
 {{- end }}
     http-request redirect location %[var(req.redirto)]
         {{- "" }} code {{ $frontend.RedirectToCode }}
         {{- "" }} if
-        {{- if $acls }} {{ $acls }}{{ end }}
+        {{- if $global.NoRedirects }} !{ path_beg{{ range $global.NoRedirects }} "{{ . }}"{{ end }} }{{ end }}
         {{- "" }} { var(req.redirto) -m found }
 {{- end }}
 {{- end }}