generated from kubernetes/kubernetes-template-project
-
Notifications
You must be signed in to change notification settings - Fork 612
Add security considerations guide, update security model #4219
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
Open
rikatz
wants to merge
3
commits into
kubernetes-sigs:main
Choose a base branch
from
rikatz:security-guide
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,241 @@ | ||
| # Gateway API security considerations | ||
|
|
||
| Gateway controllers can be deployed in a multi-tenant environment, where different | ||
| namespaces are used by different users and customers. | ||
|
|
||
| Some caution should be taken by the cluster administrators and Gateway owners to | ||
| provide a safer environment. | ||
|
|
||
| ## Avoiding hostname/domain hijacking | ||
|
|
||
| Gateway controllers work to disambiguate and detect conflicts caused by sharing | ||
| different ports, protocols, etc. between various listeners. (?) | ||
|
|
||
| Generally this conflict resolution works on a first-come, first-served basis, where | ||
| the first created resource wins in the conflict management. | ||
|
|
||
| The [hostname definition](../reference/spec.md#httproutespec) is a list, so given the | ||
| following scenario: | ||
|
|
||
| * `Gateway` accepts routes from a set of namespaces | ||
| * `HTTPRoute` with name `route1` is created on namespace `ns1`, with `creationTimestamp: 00:00:01` | ||
| and defines hostname `something.tld`. | ||
| * `HTTPRoute` with name `route2` is created on namespace `ns2`, with `creationTimestamp: 00:00:30` | ||
| and defines hostname `otherthing.tld`. | ||
|
|
||
| If the owner of `route1` adds later `otherthing.tld` to the list of hostnames, the | ||
| route will be hijacked from `route2` because `route1` is older. | ||
|
|
||
| To avoid this situation, the following actions should be taken: | ||
|
|
||
| * On Gateways, admins SHOULD ensure that hostnames are clearly delegated to a specific namespace or set of namespaces: | ||
|
|
||
| === "Good configuration" | ||
|
|
||
| ```yaml | ||
| apiVersion: gateway.networking.k8s.io/v1 | ||
| kind: Gateway | ||
| metadata: | ||
| name: gateway | ||
| spec: | ||
| gatewayClassName: some-class | ||
| listeners: | ||
| - hostname: "something.tld" | ||
| name: listener1 | ||
| port: 80 | ||
| protocol: HTTP | ||
| allowedRoutes: | ||
| namespaces: | ||
| from: Selector | ||
| selector: | ||
| matchLabels: | ||
| kubernetes.io/metadata.name: ns1 | ||
| - hostname: "otherthing.tld" | ||
| name: listener2 | ||
| port: 80 | ||
| protocol: HTTP | ||
| allowedRoutes: | ||
| namespaces: | ||
| from: Selector | ||
| selector: | ||
| matchLabels: | ||
| kubernetes.io/metadata.name: ns2 | ||
| ``` | ||
|
|
||
| === "Insecure configuration" | ||
|
|
||
| ```yaml | ||
| apiVersion: gateway.networking.k8s.io/v1 | ||
| kind: Gateway | ||
| metadata: | ||
| name: gateway | ||
| spec: | ||
| gatewayClassName: some-class | ||
| listeners: | ||
| - name: listener1 | ||
| port: 80 | ||
| protocol: HTTP | ||
| allowedRoutes: | ||
| namespaces: | ||
| from: All | ||
| ``` | ||
|
|
||
| ### More than 64 listeners | ||
|
|
||
| Gateway resource has a limitation of 64 listener entries. If you need more than 64 | ||
| listeners, you should consider allowing your users to set their hostnames directly | ||
| on the routes of each namespaces but limiting what namespace can use what hostname | ||
| by relying on a mechanism like `ValidatingAdmissionPolicy`. | ||
|
|
||
| In case you opt to use (the still experimental) `ListenerSet`, a similar mechanism | ||
| should also be considered to limit what hostnames a `ListenerSet` can claim. | ||
|
|
||
| ### Example of a ValidatingAdmissionPolicy | ||
|
|
||
| A [ValidatingAdmissionPolicy](https://kubernetes.io/docs/reference/access-authn-authz/validating-admission-policy/) | ||
| can be used to add rules that limits what namespaces can use what domains. | ||
|
|
||
| !!! warning | ||
| The validation policy shown here **IS AN EXAMPLE** and the cluster-admin should do | ||
| adjustments to their own environment! Do not copy/paste this example with proper | ||
| adjustments | ||
|
|
||
| The policy exemplified here will: | ||
|
|
||
| * Read the allowed domains from a comma-separated value of the `annotation` "domains" present on the namespace. | ||
| * Validate if all of the hostnames within `.spec.hostnames` are contained on this annotation. | ||
| * In case any of the entries are not authorized, the policy denies its admission. | ||
|
|
||
| ```yaml | ||
| apiVersion: admissionregistration.k8s.io/v1 | ||
| kind: ValidatingAdmissionPolicy | ||
| metadata: | ||
| name: httproute-hostname-policy | ||
| spec: | ||
| failurePolicy: Fail | ||
| matchConstraints: | ||
| resourceRules: | ||
| - apiGroups: ["gateway.networking.k8s.io"] | ||
| apiVersions: ["v1", "v1beta1"] | ||
| operations: ["CREATE", "UPDATE"] | ||
| resources: ["httproutes"] | ||
| variables: | ||
| - name: allowed_hostnames_str | ||
| expression: | | ||
| has(namespaceObject.metadata.annotations) && | ||
| has(namespaceObject.metadata.annotations.domains) ? | ||
| namespaceObject.metadata.annotations['domains'] : '' | ||
| - name: allowed_hostnames_list | ||
| expression: | | ||
| variables.allowed_hostnames_str.split(','). | ||
| map(h, h.trim()).filter(h, size(h) > 0) | ||
| validations: | ||
| - expression: | | ||
| !has(object.spec.hostnames) || | ||
| size(object.spec.hostnames) == 0 || | ||
| object.spec.hostnames.all(hostname, hostname in variables.allowed_hostnames_list) | ||
| message: "HTTPRoute validation failed. It contains unauthorized hostnames" | ||
| --- | ||
| apiVersion: admissionregistration.k8s.io/v1 | ||
| kind: ValidatingAdmissionPolicyBinding | ||
| metadata: | ||
| name: httproute-hostname-binding | ||
| spec: | ||
| policyName: httproute-hostname-policy | ||
| validationActions: ["Deny"] | ||
| matchResources: | ||
| namespaceSelector: {} | ||
| ``` | ||
|
|
||
| Once the policy is created, the cluster-admin should explicitly allow the usage of | ||
| domains with a command like `kubectl annotate ns default domains=www.dom1.tld,www.dom2.tld` | ||
|
|
||
| Additionally, when dealing with environments that provide DNS record creations, | ||
| admins should be aware and limit the DNS creation based on the same constraints above. | ||
|
|
||
| ## Limiting Cross-Namespace References | ||
|
|
||
| Owners of resources should be aware of the usage of [ReferenceGrants](../api-types/referencegrant.md). | ||
|
|
||
| ReferenceGrant allows resource owners to make their resources available to | ||
| Gateway API resources in other namespaces. It may be beneficial to restrict where | ||
| this can be done. | ||
|
|
||
| A `ValidatingAdmissionPolicy` can be used to limit what kind of `resource` and which `namespace` | ||
| can create a `ReferenceGrant`. | ||
|
|
||
| Below is an **EXAMPLE** that can be used to limit the usage of a `ReferenceGrant` just | ||
| on namespaces labeled with `referencegrants=allow` and only allowing objects of kind `HTTPRoute` | ||
| to reference an object of kind `Service` that MUST have a name: | ||
|
|
||
| ```yaml | ||
| apiVersion: admissionregistration.k8s.io/v1 | ||
| kind: ValidatingAdmissionPolicy | ||
| metadata: | ||
| name: reference-grant-limit | ||
| spec: | ||
| failurePolicy: Fail | ||
| matchConstraints: | ||
| resourceRules: | ||
| - apiGroups: ["gateway.networking.k8s.io"] | ||
| apiVersions: ["v1beta1"] | ||
| operations: ["CREATE", "UPDATE"] | ||
| resources: ["referencegrants"] | ||
| variables: | ||
| - name: allowed_grant_ns | ||
| expression: | | ||
| has(namespaceObject.metadata.labels) && | ||
| has(namespaceObject.metadata.labels.referencegrants) && | ||
| namespaceObject.metadata.labels['referencegrants'] == 'allow' | ||
| - name: allowed_from_kind | ||
| expression: | | ||
| object.spec.from.all(f, f.kind=='HTTPRoute') | ||
| - name: allowed_to_kind | ||
| expression: | | ||
| object.spec.to.all(t, t.kind == 'Service' && has(t.name) && t.name != '') | ||
| validations: | ||
| - expression: | | ||
| variables.allowed_grant_ns && variables.allowed_from_kind && variables.allowed_to_kind | ||
| message: "ReferenceGrant must be explicitly allowed on the namespace, from an HTTPRoute to a named service" | ||
| --- | ||
| apiVersion: admissionregistration.k8s.io/v1 | ||
| kind: ValidatingAdmissionPolicyBinding | ||
| metadata: | ||
| name: reference-grant-limit | ||
| spec: | ||
| policyName: reference-grant-limit | ||
| validationActions: ["Deny"] | ||
| matchResources: | ||
| namespaceSelector: {} | ||
| ``` | ||
|
|
||
| ReferenceGrant owners should ensure that the reference permissions being granted are as minimal as possible. | ||
|
|
||
| * Specify `to` targets in all possible ways (`group`, `kind`, **and** `name`) | ||
| * In particular, DO NOT leave `name` unspecified, even though it is optional, without a *very* good reason, as that is granting a blanket | ||
|
|
||
|
|
||
| ## Proper definition of Roles and RoleBinding | ||
|
|
||
| The creation of a new Gateway should be considered as a privileged permission. | ||
| The unguarded creation of Gateways may increase costs, infrastructure modifications | ||
| (like a LoadBalancer and a DNS record creation) and as in such case, admins should | ||
| be aware of it and create [Roles](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#role-and-clusterrole) | ||
| and [RoleBindings](https://kubernetes.io/docs/reference/access-authn-authz/rbac/#rolebinding-and-clusterrolebinding) | ||
| that reflect proper user permissions. | ||
|
|
||
| Additionally, it is highly recommended that the strict permissions are given, not | ||
| allowing regular users to modify a Gateway API status. | ||
|
|
||
| For more information about the security model of Gateway API, check [Security Model](security-model.md) | ||
|
|
||
| ## Usage and limit of GatewayClass | ||
|
|
||
| A cluster may have different GatewayClasses, with different purposes. As an example, | ||
| one GatewayClass may enforce that Gateways attached to it can only use internal load balancers. | ||
|
|
||
| Cluster admins should be aware of these requirements, and define validation | ||
| policies that limit the improper attachment of a Gateway to a GatewayClass by unauthorized users. | ||
|
|
||
| A [ValidatingAdmissionPolicy](https://kubernetes.io/docs/reference/access-authn-authz/validating-admission-policy/) | ||
| can be used to limit what namespaces can use a `GatewayClass`. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe instead of focusing on listeners, focus on the overall theme that it's possible in Gateway API for different Routes or ListenerSets to both claim the same hostname + path, and that Gateway controllers are responsible for conflict resolution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rob, I am having some bad time here to give you a proper resolution :D do you mind doing a suggestion on how this should look like (unless it is more than 1 line of change!)