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.

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

selinux: perform relabeling from initrd #846

Merged
merged 4 commits into from
Dec 6, 2019

Conversation

jlebon
Copy link
Member

@jlebon jlebon commented Aug 20, 2019

Now that we can relabel from the initrd, we should be able to
categorically resolve all SELinux and Ignition issues.

Here, we drop the hacky ignition-relabel.service and relabel all the
files we need to on the spot!

Requires: https://lore.kernel.org/selinux/20190819193032.848-1-jlebon@redhat.com/

Closes: #635

internal/exec/stages/files/files.go Outdated Show resolved Hide resolved
internal/exec/stages/files/files.go Outdated Show resolved Hide resolved
@jlebon
Copy link
Member Author

jlebon commented Oct 1, 2019

Upstream patch was just merged: https://lore.kernel.org/selinux/20190912133007.27545-1-jlebon@redhat.com/T/#m5f950ca9fc3ed374cb793fc9aed0435602b1b6ad. 🎉

It should be in the 5.5 release.

@jlebon jlebon force-pushed the pr/label-from-initrd branch from d8772fb to af99649 Compare November 1, 2019 20:14
@jlebon jlebon changed the title selinux: add new mode for relabeling from initrd selinux: perform relabeling from initrd Nov 1, 2019
@jlebon
Copy link
Member Author

jlebon commented Nov 1, 2019

OK rebased this! Since the kernel in FCOS today already supports this, and we have the spec2x branch, I went more aggressive this time and nuked all the previous code we had around this.

Still chasing down some bugs and doing some more testing, but works pretty well overall!

@jlebon jlebon force-pushed the pr/label-from-initrd branch from af99649 to 72e876f Compare November 26, 2019 19:15
@jlebon
Copy link
Member Author

jlebon commented Nov 26, 2019

OK, so this is failing on:

+ docker run --rm -e TARGET=amd64 -v '/opt/jenkins/workspace/os/Ignition BB tests:/usr/src/myapp' -w /usr/src/myapp quay.io/slowrie/ignition-builder:latest ./build_blackbox_tests
Using version from git...
Building ignition...

# github.com/coreos/ignition/v2/internal/exec/util
selabeling.c:30:29: fatal error: selinux/selinux.h: No such file or directory
 #include <selinux/selinux.h>
                             ^

Where is slowrie/ignition-builder:latest defined? Is it built from images/Dockerfile?

@arithx
Copy link
Contributor

arithx commented Nov 26, 2019

@jlebon correct, it's built from images/Dockerfile, the latest build was built based on golang:1.12 in March.

@jlebon jlebon force-pushed the pr/label-from-initrd branch from 72e876f to aa19dad Compare November 27, 2019 15:40
@jlebon
Copy link
Member Author

jlebon commented Nov 27, 2019

OK, reworked this now! The major improvement is that instead of having our own relabeling code, we just use setfiles. I had initially explored this, but I couldn't get around setfiles really wanting a loaded policy in order to validate the labels it writes. Adding -F fixes this (even though the man page doesn't make that obvious).

jlebon added a commit to jlebon/ignition-dracut that referenced this pull request Nov 27, 2019
This is required by the new relabeling code in Ignition:
coreos/ignition#846

(This is another good example of why merging this repo into
coreos/ignition would be nicer I think.)
@jlebon
Copy link
Member Author

jlebon commented Nov 27, 2019

Requires: coreos/ignition-dracut#138

@jlebon jlebon marked this pull request as ready for review November 27, 2019 15:44
@jlebon
Copy link
Member Author

jlebon commented Nov 27, 2019

That said, there's a higher-level issue for FCOS/RHCOS here: there are other things created from the initramfs that also need to be relabeled.

The few things I can think of right now are:

  • Files created by the systemd-tmpfiles invocation of coreos-populate-var.service. For example, right now /var/home mount points are failing. This used to work, so I think SELinux policy rules around systemd and unlabeled dirs might have changed with the move to f31.
  • /etc/hostname created by Afterburn
  • FIPS-related files

(And I'm sure there are others I'm forgetting.)

I think I'll add a tiny helper script to make it easier for those services to invoke setfiles. Then anything that should be distro-agnostic (like Ignition) can use setfiles directly, and all the more CoreOS-specific stuff can use the helper.

jlebon added a commit to coreos/ignition-dracut that referenced this pull request Nov 27, 2019
This is required by the new relabeling code in Ignition:
coreos/ignition#846

(This is another good example of why merging this repo into
coreos/ignition would be nicer I think.)
@jlebon jlebon force-pushed the pr/label-from-initrd branch from aa19dad to 1c28c5e Compare November 29, 2019 16:55
Now that we can relabel from the initrd, we should be able to
categorically resolve all SELinux and Ignition issues.

Here, we drop the hacky `ignition-relabel.service` and relabel all the
files we need to on the spot!

Requires: https://lore.kernel.org/selinux/20190819193032.848-1-jlebon@redhat.com/

Closes: coreos#635
This is the final piece in the SELinux + Ignition saga: we need to make
sure that both the mount point we create (and any leadings we created
too), as well as the root of the filesystem we mount are labeled.
@jlebon
Copy link
Member Author

jlebon commented Nov 29, 2019

Copy link
Contributor

@ajeddeloh ajeddeloh left a comment

Choose a reason for hiding this comment

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

LGTM, this will break relabeling on anyone using Ignition v2/spec3 that doesn't have the patch, but I don't think anyone is. Plus the patch will make it into their kernels eventually. Maybe add a note in the operator notes section on SELInux?

internal/exec/util/selinux.go Show resolved Hide resolved
We now support SELinux natively! Just make a note of this, and point at
the kernel patch needed for this to work.
@jlebon
Copy link
Member Author

jlebon commented Dec 6, 2019

Maybe add a note in the operator notes section on SELInux?

Good idea, done!

Copy link
Contributor

@ajeddeloh ajeddeloh left a comment

Choose a reason for hiding this comment

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

🚢 🇮🇹

@jlebon
Copy link
Member Author

jlebon commented Dec 6, 2019

Green checkmarks = merging! (Though we should rework CI on this repo soon so it actually runs on FCOS.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revisit SELinux labeling approach
4 participants