Support mounting /sysroot (and /boot) read-only

[cgwalters]

Add a magic file and an API for consumers to tell us it's OK
for us to remount read-write (since it'll just affect our
mount namespace).

[cgwalters]

WIP for #1265

[rh-atomic-bot]

☔ The latest upstream changes (presumably 3ca1035) made this pull request unmergeable. Please resolve the merge conflicts.

[cgwalters]

OK did the trivial rebase 🏄‍♂️ for this, and was testing out this patch to cosa:

diff --git a/src/create_disk.sh b/src/create_disk.sh
index 2f9fd779..3f01644c 100755
--- a/src/create_disk.sh
+++ b/src/create_disk.sh
@@ -111,6 +111,9 @@ fi
 
 # we use pure BLS, so don't need grub2-mkconfig
 ostree config --repo rootfs/ostree/repo set sysroot.bootloader none
+# Opt-in to https://github.com/ostreedev/ostree/pull/1767 AKA
+# https://github.com/ostreedev/ostree/issues/1265
+ostree config --repo rootfs/ostree/repo set sysroot.readonly true
 
 if [ "$arch" != "s390x" ]; then
 	# install uefi grub

And FCOS goes into emergency mode since systemd-tmpfiles-setup.service fails due to /var being read-only. I guess the pile of hacks in ostree-remount.c isn't working right.

[cgwalters]

Toying with

diff --git a/src/switchroot/ostree-remount.c b/src/switchroot/ostree-remount.c
index 4b4f5356..f36ecc48 100644
--- a/src/switchroot/ostree-remount.c
+++ b/src/switchroot/ostree-remount.c
@@ -120,47 +120,25 @@ main(int argc, char *argv[])
    */
   const bool sysroot_readonly = sysroot_is_configured_ro ();
   if (!sysroot_readonly)
-    do_remount ("/sysroot", !sysroot_readonly);
+    do_remount ("/sysroot", TRUE);
   else
     {
-      /* Since /sysroot is the real physical root, we can't simply remount it
-       * read-only here, as that'd affect e.g. /etc in and also /var if it's not
-       * a separate mount. Instead, we make new read-only bind mount to it,
-       * unmount the original, then move the bind mount to /sysroot.
+      do_remount ("/sysroot", FALSE);
+
+      /* Now, /etc is not normally a bind mount, but remounting the
+       * sysroot above made it read-only since it's on the same filesystem.
+       * Make it a self-bind mount, so we can then mount it read-write.
        */
-      static const char tmp_sysroot[] = "/etc/ostree-sysroot.tmp";
-      static const char sysroot[] = "/sysroot";
-
-      /* This temporary lives in /etc since it needs to be on the same mount. */
-      if (mkdir (tmp_sysroot, 0755) < 0)
-        err (EXIT_FAILURE, "mkdir(%s)", tmp_sysroot);
-      /* Make it a read-only bind mount to /sysroot */
-      if (mount (sysroot, tmp_sysroot, NULL, MS_BIND | MS_PRIVATE, NULL) < 0)
-        err (EXIT_FAILURE, "failed to bind mount %s %s", sysroot, tmp_sysroot);
-      if (mount (tmp_sysroot, tmp_sysroot, NULL, MS_BIND | MS_RDONLY | MS_REMOUNT, NULL) < 0)
-        err (EXIT_FAILURE, "failed to remount ro %s", tmp_sysroot);
-      if (umount (sysroot) < 0)
-        err (EXIT_FAILURE, "while remounting %s read-only: umount", sysroot);
-
-      /* HACK: We can't move a mount that's under a shared namespace. So we
-       * briefly make the sysroot private so that we can move the mount. This
-       * does introduce a race condition where if e.g. another process mounted
-       * something in / it wouldn't be visible in other mount namespaces.  But
-       * we're running quite early, before e.g. any container runtimes should
-       * be starting.
+      if (mount ("/etc", "/etc", NULL, MS_BIND, NULL) < 0)
+        err (EXIT_FAILURE, "failed to make /etc a bind mount");
+      do_remount ("/etc", TRUE);
+      /* If /var was created as as an OSTree default bind mount (instead of being a separate filesystem)
+       * then remounting the root mount read-only also remounted it.
+       * So just like /etc, we need to make it read-write by default.
+       * If it was a separate filesystem, we expect it to be writable anyways,
+       * so it doesn't hurt to remount it if so.
        */
-      if (mount ("none", "/", NULL, MS_PRIVATE, NULL) < 0)
-        err (EXIT_FAILURE, "making / private temporarily");
-      /* Do the move */
-      if (mount (tmp_sysroot, sysroot, NULL, MS_MOVE, NULL) < 0)
-        err (EXIT_FAILURE, "failed to move read-only %s mount", sysroot);
-      /* Make / shared again */
-      if (mount ("none", "/", NULL, MS_SHARED, NULL) < 0)
-        err (EXIT_FAILURE, "making / shared again");
-
-      /* And clean up */
-      if (rmdir (tmp_sysroot) < 0)
-        err (EXIT_FAILURE, "rmdir(%s)", tmp_sysroot);
+      do_remount ("/var", TRUE);
     }
 
   exit (EXIT_SUCCESS);

instead.

[cgwalters]

OK, the problem was ordering around var.mount; fixed that and things seem to work. Needs more testing around upgrades, and we now also have the cases of:

• FCOS but not configured this way
• FAH/Silverblue which don't start out with rw on kernel cmdline

[cgwalters]

OK, lifting WIP on this! I still need more testing, but it's ready for review.

I have a working FCOS with this and:

• create_disk: Enable read-only /sysroot coreos/coreos-assembler#736
• daemon: Use MountFlags=slave and opt-in to OSTree read-only /sysroot coreos/rpm-ostree#1896

Including e.g. package layering, etc.

[rh-atomic-bot]

☔ The latest upstream changes (presumably bdbce9d) made this pull request unmergeable. Please resolve the merge conflicts.

[jlebon]

Why do we need to introduce a new key here instead of just handling it based on ro being in /etc/fstab? IIUC, for FCOS at least, it'll still be rw at initrd time to avoid the machine-id mess, and then will be re-mounted ro by systemd right before ostree-remount.service (though we could still recheck & remount here as we do right now).

Might've missed discussions around this, though keeping the same API as on other traditional systems I think is worth a lot.

[cgwalters]

As of now, we don't have an /etc/fstab in FCOS anymore.

The other thing is...we're trying to create a "middle ground" around "read-only". Here the physical root is writable - it has /etc and /var by default. Wouldn't you expect having ro in /etc/fstab would be truly read-only?

This is really a magical ostree-specific flag I think to opt-in to new behavior for systems builders, not really something admins should see/change directly.

[jlebon]

The other thing is...we're trying to create a "middle ground" around "read-only". Here the physical root is writable - it has /etc and /var by default. Wouldn't you expect having ro in /etc/fstab would be truly read-only?

OK, I follow. Hmm, I think one confusion here is that "read-only root" can mean a lot of things. Particularly /etc could be interpreted as being covered (and I think it'd be really cool to support this too). I wonder if sysroot.readonly shouldn't be an enum instead. E.g. none (status quo), base (everything except /var and /etc) and full (just /var is read-write). WDYT? (Not suggesting to implement full here, just keeping the option open so we leverage the same knob).