[PATCH v9 3/6] luks2: Better error handling when setting up the cryptodisk

[Glenn Washburn]

Do some sanity checking on data coming from the luks header. If segment.size
is "dynamic", verify that the offset is not past the end of disk. Otherwise,
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, but only allow
access up to the end of the source disk.

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

diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index d30c3bfff..0e5061243 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -612,9 +612,14 @@ luks2_recover_key (grub_disk_t source,
   /* Try all keyslot */
   for (json_idx = 0; json_idx < size; json_idx++)
     {
+      typeof(source->total_sectors) max_crypt_sectors = 0;
+
+      grub_errno = GRUB_ERR_NONE;
       ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, json_idx);
       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)
        {
@@ -639,11 +644,81 @@ luks2_recover_key (grub_disk_t source,
       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/partition size, which is the maximum we allow. 
*/
+      max_crypt_sectors = grub_disk_native_sectors(source);
+      max_crypt_sectors = grub_convert_sector(max_crypt_sectors,
+                                             GRUB_DISK_SECTOR_BITS,
+                                             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.idx, crypt->offset_sectors,
+                                max_crypt_sectors);
+         continue;
+       }
+
       if (grub_strcmp (segment.size, "dynamic") == 0)
-       crypt->total_sectors = (grub_disk_native_sectors (source) >> 
(crypt->log_sector_size - GRUB_DISK_SECTOR_BITS))
-                              - 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);
+
+         if (grub_errno == GRUB_ERR_NONE)
+           {
+             crypt->total_sectors = ALIGN_UP (crypt->total_sectors,
+                                              1 << crypt->log_sector_size);
+             crypt->total_sectors >>= crypt->log_sector_size;
+           }
+         else if (grub_errno == GRUB_ERR_BAD_NUMBER)
+           {
+             grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" size"
+                                    " \"%s\" is not a parsable number,"
+                                    " skipping keyslot\n",
+                                    segment.idx, 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.
+              */
+             /*
+              * TODO: Allow reading of at least up max_crypt_sectors. Really,
+              * its very unlikely one would be booting from such a large drive
+              * anyway. Use another smaller LUKS boot device.
+              */
+             grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" size"
+                                    " %s overflowed 64-bit unsigned integer,"
+                                    " skipping keyslot\n",
+                                    segment.idx, segment.size);
+             continue;
+           }
+       }
+
+      if (crypt->total_sectors == 0)
+       {
+         grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" has zero"
+                                " sectors, skipping\n",
+                                segment.idx);
+         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.idx);
+         /* 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));
diff --git a/include/grub/disk.h b/include/grub/disk.h
index e08663cdb..f95aca929 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.  */
@@ -180,6 +182,23 @@ 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 one sector size to another. */
+static inline grub_disk_addr_t
+grub_convert_sector (grub_disk_addr_t sector,
+                    grub_size_t log_sector_size_from,
+                    grub_size_t log_sector_size_to)
+{
+  if (log_sector_size_from == log_sector_size_to)
+    return sector;
+  else if (log_sector_size_from < log_sector_size_to)
+    {
+      sector = ALIGN_UP (sector, 1 << (log_sector_size_to - 
log_sector_size_from));
+      return sector >> (log_sector_size_to - log_sector_size_from);
+    }
+  else
+    return sector << (log_sector_size_from - log_sector_size_to);
+}
+
 /* 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