Support Allow/Deny IP Subnets

[arkodg]

Description:

Describe the desired behavior, what scenario it enables and how it
would be used.

As a user, I would like to limit the client IP address to a few subnets for some cases as well as also deny specific subnets

[arkodg]

Envoy supports this https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/rbac/v3/rbac.proto#envoy-v3-api-msg-config-rbac-v3-principal

does this feature belong in ClientTrafficPolicy or SecurityPolicy ?

[tmsnan]

Vote for SecurityPolicy, access control is reasonable as a security policy.

[arkodg]

if the SecurityPolicy with ip subnet info is applied at the Gateway level, it can be overridden at the route level if another SecurityPolicy is applied at route level

[tmsnan]

Is it possible to add a disclaimer stating that the use of routing override is not recommended?

[zhaohuabing]

Should RBAC #2250 be the right home for this?

[zhaohuabing]

https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/rbac/v3/rbac.proto#envoy-v3-api-msg-config-rbac-v3-principal

if the SecurityPolicy with ip subnet info is applied at the Gateway level, it can be overridden at the route level if another SecurityPolicy is applied at route level

I didn't get it. A route level SecurityPolicy without Allow/Deny IP setting won't override the Allow/Deny IP setting in Gateway level. "Allow/Deny IP Subnets" should be only configured at the gateway level?

gateway/internal/gatewayapi/securitypolicy.go

Lines 359 to 374 in 95f4c10

	for _, http := range ir.HTTP {
	for _, r := range http.Routes {
	// Apply if not already set
	if r.CORS == nil {
	r.CORS = cors
	}
	if r.JWT == nil {
	r.JWT = jwt
	}
	if r.OIDC == nil {
	r.OIDC = oidc
	}
	if r.BasicAuth == nil {
	r.BasicAuth = basicAuth
	}
	}

[zetaab]

well at least I have use case that same gateway should be apple to serve clients from the internet, but some routes need to have allow/deny ip setting.

[arkodg]

@zhaohuabing A route level SecurityPolicy without Allow/Deny IP setting will override the Allow/Deny IP setting in Gateway level, until we implement #1934
Can you share why rbac is a better home ? If we took that approach, config would look like

jwt:
   .........
acl:
   jwt:
     claims: ......   

vs

jwt:
  .....
  claims: .......

[arkodg]

thanks for sharing your use case @zetaab, based on community feedback sounds like SecurityPolicy is the way to go here

[zhaohuabing]

@zhaohuabing A route level SecurityPolicy without Allow/Deny IP setting will override the Allow/Deny IP setting in Gateway level, until we implement #1934

This may not be intended, but the current implementation of SecurityPolicy is actually a Merge logic. For example, if a route level SecurityPolicy doesn't has OIDC configuration, but a Gateway level SecurityPolicy has, the Gateway level OIDC applies. As the below code shows:

gateway/internal/gatewayapi/securitypolicy.go

Lines 359 to 374 in 95f4c10

	for _, http := range ir.HTTP {
	for _, r := range http.Routes {
	// Apply if not already set
	if r.CORS == nil {
	r.CORS = cors
	}
	if r.JWT == nil {
	r.JWT = jwt
	}
	if r.OIDC == nil {
	r.OIDC = oidc
	}
	if r.BasicAuth == nil {
	r.BasicAuth = basicAuth
	}
	}

[arkodg]

@zhaohuabing you are right, this needs to be fixed, being tracked with #2055 which needs improvements in this logic

gateway/internal/gatewayapi/securitypolicy.go

Line 360 in 100d310

for _, r := range http.Routes {

[zinuga]

@arkodg so this wont be in GA release?

[zetaab]

@zinuga I have been implementing the api design for this in #2652 its pretty fast implement after the api design is accepted.

[arkodg]

thanks for driving this @zetaab, the API is close to getting merged, if the implementation does complete by rc (March 1st-4th), we should be able to support it in v1.0

[zirain]

#2652

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days.

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days.