-
Notifications
You must be signed in to change notification settings - Fork 40k
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
Security contexts and volumes #7925
Comments
@eparis cc |
Regarding item 1: As I have been thinking about it, SecurityContext is about how the container should run, not how it could run. So, strictly speaking, the Volume is not validated against the SecurityContext. Rather, both the Volumes and the SecurityContext are each validated against something. We need to decide what that thing is. In the earlier SecurityContext proposal (#6287), that thing was a SecurityConstraint. Another alternative, which I'd like to explore, is to express allowed SecurityContext and Volume settings using Policy. |
I'm not sure I understand what is being suggested in items 2 and 3. Pre-existing files, by default, have whatever permissions/owner/group/attributes they have on their underlying storage. There may be mechanisms to remap/override, but use of them should be the exception, not the rule. New files inherit attributes from their creating process (SecurityContext influences this). |
I think we're on the same page. I had actually typed out 'generated' volumes in a draft of the issue but that made it out in an edit. I meant my comments to apply to volumes with generated content, secrets for example. @sdminonne's downward API volume plugin would be another example.
@pweil- and I spent some time talking about exactly this today. The constraint stuff in #7893 relates to this -- for example, one piece of policy is whether the hostDir volume plugin is allowed. |
Random thoughts by eparis.... docker has z/Z options to the -v argument. This means that docker will actually SELinux relabel all of the files on a volume to make them accessable. Think of this as docker doing the SELinux equivalent of And something that we MAY wish to support. But a security policy would want to be enforced, somehow. Some sort of check between a label on the pod and the volume? Long term, if user namespaces are ever usable, I'd think we might want a similar piece of functionality for UIDs. If a pod is being run with a randomly generated UID from the host PoV you may need a way to chown the contents of a mount before it is used... |
I have to agree with @erictune. We NEED a clean separation of policy and enforcement. I come from SELinux where we had such a thing and it was hard enough for people to figure out how to set their security policies correctly. But scattering policy like statements across many areas is even harder for users. |
----- Original Message -----
Whether we call it SecurityConstraint, SecurityContextConstraint, SecurityContextPolicy, or a subset of policy, it should be clearly defined and expressible in terms of maximizing understanding of the admin to what it means. Part of my concern with generic policy is that it does not preserve that guarantee. I'm fine with SecurityContextPolicy being treated as an opaque interface that is applied, and then one possible implementation is a SecurityContextConstraints object loaded from config or associated with a service account. Our short term requirement is there be someway in the kubelet and API server to validate a security context + volumes is safe. If that's just an interface and we talk more about what the object looks like stored in the model that's fine (OpenShift can impose that via admission control in a custom way). |
What I've been batting around is 51f4ea3 which defines a set of constraints (or policy, if you'd like) that are used for the validation/enforcement. My expectation is, like @erictune says, that a SecurityContext is the end result of the enforcement and becomes what the container runs with. The definition of the constraints is separate and lives at both a cluster level or the service account level as an override. // SecurityContextConstraintsProvider is responsible for ensuring that every service account has a
// security constraints in place and that a pod's context adheres to the active constraints.
type SecurityContextConstraintsProvider interface {
// CreateContextForPod creates a security context for the pod based on what was
// requested and what the policy allows
CreateContextForContainer(pod *api.Pod, container *api.Container) *api.SecurityContext
// ValidateAgainstConstraints validates the pod against SecurityContextConstraints
ValidateAgainstConstraints(pod *api.Pod) fielderrors.ValidationErrorList
} The actual implementation of this provider can be given the cluster defaults and when either of those two methods are executed it first retrieves the SecurityContextConstraint from the ServiceAccount if it exists (using pod.spec.serviceaccount) or it uses the cluster defaults. An admission controller could examine an incoming pod to see if SecurityContext is defined. If it is then it can use the Since the types for a SecurityContext are defined I don't think it's a stretch to base a constraints object on that definition, but allowing extension points for things like 'must run as uid 2' or 'must run as a user in range 1000-2000' rather than an explicit RunAsUser=5. The security context is rigid while the constraints are fluid. Feedback is more than welcome! |
Related: #2630 |
Some thoughts about generalizing SELinux context determination:
So, ultimately, I see:
One thing we need to figure out about all this is how we should validate that all containers in a pod have interoperable SELinux contexts from the volume-sharing standpoint. |
SELinux isn't the only story though for volumes - we probably need to make Ultimately, our original goal was to ensure that the process running in the We have two options - force the user to also specify a label for the I know Tim was concerned with having multiple security settings - but if it On Tue, Jul 28, 2015 at 12:51 PM, Paul Morie notifications@github.com
Clayton Coleman | Lead Engineer, OpenShift |
/cc @rootfs |
@smarterclayton Mostly agree with what you said. I think before I code anything I want to lay out how I see SELinux context, uid, gid being handled across all volume types; I mostly see the matrix for that in my head -- will type it out once I'm sure it makes sense. |
I continue to be out of my depth on security issues, but this seems to be pointing in the right direction. What I really can't tolerate is sprinkling a little selinux over here, and some contexts over there, to the result that the codebase is littered with bits of detritus that nobody can comprehend in toto. The biggest issue for me is that we attach SecurityContext to Containers, but Volumes span containers. The traditional UNIX model of group permissions is very easy to comprehend, I'd really love to see something about that complicated. Honestly, I'd probably rather see us make Volumes work properly at the expense of per-container UIDs, if push came to shove. I hope it doesn't, of course. |
Make this easy or nobody will get it right. How does this whole mess degrade when NOT using selinux? |
@thockin When not using SELinux, the whole determination of the SELinux context should be skipped and no actions should be taken in the form of:
|
Please don't push the "is selinux enabled" into Volume plugins - it will On Wed, Jul 29, 2015 at 10:36 AM, Paul Morie notifications@github.com
|
@thockin Agree -- there will be some conditionals around 'do i need to add this argument to mount', but I don't think that's avoidable. |
If we can get groups to work I think a pod should have a group, done, end Hypothetically: Pod Everything except UID goes to pod level security context. One user On Wed, Jul 29, 2015 at 1:19 PM, Tim Hockin notifications@github.com
Clayton Coleman | Lead Engineer, OpenShift |
This work is competed, see above proposals. |
Now that #7343 is in, we can begin discussing the relationship between SecurityContexts and Volumes. There are a couple different facets:
rootcontext
should come from the pod's SELinux context.@thockin @erictune @pweil- @smarterclayton @bgrant0607
The text was updated successfully, but these errors were encountered: