Skip to content

Commit

Permalink
ensure https redirect happens before root redirect
Browse files Browse the repository at this point in the history
app-root config key configures the root path redirect in haproxy
frontend. https redirect however is configured in the backend. Because
of that haproxy is redirecting from the root path to the application
path in plain http, before redirecting to https. This is not a good
approach because it makes security scanners infer that the application
does not have a secure proxy.

This update adds a https redirect before the application redirect, in
the case the root path of the host renders its ssl-redirect to true.
  • Loading branch information
jcmoraisjr committed May 4, 2024
1 parent 10a90d2 commit af22965
Show file tree
Hide file tree
Showing 9 changed files with 71 additions and 7 deletions.
1 change: 1 addition & 0 deletions pkg/converters/ingress/annotations/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,7 @@ func (c *updater) buildGlobalSSL(d *globalData) {
ssl.ModeAsync = d.mapper.Get(ingtypes.GlobalSSLModeAsync).Bool()
ssl.Options = d.mapper.Get(ingtypes.GlobalSSLOptions).Value
ssl.RedirectCode = d.mapper.Get(ingtypes.GlobalSSLRedirectCode).Int()
ssl.SSLRedirect = d.mapper.Get(ingtypes.BackSSLRedirect).Bool()
}

func (c *updater) buildGlobalHTTPStoHTTP(d *globalData) {
Expand Down
21 changes: 21 additions & 0 deletions pkg/haproxy/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ func (c *config) WriteFrontendMaps() error {
//
RedirFromRootMap: mapBuilder.AddMap(mapsDir + "/_front_redir_fromroot.map"),
RedirFromMap: mapBuilder.AddMap(mapsDir + "/_front_redir_from.map"),
RedirRootSSLMap: mapBuilder.AddMap(mapsDir + "/_front_redir_root_ssl.map"),
RedirToMap: mapBuilder.AddMap(mapsDir + "/_front_redir_to.map"),
SSLPassthroughMap: mapBuilder.AddMap(mapsDir + "/_front_sslpassthrough.map"),
VarNamespaceMap: mapBuilder.AddMap(mapsDir + "/_front_namespace.map"),
Expand Down Expand Up @@ -274,6 +275,26 @@ func (c *config) WriteFrontendMaps() error {
// TODO wildcard/alias/alias-regex hostname can overlap
// a configured domain which doesn't have rootRedirect
if host.RootRedirect != "" {
// looking for root path configuration - if ssl redirect is enabled,
// we need to redirect to https before redirect the path.
redirectssl := func() bool {
redir := c.global.SSL.SSLRedirect
for _, path := range host.FindPath("/") {
if backend := c.backends.Items()[path.Backend.ID]; backend != nil {
if bpath := backend.FindBackendPath(path.Link); bpath != nil {
redir = bpath.SSLRedirect
if !path.Link.ComposeMatch() {
// give precedence for root path without method, header or cookie matching
return redir
}
}
}
}
return redir
}
if redirectssl() {
fmaps.RedirRootSSLMap.AddHostnameMapping(host.Hostname, "")
}
fmaps.RedirFromRootMap.AddHostnameMapping(host.Hostname, host.RootRedirect)
}
//
Expand Down
6 changes: 6 additions & 0 deletions pkg/haproxy/instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4042,6 +4042,8 @@ func TestInstanceRootRedirect(t *testing.T) {
c := setup(t)
defer c.teardown()

c.config.global.SSL.SSLRedirect = true

var h *hatypes.Host
var b = c.config.Backends().AcquireBackend("d1", "app", "8080")
h = c.config.Hosts().AcquireHost("*.d1.local")
Expand Down Expand Up @@ -4081,6 +4083,7 @@ frontend _front_http
mode http
bind :80
<<set-req-base>>
http-request redirect scheme https if { path / } { var(req.host) -i -m str -f /etc/haproxy/maps/_front_redir_root_ssl__exact.map }
http-request set-var(req.rootredir) var(req.host),map_str(/etc/haproxy/maps/_front_redir_fromroot__exact.map)
http-request set-var(req.rootredir) var(req.host),map_reg(/etc/haproxy/maps/_front_redir_fromroot__regex.map) if !{ var(req.rootredir) -m found }
http-request redirect location %[var(req.rootredir)] if { path / } { var(req.rootredir) -m found }
Expand Down Expand Up @@ -4116,6 +4119,9 @@ d2.local#/app1 d2_app_8080
`)
c.checkMap("_front_redir_fromroot__exact.map", `
d2.local /app1
`)
c.checkMap("_front_redir_root_ssl__exact.map", `
d2.local
`)
c.checkMap("_front_redir_fromroot__regex.map", `
^[^.]+\.d1\.local$ /app
Expand Down
6 changes: 6 additions & 0 deletions pkg/haproxy/types/host.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,12 @@ func (l *PathLink) Equals(other *PathLink) bool {
return l.hash == other.hash
}

// ComposeMatch returns true if the pathLink has composing match,
// by adding method, header or cookie match.
func (l *PathLink) ComposeMatch() bool {
return len(l.headers) > 0
}

// WithHostname ...
func (l *PathLink) WithHostname(hostname string) *PathLink {
l.hostname = hostname
Expand Down
2 changes: 2 additions & 0 deletions pkg/haproxy/types/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ type SSLConfig struct {
ModeAsync bool
Options string
RedirectCode int
SSLRedirect bool
}

// DHParamConfig ...
Expand Down Expand Up @@ -406,6 +407,7 @@ type FrontendMaps struct {
HTTPSSNIMap *HostsMap
//
RedirFromRootMap *HostsMap
RedirRootSSLMap *HostsMap
RedirFromMap *HostsMap
RedirToMap *HostsMap
SSLPassthroughMap *HostsMap
Expand Down
13 changes: 12 additions & 1 deletion rootfs/etc/templates/haproxy/haproxy.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -1156,8 +1156,19 @@ frontend {{ $proxy__front_http }}
http-request set-var(req.host) hdr(host),field(1,:),lower
http-request set-var(req.base) var(req.host),concat(\#,req.path)

{{- /*------------------------------------*/}}
{{- $acmeexclusive := and $global.Acme.Enabled (not $global.Acme.Shared) }}

{{- /*------------------------------------*/}}
{{- if $fmaps.RedirRootSSLMap.HasHost }}
{{- range $match := $fmaps.RedirRootSSLMap.MatchFiles }}
http-request redirect scheme https
{{- if $global.SSL.RedirectCode }} code {{ $global.SSL.RedirectCode }}{{ end }}
{{- "" }} if{{ if $acmeexclusive }} !acme-challenge{{ end }}
{{- "" }} { path / } { var(req.host) -i -m {{ $match.Method }} -f {{ $match.Filename }} }
{{- end }}
{{- end }}

{{- /*------------------------------------*/}}
{{- if $fmaps.RedirFromRootMap.HasHost }}
{{- range $match := $fmaps.RedirFromRootMap.MatchFiles }}
http-request set-var(req.rootredir) var(req.host)
Expand Down
1 change: 1 addition & 0 deletions tests/framework/framework.go
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ func (f *framework) StartController(ctx context.Context, t *testing.T) {
global.Data = map[string]string{
"http-port": "18080",
"https-port": "18443",
"stats-port": "11936",
"max-connections": "20",
}
err = f.cli.Create(ctx, &global)
Expand Down
6 changes: 2 additions & 4 deletions tests/framework/options/objects.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,13 @@ import (

type Object func(o *objectOpt)

func AddConfigKeyAnnotations(ann map[string]string) Object {
func AddConfigKeyAnnotation(key, value string) Object {
annprefix := "haproxy-ingress.github.io/"
return func(o *objectOpt) {
if o.Ann == nil {
o.Ann = make(map[string]string)
}
for k, v := range ann {
o.Ann[annprefix+k] = v
}
o.Ann[annprefix+key] = value
}
}

Expand Down
22 changes: 20 additions & 2 deletions tests/integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func TestIntegrationIngress(t *testing.T) {
svc := f.CreateService(ctx, t, httpServerPort)
_, hostname := f.CreateIngress(ctx, t, svc,
options.DefaultHostTLS(),
options.AddConfigKeyAnnotations(map[string]string{ingtypes.BackSSLRedirect: "false"}),
options.AddConfigKeyAnnotation(ingtypes.BackSSLRedirect, "false"),
)
res := f.Request(ctx, t, http.MethodGet, hostname, "/", options.ExpectResponseCode(http.StatusOK))
assert.True(t, res.EchoResponse)
Expand All @@ -61,7 +61,7 @@ func TestIntegrationIngress(t *testing.T) {
svc := f.CreateService(ctx, t, httpServerPort)
_, hostname := f.CreateIngress(ctx, t, svc,
options.DefaultHostTLS(),
options.AddConfigKeyAnnotations(map[string]string{ingtypes.BackSSLRedirect: "true"}),
options.AddConfigKeyAnnotation(ingtypes.BackSSLRedirect, "true"),
)
res := f.Request(ctx, t, http.MethodGet, hostname, "/", options.ExpectResponseCode(http.StatusFound))
assert.False(t, res.EchoResponse)
Expand Down Expand Up @@ -103,6 +103,24 @@ func TestIntegrationIngress(t *testing.T) {
assert.Equal(t, reqHeaders, res.ReqHeaders)
})

t.Run("should redirect to https before app-root", func(t *testing.T) {
t.Parallel()
svc := f.CreateService(ctx, t, httpServerPort)
_, hostname := f.CreateIngress(ctx, t, svc,
options.DefaultHostTLS(),
options.AddConfigKeyAnnotation(ingtypes.BackSSLRedirect, "true"),
options.AddConfigKeyAnnotation(ingtypes.HostAppRoot, "/app"),
)

res := f.Request(ctx, t, http.MethodGet, hostname, "/", options.ExpectResponseCode(http.StatusFound))
assert.False(t, res.EchoResponse)
assert.Equal(t, fmt.Sprintf("https://%s/", hostname), res.HTTPResponse.Header.Get("location"))

res = f.Request(ctx, t, http.MethodGet, hostname, "/", options.HTTPSRequest(true))
assert.False(t, res.EchoResponse)
assert.Equal(t, "/app", res.HTTPResponse.Header.Get("location"))
})

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

0 comments on commit af22965

Please sign in to comment.