From 8518f4a814426e7a92342298353a4cd9508cb33b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 23 Apr 2024 12:35:38 +0200 Subject: [PATCH] cryptenroll: default to block device backing /var/ rather than / With 1df4b21abdb9e562805a7b006d179507182f845e we started to default to enrolling into the LUKS device backing the root fs if none was specified (and no wipe operation is used). This changes to look for /var/ instead. On most systems /var/ is going to be on the root fs, hence this change is with little effect. However, on systems where / and /var/ is separate it makes more sense to default to /var/ because that's where the persistent and variable data is placed (i.e. where LUKS should be used) while / doesn't really have to be variable, could as well be immutable, or ephemeral. Hence /var/ should be a safer default. Or to say this differently: I think it makes sense to support systems with /var/ being on / well. I also think it makes sense to support systems with them being separate, and /var/ being variable and persistent. But any other kind of system I find much less interesting to support, and in that case people should just specify the device name. Also, while we are at it, tighten the checks a bit, insist on a dm-crypt + LUKS superblock before continuing. And finally, let's print a short message indicating the device we operate on. --- man/systemd-cryptenroll.xml | 6 ++- src/cryptenroll/cryptenroll.c | 81 ++++++++++++++++++++++++++++------- 2 files changed, 70 insertions(+), 17 deletions(-) diff --git a/man/systemd-cryptenroll.xml b/man/systemd-cryptenroll.xml index 9287d835de..5a522e36f0 100644 --- a/man/systemd-cryptenroll.xml +++ b/man/systemd-cryptenroll.xml @@ -61,8 +61,10 @@ The tool supports only LUKS2 volumes, as it stores token meta-information in the LUKS2 JSON token area, which is not available in other encryption formats. - systemd-cryptsetup operates on the device backing / if no - device is specified explicitly and no wipe operation is requested + systemd-cryptsetup operates on the device backing /var/ if + no device is specified explicitly, and no wipe operation is requested. (Note that in the typical case + where /var/ is on the same file system as the root file system, this hence enrolls a + key into the backing device of the root file system.) TPM2 PCRs and policies diff --git a/src/cryptenroll/cryptenroll.c b/src/cryptenroll/cryptenroll.c index e30cba9fd4..9ace8c1e90 100644 --- a/src/cryptenroll/cryptenroll.c +++ b/src/cryptenroll/cryptenroll.c @@ -103,6 +103,66 @@ static const char *const luks2_token_type_table[_ENROLL_TYPE_MAX] = { DEFINE_STRING_TABLE_LOOKUP(luks2_token_type, EnrollType); +static int determine_default_node(void) { + int r; + + /* If no device is specified we'll default to the backing device of /var/. + * + * Why /var/ and not just / you ask? + * + * On most systems /var/ is going to be on the root fs, hence the outcome is usually the same. + * + * However, on systems where / and /var/ are separate it makes more sense to default to /var/ because + * that's where the persistent and variable data is placed (i.e. where LUKS should be used) while / + * doesn't really have to be variable and could as well be immutable or ephemeral. Hence /var/ should + * be a better default. + * + * Or to say this differently: it makes sense to support well systems with /var/ being on /. It also + * makes sense to support well systems with them being separate, and /var/ being variable and + * persistent. But any other kind of system appears much less interesting to support, and in that + * case people should just specify the device name explicitly. */ + + dev_t devno; + r = get_block_device("/var", &devno); + if (r < 0) + return log_error_errno(r, "Failed to determine block device backing /var/: %m"); + if (r == 0) + return log_error_errno(SYNTHETIC_ERRNO(ENXIO), + "File system /var/ is on not backed by a (single) whole block device."); + + _cleanup_(sd_device_unrefp) sd_device *dev = NULL; + r = sd_device_new_from_devnum(&dev, 'b', devno); + if (r < 0) + return log_error_errno(r, "Unable to access backing block device for /var/: %m"); + + const char *dm_uuid; + r = sd_device_get_property_value(dev, "DM_UUID", &dm_uuid); + if (r == -ENOENT) + return log_error_errno(r, "Backing block device of /var/ is not a DM device: %m"); + if (r < 0) + return log_error_errno(r, "Unable to query DM_UUID udev property of backing block device for /var/): %m"); + + if (!startswith(dm_uuid, "CRYPT-LUKS2-")) + return log_error_errno(SYNTHETIC_ERRNO(ENXIO), "Block device backing /var/ is not a LUKS2 device: %m"); + + _cleanup_(sd_device_unrefp) sd_device *origin = NULL; + r = block_device_get_originating(dev, &origin); + if (r < 0) + return log_error_errno(r, "Failed to get originating device of LUKS2 device backing /var/: %m"); + + const char *dp; + r = sd_device_get_devname(origin, &dp); + if (r < 0) + return log_error_errno(r, "Failed to get device path for LUKS2 device backing /var/: %m"); + + r = free_and_strdup_warn(&arg_node, dp); + if (r < 0) + return r; + + log_info("No device specified, defaulting to '%s'.", arg_node); + return 0; +} + static int help(void) { _cleanup_free_ char *link = NULL; int r; @@ -544,24 +604,15 @@ static int parse_argv(int argc, char *argv[]) { r = parse_path_argument(argv[optind], false, &arg_node); if (r < 0) return r; - } else if (!wipe_requested()) { - dev_t devno; + } else { + if (wipe_requested()) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Wiping requested and no block device node specified, refusing."); - r = blockdev_get_root(LOG_ERR, &devno); + r = determine_default_node(); if (r < 0) return r; - if (r == 0) - return log_error_errno(SYNTHETIC_ERRNO(ENXIO), - "Root file system not backed by a (single) whole block device."); - - r = device_path_make_canonical(S_IFBLK, devno, &arg_node); - if (r < 0) - return log_error_errno(r, - "Failed to format canonical device path for devno '" DEVNUM_FORMAT_STR "': %m", - DEVNUM_FORMAT_VAL(devno)); - } else - return log_error_errno(SYNTHETIC_ERRNO(EINVAL), - "No block device node specified, refusing."); + } if (arg_enroll_type == ENROLL_FIDO2) { -- 2.25.1