* [PATCH] Cryptomount support for key files and detached header
@ 2020-11-07 16:36 reagentoo
2020-11-09 3:16 ` Glenn Washburn
0 siblings, 1 reply; 7+ messages in thread
From: reagentoo @ 2020-11-07 16:36 UTC (permalink / raw)
To: grub-devel; +Cc: Dmitry Baranov
From: Dmitry Baranov <reagentoo@gmail.com>
Hello.
Could someone make initial review?
The main difference between this patch and the previous ones is
the ability to use a master key file with a detached header.
This patch does not provide luks1 and geli support yet (luks2 only).
Best Regards,
Dmitry
---
grub-core/disk/cryptodisk.c | 99 +++++++++++++++-----
grub-core/disk/luks2.c | 178 ++++++++++++++++++++++++++++--------
include/grub/cryptodisk.h | 9 +-
3 files changed, 222 insertions(+), 64 deletions(-)
diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 13af84dd1..987a39b16 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -41,6 +41,9 @@ static const struct grub_arg_option options[] =
/* TRANSLATORS: It's still restricted to cryptodisks only. */
{"all", 'a', 0, N_("Mount all."), 0, 0},
{"boot", 'b', 0, N_("Mount all volumes with `boot' flag set."), 0, 0},
+ {"header", 'H', 0, N_("Read header from file"), 0, ARG_TYPE_STRING},
+ {"key-file", 'k', 0, N_("Read key from file"), 0, ARG_TYPE_STRING},
+ {"master-key-file", 'K', 0, N_("Use a master key stored in a file"), 0, ARG_TYPE_STRING},
{0, 0, 0, 0, 0, 0}
};
@@ -967,6 +970,7 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk)
static int check_boot, have_it;
static char *search_uuid;
+static grub_file_t hdr_file, key_file, mkey_file;
static void
cryptodisk_close (grub_cryptodisk_t dev)
@@ -991,13 +995,13 @@ grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source)
FOR_CRYPTODISK_DEVS (cr)
{
- dev = cr->scan (source, search_uuid, check_boot);
+ dev = cr->scan (source, hdr_file, search_uuid, check_boot);
if (grub_errno)
return grub_errno;
if (!dev)
continue;
- err = cr->recover_key (source, dev);
+ err = cr->recover_key (source, dev, hdr_file, key_file, mkey_file);
if (err)
{
cryptodisk_close (dev);
@@ -1038,7 +1042,7 @@ grub_cryptodisk_cheat_mount (const char *sourcedev, const char *cheat)
FOR_CRYPTODISK_DEVS (cr)
{
- dev = cr->scan (source, search_uuid, check_boot);
+ dev = cr->scan (source, 0, search_uuid, check_boot);
if (grub_errno)
return grub_errno;
if (!dev)
@@ -1087,11 +1091,59 @@ grub_cryptodisk_scan_device (const char *name,
static grub_err_t
grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
{
+ grub_err_t ret = GRUB_ERR_NONE;
+
struct grub_arg_list *state = ctxt->state;
if (argc < 1 && !state[1].set && !state[2].set)
return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name required");
+ hdr_file = NULL;
+ key_file = NULL;
+ mkey_file = NULL;
+
+ if (state[3].set)
+ {
+ if (state[0].set)
+ {
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot use UUID lookup with detached header");
+ goto err;
+ }
+
+ hdr_file = grub_file_open (state[3].arg, GRUB_FILE_TYPE_NONE);
+ if (!hdr_file)
+ {
+ ret = grub_errno;
+ goto err;
+ }
+ }
+
+ if (state[4].set)
+ {
+ if (state[5].set)
+ {
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot use key with master key");
+ goto err;
+ }
+
+ key_file = grub_file_open (state[4].arg, GRUB_FILE_TYPE_NONE);
+ if (!key_file)
+ {
+ ret = grub_errno;
+ goto err;
+ }
+ }
+
+ if (state[5].set)
+ {
+ mkey_file = grub_file_open (state[5].arg, GRUB_FILE_TYPE_NONE);
+ if (!mkey_file)
+ {
+ ret = grub_errno;
+ goto err;
+ }
+ }
+
have_it = 0;
if (state[0].set)
{
@@ -1102,7 +1154,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
{
grub_dprintf ("cryptodisk",
"already mounted as crypto%lu\n", dev->id);
- return GRUB_ERR_NONE;
+ goto err;
}
check_boot = state[2].set;
@@ -1111,8 +1163,11 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
search_uuid = NULL;
if (!have_it)
- return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such cryptodisk found");
- return GRUB_ERR_NONE;
+ {
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "no such cryptodisk found");
+ goto err;
+ }
+ goto err;
}
else if (state[1].set || (argc == 0 && state[2].set))
{
@@ -1120,11 +1175,10 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
check_boot = state[2].set;
grub_device_iterate (&grub_cryptodisk_scan_device, NULL);
search_uuid = NULL;
- return GRUB_ERR_NONE;
+ goto err;
}
else
{
- grub_err_t err;
grub_disk_t disk;
grub_cryptodisk_t dev;
char *diskname;
@@ -1147,27 +1201,30 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
{
if (disklast)
*disklast = ')';
- return grub_errno;
+ ret = grub_errno;
+ goto err;
}
dev = grub_cryptodisk_get_by_source_disk (disk);
if (dev)
- {
- grub_dprintf ("cryptodisk", "already mounted as crypto%lu\n", dev->id);
- grub_disk_close (disk);
- if (disklast)
- *disklast = ')';
- return GRUB_ERR_NONE;
- }
-
- err = grub_cryptodisk_scan_device_real (diskname, disk);
+ grub_dprintf ("cryptodisk", "already mounted as crypto%lu\n", dev->id);
+ else
+ ret = grub_cryptodisk_scan_device_real (diskname, disk);
grub_disk_close (disk);
if (disklast)
*disklast = ')';
-
- return err;
}
+
+ err:
+ if (hdr_file)
+ grub_file_close (hdr_file);
+ if (key_file)
+ grub_file_close (key_file);
+ if (mkey_file)
+ grub_file_close (mkey_file);
+
+ return ret;
}
static struct grub_disk_dev grub_cryptodisk_dev = {
@@ -1299,7 +1356,7 @@ GRUB_MOD_INIT (cryptodisk)
{
grub_disk_dev_register (&grub_cryptodisk_dev);
cmd = grub_register_extcmd ("cryptomount", grub_cmd_cryptomount, 0,
- N_("SOURCE|-u UUID|-a|-b"),
+ N_("SOURCE|-u UUID|-a|-b|-H file|-k file|-K file"),
N_("Mount a crypto device."), options);
grub_procfs_register ("luks_script", &luks_script);
}
diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index 31d7166fc..c29b472e6 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -309,13 +309,22 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, grub_luks2_digest_t *d, grub_luks2_s
/* Determine whether to use primary or secondary header */
static grub_err_t
-luks2_read_header (grub_disk_t disk, grub_luks2_header_t *outhdr)
+luks2_read_header (grub_disk_t disk, grub_file_t hdr_file, grub_luks2_header_t *outhdr)
{
grub_luks2_header_t primary, secondary, *header = &primary;
grub_err_t ret;
/* Read the primary LUKS header. */
- ret = grub_disk_read (disk, 0, 0, sizeof (primary), &primary);
+ if (hdr_file)
+ {
+ grub_file_seek (hdr_file, 0);
+ if (grub_file_read (hdr_file, &primary, sizeof (primary)) != sizeof (primary))
+ ret = GRUB_ERR_READ_ERROR;
+ else
+ ret = GRUB_ERR_NONE;
+ }
+ else
+ ret = grub_disk_read (disk, 0, 0, sizeof (primary), &primary);
if (ret)
return ret;
@@ -325,7 +334,16 @@ luks2_read_header (grub_disk_t disk, grub_luks2_header_t *outhdr)
return GRUB_ERR_BAD_SIGNATURE;
/* Read the secondary header. */
- ret = grub_disk_read (disk, 0, grub_be_to_cpu64 (primary.hdr_size), sizeof (secondary), &secondary);
+ if (hdr_file)
+ {
+ grub_file_seek (hdr_file, grub_be_to_cpu64 (primary.hdr_size));
+ if (grub_file_read (hdr_file, &secondary, sizeof (secondary)) != sizeof (secondary))
+ ret = GRUB_ERR_READ_ERROR;
+ else
+ ret = GRUB_ERR_NONE;
+ }
+ else
+ ret = grub_disk_read (disk, 0, grub_be_to_cpu64 (primary.hdr_size), sizeof (secondary), &secondary);
if (ret)
return ret;
@@ -342,7 +360,7 @@ luks2_read_header (grub_disk_t disk, grub_luks2_header_t *outhdr)
}
static grub_cryptodisk_t
-luks2_scan (grub_disk_t disk, const char *check_uuid, int check_boot)
+luks2_scan (grub_disk_t disk, grub_file_t hdr_file, const char *check_uuid, int check_boot)
{
grub_cryptodisk_t cryptodisk;
grub_luks2_header_t header;
@@ -352,7 +370,7 @@ luks2_scan (grub_disk_t disk, const char *check_uuid, int check_boot)
if (check_boot)
return NULL;
- if (luks2_read_header (disk, &header))
+ if (luks2_read_header (disk, hdr_file, &header))
{
grub_errno = GRUB_ERR_NONE;
return NULL;
@@ -415,7 +433,7 @@ luks2_verify_key (grub_luks2_digest_t *d, grub_uint8_t *candidate_key,
static grub_err_t
luks2_decrypt_key (grub_uint8_t *out_key,
- grub_disk_t disk, grub_cryptodisk_t crypt,
+ grub_disk_t disk, grub_cryptodisk_t crypt, grub_file_t hdr_file,
grub_luks2_keyslot_t *k,
const grub_uint8_t *passphrase, grub_size_t passphraselen)
{
@@ -491,7 +509,16 @@ luks2_decrypt_key (grub_uint8_t *out_key,
}
grub_errno = GRUB_ERR_NONE;
- ret = grub_disk_read (disk, 0, k->area.offset, k->area.size, split_key);
+ if (hdr_file)
+ {
+ grub_file_seek (hdr_file, k->area.offset);
+ if (grub_file_read (hdr_file, split_key, k->area.size) != k->area.size)
+ ret = GRUB_ERR_READ_ERROR;
+ else
+ ret = GRUB_ERR_NONE;
+ }
+ else
+ ret = grub_disk_read (disk, 0, k->area.offset, k->area.size, split_key);
if (ret)
{
grub_error (GRUB_ERR_IO, "Read error: %s\n", grub_errmsg);
@@ -531,12 +558,16 @@ luks2_decrypt_key (grub_uint8_t *out_key,
static grub_err_t
luks2_recover_key (grub_disk_t disk,
- grub_cryptodisk_t crypt)
+ grub_cryptodisk_t crypt,
+ grub_file_t hdr_file, grub_file_t key_file, grub_file_t mkey_file)
{
+ char cipher[32];
+ char *passphrase = NULL;
grub_uint8_t candidate_key[GRUB_CRYPTODISK_MAX_KEYLEN];
- char passphrase[MAX_PASSPHRASE], cipher[32];
+ grub_size_t candidate_key_len, passphrase_len;
char *json_header = NULL, *part = NULL, *ptr;
- grub_size_t candidate_key_len = 0, i, size;
+ grub_off_t json_header_offset;
+ grub_size_t json_header_size, i, size;
grub_luks2_header_t header;
grub_luks2_keyslot_t keyslot;
grub_luks2_digest_t digest;
@@ -545,7 +576,7 @@ luks2_recover_key (grub_disk_t disk,
grub_json_t *json = NULL, keyslots;
grub_err_t ret;
- ret = luks2_read_header (disk, &header);
+ ret = luks2_read_header (disk, hdr_file, &header);
if (ret)
return ret;
@@ -554,8 +585,19 @@ luks2_recover_key (grub_disk_t disk,
return GRUB_ERR_OUT_OF_MEMORY;
/* Read the JSON area. */
- ret = grub_disk_read (disk, 0, grub_be_to_cpu64 (header.hdr_offset) + sizeof (header),
- grub_be_to_cpu64 (header.hdr_size) - sizeof (header), json_header);
+ json_header_offset = grub_be_to_cpu64 (header.hdr_offset) + sizeof (header);
+ json_header_size = grub_be_to_cpu64 (header.hdr_size) - sizeof (header);
+
+ if (hdr_file)
+ {
+ grub_file_seek (hdr_file, json_header_offset);
+ if (grub_file_read (hdr_file, json_header, json_header_size) != json_header_size)
+ ret = GRUB_ERR_READ_ERROR;
+ else
+ ret = GRUB_ERR_NONE;
+ }
+ else
+ ret = grub_disk_read (disk, 0, json_header_offset, json_header_size, json_header);
if (ret)
goto err;
@@ -570,16 +612,47 @@ luks2_recover_key (grub_disk_t disk,
goto err;
}
- /* Get the passphrase from the user. */
- if (disk->partition)
- part = grub_partition_get_name (disk->partition);
- grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), disk->name,
- disk->partition ? "," : "", part ? : "",
- crypt->uuid);
- if (!grub_password_get (passphrase, MAX_PASSPHRASE))
+ if (mkey_file)
{
- ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not supplied");
- goto err;
+ /* Get the master key from file. */
+ candidate_key_len = grub_file_read (mkey_file, candidate_key, GRUB_CRYPTODISK_MAX_KEYLEN);
+ if (candidate_key_len == (grub_size_t)-1)
+ {
+ ret = grub_error (GRUB_ERR_FILE_READ_ERROR, "Error reading key file");
+ goto err;
+ }
+ if (candidate_key_len == 0)
+ {
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Key not supplied");
+ goto err;
+ }
+ }
+ else if (key_file)
+ {
+ /* Get the passphrase from file. */
+ passphrase = grub_malloc (GRUB_CRYPTODISK_MAX_PASSPHRASE);
+ passphrase_len = grub_file_read (key_file, passphrase, GRUB_CRYPTODISK_MAX_PASSPHRASE);
+ if (passphrase_len == (grub_size_t)-1)
+ {
+ ret = grub_error (GRUB_ERR_FILE_READ_ERROR, "Error reading passphrase file");
+ goto err;
+ }
+ }
+ else
+ {
+ /* Get the passphrase from the user. */
+ if (disk->partition)
+ part = grub_partition_get_name (disk->partition);
+ grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), disk->name,
+ disk->partition ? "," : "", part ? : "",
+ crypt->uuid);
+ passphrase = grub_malloc (MAX_PASSPHRASE);
+ if (!grub_password_get (passphrase, MAX_PASSPHRASE))
+ {
+ ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not supplied");
+ goto err;
+ }
+ passphrase_len = grub_strlen (passphrase);
}
if (grub_json_getvalue (&keyslots, json, "keyslots") ||
@@ -604,6 +677,26 @@ luks2_recover_key (grub_disk_t disk,
grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_SIZE"\n", i);
+ if (mkey_file)
+ {
+ if (candidate_key_len != keyslot.key_size)
+ {
+ grub_dprintf ("luks2", "Wrong key length for keyslot %"PRIuGRUB_SIZE": %s\n",
+ i, grub_errmsg);
+ continue;
+ }
+
+ ret = luks2_verify_key (&digest, candidate_key, candidate_key_len);
+ if (ret)
+ {
+ grub_dprintf ("luks2", "Could not open keyslot %"PRIuGRUB_SIZE": %s\n",
+ i, grub_errmsg);
+ continue;
+ }
+ }
+ else
+ candidate_key_len = keyslot.key_size;
+
/* Set up disk according to keyslot's segment. */
crypt->offset = grub_divmod64 (segment.offset, segment.sector_size, NULL);
crypt->log_sector_size = sizeof (unsigned int) * 8
@@ -613,21 +706,24 @@ luks2_recover_key (grub_disk_t disk,
else
crypt->total_length = grub_strtoull (segment.size, NULL, 10);
- ret = luks2_decrypt_key (candidate_key, disk, crypt, &keyslot,
- (const grub_uint8_t *) passphrase, grub_strlen (passphrase));
- if (ret)
+ if (passphrase)
{
- grub_dprintf ("luks2", "Decryption with keyslot %"PRIuGRUB_SIZE" failed: %s\n",
- i, grub_errmsg);
- continue;
- }
-
- ret = luks2_verify_key (&digest, candidate_key, keyslot.key_size);
- if (ret)
- {
- grub_dprintf ("luks2", "Could not open keyslot %"PRIuGRUB_SIZE": %s\n",
- i, grub_errmsg);
- continue;
+ ret = luks2_decrypt_key (candidate_key, disk, crypt, hdr_file, &keyslot,
+ (const grub_uint8_t *) passphrase, passphrase_len);
+ if (ret)
+ {
+ grub_dprintf ("luks2", "Decryption with keyslot %"PRIuGRUB_SIZE" failed: %s\n",
+ i, grub_errmsg);
+ continue;
+ }
+
+ ret = luks2_verify_key (&digest, candidate_key, candidate_key_len);
+ if (ret)
+ {
+ grub_dprintf ("luks2", "Could not open keyslot %"PRIuGRUB_SIZE": %s\n",
+ i, grub_errmsg);
+ continue;
+ }
}
/*
@@ -635,13 +731,14 @@ luks2_recover_key (grub_disk_t disk,
* where each element is either empty or holds a key.
*/
grub_printf_ (N_("Slot %"PRIuGRUB_SIZE" opened\n"), i);
-
- candidate_key_len = keyslot.key_size;
break;
}
- if (candidate_key_len == 0)
+ if (i == size)
{
- ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid passphrase");
+ if (mkey_file)
+ ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid key");
+ else
+ ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid passphrase");
goto err;
}
@@ -666,6 +763,7 @@ luks2_recover_key (grub_disk_t disk,
err:
grub_free (part);
+ grub_free (passphrase);
grub_free (json_header);
grub_json_free (json);
return ret;
diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
index e1b21e785..b53006854 100644
--- a/include/grub/cryptodisk.h
+++ b/include/grub/cryptodisk.h
@@ -21,6 +21,7 @@
#include <grub/disk.h>
#include <grub/crypto.h>
+#include <grub/file.h>
#include <grub/list.h>
#ifdef GRUB_UTIL
#include <grub/emu/hostdisk.h>
@@ -53,6 +54,7 @@ typedef enum
#define GRUB_CRYPTODISK_GF_LOG_BYTES (GRUB_CRYPTODISK_GF_LOG_SIZE - 3)
#define GRUB_CRYPTODISK_GF_BYTES (1U << GRUB_CRYPTODISK_GF_LOG_BYTES)
#define GRUB_CRYPTODISK_MAX_KEYLEN 128
+#define GRUB_CRYPTODISK_MAX_PASSPHRASE 8192
struct grub_cryptodisk;
@@ -106,9 +108,10 @@ struct grub_cryptodisk_dev
struct grub_cryptodisk_dev *next;
struct grub_cryptodisk_dev **prev;
- grub_cryptodisk_t (*scan) (grub_disk_t disk, const char *check_uuid,
- int boot_only);
- grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t dev);
+ grub_cryptodisk_t (*scan) (grub_disk_t disk, grub_file_t hdr_file,
+ const char *check_uuid, int boot_only);
+ grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t dev,
+ grub_file_t hdr_file, grub_file_t key_file, grub_file_t mkey_file);
};
typedef struct grub_cryptodisk_dev *grub_cryptodisk_dev_t;
--
2.29.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] Cryptomount support for key files and detached header
2020-11-07 16:36 [PATCH] Cryptomount support for key files and detached header reagentoo
@ 2020-11-09 3:16 ` Glenn Washburn
2020-11-09 7:56 ` Dmitry
0 siblings, 1 reply; 7+ messages in thread
From: Glenn Washburn @ 2020-11-09 3:16 UTC (permalink / raw)
To: reagentoo; +Cc: The development of GNU GRUB
I've read through the patch but not applied or tested it. However, it
looks like it does the job. In fact, its fairly similar in parts to a
patch, which adds LUKS2 keyfile and detached header support, I've been
waiting to send to the list until the previous LUKS1 keyfile and
detached header support gets included (which my patches rely on).
One implementation difference in this patch from the other one is that
this one passes the key file to recover_key as a grub_file_t instead of
passing the key contents. I prefer the previous method because it
consolidates key file reading in grub_cmd_cryptomount, whereas this
patch requires each module to do its own reading. The disadvantage is
this causes extra code in the modules that could be consolidated and I
don't see much advantage to this. By having grub_cmd_cryptomount do
the work we also get keyfile-offset and keyfile-size options in the
previous patch, for not much extra work.
A minor nit, there is not error checking on grub_file_seek calls and
grub_file_read errors are occluded. However, there are some changes in
this patch which I think make the code a bit more aesthetically
pleasing.
While, I like that this patch allows for passing a master key file, I
think this feature should be build on the previous patch series. So I
don't recommend this patch be accepted.
Glenn
On Sat, 7 Nov 2020 19:36:35 +0300
reagentoo <reagentoo@gmail.com> wrote:
> From: Dmitry Baranov <reagentoo@gmail.com>
>
> Hello.
> Could someone make initial review?
> The main difference between this patch and the previous ones is
> the ability to use a master key file with a detached header.
> This patch does not provide luks1 and geli support yet (luks2 only).
>
> Best Regards,
> Dmitry
>
> ---
> grub-core/disk/cryptodisk.c | 99 +++++++++++++++-----
> grub-core/disk/luks2.c | 178
> ++++++++++++++++++++++++++++-------- include/grub/cryptodisk.h |
> 9 +- 3 files changed, 222 insertions(+), 64 deletions(-)
>
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index 13af84dd1..987a39b16 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -41,6 +41,9 @@ static const struct grub_arg_option options[] =
> /* TRANSLATORS: It's still restricted to cryptodisks only. */
> {"all", 'a', 0, N_("Mount all."), 0, 0},
> {"boot", 'b', 0, N_("Mount all volumes with `boot' flag set."),
> 0, 0},
> + {"header", 'H', 0, N_("Read header from file"), 0,
> ARG_TYPE_STRING},
> + {"key-file", 'k', 0, N_("Read key from file"), 0,
> ARG_TYPE_STRING},
> + {"master-key-file", 'K', 0, N_("Use a master key stored in a
> file"), 0, ARG_TYPE_STRING}, {0, 0, 0, 0, 0, 0}
> };
>
> @@ -967,6 +970,7 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk)
>
> static int check_boot, have_it;
> static char *search_uuid;
> +static grub_file_t hdr_file, key_file, mkey_file;
>
> static void
> cryptodisk_close (grub_cryptodisk_t dev)
> @@ -991,13 +995,13 @@ grub_cryptodisk_scan_device_real (const char
> *name, grub_disk_t source)
> FOR_CRYPTODISK_DEVS (cr)
> {
> - dev = cr->scan (source, search_uuid, check_boot);
> + dev = cr->scan (source, hdr_file, search_uuid, check_boot);
> if (grub_errno)
> return grub_errno;
> if (!dev)
> continue;
>
> - err = cr->recover_key (source, dev);
> + err = cr->recover_key (source, dev, hdr_file, key_file,
> mkey_file); if (err)
> {
> cryptodisk_close (dev);
> @@ -1038,7 +1042,7 @@ grub_cryptodisk_cheat_mount (const char
> *sourcedev, const char *cheat)
> FOR_CRYPTODISK_DEVS (cr)
> {
> - dev = cr->scan (source, search_uuid, check_boot);
> + dev = cr->scan (source, 0, search_uuid, check_boot);
> if (grub_errno)
> return grub_errno;
> if (!dev)
> @@ -1087,11 +1091,59 @@ grub_cryptodisk_scan_device (const char *name,
> static grub_err_t
> grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char
> **args) {
> + grub_err_t ret = GRUB_ERR_NONE;
> +
> struct grub_arg_list *state = ctxt->state;
>
> if (argc < 1 && !state[1].set && !state[2].set)
> return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name
> required");
> + hdr_file = NULL;
> + key_file = NULL;
> + mkey_file = NULL;
> +
> + if (state[3].set)
> + {
> + if (state[0].set)
> + {
> + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot use UUID
> lookup with detached header");
> + goto err;
> + }
> +
> + hdr_file = grub_file_open (state[3].arg, GRUB_FILE_TYPE_NONE);
> + if (!hdr_file)
> + {
> + ret = grub_errno;
> + goto err;
> + }
> + }
> +
> + if (state[4].set)
> + {
> + if (state[5].set)
> + {
> + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot use key
> with master key");
> + goto err;
> + }
> +
> + key_file = grub_file_open (state[4].arg, GRUB_FILE_TYPE_NONE);
> + if (!key_file)
> + {
> + ret = grub_errno;
> + goto err;
> + }
> + }
> +
> + if (state[5].set)
> + {
> + mkey_file = grub_file_open (state[5].arg, GRUB_FILE_TYPE_NONE);
> + if (!mkey_file)
> + {
> + ret = grub_errno;
> + goto err;
> + }
> + }
> +
> have_it = 0;
> if (state[0].set)
> {
> @@ -1102,7 +1154,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> ctxt, int argc, char **args) {
> grub_dprintf ("cryptodisk",
> "already mounted as crypto%lu\n", dev->id);
> - return GRUB_ERR_NONE;
> + goto err;
> }
>
> check_boot = state[2].set;
> @@ -1111,8 +1163,11 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> ctxt, int argc, char **args) search_uuid = NULL;
>
> if (!have_it)
> - return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such
> cryptodisk found");
> - return GRUB_ERR_NONE;
> + {
> + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "no such
> cryptodisk found");
> + goto err;
> + }
> + goto err;
> }
> else if (state[1].set || (argc == 0 && state[2].set))
> {
> @@ -1120,11 +1175,10 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> ctxt, int argc, char **args) check_boot = state[2].set;
> grub_device_iterate (&grub_cryptodisk_scan_device, NULL);
> search_uuid = NULL;
> - return GRUB_ERR_NONE;
> + goto err;
> }
> else
> {
> - grub_err_t err;
> grub_disk_t disk;
> grub_cryptodisk_t dev;
> char *diskname;
> @@ -1147,27 +1201,30 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> ctxt, int argc, char **args) {
> if (disklast)
> *disklast = ')';
> - return grub_errno;
> + ret = grub_errno;
> + goto err;
> }
>
> dev = grub_cryptodisk_get_by_source_disk (disk);
> if (dev)
> - {
> - grub_dprintf ("cryptodisk", "already mounted as
> crypto%lu\n", dev->id);
> - grub_disk_close (disk);
> - if (disklast)
> - *disklast = ')';
> - return GRUB_ERR_NONE;
> - }
> -
> - err = grub_cryptodisk_scan_device_real (diskname, disk);
> + grub_dprintf ("cryptodisk", "already mounted as
> crypto%lu\n", dev->id);
> + else
> + ret = grub_cryptodisk_scan_device_real (diskname, disk);
>
> grub_disk_close (disk);
> if (disklast)
> *disklast = ')';
> -
> - return err;
> }
> +
> + err:
> + if (hdr_file)
> + grub_file_close (hdr_file);
> + if (key_file)
> + grub_file_close (key_file);
> + if (mkey_file)
> + grub_file_close (mkey_file);
> +
> + return ret;
> }
>
> static struct grub_disk_dev grub_cryptodisk_dev = {
> @@ -1299,7 +1356,7 @@ GRUB_MOD_INIT (cryptodisk)
> {
> grub_disk_dev_register (&grub_cryptodisk_dev);
> cmd = grub_register_extcmd ("cryptomount", grub_cmd_cryptomount, 0,
> - N_("SOURCE|-u UUID|-a|-b"),
> + N_("SOURCE|-u UUID|-a|-b|-H file|-k
> file|-K file"), N_("Mount a crypto device."), options);
> grub_procfs_register ("luks_script", &luks_script);
> }
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index 31d7166fc..c29b472e6 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -309,13 +309,22 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k,
> grub_luks2_digest_t *d, grub_luks2_s
> /* Determine whether to use primary or secondary header */
> static grub_err_t
> -luks2_read_header (grub_disk_t disk, grub_luks2_header_t *outhdr)
> +luks2_read_header (grub_disk_t disk, grub_file_t hdr_file,
> grub_luks2_header_t *outhdr) {
> grub_luks2_header_t primary, secondary, *header = &primary;
> grub_err_t ret;
>
> /* Read the primary LUKS header. */
> - ret = grub_disk_read (disk, 0, 0, sizeof (primary), &primary);
> + if (hdr_file)
> + {
> + grub_file_seek (hdr_file, 0);
> + if (grub_file_read (hdr_file, &primary, sizeof (primary)) !=
> sizeof (primary))
> + ret = GRUB_ERR_READ_ERROR;
> + else
> + ret = GRUB_ERR_NONE;
> + }
> + else
> + ret = grub_disk_read (disk, 0, 0, sizeof (primary), &primary);
> if (ret)
> return ret;
>
> @@ -325,7 +334,16 @@ luks2_read_header (grub_disk_t disk,
> grub_luks2_header_t *outhdr) return GRUB_ERR_BAD_SIGNATURE;
>
> /* Read the secondary header. */
> - ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> (primary.hdr_size), sizeof (secondary), &secondary);
> + if (hdr_file)
> + {
> + grub_file_seek (hdr_file, grub_be_to_cpu64 (primary.hdr_size));
> + if (grub_file_read (hdr_file, &secondary, sizeof (secondary))
> != sizeof (secondary))
> + ret = GRUB_ERR_READ_ERROR;
> + else
> + ret = GRUB_ERR_NONE;
> + }
> + else
> + ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> (primary.hdr_size), sizeof (secondary), &secondary); if (ret)
> return ret;
>
> @@ -342,7 +360,7 @@ luks2_read_header (grub_disk_t disk,
> grub_luks2_header_t *outhdr) }
>
> static grub_cryptodisk_t
> -luks2_scan (grub_disk_t disk, const char *check_uuid, int check_boot)
> +luks2_scan (grub_disk_t disk, grub_file_t hdr_file, const char
> *check_uuid, int check_boot) {
> grub_cryptodisk_t cryptodisk;
> grub_luks2_header_t header;
> @@ -352,7 +370,7 @@ luks2_scan (grub_disk_t disk, const char
> *check_uuid, int check_boot) if (check_boot)
> return NULL;
>
> - if (luks2_read_header (disk, &header))
> + if (luks2_read_header (disk, hdr_file, &header))
> {
> grub_errno = GRUB_ERR_NONE;
> return NULL;
> @@ -415,7 +433,7 @@ luks2_verify_key (grub_luks2_digest_t *d,
> grub_uint8_t *candidate_key,
> static grub_err_t
> luks2_decrypt_key (grub_uint8_t *out_key,
> - grub_disk_t disk, grub_cryptodisk_t crypt,
> + grub_disk_t disk, grub_cryptodisk_t crypt,
> grub_file_t hdr_file, grub_luks2_keyslot_t *k,
> const grub_uint8_t *passphrase, grub_size_t
> passphraselen) {
> @@ -491,7 +509,16 @@ luks2_decrypt_key (grub_uint8_t *out_key,
> }
>
> grub_errno = GRUB_ERR_NONE;
> - ret = grub_disk_read (disk, 0, k->area.offset, k->area.size,
> split_key);
> + if (hdr_file)
> + {
> + grub_file_seek (hdr_file, k->area.offset);
> + if (grub_file_read (hdr_file, split_key, k->area.size) !=
> k->area.size)
> + ret = GRUB_ERR_READ_ERROR;
> + else
> + ret = GRUB_ERR_NONE;
> + }
> + else
> + ret = grub_disk_read (disk, 0, k->area.offset, k->area.size,
> split_key); if (ret)
> {
> grub_error (GRUB_ERR_IO, "Read error: %s\n", grub_errmsg);
> @@ -531,12 +558,16 @@ luks2_decrypt_key (grub_uint8_t *out_key,
>
> static grub_err_t
> luks2_recover_key (grub_disk_t disk,
> - grub_cryptodisk_t crypt)
> + grub_cryptodisk_t crypt,
> + grub_file_t hdr_file, grub_file_t key_file,
> grub_file_t mkey_file) {
> + char cipher[32];
> + char *passphrase = NULL;
> grub_uint8_t candidate_key[GRUB_CRYPTODISK_MAX_KEYLEN];
> - char passphrase[MAX_PASSPHRASE], cipher[32];
> + grub_size_t candidate_key_len, passphrase_len;
> char *json_header = NULL, *part = NULL, *ptr;
> - grub_size_t candidate_key_len = 0, i, size;
> + grub_off_t json_header_offset;
> + grub_size_t json_header_size, i, size;
> grub_luks2_header_t header;
> grub_luks2_keyslot_t keyslot;
> grub_luks2_digest_t digest;
> @@ -545,7 +576,7 @@ luks2_recover_key (grub_disk_t disk,
> grub_json_t *json = NULL, keyslots;
> grub_err_t ret;
>
> - ret = luks2_read_header (disk, &header);
> + ret = luks2_read_header (disk, hdr_file, &header);
> if (ret)
> return ret;
>
> @@ -554,8 +585,19 @@ luks2_recover_key (grub_disk_t disk,
> return GRUB_ERR_OUT_OF_MEMORY;
>
> /* Read the JSON area. */
> - ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> (header.hdr_offset) + sizeof (header),
> - grub_be_to_cpu64 (header.hdr_size) - sizeof
> (header), json_header);
> + json_header_offset = grub_be_to_cpu64 (header.hdr_offset) + sizeof
> (header);
> + json_header_size = grub_be_to_cpu64 (header.hdr_size) - sizeof
> (header); +
> + if (hdr_file)
> + {
> + grub_file_seek (hdr_file, json_header_offset);
> + if (grub_file_read (hdr_file, json_header, json_header_size)
> != json_header_size)
> + ret = GRUB_ERR_READ_ERROR;
> + else
> + ret = GRUB_ERR_NONE;
> + }
> + else
> + ret = grub_disk_read (disk, 0, json_header_offset,
> json_header_size, json_header); if (ret)
> goto err;
>
> @@ -570,16 +612,47 @@ luks2_recover_key (grub_disk_t disk,
> goto err;
> }
>
> - /* Get the passphrase from the user. */
> - if (disk->partition)
> - part = grub_partition_get_name (disk->partition);
> - grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), disk->name,
> - disk->partition ? "," : "", part ? : "",
> - crypt->uuid);
> - if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> + if (mkey_file)
> {
> - ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not
> supplied");
> - goto err;
> + /* Get the master key from file. */
> + candidate_key_len = grub_file_read (mkey_file, candidate_key,
> GRUB_CRYPTODISK_MAX_KEYLEN);
> + if (candidate_key_len == (grub_size_t)-1)
> + {
> + ret = grub_error (GRUB_ERR_FILE_READ_ERROR, "Error reading
> key file");
> + goto err;
> + }
> + if (candidate_key_len == 0)
> + {
> + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Key not
> supplied");
> + goto err;
> + }
> + }
> + else if (key_file)
> + {
> + /* Get the passphrase from file. */
> + passphrase = grub_malloc (GRUB_CRYPTODISK_MAX_PASSPHRASE);
> + passphrase_len = grub_file_read (key_file, passphrase,
> GRUB_CRYPTODISK_MAX_PASSPHRASE);
> + if (passphrase_len == (grub_size_t)-1)
> + {
> + ret = grub_error (GRUB_ERR_FILE_READ_ERROR, "Error reading
> passphrase file");
> + goto err;
> + }
> + }
> + else
> + {
> + /* Get the passphrase from the user. */
> + if (disk->partition)
> + part = grub_partition_get_name (disk->partition);
> + grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "),
> disk->name,
> + disk->partition ? "," : "", part ? : "",
> + crypt->uuid);
> + passphrase = grub_malloc (MAX_PASSPHRASE);
> + if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> + {
> + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not
> supplied");
> + goto err;
> + }
> + passphrase_len = grub_strlen (passphrase);
> }
>
> if (grub_json_getvalue (&keyslots, json, "keyslots") ||
> @@ -604,6 +677,26 @@ luks2_recover_key (grub_disk_t disk,
>
> grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_SIZE"\n", i);
>
> + if (mkey_file)
> + {
> + if (candidate_key_len != keyslot.key_size)
> + {
> + grub_dprintf ("luks2", "Wrong key length for keyslot
> %"PRIuGRUB_SIZE": %s\n",
> + i, grub_errmsg);
> + continue;
> + }
> +
> + ret = luks2_verify_key (&digest, candidate_key,
> candidate_key_len);
> + if (ret)
> + {
> + grub_dprintf ("luks2", "Could not open keyslot
> %"PRIuGRUB_SIZE": %s\n",
> + i, grub_errmsg);
> + continue;
> + }
> + }
> + else
> + candidate_key_len = keyslot.key_size;
> +
> /* Set up disk according to keyslot's segment. */
> crypt->offset = grub_divmod64 (segment.offset,
> segment.sector_size, NULL); crypt->log_sector_size = sizeof (unsigned
> int) * 8 @@ -613,21 +706,24 @@ luks2_recover_key (grub_disk_t disk,
> else
> crypt->total_length = grub_strtoull (segment.size, NULL, 10);
>
> - ret = luks2_decrypt_key (candidate_key, disk, crypt, &keyslot,
> - (const grub_uint8_t *) passphrase,
> grub_strlen (passphrase));
> - if (ret)
> + if (passphrase)
> {
> - grub_dprintf ("luks2", "Decryption with keyslot
> %"PRIuGRUB_SIZE" failed: %s\n",
> - i, grub_errmsg);
> - continue;
> - }
> -
> - ret = luks2_verify_key (&digest, candidate_key,
> keyslot.key_size);
> - if (ret)
> - {
> - grub_dprintf ("luks2", "Could not open keyslot
> %"PRIuGRUB_SIZE": %s\n",
> - i, grub_errmsg);
> - continue;
> + ret = luks2_decrypt_key (candidate_key, disk, crypt,
> hdr_file, &keyslot,
> + (const grub_uint8_t *)
> passphrase, passphrase_len);
> + if (ret)
> + {
> + grub_dprintf ("luks2", "Decryption with keyslot
> %"PRIuGRUB_SIZE" failed: %s\n",
> + i, grub_errmsg);
> + continue;
> + }
> +
> + ret = luks2_verify_key (&digest, candidate_key,
> candidate_key_len);
> + if (ret)
> + {
> + grub_dprintf ("luks2", "Could not open keyslot
> %"PRIuGRUB_SIZE": %s\n",
> + i, grub_errmsg);
> + continue;
> + }
> }
>
> /*
> @@ -635,13 +731,14 @@ luks2_recover_key (grub_disk_t disk,
> * where each element is either empty or holds a key.
> */
> grub_printf_ (N_("Slot %"PRIuGRUB_SIZE" opened\n"), i);
> -
> - candidate_key_len = keyslot.key_size;
> break;
> }
> - if (candidate_key_len == 0)
> + if (i == size)
> {
> - ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid
> passphrase");
> + if (mkey_file)
> + ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid key");
> + else
> + ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid
> passphrase"); goto err;
> }
>
> @@ -666,6 +763,7 @@ luks2_recover_key (grub_disk_t disk,
>
> err:
> grub_free (part);
> + grub_free (passphrase);
> grub_free (json_header);
> grub_json_free (json);
> return ret;
> diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
> index e1b21e785..b53006854 100644
> --- a/include/grub/cryptodisk.h
> +++ b/include/grub/cryptodisk.h
> @@ -21,6 +21,7 @@
>
> #include <grub/disk.h>
> #include <grub/crypto.h>
> +#include <grub/file.h>
> #include <grub/list.h>
> #ifdef GRUB_UTIL
> #include <grub/emu/hostdisk.h>
> @@ -53,6 +54,7 @@ typedef enum
> #define GRUB_CRYPTODISK_GF_LOG_BYTES (GRUB_CRYPTODISK_GF_LOG_SIZE -
> 3) #define GRUB_CRYPTODISK_GF_BYTES (1U <<
> GRUB_CRYPTODISK_GF_LOG_BYTES) #define GRUB_CRYPTODISK_MAX_KEYLEN 128
> +#define GRUB_CRYPTODISK_MAX_PASSPHRASE 8192
>
> struct grub_cryptodisk;
>
> @@ -106,9 +108,10 @@ struct grub_cryptodisk_dev
> struct grub_cryptodisk_dev *next;
> struct grub_cryptodisk_dev **prev;
>
> - grub_cryptodisk_t (*scan) (grub_disk_t disk, const char
> *check_uuid,
> - int boot_only);
> - grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t
> dev);
> + grub_cryptodisk_t (*scan) (grub_disk_t disk, grub_file_t hdr_file,
> + const char *check_uuid, int boot_only);
> + grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t dev,
> + grub_file_t hdr_file, grub_file_t
> key_file, grub_file_t mkey_file); };
> typedef struct grub_cryptodisk_dev *grub_cryptodisk_dev_t;
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Cryptomount support for key files and detached header
2020-11-09 3:16 ` Glenn Washburn
@ 2020-11-09 7:56 ` Dmitry
2020-11-09 9:42 ` Dmitry
2020-11-09 21:34 ` Glenn Washburn
0 siblings, 2 replies; 7+ messages in thread
From: Dmitry @ 2020-11-09 7:56 UTC (permalink / raw)
To: development; +Cc: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 22793 bytes --]
Thanks for feedback. First of all I think it's preferable to introduce
the master-key option at the beginning. Because I see no point for a user
to use a standard slot key along with a detached header.
Decryption from key to master key takes a long time. (30 seconds for
argon2i).
Regarding keyfile-offset and keyfile-size options I suggest to introduce
this options in separate patches. Yes, maybe you variant to pass the
pointer and size
(correct me if I am wrong) to luks2_recover_key is better of files.
But then you need to think in advance how to add master-key option.
Dmitry
пн, 9 нояб. 2020 г. в 06:16, Glenn Washburn <development@efficientek.com>:
> I've read through the patch but not applied or tested it. However, it
> looks like it does the job. In fact, its fairly similar in parts to a
> patch, which adds LUKS2 keyfile and detached header support, I've been
> waiting to send to the list until the previous LUKS1 keyfile and
> detached header support gets included (which my patches rely on).
>
> One implementation difference in this patch from the other one is that
> this one passes the key file to recover_key as a grub_file_t instead of
> passing the key contents. I prefer the previous method because it
> consolidates key file reading in grub_cmd_cryptomount, whereas this
> patch requires each module to do its own reading. The disadvantage is
> this causes extra code in the modules that could be consolidated and I
> don't see much advantage to this. By having grub_cmd_cryptomount do
> the work we also get keyfile-offset and keyfile-size options in the
> previous patch, for not much extra work.
>
> A minor nit, there is not error checking on grub_file_seek calls and
> grub_file_read errors are occluded. However, there are some changes in
> this patch which I think make the code a bit more aesthetically
> pleasing.
>
> While, I like that this patch allows for passing a master key file, I
> think this feature should be build on the previous patch series. So I
> don't recommend this patch be accepted.
>
> Glenn
>
> On Sat, 7 Nov 2020 19:36:35 +0300
> reagentoo <reagentoo@gmail.com> wrote:
>
> > From: Dmitry Baranov <reagentoo@gmail.com>
> >
> > Hello.
> > Could someone make initial review?
> > The main difference between this patch and the previous ones is
> > the ability to use a master key file with a detached header.
> > This patch does not provide luks1 and geli support yet (luks2 only).
> >
> > Best Regards,
> > Dmitry
> >
> > ---
> > grub-core/disk/cryptodisk.c | 99 +++++++++++++++-----
> > grub-core/disk/luks2.c | 178
> > ++++++++++++++++++++++++++++-------- include/grub/cryptodisk.h |
> > 9 +- 3 files changed, 222 insertions(+), 64 deletions(-)
> >
> > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> > index 13af84dd1..987a39b16 100644
> > --- a/grub-core/disk/cryptodisk.c
> > +++ b/grub-core/disk/cryptodisk.c
> > @@ -41,6 +41,9 @@ static const struct grub_arg_option options[] =
> > /* TRANSLATORS: It's still restricted to cryptodisks only. */
> > {"all", 'a', 0, N_("Mount all."), 0, 0},
> > {"boot", 'b', 0, N_("Mount all volumes with `boot' flag set."),
> > 0, 0},
> > + {"header", 'H', 0, N_("Read header from file"), 0,
> > ARG_TYPE_STRING},
> > + {"key-file", 'k', 0, N_("Read key from file"), 0,
> > ARG_TYPE_STRING},
> > + {"master-key-file", 'K', 0, N_("Use a master key stored in a
> > file"), 0, ARG_TYPE_STRING}, {0, 0, 0, 0, 0, 0}
> > };
> >
> > @@ -967,6 +970,7 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk)
> >
> > static int check_boot, have_it;
> > static char *search_uuid;
> > +static grub_file_t hdr_file, key_file, mkey_file;
> >
> > static void
> > cryptodisk_close (grub_cryptodisk_t dev)
> > @@ -991,13 +995,13 @@ grub_cryptodisk_scan_device_real (const char
> > *name, grub_disk_t source)
> > FOR_CRYPTODISK_DEVS (cr)
> > {
> > - dev = cr->scan (source, search_uuid, check_boot);
> > + dev = cr->scan (source, hdr_file, search_uuid, check_boot);
> > if (grub_errno)
> > return grub_errno;
> > if (!dev)
> > continue;
> >
> > - err = cr->recover_key (source, dev);
> > + err = cr->recover_key (source, dev, hdr_file, key_file,
> > mkey_file); if (err)
> > {
> > cryptodisk_close (dev);
> > @@ -1038,7 +1042,7 @@ grub_cryptodisk_cheat_mount (const char
> > *sourcedev, const char *cheat)
> > FOR_CRYPTODISK_DEVS (cr)
> > {
> > - dev = cr->scan (source, search_uuid, check_boot);
> > + dev = cr->scan (source, 0, search_uuid, check_boot);
> > if (grub_errno)
> > return grub_errno;
> > if (!dev)
> > @@ -1087,11 +1091,59 @@ grub_cryptodisk_scan_device (const char *name,
> > static grub_err_t
> > grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char
> > **args) {
> > + grub_err_t ret = GRUB_ERR_NONE;
> > +
> > struct grub_arg_list *state = ctxt->state;
> >
> > if (argc < 1 && !state[1].set && !state[2].set)
> > return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name
> > required");
> > + hdr_file = NULL;
> > + key_file = NULL;
> > + mkey_file = NULL;
> > +
> > + if (state[3].set)
> > + {
> > + if (state[0].set)
> > + {
> > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot use UUID
> > lookup with detached header");
> > + goto err;
> > + }
> > +
> > + hdr_file = grub_file_open (state[3].arg, GRUB_FILE_TYPE_NONE);
> > + if (!hdr_file)
> > + {
> > + ret = grub_errno;
> > + goto err;
> > + }
> > + }
> > +
> > + if (state[4].set)
> > + {
> > + if (state[5].set)
> > + {
> > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot use key
> > with master key");
> > + goto err;
> > + }
> > +
> > + key_file = grub_file_open (state[4].arg, GRUB_FILE_TYPE_NONE);
> > + if (!key_file)
> > + {
> > + ret = grub_errno;
> > + goto err;
> > + }
> > + }
> > +
> > + if (state[5].set)
> > + {
> > + mkey_file = grub_file_open (state[5].arg, GRUB_FILE_TYPE_NONE);
> > + if (!mkey_file)
> > + {
> > + ret = grub_errno;
> > + goto err;
> > + }
> > + }
> > +
> > have_it = 0;
> > if (state[0].set)
> > {
> > @@ -1102,7 +1154,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > ctxt, int argc, char **args) {
> > grub_dprintf ("cryptodisk",
> > "already mounted as crypto%lu\n", dev->id);
> > - return GRUB_ERR_NONE;
> > + goto err;
> > }
> >
> > check_boot = state[2].set;
> > @@ -1111,8 +1163,11 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > ctxt, int argc, char **args) search_uuid = NULL;
> >
> > if (!have_it)
> > - return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such
> > cryptodisk found");
> > - return GRUB_ERR_NONE;
> > + {
> > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "no such
> > cryptodisk found");
> > + goto err;
> > + }
> > + goto err;
> > }
> > else if (state[1].set || (argc == 0 && state[2].set))
> > {
> > @@ -1120,11 +1175,10 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > ctxt, int argc, char **args) check_boot = state[2].set;
> > grub_device_iterate (&grub_cryptodisk_scan_device, NULL);
> > search_uuid = NULL;
> > - return GRUB_ERR_NONE;
> > + goto err;
> > }
> > else
> > {
> > - grub_err_t err;
> > grub_disk_t disk;
> > grub_cryptodisk_t dev;
> > char *diskname;
> > @@ -1147,27 +1201,30 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > ctxt, int argc, char **args) {
> > if (disklast)
> > *disklast = ')';
> > - return grub_errno;
> > + ret = grub_errno;
> > + goto err;
> > }
> >
> > dev = grub_cryptodisk_get_by_source_disk (disk);
> > if (dev)
> > - {
> > - grub_dprintf ("cryptodisk", "already mounted as
> > crypto%lu\n", dev->id);
> > - grub_disk_close (disk);
> > - if (disklast)
> > - *disklast = ')';
> > - return GRUB_ERR_NONE;
> > - }
> > -
> > - err = grub_cryptodisk_scan_device_real (diskname, disk);
> > + grub_dprintf ("cryptodisk", "already mounted as
> > crypto%lu\n", dev->id);
> > + else
> > + ret = grub_cryptodisk_scan_device_real (diskname, disk);
> >
> > grub_disk_close (disk);
> > if (disklast)
> > *disklast = ')';
> > -
> > - return err;
> > }
> > +
> > + err:
> > + if (hdr_file)
> > + grub_file_close (hdr_file);
> > + if (key_file)
> > + grub_file_close (key_file);
> > + if (mkey_file)
> > + grub_file_close (mkey_file);
> > +
> > + return ret;
> > }
> >
> > static struct grub_disk_dev grub_cryptodisk_dev = {
> > @@ -1299,7 +1356,7 @@ GRUB_MOD_INIT (cryptodisk)
> > {
> > grub_disk_dev_register (&grub_cryptodisk_dev);
> > cmd = grub_register_extcmd ("cryptomount", grub_cmd_cryptomount, 0,
> > - N_("SOURCE|-u UUID|-a|-b"),
> > + N_("SOURCE|-u UUID|-a|-b|-H file|-k
> > file|-K file"), N_("Mount a crypto device."), options);
> > grub_procfs_register ("luks_script", &luks_script);
> > }
> > diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> > index 31d7166fc..c29b472e6 100644
> > --- a/grub-core/disk/luks2.c
> > +++ b/grub-core/disk/luks2.c
> > @@ -309,13 +309,22 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k,
> > grub_luks2_digest_t *d, grub_luks2_s
> > /* Determine whether to use primary or secondary header */
> > static grub_err_t
> > -luks2_read_header (grub_disk_t disk, grub_luks2_header_t *outhdr)
> > +luks2_read_header (grub_disk_t disk, grub_file_t hdr_file,
> > grub_luks2_header_t *outhdr) {
> > grub_luks2_header_t primary, secondary, *header = &primary;
> > grub_err_t ret;
> >
> > /* Read the primary LUKS header. */
> > - ret = grub_disk_read (disk, 0, 0, sizeof (primary), &primary);
> > + if (hdr_file)
> > + {
> > + grub_file_seek (hdr_file, 0);
> > + if (grub_file_read (hdr_file, &primary, sizeof (primary)) !=
> > sizeof (primary))
> > + ret = GRUB_ERR_READ_ERROR;
> > + else
> > + ret = GRUB_ERR_NONE;
> > + }
> > + else
> > + ret = grub_disk_read (disk, 0, 0, sizeof (primary), &primary);
> > if (ret)
> > return ret;
> >
> > @@ -325,7 +334,16 @@ luks2_read_header (grub_disk_t disk,
> > grub_luks2_header_t *outhdr) return GRUB_ERR_BAD_SIGNATURE;
> >
> > /* Read the secondary header. */
> > - ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> > (primary.hdr_size), sizeof (secondary), &secondary);
> > + if (hdr_file)
> > + {
> > + grub_file_seek (hdr_file, grub_be_to_cpu64 (primary.hdr_size));
> > + if (grub_file_read (hdr_file, &secondary, sizeof (secondary))
> > != sizeof (secondary))
> > + ret = GRUB_ERR_READ_ERROR;
> > + else
> > + ret = GRUB_ERR_NONE;
> > + }
> > + else
> > + ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> > (primary.hdr_size), sizeof (secondary), &secondary); if (ret)
> > return ret;
> >
> > @@ -342,7 +360,7 @@ luks2_read_header (grub_disk_t disk,
> > grub_luks2_header_t *outhdr) }
> >
> > static grub_cryptodisk_t
> > -luks2_scan (grub_disk_t disk, const char *check_uuid, int check_boot)
> > +luks2_scan (grub_disk_t disk, grub_file_t hdr_file, const char
> > *check_uuid, int check_boot) {
> > grub_cryptodisk_t cryptodisk;
> > grub_luks2_header_t header;
> > @@ -352,7 +370,7 @@ luks2_scan (grub_disk_t disk, const char
> > *check_uuid, int check_boot) if (check_boot)
> > return NULL;
> >
> > - if (luks2_read_header (disk, &header))
> > + if (luks2_read_header (disk, hdr_file, &header))
> > {
> > grub_errno = GRUB_ERR_NONE;
> > return NULL;
> > @@ -415,7 +433,7 @@ luks2_verify_key (grub_luks2_digest_t *d,
> > grub_uint8_t *candidate_key,
> > static grub_err_t
> > luks2_decrypt_key (grub_uint8_t *out_key,
> > - grub_disk_t disk, grub_cryptodisk_t crypt,
> > + grub_disk_t disk, grub_cryptodisk_t crypt,
> > grub_file_t hdr_file, grub_luks2_keyslot_t *k,
> > const grub_uint8_t *passphrase, grub_size_t
> > passphraselen) {
> > @@ -491,7 +509,16 @@ luks2_decrypt_key (grub_uint8_t *out_key,
> > }
> >
> > grub_errno = GRUB_ERR_NONE;
> > - ret = grub_disk_read (disk, 0, k->area.offset, k->area.size,
> > split_key);
> > + if (hdr_file)
> > + {
> > + grub_file_seek (hdr_file, k->area.offset);
> > + if (grub_file_read (hdr_file, split_key, k->area.size) !=
> > k->area.size)
> > + ret = GRUB_ERR_READ_ERROR;
> > + else
> > + ret = GRUB_ERR_NONE;
> > + }
> > + else
> > + ret = grub_disk_read (disk, 0, k->area.offset, k->area.size,
> > split_key); if (ret)
> > {
> > grub_error (GRUB_ERR_IO, "Read error: %s\n", grub_errmsg);
> > @@ -531,12 +558,16 @@ luks2_decrypt_key (grub_uint8_t *out_key,
> >
> > static grub_err_t
> > luks2_recover_key (grub_disk_t disk,
> > - grub_cryptodisk_t crypt)
> > + grub_cryptodisk_t crypt,
> > + grub_file_t hdr_file, grub_file_t key_file,
> > grub_file_t mkey_file) {
> > + char cipher[32];
> > + char *passphrase = NULL;
> > grub_uint8_t candidate_key[GRUB_CRYPTODISK_MAX_KEYLEN];
> > - char passphrase[MAX_PASSPHRASE], cipher[32];
> > + grub_size_t candidate_key_len, passphrase_len;
> > char *json_header = NULL, *part = NULL, *ptr;
> > - grub_size_t candidate_key_len = 0, i, size;
> > + grub_off_t json_header_offset;
> > + grub_size_t json_header_size, i, size;
> > grub_luks2_header_t header;
> > grub_luks2_keyslot_t keyslot;
> > grub_luks2_digest_t digest;
> > @@ -545,7 +576,7 @@ luks2_recover_key (grub_disk_t disk,
> > grub_json_t *json = NULL, keyslots;
> > grub_err_t ret;
> >
> > - ret = luks2_read_header (disk, &header);
> > + ret = luks2_read_header (disk, hdr_file, &header);
> > if (ret)
> > return ret;
> >
> > @@ -554,8 +585,19 @@ luks2_recover_key (grub_disk_t disk,
> > return GRUB_ERR_OUT_OF_MEMORY;
> >
> > /* Read the JSON area. */
> > - ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> > (header.hdr_offset) + sizeof (header),
> > - grub_be_to_cpu64 (header.hdr_size) - sizeof
> > (header), json_header);
> > + json_header_offset = grub_be_to_cpu64 (header.hdr_offset) + sizeof
> > (header);
> > + json_header_size = grub_be_to_cpu64 (header.hdr_size) - sizeof
> > (header); +
> > + if (hdr_file)
> > + {
> > + grub_file_seek (hdr_file, json_header_offset);
> > + if (grub_file_read (hdr_file, json_header, json_header_size)
> > != json_header_size)
> > + ret = GRUB_ERR_READ_ERROR;
> > + else
> > + ret = GRUB_ERR_NONE;
> > + }
> > + else
> > + ret = grub_disk_read (disk, 0, json_header_offset,
> > json_header_size, json_header); if (ret)
> > goto err;
> >
> > @@ -570,16 +612,47 @@ luks2_recover_key (grub_disk_t disk,
> > goto err;
> > }
> >
> > - /* Get the passphrase from the user. */
> > - if (disk->partition)
> > - part = grub_partition_get_name (disk->partition);
> > - grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), disk->name,
> > - disk->partition ? "," : "", part ? : "",
> > - crypt->uuid);
> > - if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> > + if (mkey_file)
> > {
> > - ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not
> > supplied");
> > - goto err;
> > + /* Get the master key from file. */
> > + candidate_key_len = grub_file_read (mkey_file, candidate_key,
> > GRUB_CRYPTODISK_MAX_KEYLEN);
> > + if (candidate_key_len == (grub_size_t)-1)
> > + {
> > + ret = grub_error (GRUB_ERR_FILE_READ_ERROR, "Error reading
> > key file");
> > + goto err;
> > + }
> > + if (candidate_key_len == 0)
> > + {
> > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Key not
> > supplied");
> > + goto err;
> > + }
> > + }
> > + else if (key_file)
> > + {
> > + /* Get the passphrase from file. */
> > + passphrase = grub_malloc (GRUB_CRYPTODISK_MAX_PASSPHRASE);
> > + passphrase_len = grub_file_read (key_file, passphrase,
> > GRUB_CRYPTODISK_MAX_PASSPHRASE);
> > + if (passphrase_len == (grub_size_t)-1)
> > + {
> > + ret = grub_error (GRUB_ERR_FILE_READ_ERROR, "Error reading
> > passphrase file");
> > + goto err;
> > + }
> > + }
> > + else
> > + {
> > + /* Get the passphrase from the user. */
> > + if (disk->partition)
> > + part = grub_partition_get_name (disk->partition);
> > + grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "),
> > disk->name,
> > + disk->partition ? "," : "", part ? : "",
> > + crypt->uuid);
> > + passphrase = grub_malloc (MAX_PASSPHRASE);
> > + if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> > + {
> > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not
> > supplied");
> > + goto err;
> > + }
> > + passphrase_len = grub_strlen (passphrase);
> > }
> >
> > if (grub_json_getvalue (&keyslots, json, "keyslots") ||
> > @@ -604,6 +677,26 @@ luks2_recover_key (grub_disk_t disk,
> >
> > grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_SIZE"\n", i);
> >
> > + if (mkey_file)
> > + {
> > + if (candidate_key_len != keyslot.key_size)
> > + {
> > + grub_dprintf ("luks2", "Wrong key length for keyslot
> > %"PRIuGRUB_SIZE": %s\n",
> > + i, grub_errmsg);
> > + continue;
> > + }
> > +
> > + ret = luks2_verify_key (&digest, candidate_key,
> > candidate_key_len);
> > + if (ret)
> > + {
> > + grub_dprintf ("luks2", "Could not open keyslot
> > %"PRIuGRUB_SIZE": %s\n",
> > + i, grub_errmsg);
> > + continue;
> > + }
> > + }
> > + else
> > + candidate_key_len = keyslot.key_size;
> > +
> > /* Set up disk according to keyslot's segment. */
> > crypt->offset = grub_divmod64 (segment.offset,
> > segment.sector_size, NULL); crypt->log_sector_size = sizeof (unsigned
> > int) * 8 @@ -613,21 +706,24 @@ luks2_recover_key (grub_disk_t disk,
> > else
> > crypt->total_length = grub_strtoull (segment.size, NULL, 10);
> >
> > - ret = luks2_decrypt_key (candidate_key, disk, crypt, &keyslot,
> > - (const grub_uint8_t *) passphrase,
> > grub_strlen (passphrase));
> > - if (ret)
> > + if (passphrase)
> > {
> > - grub_dprintf ("luks2", "Decryption with keyslot
> > %"PRIuGRUB_SIZE" failed: %s\n",
> > - i, grub_errmsg);
> > - continue;
> > - }
> > -
> > - ret = luks2_verify_key (&digest, candidate_key,
> > keyslot.key_size);
> > - if (ret)
> > - {
> > - grub_dprintf ("luks2", "Could not open keyslot
> > %"PRIuGRUB_SIZE": %s\n",
> > - i, grub_errmsg);
> > - continue;
> > + ret = luks2_decrypt_key (candidate_key, disk, crypt,
> > hdr_file, &keyslot,
> > + (const grub_uint8_t *)
> > passphrase, passphrase_len);
> > + if (ret)
> > + {
> > + grub_dprintf ("luks2", "Decryption with keyslot
> > %"PRIuGRUB_SIZE" failed: %s\n",
> > + i, grub_errmsg);
> > + continue;
> > + }
> > +
> > + ret = luks2_verify_key (&digest, candidate_key,
> > candidate_key_len);
> > + if (ret)
> > + {
> > + grub_dprintf ("luks2", "Could not open keyslot
> > %"PRIuGRUB_SIZE": %s\n",
> > + i, grub_errmsg);
> > + continue;
> > + }
> > }
> >
> > /*
> > @@ -635,13 +731,14 @@ luks2_recover_key (grub_disk_t disk,
> > * where each element is either empty or holds a key.
> > */
> > grub_printf_ (N_("Slot %"PRIuGRUB_SIZE" opened\n"), i);
> > -
> > - candidate_key_len = keyslot.key_size;
> > break;
> > }
> > - if (candidate_key_len == 0)
> > + if (i == size)
> > {
> > - ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid
> > passphrase");
> > + if (mkey_file)
> > + ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid key");
> > + else
> > + ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid
> > passphrase"); goto err;
> > }
> >
> > @@ -666,6 +763,7 @@ luks2_recover_key (grub_disk_t disk,
> >
> > err:
> > grub_free (part);
> > + grub_free (passphrase);
> > grub_free (json_header);
> > grub_json_free (json);
> > return ret;
> > diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
> > index e1b21e785..b53006854 100644
> > --- a/include/grub/cryptodisk.h
> > +++ b/include/grub/cryptodisk.h
> > @@ -21,6 +21,7 @@
> >
> > #include <grub/disk.h>
> > #include <grub/crypto.h>
> > +#include <grub/file.h>
> > #include <grub/list.h>
> > #ifdef GRUB_UTIL
> > #include <grub/emu/hostdisk.h>
> > @@ -53,6 +54,7 @@ typedef enum
> > #define GRUB_CRYPTODISK_GF_LOG_BYTES (GRUB_CRYPTODISK_GF_LOG_SIZE -
> > 3) #define GRUB_CRYPTODISK_GF_BYTES (1U <<
> > GRUB_CRYPTODISK_GF_LOG_BYTES) #define GRUB_CRYPTODISK_MAX_KEYLEN 128
> > +#define GRUB_CRYPTODISK_MAX_PASSPHRASE 8192
> >
> > struct grub_cryptodisk;
> >
> > @@ -106,9 +108,10 @@ struct grub_cryptodisk_dev
> > struct grub_cryptodisk_dev *next;
> > struct grub_cryptodisk_dev **prev;
> >
> > - grub_cryptodisk_t (*scan) (grub_disk_t disk, const char
> > *check_uuid,
> > - int boot_only);
> > - grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t
> > dev);
> > + grub_cryptodisk_t (*scan) (grub_disk_t disk, grub_file_t hdr_file,
> > + const char *check_uuid, int boot_only);
> > + grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t dev,
> > + grub_file_t hdr_file, grub_file_t
> > key_file, grub_file_t mkey_file); };
> > typedef struct grub_cryptodisk_dev *grub_cryptodisk_dev_t;
> >
>
[-- Attachment #2: Type: text/html, Size: 31649 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Cryptomount support for key files and detached header
2020-11-09 7:56 ` Dmitry
@ 2020-11-09 9:42 ` Dmitry
2020-11-09 21:34 ` Glenn Washburn
1 sibling, 0 replies; 7+ messages in thread
From: Dmitry @ 2020-11-09 9:42 UTC (permalink / raw)
To: development; +Cc: The development of GNU GRUB
[-- Attachment #1: Type: text/plain, Size: 23794 bytes --]
And it seems the next function can be used to allow
keyfile-offset and keyfile-size:
file.h:
grub_file_t
grub_file_offset_open (grub_file_t parent, enum grub_file_type type,
grub_off_t start, grub_off_t size);
So only grub_cmd_cryptomount can be modified for this.
Dmitry
пн, 9 нояб. 2020 г. в 10:56, Dmitry <reagentoo@gmail.com>:
> Thanks for feedback. First of all I think it's preferable to introduce
> the master-key option at the beginning. Because I see no point for a user
> to use a standard slot key along with a detached header.
> Decryption from key to master key takes a long time. (30 seconds for
> argon2i).
> Regarding keyfile-offset and keyfile-size options I suggest to introduce
> this options in separate patches. Yes, maybe you variant to pass the
> pointer and size
> (correct me if I am wrong) to luks2_recover_key is better of files.
> But then you need to think in advance how to add master-key option.
>
> Dmitry
>
> пн, 9 нояб. 2020 г. в 06:16, Glenn Washburn <development@efficientek.com>:
>
>> I've read through the patch but not applied or tested it. However, it
>> looks like it does the job. In fact, its fairly similar in parts to a
>> patch, which adds LUKS2 keyfile and detached header support, I've been
>> waiting to send to the list until the previous LUKS1 keyfile and
>> detached header support gets included (which my patches rely on).
>>
>> One implementation difference in this patch from the other one is that
>> this one passes the key file to recover_key as a grub_file_t instead of
>> passing the key contents. I prefer the previous method because it
>> consolidates key file reading in grub_cmd_cryptomount, whereas this
>> patch requires each module to do its own reading. The disadvantage is
>> this causes extra code in the modules that could be consolidated and I
>> don't see much advantage to this. By having grub_cmd_cryptomount do
>> the work we also get keyfile-offset and keyfile-size options in the
>> previous patch, for not much extra work.
>>
>> A minor nit, there is not error checking on grub_file_seek calls and
>> grub_file_read errors are occluded. However, there are some changes in
>> this patch which I think make the code a bit more aesthetically
>> pleasing.
>>
>> While, I like that this patch allows for passing a master key file, I
>> think this feature should be build on the previous patch series. So I
>> don't recommend this patch be accepted.
>>
>> Glenn
>>
>> On Sat, 7 Nov 2020 19:36:35 +0300
>> reagentoo <reagentoo@gmail.com> wrote:
>>
>> > From: Dmitry Baranov <reagentoo@gmail.com>
>> >
>> > Hello.
>> > Could someone make initial review?
>> > The main difference between this patch and the previous ones is
>> > the ability to use a master key file with a detached header.
>> > This patch does not provide luks1 and geli support yet (luks2 only).
>> >
>> > Best Regards,
>> > Dmitry
>> >
>> > ---
>> > grub-core/disk/cryptodisk.c | 99 +++++++++++++++-----
>> > grub-core/disk/luks2.c | 178
>> > ++++++++++++++++++++++++++++-------- include/grub/cryptodisk.h |
>> > 9 +- 3 files changed, 222 insertions(+), 64 deletions(-)
>> >
>> > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
>> > index 13af84dd1..987a39b16 100644
>> > --- a/grub-core/disk/cryptodisk.c
>> > +++ b/grub-core/disk/cryptodisk.c
>> > @@ -41,6 +41,9 @@ static const struct grub_arg_option options[] =
>> > /* TRANSLATORS: It's still restricted to cryptodisks only. */
>> > {"all", 'a', 0, N_("Mount all."), 0, 0},
>> > {"boot", 'b', 0, N_("Mount all volumes with `boot' flag set."),
>> > 0, 0},
>> > + {"header", 'H', 0, N_("Read header from file"), 0,
>> > ARG_TYPE_STRING},
>> > + {"key-file", 'k', 0, N_("Read key from file"), 0,
>> > ARG_TYPE_STRING},
>> > + {"master-key-file", 'K', 0, N_("Use a master key stored in a
>> > file"), 0, ARG_TYPE_STRING}, {0, 0, 0, 0, 0, 0}
>> > };
>> >
>> > @@ -967,6 +970,7 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk)
>> >
>> > static int check_boot, have_it;
>> > static char *search_uuid;
>> > +static grub_file_t hdr_file, key_file, mkey_file;
>> >
>> > static void
>> > cryptodisk_close (grub_cryptodisk_t dev)
>> > @@ -991,13 +995,13 @@ grub_cryptodisk_scan_device_real (const char
>> > *name, grub_disk_t source)
>> > FOR_CRYPTODISK_DEVS (cr)
>> > {
>> > - dev = cr->scan (source, search_uuid, check_boot);
>> > + dev = cr->scan (source, hdr_file, search_uuid, check_boot);
>> > if (grub_errno)
>> > return grub_errno;
>> > if (!dev)
>> > continue;
>> >
>> > - err = cr->recover_key (source, dev);
>> > + err = cr->recover_key (source, dev, hdr_file, key_file,
>> > mkey_file); if (err)
>> > {
>> > cryptodisk_close (dev);
>> > @@ -1038,7 +1042,7 @@ grub_cryptodisk_cheat_mount (const char
>> > *sourcedev, const char *cheat)
>> > FOR_CRYPTODISK_DEVS (cr)
>> > {
>> > - dev = cr->scan (source, search_uuid, check_boot);
>> > + dev = cr->scan (source, 0, search_uuid, check_boot);
>> > if (grub_errno)
>> > return grub_errno;
>> > if (!dev)
>> > @@ -1087,11 +1091,59 @@ grub_cryptodisk_scan_device (const char *name,
>> > static grub_err_t
>> > grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char
>> > **args) {
>> > + grub_err_t ret = GRUB_ERR_NONE;
>> > +
>> > struct grub_arg_list *state = ctxt->state;
>> >
>> > if (argc < 1 && !state[1].set && !state[2].set)
>> > return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name
>> > required");
>> > + hdr_file = NULL;
>> > + key_file = NULL;
>> > + mkey_file = NULL;
>> > +
>> > + if (state[3].set)
>> > + {
>> > + if (state[0].set)
>> > + {
>> > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot use UUID
>> > lookup with detached header");
>> > + goto err;
>> > + }
>> > +
>> > + hdr_file = grub_file_open (state[3].arg, GRUB_FILE_TYPE_NONE);
>> > + if (!hdr_file)
>> > + {
>> > + ret = grub_errno;
>> > + goto err;
>> > + }
>> > + }
>> > +
>> > + if (state[4].set)
>> > + {
>> > + if (state[5].set)
>> > + {
>> > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot use key
>> > with master key");
>> > + goto err;
>> > + }
>> > +
>> > + key_file = grub_file_open (state[4].arg, GRUB_FILE_TYPE_NONE);
>> > + if (!key_file)
>> > + {
>> > + ret = grub_errno;
>> > + goto err;
>> > + }
>> > + }
>> > +
>> > + if (state[5].set)
>> > + {
>> > + mkey_file = grub_file_open (state[5].arg, GRUB_FILE_TYPE_NONE);
>> > + if (!mkey_file)
>> > + {
>> > + ret = grub_errno;
>> > + goto err;
>> > + }
>> > + }
>> > +
>> > have_it = 0;
>> > if (state[0].set)
>> > {
>> > @@ -1102,7 +1154,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t
>> > ctxt, int argc, char **args) {
>> > grub_dprintf ("cryptodisk",
>> > "already mounted as crypto%lu\n", dev->id);
>> > - return GRUB_ERR_NONE;
>> > + goto err;
>> > }
>> >
>> > check_boot = state[2].set;
>> > @@ -1111,8 +1163,11 @@ grub_cmd_cryptomount (grub_extcmd_context_t
>> > ctxt, int argc, char **args) search_uuid = NULL;
>> >
>> > if (!have_it)
>> > - return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such
>> > cryptodisk found");
>> > - return GRUB_ERR_NONE;
>> > + {
>> > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "no such
>> > cryptodisk found");
>> > + goto err;
>> > + }
>> > + goto err;
>> > }
>> > else if (state[1].set || (argc == 0 && state[2].set))
>> > {
>> > @@ -1120,11 +1175,10 @@ grub_cmd_cryptomount (grub_extcmd_context_t
>> > ctxt, int argc, char **args) check_boot = state[2].set;
>> > grub_device_iterate (&grub_cryptodisk_scan_device, NULL);
>> > search_uuid = NULL;
>> > - return GRUB_ERR_NONE;
>> > + goto err;
>> > }
>> > else
>> > {
>> > - grub_err_t err;
>> > grub_disk_t disk;
>> > grub_cryptodisk_t dev;
>> > char *diskname;
>> > @@ -1147,27 +1201,30 @@ grub_cmd_cryptomount (grub_extcmd_context_t
>> > ctxt, int argc, char **args) {
>> > if (disklast)
>> > *disklast = ')';
>> > - return grub_errno;
>> > + ret = grub_errno;
>> > + goto err;
>> > }
>> >
>> > dev = grub_cryptodisk_get_by_source_disk (disk);
>> > if (dev)
>> > - {
>> > - grub_dprintf ("cryptodisk", "already mounted as
>> > crypto%lu\n", dev->id);
>> > - grub_disk_close (disk);
>> > - if (disklast)
>> > - *disklast = ')';
>> > - return GRUB_ERR_NONE;
>> > - }
>> > -
>> > - err = grub_cryptodisk_scan_device_real (diskname, disk);
>> > + grub_dprintf ("cryptodisk", "already mounted as
>> > crypto%lu\n", dev->id);
>> > + else
>> > + ret = grub_cryptodisk_scan_device_real (diskname, disk);
>> >
>> > grub_disk_close (disk);
>> > if (disklast)
>> > *disklast = ')';
>> > -
>> > - return err;
>> > }
>> > +
>> > + err:
>> > + if (hdr_file)
>> > + grub_file_close (hdr_file);
>> > + if (key_file)
>> > + grub_file_close (key_file);
>> > + if (mkey_file)
>> > + grub_file_close (mkey_file);
>> > +
>> > + return ret;
>> > }
>> >
>> > static struct grub_disk_dev grub_cryptodisk_dev = {
>> > @@ -1299,7 +1356,7 @@ GRUB_MOD_INIT (cryptodisk)
>> > {
>> > grub_disk_dev_register (&grub_cryptodisk_dev);
>> > cmd = grub_register_extcmd ("cryptomount", grub_cmd_cryptomount, 0,
>> > - N_("SOURCE|-u UUID|-a|-b"),
>> > + N_("SOURCE|-u UUID|-a|-b|-H file|-k
>> > file|-K file"), N_("Mount a crypto device."), options);
>> > grub_procfs_register ("luks_script", &luks_script);
>> > }
>> > diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
>> > index 31d7166fc..c29b472e6 100644
>> > --- a/grub-core/disk/luks2.c
>> > +++ b/grub-core/disk/luks2.c
>> > @@ -309,13 +309,22 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k,
>> > grub_luks2_digest_t *d, grub_luks2_s
>> > /* Determine whether to use primary or secondary header */
>> > static grub_err_t
>> > -luks2_read_header (grub_disk_t disk, grub_luks2_header_t *outhdr)
>> > +luks2_read_header (grub_disk_t disk, grub_file_t hdr_file,
>> > grub_luks2_header_t *outhdr) {
>> > grub_luks2_header_t primary, secondary, *header = &primary;
>> > grub_err_t ret;
>> >
>> > /* Read the primary LUKS header. */
>> > - ret = grub_disk_read (disk, 0, 0, sizeof (primary), &primary);
>> > + if (hdr_file)
>> > + {
>> > + grub_file_seek (hdr_file, 0);
>> > + if (grub_file_read (hdr_file, &primary, sizeof (primary)) !=
>> > sizeof (primary))
>> > + ret = GRUB_ERR_READ_ERROR;
>> > + else
>> > + ret = GRUB_ERR_NONE;
>> > + }
>> > + else
>> > + ret = grub_disk_read (disk, 0, 0, sizeof (primary), &primary);
>> > if (ret)
>> > return ret;
>> >
>> > @@ -325,7 +334,16 @@ luks2_read_header (grub_disk_t disk,
>> > grub_luks2_header_t *outhdr) return GRUB_ERR_BAD_SIGNATURE;
>> >
>> > /* Read the secondary header. */
>> > - ret = grub_disk_read (disk, 0, grub_be_to_cpu64
>> > (primary.hdr_size), sizeof (secondary), &secondary);
>> > + if (hdr_file)
>> > + {
>> > + grub_file_seek (hdr_file, grub_be_to_cpu64 (primary.hdr_size));
>> > + if (grub_file_read (hdr_file, &secondary, sizeof (secondary))
>> > != sizeof (secondary))
>> > + ret = GRUB_ERR_READ_ERROR;
>> > + else
>> > + ret = GRUB_ERR_NONE;
>> > + }
>> > + else
>> > + ret = grub_disk_read (disk, 0, grub_be_to_cpu64
>> > (primary.hdr_size), sizeof (secondary), &secondary); if (ret)
>> > return ret;
>> >
>> > @@ -342,7 +360,7 @@ luks2_read_header (grub_disk_t disk,
>> > grub_luks2_header_t *outhdr) }
>> >
>> > static grub_cryptodisk_t
>> > -luks2_scan (grub_disk_t disk, const char *check_uuid, int check_boot)
>> > +luks2_scan (grub_disk_t disk, grub_file_t hdr_file, const char
>> > *check_uuid, int check_boot) {
>> > grub_cryptodisk_t cryptodisk;
>> > grub_luks2_header_t header;
>> > @@ -352,7 +370,7 @@ luks2_scan (grub_disk_t disk, const char
>> > *check_uuid, int check_boot) if (check_boot)
>> > return NULL;
>> >
>> > - if (luks2_read_header (disk, &header))
>> > + if (luks2_read_header (disk, hdr_file, &header))
>> > {
>> > grub_errno = GRUB_ERR_NONE;
>> > return NULL;
>> > @@ -415,7 +433,7 @@ luks2_verify_key (grub_luks2_digest_t *d,
>> > grub_uint8_t *candidate_key,
>> > static grub_err_t
>> > luks2_decrypt_key (grub_uint8_t *out_key,
>> > - grub_disk_t disk, grub_cryptodisk_t crypt,
>> > + grub_disk_t disk, grub_cryptodisk_t crypt,
>> > grub_file_t hdr_file, grub_luks2_keyslot_t *k,
>> > const grub_uint8_t *passphrase, grub_size_t
>> > passphraselen) {
>> > @@ -491,7 +509,16 @@ luks2_decrypt_key (grub_uint8_t *out_key,
>> > }
>> >
>> > grub_errno = GRUB_ERR_NONE;
>> > - ret = grub_disk_read (disk, 0, k->area.offset, k->area.size,
>> > split_key);
>> > + if (hdr_file)
>> > + {
>> > + grub_file_seek (hdr_file, k->area.offset);
>> > + if (grub_file_read (hdr_file, split_key, k->area.size) !=
>> > k->area.size)
>> > + ret = GRUB_ERR_READ_ERROR;
>> > + else
>> > + ret = GRUB_ERR_NONE;
>> > + }
>> > + else
>> > + ret = grub_disk_read (disk, 0, k->area.offset, k->area.size,
>> > split_key); if (ret)
>> > {
>> > grub_error (GRUB_ERR_IO, "Read error: %s\n", grub_errmsg);
>> > @@ -531,12 +558,16 @@ luks2_decrypt_key (grub_uint8_t *out_key,
>> >
>> > static grub_err_t
>> > luks2_recover_key (grub_disk_t disk,
>> > - grub_cryptodisk_t crypt)
>> > + grub_cryptodisk_t crypt,
>> > + grub_file_t hdr_file, grub_file_t key_file,
>> > grub_file_t mkey_file) {
>> > + char cipher[32];
>> > + char *passphrase = NULL;
>> > grub_uint8_t candidate_key[GRUB_CRYPTODISK_MAX_KEYLEN];
>> > - char passphrase[MAX_PASSPHRASE], cipher[32];
>> > + grub_size_t candidate_key_len, passphrase_len;
>> > char *json_header = NULL, *part = NULL, *ptr;
>> > - grub_size_t candidate_key_len = 0, i, size;
>> > + grub_off_t json_header_offset;
>> > + grub_size_t json_header_size, i, size;
>> > grub_luks2_header_t header;
>> > grub_luks2_keyslot_t keyslot;
>> > grub_luks2_digest_t digest;
>> > @@ -545,7 +576,7 @@ luks2_recover_key (grub_disk_t disk,
>> > grub_json_t *json = NULL, keyslots;
>> > grub_err_t ret;
>> >
>> > - ret = luks2_read_header (disk, &header);
>> > + ret = luks2_read_header (disk, hdr_file, &header);
>> > if (ret)
>> > return ret;
>> >
>> > @@ -554,8 +585,19 @@ luks2_recover_key (grub_disk_t disk,
>> > return GRUB_ERR_OUT_OF_MEMORY;
>> >
>> > /* Read the JSON area. */
>> > - ret = grub_disk_read (disk, 0, grub_be_to_cpu64
>> > (header.hdr_offset) + sizeof (header),
>> > - grub_be_to_cpu64 (header.hdr_size) - sizeof
>> > (header), json_header);
>> > + json_header_offset = grub_be_to_cpu64 (header.hdr_offset) + sizeof
>> > (header);
>> > + json_header_size = grub_be_to_cpu64 (header.hdr_size) - sizeof
>> > (header); +
>> > + if (hdr_file)
>> > + {
>> > + grub_file_seek (hdr_file, json_header_offset);
>> > + if (grub_file_read (hdr_file, json_header, json_header_size)
>> > != json_header_size)
>> > + ret = GRUB_ERR_READ_ERROR;
>> > + else
>> > + ret = GRUB_ERR_NONE;
>> > + }
>> > + else
>> > + ret = grub_disk_read (disk, 0, json_header_offset,
>> > json_header_size, json_header); if (ret)
>> > goto err;
>> >
>> > @@ -570,16 +612,47 @@ luks2_recover_key (grub_disk_t disk,
>> > goto err;
>> > }
>> >
>> > - /* Get the passphrase from the user. */
>> > - if (disk->partition)
>> > - part = grub_partition_get_name (disk->partition);
>> > - grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), disk->name,
>> > - disk->partition ? "," : "", part ? : "",
>> > - crypt->uuid);
>> > - if (!grub_password_get (passphrase, MAX_PASSPHRASE))
>> > + if (mkey_file)
>> > {
>> > - ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not
>> > supplied");
>> > - goto err;
>> > + /* Get the master key from file. */
>> > + candidate_key_len = grub_file_read (mkey_file, candidate_key,
>> > GRUB_CRYPTODISK_MAX_KEYLEN);
>> > + if (candidate_key_len == (grub_size_t)-1)
>> > + {
>> > + ret = grub_error (GRUB_ERR_FILE_READ_ERROR, "Error reading
>> > key file");
>> > + goto err;
>> > + }
>> > + if (candidate_key_len == 0)
>> > + {
>> > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Key not
>> > supplied");
>> > + goto err;
>> > + }
>> > + }
>> > + else if (key_file)
>> > + {
>> > + /* Get the passphrase from file. */
>> > + passphrase = grub_malloc (GRUB_CRYPTODISK_MAX_PASSPHRASE);
>> > + passphrase_len = grub_file_read (key_file, passphrase,
>> > GRUB_CRYPTODISK_MAX_PASSPHRASE);
>> > + if (passphrase_len == (grub_size_t)-1)
>> > + {
>> > + ret = grub_error (GRUB_ERR_FILE_READ_ERROR, "Error reading
>> > passphrase file");
>> > + goto err;
>> > + }
>> > + }
>> > + else
>> > + {
>> > + /* Get the passphrase from the user. */
>> > + if (disk->partition)
>> > + part = grub_partition_get_name (disk->partition);
>> > + grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "),
>> > disk->name,
>> > + disk->partition ? "," : "", part ? : "",
>> > + crypt->uuid);
>> > + passphrase = grub_malloc (MAX_PASSPHRASE);
>> > + if (!grub_password_get (passphrase, MAX_PASSPHRASE))
>> > + {
>> > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not
>> > supplied");
>> > + goto err;
>> > + }
>> > + passphrase_len = grub_strlen (passphrase);
>> > }
>> >
>> > if (grub_json_getvalue (&keyslots, json, "keyslots") ||
>> > @@ -604,6 +677,26 @@ luks2_recover_key (grub_disk_t disk,
>> >
>> > grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_SIZE"\n", i);
>> >
>> > + if (mkey_file)
>> > + {
>> > + if (candidate_key_len != keyslot.key_size)
>> > + {
>> > + grub_dprintf ("luks2", "Wrong key length for keyslot
>> > %"PRIuGRUB_SIZE": %s\n",
>> > + i, grub_errmsg);
>> > + continue;
>> > + }
>> > +
>> > + ret = luks2_verify_key (&digest, candidate_key,
>> > candidate_key_len);
>> > + if (ret)
>> > + {
>> > + grub_dprintf ("luks2", "Could not open keyslot
>> > %"PRIuGRUB_SIZE": %s\n",
>> > + i, grub_errmsg);
>> > + continue;
>> > + }
>> > + }
>> > + else
>> > + candidate_key_len = keyslot.key_size;
>> > +
>> > /* Set up disk according to keyslot's segment. */
>> > crypt->offset = grub_divmod64 (segment.offset,
>> > segment.sector_size, NULL); crypt->log_sector_size = sizeof (unsigned
>> > int) * 8 @@ -613,21 +706,24 @@ luks2_recover_key (grub_disk_t disk,
>> > else
>> > crypt->total_length = grub_strtoull (segment.size, NULL, 10);
>> >
>> > - ret = luks2_decrypt_key (candidate_key, disk, crypt, &keyslot,
>> > - (const grub_uint8_t *) passphrase,
>> > grub_strlen (passphrase));
>> > - if (ret)
>> > + if (passphrase)
>> > {
>> > - grub_dprintf ("luks2", "Decryption with keyslot
>> > %"PRIuGRUB_SIZE" failed: %s\n",
>> > - i, grub_errmsg);
>> > - continue;
>> > - }
>> > -
>> > - ret = luks2_verify_key (&digest, candidate_key,
>> > keyslot.key_size);
>> > - if (ret)
>> > - {
>> > - grub_dprintf ("luks2", "Could not open keyslot
>> > %"PRIuGRUB_SIZE": %s\n",
>> > - i, grub_errmsg);
>> > - continue;
>> > + ret = luks2_decrypt_key (candidate_key, disk, crypt,
>> > hdr_file, &keyslot,
>> > + (const grub_uint8_t *)
>> > passphrase, passphrase_len);
>> > + if (ret)
>> > + {
>> > + grub_dprintf ("luks2", "Decryption with keyslot
>> > %"PRIuGRUB_SIZE" failed: %s\n",
>> > + i, grub_errmsg);
>> > + continue;
>> > + }
>> > +
>> > + ret = luks2_verify_key (&digest, candidate_key,
>> > candidate_key_len);
>> > + if (ret)
>> > + {
>> > + grub_dprintf ("luks2", "Could not open keyslot
>> > %"PRIuGRUB_SIZE": %s\n",
>> > + i, grub_errmsg);
>> > + continue;
>> > + }
>> > }
>> >
>> > /*
>> > @@ -635,13 +731,14 @@ luks2_recover_key (grub_disk_t disk,
>> > * where each element is either empty or holds a key.
>> > */
>> > grub_printf_ (N_("Slot %"PRIuGRUB_SIZE" opened\n"), i);
>> > -
>> > - candidate_key_len = keyslot.key_size;
>> > break;
>> > }
>> > - if (candidate_key_len == 0)
>> > + if (i == size)
>> > {
>> > - ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid
>> > passphrase");
>> > + if (mkey_file)
>> > + ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid key");
>> > + else
>> > + ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid
>> > passphrase"); goto err;
>> > }
>> >
>> > @@ -666,6 +763,7 @@ luks2_recover_key (grub_disk_t disk,
>> >
>> > err:
>> > grub_free (part);
>> > + grub_free (passphrase);
>> > grub_free (json_header);
>> > grub_json_free (json);
>> > return ret;
>> > diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
>> > index e1b21e785..b53006854 100644
>> > --- a/include/grub/cryptodisk.h
>> > +++ b/include/grub/cryptodisk.h
>> > @@ -21,6 +21,7 @@
>> >
>> > #include <grub/disk.h>
>> > #include <grub/crypto.h>
>> > +#include <grub/file.h>
>> > #include <grub/list.h>
>> > #ifdef GRUB_UTIL
>> > #include <grub/emu/hostdisk.h>
>> > @@ -53,6 +54,7 @@ typedef enum
>> > #define GRUB_CRYPTODISK_GF_LOG_BYTES (GRUB_CRYPTODISK_GF_LOG_SIZE -
>> > 3) #define GRUB_CRYPTODISK_GF_BYTES (1U <<
>> > GRUB_CRYPTODISK_GF_LOG_BYTES) #define GRUB_CRYPTODISK_MAX_KEYLEN 128
>> > +#define GRUB_CRYPTODISK_MAX_PASSPHRASE 8192
>> >
>> > struct grub_cryptodisk;
>> >
>> > @@ -106,9 +108,10 @@ struct grub_cryptodisk_dev
>> > struct grub_cryptodisk_dev *next;
>> > struct grub_cryptodisk_dev **prev;
>> >
>> > - grub_cryptodisk_t (*scan) (grub_disk_t disk, const char
>> > *check_uuid,
>> > - int boot_only);
>> > - grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t
>> > dev);
>> > + grub_cryptodisk_t (*scan) (grub_disk_t disk, grub_file_t hdr_file,
>> > + const char *check_uuid, int boot_only);
>> > + grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t dev,
>> > + grub_file_t hdr_file, grub_file_t
>> > key_file, grub_file_t mkey_file); };
>> > typedef struct grub_cryptodisk_dev *grub_cryptodisk_dev_t;
>> >
>>
>
[-- Attachment #2: Type: text/html, Size: 30666 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Cryptomount support for key files and detached header
2020-11-09 7:56 ` Dmitry
2020-11-09 9:42 ` Dmitry
@ 2020-11-09 21:34 ` Glenn Washburn
2020-11-09 22:09 ` Dmitry
1 sibling, 1 reply; 7+ messages in thread
From: Glenn Washburn @ 2020-11-09 21:34 UTC (permalink / raw)
To: Dmitry; +Cc: The development of GNU GRUB
On Mon, 9 Nov 2020 10:56:51 +0300
Dmitry <reagentoo@gmail.com> wrote:
> Thanks for feedback. First of all I think it's preferable to introduce
> the master-key option at the beginning. Because I see no point for a
> user to use a standard slot key along with a detached header.
> Decryption from key to master key takes a long time. (30 seconds for
> argon2i).
Are you using the argon patches on this list, which haven't been
committed to run that test? Because currently there is no support for
Argon2.
Second detached headers have nothing to do with how long Argon2 or any
other key derivation function takes to complete, so they are
independent (or am I wrong?). Am I correct in understanding you to say
that argon2i is faster without having a detached header?
Third, I actually do currently use detached headers (as I said before I
have an unsubmitted patch which adds detached header and keyfile). And
separately, yes the key derivation can take a while (compared to running
in linux). I'm not exactly clear on why this is. I have some ideas, like
the code grub is using to do crypto or grub's runtime environment, but
I haven't really looked in to it. Do you have any ideas? And I know
there are others currently using the detached header without the master
key option, so there's definitely a point.
> Regarding keyfile-offset and keyfile-size options I suggest to
> introduce this options in separate patches. Yes, maybe you variant to
> pass the pointer and size
> (correct me if I am wrong) to luks2_recover_key is better of files.
> But then you need to think in advance how to add master-key option.
It can be added just like the key-file option was added in the previous
patch series. Read the master key file from grub_cmd_cryptomount.
Glenn
> пн, 9 нояб. 2020 г. в 06:16, Glenn Washburn
> <development@efficientek.com>:
>
> > I've read through the patch but not applied or tested it. However,
> > it looks like it does the job. In fact, its fairly similar in
> > parts to a patch, which adds LUKS2 keyfile and detached header
> > support, I've been waiting to send to the list until the previous
> > LUKS1 keyfile and detached header support gets included (which my
> > patches rely on).
> >
> > One implementation difference in this patch from the other one is
> > that this one passes the key file to recover_key as a grub_file_t
> > instead of passing the key contents. I prefer the previous method
> > because it consolidates key file reading in grub_cmd_cryptomount,
> > whereas this patch requires each module to do its own reading. The
> > disadvantage is this causes extra code in the modules that could be
> > consolidated and I don't see much advantage to this. By having
> > grub_cmd_cryptomount do the work we also get keyfile-offset and
> > keyfile-size options in the previous patch, for not much extra work.
> >
> > A minor nit, there is not error checking on grub_file_seek calls and
> > grub_file_read errors are occluded. However, there are some
> > changes in this patch which I think make the code a bit more
> > aesthetically pleasing.
> >
> > While, I like that this patch allows for passing a master key file,
> > I think this feature should be build on the previous patch series.
> > So I don't recommend this patch be accepted.
> >
> > Glenn
> >
> > On Sat, 7 Nov 2020 19:36:35 +0300
> > reagentoo <reagentoo@gmail.com> wrote:
> >
> > > From: Dmitry Baranov <reagentoo@gmail.com>
> > >
> > > Hello.
> > > Could someone make initial review?
> > > The main difference between this patch and the previous ones is
> > > the ability to use a master key file with a detached header.
> > > This patch does not provide luks1 and geli support yet (luks2
> > > only).
> > >
> > > Best Regards,
> > > Dmitry
> > >
> > > ---
> > > grub-core/disk/cryptodisk.c | 99 +++++++++++++++-----
> > > grub-core/disk/luks2.c | 178
> > > ++++++++++++++++++++++++++++-------- include/grub/cryptodisk.h |
> > > 9 +- 3 files changed, 222 insertions(+), 64 deletions(-)
> > >
> > > diff --git a/grub-core/disk/cryptodisk.c
> > > b/grub-core/disk/cryptodisk.c index 13af84dd1..987a39b16 100644
> > > --- a/grub-core/disk/cryptodisk.c
> > > +++ b/grub-core/disk/cryptodisk.c
> > > @@ -41,6 +41,9 @@ static const struct grub_arg_option options[] =
> > > /* TRANSLATORS: It's still restricted to cryptodisks only.
> > > */ {"all", 'a', 0, N_("Mount all."), 0, 0},
> > > {"boot", 'b', 0, N_("Mount all volumes with `boot' flag
> > > set."), 0, 0},
> > > + {"header", 'H', 0, N_("Read header from file"), 0,
> > > ARG_TYPE_STRING},
> > > + {"key-file", 'k', 0, N_("Read key from file"), 0,
> > > ARG_TYPE_STRING},
> > > + {"master-key-file", 'K', 0, N_("Use a master key stored in a
> > > file"), 0, ARG_TYPE_STRING}, {0, 0, 0, 0, 0, 0}
> > > };
> > >
> > > @@ -967,6 +970,7 @@ grub_util_cryptodisk_get_uuid (grub_disk_t
> > > disk)
> > >
> > > static int check_boot, have_it;
> > > static char *search_uuid;
> > > +static grub_file_t hdr_file, key_file, mkey_file;
> > >
> > > static void
> > > cryptodisk_close (grub_cryptodisk_t dev)
> > > @@ -991,13 +995,13 @@ grub_cryptodisk_scan_device_real (const char
> > > *name, grub_disk_t source)
> > > FOR_CRYPTODISK_DEVS (cr)
> > > {
> > > - dev = cr->scan (source, search_uuid, check_boot);
> > > + dev = cr->scan (source, hdr_file, search_uuid, check_boot);
> > > if (grub_errno)
> > > return grub_errno;
> > > if (!dev)
> > > continue;
> > >
> > > - err = cr->recover_key (source, dev);
> > > + err = cr->recover_key (source, dev, hdr_file, key_file,
> > > mkey_file); if (err)
> > > {
> > > cryptodisk_close (dev);
> > > @@ -1038,7 +1042,7 @@ grub_cryptodisk_cheat_mount (const char
> > > *sourcedev, const char *cheat)
> > > FOR_CRYPTODISK_DEVS (cr)
> > > {
> > > - dev = cr->scan (source, search_uuid, check_boot);
> > > + dev = cr->scan (source, 0, search_uuid, check_boot);
> > > if (grub_errno)
> > > return grub_errno;
> > > if (!dev)
> > > @@ -1087,11 +1091,59 @@ grub_cryptodisk_scan_device (const char
> > > *name, static grub_err_t
> > > grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char
> > > **args) {
> > > + grub_err_t ret = GRUB_ERR_NONE;
> > > +
> > > struct grub_arg_list *state = ctxt->state;
> > >
> > > if (argc < 1 && !state[1].set && !state[2].set)
> > > return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name
> > > required");
> > > + hdr_file = NULL;
> > > + key_file = NULL;
> > > + mkey_file = NULL;
> > > +
> > > + if (state[3].set)
> > > + {
> > > + if (state[0].set)
> > > + {
> > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot use UUID
> > > lookup with detached header");
> > > + goto err;
> > > + }
> > > +
> > > + hdr_file = grub_file_open (state[3].arg,
> > > GRUB_FILE_TYPE_NONE);
> > > + if (!hdr_file)
> > > + {
> > > + ret = grub_errno;
> > > + goto err;
> > > + }
> > > + }
> > > +
> > > + if (state[4].set)
> > > + {
> > > + if (state[5].set)
> > > + {
> > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot use key
> > > with master key");
> > > + goto err;
> > > + }
> > > +
> > > + key_file = grub_file_open (state[4].arg,
> > > GRUB_FILE_TYPE_NONE);
> > > + if (!key_file)
> > > + {
> > > + ret = grub_errno;
> > > + goto err;
> > > + }
> > > + }
> > > +
> > > + if (state[5].set)
> > > + {
> > > + mkey_file = grub_file_open (state[5].arg,
> > > GRUB_FILE_TYPE_NONE);
> > > + if (!mkey_file)
> > > + {
> > > + ret = grub_errno;
> > > + goto err;
> > > + }
> > > + }
> > > +
> > > have_it = 0;
> > > if (state[0].set)
> > > {
> > > @@ -1102,7 +1154,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > > ctxt, int argc, char **args) {
> > > grub_dprintf ("cryptodisk",
> > > "already mounted as crypto%lu\n", dev->id);
> > > - return GRUB_ERR_NONE;
> > > + goto err;
> > > }
> > >
> > > check_boot = state[2].set;
> > > @@ -1111,8 +1163,11 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > > ctxt, int argc, char **args) search_uuid = NULL;
> > >
> > > if (!have_it)
> > > - return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such
> > > cryptodisk found");
> > > - return GRUB_ERR_NONE;
> > > + {
> > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "no such
> > > cryptodisk found");
> > > + goto err;
> > > + }
> > > + goto err;
> > > }
> > > else if (state[1].set || (argc == 0 && state[2].set))
> > > {
> > > @@ -1120,11 +1175,10 @@ grub_cmd_cryptomount
> > > (grub_extcmd_context_t ctxt, int argc, char **args) check_boot =
> > > state[2].set; grub_device_iterate (&grub_cryptodisk_scan_device,
> > > NULL); search_uuid = NULL;
> > > - return GRUB_ERR_NONE;
> > > + goto err;
> > > }
> > > else
> > > {
> > > - grub_err_t err;
> > > grub_disk_t disk;
> > > grub_cryptodisk_t dev;
> > > char *diskname;
> > > @@ -1147,27 +1201,30 @@ grub_cmd_cryptomount
> > > (grub_extcmd_context_t ctxt, int argc, char **args) {
> > > if (disklast)
> > > *disklast = ')';
> > > - return grub_errno;
> > > + ret = grub_errno;
> > > + goto err;
> > > }
> > >
> > > dev = grub_cryptodisk_get_by_source_disk (disk);
> > > if (dev)
> > > - {
> > > - grub_dprintf ("cryptodisk", "already mounted as
> > > crypto%lu\n", dev->id);
> > > - grub_disk_close (disk);
> > > - if (disklast)
> > > - *disklast = ')';
> > > - return GRUB_ERR_NONE;
> > > - }
> > > -
> > > - err = grub_cryptodisk_scan_device_real (diskname, disk);
> > > + grub_dprintf ("cryptodisk", "already mounted as
> > > crypto%lu\n", dev->id);
> > > + else
> > > + ret = grub_cryptodisk_scan_device_real (diskname, disk);
> > >
> > > grub_disk_close (disk);
> > > if (disklast)
> > > *disklast = ')';
> > > -
> > > - return err;
> > > }
> > > +
> > > + err:
> > > + if (hdr_file)
> > > + grub_file_close (hdr_file);
> > > + if (key_file)
> > > + grub_file_close (key_file);
> > > + if (mkey_file)
> > > + grub_file_close (mkey_file);
> > > +
> > > + return ret;
> > > }
> > >
> > > static struct grub_disk_dev grub_cryptodisk_dev = {
> > > @@ -1299,7 +1356,7 @@ GRUB_MOD_INIT (cryptodisk)
> > > {
> > > grub_disk_dev_register (&grub_cryptodisk_dev);
> > > cmd = grub_register_extcmd ("cryptomount",
> > > grub_cmd_cryptomount, 0,
> > > - N_("SOURCE|-u UUID|-a|-b"),
> > > + N_("SOURCE|-u UUID|-a|-b|-H file|-k
> > > file|-K file"), N_("Mount a crypto device."), options);
> > > grub_procfs_register ("luks_script", &luks_script);
> > > }
> > > diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> > > index 31d7166fc..c29b472e6 100644
> > > --- a/grub-core/disk/luks2.c
> > > +++ b/grub-core/disk/luks2.c
> > > @@ -309,13 +309,22 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k,
> > > grub_luks2_digest_t *d, grub_luks2_s
> > > /* Determine whether to use primary or secondary header */
> > > static grub_err_t
> > > -luks2_read_header (grub_disk_t disk, grub_luks2_header_t *outhdr)
> > > +luks2_read_header (grub_disk_t disk, grub_file_t hdr_file,
> > > grub_luks2_header_t *outhdr) {
> > > grub_luks2_header_t primary, secondary, *header = &primary;
> > > grub_err_t ret;
> > >
> > > /* Read the primary LUKS header. */
> > > - ret = grub_disk_read (disk, 0, 0, sizeof (primary), &primary);
> > > + if (hdr_file)
> > > + {
> > > + grub_file_seek (hdr_file, 0);
> > > + if (grub_file_read (hdr_file, &primary, sizeof (primary))
> > > != sizeof (primary))
> > > + ret = GRUB_ERR_READ_ERROR;
> > > + else
> > > + ret = GRUB_ERR_NONE;
> > > + }
> > > + else
> > > + ret = grub_disk_read (disk, 0, 0, sizeof (primary),
> > > &primary); if (ret)
> > > return ret;
> > >
> > > @@ -325,7 +334,16 @@ luks2_read_header (grub_disk_t disk,
> > > grub_luks2_header_t *outhdr) return GRUB_ERR_BAD_SIGNATURE;
> > >
> > > /* Read the secondary header. */
> > > - ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> > > (primary.hdr_size), sizeof (secondary), &secondary);
> > > + if (hdr_file)
> > > + {
> > > + grub_file_seek (hdr_file, grub_be_to_cpu64
> > > (primary.hdr_size));
> > > + if (grub_file_read (hdr_file, &secondary, sizeof
> > > (secondary)) != sizeof (secondary))
> > > + ret = GRUB_ERR_READ_ERROR;
> > > + else
> > > + ret = GRUB_ERR_NONE;
> > > + }
> > > + else
> > > + ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> > > (primary.hdr_size), sizeof (secondary), &secondary); if (ret)
> > > return ret;
> > >
> > > @@ -342,7 +360,7 @@ luks2_read_header (grub_disk_t disk,
> > > grub_luks2_header_t *outhdr) }
> > >
> > > static grub_cryptodisk_t
> > > -luks2_scan (grub_disk_t disk, const char *check_uuid, int
> > > check_boot) +luks2_scan (grub_disk_t disk, grub_file_t hdr_file,
> > > const char *check_uuid, int check_boot) {
> > > grub_cryptodisk_t cryptodisk;
> > > grub_luks2_header_t header;
> > > @@ -352,7 +370,7 @@ luks2_scan (grub_disk_t disk, const char
> > > *check_uuid, int check_boot) if (check_boot)
> > > return NULL;
> > >
> > > - if (luks2_read_header (disk, &header))
> > > + if (luks2_read_header (disk, hdr_file, &header))
> > > {
> > > grub_errno = GRUB_ERR_NONE;
> > > return NULL;
> > > @@ -415,7 +433,7 @@ luks2_verify_key (grub_luks2_digest_t *d,
> > > grub_uint8_t *candidate_key,
> > > static grub_err_t
> > > luks2_decrypt_key (grub_uint8_t *out_key,
> > > - grub_disk_t disk, grub_cryptodisk_t crypt,
> > > + grub_disk_t disk, grub_cryptodisk_t crypt,
> > > grub_file_t hdr_file, grub_luks2_keyslot_t *k,
> > > const grub_uint8_t *passphrase, grub_size_t
> > > passphraselen) {
> > > @@ -491,7 +509,16 @@ luks2_decrypt_key (grub_uint8_t *out_key,
> > > }
> > >
> > > grub_errno = GRUB_ERR_NONE;
> > > - ret = grub_disk_read (disk, 0, k->area.offset, k->area.size,
> > > split_key);
> > > + if (hdr_file)
> > > + {
> > > + grub_file_seek (hdr_file, k->area.offset);
> > > + if (grub_file_read (hdr_file, split_key, k->area.size) !=
> > > k->area.size)
> > > + ret = GRUB_ERR_READ_ERROR;
> > > + else
> > > + ret = GRUB_ERR_NONE;
> > > + }
> > > + else
> > > + ret = grub_disk_read (disk, 0, k->area.offset,
> > > k->area.size, split_key); if (ret)
> > > {
> > > grub_error (GRUB_ERR_IO, "Read error: %s\n", grub_errmsg);
> > > @@ -531,12 +558,16 @@ luks2_decrypt_key (grub_uint8_t *out_key,
> > >
> > > static grub_err_t
> > > luks2_recover_key (grub_disk_t disk,
> > > - grub_cryptodisk_t crypt)
> > > + grub_cryptodisk_t crypt,
> > > + grub_file_t hdr_file, grub_file_t key_file,
> > > grub_file_t mkey_file) {
> > > + char cipher[32];
> > > + char *passphrase = NULL;
> > > grub_uint8_t candidate_key[GRUB_CRYPTODISK_MAX_KEYLEN];
> > > - char passphrase[MAX_PASSPHRASE], cipher[32];
> > > + grub_size_t candidate_key_len, passphrase_len;
> > > char *json_header = NULL, *part = NULL, *ptr;
> > > - grub_size_t candidate_key_len = 0, i, size;
> > > + grub_off_t json_header_offset;
> > > + grub_size_t json_header_size, i, size;
> > > grub_luks2_header_t header;
> > > grub_luks2_keyslot_t keyslot;
> > > grub_luks2_digest_t digest;
> > > @@ -545,7 +576,7 @@ luks2_recover_key (grub_disk_t disk,
> > > grub_json_t *json = NULL, keyslots;
> > > grub_err_t ret;
> > >
> > > - ret = luks2_read_header (disk, &header);
> > > + ret = luks2_read_header (disk, hdr_file, &header);
> > > if (ret)
> > > return ret;
> > >
> > > @@ -554,8 +585,19 @@ luks2_recover_key (grub_disk_t disk,
> > > return GRUB_ERR_OUT_OF_MEMORY;
> > >
> > > /* Read the JSON area. */
> > > - ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> > > (header.hdr_offset) + sizeof (header),
> > > - grub_be_to_cpu64 (header.hdr_size) - sizeof
> > > (header), json_header);
> > > + json_header_offset = grub_be_to_cpu64 (header.hdr_offset) +
> > > sizeof (header);
> > > + json_header_size = grub_be_to_cpu64 (header.hdr_size) - sizeof
> > > (header); +
> > > + if (hdr_file)
> > > + {
> > > + grub_file_seek (hdr_file, json_header_offset);
> > > + if (grub_file_read (hdr_file, json_header,
> > > json_header_size) != json_header_size)
> > > + ret = GRUB_ERR_READ_ERROR;
> > > + else
> > > + ret = GRUB_ERR_NONE;
> > > + }
> > > + else
> > > + ret = grub_disk_read (disk, 0, json_header_offset,
> > > json_header_size, json_header); if (ret)
> > > goto err;
> > >
> > > @@ -570,16 +612,47 @@ luks2_recover_key (grub_disk_t disk,
> > > goto err;
> > > }
> > >
> > > - /* Get the passphrase from the user. */
> > > - if (disk->partition)
> > > - part = grub_partition_get_name (disk->partition);
> > > - grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "),
> > > disk->name,
> > > - disk->partition ? "," : "", part ? : "",
> > > - crypt->uuid);
> > > - if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> > > + if (mkey_file)
> > > {
> > > - ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not
> > > supplied");
> > > - goto err;
> > > + /* Get the master key from file. */
> > > + candidate_key_len = grub_file_read (mkey_file,
> > > candidate_key, GRUB_CRYPTODISK_MAX_KEYLEN);
> > > + if (candidate_key_len == (grub_size_t)-1)
> > > + {
> > > + ret = grub_error (GRUB_ERR_FILE_READ_ERROR, "Error reading
> > > key file");
> > > + goto err;
> > > + }
> > > + if (candidate_key_len == 0)
> > > + {
> > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Key not
> > > supplied");
> > > + goto err;
> > > + }
> > > + }
> > > + else if (key_file)
> > > + {
> > > + /* Get the passphrase from file. */
> > > + passphrase = grub_malloc (GRUB_CRYPTODISK_MAX_PASSPHRASE);
> > > + passphrase_len = grub_file_read (key_file, passphrase,
> > > GRUB_CRYPTODISK_MAX_PASSPHRASE);
> > > + if (passphrase_len == (grub_size_t)-1)
> > > + {
> > > + ret = grub_error (GRUB_ERR_FILE_READ_ERROR, "Error reading
> > > passphrase file");
> > > + goto err;
> > > + }
> > > + }
> > > + else
> > > + {
> > > + /* Get the passphrase from the user. */
> > > + if (disk->partition)
> > > + part = grub_partition_get_name (disk->partition);
> > > + grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "),
> > > disk->name,
> > > + disk->partition ? "," : "", part ? : "",
> > > + crypt->uuid);
> > > + passphrase = grub_malloc (MAX_PASSPHRASE);
> > > + if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> > > + {
> > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not
> > > supplied");
> > > + goto err;
> > > + }
> > > + passphrase_len = grub_strlen (passphrase);
> > > }
> > >
> > > if (grub_json_getvalue (&keyslots, json, "keyslots") ||
> > > @@ -604,6 +677,26 @@ luks2_recover_key (grub_disk_t disk,
> > >
> > > grub_dprintf ("luks2", "Trying keyslot
> > > %"PRIuGRUB_SIZE"\n", i);
> > >
> > > + if (mkey_file)
> > > + {
> > > + if (candidate_key_len != keyslot.key_size)
> > > + {
> > > + grub_dprintf ("luks2", "Wrong key length for keyslot
> > > %"PRIuGRUB_SIZE": %s\n",
> > > + i, grub_errmsg);
> > > + continue;
> > > + }
> > > +
> > > + ret = luks2_verify_key (&digest, candidate_key,
> > > candidate_key_len);
> > > + if (ret)
> > > + {
> > > + grub_dprintf ("luks2", "Could not open keyslot
> > > %"PRIuGRUB_SIZE": %s\n",
> > > + i, grub_errmsg);
> > > + continue;
> > > + }
> > > + }
> > > + else
> > > + candidate_key_len = keyslot.key_size;
> > > +
> > > /* Set up disk according to keyslot's segment. */
> > > crypt->offset = grub_divmod64 (segment.offset,
> > > segment.sector_size, NULL); crypt->log_sector_size = sizeof
> > > (unsigned int) * 8 @@ -613,21 +706,24 @@ luks2_recover_key
> > > (grub_disk_t disk, else
> > > crypt->total_length = grub_strtoull (segment.size, NULL,
> > > 10);
> > >
> > > - ret = luks2_decrypt_key (candidate_key, disk, crypt,
> > > &keyslot,
> > > - (const grub_uint8_t *) passphrase,
> > > grub_strlen (passphrase));
> > > - if (ret)
> > > + if (passphrase)
> > > {
> > > - grub_dprintf ("luks2", "Decryption with keyslot
> > > %"PRIuGRUB_SIZE" failed: %s\n",
> > > - i, grub_errmsg);
> > > - continue;
> > > - }
> > > -
> > > - ret = luks2_verify_key (&digest, candidate_key,
> > > keyslot.key_size);
> > > - if (ret)
> > > - {
> > > - grub_dprintf ("luks2", "Could not open keyslot
> > > %"PRIuGRUB_SIZE": %s\n",
> > > - i, grub_errmsg);
> > > - continue;
> > > + ret = luks2_decrypt_key (candidate_key, disk, crypt,
> > > hdr_file, &keyslot,
> > > + (const grub_uint8_t *)
> > > passphrase, passphrase_len);
> > > + if (ret)
> > > + {
> > > + grub_dprintf ("luks2", "Decryption with keyslot
> > > %"PRIuGRUB_SIZE" failed: %s\n",
> > > + i, grub_errmsg);
> > > + continue;
> > > + }
> > > +
> > > + ret = luks2_verify_key (&digest, candidate_key,
> > > candidate_key_len);
> > > + if (ret)
> > > + {
> > > + grub_dprintf ("luks2", "Could not open keyslot
> > > %"PRIuGRUB_SIZE": %s\n",
> > > + i, grub_errmsg);
> > > + continue;
> > > + }
> > > }
> > >
> > > /*
> > > @@ -635,13 +731,14 @@ luks2_recover_key (grub_disk_t disk,
> > > * where each element is either empty or holds a key.
> > > */
> > > grub_printf_ (N_("Slot %"PRIuGRUB_SIZE" opened\n"), i);
> > > -
> > > - candidate_key_len = keyslot.key_size;
> > > break;
> > > }
> > > - if (candidate_key_len == 0)
> > > + if (i == size)
> > > {
> > > - ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid
> > > passphrase");
> > > + if (mkey_file)
> > > + ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid key");
> > > + else
> > > + ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid
> > > passphrase"); goto err;
> > > }
> > >
> > > @@ -666,6 +763,7 @@ luks2_recover_key (grub_disk_t disk,
> > >
> > > err:
> > > grub_free (part);
> > > + grub_free (passphrase);
> > > grub_free (json_header);
> > > grub_json_free (json);
> > > return ret;
> > > diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
> > > index e1b21e785..b53006854 100644
> > > --- a/include/grub/cryptodisk.h
> > > +++ b/include/grub/cryptodisk.h
> > > @@ -21,6 +21,7 @@
> > >
> > > #include <grub/disk.h>
> > > #include <grub/crypto.h>
> > > +#include <grub/file.h>
> > > #include <grub/list.h>
> > > #ifdef GRUB_UTIL
> > > #include <grub/emu/hostdisk.h>
> > > @@ -53,6 +54,7 @@ typedef enum
> > > #define GRUB_CRYPTODISK_GF_LOG_BYTES
> > > (GRUB_CRYPTODISK_GF_LOG_SIZE - 3) #define
> > > GRUB_CRYPTODISK_GF_BYTES (1U << GRUB_CRYPTODISK_GF_LOG_BYTES)
> > > #define GRUB_CRYPTODISK_MAX_KEYLEN 128 +#define
> > > GRUB_CRYPTODISK_MAX_PASSPHRASE 8192
> > >
> > > struct grub_cryptodisk;
> > >
> > > @@ -106,9 +108,10 @@ struct grub_cryptodisk_dev
> > > struct grub_cryptodisk_dev *next;
> > > struct grub_cryptodisk_dev **prev;
> > >
> > > - grub_cryptodisk_t (*scan) (grub_disk_t disk, const char
> > > *check_uuid,
> > > - int boot_only);
> > > - grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t
> > > dev);
> > > + grub_cryptodisk_t (*scan) (grub_disk_t disk, grub_file_t
> > > hdr_file,
> > > + const char *check_uuid, int boot_only);
> > > + grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t
> > > dev,
> > > + grub_file_t hdr_file, grub_file_t
> > > key_file, grub_file_t mkey_file); };
> > > typedef struct grub_cryptodisk_dev *grub_cryptodisk_dev_t;
> > >
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Cryptomount support for key files and detached header
2020-11-09 21:34 ` Glenn Washburn
@ 2020-11-09 22:09 ` Dmitry
2020-11-10 20:25 ` Glenn Washburn
0 siblings, 1 reply; 7+ messages in thread
From: Dmitry @ 2020-11-09 22:09 UTC (permalink / raw)
To: development; +Cc: The development of GNU GRUB
вт, 10 нояб. 2020 г. в 00:34, Glenn Washburn <development@efficientek.com>:
>
> On Mon, 9 Nov 2020 10:56:51 +0300
> Dmitry <reagentoo@gmail.com> wrote:
>
> > Thanks for feedback. First of all I think it's preferable to introduce
> > the master-key option at the beginning. Because I see no point for a
> > user to use a standard slot key along with a detached header.
> > Decryption from key to master key takes a long time. (30 seconds for
> > argon2i).
>
> Are you using the argon patches on this list, which haven't been
> committed to run that test? Because currently there is no support for
> Argon2.
I'm using argon2 patchset from Patrick Steinhardt:
https://lists.gnu.org/archive/html/grub-devel/2020-02/msg00040.html
More info:
https://github.com/johnlane/grub/issues/21#issuecomment-721097980
>
> Second detached headers have nothing to do with how long Argon2 or any
> other key derivation function takes to complete, so they are
> independent (or am I wrong?). Am I correct in understanding you to say
> that argon2i is faster without having a detached header?
No. I talked about usage pattern.
It makes no sense to use a detached header and a keyfile that are stored
on the same device. And therefore it is better to use a master-key that doesn't
need to be decrypted with the argon2 algorithm
(or old LUKS2_KDF_TYPE_PBKDF2 method).
In this case we need a header file only for master-key verification.
(digest comparison in luks2_verify_key).
>
> Third, I actually do currently use detached headers (as I said before I
> have an unsubmitted patch which adds detached header and keyfile). And
> separately, yes the key derivation can take a while (compared to running
> in linux). I'm not exactly clear on why this is. I have some ideas, like
> the code grub is using to do crypto or grub's runtime environment, but
> I haven't really looked in to it. Do you have any ideas? And I know
> there are others currently using the detached header without the master
> key option, so there's definitely a point.
>
The mailing lists have suggested that SSE instructions should be used
to speed up decryption:
https://lists.gnu.org/archive/html/grub-devel/2018-01/msg00026.html
> > Regarding keyfile-offset and keyfile-size options I suggest to
> > introduce this options in separate patches. Yes, maybe you variant to
> > pass the pointer and size
> > (correct me if I am wrong) to luks2_recover_key is better of files.
> > But then you need to think in advance how to add master-key option.
>
> It can be added just like the key-file option was added in the previous
> patch series. Read the master key file from grub_cmd_cryptomount.
>
> Glenn
>
>
> > пн, 9 нояб. 2020 г. в 06:16, Glenn Washburn
> > <development@efficientek.com>:
> >
> > > I've read through the patch but not applied or tested it. However,
> > > it looks like it does the job. In fact, its fairly similar in
> > > parts to a patch, which adds LUKS2 keyfile and detached header
> > > support, I've been waiting to send to the list until the previous
> > > LUKS1 keyfile and detached header support gets included (which my
> > > patches rely on).
> > >
> > > One implementation difference in this patch from the other one is
> > > that this one passes the key file to recover_key as a grub_file_t
> > > instead of passing the key contents. I prefer the previous method
> > > because it consolidates key file reading in grub_cmd_cryptomount,
> > > whereas this patch requires each module to do its own reading. The
> > > disadvantage is this causes extra code in the modules that could be
> > > consolidated and I don't see much advantage to this. By having
> > > grub_cmd_cryptomount do the work we also get keyfile-offset and
> > > keyfile-size options in the previous patch, for not much extra work.
> > >
> > > A minor nit, there is not error checking on grub_file_seek calls and
> > > grub_file_read errors are occluded. However, there are some
> > > changes in this patch which I think make the code a bit more
> > > aesthetically pleasing.
> > >
> > > While, I like that this patch allows for passing a master key file,
> > > I think this feature should be build on the previous patch series.
> > > So I don't recommend this patch be accepted.
> > >
> > > Glenn
> > >
> > > On Sat, 7 Nov 2020 19:36:35 +0300
> > > reagentoo <reagentoo@gmail.com> wrote:
> > >
> > > > From: Dmitry Baranov <reagentoo@gmail.com>
> > > >
> > > > Hello.
> > > > Could someone make initial review?
> > > > The main difference between this patch and the previous ones is
> > > > the ability to use a master key file with a detached header.
> > > > This patch does not provide luks1 and geli support yet (luks2
> > > > only).
> > > >
> > > > Best Regards,
> > > > Dmitry
> > > >
> > > > ---
> > > > grub-core/disk/cryptodisk.c | 99 +++++++++++++++-----
> > > > grub-core/disk/luks2.c | 178
> > > > ++++++++++++++++++++++++++++-------- include/grub/cryptodisk.h |
> > > > 9 +- 3 files changed, 222 insertions(+), 64 deletions(-)
> > > >
> > > > diff --git a/grub-core/disk/cryptodisk.c
> > > > b/grub-core/disk/cryptodisk.c index 13af84dd1..987a39b16 100644
> > > > --- a/grub-core/disk/cryptodisk.c
> > > > +++ b/grub-core/disk/cryptodisk.c
> > > > @@ -41,6 +41,9 @@ static const struct grub_arg_option options[] =
> > > > /* TRANSLATORS: It's still restricted to cryptodisks only.
> > > > */ {"all", 'a', 0, N_("Mount all."), 0, 0},
> > > > {"boot", 'b', 0, N_("Mount all volumes with `boot' flag
> > > > set."), 0, 0},
> > > > + {"header", 'H', 0, N_("Read header from file"), 0,
> > > > ARG_TYPE_STRING},
> > > > + {"key-file", 'k', 0, N_("Read key from file"), 0,
> > > > ARG_TYPE_STRING},
> > > > + {"master-key-file", 'K', 0, N_("Use a master key stored in a
> > > > file"), 0, ARG_TYPE_STRING}, {0, 0, 0, 0, 0, 0}
> > > > };
> > > >
> > > > @@ -967,6 +970,7 @@ grub_util_cryptodisk_get_uuid (grub_disk_t
> > > > disk)
> > > >
> > > > static int check_boot, have_it;
> > > > static char *search_uuid;
> > > > +static grub_file_t hdr_file, key_file, mkey_file;
> > > >
> > > > static void
> > > > cryptodisk_close (grub_cryptodisk_t dev)
> > > > @@ -991,13 +995,13 @@ grub_cryptodisk_scan_device_real (const char
> > > > *name, grub_disk_t source)
> > > > FOR_CRYPTODISK_DEVS (cr)
> > > > {
> > > > - dev = cr->scan (source, search_uuid, check_boot);
> > > > + dev = cr->scan (source, hdr_file, search_uuid, check_boot);
> > > > if (grub_errno)
> > > > return grub_errno;
> > > > if (!dev)
> > > > continue;
> > > >
> > > > - err = cr->recover_key (source, dev);
> > > > + err = cr->recover_key (source, dev, hdr_file, key_file,
> > > > mkey_file); if (err)
> > > > {
> > > > cryptodisk_close (dev);
> > > > @@ -1038,7 +1042,7 @@ grub_cryptodisk_cheat_mount (const char
> > > > *sourcedev, const char *cheat)
> > > > FOR_CRYPTODISK_DEVS (cr)
> > > > {
> > > > - dev = cr->scan (source, search_uuid, check_boot);
> > > > + dev = cr->scan (source, 0, search_uuid, check_boot);
> > > > if (grub_errno)
> > > > return grub_errno;
> > > > if (!dev)
> > > > @@ -1087,11 +1091,59 @@ grub_cryptodisk_scan_device (const char
> > > > *name, static grub_err_t
> > > > grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char
> > > > **args) {
> > > > + grub_err_t ret = GRUB_ERR_NONE;
> > > > +
> > > > struct grub_arg_list *state = ctxt->state;
> > > >
> > > > if (argc < 1 && !state[1].set && !state[2].set)
> > > > return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name
> > > > required");
> > > > + hdr_file = NULL;
> > > > + key_file = NULL;
> > > > + mkey_file = NULL;
> > > > +
> > > > + if (state[3].set)
> > > > + {
> > > > + if (state[0].set)
> > > > + {
> > > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot use UUID
> > > > lookup with detached header");
> > > > + goto err;
> > > > + }
> > > > +
> > > > + hdr_file = grub_file_open (state[3].arg,
> > > > GRUB_FILE_TYPE_NONE);
> > > > + if (!hdr_file)
> > > > + {
> > > > + ret = grub_errno;
> > > > + goto err;
> > > > + }
> > > > + }
> > > > +
> > > > + if (state[4].set)
> > > > + {
> > > > + if (state[5].set)
> > > > + {
> > > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot use key
> > > > with master key");
> > > > + goto err;
> > > > + }
> > > > +
> > > > + key_file = grub_file_open (state[4].arg,
> > > > GRUB_FILE_TYPE_NONE);
> > > > + if (!key_file)
> > > > + {
> > > > + ret = grub_errno;
> > > > + goto err;
> > > > + }
> > > > + }
> > > > +
> > > > + if (state[5].set)
> > > > + {
> > > > + mkey_file = grub_file_open (state[5].arg,
> > > > GRUB_FILE_TYPE_NONE);
> > > > + if (!mkey_file)
> > > > + {
> > > > + ret = grub_errno;
> > > > + goto err;
> > > > + }
> > > > + }
> > > > +
> > > > have_it = 0;
> > > > if (state[0].set)
> > > > {
> > > > @@ -1102,7 +1154,7 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > > > ctxt, int argc, char **args) {
> > > > grub_dprintf ("cryptodisk",
> > > > "already mounted as crypto%lu\n", dev->id);
> > > > - return GRUB_ERR_NONE;
> > > > + goto err;
> > > > }
> > > >
> > > > check_boot = state[2].set;
> > > > @@ -1111,8 +1163,11 @@ grub_cmd_cryptomount (grub_extcmd_context_t
> > > > ctxt, int argc, char **args) search_uuid = NULL;
> > > >
> > > > if (!have_it)
> > > > - return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such
> > > > cryptodisk found");
> > > > - return GRUB_ERR_NONE;
> > > > + {
> > > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "no such
> > > > cryptodisk found");
> > > > + goto err;
> > > > + }
> > > > + goto err;
> > > > }
> > > > else if (state[1].set || (argc == 0 && state[2].set))
> > > > {
> > > > @@ -1120,11 +1175,10 @@ grub_cmd_cryptomount
> > > > (grub_extcmd_context_t ctxt, int argc, char **args) check_boot =
> > > > state[2].set; grub_device_iterate (&grub_cryptodisk_scan_device,
> > > > NULL); search_uuid = NULL;
> > > > - return GRUB_ERR_NONE;
> > > > + goto err;
> > > > }
> > > > else
> > > > {
> > > > - grub_err_t err;
> > > > grub_disk_t disk;
> > > > grub_cryptodisk_t dev;
> > > > char *diskname;
> > > > @@ -1147,27 +1201,30 @@ grub_cmd_cryptomount
> > > > (grub_extcmd_context_t ctxt, int argc, char **args) {
> > > > if (disklast)
> > > > *disklast = ')';
> > > > - return grub_errno;
> > > > + ret = grub_errno;
> > > > + goto err;
> > > > }
> > > >
> > > > dev = grub_cryptodisk_get_by_source_disk (disk);
> > > > if (dev)
> > > > - {
> > > > - grub_dprintf ("cryptodisk", "already mounted as
> > > > crypto%lu\n", dev->id);
> > > > - grub_disk_close (disk);
> > > > - if (disklast)
> > > > - *disklast = ')';
> > > > - return GRUB_ERR_NONE;
> > > > - }
> > > > -
> > > > - err = grub_cryptodisk_scan_device_real (diskname, disk);
> > > > + grub_dprintf ("cryptodisk", "already mounted as
> > > > crypto%lu\n", dev->id);
> > > > + else
> > > > + ret = grub_cryptodisk_scan_device_real (diskname, disk);
> > > >
> > > > grub_disk_close (disk);
> > > > if (disklast)
> > > > *disklast = ')';
> > > > -
> > > > - return err;
> > > > }
> > > > +
> > > > + err:
> > > > + if (hdr_file)
> > > > + grub_file_close (hdr_file);
> > > > + if (key_file)
> > > > + grub_file_close (key_file);
> > > > + if (mkey_file)
> > > > + grub_file_close (mkey_file);
> > > > +
> > > > + return ret;
> > > > }
> > > >
> > > > static struct grub_disk_dev grub_cryptodisk_dev = {
> > > > @@ -1299,7 +1356,7 @@ GRUB_MOD_INIT (cryptodisk)
> > > > {
> > > > grub_disk_dev_register (&grub_cryptodisk_dev);
> > > > cmd = grub_register_extcmd ("cryptomount",
> > > > grub_cmd_cryptomount, 0,
> > > > - N_("SOURCE|-u UUID|-a|-b"),
> > > > + N_("SOURCE|-u UUID|-a|-b|-H file|-k
> > > > file|-K file"), N_("Mount a crypto device."), options);
> > > > grub_procfs_register ("luks_script", &luks_script);
> > > > }
> > > > diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> > > > index 31d7166fc..c29b472e6 100644
> > > > --- a/grub-core/disk/luks2.c
> > > > +++ b/grub-core/disk/luks2.c
> > > > @@ -309,13 +309,22 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k,
> > > > grub_luks2_digest_t *d, grub_luks2_s
> > > > /* Determine whether to use primary or secondary header */
> > > > static grub_err_t
> > > > -luks2_read_header (grub_disk_t disk, grub_luks2_header_t *outhdr)
> > > > +luks2_read_header (grub_disk_t disk, grub_file_t hdr_file,
> > > > grub_luks2_header_t *outhdr) {
> > > > grub_luks2_header_t primary, secondary, *header = &primary;
> > > > grub_err_t ret;
> > > >
> > > > /* Read the primary LUKS header. */
> > > > - ret = grub_disk_read (disk, 0, 0, sizeof (primary), &primary);
> > > > + if (hdr_file)
> > > > + {
> > > > + grub_file_seek (hdr_file, 0);
> > > > + if (grub_file_read (hdr_file, &primary, sizeof (primary))
> > > > != sizeof (primary))
> > > > + ret = GRUB_ERR_READ_ERROR;
> > > > + else
> > > > + ret = GRUB_ERR_NONE;
> > > > + }
> > > > + else
> > > > + ret = grub_disk_read (disk, 0, 0, sizeof (primary),
> > > > &primary); if (ret)
> > > > return ret;
> > > >
> > > > @@ -325,7 +334,16 @@ luks2_read_header (grub_disk_t disk,
> > > > grub_luks2_header_t *outhdr) return GRUB_ERR_BAD_SIGNATURE;
> > > >
> > > > /* Read the secondary header. */
> > > > - ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> > > > (primary.hdr_size), sizeof (secondary), &secondary);
> > > > + if (hdr_file)
> > > > + {
> > > > + grub_file_seek (hdr_file, grub_be_to_cpu64
> > > > (primary.hdr_size));
> > > > + if (grub_file_read (hdr_file, &secondary, sizeof
> > > > (secondary)) != sizeof (secondary))
> > > > + ret = GRUB_ERR_READ_ERROR;
> > > > + else
> > > > + ret = GRUB_ERR_NONE;
> > > > + }
> > > > + else
> > > > + ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> > > > (primary.hdr_size), sizeof (secondary), &secondary); if (ret)
> > > > return ret;
> > > >
> > > > @@ -342,7 +360,7 @@ luks2_read_header (grub_disk_t disk,
> > > > grub_luks2_header_t *outhdr) }
> > > >
> > > > static grub_cryptodisk_t
> > > > -luks2_scan (grub_disk_t disk, const char *check_uuid, int
> > > > check_boot) +luks2_scan (grub_disk_t disk, grub_file_t hdr_file,
> > > > const char *check_uuid, int check_boot) {
> > > > grub_cryptodisk_t cryptodisk;
> > > > grub_luks2_header_t header;
> > > > @@ -352,7 +370,7 @@ luks2_scan (grub_disk_t disk, const char
> > > > *check_uuid, int check_boot) if (check_boot)
> > > > return NULL;
> > > >
> > > > - if (luks2_read_header (disk, &header))
> > > > + if (luks2_read_header (disk, hdr_file, &header))
> > > > {
> > > > grub_errno = GRUB_ERR_NONE;
> > > > return NULL;
> > > > @@ -415,7 +433,7 @@ luks2_verify_key (grub_luks2_digest_t *d,
> > > > grub_uint8_t *candidate_key,
> > > > static grub_err_t
> > > > luks2_decrypt_key (grub_uint8_t *out_key,
> > > > - grub_disk_t disk, grub_cryptodisk_t crypt,
> > > > + grub_disk_t disk, grub_cryptodisk_t crypt,
> > > > grub_file_t hdr_file, grub_luks2_keyslot_t *k,
> > > > const grub_uint8_t *passphrase, grub_size_t
> > > > passphraselen) {
> > > > @@ -491,7 +509,16 @@ luks2_decrypt_key (grub_uint8_t *out_key,
> > > > }
> > > >
> > > > grub_errno = GRUB_ERR_NONE;
> > > > - ret = grub_disk_read (disk, 0, k->area.offset, k->area.size,
> > > > split_key);
> > > > + if (hdr_file)
> > > > + {
> > > > + grub_file_seek (hdr_file, k->area.offset);
> > > > + if (grub_file_read (hdr_file, split_key, k->area.size) !=
> > > > k->area.size)
> > > > + ret = GRUB_ERR_READ_ERROR;
> > > > + else
> > > > + ret = GRUB_ERR_NONE;
> > > > + }
> > > > + else
> > > > + ret = grub_disk_read (disk, 0, k->area.offset,
> > > > k->area.size, split_key); if (ret)
> > > > {
> > > > grub_error (GRUB_ERR_IO, "Read error: %s\n", grub_errmsg);
> > > > @@ -531,12 +558,16 @@ luks2_decrypt_key (grub_uint8_t *out_key,
> > > >
> > > > static grub_err_t
> > > > luks2_recover_key (grub_disk_t disk,
> > > > - grub_cryptodisk_t crypt)
> > > > + grub_cryptodisk_t crypt,
> > > > + grub_file_t hdr_file, grub_file_t key_file,
> > > > grub_file_t mkey_file) {
> > > > + char cipher[32];
> > > > + char *passphrase = NULL;
> > > > grub_uint8_t candidate_key[GRUB_CRYPTODISK_MAX_KEYLEN];
> > > > - char passphrase[MAX_PASSPHRASE], cipher[32];
> > > > + grub_size_t candidate_key_len, passphrase_len;
> > > > char *json_header = NULL, *part = NULL, *ptr;
> > > > - grub_size_t candidate_key_len = 0, i, size;
> > > > + grub_off_t json_header_offset;
> > > > + grub_size_t json_header_size, i, size;
> > > > grub_luks2_header_t header;
> > > > grub_luks2_keyslot_t keyslot;
> > > > grub_luks2_digest_t digest;
> > > > @@ -545,7 +576,7 @@ luks2_recover_key (grub_disk_t disk,
> > > > grub_json_t *json = NULL, keyslots;
> > > > grub_err_t ret;
> > > >
> > > > - ret = luks2_read_header (disk, &header);
> > > > + ret = luks2_read_header (disk, hdr_file, &header);
> > > > if (ret)
> > > > return ret;
> > > >
> > > > @@ -554,8 +585,19 @@ luks2_recover_key (grub_disk_t disk,
> > > > return GRUB_ERR_OUT_OF_MEMORY;
> > > >
> > > > /* Read the JSON area. */
> > > > - ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> > > > (header.hdr_offset) + sizeof (header),
> > > > - grub_be_to_cpu64 (header.hdr_size) - sizeof
> > > > (header), json_header);
> > > > + json_header_offset = grub_be_to_cpu64 (header.hdr_offset) +
> > > > sizeof (header);
> > > > + json_header_size = grub_be_to_cpu64 (header.hdr_size) - sizeof
> > > > (header); +
> > > > + if (hdr_file)
> > > > + {
> > > > + grub_file_seek (hdr_file, json_header_offset);
> > > > + if (grub_file_read (hdr_file, json_header,
> > > > json_header_size) != json_header_size)
> > > > + ret = GRUB_ERR_READ_ERROR;
> > > > + else
> > > > + ret = GRUB_ERR_NONE;
> > > > + }
> > > > + else
> > > > + ret = grub_disk_read (disk, 0, json_header_offset,
> > > > json_header_size, json_header); if (ret)
> > > > goto err;
> > > >
> > > > @@ -570,16 +612,47 @@ luks2_recover_key (grub_disk_t disk,
> > > > goto err;
> > > > }
> > > >
> > > > - /* Get the passphrase from the user. */
> > > > - if (disk->partition)
> > > > - part = grub_partition_get_name (disk->partition);
> > > > - grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "),
> > > > disk->name,
> > > > - disk->partition ? "," : "", part ? : "",
> > > > - crypt->uuid);
> > > > - if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> > > > + if (mkey_file)
> > > > {
> > > > - ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not
> > > > supplied");
> > > > - goto err;
> > > > + /* Get the master key from file. */
> > > > + candidate_key_len = grub_file_read (mkey_file,
> > > > candidate_key, GRUB_CRYPTODISK_MAX_KEYLEN);
> > > > + if (candidate_key_len == (grub_size_t)-1)
> > > > + {
> > > > + ret = grub_error (GRUB_ERR_FILE_READ_ERROR, "Error reading
> > > > key file");
> > > > + goto err;
> > > > + }
> > > > + if (candidate_key_len == 0)
> > > > + {
> > > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Key not
> > > > supplied");
> > > > + goto err;
> > > > + }
> > > > + }
> > > > + else if (key_file)
> > > > + {
> > > > + /* Get the passphrase from file. */
> > > > + passphrase = grub_malloc (GRUB_CRYPTODISK_MAX_PASSPHRASE);
> > > > + passphrase_len = grub_file_read (key_file, passphrase,
> > > > GRUB_CRYPTODISK_MAX_PASSPHRASE);
> > > > + if (passphrase_len == (grub_size_t)-1)
> > > > + {
> > > > + ret = grub_error (GRUB_ERR_FILE_READ_ERROR, "Error reading
> > > > passphrase file");
> > > > + goto err;
> > > > + }
> > > > + }
> > > > + else
> > > > + {
> > > > + /* Get the passphrase from the user. */
> > > > + if (disk->partition)
> > > > + part = grub_partition_get_name (disk->partition);
> > > > + grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "),
> > > > disk->name,
> > > > + disk->partition ? "," : "", part ? : "",
> > > > + crypt->uuid);
> > > > + passphrase = grub_malloc (MAX_PASSPHRASE);
> > > > + if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> > > > + {
> > > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not
> > > > supplied");
> > > > + goto err;
> > > > + }
> > > > + passphrase_len = grub_strlen (passphrase);
> > > > }
> > > >
> > > > if (grub_json_getvalue (&keyslots, json, "keyslots") ||
> > > > @@ -604,6 +677,26 @@ luks2_recover_key (grub_disk_t disk,
> > > >
> > > > grub_dprintf ("luks2", "Trying keyslot
> > > > %"PRIuGRUB_SIZE"\n", i);
> > > >
> > > > + if (mkey_file)
> > > > + {
> > > > + if (candidate_key_len != keyslot.key_size)
> > > > + {
> > > > + grub_dprintf ("luks2", "Wrong key length for keyslot
> > > > %"PRIuGRUB_SIZE": %s\n",
> > > > + i, grub_errmsg);
> > > > + continue;
> > > > + }
> > > > +
> > > > + ret = luks2_verify_key (&digest, candidate_key,
> > > > candidate_key_len);
> > > > + if (ret)
> > > > + {
> > > > + grub_dprintf ("luks2", "Could not open keyslot
> > > > %"PRIuGRUB_SIZE": %s\n",
> > > > + i, grub_errmsg);
> > > > + continue;
> > > > + }
> > > > + }
> > > > + else
> > > > + candidate_key_len = keyslot.key_size;
> > > > +
> > > > /* Set up disk according to keyslot's segment. */
> > > > crypt->offset = grub_divmod64 (segment.offset,
> > > > segment.sector_size, NULL); crypt->log_sector_size = sizeof
> > > > (unsigned int) * 8 @@ -613,21 +706,24 @@ luks2_recover_key
> > > > (grub_disk_t disk, else
> > > > crypt->total_length = grub_strtoull (segment.size, NULL,
> > > > 10);
> > > >
> > > > - ret = luks2_decrypt_key (candidate_key, disk, crypt,
> > > > &keyslot,
> > > > - (const grub_uint8_t *) passphrase,
> > > > grub_strlen (passphrase));
> > > > - if (ret)
> > > > + if (passphrase)
> > > > {
> > > > - grub_dprintf ("luks2", "Decryption with keyslot
> > > > %"PRIuGRUB_SIZE" failed: %s\n",
> > > > - i, grub_errmsg);
> > > > - continue;
> > > > - }
> > > > -
> > > > - ret = luks2_verify_key (&digest, candidate_key,
> > > > keyslot.key_size);
> > > > - if (ret)
> > > > - {
> > > > - grub_dprintf ("luks2", "Could not open keyslot
> > > > %"PRIuGRUB_SIZE": %s\n",
> > > > - i, grub_errmsg);
> > > > - continue;
> > > > + ret = luks2_decrypt_key (candidate_key, disk, crypt,
> > > > hdr_file, &keyslot,
> > > > + (const grub_uint8_t *)
> > > > passphrase, passphrase_len);
> > > > + if (ret)
> > > > + {
> > > > + grub_dprintf ("luks2", "Decryption with keyslot
> > > > %"PRIuGRUB_SIZE" failed: %s\n",
> > > > + i, grub_errmsg);
> > > > + continue;
> > > > + }
> > > > +
> > > > + ret = luks2_verify_key (&digest, candidate_key,
> > > > candidate_key_len);
> > > > + if (ret)
> > > > + {
> > > > + grub_dprintf ("luks2", "Could not open keyslot
> > > > %"PRIuGRUB_SIZE": %s\n",
> > > > + i, grub_errmsg);
> > > > + continue;
> > > > + }
> > > > }
> > > >
> > > > /*
> > > > @@ -635,13 +731,14 @@ luks2_recover_key (grub_disk_t disk,
> > > > * where each element is either empty or holds a key.
> > > > */
> > > > grub_printf_ (N_("Slot %"PRIuGRUB_SIZE" opened\n"), i);
> > > > -
> > > > - candidate_key_len = keyslot.key_size;
> > > > break;
> > > > }
> > > > - if (candidate_key_len == 0)
> > > > + if (i == size)
> > > > {
> > > > - ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid
> > > > passphrase");
> > > > + if (mkey_file)
> > > > + ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid key");
> > > > + else
> > > > + ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid
> > > > passphrase"); goto err;
> > > > }
> > > >
> > > > @@ -666,6 +763,7 @@ luks2_recover_key (grub_disk_t disk,
> > > >
> > > > err:
> > > > grub_free (part);
> > > > + grub_free (passphrase);
> > > > grub_free (json_header);
> > > > grub_json_free (json);
> > > > return ret;
> > > > diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
> > > > index e1b21e785..b53006854 100644
> > > > --- a/include/grub/cryptodisk.h
> > > > +++ b/include/grub/cryptodisk.h
> > > > @@ -21,6 +21,7 @@
> > > >
> > > > #include <grub/disk.h>
> > > > #include <grub/crypto.h>
> > > > +#include <grub/file.h>
> > > > #include <grub/list.h>
> > > > #ifdef GRUB_UTIL
> > > > #include <grub/emu/hostdisk.h>
> > > > @@ -53,6 +54,7 @@ typedef enum
> > > > #define GRUB_CRYPTODISK_GF_LOG_BYTES
> > > > (GRUB_CRYPTODISK_GF_LOG_SIZE - 3) #define
> > > > GRUB_CRYPTODISK_GF_BYTES (1U << GRUB_CRYPTODISK_GF_LOG_BYTES)
> > > > #define GRUB_CRYPTODISK_MAX_KEYLEN 128 +#define
> > > > GRUB_CRYPTODISK_MAX_PASSPHRASE 8192
> > > >
> > > > struct grub_cryptodisk;
> > > >
> > > > @@ -106,9 +108,10 @@ struct grub_cryptodisk_dev
> > > > struct grub_cryptodisk_dev *next;
> > > > struct grub_cryptodisk_dev **prev;
> > > >
> > > > - grub_cryptodisk_t (*scan) (grub_disk_t disk, const char
> > > > *check_uuid,
> > > > - int boot_only);
> > > > - grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t
> > > > dev);
> > > > + grub_cryptodisk_t (*scan) (grub_disk_t disk, grub_file_t
> > > > hdr_file,
> > > > + const char *check_uuid, int boot_only);
> > > > + grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t
> > > > dev,
> > > > + grub_file_t hdr_file, grub_file_t
> > > > key_file, grub_file_t mkey_file); };
> > > > typedef struct grub_cryptodisk_dev *grub_cryptodisk_dev_t;
> > > >
> > >
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] Cryptomount support for key files and detached header
2020-11-09 22:09 ` Dmitry
@ 2020-11-10 20:25 ` Glenn Washburn
0 siblings, 0 replies; 7+ messages in thread
From: Glenn Washburn @ 2020-11-10 20:25 UTC (permalink / raw)
To: Dmitry; +Cc: The development of GNU GRUB
On Tue, 10 Nov 2020 01:09:20 +0300
Dmitry <reagentoo@gmail.com> wrote:
> вт, 10 нояб. 2020 г. в 00:34, Glenn Washburn
> <development@efficientek.com>:
> >
> > On Mon, 9 Nov 2020 10:56:51 +0300
> > Dmitry <reagentoo@gmail.com> wrote:
> >
> > > Thanks for feedback. First of all I think it's preferable to
> > > introduce the master-key option at the beginning. Because I see
> > > no point for a user to use a standard slot key along with a
> > > detached header. Decryption from key to master key takes a long
> > > time. (30 seconds for argon2i).
> >
> > Are you using the argon patches on this list, which haven't been
> > committed to run that test? Because currently there is no support
> > for Argon2.
>
> I'm using argon2 patchset from Patrick Steinhardt:
> https://lists.gnu.org/archive/html/grub-devel/2020-02/msg00040.html
> More info:
> https://github.com/johnlane/grub/issues/21#issuecomment-721097980
>
> >
> > Second detached headers have nothing to do with how long Argon2 or
> > any other key derivation function takes to complete, so they are
> > independent (or am I wrong?). Am I correct in understanding you to
> > say that argon2i is faster without having a detached header?
>
> No. I talked about usage pattern.
> It makes no sense to use a detached header and a keyfile that are
> stored on the same device.
I take you to mean that it makes no sense because using a master key is
more efficient. And I agree.
> And therefore it is better to use a
> master-key that doesn't need to be decrypted with the argon2 algorithm
> (or old LUKS2_KDF_TYPE_PBKDF2 method).
It does not follow that _in general_ the master key feature is more
useful than detached header support. Perhaps I misunderstood what
you were initially suggesting. And on the contrary, I would say that
detached header support is more important because the master key
feature is only an efficiency enhancement, while detached header is a
functional enhancement. So I still think the preferred path forward is
to merge the previous detached header support, then add the master key
on top.
> In this case we need a header file only for master-key verification.
> (digest comparison in luks2_verify_key).
>
> >
> > Third, I actually do currently use detached headers (as I said
> > before I have an unsubmitted patch which adds detached header and
> > keyfile). And separately, yes the key derivation can take a while
> > (compared to running in linux). I'm not exactly clear on why this
> > is. I have some ideas, like the code grub is using to do crypto or
> > grub's runtime environment, but I haven't really looked in to it.
> > Do you have any ideas? And I know there are others currently using
> > the detached header without the master key option, so there's
> > definitely a point.
> >
> The mailing lists have suggested that SSE instructions should be used
> to speed up decryption:
> https://lists.gnu.org/archive/html/grub-devel/2018-01/msg00026.html
Good find. This should be a good starting point.
> > > Regarding keyfile-offset and keyfile-size options I suggest to
> > > introduce this options in separate patches. Yes, maybe you
> > > variant to pass the pointer and size
> > > (correct me if I am wrong) to luks2_recover_key is better of
> > > files. But then you need to think in advance how to add
> > > master-key option.
> >
> > It can be added just like the key-file option was added in the
> > previous patch series. Read the master key file from
> > grub_cmd_cryptomount.
> >
Also, I forgot to mention that I think any new features accepted to
grub should have accompanying documentation changes. The previous
detached header patches don't either, but I think this should be
essentially required.
Glenn
> >
> > > пн, 9 нояб. 2020 г. в 06:16, Glenn Washburn
> > > <development@efficientek.com>:
> > >
> > > > I've read through the patch but not applied or tested it.
> > > > However, it looks like it does the job. In fact, its fairly
> > > > similar in parts to a patch, which adds LUKS2 keyfile and
> > > > detached header support, I've been waiting to send to the list
> > > > until the previous LUKS1 keyfile and detached header support
> > > > gets included (which my patches rely on).
> > > >
> > > > One implementation difference in this patch from the other one
> > > > is that this one passes the key file to recover_key as a
> > > > grub_file_t instead of passing the key contents. I prefer the
> > > > previous method because it consolidates key file reading in
> > > > grub_cmd_cryptomount, whereas this patch requires each module
> > > > to do its own reading. The disadvantage is this causes extra
> > > > code in the modules that could be consolidated and I don't see
> > > > much advantage to this. By having grub_cmd_cryptomount do the
> > > > work we also get keyfile-offset and keyfile-size options in the
> > > > previous patch, for not much extra work.
> > > >
> > > > A minor nit, there is not error checking on grub_file_seek
> > > > calls and grub_file_read errors are occluded. However, there
> > > > are some changes in this patch which I think make the code a
> > > > bit more aesthetically pleasing.
> > > >
> > > > While, I like that this patch allows for passing a master key
> > > > file, I think this feature should be build on the previous
> > > > patch series. So I don't recommend this patch be accepted.
> > > >
> > > > Glenn
> > > >
> > > > On Sat, 7 Nov 2020 19:36:35 +0300
> > > > reagentoo <reagentoo@gmail.com> wrote:
> > > >
> > > > > From: Dmitry Baranov <reagentoo@gmail.com>
> > > > >
> > > > > Hello.
> > > > > Could someone make initial review?
> > > > > The main difference between this patch and the previous ones
> > > > > is the ability to use a master key file with a detached
> > > > > header. This patch does not provide luks1 and geli support
> > > > > yet (luks2 only).
> > > > >
> > > > > Best Regards,
> > > > > Dmitry
> > > > >
> > > > > ---
> > > > > grub-core/disk/cryptodisk.c | 99 +++++++++++++++-----
> > > > > grub-core/disk/luks2.c | 178
> > > > > ++++++++++++++++++++++++++++--------
> > > > > include/grub/cryptodisk.h | 9 +- 3 files changed, 222
> > > > > insertions(+), 64 deletions(-)
> > > > >
> > > > > diff --git a/grub-core/disk/cryptodisk.c
> > > > > b/grub-core/disk/cryptodisk.c index 13af84dd1..987a39b16
> > > > > 100644 --- a/grub-core/disk/cryptodisk.c
> > > > > +++ b/grub-core/disk/cryptodisk.c
> > > > > @@ -41,6 +41,9 @@ static const struct grub_arg_option
> > > > > options[] = /* TRANSLATORS: It's still restricted to
> > > > > cryptodisks only. */ {"all", 'a', 0, N_("Mount all."), 0, 0},
> > > > > {"boot", 'b', 0, N_("Mount all volumes with `boot' flag
> > > > > set."), 0, 0},
> > > > > + {"header", 'H', 0, N_("Read header from file"), 0,
> > > > > ARG_TYPE_STRING},
> > > > > + {"key-file", 'k', 0, N_("Read key from file"), 0,
> > > > > ARG_TYPE_STRING},
> > > > > + {"master-key-file", 'K', 0, N_("Use a master key stored
> > > > > in a file"), 0, ARG_TYPE_STRING}, {0, 0, 0, 0, 0, 0}
> > > > > };
> > > > >
> > > > > @@ -967,6 +970,7 @@ grub_util_cryptodisk_get_uuid (grub_disk_t
> > > > > disk)
> > > > >
> > > > > static int check_boot, have_it;
> > > > > static char *search_uuid;
> > > > > +static grub_file_t hdr_file, key_file, mkey_file;
> > > > >
> > > > > static void
> > > > > cryptodisk_close (grub_cryptodisk_t dev)
> > > > > @@ -991,13 +995,13 @@ grub_cryptodisk_scan_device_real (const
> > > > > char *name, grub_disk_t source)
> > > > > FOR_CRYPTODISK_DEVS (cr)
> > > > > {
> > > > > - dev = cr->scan (source, search_uuid, check_boot);
> > > > > + dev = cr->scan (source, hdr_file, search_uuid,
> > > > > check_boot); if (grub_errno)
> > > > > return grub_errno;
> > > > > if (!dev)
> > > > > continue;
> > > > >
> > > > > - err = cr->recover_key (source, dev);
> > > > > + err = cr->recover_key (source, dev, hdr_file, key_file,
> > > > > mkey_file); if (err)
> > > > > {
> > > > > cryptodisk_close (dev);
> > > > > @@ -1038,7 +1042,7 @@ grub_cryptodisk_cheat_mount (const char
> > > > > *sourcedev, const char *cheat)
> > > > > FOR_CRYPTODISK_DEVS (cr)
> > > > > {
> > > > > - dev = cr->scan (source, search_uuid, check_boot);
> > > > > + dev = cr->scan (source, 0, search_uuid, check_boot);
> > > > > if (grub_errno)
> > > > > return grub_errno;
> > > > > if (!dev)
> > > > > @@ -1087,11 +1091,59 @@ grub_cryptodisk_scan_device (const
> > > > > char *name, static grub_err_t
> > > > > grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc,
> > > > > char **args) {
> > > > > + grub_err_t ret = GRUB_ERR_NONE;
> > > > > +
> > > > > struct grub_arg_list *state = ctxt->state;
> > > > >
> > > > > if (argc < 1 && !state[1].set && !state[2].set)
> > > > > return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name
> > > > > required");
> > > > > + hdr_file = NULL;
> > > > > + key_file = NULL;
> > > > > + mkey_file = NULL;
> > > > > +
> > > > > + if (state[3].set)
> > > > > + {
> > > > > + if (state[0].set)
> > > > > + {
> > > > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot use
> > > > > UUID lookup with detached header");
> > > > > + goto err;
> > > > > + }
> > > > > +
> > > > > + hdr_file = grub_file_open (state[3].arg,
> > > > > GRUB_FILE_TYPE_NONE);
> > > > > + if (!hdr_file)
> > > > > + {
> > > > > + ret = grub_errno;
> > > > > + goto err;
> > > > > + }
> > > > > + }
> > > > > +
> > > > > + if (state[4].set)
> > > > > + {
> > > > > + if (state[5].set)
> > > > > + {
> > > > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "cannot use
> > > > > key with master key");
> > > > > + goto err;
> > > > > + }
> > > > > +
> > > > > + key_file = grub_file_open (state[4].arg,
> > > > > GRUB_FILE_TYPE_NONE);
> > > > > + if (!key_file)
> > > > > + {
> > > > > + ret = grub_errno;
> > > > > + goto err;
> > > > > + }
> > > > > + }
> > > > > +
> > > > > + if (state[5].set)
> > > > > + {
> > > > > + mkey_file = grub_file_open (state[5].arg,
> > > > > GRUB_FILE_TYPE_NONE);
> > > > > + if (!mkey_file)
> > > > > + {
> > > > > + ret = grub_errno;
> > > > > + goto err;
> > > > > + }
> > > > > + }
> > > > > +
> > > > > have_it = 0;
> > > > > if (state[0].set)
> > > > > {
> > > > > @@ -1102,7 +1154,7 @@ grub_cmd_cryptomount
> > > > > (grub_extcmd_context_t ctxt, int argc, char **args) {
> > > > > grub_dprintf ("cryptodisk",
> > > > > "already mounted as crypto%lu\n",
> > > > > dev->id);
> > > > > - return GRUB_ERR_NONE;
> > > > > + goto err;
> > > > > }
> > > > >
> > > > > check_boot = state[2].set;
> > > > > @@ -1111,8 +1163,11 @@ grub_cmd_cryptomount
> > > > > (grub_extcmd_context_t ctxt, int argc, char **args)
> > > > > search_uuid = NULL;
> > > > >
> > > > > if (!have_it)
> > > > > - return grub_error (GRUB_ERR_BAD_ARGUMENT, "no such
> > > > > cryptodisk found");
> > > > > - return GRUB_ERR_NONE;
> > > > > + {
> > > > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "no such
> > > > > cryptodisk found");
> > > > > + goto err;
> > > > > + }
> > > > > + goto err;
> > > > > }
> > > > > else if (state[1].set || (argc == 0 && state[2].set))
> > > > > {
> > > > > @@ -1120,11 +1175,10 @@ grub_cmd_cryptomount
> > > > > (grub_extcmd_context_t ctxt, int argc, char **args)
> > > > > check_boot = state[2].set; grub_device_iterate
> > > > > (&grub_cryptodisk_scan_device, NULL); search_uuid = NULL;
> > > > > - return GRUB_ERR_NONE;
> > > > > + goto err;
> > > > > }
> > > > > else
> > > > > {
> > > > > - grub_err_t err;
> > > > > grub_disk_t disk;
> > > > > grub_cryptodisk_t dev;
> > > > > char *diskname;
> > > > > @@ -1147,27 +1201,30 @@ grub_cmd_cryptomount
> > > > > (grub_extcmd_context_t ctxt, int argc, char **args) {
> > > > > if (disklast)
> > > > > *disklast = ')';
> > > > > - return grub_errno;
> > > > > + ret = grub_errno;
> > > > > + goto err;
> > > > > }
> > > > >
> > > > > dev = grub_cryptodisk_get_by_source_disk (disk);
> > > > > if (dev)
> > > > > - {
> > > > > - grub_dprintf ("cryptodisk", "already mounted as
> > > > > crypto%lu\n", dev->id);
> > > > > - grub_disk_close (disk);
> > > > > - if (disklast)
> > > > > - *disklast = ')';
> > > > > - return GRUB_ERR_NONE;
> > > > > - }
> > > > > -
> > > > > - err = grub_cryptodisk_scan_device_real (diskname,
> > > > > disk);
> > > > > + grub_dprintf ("cryptodisk", "already mounted as
> > > > > crypto%lu\n", dev->id);
> > > > > + else
> > > > > + ret = grub_cryptodisk_scan_device_real (diskname, disk);
> > > > >
> > > > > grub_disk_close (disk);
> > > > > if (disklast)
> > > > > *disklast = ')';
> > > > > -
> > > > > - return err;
> > > > > }
> > > > > +
> > > > > + err:
> > > > > + if (hdr_file)
> > > > > + grub_file_close (hdr_file);
> > > > > + if (key_file)
> > > > > + grub_file_close (key_file);
> > > > > + if (mkey_file)
> > > > > + grub_file_close (mkey_file);
> > > > > +
> > > > > + return ret;
> > > > > }
> > > > >
> > > > > static struct grub_disk_dev grub_cryptodisk_dev = {
> > > > > @@ -1299,7 +1356,7 @@ GRUB_MOD_INIT (cryptodisk)
> > > > > {
> > > > > grub_disk_dev_register (&grub_cryptodisk_dev);
> > > > > cmd = grub_register_extcmd ("cryptomount",
> > > > > grub_cmd_cryptomount, 0,
> > > > > - N_("SOURCE|-u UUID|-a|-b"),
> > > > > + N_("SOURCE|-u UUID|-a|-b|-H
> > > > > file|-k file|-K file"), N_("Mount a crypto device."),
> > > > > options); grub_procfs_register ("luks_script", &luks_script);
> > > > > }
> > > > > diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> > > > > index 31d7166fc..c29b472e6 100644
> > > > > --- a/grub-core/disk/luks2.c
> > > > > +++ b/grub-core/disk/luks2.c
> > > > > @@ -309,13 +309,22 @@ luks2_get_keyslot (grub_luks2_keyslot_t
> > > > > *k, grub_luks2_digest_t *d, grub_luks2_s
> > > > > /* Determine whether to use primary or secondary header */
> > > > > static grub_err_t
> > > > > -luks2_read_header (grub_disk_t disk, grub_luks2_header_t
> > > > > *outhdr) +luks2_read_header (grub_disk_t disk, grub_file_t
> > > > > hdr_file, grub_luks2_header_t *outhdr) {
> > > > > grub_luks2_header_t primary, secondary, *header = &primary;
> > > > > grub_err_t ret;
> > > > >
> > > > > /* Read the primary LUKS header. */
> > > > > - ret = grub_disk_read (disk, 0, 0, sizeof (primary),
> > > > > &primary);
> > > > > + if (hdr_file)
> > > > > + {
> > > > > + grub_file_seek (hdr_file, 0);
> > > > > + if (grub_file_read (hdr_file, &primary, sizeof
> > > > > (primary)) != sizeof (primary))
> > > > > + ret = GRUB_ERR_READ_ERROR;
> > > > > + else
> > > > > + ret = GRUB_ERR_NONE;
> > > > > + }
> > > > > + else
> > > > > + ret = grub_disk_read (disk, 0, 0, sizeof (primary),
> > > > > &primary); if (ret)
> > > > > return ret;
> > > > >
> > > > > @@ -325,7 +334,16 @@ luks2_read_header (grub_disk_t disk,
> > > > > grub_luks2_header_t *outhdr) return GRUB_ERR_BAD_SIGNATURE;
> > > > >
> > > > > /* Read the secondary header. */
> > > > > - ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> > > > > (primary.hdr_size), sizeof (secondary), &secondary);
> > > > > + if (hdr_file)
> > > > > + {
> > > > > + grub_file_seek (hdr_file, grub_be_to_cpu64
> > > > > (primary.hdr_size));
> > > > > + if (grub_file_read (hdr_file, &secondary, sizeof
> > > > > (secondary)) != sizeof (secondary))
> > > > > + ret = GRUB_ERR_READ_ERROR;
> > > > > + else
> > > > > + ret = GRUB_ERR_NONE;
> > > > > + }
> > > > > + else
> > > > > + ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> > > > > (primary.hdr_size), sizeof (secondary), &secondary); if (ret)
> > > > > return ret;
> > > > >
> > > > > @@ -342,7 +360,7 @@ luks2_read_header (grub_disk_t disk,
> > > > > grub_luks2_header_t *outhdr) }
> > > > >
> > > > > static grub_cryptodisk_t
> > > > > -luks2_scan (grub_disk_t disk, const char *check_uuid, int
> > > > > check_boot) +luks2_scan (grub_disk_t disk, grub_file_t
> > > > > hdr_file, const char *check_uuid, int check_boot) {
> > > > > grub_cryptodisk_t cryptodisk;
> > > > > grub_luks2_header_t header;
> > > > > @@ -352,7 +370,7 @@ luks2_scan (grub_disk_t disk, const char
> > > > > *check_uuid, int check_boot) if (check_boot)
> > > > > return NULL;
> > > > >
> > > > > - if (luks2_read_header (disk, &header))
> > > > > + if (luks2_read_header (disk, hdr_file, &header))
> > > > > {
> > > > > grub_errno = GRUB_ERR_NONE;
> > > > > return NULL;
> > > > > @@ -415,7 +433,7 @@ luks2_verify_key (grub_luks2_digest_t *d,
> > > > > grub_uint8_t *candidate_key,
> > > > > static grub_err_t
> > > > > luks2_decrypt_key (grub_uint8_t *out_key,
> > > > > - grub_disk_t disk, grub_cryptodisk_t crypt,
> > > > > + grub_disk_t disk, grub_cryptodisk_t crypt,
> > > > > grub_file_t hdr_file, grub_luks2_keyslot_t *k,
> > > > > const grub_uint8_t *passphrase, grub_size_t
> > > > > passphraselen) {
> > > > > @@ -491,7 +509,16 @@ luks2_decrypt_key (grub_uint8_t *out_key,
> > > > > }
> > > > >
> > > > > grub_errno = GRUB_ERR_NONE;
> > > > > - ret = grub_disk_read (disk, 0, k->area.offset,
> > > > > k->area.size, split_key);
> > > > > + if (hdr_file)
> > > > > + {
> > > > > + grub_file_seek (hdr_file, k->area.offset);
> > > > > + if (grub_file_read (hdr_file, split_key, k->area.size)
> > > > > != k->area.size)
> > > > > + ret = GRUB_ERR_READ_ERROR;
> > > > > + else
> > > > > + ret = GRUB_ERR_NONE;
> > > > > + }
> > > > > + else
> > > > > + ret = grub_disk_read (disk, 0, k->area.offset,
> > > > > k->area.size, split_key); if (ret)
> > > > > {
> > > > > grub_error (GRUB_ERR_IO, "Read error: %s\n",
> > > > > grub_errmsg); @@ -531,12 +558,16 @@ luks2_decrypt_key
> > > > > (grub_uint8_t *out_key,
> > > > >
> > > > > static grub_err_t
> > > > > luks2_recover_key (grub_disk_t disk,
> > > > > - grub_cryptodisk_t crypt)
> > > > > + grub_cryptodisk_t crypt,
> > > > > + grub_file_t hdr_file, grub_file_t key_file,
> > > > > grub_file_t mkey_file) {
> > > > > + char cipher[32];
> > > > > + char *passphrase = NULL;
> > > > > grub_uint8_t candidate_key[GRUB_CRYPTODISK_MAX_KEYLEN];
> > > > > - char passphrase[MAX_PASSPHRASE], cipher[32];
> > > > > + grub_size_t candidate_key_len, passphrase_len;
> > > > > char *json_header = NULL, *part = NULL, *ptr;
> > > > > - grub_size_t candidate_key_len = 0, i, size;
> > > > > + grub_off_t json_header_offset;
> > > > > + grub_size_t json_header_size, i, size;
> > > > > grub_luks2_header_t header;
> > > > > grub_luks2_keyslot_t keyslot;
> > > > > grub_luks2_digest_t digest;
> > > > > @@ -545,7 +576,7 @@ luks2_recover_key (grub_disk_t disk,
> > > > > grub_json_t *json = NULL, keyslots;
> > > > > grub_err_t ret;
> > > > >
> > > > > - ret = luks2_read_header (disk, &header);
> > > > > + ret = luks2_read_header (disk, hdr_file, &header);
> > > > > if (ret)
> > > > > return ret;
> > > > >
> > > > > @@ -554,8 +585,19 @@ luks2_recover_key (grub_disk_t disk,
> > > > > return GRUB_ERR_OUT_OF_MEMORY;
> > > > >
> > > > > /* Read the JSON area. */
> > > > > - ret = grub_disk_read (disk, 0, grub_be_to_cpu64
> > > > > (header.hdr_offset) + sizeof (header),
> > > > > - grub_be_to_cpu64 (header.hdr_size) -
> > > > > sizeof (header), json_header);
> > > > > + json_header_offset = grub_be_to_cpu64 (header.hdr_offset) +
> > > > > sizeof (header);
> > > > > + json_header_size = grub_be_to_cpu64 (header.hdr_size) -
> > > > > sizeof (header); +
> > > > > + if (hdr_file)
> > > > > + {
> > > > > + grub_file_seek (hdr_file, json_header_offset);
> > > > > + if (grub_file_read (hdr_file, json_header,
> > > > > json_header_size) != json_header_size)
> > > > > + ret = GRUB_ERR_READ_ERROR;
> > > > > + else
> > > > > + ret = GRUB_ERR_NONE;
> > > > > + }
> > > > > + else
> > > > > + ret = grub_disk_read (disk, 0, json_header_offset,
> > > > > json_header_size, json_header); if (ret)
> > > > > goto err;
> > > > >
> > > > > @@ -570,16 +612,47 @@ luks2_recover_key (grub_disk_t disk,
> > > > > goto err;
> > > > > }
> > > > >
> > > > > - /* Get the passphrase from the user. */
> > > > > - if (disk->partition)
> > > > > - part = grub_partition_get_name (disk->partition);
> > > > > - grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "),
> > > > > disk->name,
> > > > > - disk->partition ? "," : "", part ? : "",
> > > > > - crypt->uuid);
> > > > > - if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> > > > > + if (mkey_file)
> > > > > {
> > > > > - ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase
> > > > > not supplied");
> > > > > - goto err;
> > > > > + /* Get the master key from file. */
> > > > > + candidate_key_len = grub_file_read (mkey_file,
> > > > > candidate_key, GRUB_CRYPTODISK_MAX_KEYLEN);
> > > > > + if (candidate_key_len == (grub_size_t)-1)
> > > > > + {
> > > > > + ret = grub_error (GRUB_ERR_FILE_READ_ERROR, "Error
> > > > > reading key file");
> > > > > + goto err;
> > > > > + }
> > > > > + if (candidate_key_len == 0)
> > > > > + {
> > > > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Key not
> > > > > supplied");
> > > > > + goto err;
> > > > > + }
> > > > > + }
> > > > > + else if (key_file)
> > > > > + {
> > > > > + /* Get the passphrase from file. */
> > > > > + passphrase = grub_malloc
> > > > > (GRUB_CRYPTODISK_MAX_PASSPHRASE);
> > > > > + passphrase_len = grub_file_read (key_file, passphrase,
> > > > > GRUB_CRYPTODISK_MAX_PASSPHRASE);
> > > > > + if (passphrase_len == (grub_size_t)-1)
> > > > > + {
> > > > > + ret = grub_error (GRUB_ERR_FILE_READ_ERROR, "Error
> > > > > reading passphrase file");
> > > > > + goto err;
> > > > > + }
> > > > > + }
> > > > > + else
> > > > > + {
> > > > > + /* Get the passphrase from the user. */
> > > > > + if (disk->partition)
> > > > > + part = grub_partition_get_name (disk->partition);
> > > > > + grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "),
> > > > > disk->name,
> > > > > + disk->partition ? "," : "", part ? : "",
> > > > > + crypt->uuid);
> > > > > + passphrase = grub_malloc (MAX_PASSPHRASE);
> > > > > + if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> > > > > + {
> > > > > + ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase
> > > > > not supplied");
> > > > > + goto err;
> > > > > + }
> > > > > + passphrase_len = grub_strlen (passphrase);
> > > > > }
> > > > >
> > > > > if (grub_json_getvalue (&keyslots, json, "keyslots") ||
> > > > > @@ -604,6 +677,26 @@ luks2_recover_key (grub_disk_t disk,
> > > > >
> > > > > grub_dprintf ("luks2", "Trying keyslot
> > > > > %"PRIuGRUB_SIZE"\n", i);
> > > > >
> > > > > + if (mkey_file)
> > > > > + {
> > > > > + if (candidate_key_len != keyslot.key_size)
> > > > > + {
> > > > > + grub_dprintf ("luks2", "Wrong key length for
> > > > > keyslot %"PRIuGRUB_SIZE": %s\n",
> > > > > + i, grub_errmsg);
> > > > > + continue;
> > > > > + }
> > > > > +
> > > > > + ret = luks2_verify_key (&digest, candidate_key,
> > > > > candidate_key_len);
> > > > > + if (ret)
> > > > > + {
> > > > > + grub_dprintf ("luks2", "Could not open keyslot
> > > > > %"PRIuGRUB_SIZE": %s\n",
> > > > > + i, grub_errmsg);
> > > > > + continue;
> > > > > + }
> > > > > + }
> > > > > + else
> > > > > + candidate_key_len = keyslot.key_size;
> > > > > +
> > > > > /* Set up disk according to keyslot's segment. */
> > > > > crypt->offset = grub_divmod64 (segment.offset,
> > > > > segment.sector_size, NULL); crypt->log_sector_size = sizeof
> > > > > (unsigned int) * 8 @@ -613,21 +706,24 @@ luks2_recover_key
> > > > > (grub_disk_t disk, else
> > > > > crypt->total_length = grub_strtoull (segment.size, NULL,
> > > > > 10);
> > > > >
> > > > > - ret = luks2_decrypt_key (candidate_key, disk, crypt,
> > > > > &keyslot,
> > > > > - (const grub_uint8_t *)
> > > > > passphrase, grub_strlen (passphrase));
> > > > > - if (ret)
> > > > > + if (passphrase)
> > > > > {
> > > > > - grub_dprintf ("luks2", "Decryption with keyslot
> > > > > %"PRIuGRUB_SIZE" failed: %s\n",
> > > > > - i, grub_errmsg);
> > > > > - continue;
> > > > > - }
> > > > > -
> > > > > - ret = luks2_verify_key (&digest, candidate_key,
> > > > > keyslot.key_size);
> > > > > - if (ret)
> > > > > - {
> > > > > - grub_dprintf ("luks2", "Could not open keyslot
> > > > > %"PRIuGRUB_SIZE": %s\n",
> > > > > - i, grub_errmsg);
> > > > > - continue;
> > > > > + ret = luks2_decrypt_key (candidate_key, disk, crypt,
> > > > > hdr_file, &keyslot,
> > > > > + (const grub_uint8_t *)
> > > > > passphrase, passphrase_len);
> > > > > + if (ret)
> > > > > + {
> > > > > + grub_dprintf ("luks2", "Decryption with keyslot
> > > > > %"PRIuGRUB_SIZE" failed: %s\n",
> > > > > + i, grub_errmsg);
> > > > > + continue;
> > > > > + }
> > > > > +
> > > > > + ret = luks2_verify_key (&digest, candidate_key,
> > > > > candidate_key_len);
> > > > > + if (ret)
> > > > > + {
> > > > > + grub_dprintf ("luks2", "Could not open keyslot
> > > > > %"PRIuGRUB_SIZE": %s\n",
> > > > > + i, grub_errmsg);
> > > > > + continue;
> > > > > + }
> > > > > }
> > > > >
> > > > > /*
> > > > > @@ -635,13 +731,14 @@ luks2_recover_key (grub_disk_t disk,
> > > > > * where each element is either empty or holds a key.
> > > > > */
> > > > > grub_printf_ (N_("Slot %"PRIuGRUB_SIZE" opened\n"), i);
> > > > > -
> > > > > - candidate_key_len = keyslot.key_size;
> > > > > break;
> > > > > }
> > > > > - if (candidate_key_len == 0)
> > > > > + if (i == size)
> > > > > {
> > > > > - ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid
> > > > > passphrase");
> > > > > + if (mkey_file)
> > > > > + ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid
> > > > > key");
> > > > > + else
> > > > > + ret = grub_error (GRUB_ERR_ACCESS_DENIED, "Invalid
> > > > > passphrase"); goto err;
> > > > > }
> > > > >
> > > > > @@ -666,6 +763,7 @@ luks2_recover_key (grub_disk_t disk,
> > > > >
> > > > > err:
> > > > > grub_free (part);
> > > > > + grub_free (passphrase);
> > > > > grub_free (json_header);
> > > > > grub_json_free (json);
> > > > > return ret;
> > > > > diff --git a/include/grub/cryptodisk.h
> > > > > b/include/grub/cryptodisk.h index e1b21e785..b53006854 100644
> > > > > --- a/include/grub/cryptodisk.h
> > > > > +++ b/include/grub/cryptodisk.h
> > > > > @@ -21,6 +21,7 @@
> > > > >
> > > > > #include <grub/disk.h>
> > > > > #include <grub/crypto.h>
> > > > > +#include <grub/file.h>
> > > > > #include <grub/list.h>
> > > > > #ifdef GRUB_UTIL
> > > > > #include <grub/emu/hostdisk.h>
> > > > > @@ -53,6 +54,7 @@ typedef enum
> > > > > #define GRUB_CRYPTODISK_GF_LOG_BYTES
> > > > > (GRUB_CRYPTODISK_GF_LOG_SIZE - 3) #define
> > > > > GRUB_CRYPTODISK_GF_BYTES (1U << GRUB_CRYPTODISK_GF_LOG_BYTES)
> > > > > #define GRUB_CRYPTODISK_MAX_KEYLEN 128 +#define
> > > > > GRUB_CRYPTODISK_MAX_PASSPHRASE 8192
> > > > >
> > > > > struct grub_cryptodisk;
> > > > >
> > > > > @@ -106,9 +108,10 @@ struct grub_cryptodisk_dev
> > > > > struct grub_cryptodisk_dev *next;
> > > > > struct grub_cryptodisk_dev **prev;
> > > > >
> > > > > - grub_cryptodisk_t (*scan) (grub_disk_t disk, const char
> > > > > *check_uuid,
> > > > > - int boot_only);
> > > > > - grub_err_t (*recover_key) (grub_disk_t disk,
> > > > > grub_cryptodisk_t dev);
> > > > > + grub_cryptodisk_t (*scan) (grub_disk_t disk, grub_file_t
> > > > > hdr_file,
> > > > > + const char *check_uuid, int
> > > > > boot_only);
> > > > > + grub_err_t (*recover_key) (grub_disk_t disk,
> > > > > grub_cryptodisk_t dev,
> > > > > + grub_file_t hdr_file, grub_file_t
> > > > > key_file, grub_file_t mkey_file); };
> > > > > typedef struct grub_cryptodisk_dev *grub_cryptodisk_dev_t;
> > > > >
> > > >
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-11-10 20:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-07 16:36 [PATCH] Cryptomount support for key files and detached header reagentoo
2020-11-09 3:16 ` Glenn Washburn
2020-11-09 7:56 ` Dmitry
2020-11-09 9:42 ` Dmitry
2020-11-09 21:34 ` Glenn Washburn
2020-11-09 22:09 ` Dmitry
2020-11-10 20:25 ` Glenn Washburn
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.