grub-devel
[Top][All Lists]
Advanced

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

[PATCH v6 07/12] luks2: Better error handling when setting up the crypto


From: Glenn Washburn
Subject: [PATCH v6 07/12] luks2: Better error handling when setting up the cryptodisk
Date: Fri, 27 Nov 2020 03:03:39 -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 | 80 ++++++++++++++++++++++++++++++++++++++++--
 include/grub/disk.h    | 16 +++++++++
 2 files changed, 93 insertions(+), 3 deletions(-)

diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index b7ed0642e..01f9608e5 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -597,12 +597,26 @@ 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)
        {
@@ -616,11 +630,71 @@ 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 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.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;
+         crypt->total_sectors = grub_strtoull (segment.size, NULL, 10) >> 
crypt->log_sector_size;
+         if (grub_errno == GRUB_ERR_NONE)
+           ;
+         else if(grub_errno == GRUB_ERR_BAD_NUMBER)
+           {
+             /* TODO: Unparsable number-string, try to use the whole disk */
+             grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" size"
+                                    " \"%s\" is not a parsable number\n",
+                                    segment.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.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.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.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));
diff --git a/include/grub/disk.h b/include/grub/disk.h
index 132a1bb75..1a9e8fcf4 100644
--- a/include/grub/disk.h
+++ b/include/grub/disk.h
@@ -174,6 +174,22 @@ typedef struct grub_disk_memberlist 
*grub_disk_memberlist_t;
 /* Return value of grub_disk_get_size() 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)
+    {
+      /* Round up to the nearest log_sector_size sized sector. */
+      sector += 1ULL << ((log_sector_size / disk->log_sector_size) - 1);
+      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]