If capabilities are explicitly added then also add them to the ambien… #5043
Conversation
openshift-ci
bot
added
release-note
|
Hi @vinayakankugoyal. Thanks for your PR. I'm waiting for a cri-o member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci
bot
added
the
needs-ok-to-test
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: vinayakankugoyal The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@vinayakankugoyal thanks for the PR but the CI requires PR's to be signed. Please |
…t set. Signed-off-by: Vinayak Goyal <vinaygo@google.com>
vinayakankugoyal
force-pushed
the
sysctls
branch
from
e2c9a20 to
f700591
Compare
openshift-ci
bot
added
dco-signoff: yes
|
@mrunalp - this is RE: kubernetes/enhancements#2757 |
Codecov Report
@@ Coverage Diff @@
## master #5043 +/- ##
==========================================
- Coverage 41.72% 41.69% -0.04%
==========================================
Files 108 108
Lines 10158 10175 +17
==========================================
+ Hits 4238 4242 +4
- Misses 5471 5485 +14
+ Partials 449 448 -1 |
|
/ok-to-test full review will come later, but we can run CI for now, thanks for opening @vinayakankugoyal |
openshift-ci
bot
added
ok-to-test
| @@ -351,6 +353,30 @@ func setupCapabilities(specgen *generate.Generator, caps *types.Capability, defa | |||
| } | |||
| } | |||
|
|
|||
| if addDefaultCapbilities { | |||
There was a problem hiding this comment.
| if addDefaultCapbilities { | |
| if !addAll && !dropAll { |
to make addDefaultCapbilities obsolete
|
@vinayakankugoyal Thanks for the PR! |
|
All those would be added to to the ambient set. But that in that case the user is choosing to do so, by explicitly adding them. We should be secure by default and not add anything to the ambient set if nothing is explicitly added is a safe starting point. |
|
@vinayakankugoyal: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci
bot
added
the
needs-rebase
|
A friendly reminder that this PR had no activity for 30 days. |
github-actions
bot
added
the
lifecycle/stale
|
@vinayakankugoyal: The following test failed, say
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
@vinayakankugoyal: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
I think this PR doesn't make sense right now. I think we'd want an alternate way for container's to specify ambient caps, ideally with proper pod API support in kube. I am closing this. Thanks for your PR @vinayakankugoyal |
…t set.
What type of PR is this?
/kind feature
What this PR does / why we need it:
his change makes it so that we add capabilities to the ambient set if they are explicitly added. So in kubernetes if we add a capability to the container securityContext then that capability will be added to the ambient set in addition to the inheritable, effective, permitted and bounding sets. This will allow users to run a container as non-root while giving it just the capabilities it needs, without having to apply file capabilities and allowing privilege escalation.
If ALL capabilities are added then we don't add ALL capabilities to the ambient set because that would make a non-root user essentially root. We also don't add the defaultUnixCaps i.e. the default capabilities that are always added to inheritable, effective, permitted and bounded sets to the ambient capability set as that will also make non-root user as power full as root.
Which issue(s) this PR fixes:
Special notes for your reviewer:
Does this PR introduce a user-facing change?