From e924bb695f6de71d3779edc705113247f6258537 Mon Sep 17 00:00:00 2001 From: Eoin O'Shaughnessy Date: Fri, 15 Mar 2024 14:41:37 +0000 Subject: [PATCH 1/9] add support for apLogBundle --- config/crd/bases/k8s.nginx.org_policies.yaml | 4 + deploy/crds.yaml | 4 + internal/configs/configurator.go | 2 +- internal/configs/virtualserver.go | 97 ++++++--- internal/configs/virtualserver_test.go | 189 +++++++++++++----- pkg/apis/configuration/v1/types.go | 7 +- pkg/apis/configuration/validation/policy.go | 42 +++- .../configuration/validation/policy_test.go | 98 +++++++++ 8 files changed, 356 insertions(+), 87 deletions(-) diff --git a/config/crd/bases/k8s.nginx.org_policies.yaml b/config/crd/bases/k8s.nginx.org_policies.yaml index 1954063005..e31e43c67a 100644 --- a/config/crd/bases/k8s.nginx.org_policies.yaml +++ b/config/crd/bases/k8s.nginx.org_policies.yaml @@ -187,6 +187,8 @@ spec: securityLog: description: SecurityLog defines the security log of a WAF policy. properties: + apLogBundle: + type: string apLogConf: type: string enable: @@ -198,6 +200,8 @@ spec: items: description: SecurityLog defines the security log of a WAF policy. properties: + apLogBundle: + type: string apLogConf: type: string enable: diff --git a/deploy/crds.yaml b/deploy/crds.yaml index b9498dd6dd..4f18a91c37 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -389,6 +389,8 @@ spec: securityLog: description: SecurityLog defines the security log of a WAF policy. properties: + apLogBundle: + type: string apLogConf: type: string enable: @@ -400,6 +402,8 @@ spec: items: description: SecurityLog defines the security log of a WAF policy. properties: + apLogBundle: + type: string apLogConf: type: string enable: diff --git a/internal/configs/configurator.go b/internal/configs/configurator.go index 6ccd1e2b23..7c41dd0ea7 100644 --- a/internal/configs/configurator.go +++ b/internal/configs/configurator.go @@ -584,7 +584,7 @@ func (cnf *Configurator) addOrUpdateVirtualServer(virtualServerEx *VirtualServer name := getFileNameForVirtualServer(virtualServerEx.VirtualServer) - vsc := newVirtualServerConfigurator(cnf.cfgParams, cnf.isPlus, cnf.IsResolverConfigured(), cnf.staticCfgParams, cnf.isWildcardEnabled) + vsc := newVirtualServerConfigurator(cnf.cfgParams, cnf.isPlus, cnf.IsResolverConfigured(), cnf.staticCfgParams, cnf.isWildcardEnabled, nil) vsCfg, warnings := vsc.GenerateVirtualServerConfig(virtualServerEx, apResources, dosResources) content, err := cnf.templateExecutorV2.ExecuteVirtualServerTemplate(&vsCfg) if err != nil { diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 38d76f55e4..ff6ee6f35d 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -3,6 +3,8 @@ package configs import ( "fmt" "net/url" + "os" + "path" "sort" "strconv" "strings" @@ -25,6 +27,7 @@ const ( specContext = "spec" routeContext = "route" subRouteContext = "subroute" + defaultLogOutput = "syslog:server=localhost:514" ) var grpcConflictingErrors = map[int]bool{ @@ -240,6 +243,7 @@ type virtualServerConfigurator struct { isIPV6Disabled bool DynamicSSLReloadEnabled bool StaticSSLPath string + bundleValidator bundleValidator } type oidcPolicyCfg struct { @@ -268,7 +272,11 @@ func newVirtualServerConfigurator( isResolverConfigured bool, staticParams *StaticConfigParams, isWildcardEnabled bool, + bundleValidator bundleValidator, ) *virtualServerConfigurator { + if bundleValidator == nil { + bundleValidator = newInternalBundleValidator(appProtectBundleFolder) + } return &virtualServerConfigurator{ cfgParams: cfgParams, isPlus: isPlus, @@ -283,6 +291,7 @@ func newVirtualServerConfigurator( isIPV6Disabled: staticParams.DisableIPV6, DynamicSSLReloadEnabled: staticParams.DynamicSSLReload, StaticSSLPath: staticParams.StaticSSLPath, + bundleValidator: bundleValidator, } } @@ -789,10 +798,34 @@ type policiesCfg struct { OIDC bool WAF *version2.WAF ErrorReturn *version2.Return + BundleValidator bundleValidator +} + +type bundleValidator interface { + // validate returns the full path to the bundle and an error if the file is not accessible + validate(string) (string, error) +} + +type internalBundleValidator struct { + bundlePath string +} + +func (i internalBundleValidator) validate(bundle string) (string, error) { + bundle = path.Join(i.bundlePath, bundle) + _, err := os.Stat(bundle) + return bundle, err } -func newPoliciesConfig() *policiesCfg { - return &policiesCfg{} +func newInternalBundleValidator(b string) internalBundleValidator { + return internalBundleValidator{ + bundlePath: b, + } +} + +func newPoliciesConfig(bv bundleValidator) *policiesCfg { + return &policiesCfg{ + BundleValidator: bv, + } } type policyOwnerDetails struct { @@ -1229,42 +1262,46 @@ func (p *policiesCfg) addWAFConfig( if waf.ApBundle != "" { p.WAF.ApBundle = appProtectBundleFolder + waf.ApBundle + bundlePath, err := p.BundleValidator.validate(waf.ApBundle) + if err != nil { + res.addWarningf("WAF policy %s references an invalid or non-existing App Protect bundle %s", polKey, bundlePath) + res.isError = true + } + p.WAF.ApBundle = bundlePath } if waf.SecurityLog != nil && waf.SecurityLogs == nil { - glog.V(2).Info("the field securityLog is deprecated nad will be removed in future releases. Use field securityLogs instead") - p.WAF.ApSecurityLogEnable = true - - logConfKey := waf.SecurityLog.ApLogConf - hasNamespace := strings.Contains(logConfKey, "/") - if !hasNamespace { - logConfKey = fmt.Sprintf("%v/%v", polNamespace, logConfKey) - } - - if logConfPath, ok := apResources.LogConfs[logConfKey]; ok { - logDest := generateString(waf.SecurityLog.LogDest, "syslog:server=localhost:514") - p.WAF.ApLogConf = []string{fmt.Sprintf("%s %s", logConfPath, logDest)} - } else { - res.addWarningf("WAF policy %s references an invalid or non-existing log config %s", polKey, logConfKey) - res.isError = true - } + glog.V(2).Info("the field securityLog is deprecated and will be removed in future releases. Use field securityLogs instead") + waf.SecurityLogs = append(waf.SecurityLogs, waf.SecurityLog) } if waf.SecurityLogs != nil { p.WAF.ApSecurityLogEnable = true p.WAF.ApLogConf = []string{} for _, loco := range waf.SecurityLogs { - logConfKey := loco.ApLogConf - hasNamepace := strings.Contains(logConfKey, "/") - if !hasNamepace { - logConfKey = fmt.Sprintf("%v/%v", polNamespace, logConfKey) + logDest := generateString(loco.LogDest, defaultLogOutput) + + if loco.ApLogConf != "" { + logConfKey := loco.ApLogConf + if !strings.Contains(logConfKey, "/") { + logConfKey = fmt.Sprintf("%v/%v", polNamespace, logConfKey) + } + if logConfPath, ok := apResources.LogConfs[logConfKey]; ok { + p.WAF.ApLogConf = append(p.WAF.ApLogConf, fmt.Sprintf("%s %s", logConfPath, logDest)) + } else { + res.addWarningf("WAF policy %s references an invalid or non-existing log config %s", polKey, logConfKey) + res.isError = true + } } - if logConfPath, ok := apResources.LogConfs[logConfKey]; ok { - logDest := generateString(loco.LogDest, "syslog:server=localhost:514") - p.WAF.ApLogConf = append(p.WAF.ApLogConf, fmt.Sprintf("%s %s", logConfPath, logDest)) - } else { - res.addWarningf("WAF policy %s references an invalid or non-existing log config %s", polKey, logConfKey) - res.isError = true + + if loco.ApLogBundle != "" { + logBundle, err := p.BundleValidator.validate(loco.ApLogBundle) + if err != nil { + res.addWarningf("WAF policy %s references an invalid or non-existing log config bundle %s", polKey, logBundle) + res.isError = true + } else { + p.WAF.ApLogConf = append(p.WAF.ApLogConf, fmt.Sprintf("%s %s", logBundle, logDest)) + } } } } @@ -1278,7 +1315,7 @@ func (vsc *virtualServerConfigurator) generatePolicies( context string, policyOpts policyOptions, ) policiesCfg { - config := newPoliciesConfig() + config := newPoliciesConfig(vsc.bundleValidator) for _, p := range policyRefs { polNamespace := p.Namespace @@ -2426,7 +2463,7 @@ func createUpstreamsForPlus( isPlus := true upstreamNamer := NewUpstreamNamerForVirtualServer(virtualServerEx.VirtualServer) - vsc := newVirtualServerConfigurator(baseCfgParams, isPlus, false, staticParams, false) + vsc := newVirtualServerConfigurator(baseCfgParams, isPlus, false, staticParams, false, nil) for _, u := range virtualServerEx.VirtualServer.Spec.Upstreams { isExternalNameSvc := virtualServerEx.ExternalNameSvcs[GenerateExternalNameSvcKey(virtualServerEx.VirtualServer.Namespace, u.Service)] diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index 9739716311..c072b07277 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -5,6 +5,7 @@ import ( "fmt" "reflect" "sort" + "strings" "testing" "github.com/google/go-cmp/cmp" @@ -213,7 +214,7 @@ func TestVariableNamer(t *testing.T) { func TestGenerateVSConfig_GeneratesConfigWithGunzipOn(t *testing.T) { t.Parallel() - vsc := newVirtualServerConfigurator(&baseCfgParams, true, false, &StaticConfigParams{TLSPassthrough: true}, false) + vsc := newVirtualServerConfigurator(&baseCfgParams, true, false, &StaticConfigParams{TLSPassthrough: true}, false, &fakeBV) want := version2.VirtualServerConfig{ Upstreams: []version2.Upstream{ @@ -471,7 +472,7 @@ func TestGenerateVSConfig_GeneratesConfigWithGunzipOn(t *testing.T) { func TestGenerateVSConfig_GeneratesConfigWithGunzipOff(t *testing.T) { t.Parallel() - vsc := newVirtualServerConfigurator(&baseCfgParams, true, false, &StaticConfigParams{TLSPassthrough: true}, false) + vsc := newVirtualServerConfigurator(&baseCfgParams, true, false, &StaticConfigParams{TLSPassthrough: true}, false, &fakeBV) want := version2.VirtualServerConfig{ Upstreams: []version2.Upstream{ @@ -729,7 +730,7 @@ func TestGenerateVSConfig_GeneratesConfigWithGunzipOff(t *testing.T) { func TestGenerateVSConfig_GeneratesConfigWithNoGunzip(t *testing.T) { t.Parallel() - vsc := newVirtualServerConfigurator(&baseCfgParams, true, false, &StaticConfigParams{TLSPassthrough: true}, false) + vsc := newVirtualServerConfigurator(&baseCfgParams, true, false, &StaticConfigParams{TLSPassthrough: true}, false, &fakeBV) want := version2.VirtualServerConfig{ Upstreams: []version2.Upstream{ @@ -1469,6 +1470,7 @@ func TestGenerateVirtualServerConfigWithBackupForNGINXPlus(t *testing.T) { isResolverConfigured, &StaticConfigParams{TLSPassthrough: true}, isWildcardEnabled, + &fakeBV, ) sort.Slice(want.Upstreams, func(i, j int) bool { @@ -1775,6 +1777,7 @@ func TestGenerateVirtualServerConfig_DoesNotGenerateBackupOnMissingBackupNameFor isResolverConfigured, &StaticConfigParams{TLSPassthrough: true}, isWildcardEnabled, + &fakeBV, ) sort.Slice(want.Upstreams, func(i, j int) bool { @@ -2084,6 +2087,7 @@ func TestGenerateVirtualServerConfig_DoesNotGenerateBackupOnMissingBackupPortFor isResolverConfigured, &StaticConfigParams{TLSPassthrough: true}, isWildcardEnabled, + &fakeBV, ) got, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil, nil) @@ -2383,6 +2387,7 @@ func TestGenerateVirtualServerConfig_DoesNotGenerateBackupOnMissingBackupPortAnd isResolverConfigured, &StaticConfigParams{TLSPassthrough: true}, isWildcardEnabled, + &fakeBV, ) sort.Slice(want.Upstreams, func(i, j int) bool { @@ -2860,6 +2865,7 @@ func TestGenerateVirtualServerConfig(t *testing.T) { isResolverConfigured, &StaticConfigParams{TLSPassthrough: true}, isWildcardEnabled, + &fakeBV, ) result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil, nil) @@ -2904,6 +2910,7 @@ func TestGenerateVirtualServerConfigWithCustomHttpAndHttpsListeners(t *testing.T false, &StaticConfigParams{DisableIPV6: true}, false, + &fakeBV, ) result, warnings := vsc.GenerateVirtualServerConfig( @@ -2952,6 +2959,7 @@ func TestGenerateVirtualServerConfigWithCustomHttpListener(t *testing.T) { false, &StaticConfigParams{DisableIPV6: true}, false, + &fakeBV, ) result, warnings := vsc.GenerateVirtualServerConfig( @@ -3000,6 +3008,7 @@ func TestGenerateVirtualServerConfigWithCustomHttpsListener(t *testing.T) { false, &StaticConfigParams{DisableIPV6: true}, false, + &fakeBV, ) result, warnings := vsc.GenerateVirtualServerConfig( @@ -3048,6 +3057,7 @@ func TestGenerateVirtualServerConfigWithNilListener(t *testing.T) { false, &StaticConfigParams{DisableIPV6: true}, false, + &fakeBV, ) result, warnings := vsc.GenerateVirtualServerConfig( @@ -3187,6 +3197,7 @@ func TestGenerateVirtualServerConfigIPV6Disabled(t *testing.T) { isResolverConfigured, &StaticConfigParams{DisableIPV6: true}, isWildcardEnabled, + &fakeBV, ) result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil, nil) @@ -3553,7 +3564,7 @@ func TestGenerateVirtualServerConfigGrpcErrorPageWarning(t *testing.T) { isPlus := false isResolverConfigured := false isWildcardEnabled := true - vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, &StaticConfigParams{}, isWildcardEnabled) + vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, &StaticConfigParams{}, isWildcardEnabled, &fakeBV) result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil, nil) if diff := cmp.Diff(expected, result); diff != "" { @@ -3663,7 +3674,7 @@ func TestGenerateVirtualServerConfigWithSpiffeCerts(t *testing.T) { isResolverConfigured := false staticConfigParams := &StaticConfigParams{TLSPassthrough: true, NginxServiceMesh: true} isWildcardEnabled := false - vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, staticConfigParams, isWildcardEnabled) + vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, staticConfigParams, isWildcardEnabled, &fakeBV) result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil, nil) if diff := cmp.Diff(expected, result); diff != "" { @@ -3776,7 +3787,7 @@ func TestGenerateVirtualServerConfigWithInternalRoutes(t *testing.T) { isResolverConfigured := false staticConfigParams := &StaticConfigParams{TLSPassthrough: true, NginxServiceMesh: true, EnableInternalRoutes: true} isWildcardEnabled := false - vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, staticConfigParams, isWildcardEnabled) + vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, staticConfigParams, isWildcardEnabled, &fakeBV) result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil, nil) if diff := cmp.Diff(expected, result); diff != "" { @@ -3889,7 +3900,7 @@ func TestGenerateVirtualServerConfigWithInternalRoutesWarning(t *testing.T) { isResolverConfigured := false staticConfigParams := &StaticConfigParams{TLSPassthrough: true, NginxServiceMesh: true, EnableInternalRoutes: false} isWildcardEnabled := false - vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, staticConfigParams, isWildcardEnabled) + vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, staticConfigParams, isWildcardEnabled, &fakeBV) result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil, nil) if diff := cmp.Diff(expected, result); diff == "" { @@ -4176,7 +4187,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithSplits(t *testing.T) { isPlus := false isResolverConfigured := false isWildcardEnabled := false - vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, &StaticConfigParams{}, isWildcardEnabled) + vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, &StaticConfigParams{}, isWildcardEnabled, &fakeBV) result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil, nil) if diff := cmp.Diff(expected, result); diff != "" { @@ -4495,7 +4506,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithMatches(t *testing.T) { isPlus := false isResolverConfigured := false isWildcardEnabled := false - vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, &StaticConfigParams{}, isWildcardEnabled) + vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, &StaticConfigParams{}, isWildcardEnabled, &fakeBV) result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil, nil) if diff := cmp.Diff(expected, result); diff != "" { @@ -4822,7 +4833,7 @@ func TestGenerateVirtualServerConfigForVirtualServerRoutesWithDos(t *testing.T) isPlus := false isResolverConfigured := false - vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, &StaticConfigParams{MainAppProtectDosLoadModule: true}, false) + vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, &StaticConfigParams{MainAppProtectDosLoadModule: true}, false, &fakeBV) result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil, dosResources) if diff := cmp.Diff(expected, result.Server.Locations); diff != "" { @@ -5297,7 +5308,7 @@ func TestGenerateVirtualServerConfigForVirtualServerWithReturns(t *testing.T) { isPlus := false isResolverConfigured := false isWildcardEnabled := false - vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, &StaticConfigParams{}, isWildcardEnabled) + vsc := newVirtualServerConfigurator(&baseCfgParams, isPlus, isResolverConfigured, &StaticConfigParams{}, isWildcardEnabled, &fakeBV) result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil, nil) if !reflect.DeepEqual(result, expected) { @@ -5551,6 +5562,7 @@ func TestGenerateVirtualServerConfigJWKSPolicy(t *testing.T) { false, &StaticConfigParams{TLSPassthrough: true}, false, + &fakeBV, ) result, warnings := vsc.GenerateVirtualServerConfig(&virtualServerEx, nil, nil) @@ -6192,10 +6204,11 @@ func TestGeneratePolicies(t *testing.T) { }, } - vsc := newVirtualServerConfigurator(&ConfigParams{}, false, false, &StaticConfigParams{}, false) + vsc := newVirtualServerConfigurator(&ConfigParams{}, false, false, &StaticConfigParams{}, false, &fakeBV) for _, test := range tests { result := vsc.generatePolicies(ownerDetails, test.policyRefs, test.policies, test.context, policyOpts) + result.BundleValidator = nil if diff := cmp.Diff(test.expected, result); diff != "" { t.Errorf("generatePolicies() '%v' mismatch (-want +got):\n%s", test.msg, diff) } @@ -6215,42 +6228,84 @@ func TestGeneratePolicies_GeneratesWAFPolicyOnValidApBundle(t *testing.T) { vsName: "test", } - test := struct { + tests := []struct { + name string policyRefs []conf_v1.PolicyReference policies map[string]*conf_v1.Policy policyOpts policyOptions context string want policiesCfg }{ - policyRefs: []conf_v1.PolicyReference{ - { - Name: "waf-bundle", - Namespace: "default", + { + name: "valid bundle", + policyRefs: []conf_v1.PolicyReference{ + { + Name: "waf-bundle", + Namespace: "default", + }, }, - }, - policies: map[string]*conf_v1.Policy{ - "default/waf-bundle": { - Spec: conf_v1.PolicySpec{ - WAF: &conf_v1.WAF{ - Enable: true, - ApBundle: "testWAFPolicyBundle.tgz", + policies: map[string]*conf_v1.Policy{ + "default/waf-bundle": { + Spec: conf_v1.PolicySpec{ + WAF: &conf_v1.WAF{ + Enable: true, + ApBundle: "testWAFPolicyBundle.tgz", + }, }, }, }, + context: "route", + want: policiesCfg{ + WAF: &version2.WAF{ + Enable: "on", + ApBundle: "/etc/nginx/waf/bundles/testWAFPolicyBundle.tgz", + }, + }, }, - context: "route", - } - - vsc := newVirtualServerConfigurator(&ConfigParams{}, false, false, &StaticConfigParams{}, false) - want := policiesCfg{ - WAF: &version2.WAF{ - Enable: "on", - ApBundle: "/etc/nginx/waf/bundles/testWAFPolicyBundle.tgz", + { + name: "valid bundle with logConf", + policyRefs: []conf_v1.PolicyReference{ + { + Name: "waf-bundle", + Namespace: "default", + }, + }, + policies: map[string]*conf_v1.Policy{ + "default/waf-bundle": { + Spec: conf_v1.PolicySpec{ + WAF: &conf_v1.WAF{ + Enable: true, + ApBundle: "testWAFPolicyBundle.tgz", + SecurityLogs: []*conf_v1.SecurityLog{ + { + Enable: true, + ApLogBundle: "secops_dashboard.tgz", + }, + }, + }, + }, + }, + }, + context: "route", + want: policiesCfg{ + WAF: &version2.WAF{ + Enable: "on", + ApBundle: "/etc/nginx/waf/bundles/testWAFPolicyBundle.tgz", + ApSecurityLogEnable: true, + ApLogConf: []string{"/etc/nginx/waf/bundles/secops_dashboard.tgz syslog:server=localhost:514"}, + }, + }, }, } - got := vsc.generatePolicies(ownerDetails, test.policyRefs, test.policies, test.context, policyOptions{}) - if !cmp.Equal(want, got) { - t.Error(cmp.Diff(want, got)) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + vsc := newVirtualServerConfigurator(&ConfigParams{}, false, false, &StaticConfigParams{}, false, &fakeBV) + got := vsc.generatePolicies(ownerDetails, tc.policyRefs, tc.policies, tc.context, policyOptions{apResources: &appProtectResourcesForVS{}}) + got.BundleValidator = nil + if !cmp.Equal(tc.want, got) { + t.Error(cmp.Diff(tc.want, got)) + } + }) } } @@ -7584,13 +7639,14 @@ func TestGeneratePoliciesFails(t *testing.T) { } for _, test := range tests { - vsc := newVirtualServerConfigurator(&ConfigParams{}, false, false, &StaticConfigParams{}, false) + vsc := newVirtualServerConfigurator(&ConfigParams{}, false, false, &StaticConfigParams{}, false, &fakeBV) if test.oidcPolCfg != nil { vsc.oidcPolCfg = test.oidcPolCfg } result := vsc.generatePolicies(ownerDetails, test.policyRefs, test.policies, test.context, test.policyOpts) + result.BundleValidator = nil if diff := cmp.Diff(test.expected, result); diff != "" { t.Errorf("generatePolicies() '%v' mismatch (-want +got):\n%s", test.msg, diff) } @@ -7728,7 +7784,7 @@ func TestGenerateUpstream(t *testing.T) { }, } - vsc := newVirtualServerConfigurator(&cfgParams, false, false, &StaticConfigParams{}, false) + vsc := newVirtualServerConfigurator(&cfgParams, false, false, &StaticConfigParams{}, false, &fakeBV) result := vsc.generateUpstream(nil, name, upstream, false, endpoints, backupEndpoints) if !reflect.DeepEqual(result, expected) { t.Errorf("generateUpstream() returned %v but expected %v", result, expected) @@ -7807,7 +7863,7 @@ func TestGenerateUpstreamWithKeepalive(t *testing.T) { } for _, test := range tests { - vsc := newVirtualServerConfigurator(test.cfgParams, false, false, &StaticConfigParams{}, false) + vsc := newVirtualServerConfigurator(test.cfgParams, false, false, &StaticConfigParams{}, false, &fakeBV) result := vsc.generateUpstream(nil, name, test.upstream, false, endpoints, nil) if !reflect.DeepEqual(result, test.expected) { t.Errorf("generateUpstream() returned %v but expected %v for the case of %v", result, test.expected, test.msg) @@ -7839,7 +7895,7 @@ func TestGenerateUpstreamForExternalNameService(t *testing.T) { Resolve: true, } - vsc := newVirtualServerConfigurator(&cfgParams, true, true, &StaticConfigParams{}, false) + vsc := newVirtualServerConfigurator(&cfgParams, true, true, &StaticConfigParams{}, false, &fakeBV) result := vsc.generateUpstream(nil, name, upstream, true, endpoints, nil) if !reflect.DeepEqual(result, expected) { t.Errorf("generateUpstream() returned %v but expected %v", result, expected) @@ -7885,7 +7941,7 @@ func TestGenerateUpstreamWithNTLM(t *testing.T) { NTLM: true, } - vsc := newVirtualServerConfigurator(&cfgParams, true, false, &StaticConfigParams{}, false) + vsc := newVirtualServerConfigurator(&cfgParams, true, false, &StaticConfigParams{}, false, &fakeBV) result := vsc.generateUpstream(nil, name, upstream, false, endpoints, nil) if !reflect.DeepEqual(result, expected) { t.Errorf("generateUpstream() returned %v but expected %v", result, expected) @@ -8505,7 +8561,7 @@ func TestGenerateSSLConfig(t *testing.T) { namespace := "default" for _, test := range tests { - vsc := newVirtualServerConfigurator(&ConfigParams{}, false, false, &StaticConfigParams{}, test.wildcard) + vsc := newVirtualServerConfigurator(&ConfigParams{}, false, false, &StaticConfigParams{}, test.wildcard, &fakeBV) // it is ok to use nil as the owner result := vsc.generateSSLConfig(nil, test.inputTLS, namespace, test.inputSecretRefs, test.inputCfgParams) @@ -9078,7 +9134,7 @@ func TestGenerateSplits(t *testing.T) { owner: nil, } - vsc := newVirtualServerConfigurator(&cfgParams, false, false, &StaticConfigParams{}, false) + vsc := newVirtualServerConfigurator(&cfgParams, false, false, &StaticConfigParams{}, false, &fakeBV) for _, test := range tests { t.Run(test.msg, func(t *testing.T) { resultSplitClient, resultLocations, resultReturnLocations := generateSplits( @@ -10759,6 +10815,7 @@ func TestGenerateEndpointsForUpstream(t *testing.T) { test.isResolverConfigured, &StaticConfigParams{}, isWildcardEnabled, + &fakeBV, ) result := vsc.generateEndpointsForUpstream(test.vsEx.VirtualServer, namespace, test.upstream, test.vsEx) if !reflect.DeepEqual(result, test.expected) { @@ -10799,7 +10856,7 @@ func TestGenerateSlowStartForPlusWithInCompatibleLBMethods(t *testing.T) { } for _, lbMethod := range tests { - vsc := newVirtualServerConfigurator(&ConfigParams{}, true, false, &StaticConfigParams{}, false) + vsc := newVirtualServerConfigurator(&ConfigParams{}, true, false, &StaticConfigParams{}, false, &fakeBV) result := vsc.generateSlowStartForPlus(&conf_v1.VirtualServer{}, upstream, lbMethod) if !reflect.DeepEqual(result, expected) { @@ -10833,7 +10890,7 @@ func TestGenerateSlowStartForPlus(t *testing.T) { } for _, test := range tests { - vsc := newVirtualServerConfigurator(&ConfigParams{}, true, false, &StaticConfigParams{}, false) + vsc := newVirtualServerConfigurator(&ConfigParams{}, true, false, &StaticConfigParams{}, false, &fakeBV) result := vsc.generateSlowStartForPlus(&conf_v1.VirtualServer{}, test.upstream, test.lbMethod) if !reflect.DeepEqual(result, test.expected) { t.Errorf("generateSlowStartForPlus returned %v, but expected %v", result, test.expected) @@ -10934,7 +10991,7 @@ func TestGenerateUpstreamWithQueue(t *testing.T) { } for _, test := range tests { - vsc := newVirtualServerConfigurator(&ConfigParams{}, test.isPlus, false, &StaticConfigParams{}, false) + vsc := newVirtualServerConfigurator(&ConfigParams{}, test.isPlus, false, &StaticConfigParams{}, false, &fakeBV) result := vsc.generateUpstream(nil, test.name, test.upstream, false, []string{}, []string{}) if !reflect.DeepEqual(result, test.expected) { t.Errorf("generateUpstream() returned %v but expected %v for the case of %v", result, test.expected, test.msg) @@ -12037,10 +12094,38 @@ func TestAddWafConfig(t *testing.T) { expected: &validationResults{}, msg: "valid waf config, disable waf", }, + { + wafInput: &conf_v1.WAF{ + Enable: true, + ApBundle: "NginxDefaultPolicy.tgz", + SecurityLog: &conf_v1.SecurityLog{ + Enable: true, + ApLogBundle: "secops_dashboard.tgz", + LogDest: "syslog:server=127.0.0.1:1514", + }, + }, + polKey: "default/waf-policy", + polNamespace: "", + apResources: &appProtectResourcesForVS{ + Policies: map[string]string{ + "ns1/dataguard-alarm": "/etc/nginx/waf/nac-policies/ns1-dataguard-alarm", + }, + LogConfs: map[string]string{ + "ns2/logconf": "/etc/nginx/waf/nac-logconfs/ns2-logconf", + }, + }, + wafConfig: &version2.WAF{ + ApPolicy: "/etc/nginx/waf/nac-policies/ns1-dataguard-alarm", + ApSecurityLogEnable: true, + ApLogConf: []string{"/etc/nginx/waf/nac-logconfs/ns2-logconf"}, + }, + expected: &validationResults{}, + msg: "valid waf config using bundle", + }, } for _, test := range tests { - polCfg := newPoliciesConfig() + polCfg := newPoliciesConfig(&fakeBV) result := polCfg.addWAFConfig(test.wafInput, test.polKey, test.polNamespace, test.apResources) if diff := cmp.Diff(test.expected.warnings, result.warnings); diff != "" { t.Errorf("policiesCfg.addWAFConfig() '%v' mismatch (-want +got):\n%s", test.msg, diff) @@ -12757,4 +12842,16 @@ var ( }, }, } + + fakeBV = fakeBundleValidator{} ) + +type fakeBundleValidator struct{} + +func (*fakeBundleValidator) validate(bundle string) (string, error) { + bundle = fmt.Sprintf("/etc/nginx/waf/bundles/%s", bundle) + if strings.Contains(bundle, "invalid") { + return bundle, fmt.Errorf("invalid bundle %s", bundle) + } + return bundle, nil +} diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index 614f23f642..caae1daec1 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -670,7 +670,8 @@ type WAF struct { // SecurityLog defines the security log of a WAF policy. type SecurityLog struct { - Enable bool `json:"enable"` - ApLogConf string `json:"apLogConf"` - LogDest string `json:"logDest"` + Enable bool `json:"enable"` + ApLogConf string `json:"apLogConf"` + ApLogBundle string `json:"apLogBundle"` + LogDest string `json:"logDest"` } diff --git a/pkg/apis/configuration/validation/policy.go b/pkg/apis/configuration/validation/policy.go index 228300d24c..4dba88f948 100644 --- a/pkg/apis/configuration/validation/policy.go +++ b/pkg/apis/configuration/validation/policy.go @@ -302,23 +302,51 @@ func validateWAF(waf *v1.WAF, fieldPath *field.Path) field.ErrorList { } if waf.SecurityLog != nil { - allErrs = append(allErrs, validateLogConf(waf.SecurityLog.ApLogConf, waf.SecurityLog.LogDest, fieldPath.Child("securityLog"))...) + allErrs = append(allErrs, validateLogConf(waf.SecurityLog, fieldPath.Child("securityLog"))...) } + + if waf.SecurityLogs != nil { + allErrs = append(allErrs, validateLogConfs(waf.SecurityLogs, fieldPath.Child("securityLogs"))...) + } + return allErrs +} + +func validateLogConfs(logs []*v1.SecurityLog, fieldPath *field.Path) field.ErrorList { + allErrs := field.ErrorList{} + + for i := range logs { + allErrs = append(allErrs, validateLogConf(logs[i], fieldPath.Index(i))...) + } + return allErrs } -func validateLogConf(logConf, logDest string, fieldPath *field.Path) field.ErrorList { +func validateLogConf(logConf *v1.SecurityLog, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} - if logConf != "" { - for _, msg := range validation.IsQualifiedName(logConf) { - allErrs = append(allErrs, field.Invalid(fieldPath.Child("apLogConf"), logConf, msg)) + if logConf.ApLogConf != "" && logConf.ApLogBundle != "" { + msg := "apLogConf and apLogBundle fields in the securityLog are mutually exclusive" + allErrs = append(allErrs, + field.Invalid(fieldPath.Child("apLogConf"), logConf.ApLogConf, msg), + field.Invalid(fieldPath.Child("apLogBundle"), logConf.ApLogBundle, msg), + ) + } + + if logConf.ApLogConf != "" { + for _, msg := range validation.IsQualifiedName(logConf.ApLogConf) { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("apLogConf"), logConf.ApLogConf, msg)) + } + } + + if logConf.ApLogBundle != "" { + for _, msg := range validation.IsQualifiedName(logConf.ApLogBundle) { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("apBundle"), logConf.ApLogBundle, msg)) } } - err := ValidateAppProtectLogDestination(logDest) + err := ValidateAppProtectLogDestination(logConf.LogDest) if err != nil { - allErrs = append(allErrs, field.Invalid(fieldPath.Child("logDest"), logDest, err.Error())) + allErrs = append(allErrs, field.Invalid(fieldPath.Child("logDest"), logConf.LogDest, err.Error())) } return allErrs } diff --git a/pkg/apis/configuration/validation/policy_test.go b/pkg/apis/configuration/validation/policy_test.go index dcfe61d578..5f17bb1ee8 100644 --- a/pkg/apis/configuration/validation/policy_test.go +++ b/pkg/apis/configuration/validation/policy_test.go @@ -1754,3 +1754,101 @@ func TestValidateBasic_FailsOnMissingSecret(t *testing.T) { t.Error("want error on invalid input") } } + +func TestValidateWAF_FailsOnPresentBothApLogBundleAndApLogConf(t *testing.T) { + t.Parallel() + + waf := &v1.WAF{ + Enable: true, + ApBundle: "bundle.tgz", + SecurityLogs: []*v1.SecurityLog{ + { + ApLogConf: "confName", + ApLogBundle: "confName.tgz", + }, + }, + } + + allErrs := validateWAF(waf, field.NewPath("waf")) + if len(allErrs) == 0 { + t.Errorf("want error, got %v", allErrs) + } +} + +func TestValidateWAF_FailsOnInvalidApLogBundle(t *testing.T) { + t.Parallel() + tests := []struct { + waf *v1.WAF + valid bool + }{ + { + waf: &v1.WAF{ + Enable: true, + ApBundle: "bundle.tgz", + SecurityLogs: []*v1.SecurityLog{ + { + ApLogBundle: ".", + LogDest: "stderr", + }, + }, + }, + }, + { + waf: &v1.WAF{ + Enable: true, + ApBundle: "bundle.tgz", + SecurityLogs: []*v1.SecurityLog{ + { + ApLogBundle: "../bundle.tgz", + LogDest: "stderr", + }, + }, + }, + }, + { + waf: &v1.WAF{ + Enable: true, + ApBundle: "bundle.tgz", + SecurityLogs: []*v1.SecurityLog{ + { + ApLogBundle: "/bundle.tgz", + LogDest: "stderr", + }, + }, + }, + }, + { + waf: &v1.WAF{ + Enable: true, + ApBundle: "bundle.tgz", + SecurityLog: &v1.SecurityLog{ + ApLogBundle: "bundle.tgz", + LogDest: "stderr", + }, + }, + valid: true, + }, + { + waf: &v1.WAF{ + Enable: true, + ApBundle: "bundle.tgz", + SecurityLogs: []*v1.SecurityLog{ + { + ApLogBundle: "bundle.tgz", + LogDest: "stderr", + }, + }, + }, + valid: true, + }, + } + + for _, tc := range tests { + allErrs := validateWAF(tc.waf, field.NewPath("waf")) + if len(allErrs) == 0 && !tc.valid { + t.Errorf("want error, got %v", allErrs) + } else if len(allErrs) > 0 && tc.valid { + t.Errorf("got error %v", allErrs) + } + } +} From 55a560f5e90a8c49d82a8e59a7bf0d1ad14569d0 Mon Sep 17 00:00:00 2001 From: Eoin O'Shaughnessy Date: Fri, 15 Mar 2024 14:59:53 +0000 Subject: [PATCH 2/9] validation for log confs with bundles and vice versa --- pkg/apis/configuration/validation/policy.go | 19 ++- .../configuration/validation/policy_test.go | 113 +++++++++++++++--- 2 files changed, 109 insertions(+), 23 deletions(-) diff --git a/pkg/apis/configuration/validation/policy.go b/pkg/apis/configuration/validation/policy.go index 4dba88f948..bd87024180 100644 --- a/pkg/apis/configuration/validation/policy.go +++ b/pkg/apis/configuration/validation/policy.go @@ -279,6 +279,7 @@ func validateOIDC(oidc *v1.OIDC, fieldPath *field.Path) field.ErrorList { func validateWAF(waf *v1.WAF, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} + bundleMode := waf.ApBundle != "" // WAF Policy references either apPolicy or apBundle. if waf.ApPolicy != "" && waf.ApBundle != "" { @@ -295,33 +296,33 @@ func validateWAF(waf *v1.WAF, fieldPath *field.Path) field.ErrorList { } } - if waf.ApBundle != "" { + if bundleMode { for _, msg := range validation.IsQualifiedName(waf.ApBundle) { allErrs = append(allErrs, field.Invalid(fieldPath.Child("apBundle"), waf.ApBundle, msg)) } } if waf.SecurityLog != nil { - allErrs = append(allErrs, validateLogConf(waf.SecurityLog, fieldPath.Child("securityLog"))...) + allErrs = append(allErrs, validateLogConf(waf.SecurityLog, fieldPath.Child("securityLog"), bundleMode)...) } if waf.SecurityLogs != nil { - allErrs = append(allErrs, validateLogConfs(waf.SecurityLogs, fieldPath.Child("securityLogs"))...) + allErrs = append(allErrs, validateLogConfs(waf.SecurityLogs, fieldPath.Child("securityLogs"), bundleMode)...) } return allErrs } -func validateLogConfs(logs []*v1.SecurityLog, fieldPath *field.Path) field.ErrorList { +func validateLogConfs(logs []*v1.SecurityLog, fieldPath *field.Path, bundleMode bool) field.ErrorList { allErrs := field.ErrorList{} for i := range logs { - allErrs = append(allErrs, validateLogConf(logs[i], fieldPath.Index(i))...) + allErrs = append(allErrs, validateLogConf(logs[i], fieldPath.Index(i), bundleMode)...) } return allErrs } -func validateLogConf(logConf *v1.SecurityLog, fieldPath *field.Path) field.ErrorList { +func validateLogConf(logConf *v1.SecurityLog, fieldPath *field.Path, bundleMode bool) field.ErrorList { allErrs := field.ErrorList{} if logConf.ApLogConf != "" && logConf.ApLogBundle != "" { @@ -333,12 +334,18 @@ func validateLogConf(logConf *v1.SecurityLog, fieldPath *field.Path) field.Error } if logConf.ApLogConf != "" { + if bundleMode { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("apLogConf"), logConf.ApLogConf, "apLogConf is not supported with apBundle")) + } for _, msg := range validation.IsQualifiedName(logConf.ApLogConf) { allErrs = append(allErrs, field.Invalid(fieldPath.Child("apLogConf"), logConf.ApLogConf, msg)) } } if logConf.ApLogBundle != "" { + if !bundleMode { + allErrs = append(allErrs, field.Invalid(fieldPath.Child("apLogConf"), logConf.ApLogConf, "apLogBundle is not supported with apPolicy")) + } for _, msg := range validation.IsQualifiedName(logConf.ApLogBundle) { allErrs = append(allErrs, field.Invalid(fieldPath.Child("apBundle"), logConf.ApLogBundle, msg)) } diff --git a/pkg/apis/configuration/validation/policy_test.go b/pkg/apis/configuration/validation/policy_test.go index 5f17bb1ee8..36a4736d71 100644 --- a/pkg/apis/configuration/validation/policy_test.go +++ b/pkg/apis/configuration/validation/policy_test.go @@ -1758,30 +1758,103 @@ func TestValidateBasic_FailsOnMissingSecret(t *testing.T) { func TestValidateWAF_FailsOnPresentBothApLogBundleAndApLogConf(t *testing.T) { t.Parallel() - waf := &v1.WAF{ - Enable: true, - ApBundle: "bundle.tgz", - SecurityLogs: []*v1.SecurityLog{ - { - ApLogConf: "confName", - ApLogBundle: "confName.tgz", + tests := []struct { + name string + waf *v1.WAF + valid bool + }{ + { + name: "mutually exclusive fields", + waf: &v1.WAF{ + Enable: true, + ApBundle: "bundle.tgz", + SecurityLogs: []*v1.SecurityLog{ + { + ApLogConf: "confName", + ApLogBundle: "confName.tgz", + }, + }, + }, + valid: false, + }, + { + name: "apBundle with apLogConf", + waf: &v1.WAF{ + Enable: true, + ApBundle: "bundle.tgz", + SecurityLogs: []*v1.SecurityLog{ + { + ApLogConf: "confName", + LogDest: "stderr", + }, + }, + }, + valid: false, + }, + { + name: "apPolicy with apLogBundle", + waf: &v1.WAF{ + Enable: true, + ApPolicy: "apPolicy", + SecurityLogs: []*v1.SecurityLog{ + { + ApLogBundle: "confName.tgz", + LogDest: "stderr", + }, + }, }, + valid: false, + }, + { + name: "apBundle with apLogBundle", + waf: &v1.WAF{ + Enable: true, + ApBundle: "bundle.tgz", + SecurityLogs: []*v1.SecurityLog{ + { + ApLogBundle: "confName.tgz", + LogDest: "stderr", + }, + }, + }, + valid: true, + }, + { + name: "apPolicy with apLogConf", + waf: &v1.WAF{ + Enable: true, + ApPolicy: "apPolicy", + SecurityLogs: []*v1.SecurityLog{ + { + ApLogConf: "confName", + LogDest: "stderr", + }, + }, + }, + valid: true, }, } - - allErrs := validateWAF(waf, field.NewPath("waf")) - if len(allErrs) == 0 { - t.Errorf("want error, got %v", allErrs) + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + allErrs := validateWAF(tc.waf, field.NewPath("waf")) + if len(allErrs) == 0 && !tc.valid { + t.Errorf("want error, got %v", allErrs) + } else if len(allErrs) > 0 && tc.valid { + t.Errorf("got error %v", allErrs) + } + }) } } func TestValidateWAF_FailsOnInvalidApLogBundle(t *testing.T) { t.Parallel() tests := []struct { + name string waf *v1.WAF valid bool }{ { + name: "invalid file name 1", waf: &v1.WAF{ Enable: true, ApBundle: "bundle.tgz", @@ -1794,6 +1867,7 @@ func TestValidateWAF_FailsOnInvalidApLogBundle(t *testing.T) { }, }, { + name: "invalid file name 2", waf: &v1.WAF{ Enable: true, ApBundle: "bundle.tgz", @@ -1806,6 +1880,7 @@ func TestValidateWAF_FailsOnInvalidApLogBundle(t *testing.T) { }, }, { + name: "invalid file name 3", waf: &v1.WAF{ Enable: true, ApBundle: "bundle.tgz", @@ -1818,6 +1893,7 @@ func TestValidateWAF_FailsOnInvalidApLogBundle(t *testing.T) { }, }, { + name: "valid securityLog", waf: &v1.WAF{ Enable: true, ApBundle: "bundle.tgz", @@ -1829,6 +1905,7 @@ func TestValidateWAF_FailsOnInvalidApLogBundle(t *testing.T) { valid: true, }, { + name: "valid securityLogs", waf: &v1.WAF{ Enable: true, ApBundle: "bundle.tgz", @@ -1844,11 +1921,13 @@ func TestValidateWAF_FailsOnInvalidApLogBundle(t *testing.T) { } for _, tc := range tests { - allErrs := validateWAF(tc.waf, field.NewPath("waf")) - if len(allErrs) == 0 && !tc.valid { - t.Errorf("want error, got %v", allErrs) - } else if len(allErrs) > 0 && tc.valid { - t.Errorf("got error %v", allErrs) - } + t.Run(tc.name, func(t *testing.T) { + allErrs := validateWAF(tc.waf, field.NewPath("waf")) + if len(allErrs) == 0 && !tc.valid { + t.Errorf("want error, got %v", allErrs) + } else if len(allErrs) > 0 && tc.valid { + t.Errorf("got error %v", allErrs) + } + }) } } From bc8fd6a552e0a03152292aab72621d6f9d985123 Mon Sep 17 00:00:00 2001 From: Eoin O'Shaughnessy Date: Fri, 15 Mar 2024 15:14:06 +0000 Subject: [PATCH 3/9] apLogBundle references --- docs/content/configuration/policy-resource.md | 6 ++++-- .../integrations/app-protect-waf/configuration.md | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/content/configuration/policy-resource.md b/docs/content/configuration/policy-resource.md index b8aca9cd11..a9e0a11266 100644 --- a/docs/content/configuration/policy-resource.md +++ b/docs/content/configuration/policy-resource.md @@ -547,9 +547,11 @@ waf: |Field | Description | Type | Required | | ---| ---| ---| --- | |``enable`` | Enables NGINX App Protect WAF. | ``bool`` | Yes | -|``apPolicy`` | The [App Protect WAF policy](/nginx-ingress-controller/app-protect/configuration/#app-protect-policies) of the WAF. Accepts an optional namespace. | ``string`` | No | +|``apPolicy`` | The [App Protect WAF policy](/nginx-ingress-controller/app-protect/configuration/#app-protect-policies) of the WAF. Accepts an optional namespace. Mutually exclusive with ``apBundle``. | ``string`` | No | +|``apBundle`` | The [App Protect WAF policy bundle](/nginx-ingress-controller/app-protect/configuration/#app-protect-waf-bundles). Mutually exclusive with ``apPolicy``. | ``string`` | No | |``securityLog.enable`` | Enables security log. | ``bool`` | No | -|``securityLog.apLogConf`` | The [App Protect WAF log conf](/nginx-ingress-controller/app-protect/configuration/#app-protect-logs) resource. Accepts an optional namespace. | ``string`` | No | +|``securityLog.apLogConf`` | The [App Protect WAF log conf](/nginx-ingress-controller/app-protect/configuration/#app-protect-logs) resource. Accepts an optional namespace. Only works with ``apPolicy``. | ``string`` | No | +|``securityLog.apLogBundle`` | The [App Protect WAF log bundle](/nginx-ingress-controller/app-protect/configuration/#app-protect-waf-bundles) resource. Only works with ``apBundle``. | ``string`` | No | |``securityLog.logDest`` | The log destination for the security log. Accepted variables are ``syslog:server=:``, ``stderr``, ````. Default is ``"syslog:server=127.0.0.1:514"``. | ``string`` | No | {{% /table %}} diff --git a/docs/content/installation/integrations/app-protect-waf/configuration.md b/docs/content/installation/integrations/app-protect-waf/configuration.md index eb5e45cb44..d555d27d7f 100644 --- a/docs/content/installation/integrations/app-protect-waf/configuration.md +++ b/docs/content/installation/integrations/app-protect-waf/configuration.md @@ -212,7 +212,7 @@ You can define App Protect WAF bundles for VirtualServer custom resources by cre Before applying a policy, a WAF policy bundle must be created, then copied to a volume mounted to `/etc/nginx/waf/bundles`. -{{< note >}} NGINX Ingress Controller does not currently support `securityLogs` for policy bundles. {{< /note >}} +{{< note >}} NGINX Ingress Controller currently supports `securityLogs` for policy bundles when using `apLogBundle` instead of `apLogConf`. {{< /note >}} This example show how a policy is configured by referencing a generated WAF Policy Bundle: From 93a83a61b53f5b9c02aef345e63036475b068a64 Mon Sep 17 00:00:00 2001 From: Eoin O'Shaughnessy Date: Fri, 15 Mar 2024 15:16:31 +0000 Subject: [PATCH 4/9] fix bundle link --- docs/content/configuration/policy-resource.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/content/configuration/policy-resource.md b/docs/content/configuration/policy-resource.md index a9e0a11266..1d7f132089 100644 --- a/docs/content/configuration/policy-resource.md +++ b/docs/content/configuration/policy-resource.md @@ -548,10 +548,10 @@ waf: | ---| ---| ---| --- | |``enable`` | Enables NGINX App Protect WAF. | ``bool`` | Yes | |``apPolicy`` | The [App Protect WAF policy](/nginx-ingress-controller/app-protect/configuration/#app-protect-policies) of the WAF. Accepts an optional namespace. Mutually exclusive with ``apBundle``. | ``string`` | No | -|``apBundle`` | The [App Protect WAF policy bundle](/nginx-ingress-controller/app-protect/configuration/#app-protect-waf-bundles). Mutually exclusive with ``apPolicy``. | ``string`` | No | +|``apBundle`` | The [App Protect WAF policy bundle](/nginx-ingress-controller/app-protect/configuration/#app-protect-bundles). Mutually exclusive with ``apPolicy``. | ``string`` | No | |``securityLog.enable`` | Enables security log. | ``bool`` | No | |``securityLog.apLogConf`` | The [App Protect WAF log conf](/nginx-ingress-controller/app-protect/configuration/#app-protect-logs) resource. Accepts an optional namespace. Only works with ``apPolicy``. | ``string`` | No | -|``securityLog.apLogBundle`` | The [App Protect WAF log bundle](/nginx-ingress-controller/app-protect/configuration/#app-protect-waf-bundles) resource. Only works with ``apBundle``. | ``string`` | No | +|``securityLog.apLogBundle`` | The [App Protect WAF log bundle](/nginx-ingress-controller/app-protect/configuration/#app-protect-bundles) resource. Only works with ``apBundle``. | ``string`` | No | |``securityLog.logDest`` | The log destination for the security log. Accepted variables are ``syslog:server=:``, ``stderr``, ````. Default is ``"syslog:server=127.0.0.1:514"``. | ``string`` | No | {{% /table %}} From 77e415227db9d512cc7f60f58a5dda47b42e31e6 Mon Sep 17 00:00:00 2001 From: Eoin O'Shaughnessy Date: Fri, 15 Mar 2024 15:32:16 +0000 Subject: [PATCH 5/9] fix links in policy table --- docs/content/configuration/policy-resource.md | 8 ++++---- .../app-protect-waf/configuration.md | 18 ++++++++++++++++-- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/docs/content/configuration/policy-resource.md b/docs/content/configuration/policy-resource.md index 1d7f132089..8cfcf321ed 100644 --- a/docs/content/configuration/policy-resource.md +++ b/docs/content/configuration/policy-resource.md @@ -547,11 +547,11 @@ waf: |Field | Description | Type | Required | | ---| ---| ---| --- | |``enable`` | Enables NGINX App Protect WAF. | ``bool`` | Yes | -|``apPolicy`` | The [App Protect WAF policy](/nginx-ingress-controller/app-protect/configuration/#app-protect-policies) of the WAF. Accepts an optional namespace. Mutually exclusive with ``apBundle``. | ``string`` | No | -|``apBundle`` | The [App Protect WAF policy bundle](/nginx-ingress-controller/app-protect/configuration/#app-protect-bundles). Mutually exclusive with ``apPolicy``. | ``string`` | No | +|``apPolicy`` | The [App Protect WAF policy](/nginx-ingress-controller/installation/integrations/app-protect-waf/configuration/#nginx-app-protect-waf-policies) of the WAF. Accepts an optional namespace. Mutually exclusive with ``apBundle``. | ``string`` | No | +|``apBundle`` | The [App Protect WAF policy bundle](/nginx-ingress-controller/installation/integrations/app-protect-waf/configuration/#nginx-app-protect-waf-bundles). Mutually exclusive with ``apPolicy``. | ``string`` | No | |``securityLog.enable`` | Enables security log. | ``bool`` | No | -|``securityLog.apLogConf`` | The [App Protect WAF log conf](/nginx-ingress-controller/app-protect/configuration/#app-protect-logs) resource. Accepts an optional namespace. Only works with ``apPolicy``. | ``string`` | No | -|``securityLog.apLogBundle`` | The [App Protect WAF log bundle](/nginx-ingress-controller/app-protect/configuration/#app-protect-bundles) resource. Only works with ``apBundle``. | ``string`` | No | +|``securityLog.apLogConf`` | The [App Protect WAF log conf](/nginx-ingress-controller/installation/integrations/app-protect-waf/configuration/#nginx-app-protect-waf-logs) resource. Accepts an optional namespace. Only works with ``apPolicy``. | ``string`` | No | +|``securityLog.apLogBundle`` | The [App Protect WAF log bundle](/nginx-ingress-controller/installation/integrations/app-protect-waf/configuration/#nginx-app-protect-waf-bundles) resource. Only works with ``apBundle``. | ``string`` | No | |``securityLog.logDest`` | The log destination for the security log. Accepted variables are ``syslog:server=:``, ``stderr``, ````. Default is ``"syslog:server=127.0.0.1:514"``. | ``string`` | No | {{% /table %}} diff --git a/docs/content/installation/integrations/app-protect-waf/configuration.md b/docs/content/installation/integrations/app-protect-waf/configuration.md index d555d27d7f..1c12d9c0dd 100644 --- a/docs/content/installation/integrations/app-protect-waf/configuration.md +++ b/docs/content/installation/integrations/app-protect-waf/configuration.md @@ -206,7 +206,7 @@ spec: tag: Fruits ``` -## App Protect WAF Bundles +## NGINX App Protect WAF Bundles You can define App Protect WAF bundles for VirtualServer custom resources by creating policy bundles and putting them on a mounted volume accessible from NGINX Ingress Controller. @@ -214,7 +214,21 @@ Before applying a policy, a WAF policy bundle must be created, then copied to a {{< note >}} NGINX Ingress Controller currently supports `securityLogs` for policy bundles when using `apLogBundle` instead of `apLogConf`. {{< /note >}} -This example show how a policy is configured by referencing a generated WAF Policy Bundle: +This example shows how a policy is configured by referencing a generated WAF Policy Bundle: + + +```yaml +apiVersion: k8s.nginx.org/v1 +kind: Policy +metadata: + name: +spec: + waf: + enable: true + apBundle: ".tgz" +``` + +This example shows the same policy as above but with a log bundle used for : ```yaml From 23ac3fdad0d9eb5b3fa96b9c1172f2d728dda7d5 Mon Sep 17 00:00:00 2001 From: oseoin Date: Fri, 15 Mar 2024 15:58:13 +0000 Subject: [PATCH 6/9] Update docs/content/installation/integrations/app-protect-waf/configuration.md Co-authored-by: Alan Dooley Signed-off-by: oseoin --- .../installation/integrations/app-protect-waf/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/installation/integrations/app-protect-waf/configuration.md b/docs/content/installation/integrations/app-protect-waf/configuration.md index 1c12d9c0dd..b45830e95b 100644 --- a/docs/content/installation/integrations/app-protect-waf/configuration.md +++ b/docs/content/installation/integrations/app-protect-waf/configuration.md @@ -212,7 +212,7 @@ You can define App Protect WAF bundles for VirtualServer custom resources by cre Before applying a policy, a WAF policy bundle must be created, then copied to a volume mounted to `/etc/nginx/waf/bundles`. -{{< note >}} NGINX Ingress Controller currently supports `securityLogs` for policy bundles when using `apLogBundle` instead of `apLogConf`. {{< /note >}} +{{< note >}} NGINX Ingress Controller supports `securityLogs` for policy bundles when using `apLogBundle` instead of `apLogConf`. {{< /note >}} This example shows how a policy is configured by referencing a generated WAF Policy Bundle: From fdf0d9761d2152d0a70580392d7acc1aef245fee Mon Sep 17 00:00:00 2001 From: Eoin O'Shaughnessy Date: Fri, 15 Mar 2024 16:25:58 +0000 Subject: [PATCH 7/9] change to relref links --- docs/content/configuration/policy-resource.md | 8 ++++---- .../integrations/app-protect-waf/configuration.md | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/content/configuration/policy-resource.md b/docs/content/configuration/policy-resource.md index 8cfcf321ed..346c81e591 100644 --- a/docs/content/configuration/policy-resource.md +++ b/docs/content/configuration/policy-resource.md @@ -547,11 +547,11 @@ waf: |Field | Description | Type | Required | | ---| ---| ---| --- | |``enable`` | Enables NGINX App Protect WAF. | ``bool`` | Yes | -|``apPolicy`` | The [App Protect WAF policy](/nginx-ingress-controller/installation/integrations/app-protect-waf/configuration/#nginx-app-protect-waf-policies) of the WAF. Accepts an optional namespace. Mutually exclusive with ``apBundle``. | ``string`` | No | -|``apBundle`` | The [App Protect WAF policy bundle](/nginx-ingress-controller/installation/integrations/app-protect-waf/configuration/#nginx-app-protect-waf-bundles). Mutually exclusive with ``apPolicy``. | ``string`` | No | +|``apPolicy`` | The [App Protect WAF policy]({{< relref "installation/integrations/app-protect-waf/configuration.md#waf-policies" >}}) of the WAF. Accepts an optional namespace. Mutually exclusive with ``apBundle``. | ``string`` | No | +|``apBundle`` | The [App Protect WAF policy bundle]({{< relref "installation/integrations/app-protect-waf/configuration.md#waf-bundles" >}}). Mutually exclusive with ``apPolicy``. | ``string`` | No | |``securityLog.enable`` | Enables security log. | ``bool`` | No | -|``securityLog.apLogConf`` | The [App Protect WAF log conf](/nginx-ingress-controller/installation/integrations/app-protect-waf/configuration/#nginx-app-protect-waf-logs) resource. Accepts an optional namespace. Only works with ``apPolicy``. | ``string`` | No | -|``securityLog.apLogBundle`` | The [App Protect WAF log bundle](/nginx-ingress-controller/installation/integrations/app-protect-waf/configuration/#nginx-app-protect-waf-bundles) resource. Only works with ``apBundle``. | ``string`` | No | +|``securityLog.apLogConf`` | The [App Protect WAF log conf]({{< relref "installation/integrations/app-protect-waf/configuration.md#waf-logs" >}}) resource. Accepts an optional namespace. Only works with ``apPolicy``. | ``string`` | No | +|``securityLog.apLogBundle`` | The [App Protect WAF log bundle]({{< relref "installation/integrations/app-protect-waf/configuration.md#waf-bundles" >}}) resource. Only works with ``apBundle``. | ``string`` | No | |``securityLog.logDest`` | The log destination for the security log. Accepted variables are ``syslog:server=:``, ``stderr``, ````. Default is ``"syslog:server=127.0.0.1:514"``. | ``string`` | No | {{% /table %}} diff --git a/docs/content/installation/integrations/app-protect-waf/configuration.md b/docs/content/installation/integrations/app-protect-waf/configuration.md index b45830e95b..0aab651838 100644 --- a/docs/content/installation/integrations/app-protect-waf/configuration.md +++ b/docs/content/installation/integrations/app-protect-waf/configuration.md @@ -22,7 +22,7 @@ NGINX App Protect WAF can be enabled and configured for custom resources (Virtua - For Ingress resources, apply the [`app-protect` annotations]({{< relref "configuration/ingress-resources/advanced-configuration-with-annotations.md#app-protect" >}}) to each desired resource. -## NGINX App Protect WAF Policies +## NGINX App Protect WAF Policies {#waf-policies} NGINX App Protect WAF Policies can be created for VirtualServer, VirtualServerRoute, or Ingress resources by creating an `APPolicy` [custom resource](https://kubernetes.io/docs/concepts/extend-kubernetes/api-extension/custom-resources/). There are some caveats: @@ -99,7 +99,7 @@ spec: Notice that the fields match in name and nesting: NGINX Ingress Controller will transform the YAML into a valid JSON WAF policy config. -## NGINX App Protect WAF Logs +## NGINX App Protect WAF Logs {#waf-logs} Configuring @@ -206,7 +206,7 @@ spec: tag: Fruits ``` -## NGINX App Protect WAF Bundles +## NGINX App Protect WAF Bundles {#waf-bundles} You can define App Protect WAF bundles for VirtualServer custom resources by creating policy bundles and putting them on a mounted volume accessible from NGINX Ingress Controller. From 6c6a755d4e2441c168f8be7b3731a30843c6fcb7 Mon Sep 17 00:00:00 2001 From: Eoin O'Shaughnessy Date: Fri, 15 Mar 2024 16:39:02 +0000 Subject: [PATCH 8/9] missing apLogBundle example --- .../integrations/app-protect-waf/configuration.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/content/installation/integrations/app-protect-waf/configuration.md b/docs/content/installation/integrations/app-protect-waf/configuration.md index 0aab651838..307dcef220 100644 --- a/docs/content/installation/integrations/app-protect-waf/configuration.md +++ b/docs/content/installation/integrations/app-protect-waf/configuration.md @@ -240,6 +240,10 @@ spec: waf: enable: true apBundle: ".tgz" + securityLogs: + - enable: true + apLogBundle: ".tgz" + logDest: "syslog:server=syslog-svc.default:514" ``` ## OpenAPI Specification in NGINX Ingress Controller From 2fea782cfe0ca7071ac2887aeabd667a14343b34 Mon Sep 17 00:00:00 2001 From: Eoin O'Shaughnessy Date: Fri, 15 Mar 2024 16:47:41 +0000 Subject: [PATCH 9/9] apBundle mount note --- .../installation/integrations/app-protect-waf/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/installation/integrations/app-protect-waf/configuration.md b/docs/content/installation/integrations/app-protect-waf/configuration.md index 307dcef220..4e3b2127e4 100644 --- a/docs/content/installation/integrations/app-protect-waf/configuration.md +++ b/docs/content/installation/integrations/app-protect-waf/configuration.md @@ -212,7 +212,7 @@ You can define App Protect WAF bundles for VirtualServer custom resources by cre Before applying a policy, a WAF policy bundle must be created, then copied to a volume mounted to `/etc/nginx/waf/bundles`. -{{< note >}} NGINX Ingress Controller supports `securityLogs` for policy bundles when using `apLogBundle` instead of `apLogConf`. {{< /note >}} +{{< note >}} NGINX Ingress Controller supports `securityLogs` for policy bundles when using `apLogBundle` instead of `apLogConf`. Log bundles must also be copied to a volume mounted to `/etc/nginx/waf/bundles`. {{< /note >}} This example shows how a policy is configured by referencing a generated WAF Policy Bundle: