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

PSP Ephemeral Containers #188

Closed
phillebaba opened this issue Apr 12, 2022 · 12 comments · Fixed by #191
Closed

PSP Ephemeral Containers #188

phillebaba opened this issue Apr 12, 2022 · 12 comments · Fixed by #191

Comments

@phillebaba
Copy link
Contributor

phillebaba commented Apr 12, 2022

Newer versions of Kubernetes have introduced the concept of ephemeral containers as a beta feature. This introduces yet another container spec ephemeralContainers on top of the existing initContainers and containers. We need to update all constraints and assigns to make sure that the new spec is covered.

@phillebaba
Copy link
Contributor Author

I wrote a quick blog post about the issues that can occur if this is not fixed.

https://xenitab.github.io/blog/2022/04/12/ephemeral-container-security

Will start working on fixing this soon hopefully.

@ritazh
Copy link
Member

ritazh commented Apr 13, 2022

Thanks for raising this issue @phillebaba! Would you interested in opening a PR for the suggested changes?

@phillebaba
Copy link
Contributor Author

phillebaba commented Apr 13, 2022

Yep should not be an issue for me 😄

@mac-chaffee
Copy link
Contributor

Should we issue a CVE or other announcement for this?

It seems that any v1.23 cluster which depends on these policies to replace PodSecurityPolicies now has no protection against privileged pods. And since there's no versioning/releases for these policies, users may not know they need to upgrade ASAP once #191 is merged.

@maxsmythe
Copy link
Contributor

Putting out an announcement is a good idea. Is there a best channel for this?

@mac-chaffee
Copy link
Contributor

@maxsmythe Looks like there's a procedure outlined here: https://github.com/open-policy-agent/opa/blob/main/SECURITY.md

@srenatus
Copy link
Contributor

I'm afraid that document might be a little outdated. Recently, we've been using GitHub security advisories, and those seemed to work quite well. It'll end up issuing a cve that'll be picked up by the usual trackers. I haven't checked all the history here, but I suppose the advisory would best fit in the GK repo, since it's the main user of this framework and GK 's users are there ones we'd like to inform....?

@ritazh
Copy link
Member

ritazh commented May 11, 2022

I suppose the advisory would best fit in the GK repo, since it's the main user of this framework and GK 's users are there ones we'd like to inform....?

This issue is technically a gap in some of the policies in this gatekeeper-library when ephemeral containers are introduced. it's not a gap in gatekeeper. We currently do not have any security procedure defined for this repo.

Should we issue a CVE or other announcement for this?

I don't know if this issue requires a CVE to be assigned but at a minimum we should definitely pin this issue to the top of the repo and pin it to slack and opa announcements once the PR is merged so users can follow the issue/PR to make their own updates.

I have added this to tomorrow community call for further discussion of security procedure for this repo.

@ritazh
Copy link
Member

ritazh commented May 11, 2022

As discussed in today's community call, for issues like these in the future, we will pin the issue to the top of the repo and make announcements on slack to notify users they need to make updates.

@mac-chaffee
Copy link
Contributor

FYI sent a message to open-policy-agent-security@googlegroups.com with some more details on this subject. Mentioning it here since you mentioned the security.md doc may need an update

@mac-chaffee
Copy link
Contributor

Note to anyone seeing this pinned issue: The new constraints provided by #191 are necessary but NOT sufficient to protect your v1.23+ cluster from people running privileged ephemeralcontainers. You also need to update Gatekeeper to validate pods/ephemeralcontainers (it DOES NOT do this by default at the time of writing).

Until this PR is merged, these are the steps required:

  1. Apply the updated constraints from Add ephemeral containers to policies #191 to your cluster
  2. If you installed Gatekeeper via Helm, upgrade gatekeeper with the following extra helm values:
validatingWebhookCustomRules:
  - apiGroups: ['*']
    apiVersions: ['*']
    resources:
    - '*'
    - 'pods/ephemeralcontainers'  # If you already have custom rules, just ensure this line is present
    operations:
    - CREATE
    - UPDATE

mutatingWebhookCustomRules:
  - apiGroups: ['']
    apiVersions: ['*']
    resources:
    - '*'
    - 'pods/ephemeralcontainers'  # If you already have custom rules, just ensure this line is present
    operations:
    - CREATE
    - UPDATE
  1. If you installed Gatekeeper via manifest, run kubectl edit validatingwebhookconfiguration gatekeeper-validating-webhook-configuration and add pods/ephemeralcontainers to the list of resources (in addition to '*').

After this PR is merged, update to the latest version of Gatekeeper and ensure your validating/mutatingwebhookconfiguration is still configured to check pods/ephemeralcontainers.

@phillebaba
Copy link
Contributor Author

@mac-chaffee good catch!

I had not verified my PR outside of gator tests which explains why it was not caught there. Not really sure how we could solve this in the future however without building larger real world e2e tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants