Merge pull request #20421 from maanyagoenka/systemd-security

systemd-analyze: refactor security_info struct and add option to perform security analysis of specified unit file(s)

man/systemd-analyze.xml

@@ -770,6 +770,28 @@ Service b@0.service not loaded, b.socket cannot be started.
         operate on files inside the specified image path <replaceable>PATH</replaceable>.</para></listitem>
       </varlistentry>
 
+      <varlistentry>
+        <term><option>--offline=<replaceable>BOOL</replaceable></option></term>
+
+        <listitem><para>With <command>security</command>, perform an offline security review
+        of the specified unit file(s), i.e. does not have to rely on PID 1 to acquire security
+        information for the files like the <command>security</command> verb when used by itself does.
+        This means that <option>--offline=</option> can be used with <option>--root=</option> and
+        <option>--image=</option> as well. If a unit's overall exposure level is above that set by
+        <option>--threshold=</option> (default value is 100), <option>--offline=</option> will return
+        an error.</para></listitem>
+      </varlistentry>
+
+      <varlistentry>
+        <term><option>--threshold=<replaceable>NUMBER</replaceable></option></term>
+
+        <listitem><para>With <command>security</command>, allow the user to set a custom value
+        to compare the overall exposure level with, for the specified unit file(s). If a unit's
+        overall exposure level, is greater than that set by the user, <command>security</command>
+        will return an error. <option>--threshold=</option> can be used with <option>--offline=</option>
+        as well and its default value is 100.</para></listitem>
+      </varlistentry>
+
       <varlistentry>
         <term><option>--iterations=<replaceable>NUMBER</replaceable></option></term>
 

shell-completion/bash/systemd-analyze

@@ -144,7 +144,7 @@ _systemd_analyze() {
 
     elif __contains_word "$verb" ${VERBS[SECURITY]}; then
         if [[ $cur = -* ]]; then
-            comps='--help --version --no-pager --system --user -H --host -M --machine'
+            comps='--help --version --no-pager --system --user -H --host -M --machine --offline --threshold'
         else
             if __contains_word "--user" ${COMP_WORDS[*]}; then
                 mode=--user

shell-completion/zsh/_systemd-analyze

@@ -90,6 +90,8 @@ _arguments \
     '--root=[Add support for root argument]:PATH' \
     '--image=[Add support for discrete images]:PATH' \
     '--recursive-errors=[When verifying a unit, control dependency verification]:MODE' \
+    '--offline=[Perform a security review of the specified unit file(s)]:BOOL' \
+    '--threshold=[Set a value to compare the overall security exposure level with]: NUMBER' \
     '--no-pager[Do not pipe output into a pager]' \
     '--man=[Do (not) check for existence of man pages]:boolean:(1 0)' \
     '--order[When generating graph for dot, show only order]' \

src/analyze/analyze-security.h

@@ -1,12 +1,17 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 #pragma once
 
+#include <stdbool.h>
+
 #include "sd-bus.h"
 
+#include "unit-file.h"
+
 typedef enum AnalyzeSecurityFlags {
         ANALYZE_SECURITY_SHORT             = 1 << 0,
         ANALYZE_SECURITY_ONLY_LOADED       = 1 << 1,
         ANALYZE_SECURITY_ONLY_LONG_RUNNING = 1 << 2,
 } AnalyzeSecurityFlags;
 
-int analyze_security(sd_bus *bus, char **units, AnalyzeSecurityFlags flags);
+int analyze_security(sd_bus *bus, char **units, UnitFileScope scope, bool check_man, bool run_generators,
+                     bool offline, unsigned threshold, const char *root, AnalyzeSecurityFlags flags);

src/analyze/analyze-verify.c

@@ -33,7 +33,7 @@ static void log_syntax_callback(const char *unit, int level, void *userdata) {
         }
 }
 
-static int prepare_filename(const char *filename, char **ret) {
+int verify_prepare_filename(const char *filename, char **ret) {
         int r;
         const char *name;
         _cleanup_free_ char *abspath = NULL;
@@ -70,7 +70,7 @@ static int prepare_filename(const char *filename, char **ret) {
         return 0;
 }
 
-static int generate_path(char **var, char **filenames) {
+int verify_generate_path(char **var, char **filenames) {
         const char *old;
         char **filename;
 
@@ -266,7 +266,7 @@ int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run
         set_log_syntax_callback(log_syntax_callback, &s);
 
         /* set the path */
-        r = generate_path(&var, filenames);
+        r = verify_generate_path(&var, filenames);
         if (r < 0)
                 return log_error_errno(r, "Failed to generate unit load path: %m");
 
@@ -291,7 +291,7 @@ int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run
 
                 log_debug("Handling %s...", *filename);
 
-                k = prepare_filename(*filename, &prepared);
+                k = verify_prepare_filename(*filename, &prepared);
                 if (k < 0) {
                         log_error_errno(k, "Failed to prepare filename %s: %m", *filename);
                         if (r == 0)

src/analyze/analyze-verify.h

@@ -14,6 +14,8 @@ typedef enum RecursiveErrors {
         _RECURSIVE_ERRORS_INVALID = -EINVAL,
 } RecursiveErrors;
 
+int verify_generate_path(char **var, char **filenames);
+int verify_prepare_filename(const char *filename, char **ret);
 int verify_executable(Unit *u, const ExecCommand *exec, const char *root);
 int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run_generators, RecursiveErrors recursive_errors, const char *root);
 

src/analyze/analyze.c

@@ -91,6 +91,8 @@ static bool arg_man = true;
 static bool arg_generators = false;
 static char *arg_root = NULL;
 static char *arg_image = NULL;
+static bool arg_offline = false;
+static unsigned arg_threshold = 100;
 static unsigned arg_iterations = 1;
 static usec_t arg_base_time = USEC_INFINITY;
 
@@ -2160,7 +2162,7 @@ static int do_security(int argc, char *argv[], void *userdata) {
 
         (void) pager_open(arg_pager_flags);
 
-        return analyze_security(bus, strv_skip(argv, 1), 0);
+        return analyze_security(bus, strv_skip(argv, 1), arg_scope, arg_man, arg_generators, arg_offline, arg_threshold, arg_root, 0);
 }
 
 static int help(int argc, char *argv[], void *userdata) {
@@ -2208,6 +2210,9 @@ static int help(int argc, char *argv[], void *userdata) {
                "\nOptions:\n"
                "  -h --help                  Show this help\n"
                "     --recursive-errors=MODE Control which units are verified\n"
+               "     --offline=BOOL          Perform a security review on unit file(s)\n"
+               "     --threshold=N           Exit with a non-zero status when overall\n"
+               "                             exposure level is over threshold value\n"
                "     --version               Show package version\n"
                "     --no-pager              Do not pipe output into a pager\n"
                "     --system                Operate on system systemd instance\n"
@@ -2259,6 +2264,8 @@ static int parse_argv(int argc, char *argv[]) {
                 ARG_ITERATIONS,
                 ARG_BASE_TIME,
                 ARG_RECURSIVE_ERRORS,
+                ARG_OFFLINE,
+                ARG_THRESHOLD,
         };
 
         static const struct option options[] = {
@@ -2269,6 +2276,8 @@ static int parse_argv(int argc, char *argv[]) {
                 { "root",             required_argument, NULL, ARG_ROOT             },
                 { "image",            required_argument, NULL, ARG_IMAGE            },
                 { "recursive-errors", required_argument, NULL, ARG_RECURSIVE_ERRORS },
+                { "offline",          required_argument, NULL, ARG_OFFLINE          },
+                { "threshold",        required_argument, NULL, ARG_THRESHOLD        },
                 { "system",           no_argument,       NULL, ARG_SYSTEM           },
                 { "user",             no_argument,       NULL, ARG_USER             },
                 { "global",           no_argument,       NULL, ARG_GLOBAL           },
@@ -2387,6 +2396,19 @@ static int parse_argv(int argc, char *argv[]) {
                                 return r;
                         break;
 
+                case ARG_OFFLINE:
+                        r = parse_boolean_argument("--offline", optarg, &arg_offline);
+                        if (r < 0)
+                                return r;
+                        break;
+
+                case ARG_THRESHOLD:
+                        r = safe_atou(optarg, &arg_threshold);
+                        if (r < 0 || arg_threshold > 100)
+                                return log_error_errno(r < 0 ? r : SYNTHETIC_ERRNO(EINVAL), "Failed to parse threshold: %s", optarg);
+
+                        break;
+
                 case ARG_ITERATIONS:
                         r = safe_atou(optarg, &arg_iterations);
                         if (r < 0)
@@ -2408,6 +2430,14 @@ static int parse_argv(int argc, char *argv[]) {
                         assert_not_reached();
                 }
 
+        if (arg_offline && !streq_ptr(argv[optind], "security"))
+                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                       "Option --offline= is only supported for security right now.");
+
+        if (arg_threshold != 100 && !streq_ptr(argv[optind], "security"))
+                return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
+                                       "Option --threshold= is only supported for security right now.");
+
         if (arg_scope == UNIT_FILE_GLOBAL &&
             !STR_IN_SET(argv[optind] ?: "time", "dot", "unit-paths", "verify"))
                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
@@ -2417,9 +2447,10 @@ static int parse_argv(int argc, char *argv[]) {
                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
                                        "Option --user is not supported for cat-config right now.");
 
-        if ((arg_root || arg_image) && !STRPTR_IN_SET(argv[optind], "cat-config", "verify"))
+        if ((arg_root || arg_image) && (!STRPTR_IN_SET(argv[optind], "cat-config", "verify")) &&
+           (!(streq_ptr(argv[optind], "security") && arg_offline)))
                 return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                       "Options --root= and --image= are only supported for cat-config and verify right now.");
+                                       "Options --root= and --image= are only supported for cat-config, verify and security when used with --offline= right now.");
 
         /* Having both an image and a root is not supported by the code */
         if (arg_root && arg_image)

src/basic/set.h

@@ -26,7 +26,7 @@ static inline Set* set_free_free(Set *s) {
 
 /* no set_free_free_free */
 
-#define set_copy(s) ((Set*) _hashmap_copy(HASHMAP_BASE(h)  HASHMAP_DEBUG_SRC_ARGS))
+#define set_copy(s) ((Set*) _hashmap_copy(HASHMAP_BASE(s)  HASHMAP_DEBUG_SRC_ARGS))
 
 int _set_ensure_allocated(Set **s, const struct hash_ops *hash_ops HASHMAP_DEBUG_PARAMS);
 #define set_ensure_allocated(h, ops) _set_ensure_allocated(h, ops HASHMAP_DEBUG_SRC_ARGS)

src/journal/journalctl.c

@@ -1227,7 +1227,7 @@ static int discover_next_boot(sd_journal *j,
                 BootId **ret) {
 
         _cleanup_free_ BootId *next_boot = NULL;
-        char match[9+32+1] = "_BOOT_ID=";
+        char match[STRLEN("_BOOT_ID=") + SD_ID128_STRING_MAX] = "_BOOT_ID=";
         sd_id128_t boot_id;
         int r;
 
@@ -1336,7 +1336,7 @@ static int get_boots(
          * If no reference is given, the journal head/tail will do,
          * they're "virtual" boots after all. */
         if (boot_id && !sd_id128_is_null(*boot_id)) {
-                char match[9+32+1] = "_BOOT_ID=";
+                char match[STRLEN("_BOOT_ID=") + SD_ID128_STRING_MAX] = "_BOOT_ID=";
 
                 sd_journal_flush_matches(j);
 
@@ -1467,7 +1467,7 @@ static int list_boots(sd_journal *j) {
 }
 
 static int add_boot(sd_journal *j) {
-        char match[9+32+1] = "_BOOT_ID=";
+        char match[STRLEN("_BOOT_ID=") + SD_ID128_STRING_MAX] = "_BOOT_ID=";
         sd_id128_t boot_id;
         int r;
 

src/test/test-set.c

@@ -117,6 +117,38 @@ static void test_set_ensure_allocated(void) {
         assert_se(set_size(m) == 0);
 }
 
+static void test_set_copy(void) {
+        Set *s, *copy;
+        char *key1, *key2, *key3, *key4;
+
+        log_info("/* %s */", __func__);
+
+        key1 = strdup("key1");
+        assert_se(key1);
+        key2 = strdup("key2");
+        assert_se(key2);
+        key3 = strdup("key3");
+        assert_se(key3);
+        key4 = strdup("key4");
+        assert_se(key4);
+
+        s = set_new(&string_hash_ops);
+        assert_se(s);
+
+        assert_se(set_put(s, key1) >= 0);
+        assert_se(set_put(s, key2) >= 0);
+        assert_se(set_put(s, key3) >= 0);
+        assert_se(set_put(s, key4) >= 0);
+
+        copy = set_copy(s);
+        assert_se(copy);
+
+        assert(set_equal(s, copy));
+
+        set_free(s);
+        set_free_free(copy);
+}
+
 static void test_set_ensure_put(void) {
         _cleanup_set_free_ Set *m = NULL;
 
@@ -311,6 +343,7 @@ int main(int argc, const char *argv[]) {
         test_set_ensure_consume();
         test_set_strjoin();
         test_set_equal();
+        test_set_copy();
 
         return 0;
 }