relabel-etc: Split remount into, remount and relabel-etc #3267

ericcurtin · 2024-06-19T12:02:10Z

Split these into two different binaries we two different resonsibilities. They don't share so much functionality.

ericcurtin · 2024-06-19T12:04:00Z

Incomplete and untested right now

cgwalters

Thanks so much for working on this!

cgwalters · 2024-06-19T12:14:36Z

src/boot/ostree-relabel-etc.service

+Conflicts=umount.target
+# Run after core mounts
+After=-.mount var.mount
+After=systemd-remount-fs.service


This one can (and should) be Before=systemd-remount-fs.service instead.

OK I was thinking about this more and I think we're in a situation where actually we tell people using composefs in particular to just mount the rootfs writable in the initramfs, so the whole role of systemd-remount-fs.service is a no-op from our PoV basically.

At least...I thought we enforced that, but now I can't find it.

cgwalters · 2024-06-19T12:14:55Z

src/boot/ostree-relabel-etc.service

+OnFailure=emergency.target
+Conflicts=umount.target
+# Run after core mounts
+After=-.mount var.mount


Don't need var.mount here.
Actually let's just use:
RequiresMountsFor=/etc.

cgwalters · 2024-06-19T12:16:11Z

src/boot/ostree-relabel-etc.service

+Before=systemd-random-seed.service plymouth-read-write.service systemd-journal-flush.service systemd-sysusers.service
+Before=systemd-tmpfiles-setup.service systemd-rfkill.service systemd-rfkill.socket


We can drop all this stuff, as all of that is After=systemd-remount-fs.service.

But I'm fine going belt-and-suspenders here with e.g. Before=ostree-remount.service - which already has all those Before= itself.

To combine all this I'd suggest:

Before=local-fs-pre.target systemd-remount-fs.service # This one is already itself After=systemd-remount-fs.service, but for redundancy Before=ostree-remount.service

cgwalters · 2024-06-19T12:21:32Z

src/switchroot/ostree-relabel-etc.c

+    glnx_autofd int fd = open (OTCORE_RUN_BOOTED, O_RDONLY | O_CLOEXEC);
+    if (fd < 0)
+      {
+        /* We really expect that nowadays that everything is done in the initramfs,


Don't need this copy here.

cgwalters · 2024-06-19T12:21:51Z

src/switchroot/ostree-relabel-etc.c

+   * Today systemd remounts / (recursively) as shared, so we're undoing that as early
+   * as possible.  See also a copy of this in ostree-prepare-root.c.
+   */
+  if (mount ("none", "/sysroot", NULL, MS_REC | MS_PRIVATE, NULL) < 0)


Also drop this

cgwalters · 2024-06-19T12:24:01Z

src/boot/ostree-relabel-etc.service

+[Service]
+Type=oneshot
+RemainAfterExit=yes
+ExecStart=/usr/lib/ostree/ostree-relabel-etc


Hmm, so far we haven't introduced many new little binaries, but instead we've been tending to dispatch via hidden CLI verbs, see e.g. ostree admin finalize-staged. I'd slightly prefer that, i.e. we introduce a new ostree admin relabel-etc (which should barf if it's not run under systemd, i.e. we check INVOCATION_ID etc).

cgwalters · 2024-06-19T12:25:39Z

src/libostree/ostree-impl-system-generator.c

@@ -109,6 +109,11 @@ require_internal_units (const char *normal_dir, const char *early_dir, const cha
      < 0)
    return glnx_throw_errno_prefix (error, "symlinkat");

+  if (symlinkat (SYSTEM_DATA_UNIT_PATH "/ostree-relabel-etc.service", normal_dir_dfd,


I'm OK with this as is, but just a note: I think we can change things so that the initramfs code itself acts as a generator and writes the enablement symlink into /run directly then.

As is it's a bit silly because the initramfs writes the file in /run which says that transient etc is enabled, then we boot in the real root, run a binary which reads that file and exits if the flag isn't found...but we can bypass all that and be slightly more efficient by just telling systemd whether or not to run the binary.

Split these into two different binaries we two different resonsibilities. They don't share so much functionality. Signed-off-by: Eric Curtin <[email protected]>

cgwalters · 2024-06-19T13:44:05Z

src/switchroot/ostree-relabel-etc.c

@@ -0,0 +1,179 @@
+/*
+ * Copyright (C) 2011 Colin Walters <[email protected]>


Tangentially related, I think you can update this to be you and or/alex, or just say (C) Red Hat, Inc.

cgwalters · 2024-06-19T13:44:38Z

src/switchroot/ostree-relabel-etc.c

+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see <https://www.gnu.org/licenses/>.
+ *
+ * Author: Colin Walters <[email protected]>


I've been dropping this in new code BTW, "git log" is source of truth and I don't think I wrote any of this code.

cgwalters · 2024-06-19T13:45:31Z

src/switchroot/ostree-relabel-etc.c

+static void
+relabel_dir_for_upper (const char *upper_path, const char *real_path, gboolean is_dir)
+{
+#ifdef HAVE_SELINUX


Optional: I think this entire binary could become conditional on SELinux actually, i.e. we don't even compile or ship it if built without selinux support.

jlebon · 2024-06-19T20:07:56Z

Hmm, why aren't we doing the relabeling from the initramfs instead? Current kernels allow relabeling before a policy is loaded. This is notably used by Ignition: https://github.com/coreos/ignition/blob/b2ea35258bf5367a3522a4bea7f71063954b740c/internal/exec/util/selinux.go#L80.

From my experience, doing relabeling from the real root causes a lot of issues and quickly becomes a game of whack-a-race condition.

jlebon · 2024-06-19T20:19:17Z

Seems to work at first glance:

# https://gitlab.com/fedora/bootc/examples/-/blob/main/transient-etc/Containerfile
FROM quay.io/centos-bootc/centos-bootc:stream9
# Configure the initramfs, see https://github.com/ostreedev/ostree/blob/main/man/ostree-prepare-root.xml
RUN echo -e '[etc]\ntransient=true' >> /usr/lib/ostree/prepare-root.conf && \
    set -x; kver=$(cd /usr/lib/modules && echo *); dracut -vf /usr/lib/modules/$kver/initramfs.img $kver

I bootc install to-disk the resulting image, boot it, add rd.break, and then from the switchroot shell:

$ chroot /sysroot setfiles -vF /etc/selinux/targeted/contexts/files/file_contexts /etc
Relabeled /etc from <no context> to system_u:object_r:etc_t:s0
Relabeled /etc/systemd/system from system_u:object_r:etc_t:s0 to system_u:object_r:systemd_unit_file_t:s0
Relabeled /etc/systemd/system/basic.target.wants from system_u:object_r:etc_t:s0 to system_u:object_r:systemd_unit_file_t:s0
Relabeled /etc/systemd/system/default.target.wants from system_u:object_r:etc_t:s0 to system_u:object_r:systemd_unit_file_t:s0
Relabeled /etc/systemd/system/getty.target.wants from system_u:object_r:etc_t:s0 to system_u:object_r:systemd_unit_file_t:s0
Relabeled /etc/systemd/system/graphical.target.wants from system_u:object_r:etc_t:s0 to system_u:object_r:systemd_unit_file_t:s0
Relabeled /etc/systemd/system/local-fs.target.wants from system_u:object_r:etc_t:s0 to system_u:object_r:systemd_unit_file_t:s0
Relabeled /etc/systemd/system/multi-user.target.wants from system_u:object_r:etc_t:s0 to system_u:object_r:systemd_unit_file_t:s0
Relabeled /etc/systemd/system/network-online.target.wants from system_u:object_r:etc_t:s0 to system_u:object_r:systemd_unit_file_t:s0
Relabeled /etc/systemd/system/sockets.target.wants from system_u:object_r:etc_t:s0 to system_u:object_r:systemd_unit_file_t:s0
Relabeled /etc/systemd/system/sysinit.target.wants from system_u:object_r:etc_t:s0 to system_u:object_r:systemd_unit_file_t:s0
Relabeled /etc/systemd/system/timers.target.wants from system_u:object_r:etc_t:s0 to system_u:object_r:systemd_unit_file_t:s0

(Though you'd normally add setfiles to your initrd and use -r /sysroot instead of doing a chroot.)

cgwalters · 2024-06-19T23:51:25Z

Relabeling from the initramfs indeed sounds way better. The only thing I'm a little uncertain about is the semantics for how e.g. selinux=0 or other flags for configuring it in the real root are handled. Maybe running setfiles handles this already?

ericcurtin · 2024-06-20T11:51:41Z

I like the idea of using setfiles or something similar, less code to maintain...

I think selinux=0, etc. is easy to handle in systemd .service file:

ConditionKernelCommandLine=!selinux=0

We may be able to achieve this with zero C code here, just write a well crafted systemd .service file.

Although, I would rather we relabelled from rootfs, even if it meant running this .service file in it's own exclusive time slot.

We put a lot of effort in Automotive for example to make our initramfs super tiny like < 10M .

Ideally we wouldn't relabel on boot at all tbh, it's another thing to do during boot and again we are trying to cut down on doing tasks at boot to make that as fast as possible. Any reason we can't do this at image composition time or shutdown?

/.autorelabel for example seems to perform relabeling on shutdown

cgwalters · 2024-07-18T15:38:12Z

Ideally we wouldn't relabel on boot at all tbh, it's another thing to do during boot and again we are trying to cut down on doing tasks at boot to make that as fast as possible. Any reason we can't do this at image composition time or shutdown?

Automotive is using transient etc, this is about the in-memory inodes for the overlayfs. There's no real way to shortcut this today, we have a kind of complex interlock between selinux policy loading vs the mount.

However as far as the cost...slimming down what's in /etc by default will inherently make this cheaper.

ericcurtin self-assigned this Jun 19, 2024

ericcurtin marked this pull request as draft June 19, 2024 12:02

openshift-ci bot added the do-not-merge/work-in-progress label Jun 19, 2024

github-actions bot added the area/prepare-root Issue relates to ostree-prepare-root label Jun 19, 2024

ericcurtin force-pushed the ostree-etc-relabel branch from 4c565cd to b18a2ed Compare June 19, 2024 12:13

cgwalters requested changes Jun 19, 2024

View reviewed changes

relabel-etc: Split remount into, remount and relabel-etc

0a19594

Split these into two different binaries we two different resonsibilities. They don't share so much functionality. Signed-off-by: Eric Curtin <[email protected]>

ericcurtin force-pushed the ostree-etc-relabel branch from b18a2ed to 0a19594 Compare June 19, 2024 12:37

cgwalters reviewed Jun 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

relabel-etc: Split remount into, remount and relabel-etc #3267

relabel-etc: Split remount into, remount and relabel-etc #3267

ericcurtin commented Jun 19, 2024

ericcurtin commented Jun 19, 2024

cgwalters left a comment

cgwalters Jun 19, 2024

cgwalters Jun 19, 2024

cgwalters Jun 19, 2024

cgwalters Jun 19, 2024

cgwalters Jun 19, 2024

cgwalters Jun 19, 2024

cgwalters Jun 19, 2024

cgwalters Jun 19, 2024

cgwalters Jun 19, 2024

cgwalters Jun 19, 2024

cgwalters Jun 19, 2024

jlebon commented Jun 19, 2024

jlebon commented Jun 19, 2024

cgwalters commented Jun 19, 2024

ericcurtin commented Jun 20, 2024 •

edited

Loading

cgwalters commented Jul 18, 2024

		Before=systemd-random-seed.service plymouth-read-write.service systemd-journal-flush.service systemd-sysusers.service
		Before=systemd-tmpfiles-setup.service systemd-rfkill.service systemd-rfkill.socket

		@@ -0,0 +1,179 @@
		/*
		* Copyright (C) 2011 Colin Walters <[email protected]>

relabel-etc: Split remount into, remount and relabel-etc #3267

Are you sure you want to change the base?

relabel-etc: Split remount into, remount and relabel-etc #3267

Conversation

ericcurtin commented Jun 19, 2024

ericcurtin commented Jun 19, 2024

cgwalters left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jlebon commented Jun 19, 2024

jlebon commented Jun 19, 2024

cgwalters commented Jun 19, 2024

ericcurtin commented Jun 20, 2024 • edited Loading

cgwalters commented Jul 18, 2024

ericcurtin commented Jun 20, 2024 •

edited

Loading