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

Enable PodSecurityPolicies by default #226

Merged
merged 1 commit into from
Nov 2, 2018

Conversation

mvladev
Copy link

@mvladev mvladev commented Jun 19, 2018

What this PR does / why we need it:
PodSecurityPolicy admission controller is enabled by default for all Shoot clusters.

  • gardener.unprivileged PSP limits the Pod privileges to the absolute minimum
  • gardener.privileged PSP grants full unrestricted Pod creation.

By default, all authenticated users are given access to both PSP, thus keeping the old behavior (as if PodSecurityPolicy admission controller is not enabled). Those roles are assigned using spec.kubernetes.allowPrivilegedContainers of a Shoot resource.

Cluster operators are encouraged to create PodSecurityPolicies in advanced and test their behavior.

Which issue(s) this PR fixes:
Fixes #211

Release note:

The PodSecurityPolicy admission plugin is now enabled by default for all shoots. Cluster administrators seeking to increase the security of their clusters can enable stricter rules by setting `spec.kubernetes.allowPrivilegedContainers` to `false` in the `Shoot` resource: `kubectl patch shoot my-shoot -p '{"spec":{"kubernetes":{"allowPrivilegedContainers":false}}}'`

@rfranzke rfranzke added kind/enhancement Enhancement, improvement, extension status/under-investigation area/security Security related platform/all size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 19, 2018
@rfranzke
Copy link
Member

Thanks @mvladev. Review postponed until Gardener 0.7.0 has been released.

Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we use garden.sapcloud.io: as prefix instead of gardener: (like we do it when creating secrets or certificates for components needed/deployed by Gardener)?

Looks good generally. However, one issue I found is with Heapster. PTAL at my comments inline.

Regarding enabling/disabling privileged containers: Currently, the user has to patch the ClusterRoleBinding which is only ensured to exist by the kube-addon-manager. Can we use the allowPrivilegedContainers field to enable/disable? That would be much nicer and a clearer contract.

Can you also please remove https://github.com/gardener/gardener/blob/master/charts/shoot-cloud-config/charts/original/templates/kubelet/_kubelet.flags#L3 and https://github.com/gardener/gardener/blob/master/charts/shoot-cloud-config/charts/original/templates/kubelet/_kubelet.flags#L40 (as the --allow-privileged flag is deprecated and will be removed in 1.13)?

Could you also please keep in mind to add a document describing the behaviour, how to disable/enable privileged mode, etc. (can be done later as well)?

@rfranzke
Copy link
Member

rfranzke commented Jun 25, 2018

⚠️ kubernetes/kubernetes#63442 -> Changes the default of the deprecated --allow-privileged kubelet flag.

@rfranzke
Copy link
Member

ping @mvladev

@rfranzke
Copy link
Member

rfranzke commented Sep 7, 2018

What's the status with this one? Shall we close it and you reopen it once you have rebased and addressed the initial feedback?

@mvladev
Copy link
Author

mvladev commented Sep 7, 2018

I'll update the PR on Monday

@mvladev mvladev force-pushed the enable-psp branch 3 times, most recently from e147fed to dd24748 Compare September 11, 2018 07:54
@ThormaehlenFred
Copy link
Contributor

@ThormaehlenFred @alban I've tried to re-use the PSP e2e-test, but by default it creates a Pod with
apparmor annotation: https://github.com/kubernetes/kubernetes/blob/1f0f4cd7eb1d6a14dd3957bf668b7afe93498bf5/test/e2e/auth/pod_security_policy.go#L238
which is blocked by kubelet as we don't have apparmor on our nodes

Thank you @mvladev for checking. So, as we are not about to enable AppArmor, it looks like we should look for another test or create a new test in go which we can use to test if PSP really works in our clusters. Is there already a GitHub issue we can link here to not forget that there is a test needed?

@rfranzke
Copy link
Member

Copied from a conversation with @mvladev in Slack:

so, monocular is the only thing that is failing:

addons-monocular-api-7c9db78989-2hjsd                             1/1     Running            5          71m
addons-monocular-ui-747d88b986-9tcvd                              0/1     CrashLoopBackOff   17         71m

INFO  ==> Starting nginx...
nginx: [alert] could not open error log file: open() "/opt/bitnami/nginx/logs/error.log" failed (13: Permission denied)
2018/10/30 12:16:12 [emerg] 23#0: open() "/opt/bitnami/nginx/conf/nginx.conf" failed (2: No such file or directory)

I guess it's even only the UI, API should be fine (5 restarts are normal because it needs to download all the charts) (edited)

Other than that the only thing I'm concerned about is the kubelet config

What was the motivation to merge the pre 1.10 with the >= 1.10 parts?

Can't we just keep this part? This will also allow us to easily remove the <1.10 part of the config once we drop 1.9

@mvladev
Copy link
Author

mvladev commented Nov 1, 2018

Updated monocular-ui in 11fd7af

PodSecurityPolicy admission controller is enabled by default for all Shoot
clusters.
- gardener.unprivileged PSP limits the Pod privileges to the absolute minimum
- gardener.privileged PSP grants full unrestricted Pod creation.

By default, all authenticated users are given access to both PSP, thus keeping
the old behavior (as if `PodSecurityPolicy` admission controller is not
enabled).

Cluster administrators, seeking to increase the security of their clusters can
disable this behavior by setting `spec.kubernetes.allowPrivilegedContainers` to
`false` in the `Shoot` resource. For example:

`kubectl patch shoot my-shoot -p '{"spec":{"kubernetes":{"allowPrivilegedContainers":false}}}'`
Copy link
Member

@rfranzke rfranzke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@rfranzke rfranzke merged commit 5ef62ad into gardener:master Nov 2, 2018
@mvladev mvladev deleted the enable-psp branch June 14, 2019 13:37
ialidzhikov pushed a commit to ialidzhikov/gardener that referenced this pull request Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security Security related kind/enhancement Enhancement, improvement, extension platform/all size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use PodSecurityPolicies for enabling/disabling privileged containers
7 participants