Open Stack

On 12/13/18 7:16 PM, Mikhail Fedosin wrote:
> Hello!
>
> One of the most important tasks for this release cycle is to provide
> full SELinux support in the installed overcloud. Some work in this
> direction has already been done and in many respects, due to the
> recent changes, support is almost ready.
>
> However, I would like to ask a few questions about what else can be
> improved.
> In short, to provide SELinux support the main challenge for us is to
> ensure that docker can write to the directories protected by SELinux.
> To allow writing there we change the directory type to
> container_file_t or its alias svirt_sandbox_file_t.
>
> There are two ways to do this:
> 1. Explicitly - specify the type when creating a directory.
> # semanage fcontext -at container_file_t "/my_folder(/.*)?"
> # restorecon -R /my_folder
> # docker run -v /my_folder:/tmp/my_folder ...
> 2. Implicitly - use the suffix :z when mounting the partition when
> creating a container.
> # docker run -v /my_folder:/tmp/my_folder:z ...
>
> Both methods change selinux type of /my_folder to container_file_t and
> allow to write to the section inside the container.
> You can read more about this feature in the article
> https://www.projectatomic.io/blog/2015/06/using-volumes-with-docker-can-cause-problems-with-selinux/
> To see how it works, I wrote a small playbook that creates two
> directories on the host system and modifies their selinux types in two
> ways: http://paste.openstack.org/show/737234/
>
> Currently there is no consensus in TripleO which of the ways to use.
> For example, in many cases both methods are used, which adds
> unnecessary redundancy:
> https://github.com/openstack/tripleo-heat-templates/blob/89d13583f7c706d5c7fbd5052fbedec53474f248/docker/services/cinder-api.yaml#L193-L194
> https://github.com/openstack/tripleo-heat-templates/blob/89d13583f7c706d5c7fbd5052fbedec53474f248/docker/services/cinder-api.yaml#L218-L219
>
> In some cases there is only :z
> https://github.com/openstack/tripleo-heat-templates/blob/89d13583f7c706d5c7fbd5052fbedec53474f248/docker/services/liquidio-compute-config.yaml#L97
>
> In some, only explicit setting of svirt_sandbox_file_t on the directory
> https://github.com/openstack/tripleo-heat-templates/blob/89d13583f7c706d5c7fbd5052fbedec53474f248/docker/services/swift-proxy.yaml#L219
> https://github.com/openstack/tripleo-heat-templates/blob/89d13583f7c706d5c7fbd5052fbedec53474f248/docker/services/swift-proxy.yaml#L248
>
> Some services still do not have SELinux support:
> https://github.com/openstack/tripleo-heat-templates/blob/89d13583f7c706d5c7fbd5052fbedec53474f248/docker/services/etcd.yaml
> https://github.com/openstack/tripleo-heat-templates/blob/89d13583f7c706d5c7fbd5052fbedec53474f248/docker/services/zaqar.yaml
> https://github.com/openstack/tripleo-heat-templates/blob/89d13583f7c706d5c7fbd5052fbedec53474f248/docker/services/tacker.yaml
>
> Such inconsistency leads to hard to find bugs and makes it difficult
> to understand the code. 
> An example of such a bug could be this:
> Here we set selinux type to svirt_sandbox_file_t for /var/log/swift
> https://github.com/openstack/tripleo-heat-templates/blob/89d13583f7c706d5c7fbd5052fbedec53474f248/docker/services/swift-proxy.yaml#L249
> And rewrite it back later:
> https://github.com/openstack/tripleo-heat-templates/blob/89d13583f7c706d5c7fbd5052fbedec53474f248/docker/services/swift-storage.yaml#L462
> For this reason, I want to come to an agreement on how we will
> implement SELinux support.
>
> In my understanding, the best solution would be to use the suffix :z
> only. Its advantage is that docker (and podman) checks the directory's
> selinux type before launching the container and changes it
> accordingly. In other words, if the type was accidentally changed
> during the execution of the container, then when it is restarted ":z"
> will return the type back. In the case of explicit type setting, we
> get an error.
>
> Therefore, I propose to remove the explicit setting
> svirt_sandbox_file_t and add the suffix ":z" where it is still missing.
>
> What do you think?
This sounds like a reasonable solution. I actually didn't know we were
expliclty changing stuff to svirt_sandbox_file_t, and thought we were
solely relying on :z. Thanks for bringing this up. I like your proposal.
>
> Best regards,
> Mikhail Fedosin