Fix selinux aliases for /usr/etc/systemd/system

[alexlarsson]

The previous fix for /usr/etc
(#4640) was only partial. We also need to make sure /usr/etc/systemd/systemd is aliased, because aliases are not applied recursively, so the original alias for /etc/systemd/system is not applied.

[gemini-code-assist]

Code Review

This pull request fixes a partial SELinux alias for /usr/etc by adding a specific alias for /usr/etc/systemd/system. The change in rust/src/composepost.rs is correct and addresses the issue of non-recursive aliases. My only suggestion is to add a comment to explain the new alias for better code maintainability, consistent with the existing style in the file. The update to the libglnx submodule is also noted.

[jlebon]

Hmm, we already today somewhat parse the file. So in theory, we could also check for any file that starts with /etc and automatically add the /usr/etc equivalent for it too, right? That'd make this less whack-a-mole-y.

Though honestly feels like an RFE against libselinux to actually recursively apply substitutions.

[jlebon]

Hmm, we already today somewhat parse the file. So in theory, we could also check for any file that starts with /etc and automatically add the /usr/etc equivalent for it too, right? That'd make this less whack-a-mole-y.

Basically something like

diff --git a/rust/src/composepost.rs b/rust/src/composepost.rs
index 3c94e0fa..2af0daed 100644
--- a/rust/src/composepost.rs
+++ b/rust/src/composepost.rs
@@ -401,6 +401,9 @@ fn postprocess_subs_dist(rootfs_dfd: &Dir) -> Result<()> {
                     if line.starts_with("/var/home ") {
                         writeln!(w, "# https://github.com/projectatomic/rpm-ostree/pull/1754")?;
                         write!(w, "# ")?;
+                    } else if line.starts_with("/etc/") {
+                        writeln!(w, "# https://github.com/coreos/rpm-ostree/pull/4640")?;
+                        write!(w, "/usr{}", line)?;
                     }
                     writeln!(w, "{}", line)?;
                 }

(Untested to be clear)

[alexlarsson]

/retest

[alexlarsson]

@jlebon Sure, that seems like a better solution.

[alexlarsson]

Although its not really all that likely that there will be a lot more /etc aliases.

[cgwalters]

Let's go with jlebon's suggestion

[alexlarsson]

also, /usr/etc is already in the file since this change: fedora-selinux/selinux-policy@56a0bacb
so we should avoid doing it twice.

[alexlarsson]

Also, the ordering is important here. The /usr/etc/systemd line has to be after the /usr/etc one for this to work.

[openshift-ci]

@alexlarsson: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/fcos-e2e	4dd8496	link	true	/test fcos-e2e

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

[cgwalters]

I think this looks sane, and if you've tested it I'm OK with it. However I'm going to do a followup to add some unit tests here

[cgwalters]

Is the trailing space here intentional?

[alexlarsson]

yes, this is to catch the base rule, like "/usr/etc /etc". It should not match e.g. "/usr/etc/foo /other"

[alexlarsson]

I did test it in the context of automotive/osbuild use, and it worked. With this my bootc generated commit gets zero differences compared to base commit.

[cgwalters]

Potential fallout in coreos/fedora-coreos-tracker#2030