Skip to content

SecurityPolicy: Gateway-targeted authorization is attached to L4 listeners without validation #9868

Description

@zhaohuabing

Description:

A SecurityPolicy that targets a Gateway, a ListenerSet, or one of their listeners is never checked against the L4 rules. validateSecurityPolicyForL4 only runs when the policy targets a TCPRoute or a UDPRoute. For every other target the whole authorization object is attached to the TCP and UDP listeners of that Gateway as-is.

The L4 matcher only reads principal.clientCIDRs. Everything else in a rule is thrown away, and that changes what the rule means:

rule as written what the L4 listener gets
Allow if operation.methods: [GET] and clientCIDRs: 10.0.0.0/8 Allow from 10.0.0.0/8. The method check is gone, so the rule allows more than it says.
Allow if headers: x-trusted=yes, no clientCIDRs The rule disappears. Traffic falls through to defaultAction.

The first one is the problem. Dropping a check from an authorization rule opens up access instead of restricting it. The second one changes the result in whichever direction defaultAction points.

In both cases the policy reports Accepted=True and says nothing about it.

Repro:

Apply a Gateway with an HTTP listener and a UDP listener, a UDPRoute on the UDP listener, and this policy:

apiVersion: gateway.envoyproxy.io/v1alpha1
kind: SecurityPolicy
metadata:
  name: sp-gw-mixed
spec:
  targetRefs:
  - group: gateway.networking.k8s.io
    kind: Gateway
    name: gw-mixed
  authorization:
    defaultAction: Deny
    rules:
    - action: Allow
      name: allow-get-from-corp
      operation:
        methods: ["GET"]
      principal:
        clientCIDRs:
        - 10.0.0.0/8
    - action: Allow
      name: allow-by-header
      principal:
        headers:
        - name: x-trusted
          values: ["yes"]

egctl x translate --from gateway-api --to xds gives a udp_proxy matcher with one entry. Its only check is 10.0.0.0/8. The operation and the header rule are both missing.

The same thing happens when the policy names the UDP listener directly with sectionName: udp.

Cause:

  • The validator dispatch in internal/gatewayapi/securitypolicy.go picks validateSecurityPolicyForL4 only for KindTCPRoute and KindUDPRoute. A Gateway target goes to validateSecurityPolicy, which allows HTTP-only fields.
  • translateSecurityPolicyForListeners then copies the whole ir.Authorization onto every matching route in x.TCP and x.UDP.

Affected:

Policies that target a route are handled correctly. #9833 also made the L4 validator reject operation, which it had been missing.

What should happen:

There are three kinds of target, and they do not all want the same answer.

  1. The policy names an L4 listener, for example Gateway plus sectionName on a UDP listener. The user clearly meant that listener, so HTTP-only fields there are simply wrong. Reject the policy, the same as a route target already does.

  2. The policy targets a whole Gateway or ListenerSet that has both HTTP and L4 listeners. Do not reject it. Rejecting would take working JWT or OIDC auth away from the HTTP listeners in order to fix the L4 listeners, and it would break existing users whose Gateway-wide policy happens to sit on a Gateway that also has a TCP listener. Gateway-wide policies are meant to cover several protocols. cors is already ignored on TCP listeners for the same reason.

  3. The policy targets a Gateway whose listeners are all L4. Nothing can ever honour it, so rejecting costs nothing.

One rule covers all three and keeps today's behaviour for route targets:

Reject the policy if it cannot take effect on any listener it targets. Otherwise apply it where it can work, drop the rules that do not fully fit on the L4 listeners, and say so in the status.

Dropping the whole rule instead of just the parts that do not fit is what removes the over-broad access. An over-broad rule becomes no rule, which under a Deny default fails closed.

Status reporting:

Put a short note in the Accepted condition message. A separate Warning condition is more than this needs, since the policy really is accepted for the listeners it can serve.

Two things to know before writing it:

  • SetAcceptedForPolicyAncestor in internal/gatewayapi/status/policy.go hardcodes "Policy has been accepted." and takes no message argument. It also returns early if an Accepted condition is already set for that ancestor, so the message has to be written when Accepted is first set, not appended afterwards. Either add a variant that takes a message, or call SetConditionForPolicyAncestor directly.
  • A policy that targets a whole Gateway gets a single ancestorRef with no sectionName. There is no per-listener ancestor to attach a per-listener message to, so the message has to name the skipped listeners itself.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    kind/bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions