From 10e82fde7b01e42aedbbfa978a7bfca47f21d341 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Wed, 29 Oct 2025 22:39:48 +0100 Subject: [PATCH] parse-util: Add parse_capability_set() Let's extract common capability parsing code into a generic function parse_capability_set() with a comprehensive set of unit tests. We also replace usages of UINT64_MAX with CAP_MASK_UNSET where applicable and replace the default value of CapabilityBoundingSet with CAP_MASK_ALL which more clearly identifies that it is initialized to all capabilities. AI (copilot) was used to extract the generic function and write the unit tests, with manual review and fixing afterwards to make sure everything was correct. --- src/analyze/analyze-security.c | 3 +- src/basic/parse-util.c | 38 ++++++++++ src/basic/parse-util.h | 2 + src/core/execute.c | 2 +- src/core/load-fragment.c | 28 ++------ src/core/main.c | 2 +- src/home/homectl.c | 40 ++++------- src/login/pam_systemd.c | 16 ++--- src/nspawn/nspawn-oci.c | 2 +- src/nspawn/nspawn.c | 2 +- src/shared/user-record-show.c | 5 +- src/test/test-parse-util.c | 124 +++++++++++++++++++++++++++++++++ 12 files changed, 197 insertions(+), 67 deletions(-) diff --git a/src/analyze/analyze-security.c b/src/analyze/analyze-security.c index 1856a5b229..d5adcf7cab 100644 --- a/src/analyze/analyze-security.c +++ b/src/analyze/analyze-security.c @@ -13,6 +13,7 @@ #include "bus-map-properties.h" #include "bus-unit-util.h" #include "bus-util.h" +#include "capability-util.h" #include "copy.h" #include "env-util.h" #include "fd-util.h" @@ -135,7 +136,7 @@ static SecurityInfo *security_info_new(void) { *info = (SecurityInfo) { .default_dependencies = true, - .capability_bounding_set = UINT64_MAX, + .capability_bounding_set = CAP_MASK_ALL, .restrict_namespaces = UINT64_MAX, ._umask = 0002, }; diff --git a/src/basic/parse-util.c b/src/basic/parse-util.c index 90fe538a22..c16b2ecdf6 100644 --- a/src/basic/parse-util.c +++ b/src/basic/parse-util.c @@ -7,6 +7,8 @@ #include #include "alloc-util.h" +#include "capability-list.h" +#include "capability-util.h" #include "errno-list.h" #include "extract-word.h" #include "locale-util.h" @@ -809,3 +811,39 @@ bool nft_identifier_valid(const char *id) { return in_charset(id + 1, ALPHANUMERICAL "/\\_."); } + +int parse_capability_set(const char *s, uint64_t initial, uint64_t *current) { + int r; + + assert(s); + assert(current); + + if (isempty(s)) { + *current = CAP_MASK_UNSET; + return 1; + } + + bool invert = false; + if (s[0] == '~') { + invert = true; + s++; + } + + uint64_t parsed; + r = capability_set_from_string(s, &parsed); + if (r < 0) + return r; + + if (parsed == 0 || *current == initial) + /* "~" or uninitialized data -> replace */ + *current = invert ? all_capabilities() & ~parsed : parsed; + else { + /* previous data -> merge */ + if (invert) + *current &= ~parsed; + else + *current |= parsed; + } + + return r; +} diff --git a/src/basic/parse-util.h b/src/basic/parse-util.h index 4927bc9e56..39add96ec7 100644 --- a/src/basic/parse-util.h +++ b/src/basic/parse-util.h @@ -22,6 +22,8 @@ int parse_errno(const char *t); int parse_fd(const char *t); int parse_user_shell(const char *s, char **ret_sh, bool *ret_copy); +int parse_capability_set(const char *s, uint64_t initial, uint64_t *capability_set); + #define SAFE_ATO_REFUSE_PLUS_MINUS (1U << 30) #define SAFE_ATO_REFUSE_LEADING_ZERO (1U << 29) #define SAFE_ATO_REFUSE_LEADING_WHITESPACE (1U << 28) diff --git a/src/core/execute.c b/src/core/execute.c index 5d4c26934d..abf67a4ed4 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -636,7 +636,7 @@ void exec_context_init(ExecContext *c) { .timer_slack_nsec = NSEC_INFINITY, .personality = PERSONALITY_INVALID, .timeout_clean_usec = USEC_INFINITY, - .capability_bounding_set = CAP_MASK_UNSET, + .capability_bounding_set = CAP_MASK_ALL, .restrict_namespaces = NAMESPACE_FLAGS_INITIAL, .delegate_namespaces = NAMESPACE_FLAGS_INITIAL, .log_level_max = -1, diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 352ee7d677..2f212b6e7a 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -17,7 +17,6 @@ #include "bpf-restrict-fs.h" #include "bus-error.h" #include "calendarspec.h" -#include "capability-list.h" #include "capability-util.h" #include "cgroup-setup.h" #include "condition.h" @@ -1873,41 +1872,22 @@ int config_parse_capability_set( void *userdata) { uint64_t *capability_set = ASSERT_PTR(data); - uint64_t sum = 0, initial, def; - bool invert = false; int r; assert(filename); assert(lvalue); assert(rvalue); - if (rvalue[0] == '~') { - invert = true; - rvalue++; - } + uint64_t initial = streq(lvalue, "CapabilityBoundingSet") ? CAP_MASK_ALL : 0; - if (streq(lvalue, "CapabilityBoundingSet")) { - initial = CAP_MASK_ALL; /* initialized to all bits on */ - def = CAP_MASK_UNSET; /* not set */ - } else - def = initial = 0; /* All bits off */ - - r = capability_set_from_string(rvalue, &sum); + r = parse_capability_set(rvalue, initial, capability_set); if (r < 0) { log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse %s= specifier '%s', ignoring: %m", lvalue, rvalue); return 0; } - if (sum == 0 || *capability_set == def) - /* "", "~" or uninitialized data -> replace */ - *capability_set = invert ? ~sum : sum; - else { - /* previous data -> merge */ - if (invert) - *capability_set &= ~sum; - else - *capability_set |= sum; - } + if (*capability_set == CAP_MASK_UNSET) + *capability_set = 0; return 0; } diff --git a/src/core/main.c b/src/core/main.c index f1c424684f..183aac5379 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2768,7 +2768,7 @@ static void reset_arguments(void) { arg_default_environment = strv_free(arg_default_environment); arg_manager_environment = strv_free(arg_manager_environment); - arg_capability_bounding_set = CAP_MASK_UNSET; + arg_capability_bounding_set = CAP_MASK_ALL; arg_no_new_privs = false; arg_protect_system = -1; arg_timer_slack_nsec = NSEC_INFINITY; diff --git a/src/home/homectl.c b/src/home/homectl.c index a73d94744a..556ea9be81 100644 --- a/src/home/homectl.c +++ b/src/home/homectl.c @@ -107,8 +107,8 @@ static sd_json_format_flags_t arg_json_format_flags = SD_JSON_FORMAT_OFF; static bool arg_and_resize = false; static bool arg_and_change_password = false; static ExportFormat arg_export_format = EXPORT_FORMAT_FULL; -static uint64_t arg_capability_bounding_set = UINT64_MAX; -static uint64_t arg_capability_ambient_set = UINT64_MAX; +static uint64_t arg_capability_bounding_set = CAP_MASK_UNSET; +static uint64_t arg_capability_ambient_set = CAP_MASK_UNSET; static char *arg_blob_dir = NULL; static bool arg_blob_clear = false; static Hashmap *arg_blob_files = NULL; @@ -4784,9 +4784,8 @@ static int parse_argv(int argc, char *argv[]) { case ARG_CAPABILITY_AMBIENT_SET: case ARG_CAPABILITY_BOUNDING_SET: { _cleanup_strv_free_ char **l = NULL; - bool subtract = false; - uint64_t parsed, *which, updated; - const char *p, *field; + uint64_t *which; + const char *field; if (c == ARG_CAPABILITY_AMBIENT_SET) { which = &arg_capability_ambient_set; @@ -4797,42 +4796,27 @@ static int parse_argv(int argc, char *argv[]) { field = "capabilityBoundingSet"; } - if (isempty(optarg)) { + r = parse_capability_set(optarg, CAP_MASK_UNSET, which); + if (r == 0) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid capabilities in capability string '%s'.", optarg); + if (r < 0) + return log_error_errno(r, "Failed to parse capability string '%s': %m", optarg); + + if (*which == CAP_MASK_UNSET) { r = drop_from_identity(field); if (r < 0) return r; - *which = UINT64_MAX; break; } - p = optarg; - if (*p == '~') { - subtract = true; - p++; - } - - r = capability_set_from_string(p, &parsed); - if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Invalid capabilities in capability string '%s'.", p); - if (r < 0) - return log_error_errno(r, "Failed to parse capability string '%s': %m", p); - - if (*which == UINT64_MAX) - updated = subtract ? all_capabilities() & ~parsed : parsed; - else if (subtract) - updated = *which & ~parsed; - else - updated = *which | parsed; - - if (capability_set_to_strv(updated, &l) < 0) + if (capability_set_to_strv(*which, &l) < 0) return log_oom(); r = sd_json_variant_set_field_strv(match_identity ?: &arg_identity_extra, field, l); if (r < 0) return log_error_errno(r, "Failed to set %s field: %m", field); - *which = updated; break; } diff --git a/src/login/pam_systemd.c b/src/login/pam_systemd.c index 52f212757a..e4eb72553e 100644 --- a/src/login/pam_systemd.c +++ b/src/login/pam_systemd.c @@ -96,7 +96,7 @@ static int parse_caps( if (!caps) continue; - if (*caps == UINT64_MAX) + if (*caps == CAP_MASK_UNSET) b = subtract ? all_capabilities() : 0; else b = *caps; @@ -764,14 +764,14 @@ static int apply_user_record_settings( uint64_t a, b; a = user_record_capability_ambient_set(ur); - if (a == UINT64_MAX) + if (a == CAP_MASK_UNSET) a = default_capability_ambient_set; b = user_record_capability_bounding_set(ur); - if (b == UINT64_MAX) + if (b == CAP_MASK_UNSET) b = default_capability_bounding_set; - if (a != UINT64_MAX && a != 0) { + if (a != CAP_MASK_UNSET && a != 0) { a &= b; r = capability_ambient_set_apply(a, /* also_inherit= */ true); @@ -780,7 +780,7 @@ static int apply_user_record_settings( "Failed to set ambient capabilities, ignoring: %m"); } - if (b != UINT64_MAX && !cap_test_all(b)) { + if (b != CAP_MASK_UNSET && !cap_test_all(b)) { r = capability_bounding_set_drop(b, /* right_now= */ false); if (r < 0) pam_syslog_errno(handle, LOG_ERR, r, @@ -802,7 +802,7 @@ static uint64_t pick_default_capability_ambient_set( return ur && user_record_disposition(ur) == USER_REGULAR && - (streq_ptr(service, "systemd-user") || !isempty(seat)) ? (UINT64_C(1) << CAP_WAKE_ALARM) : UINT64_MAX; + (streq_ptr(service, "systemd-user") || !isempty(seat)) ? (UINT64_C(1) << CAP_WAKE_ALARM) : CAP_MASK_UNSET; } typedef struct SessionContext { @@ -1735,7 +1735,7 @@ _public_ PAM_EXTERN int pam_sm_open_session( pam_log_setup(); - uint64_t default_capability_bounding_set = UINT64_MAX, default_capability_ambient_set = UINT64_MAX; + uint64_t default_capability_bounding_set = CAP_MASK_UNSET, default_capability_ambient_set = CAP_MASK_UNSET; const char *class_pam = NULL, *type_pam = NULL, *desktop_pam = NULL, *area_pam = NULL; bool debug = false; if (parse_argv(handle, @@ -1800,7 +1800,7 @@ _public_ PAM_EXTERN int pam_sm_open_session( if (r != PAM_SUCCESS) return r; - if (default_capability_ambient_set == UINT64_MAX) + if (default_capability_ambient_set == CAP_MASK_UNSET) default_capability_ambient_set = pick_default_capability_ambient_set(ur, c.service, c.seat); r = apply_user_record_settings(handle, ur, debug, default_capability_bounding_set, default_capability_ambient_set); diff --git a/src/nspawn/nspawn-oci.c b/src/nspawn/nspawn-oci.c index 073f0f3ea9..b878d02351 100644 --- a/src/nspawn/nspawn-oci.c +++ b/src/nspawn/nspawn-oci.c @@ -324,7 +324,7 @@ static int oci_capabilities(const char *name, sd_json_variant *v, sd_json_dispat if (r < 0) return r; - if (s->full_capabilities.bounding != UINT64_MAX) { + if (s->full_capabilities.bounding != CAP_MASK_UNSET) { s->capability = s->full_capabilities.bounding; s->drop_capability = ~s->full_capabilities.bounding; } diff --git a/src/nspawn/nspawn.c b/src/nspawn/nspawn.c index 4cd16d9bc0..000ac3cce6 100644 --- a/src/nspawn/nspawn.c +++ b/src/nspawn/nspawn.c @@ -1694,7 +1694,7 @@ static int verify_arguments(void) { return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Cannot use --port= without private networking."); if (arg_caps_ambient) { - if (arg_caps_ambient == UINT64_MAX) + if (arg_caps_ambient == CAP_MASK_UNSET) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "AmbientCapability= does not support the value all."); if ((arg_caps_ambient & arg_caps_retain) != arg_caps_ambient) diff --git a/src/shared/user-record-show.c b/src/shared/user-record-show.c index f6ce5cff77..a43328c9fe 100644 --- a/src/shared/user-record-show.c +++ b/src/shared/user-record-show.c @@ -2,6 +2,7 @@ #include "alloc-util.h" #include "capability-list.h" +#include "capability-util.h" #include "format-util.h" #include "glyph-util.h" #include "hashmap.h" @@ -404,7 +405,7 @@ void user_record_show(UserRecord *hr, bool show_full_group_info) { printf(" Access Mode: 0%03o\n", user_record_access_mode(hr)); uint64_t caps = user_record_capability_bounding_set(hr); - if (caps != UINT64_MAX) { + if (caps != CAP_MASK_UNSET) { _cleanup_free_ char *scaps = NULL; (void) capability_set_to_string_negative(caps, &scaps); @@ -412,7 +413,7 @@ void user_record_show(UserRecord *hr, bool show_full_group_info) { } caps = user_record_capability_ambient_set(hr); - if (caps != UINT64_MAX) { + if (caps != CAP_MASK_UNSET) { _cleanup_free_ char *scaps = NULL; (void) capability_set_to_string(caps, &scaps); diff --git a/src/test/test-parse-util.c b/src/test/test-parse-util.c index 12692eb731..0c2cc500b1 100644 --- a/src/test/test-parse-util.c +++ b/src/test/test-parse-util.c @@ -5,6 +5,7 @@ #include #include +#include "capability-util.h" #include "locale-util.h" #include "parse-util.h" #include "tests.h" @@ -888,4 +889,127 @@ TEST(nft_identifier_valid) { ASSERT_FALSE(nft_identifier_valid(s)); } +static uint64_t make_cap(int cap) { + return ((uint64_t) 1ULL << (uint64_t) cap); +} + +TEST(parse_capability_set) { + uint64_t current; + + /* Empty string resets to CAP_MASK_UNSET */ + current = 0x1234; + ASSERT_OK(parse_capability_set("", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, CAP_MASK_UNSET); + + /* Single capability by name - replaces if current == initial */ + current = CAP_MASK_UNSET; + ASSERT_OK(parse_capability_set("cap_chown", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, make_cap(CAP_CHOWN)); + + /* Single capability by name - merges if current != initial */ + current = make_cap(CAP_SETUID); + ASSERT_OK(parse_capability_set("cap_chown", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, make_cap(CAP_CHOWN) | make_cap(CAP_SETUID)); + + /* Multiple capabilities - replaces when current == initial */ + current = CAP_MASK_UNSET; + ASSERT_OK(parse_capability_set("cap_chown cap_setuid", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, make_cap(CAP_CHOWN) | make_cap(CAP_SETUID)); + + /* Multiple capabilities - merges when current != initial */ + current = make_cap(CAP_SETGID); + ASSERT_OK(parse_capability_set("cap_chown cap_setuid", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, make_cap(CAP_CHOWN) | make_cap(CAP_SETUID) | make_cap(CAP_SETGID)); + + /* Inverted capabilities - replaces with complement when current == initial */ + current = CAP_MASK_UNSET; + ASSERT_OK(parse_capability_set("~cap_chown", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, all_capabilities() & ~make_cap(CAP_CHOWN)); + + /* Inverted capabilities - removes from current when current != initial */ + current = all_capabilities(); + ASSERT_OK(parse_capability_set("~cap_chown", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, all_capabilities() & ~make_cap(CAP_CHOWN)); + + /* Inverted multiple capabilities */ + current = all_capabilities(); + ASSERT_OK(parse_capability_set("~cap_chown cap_setuid", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, all_capabilities() & ~(make_cap(CAP_CHOWN) | make_cap(CAP_SETUID))); + + /* Tilde alone resets to all capabilities complement (i.e., empty) */ + current = 0x1234; + ASSERT_OK(parse_capability_set("~", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, all_capabilities()); + + /* Sequential calls - testing merge behavior */ + current = CAP_MASK_UNSET; + ASSERT_OK(parse_capability_set("cap_chown", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, make_cap(CAP_CHOWN)); + ASSERT_OK(parse_capability_set("cap_setuid", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, make_cap(CAP_CHOWN) | make_cap(CAP_SETUID)); + + /* Sequential calls with invert */ + current = all_capabilities(); + ASSERT_OK(parse_capability_set("~cap_chown", CAP_MASK_UNSET, ¤t)); + ASSERT_OK(parse_capability_set("~cap_setuid", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, all_capabilities() & ~(make_cap(CAP_CHOWN) | make_cap(CAP_SETUID))); + + /* Numeric capability */ + current = CAP_MASK_UNSET; + ASSERT_OK(parse_capability_set("0", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, make_cap(0)); + + current = CAP_MASK_UNSET; + ASSERT_OK(parse_capability_set("5", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, make_cap(5)); + + /* Mixed numeric and named capabilities */ + current = CAP_MASK_UNSET; + ASSERT_OK(parse_capability_set("0 cap_chown 5", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, make_cap(0) | make_cap(CAP_CHOWN) | make_cap(5)); + + /* Invalid capabilities are ignored but function returns 0 */ + current = CAP_MASK_UNSET; + ASSERT_OK_ZERO(parse_capability_set("invalid_cap", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, 0U); + + /* Mix of valid and invalid capabilities */ + current = CAP_MASK_UNSET; + ASSERT_OK_ZERO(parse_capability_set("cap_chown invalid_cap cap_setuid", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, make_cap(CAP_CHOWN) | make_cap(CAP_SETUID)); + + /* Case insensitivity */ + current = CAP_MASK_UNSET; + ASSERT_OK(parse_capability_set("CAP_CHOWN", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, make_cap(CAP_CHOWN)); + + current = CAP_MASK_UNSET; + ASSERT_OK(parse_capability_set("CaP_ChOwN", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, make_cap(CAP_CHOWN)); + + /* Inverted with invalid capabilities */ + current = all_capabilities(); + ASSERT_OK_ZERO(parse_capability_set("~invalid_cap", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, all_capabilities()); + + /* Inverted with mix of valid and invalid */ + current = all_capabilities(); + ASSERT_OK_ZERO(parse_capability_set("~cap_chown invalid_cap", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, all_capabilities() & ~make_cap(CAP_CHOWN)); + + /* Whitespace handling */ + current = 0; + ASSERT_OK(parse_capability_set(" cap_chown cap_setuid ", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, make_cap(CAP_CHOWN) | make_cap(CAP_SETUID)); + + /* Testing that initial value determines replace vs merge */ + current = make_cap(CAP_SETGID); + ASSERT_OK(parse_capability_set("cap_chown", make_cap(CAP_SETGID), ¤t)); + ASSERT_EQ(current, make_cap(CAP_CHOWN)); /* Replace because current == initial */ + + current = make_cap(CAP_SETGID); + ASSERT_OK(parse_capability_set("cap_chown", CAP_MASK_UNSET, ¤t)); + ASSERT_EQ(current, make_cap(CAP_CHOWN) | make_cap(CAP_SETGID)); /* Merge because current != initial */ +} + DEFINE_TEST_MAIN(LOG_INFO);