From d0eff7a12d44ac98371431d22c18ec4c50a283ba Mon Sep 17 00:00:00 2001 From: Adrian Vovk Date: Wed, 31 Jan 2024 23:49:24 -0500 Subject: [PATCH 1/5] homework: Always upload volume key to keyring This commit makes homework always upload the LUKS volume key into the kernel keyring. This is different from previous behavior in three notable ways: - Previously, we'd only upload if auto-resize was on. In preparation for upcoming changes, now we always upload - Previously, we'd upload the user's actual password (or a password obtained from a FIDO key or similar). Now, we upload the LUKS volume key itself, to remove a layer of unnecessary indirection. - Previously, Lock() wouldn't remove the key from the kernel keyring. This, of course, defeats the purpose of Lock(), so now it removes the key This commit also allows the LUKS volume to be unlocked using the volume key we obtained from the keyring. --- meson.build | 6 +- src/home/homework-luks.c | 210 +++++++++++++---------------- src/home/homework-password-cache.c | 40 +++--- src/home/homework-password-cache.h | 12 +- src/home/homework.c | 3 + src/shared/cryptsetup-util.c | 8 +- src/shared/cryptsetup-util.h | 4 +- 7 files changed, 134 insertions(+), 149 deletions(-) diff --git a/meson.build b/meson.build index 8d1cd8a9ed..2d36af0d3a 100644 --- a/meson.build +++ b/meson.build @@ -1251,7 +1251,8 @@ foreach ident : ['crypt_set_metadata_size', 'crypt_reencrypt_init_by_passphrase', 'crypt_reencrypt', 'crypt_set_data_offset', - 'crypt_set_keyring_to_link'] + 'crypt_set_keyring_to_link', + 'crypt_resume_by_volume_key'] have_ident = have and cc.has_function( ident, prefix : '#include ', @@ -1564,7 +1565,8 @@ conf.set10('ENABLE_IMPORTD', have) have = get_option('homed').require( conf.get('HAVE_OPENSSL') == 1 and conf.get('HAVE_LIBFDISK') == 1 and - conf.get('HAVE_LIBCRYPTSETUP') == 1, + conf.get('HAVE_LIBCRYPTSETUP') == 1 and + conf.get('HAVE_CRYPT_RESUME_BY_VOLUME_KEY') == 1, error_message : 'openssl, fdisk and libcryptsetup required').allowed() conf.set10('ENABLE_HOMED', have) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index 8dcb5c5d46..d70926fe33 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -256,43 +256,30 @@ static int run_fsck(const char *node, const char *fstype) { DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(key_serial_t, keyring_unlink, -1); -static int upload_to_keyring( - UserRecord *h, - const char *password, - key_serial_t *ret_key_serial) { +static int upload_to_keyring(UserRecord *h, const void *vk, size_t vks, key_serial_t *ret) { _cleanup_free_ char *name = NULL; key_serial_t serial; assert(h); - assert(password); + assert(vk); + assert(vks > 0); - /* If auto-shrink-on-logout is turned on, we need to keep the key we used to unlock the LUKS volume - * around, since we'll need it when automatically resizing (since we can't ask the user there - * again). We do this by uploading it into the kernel keyring, specifically the "session" one. This - * is done under the assumption systemd-homed gets its private per-session keyring (i.e. default - * service behaviour, given that KeyringMode=private is the default). It will survive between our - * systemd-homework invocations that way. - * - * If auto-shrink-on-logout is disabled we'll skip this step, to be frugal with sensitive data. */ - - if (user_record_auto_resize_mode(h) != AUTO_RESIZE_SHRINK_AND_GROW) { /* Won't need it */ - if (ret_key_serial) - *ret_key_serial = -1; - return 0; - } + /* We upload the LUKS volume key into the kernel session keyring, under the assumption that + * systemd-homed gets its own private session keyring (i.e. the default service behavior, given + * that KeyringMode=private is the default). That way, the key will survive between invocations + * of systemd-homework. */ name = strjoin("homework-user-", h->user_name); if (!name) return -ENOMEM; - serial = add_key("user", name, password, strlen(password), KEY_SPEC_SESSION_KEYRING); + serial = add_key("user", name, vk, vks, KEY_SPEC_SESSION_KEYRING); if (serial == -1) return -errno; - if (ret_key_serial) - *ret_key_serial = serial; - + if (ret) + *ret = serial; return 1; } @@ -301,13 +288,14 @@ static int luks_try_passwords( struct crypt_device *cd, char **passwords, void *volume_key, - size_t *volume_key_size, - key_serial_t *ret_key_serial) { + size_t *volume_key_size) { int r; assert(h); assert(cd); + assert(volume_key); + assert(volume_key_size); STRV_FOREACH(pp, passwords) { size_t vks = *volume_key_size; @@ -320,16 +308,6 @@ static int luks_try_passwords( *pp, strlen(*pp)); if (r >= 0) { - if (ret_key_serial) { - /* If ret_key_serial is non-NULL, let's try to upload the password that - * worked, and return its serial. */ - r = upload_to_keyring(h, *pp, ret_key_serial); - if (r < 0) { - log_debug_errno(r, "Failed to upload LUKS password to kernel keyring, ignoring: %m"); - *ret_key_serial = -1; - } - } - *volume_key_size = vks; return 0; } @@ -340,6 +318,66 @@ static int luks_try_passwords( return -ENOKEY; } +static int luks_get_volume_key( + UserRecord *h, + struct crypt_device *cd, + const PasswordCache *cache, + void *volume_key, + size_t *volume_key_size, + key_serial_t *ret_key_serial) { + + char **list; + size_t vks; + int r; + + assert(h); + assert(cd); + assert(volume_key); + assert(volume_key_size); + + if (cache && cache->volume_key) { + /* Shortcut: If volume key was loaded from the keyring then just use it */ + if (cache->volume_key_size > *volume_key_size) + return log_error_errno(SYNTHETIC_ERRNO(ENOBUFS), + "LUKS volume key from kernel keyring too big for buffer (need %zu bytes, have %zu)", + cache->volume_key_size, *volume_key_size); + memcpy(volume_key, cache->volume_key, cache->volume_key_size); + *volume_key_size = cache->volume_key_size; + if (ret_key_serial) + *ret_key_serial = -1; /* Key came from keyring. No need to re-upload it */ + return 0; + } + + vks = *volume_key_size; + + FOREACH_ARGUMENT(list, + cache ? cache->pkcs11_passwords : NULL, + cache ? cache->fido2_passwords : NULL, + h->password) { + + r = luks_try_passwords(h, cd, list, volume_key, &vks); + if (r == -ENOKEY) + continue; + if (r < 0) + return r; + + /* We got a volume key! */ + + if (ret_key_serial) { + r = upload_to_keyring(h, volume_key, vks, ret_key_serial); + if (r < 0) { + log_warning_errno(r, "Failed to upload LUKS volume key to kernel keyring, ignoring: %m"); + *ret_key_serial = -1; + } + } + + *volume_key_size = vks; + return 0; + } + + return -ENOKEY; +} + static int luks_setup( UserRecord *h, const char *node, @@ -348,7 +386,6 @@ static int luks_setup( const char *cipher, const char *cipher_mode, uint64_t volume_key_size, - char **passwords, const PasswordCache *cache, bool discard, struct crypt_device **ret, @@ -414,18 +451,7 @@ static int luks_setup( if (!vk) return log_oom(); - r = -ENOKEY; - char **list; - FOREACH_ARGUMENT(list, - cache ? cache->keyring_passswords : NULL, - cache ? cache->pkcs11_passwords : NULL, - cache ? cache->fido2_passwords : NULL, - passwords) { - - r = luks_try_passwords(h, cd, list, vk, &vks, ret_key_serial ? &key_serial : NULL); - if (r != -ENOKEY) - break; - } + r = luks_get_volume_key(h, cd, cache, vk, &vks, ret_key_serial ? &key_serial : NULL); if (r == -ENOKEY) return log_error_errno(r, "No valid password for LUKS superblock."); if (r < 0) @@ -556,18 +582,7 @@ static int luks_open( if (!vk) return log_oom(); - r = -ENOKEY; - char **list; - FOREACH_ARGUMENT(list, - cache ? cache->keyring_passswords : NULL, - cache ? cache->pkcs11_passwords : NULL, - cache ? cache->fido2_passwords : NULL, - h->password) { - - r = luks_try_passwords(h, setup->crypt_device, list, vk, &vks, NULL); - if (r != -ENOKEY) - break; - } + r = luks_get_volume_key(h, setup->crypt_device, cache, vk, &vks, NULL); if (r == -ENOKEY) return log_error_errno(r, "No valid password for LUKS superblock."); if (r < 0) @@ -1401,7 +1416,6 @@ int home_setup_luks( h->luks_cipher, h->luks_cipher_mode, h->luks_volume_key_size, - h->password, cache, user_record_luks_discard(h) || user_record_luks_offline_discard(h), &setup->crypt_device, @@ -3619,18 +3633,7 @@ int home_passwd_luks( if (!volume_key) return log_oom(); - r = -ENOKEY; - char **list; - FOREACH_ARGUMENT(list, - cache ? cache->keyring_passswords : NULL, - cache ? cache->pkcs11_passwords : NULL, - cache ? cache->fido2_passwords : NULL, - h->password) { - - r = luks_try_passwords(h, setup->crypt_device, list, volume_key, &volume_key_size, NULL); - if (r != -ENOKEY) - break; - } + r = luks_get_volume_key(h, setup->crypt_device, cache, volume_key, &volume_key_size, NULL); if (r == -ENOKEY) return log_error_errno(SYNTHETIC_ERRNO(ENOKEY), "Failed to unlock LUKS superblock with supplied passwords."); if (r < 0) @@ -3673,11 +3676,6 @@ int home_passwd_luks( return log_error_errno(r, "Failed to set up LUKS password: %m"); log_info("Updated LUKS key slot %zu.", i); - - /* If we changed the password, then make sure to update the copy in the keyring, so that - * auto-rebalance continues to work. We only do this if we operate on an active home dir. */ - if (i == 0 && FLAGS_SET(flags, HOME_SETUP_ALREADY_ACTIVATED)) - upload_to_keyring(h, effective_passwords[i], NULL); } return 1; @@ -3715,35 +3713,10 @@ int home_lock_luks(UserRecord *h, HomeSetup *setup) { return 0; } -static int luks_try_resume( - struct crypt_device *cd, - const char *dm_name, - char **password) { - - int r; - - assert(cd); - assert(dm_name); - - STRV_FOREACH(pp, password) { - r = sym_crypt_resume_by_passphrase( - cd, - dm_name, - CRYPT_ANY_SLOT, - *pp, - strlen(*pp)); - if (r >= 0) { - log_info("Resumed LUKS device %s.", dm_name); - return 0; - } - - log_debug_errno(r, "Password %zu didn't work for resuming device: %m", (size_t) (pp - password)); - } - - return -ENOKEY; -} - int home_unlock_luks(UserRecord *h, HomeSetup *setup, const PasswordCache *cache) { + _cleanup_(keyring_unlinkp) key_serial_t key_serial = -1; + _cleanup_(erase_and_freep) void *vk = NULL; + size_t vks; int r; assert(h); @@ -3756,22 +3729,27 @@ int home_unlock_luks(UserRecord *h, HomeSetup *setup, const PasswordCache *cache log_info("Discovered used LUKS device %s.", setup->dm_node); - r = -ENOKEY; - char **list; - FOREACH_ARGUMENT(list, - cache ? cache->pkcs11_passwords : NULL, - cache ? cache->fido2_passwords : NULL, - h->password) { + r = sym_crypt_get_volume_key_size(setup->crypt_device); + if (r <= 0) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to determine LUKS volume key size"); + vks = (size_t) r; - r = luks_try_resume(setup->crypt_device, setup->dm_name, list); - if (r != -ENOKEY) - break; - } + vk = malloc(vks); + if (!vk) + return log_oom(); + + r = luks_get_volume_key(h, setup->crypt_device, cache, vk, &vks, &key_serial); if (r == -ENOKEY) return log_error_errno(r, "No valid password for LUKS superblock."); + if (r < 0) + return log_error_errno(r, "Failed to unlock LUKS superblock: %m"); + + r = sym_crypt_resume_by_volume_key(setup->crypt_device, setup->dm_name, vk, vks); if (r < 0) return log_error_errno(r, "Failed to resume LUKS superblock: %m"); + TAKE_KEY_SERIAL(key_serial); /* Leave key in kernel keyring */ + log_info("LUKS device resumed."); return 0; } diff --git a/src/home/homework-password-cache.c b/src/home/homework-password-cache.c index 00a0f69bc9..b8202ef695 100644 --- a/src/home/homework-password-cache.c +++ b/src/home/homework-password-cache.c @@ -9,49 +9,41 @@ void password_cache_free(PasswordCache *cache) { if (!cache) return; + cache->volume_key = erase_and_free(cache->volume_key); cache->pkcs11_passwords = strv_free_erase(cache->pkcs11_passwords); cache->fido2_passwords = strv_free_erase(cache->fido2_passwords); - cache->keyring_passswords = strv_free_erase(cache->keyring_passswords); } void password_cache_load_keyring(UserRecord *h, PasswordCache *cache) { - _cleanup_(erase_and_freep) void *p = NULL; _cleanup_free_ char *name = NULL; - char **strv; + _cleanup_(erase_and_freep) void *vk = NULL; + size_t vks; key_serial_t serial; - size_t sz; int r; assert(h); assert(cache); - /* Loads the password we need to for automatic resizing from the kernel keyring */ - name = strjoin("homework-user-", h->user_name); if (!name) return (void) log_oom(); serial = request_key("user", name, NULL, 0); - if (serial == -1) - return (void) log_debug_errno(errno, "Failed to request key '%s', ignoring: %m", name); + if (serial == -1) { + if (errno == ENOKEY) { + log_info("Home volume key is not available in kernel keyring."); + return; + } + return (void) log_warning_errno(errno, "Failed to request key '%s', ignoring: %m", name); + } - r = keyring_read(serial, &p, &sz); + r = keyring_read(serial, &vk, &vks); if (r < 0) - return (void) log_debug_errno(r, "Failed to read keyring key '%s', ignoring: %m", name); + return (void) log_warning_errno(r, "Failed to read keyring key '%s', ignoring: %m", name); - if (memchr(p, 0, sz)) - return (void) log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Cached password contains embedded NUL byte, ignoring."); + log_info("Successfully acquired home volume key from kernel keyring."); - strv = new(char*, 2); - if (!strv) - return (void) log_oom(); - - strv[0] = TAKE_PTR(p); /* Note that keyring_read() will NUL terminate implicitly, hence we don't have - * to NUL terminate manually here: it's a valid string. */ - strv[1] = NULL; - - strv_free_erase(cache->keyring_passswords); - cache->keyring_passswords = strv; - - log_debug("Successfully acquired home key from kernel keyring."); + erase_and_free(cache->volume_key); + cache->volume_key = TAKE_PTR(vk); + cache->volume_key_size = vks; } diff --git a/src/home/homework-password-cache.h b/src/home/homework-password-cache.h index fdfbcfe4e0..e2d86eb939 100644 --- a/src/home/homework-password-cache.h +++ b/src/home/homework-password-cache.h @@ -5,8 +5,9 @@ #include "user-record.h" typedef struct PasswordCache { - /* Passwords acquired from the kernel keyring */ - char **keyring_passswords; + /* The volume key from the kernel keyring */ + void *volume_key; + size_t volume_key_size; /* Decoding passwords from security tokens is expensive and typically requires user interaction, * hence cache any we already figured out. */ @@ -20,9 +21,12 @@ static inline bool password_cache_contains(const PasswordCache *cache, const cha if (!cache) return false; + /* Used to decide whether or not to set a minimal PBKDF, under the assumption that if + * the cache contains a password then the password came from a hardware token of some kind + * and is thus naturally high-entropy. */ + return strv_contains(cache->pkcs11_passwords, p) || - strv_contains(cache->fido2_passwords, p) || - strv_contains(cache->keyring_passswords, p); + strv_contains(cache->fido2_passwords, p); } void password_cache_load_keyring(UserRecord *h, PasswordCache *cache); diff --git a/src/home/homework.c b/src/home/homework.c index 531443e757..e46acf7ed5 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -1883,6 +1883,9 @@ static int home_lock(UserRecord *h) { unit_freezer_done(&freezer); /* Don't thaw the user session. */ + /* Explicitly flush any per-user key from the keyring */ + (void) keyring_flush(h); + log_info("Everything completed."); return 1; } diff --git a/src/shared/cryptsetup-util.c b/src/shared/cryptsetup-util.c index 77ac85965f..cbbc85a5cc 100644 --- a/src/shared/cryptsetup-util.c +++ b/src/shared/cryptsetup-util.c @@ -33,7 +33,9 @@ DLSYM_FUNCTION(crypt_keyslot_destroy); DLSYM_FUNCTION(crypt_keyslot_max); DLSYM_FUNCTION(crypt_load); DLSYM_FUNCTION(crypt_resize); -DLSYM_FUNCTION(crypt_resume_by_passphrase); +#if HAVE_CRYPT_RESUME_BY_VOLUME_KEY +DLSYM_FUNCTION(crypt_resume_by_volume_key); +#endif DLSYM_FUNCTION(crypt_set_data_device); DLSYM_FUNCTION(crypt_set_debug_level); DLSYM_FUNCTION(crypt_set_log_callback); @@ -276,7 +278,9 @@ int dlopen_cryptsetup(void) { DLSYM_ARG(crypt_keyslot_max), DLSYM_ARG(crypt_load), DLSYM_ARG(crypt_resize), - DLSYM_ARG(crypt_resume_by_passphrase), +#if HAVE_CRYPT_RESUME_BY_VOLUME_KEY + DLSYM_ARG(crypt_resume_by_volume_key), +#endif DLSYM_ARG(crypt_set_data_device), DLSYM_ARG(crypt_set_debug_level), DLSYM_ARG(crypt_set_log_callback), diff --git a/src/shared/cryptsetup-util.h b/src/shared/cryptsetup-util.h index 8a4416b4fc..f00ac367b6 100644 --- a/src/shared/cryptsetup-util.h +++ b/src/shared/cryptsetup-util.h @@ -41,7 +41,9 @@ DLSYM_PROTOTYPE(crypt_keyslot_destroy); DLSYM_PROTOTYPE(crypt_keyslot_max); DLSYM_PROTOTYPE(crypt_load); DLSYM_PROTOTYPE(crypt_resize); -DLSYM_PROTOTYPE(crypt_resume_by_passphrase); +#if HAVE_CRYPT_RESUME_BY_VOLUME_KEY +DLSYM_PROTOTYPE(crypt_resume_by_volume_key); +#endif DLSYM_PROTOTYPE(crypt_set_data_device); DLSYM_PROTOTYPE(crypt_set_debug_level); DLSYM_PROTOTYPE(crypt_set_log_callback); From 5ec87d577f92effe27a62e965e02a6f9a40f81cc Mon Sep 17 00:00:00 2001 From: Adrian Vovk Date: Thu, 1 Feb 2024 11:43:48 -0500 Subject: [PATCH 2/5] 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 6fe3bb3ce0..726d9d9832 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 624cbdb3d3..dd3603efa7 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 09e86f37f3..a2892d00a3 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 7f42461a45..c0f1b83deb 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 d4eaf1821b..5f345b3d40 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 e46acf7ed5..d97e8fd9f0 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 4bf9922ef3..5aef06bf22 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() { From d94c7eef121dd5dbab758722a199125d49f85d55 Mon Sep 17 00:00:00 2001 From: Adrian Vovk Date: Thu, 1 Feb 2024 13:35:03 -0500 Subject: [PATCH 3/5] homework: Implement offline updates This makes it possible to update a home record (and blob directory) of a home area that's either completely absent (i.e. on a USB stick that's unplugged) or just inaccessible due to lack of authentication --- man/homectl.xml | 10 +++++++ man/org.freedesktop.home1.xml | 12 ++++++-- src/home/home-util.h | 7 +++++ src/home/homectl.c | 14 ++++++++- src/home/homed-home-bus.c | 7 +++-- src/home/homed-home.c | 29 ++++++++++++++----- src/home/homed-manager-bus.c | 7 +++-- src/home/homed-operation.h | 1 + src/home/homework.c | 53 +++++++++++++++++++++++++++-------- test/units/testsuite-46.sh | 18 +++++++++++- 10 files changed, 131 insertions(+), 27 deletions(-) diff --git a/man/homectl.xml b/man/homectl.xml index 1a0535cd4a..f1bade2053 100644 --- a/man/homectl.xml +++ b/man/homectl.xml @@ -169,6 +169,16 @@ + + + + Do not attempt to update the copy of the user record and blob directory that is embedded inside + of the home area. This allows for operation on home areas that are absent, or without needing to authenticate as + the user being modified. + + + + diff --git a/man/org.freedesktop.home1.xml b/man/org.freedesktop.home1.xml index 726d9d9832..9334f1a596 100644 --- a/man/org.freedesktop.home1.xml +++ b/man/org.freedesktop.home1.xml @@ -334,8 +334,16 @@ node /org/freedesktop/home1 { Directories for more info). The blobs argument works in the same way as CreateHomeEx(), so check there for details. The new blob directory contents passed into this method will completely replace the user's existing blob directory. The flags argument - may be used for future expansion, but for now pass 0. This method is equivalent to UpdateEx() - on the org.freedesktop.home1.Home interface. + can be used to further customize the behavior of this method via flags defined as follows: + +#define SD_HOMED_UPDATE_OFFLINE (UINT64_C(1) << 0) + + When SD_HOMED_UPDATE_OFFLINE (0x01) is set, no attempt is made to update the copies + of the user record and blob directory that are embedded into the home directory. Changes will be stored, however, + and may be propagated into the home directory the next time it is reconciled (most likely when the user next logs in). + Note that any changes made with this flag set may be lost if the home area has a newer record, which can happen + if the home area is updated on another machine after this method call. This method is equivalent to + UpdateEx() 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 optionally a user record consisting only of the secret diff --git a/src/home/home-util.h b/src/home/home-util.h index f2e5787a54..42131b9f41 100644 --- a/src/home/home-util.h +++ b/src/home/home-util.h @@ -9,6 +9,13 @@ #include "time-util.h" #include "user-record.h" +/* Flags supported by UpdateEx() */ +#define SD_HOMED_UPDATE_OFFLINE (UINT64_C(1) << 0) +#define SD_HOMED_UPDATE_FLAGS_ALL (SD_HOMED_UPDATE_OFFLINE) + +/* Flags supported by CreateHomeEx() */ +#define SD_HOMED_CREATE_FLAGS_ALL (0) + /* Put some limits on disk sizes: not less than 5M, not more than 5T */ #define USER_DISK_SIZE_MIN (UINT64_C(5)*1024*1024) #define USER_DISK_SIZE_MAX (UINT64_C(5)*1024*1024*1024*1024) diff --git a/src/home/homectl.c b/src/home/homectl.c index 11a138070b..2a7917d7f9 100644 --- a/src/home/homectl.c +++ b/src/home/homectl.c @@ -61,6 +61,7 @@ static bool arg_legend = true; static bool arg_ask_password = true; static BusTransport arg_transport = BUS_TRANSPORT_LOCAL; static const char *arg_host = NULL; +static bool arg_offline = false; static const char *arg_identity = NULL; static JsonVariant *arg_identity_extra = NULL; static JsonVariant *arg_identity_extra_privileged = NULL; @@ -1712,6 +1713,7 @@ static int update_home(int argc, char *argv[], void *userdata) { _cleanup_free_ char *buffer = NULL; _cleanup_hashmap_free_ Hashmap *blobs = NULL; const char *username; + uint64_t flags = 0; int r; if (argc >= 2) @@ -1754,6 +1756,9 @@ static int update_home(int argc, char *argv[], void *userdata) { if (arg_and_resize || arg_and_change_password) log_info("Updating home directory."); + if (arg_offline) + flags |= SD_HOMED_UPDATE_OFFLINE; + for (;;) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL; @@ -1777,7 +1782,7 @@ static int update_home(int argc, char *argv[], void *userdata) { if (r < 0) return bus_log_create_error(r); - r = sd_bus_message_append(m, "t", UINT64_C(0)); + r = sd_bus_message_append(m, "t", flags); if (r < 0) return bus_log_create_error(r); @@ -2564,6 +2569,7 @@ static int help(int argc, char *argv[], void *userdata) { " --no-pager Do not pipe output into a pager\n" " --no-legend Do not show the headers and footers\n" " --no-ask-password Do not ask for system passwords\n" + " --offline Don't update record embedded in home directory\n" " -H --host=[USER@]HOST Operate on remote host\n" " -M --machine=CONTAINER Operate on local container\n" " --identity=PATH Read JSON identity from file\n" @@ -2723,6 +2729,7 @@ static int parse_argv(int argc, char *argv[]) { ARG_NO_PAGER, ARG_NO_LEGEND, ARG_NO_ASK_PASSWORD, + ARG_OFFLINE, ARG_REALM, ARG_EMAIL_ADDRESS, ARG_DISK_SIZE, @@ -2808,6 +2815,7 @@ static int parse_argv(int argc, char *argv[]) { { "no-pager", no_argument, NULL, ARG_NO_PAGER }, { "no-legend", no_argument, NULL, ARG_NO_LEGEND }, { "no-ask-password", no_argument, NULL, ARG_NO_ASK_PASSWORD }, + { "offline", no_argument, NULL, ARG_OFFLINE }, { "host", required_argument, NULL, 'H' }, { "machine", required_argument, NULL, 'M' }, { "identity", required_argument, NULL, 'I' }, @@ -2933,6 +2941,10 @@ static int parse_argv(int argc, char *argv[]) { arg_ask_password = false; break; + case ARG_OFFLINE: + arg_offline = true; + break; + case 'H': arg_transport = BUS_TRANSPORT_REMOTE; arg_host = optarg; diff --git a/src/home/homed-home-bus.c b/src/home/homed-home-bus.c index dd3603efa7..23578fe314 100644 --- a/src/home/homed-home-bus.c +++ b/src/home/homed-home-bus.c @@ -6,6 +6,7 @@ #include "bus-polkit.h" #include "fd-util.h" #include "format-util.h" +#include "home-util.h" #include "homed-bus.h" #include "homed-home-bus.h" #include "homed-home.h" @@ -432,8 +433,8 @@ int bus_home_update_record( if (r < 0) return r; - if (flags != 0) - return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Provided flags are unsupported."); + if ((flags & ~SD_HOMED_UPDATE_FLAGS_ALL) != 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid flags provided."); r = home_verify_polkit_async( h, @@ -457,6 +458,8 @@ int bus_home_update_record( if (r < 0) return r; + h->current_operation->call_flags = flags; + return 1; } diff --git a/src/home/homed-home.c b/src/home/homed-home.c index a2892d00a3..447e8c597c 100644 --- a/src/home/homed-home.c +++ b/src/home/homed-home.c @@ -955,10 +955,13 @@ static void home_create_finish(Home *h, int ret, UserRecord *hr) { static void home_change_finish(Home *h, int ret, UserRecord *hr) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + uint64_t flags; int r; assert(h); + flags = h->current_operation ? h->current_operation->call_flags : 0; + if (ret < 0) { (void) home_count_bad_authentication(h, ret, /* save= */ true); @@ -969,17 +972,22 @@ static void home_change_finish(Home *h, int ret, UserRecord *hr) { } if (hr) { - r = home_set_record(h, hr); - if (r < 0) - log_warning_errno(r, "Failed to update home record, ignoring: %m"); - else { + if (!FLAGS_SET(flags, SD_HOMED_UPDATE_OFFLINE)) { r = user_record_good_authentication(h->record); if (r < 0) log_warning_errno(r, "Failed to increase good authentication counter, ignoring: %m"); + } + r = home_set_record(h, hr); + if (r >= 0) r = home_save_record(h); - if (r < 0) - log_warning_errno(r, "Failed to write home record to disk, ignoring: %m"); + 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"); + sd_bus_error_set(&error, SD_BUS_ERROR_FAILED, "Failed to cache changes to home record"); + goto finish; + } else + log_warning_errno(r, "Failed to update home record, ignoring: %m"); } } @@ -1312,6 +1320,11 @@ static int home_start_work( _exit(EXIT_FAILURE); } + if (setenv("SYSTEMD_HOMEWORK_UPDATE_OFFLINE", one_zero(FLAGS_SET(flags, SD_HOMED_UPDATE_OFFLINE)), 1) < 0) { + log_error_errno(errno, "Failed to set $SYSTEMD_HOMEWORK_UPDATE_OFFLINE: %m"); + _exit(EXIT_FAILURE); + } + r = setenv_systemd_exec_pid(true); if (r < 0) log_warning_errno(r, "Failed to update $SYSTEMD_EXEC_PID, ignoring: %m"); @@ -1783,7 +1796,9 @@ int home_update(Home *h, UserRecord *hr, Hashmap *blobs, uint64_t flags, sd_bus_ case HOME_UNFIXATED: return sd_bus_error_setf(error, BUS_ERROR_HOME_UNFIXATED, "Home %s has not been fixated yet.", h->user_name); case HOME_ABSENT: - return sd_bus_error_setf(error, BUS_ERROR_HOME_ABSENT, "Home %s is currently missing or not plugged in.", h->user_name); + if (!FLAGS_SET(flags, SD_HOMED_UPDATE_OFFLINE)) + return sd_bus_error_setf(error, BUS_ERROR_HOME_ABSENT, "Home %s is currently missing or not plugged in.", h->user_name); + break; /* offline updates are compatible w/ an absent home area */ case HOME_LOCKED: return sd_bus_error_setf(error, BUS_ERROR_HOME_LOCKED, "Home %s is currently locked.", h->user_name); case HOME_INACTIVE: diff --git a/src/home/homed-manager-bus.c b/src/home/homed-manager-bus.c index 403a7d0213..58cd037105 100644 --- a/src/home/homed-manager-bus.c +++ b/src/home/homed-manager-bus.c @@ -6,6 +6,7 @@ #include "bus-common-errors.h" #include "bus-polkit.h" #include "format-util.h" +#include "home-util.h" #include "homed-bus.h" #include "homed-home-bus.h" #include "homed-manager-bus.h" @@ -507,8 +508,8 @@ static int method_create_home(sd_bus_message *message, void *userdata, sd_bus_er r = sd_bus_message_read(message, "t", &flags); if (r < 0) return r; - if (flags != 0) - return sd_bus_error_setf(error, SD_BUS_ERROR_NOT_SUPPORTED, "Provided flags are unsupported."); + if ((flags & ~SD_HOMED_CREATE_FLAGS_ALL) != 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid flags provided."); } r = bus_verify_polkit_async( @@ -538,6 +539,8 @@ static int method_create_home(sd_bus_message *message, void *userdata, sd_bus_er if (r < 0) return r; + h->current_operation->call_flags = flags; + return 1; fail: diff --git a/src/home/homed-operation.h b/src/home/homed-operation.h index 004246a4e6..af165bb4a5 100644 --- a/src/home/homed-operation.h +++ b/src/home/homed-operation.h @@ -39,6 +39,7 @@ typedef struct Operation { sd_bus_message *message; UserRecord *secret; + uint64_t call_flags; /* flags passed into UpdateEx() or CreateHomeEx() */ int send_fd; /* pipe fd for AcquireHome() which is taken already when we start the operation */ int result; /* < 0 if not completed yet, == 0 on failure, > 0 on success */ diff --git a/src/home/homework.c b/src/home/homework.c index d97e8fd9f0..afc1142298 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -1551,6 +1551,21 @@ static int home_remove(UserRecord *h) { return 0; } +static int home_basic_validate_update(UserRecord *h) { + assert(h); + + if (!h->user_name) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "User record lacks user name, refusing."); + + if (!uid_is_valid(h->uid)) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "User record lacks UID, refusing."); + + if (!IN_SET(user_record_storage(h), USER_LUKS, USER_DIRECTORY, USER_SUBVOLUME, USER_FSCRYPT, USER_CIFS)) + return log_error_errno(SYNTHETIC_ERRNO(ENOTTY), "Processing home directories of type '%s' currently not supported.", user_storage_to_string(user_record_storage(h))); + + return 0; +} + static int home_validate_update(UserRecord *h, HomeSetup *setup, HomeSetupFlags *flags) { bool has_mount = false; int r; @@ -1558,12 +1573,9 @@ static int home_validate_update(UserRecord *h, HomeSetup *setup, HomeSetupFlags assert(h); assert(setup); - if (!h->user_name) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "User record lacks user name, refusing."); - if (!uid_is_valid(h->uid)) - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "User record lacks UID, refusing."); - if (!IN_SET(user_record_storage(h), USER_LUKS, USER_DIRECTORY, USER_SUBVOLUME, USER_FSCRYPT, USER_CIFS)) - return log_error_errno(SYNTHETIC_ERRNO(ENOTTY), "Processing home directories of type '%s' currently not supported.", user_storage_to_string(user_record_storage(h))); + r = home_basic_validate_update(h); + if (r < 0) + return r; r = user_record_test_home_directory_and_warn(h); if (r < 0) @@ -1610,19 +1622,31 @@ static int home_update(UserRecord *h, Hashmap *blobs, UserRecord **ret) { _cleanup_(home_setup_done) HomeSetup setup = HOME_SETUP_INIT; _cleanup_(password_cache_free) PasswordCache cache = {}; HomeSetupFlags flags = 0; + bool offline; int r; assert(h); assert(ret); - password_cache_load_keyring(h, &cache); + offline = getenv_bool("SYSTEMD_HOMEWORK_UPDATE_OFFLINE") > 0; - r = user_record_authenticate(h, h, &cache, /* strict_verify= */ true); - if (r < 0) - return r; - assert(r > 0); /* Insist that a password was verified */ + if (!offline) { + password_cache_load_keyring(h, &cache); - r = home_validate_update(h, &setup, &flags); + 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); + } else { + /* In offline mode we skip all authentication, since we're + * not propagating anything into the home area. The new home + * records's authentication will still be checked when the user + * next logs in, so this is fine */ + + r = home_basic_validate_update(h); + } if (r < 0) return r; @@ -1630,6 +1654,11 @@ static int home_update(UserRecord *h, Hashmap *blobs, UserRecord **ret) { if (r < 0) return r; + if (offline) { + log_info("Offline update requested. Not touching embedded records."); + return user_record_clone(h, USER_RECORD_LOAD_MASK_SECRET|USER_RECORD_PERMISSIVE, ret); + } + r = home_setup(h, flags, &setup, &cache, &header_home); if (r < 0) return r; diff --git a/test/units/testsuite-46.sh b/test/units/testsuite-46.sh index 5aef06bf22..f827510640 100755 --- a/test/units/testsuite-46.sh +++ b/test/units/testsuite-46.sh @@ -74,13 +74,19 @@ inspect test-user homectl deactivate test-user inspect test-user +homectl update test-user --real-name "Offline test" --offline +inspect test-user + PASSWORD=xEhErW0ndafV4s homectl activate test-user inspect test-user +# Ensure that the offline changes were propagated in +grep "Offline test" /home/test-user/.identity + homectl deactivate test-user inspect test-user -PASSWORD=xEhErW0ndafV4s homectl update test-user --real-name="Offline test" +PASSWORD=xEhErW0ndafV4s homectl update test-user --real-name="Inactive test" inspect test-user PASSWORD=xEhErW0ndafV4s homectl activate test-user @@ -326,6 +332,16 @@ checkblob barely-fits /tmp/external-barely-fits (! PASSWORD=EMJuc3zQaMibJo homectl update blob-user -b файл=/tmp/external-test3 ) (! PASSWORD=EMJuc3zQaMibJo homectl update blob-user -b special@chars=/tmp/external-test3 ) +# Make sure offline updates to blobs get propagated in +homectl deactivate blob-user +inspect blob-user +homectl update blob-user --offline -b barely-fits= -b propagated=/tmp/external-test3 +inspect blob-user +PASSWORD=EMJuc3zQaMibJo homectl activate blob-user +inspect blob-user +(! checkblob barely-fits /tmp/external-barely-fits ) +checkblob propagated /tmp/external-test3 + homectl deactivate blob-user wait_for_state blob-user inactive homectl remove blob-user From 44aaff689b39b497e7d62c6ccd37383c93d6a53e Mon Sep 17 00:00:00 2001 From: Adrian Vovk Date: Thu, 1 Feb 2024 14:15:48 -0500 Subject: [PATCH 4/5] update TODO --- TODO | 2 -- 1 file changed, 2 deletions(-) diff --git a/TODO b/TODO index 96d606ca34..097c639d13 100644 --- a/TODO +++ b/TODO @@ -2277,8 +2277,6 @@ Features: - fingerprint authentication, pattern authentication, … - make sure "classic" user records can also be managed by homed - make size of $XDG_RUNTIME_DIR configurable in user record - - query password from kernel keyring first - - update even if record is "absent" - move acct mgmt stuff from pam_systemd_home to pam_systemd? - when "homectl --pkcs11-token-uri=" is used, synthesize ssh-authorized-keys records for all keys we have private keys on the stick for - make slice for users configurable (requires logind rework) From 269a3fe245581615eeaa9b2f616c81242b9d17df Mon Sep 17 00:00:00 2001 From: Adrian Vovk Date: Tue, 5 Mar 2024 12:25:42 -0500 Subject: [PATCH 5/5] TEST-46-HOMED: Disable auth rate-limiting Rate limiting authentication attempts in the test can cause somewhat sporadic test failures: adding a test case might suddenly cause future test cases to fail because of too many authentication attempts too quickly We're not trying to test the rate-limiting, we're trying to test the functionality of homed. So we effectively disable rate-limiting on all the home areas we create --- test/units/testsuite-46.sh | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/test/units/testsuite-46.sh b/test/units/testsuite-46.sh index f827510640..b20b39d762 100755 --- a/test/units/testsuite-46.sh +++ b/test/units/testsuite-46.sh @@ -42,13 +42,16 @@ mount -t tmpfs tmpfs /home -o size=290M # we enable --luks-discard= since we run our tests in a tight VM, hence don't # needlessly pressure for storage. We also set the cheapest KDF, since we don't -# want to waste CI CPU cycles on it. +# want to waste CI CPU cycles on it. We also effectively disable rate-limiting on +# the user by allowing 1000 logins per second NEWPASSWORD=xEhErW0ndafV4s homectl create test-user \ --disk-size=min \ --luks-discard=yes \ --image-path=/home/test-user.home \ --luks-pbkdf-type=pbkdf2 \ - --luks-pbkdf-time-cost=1ms + --luks-pbkdf-time-cost=1ms \ + --rate-limit-interval=1s \ + --rate-limit-burst=1000 inspect test-user PASSWORD=xEhErW0ndafV4s homectl authenticate test-user @@ -168,7 +171,9 @@ if ! systemd-detect-virt -cq ; then --luks-discard=yes \ --image-path=/home/test-user2.home \ --luks-pbkdf-type=pbkdf2 \ - --luks-pbkdf-time-cost=1ms + --luks-pbkdf-time-cost=1ms \ + --rate-limit-interval=1s \ + --rate-limit-burst=1000 inspect test-user2 # activate second user @@ -198,7 +203,9 @@ PASSWORD=xEhErW0ndafV4s homectl with test-user -- test ! -f /home/test-user/xyz (! PASSWORD=xEhErW0ndafV4s homectl with test-user -- test -f /home/test-user/xyz) PASSWORD=xEhErW0ndafV4s homectl with test-user -- touch /home/test-user/xyz PASSWORD=xEhErW0ndafV4s homectl with test-user -- test -f /home/test-user/xyz -# CAREFUL adding more `homectl with` tests here. Auth can get rate-limited and cause the tests to fail. +PASSWORD=xEhErW0ndafV4s homectl with test-user -- rm /home/test-user/xyz +PASSWORD=xEhErW0ndafV4s homectl with test-user -- test ! -f /home/test-user/xyz +(! PASSWORD=xEhErW0ndafV4s homectl with test-user -- test -f /home/test-user/xyz) wait_for_state test-user inactive homectl remove test-user @@ -231,6 +238,7 @@ dd if=/dev/urandom of=/tmp/external-toobig bs=1M count=65 NEWPASSWORD=EMJuc3zQaMibJo homectl create blob-user \ --disk-size=min --luks-discard=yes \ --luks-pbkdf-type=pbkdf2 --luks-pbkdf-time-cost=1ms \ + --rate-limit-interval=1s --rate-limit-burst=1000 \ --uid=12345 \ --blob=/tmp/blob1 inspect blob-user @@ -511,6 +519,8 @@ if command -v ssh &>/dev/null && command -v sshd &>/dev/null && ! [[ -v ASAN_OPT --luks-discard=yes \ --luks-pbkdf-type=pbkdf2 \ --luks-pbkdf-time-cost=1ms \ + --rate-limit-interval=1s \ + --rate-limit-burst=1000 \ --enforce-password-policy=no \ --ssh-authorized-keys=@/tmp/homed.id_ecdsa.pub \ --stop-delay=0 \