From 89f739adb7d5c5d30958ff91b8012af439044e10 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Tue, 13 Feb 2024 19:58:14 -0500
Subject: [PATCH] install: Mount `/boot` readonly by default

As we want to support enabling `root.transient` in some images,
this means that things like `apt|dnf install foo` literally
just works out of the box.

However...we have a looming danger around things like
kernels.  Typically the package installation scripts
for those aren't going to handle this correctly.

Let's mount `/boot` readonly by default, as we have been doing
in Fedora CoreOS and derivatives for a while.

Now I'm not totally happy with this because ultimately
I think this should be configurable by the OS, not hardcoded
in bootc.  We have some thought to put in to exactly how
that's exposed.

But for now let's set the precedent here.

Signed-off-by: Colin Walters <walters@verbum.org>
---
 lib/src/install.rs          | 26 +++++++++++++++++++++++++-
 lib/src/install/baseline.rs |  7 ++++++-
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/lib/src/install.rs b/lib/src/install.rs
index c40226fdd..5cf7e7db0 100644
--- a/lib/src/install.rs
+++ b/lib/src/install.rs
@@ -362,6 +362,15 @@ impl MountSpec {
             self.source, self.target, self.fstype, options
         )
     }
+
+    /// Append a mount option
+    pub(crate) fn push_option(&mut self, opt: &str) {
+        let options = self.options.get_or_insert_with(Default::default);
+        if !options.is_empty() {
+            options.push(',');
+        }
+        options.push_str(opt);
+    }
 }
 
 impl FromStr for MountSpec {
@@ -1308,13 +1317,18 @@ pub(crate) async fn install_to_filesystem(opts: InstallToFilesystemOpts) -> Resu
     tracing::debug!("Backing device: {backing_device}");
 
     let rootarg = format!("root={root_mount_spec}");
-    let boot = if let Some(spec) = fsopts.boot_mount_spec {
+    let mut boot = if let Some(spec) = fsopts.boot_mount_spec {
         Some(MountSpec::new(&spec, "/boot"))
     } else {
         boot_uuid
             .as_deref()
             .map(|boot_uuid| MountSpec::new_uuid_src(boot_uuid, "/boot"))
     };
+    // Ensure that we mount /boot readonly because it's really owned by bootc/ostree
+    // and we don't want e.g. apt/dnf trying to mutate it.
+    if let Some(boot) = boot.as_mut() {
+        boot.push_option("ro");
+    }
     // By default, we inject a boot= karg because things like FIPS compliance currently
     // require checking in the initramfs.
     let bootarg = boot.as_ref().map(|boot| format!("boot={}", &boot.source));
@@ -1354,3 +1368,13 @@ fn install_opts_serializable() {
     .unwrap();
     assert_eq!(c.block_opts.device, "/dev/vda");
 }
+
+#[test]
+fn test_mountspec() {
+    let mut ms = MountSpec::new("/dev/vda4", "/boot");
+    assert_eq!(ms.to_fstab(), "/dev/vda4 /boot auto defaults 0 0");
+    ms.push_option("ro");
+    assert_eq!(ms.to_fstab(), "/dev/vda4 /boot auto ro 0 0");
+    ms.push_option("relatime");
+    assert_eq!(ms.to_fstab(), "/dev/vda4 /boot auto ro,relatime 0 0");
+}
diff --git a/lib/src/install/baseline.rs b/lib/src/install/baseline.rs
index 182d361fa..39cb316a5 100644
--- a/lib/src/install/baseline.rs
+++ b/lib/src/install/baseline.rs
@@ -348,7 +348,12 @@ pub(crate) fn install_create_rootfs(
     let rootarg = format!("root=UUID={root_uuid}");
     let bootsrc = format!("UUID={boot_uuid}");
     let bootarg = format!("boot={bootsrc}");
-    let boot = MountSpec::new(bootsrc.as_str(), "/boot");
+    let boot = MountSpec {
+        source: bootsrc.into(),
+        target: "/boot".into(),
+        fstype: MountSpec::AUTO.into(),
+        options: Some("ro".into()),
+    };
     let kargs = root_blockdev_kargs
         .into_iter()
         .flatten()