-
Notifications
You must be signed in to change notification settings - Fork 558
Adopt CIS Kubernetes Benchmark, Part 1: API Server. #1989
Conversation
b2cd25b
to
04cefdc
Compare
@@ -48,6 +48,15 @@ spec: | |||
- "--requestheader-extra-headers-prefix=X-Remote-Extra-" | |||
- "--requestheader-group-headers=X-Remote-Group" | |||
- "--requestheader-username-headers=X-Remote-User" | |||
- "--anonymous-auth=false" | |||
- "--profiling=false" |
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.
how does this affect each existing kubernetes version including those created by acs-engine, and those created by services that use acs-engine?
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.
These changes are based on CIS benchmark, for example, "--anonymous-auth=false" is recommended here:
https://workbench.cisecurity.org/sections/35552/recommendations/98632
I think ideally, each version of Kubernetes should have their own security guidelines, which translates into a bunch of recommended argument settings in yaml files.
For now, the CIS benchmark I used is for Kubernetes 1.8, the recommendations looks pretty generic across multiple versions of Kubernetes to me. So I have not see the needs to do some further versioning yet. Plus, these changes will need to pass multiple version e2e tests before they will hit prod.
Yes, it should affect both acs-engine and the RPs that consumes it.
9d57e65
to
44d8219
Compare
44d8219
to
1887709
Compare
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.
lgtm
1887709
to
4c1d2b7
Compare
This breaks acs-engine generated cluster for existing versions. For the following pod, it would succeed with
Suppose we should support configurable 'admission-control': provide a recommended value as default, and allow user to override. Update: Looks like APIServerConfig is already configurable. Just need to ensure not to break existing user experience. |
@karataliu Thanks for reporting, could you give an example workflow that will fail without the manual config override? (And can you clarify which config override is now necessary for older clusters?) |
@karataliu Thank you for reporting the issue! Actually the "admission-control" parameter has been made user configurable after this PR. I will investigate the issue you reported to see if the "SecurityContextDeny" should be removed from the default config. |
@jackfrancis @JunSun17
Test yaml:
Three combinations:
Result: Pod run successfully
Result: Failure
Result: Pod run successfully |
@karataliu Thanks for providing the details on this issue. I checked the Kubernetes document on this section: https://kubernetes.io/docs/admin/admission-controllers/#securitycontextdeny, and it says
Because we have not implement Pod Security Policy, my understanding is the above statement asks to set the SecurityContextDeny flag in admission control. I do not know how many contains are actively setting their own pod level security context, and thus are impacted by this default setting. If it is a common case, I think it makes sense to remove this flag for now and probably implement Pod Security Policy to mediate this issue. Feel free to share your opinion on this. |
@jackfrancis @karataliu Great, #2048 just come in in time. I see the seLinux policy is set as:
@karataliu do we need to set nginx security context? Is it come in by default or by customization? |
Since acs-engine by default uses ubuntu as host vm image, selinux is thus unavailable in this case. I found those when running k8s e2e tests, some of which would fail on a v0.12.0 acs-engine generated cluster. The selinux configuration comes with following: Other failures are also related to the admission-control parameter change:
As we can see, |
@karataliu I did some testing and verified by making some changes in PR #2048, using a proper PSP file, the nginx deployment works. It should be the long term solution and documented in Issue #2123. For now, to reduce impact of this setting, I will remove SecurityContextDeny, as in PR #2125. |
What this PR does / why we need it: Multiple issues are reported by running Kubernetes Auto Analyzer (https://github.com/nccgroup/kube-auto-analyzer). This PR tries to address those issues that can be easily fixed.
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)
format, will close that issue when PR gets merged): fixes #1828Special notes for your reviewer: No need to review yet, will go through test first to see if it breaks things.
Release note: