From cbffdcecaecd61ec73813d536e1cce406b552e1e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 May 2020 17:02:12 +0200 Subject: [PATCH 01/10] homed: return a better error when a home has no authentication information defined We can't log into home entries that have no password or PKCS#11 token. Return a proper, useful error in that case. See: #15178 --- src/home/homed-home.c | 2 ++ src/home/homework.c | 1 + src/libsystemd/sd-bus/bus-common-errors.c | 1 + src/libsystemd/sd-bus/bus-common-errors.h | 1 + 4 files changed, 5 insertions(+) diff --git a/src/home/homed-home.c b/src/home/homed-home.c index be1a710ccc..cf11c05d60 100644 --- a/src/home/homed-home.c +++ b/src/home/homed-home.c @@ -471,6 +471,8 @@ static int convert_worker_errno(Home *h, int e, sd_bus_error *error) { return sd_bus_error_setf(error, BUS_ERROR_HOME_NOT_ACTIVE, "Home %s is currently not active", h->user_name); case -ENOSPC: return sd_bus_error_setf(error, BUS_ERROR_NO_DISK_SPACE, "Not enough disk space for home %s", h->user_name); + case -EKEYREVOKED: + return sd_bus_error_setf(error, BUS_ERROR_HOME_CANT_AUTHENTICATE, "Home %s has no password or other authentication mechanism defined.", h->user_name); } return 0; diff --git a/src/home/homework.c b/src/home/homework.c index 76fd79fc2a..2c10997b6b 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -1489,6 +1489,7 @@ static int run(int argc, char *argv[]) { * EBUSY → file system is currently active * ENOEXEC → file system is currently not active * ENOSPC → not enough disk space for operation + * EKEYREVOKED → user record has not suitable hashed password or pkcs#11 entry, we cannot authenticate */ if (streq(argv[1], "activate")) diff --git a/src/libsystemd/sd-bus/bus-common-errors.c b/src/libsystemd/sd-bus/bus-common-errors.c index 174f1228af..28f98cebce 100644 --- a/src/libsystemd/sd-bus/bus-common-errors.c +++ b/src/libsystemd/sd-bus/bus-common-errors.c @@ -134,6 +134,7 @@ BUS_ERROR_MAP_ELF_REGISTER const sd_bus_error_map bus_common_errors[] = { SD_BUS_ERROR_MAP(BUS_ERROR_HOME_NOT_LOCKED, ENOEXEC), SD_BUS_ERROR_MAP(BUS_ERROR_TOO_MANY_OPERATIONS, ENOBUFS), SD_BUS_ERROR_MAP(BUS_ERROR_AUTHENTICATION_LIMIT_HIT, ETOOMANYREFS), + SD_BUS_ERROR_MAP(BUS_ERROR_HOME_CANT_AUTHENTICATE, EKEYREVOKED), SD_BUS_ERROR_MAP_END }; diff --git a/src/libsystemd/sd-bus/bus-common-errors.h b/src/libsystemd/sd-bus/bus-common-errors.h index dc58f88bbd..68ecbd65dd 100644 --- a/src/libsystemd/sd-bus/bus-common-errors.h +++ b/src/libsystemd/sd-bus/bus-common-errors.h @@ -115,5 +115,6 @@ #define BUS_ERROR_NO_DISK_SPACE "org.freedesktop.home1.NoDiskSpace" #define BUS_ERROR_TOO_MANY_OPERATIONS "org.freedesktop.home1.TooManyOperations" #define BUS_ERROR_AUTHENTICATION_LIMIT_HIT "org.freedesktop.home1.AuthenticationLimitHit" +#define BUS_ERROR_HOME_CANT_AUTHENTICATE "org.freedesktop.home1.HomeCantAuthenticate" BUS_ERROR_MAP_ELF_USE(bus_common_errors); From 5b3f4a20ea164bba1f7bf65814c4236a18f9beb5 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 May 2020 17:14:48 +0200 Subject: [PATCH 02/10] fileio: sync directory after rename, too --- src/basic/fileio.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/basic/fileio.c b/src/basic/fileio.c index 6b84d14624..00dce02064 100644 --- a/src/basic/fileio.c +++ b/src/basic/fileio.c @@ -202,6 +202,13 @@ static int write_string_file_atomic( goto fail; } + if (FLAGS_SET(flags, WRITE_STRING_FILE_SYNC)) { + /* Sync the rename, too */ + r = fsync_directory_of_file(fileno(f)); + if (r < 0) + return r; + } + return 0; fail: From e4005ffe00d321e027280147a9959ee6eb030cbf Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 May 2020 17:14:51 +0200 Subject: [PATCH 03/10] homed: when updating local copy of user record, sync to disk Apparently xfs needs us to sync explicitly, see #15178. --- src/home/homed-home.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/home/homed-home.c b/src/home/homed-home.c index cf11c05d60..47ee7d2328 100644 --- a/src/home/homed-home.c +++ b/src/home/homed-home.c @@ -292,7 +292,7 @@ int home_save_record(Home *h) { fn = strjoina("/var/lib/systemd/home/", h->user_name, ".identity"); - r = write_string_file(fn, text, WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_MODE_0600); + r = write_string_file(fn, text, WRITE_STRING_FILE_ATOMIC|WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_MODE_0600|WRITE_STRING_FILE_SYNC); if (r < 0) return r; From 20f4a308bf3aec5b80ae9fc9d155a46e235a64d9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 May 2020 17:25:50 +0200 Subject: [PATCH 04/10] homed: automatically clean up empty user record files See: #15178 --- src/home/homed-manager.c | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/home/homed-manager.c b/src/home/homed-manager.c index df0ed2f4f3..da37c092a2 100644 --- a/src/home/homed-manager.c +++ b/src/home/homed-manager.c @@ -329,16 +329,31 @@ static int manager_add_home_by_record( _cleanup_(user_record_unrefp) UserRecord *hr = NULL; unsigned line, column; int r, is_signed; + struct stat st; Home *h; assert(m); assert(name); assert(fname); + if (fstatat(dir_fd, fname, &st, 0) < 0) + return log_error_errno(errno, "Failed to stat identity record %s: %m", fname); + + if (!S_ISREG(st.st_mode)) { + log_debug("Identity record file %s is not a regular file, ignoring.", fname); + return 0; + } + + if (st.st_size == 0) + goto unlink_this_file; + r = json_parse_file_at(NULL, dir_fd, fname, JSON_PARSE_SENSITIVE, &v, &line, &column); if (r < 0) return log_error_errno(r, "Failed to parse identity record at %s:%u%u: %m", fname, line, column); + if (json_variant_is_blank_object(v)) + goto unlink_this_file; + hr = user_record_new(); if (!hr) return log_oom(); @@ -394,6 +409,19 @@ static int manager_add_home_by_record( h->signed_locally = is_signed == USER_RECORD_SIGNED_EXCLUSIVE; return 1; + +unlink_this_file: + /* If this is an empty file, then let's just remove it. An empty file is not useful in any case, and + * apparently xfs likes to leave empty files around when not unmounted cleanly (see + * https://github.com/systemd/systemd/issues/15178 for example). Note that we don't delete non-empty + * files even if they are invalid, because that's just too risky, we might delete data the user still + * needs. But empty files are never useful, hence let's just remove them. */ + + if (unlinkat(dir_fd, fname, 0) < 0) + return log_error_errno(errno, "Failed to remove empty user record file %s: %m", fname); + + log_notice("Discovered empty user record file /var/lib/systemd/home/%s, removed automatically.", fname); + return 0; } static int manager_enumerate_records(Manager *m) { From b84719269983328d45b2c121590c8a2507fe7837 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 May 2020 17:26:17 +0200 Subject: [PATCH 05/10] homed: make sure we log about invalid user records we load --- src/home/homed-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/home/homed-manager.c b/src/home/homed-manager.c index da37c092a2..8563b36e01 100644 --- a/src/home/homed-manager.c +++ b/src/home/homed-manager.c @@ -358,7 +358,7 @@ static int manager_add_home_by_record( if (!hr) return log_oom(); - r = user_record_load(hr, v, USER_RECORD_LOAD_REFUSE_SECRET); + r = user_record_load(hr, v, USER_RECORD_LOAD_REFUSE_SECRET|USER_RECORD_LOG); if (r < 0) return r; From e8dd54ab3cfc37f250f737711375a25505ada3ab Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 May 2020 18:06:27 +0200 Subject: [PATCH 06/10] homed: fix typo --- src/home/homed-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/home/homed-manager.c b/src/home/homed-manager.c index 8563b36e01..525e0c7d66 100644 --- a/src/home/homed-manager.c +++ b/src/home/homed-manager.c @@ -1365,7 +1365,7 @@ static int manager_generate_key_pair(Manager *m) { if (rename(temp_private, "/var/lib/systemd/home/local.private") < 0) { (void) unlink_noerrno("/var/lib/systemd/home/local.public"); /* try to remove the file we already created */ - return log_error_errno(errno, "Failed to move privtate key file into place: %m"); + return log_error_errno(errno, "Failed to move private key file into place: %m"); } temp_private = mfree(temp_private); From fa3709c5fb23ee1741218871758bdee3ded0190c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 May 2020 18:06:56 +0200 Subject: [PATCH 07/10] homed: also fsync private/public key pair when storing it --- src/home/homed-manager.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/home/homed-manager.c b/src/home/homed-manager.c index 525e0c7d66..ea5791ae49 100644 --- a/src/home/homed-manager.c +++ b/src/home/homed-manager.c @@ -1337,7 +1337,7 @@ static int manager_generate_key_pair(Manager *m) { if (PEM_write_PUBKEY(fpublic, m->private_key) <= 0) return log_error_errno(SYNTHETIC_ERRNO(EIO), "Failed to write public key."); - r = fflush_and_check(fpublic); + r = fflush_sync_and_check(fpublic); if (r < 0) return log_error_errno(r, "Failed to write private key: %m"); @@ -1351,7 +1351,7 @@ static int manager_generate_key_pair(Manager *m) { if (PEM_write_PrivateKey(fprivate, m->private_key, NULL, NULL, 0, NULL, 0) <= 0) return log_error_errno(SYNTHETIC_ERRNO(EIO), "Failed to write private key pair."); - r = fflush_and_check(fprivate); + r = fflush_sync_and_check(fprivate); if (r < 0) return log_error_errno(r, "Failed to write private key: %m"); @@ -1369,6 +1369,10 @@ static int manager_generate_key_pair(Manager *m) { } temp_private = mfree(temp_private); + r = fsync_path_at(AT_FDCWD, "/var/lib/systemd/home/"); + if (r < 0) + log_warning_errno(r, "Failed to sync /var/lib/systemd/home/, ignoring: %m"); + return 1; } From 2fcbf417b615539eec3e53444b964593d6415940 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 May 2020 18:07:23 +0200 Subject: [PATCH 08/10] bus-util: actually register the object manager --- src/shared/bus-util.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/shared/bus-util.c b/src/shared/bus-util.c index c7611a6e85..379aecf730 100644 --- a/src/shared/bus-util.c +++ b/src/shared/bus-util.c @@ -1591,6 +1591,12 @@ int bus_add_implementation(sd_bus *bus, const BusObjectImplementation *impl, voi impl->path); } + if (impl->manager) { + r = sd_bus_add_object_manager(bus, NULL, impl->path); + if (r < 0) + return log_error_errno(r, "Failed to add object manager for %s: %m", impl->path); + } + for (size_t i = 0; impl->children && impl->children[i]; i++) { r = bus_add_implementation(bus, impl->children[i], userdata); if (r < 0) From 1a53adb3aba43405c204aeb22a544323f1ea4949 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 May 2020 18:12:41 +0200 Subject: [PATCH 09/10] homed: include error string when in log message if quota doesn't work --- src/home/homed-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/home/homed-manager.c b/src/home/homed-manager.c index ea5791ae49..7d951bee3b 100644 --- a/src/home/homed-manager.c +++ b/src/home/homed-manager.c @@ -513,7 +513,7 @@ static int search_quota(uid_t uid, const char *exclude_quota_path) { if (ERRNO_IS_NOT_SUPPORTED(r)) log_debug_errno(r, "No UID quota support on %s, ignoring.", where); else - log_warning_errno(r, "Failed to query quota on %s, ignoring.", where); + log_warning_errno(r, "Failed to query quota on %s, ignoring: %m", where); continue; } From c8f145adbb17fb448a78cfaf65f67f22026e6792 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 20 May 2020 18:38:07 +0200 Subject: [PATCH 10/10] homed: don't insist on authentication against host-copy user record homed maintains two or three copies of the user's identity record per home directory: one on the host, one inside the LUKS header, and one embedded in the home directory. Previously we'd insist that if a user logs in they have to authenticate against all three, as a safety feature. This broke logging into unfixated records however, since in that case the host version is synthetic and thus does not carry any authentication data. Let's hence losen the strictness here: accept authentication against host records that carry no auth data. This should be safe as we know after all that the second/third record will catch invalid accesses. Fixes: #15178 --- src/home/homework-luks.c | 3 ++- src/home/homework.c | 37 ++++++++++++++++++++++++++----------- src/home/homework.h | 2 +- 3 files changed, 29 insertions(+), 13 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index caa4168265..44face3389 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -727,9 +727,10 @@ static int luks_validate_home_record( if (!user_record_compatible(h, lhr)) return log_error_errno(SYNTHETIC_ERRNO(EREMCHG), "LUKS home record not compatible with host record, refusing."); - r = user_record_authenticate(lhr, h, pkcs11_decrypted_passwords); + r = user_record_authenticate(lhr, h, pkcs11_decrypted_passwords, /* strict_verify= */ true); if (r < 0) return r; + assert(r > 0); /* Insist that a password was verified */ *ret_luks_home_record = TAKE_PTR(lhr); return 0; diff --git a/src/home/homework.c b/src/home/homework.c index 2c10997b6b..316933cf4e 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -35,7 +35,8 @@ int user_record_authenticate( UserRecord *h, UserRecord *secret, - char ***pkcs11_decrypted_passwords) { + char ***pkcs11_decrypted_passwords, + bool strict_verify) { bool need_password = false, need_token = false, need_pin = false, need_protected_authentication_path_permitted = false, pin_locked = false, pin_incorrect = false, pin_incorrect_few_tries_left = false, pin_incorrect_one_try_left = false; @@ -66,7 +67,7 @@ int user_record_authenticate( return log_error_errno(r, "Failed to validate password of record: %m"); else { log_info("Provided password unlocks user record."); - return 0; + return 1; } /* Second, let's see if any of the PKCS#11 security tokens are plugged in and help us */ @@ -86,7 +87,7 @@ int user_record_authenticate( return log_error_errno(r, "Failed to check supplied PKCS#11 password: %m"); if (r > 0) { log_info("Previously acquired PKCS#11 password unlocks user record."); - return 0; + return 1; } } @@ -129,7 +130,7 @@ int user_record_authenticate( if (r < 0) return log_oom(); - return 0; + return 1; } #else need_token = true; @@ -156,7 +157,18 @@ int user_record_authenticate( return -ENOKEY; /* Hmm, this means neither PCKS#11 nor classic hashed passwords were supplied, we cannot authenticate this reasonably */ - return log_debug_errno(SYNTHETIC_ERRNO(EKEYREVOKED), "No hashed passwords and no PKCS#11 tokens defined, cannot authenticate user record."); + if (strict_verify) + return log_debug_errno(SYNTHETIC_ERRNO(EKEYREVOKED), + "No hashed passwords and no PKCS#11 tokens defined, cannot authenticate user record, refusing."); + + /* If strict verification is off this means we are possibly in the case where we encountered an + * unfixated record, i.e. a synthetic one that accordingly lacks any authentication data. In this + * case, allow the authentication to pass for now, so that the second (or third) authentication level + * (the ones of the user record in the LUKS header or inside the home directory) will then catch + * invalid passwords. The second/third authentication always runs in strict verification mode. */ + log_debug("No hashed passwords and no PKCS#11 tokens defined in record, cannot authenticate user record. " + "Deferring to embedded user record."); + return 0; } int home_setup_undo(HomeSetup *setup) { @@ -402,9 +414,10 @@ int home_load_embedded_identity( return log_error_errno(SYNTHETIC_ERRNO(EREMCHG), "Embedded home record not compatible with host record, refusing."); /* Insist that credentials the user supplies also unlocks any embedded records. */ - r = user_record_authenticate(embedded_home, h, pkcs11_decrypted_passwords); + r = user_record_authenticate(embedded_home, h, pkcs11_decrypted_passwords, /* strict_verify= */ true); if (r < 0) return r; + assert(r > 0); /* Insist that a password was verified */ /* At this point we have three records to deal with: * @@ -615,7 +628,7 @@ static int home_activate(UserRecord *h, UserRecord **ret_home) { if (!IN_SET(user_record_storage(h), USER_LUKS, USER_DIRECTORY, USER_SUBVOLUME, USER_FSCRYPT, USER_CIFS)) return log_error_errno(SYNTHETIC_ERRNO(ENOTTY), "Activating home directories of type '%s' currently not supported.", user_storage_to_string(user_record_storage(h))); - r = user_record_authenticate(h, h, &pkcs11_decrypted_passwords); + r = user_record_authenticate(h, h, &pkcs11_decrypted_passwords, /* strict_verify= */ false); if (r < 0) return r; @@ -1177,9 +1190,10 @@ static int home_update(UserRecord *h, UserRecord **ret) { assert(h); assert(ret); - r = user_record_authenticate(h, h, &pkcs11_decrypted_passwords); + r = user_record_authenticate(h, h, &pkcs11_decrypted_passwords, /* strict_verify= */ true); if (r < 0) return r; + assert(r > 0); /* Insist that a password was verified */ r = home_validate_update(h, &setup); if (r < 0) @@ -1233,9 +1247,10 @@ static int home_resize(UserRecord *h, UserRecord **ret) { if (h->disk_size == UINT64_MAX) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "No target size specified, refusing."); - r = user_record_authenticate(h, h, &pkcs11_decrypted_passwords); + r = user_record_authenticate(h, h, &pkcs11_decrypted_passwords, /* strict_verify= */ true); if (r < 0) return r; + assert(r > 0); /* Insist that a password was verified */ r = home_validate_update(h, &setup); if (r < 0) @@ -1343,7 +1358,7 @@ static int home_inspect(UserRecord *h, UserRecord **ret_home) { assert(h); assert(ret_home); - r = user_record_authenticate(h, h, &pkcs11_decrypted_passwords); + r = user_record_authenticate(h, h, &pkcs11_decrypted_passwords, /* strict_verify= */ false); if (r < 0) return r; @@ -1413,7 +1428,7 @@ static int home_unlock(UserRecord *h) { /* Note that we don't check if $HOME is actually mounted, since we want to avoid disk accesses on * that mount until we have resumed the device. */ - r = user_record_authenticate(h, h, &pkcs11_decrypted_passwords); + r = user_record_authenticate(h, h, &pkcs11_decrypted_passwords, /* strict_verify= */ false); if (r < 0) return r; diff --git a/src/home/homework.h b/src/home/homework.h index 3bcf3ad9b0..46641172a4 100644 --- a/src/home/homework.h +++ b/src/home/homework.h @@ -56,6 +56,6 @@ int home_load_embedded_identity(UserRecord *h, int root_fd, UserRecord *header_h int home_store_embedded_identity(UserRecord *h, int root_fd, uid_t uid, UserRecord *old_home); int home_extend_embedded_identity(UserRecord *h, UserRecord *used, HomeSetup *setup); -int user_record_authenticate(UserRecord *h, UserRecord *secret, char ***pkcs11_decrypted_passwords); +int user_record_authenticate(UserRecord *h, UserRecord *secret, char ***pkcs11_decrypted_passwords, bool strict_verify); int home_sync_and_statfs(int root_fd, struct statfs *ret);