v5 for detached headers and key files

v5 for detached headers and key files

[Denis 'GNUtoo' Carikli]

Hi,

I've now addressed the comments of the following patch:
- [ 5/6] cryptodisk: enable the backends to implement key.

Denis.

[v5][ 1/6] cryptodisk: luks: unify grub_cryptodisk_dev function names

[Denis 'GNUtoo' Carikli]

Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
---
ChangeLog since v4:
- Added Reviewed-by tag
---
 grub-core/disk/luks.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
index 410cd6f84..28585806a 100644
--- a/grub-core/disk/luks.c
+++ b/grub-core/disk/luks.c
@@ -65,8 +65,7 @@ gcry_err_code_t AF_merge (const gcry_md_spec_t * hash, grub_uint8_t * src,
 			  grub_size_t blocknumbers);
 
 static grub_cryptodisk_t
-configure_ciphers (grub_disk_t disk, const char *check_uuid,
-		   int check_boot)
+luks_scan (grub_disk_t disk, const char *check_uuid, int check_boot)
 {
   grub_cryptodisk_t newdev;
   const char *iptr;
@@ -310,7 +309,7 @@ luks_recover_key (grub_disk_t source,
 }
 
 struct grub_cryptodisk_dev luks_crypto = {
-  .scan = configure_ciphers,
+  .scan = luks_scan,
   .recover_key = luks_recover_key
 };
 
-- 
2.27.0

[v5][ 2/6] cryptodisk: geli: unify grub_cryptodisk_dev function names

[Denis 'GNUtoo' Carikli]

Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
---
ChangeLog since v4:
- Added Reviewed-by tag
---
 grub-core/disk/geli.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/grub-core/disk/geli.c b/grub-core/disk/geli.c
index e9d23299a..581631c1d 100644
--- a/grub-core/disk/geli.c
+++ b/grub-core/disk/geli.c
@@ -242,8 +242,7 @@ grub_util_get_geli_uuid (const char *dev)
 #endif
 
 static grub_cryptodisk_t
-configure_ciphers (grub_disk_t disk, const char *check_uuid,
-		   int boot_only)
+geli_scan (grub_disk_t disk, const char *check_uuid, int boot_only)
 {
   grub_cryptodisk_t newdev;
   struct grub_geli_phdr header;
@@ -398,7 +397,7 @@ configure_ciphers (grub_disk_t disk, const char *check_uuid,
 }
 
 static grub_err_t
-recover_key (grub_disk_t source, grub_cryptodisk_t dev)
+geli_recover_key (grub_disk_t source, grub_cryptodisk_t dev)
 {
   grub_size_t keysize;
   grub_uint8_t digest[GRUB_CRYPTO_MAX_MDLEN];
@@ -580,8 +579,8 @@ recover_key (grub_disk_t source, grub_cryptodisk_t dev)
 }
 
 struct grub_cryptodisk_dev geli_crypto = {
-  .scan = configure_ciphers,
-  .recover_key = recover_key
+  .scan = geli_scan,
+  .recover_key = geli_recover_key
 };
 
 GRUB_MOD_INIT (geli)
-- 
2.27.0

[v5][ 3/6] cryptodisk: enable the backends to implement detached headers

[Denis 'GNUtoo' Carikli]

From: John Lane <john@lane.uk.net>

Signed-off-by: John Lane <john@lane.uk.net>
GNUtoo@cyberdimension.org: rebase, patch split, small fixes, commit message
Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
---
ChangeLog since v4:
- Added Reviewed-by tag
---
 grub-core/disk/cryptodisk.c | 24 ++++++++++++++++++++----
 grub-core/disk/geli.c       | 15 +++++++++++++--
 grub-core/disk/luks.c       | 14 +++++++++++---
 grub-core/disk/luks2.c      | 15 ++++++++++++---
 include/grub/cryptodisk.h   |  6 ++++--
 include/grub/file.h         |  2 ++
 6 files changed, 62 insertions(+), 14 deletions(-)

diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 1897acc4b..6ad2e486e 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -41,6 +41,7 @@ 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},
     {0, 0, 0, 0, 0, 0}
   };
 
@@ -970,6 +971,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;
 
 static void
 cryptodisk_close (grub_cryptodisk_t dev)
@@ -994,13 +996,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, search_uuid, check_boot, hdr);
     if (grub_errno)
       return grub_errno;
     if (!dev)
       continue;
     
-    err = cr->recover_key (source, dev);
+    err = cr->recover_key (source, dev, hdr);
     if (err)
     {
       cryptodisk_close (dev);
@@ -1041,7 +1043,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, search_uuid, check_boot, NULL);
     if (grub_errno)
       return grub_errno;
     if (!dev)
@@ -1095,6 +1097,20 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
   if (argc < 1 && !state[1].set && !state[2].set)
     return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name required");
 
+  if (state[3].set) /* Detached header */
+    {
+      if (state[0].set)
+	return grub_error (GRUB_ERR_BAD_ARGUMENT,
+			   N_("Cannot use UUID lookup with detached header"));
+
+      hdr = grub_file_open (state[3].arg,
+			    GRUB_FILE_TYPE_CRYPTODISK_DETACHED_HEADER);
+      if (!hdr)
+	return grub_errno;
+    }
+  else
+    hdr = NULL;
+
   have_it = 0;
   if (state[0].set)
     {
@@ -1302,7 +1318,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"),
 			      N_("Mount a crypto device."), options);
   grub_procfs_register ("luks_script", &luks_script);
 }
diff --git a/grub-core/disk/geli.c b/grub-core/disk/geli.c
index 581631c1d..acd09d874 100644
--- a/grub-core/disk/geli.c
+++ b/grub-core/disk/geli.c
@@ -52,6 +52,7 @@
 #include <grub/dl.h>
 #include <grub/err.h>
 #include <grub/disk.h>
+#include <grub/file.h>
 #include <grub/crypto.h>
 #include <grub/partition.h>
 #include <grub/i18n.h>
@@ -121,6 +122,7 @@ enum
 
 /* FIXME: support version 0.  */
 /* FIXME: support big-endian pre-version-4 volumes.  */
+/* FIXME: support for detached headers.  */
 /* FIXME: support for keyfiles.  */
 /* FIXME: support for HMAC.  */
 const char *algorithms[] = {
@@ -242,7 +244,8 @@ grub_util_get_geli_uuid (const char *dev)
 #endif
 
 static grub_cryptodisk_t
-geli_scan (grub_disk_t disk, const char *check_uuid, int boot_only)
+geli_scan (grub_disk_t disk, const char *check_uuid, int boot_only,
+	   grub_file_t hdr)
 {
   grub_cryptodisk_t newdev;
   struct grub_geli_phdr header;
@@ -254,6 +257,10 @@ geli_scan (grub_disk_t disk, const char *check_uuid, int boot_only)
   grub_disk_addr_t sector;
   grub_err_t err;
 
+  /* Detached headers are not implemented yet */
+  if (hdr)
+    return NULL;
+
   if (2 * GRUB_MD_SHA256->mdlen + 1 > GRUB_CRYPTODISK_MAX_UUID_LENGTH)
     return NULL;
 
@@ -397,7 +404,7 @@ geli_scan (grub_disk_t disk, const char *check_uuid, int boot_only)
 }
 
 static grub_err_t
-geli_recover_key (grub_disk_t source, grub_cryptodisk_t dev)
+geli_recover_key (grub_disk_t source, grub_cryptodisk_t dev, grub_file_t hdr)
 {
   grub_size_t keysize;
   grub_uint8_t digest[GRUB_CRYPTO_MAX_MDLEN];
@@ -413,6 +420,10 @@ geli_recover_key (grub_disk_t source, grub_cryptodisk_t dev)
   grub_disk_addr_t sector;
   grub_err_t err;
 
+  /* Detached headers are not implemented yet */
+  if (hdr)
+    return GRUB_ERR_NOT_IMPLEMENTED_YET;
+
   if (dev->cipher->cipher->blocksize > GRUB_CRYPTO_MAX_CIPHER_BLOCKSIZE)
     return grub_error (GRUB_ERR_BUG, "cipher block is too long");
 
diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
index 28585806a..ffeb679d1 100644
--- a/grub-core/disk/luks.c
+++ b/grub-core/disk/luks.c
@@ -65,7 +65,8 @@ gcry_err_code_t AF_merge (const gcry_md_spec_t * hash, grub_uint8_t * src,
 			  grub_size_t blocknumbers);
 
 static grub_cryptodisk_t
-luks_scan (grub_disk_t disk, const char *check_uuid, int check_boot)
+luks_scan (grub_disk_t disk, const char *check_uuid, int check_boot,
+	   grub_file_t hdr)
 {
   grub_cryptodisk_t newdev;
   const char *iptr;
@@ -77,6 +78,10 @@ luks_scan (grub_disk_t disk, const char *check_uuid, int check_boot)
   char hashspec[sizeof (header.hashSpec) + 1];
   grub_err_t err;
 
+  /* Detached headers are not implemented yet */
+  if (hdr)
+    return NULL;
+
   if (check_boot)
     return NULL;
 
@@ -149,8 +154,7 @@ luks_scan (grub_disk_t disk, const char *check_uuid, int check_boot)
 }
 
 static grub_err_t
-luks_recover_key (grub_disk_t source,
-		  grub_cryptodisk_t dev)
+luks_recover_key (grub_disk_t source, grub_cryptodisk_t dev, grub_file_t hdr)
 {
   struct grub_luks_phdr header;
   grub_size_t keysize;
@@ -163,6 +167,10 @@ luks_recover_key (grub_disk_t source,
   grub_size_t max_stripes = 1;
   char *tmp;
 
+  /* Detached headers are not implemented yet */
+  if (hdr)
+    return GRUB_ERR_NOT_IMPLEMENTED_YET;
+
   err = grub_disk_read (source, 0, 0, sizeof (header), &header);
   if (err)
     return err;
diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index e3ff7c83d..bc00e8bbc 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -342,11 +342,16 @@ 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, const char *check_uuid, int check_boot,
+	    grub_file_t hdr_file)
 {
   grub_cryptodisk_t cryptodisk;
   grub_luks2_header_t header;
 
+  /* Detached headers are not implemented yet */
+  if (hdr_file)
+    return NULL;
+
   if (check_boot)
     return NULL;
 
@@ -523,8 +528,8 @@ luks2_decrypt_key (grub_uint8_t *out_key,
 }
 
 static grub_err_t
-luks2_recover_key (grub_disk_t disk,
-		   grub_cryptodisk_t crypt)
+luks2_recover_key (grub_disk_t disk, grub_cryptodisk_t crypt,
+		   grub_file_t hdr_file)
 {
   grub_uint8_t candidate_key[GRUB_CRYPTODISK_MAX_KEYLEN];
   char passphrase[MAX_PASSPHRASE], cipher[32];
@@ -538,6 +543,10 @@ luks2_recover_key (grub_disk_t disk,
   grub_json_t *json = NULL, keyslots;
   grub_err_t ret;
 
+  /* Detached headers are not implemented yet */
+  if (hdr_file)
+    return GRUB_ERR_NOT_IMPLEMENTED_YET;
+
   ret = luks2_read_header (disk, &header);
   if (ret)
     return ret;
diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
index e1b21e785..e24b1b8cb 100644
--- a/include/grub/cryptodisk.h
+++ b/include/grub/cryptodisk.h
@@ -20,6 +20,7 @@
 #define GRUB_CRYPTODISK_HEADER	1
 
 #include <grub/disk.h>
+#include <grub/file.h>
 #include <grub/crypto.h>
 #include <grub/list.h>
 #ifdef GRUB_UTIL
@@ -107,8 +108,9 @@ struct grub_cryptodisk_dev
   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);
+			     int boot_only, grub_file_t hdr);
+  grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t dev,
+			     grub_file_t hdr);
 };
 typedef struct grub_cryptodisk_dev *grub_cryptodisk_dev_t;
 
diff --git a/include/grub/file.h b/include/grub/file.h
index 31567483c..a7d7be853 100644
--- a/include/grub/file.h
+++ b/include/grub/file.h
@@ -90,6 +90,8 @@ enum grub_file_type
     GRUB_FILE_TYPE_FONT,
     /* File holding encryption key for encrypted ZFS.  */
     GRUB_FILE_TYPE_ZFS_ENCRYPTION_KEY,
+    /* File holiding the encryption metadata header */
+    GRUB_FILE_TYPE_CRYPTODISK_DETACHED_HEADER,
     /* File we open n grub-fstest.  */
     GRUB_FILE_TYPE_FSTEST,
     /* File we open n grub-mount.  */
-- 
2.27.0

[v5][ 4/6] cryptodisk: add support for LUKS1 detached headers

[Denis 'GNUtoo' Carikli]

From: John Lane <john@lane.uk.net>

cryptsetup supports having a detached header through the
--header command line argument for both LUKS1 and LUKS2.

This adds support for LUKS1 detached headers.

Signed-off-by: John Lane <john@lane.uk.net>
GNUtoo@cyberdimension.org: rebase, small fixes, commit message
Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
---
ChangeLog since v4:
- Added Reviewed-by tag
---
 grub-core/disk/luks.c | 48 ++++++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
index ffeb679d1..0b20908ac 100644
--- a/grub-core/disk/luks.c
+++ b/grub-core/disk/luks.c
@@ -23,6 +23,7 @@
 #include <grub/dl.h>
 #include <grub/err.h>
 #include <grub/disk.h>
+#include <grub/file.h>
 #include <grub/crypto.h>
 #include <grub/partition.h>
 #include <grub/i18n.h>
@@ -76,17 +77,23 @@ luks_scan (grub_disk_t disk, const char *check_uuid, int check_boot,
   char ciphername[sizeof (header.cipherName) + 1];
   char ciphermode[sizeof (header.cipherMode) + 1];
   char hashspec[sizeof (header.hashSpec) + 1];
-  grub_err_t err;
-
-  /* Detached headers are not implemented yet */
-  if (hdr)
-    return NULL;
+  grub_err_t err = GRUB_ERR_NONE;
 
   if (check_boot)
     return NULL;
 
   /* Read the LUKS header.  */
-  err = grub_disk_read (disk, 0, 0, sizeof (header), &header);
+  if (hdr)
+    {
+      if (grub_file_seek (hdr, 0) == (grub_off_t) -1)
+	return NULL;
+
+      if (grub_file_read (hdr, &header, sizeof (header)) != sizeof (header))
+	return NULL;
+    }
+  else
+    err = grub_disk_read (disk, 0, 0, sizeof (header), &header);
+
   if (err)
     {
       if (err == GRUB_ERR_OUT_OF_RANGE)
@@ -163,15 +170,22 @@ luks_recover_key (grub_disk_t source, grub_cryptodisk_t dev, grub_file_t hdr)
   grub_uint8_t candidate_digest[sizeof (header.mkDigest)];
   unsigned i;
   grub_size_t length;
-  grub_err_t err;
+  grub_err_t err = GRUB_ERR_NONE;
   grub_size_t max_stripes = 1;
   char *tmp;
+  grub_uint32_t sector;
 
-  /* Detached headers are not implemented yet */
   if (hdr)
-    return GRUB_ERR_NOT_IMPLEMENTED_YET;
+    {
+      if (grub_file_seek (hdr, 0) == (grub_off_t) -1)
+	return grub_errno;
+
+      if (grub_file_read (hdr, &header, sizeof (header)) != sizeof (header))
+	return grub_errno;
+    }
+  else
+    err = grub_disk_read (source, 0, 0, sizeof (header), &header);
 
-  err = grub_disk_read (source, 0, 0, sizeof (header), &header);
   if (err)
     return err;
 
@@ -240,13 +254,19 @@ luks_recover_key (grub_disk_t source, grub_cryptodisk_t dev, grub_file_t hdr)
 	  return grub_crypto_gcry_error (gcry_err);
 	}
 
+      sector = grub_be_to_cpu32 (header.keyblock[i].keyMaterialOffset);
       length = (keysize * grub_be_to_cpu32 (header.keyblock[i].stripes));
 
       /* Read and decrypt the key material from the disk.  */
-      err = grub_disk_read (source,
-			    grub_be_to_cpu32 (header.keyblock
-					      [i].keyMaterialOffset), 0,
-			    length, split_key);
+      if (hdr)
+      {
+        if (grub_file_seek (hdr, sector * 512))
+          return grub_errno;
+        if (grub_file_read (hdr, split_key, length) != (grub_ssize_t)length)
+          return grub_errno;
+      }
+      else
+        err = grub_disk_read (source, sector, 0, length, split_key);
       if (err)
 	{
 	  grub_free (split_key);
-- 
2.27.0

Re: [v5][ 4/6] cryptodisk: add support for LUKS1 detached headers

[Glenn Washburn]

Thanks Denis for taking the lead in trying to get these patches
included.  One issue I've found that make LUKS1 detached header support
unusable is below.

On Thu, 11 Jun 2020 18:18:05 +0200
Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org> wrote:

> From: John Lane <john@lane.uk.net>
> 
> cryptsetup supports having a detached header through the
> --header command line argument for both LUKS1 and LUKS2.
> 
> This adds support for LUKS1 detached headers.
> 
> Signed-off-by: John Lane <john@lane.uk.net>
> GNUtoo@cyberdimension.org: rebase, small fixes, commit message
> Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
> Reviewed-by: Patrick Steinhardt <ps@pks.im>
> ---
> ChangeLog since v4:
> - Added Reviewed-by tag
> ---
>  grub-core/disk/luks.c | 48
> ++++++++++++++++++++++++++++++------------- 1 file changed, 34
> insertions(+), 14 deletions(-)
> 
> diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
> index ffeb679d1..0b20908ac 100644
> --- a/grub-core/disk/luks.c
> +++ b/grub-core/disk/luks.c

...

> @@ -240,13 +254,19 @@ luks_recover_key (grub_disk_t source,
> grub_cryptodisk_t dev, grub_file_t hdr) return grub_crypto_gcry_error
> (gcry_err); }
>  
> +      sector = grub_be_to_cpu32
> (header.keyblock[i].keyMaterialOffset); length = (keysize *
> grub_be_to_cpu32 (header.keyblock[i].stripes)); 
>        /* Read and decrypt the key material from the disk.  */
> -      err = grub_disk_read (source,
> -			    grub_be_to_cpu32 (header.keyblock
> -
> [i].keyMaterialOffset), 0,
> -			    length, split_key);
> +      if (hdr)
> +      {
> +        if (grub_file_seek (hdr, sector * 512))

The return value is properly checked for all the other seeks but this
one.  Without this fixed, cryptomount with a header file on a LUKS
device and properly entered password will exit early with grub_errno
indicating success. However the key will not be properly setup, so the
blocks will decrypt to garbage.

> +          return grub_errno;
> +        if (grub_file_read (hdr, split_key, length) !=
> (grub_ssize_t)length)
> +          return grub_errno;
> +      }
> +      else
> +        err = grub_disk_read (source, sector, 0, length, split_key);
>        if (err)
>  	{
>  	  grub_free (split_key);

[v5][ 5/6] cryptodisk: enable the backends to implement key files

[Denis 'GNUtoo' Carikli]

From: John Lane <john@lane.uk.net>

Signed-off-by: John Lane <john@lane.uk.net>
GNUtoo@cyberdimension.org: rebase, patch split, small fixes, commit message
Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
---
Changelog since v3:
-------------------
- Fixed the size formating with PRIuGRUB_SIZE
- Added Reviewed-by

ChangeLog since v4:
-------------------
- Style fixes:
  - Added missing space between function and '('
  - Removed trailing backslashes in split strings
---
 grub-core/disk/cryptodisk.c | 87 ++++++++++++++++++++++++++++++++++++-
 grub-core/disk/geli.c       |  7 +--
 grub-core/disk/luks.c       |  7 ++-
 grub-core/disk/luks2.c      |  7 +--
 include/grub/cryptodisk.h   |  5 ++-
 include/grub/file.h         |  2 +
 6 files changed, 106 insertions(+), 9 deletions(-)

diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index 6ad2e486e..dd94736d3 100644
--- a/grub-core/disk/cryptodisk.c
+++ b/grub-core/disk/cryptodisk.c
@@ -42,6 +42,9 @@ static const struct grub_arg_option options[] =
     {"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},
+    {"keyfile", 'k', 0, N_("Key file"), 0, ARG_TYPE_STRING},
+    {"keyfile-offset", 'O', 0, N_("Key file offset (bytes)"), 0, ARG_TYPE_INT},
+    {"keyfile-size", 'S', 0, N_("Key file data size (bytes)"), 0, ARG_TYPE_INT},
     {0, 0, 0, 0, 0, 0}
   };
 
@@ -972,6 +975,8 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk)
 static int check_boot, have_it;
 static char *search_uuid;
 static grub_file_t hdr;
+static grub_uint8_t *key, keyfile_buffer[GRUB_CRYPTODISK_MAX_KEYFILE_SIZE];
+static grub_ssize_t key_size;
 
 static void
 cryptodisk_close (grub_cryptodisk_t dev)
@@ -1002,7 +1007,7 @@ grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source)
     if (!dev)
       continue;
     
-    err = cr->recover_key (source, dev, hdr);
+    err = cr->recover_key (source, dev, hdr, key, key_size);
     if (err)
     {
       cryptodisk_close (dev);
@@ -1112,6 +1117,86 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, int argc, char **args)
     hdr = NULL;
 
   have_it = 0;
+  key = NULL;
+
+  if (state[4].set) /* keyfile */
+    {
+      const char *p = NULL;
+      grub_file_t keyfile;
+      int keyfile_offset;
+      grub_size_t requested_keyfile_size = 0;
+
+
+      if (state[5].set) /* keyfile-offset */
+	{
+	  keyfile_offset = grub_strtoul (state[5].arg, &p, 0);
+
+	  if (grub_errno != GRUB_ERR_NONE)
+	    return grub_errno;
+
+	  if (*p != '\0')
+	    return grub_error (GRUB_ERR_BAD_ARGUMENT,
+			       N_("unrecognized number"));
+	}
+      else
+	{
+	  keyfile_offset = 0;
+	}
+
+      if (state[6].set) /* keyfile-size */
+	{
+	  requested_keyfile_size = grub_strtoul (state[6].arg, &p, 0);
+
+	  if (*p != '\0')
+	    return grub_error (GRUB_ERR_BAD_ARGUMENT,
+			       N_("unrecognized number"));
+
+	  if (grub_errno != GRUB_ERR_NONE)
+	    return grub_errno;
+
+	  if (requested_keyfile_size > GRUB_CRYPTODISK_MAX_KEYFILE_SIZE)
+	    return grub_error (GRUB_ERR_OUT_OF_RANGE,
+			      N_("Key file size exceeds maximum (%"
+				 PRIuGRUB_SIZE ")\n"),
+			      GRUB_CRYPTODISK_MAX_KEYFILE_SIZE);
+
+	  if (requested_keyfile_size == 0)
+	    return grub_error (GRUB_ERR_OUT_OF_RANGE,
+			      N_("Key file size is 0\n"));
+	}
+
+      keyfile = grub_file_open (state[4].arg,
+				GRUB_FILE_TYPE_CRYPTODISK_ENCRYPTION_KEY);
+      if (!keyfile)
+	return grub_errno;
+
+      if (grub_file_seek (keyfile, keyfile_offset) == (grub_off_t)-1)
+	return grub_errno;
+
+      if (requested_keyfile_size)
+	{
+	  if (requested_keyfile_size > (keyfile->size - keyfile_offset))
+	    return grub_error (GRUB_ERR_FILE_READ_ERROR,
+			       N_("Keyfile is too small: "
+				  "requested %" PRIuGRUB_SIZE " bytes, "
+				  "but the file only has %" PRIuGRUB_SIZE
+				  " bytes.\n"),
+			       requested_keyfile_size,
+			       keyfile->size);
+
+	  key_size = requested_keyfile_size;
+	}
+      else
+	{
+	  key_size = keyfile->size - keyfile_offset;
+	}
+
+      if (grub_file_read (keyfile, keyfile_buffer, key_size) != key_size)
+	return grub_error (GRUB_ERR_FILE_READ_ERROR,
+			   (N_("Error reading key file\n")));
+      key = keyfile_buffer;
+    }
+
   if (state[0].set)
     {
       grub_cryptodisk_t dev;
diff --git a/grub-core/disk/geli.c b/grub-core/disk/geli.c
index acd09d874..159ac0f96 100644
--- a/grub-core/disk/geli.c
+++ b/grub-core/disk/geli.c
@@ -404,7 +404,8 @@ geli_scan (grub_disk_t disk, const char *check_uuid, int boot_only,
 }
 
 static grub_err_t
-geli_recover_key (grub_disk_t source, grub_cryptodisk_t dev, grub_file_t hdr)
+geli_recover_key (grub_disk_t source, grub_cryptodisk_t dev, grub_file_t hdr,
+		  grub_uint8_t *key, grub_size_t keyfile_size)
 {
   grub_size_t keysize;
   grub_uint8_t digest[GRUB_CRYPTO_MAX_MDLEN];
@@ -420,8 +421,8 @@ geli_recover_key (grub_disk_t source, grub_cryptodisk_t dev, grub_file_t hdr)
   grub_disk_addr_t sector;
   grub_err_t err;
 
-  /* Detached headers are not implemented yet */
-  if (hdr)
+  /* Detached headers and keyfiles are not implemented yet */
+  if (hdr || key || keyfile_size)
     return GRUB_ERR_NOT_IMPLEMENTED_YET;
 
   if (dev->cipher->cipher->blocksize > GRUB_CRYPTO_MAX_CIPHER_BLOCKSIZE)
diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
index 0b20908ac..8dde70d8d 100644
--- a/grub-core/disk/luks.c
+++ b/grub-core/disk/luks.c
@@ -161,7 +161,8 @@ luks_scan (grub_disk_t disk, const char *check_uuid, int check_boot,
 }
 
 static grub_err_t
-luks_recover_key (grub_disk_t source, grub_cryptodisk_t dev, grub_file_t hdr)
+luks_recover_key (grub_disk_t source, grub_cryptodisk_t dev, grub_file_t hdr,
+		  grub_uint8_t *keyfile_bytes, grub_size_t keyfile_bytes_size)
 {
   struct grub_luks_phdr header;
   grub_size_t keysize;
@@ -175,6 +176,10 @@ luks_recover_key (grub_disk_t source, grub_cryptodisk_t dev, grub_file_t hdr)
   char *tmp;
   grub_uint32_t sector;
 
+  /* Keyfiles are not implemented yet */
+  if (keyfile_bytes || keyfile_bytes_size)
+     return GRUB_ERR_NOT_IMPLEMENTED_YET;
+
   if (hdr)
     {
       if (grub_file_seek (hdr, 0) == (grub_off_t) -1)
diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index bc00e8bbc..6a38a1f4d 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -529,7 +529,8 @@ luks2_decrypt_key (grub_uint8_t *out_key,
 
 static grub_err_t
 luks2_recover_key (grub_disk_t disk, grub_cryptodisk_t crypt,
-		   grub_file_t hdr_file)
+		   grub_file_t hdr_file, grub_uint8_t *key,
+		   grub_size_t keyfile_size)
 {
   grub_uint8_t candidate_key[GRUB_CRYPTODISK_MAX_KEYLEN];
   char passphrase[MAX_PASSPHRASE], cipher[32];
@@ -543,8 +544,8 @@ luks2_recover_key (grub_disk_t disk, grub_cryptodisk_t crypt,
   grub_json_t *json = NULL, keyslots;
   grub_err_t ret;
 
-  /* Detached headers are not implemented yet */
-  if (hdr_file)
+  /* Detached headers and keyfiles are not implemented yet */
+  if (hdr_file || key || keyfile_size)
     return GRUB_ERR_NOT_IMPLEMENTED_YET;
 
   ret = luks2_read_header (disk, &header);
diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
index e24b1b8cb..6d2610f93 100644
--- a/include/grub/cryptodisk.h
+++ b/include/grub/cryptodisk.h
@@ -55,6 +55,8 @@ typedef enum
 #define GRUB_CRYPTODISK_GF_BYTES (1U << GRUB_CRYPTODISK_GF_LOG_BYTES)
 #define GRUB_CRYPTODISK_MAX_KEYLEN 128
 
+#define GRUB_CRYPTODISK_MAX_KEYFILE_SIZE 8192
+
 struct grub_cryptodisk;
 
 typedef gcry_err_code_t
@@ -110,7 +112,8 @@ struct grub_cryptodisk_dev
   grub_cryptodisk_t (*scan) (grub_disk_t disk, const char *check_uuid,
 			     int boot_only, grub_file_t hdr);
   grub_err_t (*recover_key) (grub_disk_t disk, grub_cryptodisk_t dev,
-			     grub_file_t hdr);
+			     grub_file_t hdr, grub_uint8_t *key,
+			     grub_size_t keyfile_size);
 };
 typedef struct grub_cryptodisk_dev *grub_cryptodisk_dev_t;
 
diff --git a/include/grub/file.h b/include/grub/file.h
index a7d7be853..97678aa45 100644
--- a/include/grub/file.h
+++ b/include/grub/file.h
@@ -92,6 +92,8 @@ enum grub_file_type
     GRUB_FILE_TYPE_ZFS_ENCRYPTION_KEY,
     /* File holiding the encryption metadata header */
     GRUB_FILE_TYPE_CRYPTODISK_DETACHED_HEADER,
+    /* File holiding the encryption key */
+    GRUB_FILE_TYPE_CRYPTODISK_ENCRYPTION_KEY,
     /* File we open n grub-fstest.  */
     GRUB_FILE_TYPE_FSTEST,
     /* File we open n grub-mount.  */
-- 
2.27.0

[v5][ 6/6] cryptodisk: Add support for LUKS1 key files

[Denis 'GNUtoo' Carikli]

cryptsetup supports key files thourh the --key-file
--header command line argument for both LUKS1 and LUKS2.

This adds support for LUKS1 key files.

Signed-off-by: John Lane <john@lane.uk.net>
GNUtoo@cyberdimension.org: rebase, fixes, commit message
Signed-off-by: Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
---
ChangeLog since v4:
- Added Reviewed-by tag
---
 grub-core/disk/luks.c | 43 ++++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
index 8dde70d8d..376895259 100644
--- a/grub-core/disk/luks.c
+++ b/grub-core/disk/luks.c
@@ -167,7 +167,9 @@ luks_recover_key (grub_disk_t source, grub_cryptodisk_t dev, grub_file_t hdr,
   struct grub_luks_phdr header;
   grub_size_t keysize;
   grub_uint8_t *split_key = NULL;
-  char passphrase[MAX_PASSPHRASE] = "";
+  char interactive_passphrase[MAX_PASSPHRASE] = "";
+  grub_uint8_t *passphrase;
+  grub_size_t passphrase_length;
   grub_uint8_t candidate_digest[sizeof (header.mkDigest)];
   unsigned i;
   grub_size_t length;
@@ -176,10 +178,6 @@ luks_recover_key (grub_disk_t source, grub_cryptodisk_t dev, grub_file_t hdr,
   char *tmp;
   grub_uint32_t sector;
 
-  /* Keyfiles are not implemented yet */
-  if (keyfile_bytes || keyfile_bytes_size)
-     return GRUB_ERR_NOT_IMPLEMENTED_YET;
-
   if (hdr)
     {
       if (grub_file_seek (hdr, 0) == (grub_off_t) -1)
@@ -208,18 +206,29 @@ luks_recover_key (grub_disk_t source, grub_cryptodisk_t dev, grub_file_t hdr,
   if (!split_key)
     return grub_errno;
 
-  /* Get the passphrase from the user.  */
-  tmp = NULL;
-  if (source->partition)
-    tmp = grub_partition_get_name (source->partition);
-  grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), source->name,
-	       source->partition ? "," : "", tmp ? : "",
-	       dev->uuid);
-  grub_free (tmp);
-  if (!grub_password_get (passphrase, MAX_PASSPHRASE))
+  if (keyfile_bytes)
     {
-      grub_free (split_key);
-      return grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not supplied");
+      /* Use bytestring from key file as passphrase */
+      passphrase = keyfile_bytes;
+      passphrase_length = keyfile_bytes_size;
+    }
+  else
+    {
+      /* Get the passphrase from the user.  */
+      tmp = NULL;
+      if (source->partition)
+        tmp = grub_partition_get_name (source->partition);
+      grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), source->name,
+		    source->partition ? "," : "", tmp ? : "", dev->uuid);
+      grub_free (tmp);
+      if (!grub_password_get (interactive_passphrase, MAX_PASSPHRASE))
+        {
+          grub_free (split_key);
+          return grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not supplied");
+        }
+
+      passphrase = (grub_uint8_t *)interactive_passphrase;
+      passphrase_length = grub_strlen (interactive_passphrase);
     }
 
   /* Try to recover master key from each active keyslot.  */
@@ -237,7 +246,7 @@ luks_recover_key (grub_disk_t source, grub_cryptodisk_t dev, grub_file_t hdr,
 
       /* Calculate the PBKDF2 of the user supplied passphrase.  */
       gcry_err = grub_crypto_pbkdf2 (dev->hash, (grub_uint8_t *) passphrase,
-				     grub_strlen (passphrase),
+				     passphrase_length,
 				     header.keyblock[i].passwordSalt,
 				     sizeof (header.keyblock[i].passwordSalt),
 				     grub_be_to_cpu32 (header.keyblock[i].
-- 
2.27.0

[PATCH] forgotten in Subject, Was: v5 for detached headers and key files

[Denis 'GNUtoo' Carikli]

[-- Attachment #1: Type: text/plain, Size: 497 bytes --]

On Thu, 11 Jun 2020 18:18:01 +0200
Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org> wrote:

Hi,

The patches Subject ends up like that because I forgot the PATCH in
git format-patch --subject-prefix:
> Subject: [v5][ 1/6] cryptodisk: luks: unify grub_cryptodisk_dev
> function names

Do I have to do something about it? Or should I instead leave it like
that and hope that people don't filter it out because they don't
contain [PATCH].

I'm sorry for the inconvenience.

Denis.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] forgotten in Subject, Was: v5 for detached headers and key files

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 740 bytes --]

On Fri, Jun 12, 2020 at 07:30:00AM +0200, Denis 'GNUtoo' Carikli wrote:
> On Thu, 11 Jun 2020 18:18:01 +0200
> Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org> wrote:
> 
> Hi,
> 
> The patches Subject ends up like that because I forgot the PATCH in
> git format-patch --subject-prefix:
> > Subject: [v5][ 1/6] cryptodisk: luks: unify grub_cryptodisk_dev
> > function names
> 
> Do I have to do something about it? Or should I instead leave it like
> that and hope that people don't filter it out because they don't
> contain [PATCH].

Instead of manually adjusting the prefix, you can just say `git
format-patch -v5` to set the patch set's version. I noticed that earlier
but forgot to point this out to you.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] forgotten in Subject, Was: v5 for detached headers and key files

[Denis 'GNUtoo' Carikli]

[-- Attachment #1: Type: text/plain, Size: 1151 bytes --]

On Fri, 12 Jun 2020 10:33:45 +0200
Patrick Steinhardt <ps@pks.im> wrote:

> On Fri, Jun 12, 2020 at 07:30:00AM +0200, Denis 'GNUtoo' Carikli
> wrote:
> > On Thu, 11 Jun 2020 18:18:01 +0200
> > Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org> wrote:
> > 
> > Hi,
> > 
> > The patches Subject ends up like that because I forgot the PATCH in
> > git format-patch --subject-prefix:
> > > Subject: [v5][ 1/6] cryptodisk: luks: unify grub_cryptodisk_dev
> > > function names
> > 
> > Do I have to do something about it? Or should I instead leave it
> > like that and hope that people don't filter it out because they
> > don't contain [PATCH].
> 
> Instead of manually adjusting the prefix, you can just say `git
> format-patch -v5` to set the patch set's version. I noticed that
> earlier but forgot to point this out to you.
Thanks a lot. I'll try it next time as something like that is way more
convenient. 

It also makes it look always the same instead of having many variations
between things like [PATCH v1][1/3], [Patch v1][1/3], [Patch][v1 1/3],
etc and gets rid of the many half-messed up subject prefixes.

Denis.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] forgotten in Subject, Was: v5 for detached headers and key files

[Denis 'GNUtoo' Carikli]

[-- Attachment #1: Type: text/plain, Size: 171 bytes --]

Hi,

Do I need to resend the patches with [PATCH] in the topic?

Or do I need to do something to get the patch merged now that there is
a Reviewed-by tag?

Denis.

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] forgotten in Subject, Was: v5 for detached headers and key files

[Patrick Steinhardt]

[-- Attachment #1: Type: text/plain, Size: 535 bytes --]

On Wed, Jul 22, 2020 at 04:11:24PM +0200, Denis 'GNUtoo' Carikli wrote:
> Hi,
> 
> Do I need to resend the patches with [PATCH] in the topic?
> 
> Or do I need to do something to get the patch merged now that there is
> a Reviewed-by tag?
> 
> Denis.

Hi,

Daniel hasn't been active recently. My assumption is that he's
currently simply lacking the time to get patches in. With all the
crazyness going on in the world I wouldn't be too surprised.

So I guess there's nothing you can do right no but wait :)

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]