From: Daan De Meyer Date: Wed, 29 Oct 2025 21:39:48 +0000 (+0100) Subject: parse-util: Add parse_capability_set() X-Git-Tag: v259-rc1~196 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=10e82fde7b01e42aedbbfa978a7bfca47f21d341;p=thirdparty%2Fsystemd.git 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. --- diff --git a/src/analyze/analyze-security.c b/src/analyze/analyze-security.c index 1856a5b229d..d5adcf7cabc 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 90fe538a22d..c16b2ecdf68 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 4927bc9e562..39add96ec77 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 5d4c26934dd..abf67a4ed4a 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 352ee7d6775..2f212b6e7a1 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 f1c424684f7..183aac53791 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 a73d94744ad..556ea9be815 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 52f212757a6..e4eb72553e5 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 073f0f3ea94..b878d023516 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 4cd16d9bc07..000ac3cce67 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 f6ce5cff772..a43328c9fe0 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 12692eb7315..0c2cc500b13 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);