Skip to content

Commit d17ce22

Browse files
committed
fix: flag inverted TrafficProtectionPolicy paranoia levels
A TrafficProtectionPolicy whose OWASP CRS detection paranoia level is below its blocking paranoia level is an illegal configuration: CRS rule 901500 fails closed and denies every request at phase 1 with HTTP 500 before any attack evaluation. Such a policy silently 500s 100% of traffic to the protected route with no signal to the tenant about why. Reject the policy during reconciliation: set Accepted=False with reason Invalid on the affected ancestor and skip attachment so the broken directives are never emitted to the data plane. This also surfaces already-persisted policies that carry the inverted configuration. Key changes: - add paranoiaLevelsResolveError guard in the TPP reconciler - set Accepted=False/Invalid and skip attach for HTTPRoute and Gateway targets when detection < blocking - add unit coverage for the guard and both process paths
1 parent e159acf commit d17ce22

2 files changed

Lines changed: 149 additions & 0 deletions

File tree

internal/controller/trafficprotectionpolicy_controller.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -796,6 +796,17 @@ func (r *TrafficProtectionPolicyReconciler) processTrafficProtectionPolicyForHTT
796796
route.attachedToRouteRules.Insert(routeRuleName)
797797
}
798798

799+
if resolveErr := paranoiaLevelsResolveError(policy); resolveErr != nil {
800+
gatewaystatus.SetResolveErrorForPolicyAncestor(
801+
&policy.Status.PolicyStatus,
802+
ancestorRef,
803+
string(r.Config.Gateway.ControllerName),
804+
policy.Generation,
805+
resolveErr,
806+
)
807+
return policyAttachments
808+
}
809+
799810
gatewaystatus.SetConditionForPolicyAncestor(&policy.Status.PolicyStatus,
800811
ancestorRef,
801812
string(r.Config.Gateway.ControllerName),
@@ -933,6 +944,17 @@ func (r *TrafficProtectionPolicyReconciler) processTrafficProtectionPolicyForGat
933944
gateway.attachedToListeners.Insert(listenerName)
934945
}
935946

947+
if resolveErr := paranoiaLevelsResolveError(policy); resolveErr != nil {
948+
gatewaystatus.SetResolveErrorForPolicyAncestor(
949+
&policy.Status.PolicyStatus,
950+
ancestorRef,
951+
string(r.Config.Gateway.ControllerName),
952+
policy.Generation,
953+
resolveErr,
954+
)
955+
return policyAttachments
956+
}
957+
936958
gatewaystatus.SetConditionForPolicyAncestor(&policy.Status.PolicyStatus,
937959
ancestorRef,
938960
string(r.Config.Gateway.ControllerName),
@@ -1208,6 +1230,28 @@ func getVHostConstraintForGateway(namespace string, gateway *gatewayv1.Gateway)
12081230
)
12091231
}
12101232

1233+
// paranoiaLevelsResolveError returns a resolve error when a policy's OWASP CRS
1234+
// paranoia levels are inverted (detection below blocking). CRS rule 901500 fails
1235+
// closed on this configuration and denies every request with HTTP 500 before any
1236+
// attack evaluation, so the policy must not be attached.
1237+
func paranoiaLevelsResolveError(policy *policyContext) *gatewaystatus.PolicyResolveError {
1238+
for _, ruleSet := range policy.Spec.RuleSets {
1239+
if ruleSet.Type != networkingv1alpha.TrafficProtectionPolicyOWASPCoreRuleSet {
1240+
continue
1241+
}
1242+
levels := ruleSet.OWASPCoreRuleSet.ParanoiaLevels
1243+
if levels.Detection < levels.Blocking {
1244+
return &gatewaystatus.PolicyResolveError{
1245+
Reason: gatewayv1.PolicyReasonInvalid,
1246+
Message: fmt.Sprintf(
1247+
"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",
1248+
levels.Detection, levels.Blocking),
1249+
}
1250+
}
1251+
}
1252+
return nil
1253+
}
1254+
12111255
func (r *TrafficProtectionPolicyReconciler) getCorazaDirectivesForTrafficProtectionPolicy(
12121256
policy *policyContext,
12131257
) []string {

internal/controller/trafficprotectionpolicy_controller_test.go

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -518,6 +518,43 @@ func TestProcessTrafficProtectionPolicyForHTTPRoute(t *testing.T) {
518518
}
519519
},
520520
},
521+
{
522+
name: "inverted paranoia levels rejected",
523+
policy: &policyContext{
524+
TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1", func(tpp *networkingv1alpha.TrafficProtectionPolicy) {
525+
tpp.Spec.RuleSets[0].OWASPCoreRuleSet.ParanoiaLevels = networkingv1alpha.ParanoiaLevels{
526+
Blocking: 2,
527+
Detection: 1,
528+
}
529+
})),
530+
},
531+
routeMap: map[client.ObjectKey]*policyRouteTargetContext{
532+
{Namespace: "default", Name: "route-1"}: {
533+
HTTPRoute: newHTTPRoute("default", "route-1"),
534+
},
535+
},
536+
gatewayMap: map[client.ObjectKey]*policyGatewayTargetContext{
537+
{Namespace: "default", Name: "gateway-1"}: {
538+
Gateway: ptr.To(newGatewayFunc("default", "gateway-1")),
539+
},
540+
},
541+
targetRef: gatewayv1alpha2.LocalPolicyTargetReferenceWithSectionName{
542+
LocalPolicyTargetReference: gatewayv1.LocalPolicyTargetReference{
543+
Kind: "HTTPRoute",
544+
Name: "route-1",
545+
},
546+
},
547+
assert: func(t *testContext, policyAttachments []policyAttachment) {
548+
assert.Empty(t, policyAttachments, "invalid policy must not be attached")
549+
if assert.Len(t, t.policy.Status.Ancestors, 1) {
550+
if assert.Len(t, t.policy.Status.Ancestors[0].Conditions, 1) {
551+
cond := t.policy.Status.Ancestors[0].Conditions[0]
552+
assert.Equal(t, string(gatewayv1.PolicyReasonInvalid), cond.Reason, "expected invalid reason")
553+
assert.Equal(t, metav1.ConditionFalse, cond.Status, "expected Accepted=False")
554+
}
555+
}
556+
},
557+
},
521558
}
522559

523560
for _, tt := range tests {
@@ -649,6 +686,38 @@ func TestProcessTrafficProtectionPolicyForGateway(t *testing.T) {
649686
}
650687
},
651688
},
689+
{
690+
name: "inverted paranoia levels rejected",
691+
policy: &policyContext{
692+
TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1", func(tpp *networkingv1alpha.TrafficProtectionPolicy) {
693+
tpp.Spec.RuleSets[0].OWASPCoreRuleSet.ParanoiaLevels = networkingv1alpha.ParanoiaLevels{
694+
Blocking: 2,
695+
Detection: 1,
696+
}
697+
})),
698+
},
699+
gatewayMap: map[client.ObjectKey]*policyGatewayTargetContext{
700+
{Namespace: "default", Name: "gateway-1"}: {
701+
Gateway: ptr.To(newGatewayFunc("default", "gateway-1")),
702+
},
703+
},
704+
targetRef: gatewayv1alpha2.LocalPolicyTargetReferenceWithSectionName{
705+
LocalPolicyTargetReference: gatewayv1.LocalPolicyTargetReference{
706+
Kind: "Gateway",
707+
Name: "gateway-1",
708+
},
709+
},
710+
assert: func(t *testContext, policyAttachments []policyAttachment) {
711+
assert.Empty(t, policyAttachments, "invalid policy must not be attached")
712+
if assert.Len(t, t.policy.Status.Ancestors, 1) {
713+
if assert.Len(t, t.policy.Status.Ancestors[0].Conditions, 1) {
714+
cond := t.policy.Status.Ancestors[0].Conditions[0]
715+
assert.Equal(t, string(gatewayv1.PolicyReasonInvalid), cond.Reason, "expected invalid reason")
716+
assert.Equal(t, metav1.ConditionFalse, cond.Status, "expected Accepted=False")
717+
}
718+
}
719+
},
720+
},
652721
}
653722

654723
for _, tt := range tests {
@@ -675,6 +744,42 @@ func TestProcessTrafficProtectionPolicyForGateway(t *testing.T) {
675744
}
676745
}
677746

747+
func TestParanoiaLevelsResolveError(t *testing.T) {
748+
tests := []struct {
749+
name string
750+
blocking int
751+
detection int
752+
wantError bool
753+
}{
754+
{name: "equal levels", blocking: 2, detection: 2, wantError: false},
755+
{name: "higher detection", blocking: 1, detection: 3, wantError: false},
756+
{name: "detection below blocking", blocking: 2, detection: 1, wantError: true},
757+
{name: "defaulted detection below blocking", blocking: 4, detection: 1, wantError: true},
758+
}
759+
760+
for _, tt := range tests {
761+
t.Run(tt.name, func(t *testing.T) {
762+
policy := &policyContext{
763+
TrafficProtectionPolicy: ptr.To(newTrafficProtectionPolicy("default", "tpp-1", func(tpp *networkingv1alpha.TrafficProtectionPolicy) {
764+
tpp.Spec.RuleSets[0].OWASPCoreRuleSet.ParanoiaLevels = networkingv1alpha.ParanoiaLevels{
765+
Blocking: tt.blocking,
766+
Detection: tt.detection,
767+
}
768+
})),
769+
}
770+
771+
resolveErr := paranoiaLevelsResolveError(policy)
772+
if tt.wantError {
773+
if assert.NotNil(t, resolveErr) {
774+
assert.Equal(t, gatewayv1.PolicyReasonInvalid, resolveErr.Reason)
775+
}
776+
} else {
777+
assert.Nil(t, resolveErr)
778+
}
779+
})
780+
}
781+
}
782+
678783
func TestGetDesiredEnvoyPatchPolicies(t *testing.T) {
679784

680785
operatorConfig := config.NetworkServicesOperator{

0 commit comments

Comments
 (0)