v7 for detached headers and key files

v7 for detached headers and key files

[Denis 'GNUtoo' Carikli]

Hi,

I've addressed the issues found in the last round.

Like the last time I also tested it on real hardware.

I'm also sorry for the delay.

Denis.

[PATCH v7 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
ChangeLog since v5:
- No changes
ChangeLog since v6:
- No changes

---
 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 aa9877b68..229036d1d 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;
@@ -311,7 +310,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.29.2

[PATCH v7 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
ChangeLog since v5:
- No changes
ChangeLog since v6:
- No changes
---
 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 0175ce4c4..203a9081b 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.29.2

[PATCH v7 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
ChangeLog since v5:
- No changes
ChangeLog since v6: Rebase only.
The rebase was needed due these commits:
  - 0eb44d319 luks2: Rename source disk variable named
                     "disk" to "source" as in luks.c
    => disk was also renamed to source in this rebased patch.
  - ee12785f7 luks2: Strip dashes off of the UUID
    => No changes to this patch, it just shows in the
       context lines.
---
 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 473c93976..fec949ad0 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}
   };
 
@@ -967,6 +968,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)
@@ -991,13 +993,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);
@@ -1038,7 +1040,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)
@@ -1092,6 +1094,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)
     {
@@ -1299,7 +1315,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 203a9081b..d4d537e05 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 229036d1d..40cc162fb 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;
 
@@ -150,8 +155,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;
@@ -164,6 +168,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 d96764a02..237b2aa77 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -342,13 +342,18 @@ 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;
   char uuid[sizeof (header.uuid) + 1];
   grub_size_t i, j;
 
+  /* Detached headers are not implemented yet */
+  if (hdr_file)
+    return NULL;
+
   if (check_boot)
     return NULL;
 
@@ -530,8 +535,8 @@ luks2_decrypt_key (grub_uint8_t *out_key,
 }
 
 static grub_err_t
-luks2_recover_key (grub_disk_t source,
-		   grub_cryptodisk_t crypt)
+luks2_recover_key (grub_disk_t source, grub_cryptodisk_t crypt,
+		   grub_file_t hdr_file)
 {
   grub_uint8_t candidate_key[GRUB_CRYPTODISK_MAX_KEYLEN];
   char passphrase[MAX_PASSPHRASE], cipher[32];
@@ -545,6 +550,10 @@ luks2_recover_key (grub_disk_t source,
   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 (source, &header);
   if (ret)
     return ret;
diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
index c9bf7597a..d6f2f6e2c 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
@@ -112,8 +113,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.29.2

[PATCH v7 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
ChangeLog since v5:
- Fixed missing grub_file_seek return value check
ChangeLog since v6:
- No changes
---
 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 40cc162fb..1c518902b 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)
@@ -164,15 +171,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;
 
@@ -241,13 +255,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) == (grub_off_t) -1)
+          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.29.2

[PATCH v7 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

ChangeLog since v5:
-------------------
- No changes

ChangeLog since v6:
-------------------
- Fixed format string conversions issues found by Glenn Washburn:
  - "The type of keyfile->size is grub_off_t which is 
     typedef'd from grub_uint64_t. 
     [...] when compiling for i386, PRIuGRUB_SIZE expands
     to %lu, which accepts a 32-bit uint.
     This will cause the strict format string checking to fail 
     the build."
  - "The macro GRUB_CRYPTODISK_MAX_KEYFILE_SIZE gets expanded
     to an integer literal which gets type cast as an int,
     but PRIuGRUB_SIZE expects long or long long."
- Rebased. The rebase was needed due this commits:
  - 0eb44d319 luks2: Rename source disk variable named
                     "disk" to "source" as in luks.c
    => No changes to this patch, it just shows in the
       context lines.


---
 grub-core/disk/cryptodisk.c | 86 ++++++++++++++++++++++++++++++++++++-
 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, 105 insertions(+), 9 deletions(-)

diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
index fec949ad0..11c0f7ab6 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}
   };
 
@@ -969,6 +972,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)
@@ -999,7 +1004,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);
@@ -1109,6 +1114,85 @@ 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 (%d)\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_UINT64_T
+				  " 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 d4d537e05..8fb24cc5b 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 1c518902b..b7867585a 100644
--- a/grub-core/disk/luks.c
+++ b/grub-core/disk/luks.c
@@ -162,7 +162,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;
@@ -176,6 +177,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 237b2aa77..f3e293a9e 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -536,7 +536,8 @@ luks2_decrypt_key (grub_uint8_t *out_key,
 
 static grub_err_t
 luks2_recover_key (grub_disk_t source, 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];
@@ -550,8 +551,8 @@ luks2_recover_key (grub_disk_t source, 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 (source, &header);
diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
index d6f2f6e2c..572dbc782 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
@@ -115,7 +117,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.29.2

[PATCH v7 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
ChangeLog since v5:
- No changes
ChangeLog since v6:
- No changes
---
 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 b7867585a..0f6948f73 100644
--- a/grub-core/disk/luks.c
+++ b/grub-core/disk/luks.c
@@ -168,7 +168,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;
@@ -177,10 +179,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)
@@ -209,18 +207,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.  */
@@ -238,7 +247,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.29.2

Re: v7 for detached headers and key files

[Glenn Washburn]

On Thu, 10 Dec 2020 10:14:53 +0100
Denis 'GNUtoo' Carikli <GNUtoo@cyberdimension.org> wrote:

> Hi,
> 
> I've addressed the issues found in the last round.
> 
> Like the last time I also tested it on real hardware.
> 
> I'm also sorry for the delay.

Great, thanks for the update. Since Grub is currently in a feature
freeze until the release, this won't get accepted until after. It'll
probably be less work for you to wait until after the release, then
rebase and fix any issues that might have arisen from now til then.
Though, at this point, probably not much will need to change in a
rebase.  Regardlesss, I doubt anyone will even look at them until then.
This is a feature that I really want in Grub because my testing patches
rely on this infrastructure, so I don't want it to get forgotten about
either.

Also, when updating a patch series, its nice to use the "--cover-letter"
and "--range-diff" options to git format-patch to show quickly what has
changed since the last patch series version.

Glenn

Re: v7 for detached headers and key files

[Denis 'GNUtoo' Carikli]

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

On Wed, 16 Dec 2020 12:35:13 -0600
Glenn Washburn <development@efficientek.com> wrote:
> Great, thanks for the update. Since Grub is currently in a feature
> freeze until the release, this won't get accepted until after.
At least, it makes it easier for someone else to pickup the patch set,
rebase it and push it if for a reason or another I don't find the time
anymore to do it.

> Also, when updating a patch series, its nice to use the
> "--cover-letter" and "--range-diff" options to git format-patch to
> show quickly what has changed since the last patch series version.
Oh nice, thanks a lot!

I didn't know "--cover-letter" for git-format-patch and I never heard
of --range-diff. 

I was using meld and a very manual process for that that was also error
prone as you can't store individual patch files in git (the ChangeLog
is lost in that case).

I'll try to remember to check that out next time I send a new revision
of a patchset.

Denis.

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

Re: v7 for detached headers and key files

[Denis 'GNUtoo' Carikli]

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

Hi,

Since the last time where I sent this patch serie (December 2020), GRUB
2.06 is out and many of the security issues have now been fixed, so I
hope it's a good time to ping again for this patch now.

So far all the concerns found in the previous reviews have been
addressed, and no new issues were found in the v7, but it still
needed a more in depth review.

The v7 patch set also still applies on top of master as of today.

Denis.

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

Re: v7 for detached headers and key files

[Daniel Kiper]

Hey Denis,

On Wed, Aug 18, 2021 at 12:30:26AM +0200, Denis 'GNUtoo' Carikli wrote:
> Hi,
>
> Since the last time where I sent this patch serie (December 2020), GRUB
> 2.06 is out and many of the security issues have now been fixed, so I
> hope it's a good time to ping again for this patch now.
>
> So far all the concerns found in the previous reviews have been
> addressed, and no new issues were found in the v7, but it still
> needed a more in depth review.
>
> The v7 patch set also still applies on top of master as of today.

Adding to my TODO list. Expect a review in the following weeks...

Daniel