-
Notifications
You must be signed in to change notification settings - Fork 182
OCPBUGS-55013: SCC: add hostmount-anyuid-v2 #1834
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
OCPBUGS-55013: SCC: add hostmount-anyuid-v2 #1834
Conversation
|
@haircommander: This pull request references Jira Issue OCPBUGS-55013, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
|
@haircommander: This pull request references Jira Issue OCPBUGS-55013, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
| restricted SCC but allows host mounts any UID, and any SELinux label by a pod. This is primarily | ||
| used by the persistent volume recycler. WARNING: this SCC allows host file | ||
| system access as any UID, including UID 0. Grant with caution. | ||
| name: hostmount-anyuid |
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.
Hey @haircommander , thanks for the quick follow up! Should this be
| name: hostmount-anyuid | |
| name: hostmount-anyuid-v2 |
?
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.
absolutely good catch
|
/cc @ibihim |
b7a58f6 to
e72e1d5
Compare
|
/retest |
1 similar comment
|
/retest |
| hostmount-anyuid-v2 provides all the features of the | ||
| restricted SCC but allows host mounts any UID, and any SELinux label by a pod. This is primarily | ||
| used by the persistent volume recycler. WARNING: this SCC allows host file | ||
| system access as any UID, including UID 0. Grant with caution. |
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.
Well, in good practice of the v2s, we should mention that we are by-passing SELinux context as well.
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.
I think this is captured with and any SELinux label by a pod
| users: | ||
| - system:serviceaccount:openshift-infra:pv-recycler-controller |
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.
we are moving away from specifying subjects directly in the SCC and it seems that pv-recycler-controller works just fine with with old SCC, so let start out clean in v2.
| supplementalGroups: | ||
| type: RunAsAny | ||
| users: | ||
| - system:serviceaccount:openshift-infra:pv-recycler-controller |
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.
Curious if we can drop this.
the intent of hostmount-anyuid is to allow a pod access to paths on the host. The problem is, hostPath volumes aren't selinux relabeled, and there are paths that the default selinux type `container_t` cannot access. This breaks expectation, and makes pods that rely on hostmount-anyuid brittle to selinux changes on the host (see https://issues.redhat.com/browse/OCPBUGS-55013) Instead of relaxing permissions of all paths on the host, or increasing the ability of container_t, we should trust pods that are granted access to this already powerful SCC to use its power fully. Signed-off-by: Peter Hunt <pehunt@redhat.com>
e72e1d5 to
8ecdca3
Compare
|
@haircommander: 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-sigs/prow repository. I understand the commands that are listed here. |
ibihim-unsafe
left a comment
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.
Looks good. Please create a PR for e2e tests as well.
ibihim
left a comment
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.
Looks good. Please create a PR for e2e tests as well.
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: haircommander, ibihim-unsafe, vrutkovs The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
438e1f6
into
openshift:main
|
@haircommander: Jira Issue OCPBUGS-55013: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-55013 has been moved to the MODIFIED state. In response to this:
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 openshift-eng/jira-lifecycle-plugin repository. |
|
/cherry-pick release-4.19 |
|
@Prashanth684: new pull request created: #1844 In response to this:
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-sigs/prow repository. |
|
[ART PR BUILD NOTIFIER] Distgit: ose-cluster-kube-apiserver-operator |
the intent of hostmount-anyuid is to allow a pod access to paths on the host. The problem is, hostPath volumes aren't selinux relabeled, and there are paths that the default selinux type
container_tcannot access.This breaks expectation, and makes pods that rely on hostmount-anyuid brittle to selinux changes on the host (see https://issues.redhat.com/browse/OCPBUGS-55013)
Instead of relaxing permissions of all paths on the host, or increasing the ability of container_t, we should trust pods that are granted access to this already powerful SCC to use its power fully.