Merge pull request #274 from datum-cloud/fix/273-neutralize-inverted-edge

fix: neutralize persisted inverted TPP on the coraza edge path (#252 follow-up)

Author: ecv

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.
 	//

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")
+		})
+	}
+}

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(

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 {

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))