Skip to content

Commit

Permalink
Merge pull request #963 from mac-chaffee/revert-waf-fail-closed
Browse files Browse the repository at this point in the history 
Revert "Merge pull request #954 from mac-chaffee/fail-closed"
  • Loading branch information
@jcmoraisjr
jcmoraisjr authored Nov 21, 2022
2 parents ff2953b + b0c381e commit 635669c
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 70 deletions.
7 changes: 1 addition & 6 deletions docs/content/en/docs/configuration/keys.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,6 @@ The table below describes all supported configuration keys.
| [`username`](#security) | haproxy user name | Global | `haproxy` |
| [`var-namespace`](#var-namespace) | [true\|false] | Host | `false` |
| [`waf`](#waf) | "modsecurity" | Path | |
| [`waf-fail-closed`](#waf) | [true\|false] | Path | `true` |
| [`waf-mode`](#waf) | [deny\|detect] | Path | `deny` (if waf is set) |
| [`whitelist-source-range`](#allowlist) | Comma-separated IPs or CIDRs | Path | |
| [`worker-max-reloads`](#master-worker) | number of reloads | Global | `0` |
Expand Down Expand Up @@ -2856,20 +2855,16 @@ See also:
| Configuration key | Scope | Default | Since |
|-------------------|--------|---------|-------|
| `waf` | `Path` | | |
| `waf-fail-closed` | `Path` | `true` | v0.14 |
| `waf-mode` | `Path` | `deny` | v0.9 |


Defines which web application firewall (WAF) implementation should be used
to validate requests. Currently the only supported value is `modsecurity`.

This configuration has no effect if the ModSecurity endpoints are not configured.

The `waf-fail-closed` key determines whether an error from the WAF (such as a timeout) should
cause the request to be denied ("true") or allowed ("false"). In v0.14, this defaults to "false" for backwards compatibility. In v0.15, this defaults to "true" which means that errors or timeouts will cause requests to be denied.

The `waf-mode` key defines whether the WAF should be `deny` or `detect` for that Backend.
If the WAF is in `detect` mode the requests are passed to ModSecurity and logged, but not denied.

The default behavior here is `deny` if `waf` is set to `modsecurity`.

See also:
Expand Down
3 changes: 0 additions & 3 deletions pkg/converters/ingress/annotations/backend.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -997,11 +997,8 @@ func (c *updater) buildBackendWAF(d *backData) {
c.logger.Warn("ignoring invalid WAF mode '%s' on %s, using 'deny' instead", mode, wafMode.Source)
mode = "deny"
}
wafFailClosed := config.Get(ingtypes.BackWAFFailClosed).Bool()

path.WAF.Module = module
path.WAF.Mode = mode
path.WAF.FailClosed = wafFailClosed
}
}

Expand Down
1 change: 0 additions & 1 deletion pkg/converters/ingress/defaults.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ func createDefaults() map[string]string {
types.BackTimeoutServer: "50s",
types.BackTimeoutServerFin: "50s",
types.BackTimeoutTunnel: "1h",
types.BackWAFFailClosed: "true",
types.BackWAFMode: "deny",
//
types.GlobalAcmeExpiring: "30",
Expand Down
1 change: 0 additions & 1 deletion pkg/converters/ingress/types/annotations.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ const (
BackTimeoutTunnel = "timeout-tunnel"
BackUseResolver = "use-resolver"
BackWAF = "waf"
BackWAFFailClosed = "waf-fail-closed"
BackWAFMode = "waf-mode"
BackWhitelistSourceRange = "whitelist-source-range"
)
Expand Down
83 changes: 30 additions & 53 deletions pkg/haproxy/instance_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4794,7 +4794,6 @@ frontend healthz
func TestModSecurity(t *testing.T) {
testCases := []struct {
waf string
wafFailClosed bool
wafmode string
path string
endpoints []string
Expand All @@ -4804,42 +4803,38 @@ func TestModSecurity(t *testing.T) {
modsecAgentExp string
}{
{
waf: "modsecurity",
wafFailClosed: true,
wafmode: "On",
endpoints: []string{},
backendExp: ``,
modsecExp: ``,
waf: "modsecurity",
wafmode: "On",
endpoints: []string{},
backendExp: ``,
modsecExp: ``,
},
{
waf: "",
wafFailClosed: true,
wafmode: "",
endpoints: []string{"10.0.0.101:12345"},
backendExp: ``,
waf: "",
wafmode: "",
endpoints: []string{"10.0.0.101:12345"},
backendExp: ``,
modsecExp: `
timeout connect 1s
timeout server 2s
server modsec-spoa0 10.0.0.101:12345`,
},
{
waf: "modsecurity",
wafFailClosed: true,
wafmode: "deny",
endpoints: []string{"10.0.0.101:12345"},
waf: "modsecurity",
wafmode: "deny",
endpoints: []string{"10.0.0.101:12345"},
backendExp: `
filter spoe engine modsecurity config /etc/haproxy/spoe-modsecurity.conf
http-request deny if !{ var(txn.modsec.code) -m int eq 0 }`,
http-request deny if { var(txn.modsec.code) -m int gt 0 }`,
modsecExp: `
timeout connect 1s
timeout server 2s
server modsec-spoa0 10.0.0.101:12345`,
},
{
waf: "modsecurity",
wafFailClosed: true,
wafmode: "detect",
endpoints: []string{"10.0.0.101:12345"},
waf: "modsecurity",
wafmode: "detect",
endpoints: []string{"10.0.0.101:12345"},
backendExp: `
filter spoe engine modsecurity config /etc/haproxy/spoe-modsecurity.conf`,
modsecExp: `
Expand All @@ -4848,42 +4843,39 @@ func TestModSecurity(t *testing.T) {
server modsec-spoa0 10.0.0.101:12345`,
},
{
waf: "modsecurity",
wafFailClosed: true,
wafmode: "deny",
endpoints: []string{"10.0.0.101:12345", "10.0.0.102:12345"},
waf: "modsecurity",
wafmode: "deny",
endpoints: []string{"10.0.0.101:12345", "10.0.0.102:12345"},
backendExp: `
filter spoe engine modsecurity config /etc/haproxy/spoe-modsecurity.conf
http-request deny if !{ var(txn.modsec.code) -m int eq 0 }`,
http-request deny if { var(txn.modsec.code) -m int gt 0 }`,
modsecExp: `
timeout connect 1s
timeout server 2s
server modsec-spoa0 10.0.0.101:12345
server modsec-spoa1 10.0.0.102:12345`,
},
{
waf: "modsecurity",
wafFailClosed: true,
wafmode: "deny",
endpoints: []string{"10.0.0.101:12345"},
path: "/sub",
waf: "modsecurity",
wafmode: "deny",
endpoints: []string{"10.0.0.101:12345"},
path: "/sub",
backendExp: `
# path02 = d1.local/
# path01 = d1.local/sub
http-request set-var(txn.pathID) var(req.base),lower,map_beg(/etc/haproxy/maps/_back_d1_app_8080_idpath__begin.map)
filter spoe engine modsecurity config /etc/haproxy/spoe-modsecurity.conf
http-request deny if !{ var(txn.modsec.code) -m int eq 0 } { var(txn.pathID) -m str path01 }`,
http-request deny if { var(txn.modsec.code) -m int gt 0 } { var(txn.pathID) -m str path01 }`,
modsecExp: `
timeout connect 1s
timeout server 2s
server modsec-spoa0 10.0.0.101:12345`,
},
// Test setting custom args
{
waf: "modsecurity",
wafFailClosed: true,
wafmode: "detect",
endpoints: []string{"10.0.0.101:12345"},
waf: "modsecurity",
wafmode: "detect",
endpoints: []string{"10.0.0.101:12345"},
backendExp: `
filter spoe engine modsecurity config /etc/haproxy/spoe-modsecurity.conf`,
modsecExp: `
Expand All @@ -4896,20 +4888,6 @@ func TestModSecurity(t *testing.T) {
args unique-id method path query req.ver req.hdrs_bin
event on-backend-http-request`,
},
// Test waf-fail-closed=false
{
waf: "modsecurity",
wafFailClosed: false,
wafmode: "deny",
endpoints: []string{"10.0.0.101:12345"},
backendExp: `
filter spoe engine modsecurity config /etc/haproxy/spoe-modsecurity.conf
http-request deny if { var(txn.modsec.code) -m int gt 0 }`,
modsecExp: `
timeout connect 1s
timeout server 2s
server modsec-spoa0 10.0.0.101:12345`,
},
}
for _, test := range testCases {
c := setup(t)
Expand All @@ -4923,9 +4901,8 @@ func TestModSecurity(t *testing.T) {
}
h.AddPath(b, test.path, hatypes.MatchBegin)
b.FindBackendPath(h.FindPath(test.path)[0].Link).WAF = hatypes.WAF{
Module: test.waf,
Mode: test.wafmode,
FailClosed: test.wafFailClosed,
Module: test.waf,
Mode: test.wafmode,
}
if test.path != "/" {
h.AddPath(b, "/", hatypes.MatchBegin)
Expand Down
2 changes: 0 additions & 2 deletions pkg/haproxy/types/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -850,8 +850,6 @@ type HSTS struct {
type WAF struct {
// Mode defines On or DetectionOnly
Mode string
// Whether WAF errors should cause the request to be denied
FailClosed bool
// Which WAF Module should be used
Module string
}
Expand Down
4 changes: 0 additions & 4 deletions rootfs/etc/templates/haproxy/haproxy.tmpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -582,11 +582,7 @@ backend {{ $backend.ID }}
{{- range $i, $waf := $wafCfg.Items }}
{{- if eq $waf.Mode "deny" }}
{{- range $pathIDs := $wafCfg.PathIDs $i }}
{{- if $waf.FailClosed }}
http-request deny if !{ var(txn.modsec.code) -m int eq 0 }
{{- else }}
http-request deny if { var(txn.modsec.code) -m int gt 0 }
{{- end }}
{{- if $pathIDs }} { var(txn.pathID) -m str {{ $pathIDs }} }{{ end }}
{{- end }}
{{- end }}
Expand Down

0 comments on commit 635669c

Please sign in to comment.