homed: verify privileged worker changes

Workers may return a newer embedded identity while processing
an owner update. Accept self-modifiable changes, but require
privileged changes to carry a trusted signature.

Follow-up for 70a5db5822c8056b53d9a4a9273ad12cb5f87a92

diff --git a/src/home/homed-home.c b/src/home/homed-home.c
index 49c524623cc30117b3bcb97134940a7887a3b115..c50ee7084806c1a2610f40facd4ee77b8bc737b0 100644 (file)
--- a/src/home/homed-home.c
+++ b/src/home/homed-home.c
@@ -999,6 +999,60 @@ static void home_change_finish(Home *h, int ret, UserRecord *hr) {
         }
 
         if (hr) {
+                _cleanup_(user_record_unrefp) UserRecord *signed_hr = NULL;
+                bool signed_locally = false;
+                int allowed;
+
+                allowed = user_record_self_changes_allowed(h->record, hr);
+                if (allowed < 0) {
+                        r = log_error_errno(allowed, "Failed to determine whether worker returned permitted changes: %m");
+                        goto finish;
+                }
+
+                if (!allowed) {
+                        r = home_verify_user_record(h, hr, &signed_locally, &error);
+                        if (r < 0)
+                                goto finish;
+                } else {
+                        int is_signed = manager_verify_user_record(h->manager, hr);
+
+                        switch (is_signed) {
+
+                        case USER_RECORD_SIGNED_EXCLUSIVE:
+                                signed_locally = true;
+                                break;
+
+                        case USER_RECORD_SIGNED:
+                        case USER_RECORD_FOREIGN:
+                                break;
+
+                        case USER_RECORD_UNSIGNED:
+                                if (h->signed_locally <= 0) {
+                                        r = sd_bus_error_setf(&error, BUS_ERROR_HOME_RECORD_SIGNED,
+                                                              "Home %s is signed and cannot be modified locally.", h->user_name);
+                                        goto finish;
+                                }
+
+                                r = manager_sign_user_record(h->manager, hr, &signed_hr, &error);
+                                if (r < 0)
+                                        goto finish;
+
+                                hr = signed_hr;
+                                signed_locally = true;
+                                break;
+
+                        case -ENOKEY:
+                                r = home_verify_user_record(h, hr, &signed_locally, &error);
+                                assert(r < 0);
+                                goto finish;
+
+                        default:
+                                assert(is_signed < 0);
+                                r = log_error_errno(is_signed, "Failed to verify worker returned record: %m");
+                                goto finish;
+                        }
+                }
+
                 if (!FLAGS_SET(flags, SD_HOMED_UPDATE_OFFLINE)) {
                         r = user_record_good_authentication(h->record);
                         if (r < 0)
@@ -1006,8 +1060,11 @@ static void home_change_finish(Home *h, int ret, UserRecord *hr) {
                 }
 
                 r = home_set_record(h, hr);
-                if (r >= 0)
+                if (r >= 0) {
+                        h->signed_locally = signed_locally;
+
                         r = home_save_record(h);
+                }
                 if (r < 0) {
                         if (FLAGS_SET(flags, SD_HOMED_UPDATE_OFFLINE)) {
                                 log_error_errno(r, "Failed to update home record and write it to disk: %m");

diff --git a/test/units/TEST-46-HOMED.sh b/test/units/TEST-46-HOMED.sh
index 08d96e57fe9596880067310aca6620cd99ceb16b..aaa40455d18d69525f963e682df64ab376f04228 100755 (executable)
--- a/test/units/TEST-46-HOMED.sh
+++ b/test/units/TEST-46-HOMED.sh
@@ -1110,7 +1110,7 @@ testcase_deactivate_busy() {
 }
 
 testcase_identity_groups() {
-    NEWPASSWORD=foobar homectl create idgrouptest --enforce-password-policy=no
+    NEWPASSWORD=foobar homectl create idgrouptest --storage=directory --shell=/bin/bash --enforce-password-policy=no --rebalance-weight=off
     PASSWORD=foobar homectl activate idgrouptest
 
     machinectl shell idgrouptest@ /usr/bin/bash -euxo pipefail -c "jq '.memberOf = ((.memberOf // []) + [\"systemd-journal\"] | unique) | .lastChangeUSec = ((.lastChangeUSec // 0) + 3600000000)' /home/idgrouptest/.identity > /home/idgrouptest/.identity.new && mv -f /home/idgrouptest/.identity.new /home/idgrouptest/.identity"
@@ -1120,11 +1120,61 @@ testcase_identity_groups() {
 
     local groups
     groups="$(machinectl shell idgrouptest@ /usr/bin/groups)"
-    (! grep -q systemd-journal <<<"$groups")
+    (! grep systemd-journal <<<"$groups" >/dev/null)
 
     homectl deactivate idgrouptest ||:
     wait_for_state idgrouptest inactive
     homectl remove idgrouptest
+
+    # Install a PK rule that allows 'idgrouptest2' user to update homed even
+    # though they are not on an fg console, just for testing
+    mkdir -p /etc/polkit-1/rules.d
+    cat >/etc/polkit-1/rules.d/updatehome.rules <<'EOF'
+polkit.addRule(function(action, subject) {
+    if (action.id == "org.freedesktop.home1.update-home-by-owner" &&
+        subject.user == "idgrouptest2") {
+        return polkit.Result.YES;
+    }
+});
+EOF
+    trap 'rm -f /etc/polkit-1/rules.d/updatehome.rules' RETURN ERR EXIT
+    systemctl try-reload-or-restart polkit.service
+
+    NEWPASSWORD=foobar homectl create idgrouptest2 --storage=directory --shell=/bin/bash --enforce-password-policy=no --rebalance-weight=off
+    PASSWORD=foobar homectl activate idgrouptest2
+
+    cat >/tmp/idgrouptest-add-group.sh <<'EOF'
+#!/bin/bash
+set -exuo pipefail
+
+loginctl show-session "${XDG_SESSION_ID:?}" -p Active --value | grep '^yes$' >/dev/null
+RECORD="$(busctl -j call org.freedesktop.home1 /org/freedesktop/home1 org.freedesktop.home1.Manager GetUserRecordByName s "$USER" | jq -r '.data[0]')"
+TS="$(printf '%s' "$RECORD" | jq '.lastChangeUSec')"
+NEW_TS=$((TS + 172800000000))
+jq --arg g systemd-journal --argjson ts "$NEW_TS" \
+   '.memberOf = ((.memberOf // []) + [$g]) | .lastChangeUSec = $ts' \
+   ~/.identity > ~/.identity.new
+mv -f ~/.identity.new ~/.identity
+# Ensure the identity update is persisted before UpdateHomeEx reads it.
+sync
+
+UPDATE_TS=$((TS + 1))
+UPDATE_RECORD="$(printf '%s' "$RECORD" | jq -c --argjson ts "$UPDATE_TS" --arg p foobar 'del(.binding, .status, .signature) | .lastChangeUSec = $ts | .secret = {password: [$p]}')"
+(! busctl call org.freedesktop.home1 /org/freedesktop/home1 org.freedesktop.home1.Manager UpdateHomeEx "sa{sh}t" "$UPDATE_RECORD" 0 0 )
+EOF
+    chmod +x /tmp/idgrouptest-add-group.sh
+    machinectl shell idgrouptest2@ /tmp/idgrouptest-add-group.sh
+    rm -f /tmp/idgrouptest-add-group.sh
+
+    PASSWORD=foobar homectl authenticate idgrouptest2
+
+    local groups
+    groups="$(machinectl shell idgrouptest2@ /usr/bin/groups)"
+    (! grep systemd-journal <<<"$groups" >/dev/null)
+
+    homectl deactivate idgrouptest2 ||:
+    wait_for_state idgrouptest2 inactive
+    homectl remove idgrouptest2
 }
 
 run_testcases