From 383041ecdee2b476f4f3094bdaf7999515d7b0cd Mon Sep 17 00:00:00 2001 From: Lior Okman Date: Mon, 18 Mar 2024 09:17:43 +0200 Subject: [PATCH] Make SecurityPolicy validate correctly. Signed-off-by: Lior Okman --- internal/gatewayapi/securitypolicy.go | 23 +- .../testdata/conflicting-policies.out.yaml | 2 +- .../merge-with-isolated-policies-2.in.yaml | 43 +- .../merge-with-isolated-policies-2.out.yaml | 667 ++++++++++++++++++ 4 files changed, 727 insertions(+), 8 deletions(-) create mode 100755 internal/gatewayapi/testdata/merge-with-isolated-policies-2.out.yaml diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go index 4a85f726a7fa..5a0ae8a5cde8 100644 --- a/internal/gatewayapi/securitypolicy.go +++ b/internal/gatewayapi/securitypolicy.go @@ -194,7 +194,7 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security irKey := t.getIRKey(targetedGateway.Gateway) // Should exist since we've validated this xds := xdsIR[irKey] - err := validatePortOverlapForSecurityPolicyGateway(xds) + err := validatePortOverlapForSecurityPolicyGateway(xds, targetedGateway) if err == nil { err = t.translateSecurityPolicyForGateway(policy, targetedGateway, resources, xdsIR) } @@ -522,11 +522,26 @@ func (t *Translator) translateSecurityPolicyForGateway( return errs } -func validatePortOverlapForSecurityPolicyGateway(xds *ir.Xds) error { +// should return error if the policy attaches to listeners that originate from gateways other than the one requested on the policy. +func validatePortOverlapForSecurityPolicyGateway(xds *ir.Xds, targetedGateway *GatewayContext) error { + targetedGwName := irStringKey(targetedGateway.Namespace, targetedGateway.Name) + relevantPorts := map[uint32]bool{} + for _, listener := range targetedGateway.listeners { + containerPort := servicePortToContainerPort(int32(listener.Port)) + relevantPorts[uint32(containerPort)] = true + } affectedListeners := []string{} for _, http := range xds.HTTP { - if sameListeners := listenersWithSameHTTPPort(xds, http); len(sameListeners) != 0 { - affectedListeners = append(affectedListeners, sameListeners...) + if _, found := relevantPorts[http.Port]; !found { + continue + } + // look for listeners on this XDS that aren't from the targetedGateway + for _, currListener := range listenersWithSameHTTPPort(xds, http) { + listenerName := currListener[0:strings.LastIndex(currListener, "/")] + if listenerName != targetedGwName { + affectedListeners = append(affectedListeners, currListener) + } + } } diff --git a/internal/gatewayapi/testdata/conflicting-policies.out.yaml b/internal/gatewayapi/testdata/conflicting-policies.out.yaml index 7e0d52a41d16..37b1d55f5f2c 100644 --- a/internal/gatewayapi/testdata/conflicting-policies.out.yaml +++ b/internal/gatewayapi/testdata/conflicting-policies.out.yaml @@ -261,7 +261,7 @@ securityPolicies: namespace: default conditions: - lastTransitionTime: null - message: 'Affects multiple listeners: default/mfqjpuycbgjrtdww/http, default/gateway-1/http' + message: 'Affects multiple listeners: default/gateway-1/http' reason: Invalid status: "False" type: Accepted diff --git a/internal/gatewayapi/testdata/merge-with-isolated-policies-2.in.yaml b/internal/gatewayapi/testdata/merge-with-isolated-policies-2.in.yaml index 83d9eed4343c..888c4e4b7133 100644 --- a/internal/gatewayapi/testdata/merge-with-isolated-policies-2.in.yaml +++ b/internal/gatewayapi/testdata/merge-with-isolated-policies-2.in.yaml @@ -90,6 +90,44 @@ httpRoutes: backendRefs: - name: service-2 port: 8080 + - apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-3 + spec: + hostnames: + - bar.example.com + parentRefs: + - namespace: default + name: gateway-2 + sectionName: http + rules: + - matches: + - path: + value: "/" + backendRefs: + - name: service-1 + port: 8080 + - apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: httproute-4 + spec: + hostnames: + - foo.example.com + parentRefs: + - namespace: default + name: gateway-2 + sectionName: http-2 + rules: + - matches: + - path: + value: "/" + backendRefs: + - name: service-2 + port: 8080 securityPolicies: - apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy @@ -123,10 +161,9 @@ securityPolicies: spec: targetRef: group: gateway.networking.k8s.io - kind: Gateway - name: gateway-2 + kind: HTTPRoute + name: httproute-3 namespace: default - sectionName: http cors: allowOrigins: - "*" diff --git a/internal/gatewayapi/testdata/merge-with-isolated-policies-2.out.yaml b/internal/gatewayapi/testdata/merge-with-isolated-policies-2.out.yaml new file mode 100755 index 000000000000..25b7c579d54c --- /dev/null +++ b/internal/gatewayapi/testdata/merge-with-isolated-policies-2.out.yaml @@ -0,0 +1,667 @@ +backendTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: BackendTrafficPolicy + metadata: + creationTimestamp: null + name: policy-for-gateway + namespace: default + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: default + tcpKeepalive: + idleTime: 20m + interval: 60s + probes: 3 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: default + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +clientTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: ClientTrafficPolicy + metadata: + creationTimestamp: null + name: target-gateway-2 + namespace: default + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + namespace: default + sectionName: http + timeout: + http: + requestReceivedTimeout: 5s + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + namespace: default + sectionName: http + conditions: + - lastTransitionTime: null + message: 'Affects additional listeners: default/gateway-2/http-2' + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: ClientTrafficPolicy + metadata: + creationTimestamp: null + name: target-gateway + namespace: default + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: default + timeout: + http: + requestReceivedTimeout: 5s + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: default + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +gateways: +- apiVersion: gateway.networking.k8s.io/v1beta1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-1 + namespace: default + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: Same + hostname: bar.example.com + name: http + port: 80 + protocol: HTTP + - allowedRoutes: + namespaces: + from: Same + hostname: foo.example.com + name: http-2 + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http-2 + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +- apiVersion: gateway.networking.k8s.io/v1beta1 + kind: Gateway + metadata: + creationTimestamp: null + name: gateway-2 + namespace: default + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: Same + hostname: bar.example.com + name: http + port: 81 + protocol: HTTP + - allowedRoutes: + namespaces: + from: Same + hostname: foo.example.com + name: http-2 + port: 81 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http-2 + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-1 + namespace: default + spec: + hostnames: + - bar.example.com + parentRefs: + - name: gateway-1 + namespace: default + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: / + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: default + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-2 + namespace: default + spec: + hostnames: + - foo.example.com + parentRefs: + - name: gateway-1 + namespace: default + sectionName: http-2 + rules: + - backendRefs: + - name: service-2 + port: 8080 + matches: + - path: + value: / + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: default + sectionName: http-2 +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-3 + namespace: default + spec: + hostnames: + - bar.example.com + parentRefs: + - name: gateway-2 + namespace: default + sectionName: http + rules: + - backendRefs: + - name: service-1 + port: 8080 + matches: + - path: + value: / + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-2 + namespace: default + sectionName: http +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + creationTimestamp: null + name: httproute-4 + namespace: default + spec: + hostnames: + - foo.example.com + parentRefs: + - name: gateway-2 + namespace: default + sectionName: http-2 + rules: + - backendRefs: + - name: service-2 + port: 8080 + matches: + - path: + value: / + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-2 + namespace: default + sectionName: http-2 +infraIR: + envoy-gateway-class: + proxy: + config: + apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: EnvoyProxy + metadata: + creationTimestamp: null + name: test + namespace: envoy-gateway-system + spec: + logging: {} + mergeGateways: true + status: {} + listeners: + - address: null + name: default/gateway-1/http + ports: + - containerPort: 10080 + name: default/gateway-1/http + protocol: HTTP + servicePort: 80 + - address: null + name: default/gateway-2/http + ports: + - containerPort: 10081 + name: default/gateway-2/http + protocol: HTTP + servicePort: 81 + metadata: + labels: + gateway.envoyproxy.io/owning-gatewayclass: envoy-gateway-class + name: envoy-gateway-class +securityPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-for-route-2 + namespace: default + spec: + cors: + allowHeaders: + - x-header-5 + - x-header-6 + allowMethods: + - GET + - POST + allowOrigins: + - '*' + exposeHeaders: + - x-header-7 + - x-header-8 + maxAge: 33m20s + targetRef: + group: gateway.networking.k8s.io + kind: HTTPRoute + name: httproute-3 + namespace: default + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-2 + namespace: default + sectionName: http + conditions: + - lastTransitionTime: null + message: 'Affects multiple listeners: default/gateway-2/http-2' + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: SecurityPolicy + metadata: + creationTimestamp: null + name: policy-for-route-1 + namespace: default + spec: + cors: + allowHeaders: + - x-header-5 + - x-header-6 + allowMethods: + - GET + - POST + allowOrigins: + - '*' + exposeHeaders: + - x-header-7 + - x-header-8 + maxAge: 33m20s + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: default + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: default + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +xdsIR: + envoy-gateway-class: + accessLog: + text: + - path: /dev/stdout + http: + - address: 0.0.0.0 + hostnames: + - bar.example.com + isHTTP2: false + name: default/gateway-1/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - backendWeights: + invalid: 0 + valid: 0 + cors: + allowHeaders: + - x-header-5 + - x-header-6 + allowMethods: + - GET + - POST + allowOrigins: + - distinct: false + name: "" + safeRegex: .* + exposeHeaders: + - x-header-7 + - x-header-8 + maxAge: 33m20s + destination: + name: httproute/default/httproute-1/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: bar.example.com + isHTTP2: false + name: httproute/default/httproute-1/rule/0/match/0/bar_example_com + pathMatch: + distinct: false + name: "" + prefix: / + tcpKeepalive: + idleTime: 1200 + interval: 60 + probes: 3 + timeout: + http: + requestReceivedTimeout: 5s + - address: 0.0.0.0 + hostnames: + - foo.example.com + isHTTP2: false + name: default/gateway-1/http-2 + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - backendWeights: + invalid: 0 + valid: 0 + cors: + allowHeaders: + - x-header-5 + - x-header-6 + allowMethods: + - GET + - POST + allowOrigins: + - distinct: false + name: "" + safeRegex: .* + exposeHeaders: + - x-header-7 + - x-header-8 + maxAge: 33m20s + destination: + name: httproute/default/httproute-2/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: foo.example.com + isHTTP2: false + name: httproute/default/httproute-2/rule/0/match/0/foo_example_com + pathMatch: + distinct: false + name: "" + prefix: / + tcpKeepalive: + idleTime: 1200 + interval: 60 + probes: 3 + timeout: + http: + requestReceivedTimeout: 5s + - address: 0.0.0.0 + hostnames: + - bar.example.com + isHTTP2: false + name: default/gateway-2/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10081 + routes: + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: httproute/default/httproute-3/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: bar.example.com + isHTTP2: false + name: httproute/default/httproute-3/rule/0/match/0/bar_example_com + pathMatch: + distinct: false + name: "" + prefix: / + - address: 0.0.0.0 + hostnames: + - foo.example.com + isHTTP2: false + name: default/gateway-2/http-2 + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10081 + routes: + - backendWeights: + invalid: 0 + valid: 0 + destination: + name: httproute/default/httproute-4/rule/0 + settings: + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + protocol: HTTP + weight: 1 + hostname: foo.example.com + isHTTP2: false + name: httproute/default/httproute-4/rule/0/match/0/foo_example_com + pathMatch: + distinct: false + name: "" + prefix: /