Skip to content

Commit

Permalink
install: Mount /boot readonly by default
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
cgwalters committed Feb 14, 2024
1 parent c19d87f commit 89f739a
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 2 deletions.
26 changes: 25 additions & 1 deletion lib/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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");
}
7 changes: 6 additions & 1 deletion lib/src/install/baseline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 89f739a

Please sign in to comment.