Conversation
Member
Author
|
Needs a rebase when #1518 is merged |
|
Build action triggered: https://github.com/flatcar/scripts/actions/runs/7395050910 |
Member
Author
|
I've started a test run and when that passes I'll try again with flatcar/mantle#487 |
Member
Author
|
The |
Member
Author
|
We get a lot of denials - the image only works in permissive mode: |
Member
|
I think that if we want to start labeling the whole filesystem, we should at least start enabling selinux USE flag globally and pull in the missing |
tormath1
had a problem deploying
to
development
November 29, 2024 08:50 — with 
GitHub Actions
Failure
GitHub Actions
Failure
So far we did not correctly label /usr because it broke certain things like Docker. With the sysext Docker and new policies we should try again. First generate the policy before branching off the base squashfs (which already misses a lot of things because they the most postprocessing is done late in finish_image!). Then label /usr and also the sysext contents in their folder - not in the overlay mount because this would operate on the whole image.
This is missing for containerd and docker labels: Current: ``` $ selabel_lookup -k /usr/bin/docker Default context: system_u:object_r:bin_t:s0 ``` Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com>
tormath1
requested a deployment
to
development
September 8, 2026 12:43 — with
GitHub Actions
Waiting
GitHub Actions
Waiting
3 tasks
tormath1
marked this pull request as draft
September 8, 2026 12:45
There was a problem hiding this comment.
🟡 Changes recommended
There are a couple of concrete correctness/consistency issues (notably setfiles path symlink handling in build_sysext, plus KEYWORDS consistency for riscv) that should be addressed before landing.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Updates the image build flow to generate SELinux policy earlier and apply file labeling to /usr and systemd-sysext contents, aiming to avoid previous Docker breakage while getting correct contexts into the immutable image artifacts.
Changes:
- Add
sec-policy/selinux-dockerto the base OS dependency set and portage-stable package list. - Split out
finish_image_common()to build SELinux policy before branching off the base squashfs, then extend labeling to/usrduringfinish_image. - Label sysext
/usrcontents directly inbuild_sysextafter the overlay unmount.
File summaries
| File | Description |
|---|---|
| sdk_container/src/third_party/portage-stable/sec-policy/selinux-docker/selinux-docker-9999.ebuild | New live ebuild for SELinux Docker policy. |
| sdk_container/src/third_party/portage-stable/sec-policy/selinux-docker/selinux-docker-2.20260616_p1.ebuild | New versioned ebuild for SELinux Docker policy. |
| sdk_container/src/third_party/portage-stable/sec-policy/selinux-docker/selinux-docker-2.20260312_p1.ebuild | New versioned ebuild for SELinux Docker policy. |
| sdk_container/src/third_party/portage-stable/sec-policy/selinux-docker/selinux-docker-2.20250618_p1.ebuild | New versioned ebuild for SELinux Docker policy. |
| sdk_container/src/third_party/portage-stable/sec-policy/selinux-docker/metadata.xml | Adds Gentoo SELinux team metadata for the new package. |
| sdk_container/src/third_party/portage-stable/sec-policy/selinux-docker/Manifest | Adds distfile entries required by the new policy package. |
| sdk_container/src/third_party/coreos-overlay/coreos-base/coreos/coreos-0.0.1.ebuild | Pulls selinux-docker policy into the base OS SELinux policy set. |
| build_sysext | Adds setfiles labeling for sysext /usr after overlay unmount. |
| build_library/prod_image_util.sh | Calls finish_image_common() prior to sysext branching in prod image builds. |
| build_library/dev_container_util.sh | Calls finish_image_common() prior to finish_image() in dev container builds. |
| build_library/build_image_util.sh | Introduces finish_image_common() and expands SELinux labeling to include /usr. |
| .github/workflows/portage-stable-packages-list | Ensures selinux-docker is included in the portage-stable package list. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 6
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
245
to
248
| # Unmount in order to get rid of the overlay | ||
| umount "${BUILD_DIR}/${FLAGS_install_root_basename}" | ||
| sudo setfiles -Dv -r "${BUILD_DIR}/${FLAGS_install_root_basename}" "${BUILD_DIR}/fs-root"/etc/selinux/mcs/contexts/files/file_contexts "${BUILD_DIR}/${FLAGS_install_root_basename}"/usr | ||
| umount "${BUILD_DIR}/fs-root" |
Comment on lines
+12
to
+14
| if [[ ${PV} != 9999* ]] ; then | ||
| KEYWORDS="amd64 arm arm64 x86" | ||
| fi |
Comment on lines
+12
to
+14
| if [[ ${PV} != 9999* ]] ; then | ||
| KEYWORDS="amd64 arm arm64 x86" | ||
| fi |
Comment on lines
+12
to
+14
| if [[ ${PV} != 9999* ]] ; then | ||
| KEYWORDS="~amd64 ~arm ~arm64 ~x86" | ||
| fi |
Comment on lines
+12
to
+14
| if [[ ${PV} != 9999* ]] ; then | ||
| KEYWORDS="~amd64 ~arm ~arm64 ~x86" | ||
| fi |
Comment on lines
+535
to
+554
| finish_image_common() { | ||
| local image_name="$1" | ||
| local disk_layout="$2" | ||
| local root_fs_dir="$3" | ||
| local image_contents="$4" | ||
| local image_contents_wtd="$5" | ||
| local image_kernel="$6" | ||
| local pcr_policy="$7" | ||
| local image_grub="$8" | ||
| local image_shim="$9" | ||
| local image_kconfig="${10}" | ||
| local image_initrd_contents="${11}" | ||
| local image_initrd_contents_wtd="${12}" | ||
| local image_disk_space_usage="${13}" | ||
|
|
||
| # Build the selinux policy | ||
| if pkg_use_enabled coreos-base/coreos selinux; then | ||
| sudo chroot "${root_fs_dir}" bash -c "cd /usr/share/selinux/mcs && semodule -s mcs -i *.pp" | ||
| fi | ||
| } |
This file contains hidden or 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
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.
So far we did not correctly label /usr because it broke certain things like Docker. With the sysext Docker and new policies we should try again.
First generate the policy before branching off the base squashfs (which already misses a lot of things because they the most postprocessing is done late in finish_image!). Then label /usr and also the sysext contents in their folder - not in the overlay mount because this would operate on the whole image.
How to use
Hope that setfiles is clever enough
Verify with flatcar/mantle#487
Testing done
The sysext contents have the right label:
changelog/directory (user-facing change, bug fix, security fix, update)/bootand/usrsize, packages, list files for any missing binaries, kernel modules, config files, kernel modules, etc.