From ac8e381e262f16d77bef2bc77de1d864042dd5f4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Nov 2024 16:37:14 +0100 Subject: [PATCH 1/2] user-record: only synthesize default list of self-modifiable fields for *regular* users For system users we should lock things down, hence generate an empty list. This is mostly a safety precaution, but also hides really confusing output of "userdbctl user" for an system user. Follow-up for: a192250eda1e5cc1f8fc799cf9b85d37e7fa0519 --- src/shared/user-record.c | 19 ++++++++++++++++--- src/test/test-user-record.c | 2 +- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/shared/user-record.c b/src/shared/user-record.c index a63b907c74..4557718023 100644 --- a/src/shared/user-record.c +++ b/src/shared/user-record.c @@ -2165,8 +2165,15 @@ const char** user_record_self_modifiable_fields(UserRecord *h) { assert(h); + /* Note: if the self_modifiable_fields field in UserRecord is NULL we'll apply a default, if we have + * one. If it is a non-NULL empty strv, we'll report it as explicit empty list. When the field is + * NULL and we have no default list we'll return NULL. */ + /* Note that we intentionally distinguish between NULL and an empty array here */ - return (const char**) h->self_modifiable_fields ?: (const char**) default_fields; + if (h->self_modifiable_fields) + return (const char**) h->self_modifiable_fields; + + return user_record_disposition(h) == USER_REGULAR ? (const char**) default_fields : NULL; } const char** user_record_self_modifiable_blobs(UserRecord *h) { @@ -2180,7 +2187,10 @@ const char** user_record_self_modifiable_blobs(UserRecord *h) { assert(h); /* Note that we intentionally distinguish between NULL and an empty array here */ - return (const char**) h->self_modifiable_blobs ?: (const char**) default_blobs; + if (h->self_modifiable_blobs) + return (const char**) h->self_modifiable_blobs; + + return user_record_disposition(h) == USER_REGULAR ? (const char**) default_blobs : NULL; } const char** user_record_self_modifiable_privileged(UserRecord *h) { @@ -2201,7 +2211,10 @@ const char** user_record_self_modifiable_privileged(UserRecord *h) { assert(h); /* Note that we intentionally distinguish between NULL and an empty array here */ - return (const char**) h->self_modifiable_privileged ?: (const char**) default_fields; + if (h->self_modifiable_privileged) + return (const char**) h->self_modifiable_privileged; + + return user_record_disposition(h) == USER_REGULAR ? (const char**) default_fields : NULL; } static int remove_self_modifiable_json_fields_common(UserRecord *current, sd_json_variant **target) { diff --git a/src/test/test-user-record.c b/src/test/test-user-record.c index 3a7e8e28af..2a4df190a1 100644 --- a/src/test/test-user-record.c +++ b/src/test/test-user-record.c @@ -9,7 +9,7 @@ ({ \ typeof(ret) _r = (ret); \ user_record_unref(*_r); \ - assert_se(user_record_build((ret), SD_JSON_BUILD_OBJECT(__VA_ARGS__)) >= 0); \ + assert_se(user_record_build((ret), SD_JSON_BUILD_OBJECT(SD_JSON_BUILD_PAIR_STRING("disposition", "regular"), __VA_ARGS__)) >= 0); \ 0; \ }) From ebe37f771cb44daa216c34cefc41ba71e93413ee Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Nov 2024 16:35:32 +0100 Subject: [PATCH 2/2] user-record: distinguish explicit and implicit empty modifiable lists case We now distinguish two cases: where the list of self modifiable fields is explicitly set to empty, and where the default is empty. Let's display them differently in the output. When set explicitly to empty let's mention the admin, otherwise just say "none". --- src/shared/user-record-show.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/shared/user-record-show.c b/src/shared/user-record-show.c index e6de0cd002..4d8ffe1c35 100644 --- a/src/shared/user-record-show.c +++ b/src/shared/user-record-show.c @@ -28,21 +28,28 @@ const char* user_record_state_color(const char *state) { return NULL; } -static void dump_self_modifiable(const char *heading, char **field, const char **value) { +static void dump_self_modifiable( + const char *heading, + char **field, + const char **value) { + assert(heading); /* Helper function for printing the various self_modifiable_* fields from the user record */ - if (strv_isempty((char**) value)) - /* Case 1: the array is explicitly set to be empty by the administrator */ - printf("%13s %sDisabled by Administrator%s\n", heading, ansi_highlight_red(), ansi_normal()); + if (!value) + /* Case 1: no value is set and no default either */ + printf("%13s %snone%s\n", heading, ansi_highlight(), ansi_normal()); + else if (strv_isempty((char**) value)) + /* Case 2: the array is explicitly set to empty by the administrator */ + printf("%13s %sdisabled by administrator%s\n", heading, ansi_highlight_red(), ansi_normal()); else if (!field) - /* Case 2: we have values, but the field is NULL. This means that we're using the defaults. + /* Case 3: we have values, but the field is NULL. This means that we're using the defaults. * We list them anyways, because they're security-sensitive to the administrator */ STRV_FOREACH(i, value) printf("%13s %s%s%s\n", i == value ? heading : "", ansi_grey(), *i, ansi_normal()); else - /* Case 3: we have a list provided by the administrator */ + /* Case 4: we have a list provided by the administrator */ STRV_FOREACH(i, value) printf("%13s %s\n", i == value ? heading : "", *i); }