-
Couldn't load subscription status.
- Fork 579
feat(securitypolicy): Update GatewayAPI to support tcp security policy #7171
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 31 commits
9ed4c7b
76c8c9e
c574ec1
1b86ebf
9d7efb3
a28a214
b1a5ba8
b7838a8
414cf38
2e13b1e
da8909e
96cbe84
43fe55a
52ac1fa
b61caa6
f0520a4
220b334
45e57b4
6bc7270
f78696c
03970e1
8c0649e
67814ea
36a928d
3ffcae5
253d5f6
07a37dc
9bd4d86
25ff0a4
5c9694e
f08d348
5156577
f0e49e1
32aa8c2
8a68216
6753bca
a56cc92
84a79cc
b58186c
a989171
7e601e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,7 +94,7 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security | |
| // 3. Then translate Policies targeting Listeners | ||
| // 4. Finally, the policies targeting Gateways | ||
|
|
||
| // Process the policies targeting RouteRules | ||
| // Process the policies targeting RouteRules (HTTP + TCP) | ||
| for _, currPolicy := range securityPolicies { | ||
| policyName := utils.NamespacedName(currPolicy) | ||
| targetRefs := getPolicyTargetRefs(currPolicy.Spec.PolicyTargetReferences, routes, currPolicy.Namespace) | ||
|
|
@@ -107,12 +107,12 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security | |
| res = append(res, policy) | ||
| } | ||
|
|
||
| t.processSecurityPolicyForHTTPRoute(resources, xdsIR, | ||
| t.processSecurityPolicyForRoute(resources, xdsIR, | ||
| routeMap, gatewayRouteMap, policy, currTarget) | ||
| } | ||
| } | ||
| } | ||
| // Process the policies targeting xRoutes | ||
| // Process the policies targeting xRoutes (HTTP + TCP) | ||
| for _, currPolicy := range securityPolicies { | ||
| policyName := utils.NamespacedName(currPolicy) | ||
| targetRefs := getPolicyTargetRefs(currPolicy.Spec.PolicyTargetReferences, routes, currPolicy.Namespace) | ||
|
|
@@ -125,7 +125,7 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security | |
| res = append(res, policy) | ||
| } | ||
|
|
||
| t.processSecurityPolicyForHTTPRoute(resources, xdsIR, | ||
| t.processSecurityPolicyForRoute(resources, xdsIR, | ||
| routeMap, gatewayRouteMap, policy, currTarget) | ||
| } | ||
| } | ||
|
|
@@ -176,7 +176,7 @@ func (t *Translator) ProcessSecurityPolicies(securityPolicies []*egv1a1.Security | |
| return res | ||
| } | ||
|
|
||
| func (t *Translator) processSecurityPolicyForHTTPRoute( | ||
| func (t *Translator) processSecurityPolicyForRoute( | ||
| resources *resource.Resources, | ||
| xdsIR resource.XdsIRMap, | ||
| routeMap map[policyTargetRouteKey]*policyRouteTargetContext, | ||
|
|
@@ -244,12 +244,20 @@ func (t *Translator) processSecurityPolicyForHTTPRoute( | |
| return | ||
| } | ||
|
|
||
| if err := validateSecurityPolicy(policy); err != nil { | ||
| // Protocol-specific validation: pick the appropriate validator and message, | ||
| // 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 | ||
| errMsg = "invalid SecurityPolicy for TCP route" | ||
| } | ||
| if err := validator(policy); err != nil { | ||
| status.SetTranslationErrorForPolicyAncestors(&policy.Status, | ||
| parentGateways, | ||
| t.GatewayControllerName, | ||
| policy.Generation, | ||
| status.Error2ConditionMsg(fmt.Errorf("invalid SecurityPolicy: %w", err)), | ||
| status.Error2ConditionMsg(fmt.Errorf("%s: %w", errMsg, err)), | ||
| ) | ||
|
|
||
| return | ||
|
|
@@ -392,6 +400,44 @@ func validateSecurityPolicy(p *egv1a1.SecurityPolicy) error { | |
| return nil | ||
| } | ||
|
|
||
| // validateSecurityPolicyForTCP ensures SecurityPolicy usage on TCP is compatible. | ||
| // | ||
| // TCP supports Authorization with ClientCIDRs ONLY. | ||
| // - Principals.JWT => invalid (HTTP-only) | ||
| // - Principals.Headers => invalid (HTTP-only) | ||
| // - Empty/no Authorization is allowed and results in no-op on TCP. | ||
| // Returns an error when any HTTP-only field is present or CIDRs are invalid. | ||
| func validateSecurityPolicyForTCP(p *egv1a1.SecurityPolicy) error { | ||
| if p.Spec.CORS != 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)") | ||
| } | ||
| if p.Spec.Authorization == nil || len(p.Spec.Authorization.Rules) == 0 { | ||
| return nil | ||
| } | ||
| for i, rule := range p.Spec.Authorization.Rules { | ||
| if rule.Principal.JWT != nil { | ||
| return fmt.Errorf("rule %d: JWT not supported for TCP", i) | ||
| } | ||
| if len(rule.Principal.Headers) > 0 { | ||
| return fmt.Errorf("rule %d: headers not supported for TCP", i) | ||
| } | ||
| if err := validateCIDRs(rule.Principal.ClientCIDRs); err != nil { | ||
| return fmt.Errorf("rule %d: %w", i, err) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| // validateCIDRs validates CIDR strings for TCP authorization rules. | ||
| func validateCIDRs(cidrs []egv1a1.CIDR) error { | ||
| for _, c := range cidrs { | ||
| if _, _, err := net.ParseCIDR(string(c)); err != nil { | ||
| return fmt.Errorf("invalid ClientCIDR %q: %w", c, err) | ||
| } | ||
| } | ||
| return nil | ||
| } | ||
|
|
||
| func validateAPIKeyAuth(apiKeyAuth *egv1a1.APIKeyAuth) error { | ||
| for _, keySource := range apiKeyAuth.ExtractFrom { | ||
| // only one of headers, params or cookies is supposed to be specified. | ||
|
|
@@ -657,10 +703,46 @@ func (t *Translator) translateSecurityPolicyForRoute( | |
| } | ||
|
|
||
| irKey := t.getIRKey(gtwCtx.Gateway) | ||
| for _, listener := range parentRefCtx.listeners { | ||
| irListener := xdsIR[irKey].GetHTTPListener(irListenerName(listener)) | ||
| if irListener != nil { | ||
| switch route.GetRouteType() { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. curious why this code is needed when its NA for TCPRoute, which is checked in validateSecurityPolicyForTCP There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. which part are you referring to, why we do switch.route? TCP is applied differently with then there's this line if target.SectionName != nil && string(*target.SectionName) != r.Destination.Metadata.SectionName {
continue
}```
vs this one for httproute
```go
if target.SectionName != nil && string(*target.SectionName) != r.Metadata.SectionName {
continue
}those have to be different. let me see if i can reduce the difference |
||
| case resource.KindTCPRoute: | ||
| // Only client-IP Authorization is applicable for TCP routes. | ||
| // TCP IR route names are flat. The computed prefix includes a trailing | ||
| // '/' (e.g. "tcproute/default/tcp-app-2/"), so trim the suffix to get | ||
| // the exact TCP route name used in the IR: | ||
| // prefix == "tcproute/default/tcp-app-2/" -> expectedTCPRouteName == "tcproute/default/tcp-app-2" | ||
| expectedTCPRouteName := strings.TrimSuffix(prefix, "/") | ||
| for _, listener := range parentRefCtx.listeners { | ||
| tl := xdsIR[irKey].GetTCPListener(irListenerName(listener)) | ||
| if tl == nil { | ||
| continue | ||
| } | ||
| for _, r := range tl.Routes { | ||
| if r == nil { | ||
| continue | ||
| } | ||
davem-git marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // if already set - there's a specific level policy, so skip. | ||
| if r.Authorization != nil { | ||
| continue | ||
| } | ||
| // exact match only (TCP route names are flat in IR) | ||
| if r.Name != expectedTCPRouteName { | ||
| continue | ||
| } | ||
| // Only authorization for TCP | ||
| authCopy := *authorization | ||
| r.Authorization = &authCopy | ||
| } | ||
| } | ||
| case resource.KindHTTPRoute, resource.KindGRPCRoute: | ||
| for _, listener := range parentRefCtx.listeners { | ||
| irListener := xdsIR[irKey].GetHTTPListener(irListenerName(listener)) | ||
| if irListener == nil { | ||
| continue | ||
| } | ||
| for _, r := range irListener.Routes { | ||
| if r == nil { | ||
| continue | ||
| } | ||
| // If specified the sectionName must match route rule from ir route metadata. | ||
| if target.SectionName != nil && string(*target.SectionName) != r.Metadata.SectionName { | ||
| continue | ||
|
|
@@ -828,6 +910,42 @@ func (t *Translator) translateSecurityPolicyForGateway( | |
| } | ||
| } | ||
| } | ||
|
|
||
| // Pre-create a TCP-only authorization object to avoid re-allocation | ||
| var tcpAuthorization *ir.Authorization | ||
| if authorization != nil { | ||
| authCopy := *authorization | ||
| tcpAuthorization = &authCopy | ||
| } | ||
|
|
||
| // Apply to TCP listeners (Authorization only). Support metadata nil fallback by parsing section name from listener name suffix. | ||
davem-git marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if tcpAuthorization != nil { | ||
| for _, tl := range x.TCP { | ||
| if tl == nil || len(tl.Routes) == 0 { | ||
| continue | ||
| } | ||
| // TCPListener name has same format namespace/gatewayName/listenerName | ||
| gatewayNameEnd := strings.LastIndex(tl.Name, "/") | ||
| gatewayName := tl.Name[0:gatewayNameEnd] | ||
| if t.MergeGateways && gatewayName != policyTarget { | ||
| continue | ||
| } | ||
| // If specified the sectionName must match listenerName from ir listener metadata. | ||
| if target.SectionName != nil && string(*target.SectionName) != tl.Metadata.SectionName { | ||
| continue | ||
| } | ||
| // A Policy targeting the specific scope(xRoute rule, xRoute, Gateway listener) wins over a policy | ||
| // targeting a lesser specific scope(Gateway). | ||
| for _, r := range tl.Routes { | ||
| // if already set - there's a specific level policy, so skip. | ||
| if r.Authorization != nil { | ||
| continue | ||
| } | ||
| r.Authorization = tcpAuthorization | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return errs | ||
| } | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.