Merge pull request #5276 from bhavin192/mod-sec-disable

Fix the ability to disable ModSecurity at location level
This commit is contained in:
Kubernetes Prow Robot 2020-03-22 12:48:44 -07:00 committed by GitHub
commit 4fc37ea6fd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 84 additions and 27 deletions

View file

@ -25,6 +25,7 @@ import (
// Config contains ModSecurity Configuration items
type Config struct {
Enable bool `json:"enable-modsecurity"`
EnableSet bool `json:"enable-modsecurity-set"`
OWASPRules bool `json:"enable-owasp-core-rules"`
TransactionID string `json:"modsecurity-transaction-id"`
Snippet string `json:"modsecurity-snippet"`
@ -41,6 +42,9 @@ func (modsec1 *Config) Equal(modsec2 *Config) bool {
if modsec1.Enable != modsec2.Enable {
return false
}
if modsec1.EnableSet != modsec2.EnableSet {
return false
}
if modsec1.OWASPRules != modsec2.OWASPRules {
return false
}
@ -69,9 +73,11 @@ func (a modSecurity) Parse(ing *networking.Ingress) (interface{}, error) {
var err error
config := &Config{}
config.EnableSet = true
config.Enable, err = parser.GetBoolAnnotation("enable-modsecurity", ing)
if err != nil {
config.Enable = false
config.EnableSet = false
}
config.OWASPRules, err = parser.GetBoolAnnotation("enable-owasp-core-rules", ing)

View file

@ -41,22 +41,22 @@ func TestParse(t *testing.T) {
annotations map[string]string
expected Config
}{
{map[string]string{enable: "true"}, Config{true, false, "", ""}},
{map[string]string{enable: "false"}, Config{false, false, "", ""}},
{map[string]string{enable: ""}, Config{false, false, "", ""}},
{map[string]string{enable: "true"}, Config{true, true, false, "", ""}},
{map[string]string{enable: "false"}, Config{false, true, false, "", ""}},
{map[string]string{enable: ""}, Config{false, false, false, "", ""}},
{map[string]string{owasp: "true"}, Config{false, true, "", ""}},
{map[string]string{owasp: "false"}, Config{false, false, "", ""}},
{map[string]string{owasp: ""}, Config{false, false, "", ""}},
{map[string]string{owasp: "true"}, Config{false, false, true, "", ""}},
{map[string]string{owasp: "false"}, Config{false, false, false, "", ""}},
{map[string]string{owasp: ""}, Config{false, false, false, "", ""}},
{map[string]string{transID: "ok"}, Config{false, false, "ok", ""}},
{map[string]string{transID: ""}, Config{false, false, "", ""}},
{map[string]string{transID: "ok"}, Config{false, false, false, "ok", ""}},
{map[string]string{transID: ""}, Config{false, false, false, "", ""}},
{map[string]string{snippet: "ModSecurity Rule"}, Config{false, false, "", "ModSecurity Rule"}},
{map[string]string{snippet: ""}, Config{false, false, "", ""}},
{map[string]string{snippet: "ModSecurity Rule"}, Config{false, false, false, "", "ModSecurity Rule"}},
{map[string]string{snippet: ""}, Config{false, false, false, "", ""}},
{map[string]string{}, Config{false, false, "", ""}},
{nil, Config{false, false, "", ""}},
{map[string]string{}, Config{false, false, false, "", ""}},
{nil, Config{false, false, false, "", ""}},
}
ing := &networking.Ingress{

View file

@ -1345,14 +1345,15 @@ func shouldLoadOpentracingModule(c interface{}, s interface{}) bool {
func buildModSecurityForLocation(cfg config.Configuration, location *ingress.Location) string {
isMSEnabledInLoc := location.ModSecurity.Enable
isMSEnableSetInLoc := location.ModSecurity.EnableSet
isMSEnabled := cfg.EnableModsecurity
if !isMSEnabled && !isMSEnabledInLoc {
return ""
}
if !isMSEnabledInLoc {
return ""
if isMSEnableSetInLoc && !isMSEnabledInLoc {
return "modsecurity off;"
}
var buffer bytes.Buffer

View file

@ -1376,6 +1376,8 @@ func TestModSecurityForLocation(t *testing.T) {
modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;
`
modsecOff := "modsecurity off;"
modsecRule := `modsecurity_rules '
#RULE#
';
@ -1394,30 +1396,34 @@ modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;
isEnabledInCM bool
isOwaspEnabledInCM bool
isEnabledInLoc bool
isEnableSetInLoc bool
isOwaspEnabledInLoc bool
snippet string
transactionID string
expected string
}{
{"configmap enabled, configmap OWASP disabled, without annotation, snippet or transaction ID", true, false, false, false, "", "", ""},
{"configmap enabled, configmap OWASP disabled, without annotation, snippet and with transaction ID", true, false, false, false, "", transactionID, ""},
{"configmap enabled, configmap OWASP enabled, without annotation, OWASP enabled", true, true, false, false, "", "", ""},
{"configmap enabled, configmap OWASP enabled, without annotation, OWASP disabled, with snippet and no transaction ID", true, true, true, false, testRule, "", modsecRule},
{"configmap enabled, configmap OWASP enabled, without annotation, OWASP disabled, with snippet and transaction ID", true, true, true, false, testRule, transactionID, fmt.Sprintf("%v%v", modsecRule, transactionCfg)},
{"configmap enabled, with annotation, OWASP disabled", true, false, true, false, "", "", ""},
{"configmap enabled, configmap OWASP disabled, with annotation, OWASP enabled, no snippet and no transaction ID", true, false, true, true, "", "", owaspRules},
{"configmap enabled, configmap OWASP disabled, with annotation, OWASP enabled, with snippet and no transaction ID", true, false, true, true, "", "", owaspRules},
{"configmap enabled, configmap OWASP disabled, with annotation, OWASP enabled, with snippet and transaction ID", true, false, true, true, "", transactionID, fmt.Sprintf("%v%v", owaspRules, transactionCfg)},
{"configmap enabled, OWASP configmap enabled, with annotation, OWASP disabled", true, true, true, false, "", "", ""},
{"configmap disabled, with annotation, OWASP disabled", false, false, true, false, "", "", loadModule},
{"configmap disabled, with annotation, OWASP disabled", false, false, true, false, testRule, "", fmt.Sprintf("%v%v", loadModule, modsecRule)},
{"configmap disabled, with annotation, OWASP enabled", false, false, true, false, testRule, "", fmt.Sprintf("%v%v", loadModule, modsecRule)},
{"configmap enabled, configmap OWASP disabled, without annotation, snippet or transaction ID", true, false, false, false, false, "", "", ""},
{"configmap enabled, configmap OWASP disabled, without annotation, snippet and with transaction ID", true, false, false, false, false, "", transactionID, transactionCfg},
{"configmap enabled, configmap OWASP enabled, without annotation, OWASP enabled", true, true, false, false, false, "", "", ""},
{"configmap enabled, configmap OWASP enabled, without annotation, OWASP disabled, with snippet and no transaction ID", true, true, false, false, false, testRule, "", modsecRule},
{"configmap enabled, configmap OWASP enabled, without annotation, OWASP disabled, with snippet and transaction ID", true, true, false, false, false, testRule, transactionID, fmt.Sprintf("%v%v", modsecRule, transactionCfg)},
{"configmap enabled, configmap OWASP disabled, annotation enabled, OWASP disabled", true, false, true, true, false, "", "", ""},
{"configmap enabled, configmap OWASP disabled, annotation enabled, OWASP enabled, no snippet and no transaction ID", true, false, true, true, true, "", "", owaspRules},
{"configmap enabled, configmap OWASP disabled, annotation disabled, OWASP disabled, no snippet and no transaction ID", true, false, false, true, false, "", "", modsecOff},
{"configmap enabled, configmap OWASP disabled, annotation enabled, OWASP enabled, with snippet and no transaction ID", true, false, true, true, true, "", "", owaspRules},
{"configmap enabled, configmap OWASP disabled, annotation enabled, OWASP enabled, with snippet and transaction ID", true, false, true, true, true, "", transactionID, fmt.Sprintf("%v%v", owaspRules, transactionCfg)},
{"configmap enabled, configmap OWASP enabled, annotation enabled, OWASP disabled", true, true, true, true, false, "", "", ""},
{"configmap disabled, annotation enabled, OWASP disabled", false, false, true, true, false, "", "", loadModule},
{"configmap disabled, annotation disabled, OWASP disabled", false, false, false, true, false, "", "", ""},
{"configmap disabled, annotation enabled, OWASP disabled", false, false, true, true, false, testRule, "", fmt.Sprintf("%v%v", loadModule, modsecRule)},
{"configmap disabled, annotation enabled, OWASP enabled", false, false, true, true, false, testRule, "", fmt.Sprintf("%v%v", loadModule, modsecRule)},
}
for _, testCase := range testCases {
il := &ingress.Location{
ModSecurity: modsecurity.Config{
Enable: testCase.isEnabledInLoc,
EnableSet: testCase.isEnableSetInLoc,
OWASPRules: testCase.isOwaspEnabledInLoc,
Snippet: testCase.snippet,
TransactionID: testCase.transactionID,

View file

@ -104,4 +104,48 @@ var _ = framework.DescribeAnnotation("modsecurity owasp", func() {
strings.Contains(server, "SecRuleEngine On")
})
})
ginkgo.It("should enable modsecurity without using 'modsecurity on;'", func() {
f.SetNginxConfigMapData(map[string]string{
"enable-modsecurity": "true"},
)
host := "modsecurity.foo.com"
nameSpace := f.Namespace
annotations := map[string]string{
"nginx.ingress.kubernetes.io/enable-modsecurity": "true",
}
ing := framework.NewSingleIngress(host, "/", host, nameSpace, framework.EchoService, 80, annotations)
f.EnsureIngress(ing)
f.WaitForNginxServer(host,
func(server string) bool {
return !strings.Contains(server, "modsecurity on;") &&
!strings.Contains(server, "modsecurity_rules_file /etc/nginx/modsecurity/modsecurity.conf;")
})
})
ginkgo.It("should disable modsecurity using 'modsecurity off;'", func() {
f.SetNginxConfigMapData(map[string]string{
"enable-modsecurity": "true"},
)
host := "modsecurity.foo.com"
nameSpace := f.Namespace
annotations := map[string]string{
"nginx.ingress.kubernetes.io/enable-modsecurity": "false",
}
ing := framework.NewSingleIngress(host, "/", host, nameSpace, framework.EchoService, 80, annotations)
f.EnsureIngress(ing)
f.WaitForNginxServer(host,
func(server string) bool {
return strings.Contains(server, "modsecurity off;")
})
})
})