From: Adrian Vovk Date: Fri, 2 Feb 2024 03:53:09 +0000 (-0500) Subject: homed: Allow user to change parts of their record X-Git-Tag: v257-rc1~62^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a192250eda1e5cc1f8fc799cf9b85d37e7fa0519;p=thirdparty%2Fsystemd.git homed: Allow user to change parts of their record This allows an unprivileged user that is active at the console to change the fields that are in the selfModifiable allowlists (introduced in a previous commit) without authenticating as a system administrator. Administrators can disable this behavior per-user by setting the relevant selfModifiable allowlists, or system-wide by changing the policy of the org.freedesktop.home1.update-home-by-owner Polkit action. --- diff --git a/src/home/homed-home-bus.c b/src/home/homed-home-bus.c index 290e05ac6b2..80e2773447f 100644 --- a/src/home/homed-home-bus.c +++ b/src/home/homed-home-bus.c @@ -55,7 +55,7 @@ static int property_get_state( return sd_bus_message_append(reply, "s", home_state_to_string(home_get_state(h))); } -int bus_home_client_is_trusted(Home *h, sd_bus_message *message) { +static int bus_home_client_is_trusted(Home *h, sd_bus_message *message, bool strict) { _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *creds = NULL; uid_t euid; int r; @@ -73,7 +73,7 @@ int bus_home_client_is_trusted(Home *h, sd_bus_message *message) { if (r < 0) return r; - return euid == 0 || h->uid == euid; + return (!strict && euid == 0) || h->uid == euid; } static int home_verify_polkit_async( @@ -117,7 +117,7 @@ int bus_home_get_record_json( assert(h); assert(ret); - trusted = bus_home_client_is_trusted(h, message); + trusted = bus_home_client_is_trusted(h, message, /* strict= */ false); if (trusted < 0) { log_warning_errno(trusted, "Failed to determine whether client is trusted, assuming untrusted."); trusted = false; @@ -423,7 +423,7 @@ int bus_home_update_record( Hashmap *blobs, uint64_t flags, sd_bus_error *error) { - int r; + int r, relax_access; assert(h); assert(message); @@ -436,10 +436,32 @@ int bus_home_update_record( if ((flags & ~SD_HOMED_UPDATE_FLAGS_ALL) != 0) return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid flags provided."); + if (blobs) { + const char *failed = NULL; + r = user_record_ensure_blob_manifest(hr, blobs, &failed); + if (r == -EINVAL) + return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Provided blob files do not correspond to blob manifest."); + if (r < 0) + return sd_bus_error_set_errnof(error, r, "Failed to generate hash for blob %s: %m", strnull(failed)); + } + + relax_access = user_record_self_changes_allowed(h->record, hr); + if (relax_access < 0) { + log_warning_errno(relax_access, "Failed to determine if changes to user record are permitted, assuming not: %m"); + relax_access = false; + } else if (relax_access) { + relax_access = bus_home_client_is_trusted(h, message, /* strict= */ true); + if (relax_access < 0) { + log_warning_errno(relax_access, "Failed to determine whether client is trusted, assuming not: %m"); + relax_access = false; + } + } + r = home_verify_polkit_async( h, message, - "org.freedesktop.home1.update-home", + relax_access ? "org.freedesktop.home1.update-home-by-owner" + : "org.freedesktop.home1.update-home", UID_INVALID, error); if (r < 0) @@ -561,7 +583,7 @@ int bus_home_method_change_password( h, message, "org.freedesktop.home1.passwd-home", - h->uid, + h->uid, /* Always let a user change their own password. Safe b/c homework will always re-check password */ error); if (r < 0) return r; diff --git a/src/home/homed-home-bus.h b/src/home/homed-home-bus.h index 1644bc8066a..8d4ddf909f3 100644 --- a/src/home/homed-home-bus.h +++ b/src/home/homed-home-bus.h @@ -6,7 +6,6 @@ #include "bus-object.h" #include "homed-home.h" -int bus_home_client_is_trusted(Home *h, sd_bus_message *message); int bus_home_get_record_json(Home *h, sd_bus_message *message, char **ret, bool *ret_incomplete); int bus_home_method_activate(sd_bus_message *message, void *userdata, sd_bus_error *error); diff --git a/src/home/homed-home.c b/src/home/homed-home.c index 664397e55e0..32691e4f815 100644 --- a/src/home/homed-home.c +++ b/src/home/homed-home.c @@ -1727,15 +1727,6 @@ static int home_update_internal( secret = saved_secret; } - if (blobs) { - const char *failed = NULL; - r = user_record_ensure_blob_manifest(hr, blobs, &failed); - if (r == -EINVAL) - return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Provided blob files do not correspond to blob manifest."); - if (r < 0) - return sd_bus_error_set_errnof(error, r, "Failed to generate hash for blob %s: %m", strnull(failed)); - } - r = manager_verify_user_record(h->manager, hr); switch (r) { diff --git a/src/home/org.freedesktop.home1.policy b/src/home/org.freedesktop.home1.policy index 3b19ed3ed5f..d3317772acb 100644 --- a/src/home/org.freedesktop.home1.policy +++ b/src/home/org.freedesktop.home1.policy @@ -49,6 +49,16 @@ + + Update your home area + Authentication is required to update your home area. + + auth_admin_keep + auth_admin_keep + yes + + + Resize a home area Authentication is required to resize a user's home area. diff --git a/src/shared/user-record.c b/src/shared/user-record.c index 35512cbf51a..47fd5cc3118 100644 --- a/src/shared/user-record.c +++ b/src/shared/user-record.c @@ -1475,7 +1475,6 @@ int user_group_record_mangle( assert(v); assert(ret_variant); - assert(ret_mask); /* Note that this function is shared with the group record parser, hence we try to be generic in our * log message wording here, to cover both cases. */ @@ -1563,7 +1562,8 @@ int user_group_record_mangle( else *ret_variant = sd_json_variant_ref(v); - *ret_mask = m; + if (ret_mask) + *ret_mask = m; return 0; } @@ -2238,6 +2238,172 @@ const char** user_record_self_modifiable_privileged(UserRecord *h) { return (const char**) h->self_modifiable_privileged ?: (const char**) default_fields; } +static int remove_self_modifiable_json_fields_common(UserRecord *current, sd_json_variant **target) { + _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL, *blobs = NULL; + char **allowed; + int r; + + assert(current); + assert(target); + + if (!sd_json_variant_is_object(*target)) + return -EINVAL; + + v = sd_json_variant_ref(*target); + + /* Handle basic fields */ + allowed = (char**) user_record_self_modifiable_fields(current); + r = sd_json_variant_filter(&v, allowed); + if (r < 0) + return r; + + /* Handle blobs */ + blobs = sd_json_variant_ref(sd_json_variant_by_key(v, "blobManifest")); + if (blobs) { + /* The blobManifest contains the sha256 hashes of the blobs, + * which are enforced by the service managing the user. So, by + * comparing the blob manifests like this, we're actually comparing + * the contents of the blob directories & files */ + + allowed = (char**) user_record_self_modifiable_blobs(current); + r = sd_json_variant_filter(&blobs, allowed); + if (r < 0) + return r; + + if (sd_json_variant_is_blank_object(blobs)) + r = sd_json_variant_filter(&v, STRV_MAKE("blobManifest")); + else + r = sd_json_variant_set_field(&v, "blobManifest", blobs); + if (r < 0) + return r; + } + + JSON_VARIANT_REPLACE(*target, TAKE_PTR(v)); + return 0; +} + +static int remove_self_modifiable_json_fields(UserRecord *current, UserRecord *h, sd_json_variant **ret) { + _cleanup_(sd_json_variant_unrefp) sd_json_variant *v = NULL, *privileged = NULL; + sd_json_variant *per_machine; + char **allowed; + int r; + + assert(current); + assert(h); + assert(ret); + + r = user_group_record_mangle(h->json, USER_RECORD_EXTRACT_SIGNABLE|USER_RECORD_PERMISSIVE, &v, NULL); + if (r < 0) + return r; + + /* Handle the regular section */ + r = remove_self_modifiable_json_fields_common(current, &v); + if (r < 0) + return r; + + /* Handle the perMachine section */ + per_machine = sd_json_variant_by_key(v, "perMachine"); + if (per_machine) { + _cleanup_(sd_json_variant_unrefp) sd_json_variant *new_per_machine = NULL; + sd_json_variant *e; + + if (!sd_json_variant_is_array(per_machine)) + return -EINVAL; + + JSON_VARIANT_ARRAY_FOREACH(e, per_machine) { + _cleanup_(sd_json_variant_unrefp) sd_json_variant *z = NULL; + + if (!sd_json_variant_is_object(e)) + return -EINVAL; + + r = per_machine_match(e, 0); + if (r < 0) + return r; + if (r == 0) { + /* It's only permissible to change anything inside of matching perMachine sections */ + r = sd_json_variant_append_array(&new_per_machine, e); + if (r < 0) + return r; + continue; + } + + z = sd_json_variant_ref(e); + + r = remove_self_modifiable_json_fields_common(current, &z); + if (r < 0) + return r; + + if (!sd_json_variant_is_blank_object(z)) { + r = sd_json_variant_append_array(&new_per_machine, z); + if (r < 0) + return r; + } + } + + if (sd_json_variant_is_blank_array(new_per_machine)) + r = sd_json_variant_filter(&v, STRV_MAKE("perMachine")); + else + r = sd_json_variant_set_field(&v, "perMachine", new_per_machine); + if (r < 0) + return r; + } + + /* Handle the privileged section */ + privileged = sd_json_variant_ref(sd_json_variant_by_key(v, "privileged")); + if (privileged) { + allowed = (char**) user_record_self_modifiable_privileged(current); + r = sd_json_variant_filter(&privileged, allowed); + if (r < 0) + return r; + + if (sd_json_variant_is_blank_object(privileged)) + r = sd_json_variant_filter(&v, STRV_MAKE("privileged")); + else + r = sd_json_variant_set_field(&v, "privileged", privileged); + if (r < 0) + return r; + } + + JSON_VARIANT_REPLACE(*ret, TAKE_PTR(v)); + return 0; +} + +int user_record_self_changes_allowed(UserRecord *current, UserRecord *incoming) { + _cleanup_(sd_json_variant_unrefp) sd_json_variant *vc = NULL, *vi = NULL; + int r; + + assert(current); + assert(incoming); + + /* We remove the fields that the user is allowed to change and then + * compare the resulting JSON records. If they are not equal, that + * means a disallowed field has been changed and thus we should + * require administrator permission to apply the changes. */ + + r = remove_self_modifiable_json_fields(current, current, &vc); + if (r < 0) + return r; + + /* Note that we use `current` as the source of the allowlist, and not + * `incoming`. This prevents the user from adding fields. Consider a + * scenario that would've been possible if we had messed up this check: + * + * 1) A user starts out with no group memberships and no custom allowlist. + * Thus, this user is not an administrator, and the `memberOf` and + * `selfModifiableFields` fields are unset in their record. + * 2) This user crafts a request to add the following to their record: + * { "memberOf": ["wheel"], "selfModifiableFields": ["memberOf", "selfModifiableFields"] } + * 3) We remove the `mebmerOf` and `selfModifiabileFields` fields from `incoming` + * 4) `current` and `incoming` compare as equal, so we let the change happen + * 5) the user has granted themselves administrator privileges + */ + r = remove_self_modifiable_json_fields(current, incoming, &vi); + if (r < 0) + return r; + + return sd_json_variant_equal(vc, vi); +} + uint64_t user_record_ratelimit_next_try(UserRecord *h) { assert(h); diff --git a/src/shared/user-record.h b/src/shared/user-record.h index acbb8eca735..b539b3f55e3 100644 --- a/src/shared/user-record.h +++ b/src/shared/user-record.h @@ -438,6 +438,7 @@ int user_record_languages(UserRecord *h, char ***ret); const char **user_record_self_modifiable_fields(UserRecord *h); const char **user_record_self_modifiable_blobs(UserRecord *h); const char **user_record_self_modifiable_privileged(UserRecord *h); +int user_record_self_changes_allowed(UserRecord *current, UserRecord *new); int user_record_build_image_path(UserStorage storage, const char *user_name_and_realm, char **ret);