diff --git a/api/v1alpha/trafficprotectionpolicy_types.go b/api/v1alpha/trafficprotectionpolicy_types.go index fc0350a1..d1ea3faf 100644 --- a/api/v1alpha/trafficprotectionpolicy_types.go +++ b/api/v1alpha/trafficprotectionpolicy_types.go @@ -129,6 +129,19 @@ type ParanoiaLevels struct { Detection int `json:"detection,omitempty"` } +func (s *TrafficProtectionPolicySpec) InvertedParanoiaLevels() *ParanoiaLevels { + for i := range s.RuleSets { + if s.RuleSets[i].Type != TrafficProtectionPolicyOWASPCoreRuleSet { + continue + } + levels := &s.RuleSets[i].OWASPCoreRuleSet.ParanoiaLevels + if levels.Detection < levels.Blocking { + return levels + } + } + return nil +} + type OWASPRuleExclusions struct { // Tags is a list of rule tags to disable. // diff --git a/api/v1alpha/trafficprotectionpolicy_types_test.go b/api/v1alpha/trafficprotectionpolicy_types_test.go new file mode 100644 index 00000000..d89cb6bd --- /dev/null +++ b/api/v1alpha/trafficprotectionpolicy_types_test.go @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: AGPL-3.0-only + +package v1alpha_test + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + networkingv1alpha "go.datum.net/network-services-operator/api/v1alpha" +) + +func TestTrafficProtectionPolicySpec_InvertedParanoiaLevels(t *testing.T) { + t.Parallel() + + owaspRuleSet := func(blocking, detection int) networkingv1alpha.TrafficProtectionPolicyRuleSet { + return networkingv1alpha.TrafficProtectionPolicyRuleSet{ + Type: networkingv1alpha.TrafficProtectionPolicyOWASPCoreRuleSet, + OWASPCoreRuleSet: networkingv1alpha.OWASPCRS{ + ParanoiaLevels: networkingv1alpha.ParanoiaLevels{ + Blocking: blocking, + Detection: detection, + }, + }, + } + } + + tests := []struct { + name string + ruleSets []networkingv1alpha.TrafficProtectionPolicyRuleSet + wantInverted bool + }{ + { + name: "no rulesets", + ruleSets: nil, + wantInverted: false, + }, + { + name: "equal levels", + ruleSets: []networkingv1alpha.TrafficProtectionPolicyRuleSet{owaspRuleSet(2, 2)}, + wantInverted: false, + }, + { + name: "higher detection", + ruleSets: []networkingv1alpha.TrafficProtectionPolicyRuleSet{owaspRuleSet(1, 3)}, + wantInverted: false, + }, + { + name: "detection below blocking", + ruleSets: []networkingv1alpha.TrafficProtectionPolicyRuleSet{owaspRuleSet(2, 1)}, + wantInverted: true, + }, + { + name: "detection far below blocking", + ruleSets: []networkingv1alpha.TrafficProtectionPolicyRuleSet{owaspRuleSet(4, 1)}, + wantInverted: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + spec := networkingv1alpha.TrafficProtectionPolicySpec{RuleSets: tt.ruleSets} + levels := spec.InvertedParanoiaLevels() + + if tt.wantInverted { + if assert.NotNil(t, levels, "inverted ruleset must be reported") { + assert.Less(t, levels.Detection, levels.Blocking, + "reported levels must be inverted") + } + return + } + assert.Nil(t, levels, "no ruleset is inverted") + }) + } +} diff --git a/internal/controller/trafficprotectionpolicy_controller.go b/internal/controller/trafficprotectionpolicy_controller.go index f5ee779d..b179bc2c 100644 --- a/internal/controller/trafficprotectionpolicy_controller.go +++ b/internal/controller/trafficprotectionpolicy_controller.go @@ -1235,21 +1235,16 @@ func getVHostConstraintForGateway(namespace string, gateway *gatewayv1.Gateway) // closed on this configuration and denies every request with HTTP 500 before any // attack evaluation, so the policy must not be attached. func paranoiaLevelsResolveError(policy *policyContext) *gatewaystatus.PolicyResolveError { - for _, ruleSet := range policy.Spec.RuleSets { - if ruleSet.Type != networkingv1alpha.TrafficProtectionPolicyOWASPCoreRuleSet { - continue - } - levels := ruleSet.OWASPCoreRuleSet.ParanoiaLevels - if levels.Detection < levels.Blocking { - return &gatewaystatus.PolicyResolveError{ - Reason: gatewayv1.PolicyReasonInvalid, - Message: fmt.Sprintf( - "OWASPCoreRuleSet detection paranoia level (%d) must be greater than or equal to blocking paranoia level (%d); this is an illegal CRS configuration that would deny all traffic with HTTP 500", - levels.Detection, levels.Blocking), - } - } + levels := policy.Spec.InvertedParanoiaLevels() + if levels == nil { + return nil + } + return &gatewaystatus.PolicyResolveError{ + Reason: gatewayv1.PolicyReasonInvalid, + Message: fmt.Sprintf( + "OWASPCoreRuleSet detection paranoia level (%d) must be greater than or equal to blocking paranoia level (%d); this is an illegal CRS configuration that would deny all traffic with HTTP 500", + levels.Detection, levels.Blocking), } - return nil } func (r *TrafficProtectionPolicyReconciler) getCorazaDirectivesForTrafficProtectionPolicy( diff --git a/internal/extensionserver/cache/index.go b/internal/extensionserver/cache/index.go index 1ab53da1..171a097d 100644 --- a/internal/extensionserver/cache/index.go +++ b/internal/extensionserver/cache/index.go @@ -225,6 +225,10 @@ func computeCorazaDirectives( tpp *networkingv1alpha.TrafficProtectionPolicy, baseDirectives []string, ) []string { + if tpp.Spec.InvertedParanoiaLevels() != nil { + return nil + } + var owaspCRS *networkingv1alpha.OWASPCRS for _, ruleSet := range tpp.Spec.RuleSets { if ruleSet.Type == networkingv1alpha.TrafficProtectionPolicyOWASPCoreRuleSet { diff --git a/internal/extensionserver/cache/index_test.go b/internal/extensionserver/cache/index_test.go index ed59a78d..5ce69ea9 100644 --- a/internal/extensionserver/cache/index_test.go +++ b/internal/extensionserver/cache/index_test.go @@ -485,6 +485,33 @@ func TestBuildPolicyIndexFromClient_TPPFieldsPreserved(t *testing.T) { assert.NotEmpty(t, info.Directives, "OWASP CRS rules must generate non-empty directives") } +func TestBuildPolicyIndexFromClient_InvertedParanoia_NoDirectivesEmitted(t *testing.T) { + scheme := indexTestScheme(t) + + inverted := newTPP("test-ns", "inverted-tpp", withOWASPCRS(5, 4, 2, 1)) + inverted.Spec.Mode = networkingv1alpha.TrafficProtectionPolicyEnforce + + valid := newTPP("test-ns", "valid-tpp", withOWASPCRS(5, 4, 1, 1)) + valid.Spec.Mode = networkingv1alpha.TrafficProtectionPolicyEnforce + + cl := fake.NewClientBuilder().WithScheme(scheme).WithObjects(inverted, valid).Build() + + idx, err := BuildPolicyIndexFromClient(context.Background(), cl, nil) + require.NoError(t, err) + + byName := map[string]TPPInfo{} + for _, info := range idx.TPPs["test-ns"] { + byName[info.Name] = info + } + require.Contains(t, byName, "inverted-tpp") + require.Contains(t, byName, "valid-tpp") + + assert.Empty(t, byName["inverted-tpp"].Directives, + "inverted policy must yield no directives so the mutation layer withholds it from the data plane") + assert.NotEmpty(t, byName["valid-tpp"].Directives, + "valid policy must still emit directives") +} + // ============================================================================= // computeCorazaDirectives unit tests // @@ -594,6 +621,36 @@ func TestComputeCorazaDirectives_ParanoiaLevels(t *testing.T) { assert.True(t, foundDetection, "detection paranoia level 4 must appear in directives") } +func TestComputeCorazaDirectives_InvertedParanoiaLevels(t *testing.T) { + tests := []struct { + name string + blocking int + detection int + wantEmitted bool + }{ + {name: "equal levels emitted", blocking: 2, detection: 2, wantEmitted: true}, + {name: "higher detection emitted", blocking: 1, detection: 3, wantEmitted: true}, + {name: "inverted not emitted", blocking: 2, detection: 1, wantEmitted: false}, + {name: "inverted far not emitted", blocking: 4, detection: 1, wantEmitted: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tpp := newTPP("ns", "tpp", withOWASPCRS(5, 4, tt.blocking, tt.detection)) + tpp.Spec.Mode = networkingv1alpha.TrafficProtectionPolicyEnforce + + result := computeCorazaDirectives(tpp, nil) + + if tt.wantEmitted { + assert.NotEmpty(t, result, "valid paranoia levels must emit directives") + return + } + assert.Nil(t, result, + "inverted paranoia levels must emit no directives (withheld from the data plane)") + }) + } +} + func TestComputeCorazaDirectives_IncludeOWASPCRSAppendedAfterActions(t *testing.T) { tpp := newTPP("ns", "tpp", withOWASPCRS(5, 4, 1, 1)) diff --git a/test/e2e/trafficprotectionpolicy-neutralize-inverted/chainsaw-test.yaml b/test/e2e/trafficprotectionpolicy-neutralize-inverted/chainsaw-test.yaml new file mode 100644 index 00000000..5482d51f --- /dev/null +++ b/test/e2e/trafficprotectionpolicy-neutralize-inverted/chainsaw-test.yaml @@ -0,0 +1,236 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/kyverno/chainsaw/main/.schemas/json/test-chainsaw-v1alpha1.json +apiVersion: chainsaw.kyverno.io/v1alpha1 +kind: Test +metadata: + name: trafficprotectionpolicy-neutralize-inverted +# +# What this proves (network-services-operator#242 / #251 / #252 / #273): +# Mechanism A is an inverted OWASP policy (blocking paranoia > detection). CRS +# rule 901500 then fails closed and returns HTTP 500 on 100% of traffic. #251 +# added a CRD CEL rule (self.detection >= self.blocking) that REJECTS inverted +# writes at admission, and #252 set Accepted=False + skipped attachment on the +# EnvoyPatchPolicy path — but that did NOT neutralize the real Coraza edge data +# plane, where the extension server still emitted the inverted policy's +# directives (#273). This test injects a grandfathered inverted policy and +# asserts a benign GET / returns 200: the extension server now withholds the +# inverted policy's directives, so CRS 901500 never fires. +# +# Admission bypass (Case 2 injection): +# #251's CEL rule blocks `kubectl apply` of an inverted TrafficProtectionPolicy, +# so we cannot create one directly. To reproduce a policy that was persisted +# before the CEL rule existed (a grandfathered object), the setup step strips the +# x-kubernetes-validations block from the CRD's paranoiaLevels schema, creates the +# inverted {detection:1, blocking:2} policy while validation is absent, then +# restores the CRD validation. The apiserver does not re-validate existing +# objects on CRD update, so the inverted object survives exactly like a +# grandfathered one, and subsequent writes are rejected again. The strip/restore +# is deterministic: the CRD has a single version (index 0) and one fixed JSON +# path to the validation block. +# +# Precondition: the downstream (nso-infra) must have the WAF data plane wired up +# (`make prepare-infra-cluster` -> `make downstream-waf-dataplane`). +spec: + cluster: nso-infra + # EG only reconciles namespaces carrying this label. + namespaceTemplate: + metadata: + labels: + meta.datumapis.com/upstream-cluster-name: e2e + steps: + - name: Deploy a backend + try: + - apply: + resource: + apiVersion: apps/v1 + kind: Deployment + metadata: + name: echo + spec: + replicas: 1 + selector: + matchLabels: + app: echo + template: + metadata: + labels: + app: echo + spec: + containers: + - name: echo + image: hashicorp/http-echo:1.0 + args: ["-text=hello from backend", "-listen=:8080"] + ports: + - containerPort: 8080 + - apply: + resource: + apiVersion: v1 + kind: Service + metadata: + name: echo + spec: + selector: + app: echo + ports: + - port: 80 + targetPort: 8080 + - assert: + resource: + apiVersion: apps/v1 + kind: Deployment + metadata: + name: echo + status: + availableReplicas: 1 + + - name: Route through the WAF gateway + bindings: + - name: hostname + value: (join('.', [$namespace, 'e2e.test'])) + try: + - apply: + resource: + apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: waf-gw + spec: + gatewayClassName: datum-downstream-gateway + listeners: + - name: http + protocol: HTTP + port: 80 + hostname: ($hostname) + allowedRoutes: + namespaces: + from: Same + - apply: + resource: + apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: echo + spec: + parentRefs: + - name: waf-gw + hostnames: + - ($hostname) + rules: + - matches: + - path: + type: PathPrefix + value: / + backendRefs: + - name: echo + port: 80 + - assert: + timeout: 3m + resource: + apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: waf-gw + status: + (conditions[?type == 'Programmed']): + - status: "True" + + - name: Inject a grandfathered inverted policy by bypassing admission + description: > + Strip the CRD's paranoiaLevels CEL validation, create the inverted + {detection:1, blocking:2} policy, then restore the validation so the + object survives like a grandfathered one and future writes are rejected + again. + try: + - script: + timeout: 120s + content: | + set -eu + crd=trafficprotectionpolicies.networking.datumapis.com + path=/spec/versions/0/schema/openAPIV3Schema/properties/spec/properties/ruleSets/items/properties/owaspCoreRuleSet/properties/paranoiaLevels/x-kubernetes-validations + + kubectl patch crd "${crd}" --type=json \ + -p "[{\"op\":\"remove\",\"path\":\"${path}\"}]" + + restore() { + kubectl patch crd "${crd}" --type=json -p "[{\"op\":\"add\",\"path\":\"${path}\",\"value\":[{\"message\":\"detection paranoia level must be greater than or equal to blocking paranoia level\",\"rule\":\"self.detection >= self.blocking\"}]}]" + } + trap restore EXIT + + created=false + for i in $(seq 1 20); do + if cat <<'EOF' | kubectl apply -f - + apiVersion: networking.datumapis.com/v1alpha + kind: TrafficProtectionPolicy + metadata: + name: inverted-waf + spec: + mode: Enforce + targetRefs: + - group: gateway.networking.k8s.io + kind: Gateway + name: waf-gw + ruleSets: + - type: OWASPCoreRuleSet + owaspCoreRuleSet: + paranoiaLevels: + detection: 1 + blocking: 2 + EOF + then + created=true + break + fi + echo "apply attempt ${i} failed (schema cache lag?), retrying" + sleep 2 + done + + if [ "${created}" != "true" ]; then + echo "failed to inject inverted policy while CEL was stripped" + exit 1 + fi + + blocking=$(kubectl get trafficprotectionpolicy inverted-waf \ + -o jsonpath='{.spec.ruleSets[0].owaspCoreRuleSet.paranoiaLevels.blocking}') + detection=$(kubectl get trafficprotectionpolicy inverted-waf \ + -o jsonpath='{.spec.ruleSets[0].owaspCoreRuleSet.paranoiaLevels.detection}') + echo "injected inverted policy: blocking=${blocking} detection=${detection}" + if [ "${blocking}" != "2" ] || [ "${detection}" != "1" ]; then + echo "inverted policy not persisted as expected" + exit 1 + fi + + - name: A benign GET must return 200, not 500 + description: > + The neutralized behavior. The extension server withholds the inverted + policy's Coraza directives from the edge data plane, so CRS rule 901500 + never fires and benign traffic is unaffected. + bindings: + - name: hostname + value: (join('.', [$namespace, 'e2e.test'])) + try: + - script: + timeout: 180s + env: + - name: HOSTNAME + value: ($hostname) + content: | + set -u + for i in $(seq 1 40); do + code=$(curl -s -o /dev/null -w '%{http_code}' --max-time 5 \ + -H "Host: ${HOSTNAME}" http://localhost:30080/) || code=000 + echo "attempt ${i}: HTTP ${code}" + if [ "${code}" = "200" ]; then + exit 0 + fi + if [ "${code}" = "500" ]; then + echo "inverted policy still enforced on the edge (500) — regression of #273" + fi + sleep 3 + done + echo "benign request never returned 200 (regression of network-services-operator#273)" + exit 1 + finally: + - delete: + ref: + apiVersion: networking.datumapis.com/v1alpha + kind: TrafficProtectionPolicy + name: inverted-waf