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
Add --security-opts options to allow user to customize container labels. #7425
Conversation
@rhatdan can you give me a couple of examples to test this one my machine? I guess I can set some options then look at the process labels? |
Docs LGTM |
I would love to have an example of how and why to use this in the documentation - otherwise, yup, Docs LGTM too |
I added some examples. |
If you wanted to try out the svirt_apache_t example, here is some simple policy I threw together for it. allow svirt_apache_t self:tcp_socket create_stream_socket_perms; sysnet_dns_name_resolve(svirt_apache_t) Now run make -f /usr/share/selinux/devel/Makefile svirt_apache.ppsemodule -i svirt_apache.ppNow you should be ready to use the svirt_apache_t type. docker run --label-opt type:svirt_apache_t -v /usr:/usr --rm -ti fedora /bin/sh Of course there is probably more policy you would need to write to fully get apache to run in this environment, but it is a quick example of how to do it. |
## Using alternative labeling | ||
|
||
If you want to use the same label for multiple containers you can override use | ||
the label-opt flag to select an MCS level. THis is a common practive for MLS |
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.
This :)
merci for the example - and the one in here. @fredlf it would be great if we can work out how to flesh out the Apache example Dan's given us here post merge. Docs LGTM - minor nits can be carried by whomever touches the PR with git next :) |
Code LGTM, is there a way to add some test ? |
@vieux Do we have any way to test docker run? Do you have examples? docker run --rm -ts --label-opt level:s0:c0,c100 ps -eZ | grep s0:c0,c100 docker run --rm -ts --label-opt disable ps -eZ | grep -v svirt Would be another. |
@rhatdan See integration-cli for |
@rhatdan I know you're looking at this from a SELinux perspective, but I'm wondering whether @smarterclayton can offer an opinion on whether the same functionality can do double-duty for a label-based orchestration tech like kubernetes. It would be fantastic if the |
Modifying a container while it is running, really has no precedent from docker, as far as I know. I know that google and Red Hat have been working on docker exec, but this is about adding a process to a running container. The reason I called this --label-opt rather then --selinux-opt was to allow other labeling systemd like smack to be able to use the same options. But if you can think of other tools using it, that is fine with me. As long as we don't overload the name value pairs. |
I know there was a desire to have things like labels in Docker (metadata related to a container that could be set by consumer applications for identification / tweaking), but I suspect it would not overlap with selinux labels unless there was a really strong 1-1 connection, because of side effects (i.e. you use a label to identify a container that means something different in selinux). |
Yeah, we'd really like to be able to attach semi-arbitrary metadata to containers (and be able to query that back), but this seems like it is coupled to selinux at a low level, unless I am misunderstanding |
That is a good point. Maybe a better flag name for this feature could be something like |
Main questions seem to be around terminology. SELinux needs a concept of immutable labels which are set at container startup, and can't be modified. Orchestration technologies - eg. Kubernetes, and if I'm understand correctly, openshift in future, look for a more dynamic labelling feature that can be added and removed. Normally I'd think of that as Tags, but Docker already has a meaning for Tag, and Google calls in labels in their context. https://github.com/GoogleCloudPlatform/kubernetes/blob/master/docs/labels.md I guess the points I'm aiming for are: b) if there are only 2 that are considered critical, should they have slightly more targetted names? Even without kubernetes running in my environments, I'd really like to be able to easily label running containers with their current status/purpose etc, and script my other parts of my systems to recognise that. A particular deployment might be an RC, and then after testing I want to promote it to production and rebalance over it, without making any changes to the actual running state of the containers. That would also potentially require adding/removing labels to interact with the emitted docker events, which again might be quite different than this specific use-case. Just brainstorming, but the above feature is still K,V pairs, so has the overall interface required. Would it overcomplicate things too much if the labels concept extended to having a namespace type feature? Docker already does :repo/:image/[:tag] what if for @rhatdan use case this was
and orchestration techs etc could do
Then specific 3rd party services, like selinux, kubernetes, openshift (which is using pods in future I think) or anything else could pay attention only to the labels that correspond to their namespace. |
I agree with @thaJeztah, what you are describing is more the META patch which I have a pull request available. My pull request is more about labeling the content of the image, you could easily extend that concept to add labels or META data about the running container. @crosbymichael I do not want to move SElinux into the label, since I think other security labeling systems could take advantage of the options. Maybe security-opt might be better to prevent confusion like the kubernetes guys are having. |
Agree with @crosbymichael, this should be called something like |
I have no problem, except that --security-option might lead people to look at configuring capabilities with it. But we are looking to add a new patch with libseccomp functionality, and we are going to want options for that. I will change to --security-opt flag. |
Maybe it should have something like
So that it sends these options to the labeling system |
Well I changed to --security-opt, but did not do the label:user:USER change. |
@crosbymichael Switched to using --security-opt label:user:USER Since we are working on the seccomp subsystem, this made sense. |
Ok, thanks. I'll review this week |
ping @shykes please review this new flag. We can also use this to set apparmor profiles via |
6f14c26
to
96ed3a6
Compare
e375ae4
to
b34f874
Compare
b34f874
to
c1390b8
Compare
@crosbymichael @shykes @cpuguy83 Anything I can do to move this one along? |
…guration security-opts will allow you to customise the security subsystem. For example the labeling system like SELinux will run on a container. --security-opt="label:user:USER" : Set the label user for the container --security-opt="label:role:ROLE" : Set the label role for the container --security-opt="label:type:TYPE" : Set the label type for the container --security-opt="label:level:LEVEL" : Set the label level for the container --security-opt="label:disabled" : Turn off label confinement for the container Since we are passing a list of string options instead of a space separated string of options, I will change function calls to use InitLabels instead of GenLabels. Genlabels interface is Depracated. Docker-DCO-1.1-Signed-off-by: Dan Walsh <dwalsh@redhat.com> (github: rhatdan)
c1390b8
to
ead5d45
Compare
In order to get the --ipc command to work well with SELinux, I will need this patch to be merged? |
Ok, thanks for letting us know |
plabel, _, _ := label.InitLabels(nil) | ||
if plabel != "" { | ||
defer deleteAllContainers() | ||
cmd := exec.Command(dockerBinary, "run", "--security-opt", "label:disable", "busybox", "ps", "-eZ") |
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.
busybox
compiled ps
doesn't support -Z
Some docs issues. Please LMK if my comments aren't clear. |
@@ -114,4 +114,5 @@ type Command struct { | |||
CapDrop []string `json:"cap_drop"` | |||
ContainerPid int `json:"container_pid"` // the pid for the process inside a container | |||
ProcessConfig ProcessConfig `json:"process_config"` // Describes the init process of the container. | |||
SecurityOpt []string `json:"security_opt"` |
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 don't see where this one is filled from config.SecurityOpt
security-opts will allow you to customise the way that a labeling system like
SELinux will run on a container.
Since we are passing a list of string options instead of a space separated
string of options, I will change function calls to use InitLabels instead of
GenLabels. Genlabels interface is Depracated.
Docker-DCO-1.1-Signed-off-by: Dan Walsh dwalsh@redhat.com (github: rhatdan)