[PATCH v3 00/10] Cryptodisk fixes for v2.06 redux

[Glenn Washburn]

Heres an updated patch series which addresses comment from Patrick. The only
code change is adding a slot_key member to grub_luks2_keyslot and using that
instead of an extra out parameter to luks2_get_keyslot.

Glenn Washburn (10):
  luks2: Fix use of incorrect index and some grub_error() messages.
  luks2: Improve readability in luks2_get_keyslot.
  luks2: Use more intuitive keyslot key instead of index when naming
    keyslot.
  luks2: grub_cryptodisk_t->total_length is the max number of device
    native sectors
  cryptodisk: Fix cipher IV mode 'plain64' always being set as 'plain'.
  cryptodisk: Properly handle non-512 byte sized sectors.
  cryptodisk: Replace some literals with constants in
    grub_cryptodisk_endecrypt.
  cryptodisk: Rename total_length field in grub_cryptodisk_t to
    total_sectors.
  cryptodisk: Rename offset in grub_cryptodisk_t to offset_sectors.
  luks2: Rename source disk variabled named 'disk' to 'source' as in
    luks.c.

 grub-core/disk/cryptodisk.c | 78 +++++++++++++++++++--------------
 grub-core/disk/geli.c       |  4 +-
 grub-core/disk/luks.c       |  9 ++--
 grub-core/disk/luks2.c      | 86 ++++++++++++++++++++-----------------
 include/grub/cryptodisk.h   | 18 ++++++--
 include/grub/types.h        |  3 ++
 6 files changed, 117 insertions(+), 81 deletions(-)

Range-diff against v2:
1:  e2433b8ab ! 1:  1f65a04e0 luks2: Use more intuitive keyslot key instead of 
index when naming keyslot.
    @@ Commit message
         cryptsetup.
     
      ## grub-core/disk/luks2.c ##
    -@@ grub-core/disk/luks2.c: luks2_parse_digest (grub_luks2_digest_t *out, 
const grub_json_t *digest)
    +@@ grub-core/disk/luks2.c: typedef struct grub_luks2_header 
grub_luks2_header_t;
      
    - static grub_err_t
    - luks2_get_keyslot (grub_luks2_keyslot_t *k, grub_luks2_digest_t *d, 
grub_luks2_segment_t *s,
    --             const grub_json_t *root, grub_size_t keyslot_idx)
    -+             grub_uint64_t *keyslot_key, const grub_json_t *root, 
grub_size_t keyslot_idx)
    + struct grub_luks2_keyslot
    + {
    ++  grub_uint64_t slot_key;
    +   grub_int64_t key_size;
    +   grub_int64_t priority;
    +   struct
    +@@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k, 
grub_luks2_digest_t *d, grub_luks2_s
      {
        grub_json_t keyslots, keyslot, digests, digest, segments, segment;
        grub_size_t i, size;
    @@ grub-core/disk/luks2.c: luks2_parse_digest (grub_luks2_digest_t *out, 
const grub
        if (grub_json_getvalue (&keyslots, root, "keyslots") ||
            grub_json_getchild (&keyslot, &keyslots, keyslot_idx) ||
     -      grub_json_getuint64 (&keyslot_key, &keyslot, NULL) ||
    -+      grub_json_getuint64 (keyslot_key, &keyslot, NULL) ||
    ++      grub_json_getuint64 (&k->slot_key, &keyslot, NULL) ||
            grub_json_getchild (&keyslot, &keyslot, 0) ||
            luks2_parse_keyslot (k, &keyslot))
          return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse keyslot 
index %"PRIuGRUB_SIZE, keyslot_idx);
    @@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k, 
grub_luks2_d
        return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest index 
%"PRIuGRUB_SIZE, i);
      
     -      if ((d->keyslots & (1 << keyslot_key)))
    -+      if ((d->keyslots & (1 << *keyslot_key)))
    ++      if ((d->keyslots & (1 << k->slot_key)))
        break;
          }
        if (i == size)
     -      return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot 
\"%"PRIuGRUB_UINT64_T"\"", keyslot_key);
    -+      return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot 
\"%"PRIuGRUB_UINT64_T"\"", *keyslot_key);
    ++      return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot 
\"%"PRIuGRUB_UINT64_T"\"", k->slot_key);
      
        /* Get segment that matches the digest. */
        if (grub_json_getvalue (&segments, root, "segments") ||
     @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk,
    -   /* Try all keyslot */
    -   for (i = 0; i < size; i++)
    -     {
    --      ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, i);
    -+      grub_uint64_t keyslot_key;
    -+      ret = luks2_get_keyslot (&keyslot, &digest, &segment, &keyslot_key, 
json, i);
    -       if (ret)
    -   goto err;
      
            if (keyslot.priority == 0)
        {
     -    grub_dprintf ("luks2", "Ignoring keyslot %"PRIuGRUB_SIZE" due to 
priority\n", i);
    -+    grub_dprintf ("luks2", "Ignoring keyslot %"PRIuGRUB_UINT64_T" due to 
priority\n", keyslot_key);
    ++    grub_dprintf ("luks2", "Ignoring keyslot %"PRIuGRUB_UINT64_T" due to 
priority\n", keyslot.slot_key);
          continue;
              }
      
     -      grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_SIZE"\n", i);
    -+      grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_UINT64_T"\n", 
keyslot_key);
    ++      grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_UINT64_T"\n", 
keyslot.slot_key);
      
            /* Set up disk according to keyslot's segment. */
            crypt->offset = grub_divmod64 (segment.offset, segment.sector_size, 
NULL);
    @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk,
     -    grub_dprintf ("luks2", "Decryption with keyslot %"PRIuGRUB_SIZE" 
failed: %s\n",
     -                  i, grub_errmsg);
     +    grub_dprintf ("luks2", "Decryption with keyslot %"PRIuGRUB_UINT64_T" 
failed: %s\n",
    -+                  keyslot_key, grub_errmsg);
    ++                  keyslot.slot_key, grub_errmsg);
          continue;
        }
      
    @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk,
     -    grub_dprintf ("luks2", "Could not open keyslot %"PRIuGRUB_SIZE": 
%s\n",
     -                  i, grub_errmsg);
     +    grub_dprintf ("luks2", "Could not open keyslot %"PRIuGRUB_UINT64_T": 
%s\n",
    -+                  keyslot_key, grub_errmsg);
    ++                  keyslot.slot_key, grub_errmsg);
          continue;
        }
      
    @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk,
             * where each element is either empty or holds a key.
             */
     -      grub_printf_ (N_("Slot %"PRIuGRUB_SIZE" opened\n"), i);
    -+      grub_printf_ (N_("Slot %"PRIuGRUB_UINT64_T" opened\n"), 
keyslot_key);
    ++      grub_printf_ (N_("Slot %"PRIuGRUB_UINT64_T" opened\n"), 
keyslot.slot_key);
      
            candidate_key_len = keyslot.key_size;
            break;
2:  3baffdd4f ! 2:  fcb72f70d luks2: grub_cryptodisk_t->total_length is the max 
number of device native sectors
    @@ Commit message
         The total_length field is named confusingly because length usually 
refers to
         bytes, whereas in this case its really the total number of sectors on 
the
         device. Also counter-intuitively, grub_disk_get_size returns the total
    -    number of device native sectors sectors. We need to convert the 
sectors from
    -    the size of the underlying device to the cryptodisk sector size. And
    +    number of device native sectors. We need to convert the sectors from 
the
    +    size of the underlying device to the cryptodisk sector size. And
         segment.size is in bytes which need to be converted to cryptodisk 
sectors.
     
         Also, removed an empty statement.
3:  6da3d8598 = 3:  e8e069cae cryptodisk: Fix cipher IV mode 'plain64' always 
being set as 'plain'.
4:  fd7cb6b16 = 4:  6fe26ba56 cryptodisk: Properly handle non-512 byte sized 
sectors.
5:  b33733199 = 5:  3918a9013 cryptodisk: Replace some literals with constants 
in grub_cryptodisk_endecrypt.
6:  de4f6b2e5 ! 6:  28e0cac66 cryptodisk: Rename total_length field in 
grub_cryptodisk_t to total_sectors.
    @@ Metadata
      ## Commit message ##
         cryptodisk: Rename total_length field in grub_cryptodisk_t to 
total_sectors.
     
    -    This makes the creates an alignment with grub_disk_t naming of the same
    -    field and is more intuitive as to how it should be used.
    +    This creates an alignment with grub_disk_t naming of the same field 
and is
    +    more intuitive as to how it should be used.
     
      ## grub-core/disk/cryptodisk.c ##
     @@ grub-core/disk/cryptodisk.c: grub_cryptodisk_open (const char *name, 
grub_disk_t disk)
7:  a165791de ! 7:  74c6232c9 cryptodisk: Rename offset in grub_cryptodisk_t to 
offset_sectors.
    @@ grub-core/disk/luks.c: configure_ciphers (grub_disk_t disk, const char 
*check_uu
     
      ## grub-core/disk/luks2.c ##
     @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk,
    -       grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_UINT64_T"\n", 
keyslot_key);
    +       grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_UINT64_T"\n", 
keyslot.slot_key);
      
            /* Set up disk according to keyslot's segment. */
     -      crypt->offset = grub_divmod64 (segment.offset, segment.sector_size, 
NULL);
8:  86beb5be8 = 8:  738fe0139 luks2: Rename source disk variabled named 'disk' 
to 'source' as in luks.c.
-- 
2.27.0