Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 73 additions & 17 deletions internal/gatewayapi/securitypolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -512,9 +512,13 @@ func (t *Translator) processSecurityPolicyForRoute(
// then run it once to keep the flow linear and easier to read.
validator := validateSecurityPolicy
errMsg := "invalid SecurityPolicy"
if currTarget.Kind == resource.KindTCPRoute {
validator = validateSecurityPolicyForTCP
switch currTarget.Kind {
case resource.KindTCPRoute:
validator = func(p *egv1a1.SecurityPolicy) error { return validateSecurityPolicyForL4(p, "TCP") }
errMsg = "invalid SecurityPolicy for TCP route"
case resource.KindUDPRoute:
validator = func(p *egv1a1.SecurityPolicy) error { return validateSecurityPolicyForL4(p, "UDP") }
errMsg = "invalid SecurityPolicy for UDP route"
}
if err := validator(policy); err != nil {
status.SetTranslationErrorForPolicyAncestors(&policy.Status,
Expand Down Expand Up @@ -968,36 +972,38 @@ func validateSecurityPolicy(p *egv1a1.SecurityPolicy) error {
return nil
}

// validateSecurityPolicyForTCP ensures SecurityPolicy usage on TCP is compatible.
// validateSecurityPolicyForL4 ensures SecurityPolicy usage on an L4 protocol
// (TCP or UDP) is compatible. proto names the protocol in the returned errors.
//
// TCP supports Authorization with ClientCIDRs ONLY.
// L4 supports Authorization with ClientCIDRs ONLY, because there is no HTTP
// request to inspect:
// - Principals.JWT => invalid (HTTP-only)
// - Principals.Headers => invalid (HTTP-only)
// - Empty/no Authorization is allowed and results in no-op on TCP.
// - Empty/no Authorization is allowed and results in no-op on L4.
// Returns an error when any HTTP-only field is present or CIDRs are invalid.
func validateSecurityPolicyForTCP(p *egv1a1.SecurityPolicy) error {
func validateSecurityPolicyForL4(p *egv1a1.SecurityPolicy, proto string) error {
if p.Spec.CORS != nil || p.Spec.CSRF != nil || p.Spec.JWT != nil || p.Spec.OIDC != nil || p.Spec.APIKeyAuth != nil || p.Spec.BasicAuth != nil || p.Spec.ExtAuth != nil {
return fmt.Errorf("only authorization is supported for TCP (routes/listeners)")
return fmt.Errorf("only authorization is supported for %s (routes/listeners)", proto)
}
if p.Spec.Authorization == nil || len(p.Spec.Authorization.Rules) == 0 {
return nil
}
for i := range p.Spec.Authorization.Rules {
rule := &p.Spec.Authorization.Rules[i]
if rule.CEL != nil {
return fmt.Errorf("rule %d: CEL not supported for TCP", i)
return fmt.Errorf("rule %d: CEL not supported for %s", i, proto)
}
if rule.Principal == nil {
continue
}
if rule.Principal.JWT != nil {
return fmt.Errorf("rule %d: JWT not supported for TCP", i)
return fmt.Errorf("rule %d: JWT not supported for %s", i, proto)
}
if len(rule.Principal.Headers) > 0 {
return fmt.Errorf("rule %d: headers not supported for TCP", i)
return fmt.Errorf("rule %d: headers not supported for %s", i, proto)
}
if len(rule.Principal.ClientIPGeoLocations) > 0 {
return fmt.Errorf("rule %d: clientIPGeoLocations not supported for TCP", i)
return fmt.Errorf("rule %d: clientIPGeoLocations not supported for %s", i, proto)
}
if err := validateCIDRs(rule.Principal.ClientCIDRs); err != nil {
return fmt.Errorf("rule %d: %w", i, err)
Expand All @@ -1006,7 +1012,7 @@ func validateSecurityPolicyForTCP(p *egv1a1.SecurityPolicy) error {
return nil
}

// validateCIDRs validates CIDR strings for TCP authorization rules.
// validateCIDRs validates CIDR strings for L4 authorization rules.
func validateCIDRs(cidrs []egv1a1.CIDR) error {
for _, c := range cidrs {
if _, _, err := net.ParseCIDR(string(c)); err != nil {
Expand Down Expand Up @@ -1385,6 +1391,9 @@ func (t *Translator) translateSecurityPolicyForRoute(
continue
}
tl := xdsIR[irKey].GetTCPListener(irListenerName(listener))
if tl == nil {
continue
}
for _, r := range tl.Routes {
// If target.SectionName is specified it must match the route-rule section name
// in the IR. For HTTP/GRPC routes this is r.Metadata.SectionName; for TCP
Expand All @@ -1403,6 +1412,34 @@ func (t *Translator) translateSecurityPolicyForRoute(
}
}
}
case resource.KindUDPRoute:
for _, listener := range parentRefCtx.listeners {
// If targetListener is set, only apply to that exact listener.
if targetListener != nil && targetListenerName != irListenerName(listener) {
continue
}
ul := xdsIR[irKey].GetUDPListener(irListenerName(listener))
// A UDP listener holds at most one route: when several UDPRoutes name the
// same listener only the oldest one is attached, so a policy targeting any
// of the others has nothing to apply to.
if ul == nil || ul.Route == nil {
continue
}
r := ul.Route
// As with TCP, the route-rule section name lives on the destination metadata.
if target.SectionName != nil && string(*target.SectionName) != r.Destination.Metadata.SectionName {
continue
}

if r.Authorization != nil {
continue
}
// Only authorization for UDP
if authorization != nil {
authCopy := *authorization
r.Authorization = &authCopy
}
}
case resource.KindHTTPRoute, resource.KindGRPCRoute:
var (
hasBaseErrs = errs != nil
Expand Down Expand Up @@ -1695,15 +1732,15 @@ func (t *Translator) translateSecurityPolicyForListeners(
)
}

// Pre-create a TCP-only authorization object to avoid re-allocation
var tcpAuthorization *ir.Authorization
// Pre-create an L4-only authorization object to avoid re-allocation
var l4Authorization *ir.Authorization
if authorization != nil {
authCopy := *authorization
tcpAuthorization = &authCopy
l4Authorization = &authCopy
}

// Apply to TCP listeners (Authorization only).
if tcpAuthorization != nil {
if l4Authorization != nil {
for _, tl := range x.TCP {
if tl == nil || len(tl.Routes) == 0 {
continue
Expand All @@ -1718,8 +1755,27 @@ func (t *Translator) translateSecurityPolicyForListeners(
if r.Authorization != nil {
continue
}
r.Authorization = tcpAuthorization
r.Authorization = l4Authorization
}
}
}

// Apply to UDP listeners (Authorization only).
if l4Authorization != nil {
for _, ul := range x.UDP {
// A UDP listener holds at most one route.
if ul == nil || ul.Route == nil {
continue
}
if !listenerNames.Has(ul.Name) {
continue
}
// A Policy targeting the specific scope(xRoute rule, xRoute, Gateway listener) wins over a policy
// targeting a lesser specific scope(Gateway).
if ul.Route.Authorization != nil {
continue
}
ul.Route.Authorization = l4Authorization
}
}

Expand Down
43 changes: 31 additions & 12 deletions internal/gatewayapi/securitypolicy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1056,7 +1056,7 @@ func SetRouteParentContext(route RouteContext, parentRef gwapiv1.ParentReference
route.SetRouteParentContext(parentRef, ctx)
}

// --- TCP branch: validateSecurityPolicyForTCP(...) returns err -> SetTranslationErrorForPolicyAncestors(...) + return
// --- L4 branch: validateSecurityPolicyForL4(...) returns err -> SetTranslationErrorForPolicyAncestors(...) + return
func Test_SecurityPolicy_TCP_Invalid_setsStatus_and_returns(t *testing.T) {
tr := &Translator{GatewayControllerName: "gateway.envoyproxy.io/gatewayclass-controller"}
trContext := &TranslatorContext{}
Expand Down Expand Up @@ -1208,7 +1208,7 @@ func Test_SecurityPolicy_HTTP_Invalid_setsStatus_and_returns(t *testing.T) {
require.True(t, hasParentFalseCondition(policy))
}

func Test_validateSecurityPolicyForTCP_Table(t *testing.T) {
func Test_validateSecurityPolicyForL4_Table(t *testing.T) {
tests := []struct {
name string
spec egv1a1.SecurityPolicySpec
Expand Down Expand Up @@ -1375,17 +1375,36 @@ func Test_validateSecurityPolicyForTCP_Table(t *testing.T) {
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
p := &egv1a1.SecurityPolicy{Spec: tc.spec}
err := validateSecurityPolicyForTCP(p)
if tc.wantErr {
require.Error(t, err)
} else {
require.NoError(t, err)
}
})
// TCP and UDP share the same rules, so every case must hold for both.
for _, proto := range []string{"TCP", "UDP"} {
for _, tc := range tests {
t.Run(proto+"/"+tc.name, func(t *testing.T) {
p := &egv1a1.SecurityPolicy{Spec: tc.spec}
err := validateSecurityPolicyForL4(p, proto)
if tc.wantErr {
require.Error(t, err)
} else {
require.NoError(t, err)
}
})
}
}

// Rejections that are about the protocol rather than the input name it, so the
// status condition tells the user which listener protocol refused the field.
t.Run("protocol named in protocol-specific errors", func(t *testing.T) {
p := &egv1a1.SecurityPolicy{Spec: egv1a1.SecurityPolicySpec{
Authorization: &egv1a1.Authorization{
Rules: []egv1a1.AuthorizationRule{{
Action: egv1a1.AuthorizationActionAllow,
Principal: &egv1a1.Principal{Headers: []egv1a1.AuthorizationHeaderMatch{{Name: "x-user", Values: []string{"foo"}}}},
}},
},
}}

require.ErrorContains(t, validateSecurityPolicyForL4(p, "TCP"), "headers not supported for TCP")
require.ErrorContains(t, validateSecurityPolicyForL4(p, "UDP"), "headers not supported for UDP")
})
}

func Test_validateAuthorizationGeoIPForHTTP(t *testing.T) {
Expand Down
Loading
Loading