From 13c02e7bd54e4420c392bd76c0fcf1846c10f99c Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 26 Nov 2021 17:50:34 +0000 Subject: [PATCH 1/3] portable: move profile search helper to path-lookup Will be used in systemd-analyze later --- src/basic/path-lookup.c | 28 ++++++++++++++++++++++++++++ src/basic/path-lookup.h | 3 +++ src/portable/portable.c | 33 ++------------------------------- 3 files changed, 33 insertions(+), 31 deletions(-) diff --git a/src/basic/path-lookup.c b/src/basic/path-lookup.c index 1969aa98823..6fb8c40e7a5 100644 --- a/src/basic/path-lookup.c +++ b/src/basic/path-lookup.c @@ -8,6 +8,7 @@ #include "fs-util.h" #include "log.h" #include "macro.h" +#include "nulstr-util.h" #include "path-lookup.h" #include "path-util.h" #include "stat-util.h" @@ -864,3 +865,30 @@ char **env_generator_binary_paths(bool is_system) { return TAKE_PTR(paths); } + +int find_portable_profile(const char *name, const char *unit, char **ret_path) { + const char *p, *dot; + + assert(name); + assert(ret_path); + + assert_se(dot = strrchr(unit, '.')); + + NULSTR_FOREACH(p, PORTABLE_PROFILE_DIRS) { + _cleanup_free_ char *joined = NULL; + + joined = strjoin(p, "/", name, "/", dot + 1, ".conf"); + if (!joined) + return -ENOMEM; + + if (laccess(joined, F_OK) >= 0) { + *ret_path = TAKE_PTR(joined); + return 0; + } + + if (errno != ENOENT) + return -errno; + } + + return -ENOENT; +} diff --git a/src/basic/path-lookup.h b/src/basic/path-lookup.h index 088bb9b57cf..af85dc7b4f6 100644 --- a/src/basic/path-lookup.h +++ b/src/basic/path-lookup.h @@ -72,3 +72,6 @@ char **env_generator_binary_paths(bool is_system); #define NETWORK_DIRS ((const char* const*) CONF_PATHS_STRV("systemd/network")) #define NETWORK_DIRS_NULSTR CONF_PATHS_NULSTR("systemd/network") + +#define PORTABLE_PROFILE_DIRS CONF_PATHS_NULSTR("systemd/portable/profile") +int find_portable_profile(const char *name, const char *unit, char **ret_path); diff --git a/src/portable/portable.c b/src/portable/portable.c index 2d1006eabdb..8b088f3e32b 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -40,8 +40,6 @@ #include "tmpfile-util.h" #include "user-util.h" -static const char profile_dirs[] = CONF_PATHS_NULSTR("systemd/portable/profile"); - /* Markers used in the first line of our 20-portable.conf unit file drop-in to determine, that a) the unit file was * dropped there by the portable service logic and b) for which image it was dropped there. */ #define PORTABLE_DROPIN_MARKER_BEGIN "# Drop-in created for image '" @@ -981,33 +979,6 @@ static int install_chroot_dropin( return 0; } -static int find_profile(const char *name, const char *unit, char **ret) { - const char *p, *dot; - - assert(name); - assert(ret); - - assert_se(dot = strrchr(unit, '.')); - - NULSTR_FOREACH(p, profile_dirs) { - _cleanup_free_ char *joined = NULL; - - joined = strjoin(p, "/", name, "/", dot + 1, ".conf"); - if (!joined) - return -ENOMEM; - - if (laccess(joined, F_OK) >= 0) { - *ret = TAKE_PTR(joined); - return 0; - } - - if (errno != ENOENT) - return -errno; - } - - return -ENOENT; -} - static int install_profile_dropin( const char *image_path, const PortableMetadata *m, @@ -1028,7 +999,7 @@ static int install_profile_dropin( if (!profile) return 0; - r = find_profile(profile, m->name, &from); + r = find_portable_profile(profile, m->name, &from); if (r < 0) { if (r != -ENOENT) return log_debug_errno(errno, "Profile '%s' is not accessible: %m", profile); @@ -1790,7 +1761,7 @@ int portable_get_state( int portable_get_profiles(char ***ret) { assert(ret); - return conf_files_list_nulstr(ret, NULL, NULL, CONF_FILES_DIRECTORY|CONF_FILES_BASENAME|CONF_FILES_FILTER_MASKED, profile_dirs); + return conf_files_list_nulstr(ret, NULL, NULL, CONF_FILES_DIRECTORY|CONF_FILES_BASENAME|CONF_FILES_FILTER_MASKED, PORTABLE_PROFILE_DIRS); } static const char* const portable_change_type_table[_PORTABLE_CHANGE_TYPE_MAX] = { From 83de7427dc7897669cfd83b3af7d6bbb914a307c Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 26 Nov 2021 18:07:37 +0000 Subject: [PATCH 2/3] shell-completion: add offline/root/image to systemd-analyze --- shell-completion/bash/systemd-analyze | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/shell-completion/bash/systemd-analyze b/shell-completion/bash/systemd-analyze index 1972441386e..350dcabb948 100644 --- a/shell-completion/bash/systemd-analyze +++ b/shell-completion/bash/systemd-analyze @@ -145,14 +145,17 @@ _systemd_analyze() { elif __contains_word "$verb" ${VERBS[SECURITY]}; then if [[ $cur = -* ]]; then - comps='--help --version --no-pager --system --user -H --host -M --machine --offline --threshold --security-policy --json=off --json=pretty --json=short' - else + comps='--help --version --no-pager --system --user -H --host -M --machine --offline --threshold --security-policy --json=off --json=pretty --json=short --root --image' + elif ! __contains_word "--offline" ${COMP_WORDS[*]}; then if __contains_word "--user" ${COMP_WORDS[*]}; then mode=--user else mode=--system fi comps=$( __get_services $mode ) + else + comps="$CONFIGS $( compgen -A file -- "$cur" )" + compopt -o filenames fi elif __contains_word "$verb" ${VERBS[CONDITION]}; then From 0446921131218c3f23f8937f85294b3c244a923b Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 26 Nov 2021 15:46:40 +0000 Subject: [PATCH 3/3] analyze: add --profile switch to security verb Allows to pass a portable profile when doing offline analysis of units. Especially useful for analyzing portable images, since a lot of the security-relevant settings in those cases come from the profiles, but they are not shipped in the portable images. --- man/systemd-analyze.xml | 9 ++++++ shell-completion/bash/systemd-analyze | 2 +- shell-completion/zsh/_systemd-analyze | 1 + src/analyze/analyze-security.c | 40 ++++++++++++++++++++++++++- src/analyze/analyze-security.h | 1 + src/analyze/analyze.c | 25 +++++++++++++++++ test/units/testsuite-65.sh | 13 +++++++++ 7 files changed, 89 insertions(+), 2 deletions(-) diff --git a/man/systemd-analyze.xml b/man/systemd-analyze.xml index c0b6b477d6a..6482fcfe485 100644 --- a/man/systemd-analyze.xml +++ b/man/systemd-analyze.xml @@ -818,6 +818,15 @@ $ systemd-analyze verify /tmp/source:alias.service an error. + + + + With security , takes into + consideration the specified portable profile when assessing the unit(s) settings. + The profile can be passed by name, in which case the well-known system locations will + be searched, or it can be the full path to a specific drop-in file. + + diff --git a/shell-completion/bash/systemd-analyze b/shell-completion/bash/systemd-analyze index 350dcabb948..ddee57b0e71 100644 --- a/shell-completion/bash/systemd-analyze +++ b/shell-completion/bash/systemd-analyze @@ -145,7 +145,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 --threshold --security-policy --json=off --json=pretty --json=short --root --image' + comps='--help --version --no-pager --system --user -H --host -M --machine --offline --threshold --security-policy --json=off --json=pretty --json=short --root --image --profile=default --profile=nonetwork --profile=strict --profile=trusted' elif ! __contains_word "--offline" ${COMP_WORDS[*]}; then if __contains_word "--user" ${COMP_WORDS[*]}; then mode=--user diff --git a/shell-completion/zsh/_systemd-analyze b/shell-completion/zsh/_systemd-analyze index 468fd0e5829..639964f0645 100644 --- a/shell-completion/zsh/_systemd-analyze +++ b/shell-completion/zsh/_systemd-analyze @@ -87,6 +87,7 @@ _arguments \ '--threshold=[Set a value to compare the overall security exposure level with]: NUMBER' \ '--security-policy=[Allow user to use customized requirements to compare unit file(s) against]: PATH' \ '--json=[Generate a JSON output of the security analysis table]:MODE:(pretty short off)' \ + '--profile=[Include the specified profile in the security review of the unit(s)]: PATH' \ '--no-pager[Do not pipe output into a pager]' \ '--man=[Do (not) check for existence of man pages]:BOOL:(yes no)' \ '--generators=[Do (not) run unit generators]:BOOL:(yes no)' \ diff --git a/src/analyze/analyze-security.c b/src/analyze/analyze-security.c index d3e011e903f..d52518677cf 100644 --- a/src/analyze/analyze-security.c +++ b/src/analyze/analyze-security.c @@ -9,6 +9,7 @@ #include "bus-map-properties.h" #include "bus-unit-util.h" #include "bus-util.h" +#include "copy.h" #include "env-util.h" #include "format-table.h" #include "in-addr-prefix-util.h" @@ -17,6 +18,7 @@ #include "manager.h" #include "missing_capability.h" #include "missing_sched.h" +#include "mkdir.h" #include "nulstr-util.h" #include "parse-util.h" #include "path-util.h" @@ -2646,6 +2648,7 @@ static int offline_security_checks(char **filenames, bool run_generators, unsigned threshold, const char *root, + const char *profile, PagerFlags pager_flags, JsonFormatFlags json_format_flags) { @@ -2682,6 +2685,13 @@ static int offline_security_checks(char **filenames, if (r < 0) return r; + if (profile) { + /* Ensure the temporary directory is in the search path, so that we can add drop-ins. */ + r = strv_extend(&m->lookup_paths.search_path, m->lookup_paths.temporary_dir); + if (r < 0) + return log_oom(); + } + log_debug("Loading remaining units from the command line..."); STRV_FOREACH(filename, filenames) { @@ -2697,6 +2707,33 @@ static int offline_security_checks(char **filenames, continue; } + /* When a portable image is analyzed, the profile is what provides a good chunk of + * the security-related settings, but they are obviously not shipped with the image. + * This allows to take them in consideration. */ + if (profile) { + _cleanup_free_ char *unit_name = NULL, *dropin = NULL, *profile_path = NULL; + + r = path_extract_filename(prepared, &unit_name); + if (r < 0) + return log_oom(); + + dropin = strjoin(m->lookup_paths.temporary_dir, "/", unit_name, ".d/profile.conf"); + if (!dropin) + return log_oom(); + (void) mkdir_parents(dropin, 0755); + + if (!is_path(profile)) { + r = find_portable_profile(profile, unit_name, &profile_path); + if (r < 0) + return log_error_errno(r, "Failed to find portable profile %s: %m", profile); + profile = profile_path; + } + + r = copy_file(profile, dropin, 0, 0644, 0, 0, 0); + if (r < 0) + return log_error_errno(r, "Failed to copy: %m"); + } + k = manager_load_startable_unit_or_warn(m, NULL, prepared, &units[count]); if (k < 0) { if (r == 0) @@ -2725,6 +2762,7 @@ int analyze_security(sd_bus *bus, bool offline, unsigned threshold, const char *root, + const char *profile, JsonFormatFlags json_format_flags, PagerFlags pager_flags, AnalyzeSecurityFlags flags) { @@ -2735,7 +2773,7 @@ int analyze_security(sd_bus *bus, assert(bus); if (offline) - return offline_security_checks(units, policy, scope, check_man, run_generators, threshold, root, pager_flags, json_format_flags); + return offline_security_checks(units, policy, scope, check_man, run_generators, threshold, root, profile, pager_flags, json_format_flags); if (strv_length(units) != 1) { overview_table = table_new("unit", "exposure", "predicate", "happy"); diff --git a/src/analyze/analyze-security.h b/src/analyze/analyze-security.h index 492881c3853..07483248eeb 100644 --- a/src/analyze/analyze-security.h +++ b/src/analyze/analyze-security.h @@ -24,6 +24,7 @@ int analyze_security(sd_bus *bus, bool offline, unsigned threshold, const char *root, + const char *profile, JsonFormatFlags json_format_flags, PagerFlags pager_flags, AnalyzeSecurityFlags flags); diff --git a/src/analyze/analyze.c b/src/analyze/analyze.c index 740faa507bb..a641be4179b 100644 --- a/src/analyze/analyze.c +++ b/src/analyze/analyze.c @@ -105,6 +105,7 @@ static usec_t arg_base_time = USEC_INFINITY; static char *arg_unit = NULL; static JsonFormatFlags arg_json_format_flags = JSON_FORMAT_OFF; static bool arg_quiet = false; +static char *arg_profile = NULL; STATIC_DESTRUCTOR_REGISTER(arg_dot_from_patterns, strv_freep); STATIC_DESTRUCTOR_REGISTER(arg_dot_to_patterns, strv_freep); @@ -112,6 +113,7 @@ STATIC_DESTRUCTOR_REGISTER(arg_root, freep); STATIC_DESTRUCTOR_REGISTER(arg_image, freep); STATIC_DESTRUCTOR_REGISTER(arg_security_policy, freep); STATIC_DESTRUCTOR_REGISTER(arg_unit, freep); +STATIC_DESTRUCTOR_REGISTER(arg_profile, freep); typedef struct BootTimes { usec_t firmware_time; @@ -2423,6 +2425,7 @@ static int do_security(int argc, char *argv[], void *userdata) { arg_offline, arg_threshold, arg_root, + arg_profile, arg_json_format_flags, arg_pager_flags, /*flags=*/ 0); @@ -2497,6 +2500,8 @@ static int help(int argc, char *argv[], void *userdata) { " --iterations=N Show the specified number of iterations\n" " --base-time=TIMESTAMP Calculate calendar times relative to\n" " specified time\n" + " --profile=name|PATH Include the specified profile in the\n" + " security review of the unit(s)\n" " -h --help Show this help\n" " --version Show package version\n" " -q --quiet Do not emit hints\n" @@ -2536,6 +2541,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_THRESHOLD, ARG_SECURITY_POLICY, ARG_JSON, + ARG_PROFILE, }; static const struct option options[] = { @@ -2565,6 +2571,7 @@ static int parse_argv(int argc, char *argv[]) { { "base-time", required_argument, NULL, ARG_BASE_TIME }, { "unit", required_argument, NULL, 'U' }, { "json", required_argument, NULL, ARG_JSON }, + { "profile", required_argument, NULL, ARG_PROFILE }, {} }; @@ -2713,6 +2720,24 @@ static int parse_argv(int argc, char *argv[]) { break; + case ARG_PROFILE: + if (isempty(optarg)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Profile file name is empty"); + + if (is_path(optarg)) { + r = parse_path_argument(optarg, /* suppress_root= */ false, &arg_profile); + if (r < 0) + return r; + if (!endswith(arg_profile, ".conf")) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Profile file name must end with .conf: %s", arg_profile); + } else { + r = free_and_strdup(&arg_profile, optarg); + if (r < 0) + return log_oom(); + } + + break; + case 'U': { _cleanup_free_ char *mangled = NULL; diff --git a/test/units/testsuite-65.sh b/test/units/testsuite-65.sh index f98a933cb14..245f74c5d93 100755 --- a/test/units/testsuite-65.sh +++ b/test/units/testsuite-65.sh @@ -573,7 +573,20 @@ systemd-analyze security --threshold=90 --offline=true \ --security-policy=/tmp/testfile.json \ --root=/tmp/img/ testfile.service +# The strict profile adds a lot of sanboxing options +systemd-analyze security --threshold=20 --offline=true \ + --security-policy=/tmp/testfile.json \ + --profile=strict \ + --root=/tmp/img/ testfile.service + set +e +# The trusted profile doesn't add any sanboxing options +systemd-analyze security --threshold=20 --offline=true \ + --security-policy=/tmp/testfile.json \ + --profile=/usr/lib/systemd/portable/profile/trusted/service.conf \ + --root=/tmp/img/ testfile.service \ + && { echo 'unexpected success'; exit 1; } + systemd-analyze security --threshold=50 --offline=true \ --security-policy=/tmp/testfile.json \ --root=/tmp/img/ testfile.service \