From 4eab5056145b452252bba9c30d94903322ba0958 Mon Sep 17 00:00:00 2001
From: Colin Walters <walters@verbum.org>
Date: Fri, 2 Oct 2020 12:40:27 -0400
Subject: [PATCH] overlay/boot-mount-generator: Mount /boot and /boot/efi
 read-only

ostree has had support for leaving `/boot` mounted read-only
for a long time: https://github.com/ostreedev/ostree/pull/1767
(And then later extended to `/sysroot`)

Particularly for CoreOS, only a few things should be touching
`/boot`, and we control all of them.  Those projects should
create a new mount namespace and remount these partitions
writable just while they need it.

The main thing we're accomplishing here is making the system
more resilient against accidental damage from a sysadmin
root shell as well as configuration management tools like
Puppet/Ansible.  None of those should be directly manipulating
files on these partitions, they should go through the API
of one of our projects (e.g. `rpm-ostree kargs`, `bootupctl`) etc.
---
 .../system-generators/coreos-boot-mount-generator  | 14 ++++++++------
 tests/kola/misc-ro                                 | 13 +++++++++----
 2 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator b/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator
index f790b74671..c0b94e97cd 100755
--- a/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator
+++ b/overlay.d/05core/usr/lib/systemd/system-generators/coreos-boot-mount-generator
@@ -35,8 +35,9 @@ fi
 mk_mount() {
     local mount_pt="${1}"; shift
     local label="${1}"; shift
-    local path="/dev/disk/by-label/${label}"
+    local options="${1}"; shift
 
+    local path="/dev/disk/by-label/${label}"
     local unit_name=$(systemd-escape -p ${mount_pt} --suffix=mount)
 
     eval $(udevadm info --query property --export "${path}")
@@ -61,6 +62,7 @@ After=systemd-fsck@${device}.service
 [Mount]
 What=${path}
 Where=${mount_pt}
+Options=${options}
 EOF
 
     add_wants "${unit_name}"
@@ -71,19 +73,19 @@ EOF
 # ConditionPathExists won't work here because conditions don't affect
 # the dependency on the underlying device unit.
 if [ ! -f /run/ostree-live ]; then
-    mk_mount /boot boot
+    # We mount read-only by default mostly to protect
+    # against accidental damage.  Only a few things
+    # owned by CoreOS should be touching /boot or the ESP.
+    mk_mount /boot boot ro
 
     # Only mount the EFI System Partition on machines where it exists,
     # which are 1) machines actually booted through EFI, and 2) x86_64
     # when booted through BIOS.
     if [ "$(uname -m)" = "x86_64" -o -d /sys/firmware/efi ]; then
-        mk_mount /boot/efi EFI-SYSTEM
         # In the general case the ESP might have per-machine or private
         # data on it.  Let's not make it world readable on general
         # principle.
         # https://github.com/coreos/fedora-coreos-tracker/issues/640
-        cat >>${UNIT_DIR}/boot-efi.mount << EOF
-Options=umask=0077
-EOF
+        mk_mount /boot/efi EFI-SYSTEM ro,umask=0077
     fi
 fi
diff --git a/tests/kola/misc-ro b/tests/kola/misc-ro
index 04db7286fb..5d4a00aa50 100755
--- a/tests/kola/misc-ro
+++ b/tests/kola/misc-ro
@@ -46,10 +46,15 @@ if ip link | grep -o -e " eth[0-9]:"; then
 fi
 ok nic naming
 
-if test -w /sysroot; then
-    fatal "found writable /sysroot"
-fi
-ok sysroot ro
+for part in /sysroot /boot /boot/efi; do
+    if ! findmnt -n -o options ${part} | grep -q "ro,"; then
+        fatal "${part} is missing ro option"
+    fi
+    if test -w "${part}" || touch "${part}/somefile" 2>/dev/null; then
+        fatal "${part} is writable"
+    fi
+done
+ok read-only partitions
 
 if ! lsattr -d / | grep -qe '--i--'; then
     fatal "missing immutable bit on /"