Disable reading SystemdOptions EFI Var when in SecureBoot mode

In SecureBoot mode this is probably not what you want. As your cmdline
is cryptographically signed like when using Type #2 EFI Unified Kernel
Images (https://systemd.io/BOOT_LOADER_SPECIFICATION/) The user's
intention is then that the cmdline should not be modified.  You want to
make sure that the system starts up as exactly specified in the signed
artifact.

5 files changed, 66 insertions, 51 deletions

diff --git a/src/basic/efivars.c b/src/basic/efivars.c
index ea43abd7b3..502c3a0c44 100644
--- a/src/basic/efivars.c
+++ b/src/basic/efivars.c
@@ -20,6 +20,7 @@
 #include "strv.h"
 #include "time-util.h"
 #include "utf8.h"
+#include "virt.h"
 
 #if ENABLE_EFI
 
@@ -221,6 +222,41 @@ int efi_set_variable_string(sd_id128_t vendor, const char *name, const char *v)
         return efi_set_variable(vendor, name, u16, (char16_strlen(u16) + 1) * sizeof(char16_t));
 }
 
+bool is_efi_boot(void) {
+        if (detect_container() > 0)
+                return false;
+
+        return access("/sys/firmware/efi/", F_OK) >= 0;
+}
+
+static int read_flag(const char *varname) {
+        _cleanup_free_ void *v = NULL;
+        uint8_t b;
+        size_t s;
+        int r;
+
+        if (!is_efi_boot()) /* If this is not an EFI boot, assume the queried flags are zero */
+                return 0;
+
+        r = efi_get_variable(EFI_VENDOR_GLOBAL, varname, NULL, &v, &s);
+        if (r < 0)
+                return r;
+
+        if (s != 1)
+                return -EINVAL;
+
+        b = *(uint8_t *)v;
+        return !!b;
+}
+
+bool is_efi_secure_boot(void) {
+        return read_flag("SecureBoot") > 0;
+}
+
+bool is_efi_secure_boot_setup_mode(void) {
+        return read_flag("SetupMode") > 0;
+}
+
 int systemd_efi_options_variable(char **line) {
         const char *e;
         int r;
diff --git a/src/basic/efivars.h b/src/basic/efivars.h
index 46ca58d0a5..13a33c6605 100644
--- a/src/basic/efivars.h
+++ b/src/basic/efivars.h
@@ -28,6 +28,10 @@ int efi_get_variable_string(sd_id128_t vendor, const char *name, char **p);
 int efi_set_variable(sd_id128_t vendor, const char *name, const void *value, size_t size);
 int efi_set_variable_string(sd_id128_t vendor, const char *name, const char *p);
 
+bool is_efi_boot(void);
+bool is_efi_secure_boot(void);
+bool is_efi_secure_boot_setup_mode(void);
+
 int systemd_efi_options_variable(char **line);
 
 #else
@@ -52,6 +56,18 @@ static inline int efi_set_variable_string(sd_id128_t vendor, const char *name, c
         return -EOPNOTSUPP;
 }
 
+static inline bool is_efi_boot(void) {
+        return false;
+}
+
+static inline bool is_efi_secure_boot(void) {
+        return false;
+}
+
+static inline bool is_efi_secure_boot_setup_mode(void) {
+        return false;
+}
+
 static inline int systemd_efi_options_variable(char **line) {
         return -ENODATA;
 }
diff --git a/src/basic/proc-cmdline.c b/src/basic/proc-cmdline.c
index d3d99d9a7f..1af58717c6 100644
--- a/src/basic/proc-cmdline.c
+++ b/src/basic/proc-cmdline.c
@@ -39,6 +39,18 @@ int proc_cmdline(char **ret) {
                 return read_one_line_file("/proc/cmdline", ret);
 }
 
+/* In SecureBoot mode this is probably not what you want. As your cmdline is
+ * cryptographically signed like when using Type #2 EFI Unified Kernel Images
+ * (https://systemd.io/BOOT_LOADER_SPECIFICATION/) The user's intention is then
+ * that the cmdline should not be modified.  You want to make sure that the
+ * system starts up as exactly specified in the signed artifact. */
+static int systemd_options_variable(char **line) {
+        if (is_efi_secure_boot())
+                return -ENODATA;
+
+        return systemd_efi_options_variable(line);
+}
+
 static int proc_cmdline_extract_first(const char **p, char **ret_word, ProcCmdlineFlags flags) {
         const char *q = *p;
         int r;
@@ -119,7 +131,7 @@ int proc_cmdline_parse(proc_cmdline_parse_t parse_item, void *data, ProcCmdlineF
 
         /* We parse the EFI variable first, because later settings have higher priority. */
 
-        r = systemd_efi_options_variable(&line);
+        r = systemd_options_variable(&line);
         if (r < 0 && r != -ENODATA)
                 log_debug_errno(r, "Failed to get SystemdOptions EFI variable, ignoring: %m");
 
@@ -250,7 +262,7 @@ int proc_cmdline_get_key(const char *key, ProcCmdlineFlags flags, char **ret_val
                 return r;
 
         line = mfree(line);
-        r = systemd_efi_options_variable(&line);
+        r = systemd_options_variable(&line);
         if (r == -ENODATA)
                 return false; /* Not found */
         if (r < 0)
diff --git a/src/shared/efi-loader.c b/src/shared/efi-loader.c
index 108f31d502..b05dc91ecf 100644
--- a/src/shared/efi-loader.c
+++ b/src/shared/efi-loader.c
@@ -63,40 +63,6 @@ struct device_path device_path__contents;
 struct device_path__packed device_path__contents _packed_;
 assert_cc(sizeof(struct device_path) == sizeof(struct device_path__packed));
 
-bool is_efi_boot(void) {
-        if (detect_container() > 0)
-                return false;
-
-        return access("/sys/firmware/efi/", F_OK) >= 0;
-}
-
-static int read_flag(const char *varname) {
-        _cleanup_free_ void *v = NULL;
-        uint8_t b;
-        size_t s;
-        int r;
-
-        if (!is_efi_boot()) /* If this is not an EFI boot, assume the queried flags are zero */
-                return 0;
-
-        r = efi_get_variable(EFI_VENDOR_GLOBAL, varname, NULL, &v, &s);
-        if (r < 0)
-                return r;
-
-        if (s != 1)
-                return -EINVAL;
-
-        b = *(uint8_t *)v;
-        return !!b;
-}
-
-bool is_efi_secure_boot(void) {
-        return read_flag("SecureBoot") > 0;
-}
-
-bool is_efi_secure_boot_setup_mode(void) {
-        return read_flag("SetupMode") > 0;
-}
 
 int efi_reboot_to_firmware_supported(void) {
         _cleanup_free_ void *v = NULL;
diff --git a/src/shared/efi-loader.h b/src/shared/efi-loader.h
index 7d41fbb359..96208d25bf 100644
--- a/src/shared/efi-loader.h
+++ b/src/shared/efi-loader.h
@@ -5,9 +5,6 @@
 
 #if ENABLE_EFI
 
-bool is_efi_boot(void);
-bool is_efi_secure_boot(void);
-bool is_efi_secure_boot_setup_mode(void);
 int efi_reboot_to_firmware_supported(void);
 int efi_get_reboot_to_firmware(void);
 int efi_set_reboot_to_firmware(bool value);
@@ -28,18 +25,6 @@ int efi_loader_get_features(uint64_t *ret);
 
 #else
 
-static inline bool is_efi_boot(void) {
-        return false;
-}
-
-static inline bool is_efi_secure_boot(void) {
-        return false;
-}
-
-static inline bool is_efi_secure_boot_setup_mode(void) {
-        return false;
-}
-
 static inline int efi_reboot_to_firmware_supported(void) {
         return -EOPNOTSUPP;
 }