[PATCH v5 00/11] Cryptodisk fixes for v2.06 redux

[Glenn Washburn]

This incorporates changes as noted in the v4 thread. This series has been
rebased on the new master from accepted patches from v4. This means that patch
8 from v4 would be patch 1 in this series.  However, I reworked 8-10 from v4
from 3 patches into 2, and I believe simplified the changeset. So v4 patches
8-10 are equivalent to patches 1-2 here.

Also, the v4 patch 14 has been nearly completely reworked to be easier to follow
and more precisely reflect the error handling I wanted.

The changes that created and used grub_log2ull in v4 patch 14 has been pulled
out into two separate patches. And there is a new patch converting spaces to
tabs missed in the original commit of luks2.

Glenn

Glenn Washburn (11):
  luks2: Add slot_key member to struct grub_luks2_keyslot/segment/digest
  luks: Use more intuitive slot key instead of index in user messages.
  cryptodisk: Replace some literals with constants in
    grub_cryptodisk_endecrypt
  luks2: grub_cryptodisk_t->total_sectors is the max number of device
    native sectors
  cryptodisk: Properly handle non-512 byte sized sectors
  luks2: Better error handling when setting up the cryptodisk
  luks2: Error check segment.sector_size
  whitespace: convert 8 spaces to tabs.
  mips: Enable __clzdi2()
  misc: Add grub_log2ull macro for calculating log base 2 of 64-bit
    integers
  luks2: Use grub_log2ull to calculate log_sector_size and improve
    readability

 grub-core/disk/cryptodisk.c  |  64 ++++++++++------
 grub-core/disk/luks.c        |   5 +-
 grub-core/disk/luks2.c       | 144 ++++++++++++++++++++++++++++-------
 grub-core/kern/compiler-rt.c |   2 +-
 include/grub/compiler-rt.h   |   2 +-
 include/grub/cryptodisk.h    |   8 +-
 include/grub/disk.h          |  16 ++++
 include/grub/misc.h          |   3 +
 include/grub/types.h         |   5 ++
 9 files changed, 192 insertions(+), 57 deletions(-)

Range-diff against v4:
 1:  f1d530757 <  -:  --------- luks2: Split idx into three variables: 
keyslot_key, digest_key, segment_key.
 2:  b90ab8e75 <  -:  --------- luks2: Improve error messages in 
luks2_get_keyslot.
 -:  --------- >  1:  ec506b668 luks2: Add slot_key member to struct 
grub_luks2_keyslot/segment/digest
 3:  51add6875 !  2:  3a9fca36f luks2: Use more intuitive keyslot key instead 
of index when naming keyslot.
    @@ Metadata
     Author: Glenn Washburn <development@efficientek.com>
     
      ## Commit message ##
    -    luks2: Use more intuitive keyslot key instead of index when naming 
keyslot.
    +    luks: Use more intuitive slot key instead of index in user messages.
     
    -    Use the keyslot key value in the keyslot json array rather than the 
index of
    -    the keyslot in the json array. This is less confusing for the end 
user. For
    -    example, say you have a LUKS2 device with a key in slot 1 and slot 4. 
When
    -    using the password for slot 4 to unlock the device, the messages using 
the
    -    index of the keyslot will mention keyslot 1 (its a zero-based index).
    -    Furthermore, with this change the keyslot number will align with the 
number
    -    used to reference the keyslot when using the --key-slot argument to
    -    cryptsetup.
    +    Use the slot key name in the json array rather than the 0 based index 
in the
    +    json array for keyslots, segments, and digests. This is less confusing 
for
    +    the end user. For example, say you have a LUKS2 device with a key in 
slot 1
    +    and slot 4. When using the password for slot 4 to unlock the device, 
the
    +    messages using the index of the keyslot will mention keyslot 1 (its a
    +    zero-based index). Furthermore, with this change the keyslot number 
will
    +    align with the number used to reference the keyslot when using the
    +    --key-slot argument to cryptsetup. Error messages now distinguish 
between
    +    indexes and slot keys. The former include the string "index" in the 
error
    +    string, and the later are surrounded in quotes.
     
      ## grub-core/disk/luks2.c ##
    -@@ grub-core/disk/luks2.c: typedef struct grub_luks2_header 
grub_luks2_header_t;
    - 
    - struct grub_luks2_keyslot
    - {
    -+  grub_uint64_t slot_key;
    -   grub_int64_t key_size;
    -   grub_int64_t priority;
    -   struct
    -@@ grub-core/disk/luks2.c: typedef struct grub_luks2_keyslot 
grub_luks2_keyslot_t;
    - 
    - struct grub_luks2_segment
    - {
    -+  grub_uint64_t slot_key;
    -   grub_uint64_t offset;
    -   const char      *size;
    -   const char      *encryption;
    -@@ grub-core/disk/luks2.c: typedef struct grub_luks2_segment 
grub_luks2_segment_t;
    - 
    - struct grub_luks2_digest
    - {
    -+  grub_uint64_t slot_key;
    -   /* Both keyslots and segments are interpreted as bitfields here */
    -   grub_uint64_t   keyslots;
    -   grub_uint64_t   segments;
     @@ 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_uint64_t keyslot_key, digest_key, segment_key;
    -+  grub_uint64_t digest_key, segment_key;
    - 
    -   /* Get nth keyslot */
    -   if (grub_json_getvalue (&keyslots, root, "keyslots") ||
    -       grub_json_getchild (&keyslot, &keyslots, keyslot_idx) ||
    --      grub_json_getuint64 (&keyslot_key, &keyslot, NULL) ||
    -+      grub_json_getuint64 (&k->slot_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);
    +-    return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse keyslot 
%"PRIuGRUB_SIZE, keyslot_idx);
    ++    return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse keyslot 
index %"PRIuGRUB_SIZE, keyslot_idx);
    + 
    +   /* Get digest that matches the keyslot. */
    +   if (grub_json_getvalue (&digests, root, "digests") ||
     @@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k, 
grub_luks2_digest_t *d, grub_luks2_s
    +     grub_json_getuint64 (&d->slot_key, &digest, NULL) ||
    +           grub_json_getchild (&digest, &digest, 0) ||
                luks2_parse_digest (d, &digest))
    -   return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest index 
%"PRIuGRUB_SIZE, i);
    +-  return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest 
%"PRIuGRUB_SIZE, i);
    ++  return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse digest index 
%"PRIuGRUB_SIZE, i);
      
    --      if ((d->keyslots & (1 << keyslot_key)))
    -+      d->slot_key = digest_key;
    -+      if ((d->keyslots & (1 << k->slot_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_SIZE, keyslot_idx);
     +      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_get_keyslot (grub_luks2_keyslot_t *k, 
grub_luks2_digest_t *d, grub_luks2_s
    +     grub_json_getuint64 (&s->slot_key, &segment, NULL) ||
    +     grub_json_getchild (&segment, &segment, 0) ||
                luks2_parse_segment (s, &segment))
    -   return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse segment 
index %"PRIuGRUB_SIZE, i);
    +-  return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse segment 
%"PRIuGRUB_SIZE, i);
    ++  return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not parse segment 
index %"PRIuGRUB_SIZE, i);
      
    -+      s->slot_key = segment_key;
    -       if ((d->segments & (1 << segment_key)))
    +       if ((d->segments & (1 << s->slot_key)))
        break;
          }
    +   if (i == size)
    +-    return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest 
%"PRIuGRUB_SIZE);
    ++    return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No segment for digest 
\"%"PRIuGRUB_UINT64_T"\"", d->slot_key);
    + 
    +   return GRUB_ERR_NONE;
    + }
     @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source,
      
            if (keyslot.priority == 0)
 4:  39e382053 !  3:  f45f5f3dd cryptodisk: Replace some literals with 
constants in grub_cryptodisk_endecrypt.
    @@ Metadata
     Author: Glenn Washburn <development@efficientek.com>
     
      ## Commit message ##
    -    cryptodisk: Replace some literals with constants in 
grub_cryptodisk_endecrypt.
    +    cryptodisk: Replace some literals with constants in 
grub_cryptodisk_endecrypt
     
         This should improve readability of code by providing clues as to what 
the
         value represents. The new macro GRUB_TYPE_BITS(type) returns the 
number of
    @@ include/grub/types.h: typedef grub_int32_t       grub_ssize_t;
      #endif
      # define GRUB_LONG_MIN (-GRUB_LONG_MAX - 1)
      
    -+#define GRUB_TYPE_U_MAX(type) ((2 * ((1ULL << (GRUB_TYPE_BITS (type) - 
1)) - 1)) + 1)
    ++#define GRUB_TYPE_U_MAX(type) ((typeof (1ULL))((typeof (type))(~0)))
     +#define GRUB_TYPE_U_MIN(type) 0ULL
     +
      typedef grub_uint64_t grub_properly_aligned_t;
 5:  7696a000f !  4:  da2b63607 luks2: grub_cryptodisk_t->total_length is the 
max number of device native sectors
    @@ Metadata
     Author: Glenn Washburn <development@efficientek.com>
     
      ## Commit message ##
    -    luks2: grub_cryptodisk_t->total_length is the max number of device 
native sectors
    +    luks2: grub_cryptodisk_t->total_sectors is the max number of device 
native sectors
     
    -    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. 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.
    +    We need to convert the sectors from the size of the underlying device 
to the
    +    cryptodisk sector size; segment.size is in bytes which need to be 
converted
    +    to cryptodisk sectors as well. And counter-intuitively, 
grub_disk_get_size
    +    returns the total number of device native sectors.
     
         Also, removed an empty statement.
     
 6:  43437eb13 !  5:  036304262 cryptodisk: Properly handle non-512 byte sized 
sectors.
    @@ Metadata
     Author: Glenn Washburn <development@efficientek.com>
     
      ## Commit message ##
    -    cryptodisk: Properly handle non-512 byte sized sectors.
    +    cryptodisk: Properly handle non-512 byte sized sectors
     
         By default, dm-crypt internally uses an IV that corresponds to 512-byte
         sectors, even when a larger sector size is specified. What this means 
is
    @@ grub-core/disk/cryptodisk.c: grub_cryptodisk_endecrypt (struct 
grub_cryptodisk *
          {
            grub_size_t sz = ((dev->cipher->cipher->blocksize
                         + sizeof (grub_uint32_t) - 1)
    -                   / sizeof (grub_uint32_t));
    --      grub_uint32_t iv[(GRUB_CRYPTO_MAX_CIPHER_BLOCKSIZE + 3) / 4];
    -+      grub_uint32_t iv[(GRUB_CRYPTO_MAX_CIPHER_BLOCKSIZE + 3) / 4] 
__attribute__((aligned (sizeof (grub_uint64_t))));
    - 
    -       if (dev->rekey)
    -   {
     @@ grub-core/disk/cryptodisk.c: grub_cryptodisk_endecrypt (struct 
grub_cryptodisk *dev,
            if (!ctx)
              return GPG_ERR_OUT_OF_MEMORY;
    @@ grub-core/disk/cryptodisk.c: grub_cryptodisk_endecrypt (struct 
grub_cryptodisk *
     +     * 32 bits.
     +     */
     +    {
    -+      grub_uint64_t *iv64 = (grub_uint64_t *) iv;
    -+      *iv64 = grub_cpu_to_le64 (sector << (log_sector_size
    ++      grub_uint64_t iv64;
    ++
    ++      iv64 = grub_cpu_to_le64 (sector << (log_sector_size
     +                                           - 
GRUB_CRYPTODISK_IV_LOG_SIZE));
    ++      grub_set_unaligned64 (iv, iv64);
     +      if (dev->mode_iv == GRUB_CRYPTODISK_MODE_IV_PLAIN)
     +        iv[1] = 0;
     +    }
 7:  71dadcd07 <  -:  --------- luks2: Better error handling when setting up 
the cryptodisk.
 -:  --------- >  6:  7bf56c4b3 luks2: Better error handling when setting up 
the cryptodisk
 8:  cc96baf5a !  7:  b167b014d luks2: Error check segment.sector_size.
    @@ Metadata
     Author: Glenn Washburn <development@efficientek.com>
     
      ## Commit message ##
    -    luks2: Error check segment.sector_size.
    +    luks2: Error check segment.sector_size
     
      ## grub-core/disk/luks2.c ##
     @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source,
    @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source,
      
     +      /* Sector size should be one of 512, 1024, 2048, or 4096. */
     +      if (!(segment.sector_size == 512 || segment.sector_size == 1024 ||
    -+            segment.sector_size == 2048 || segment.sector_size == 4096))
    ++      segment.sector_size == 2048 || segment.sector_size == 4096))
     +  {
     +    grub_dprintf ("luks2", "Segment \"%"PRIuGRUB_UINT64_T"\" sector"
     +                           " size %"PRIuGRUB_UINT64_T" is not one of"
    -+                           " 512, 1024, 2048, or 4096.",
    ++                           " 512, 1024, 2048, or 4096\n",
     +                           segment.slot_key, segment.sector_size);
     +    continue;
     +  }
     +
            /* Set up disk according to keyslot's segment. */
            crypt->offset_sectors = grub_divmod64 (segment.offset, 
segment.sector_size, NULL);
    -       crypt->log_sector_size = grub_log2ull (segment.sector_size);
    +       crypt->log_sector_size = sizeof (unsigned int) * 8
 -:  --------- >  8:  c503ecbb8 whitespace: convert 8 spaces to tabs.
 -:  --------- >  9:  8316e94ce mips: Enable __clzdi2()
 -:  --------- > 10:  37257725c misc: Add grub_log2ull macro for calculating 
log base 2 of 64-bit integers
 -:  --------- > 11:  8dd088cad luks2: Use grub_log2ull to calculate 
log_sector_size and improve readability
-- 
2.27.0