From 5a9f1b05ed6dad48958097fb37811668e69447fb Mon Sep 17 00:00:00 2001 From: Michal Sekletar Date: Tue, 12 Dec 2017 17:49:14 +0100 Subject: [PATCH 1/2] cryptsetup: when unlocking always put path to the object into Id Some ask-password agents (e.g. clevis-luks-askpass) use Id option from /run/systemd/ask-password/ask* file in order to obtain the password for the device. Id option should be in the following format, e.g. Id=subsystem:data. Where data part is supposed to identify object that ask-password query is done for. Since e51b9486d1b59e72c293028fed1384f4e4ef09aa this field has format Id=cryptsetup:/dev/block/major:minor when systemd-cryptsetup is unlocking encrypted block device. However, crypttab also supports encrypted image files in which case we usually set data part of Id to "vol on mountpoint". This is unexpected and actually breaks network based device encryption as implemented by clevis. Example: $ cat /etc/crypttab clevis-unlocked /clevis-test-disk-image none luks,_netdev $ systemctl start 'systemd-cryptsetup@clevis\x2dunlocked.service' $ grep Id /run/systemd/ask-password/ask* Before: $ Id=cryptsetup:clevis-unlocked on /clevis-test-disk-image-mnt After: $ Id=cryptsetup:/clevis-test-disk-image --- src/cryptsetup/cryptsetup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c index ab4d24ca3b..b6d965e24f 100644 --- a/src/cryptsetup/cryptsetup.c +++ b/src/cryptsetup/cryptsetup.c @@ -365,7 +365,7 @@ static int get_password(const char *vol, const char *src, usec_t until, bool acc escaped_name = maj_min; maj_min = NULL; } else - escaped_name = cescape(name); + escaped_name = cescape(src); if (!escaped_name) return log_oom(); From ea7e7c1e9c3b579ee94a11a192f1013ee4cb829e Mon Sep 17 00:00:00 2001 From: Michal Sekletar Date: Tue, 12 Dec 2017 20:00:31 +0100 Subject: [PATCH 2/2] cryptsetup: use more descriptive name for the variable and drop redundant function Let's rename escaped_name to disk_path since this is an actual content that pointer refers to. It is either path to encrypted block device or path to encrypted image file. Also drop redundant function disk_major_minor(). src is always set, and it always points to either encrypted block device path (or symlink to such device) or to encrypted image. In case it is set to device path there is no need to reset it to /dev/block/major:minor symlink since those paths are equivalent. --- src/cryptsetup/cryptsetup.c | 39 +++++++------------------------------ 1 file changed, 7 insertions(+), 32 deletions(-) diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c index b6d965e24f..21c51022ef 100644 --- a/src/cryptsetup/cryptsetup.c +++ b/src/cryptsetup/cryptsetup.c @@ -249,23 +249,6 @@ static int parse_options(const char *options) { return 0; } -static int disk_major_minor(const char *path, char **ret) { - struct stat st; - - assert(path); - - if (stat(path, &st) < 0) - return -errno; - - if (!S_ISBLK(st.st_mode)) - return -EINVAL; - - if (asprintf(ret, "/dev/block/%d:%d", major(st.st_rdev), minor(st.st_rdev)) < 0) - return -errno; - - return 0; -} - static char* disk_description(const char *path) { static const char name_fields[] = @@ -324,7 +307,7 @@ static char *disk_mount_point(const char *label) { } static int get_password(const char *vol, const char *src, usec_t until, bool accept_cached, char ***ret) { - _cleanup_free_ char *description = NULL, *name_buffer = NULL, *mount_point = NULL, *maj_min = NULL, *text = NULL, *escaped_name = NULL; + _cleanup_free_ char *description = NULL, *name_buffer = NULL, *mount_point = NULL, *text = NULL, *disk_path = NULL; _cleanup_strv_free_erase_ char **passwords = NULL; const char *name = NULL; char **p, *id; @@ -337,6 +320,10 @@ static int get_password(const char *vol, const char *src, usec_t until, bool acc description = disk_description(src); mount_point = disk_mount_point(vol); + disk_path = cescape(src); + if (!disk_path) + return log_oom(); + if (description && streq(vol, description)) /* If the description string is simply the * volume name, then let's not show this @@ -358,19 +345,7 @@ static int get_password(const char *vol, const char *src, usec_t until, bool acc if (asprintf(&text, "Please enter passphrase for disk %s!", name) < 0) return log_oom(); - if (src) - (void) disk_major_minor(src, &maj_min); - - if (maj_min) { - escaped_name = maj_min; - maj_min = NULL; - } else - escaped_name = cescape(src); - - if (!escaped_name) - return log_oom(); - - id = strjoina("cryptsetup:", escaped_name); + id = strjoina("cryptsetup:", disk_path); r = ask_password_auto(text, "drive-harddisk", id, "cryptsetup", until, ASK_PASSWORD_PUSH_CACHE | (accept_cached*ASK_PASSWORD_ACCEPT_CACHED), @@ -386,7 +361,7 @@ static int get_password(const char *vol, const char *src, usec_t until, bool acc if (asprintf(&text, "Please enter passphrase for disk %s! (verification)", name) < 0) return log_oom(); - id = strjoina("cryptsetup-verification:", escaped_name); + id = strjoina("cryptsetup-verification:", disk_path); r = ask_password_auto(text, "drive-harddisk", id, "cryptsetup", until, ASK_PASSWORD_PUSH_CACHE, &passwords2); if (r < 0)