diff options
author | Maanya Goenka <t-magoenka@microsoft.com> | 2021-08-17 10:40:15 -0700 |
---|---|---|
committer | Maanya Goenka <t-magoenka@microsoft.com> | 2021-08-20 10:59:13 -0700 |
commit | dfbda8799cd4184ca61d4cd4283f76e5776d253d (patch) | |
tree | 631f57fff22902190e97f488f1ed53e97b86992e | |
parent | bb43d853190052b3d2984ae08299ddf0a97b86f5 (diff) | |
download | systemd-dfbda8799cd4184ca61d4cd4283f76e5776d253d.tar.gz |
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
-rw-r--r-- | man/systemd-analyze.xml | 10 | ||||
-rw-r--r-- | shell-completion/bash/systemd-analyze | 2 | ||||
-rw-r--r-- | shell-completion/zsh/_systemd-analyze | 1 | ||||
-rw-r--r-- | src/analyze/analyze-security.c | 30 | ||||
-rw-r--r-- | src/analyze/analyze-security.h | 3 | ||||
-rw-r--r-- | src/analyze/analyze.c | 18 |
6 files changed, 50 insertions, 14 deletions
diff --git a/man/systemd-analyze.xml b/man/systemd-analyze.xml index 3c276360cd..a2f9154791 100644 --- a/man/systemd-analyze.xml +++ b/man/systemd-analyze.xml @@ -783,6 +783,16 @@ Service b@0.service not loaded, b.socket cannot be started. </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> <listitem><para>When used with the <command>calendar</command> command, show the specified number of diff --git a/shell-completion/bash/systemd-analyze b/shell-completion/bash/systemd-analyze index b7b92f0e00..6f33d53cfc 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 6db4da6a35..f91357cb61 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 6daa18ac1f..24500e3a5b 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 b9ea2586b9..57a93afbef 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 8db03f0eda..9bc7e606e8 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), |