From 5ec87d577f92effe27a62e965e02a6f9a40f81cc Mon Sep 17 00:00:00 2001 From: Adrian Vovk Date: Thu, 1 Feb 2024 11:43:48 -0500 Subject: [PATCH] homework: Accept volume key from keyring This bypasses authentication (i.e. user_record_authenticate) if the volume key was loaded from the keyring and no secret section is provided. This also changes Update() and Resize() to always try and load the volume key from the keyring. This makes the secret section optional for these methods while still letting them function (as long as the home area is active) --- man/org.freedesktop.home1.xml | 19 ++++++----- src/home/homed-home-bus.c | 4 +-- src/home/homed-home.c | 3 +- src/home/homed-home.h | 2 +- src/home/homed-manager.c | 2 +- src/home/homework.c | 59 ++++++++++++++++++++--------------- test/units/testsuite-46.sh | 43 ++++++++++++++++++++----- 7 files changed, 83 insertions(+), 49 deletions(-) diff --git a/man/org.freedesktop.home1.xml b/man/org.freedesktop.home1.xml index 6fe3bb3ce08..726d9d98325 100644 --- a/man/org.freedesktop.home1.xml +++ b/man/org.freedesktop.home1.xml @@ -320,10 +320,10 @@ node /org/freedesktop/home1 { interface. UpdateHome() updates a locally registered user record. Takes a fully - specified JSON user record as argument (including the secret section). A user with a - matching name and realm must be registered locally already, and the last change timestamp of the newly - supplied record must be newer than the previously existing user record. Note this operation updates the - user record only, it does not propagate passwords/authentication tokens from the user record to the + specified JSON user record as argument (possibly including the secret section). A user + with a matching name and realm must be registered locally already, and the last change timestamp of the + newly supplied record must be newer than the previously existing user record. Note this operation updates + the user record only, it does not propagate passwords/authentication tokens from the user record to the storage back-end, or resizes the storage back-end. Typically a home directory is first updated, and then the password of the underlying storage updated using ChangePasswordHome() as well as the storage resized using ResizeHome(). This method is equivalent to @@ -338,13 +338,12 @@ node /org/freedesktop/home1 { on the org.freedesktop.home1.Home interface. ResizeHome() resizes the storage associated with a user record. Takes a user - name, a disk size in bytes and a user record consisting only of the secret section - as argument. If the size is specified as UINT64_MAX the storage is resized to the - size already specified in the user record. Typically, if the user record is updated using + name, a disk size in bytes, and optionally a user record consisting only of the secret + section as arguments. If the size is specified as UINT64_MAX the storage is resized to + the size already specified in the user record. Typically, if the user record is updated using UpdateHome() above this is used to propagate the size configured there-in down to - the underlying storage back-end. This method is equivalent to - Resize() on the org.freedesktop.home1.Home - interface. + the underlying storage back-end. This method is equivalent to Resize() on the + org.freedesktop.home1.Home interface. ChangePasswordHome() changes the passwords/authentication tokens of a home directory. Takes a user name, and two JSON user record objects, each consisting only of the diff --git a/src/home/homed-home-bus.c b/src/home/homed-home-bus.c index 624cbdb3d33..dd3603efa7f 100644 --- a/src/home/homed-home-bus.c +++ b/src/home/homed-home-bus.c @@ -473,7 +473,7 @@ int bus_home_method_update( assert(message); - r = bus_message_read_home_record(message, USER_RECORD_REQUIRE_REGULAR|USER_RECORD_REQUIRE_SECRET|USER_RECORD_ALLOW_PRIVILEGED|USER_RECORD_ALLOW_PER_MACHINE|USER_RECORD_ALLOW_SIGNATURE|USER_RECORD_PERMISSIVE, &hr, error); + r = bus_message_read_home_record(message, USER_RECORD_REQUIRE_REGULAR|USER_RECORD_ALLOW_SECRET|USER_RECORD_ALLOW_PRIVILEGED|USER_RECORD_ALLOW_PER_MACHINE|USER_RECORD_ALLOW_SIGNATURE|USER_RECORD_PERMISSIVE, &hr, error); if (r < 0) return r; @@ -521,7 +521,7 @@ int bus_home_method_resize( if (r == 0) return 1; /* Will call us back */ - r = home_resize(h, sz, secret, /* automatic= */ false, error); + r = home_resize(h, sz, secret, error); if (r < 0) return r; diff --git a/src/home/homed-home.c b/src/home/homed-home.c index 09e86f37f36..a2892d00a37 100644 --- a/src/home/homed-home.c +++ b/src/home/homed-home.c @@ -1810,7 +1810,6 @@ int home_update(Home *h, UserRecord *hr, Hashmap *blobs, uint64_t flags, sd_bus_ int home_resize(Home *h, uint64_t disk_size, UserRecord *secret, - bool automatic, sd_bus_error *error) { _cleanup_(user_record_unrefp) UserRecord *c = NULL; @@ -1886,7 +1885,7 @@ int home_resize(Home *h, c = TAKE_PTR(signed_c); } - r = home_update_internal(h, automatic ? "resize-auto" : "resize", c, secret, NULL, 0, error); + r = home_update_internal(h, "resize", c, secret, NULL, 0, error); if (r < 0) return r; diff --git a/src/home/homed-home.h b/src/home/homed-home.h index 7f42461a45d..c0f1b83deba 100644 --- a/src/home/homed-home.h +++ b/src/home/homed-home.h @@ -192,7 +192,7 @@ int home_deactivate(Home *h, bool force, sd_bus_error *error); int home_create(Home *h, UserRecord *secret, Hashmap *blobs, uint64_t flags, sd_bus_error *error); int home_remove(Home *h, sd_bus_error *error); int home_update(Home *h, UserRecord *new_record, Hashmap *blobs, uint64_t flags, sd_bus_error *error); -int home_resize(Home *h, uint64_t disk_size, UserRecord *secret, bool automatic, sd_bus_error *error); +int home_resize(Home *h, uint64_t disk_size, UserRecord *secret, sd_bus_error *error); int home_passwd(Home *h, UserRecord *new_secret, UserRecord *old_secret, sd_bus_error *error); int home_unregister(Home *h, sd_bus_error *error); int home_lock(Home *h, sd_bus_error *error); diff --git a/src/home/homed-manager.c b/src/home/homed-manager.c index d4eaf1821b4..5f345b3d407 100644 --- a/src/home/homed-manager.c +++ b/src/home/homed-manager.c @@ -2025,7 +2025,7 @@ static int manager_rebalance_apply(Manager *m) { h->rebalance_pending = false; - r = home_resize(h, h->rebalance_goal, /* secret= */ NULL, /* automatic= */ true, &error); + r = home_resize(h, h->rebalance_goal, /* secret= */ NULL, &error); if (r < 0) log_warning_errno(r, "Failed to resize home '%s' for rebalancing, ignoring: %s", h->user_name, bus_error_message(&error, r)); diff --git a/src/home/homework.c b/src/home/homework.c index e46acf7ed57..d97e8fd9f09 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -66,9 +66,25 @@ int user_record_authenticate( * times over the course of an operation (think: on login we authenticate the host user record, the * record embedded in the LUKS record and the one embedded in $HOME). Hence we keep a list of * passwords we already decrypted, so that we don't have to do the (slow and potentially interactive) - * PKCS#11/FIDO2 dance for the relevant token again and again. */ + * PKCS#11/FIDO2 dance for the relevant token again and again. + * + * The 'cache' parameter might also contain the LUKS volume key, loaded from the kernel keyring. + * In this case, authentication becomes optional - if a secret section is provided it will be + * verified, but if missing then authentication is skipped entirely. Thus, callers should + * consider carefuly whether it is safe to load the volume key into 'cache' before doing so. + * Note that most of the time this is safe, because the home area must be active for the key + * to exist in the keyring, and the user would have had to authenticate when activating their + * home area; however, for some methods (i.e. ChangePassword, Authenticate) it makes more sense + * to force re-authentication. */ - /* First, let's see if the supplied plain-text passwords work? */ + /* First, let's see if we already have a volume key from the keyring */ + if (cache && cache->volume_key && + json_variant_is_blank_object(json_variant_by_key(secret->json, "secret"))) { + log_info("LUKS volume key from keyring unlocks user record."); + return 1; + } + + /* Next, let's see if the supplied plain-text passwords work? */ r = user_record_test_password(h, secret); if (r == -ENOKEY) need_password = true; @@ -101,7 +117,7 @@ int user_record_authenticate( else log_info("None of the supplied plaintext passwords unlock the user record's hashed recovery keys."); - /* Second, test cached PKCS#11 passwords */ + /* Next, test cached PKCS#11 passwords */ for (size_t n = 0; n < h->n_pkcs11_encrypted_key; n++) STRV_FOREACH(pp, cache->pkcs11_passwords) { r = test_password_one(h->pkcs11_encrypted_key[n].hashed_password, *pp); @@ -113,7 +129,7 @@ int user_record_authenticate( } } - /* Third, test cached FIDO2 passwords */ + /* Next, test cached FIDO2 passwords */ for (size_t n = 0; n < h->n_fido2_hmac_salt; n++) /* See if any of the previously calculated passwords work */ STRV_FOREACH(pp, cache->fido2_passwords) { @@ -126,7 +142,7 @@ int user_record_authenticate( } } - /* Fourth, let's see if any of the PKCS#11 security tokens are plugged in and help us */ + /* Next, let's see if any of the PKCS#11 security tokens are plugged in and help us */ for (size_t n = 0; n < h->n_pkcs11_encrypted_key; n++) { #if HAVE_P11KIT _cleanup_(pkcs11_callback_data_release) struct pkcs11_callback_data data = { @@ -182,7 +198,7 @@ int user_record_authenticate( #endif } - /* Fifth, let's see if any of the FIDO2 security tokens are plugged in and help us */ + /* Next, let's see if any of the FIDO2 security tokens are plugged in and help us */ for (size_t n = 0; n < h->n_fido2_hmac_salt; n++) { #if HAVE_LIBFIDO2 _cleanup_(erase_and_freep) char *decrypted_password = NULL; @@ -1599,6 +1615,8 @@ static int home_update(UserRecord *h, Hashmap *blobs, UserRecord **ret) { assert(h); assert(ret); + password_cache_load_keyring(h, &cache); + r = user_record_authenticate(h, h, &cache, /* strict_verify= */ true); if (r < 0) return r; @@ -1654,7 +1672,7 @@ static int home_update(UserRecord *h, Hashmap *blobs, UserRecord **ret) { return 0; } -static int home_resize(UserRecord *h, bool automatic, UserRecord **ret) { +static int home_resize(UserRecord *h, UserRecord **ret) { _cleanup_(home_setup_done) HomeSetup setup = HOME_SETUP_INIT; _cleanup_(password_cache_free) PasswordCache cache = {}; HomeSetupFlags flags = 0; @@ -1666,26 +1684,17 @@ static int home_resize(UserRecord *h, bool automatic, UserRecord **ret) { if (h->disk_size == UINT64_MAX) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No target size specified, refusing."); - if (automatic) - /* In automatic mode don't want to ask the user for the password, hence load it from the kernel keyring */ - password_cache_load_keyring(h, &cache); - else { - /* In manual mode let's ensure the user is fully authenticated */ - r = user_record_authenticate(h, h, &cache, /* strict_verify= */ true); - if (r < 0) - return r; - assert(r > 0); /* Insist that a password was verified */ - } + password_cache_load_keyring(h, &cache); + + r = user_record_authenticate(h, h, &cache, /* strict_verify= */ true); + if (r < 0) + return r; + assert(r > 0); /* Insist that a password was verified */ r = home_validate_update(h, &setup, &flags); if (r < 0) return r; - /* In automatic mode let's skip syncing identities, because we can't validate them, since we can't - * ask the user for reauthentication */ - if (automatic) - flags |= HOME_SETUP_RESIZE_DONT_SYNC_IDENTITIES; - switch (user_record_storage(h)) { case USER_LUKS: @@ -2053,10 +2062,8 @@ static int run(int argc, char *argv[]) { r = home_remove(home); else if (streq(argv[1], "update")) r = home_update(home, blobs, &new_home); - else if (streq(argv[1], "resize")) /* Resize on user request */ - r = home_resize(home, false, &new_home); - else if (streq(argv[1], "resize-auto")) /* Automatic resize */ - r = home_resize(home, true, &new_home); + else if (streq(argv[1], "resize")) + r = home_resize(home, &new_home); else if (streq(argv[1], "passwd")) r = home_passwd(home, &new_home); else if (streq(argv[1], "inspect")) diff --git a/test/units/testsuite-46.sh b/test/units/testsuite-46.sh index 4bf9922ef3a..5aef06bf228 100755 --- a/test/units/testsuite-46.sh +++ b/test/units/testsuite-46.sh @@ -89,6 +89,37 @@ inspect test-user homectl deactivate test-user inspect test-user +# Do some keyring tests, but only on real kernels, since keyring access inside of containers will fail +# (See: https://github.com/systemd/systemd/issues/17606) +if ! systemd-detect-virt -cq ; then + PASSWORD=xEhErW0ndafV4s homectl activate test-user + inspect test-user + + # Key should now be in the keyring + homectl update test-user --real-name "Keyring Test" + inspect test-user + + # These commands shouldn't use the keyring + (! timeout 5s homectl authenticate test-user ) + (! NEWPASSWORD="foobar" timeout 5s homectl passwd test-user ) + + homectl lock test-user + inspect test-user + + # Key should be gone from keyring + (! timeout 5s homectl update test-user --real-name "Keyring Test 2" ) + + PASSWORD=xEhErW0ndafV4s homectl unlock test-user + inspect test-user + + # Key should have been re-instantiated into the keyring + homectl update test-user --real-name "Keyring Test 3" + inspect test-user + + homectl deactivate test-user + inspect test-user +fi + # Do some resize tests, but only if we run on real kernels, as quota inside of containers will fail if ! systemd-detect-virt -cq ; then # grow while inactive @@ -150,6 +181,11 @@ if ! systemd-detect-virt -cq ; then homectl rebalance inspect test-user inspect test-user2 + + wait_for_state test-user2 active + homectl deactivate test-user2 + wait_for_state test-user2 inactive + homectl remove test-user2 fi PASSWORD=xEhErW0ndafV4s homectl with test-user -- test ! -f /home/test-user/xyz @@ -161,13 +197,6 @@ PASSWORD=xEhErW0ndafV4s homectl with test-user -- test -f /home/test-user/xyz wait_for_state test-user inactive homectl remove test-user -if ! systemd-detect-virt -cq ; then - wait_for_state test-user2 active - homectl deactivate test-user2 - wait_for_state test-user2 inactive - homectl remove test-user2 -fi - # blob directory tests # See docs/USER_RECORD_BLOB_DIRS.md checkblob() {