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.

man/journald.conf.xml

@@ -365,7 +365,7 @@
         <term><varname>MaxLevelWall=</varname></term>
 
         <listitem><para>Controls the maximum log level of messages
-        that are stored on disk, forwarded to syslog, kmsg, the
+        that are stored in the journal, forwarded to syslog, kmsg, the
         console or wall (if that is enabled, see above). As argument,
         takes one of
         <literal>emerg</literal>,
@@ -381,8 +381,8 @@
         are stored/forwarded, messages above are dropped. Defaults to
         <literal>debug</literal> for <varname>MaxLevelStore=</varname>
         and <varname>MaxLevelSyslog=</varname>, to ensure that the all
-        messages are written to disk and forwarded to syslog. Defaults
+        messages are stored in the journal and forwarded to syslog.
-        to
+        Defaults to
         <literal>notice</literal> for <varname>MaxLevelKMsg=</varname>,
         <literal>info</literal> for <varname>MaxLevelConsole=</varname>,
         and <literal>emerg</literal> for

man/standard-conf.xml

@@ -11,30 +11,31 @@
   <refsection id='confd'>
     <title>Configuration Directories and Precedence</title>
 
-    <para>Configuration files are read from directories in <filename>/etc/</filename>, <filename>/run/</filename>,
+    <para>Configuration files are read from directories in <filename>/etc/</filename>,
-    <filename>/usr/local/lib/</filename>, and <filename>/usr/lib/</filename>, in order of precedence.  Each
+    <filename>/run/</filename>, <filename>/usr/local/lib/</filename>, and <filename>/usr/lib/</filename>, in
-    configuration file in these configuration directories shall be named in the style of
+    order of precedence, as listed in the SYNOPSIS section above. Files must have the the
-    <filename><replaceable>filename</replaceable>.conf</filename>.  Files in <filename>/etc/</filename> override files
+    <literal>.conf</literal> extension. Files in <filename>/etc/</filename> override files with the same name
-    with the same name in <filename>/run/</filename>, <filename>/usr/local/lib/</filename>, and
+    in <filename>/run/</filename>, <filename>/usr/local/lib/</filename>, and
-    <filename>/usr/lib/</filename>. Files in <filename>/run/</filename> override files with the same name under
+    <filename>/usr/lib/</filename>. Files in <filename>/run/</filename> override files with the same name
-    <filename>/usr/</filename>.</para>
+    under <filename>/usr/</filename>.</para>
 
-    <para>Packages should install their configuration files in <filename>/usr/lib/</filename> (distribution packages)
+    <para>All configuration files are sorted by their filename in lexicographic order, regardless of which of
-    or <filename>/usr/local/lib/</filename> (local installs). Files in <filename>/etc/</filename> are
+    the directories they reside in. If multiple files specify the same option, the entry in the file with the
-    reserved for the local administrator, who may use this logic to override the
+    lexicographically latest name will take precedence. Thus, the configuration in a certain file may either
-    configuration files installed by vendor packages. All configuration files
+    be replaced completely (by placing a file with the same name in a directory with higher priority), or
-    are sorted by their filename in lexicographic order, regardless of which of
+    individual settings might be changed (by specifying additional settings in a file with a different name
-    the directories they reside in. If multiple files specify the same option,
+    that is ordered later).</para>
-    the entry in the file with the lexicographically latest name will take
-    precedence. It is recommended to prefix all filenames with a two-digit number
-    and a dash, to simplify the ordering of the files.</para>
 
-    <para>If the administrator wants to disable a configuration file supplied by
+    <para>Packages should install their configuration files in <filename>/usr/lib/</filename> (distribution
-    the vendor, the recommended way is to place a symlink to
+    packages) or <filename>/usr/local/lib/</filename> (local installs). Files in <filename>/etc/</filename>
-    <filename>/dev/null</filename> in the configuration directory in
+    are reserved for the local administrator, who may use this logic to override the configuration files
-    <filename>/etc/</filename>, with the same filename as the vendor
+    installed by vendor packages. It is recommended to prefix all filenames with a two-digit number and a
-    configuration file. If the vendor configuration file is included in
+    dash, to simplify the ordering of the files.</para>
-    the initrd image, the image has to be regenerated.</para>
+
+    <para>If the administrator wants to disable a configuration file supplied by the vendor, the recommended
+    way is to place a symlink to <filename>/dev/null</filename> in the configuration directory in
+    <filename>/etc/</filename>, with the same filename as the vendor configuration file. If the vendor
+    configuration file is included in the initrd image, the image has to be regenerated.</para>
   </refsection>
 
   <refsection id='main-conf'>
@@ -48,25 +49,20 @@
     can be edited to create local overrides.
     </para>
 
-    <para>When packages need to customize the configuration, they can
+    <para>When packages need to customize the configuration, they can install configuration snippets in
-    install configuration snippets in
+    <filename>/usr/lib/systemd/*.conf.d/</filename> or <filename>/usr/local/lib/systemd/*.conf.d/</filename>.
-    <filename>/usr/lib/systemd/*.conf.d/</filename> or
+    The main configuration file is read before any of the configuration directories, and has the lowest
-    <filename>/usr/local/lib/systemd/*.conf.d/</filename>. Files in
+    precedence; entries in a file in any configuration directory override entries in the single configuration
-    <filename>/etc/</filename> are reserved for the local
+    file. Files in the <filename>*.conf.d/</filename> configuration subdirectories are sorted by their
-    administrator, who may use this logic to override the
+    filename in lexicographic order, regardless of in which of the subdirectories they reside. When multiple
-    configuration files installed by vendor packages. The main
+    files specify the same option, for options which accept just a single value, the entry in the file with
-    configuration file is read before any of the configuration
+    the lexicographically latest name takes precedence. For options which accept a list of values, entries
-    directories, and has the lowest precedence; entries in a file in
+    are collected as they occur in files sorted lexicographically.</para>
-    any configuration directory override entries in the single
+
-    configuration file. Files in the <filename>*.conf.d/</filename>
+    <para>Files in <filename>/etc/</filename> are reserved for the local administrator, who may use this
-    configuration subdirectories are sorted by their filename in lexicographic
+    logic to override the configuration files installed by vendor packages. It is recommended to prefix all
-    order, regardless of which of the subdirectories they reside in. When
+    filenames in those subdirectories with a two-digit number and a dash, to simplify the ordering of the
-    multiple files specify the same option, for options which accept just a
+    files.</para>
-    single value, the entry in the file with the lexicographically latest name
-    takes precedence. For options which accept a list of values, entries are
-    collected as they occur in files sorted lexicographically. It is recommended
-    to prefix all filenames in those subdirectories with a two-digit number and
-    a dash, to simplify the ordering of the files.</para>
 
     <para>To disable a configuration file supplied by the vendor, the
     recommended way is to place a symlink to

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;

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;
 }

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)

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;

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;
 }

src/sysctl/sysctl.c

@@ -11,6 +11,7 @@
 
 #include "conf-files.h"
 #include "def.h"
+#include "errno-util.h"
 #include "fd-util.h"
 #include "fileio.h"
 #include "hashmap.h"
@@ -85,13 +86,15 @@ static int apply_all(OrderedHashmap *sysctl_options) {
                 k = sysctl_write(option->key, option->value);
                 if (k < 0) {
                         /* If the sysctl is not available in the kernel or we are running with reduced
-                         * privileges and cannot write it, then log about the issue at LOG_NOTICE level, and
+                         * privileges and cannot write it, then log about the issue, and proceed without
-                         * proceed without failing. (EROFS is treated as a permission problem here, since
+                         * failing. (EROFS is treated as a permission problem here, since that's how
-                         * that's how container managers usually protected their sysctls.) In all other cases
+                         * container managers usually protected their sysctls.) In all other cases log an
-                         * log an error and make the tool fail. */
+                         * error and make the tool fail. */
 
-                        if (IN_SET(k, -EPERM, -EACCES, -EROFS, -ENOENT) || option->ignore_failure)
+                        if (option->ignore_failure || k == -EROFS || ERRNO_IS_PRIVILEGE(k))
-                                log_notice_errno(k, "Couldn't write '%s' to '%s', ignoring: %m", option->value, option->key);
+                                log_debug_errno(k, "Couldn't write '%s' to '%s', ignoring: %m", option->value, option->key);
+                        else if (k == -ENOENT)
+                                log_info_errno(k, "Couldn't write '%s' to '%s', ignoring: %m", option->value, option->key);
                         else {
                                 log_error_errno(k, "Couldn't write '%s' to '%s': %m", option->value, option->key);
                                 if (r == 0)
@@ -122,7 +125,7 @@ static bool test_prefix(const char *p) {
         return false;
 }
 
-static int parse_file(OrderedHashmap *sysctl_options, const char *path, bool ignore_enoent) {
+static int parse_file(OrderedHashmap **sysctl_options, const char *path, bool ignore_enoent) {
         _cleanup_fclose_ FILE *f = NULL;
         unsigned c = 0;
         int r;
@@ -183,7 +186,10 @@ static int parse_file(OrderedHashmap *sysctl_options, const char *path, bool ign
                 if (!test_prefix(p))
                         continue;
 
-                existing = ordered_hashmap_get(sysctl_options, p);
+                if (ordered_hashmap_ensure_allocated(sysctl_options, &option_hash_ops) < 0)
+                        return log_oom();
+
+                existing = ordered_hashmap_get(*sysctl_options, p);
                 if (existing) {
                         if (streq(value, existing->value)) {
                                 existing->ignore_failure = existing->ignore_failure || ignore_failure;
@@ -191,14 +197,14 @@ static int parse_file(OrderedHashmap *sysctl_options, const char *path, bool ign
                         }
 
                         log_debug("Overwriting earlier assignment of %s at '%s:%u'.", p, path, c);
-                        option_free(ordered_hashmap_remove(sysctl_options, p));
+                        option_free(ordered_hashmap_remove(*sysctl_options, p));
                 }
 
                 new_option = option_new(p, value, ignore_failure);
                 if (!new_option)
                         return log_oom();
 
-                k = ordered_hashmap_put(sysctl_options, new_option->key, new_option);
+                k = ordered_hashmap_put(*sysctl_options, new_option->key, new_option);
                 if (k < 0)
                         return log_error_errno(k, "Failed to add sysctl variable %s to hashmap: %m", p);
 
@@ -320,17 +326,13 @@ static int run(int argc, char *argv[]) {
 
         umask(0022);
 
-        sysctl_options = ordered_hashmap_new(&option_hash_ops);
-        if (!sysctl_options)
-                return log_oom();
-
         if (argc > optind) {
                 int i;
 
                 r = 0;
 
                 for (i = optind; i < argc; i++) {
-                        k = parse_file(sysctl_options, argv[i], false);
+                        k = parse_file(&sysctl_options, argv[i], false);
                         if (k < 0 && r == 0)
                                 r = k;
                 }
@@ -349,7 +351,7 @@ static int run(int argc, char *argv[]) {
                 }
 
                 STRV_FOREACH(f, files) {
-                        k = parse_file(sysctl_options, *f, true);
+                        k = parse_file(&sysctl_options, *f, true);
                         if (k < 0 && r == 0)
                                 r = k;
                 }