From: Maanya Goenka <t-magoenka@microsoft.com>
Date: Tue, 17 Aug 2021 17:40:15 +0000 (-0700)
Subject: systemd-analyze: add new 'security' option to compare unit's overall exposure level... 
X-Git-Tag: v250-rc1~794^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=dfbda8799cd4184ca61d4cd4283f76e5776d253d;p=thirdparty%2Fsystemd.git

systemd-analyze: add new 'security' option to compare unit's overall exposure level with

--threshold option added to work with security verb and with the --offline option so that
users can determine what qualifies as a security threat. The threshold set by the user is
compared with the overall exposure level assigned to a unit file and if the exposure is
higher than the threshold, 'security' will return a non-zero exit status. The default value
of the --threshold option is 100.

Example Run:

1. testcase.service is a unit file created for testing the --threshold option

    maanya-goenka@debian:~/systemd (systemd-security)$ cat<<EOF>testcase.service

    > [Service]
    > ExecStart = echo hello
    > EOF

    For the purposes of this demo, the security table outputted below has been cut to show only the first two security settings.

    maanya-goenka@debian:~/systemd (systemd-security)$ sudo build/systemd-analyze security --offline=true testcase.service
    /usr/lib/systemd/system/plymouth-start.service:15: Unit configured to use KillMode=none. This is unsafe, as it disables systemd's
    process lifecycle management for the service. Please update your service to use a safer KillMode=, such as 'mixed' or 'control-group'.
    Support for KillMode=none is deprecated and will eventually be removed.
    /usr/lib/systemd/system/gdm.service:30: Standard output type syslog is obsolete, automatically updating to journal. Please update your
    unit file, and consider removing the setting altogether.
    /usr/lib/systemd/system/dbus.socket:5: ListenStream= references a path below legacy directory /var/run/, updating
    /var/run/dbus/system_bus_socket → /run/dbus/system_bus_socket; please update the unit file accordingly.

      NAME                                        DESCRIPTION                                                       EXPOSURE
    ✗ PrivateNetwork=                             Service has access to the host's network                          0.5
    ✗ User=/DynamicUser=                          Service runs as root user                                         0.4

    → Overall exposure level for testcase.service: 9.6 UNSAFE 😨

    maanya-goenka@debian:~/systemd (systemd-security)$ echo $? 0

2. Next, we use the same testcase.service file but add an additional --threshold=60 parameter. We would expect 'security' to exit
   with a non-zero status because the overall exposure level (= 96) is higher than the set threshold (= 60).

    maanya-goenka@debian:~/systemd (systemd-security)$ sudo build/systemd-analyze security --offline=true --threshold=60 testcase.service
    /usr/lib/systemd/system/plymouth-start.service:15: Unit configured to use KillMode=none. This is unsafe, as it disables systemd's
    process lifecycle management for the service. Please update your service to use a safer KillMode=, such as 'mixed' or 'control-group'.
    Support for KillMode=none is deprecated and will eventually be removed.
    /usr/lib/systemd/system/gdm.service:30: Standard output type syslog is obsolete, automatically updating to journal. Please update your
    unit file, and consider removing the setting altogether.
    /usr/lib/systemd/system/dbus.socket:5: ListenStream= references a path below legacy directory /var/run/, updating
    /var/run/dbus/system_bus_socket → /run/dbus/system_bus_socket; please update the unit file accordingly.

      NAME                                        DESCRIPTION                                                       EXPOSURE
    ✗ PrivateNetwork=                             Service has access to the host's network                          0.5
    ✗ User=/DynamicUser=                          Service runs as root user                                         0.4

    → Overall exposure level for testcase.service: 9.6 UNSAFE 😨

    maanya-goenka@debian:~/systemd (systemd-security)$ echo $? 1
---

diff --git a/man/systemd-analyze.xml b/man/systemd-analyze.xml
index 3c276360cd5..a2f91547914 100644
--- a/man/systemd-analyze.xml
+++ b/man/systemd-analyze.xml
@@ -782,6 +782,16 @@ Service b@0.service not loaded, b.socket cannot be started.
         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>
 
diff --git a/shell-completion/bash/systemd-analyze b/shell-completion/bash/systemd-analyze
index b7b92f0e00e..6f33d53cfc4 100644
--- a/shell-completion/bash/systemd-analyze
+++ b/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 --offline'
+            comps='--help --version --no-pager --system --user -H --host -M --machine --offline --threshold'
         else
             if __contains_word "--user" ${COMP_WORDS[*]}; then
                 mode=--user
diff --git a/shell-completion/zsh/_systemd-analyze b/shell-completion/zsh/_systemd-analyze
index 6db4da6a35e..f91357cb61e 100644
--- a/shell-completion/zsh/_systemd-analyze
+++ b/shell-completion/zsh/_systemd-analyze
@@ -91,6 +91,7 @@ _arguments \
     '--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]' \
diff --git a/src/analyze/analyze-security.c b/src/analyze/analyze-security.c
index 6daa18ac1fa..24500e3a5b9 100644
--- a/src/analyze/analyze-security.c
+++ b/src/analyze/analyze-security.c
@@ -1527,7 +1527,7 @@ static const struct security_assessor security_assessor_table[] = {
         },
 };
 
-static int assess(const SecurityInfo *info, Table *overview_table, AnalyzeSecurityFlags flags) {
+static int assess(const SecurityInfo *info, Table *overview_table, AnalyzeSecurityFlags flags, unsigned threshold) {
         static const struct {
                 uint64_t exposure;
                 const char *name;
@@ -1723,6 +1723,10 @@ static int assess(const SecurityInfo *info, Table *overview_table, AnalyzeSecuri
                         return table_log_add_error(r);
         }
 
+        /* Return error when overall exposure level is over threshold */
+        if (exposure > threshold)
+                return -EINVAL;
+
         return 0;
 }
 
@@ -2188,7 +2192,9 @@ static int acquire_security_info(sd_bus *bus, const char *name, SecurityInfo *in
         return 0;
 }
 
-static int analyze_security_one(sd_bus *bus, const char *name, Table *overview_table, AnalyzeSecurityFlags flags) {
+static int analyze_security_one(sd_bus *bus, const char *name, Table *overview_table,
+                                AnalyzeSecurityFlags flags, unsigned threshold) {
+
         _cleanup_(security_info_freep) SecurityInfo *info = security_info_new();
         if (!info)
                 return log_oom();
@@ -2204,7 +2210,7 @@ static int analyze_security_one(sd_bus *bus, const char *name, Table *overview_t
         if (r < 0)
                 return r;
 
-        r = assess(info, overview_table, flags);
+        r = assess(info, overview_table, flags, threshold);
         if (r < 0)
                 return r;
 
@@ -2390,7 +2396,7 @@ static int get_security_info(Unit *u, ExecContext *c, CGroupContext *g, Security
         return 0;
 }
 
-static int offline_security_check(Unit *u) {
+static int offline_security_check(Unit *u, unsigned threshold) {
         _cleanup_(table_unrefp) Table *overview_table = NULL;
         AnalyzeSecurityFlags flags = 0;
         _cleanup_(security_info_freep) SecurityInfo *info = NULL;
@@ -2405,10 +2411,10 @@ static int offline_security_check(Unit *u) {
         if (r < 0)
               return r;
 
-        return assess(info, overview_table, flags);
+        return assess(info, overview_table, flags, threshold);
 }
 
-static int offline_security_checks(char **filenames, UnitFileScope scope, bool check_man, bool run_generators, const char *root) {
+static int offline_security_checks(char **filenames, UnitFileScope scope, bool check_man, bool run_generators, unsigned threshold, const char *root) {
         const ManagerTestRunFlags flags =
                 MANAGER_TEST_RUN_MINIMAL |
                 MANAGER_TEST_RUN_ENV_GENERATORS |
@@ -2467,7 +2473,7 @@ static int offline_security_checks(char **filenames, UnitFileScope scope, bool c
         }
 
         for (size_t i = 0; i < count; i++) {
-                k = offline_security_check(units[i]);
+                k = offline_security_check(units[i], threshold);
                 if (k < 0 && r == 0)
                         r = k;
         }
@@ -2475,14 +2481,16 @@ static int offline_security_checks(char **filenames, UnitFileScope scope, bool c
         return r;
 }
 
-int analyze_security(sd_bus *bus, char **units, UnitFileScope scope, bool check_man, bool run_generators, bool offline, const char *root, 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) {
+
         _cleanup_(table_unrefp) Table *overview_table = NULL;
         int ret = 0, r;
 
         assert(bus);
 
         if (offline)
-                return offline_security_checks(units, scope, check_man, run_generators, root);
+                return offline_security_checks(units, scope, check_man, run_generators, threshold, root);
 
         if (strv_length(units) != 1) {
                 overview_table = table_new("unit", "exposure", "predicate", "happy");
@@ -2542,7 +2550,7 @@ int analyze_security(sd_bus *bus, char **units, UnitFileScope scope, bool check_
                 flags |= ANALYZE_SECURITY_SHORT|ANALYZE_SECURITY_ONLY_LOADED|ANALYZE_SECURITY_ONLY_LONG_RUNNING;
 
                 STRV_FOREACH(i, list) {
-                        r = analyze_security_one(bus, *i, overview_table, flags);
+                        r = analyze_security_one(bus, *i, overview_table, flags, threshold);
                         if (r < 0 && ret >= 0)
                                 ret = r;
                 }
@@ -2577,7 +2585,7 @@ int analyze_security(sd_bus *bus, char **units, UnitFileScope scope, bool check_
                         } else
                                 name = mangled;
 
-                        r = analyze_security_one(bus, name, overview_table, flags);
+                        r = analyze_security_one(bus, name, overview_table, flags, threshold);
                         if (r < 0 && ret >= 0)
                                 ret = r;
                 }
diff --git a/src/analyze/analyze-security.h b/src/analyze/analyze-security.h
index b9ea2586b92..57a93afbef4 100644
--- a/src/analyze/analyze-security.h
+++ b/src/analyze/analyze-security.h
@@ -13,4 +13,5 @@ typedef enum AnalyzeSecurityFlags {
         ANALYZE_SECURITY_ONLY_LONG_RUNNING = 1 << 2,
 } AnalyzeSecurityFlags;
 
-int analyze_security(sd_bus *bus, char **units, UnitFileScope scope, bool check_man, bool run_generators, bool offline, const char *root, 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);
diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c
index 8db03f0eda2..9bc7e606e82 100644
--- a/src/analyze/analyze.c
+++ b/src/analyze/analyze.c
@@ -92,6 +92,7 @@ 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;
 
@@ -2161,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), arg_scope, arg_man, arg_generators, arg_offline, arg_root, 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) {
@@ -2210,6 +2211,8 @@ static int help(int argc, char *argv[], void *userdata) {
                "  -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"
@@ -2262,6 +2265,7 @@ static int parse_argv(int argc, char *argv[]) {
                 ARG_BASE_TIME,
                 ARG_RECURSIVE_ERRORS,
                 ARG_OFFLINE,
+                ARG_THRESHOLD,
         };
 
         static const struct option options[] = {
@@ -2273,6 +2277,7 @@ static int parse_argv(int argc, char *argv[]) {
                 { "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           },
@@ -2397,6 +2402,13 @@ static int parse_argv(int argc, char *argv[]) {
                                 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)
@@ -2422,6 +2434,10 @@ static int parse_argv(int argc, char *argv[]) {
                 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),