From 8142d73574b4743081e096bf15bee65e8214b656 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 20 Feb 2023 11:33:48 +0100 Subject: [PATCH 1/9] =?UTF-8?q?cap-list:=20rename=20capability=5Fset=5Fto?= =?UTF-8?q?=5Fstring=5Falloc()=20=E2=86=92=20capability=5Fset=5Fto=5Fstrin?= =?UTF-8?q?g()?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We typically don't use the _alloc() suffix anymore for anything, hence drop it here too. --- src/basic/cap-list.c | 7 +++---- src/basic/cap-list.h | 2 +- src/core/dbus-execute.c | 2 +- src/core/execute.c | 4 ++-- src/shared/bus-print-properties.c | 2 +- src/test/test-cap-list.c | 2 +- 6 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/basic/cap-list.c b/src/basic/cap-list.c index fb1ad14fd3..b764d23cca 100644 --- a/src/basic/cap-list.c +++ b/src/basic/cap-list.c @@ -56,11 +56,11 @@ int capability_list_length(void) { return (int) ELEMENTSOF(capability_names); } -int capability_set_to_string_alloc(uint64_t set, char **s) { +int capability_set_to_string(uint64_t set, char **ret) { _cleanup_free_ char *str = NULL; size_t n = 0; - assert(s); + assert(ret); for (unsigned i = 0; i <= cap_last_cap(); i++) if (set & (UINT64_C(1) << i)) { @@ -88,8 +88,7 @@ int capability_set_to_string_alloc(uint64_t set, char **s) { str[n > 0 ? n - 1 : 0] = '\0'; /* truncate the last space, if it's there */ - *s = TAKE_PTR(str); - + *ret = TAKE_PTR(str); return 0; } diff --git a/src/basic/cap-list.h b/src/basic/cap-list.h index 71235d6a28..42d29c4718 100644 --- a/src/basic/cap-list.h +++ b/src/basic/cap-list.h @@ -7,5 +7,5 @@ const char *capability_to_name(int id); int capability_from_name(const char *name); int capability_list_length(void); -int capability_set_to_string_alloc(uint64_t set, char **s); +int capability_set_to_string(uint64_t set, char **ret); int capability_set_from_string(const char *s, uint64_t *set); diff --git a/src/core/dbus-execute.c b/src/core/dbus-execute.c index f514b8fd12..b07b5775ff 100644 --- a/src/core/dbus-execute.c +++ b/src/core/dbus-execute.c @@ -1671,7 +1671,7 @@ static BUS_DEFINE_SET_TRANSIENT_PARSE(proc_subset, ProcSubset, proc_subset_from_ static BUS_DEFINE_SET_TRANSIENT_PARSE(preserve_mode, ExecPreserveMode, exec_preserve_mode_from_string); static BUS_DEFINE_SET_TRANSIENT_PARSE_PTR(personality, unsigned long, parse_personality); static BUS_DEFINE_SET_TRANSIENT_TO_STRING_ALLOC(secure_bits, "i", int32_t, int, "%" PRIi32, secure_bits_to_string_alloc_with_check); -static BUS_DEFINE_SET_TRANSIENT_TO_STRING_ALLOC(capability, "t", uint64_t, uint64_t, "%" PRIu64, capability_set_to_string_alloc); +static BUS_DEFINE_SET_TRANSIENT_TO_STRING_ALLOC(capability, "t", uint64_t, uint64_t, "%" PRIu64, capability_set_to_string); static BUS_DEFINE_SET_TRANSIENT_TO_STRING_ALLOC(namespace_flag, "t", uint64_t, unsigned long, "%" PRIu64, namespace_flags_to_string); static BUS_DEFINE_SET_TRANSIENT_TO_STRING(mount_flags, "t", uint64_t, unsigned long, "%" PRIu64, mount_propagation_flag_to_string_with_check); diff --git a/src/core/execute.c b/src/core/execute.c index 36d664c7ee..5116a73617 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -6195,7 +6195,7 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) { if (c->capability_bounding_set != CAP_ALL) { _cleanup_free_ char *str = NULL; - r = capability_set_to_string_alloc(c->capability_bounding_set, &str); + r = capability_set_to_string(c->capability_bounding_set, &str); if (r >= 0) fprintf(f, "%sCapabilityBoundingSet: %s\n", prefix, str); } @@ -6203,7 +6203,7 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) { if (c->capability_ambient_set != 0) { _cleanup_free_ char *str = NULL; - r = capability_set_to_string_alloc(c->capability_ambient_set, &str); + r = capability_set_to_string(c->capability_ambient_set, &str); if (r >= 0) fprintf(f, "%sAmbientCapabilities: %s\n", prefix, str); } diff --git a/src/shared/bus-print-properties.c b/src/shared/bus-print-properties.c index 9369866fa1..db41ad2495 100644 --- a/src/shared/bus-print-properties.c +++ b/src/shared/bus-print-properties.c @@ -145,7 +145,7 @@ static int bus_print_property(const char *name, const char *expected_value, sd_b } else if (STR_IN_SET(name, "CapabilityBoundingSet", "AmbientCapabilities")) { _cleanup_free_ char *s = NULL; - r = capability_set_to_string_alloc(u, &s); + r = capability_set_to_string(u, &s); if (r < 0) return r; diff --git a/src/test/test-cap-list.c b/src/test/test-cap-list.c index 68f0461a62..31c61b8d8d 100644 --- a/src/test/test-cap-list.c +++ b/src/test/test-cap-list.c @@ -57,7 +57,7 @@ static void test_capability_set_one(uint64_t c, const char *t) { _cleanup_free_ char *t1 = NULL; uint64_t c1, c_masked = c & all_capabilities(); - assert_se(capability_set_to_string_alloc(c, &t1) == 0); + assert_se(capability_set_to_string(c, &t1) == 0); assert_se(streq(t1, t)); assert_se(capability_set_from_string(t1, &c1) == 0); From 3f444e94f5a7ccc8d4f16a331858b0a4e717b773 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 20 Feb 2023 12:25:44 +0100 Subject: [PATCH 2/9] cap-list: modernize capability_set_from_string() a bit Make return parameter optional. And return whether there were any caps we didn't recognize via 0/1 return value. --- src/basic/cap-list.c | 25 +++++++++++++------------ src/basic/cap-list.h | 2 +- src/test/test-cap-list.c | 12 ++++++------ 3 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/basic/cap-list.c b/src/basic/cap-list.c index b764d23cca..0af37b94ee 100644 --- a/src/basic/cap-list.c +++ b/src/basic/cap-list.c @@ -92,29 +92,30 @@ int capability_set_to_string(uint64_t set, char **ret) { return 0; } -int capability_set_from_string(const char *s, uint64_t *set) { +int capability_set_from_string(const char *s, uint64_t *ret) { uint64_t val = 0; - - assert(set); + bool good = true; for (const char *p = s;;) { _cleanup_free_ char *word = NULL; int r; - r = extract_first_word(&p, &word, NULL, EXTRACT_UNQUOTE); - if (r == -ENOMEM) + r = extract_first_word(&p, &word, NULL, EXTRACT_UNQUOTE|EXTRACT_RELAX); + if (r < 0) return r; - if (r <= 0) + if (r == 0) break; r = capability_from_name(word); - if (r < 0) - continue; - - val |= ((uint64_t) UINT64_C(1)) << (uint64_t) r; + if (r < 0) { + log_debug_errno(r, "Failed to parse capability '%s', ignoring: %m", word); + good = false; + } else + val |= UINT64_C(1) << r; } - *set = val; + if (ret) + *ret = val; - return 0; + return good; } diff --git a/src/basic/cap-list.h b/src/basic/cap-list.h index 42d29c4718..888e9223e7 100644 --- a/src/basic/cap-list.h +++ b/src/basic/cap-list.h @@ -8,4 +8,4 @@ int capability_from_name(const char *name); int capability_list_length(void); int capability_set_to_string(uint64_t set, char **ret); -int capability_set_from_string(const char *s, uint64_t *set); +int capability_set_from_string(const char *s, uint64_t *ret); diff --git a/src/test/test-cap-list.c b/src/test/test-cap-list.c index 31c61b8d8d..cc5f3a0c35 100644 --- a/src/test/test-cap-list.c +++ b/src/test/test-cap-list.c @@ -60,7 +60,7 @@ static void test_capability_set_one(uint64_t c, const char *t) { assert_se(capability_set_to_string(c, &t1) == 0); assert_se(streq(t1, t)); - assert_se(capability_set_from_string(t1, &c1) == 0); + assert_se(capability_set_from_string(t1, &c1) > 0); assert_se(c1 == c_masked); free(t1); @@ -73,19 +73,19 @@ static void test_capability_set_one(uint64_t c, const char *t) { TEST(capability_set_from_string) { uint64_t c; - assert_se(capability_set_from_string(NULL, &c) == 0); + assert_se(capability_set_from_string(NULL, &c) > 0); assert_se(c == 0); - assert_se(capability_set_from_string("", &c) == 0); + assert_se(capability_set_from_string("", &c) > 0); assert_se(c == 0); - assert_se(capability_set_from_string("0", &c) == 0); + assert_se(capability_set_from_string("0", &c) > 0); assert_se(c == UINT64_C(1)); - assert_se(capability_set_from_string("1", &c) == 0); + assert_se(capability_set_from_string("1", &c) > 0); assert_se(c == UINT64_C(1) << 1); - assert_se(capability_set_from_string("0 1 2 3", &c) == 0); + assert_se(capability_set_from_string("0 1 2 3", &c) > 0); assert_se(c == (UINT64_C(1) << 4) - 1); } From d0e67c69ba7fa9984a2d96caa8ad4b7762ed3160 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 20 Feb 2023 12:26:46 +0100 Subject: [PATCH 3/9] cap-list: refuse parsing numeric capability 63 We refuse it otherwise currently, simply because we cannot store it in a uint64_t caps mask value anymore while retaining the ability to use UINT64_MAX as "unset" marker. The check actually was in place already, just one off. --- src/basic/cap-list.c | 6 +++--- src/basic/capability-util.c | 8 ++++---- src/test/test-cap-list.c | 7 ++++--- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/basic/cap-list.c b/src/basic/cap-list.c index 0af37b94ee..bcf7e597c2 100644 --- a/src/basic/cap-list.c +++ b/src/basic/cap-list.c @@ -35,10 +35,10 @@ int capability_from_name(const char *name) { /* Try to parse numeric capability */ r = safe_atoi(name, &i); if (r >= 0) { - if (i >= 0 && i < 64) - return i; - else + if (i < 0 || i >= 63) return -EINVAL; + + return i; } /* Try to parse string capability */ diff --git a/src/basic/capability-util.c b/src/basic/capability-util.c index 5ab110d341..13aa6fee43 100644 --- a/src/basic/capability-util.c +++ b/src/basic/capability-util.c @@ -47,11 +47,11 @@ unsigned cap_last_cap(void) { r = safe_atolu(content, &p); if (r >= 0) { - if (p > 63) /* Safety for the future: if one day the kernel learns more than 64 caps, + if (p > 62) /* Safety for the future: if one day the kernel learns more than 64 caps, * then we are in trouble (since we, as much userspace and kernel space * store capability masks in uint64_t types). Let's hence protect * ourselves against that and always cap at 63 for now. */ - p = 63; + p = 62; saved = p; valid = true; @@ -60,7 +60,7 @@ unsigned cap_last_cap(void) { } /* fall back to syscall-probing for pre linux-3.2 */ - p = MIN((unsigned long) CAP_LAST_CAP, 63U); + p = MIN((unsigned long) CAP_LAST_CAP, 62U); if (prctl(PR_CAPBSET_READ, p) < 0) { @@ -72,7 +72,7 @@ unsigned cap_last_cap(void) { } else { /* Hmm, look upwards, until we find one that doesn't work */ - for (; p < 63; p++) + for (; p < 62; p++) if (prctl(PR_CAPBSET_READ, p+1) < 0) break; } diff --git a/src/test/test-cap-list.c b/src/test/test-cap-list.c index cc5f3a0c35..517641a019 100644 --- a/src/test/test-cap-list.c +++ b/src/test/test-cap-list.c @@ -29,7 +29,8 @@ TEST(cap_list) { assert_se(capability_from_name("cAp_aUdIt_rEAd") == CAP_AUDIT_READ); assert_se(capability_from_name("0") == 0); assert_se(capability_from_name("15") == 15); - assert_se(capability_from_name("63") == 63); + assert_se(capability_from_name("62") == 62); + assert_se(capability_from_name("63") == -EINVAL); assert_se(capability_from_name("64") == -EINVAL); assert_se(capability_from_name("-1") == -EINVAL); @@ -117,9 +118,9 @@ static void test_capability_set_to_string_invalid(uint64_t invalid_cap_set) { TEST(capability_set_to_string) { test_capability_set_to_string_invalid(0); - /* once the kernel supports 63 caps, there are no 'invalid' numbers + /* once the kernel supports 62 caps, there are no 'invalid' numbers * for us to test with */ - if (cap_last_cap() < 63) + if (cap_last_cap() < 62) test_capability_set_to_string_invalid(all_capabilities() + 1); } From c52c4d6974e3bc6d8ffdd3b73207617e06cd5157 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 20 Feb 2023 11:43:13 +0100 Subject: [PATCH 4/9] cap-list: add CAPABILITY_TO_STRING() macro using compound initialization to allocate fallback buffer Let's add a helper that can return a numeric string in case we don't recognize a name for a capability. --- src/basic/cap-list.c | 17 ++++++++++++++++- src/basic/cap-list.h | 8 ++++++++ src/test/test-cap-list.c | 9 +++++++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/basic/cap-list.c b/src/basic/cap-list.c index bcf7e597c2..7843efc994 100644 --- a/src/basic/cap-list.c +++ b/src/basic/cap-list.c @@ -19,13 +19,28 @@ static const struct capability_name* lookup_capability(register const char *str, const char *capability_to_name(int id) { if (id < 0) return NULL; - if ((size_t) id >= ELEMENTSOF(capability_names)) return NULL; return capability_names[id]; } +const char *capability_to_string(int id, char buf[static CAPABILITY_TO_STRING_MAX]) { + const char *p; + + if (id < 0) + return NULL; + if (id >= 63) /* refuse caps >= 63 since we can't store them in a uint64_t mask anymore, and still retain UINT64_MAX as marker for "unset" */ + return NULL; + + p = capability_to_name(id); + if (p) + return p; + + sprintf(buf, "0x%x", (unsigned) id); /* numerical fallback */ + return buf; +} + int capability_from_name(const char *name) { const struct capability_name *sc; int r, i; diff --git a/src/basic/cap-list.h b/src/basic/cap-list.h index 888e9223e7..bddadcbae5 100644 --- a/src/basic/cap-list.h +++ b/src/basic/cap-list.h @@ -3,7 +3,15 @@ #include +/* Space for capability_to_string() in case we write out a numeric capability because we don't know the name + * for it. "0x3e" is the largest string we might output, in both sensese of the word "largest": two chars for + * "0x", two bytes for the hex value, and one trailing NUL byte. */ +#define CAPABILITY_TO_STRING_MAX (2 + 2 + 1) + const char *capability_to_name(int id); +const char *capability_to_string(int id, char buf[static CAPABILITY_TO_STRING_MAX]); +#define CAPABILITY_TO_STRING(id) capability_to_string(id, (char[CAPABILITY_TO_STRING_MAX]) {}) + int capability_from_name(const char *name); int capability_list_length(void); diff --git a/src/test/test-cap-list.c b/src/test/test-cap-list.c index 517641a019..8df425bdd2 100644 --- a/src/test/test-cap-list.c +++ b/src/test/test-cap-list.c @@ -14,6 +14,13 @@ TEST(cap_list) { assert_se(!capability_to_name(-1)); assert_se(!capability_to_name(capability_list_length())); + assert_se(!capability_to_name(63)); + assert_se(!capability_to_name(64)); + + assert_se(!CAPABILITY_TO_STRING(-1)); + if (capability_list_length() <= 62) + assert_se(streq(CAPABILITY_TO_STRING(62), "0x3e")); + assert_se(!CAPABILITY_TO_STRING(64)); for (int i = 0; i < capability_list_length(); i++) { const char *n; @@ -21,6 +28,8 @@ TEST(cap_list) { assert_se(n = capability_to_name(i)); assert_se(capability_from_name(n) == i); printf("%s = %i\n", n, i); + + assert_se(streq(CAPABILITY_TO_STRING(i), n)); } assert_se(capability_from_name("asdfbsd") == -EINVAL); From ebb93f3ccfbab668c4e3f77041d97162fadb742b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 20 Feb 2023 11:45:07 +0100 Subject: [PATCH 5/9] cap-list: rework capability_set_to_string() Let's use strextend_with_separator() and CAPABILITY_TO_STRING(). --- src/basic/cap-list.c | 37 +++++++++++++++---------------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/src/basic/cap-list.c b/src/basic/cap-list.c index 7843efc994..3b506ed596 100644 --- a/src/basic/cap-list.c +++ b/src/basic/cap-list.c @@ -10,6 +10,7 @@ #include "macro.h" #include "parse-util.h" #include "stdio-util.h" +#include "string-util.h" static const struct capability_name* lookup_capability(register const char *str, register GPERF_LEN_TYPE len); @@ -73,35 +74,27 @@ int capability_list_length(void) { int capability_set_to_string(uint64_t set, char **ret) { _cleanup_free_ char *str = NULL; - size_t n = 0; assert(ret); - for (unsigned i = 0; i <= cap_last_cap(); i++) - if (set & (UINT64_C(1) << i)) { - const char *p; - char buf[2 + 16 + 1]; - size_t add; + for (unsigned i = 0; i <= cap_last_cap(); i++) { + const char *p; - p = capability_to_name(i); - if (!p) { - xsprintf(buf, "0x%x", i); - p = buf; - } + if (!FLAGS_SET(set, UINT64_C(1) << i)) + continue; - add = strlen(p); + p = CAPABILITY_TO_STRING(i); + assert(p); - if (!GREEDY_REALLOC(str, n + add + 2)) - return -ENOMEM; + if (!strextend_with_separator(&str, " ", p)) + return -ENOMEM; + } - strcpy(mempcpy(str + n, p, add), " "); - n += add + 1; - } - - if (!GREEDY_REALLOC(str, n + 1)) - return -ENOMEM; - - str[n > 0 ? n - 1 : 0] = '\0'; /* truncate the last space, if it's there */ + if (!str) { + str = new0(char, 1); + if (!str) + return -ENOMEM; + } *ret = TAKE_PTR(str); return 0; From 6c5fff017ea460ecda9915654b4d2883fea04ba7 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 20 Feb 2023 11:30:56 +0100 Subject: [PATCH 6/9] cap-list: make sure never to accidentally return more than 63 caps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rest of our codebase stores caps masks in a uint64_t, and also assumes UINT64_MAX was a suitable value for "unset mask". Hence refuse any caps outside of 0…62. (right now the kernel knows 40 caps, hence 22 more to go before we have to reconsider our life's choices.) --- src/basic/cap-list.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/basic/cap-list.c b/src/basic/cap-list.c index 3b506ed596..811adb0242 100644 --- a/src/basic/cap-list.c +++ b/src/basic/cap-list.c @@ -20,7 +20,7 @@ static const struct capability_name* lookup_capability(register const char *str, const char *capability_to_name(int id) { if (id < 0) return NULL; - if ((size_t) id >= ELEMENTSOF(capability_names)) + if (id >= capability_list_length()) return NULL; return capability_names[id]; @@ -65,11 +65,13 @@ int capability_from_name(const char *name) { return sc->id; } -/* This is the number of capability names we are *compiled* with. - * For the max capability number of the currently-running kernel, - * use cap_last_cap(). */ +/* This is the number of capability names we are *compiled* with. For the max capability number of the + * currently-running kernel, use cap_last_cap(). Note that this one returns the size of the array, i.e. one + * value larger than the last known capability. This is different from cap_last_cap() which returns the + * highest supported capability. Hence with everyone agreeing on the same capabilities list, this function + * will return one higher than cap_last_cap(). */ int capability_list_length(void) { - return (int) ELEMENTSOF(capability_names); + return (int) MIN(ELEMENTSOF(capability_names), 63U); } int capability_set_to_string(uint64_t set, char **ret) { From 3fd5190b5e0f2ba08b12cb53e3c27fc1e54a4496 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 20 Feb 2023 12:53:26 +0100 Subject: [PATCH 7/9] capability-util: add CAP_MASK_ALL + CAP_MASK_UNSET macros We should be more careful with distinguishing the cases "all bits set in caps mask" from "cap mask invalid". We so far mostly used UINT64_MAX for both, which is not correct though (as it would mean AmbientCapabilities=~0 followed by AmbientCapabilities=0) would result in capability 63 to be set (which we don't really allow, since that means unset). --- src/basic/capability-util.c | 16 ++++++++-------- src/basic/capability-util.h | 18 +++++++++++------- src/core/execute.c | 4 ++-- src/core/load-fragment.c | 12 +++++++----- src/core/main.c | 2 +- 5 files changed, 29 insertions(+), 23 deletions(-) diff --git a/src/basic/capability-util.c b/src/basic/capability-util.c index 13aa6fee43..8cc75c06c8 100644 --- a/src/basic/capability-util.c +++ b/src/basic/capability-util.c @@ -405,7 +405,7 @@ bool capability_quintet_mangle(CapabilityQuintet *q) { combined = q->effective | q->bounding | q->inheritable | q->permitted; - ambient_supported = q->ambient != UINT64_MAX; + ambient_supported = q->ambient != CAP_MASK_UNSET; if (ambient_supported) combined |= q->ambient; @@ -437,7 +437,7 @@ int capability_quintet_enforce(const CapabilityQuintet *q) { _cleanup_cap_free_ cap_t c = NULL, modified = NULL; int r; - if (q->ambient != UINT64_MAX) { + if (q->ambient != CAP_MASK_UNSET) { bool changed = false; c = cap_get_proc(); @@ -479,7 +479,7 @@ int capability_quintet_enforce(const CapabilityQuintet *q) { return r; } - if (q->inheritable != UINT64_MAX || q->permitted != UINT64_MAX || q->effective != UINT64_MAX) { + if (q->inheritable != CAP_MASK_UNSET || q->permitted != CAP_MASK_UNSET || q->effective != CAP_MASK_UNSET) { bool changed = false; if (!c) { @@ -492,7 +492,7 @@ int capability_quintet_enforce(const CapabilityQuintet *q) { uint64_t m = UINT64_C(1) << i; cap_value_t cv = (cap_value_t) i; - if (q->inheritable != UINT64_MAX) { + if (q->inheritable != CAP_MASK_UNSET) { cap_flag_value_t old_value, new_value; if (cap_get_flag(c, cv, CAP_INHERITABLE, &old_value) < 0) { @@ -515,7 +515,7 @@ int capability_quintet_enforce(const CapabilityQuintet *q) { } } - if (q->permitted != UINT64_MAX) { + if (q->permitted != CAP_MASK_UNSET) { cap_flag_value_t old_value, new_value; if (cap_get_flag(c, cv, CAP_PERMITTED, &old_value) < 0) { @@ -535,7 +535,7 @@ int capability_quintet_enforce(const CapabilityQuintet *q) { } } - if (q->effective != UINT64_MAX) { + if (q->effective != CAP_MASK_UNSET) { cap_flag_value_t old_value, new_value; if (cap_get_flag(c, cv, CAP_EFFECTIVE, &old_value) < 0) { @@ -559,7 +559,7 @@ int capability_quintet_enforce(const CapabilityQuintet *q) { if (changed) { /* In order to change the bounding caps, we need to keep CAP_SETPCAP for a bit * longer. Let's add it to our list hence for now. */ - if (q->bounding != UINT64_MAX) { + if (q->bounding != CAP_MASK_UNSET) { cap_value_t cv = CAP_SETPCAP; modified = cap_dup(c); @@ -587,7 +587,7 @@ int capability_quintet_enforce(const CapabilityQuintet *q) { } } - if (q->bounding != UINT64_MAX) { + if (q->bounding != CAP_MASK_UNSET) { r = capability_bounding_set_drop(q->bounding, false); if (r < 0) return r; diff --git a/src/basic/capability-util.h b/src/basic/capability-util.h index 4d1b0521f4..48e8db35f6 100644 --- a/src/basic/capability-util.h +++ b/src/basic/capability-util.h @@ -9,7 +9,11 @@ #include "macro.h" #include "missing_capability.h" -#define CAP_ALL UINT64_MAX +/* Special marker used when storing a capabilities mask as "unset" */ +#define CAP_MASK_UNSET UINT64_MAX + +/* All possible capabilities bits on */ +#define CAP_MASK_ALL UINT64_C(0x7fffffffffffffff) unsigned cap_last_cap(void); int have_effective_cap(int value); @@ -59,14 +63,14 @@ typedef struct CapabilityQuintet { assert_cc(CAP_LAST_CAP < 64); -#define CAPABILITY_QUINTET_NULL { UINT64_MAX, UINT64_MAX, UINT64_MAX, UINT64_MAX, UINT64_MAX } +#define CAPABILITY_QUINTET_NULL { CAP_MASK_UNSET, CAP_MASK_UNSET, CAP_MASK_UNSET, CAP_MASK_UNSET, CAP_MASK_UNSET } static inline bool capability_quintet_is_set(const CapabilityQuintet *q) { - return q->effective != UINT64_MAX || - q->bounding != UINT64_MAX || - q->inheritable != UINT64_MAX || - q->permitted != UINT64_MAX || - q->ambient != UINT64_MAX; + return q->effective != CAP_MASK_UNSET || + q->bounding != CAP_MASK_UNSET || + q->inheritable != CAP_MASK_UNSET || + q->permitted != CAP_MASK_UNSET || + q->ambient != CAP_MASK_UNSET; } /* Mangles the specified caps quintet taking the current bounding set into account: diff --git a/src/core/execute.c b/src/core/execute.c index 5116a73617..f38a5a41fe 100644 --- a/src/core/execute.c +++ b/src/core/execute.c @@ -5466,7 +5466,7 @@ void exec_context_init(ExecContext *c) { for (ExecDirectoryType t = 0; t < _EXEC_DIRECTORY_TYPE_MAX; t++) c->directories[t].mode = 0755; c->timeout_clean_usec = USEC_INFINITY; - c->capability_bounding_set = CAP_ALL; + c->capability_bounding_set = CAP_MASK_UNSET; assert_cc(NAMESPACE_FLAGS_INITIAL != NAMESPACE_FLAGS_ALL); c->restrict_namespaces = NAMESPACE_FLAGS_INITIAL; c->log_level_max = -1; @@ -6192,7 +6192,7 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) { fprintf(f, "%sSecure Bits: %s\n", prefix, str); } - if (c->capability_bounding_set != CAP_ALL) { + if (c->capability_bounding_set != CAP_MASK_UNSET) { _cleanup_free_ char *str = NULL; r = capability_set_to_string(c->capability_bounding_set, &str); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index ec782703a7..d4a874eafa 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -1886,7 +1886,7 @@ int config_parse_capability_set( void *userdata) { uint64_t *capability_set = ASSERT_PTR(data); - uint64_t sum = 0, initial = 0; + uint64_t sum = 0, initial, def; bool invert = false; int r; @@ -1899,9 +1899,11 @@ int config_parse_capability_set( rvalue++; } - if (streq(lvalue, "CapabilityBoundingSet")) - initial = CAP_ALL; /* initialized to all bits on */ - /* else "AmbientCapabilities" initialized to all bits off */ + 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); if (r < 0) { @@ -1909,7 +1911,7 @@ int config_parse_capability_set( return 0; } - if (sum == 0 || *capability_set == initial) + if (sum == 0 || *capability_set == def) /* "", "~" or uninitialized data -> replace */ *capability_set = invert ? ~sum : sum; else { diff --git a/src/core/main.c b/src/core/main.c index f28448f9e4..1af9b8b505 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -2458,7 +2458,7 @@ static void reset_arguments(void) { arg_manager_environment = strv_free(arg_manager_environment); rlimit_free_all(arg_default_rlimit); - arg_capability_bounding_set = CAP_ALL; + arg_capability_bounding_set = CAP_MASK_UNSET; arg_no_new_privs = false; arg_timer_slack_nsec = NSEC_INFINITY; arg_default_timer_accuracy_usec = 1 * USEC_PER_MINUTE; From 3b3ebabfa6e6b25e0ec947875b90307078608139 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 20 Feb 2023 16:21:25 +0100 Subject: [PATCH 8/9] capability-util: add macro for largest cap we're willing to accept Let's hide the hard to grasp 62 behind a name. --- src/basic/cap-list.c | 6 +++--- src/basic/capability-util.c | 16 +++++++++------- src/basic/capability-util.h | 4 ++++ 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/src/basic/cap-list.c b/src/basic/cap-list.c index 811adb0242..45a5b371e8 100644 --- a/src/basic/cap-list.c +++ b/src/basic/cap-list.c @@ -31,7 +31,7 @@ const char *capability_to_string(int id, char buf[static CAPABILITY_TO_STRING_MA if (id < 0) return NULL; - if (id >= 63) /* refuse caps >= 63 since we can't store them in a uint64_t mask anymore, and still retain UINT64_MAX as marker for "unset" */ + if (id > CAP_LIMIT) /* refuse caps > 62 since we can't store them in a uint64_t mask anymore, and still retain UINT64_MAX as marker for "unset" */ return NULL; p = capability_to_name(id); @@ -51,7 +51,7 @@ int capability_from_name(const char *name) { /* Try to parse numeric capability */ r = safe_atoi(name, &i); if (r >= 0) { - if (i < 0 || i >= 63) + if (i < 0 || i > CAP_LIMIT) return -EINVAL; return i; @@ -71,7 +71,7 @@ int capability_from_name(const char *name) { * highest supported capability. Hence with everyone agreeing on the same capabilities list, this function * will return one higher than cap_last_cap(). */ int capability_list_length(void) { - return (int) MIN(ELEMENTSOF(capability_names), 63U); + return MIN((int) ELEMENTSOF(capability_names), CAP_LIMIT + 1); } int capability_set_to_string(uint64_t set, char **ret) { diff --git a/src/basic/capability-util.c b/src/basic/capability-util.c index 8cc75c06c8..5dd2f0a63c 100644 --- a/src/basic/capability-util.c +++ b/src/basic/capability-util.c @@ -47,11 +47,13 @@ unsigned cap_last_cap(void) { r = safe_atolu(content, &p); if (r >= 0) { - if (p > 62) /* Safety for the future: if one day the kernel learns more than 64 caps, - * then we are in trouble (since we, as much userspace and kernel space - * store capability masks in uint64_t types). Let's hence protect - * ourselves against that and always cap at 63 for now. */ - p = 62; + if (p > CAP_LIMIT) /* Safety for the future: if one day the kernel learns more than + * 64 caps, then we are in trouble (since we, as much userspace + * and kernel space store capability masks in uint64_t types). We + * also want to use UINT64_MAX as marker for "unset". Hence let's + * hence protect ourselves against that and always cap at 62 for + * now. */ + p = CAP_LIMIT; saved = p; valid = true; @@ -60,7 +62,7 @@ unsigned cap_last_cap(void) { } /* fall back to syscall-probing for pre linux-3.2 */ - p = MIN((unsigned long) CAP_LAST_CAP, 62U); + p = (unsigned long) MIN(CAP_LAST_CAP, CAP_LIMIT); if (prctl(PR_CAPBSET_READ, p) < 0) { @@ -72,7 +74,7 @@ unsigned cap_last_cap(void) { } else { /* Hmm, look upwards, until we find one that doesn't work */ - for (; p < 62; p++) + for (; p < CAP_LIMIT; p++) if (prctl(PR_CAPBSET_READ, p+1) < 0) break; } diff --git a/src/basic/capability-util.h b/src/basic/capability-util.h index 48e8db35f6..07b13862c7 100644 --- a/src/basic/capability-util.h +++ b/src/basic/capability-util.h @@ -15,6 +15,10 @@ /* All possible capabilities bits on */ #define CAP_MASK_ALL UINT64_C(0x7fffffffffffffff) +/* The largest capability we can deal with, given we want to be able to store cap masks in uint64_t but still + * be able to use UINT64_MAX as indicator for "not set". The latter makes capability 63 unavailable. */ +#define CAP_LIMIT 62 + unsigned cap_last_cap(void); int have_effective_cap(int value); int capability_gain_cap_setpcap(cap_t *return_caps); From 76875ba9cbf750ceef3aa6b2725c98a23e7e8c29 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 20 Feb 2023 16:26:46 +0100 Subject: [PATCH 9/9] capability-util: use UINT32_MAX as shortcut where appropriatea --- src/basic/capability-util.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/basic/capability-util.c b/src/basic/capability-util.c index 5dd2f0a63c..bda7cbf45c 100644 --- a/src/basic/capability-util.c +++ b/src/basic/capability-util.c @@ -283,8 +283,8 @@ static int drop_from_file(const char *fn, uint64_t keep) { if (current == after) return 0; - lo = after & UINT32_C(0xFFFFFFFF); - hi = (after >> 32) & UINT32_C(0xFFFFFFFF); + lo = after & UINT32_MAX; + hi = (after >> 32) & UINT32_MAX; return write_string_filef(fn, 0, "%" PRIu32 " %" PRIu32, lo, hi); }