grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v7 12/17] luks2: Better error handling when setting up the crypto


From: Glenn Washburn
Subject: [PATCH v7 12/17] luks2: Better error handling when setting up the cryptodisk
Date: Fri, 4 Dec 2020 10:43:41 -0600

First, check to make sure that source disk has a known size. If not, print
debug message and return error. There are 4 cases where
GRUB_DISK_SIZE_UNKNOWN is set (biosdisk, obdisk, ofdisk, and uboot), and in
all those cases processing continues. So this is probably a bit
conservative. However, 3 of the cases seem pathological, and the other,
biosdisk, happens when booting from a cd. Since I doubt booting from a LUKS2
volume on a cd is a big use case, we'll error until someone complains.

Do some sanity checking on data coming from the luks header. If segment.size
is "dynamic",

Check for errors from grub_strtoull when converting segment size from
string. If a GRUB_ERR_BAD_NUMBER error was returned, then the string was
not a valid parsable number, so skip the key. If GRUB_ERR_OUT_OF_RANGE was
returned, then there was an overflow in converting to a 64-bit unsigned
integer. So this could be a very large disk (perhaps large raid array).
In this case, we want to continue to try to use this key so long as the
source disk's size is greater than this segment size. Otherwise, this is
an invalid segment, and continue on to the next key.

Signed-off-by: Glenn Washburn <development@efficientek.com>
---
 grub-core/disk/luks2.c | 98 +++++++++++++++++++++++++++++++++++++-----
 include/grub/disk.h    | 17 ++++++++
 2 files changed, 105 insertions(+), 10 deletions(-)

diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index de2e70796..1bb3a333d 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -290,7 +290,7 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
grub_luks2_digest_t *d, grub_luks2_s
        break;
     }
   if (i == size)
-      return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot 
\"%"PRIuGRUB_UINT64_T"\"", k->slot_key);
+      return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot 
\"%"PRIuGRUB_UINT64_T"\"", k->json_slot_key);
 
   /* Get segment that matches the digest. */
   if (grub_json_getvalue (&segments, root, "segments") ||
@@ -308,7 +308,7 @@ luks2_get_keyslot (grub_luks2_keyslot_t *k, 
grub_luks2_digest_t *d, grub_luks2_s
        break;
     }
   if (i == size)
-    return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest 
\"%"PRIuGRUB_UINT64_T"\"", d->slot_key);
+    return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest 
\"%"PRIuGRUB_UINT64_T"\"", d->json_slot_key);
 
   return GRUB_ERR_NONE;
 }
@@ -600,37 +600,115 @@ luks2_recover_key (grub_disk_t source,
       goto err;
     }
 
+  if (source->total_sectors == GRUB_DISK_SIZE_UNKNOWN)
+    {
+      /* FIXME: Allow use of source disk, and maybe cause errors in read. */
+      grub_dprintf ("luks2", "Source disk %s has an unknown size, "
+                            "conservatively returning error\n", source->name);
+      ret = grub_error (GRUB_ERR_BUG, "Unknown size of luks2 source device");
+      goto err;
+    }
+
   /* Try all keyslot */
   for (i = 0; i < size; i++)
     {
+      typeof(source->total_sectors) max_crypt_sectors = 0;
+
+      grub_errno = GRUB_ERR_NONE;
       ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, i);
       if (ret)
        goto err;
+      if (grub_errno != GRUB_ERR_NONE)
+         grub_dprintf ("luks2", "Ignoring unhandled error %d from 
luks2_get_keyslot\n", grub_errno);
 
       if (keyslot.priority == 0)
        {
-         grub_dprintf ("luks2", "Ignoring keyslot \"%"PRIuGRUB_UINT64_T"\" due 
to priority\n", keyslot.slot_key);
+         grub_dprintf ("luks2", "Ignoring keyslot \"%"PRIuGRUB_UINT64_T"\" due 
to priority\n", keyslot.json_slot_key);
          continue;
         }
 
-      grub_dprintf ("luks2", "Trying keyslot \"%"PRIuGRUB_UINT64_T"\"\n", 
keyslot.slot_key);
+      grub_dprintf ("luks2", "Trying keyslot \"%"PRIuGRUB_UINT64_T"\"\n", 
keyslot.json_slot_key);
 
       /* Set up disk according to keyslot's segment. */
       crypt->offset_sectors = grub_divmod64 (segment.offset, 
segment.sector_size, NULL);
       crypt->log_sector_size = sizeof (unsigned int) * 8
                - __builtin_clz ((unsigned int) segment.sector_size) - 1;
+      /* Set to the source disk size, which is the maximum we allow. */
+      max_crypt_sectors = grub_disk_convert_sector(source,
+                                                  source->total_sectors,
+                                                  crypt->log_sector_size);
+
+      if (max_crypt_sectors < crypt->offset_sectors)
+       {
+         grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" has offset"
+                                " %"PRIuGRUB_UINT64_T" which is greater than"
+                                " source disk size %"PRIuGRUB_UINT64_T","
+                                " skipping\n",
+                                segment.json_slot_key, crypt->offset_sectors,
+                                max_crypt_sectors);
+         continue;
+       }
+
       if (grub_strcmp (segment.size, "dynamic") == 0)
-       crypt->total_sectors = (grub_disk_get_size (source) >> 
(crypt->log_sector_size - source->log_sector_size))
-                              - crypt->offset_sectors;
+       crypt->total_sectors = max_crypt_sectors - crypt->offset_sectors;
       else
-       crypt->total_sectors = grub_strtoull (segment.size, NULL, 10) >> 
crypt->log_sector_size;
+       {
+         grub_errno = GRUB_ERR_NONE;
+         /* Convert segment.size to sectors, rounding up to nearest sector */
+         crypt->total_sectors = grub_strtoull (segment.size, NULL, 10);
+         crypt->total_sectors = ALIGN_UP (crypt->total_sectors,
+                                          1 << crypt->log_sector_size);
+         crypt->total_sectors >>= crypt->log_sector_size;
+
+         if (grub_errno == GRUB_ERR_NONE)
+           ;
+         else if (grub_errno == GRUB_ERR_BAD_NUMBER)
+           {
+             grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" size"
+                                    " \"%s\" is not a parsable number\n",
+                                    segment.json_slot_key, segment.size);
+             continue;
+           }
+         else if (grub_errno == GRUB_ERR_OUT_OF_RANGE)
+           {
+             /*
+              * There was an overflow in parsing segment.size, so disk must
+              * be very large or the string is incorrect.
+              */
+             grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" size"
+                                    " %s overflowed 64-bit unsigned integer,"
+                                    " the end of the crypto device will be"
+                                    " inaccessible\n",
+                                    segment.json_slot_key, segment.size);
+             if (crypt->total_sectors > max_crypt_sectors)
+               crypt->total_sectors = max_crypt_sectors;
+           }
+       }
+
+      if (crypt->total_sectors == 0)
+       {
+         grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" has zero"
+                                " sectors, skipping\n",
+                                segment.json_slot_key);
+         continue;
+       }
+      else if (max_crypt_sectors < (crypt->offset_sectors + 
crypt->total_sectors))
+       {
+         grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" has last"
+                                " data position greater than source disk size,"
+                                " the end of the crypto device will be"
+                                " inaccessible\n",
+                                segment.json_slot_key);
+         /* Allow decryption up to the end of the source disk. */
+         crypt->total_sectors = max_crypt_sectors - crypt->offset_sectors;
+       }
 
       ret = luks2_decrypt_key (candidate_key, source, crypt, &keyslot,
                               (const grub_uint8_t *) passphrase, grub_strlen 
(passphrase));
       if (ret)
        {
          grub_dprintf ("luks2", "Decryption with keyslot 
\"%"PRIuGRUB_UINT64_T"\" failed: %s\n",
-                       keyslot.slot_key, grub_errmsg);
+                       keyslot.json_slot_key, grub_errmsg);
          continue;
        }
 
@@ -638,7 +716,7 @@ luks2_recover_key (grub_disk_t source,
       if (ret)
        {
          grub_dprintf ("luks2", "Could not open keyslot 
\"%"PRIuGRUB_UINT64_T"\": %s\n",
-                       keyslot.slot_key, grub_errmsg);
+                       keyslot.json_slot_key, grub_errmsg);
          continue;
        }
 
@@ -646,7 +724,7 @@ luks2_recover_key (grub_disk_t source,
        * TRANSLATORS: It's a cryptographic key slot: one element of an array
        * where each element is either empty or holds a key.
        */
-      grub_printf_ (N_("Slot \"%"PRIuGRUB_UINT64_T"\" opened\n"), 
keyslot.slot_key);
+      grub_printf_ (N_("Slot \"%"PRIuGRUB_UINT64_T"\" opened\n"), 
keyslot.json_slot_key);
 
       candidate_key_len = keyslot.key_size;
       break;
diff --git a/include/grub/disk.h b/include/grub/disk.h
index 0fb727d3d..f9227f285 100644
--- a/include/grub/disk.h
+++ b/include/grub/disk.h
@@ -27,6 +27,8 @@
 #include <grub/device.h>
 /* For NULL.  */
 #include <grub/mm.h>
+/* For ALIGN_UP.  */
+#include <grub/misc.h>
 
 /* These are used to set a device id. When you add a new disk device,
    you must define a new id for it here.  */
@@ -174,6 +176,21 @@ typedef struct grub_disk_memberlist 
*grub_disk_memberlist_t;
 /* Return value of grub_disk_native_sectors() in case disk size is unknown. */
 #define GRUB_DISK_SIZE_UNKNOWN  0xffffffffffffffffULL
 
+/* Convert sector number from disk sized sectors to a log-size sized sector. */
+static inline grub_disk_addr_t
+grub_disk_convert_sector (grub_disk_t disk,
+                         grub_disk_addr_t sector,
+                         grub_size_t log_sector_size)
+{
+  if (disk->log_sector_size < log_sector_size)
+    {
+      sector = ALIGN_UP (sector, 1 << (log_sector_size / 
disk->log_sector_size));
+      return sector >> (log_sector_size - disk->log_sector_size);
+    }
+  else
+    return sector << (disk->log_sector_size - log_sector_size);
+}
+
 /* Convert to GRUB native disk sized sector from disk sized sector. */
 static inline grub_disk_addr_t
 grub_disk_from_native_sector (grub_disk_t disk, grub_disk_addr_t sector)
-- 
2.27.0




reply via email to

[Prev in Thread] Current Thread [Next in Thread]