Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add MCS constraints to the SELinux policy #1733

Merged
merged 2 commits into from
Sep 28, 2021
Merged

add MCS constraints to the SELinux policy #1733

merged 2 commits into from
Sep 28, 2021

Conversation

bcressey
Copy link
Contributor

@bcressey bcressey commented Sep 10, 2021 •
edited
Loading

Issue number:
Fixes #1712
#1569

Description of changes:
This adds mlsconstrain statements to the SELinux policy so that the MCS category pairs assigned to container processes and files are used for access decisions.

The CIL documentation on mlsconstrain is helpful to understand what's happening, but briefly:

  • dom is the dominance test described in the comments
  • h1 is the source "high" context (typically a subject / process)
  • h2 is the target "high" context (either an object / file, or another subject / process)

The "low" context (l1, l2) should be just s0 for everything (with no categories), and is not useful for making the access decision.

Some special cases, since it's hard to write useful comments about what's not included:

  • (files (load)) has an exception for "subject" files (like /proc/<pid>/<file>), so ps works for everyone.
  • (files (describe)) does not have a constraint, so ls works for everyone.
  • (files (relabel)) and (processes (transform)) are "relabel" actions; they're not constrained because they're currently reserved for trusted (read: "not container") processes.

Testing done:
Tested these scenarios.

Action Privileged container Unprivileged container
Create files in an emptyDir volume ✅ ✅
Create files in a hostPath volume ✅ ✅
Create files in an Amazon EBS volume ✅ ✅
Create files in an Amazon EFS volume ✅ ✅
Create files in a Docker image volume ✅ ✅
Signal processes with a different level ✅ ❌
List files in a directory with a different level ✅ ❌
Create files in a directory with a different level ✅ ❌
Relabel files with the same level ✅ ✅
Relabel files with a different level ❌ ❌
Relabel processes with the same level ✅ ✅
Relabel processes with a different level ❌ ❌
List files in host's /etc ✅ ❌
Create files in host's /etc ❌ ❌
List files for host's datastore ✅ ❌
Create files in host's datastore ❌ ❌

Verified that podman and pip can run inside a container without unexpected denials.

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.

@bcressey
Copy link
Contributor Author

Added a new rule to prevent an untrusted process from altering its own range.

@bcressey
Copy link
Contributor Author

Modified the rules so that untrusted processes cannot adjust their own labels, even if the resulting value is the same. This aligns with the rules for relabeling files.

@bcressey bcressey marked this pull request as ready for review September 14, 2021 22:02
@bcressey
Copy link
Contributor Author

Fixed a typo spotted by @arnaldo2792.

arnaldo2792
arnaldo2792 approved these changes Sep 15, 2021
View reviewed changes
Copy link
Contributor

@arnaldo2792 arnaldo2792 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@bcressey
selinux-policy: relocate some label permissions
cf75284 
This moves certain "odd" permissions from the sets that control file
relabels and process transitions, to the more general set of actions
for managing system behavior.

The goal is to allow container processes to perform "no-op" relabels
and transitions, subject to policy constraints to ensure that the old
and new labels do not actually change.

The relabel actions for the `filesystem` class aren't constrained by
`mlsvalidatetrans` rules, so there's no way to assert that the label
would be unchanged afterwards.

It's not clear what the actions for the `kernel_service` class are
meant to do. The reference policy only allows them for `cachefilesd`,
so it should be fine to reserve them for trusted processes.

Signed-off-by: Ben Cressey <bcressey@amazon.com>
@bcressey
selinux-policy: implement MCS isolation
d657417 
Docker and containerd's CRI plugin currently allocate two random
categories for all containers, and use them to label processes and
mounts. The intent is to add an additional layer of access control
between mutually untrusting containers. However, this is not enforced
by SELinux automatically; the policy needs rules to govern access.

This adds mlsconstrain rules to the policy so that these categories
actually have an effect. The constraints are based on the ones found
in the SELinux Reference Policy:
  https://github.com/SELinuxProject/refpolicy/blob/master/policy/mcs

They aren't exactly the same since our type attribute sets and class
permission sets are not as fine-grained. However, the broad strokes
are similar. File, directory, and process actions will be restricted
unless the subject's label dominates the target's label, which is
only true for targets in the same container, or for subjects that are
privileged.

All processes could previously set their existing label as the label
to use for newly created processes; this is now constrained so that
the level and categories cannot change unless the process is trusted.

Untrusted processes could not previously perform any relabel actions.
This is relaxed slightly to permit processes in containers to relabel
files in the container, but constrained so that the effective label
cannot change. This fixes programs (`pip`, `podman`) that attempt to
preserve the extended attributes of source files when copying them.

Signed-off-by: Ben Cressey <bcressey@amazon.com>
@bcressey
Copy link
Contributor Author

Added support for "no-op" relabeling actions where the same label is reapplied. Updated local tests to verify that relabeling actions that do not change the label are allowed, while those that change the label are denied.

@bcressey bcressey mentioned this pull request Sep 16, 2021
Closed
@peimanja peimanja mentioned this pull request Sep 27, 2021
Merged
@peimanja
Copy link

any update on this one?

@bcressey
Copy link
Contributor Author

It should be merged soon, just waiting on one more review since I reworked it a bit since the reviewers last saw it.

Let me know if there's a test case that you're interested in that you'd like me to try?

@peimanja
Copy link

peimanja commented Sep 28, 2021 •
edited
Loading

Let me know if there's a test case that you're interested in that you'd like me to try?

Thanks @bcressey! Basically we run github actions on self-hosted runners on Bottlerocket with dind runners for building images.

Workaround you've mentioned in #1569 (comment) works fine but would be nice to get a cleaner way of doing that.

samuelkarp
samuelkarp approved these changes Sep 28, 2021
View reviewed changes
Copy link
Contributor

@samuelkarp samuelkarp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bcressey
Copy link
Contributor Author

Workaround you've mentioned in #1569 (comment) works fine but would be nice to get a cleaner way of doing that.

I've been using a version of the pod spec from that issue, with privileged: true and without seLinuxOptions, and it works fine after these changes.

@bcressey bcressey merged commit d288005 into bottlerocket-os:develop Sep 28, 2021
@bcressey bcressey deleted the mcs-isolation branch September 28, 2021 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arnaldo2792 arnaldo2792 arnaldo2792 approved these changes

@samuelkarp samuelkarp samuelkarp approved these changes

@tjkirch tjkirch tjkirch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

enable MCS isolation in the SELinux policy
5 participants
@bcressey @peimanja @tjkirch @samuelkarp @arnaldo2792