Skip to content
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

Lock Down the Label That Ignores the Gatekeeper-System Namespace #231

Closed
maxsmythe opened this issue Aug 27, 2019 · 4 comments · Fixed by #350
Closed

Lock Down the Label That Ignores the Gatekeeper-System Namespace #231

maxsmythe opened this issue Aug 27, 2019 · 4 comments · Fixed by #350
Assignees
Labels
blocking GA Fail Closed ... and other ways to improve security
Milestone

Comments

@maxsmythe
Copy link
Contributor

kubebuilder does this by default, and we embraced it by adding a label to gatekeeper-system.

We shouldn't do this as there is no way to ACL labels, meaning anyone with the power to create/edit namespaces can exempt themselves from constraints.

Since gatekeeper already does not self-manage, we should remove this, though there is a bootstrapping problem once we start failing closed: If the admission controller auto-rejects when we launch a pod, how can we re-launch a pod if gatekeeper ever goes down.

It'd be good if we could ignore specific namespaces at the validatingwebhookconfiguration level, rather than relying on selectors which, as stated above, are insecure.

@ritazh
Copy link
Member

ritazh commented Aug 28, 2019

+1

Just to clarify, kubebuilder enables this behavior in gatekeeper, but namespaceselector is a property of the admission webhook configuration:
https://kubernetes.io/docs/reference/access-authn-authz/extensible-admission-controllers/#matching-requests-namespaceselector

So maybe we should open up an issue for https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/api/admissionregistration/v1/types.go#L260

@ritazh
Copy link
Member

ritazh commented Aug 28, 2019

Another option is to configure gatekeeper to ignore certain namespaces e.g. kube-system at init time.

@maxsmythe
Copy link
Contributor Author

maxsmythe commented Aug 28, 2019

Configuring the Gatekeeper source code won't help as it's the API server that controls what happens when the pod is down.

There is some interesting context surrounding this issue:

kubernetes/kubernetes#54522

https://docs.google.com/document/d/1pw6FyobY3pVxfWYwmAFuF5WJpvjfq-M3Ciz996kblVc/edit#heading=h.ukbesxe0yjd

@maxsmythe maxsmythe added the Fail Closed ... and other ways to improve security label Sep 12, 2019
@maxsmythe
Copy link
Contributor Author

We could add a second validating webhook endpoint, v1/nslabels that looks for an "ignore me" label and only allows it for certain namespaces. This webhook would watch Namespaces only. This gets around the self-management-causes-hard-down problem because validating namespaces wont prevent Pod creation.

Open question: whether and how the list of ignorable namespaces could be configured:

  1. Do not allow the list to be configured (hardcoded)
  2. Provide the list of "ignorable" namespaces via flags (requires restart to make changes)
  3. Make the list configurable on the config resource.

@maxsmythe maxsmythe changed the title Using a Namespace Selector to Gate the ValidatingWebhookConfiguration is Bad Lock Down the Label That Ignores the Gatekeeper-System Namespace Dec 6, 2019
@sozercan sozercan added this to the GA milestone Dec 11, 2019
maxsmythe added a commit to maxsmythe/gatekeeper that referenced this issue Dec 17, 2019
maxsmythe added a commit to maxsmythe/gatekeeper that referenced this issue Dec 18, 2019
@maxsmythe maxsmythe self-assigned this Dec 21, 2019
maxsmythe added a commit to maxsmythe/gatekeeper that referenced this issue Dec 21, 2019
maxsmythe added a commit to maxsmythe/gatekeeper that referenced this issue Jan 9, 2020
maxsmythe added a commit to maxsmythe/gatekeeper that referenced this issue Jan 10, 2020
maxsmythe added a commit to maxsmythe/gatekeeper that referenced this issue Jan 17, 2020
maxsmythe added a commit to maxsmythe/gatekeeper that referenced this issue Jan 22, 2020
maxsmythe added a commit to maxsmythe/gatekeeper that referenced this issue Jan 23, 2020
maxsmythe added a commit to maxsmythe/gatekeeper that referenced this issue Jan 23, 2020
maxsmythe added a commit to maxsmythe/gatekeeper that referenced this issue Jan 24, 2020
maxsmythe added a commit that referenced this issue Jan 24, 2020
…ces (#350)

* Add a webhook to reject the gatekeeper-ignore label on non-GK namespaces

Fixes #231

Signed-off-by: Max Smythe <smythe@google.com>

* Namespace label webhook should fail hard

Signed-off-by: Max Smythe <smythe@google.com>

* Fix lint errors

Signed-off-by: Max Smythe <smythe@google.com>

* Wait for webhook on e2e tests; add gatekeeper label

Signed-off-by: Max Smythe <smythe@google.com>

* Remove unnecessary return code

Signed-off-by: Max Smythe <smythe@google.com>

* use --resolve instead of --connect-to for ubuntu compatibility

Signed-off-by: Max Smythe <smythe@google.com>

* Move cleaning of temp file closer to its creation

Signed-off-by: Max Smythe <smythe@google.com>

* Incorporate feedback from community mtg

Signed-off-by: Max Smythe <smythe@google.com>

* Fix manifests, add e2e tests

Signed-off-by: Max Smythe <smythe@google.com>

* Add README

Signed-off-by: Max Smythe <smythe@google.com>

* Add flag to helm chart

Signed-off-by: Max Smythe <smythe@google.com>

* Regenerate helm chart

Signed-off-by: Max Smythe <smythe@google.com>

* Update README to include `does not exlude audit`

Signed-off-by: Max Smythe <smythe@google.com>

* Add DR instructions to README

Signed-off-by: Max Smythe <smythe@google.com>

* Use staging manifests

Signed-off-by: Max Smythe <smythe@google.com>

* Add webhook customization readme and tweak flag name

Signed-off-by: Max Smythe <smythe@google.com>

* Add flag change to manifest

Signed-off-by: Max Smythe <smythe@google.com>

* Add remaining namespaces -> namespace flag changes

Signed-off-by: Max Smythe <smythe@google.com>

* Update deprecation comment

Signed-off-by: Max Smythe <smythe@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocking GA Fail Closed ... and other ways to improve security
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants