feat(translator): implement TCP security policy support by davem-git · Pull Request #6496 · envoyproxy/gateway

davem-git · 2025-07-09T20:50:58Z

feat(SecurityPolicies): add new featureAdding Security Policies for TCPGateways

adding security policies to TCPGateways using the existing API's

What this PR does / why we need it:

This PR will allow us to run authorization security policies only on TCPGateways
Which issue(s) this PR fixes:

Fixes # #4908

Release Notes: Yes/No

davem-git · 2025-07-09T20:52:13Z

This now works. I added. a security policy to the tcp example and you can verify getting rejected from that policy

api/v1alpha1/envoyproxy_types.go

internal/ir/xds.go

internal/gatewayapi/securitypolicy.go

davem-git · 2025-07-11T21:19:18Z

didn't rebase cleanly trying that again

internal/gatewayapi/securitypolicy.go

internal/ir/xds.go

internal/xds/translator/listener.go

zhaohuabing

Can we add an e2e test to verify this works?

davem-git · 2025-07-16T18:21:20Z

@zhaohuabing Running these e2e tests seems to fail on static-file-server, it doesn't have an image in docker. is there away to run them to just run my new tests?

davem-git · 2025-07-21T14:58:50Z

@arkodg any advice Running these e2e tests seems to fail on static-file-server, it doesn't have an image in docker. is there away to run them to just run my new tests?

arkodg · 2025-07-21T15:05:43Z

@davem-git recommend trying out these commands locally, make help has a lot more suggestions

make build
make test
make e2e
make generate && make manifests
make testdata

arkodg · 2025-07-21T15:06:14Z

also suggest rebasing to main, which make it easier to run e2e tests

davem-git · 2025-09-23T20:52:17Z

add some release note and updated documentation

…y-policy-features

…ateway into tcp-security-policy-features

Signed-off-by: davem-git <[email protected]>

davem-git · 2025-09-24T17:39:50Z

I was able merge in the changes from main. please review it. I also noticed that these test that are failing, seem to fail on other branches https://github.com/envoyproxy/gateway/pull/6851/commits but were merged in anyways.

Signed-off-by: davem-git <[email protected]>

davem-git · 2025-09-26T20:13:04Z

@arkodg any update on this?

arkodg · 2025-09-26T20:39:30Z

hey @davem-git the complexity this PR introduces is still very high, are there ways we can make the logic more readable ?

for e.g. can we pass individual routes in

gateway/internal/gatewayapi/translator.go

Line 146 in 8fdebee

for _, tcpRoute := range tcpRoutes {

to avoid the if TCP logic inside the policy

davem-git · 2025-09-26T21:07:51Z

Will do, any other comments? I've had this open a while with little feedback. The policies apply and work as expected, without input from your end this PR has dragged out for months.

arkodg · 2025-09-26T21:16:21Z

hey @davem-git lets collab together to get this into v1.6, if youre on the Envoy Slack lets start a convo there (my handle is on slack is @ArkoDasgupta) ?

Signed-off-by: davem-git <[email protected]>

zhaohuabing · 2025-10-09T08:21:43Z

api/v1alpha1/securitypolicy_types.go

+// +kubebuilder:validation:XValidation:rule="has(self.targetRef) ? self.targetRef.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute', 'TCPRoute'] : true", message="this policy can only have a targetRef.kind of Gateway/HTTPRoute/GRPCRoute/TCPRoute"
 // +kubebuilder:validation:XValidation:rule="has(self.targetRefs) ? self.targetRefs.all(ref, ref.group == 'gateway.networking.k8s.io') : true ", message="this policy can only have a targetRefs[*].group of gateway.networking.k8s.io"
-// +kubebuilder:validation:XValidation:rule="has(self.targetRefs) ? self.targetRefs.all(ref, ref.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute']) : true ", message="this policy can only have a targetRefs[*].kind of Gateway/HTTPRoute/GRPCRoute"
+// +kubebuilder:validation:XValidation:rule="has(self.targetRefs) ? self.targetRefs.all(ref, ref.kind in ['Gateway', 'HTTPRoute', 'GRPCRoute', 'TCPRoute']) : true ", message="this policy can only have a targetRefs[*].kind of Gateway/HTTPRoute/GRPCRoute/TCPRoute"


As mentioned in https://github.com/envoyproxy/gateway/pull/6496/files#r2415900704, can we add a comment saying that only client-IP authorization will be applied to TCPRoute?

We also need to update the comment of the ClientCIDRs field on how to get client IP from TCP requests.

I updated this on #7168. @arkodg suggested breaking this up in to 5 PR's

zhaohuabing · 2025-10-09T08:40:52Z

internal/xds/translator/listener.go

 	return nil
 }

+func buildTCPRBACMatcherFromRules(rules []*ir.AuthorizationRule, defaultAction egv1a1.AuthorizationAction) *rbacconfig.RBAC {


Nit: can we move all the authorization-related methods to a standalone file tcp_authorization.go?

zhaohuabing · 2025-10-09T08:47:55Z

internal/gatewayapi/securitypolicy.go

+		if len(rule.Principal.Headers) > 0 {
+			return fmt.Errorf("rule %d: headers not supported for TCP", i)
+		}
+		if rule.Action == egv1a1.AuthorizationActionAllow && len(rule.Principal.ClientCIDRs) == 0 {


What happens when rule.Action == egv1a1.AuthorizationActionDeny && len(rule.Principal.ClientCIDRs) == 0 ?

I think it defaults to deny, and blocks everything Do we want it to do that? I know we wanted to allow placeholders for security policy, I would think that would be fine if they just didn't add the authorization to the security policy

i guess it doesn't matter. When i try this

apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy metadata: namespace: default name: tcp-app-2 spec: targetRef: group: gateway.networking.k8s.io kind: TCPRoute name: tcp-app-2 authorization: defaultAction: Allow

or

apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy metadata: namespace: default name: tcp-app-2 spec: targetRef: group: gateway.networking.k8s.io kind: TCPRoute name: tcp-app-2 authorization: defaultAction: Deny

Both are accepted and do nothing

HTTP/1.1 200 OK Content-Type: application/json X-Content-Type-Options: nosniff Date: Thu, 09 Oct 2025 20:02:19 GMT Content-Length: 189 { "path": "/", "host": "backend", "method": "GET", "proto": "HTTP/1.1", "headers": {}, "namespace": "default", "ingress": "", "service": "bar", "pod": "backend-2-958dbb978-ffncm" }%

When it try this

apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy metadata: namespace: default name: tcp-app-2 spec: targetRef: group: gateway.networking.k8s.io kind: TCPRoute name: tcp-app-2 authorization: defaultAction: Deny rules: - action: Allow principal: clientCIDRs:

or this

apiVersion: gateway.envoyproxy.io/v1alpha1 kind: SecurityPolicy metadata: namespace: default name: tcp-app-2 spec: targetRef: group: gateway.networking.k8s.io kind: TCPRoute name: tcp-app-2 authorization: defaultAction: Allow rules: - action: Deny principal: clientCIDRs:

I get this

The SecurityPolicy "tcp-app-2" is invalid: spec.authorization.rules[0].principal: Invalid value: "object": at least one of clientCIDRs, jwt, or headers must be specified

so I don't think its ever hit, i'll remove it and test again

zhaohuabing · 2025-10-09T09:21:27Z

internal/gatewayapi/securitypolicy.go

+			if routeIsTCP {
+				if tl := xdsIR[irKey].GetTCPListener(irListenerName(listener)); tl != nil {
+					for _, r := range tl.Routes {
+						if r == nil || r.Security != nil {


nit: I don't think r would be nil

Suggested change

if r == nil || r.Security != nil {

if r.Security != nil {

thanks added #7168

zhaohuabing · 2025-10-09T09:34:25Z

internal/gatewayapi/securitypolicy.go

+					// Nothing else to do for this listener in TCP mode
+					continue
+				}
+			}


Should we just return here?

Maybe we can use a switch to make the logic easier to read:

switch route.GetRouteType() { case resource.KindTCPRoute: for _, listener := range parentRefCtx.listeners { if tl := xdsIR[irKey].GetTCPListener(irListenerName(listener)); tl != nil { for _, r := range tl.Routes { if r.Name != expectedTCPRouteName { // exact match only continue } // if already set - there's a specific level policy, so skip. if r.Security != nil { continue } // Only authorization for TCP r.Security = &ir.SecurityFeatures{Authorization: authorization} } // Nothing else to do for this listener in TCP mode continue } } case resource.KindHTTPRoute, resource.KindGRPCRoute: // The original HTTP/GRPC route logic }

I think i got this. Can you please review it again? #7168

zhaohuabing · 2025-10-09T09:36:55Z

internal/gatewayapi/securitypolicy.go

+	var tcpSecurityFeatures *ir.SecurityFeatures
+	if authorization != nil {
+		tcpSecurityFeatures = &ir.SecurityFeatures{Authorization: authorization}
+	}


Nit: move this code block right before line 915.

moved, please see other pr, trying to keep them in sync to help with the review

…y-policy-features

… PR's Signed-off-by: davem-git <[email protected]>

…ateway into tcp-security-policy-features

…cy.go Signed-off-by: davem-git <[email protected]>

…y-policy-features

davem-git requested a review from a team as a code owner July 9, 2025 20:50

davem-git mentioned this pull request Jul 9, 2025

security policies for tcp gateways not working #4908

Closed

davem-git force-pushed the tcp-security-policy-features branch from 87ca72a to 5a4378c Compare July 9, 2025 21:06

arkodg requested a review from zhaohuabing July 9, 2025 22:12

arkodg added this to the v1.5.0-rc.1 Release milestone Jul 9, 2025

zhaohuabing reviewed Jul 10, 2025

View reviewed changes

api/v1alpha1/envoyproxy_types.go Show resolved Hide resolved

davem-git force-pushed the tcp-security-policy-features branch 4 times, most recently from e6dbbc0 to 94ffdf1 Compare July 10, 2025 19:26

arkodg reviewed Jul 10, 2025

View reviewed changes

internal/ir/xds.go Outdated Show resolved Hide resolved

arkodg reviewed Jul 10, 2025

View reviewed changes

internal/ir/xds.go Outdated Show resolved Hide resolved

arkodg reviewed Jul 10, 2025

View reviewed changes

internal/gatewayapi/securitypolicy.go Outdated Show resolved Hide resolved

arkodg reviewed Jul 10, 2025

View reviewed changes

internal/gatewayapi/securitypolicy.go Outdated Show resolved Hide resolved

davem-git force-pushed the tcp-security-policy-features branch from 899bbdb to 7f8c58f Compare July 11, 2025 21:15

davem-git force-pushed the tcp-security-policy-features branch from 925009c to 42ffe56 Compare July 11, 2025 21:22

arkodg requested a review from zhaohuabing July 11, 2025 21:24

davem-git force-pushed the tcp-security-policy-features branch from ec62e0e to afa5c43 Compare July 11, 2025 21:25

zhaohuabing reviewed Jul 12, 2025

View reviewed changes

internal/gatewayapi/securitypolicy.go Outdated Show resolved Hide resolved

zhaohuabing reviewed Jul 12, 2025

View reviewed changes

internal/ir/xds.go Outdated Show resolved Hide resolved

zhaohuabing reviewed Jul 12, 2025

View reviewed changes

internal/xds/translator/listener.go Outdated Show resolved Hide resolved

zhaohuabing requested changes Jul 12, 2025

View reviewed changes

davem-git force-pushed the tcp-security-policy-features branch 2 times, most recently from 52ff275 to 86bd7b4 Compare July 16, 2025 21:53

davem-git added 4 commits September 23, 2025 15:52

Merge branch 'main' of github.com:envoyproxy/gateway into tcp-securit…

b423031

…y-policy-features

Merge branch 'tcp-security-policy-features' of github.com:davem-git/g…

4b8050a

…ateway into tcp-security-policy-features

fixed merge conflict

f89fb97

Signed-off-by: davem-git <[email protected]>

reset securitypolicy.go from main and added my changes to it

f4bf5ba

Signed-off-by: davem-git <[email protected]>

davem-git added 4 commits September 25, 2025 11:29

Merge branch 'main' into tcp-security-policy-features

d1616f2

fixed error in test

e9d6b24

Signed-off-by: davem-git <[email protected]>

Merge branch 'main' into tcp-security-policy-features

4bf52f4

Merge branch 'main' into tcp-security-policy-features

7ef6b51

davem-git added 3 commits October 8, 2025 10:43

rebase

470f778

Signed-off-by: davem-git <[email protected]>

rebases ran lint and test data

72f9eb5

Signed-off-by: davem-git <[email protected]>

Merge branch 'main' into tcp-security-policy-features

719706e

zhaohuabing reviewed Oct 9, 2025

View reviewed changes

This was referenced Oct 9, 2025

feat(securitypolicy): Update GatewayAPI to support tcp security policy #7171

Merged

feat(securitypolicy): Update API to support tcp security policy #7168

Merged

davem-git added 6 commits October 9, 2025 14:21

Merge branch 'main' of github.com:envoyproxy/gateway into tcp-securit…

9e868b4

…y-policy-features

feat(securitypolicy): trying to keep this branch in sync the split up…

8689ef1

… PR's Signed-off-by: davem-git <[email protected]>

Merge branch 'tcp-security-policy-features' of github.com:davem-git/g…

3c7cc20

…ateway into tcp-security-policy-features

feat(securitypolicy): format and removed unneded line in securitypoli…

4d31121

…cy.go Signed-off-by: davem-git <[email protected]>

Merge branch 'main' of github.com:envoyproxy/gateway into tcp-securit…

5fb23bf

…y-policy-features

Merge branch 'main' of github.com:envoyproxy/gateway into tcp-securit…

649b648

…y-policy-features

zhaohuabing closed this Oct 20, 2025

Conversation

davem-git commented Jul 9, 2025

Uh oh!

davem-git commented Jul 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davem-git commented Jul 11, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zhaohuabing left a comment

Choose a reason for hiding this comment

Uh oh!

davem-git commented Jul 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davem-git commented Jul 21, 2025

Uh oh!

arkodg commented Jul 21, 2025

Uh oh!

arkodg commented Jul 21, 2025

Uh oh!

davem-git commented Sep 23, 2025

Uh oh!

davem-git commented Sep 24, 2025

Uh oh!

davem-git commented Sep 26, 2025

Uh oh!

arkodg commented Sep 26, 2025

Uh oh!

davem-git commented Sep 26, 2025

Uh oh!

arkodg commented Sep 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhaohuabing Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davem-git Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davem-git Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zhaohuabing Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

davem-git commented Jul 16, 2025 •

edited

Loading

zhaohuabing Oct 9, 2025 •

edited

Loading

davem-git Oct 9, 2025 •

edited

Loading

davem-git Oct 9, 2025 •

edited

Loading

zhaohuabing Oct 9, 2025 •

edited

Loading