modify default label for containers #1318
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tjkirch
approved these changes
Feb 11, 2021
bcressey
force-pushed
the
default-container-label
branch
2 times, most recently
from
1a0f7e9
to
0a5aabb
Compare
|
The two force pushes together are what I intended to be a rebase with no changes. I'll push the actual changes for this PR shortly. |
bcressey
force-pushed
the
default-container-label
branch
from
0a5aabb
to
1a0f7e9
Compare
webern
approved these changes
Feb 16, 2021
This changes the default label from `container_t`, which is meant as the unprivileged type, to `control_t`, which has higher privileges, including the ability to write to the host API socket. This is better aligned with the SELinux logic in containerd's CRI plugin and in the docker daemon, where the label from lxc_contexts is used for unprivileged containers, while privileged containers are not explicitly labeled and follow the default transition rules. Signed-off-by: Ben Cressey <bcressey@amazon.com>
This documents the switch to running privileged containers with the `control_t` label by default, and describes the implications. Signed-off-by: Ben Cressey <bcressey@amazon.com>
bcressey
force-pushed
the
default-container-label
branch
from
1a0f7e9
to
2653973
Compare
samuelkarp
approved these changes
Feb 16, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue number:
N/A
Description of changes:
The goal here is to better align our SELinux policy transitions with the behavior of Docker and containerd's CRI plugin, which expect that privileged containers will take the default path and end up with a more powerful label, while unprivileged ones will allocate an MCS pair that facilitates isolation between containers.
Although we don't yet support MCS isolation, this lays the groundwork for that by introducing a distinct label for privileged containers. That will let us add more restrictions to the
container_ttype, such as MCS constraints, without simultaneously restricting thecontrol_ttype in ways that are incompatible with expectations in the ecosystem.Note that
control_tand privileged are already quite similar in terms of their ability to affect the system:control_toffers highly privileged access to the host by means of the API, while privileged containers can bypass many access controls without using the API.Instead of repurposing the
control_ttype and granting the additional permissions to privileged containers, we could define a new type, such asprivileged_t. Maintaining a separate but similar label would require duplicating many additional rules over time; for example, both labels would need access to host sockets, and both would need to be able to bypass MCS constraints.Testing done:
I verified that Docker now uses
control_tfor "privileged" and "label disabled" containers.As part of the work to update to containerd 1.4, I also verified that
control_tis used for privileged containers launched via CRI. Until then,container_twill continue to be used because of our SELinux patches on containerd 1.3.Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.