treefile: Add option to label `/usr/etc` as `/etc` by cgwalters · Pull Request #4640 · coreos/rpm-ostree

cgwalters · 2023-10-02T15:07:20Z

This is intended for use with ostreedev/ostree#2868 but is conceptually orthogonal to it; we probably want to try switching to this by default actually.

openshift-ci · 2023-10-02T15:07:24Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

cgwalters · 2023-10-02T15:31:49Z

Running the coreos kola tests against this, ext.config.extensions.package fails with
Oct 02 15:22:01 qemu0 kola-runext-package[1876]: error: Changed directories are not supported yet

because we need to make client side layering also auto-inherit this behavior if present in the base image.

jlebon

Not strictly required, but would be good to add a substitution rule for it too so that SELinux userspace tooling is aware of this too. (Could probably just add it as part of postprocess_subs_dist().)

jlebon · 2023-10-02T16:17:54Z

docs/treefile.md

    to the UNIX epoch time to be used as the build timestamp. Currently only
    fully supports the `bdb` backend. Somewhat experimental.

+ * `label-usretc-as-etc`: boolean, optional: If enabled, this sets the SELinux label for


I wonder if we even need a new knob for this. Should be safe to always do this, right? Since we basically own /usr/etc anyway.

Yes, I'm being conservative here by making it opt in (both here and on the ostree side).

Dunno. I guess we could try to actually do it by default ostree side, and add an option to disable it in case we regress things somehow.

This is a nontrivial change though, and so far we've been really conservative with options that should be on by default but aren't (sysroot.readonly is a good example) to ensure that updating ostree doesn't break anything.

Yeah, I'm less sure about making it default in libostree. I think it makes sense to make it opt in there. For rpm-ostree, it seems safer to do but cool of course to be conservative there.

I think making it opt-in in ostree. but always enable in rpm-ostree (possibly with an opt-out) is the right approach.

It's painful every time we add an opt-in for sure because we end up having to update so many different manifest/treefiles (coreos vs edge vs atomic desktop etc etc.)

However I still just feel we need to be conservative because this can easily have non-obvious effects.

Even on-by-default with a knob to turn it off gets tricky because then the build system requires a new rpm-ostree (old ones will barf on the unexpected field).

Not only that, we need a new version of osbuild, and any other tools that call out to rpm-ostree to compose a commit.

alexlarsson · 2023-10-13T07:48:53Z

So, all the base stuff in ostree has landed, maybe time to un-draft this?

cgwalters · 2023-10-20T20:55:36Z

Well, we have the annoying thing that we have to wait through the new ostree to go all the way through the fedora 7 days bodhi update right now.

I just did coreos/fedora-coreos-config#2684
which should cut that down to one day at least...

cgwalters · 2023-10-24T18:24:58Z

Ah right, the ostree update hasn't made it out to c9s prod. Let's split out that hack to #4672

Man, it's times like this that really makes one feel actively punished for splitting things out into multiple components...

(Or of course, using Nix where we aren't playing "rpmfind" but are just referencing source code that gets built if not present in the build cache...)

Prep for new release.

ci/installdeps.sh

+
+    osid="$(. /etc/os-release && echo $ID)"
+    if [ "${osid}" == centos ]; then
+        dnf -y update https://kojihub.stream.centos.org/kojifiles/packages/ostree/2023.7/2.el9/$(arch)/ostree-{,libs-,devel-}2023.7-2.el9.$(arch).rpm


ci/installdeps.sh

+
+    osid="$(. /etc/os-release && echo $ID)"
+    if [ "${osid}" == centos ]; then
+        dnf -y update https://kojihub.stream.centos.org/kojifiles/packages/ostree/2023.7/2.el9/$(arch)/ostree-{,libs-,devel-}2023.7-2.el9.$(arch).rpm


jlebon

LGTM overall. Thoughts on #4640 (review) ?

tests/compose/test-misc-tweaks.sh

Depends on ostreedev/ostree#3063 This is intended for use with ostreedev/ostree#2868 but is conceptually orthogonal to it; we probably want to try switching to this by default actually.

cgwalters · 2023-10-24T22:50:03Z

Not strictly required, but would be good to add a substitution rule for it too so that SELinux userspace tooling is aware of this too. (Could probably just add it as part of postprocess_subs_dist().)

Hmmm. I think what's a bit weird about that is logically nothing else should put things in /usr/etc. It's effectively an ostree implementation detail. It's hence not totally clear to me what we'd gain by doing that then.

That said I had completely forgotten that postprocess_subs_dist existed. I feel like it'd be better for it not to exist at all? Hmm, why did we not land the /home -> /var/home equivalency in upstream policy?

jlebon · 2023-10-25T14:15:29Z

Not strictly required, but would be good to add a substitution rule for it too so that SELinux userspace tooling is aware of this too. (Could probably just add it as part of postprocess_subs_dist().)

Hmmm. I think what's a bit weird about that is logically nothing else should put things in /usr/etc. It's effectively an ostree implementation detail. It's hence not totally clear to me what we'd gain by doing that then.

One concrete example is doing usroverlay and running restorecon on /usr/etc would mess up all the labels again.

That said I had completely forgotten that postprocess_subs_dist existed. I feel like it'd be better for it not to exist at all? Hmm, why did we not land the /home -> /var/home equivalency in upstream policy?

The history there is messy, but I think we do still need it. IIRC this is related to the /etc/default/useradd hack we do. That lives here so it makes sense to have this live alongside it. Lots more context in https://src.fedoraproject.org/rpms/selinux-policy/pull-request/14 and #1754.

This avoids the unnecessary `b` for bytestrings and literal `\n` newlines.

This ensures file labeling equivalency outside of ostree too.

jlebon

LGTM!

This is a feature that was added in rpm-ostree 2023.10 and is needed for the new transient /etc feature to work. What it does is change the labeling of /usr/etc to match those of /etc, so that /usr/etc can be used directly as a bind-mount or an overlay mount when mounted on /etc. See coreos/rpm-ostree#4640 for details.

alexlarsson · 2025-09-08T14:42:42Z

It turns out this isn't quite complete. There is a pre-existing alias already:

/etc/systemd/system /usr/lib/systemd/system

Which means that /etc/systemd/system is special:

# matchpathcon /etc/systemd/system/foo.service
/etc/systemd/system/foo.service	system_u:object_r:systemd_unit_file_t:s0

However, the aliases are not recursively applied, so this rule doesn't apply to /usr/etc/systemd/system:

matchpathcon /usr/etc/systemd/system/foo.service
/usr/etc/systemd/system/foo.service	system_u:object_r:etc_t:s0

To fix this we need to also add:

/usr/etc/systemd/system /usr/lib/systemd/system

The previous fix for /usr/etc (coreos#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.

The previous fix for /usr/etc (coreos#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. Also, since fedora-selinux/selinux-policy@56a0bacb this file already contains an alias for /usr/etc, so avoid duplicating it. Rather than hardcoding /usr/etc/systemd/system, we scan for all aliases in /etc, and duplicate them in /usr. Note: Order is important, the /usr/etc/.. subdir aliases must come after the main /usr/etc alias.

openshift-ci bot added the do-not-merge/work-in-progress label Oct 2, 2023

cgwalters mentioned this pull request Oct 2, 2023

repo: Add an option to label /usr/etc as /etc ostreedev/ostree#3063

Merged

jlebon reviewed Oct 2, 2023

View reviewed changes

cgwalters mentioned this pull request Oct 2, 2023

Support transient /etc ostreedev/ostree#3062

Merged

cgwalters mentioned this pull request Oct 20, 2023

buildroot: Enable testing-devel coreos/fedora-coreos-config#2684

Merged

cgwalters marked this pull request as ready for review October 24, 2023 14:43

openshift-ci bot removed the do-not-merge/work-in-progress label Oct 24, 2023

cgwalters force-pushed the label-usretc-as-etc branch 3 times, most recently from 5dfdf32 to a70e776 Compare October 24, 2023 16:59

Hard require ostree 2023.7

d24e0f2

Prep for new release.

cgwalters force-pushed the label-usretc-as-etc branch from a70e776 to b49dbb1 Compare October 24, 2023 19:55

github-advanced-security bot found potential problems Oct 24, 2023

View reviewed changes

jlebon approved these changes Oct 24, 2023

View reviewed changes

tests/compose/test-misc-tweaks.sh Show resolved Hide resolved

treefile: Add selinux-label-version

5083302

Depends on ostreedev/ostree#3063 This is intended for use with ostreedev/ostree#2868 but is conceptually orthogonal to it; we probably want to try switching to this by default actually.

cgwalters force-pushed the label-usretc-as-etc branch from b49dbb1 to 5083302 Compare October 24, 2023 22:03

cgwalters enabled auto-merge (rebase) October 24, 2023 22:45

cgwalters added 2 commits October 25, 2023 16:12

composepost: use writeln!

c477309

This avoids the unnecessary `b` for bytestrings and literal `\n` newlines.

composepost: Add /usr/etc -> /etc to SELinux subs_dist

bbc455e

This ensures file labeling equivalency outside of ostree too.

jlebon approved these changes Oct 25, 2023

View reviewed changes

cgwalters merged commit 8c83311 into coreos:main Oct 25, 2023

alexlarsson mentioned this pull request Oct 27, 2023

Add selinux-label-version to the org.osbuild.ostree.commit stage osbuild/osbuild#1415

Merged

alexlarsson mentioned this pull request Sep 8, 2025

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

Merged

Conversation

cgwalters commented Oct 2, 2023

Uh oh!

openshift-ci bot commented Oct 2, 2023

Uh oh!

cgwalters commented Oct 2, 2023

Uh oh!

jlebon left a comment

Choose a reason for hiding this comment

Uh oh!

jlebon Oct 2, 2023

Choose a reason for hiding this comment

Uh oh!

cgwalters Oct 2, 2023

Choose a reason for hiding this comment

Uh oh!

cgwalters Oct 2, 2023

Choose a reason for hiding this comment

Uh oh!

jlebon Oct 2, 2023

Choose a reason for hiding this comment

Uh oh!

alexlarsson Oct 9, 2023

Choose a reason for hiding this comment

Uh oh!

cgwalters Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

alexlarsson Oct 20, 2023

Choose a reason for hiding this comment

Uh oh!

alexlarsson commented Oct 13, 2023

Uh oh!

cgwalters commented Oct 20, 2023

Uh oh!

cgwalters commented Oct 24, 2023

Uh oh!

Check warning

Check warning

jlebon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cgwalters commented Oct 24, 2023

Uh oh!

jlebon commented Oct 25, 2023

Uh oh!

jlebon left a comment

Choose a reason for hiding this comment

Uh oh!

alexlarsson commented Sep 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants