NE-1112: Add Security Policy for Gateway API #1741
Conversation
Update the Gateway API proposal to include the security policy as well as a new section on Gateway Topology.
openshift-ci-robot
added
the
jira/valid-reference
|
@gcs278: This pull request references NE-1112 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the epic to target the "4.19.0" version, but no target version was set. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
/assign @shaneutt |
|
/assign @Thealisyed |
|
Inactive enhancement proposals go stale after 28d of inactivity. See https://github.com/openshift/enhancements#life-cycle for details. Mark the proposal as fresh by commenting If this proposal is safe to close now please do so with /lifecycle stale |
openshift-ci
bot
added
the
lifecycle/stale
|
/remove-lifecycle stale |
openshift-ci
bot
removed
the
lifecycle/stale
| With shared gateway topology, a single load balancer type service and proxy deployment is created and serves | ||
| routes across multiple namespaces. The Gateway must allow the desired application namespaces via the | ||
| `spec.listeners.allowedRoutes.namespaces` field. This topology mirrors the existing OpenShift router topology and | ||
| is ideal for users that are cost sensitive and do not have strict performance or security requirements. |
There was a problem hiding this comment.
nit: It should probably be made clear why this is topology is potentially cheaper and/or slower or less secure. Otherwise, it leaves the question rather open-ended.
There was a problem hiding this comment.
It is cost-effective as multiple users share the same infrastructure which helps distributing expenses. However, this can lead to slower performance since resources like processing power and bandwidth are divided among users potentially causing congestion. Additionally the shared gateways pose security risks as a compromise of one user’s data could affect others due to the lack of isolation in the shared environment.
There was a problem hiding this comment.
On Gateway API gateways, which are virtual, this is not evident. Unless you have a reference, I think we need to remove the claim that this topology is cheaper, slower, or less secure.
There was a problem hiding this comment.
I know my comment is a little late, but to clarify, most of the cost has to do with the load balancer and associated cloud resources. For example, if you're on AWS, then using a shared gateway means you pay for 1 ELB and 1 Route 53 DNS record. In contrast, if you need to create multiple gateways, you end up paying AWS for multiple ELBs and multiple DNS records. (Technically, we typically create 2 DNS records per ELB: one in the private hosted zone and one on the public hosted zone. Anyway, the point about cost stands.)
There was a problem hiding this comment.
Adding an example like that would be fine.
| This cross-namespace topology can only be implemented using the Gateway-to-xRoute hierarchy, as Istio does not support | ||
| cross-namespace Gateway merging. Therefore, a limitation of this topology is that xRoutes lack an API for | ||
| specifying TLS certificates, requiring all application certificates to be managed within a cluster-wide Gateway. |
There was a problem hiding this comment.
Remove this paragraph. It's not clear what Gateway-to-xRoute hierarchy is, nor why we bring up cross-namespace gateway merging in this section. We can definitely specify certificates for HTTPRoutes.
| This cross-namespace topology can only be implemented using the Gateway-to-xRoute hierarchy, as Istio does not support | |
| cross-namespace Gateway merging. Therefore, a limitation of this topology is that xRoutes lack an API for | |
| specifying TLS certificates, requiring all application certificates to be managed within a cluster-wide Gateway. |
|
|
||
| With a dedicated gateway topology, a load balancer type service and proxy deployment is created to serve the xRoutes | ||
| in a dedicated namespace. This approach is more suitable for multi-tenant environments or applications that require a | ||
| dedicated gateway to meet specific security or performance requirements. |
There was a problem hiding this comment.
As far as I know, we don't really have any data that says dedicated gateways are more secure or better performing, so it's best not to mention that here or in our docs.
There was a problem hiding this comment.
I think the security comes from restricting who can use the certificates on a gateway and from having a dedicated proxy and load-balancer for each gateway.
| Gateway[Gateway] -.-> ProxyDeployment(Proxy Deployment) | ||
| Gateway[Gateway] -.-> LBService(LB Service) |
There was a problem hiding this comment.
Same issue as the other diagram.
| Gateway[Gateway] -.-> ProxyDeployment(Proxy Deployment) | ||
| Gateway[Gateway] -.-> LBService(LB Service) |
There was a problem hiding this comment.
This diagram is also a little murky, as it breaks the abstraction at the Proxy Deployment and LB Service level. These two objects can be replaced by Service, as the Deployment is not a user-managed object. There can also be multiple Services.
| Gateway[Gateway] -.-> ProxyDeployment(Proxy Deployment) | |
| Gateway[Gateway] -.-> LBService(LB Service) | |
| Gateway[Gateway] -.-> Service(Service) |
| Gateway API upstream is still defining guidelines for DNS record management (see | ||
| [kubernetes-sigs/gateway-api#2627](https://github.com/kubernetes-sigs/gateway-api/issues/2627)). |
There was a problem hiding this comment.
| Gateway API upstream is still defining guidelines for DNS record management (see | |
| [kubernetes-sigs/gateway-api#2627](https://github.com/kubernetes-sigs/gateway-api/issues/2627)). | |
| Gateway API upstream has no standard for DNS record management (see | |
| [kubernetes-sigs/gateway-api#2627](https://github.com/kubernetes-sigs/gateway-api/issues/2627)). |
| field is [restricted](https://github.com/istio/istio/blob/915ac2bf05dbc7a08ae61a8bc39fcd6ea1f3d11a/pilot/pkg/config/kube/gateway/context.go#L78) | ||
| to Gateways in the same namespace. Work is underway to develop a mechanism for | ||
| [Gateway merging](https://docs.google.com/document/d/1qj7Xog2t2fWRuzOeTsWkabUaVeOF7_2t_7appe8EXwA/edit?usp=sharing), | ||
| and it is important to review the outcome for any potential security implications. |
There was a problem hiding this comment.
| and it is important to review the outcome for any potential security implications. | |
| and it is important to review the outcome for any potential security implications or updates needed in this design. |
| |-----------------------|--------------|----------------------|----------------------|----------------| | ||
| | cluster-admin | All | All | All | All | | ||
| | admin | None | **Get, List, Watch** | **All** | None | | ||
| | edit | None | **Get, List, Watch** | **All** | None | |
There was a problem hiding this comment.
If there's no difference between the admin and edit cluster roles, we should combine them.
| implement one of these security models. To support this, the Ingress Operator will add additional RBAC | ||
| permissions for the following default ClusterRoles: | ||
|
|
||
| | OpenShift ClusterRole | GatewayClass | Gateway | xRoute Types | ReferenceGrant | |
There was a problem hiding this comment.
ReferenceGrant is the only way an application developer (or admin) can allow xRoute access to a Service they own/manage in a different namespace. I think we need to re-consider allowing only cluster-admin access to ReferenceGrants.
From the docs: "A ReferenceGrant can be used to enable cross namespace references within Gateway API. In particular, Routes may forward traffic to backends in other namespaces, or Gateways may refer to Secrets in another namespace". Routes being able to forward traffic to backends in other namespaces may be useful to application developers trying to grant access to their applications.
There was a problem hiding this comment.
If this becomes a sticking point we should consider pulling ReferenceGrant support out for GA so we can make an iteration specifically about it. We need to ensure that we don't allow cross namespace references by default as a consequence of such a thing, however.
There was a problem hiding this comment.
No, I don't think it would be necessary to add more work by removing the ReferenceGrant CRD. Let's discuss the merits of allowing app developers and admins access to ReferenceGrant. If it's easier to bar it in the beginning and open it up later, let's do that.
There was a problem hiding this comment.
To be clear, we're talking about whether the default RBAC policy will allow creating referencegrants, right? That is, the CRD will be there in any case, and the cluster-admin could define RBAC to allow broader use of referencegrants, right?
| (e.g., Istio), installing the Gateway API CRDs, and creating the GatewayClass. However, since the Ingress Operator | ||
| handles the installation of OSSM and the CRDs, the Infrastructure Operator is only responsible for creating | ||
| and managing the GatewayClass. Cluster Operators create and manage the Gateways, while Application Developers create | ||
| and manage the routes. |
There was a problem hiding this comment.
| and manage the routes. | |
| and manage the routes and referenceGrants. |
There was a problem hiding this comment.
This depends on the outcome of #1741 (comment)
There was a problem hiding this comment.
Needs several updates regarding Gateway merging and topologies, we need to trim things down to the smallest, safest deliverable for the initial release.
Since Grant wont have capacity to finish this EP, I recommend we take care of all the current comments and suggestions, close this, and re-open labeling it as tide/merge-method-rebase (as this will help maintain Grant's attribution).
| #### Gateway Topology | ||
|
|
||
| Users have the option to deploy their Gateways using two distinct topologies: shared gateways or dedicated gateways. | ||
| Each topology addresses specific customer requirements, comes with different security implications (see [Security Policy](#security-policy)), | ||
| and will be supported in OpenShift. | ||
|
|
||
| ##### Shared Gateway Topology | ||
|
|
||
| With shared gateway topology, a single load balancer type service and proxy deployment is created and serves | ||
| routes across multiple namespaces. The Gateway must allow the desired application namespaces via the | ||
| `spec.listeners.allowedRoutes.namespaces` field. This topology mirrors the existing OpenShift router topology and | ||
| is ideal for users that are cost sensitive and do not have strict performance or security requirements. | ||
|
|
||
| ```mermaid | ||
| flowchart TD | ||
| subgraph foo-app namespace | ||
| xRoute | ||
| end | ||
| subgraph bar-app namespace | ||
| xRoute2 | ||
| end | ||
| subgraph openshift-ingress namespace | ||
| Gateway | ||
| ProxyDeployment | ||
| LBService | ||
| end | ||
| Gateway[Gateway] -.-> ProxyDeployment(Proxy Deployment) | ||
| Gateway[Gateway] -.-> LBService(LB Service) | ||
| xRoute[xRoute] --> Gateway[Gateway] | ||
| xRoute2[xRoute] --> Gateway[Gateway] | ||
| ``` | ||
|
|
||
| TBD. | ||
| This cross-namespace topology can only be implemented using the Gateway-to-xRoute hierarchy, as Istio does not support | ||
| cross-namespace Gateway merging. Therefore, a limitation of this topology is that xRoutes lack an API for | ||
| specifying TLS certificates, requiring all application certificates to be managed within a cluster-wide Gateway. | ||
|
|
||
| Users can have multiple shared Gateways, similar to the concept of [sharding](https://docs.openshift.com/container-platform/latest/networking/configuring_ingress_cluster_traffic/configuring-ingress-cluster-traffic-ingress-controller.html#nw-ingress-sharding_configuring-ingress-cluster-traffic-ingress-controller) in OpenShift. | ||
|
|
There was a problem hiding this comment.
The shared Gateways topology will not be available for GA (we expect the upcoming listenersets feature to be part of our solution). I suggest we still refer to it, but instead of having a whole section about it as if it were something we can do today, we should make a special note that it's anticipated for a later release and provide some context, including a link to https://gateway-api.sigs.k8s.io/geps/gep-1713/.
There was a problem hiding this comment.
We won't be using ListenerSets for quite some time to come, so keeping the idea of Shared Gateways, as described here, should be fine.
There was a problem hiding this comment.
There might be a misunderstanding, I was not trying to suggest we remove it but rather than we caveat it.
| ##### Dedicated Gateway Topology | ||
|
|
||
| With a dedicated gateway topology, a load balancer type service and proxy deployment is created to serve the xRoutes | ||
| in a dedicated namespace. This approach is more suitable for multi-tenant environments or applications that require a |
There was a problem hiding this comment.
| in a dedicated namespace. This approach is more suitable for multi-tenant environments or applications that require a | |
| in a dedicated namespace. This approach is suitable for multi-tenant environments or applications that require a |
| OpenShift Service Mesh and Istio have a feature called [automated deployment](https://istio.io/latest/docs/tasks/traffic-management/ingress/gateway-api/#automated-deployment) | ||
| Istio has a feature called [automated deployment](https://istio.io/latest/docs/tasks/traffic-management/ingress/gateway-api/#automated-deployment) | ||
| that creates an Envoy deployment and service in the same namespace for each Gateway | ||
| if the Gateway's `spec.addresses` field is left unset. This enables users to seamlessly | ||
| create a "shard" per Gateway, much like how each IngressController object is a shard | ||
| for routes. It is enabled via the `PILOT_ENABLE_GATEWAY_API_DEPLOYMENT_CONTROLLER` | ||
| env variable. | ||
| if the Gateway's `spec.addresses` field is left unset. It is enabled via the | ||
| `PILOT_ENABLE_GATEWAY_API_DEPLOYMENT_CONTROLLER` env variable. | ||
|
|
||
| Conversely, with [manual deployments](https://istio.io/latest/docs/tasks/traffic-management/ingress/gateway-api/#manual-deployment), | ||
| if the Gateway has the `spec.addresses` field set, then it must manually link | ||
| to an [ingress gateway](https://istio.io/latest/docs/tasks/traffic-management/ingress/ingress-control/#configuring-ingress-using-a-gateway). | ||
| The user needs to make their own ingress gateway service and deployment in the same | ||
| namespace to manually link to, or they need to use the default ingress gateway (if enabled). | ||
|
|
||
| If the ServiceMeshControlPlane's `spec.gateways.ingress.enabled` field is set to `true`, | ||
| Istio creates an `istio-ingressgateway` service, in the same namespace as the control plane, | ||
| that is a ready-to-use proxy which gateways can be manually linked to. Istio [discourages](https://istio.io/latest/docs/setup/additional-setup/gateway/) | ||
| the use of this ingress gateway as it couples the gateway to the control plane. | ||
|
|
||
| _To summarize, there are four ways a user could link Gateways to Ingress Gateways:_ | ||
| 1. Use automated deployments | ||
| 2. Use a manual deployment, manually create a new ingress gateway service and deployment, and link to this service | ||
| 3. Use a manual deployment and link to the existing `istio-ingressgateway` ingress gateway | ||
| 4. Use a manual deployment and link to a previously created automated deployment ingress gateway | ||
| Users can also link a Gateway to a previously created automated deployment belonging to another | ||
| Gateway. | ||
|
|
||
| OpenShift will not inhibit or alter the functionality of automated deployments, except | ||
| restricting creation of Gateways to specific users (_see the automated deployment section | ||
| in Risks and Mitigations_). | ||
| restricting creation of Gateways to specific users (see [RBAC](#rbac)). | ||
|
|
||
| Nor will OpenShift inhibit or alter the functionality of manual deployments. Users | ||
| are responsible for understanding and creating links to manual deployments when creating | ||
| Gateways. | ||
|
|
||
| The choice between these two features depends on the user's desired sharding scheme. Manual | ||
| The choice between automated and manual deployments depends on whether a user prefers control | ||
| over creating their own ingress gateway deployments and services, and whether they want | ||
| multiple Gateway objects to share a single ingress gateway deployment and service. Manual | ||
| linking, being more expressive, can establish a many-to-one Gateway-to-Gateway-Deployment | ||
| relationship, while automated deployments strictly establish a one-to-one | ||
| Gateway-to-Gateway-Deployment relationship. Arguably, automated deployments are more portable | ||
| among Gateway API implementations due to the fact manually deployments require linking an | ||
| Istio-specific service address. |
There was a problem hiding this comment.
Because manual deployments mode is a holdover from a much earlier time in Istio, and since we will not be supporting it (again in favor of listenersets), we should remove this section and document that we will not be supporting gateway merging at this time (because upstream doesn't support it, but we're working on that in https://gateway-api.sigs.k8s.io/geps/gep-1713/).
There was a problem hiding this comment.
How can we prevent using the solution Istio/OSSM provides for gateway merging? It's outside our purview.
There was a problem hiding this comment.
It was done in a combination of upstream Istio and OSSM, and is already released in OSSM 3.0.0:
- gateway-api: allow disabling manual deployments openshift-service-mesh/istio#281
- Allow setting vendor-specific defaults istio-ecosystem/sail-operator#636
This ended up being mutual for both Gateway API and OSSM, because the OSSM team also did not want to have to support the current "manual deployments" mode which also technically enables Gateway merging.
| ##### Allowing All Namespaces for Gateways | ||
|
|
||
| Unlike OpenShift ingress, neither Gateway API nor Istio restrict Gateway objects to specific namespaces. This design | ||
| allows for different operational groups to create and manage their own [dedicated Gateway](#dedicated-gateway-topology) | ||
| in an application namespace rather than relying on a [shared Gateway](#shared-gateway-topology) residing in another | ||
| namespace. This departs from the current OpenShift ingress approach of all router pods being confined to the | ||
| `openshift-ingress` namespace. Our design will allow Gateways to be created in any namespace, provided the user has | ||
| sufficient [RBAC](#rbac) permissions. |
There was a problem hiding this comment.
Slightly worried this language might cause a bit of confusion with "allowing gateways across namespaces".
| ##### Allowing All Namespaces for Gateways | |
| Unlike OpenShift ingress, neither Gateway API nor Istio restrict Gateway objects to specific namespaces. This design | |
| allows for different operational groups to create and manage their own [dedicated Gateway](#dedicated-gateway-topology) | |
| in an application namespace rather than relying on a [shared Gateway](#shared-gateway-topology) residing in another | |
| namespace. This departs from the current OpenShift ingress approach of all router pods being confined to the | |
| `openshift-ingress` namespace. Our design will allow Gateways to be created in any namespace, provided the user has | |
| sufficient [RBAC](#rbac) permissions. | |
| ##### Gateways in Namespaces | |
| Unlike OpenShift ingress, neither Gateway API nor Istio restrict Gateway objects to specific namespaces. This design | |
| allows for different operational groups to create and manage their own [dedicated Gateway](#dedicated-gateway-topology) | |
| in an application namespace rather than relying on a [shared Gateway](#shared-gateway-topology) residing in another | |
| namespace. This departs from the current OpenShift ingress approach of all router pods being confined to the | |
| `openshift-ingress` namespace. Our design will allow Gateways to be created in any namespace, provided the user has | |
| sufficient [RBAC](#rbac) permissions. |
| ###### Gateway Merging Across Namespaces | ||
|
|
||
| Allowing Gateways in all namespaces can pose a potential security risk for Gateways that merge listeners | ||
| across namespaces, as one Gateway or listener could preempt another listener in a different namespace. However, | ||
| there currently isn't a way to merge Gateways across namespaces in Istio as the `spec.addresses` | ||
| field is [restricted](https://github.com/istio/istio/blob/915ac2bf05dbc7a08ae61a8bc39fcd6ea1f3d11a/pilot/pkg/config/kube/gateway/context.go#L78) | ||
| to Gateways in the same namespace. Work is underway to develop a mechanism for | ||
| [Gateway merging](https://docs.google.com/document/d/1qj7Xog2t2fWRuzOeTsWkabUaVeOF7_2t_7appe8EXwA/edit?usp=sharing), | ||
| and it is important to review the outcome for any potential security implications. |
There was a problem hiding this comment.
We wont be merging listeners with Istio's current mechanism, due to it being implementation-specific and a maintenance problem:
| ###### Gateway Merging Across Namespaces | |
| Allowing Gateways in all namespaces can pose a potential security risk for Gateways that merge listeners | |
| across namespaces, as one Gateway or listener could preempt another listener in a different namespace. However, | |
| there currently isn't a way to merge Gateways across namespaces in Istio as the `spec.addresses` | |
| field is [restricted](https://github.com/istio/istio/blob/915ac2bf05dbc7a08ae61a8bc39fcd6ea1f3d11a/pilot/pkg/config/kube/gateway/context.go#L78) | |
| to Gateways in the same namespace. Work is underway to develop a mechanism for | |
| [Gateway merging](https://docs.google.com/document/d/1qj7Xog2t2fWRuzOeTsWkabUaVeOF7_2t_7appe8EXwA/edit?usp=sharing), | |
| and it is important to review the outcome for any potential security implications. |
We're looking to https://gateway-api.sigs.k8s.io/geps/gep-1713/ as the Gateway API native approach to this in the future.
| implement one of these security models. To support this, the Ingress Operator will add additional RBAC | ||
| permissions for the following default ClusterRoles: | ||
|
|
||
| | OpenShift ClusterRole | GatewayClass | Gateway | xRoute Types | ReferenceGrant | |
There was a problem hiding this comment.
If this becomes a sticking point we should consider pulling ReferenceGrant support out for GA so we can make an iteration specifically about it. We need to ensure that we don't allow cross namespace references by default as a consequence of such a thing, however.
| For this initial RBAC implementation, write access to Gateways (other than `cluster-admin`) is not included due | ||
| to their complexity and potential security risks. However, users can still create custom roles, such as a | ||
| `Gateway Operator` role, to enable a role with the ability to write Gateways without requiring super-user privileges. | ||
|
|
||
| The following diagram maps the personas in Gateway API's [Simple 3 Tier Model](https://gateway-api.sigs.k8s.io/concepts/security-model/#write-permissions-for-simple-3-tier-model) to potential OpenShift ClusterRoles: | ||
| ```mermaid | ||
| flowchart LR | ||
| InfraOperator[Infrastructure Operator] --> ClusterAdmin(cluster-admin) | ||
| subgraph ClusterRole | ||
| ClusterAdmin | ||
| Admin | ||
| Edit | ||
| end | ||
| subgraph Gateway API Persona | ||
| InfraOperator | ||
| ClusterOperator | ||
| AppDev | ||
| end | ||
| ClusterOperator[Cluster Operator] --> ClusterAdmin(cluster-admin) | ||
| AppDev[Application Developer] --> Admin(admin) | ||
| AppDev[Application Developer] --> Edit(edit) | ||
| ``` | ||
|
|
||
| The Infrastructure Operator is generally responsible for installing and configuring the Gateway API provider | ||
| (e.g., Istio), installing the Gateway API CRDs, and creating the GatewayClass. However, since the Ingress Operator | ||
| handles the installation of OSSM and the CRDs, the Infrastructure Operator is only responsible for creating | ||
| and managing the GatewayClass. Cluster Operators create and manage the Gateways, while Application Developers create | ||
| and manage the routes. | ||
|
|
||
| The [Advanced 4 Tier Model](https://gateway-api.sigs.k8s.io/concepts/security-model/#write-permissions-for-advanced-4-tier-model) | ||
| is not implementable using the default ClusterRoles. However, as mentioned above, users can create a custom | ||
| `Gateway Operator` role and bind it to a specific namespace to implement the Application Admin persona. | ||
|
|
There was a problem hiding this comment.
We may want to simplify all this language down to "you must be a cluster admin for anything except *Routes" (edit: and ReferenceGrant) for GA.
There was a problem hiding this comment.
Yeah, unless we have reason to do something like this, the also ReferenceGrant for sure.
|
This is being approved as-is with the intention of following up in a separate PR. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: candita The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
@gcs278: all tests passed! Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
The follow-up PR has just been posted: #1771. |
Update the Gateway API proposal to include the security policy as well as a new section on Gateway Topology.