rbac: support match based on destination cluster

[lizan]

Description:
Envoy can route traffic to clusters based on external given input, e.g. cluster_header in http_conn_manager, and there will be similar proposal for TCP in #4444. We should disallow traffic from listeners going to certain clusters, e.g. xDS API cluster (especially SDS), ratelimit cluster, ext_authz cluster etc.

cc @rshriram

Relevant Links:
#4444 (comment)

[htuch]

@lizan is the more general way to solve this to provide a list of allowed routable clusters (or blacklist) in the RouteConfiguration? I.e. is this more a policy issue best addressed in the control plane, where you would control beyond "private/internal" marking at the cluster level.

[lizan]

The more general way to solve this to provide a list of allowed routable clusters (or blacklist) in the RouteConfiguration?

The general allow/deny list is already achievable with RBAC filter for both TCP and HTTP. Making a cluster private/internal is just an additional way to prevent leaking sensitive information from certain clusters (especially SDS) that you would never want traffic routed to.

[mattklein123]

In general I would also prefer that we handle this in the RBAC filters. Is that what is being proposed here?

[lizan]

@mattklein123 No, I'm proposing add a flag to cluster level that preventing traffic not generated by Envoy itself being routed to, without relying RBAC to be configured on every listeners/HCM. It is non-trivial to configure every listener/HCM with RBAC correctly to avoid external traffic to a specific cluster.

[mattklein123]

It is non-trivial to configure every listener/HCM with RBAC correctly to avoid external traffic to a specific cluster.

To play devils advocate, why? This seems like an RBAC problem? Can't we have an RBAC rule to deny traffic that is destined for a particular route? It seems pretty straightforward to me?

In general I would prefer to not add special cases to the router for things like this that IMO should be handled by RBAC?

[lizan]

We don't have a mechanism to enforce RBAC globally across all listeners/HCMs? If there is a way to do that I'm fine with that too.

[mattklein123]

We don't have a mechanism to enforce RBAC globally across all listeners/HCMs? If there is a way to do that I'm fine with that too.

Install the filter on every listener?

[lizan]

Install the filter on every listener?

Yeah sure it works, but IMO it is too much for just disallowing traffic to a certain clusters. (Note clusters might have credentials configured with it.) In HCM, the RBAC filter comes in front of router, so it need to be configured to match what cluster_header configured in router or delivered by RDS.

It is technically possible to let control planes to figure out all of this and install RBAC filter (perhaps merge existing RBAC rules) accordingly. Though in most cases only certain clusters are private for internal use, there should be a fail-safe flag on cluster side for security in Envoy core, instead of having all logics above to compute RBAC rules. Thoughts?

[mattklein123]

It is technically possible to let control planes to figure out all of this and install RBAC filter (perhaps merge existing RBAC rules) accordingly. Though in most cases only certain clusters are private for internal use, there should be a fail-safe flag on cluster side for security in Envoy core, instead of having all logics above to compute RBAC rules. Thoughts?

I see your point. Where I am mainly coming from is it's not clear to me that we should have multiple ways of doing something unless it is really necessary. This feels like an RBAC problem that we should solve with the RBAC filter, even if the management server needs to machine generate more config. @envoyproxy/maintainers anyone have any thoughts on this?

[ggreenway]

I tend to agree with @mattklein123. I think my objection is that this change assumes there are only 2 classes of clusters. What if I have 3 classes instead, which are "routable from listener1 via header", "routable from listener2 via header", and "envoy-internal (xDS, etc)"?

[lizan]

@ggreenway this is only for routable or not, more than that should be RBAC.

Another point: this is also the difference between GoogleGrpc and EnvoyGrpc. xDS configured by REST or EnvoyGrpc has the risk that a unintended traffic could be routed to by misconfiguration, while GoogleGrpc is not routable at all.

[rshriram]

to throw to this mix, consider the case where the cluster name is taken from the SNI value? RBAC gets more complicated here as well.

[htuch]

My earlier suggestion is really a preference that we put mechanism in Envoy and the policy in the management server. I think the question is how complicated is the implementation of a general whitelist/blacklist? If it's going to be O(100) lines of code, I think its worth making it general, any more complicated and yeah, sure, just a non-routable tag would make sense.

An early API design principle is that the protos are intended to be machine generated, so we can err away from brevity here.

[lizan]

With current implementations, it is tricky for control-planes to generate, especially for cluster_header in HTTP with the filter order (per route config cannot have multiple configs for same filter) and route cache if filters in middle set that header. If we have upstream filter #173 and RBAC there, I agree that the rule would be pretty simple enough to generate so we don't need this flag.

[mattklein123]

@lizan sorry I still don't understand why this is hard to do with the existing RBAC filter. Can you describe your use case in more detail?

[stale]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

[lizan]

Sorry I didn't have a chance to revisit this. On a second thought, with current RBAC filter we at least need add destination cluster rule to make this easy. With that it will be fairly easy to setup RBAC rules to prevent traffic to certain cluster, without complexing RBAC rules due to route configuration etc.

[mattklein123]

On a second thought, with current RBAC filter we at least need add destination cluster rule to make this easy.

Agreed. This is generally useful. Please add!

[stale]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

[yangminzhu]

@lizan
Is the rule to be added to the RBAC policy just a string of the cluster name?

[lizan]

@yangminzhu yeah adding a rule to RBAC policy would be good.

[rwaweber]

Hey all, sorry to dust off an old issue!
I could be wrong, but it doesn't look like this made it into the rbac filter. Given its been a few years, this very well may not be the best place to put this anymore, but I would love to be able to use this feature to be able to leverage the rich RBAC logic to pass a request directly to a destination cluster. Happy to help if I can!