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/docs/content/configuration/policy-resource.md b/docs/content/configuration/policy-resource.md index b8aca9cd11..346c81e591 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]({{< 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/app-protect/configuration/#app-protect-logs) resource. Accepts an optional namespace. | ``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 eb5e45cb44..4e3b2127e4 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,15 +206,15 @@ spec: tag: Fruits ``` -## 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. 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 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 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 @@ -228,6 +228,24 @@ spec: apBundle: ".tgz" ``` +This example shows the same policy as above but with a log bundle used for : + + +```yaml +apiVersion: k8s.nginx.org/v1 +kind: Policy +metadata: + name: +spec: + waf: + enable: true + apBundle: ".tgz" + securityLogs: + - enable: true + apLogBundle: ".tgz" + logDest: "syslog:server=syslog-svc.default:514" +``` + ## OpenAPI Specification in NGINX Ingress Controller The OpenAPI Specification defines the spec file format needed to describe RESTful APIs. The spec file can be written either in JSON or YAML. Using a spec file simplifies the work of implementing API protection. Refer to the [OpenAPI Specification](https://github.com/OAI/OpenAPI-Specification) (formerly called Swagger) for details. 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..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,30 +296,64 @@ 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.ApLogConf, waf.SecurityLog.LogDest, 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"), bundleMode)...) + } + return allErrs +} + +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), bundleMode)...) + } + return allErrs } -func validateLogConf(logConf, logDest string, fieldPath *field.Path) field.ErrorList { +func validateLogConf(logConf *v1.SecurityLog, fieldPath *field.Path, bundleMode bool) 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 != "" { + 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)) } } - 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..36a4736d71 100644 --- a/pkg/apis/configuration/validation/policy_test.go +++ b/pkg/apis/configuration/validation/policy_test.go @@ -1754,3 +1754,180 @@ func TestValidateBasic_FailsOnMissingSecret(t *testing.T) { t.Error("want error on invalid input") } } + +func TestValidateWAF_FailsOnPresentBothApLogBundleAndApLogConf(t *testing.T) { + t.Parallel() + + 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, + }, + } + 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", + SecurityLogs: []*v1.SecurityLog{ + { + ApLogBundle: ".", + LogDest: "stderr", + }, + }, + }, + }, + { + name: "invalid file name 2", + waf: &v1.WAF{ + Enable: true, + ApBundle: "bundle.tgz", + SecurityLogs: []*v1.SecurityLog{ + { + ApLogBundle: "../bundle.tgz", + LogDest: "stderr", + }, + }, + }, + }, + { + name: "invalid file name 3", + waf: &v1.WAF{ + Enable: true, + ApBundle: "bundle.tgz", + SecurityLogs: []*v1.SecurityLog{ + { + ApLogBundle: "/bundle.tgz", + LogDest: "stderr", + }, + }, + }, + }, + { + name: "valid securityLog", + waf: &v1.WAF{ + Enable: true, + ApBundle: "bundle.tgz", + SecurityLog: &v1.SecurityLog{ + ApLogBundle: "bundle.tgz", + LogDest: "stderr", + }, + }, + valid: true, + }, + { + name: "valid securityLogs", + waf: &v1.WAF{ + Enable: true, + ApBundle: "bundle.tgz", + SecurityLogs: []*v1.SecurityLog{ + { + ApLogBundle: "bundle.tgz", + LogDest: "stderr", + }, + }, + }, + valid: true, + }, + } + + 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) + } + }) + } +}