From bc5890c671ce243da2e69e287cc6ff75758f5460 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Oct 2021 15:03:34 +0200 Subject: [PATCH 01/16] homework: use HomeSetup in home_create_luks() too We use it for all other LUKS operations these days, and for all home_create_xyz() calls for other backends, let's use it for the LUKS backend too. --- src/home/homework-luks.c | 18 ++++++++++-------- src/home/homework-luks.h | 2 +- src/home/homework.c | 2 +- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index e09d40f66b6..641acaeb321 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -1992,6 +1992,7 @@ static int home_truncate( int home_create_luks( UserRecord *h, + HomeSetup *setup, const PasswordCache *cache, char **effective_passwords, UserRecord **ret_home) { @@ -2004,13 +2005,14 @@ int home_create_luks( sd_id128_t partition_uuid, fs_uuid, luks_uuid, disk_uuid; _cleanup_(loop_device_unrefp) LoopDevice *loop = NULL; _cleanup_(sym_crypt_freep) struct crypt_device *cd = NULL; - _cleanup_close_ int image_fd = -1, root_fd = -1; + _cleanup_close_ int image_fd = -1; const char *fstype, *ip; struct statfs sfs; int r; assert(h); assert(h->storage < 0 || h->storage == USER_LUKS); + assert(setup); assert(ret_home); r = dlopen_cryptsetup(); @@ -2256,17 +2258,17 @@ int home_create_luks( goto fail; } - root_fd = open(subdir, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); - if (root_fd < 0) { + setup->root_fd = open(subdir, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); + if (setup->root_fd < 0) { r = log_error_errno(errno, "Failed to open user directory in mounted image file: %m"); goto fail; } - r = home_populate(h, root_fd); + r = home_populate(h, setup->root_fd); if (r < 0) goto fail; - r = home_sync_and_statfs(root_fd, &sfs); + r = home_sync_and_statfs(setup->root_fd, &sfs); if (r < 0) goto fail; @@ -2296,12 +2298,12 @@ int home_create_luks( } if (user_record_luks_offline_discard(h)) { - r = run_fitrim(root_fd); + r = run_fitrim(setup->root_fd); if (r < 0) goto fail; } - root_fd = safe_close(root_fd); + setup->root_fd = safe_close(setup->root_fd); r = umount_verbose(LOG_ERR, HOME_RUNTIME_WORK_DIR, UMOUNT_NOFOLLOW); if (r < 0) @@ -2370,7 +2372,7 @@ int home_create_luks( fail: /* Let's close all files before we unmount the file system, to avoid EBUSY */ - root_fd = safe_close(root_fd); + setup->root_fd = safe_close(setup->root_fd); if (mounted) (void) umount_verbose(LOG_WARNING, HOME_RUNTIME_WORK_DIR, UMOUNT_NOFOLLOW); diff --git a/src/home/homework-luks.h b/src/home/homework-luks.h index f8d22bb647a..1896e3ce6c4 100644 --- a/src/home/homework-luks.h +++ b/src/home/homework-luks.h @@ -13,7 +13,7 @@ int home_trim_luks(UserRecord *h); int home_store_header_identity_luks(UserRecord *h, HomeSetup *setup, UserRecord *old_home); -int home_create_luks(UserRecord *h, const PasswordCache *cache, char **effective_passwords, UserRecord **ret_home); +int home_create_luks(UserRecord *h, HomeSetup *setup, const PasswordCache *cache, char **effective_passwords, UserRecord **ret_home); int home_get_state_luks(UserRecord *h, HomeSetup *setup); diff --git a/src/home/homework.c b/src/home/homework.c index 6b60fddc2cf..170f7f26fb8 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -1279,7 +1279,7 @@ static int home_create(UserRecord *h, UserRecord **ret_home) { switch (user_record_storage(h)) { case USER_LUKS: - r = home_create_luks(h, &cache, effective_passwords, &new_home); + r = home_create_luks(h, &setup, &cache, effective_passwords, &new_home); break; case USER_DIRECTORY: From 203f06aa1e6d4b5530ae8eaaa307652285601769 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Oct 2021 15:10:54 +0200 Subject: [PATCH 02/16] homework: make use of .undo_mount field of HomeSetup for LUKS backend too Let's move more stuff to common infra for the backends. --- src/home/homework-luks.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index 641acaeb321..b900855bcd6 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -1151,7 +1151,7 @@ int home_setup_luks( _cleanup_(sym_crypt_freep) struct crypt_device *cd = NULL; _cleanup_(erase_and_freep) void *volume_key = NULL; _cleanup_close_ int opened_image_fd = -1, root_fd = -1; - bool dm_activated = false, mounted = false; + bool dm_activated = false; size_t volume_key_size = 0; bool marked_dirty = false; uint64_t offset, size; @@ -1332,7 +1332,7 @@ int home_setup_luks( if (r < 0) goto fail; - mounted = true; + setup->undo_mount = true; root_fd = open(subdir, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); if (root_fd < 0) { @@ -1364,7 +1364,6 @@ int home_setup_luks( setup->volume_key = TAKE_PTR(volume_key); setup->volume_key_size = volume_key_size; - setup->undo_mount = mounted; setup->undo_dm = dm_activated; if (ret_luks_home) @@ -1373,8 +1372,7 @@ int home_setup_luks( return 0; fail: - if (mounted) - (void) umount_verbose(LOG_ERR, HOME_RUNTIME_WORK_DIR, UMOUNT_NOFOLLOW); + home_setup_undo_mount(setup, LOG_ERR); if (dm_activated) (void) sym_crypt_deactivate_by_name(cd, setup->dm_name, 0); @@ -2000,7 +1998,7 @@ int home_create_luks( _cleanup_free_ char *dm_name = NULL, *dm_node = NULL, *subdir = NULL, *disk_uuid_path = NULL, *temporary_image_path = NULL; uint64_t encrypted_size, host_size = 0, partition_offset = 0, partition_size = 0; /* Unnecessary initialization to appease gcc */ - bool image_created = false, dm_activated = false, mounted = false; + bool image_created = false, dm_activated = false; _cleanup_(user_record_unrefp) UserRecord *new_home = NULL; sd_id128_t partition_uuid, fs_uuid, luks_uuid, disk_uuid; _cleanup_(loop_device_unrefp) LoopDevice *loop = NULL; @@ -2243,7 +2241,7 @@ int home_create_luks( if (r < 0) goto fail; - mounted = true; + setup->undo_mount = true; subdir = path_join(HOME_RUNTIME_WORK_DIR, user_record_user_name_and_realm(h)); if (!subdir) { @@ -2305,12 +2303,10 @@ int home_create_luks( setup->root_fd = safe_close(setup->root_fd); - r = umount_verbose(LOG_ERR, HOME_RUNTIME_WORK_DIR, UMOUNT_NOFOLLOW); + r = home_setup_undo_mount(setup, LOG_ERR); if (r < 0) goto fail; - mounted = false; - r = sym_crypt_deactivate_by_name(cd, dm_name, 0); if (r < 0) { log_error_errno(r, "Failed to deactivate LUKS device: %m"); @@ -2373,9 +2369,7 @@ int home_create_luks( fail: /* Let's close all files before we unmount the file system, to avoid EBUSY */ setup->root_fd = safe_close(setup->root_fd); - - if (mounted) - (void) umount_verbose(LOG_WARNING, HOME_RUNTIME_WORK_DIR, UMOUNT_NOFOLLOW); + (void) home_setup_undo_mount(setup, LOG_WARNING); if (dm_activated) (void) sym_crypt_deactivate_by_name(cd, dm_name, 0); @@ -2477,11 +2471,10 @@ static int ext4_offline_resize_fs(HomeSetup *setup, uint64_t new_size, bool disc } if (setup->undo_mount) { - r = umount_verbose(LOG_ERR, HOME_RUNTIME_WORK_DIR, UMOUNT_NOFOLLOW); + r = home_setup_undo_mount(setup, LOG_ERR); if (r < 0) return r; - setup->undo_mount = false; re_mount = true; } From fc032ae197b04c151558ec7234025a58ca1ef165 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Oct 2021 09:18:39 +0200 Subject: [PATCH 03/16] homework: teach home_lock() + home_unlock() + home_deactivate() to use HomeSetup, too This is just some minor refactoring, to make these two operations work like the rest. home_lock_luks() will now use the root_fd field of HomeSetup already, but for home_unlock_luks() + home_deactivate() this change has no effect for now. (But a later commit will change this.) --- src/home/homework-luks.c | 22 ++++++++++++++-------- src/home/homework-luks.h | 6 +++--- src/home/homework.c | 9 ++++++--- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index b900855bcd6..c1c67f7ca75 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -1483,12 +1483,16 @@ int home_activate_luks( return 1; } -int home_deactivate_luks(UserRecord *h) { +int home_deactivate_luks(UserRecord *h, HomeSetup *setup) { _cleanup_(sym_crypt_freep) struct crypt_device *cd = NULL; _cleanup_free_ char *dm_name = NULL, *dm_node = NULL; bool we_detached; int r; + assert(h); + assert(setup); + assert(!setup->crypt_device); + /* Note that the DM device and loopback device are set to auto-detach, hence strictly speaking we * don't have to explicitly have to detach them. However, we do that nonetheless (in case of the DM * device), to avoid races: by explicitly detaching them we know when the detaching is complete. We @@ -3147,18 +3151,19 @@ int home_passwd_luks( return 1; } -int home_lock_luks(UserRecord *h) { +int home_lock_luks(UserRecord *h, HomeSetup *setup) { _cleanup_(sym_crypt_freep) struct crypt_device *cd = NULL; _cleanup_free_ char *dm_name = NULL, *dm_node = NULL; - _cleanup_close_ int root_fd = -1; const char *p; int r; assert(h); + assert(setup); + assert(setup->root_fd < 0); assert_se(p = user_record_home_directory(h)); - root_fd = open(p, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); - if (root_fd < 0) + setup->root_fd = open(p, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); + if (setup->root_fd < 0) return log_error_errno(errno, "Failed to open home directory: %m"); r = make_dm_names(h->user_name, &dm_name, &dm_node); @@ -3176,10 +3181,10 @@ int home_lock_luks(UserRecord *h) { log_info("Discovered used LUKS device %s.", dm_node); cryptsetup_enable_logging(cd); - if (syncfs(root_fd) < 0) /* Snake oil, but let's better be safe than sorry */ + if (syncfs(setup->root_fd) < 0) /* Snake oil, but let's better be safe than sorry */ return log_error_errno(errno, "Failed to synchronize file system %s: %m", p); - root_fd = safe_close(root_fd); + setup->root_fd = safe_close(setup->root_fd); log_info("File system synchronized."); @@ -3222,13 +3227,14 @@ static int luks_try_resume( return -ENOKEY; } -int home_unlock_luks(UserRecord *h, const PasswordCache *cache) { +int home_unlock_luks(UserRecord *h, HomeSetup *setup, const PasswordCache *cache) { _cleanup_free_ char *dm_name = NULL, *dm_node = NULL; _cleanup_(sym_crypt_freep) struct crypt_device *cd = NULL; char **list; int r; assert(h); + assert(setup); r = make_dm_names(h->user_name, &dm_name, &dm_node); if (r < 0) diff --git a/src/home/homework-luks.h b/src/home/homework-luks.h index 1896e3ce6c4..25d096a175c 100644 --- a/src/home/homework-luks.h +++ b/src/home/homework-luks.h @@ -8,7 +8,7 @@ int home_setup_luks(UserRecord *h, HomeSetupFlags flags, const char *force_image_path, PasswordCache *cache, HomeSetup *setup, UserRecord **ret_luks_home); int home_activate_luks(UserRecord *h, HomeSetup *setup, PasswordCache *cache, UserRecord **ret_home); -int home_deactivate_luks(UserRecord *h); +int home_deactivate_luks(UserRecord *h, HomeSetup *setup); int home_trim_luks(UserRecord *h); int home_store_header_identity_luks(UserRecord *h, HomeSetup *setup, UserRecord *old_home); @@ -21,8 +21,8 @@ int home_resize_luks(UserRecord *h, HomeSetupFlags flags, PasswordCache *cache, int home_passwd_luks(UserRecord *h, HomeSetup *setup, const PasswordCache *cache, char **effective_passwords); -int home_lock_luks(UserRecord *h); -int home_unlock_luks(UserRecord *h, const PasswordCache *cache); +int home_lock_luks(UserRecord *h, HomeSetup *setup); +int home_unlock_luks(UserRecord *h, HomeSetup *setup, const PasswordCache *cache); static inline uint64_t luks_volume_key_size_convert(struct crypt_device *cd) { int k; diff --git a/src/home/homework.c b/src/home/homework.c index 170f7f26fb8..318ee801ddd 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -883,6 +883,7 @@ static int home_activate(UserRecord *h, UserRecord **ret_home) { } static int home_deactivate(UserRecord *h, bool force) { + _cleanup_(home_setup_done) HomeSetup setup = HOME_SETUP_INIT; bool done = false; int r; @@ -919,7 +920,7 @@ static int home_deactivate(UserRecord *h, bool force) { log_info("Directory %s is already unmounted.", user_record_home_directory(h)); if (user_record_storage(h) == USER_LUKS) { - r = home_deactivate_luks(h); + r = home_deactivate_luks(h, &setup); if (r < 0) return r; if (r > 0) @@ -1685,6 +1686,7 @@ static int home_inspect(UserRecord *h, UserRecord **ret_home) { } static int home_lock(UserRecord *h) { + _cleanup_(home_setup_done) HomeSetup setup = HOME_SETUP_INIT; int r; assert(h); @@ -1700,7 +1702,7 @@ static int home_lock(UserRecord *h) { if (r != USER_TEST_MOUNTED) return log_error_errno(SYNTHETIC_ERRNO(ENOEXEC), "Home directory of %s is not mounted, can't lock.", h->user_name); - r = home_lock_luks(h); + r = home_lock_luks(h, &setup); if (r < 0) return r; @@ -1709,6 +1711,7 @@ static int home_lock(UserRecord *h) { } static int home_unlock(UserRecord *h) { + _cleanup_(home_setup_done) HomeSetup setup = HOME_SETUP_INIT; _cleanup_(password_cache_free) PasswordCache cache = {}; int r; @@ -1726,7 +1729,7 @@ static int home_unlock(UserRecord *h) { if (r < 0) return r; - r = home_unlock_luks(h, &cache); + r = home_unlock_luks(h, &setup, &cache); if (r < 0) return r; From f7800049ebe83921180bca2a4ba80d0638348383 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 25 Oct 2021 21:33:08 +0200 Subject: [PATCH 04/16] homework: move all DM detachment/freeing into HomeSetup We actually already detach/free the LUKS DM devices for most operations via HomeSetup, let's move the creation logic to also do this, in order to unify behaviour between operations. --- src/home/homework-luks.c | 160 +++++++++++++++++---------------------- src/home/homework.c | 38 +++++++--- src/home/homework.h | 1 + 3 files changed, 98 insertions(+), 101 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index c1c67f7ca75..fd9baf84a79 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -509,23 +509,23 @@ static int fs_validate( return 0; } -static int make_dm_names(const char *user_name, char **ret_dm_name, char **ret_dm_node) { - _cleanup_free_ char *name = NULL, *node = NULL; +static int make_dm_names(UserRecord *h, HomeSetup *setup) { + assert(h); + assert(h->user_name); + assert(setup); - assert(user_name); - assert(ret_dm_name); - assert(ret_dm_node); + if (!setup->dm_name) { + setup->dm_name = strjoin("home-", h->user_name); + if (!setup->dm_name) + return log_oom(); + } - name = strjoin("home-", user_name); - if (!name) - return log_oom(); + if (!setup->dm_node) { + setup->dm_node = path_join("/dev/mapper/", setup->dm_name); + if (!setup->dm_node) + return log_oom(); + } - node = path_join("/dev/mapper/", name); - if (!node) - return log_oom(); - - *ret_dm_name = TAKE_PTR(name); - *ret_dm_node = TAKE_PTR(node); return 0; } @@ -1148,10 +1148,8 @@ int home_setup_luks( sd_id128_t found_partition_uuid, found_luks_uuid, found_fs_uuid; _cleanup_(user_record_unrefp) UserRecord *luks_home = NULL; _cleanup_(loop_device_unrefp) LoopDevice *loop = NULL; - _cleanup_(sym_crypt_freep) struct crypt_device *cd = NULL; _cleanup_(erase_and_freep) void *volume_key = NULL; _cleanup_close_ int opened_image_fd = -1, root_fd = -1; - bool dm_activated = false; size_t volume_key_size = 0; bool marked_dirty = false; uint64_t offset, size; @@ -1161,6 +1159,7 @@ int home_setup_luks( assert(setup); assert(setup->dm_name); assert(setup->dm_node); + assert(!setup->crypt_device); assert(user_record_storage(h) == USER_LUKS); @@ -1175,18 +1174,18 @@ int home_setup_luks( r = luks_open(setup->dm_name, h->password, cache, - &cd, + &setup->crypt_device, &found_luks_uuid, &volume_key, &volume_key_size); if (r < 0) return r; - r = luks_validate_home_record(cd, h, volume_key, cache, &luks_home); + r = luks_validate_home_record(setup->crypt_device, h, volume_key, cache, &luks_home); if (r < 0) return r; - n = sym_crypt_get_device_name(cd); + n = sym_crypt_get_device_name(setup->crypt_device); if (!n) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to determine backing device for DM %s.", setup->dm_name); @@ -1307,16 +1306,16 @@ int home_setup_luks( h->password, cache, user_record_luks_discard(h) || user_record_luks_offline_discard(h), - &cd, + &setup->crypt_device, &found_luks_uuid, &volume_key, &volume_key_size); if (r < 0) return r; - dm_activated = true; + setup->undo_dm = true; - r = luks_validate_home_record(cd, h, volume_key, cache, &luks_home); + r = luks_validate_home_record(setup->crypt_device, h, volume_key, cache, &luks_home); if (r < 0) goto fail; @@ -1354,7 +1353,6 @@ int home_setup_luks( } setup->loop = TAKE_PTR(loop); - setup->crypt_device = TAKE_PTR(cd); setup->root_fd = TAKE_FD(root_fd); setup->found_partition_uuid = found_partition_uuid; setup->found_luks_uuid = found_luks_uuid; @@ -1364,8 +1362,6 @@ int home_setup_luks( setup->volume_key = TAKE_PTR(volume_key); setup->volume_key_size = volume_key_size; - setup->undo_dm = dm_activated; - if (ret_luks_home) *ret_luks_home = TAKE_PTR(luks_home); @@ -1373,9 +1369,7 @@ int home_setup_luks( fail: home_setup_undo_mount(setup, LOG_ERR); - - if (dm_activated) - (void) sym_crypt_deactivate_by_name(cd, setup->dm_name, 0); + home_setup_undo_dm(setup, LOG_ERR); if (image_fd >= 0 && marked_dirty) (void) run_mark_dirty(image_fd, false); @@ -1484,8 +1478,6 @@ int home_activate_luks( } int home_deactivate_luks(UserRecord *h, HomeSetup *setup) { - _cleanup_(sym_crypt_freep) struct crypt_device *cd = NULL; - _cleanup_free_ char *dm_name = NULL, *dm_node = NULL; bool we_detached; int r; @@ -1503,27 +1495,27 @@ int home_deactivate_luks(UserRecord *h, HomeSetup *setup) { if (r < 0) return r; - r = make_dm_names(h->user_name, &dm_name, &dm_node); + r = make_dm_names(h, setup); if (r < 0) return r; - r = sym_crypt_init_by_name(&cd, dm_name); + r = sym_crypt_init_by_name(&setup->crypt_device, setup->dm_name); if (IN_SET(r, -ENODEV, -EINVAL, -ENOENT)) { - log_debug_errno(r, "LUKS device %s has already been detached.", dm_name); + log_debug_errno(r, "LUKS device %s has already been detached.", setup->dm_name); we_detached = false; } else if (r < 0) - return log_error_errno(r, "Failed to initialize cryptsetup context for %s: %m", dm_name); + return log_error_errno(r, "Failed to initialize cryptsetup context for %s: %m", setup->dm_name); else { - log_info("Discovered used LUKS device %s.", dm_node); + log_info("Discovered used LUKS device %s.", setup->dm_node); - cryptsetup_enable_logging(cd); + cryptsetup_enable_logging(setup->crypt_device); - r = sym_crypt_deactivate_by_name(cd, dm_name, 0); + r = sym_crypt_deactivate_by_name(setup->crypt_device, setup->dm_name, 0); if (IN_SET(r, -ENODEV, -EINVAL, -ENOENT)) { - log_debug_errno(r, "LUKS device %s is already detached.", dm_node); + log_debug_errno(r, "LUKS device %s is already detached.", setup->dm_node); we_detached = false; } else if (r < 0) - return log_info_errno(r, "LUKS device %s couldn't be deactivated: %m", dm_node); + return log_info_errno(r, "LUKS device %s couldn't be deactivated: %m", setup->dm_node); else { log_info("LUKS device detaching completed."); we_detached = true; @@ -1999,15 +1991,14 @@ int home_create_luks( char **effective_passwords, UserRecord **ret_home) { - _cleanup_free_ char *dm_name = NULL, *dm_node = NULL, *subdir = NULL, *disk_uuid_path = NULL, *temporary_image_path = NULL; + _cleanup_free_ char *subdir = NULL, *disk_uuid_path = NULL, *temporary_image_path = NULL; uint64_t encrypted_size, host_size = 0, partition_offset = 0, partition_size = 0; /* Unnecessary initialization to appease gcc */ - bool image_created = false, dm_activated = false; _cleanup_(user_record_unrefp) UserRecord *new_home = NULL; sd_id128_t partition_uuid, fs_uuid, luks_uuid, disk_uuid; _cleanup_(loop_device_unrefp) LoopDevice *loop = NULL; - _cleanup_(sym_crypt_freep) struct crypt_device *cd = NULL; _cleanup_close_ int image_fd = -1; + bool image_created = false; const char *fstype, *ip; struct statfs sfs; int r; @@ -2065,16 +2056,16 @@ int home_create_luks( } else fs_uuid = h->file_system_uuid; - r = make_dm_names(h->user_name, &dm_name, &dm_node); + r = make_dm_names(h, setup); if (r < 0) return r; - r = access(dm_node, F_OK); + r = access(setup->dm_node, F_OK); if (r < 0) { if (errno != ENOENT) - return log_error_errno(errno, "Failed to determine whether %s exists: %m", dm_node); + return log_error_errno(errno, "Failed to determine whether %s exists: %m", setup->dm_node); } else - return log_error_errno(SYNTHETIC_ERRNO(EEXIST), "Device mapper device %s already exists, refusing.", dm_node); + return log_error_errno(SYNTHETIC_ERRNO(EEXIST), "Device mapper device %s already exists, refusing.", setup->dm_node); if (path_startswith(ip, "/dev/")) { _cleanup_free_ char *sysfs = NULL; @@ -2214,34 +2205,34 @@ int home_create_luks( log_info("Setting up loopback device %s completed.", loop->node ?: ip); r = luks_format(loop->node, - dm_name, + setup->dm_name, luks_uuid, user_record_user_name_and_realm(h), cache, effective_passwords, user_record_luks_discard(h) || user_record_luks_offline_discard(h), h, - &cd); + &setup->crypt_device); if (r < 0) goto fail; - dm_activated = true; + setup->undo_dm = true; - r = block_get_size_by_path(dm_node, &encrypted_size); + r = block_get_size_by_path(setup->dm_node, &encrypted_size); if (r < 0) { log_error_errno(r, "Failed to get encrypted block device size: %m"); goto fail; } - log_info("Setting up LUKS device %s completed.", dm_node); + log_info("Setting up LUKS device %s completed.", setup->dm_node); - r = make_filesystem(dm_node, fstype, user_record_user_name_and_realm(h), fs_uuid, user_record_luks_discard(h)); + r = make_filesystem(setup->dm_node, fstype, user_record_user_name_and_realm(h), fs_uuid, user_record_luks_discard(h)); if (r < 0) goto fail; log_info("Formatting file system completed."); - r = home_unshare_and_mount(dm_node, fstype, user_record_luks_discard(h), user_record_mount_flags(h)); + r = home_unshare_and_mount(setup->dm_node, fstype, user_record_luks_discard(h), user_record_mount_flags(h)); if (r < 0) goto fail; @@ -2287,9 +2278,9 @@ int home_create_luks( partition_uuid, luks_uuid, fs_uuid, - sym_crypt_get_cipher(cd), - sym_crypt_get_cipher_mode(cd), - luks_volume_key_size_convert(cd), + sym_crypt_get_cipher(setup->crypt_device), + sym_crypt_get_cipher_mode(setup->crypt_device), + luks_volume_key_size_convert(setup->crypt_device), fstype, NULL, h->uid, @@ -2311,16 +2302,9 @@ int home_create_luks( if (r < 0) goto fail; - r = sym_crypt_deactivate_by_name(cd, dm_name, 0); - if (r < 0) { - log_error_errno(r, "Failed to deactivate LUKS device: %m"); + r = home_setup_undo_dm(setup, LOG_ERR); + if (r < 0) goto fail; - } - - sym_crypt_free(cd); - cd = NULL; - - dm_activated = false; loop = loop_device_unref(loop); @@ -2374,9 +2358,7 @@ fail: /* Let's close all files before we unmount the file system, to avoid EBUSY */ setup->root_fd = safe_close(setup->root_fd); (void) home_setup_undo_mount(setup, LOG_WARNING); - - if (dm_activated) - (void) sym_crypt_deactivate_by_name(cd, dm_name, 0); + (void) home_setup_undo_dm(setup, LOG_WARNING); loop = loop_device_unref(loop); @@ -2387,22 +2369,18 @@ fail: } int home_get_state_luks(UserRecord *h, HomeSetup *setup) { - _cleanup_free_ char *dm_name = NULL, *dm_node = NULL; int r; assert(h); assert(setup); - r = make_dm_names(h->user_name, &dm_name, &dm_node); + r = make_dm_names(h, setup); if (r < 0) return r; - r = access(dm_node, F_OK); + r = access(setup->dm_node, F_OK); if (r < 0 && errno != ENOENT) - return log_error_errno(errno, "Failed to determine whether %s exists: %m", dm_node); - - free_and_replace(setup->dm_name, dm_name); - free_and_replace(setup->dm_node, dm_node); + return log_error_errno(errno, "Failed to determine whether %s exists: %m", setup->dm_node); return r >= 0; } @@ -3152,21 +3130,20 @@ int home_passwd_luks( } int home_lock_luks(UserRecord *h, HomeSetup *setup) { - _cleanup_(sym_crypt_freep) struct crypt_device *cd = NULL; - _cleanup_free_ char *dm_name = NULL, *dm_node = NULL; const char *p; int r; assert(h); assert(setup); assert(setup->root_fd < 0); + assert(!setup->crypt_device); assert_se(p = user_record_home_directory(h)); setup->root_fd = open(p, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); if (setup->root_fd < 0) return log_error_errno(errno, "Failed to open home directory: %m"); - r = make_dm_names(h->user_name, &dm_name, &dm_node); + r = make_dm_names(h, setup); if (r < 0) return r; @@ -3174,12 +3151,12 @@ int home_lock_luks(UserRecord *h, HomeSetup *setup) { if (r < 0) return r; - r = sym_crypt_init_by_name(&cd, dm_name); + r = sym_crypt_init_by_name(&setup->crypt_device, setup->dm_name); if (r < 0) - return log_error_errno(r, "Failed to initialize cryptsetup context for %s: %m", dm_name); + return log_error_errno(r, "Failed to initialize cryptsetup context for %s: %m", setup->dm_name); - log_info("Discovered used LUKS device %s.", dm_node); - cryptsetup_enable_logging(cd); + log_info("Discovered used LUKS device %s.", setup->dm_node); + cryptsetup_enable_logging(setup->crypt_device); if (syncfs(setup->root_fd) < 0) /* Snake oil, but let's better be safe than sorry */ return log_error_errno(errno, "Failed to synchronize file system %s: %m", p); @@ -3190,9 +3167,9 @@ int home_lock_luks(UserRecord *h, HomeSetup *setup) { /* Note that we don't invoke FIFREEZE here, it appears libcryptsetup/device-mapper already does that on its own for us */ - r = sym_crypt_suspend(cd, dm_name); + r = sym_crypt_suspend(setup->crypt_device, setup->dm_name); if (r < 0) - return log_error_errno(r, "Failed to suspend cryptsetup device: %s: %m", dm_node); + return log_error_errno(r, "Failed to suspend cryptsetup device: %s: %m", setup->dm_node); log_info("LUKS device suspended."); return 0; @@ -3228,15 +3205,14 @@ static int luks_try_resume( } int home_unlock_luks(UserRecord *h, HomeSetup *setup, const PasswordCache *cache) { - _cleanup_free_ char *dm_name = NULL, *dm_node = NULL; - _cleanup_(sym_crypt_freep) struct crypt_device *cd = NULL; char **list; int r; assert(h); assert(setup); + assert(!setup->crypt_device); - r = make_dm_names(h->user_name, &dm_name, &dm_node); + r = make_dm_names(h, setup); if (r < 0) return r; @@ -3244,19 +3220,19 @@ int home_unlock_luks(UserRecord *h, HomeSetup *setup, const PasswordCache *cache if (r < 0) return r; - r = sym_crypt_init_by_name(&cd, dm_name); + r = sym_crypt_init_by_name(&setup->crypt_device, setup->dm_name); if (r < 0) - return log_error_errno(r, "Failed to initialize cryptsetup context for %s: %m", dm_name); + return log_error_errno(r, "Failed to initialize cryptsetup context for %s: %m", setup->dm_name); - log_info("Discovered used LUKS device %s.", dm_node); - cryptsetup_enable_logging(cd); + log_info("Discovered used LUKS device %s.", setup->dm_node); + cryptsetup_enable_logging(setup->crypt_device); r = -ENOKEY; FOREACH_POINTER(list, cache ? cache->pkcs11_passwords : NULL, cache ? cache->fido2_passwords : NULL, h->password) { - r = luks_try_resume(cd, dm_name, list); + r = luks_try_resume(setup->crypt_device, setup->dm_name, list); if (r != -ENOKEY) break; } diff --git a/src/home/homework.c b/src/home/homework.c index 318ee801ddd..8634a932304 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -314,6 +314,32 @@ int home_setup_undo_mount(HomeSetup *setup, int level) { return 1; } +int home_setup_undo_dm(HomeSetup *setup, int level) { + int r, ret; + + assert(setup); + + if (setup->undo_dm) { + assert(setup->crypt_device); + assert(setup->dm_name); + + r = sym_crypt_deactivate_by_name(setup->crypt_device, setup->dm_name, 0); + if (r < 0) + return log_full_errno(level, r, "Failed to deactivate LUKS device: %m"); + + setup->undo_dm = false; + ret = 1; + } else + ret = 0; + + if (setup->crypt_device) { + sym_crypt_free(setup->crypt_device); + setup->crypt_device = NULL; + } + + return ret; +} + int home_setup_done(HomeSetup *setup) { int r = 0, q; @@ -336,11 +362,9 @@ int home_setup_done(HomeSetup *setup) { if (q < 0) r = q; - if (setup->undo_dm && setup->crypt_device && setup->dm_name) { - q = sym_crypt_deactivate_by_name(setup->crypt_device, setup->dm_name, 0); - if (q < 0) - r = q; - } + q = home_setup_undo_dm(setup, LOG_DEBUG); + if (q < 0) + r = q; if (setup->image_fd >= 0) { if (setup->do_offline_fallocate) { @@ -368,10 +392,6 @@ int home_setup_done(HomeSetup *setup) { setup->dm_node = mfree(setup->dm_node); setup->loop = loop_device_unref(setup->loop); - if (setup->crypt_device) { - sym_crypt_free(setup->crypt_device); - setup->crypt_device = NULL; - } setup->volume_key = erase_and_free(setup->volume_key); setup->volume_key_size = 0; diff --git a/src/home/homework.h b/src/home/homework.h index 1b56fbbd8f1..fe8cbb46358 100644 --- a/src/home/homework.h +++ b/src/home/homework.h @@ -76,6 +76,7 @@ typedef enum HomeSetupFlags { int home_setup_done(HomeSetup *setup); int home_setup_undo_mount(HomeSetup *setup, int level); +int home_setup_undo_dm(HomeSetup *setup, int level); int home_setup(UserRecord *h, HomeSetupFlags flags, PasswordCache *cache, HomeSetup *setup, UserRecord **ret_header_home); From e4d1e79bc56e08921a99604610eeb82a7df325f1 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Oct 2021 10:52:21 +0200 Subject: [PATCH 05/16] homework: move all LoopDevice handling into HomeSetup too Similar story as with the DM objects: let's maintain it all as part of HomeSetup. We do that for part of the operations already, let's unify that. --- src/home/homework-luks.c | 32 +++++++++++++++----------------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index fd9baf84a79..8e0ddd77390 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -1147,7 +1147,6 @@ int home_setup_luks( sd_id128_t found_partition_uuid, found_luks_uuid, found_fs_uuid; _cleanup_(user_record_unrefp) UserRecord *luks_home = NULL; - _cleanup_(loop_device_unrefp) LoopDevice *loop = NULL; _cleanup_(erase_and_freep) void *volume_key = NULL; _cleanup_close_ int opened_image_fd = -1, root_fd = -1; size_t volume_key_size = 0; @@ -1160,6 +1159,7 @@ int home_setup_luks( assert(setup->dm_name); assert(setup->dm_node); assert(!setup->crypt_device); + assert(!setup->loop); assert(user_record_storage(h) == USER_LUKS); @@ -1189,21 +1189,21 @@ int home_setup_luks( if (!n) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to determine backing device for DM %s.", setup->dm_name); - r = loop_device_open(n, O_RDWR, &loop); + r = loop_device_open(n, O_RDWR, &setup->loop); if (r < 0) return log_error_errno(r, "Failed to open loopback device %s: %m", n); - if (ioctl(loop->fd, LOOP_GET_STATUS64, &info) < 0) { + if (ioctl(setup->loop->fd, LOOP_GET_STATUS64, &info) < 0) { _cleanup_free_ char *sysfs = NULL; struct stat st; if (!IN_SET(errno, ENOTTY, EINVAL)) return log_error_errno(errno, "Failed to get block device metrics of %s: %m", n); - if (ioctl(loop->fd, BLKGETSIZE64, &size) < 0) + if (ioctl(setup->loop->fd, BLKGETSIZE64, &size) < 0) return log_error_errno(r, "Failed to read block device size of %s: %m", n); - if (fstat(loop->fd, &st) < 0) + if (fstat(setup->loop->fd, &st) < 0) return log_error_errno(r, "Failed to stat block device %s: %m", n); assert(S_ISBLK(st.st_mode)); @@ -1245,7 +1245,7 @@ int home_setup_luks( found_partition_uuid = found_fs_uuid = SD_ID128_NULL; - log_info("Discovered used loopback device %s.", loop->node); + log_info("Discovered used loopback device %s.", setup->loop->node); root_fd = open(user_record_home_directory(h), O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); if (root_fd < 0) { @@ -1287,7 +1287,7 @@ int home_setup_luks( return r; } - r = loop_device_make(image_fd, O_RDWR, offset, size, 0, &loop); + r = loop_device_make(image_fd, O_RDWR, offset, size, 0, &setup->loop); if (r == -ENOENT) { log_error_errno(r, "Loopback block device support is not available on this system."); return -ENOLINK; /* make recognizable */ @@ -1295,9 +1295,9 @@ int home_setup_luks( if (r < 0) return log_error_errno(r, "Failed to allocate loopback context: %m"); - log_info("Setting up loopback device %s completed.", loop->node ?: ip); + log_info("Setting up loopback device %s completed.", setup->loop->node ?: ip); - r = luks_setup(loop->node ?: ip, + r = luks_setup(setup->loop->node ?: ip, setup->dm_name, h->luks_uuid, h->luks_cipher, @@ -1352,7 +1352,6 @@ int home_setup_luks( setup->do_mark_clean = marked_dirty; } - setup->loop = TAKE_PTR(loop); setup->root_fd = TAKE_FD(root_fd); setup->found_partition_uuid = found_partition_uuid; setup->found_luks_uuid = found_luks_uuid; @@ -1996,7 +1995,6 @@ int home_create_luks( host_size = 0, partition_offset = 0, partition_size = 0; /* Unnecessary initialization to appease gcc */ _cleanup_(user_record_unrefp) UserRecord *new_home = NULL; sd_id128_t partition_uuid, fs_uuid, luks_uuid, disk_uuid; - _cleanup_(loop_device_unrefp) LoopDevice *loop = NULL; _cleanup_close_ int image_fd = -1; bool image_created = false; const char *fstype, *ip; @@ -2182,7 +2180,7 @@ int home_create_luks( log_info("Writing of partition table completed."); - r = loop_device_make(image_fd, O_RDWR, partition_offset, partition_size, 0, &loop); + r = loop_device_make(image_fd, O_RDWR, partition_offset, partition_size, 0, &setup->loop); if (r < 0) { if (r == -ENOENT) { /* this means /dev/loop-control doesn't exist, i.e. we are in a container * or similar and loopback bock devices are not available, return a @@ -2196,15 +2194,15 @@ int home_create_luks( goto fail; } - r = loop_device_flock(loop, LOCK_EX); /* make sure udev won't read before we are done */ + r = loop_device_flock(setup->loop, LOCK_EX); /* make sure udev won't read before we are done */ if (r < 0) { log_error_errno(r, "Failed to take lock on loop device: %m"); goto fail; } - log_info("Setting up loopback device %s completed.", loop->node ?: ip); + log_info("Setting up loopback device %s completed.", setup->loop->node ?: ip); - r = luks_format(loop->node, + r = luks_format(setup->loop->node, setup->dm_name, luks_uuid, user_record_user_name_and_realm(h), @@ -2306,7 +2304,7 @@ int home_create_luks( if (r < 0) goto fail; - loop = loop_device_unref(loop); + setup->loop = loop_device_unref(setup->loop); if (!user_record_luks_offline_discard(h)) { r = run_fallocate(image_fd, NULL /* refresh stat() data */); @@ -2360,7 +2358,7 @@ fail: (void) home_setup_undo_mount(setup, LOG_WARNING); (void) home_setup_undo_dm(setup, LOG_WARNING); - loop = loop_device_unref(loop); + setup->loop = loop_device_unref(setup->loop); if (image_created) (void) unlink(temporary_image_path); From a23cf7f4c767cd2825ea2384f3d9faea85e2b316 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Oct 2021 11:07:04 +0200 Subject: [PATCH 06/16] homework: unify similar code for opening existing LUKS DM devices Let's introduce a new helper acquire_open_luks_device() that combines a few steps we keep doing at three places into one. --- src/home/homework-luks.c | 161 ++++++++++++++++++++------------------- 1 file changed, 81 insertions(+), 80 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index 8e0ddd77390..f30acb0c1cf 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -384,39 +384,88 @@ static int luks_setup( return 0; } +static int make_dm_names(UserRecord *h, HomeSetup *setup) { + assert(h); + assert(h->user_name); + assert(setup); + + if (!setup->dm_name) { + setup->dm_name = strjoin("home-", h->user_name); + if (!setup->dm_name) + return log_oom(); + } + + if (!setup->dm_node) { + setup->dm_node = path_join("/dev/mapper/", setup->dm_name); + if (!setup->dm_node) + return log_oom(); + } + + return 0; +} + +static int acquire_open_luks_device( + UserRecord *h, + HomeSetup *setup, + bool graceful) { + + _cleanup_(sym_crypt_freep) struct crypt_device *cd = NULL; + int r; + + assert(h); + assert(setup); + assert(!setup->crypt_device); + + r = dlopen_cryptsetup(); + if (r < 0) + return r; + + r = make_dm_names(h, setup); + if (r < 0) + return r; + + r = sym_crypt_init_by_name(&cd, setup->dm_name); + if (IN_SET(r, -ENODEV, -EINVAL, -ENOENT) && graceful) + return 0; + if (r < 0) + return log_error_errno(r, "Failed to initialize cryptsetup context for %s: %m", setup->dm_name); + + cryptsetup_enable_logging(cd); + + setup->crypt_device = TAKE_PTR(cd); + return 1; +} + static int luks_open( - const char *dm_name, - char **passwords, + UserRecord *h, + HomeSetup *setup, const PasswordCache *cache, - struct crypt_device **ret, sd_id128_t *ret_found_uuid, void **ret_volume_key, size_t *ret_volume_key_size) { - _cleanup_(sym_crypt_freep) struct crypt_device *cd = NULL; _cleanup_(erase_and_freep) void *vk = NULL; sd_id128_t p; char **list; size_t vks; int r; - assert(dm_name); - assert(ret); + assert(h); + assert(setup); + assert(!setup->crypt_device); /* Opens a LUKS device that is already set up. Re-validates the password while doing so (which also * provides us with the volume key, which we want). */ - r = sym_crypt_init_by_name(&cd, dm_name); + r = acquire_open_luks_device(h, setup, /* graceful= */ false); if (r < 0) - return log_error_errno(r, "Failed to initialize cryptsetup context for %s: %m", dm_name); + return r; - cryptsetup_enable_logging(cd); - - r = sym_crypt_load(cd, CRYPT_LUKS2, NULL); + r = sym_crypt_load(setup->crypt_device, CRYPT_LUKS2, NULL); if (r < 0) return log_error_errno(r, "Failed to load LUKS superblock: %m"); - r = sym_crypt_get_volume_key_size(cd); + 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; @@ -424,7 +473,7 @@ static int luks_open( if (ret_found_uuid) { const char *s; - s = sym_crypt_get_uuid(cd); + s = sym_crypt_get_uuid(setup->crypt_device); if (!s) return log_error_errno(SYNTHETIC_ERRNO(EMEDIUMTYPE), "LUKS superblock has no UUID."); @@ -441,8 +490,8 @@ static int luks_open( FOREACH_POINTER(list, cache ? cache->pkcs11_passwords : NULL, cache ? cache->fido2_passwords : NULL, - passwords) { - r = luks_try_passwords(cd, list, vk, &vks); + h->password) { + r = luks_try_passwords(setup->crypt_device, list, vk, &vks); if (r != -ENOKEY) break; } @@ -451,18 +500,16 @@ static int luks_open( if (r < 0) return log_error_errno(r, "Failed to unlocks LUKS superblock: %m"); - log_info("Discovered used LUKS device /dev/mapper/%s, and validated password.", dm_name); + log_info("Discovered used LUKS device /dev/mapper/%s, and validated password.", setup->dm_name); /* This is needed so that crypt_resize() can operate correctly for pre-existing LUKS devices. We need * to tell libcryptsetup the volume key explicitly, so that it is in the kernel keyring. */ - r = sym_crypt_activate_by_volume_key(cd, NULL, vk, vks, CRYPT_ACTIVATE_KEYRING_KEY); + r = sym_crypt_activate_by_volume_key(setup->crypt_device, NULL, vk, vks, CRYPT_ACTIVATE_KEYRING_KEY); if (r < 0) return log_error_errno(r, "Failed to upload volume key again: %m"); log_info("Successfully re-activated LUKS device."); - *ret = TAKE_PTR(cd); - if (ret_found_uuid) *ret_found_uuid = p; if (ret_volume_key) @@ -509,26 +556,6 @@ static int fs_validate( return 0; } -static int make_dm_names(UserRecord *h, HomeSetup *setup) { - assert(h); - assert(h->user_name); - assert(setup); - - if (!setup->dm_name) { - setup->dm_name = strjoin("home-", h->user_name); - if (!setup->dm_name) - return log_oom(); - } - - if (!setup->dm_node) { - setup->dm_node = path_join("/dev/mapper/", setup->dm_name); - if (!setup->dm_node) - return log_oom(); - } - - return 0; -} - static int luks_validate( int fd, const char *label, @@ -1171,10 +1198,9 @@ int home_setup_luks( struct loop_info64 info; const char *n; - r = luks_open(setup->dm_name, - h->password, + r = luks_open(h, + setup, cache, - &setup->crypt_device, &found_luks_uuid, &volume_key, &volume_key_size); @@ -1490,21 +1516,13 @@ int home_deactivate_luks(UserRecord *h, HomeSetup *setup) { * don't bother about the loopback device because unlike the DM device it doesn't have a fixed * name. */ - r = dlopen_cryptsetup(); + r = acquire_open_luks_device(h, setup, /* graceful= */ true); if (r < 0) - return r; - - r = make_dm_names(h, setup); - if (r < 0) - return r; - - r = sym_crypt_init_by_name(&setup->crypt_device, setup->dm_name); - if (IN_SET(r, -ENODEV, -EINVAL, -ENOENT)) { - log_debug_errno(r, "LUKS device %s has already been detached.", setup->dm_name); - we_detached = false; - } else if (r < 0) return log_error_errno(r, "Failed to initialize cryptsetup context for %s: %m", setup->dm_name); - else { + if (r == 0) { + log_debug("LUKS device %s has already been detached.", setup->dm_name); + we_detached = false; + } else { log_info("Discovered used LUKS device %s.", setup->dm_node); cryptsetup_enable_logging(setup->crypt_device); @@ -3136,26 +3154,18 @@ int home_lock_luks(UserRecord *h, HomeSetup *setup) { assert(setup->root_fd < 0); assert(!setup->crypt_device); + r = acquire_open_luks_device(h, setup, /* graceful= */ false); + if (r < 0) + return r; + + log_info("Discovered used LUKS device %s.", setup->dm_node); + assert_se(p = user_record_home_directory(h)); + setup->root_fd = open(p, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); if (setup->root_fd < 0) return log_error_errno(errno, "Failed to open home directory: %m"); - r = make_dm_names(h, setup); - if (r < 0) - return r; - - r = dlopen_cryptsetup(); - if (r < 0) - return r; - - r = sym_crypt_init_by_name(&setup->crypt_device, setup->dm_name); - if (r < 0) - return log_error_errno(r, "Failed to initialize cryptsetup context for %s: %m", setup->dm_name); - - log_info("Discovered used LUKS device %s.", setup->dm_node); - cryptsetup_enable_logging(setup->crypt_device); - if (syncfs(setup->root_fd) < 0) /* Snake oil, but let's better be safe than sorry */ return log_error_errno(errno, "Failed to synchronize file system %s: %m", p); @@ -3210,20 +3220,11 @@ int home_unlock_luks(UserRecord *h, HomeSetup *setup, const PasswordCache *cache assert(setup); assert(!setup->crypt_device); - r = make_dm_names(h, setup); + r = acquire_open_luks_device(h, setup, /* graceful= */ false); if (r < 0) return r; - r = dlopen_cryptsetup(); - if (r < 0) - return r; - - r = sym_crypt_init_by_name(&setup->crypt_device, setup->dm_name); - if (r < 0) - return log_error_errno(r, "Failed to initialize cryptsetup context for %s: %m", setup->dm_name); - log_info("Discovered used LUKS device %s.", setup->dm_node); - cryptsetup_enable_logging(setup->crypt_device); r = -ENOKEY; FOREACH_POINTER(list, From a70e0ad764b5e797e40455c3bbeffacff58ae7df Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Oct 2021 13:40:16 +0200 Subject: [PATCH 07/16] homework: replace homegrown syncfs_path() reimplementation by syncfs_path() --- src/home/homework-luks.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index f30acb0c1cf..56e232523d6 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -3161,15 +3161,9 @@ int home_lock_luks(UserRecord *h, HomeSetup *setup) { log_info("Discovered used LUKS device %s.", setup->dm_node); assert_se(p = user_record_home_directory(h)); - - setup->root_fd = open(p, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); - if (setup->root_fd < 0) - return log_error_errno(errno, "Failed to open home directory: %m"); - - if (syncfs(setup->root_fd) < 0) /* Snake oil, but let's better be safe than sorry */ - return log_error_errno(errno, "Failed to synchronize file system %s: %m", p); - - setup->root_fd = safe_close(setup->root_fd); + r = syncfs_path(AT_FDCWD, p); + if (r < 0) /* Snake oil, but let's better be safe than sorry */ + return log_error_errno(r, "Failed to synchronize file system %s: %m", p); log_info("File system synchronized."); From c00b2ddc6e5995421d75740d8d6ec0ae10d92ee9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Oct 2021 17:16:13 +0200 Subject: [PATCH 08/16] homework: always pass HomeSetup param first, PasswordCache second Some of our operations did it one way, most the other. Let's unify on the order that is more popular. --- src/home/homework-cifs.c | 2 +- src/home/homework-directory.c | 6 +++--- src/home/homework-directory.h | 2 +- src/home/homework-fscrypt.c | 4 ++-- src/home/homework-fscrypt.h | 2 +- src/home/homework-luks.c | 8 ++++---- src/home/homework-luks.h | 4 ++-- src/home/homework.c | 16 ++++++++-------- src/home/homework.h | 2 +- 9 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/home/homework-cifs.c b/src/home/homework-cifs.c index 6a4431c2299..c76d6a6b13e 100644 --- a/src/home/homework-cifs.c +++ b/src/home/homework-cifs.c @@ -159,7 +159,7 @@ int home_activate_cifs( assert_se(hdo = user_record_home_directory(h)); hd = strdupa_safe(hdo); /* copy the string out, since it might change later in the home record object */ - r = home_setup(h, 0, cache, setup, &header_home); + r = home_setup(h, 0, setup, cache, &header_home); if (r < 0) return r; diff --git a/src/home/homework-directory.c b/src/home/homework-directory.c index 3418034bd1a..af13fa026a9 100644 --- a/src/home/homework-directory.c +++ b/src/home/homework-directory.c @@ -74,7 +74,7 @@ int home_activate_directory( assert_se(hdo = user_record_home_directory(h)); hd = strdupa_safe(hdo); - r = home_setup(h, 0, cache, setup, &header_home); + r = home_setup(h, 0, setup, cache, &header_home); if (r < 0) return r; @@ -259,8 +259,8 @@ int home_create_directory_or_subvolume(UserRecord *h, HomeSetup *setup, UserReco int home_resize_directory( UserRecord *h, HomeSetupFlags flags, - PasswordCache *cache, HomeSetup *setup, + PasswordCache *cache, UserRecord **ret_home) { _cleanup_(user_record_unrefp) UserRecord *embedded_home = NULL, *new_home = NULL; @@ -271,7 +271,7 @@ int home_resize_directory( assert(ret_home); assert(IN_SET(user_record_storage(h), USER_DIRECTORY, USER_SUBVOLUME, USER_FSCRYPT)); - r = home_setup(h, flags, cache, setup, NULL); + r = home_setup(h, flags, setup, cache, NULL); if (r < 0) return r; diff --git a/src/home/homework-directory.h b/src/home/homework-directory.h index 92cc755546c..ecbb2f143cb 100644 --- a/src/home/homework-directory.h +++ b/src/home/homework-directory.h @@ -7,4 +7,4 @@ int home_setup_directory(UserRecord *h, HomeSetup *setup); int home_activate_directory(UserRecord *h, HomeSetup *setup, PasswordCache *cache, UserRecord **ret_home); int home_create_directory_or_subvolume(UserRecord *h, HomeSetup *setup, UserRecord **ret_home); -int home_resize_directory(UserRecord *h, HomeSetupFlags flags, PasswordCache *cache, HomeSetup *setup, UserRecord **ret_home); +int home_resize_directory(UserRecord *h, HomeSetupFlags flags, HomeSetup *setup, PasswordCache *cache, UserRecord **ret_home); diff --git a/src/home/homework-fscrypt.c b/src/home/homework-fscrypt.c index 057bf0d45ed..f9fef73f758 100644 --- a/src/home/homework-fscrypt.c +++ b/src/home/homework-fscrypt.c @@ -282,8 +282,8 @@ static int fscrypt_setup( int home_setup_fscrypt( UserRecord *h, - const PasswordCache *cache, - HomeSetup *setup) { + HomeSetup *setup, + const PasswordCache *cache) { _cleanup_(erase_and_freep) void *volume_key = NULL; struct fscrypt_policy policy = {}; diff --git a/src/home/homework-fscrypt.h b/src/home/homework-fscrypt.h index e9262c6349a..7c2d7aace41 100644 --- a/src/home/homework-fscrypt.h +++ b/src/home/homework-fscrypt.h @@ -4,7 +4,7 @@ #include "homework.h" #include "user-record.h" -int home_setup_fscrypt(UserRecord *h, const PasswordCache *cache, HomeSetup *setup); +int home_setup_fscrypt(UserRecord *h, HomeSetup *setup, const PasswordCache *cache); int home_create_fscrypt(UserRecord *h, HomeSetup *setup, char **effective_passwords, UserRecord **ret_home); diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index 56e232523d6..cd584b0f45e 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -1168,8 +1168,8 @@ int home_setup_luks( UserRecord *h, HomeSetupFlags flags, const char *force_image_path, - PasswordCache *cache, HomeSetup *setup, + PasswordCache *cache, UserRecord **ret_luks_home) { sd_id128_t found_partition_uuid, found_luks_uuid, found_fs_uuid; @@ -1446,8 +1446,8 @@ int home_activate_luks( h, 0, NULL, - cache, setup, + cache, &luks_home_record); if (r < 0) return r; @@ -2735,8 +2735,8 @@ static int apply_resize_partition(int fd, sd_id128_t disk_uuids, struct fdisk_ta int home_resize_luks( UserRecord *h, HomeSetupFlags flags, - PasswordCache *cache, HomeSetup *setup, + PasswordCache *cache, UserRecord **ret_home) { uint64_t old_image_size, new_image_size, old_fs_size, new_fs_size, crypto_offset, new_partition_size; @@ -2835,7 +2835,7 @@ int home_resize_luks( new_image_size = new_image_size_rounded; } - r = home_setup_luks(h, flags, whole_disk, cache, setup, &header_home); + r = home_setup_luks(h, flags, whole_disk, setup, cache, &header_home); if (r < 0) return r; diff --git a/src/home/homework-luks.h b/src/home/homework-luks.h index 25d096a175c..f8af66e083e 100644 --- a/src/home/homework-luks.h +++ b/src/home/homework-luks.h @@ -5,7 +5,7 @@ #include "homework.h" #include "user-record.h" -int home_setup_luks(UserRecord *h, HomeSetupFlags flags, const char *force_image_path, PasswordCache *cache, HomeSetup *setup, UserRecord **ret_luks_home); +int home_setup_luks(UserRecord *h, HomeSetupFlags flags, const char *force_image_path, HomeSetup *setup, PasswordCache *cache, UserRecord **ret_luks_home); int home_activate_luks(UserRecord *h, HomeSetup *setup, PasswordCache *cache, UserRecord **ret_home); int home_deactivate_luks(UserRecord *h, HomeSetup *setup); @@ -17,7 +17,7 @@ int home_create_luks(UserRecord *h, HomeSetup *setup, const PasswordCache *cache int home_get_state_luks(UserRecord *h, HomeSetup *setup); -int home_resize_luks(UserRecord *h, HomeSetupFlags flags, PasswordCache *cache, HomeSetup *setup, UserRecord **ret_home); +int home_resize_luks(UserRecord *h, HomeSetupFlags flags, HomeSetup *setup, PasswordCache *cache, UserRecord **ret_home); int home_passwd_luks(UserRecord *h, HomeSetup *setup, const PasswordCache *cache, char **effective_passwords); diff --git a/src/home/homework.c b/src/home/homework.c index 8634a932304..22a960efd5f 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -407,8 +407,8 @@ int home_setup_done(HomeSetup *setup) { int home_setup( UserRecord *h, HomeSetupFlags flags, - PasswordCache *cache, HomeSetup *setup, + PasswordCache *cache, UserRecord **ret_header_home) { int r; @@ -429,7 +429,7 @@ int home_setup( switch (user_record_storage(h)) { case USER_LUKS: - return home_setup_luks(h, flags, NULL, cache, setup, ret_header_home); + return home_setup_luks(h, flags, NULL, setup, cache, ret_header_home); case USER_SUBVOLUME: case USER_DIRECTORY: @@ -437,7 +437,7 @@ int home_setup( break; case USER_FSCRYPT: - r = home_setup_fscrypt(h, cache, setup); + r = home_setup_fscrypt(h, setup, cache); break; case USER_CIFS: @@ -1521,7 +1521,7 @@ static int home_update(UserRecord *h, UserRecord **ret) { if (r < 0) return r; - r = home_setup(h, flags, &cache, &setup, &header_home); + r = home_setup(h, flags, &setup, &cache, &header_home); if (r < 0) return r; @@ -1579,12 +1579,12 @@ static int home_resize(UserRecord *h, UserRecord **ret) { switch (user_record_storage(h)) { case USER_LUKS: - return home_resize_luks(h, flags, &cache, &setup, ret); + return home_resize_luks(h, flags, &setup, &cache, ret); case USER_DIRECTORY: case USER_SUBVOLUME: case USER_FSCRYPT: - return home_resize_directory(h, flags, &cache, &setup, ret); + return home_resize_directory(h, flags, &setup, &cache, ret); default: return log_error_errno(SYNTHETIC_ERRNO(ENOTTY), "Resizing home directories of type '%s' currently not supported.", user_storage_to_string(user_record_storage(h))); @@ -1613,7 +1613,7 @@ static int home_passwd(UserRecord *h, UserRecord **ret_home) { if (r < 0) return r; - r = home_setup(h, flags, &cache, &setup, &header_home); + r = home_setup(h, flags, &setup, &cache, &header_home); if (r < 0) return r; @@ -1683,7 +1683,7 @@ static int home_inspect(UserRecord *h, UserRecord **ret_home) { if (r < 0) return r; - r = home_setup(h, flags, &cache, &setup, &header_home); + r = home_setup(h, flags, &setup, &cache, &header_home); if (r < 0) return r; diff --git a/src/home/homework.h b/src/home/homework.h index fe8cbb46358..076033526c7 100644 --- a/src/home/homework.h +++ b/src/home/homework.h @@ -78,7 +78,7 @@ int home_setup_done(HomeSetup *setup); int home_setup_undo_mount(HomeSetup *setup, int level); int home_setup_undo_dm(HomeSetup *setup, int level); -int home_setup(UserRecord *h, HomeSetupFlags flags, PasswordCache *cache, HomeSetup *setup, UserRecord **ret_header_home); +int home_setup(UserRecord *h, HomeSetupFlags flags, HomeSetup *setup, PasswordCache *cache, UserRecord **ret_header_home); int home_refresh(UserRecord *h, HomeSetup *setup, UserRecord *header_home, PasswordCache *cache, struct statfs *ret_statfs, UserRecord **ret_new_home); From ae4d05f6496717a5024b64f709d73598e9a60343 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Oct 2021 17:20:56 +0200 Subject: [PATCH 09/16] homework: rework home_setup_luks() to store its root_fd also in HomeSetup --- src/home/homework-luks.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index cd584b0f45e..0ff16459af3 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -1175,7 +1175,7 @@ int home_setup_luks( sd_id128_t found_partition_uuid, found_luks_uuid, found_fs_uuid; _cleanup_(user_record_unrefp) UserRecord *luks_home = NULL; _cleanup_(erase_and_freep) void *volume_key = NULL; - _cleanup_close_ int opened_image_fd = -1, root_fd = -1; + _cleanup_close_ int opened_image_fd = -1; size_t volume_key_size = 0; bool marked_dirty = false; uint64_t offset, size; @@ -1185,6 +1185,7 @@ int home_setup_luks( assert(setup); assert(setup->dm_name); assert(setup->dm_node); + assert(setup->root_fd < 0); assert(!setup->crypt_device); assert(!setup->loop); @@ -1273,8 +1274,8 @@ int home_setup_luks( log_info("Discovered used loopback device %s.", setup->loop->node); - root_fd = open(user_record_home_directory(h), O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); - if (root_fd < 0) { + setup->root_fd = open(user_record_home_directory(h), O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); + if (setup->root_fd < 0) { r = log_error_errno(errno, "Failed to open home directory: %m"); goto fail; } @@ -1359,14 +1360,14 @@ int home_setup_luks( setup->undo_mount = true; - root_fd = open(subdir, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); - if (root_fd < 0) { + setup->root_fd = open(subdir, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); + if (setup->root_fd < 0) { r = log_error_errno(errno, "Failed to open home directory: %m"); goto fail; } if (user_record_luks_discard(h)) - (void) run_fitrim(root_fd); + (void) run_fitrim(setup->root_fd); /* And now, fill in everything */ if (opened_image_fd >= 0) { @@ -1378,7 +1379,6 @@ int home_setup_luks( setup->do_mark_clean = marked_dirty; } - setup->root_fd = TAKE_FD(root_fd); setup->found_partition_uuid = found_partition_uuid; setup->found_luks_uuid = found_luks_uuid; setup->found_fs_uuid = found_fs_uuid; From 80ffbbfbfb9b8da2def258ddcb24e0d044143dca Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Oct 2021 17:27:28 +0200 Subject: [PATCH 10/16] homework: move image_fd field in home_setup_luks() also into HomeSetup Let's unify even more codepaths, and let's reuse the .image_fd field we already maintain. --- src/home/homework-luks.c | 36 +++++++++++++----------------------- 1 file changed, 13 insertions(+), 23 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index 0ff16459af3..d20df172fbf 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -1175,11 +1175,9 @@ int home_setup_luks( sd_id128_t found_partition_uuid, found_luks_uuid, found_fs_uuid; _cleanup_(user_record_unrefp) UserRecord *luks_home = NULL; _cleanup_(erase_and_freep) void *volume_key = NULL; - _cleanup_close_ int opened_image_fd = -1; size_t volume_key_size = 0; - bool marked_dirty = false; uint64_t offset, size; - int r, image_fd = -1; + int r; assert(h); assert(setup); @@ -1292,29 +1290,27 @@ int home_setup_luks( /* Reuse the image fd if it has already been opened by an earlier step */ if (setup->image_fd < 0) { - opened_image_fd = open_image_file(h, force_image_path, &st); - if (opened_image_fd < 0) - return opened_image_fd; + setup->image_fd = open_image_file(h, force_image_path, &st); + if (setup->image_fd < 0) + return setup->image_fd; + } - image_fd = opened_image_fd; - } else - image_fd = setup->image_fd; - - r = luks_validate(image_fd, user_record_user_name_and_realm(h), h->partition_uuid, &found_partition_uuid, &offset, &size); + r = luks_validate(setup->image_fd, user_record_user_name_and_realm(h), h->partition_uuid, &found_partition_uuid, &offset, &size); if (r < 0) return log_error_errno(r, "Failed to validate disk label: %m"); /* Everything before this point left the image untouched. We are now starting to make * changes, hence mark the image dirty */ - marked_dirty = run_mark_dirty(image_fd, true) > 0; + if (run_mark_dirty(setup->image_fd, true) > 0) + setup->do_mark_clean = true; if (!user_record_luks_discard(h)) { - r = run_fallocate(image_fd, &st); + r = run_fallocate(setup->image_fd, &st); if (r < 0) return r; } - r = loop_device_make(image_fd, O_RDWR, offset, size, 0, &setup->loop); + r = loop_device_make(setup->image_fd, O_RDWR, offset, size, 0, &setup->loop); if (r == -ENOENT) { log_error_errno(r, "Loopback block device support is not available on this system."); return -ENOLINK; /* make recognizable */ @@ -1369,14 +1365,7 @@ int home_setup_luks( if (user_record_luks_discard(h)) (void) run_fitrim(setup->root_fd); - /* And now, fill in everything */ - if (opened_image_fd >= 0) { - safe_close(setup->image_fd); - setup->image_fd = TAKE_FD(opened_image_fd); - } - setup->do_offline_fallocate = !(setup->do_offline_fitrim = user_record_luks_offline_discard(h)); - setup->do_mark_clean = marked_dirty; } setup->found_partition_uuid = found_partition_uuid; @@ -1393,11 +1382,12 @@ int home_setup_luks( return 0; fail: + setup->root_fd = safe_close(setup->root_fd); home_setup_undo_mount(setup, LOG_ERR); home_setup_undo_dm(setup, LOG_ERR); - if (image_fd >= 0 && marked_dirty) - (void) run_mark_dirty(image_fd, false); + if (setup->image_fd >= 0 && setup->do_mark_clean) + (void) run_mark_dirty(setup->image_fd, false); return r; } From d33f0241007f6ca8fed3bba204934e1e9aa00c9f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Oct 2021 17:28:51 +0200 Subject: [PATCH 11/16] homework: get rid of manual clean up path in home_setup_luks() Now that we stored all our different objects inside the HomeSetup structure, we can get rid of our manual clean-up path, since home_setup_done() will clean up everything stored therein anyway, in the right order. This is the main reason we moved everything into HomeSetup in the previous commits: so that we can share clean-up paths for these objects with everything else. --- src/home/homework-luks.c | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index d20df172fbf..1ff4d504177 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -1273,10 +1273,8 @@ int home_setup_luks( log_info("Discovered used loopback device %s.", setup->loop->node); setup->root_fd = open(user_record_home_directory(h), O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); - if (setup->root_fd < 0) { - r = log_error_errno(errno, "Failed to open home directory: %m"); - goto fail; - } + if (setup->root_fd < 0) + return log_error_errno(errno, "Failed to open home directory: %m"); } else { _cleanup_free_ char *fstype = NULL, *subdir = NULL; const char *ip; @@ -1340,27 +1338,25 @@ int home_setup_luks( r = luks_validate_home_record(setup->crypt_device, h, volume_key, cache, &luks_home); if (r < 0) - goto fail; + return r; r = fs_validate(setup->dm_node, h->file_system_uuid, &fstype, &found_fs_uuid); if (r < 0) - goto fail; + return r; r = run_fsck(setup->dm_node, fstype); if (r < 0) - goto fail; + return r; r = home_unshare_and_mount(setup->dm_node, fstype, user_record_luks_discard(h), user_record_mount_flags(h)); if (r < 0) - goto fail; + return r; setup->undo_mount = true; setup->root_fd = open(subdir, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); - if (setup->root_fd < 0) { - r = log_error_errno(errno, "Failed to open home directory: %m"); - goto fail; - } + if (setup->root_fd < 0) + return log_error_errno(errno, "Failed to open home directory: %m"); if (user_record_luks_discard(h)) (void) run_fitrim(setup->root_fd); @@ -1380,16 +1376,6 @@ int home_setup_luks( *ret_luks_home = TAKE_PTR(luks_home); return 0; - -fail: - setup->root_fd = safe_close(setup->root_fd); - home_setup_undo_mount(setup, LOG_ERR); - home_setup_undo_dm(setup, LOG_ERR); - - if (setup->image_fd >= 0 && setup->do_mark_clean) - (void) run_mark_dirty(setup->image_fd, false); - - return r; } static void print_size_summary(uint64_t host_size, uint64_t encrypted_size, struct statfs *sfs) { From 32dda527d838d375473456fc757b8e377c07ef38 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Oct 2021 17:36:36 +0200 Subject: [PATCH 12/16] homework: move destruction of temporary image file into HomeSetup Let's simplify things further a bit and move the destruction of the temporary image file we operate on when creating a LUKS home into HomeSetup, like all our other resources. --- src/home/homework-luks.c | 29 +++++++++++++++-------------- src/home/homework.c | 8 ++++++++ src/home/homework.h | 2 ++ 3 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index 1ff4d504177..8333f896b17 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -1984,13 +1984,12 @@ int home_create_luks( char **effective_passwords, UserRecord **ret_home) { - _cleanup_free_ char *subdir = NULL, *disk_uuid_path = NULL, *temporary_image_path = NULL; + _cleanup_free_ char *subdir = NULL, *disk_uuid_path = NULL; uint64_t encrypted_size, host_size = 0, partition_offset = 0, partition_size = 0; /* Unnecessary initialization to appease gcc */ _cleanup_(user_record_unrefp) UserRecord *new_home = NULL; sd_id128_t partition_uuid, fs_uuid, luks_uuid, disk_uuid; _cleanup_close_ int image_fd = -1; - bool image_created = false; const char *fstype, *ip; struct statfs sfs; int r; @@ -1998,6 +1997,7 @@ int home_create_luks( assert(h); assert(h->storage < 0 || h->storage == USER_LUKS); assert(setup); + assert(!setup->temporary_image_path); assert(ret_home); r = dlopen_cryptsetup(); @@ -2123,7 +2123,7 @@ int home_create_luks( log_info("Full device discard completed."); } } else { - _cleanup_free_ char *parent = NULL; + _cleanup_free_ char *parent = NULL, *t = NULL; parent = dirname_malloc(ip); if (!parent) @@ -2140,22 +2140,22 @@ int home_create_luks( if (!supported_fs_size(fstype, host_size)) return log_error_errno(SYNTHETIC_ERRNO(ERANGE), "Selected file system size too small for %s.", fstype); - r = tempfn_random(ip, "homework", &temporary_image_path); + r = tempfn_random(ip, "homework", &t); if (r < 0) return log_error_errno(r, "Failed to derive temporary file name for %s: %m", ip); - image_fd = open(temporary_image_path, O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW, 0600); + image_fd = open(t, O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW, 0600); if (image_fd < 0) - return log_error_errno(errno, "Failed to create home image %s: %m", temporary_image_path); + return log_error_errno(errno, "Failed to create home image %s: %m", t); - image_created = true; + setup->temporary_image_path = TAKE_PTR(t); r = chattr_fd(image_fd, FS_NOCOW_FL, FS_NOCOW_FL, NULL); if (r < 0) log_full_errno(ERRNO_IS_NOT_SUPPORTED(r) ? LOG_DEBUG : LOG_WARNING, r, - "Failed to set file attributes on %s, ignoring: %m", temporary_image_path); + "Failed to set file attributes on %s, ignoring: %m", setup->temporary_image_path); - r = home_truncate(h, image_fd, temporary_image_path, host_size); + r = home_truncate(h, image_fd, setup->temporary_image_path, host_size); if (r < 0) goto fail; @@ -2184,7 +2184,7 @@ int home_create_luks( goto fail; } - log_error_errno(r, "Failed to set up loopback device for %s: %m", temporary_image_path); + log_error_errno(r, "Failed to set up loopback device for %s: %m", setup->temporary_image_path); goto fail; } @@ -2327,12 +2327,13 @@ int home_create_luks( * lock that ensures udev doesn't interfere with what we are doing */ image_fd = safe_close(image_fd); - if (temporary_image_path) { - if (rename(temporary_image_path, ip) < 0) { + if (setup->temporary_image_path) { + if (rename(setup->temporary_image_path, ip) < 0) { log_error_errno(errno, "Failed to rename image file: %m"); goto fail; } + setup->temporary_image_path = mfree(setup->temporary_image_path); log_info("Moved image file into place."); } @@ -2354,8 +2355,8 @@ fail: setup->loop = loop_device_unref(setup->loop); - if (image_created) - (void) unlink(temporary_image_path); + if (setup->temporary_image_path) + (void) unlink(setup->temporary_image_path); return r; } diff --git a/src/home/homework.c b/src/home/homework.c index 22a960efd5f..7a2d816e59b 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -382,6 +382,14 @@ int home_setup_done(HomeSetup *setup) { setup->image_fd = safe_close(setup->image_fd); } + if (setup->temporary_image_path) { + if (unlink(setup->temporary_image_path) < 0) + log_debug_errno(errno, "Failed to remove temporary image file '%s', ignoring: %m", + setup->temporary_image_path); + + setup->temporary_image_path = mfree(setup->temporary_image_path); + } + setup->undo_mount = false; setup->undo_dm = false; setup->do_offline_fitrim = false; diff --git a/src/home/homework.h b/src/home/homework.h index 076033526c7..d7ad6fbdfaa 100644 --- a/src/home/homework.h +++ b/src/home/homework.h @@ -39,6 +39,8 @@ typedef struct HomeSetup { uint64_t partition_size; char *mount_suffix; /* The directory to use as home dir is this path below /run/systemd/user-home-mount */ + + char *temporary_image_path; } HomeSetup; typedef struct PasswordCache { From ebcdfc79abb9e44cca0e2efac6c2eb243221d5af Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Oct 2021 17:40:21 +0200 Subject: [PATCH 13/16] homework: get rid of manual error path in home_create_luks() Now that all objects we need to destroy are managed by the HomeSetup object we can drop our manual destruction path and just use the normal clean-up logic implemented for HomeSetup anyway. More unification, yay! --- src/home/homework-luks.c | 103 +++++++++++++-------------------------- 1 file changed, 34 insertions(+), 69 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index 8333f896b17..e4047a092bc 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -2157,7 +2157,7 @@ int home_create_luks( r = home_truncate(h, image_fd, setup->temporary_image_path, host_size); if (r < 0) - goto fail; + return r; log_info("Allocating image file completed."); } @@ -2170,7 +2170,7 @@ int home_create_luks( &partition_size, &disk_uuid); if (r < 0) - goto fail; + return r; log_info("Writing of partition table completed."); @@ -2180,19 +2180,15 @@ int home_create_luks( * or similar and loopback bock devices are not available, return a * recognizable error in this case. */ log_error_errno(r, "Loopback block device support is not available on this system."); - r = -ENOLINK; - goto fail; + return -ENOLINK; /* Make recognizable */ } - log_error_errno(r, "Failed to set up loopback device for %s: %m", setup->temporary_image_path); - goto fail; + return log_error_errno(r, "Failed to set up loopback device for %s: %m", setup->temporary_image_path); } r = loop_device_flock(setup->loop, LOCK_EX); /* make sure udev won't read before we are done */ - if (r < 0) { - log_error_errno(r, "Failed to take lock on loop device: %m"); - goto fail; - } + if (r < 0) + return log_error_errno(r, "Failed to take lock on loop device: %m"); log_info("Setting up loopback device %s completed.", setup->loop->node ?: ip); @@ -2206,62 +2202,52 @@ int home_create_luks( h, &setup->crypt_device); if (r < 0) - goto fail; + return r; setup->undo_dm = true; r = block_get_size_by_path(setup->dm_node, &encrypted_size); - if (r < 0) { - log_error_errno(r, "Failed to get encrypted block device size: %m"); - goto fail; - } + if (r < 0) + return log_error_errno(r, "Failed to get encrypted block device size: %m"); log_info("Setting up LUKS device %s completed.", setup->dm_node); r = make_filesystem(setup->dm_node, fstype, user_record_user_name_and_realm(h), fs_uuid, user_record_luks_discard(h)); if (r < 0) - goto fail; + return r; log_info("Formatting file system completed."); r = home_unshare_and_mount(setup->dm_node, fstype, user_record_luks_discard(h), user_record_mount_flags(h)); if (r < 0) - goto fail; + return r; setup->undo_mount = true; subdir = path_join(HOME_RUNTIME_WORK_DIR, user_record_user_name_and_realm(h)); - if (!subdir) { - r = log_oom(); - goto fail; - } + if (!subdir) + return log_oom(); /* Prefer using a btrfs subvolume if we can, fall back to directory otherwise */ r = btrfs_subvol_make_fallback(subdir, 0700); - if (r < 0) { - log_error_errno(r, "Failed to create user directory in mounted image file: %m"); - goto fail; - } + if (r < 0) + return log_error_errno(r, "Failed to create user directory in mounted image file: %m"); setup->root_fd = open(subdir, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOFOLLOW); - if (setup->root_fd < 0) { - r = log_error_errno(errno, "Failed to open user directory in mounted image file: %m"); - goto fail; - } + if (setup->root_fd < 0) + return log_error_errno(errno, "Failed to open user directory in mounted image file: %m"); r = home_populate(h, setup->root_fd); if (r < 0) - goto fail; + return r; r = home_sync_and_statfs(setup->root_fd, &sfs); if (r < 0) - goto fail; + return r; r = user_record_clone(h, USER_RECORD_LOAD_MASK_SECRET|USER_RECORD_LOG|USER_RECORD_PERMISSIVE, &new_home); - if (r < 0) { - log_error_errno(r, "Failed to clone record: %m"); - goto fail; - } + if (r < 0) + return log_error_errno(r, "Failed to clone record: %m"); r = user_record_add_binding( new_home, @@ -2277,50 +2263,44 @@ int home_create_luks( NULL, h->uid, (gid_t) h->uid); - if (r < 0) { - log_error_errno(r, "Failed to add binding to record: %m"); - goto fail; - } + if (r < 0) + return log_error_errno(r, "Failed to add binding to record: %m"); if (user_record_luks_offline_discard(h)) { r = run_fitrim(setup->root_fd); if (r < 0) - goto fail; + return r; } setup->root_fd = safe_close(setup->root_fd); r = home_setup_undo_mount(setup, LOG_ERR); if (r < 0) - goto fail; + return r; r = home_setup_undo_dm(setup, LOG_ERR); if (r < 0) - goto fail; + return r; setup->loop = loop_device_unref(setup->loop); if (!user_record_luks_offline_discard(h)) { - r = run_fallocate(image_fd, NULL /* refresh stat() data */); + r= run_fallocate(image_fd, NULL /* refresh stat() data */); if (r < 0) - goto fail; + return r; } /* Sync everything to disk before we move things into place under the final name. */ - if (fsync(image_fd) < 0) { - r = log_error_errno(r, "Failed to synchronize image to disk: %m"); - goto fail; - } + if (fsync(image_fd) < 0) + return log_error_errno(r, "Failed to synchronize image to disk: %m"); if (disk_uuid_path) (void) ioctl(image_fd, BLKRRPART, 0); else { /* If we operate on a file, sync the containing directory too. */ r = fsync_directory_of_file(image_fd); - if (r < 0) { - log_error_errno(r, "Failed to synchronize directory of image file to disk: %m"); - goto fail; - } + if (r < 0) + return log_error_errno(r, "Failed to synchronize directory of image file to disk: %m"); } /* Let's close the image fd now. If we are operating on a real block device this will release the BSD @@ -2328,10 +2308,8 @@ int home_create_luks( image_fd = safe_close(image_fd); if (setup->temporary_image_path) { - if (rename(setup->temporary_image_path, ip) < 0) { - log_error_errno(errno, "Failed to rename image file: %m"); - goto fail; - } + if (rename(setup->temporary_image_path, ip) < 0) + return log_error_errno(errno, "Failed to rename image file: %m"); setup->temporary_image_path = mfree(setup->temporary_image_path); log_info("Moved image file into place."); @@ -2346,19 +2324,6 @@ int home_create_luks( *ret_home = TAKE_PTR(new_home); return 0; - -fail: - /* Let's close all files before we unmount the file system, to avoid EBUSY */ - setup->root_fd = safe_close(setup->root_fd); - (void) home_setup_undo_mount(setup, LOG_WARNING); - (void) home_setup_undo_dm(setup, LOG_WARNING); - - setup->loop = loop_device_unref(setup->loop); - - if (setup->temporary_image_path) - (void) unlink(setup->temporary_image_path); - - return r; } int home_get_state_luks(UserRecord *h, HomeSetup *setup) { From a2bc39786a8e33daa47ad7bc4fc241532ce19b80 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Oct 2021 17:44:14 +0200 Subject: [PATCH 14/16] homework: also move to using .image_fd field of HomeSetup in home_create_luks() One last resource we can manage with HomeSetup, instead of doing that manually. --- src/home/homework-luks.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index e4047a092bc..b579e3be261 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -1989,7 +1989,6 @@ int home_create_luks( host_size = 0, partition_offset = 0, partition_size = 0; /* Unnecessary initialization to appease gcc */ _cleanup_(user_record_unrefp) UserRecord *new_home = NULL; sd_id128_t partition_uuid, fs_uuid, luks_uuid, disk_uuid; - _cleanup_close_ int image_fd = -1; const char *fstype, *ip; struct statfs sfs; int r; @@ -1998,6 +1997,7 @@ int home_create_luks( assert(h->storage < 0 || h->storage == USER_LUKS); assert(setup); assert(!setup->temporary_image_path); + assert(setup->image_fd < 0); assert(ret_home); r = dlopen_cryptsetup(); @@ -2066,11 +2066,11 @@ int home_create_luks( /* Let's place the home directory on a real device, i.e. an USB stick or such */ - image_fd = open(ip, O_RDWR|O_CLOEXEC|O_NOCTTY|O_NONBLOCK); - if (image_fd < 0) + setup->image_fd = open(ip, O_RDWR|O_CLOEXEC|O_NOCTTY|O_NONBLOCK); + if (setup->image_fd < 0) return log_error_errno(errno, "Failed to open device %s: %m", ip); - if (fstat(image_fd, &st) < 0) + if (fstat(setup->image_fd, &st) < 0) return log_error_errno(errno, "Failed to stat device %s: %m", ip); if (!S_ISBLK(st.st_mode)) return log_error_errno(SYNTHETIC_ERRNO(ENOTBLK), "Device is not a block device, refusing."); @@ -2083,10 +2083,10 @@ int home_create_luks( } else return log_error_errno(SYNTHETIC_ERRNO(ENOTBLK), "Operating on partitions is currently not supported, sorry. Please specify a top-level block device."); - if (flock(image_fd, LOCK_EX) < 0) /* make sure udev doesn't read from it while we operate on the device */ + if (flock(setup->image_fd, LOCK_EX) < 0) /* make sure udev doesn't read from it while we operate on the device */ return log_error_errno(errno, "Failed to lock block device %s: %m", ip); - if (ioctl(image_fd, BLKGETSIZE64, &block_device_size) < 0) + if (ioctl(setup->image_fd, BLKGETSIZE64, &block_device_size) < 0) return log_error_errno(errno, "Failed to read block device size: %m"); if (h->disk_size == UINT64_MAX) { @@ -2116,7 +2116,7 @@ int home_create_luks( if (user_record_luks_discard(h) || user_record_luks_offline_discard(h)) { /* If we want online or offline discard, discard once before we start using things. */ - if (ioctl(image_fd, BLKDISCARD, (uint64_t[]) { 0, block_device_size }) < 0) + if (ioctl(setup->image_fd, BLKDISCARD, (uint64_t[]) { 0, block_device_size }) < 0) log_full_errno(errno == EOPNOTSUPP ? LOG_DEBUG : LOG_WARNING, errno, "Failed to issue full-device BLKDISCARD on device, ignoring: %m"); else @@ -2144,18 +2144,18 @@ int home_create_luks( if (r < 0) return log_error_errno(r, "Failed to derive temporary file name for %s: %m", ip); - image_fd = open(t, O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW, 0600); - if (image_fd < 0) + setup->image_fd = open(t, O_RDWR|O_CREAT|O_EXCL|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW, 0600); + if (setup->image_fd < 0) return log_error_errno(errno, "Failed to create home image %s: %m", t); setup->temporary_image_path = TAKE_PTR(t); - r = chattr_fd(image_fd, FS_NOCOW_FL, FS_NOCOW_FL, NULL); + r = chattr_fd(setup->image_fd, FS_NOCOW_FL, FS_NOCOW_FL, NULL); if (r < 0) log_full_errno(ERRNO_IS_NOT_SUPPORTED(r) ? LOG_DEBUG : LOG_WARNING, r, "Failed to set file attributes on %s, ignoring: %m", setup->temporary_image_path); - r = home_truncate(h, image_fd, setup->temporary_image_path, host_size); + r = home_truncate(h, setup->image_fd, setup->temporary_image_path, host_size); if (r < 0) return r; @@ -2163,7 +2163,7 @@ int home_create_luks( } r = make_partition_table( - image_fd, + setup->image_fd, user_record_user_name_and_realm(h), partition_uuid, &partition_offset, @@ -2174,7 +2174,7 @@ int home_create_luks( log_info("Writing of partition table completed."); - r = loop_device_make(image_fd, O_RDWR, partition_offset, partition_size, 0, &setup->loop); + r = loop_device_make(setup->image_fd, O_RDWR, partition_offset, partition_size, 0, &setup->loop); if (r < 0) { if (r == -ENOENT) { /* this means /dev/loop-control doesn't exist, i.e. we are in a container * or similar and loopback bock devices are not available, return a @@ -2285,27 +2285,27 @@ int home_create_luks( setup->loop = loop_device_unref(setup->loop); if (!user_record_luks_offline_discard(h)) { - r= run_fallocate(image_fd, NULL /* refresh stat() data */); + r= run_fallocate(setup->image_fd, NULL /* refresh stat() data */); if (r < 0) return r; } /* Sync everything to disk before we move things into place under the final name. */ - if (fsync(image_fd) < 0) + if (fsync(setup->image_fd) < 0) return log_error_errno(r, "Failed to synchronize image to disk: %m"); if (disk_uuid_path) - (void) ioctl(image_fd, BLKRRPART, 0); + (void) ioctl(setup->image_fd, BLKRRPART, 0); else { /* If we operate on a file, sync the containing directory too. */ - r = fsync_directory_of_file(image_fd); + r = fsync_directory_of_file(setup->image_fd); if (r < 0) return log_error_errno(r, "Failed to synchronize directory of image file to disk: %m"); } /* Let's close the image fd now. If we are operating on a real block device this will release the BSD * lock that ensures udev doesn't interfere with what we are doing */ - image_fd = safe_close(image_fd); + setup->image_fd = safe_close(setup->image_fd); if (setup->temporary_image_path) { if (rename(setup->temporary_image_path, ip) < 0) From 1147c538bbb6a2d3d5ba2e40f1437bcbeb22b33e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Oct 2021 18:34:57 +0200 Subject: [PATCH 15/16] homework: teach luks backend uid mapping This teachs the LUKS backend UID mapping, similar to the existing logic for the "directory", "subvolume" and "fscrypt" backends: the files will be owned by "nobody" on the fs itself, but will be mapped to logging in user via uidmapped mounts. This way LUKS home dirs become truly portable: no local UID info will leak onto the images anymore, and the need to recursively chown them on activation goes away. This means activation is always as performant as it should be. --- src/home/homework-luks.c | 15 +++++++++++++++ src/home/homework-mount.c | 16 +++++++++++++--- src/home/homework.c | 12 +++++++++--- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/home/homework-luks.c b/src/home/homework-luks.c index b579e3be261..d1208e46787 100644 --- a/src/home/homework-luks.c +++ b/src/home/homework-luks.c @@ -45,6 +45,7 @@ #include "strv.h" #include "sync-util.h" #include "tmpfile-util.h" +#include "user-util.h" /* Round down to the nearest 4K size. Given that newer hardware generally prefers 4K sectors, let's align our * partitions to that too. In the worst case we'll waste 3.5K per partition that way, but I think I can live @@ -1989,6 +1990,7 @@ int home_create_luks( host_size = 0, partition_offset = 0, partition_size = 0; /* Unnecessary initialization to appease gcc */ _cleanup_(user_record_unrefp) UserRecord *new_home = NULL; sd_id128_t partition_uuid, fs_uuid, luks_uuid, disk_uuid; + _cleanup_close_ int mount_fd = -1; const char *fstype, *ip; struct statfs sfs; int r; @@ -2237,6 +2239,19 @@ int home_create_luks( if (setup->root_fd < 0) return log_error_errno(errno, "Failed to open user directory in mounted image file: %m"); + (void) home_shift_uid(setup->root_fd, NULL, UID_NOBODY, h->uid, &mount_fd); + + if (mount_fd >= 0) { + /* If we have established a new mount, then we can use that as new root fd to our home directory. */ + safe_close(setup->root_fd); + + setup->root_fd = fd_reopen(mount_fd, O_RDONLY|O_CLOEXEC|O_DIRECTORY); + if (setup->root_fd < 0) + return log_error_errno(setup->root_fd, "Unable to convert mount fd into proper directory fd: %m"); + + mount_fd = safe_close(mount_fd); + } + r = home_populate(h, setup->root_fd); if (r < 0) return r; diff --git a/src/home/homework-mount.c b/src/home/homework-mount.c index 5758e85839d..82b461a9873 100644 --- a/src/home/homework-mount.c +++ b/src/home/homework-mount.c @@ -84,7 +84,17 @@ int home_unshare_and_mount(const char *node, const char *fstype, bool discard, u if (r < 0) return r; - return home_mount_node(node, fstype, discard, flags); + r = home_mount_node(node, fstype, discard, flags); + if (r < 0) + return r; + + r = mount_nofollow_verbose(LOG_ERR, NULL, HOME_RUNTIME_WORK_DIR, NULL, MS_PRIVATE, NULL); + if (r < 0) { + (void) umount_verbose(LOG_ERR, HOME_RUNTIME_WORK_DIR, UMOUNT_NOFOLLOW); + return r; + } + + return 0; } int home_move_mount(const char *mount_suffix, const char *target) { @@ -111,9 +121,9 @@ int home_move_mount(const char *mount_suffix, const char *target) { if (r < 0) return r; - r = umount_verbose(LOG_ERR, HOME_RUNTIME_WORK_DIR, UMOUNT_NOFOLLOW); + r = umount_recursive(HOME_RUNTIME_WORK_DIR, 0); if (r < 0) - return r; + return log_error_errno(r, "Failed to unmount %s: %m", HOME_RUNTIME_WORK_DIR); log_info("Moving to final mount point %s completed.", target); return 0; diff --git a/src/home/homework.c b/src/home/homework.c index 7a2d816e59b..a5da4cf54ba 100644 --- a/src/home/homework.c +++ b/src/home/homework.c @@ -306,9 +306,15 @@ int home_setup_undo_mount(HomeSetup *setup, int level) { if (!setup->undo_mount) return 0; - r = umount_verbose(level, HOME_RUNTIME_WORK_DIR, UMOUNT_NOFOLLOW); - if (r < 0) - return r; + r = umount_recursive(HOME_RUNTIME_WORK_DIR, 0); + if (r < 0) { + if (level >= LOG_DEBUG) /* umount_recursive() does debug level logging anyway, no need to + * repeat that here */ + return r; + + /* If a higher log level is requested, the generate a non-debug mesage here too. */ + return log_full_errno(level, r, "Failed to unmount mount tree below %s: %m", HOME_RUNTIME_WORK_DIR); + } setup->undo_mount = false; return 1; From cc4cbe49cec2ec8c81f89bb45e27d8d90be8c57d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 26 Oct 2021 18:41:12 +0200 Subject: [PATCH 16/16] update TODO --- TODO | 2 -- 1 file changed, 2 deletions(-) diff --git a/TODO b/TODO index d19808ec2fe..9c8bdfdaa53 100644 --- a/TODO +++ b/TODO @@ -1279,8 +1279,6 @@ Features: - support new FS_IOC_ADD_ENCRYPTION_KEY ioctl for setting up fscrypt - maybe pre-create ~/.cache as subvol so that it can have separate quota easily? - - if kernel 5.12 uid mapping mounts exist, use that instead of recursive - chowns. - add a switch to homectl (maybe called --first-boot) where it will check if any non-system users exist, and if not prompts interactively for basic user info, mimicking systemd-firstboot. Then, place this in a service that runs