return 500 error for failed SecurityPolicies to avoid unauthorized access to xRoutes

internal/gatewayapi/securitypolicy.go

@@ -394,8 +394,6 @@ func (t *Translator) translateSecurityPolicyForRoute(
 	}
 
 	// Apply IR to all relevant routes
-	// Note: there are multiple features in a security policy, even if some of them
-	// are invalid, we still want to apply the valid ones.
 	prefix := irRoutePrefix(route)
 	parentRefs := GetParentReferences(route)
 	for _, p := range parentRefs {
@@ -430,6 +428,12 @@ func (t *Translator) translateSecurityPolicyForRoute(
 							ExtAuth:       extAuth,
 							Authorization: authorization,
 						}
+						if errs != nil {
+							// Return a 500 direct response to avoid unauthorized access
+							r.DirectResponse = &ir.DirectResponse{
+								StatusCode: 500,
+							}
+						}
 					}
 				}
 			}
@@ -523,7 +527,6 @@ func (t *Translator) translateSecurityPolicyForGateway(
 			if r.Security != nil {
 				continue
 			}
-
 			r.Security = &ir.SecurityFeatures{
 				CORS:          cors,
 				JWT:           jwt,
@@ -532,6 +535,12 @@ func (t *Translator) translateSecurityPolicyForGateway(
 				ExtAuth:       extAuth,
 				Authorization: authorization,
 			}
+			if errs != nil {
+				// Return a 500 direct response to avoid unauthorized access
+				r.DirectResponse = &ir.DirectResponse{
+					StatusCode: 500,
+				}
+			}
 		}
 	}
 	return errs

internal/gatewayapi/testdata/securitypolicy-with-extauth-invalid-no-matching-port.out.yaml

@@ -160,6 +160,8 @@ xdsIR:
               port: 8080
             protocol: HTTP
             weight: 1
+        directResponse:
+          statusCode: 500
         hostname: www.foo.com
         isHTTP2: false
         metadata:

internal/gatewayapi/testdata/securitypolicy-with-extauth-invalid-no-port.out.yaml

@@ -160,6 +160,8 @@ xdsIR:
               port: 8080
             protocol: HTTP
             weight: 1
+        directResponse:
+          statusCode: 500
         hostname: www.foo.com
         isHTTP2: false
         metadata:

internal/gatewayapi/testdata/securitypolicy-with-extauth-invalid-no-reference-grant.out.yaml

@@ -161,6 +161,8 @@ xdsIR:
               port: 8080
             protocol: HTTP
             weight: 1
+        directResponse:
+          statusCode: 500
         hostname: www.foo.com
         isHTTP2: false
         metadata:

internal/gatewayapi/testdata/securitypolicy-with-extauth-invalid-no-service.out.yaml

@@ -160,6 +160,8 @@ xdsIR:
               port: 8080
             protocol: HTTP
             weight: 1
+        directResponse:
+          statusCode: 500
         hostname: www.foo.com
         isHTTP2: false
         metadata:

internal/gatewayapi/testdata/securitypolicy-with-jwt-and-invalid-oidc.out.yaml

@@ -258,6 +258,8 @@ xdsIR:
               port: 8080
             protocol: HTTP
             weight: 1
+        directResponse:
+          statusCode: 500
         hostname: gateway.envoyproxy.io
         isHTTP2: false
         metadata:
@@ -290,6 +292,8 @@ xdsIR:
               port: 8080
             protocol: HTTP
             weight: 1
+        directResponse:
+          statusCode: 500
         hostname: gateway.envoyproxy.io
         isHTTP2: false
         metadata:

test/e2e/testdata/securitypolicy-translation-failed.yaml

@@ -0,0 +1,39 @@
+---
+apiVersion: gateway.networking.k8s.io/v1
+kind: HTTPRoute
+metadata:
+  name: http-with-oidc
+  namespace: gateway-conformance-infra
+spec:
+  parentRefs:
+  - name: same-namespace
+  hostnames: ["www.example.com"]
+  rules:
+  - matches:
+    - path:
+        type: PathPrefix
+        value: /myapp              # This is the path that will be protected by OIDC
+    backendRefs:
+    - name: infra-backend-v1
+      port: 8080
+---
+apiVersion: gateway.envoyproxy.io/v1alpha1
+kind: SecurityPolicy
+metadata:
+  name: oidc-test
+  namespace: gateway-conformance-infra
+spec:
+  targetRefs:
+  - group: gateway.networking.k8s.io
+    kind: HTTPRoute
+    name: http-with-oidc
+  oidc:
+    provider:
+      issuer: "http://keycloak.gateway-conformance-infra/realms/master"
+      authorizationEndpoint: "http://keycloak.gateway-conformance-infra/realms/master/protocol/openid-connect/auth"
+      tokenEndpoint: "http://keycloak.gateway-conformance-infra/realms/master/protocol/openid-connect/token"
+    clientID: "oidctest"
+    clientSecret:
+      name: "oidctest-secret"
+    redirectURL: "http://www.example.com/myapp/oauth2/callback"
+    logoutPath: "/myapp/logout"

test/e2e/tests/securitypolicy_transaltion_failed.go

@@ -0,0 +1,62 @@
+// Copyright Envoy Gateway Authors
+// SPDX-License-Identifier: Apache-2.0
+// The full text of the Apache license is available in the LICENSE file at
+// the root of the repo.
+
+//go:build e2e
+// +build e2e
+
+package tests
+
+import (
+	"testing"
+
+	"k8s.io/apimachinery/pkg/types"
+	gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
+	gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
+	"sigs.k8s.io/gateway-api/conformance/utils/http"
+	"sigs.k8s.io/gateway-api/conformance/utils/kubernetes"
+	"sigs.k8s.io/gateway-api/conformance/utils/suite"
+
+	"github.com/envoyproxy/gateway/internal/gatewayapi"
+)
+
+func init() {
+	ConformanceTests = append(ConformanceTests, FailedSecurityPolicyDirectResponseTest)
+}
+
+// FailedSecurityPolicyDirectResponseTest tests the direct 500 response for HTTPRoute targeted by a failed SecurityPolicy.
+var FailedSecurityPolicyDirectResponseTest = suite.ConformanceTest{
+	ShortName:   "FailedSecurityPolicyDirectResponse",
+	Description: "Test direct 500 response when failed to translate SecurityPolicy",
+	Manifests:   []string{"testdata/securitypolicy-translation-failed.yaml"},
+	Test: func(t *testing.T, suite *suite.ConformanceTestSuite) {
+		t.Run("http route with failed SecurityPolicy", func(t *testing.T) {
+			ns := "gateway-conformance-infra"
+			routeNN := types.NamespacedName{Name: "http-with-oidc", Namespace: ns}
+			gwNN := types.NamespacedName{Name: "same-namespace", Namespace: ns}
+			gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN)
+
+			ancestorRef := gwapiv1a2.ParentReference{
+				Group:     gatewayapi.GroupPtr(gwapiv1.GroupName),
+				Kind:      gatewayapi.KindPtr(gatewayapi.KindGateway),
+				Namespace: gatewayapi.NamespacePtr(gwNN.Namespace),
+				Name:      gwapiv1.ObjectName(gwNN.Name),
+			}
+			SecurityPolicyMustFail(t, suite.Client, types.NamespacedName{Name: "oidc-test", Namespace: ns}, suite.ControllerName, ancestorRef, "")
+
+			expectedResponse := http.ExpectedResponse{
+				Request: http.Request{
+					Host: "www.example.com",
+					Path: "/myapp",
+				},
+				Response: http.Response{
+					StatusCode: 500,
+				},
+				Namespace: ns,
+			}
+
+			http.MakeRequestAndExpectEventuallyConsistentResponse(t, suite.RoundTripper, suite.TimeoutConfig, gwAddr, expectedResponse)
+		})
+	},
+}

test/e2e/tests/utils.go

@@ -104,6 +104,33 @@ func SecurityPolicyMustBeAccepted(t *testing.T, client client.Client, policyName
 	require.NoErrorf(t, waitErr, "error waiting for SecurityPolicy to be accepted")
 }
 
+// SecurityPolicyMustFail waits for an SecurityPolicy to fail with the specified reason.
+func SecurityPolicyMustFail(
+	t *testing.T, client client.Client, policyName types.NamespacedName,
+	controllerName string, ancestorRef gwapiv1a2.ParentReference, message string,
+) {
+	t.Helper()
+
+	policy := &egv1a1.SecurityPolicy{}
+	waitErr := wait.PollUntilContextTimeout(
+		context.Background(), 1*time.Second, 60*time.Second,
+		true, func(ctx context.Context) (bool, error) {
+			err := client.Get(ctx, policyName, policy)
+			if err != nil {
+				return false, fmt.Errorf("error fetching SecurityPolicy: %w", err)
+			}
+
+			if policyFailAcceptedByAncestor(policy.Status.Ancestors, controllerName, ancestorRef, message) {
+				t.Logf("SecurityPolicy has been failed: %v", policy)
+				return true, nil
+			}
+
+			return false, nil
+		})
+
+	require.NoErrorf(t, waitErr, "error waiting for SecurityPolicy to fail with message: %s policy %v", message, policy)
+}
+
 // BackendTrafficPolicyMustBeAccepted waits for the specified BackendTrafficPolicy to be accepted.
 func BackendTrafficPolicyMustBeAccepted(t *testing.T, client client.Client, policyName types.NamespacedName, controllerName string, ancestorRef gwapiv1a2.ParentReference) {
 	t.Helper()