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

defaultAllowPrivilegeEscalation Support #55

Closed
marshallford opened this issue Jun 21, 2020 · 9 comments
Closed

defaultAllowPrivilegeEscalation Support #55

marshallford opened this issue Jun 21, 2020 · 9 comments
Labels

Comments

@marshallford
Copy link

Describe the solution you'd like
I believe this would require a mutating webhook, but I'd like to be able to toggle a flag (on a Constraint?) similar to the defaultAllowPrivilegeEscalation field in the PodSecurityPolicy resource.

After reading the No New Privileges Design Doc I think I'm beginning to understand why the existing Gatekeeper PSP library policy allow-privilege-escalation needs to check every initContainer and container. By default, if allowPrivilegeEscalation isn't explicitly set to false, then the nonroot containers can escalate.

@maxsmythe
Copy link
Contributor

Mutating webhooks are likely a ways away as the ability to write them in Rego is harder than initially thought due to the need for recursion (allowing infinite recursion would make Rego turing complete).

In the short term, it is possible to create a validation webhook that rejects the resource if it is missing the required field. This is not as convenient for users in the short term, but does have the advantage that their configuration files more closely match the actual internal state of the system.

@marshallford
Copy link
Author

For fun, I wrote a minimal mutating webhook for this use case. The webhook closely mimics the functionality found in the PSP admissions controller. This solution isn't perfect for a number of reasons, but it has been helpful.

@maxsmythe
Copy link
Contributor

Nice!

One coder quality-of-life thing I'm noticing is that I think the admission framework will calculate the mutation patches for you if you return a modified object, though I may be misremembering.

Which is better might be a matter of taste, but personally I like the idea of not having to worry about accidentally coding incompatible JSON patches and just modifying the inbound object directly.

It's good for us to keep an eye on what people may want to use mutation for, as that will definitely impact its design.

@marshallford
Copy link
Author

A framework as in a golang library?

@maxsmythe
Copy link
Contributor

Yep, the controller-runtime library:

https://github.com/kubernetes-sigs/controller-runtime/blob/229c3c357d9e6b07c3d6774010c35161a82b08f9/pkg/webhook/admission/response.go#L83-L98

@marshallford
Copy link
Author

marshallford commented Jul 21, 2020

Thanks!

One last question for you since the #kubernetes-users channel on Slack wasn't too helpful:

Any advice for writing a golang k8s webhooks that support both v1beta1 admissionregistration and v1? Is there a helper function that converts between the two? Does listing both versions via the admissionReviewVersions field have any automagical properties that I'm not aware of?

@maxsmythe
Copy link
Contributor

To be honest, I'm not sure off the top of my head.

My understanding is that you can specify which version of the review object you want via the *WebhookConfiguration resources.

The version of the review object is embedded in the object itself, so you could inspect the request and deserialize to the appropriate object, but I haven't dug into the differences between the two resources to see if there are any fields that need to be handled differently depending on version.

@sozercan sozercan transferred this issue from open-policy-agent/gatekeeper Feb 10, 2021
@sathieu
Copy link
Contributor

sathieu commented Oct 13, 2021

It looks like library/experimental/mutation/pod-security-policy/allow-privilege-escalation implements this. However, mutations are still experimental and this mutation is too.

ctrought pushed a commit to ctrought/gatekeeper-library that referenced this issue Jan 16, 2023
@stale
Copy link

stale bot commented Feb 1, 2023

This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 1, 2023
@stale stale bot closed this as completed Feb 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants