[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v4 00/15] Cryptodisk fixes for v2.06 redux
|
From: |
Glenn Washburn |
|
Subject: |
[PATCH v4 00/15] Cryptodisk fixes for v2.06 redux |
|
Date: |
Fri, 6 Nov 2020 22:44:20 -0600 |
Here we go again. I have not added RB sigs to any patches since I've had to
modify them in moving them earlier in the patch set. This would be the first
three rename patches. The first two patches of v3 have been reworked into 5
patches (05-09) which combine to be equivalent. Most of the changes since v3 are
added error handling around luks2 cryptodisk setup. Also, I addressed an IV
alignment issue in grub_cryptodisk_endecrypt for the
GRUB_CRYPTODISK_MODE_IV_PLAIN* case, found by Daniel. I'm not sure it works
(I can't reproduce the build error) or that its the desired way of doing it.
Glenn
Glenn Washburn (15):
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.
types: Define GRUB_CHAR_BIT based on compiler macro instead of using
literal.
luks2: Use correct index variable when looping in luks2_get_keyslot.
luks2: Rename variable i to keyslot_idx in luks2_get_keyslot.
luks2: Rename index variable j to i.
luks2: Split idx into three variables: keyslot_key, digest_key,
segment_key.
luks2: Improve error messages in luks2_get_keyslot.
luks2: Use more intuitive keyslot key instead of index when naming
keyslot.
cryptodisk: Replace some literals with constants in
grub_cryptodisk_endecrypt.
luks2: grub_cryptodisk_t->total_length 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.
grub-core/disk/cryptodisk.c | 76 ++++++++++-------
grub-core/disk/geli.c | 4 +-
grub-core/disk/luks.c | 9 +-
grub-core/disk/luks2.c | 165 +++++++++++++++++++++++++++---------
include/grub/cryptodisk.h | 18 +++-
include/grub/misc.h | 2 +
include/grub/types.h | 15 +++-
7 files changed, 205 insertions(+), 84 deletions(-)
Range-diff against v3:
5: e8e069cae = 1: 90fedb50c cryptodisk: Fix cipher IV mode 'plain64' always
being set as 'plain'.
8: 28e0cac66 ! 2: d72632f3f cryptodisk: Rename total_length field in
grub_cryptodisk_t to total_sectors.
@@ grub-core/disk/luks.c
@@ grub-core/disk/luks.c: configure_ciphers (grub_disk_t disk, const char
*check_uuid,
newdev->offset = grub_be_to_cpu32 (header.payloadOffset);
newdev->source_disk = NULL;
- newdev->log_sector_size = LUKS1_LOG_SECTOR_SIZE;
+ newdev->log_sector_size = 9;
- newdev->total_length = grub_disk_get_size (disk) - newdev->offset;
+ newdev->total_sectors = grub_disk_get_size (disk) - newdev->offset;
grub_memcpy (newdev->uuid, uuid, sizeof (uuid));
@@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk,
crypt->log_sector_size = sizeof (unsigned int) * 8
- __builtin_clz ((unsigned int) segment.sector_size) - 1;
if (grub_strcmp (segment.size, "dynamic") == 0)
-- crypt->total_length = (grub_disk_get_size (disk) >>
(crypt->log_sector_size - disk->log_sector_size))
-+ crypt->total_sectors = (grub_disk_get_size (disk) >>
(crypt->log_sector_size - disk->log_sector_size))
- - crypt->offset;
+- crypt->total_length = grub_disk_get_size (disk) - crypt->offset;
++ crypt->total_sectors = grub_disk_get_size (disk) - crypt->offset;
else
-- crypt->total_length = grub_strtoull (segment.size, NULL, 10) >>
crypt->log_sector_size;
-+ crypt->total_sectors = grub_strtoull (segment.size, NULL, 10) >>
crypt->log_sector_size;
+- crypt->total_length = grub_strtoull (segment.size, NULL, 10);
++ crypt->total_sectors = grub_strtoull (segment.size, NULL, 10);
ret = luks2_decrypt_key (candidate_key, disk, crypt, &keyslot,
(const grub_uint8_t *) passphrase, grub_strlen
(passphrase));
9: 74c6232c9 ! 3: d79204f6c 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
- newdev->offset = grub_be_to_cpu32 (header.payloadOffset);
+ newdev->offset_sectors = grub_be_to_cpu32 (header.payloadOffset);
newdev->source_disk = NULL;
- newdev->log_sector_size = LUKS1_LOG_SECTOR_SIZE;
+ newdev->log_sector_size = 9;
- newdev->total_sectors = grub_disk_get_size (disk) - newdev->offset;
+ newdev->total_sectors = grub_disk_get_size (disk) -
newdev->offset_sectors;
grub_memcpy (newdev->uuid, uuid, sizeof (uuid));
@@ 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.slot_key);
+ grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_SIZE"\n", i);
/* 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,
crypt->log_sector_size = sizeof (unsigned int) * 8
- __builtin_clz ((unsigned int) segment.sector_size) - 1;
if (grub_strcmp (segment.size, "dynamic") == 0)
- crypt->total_sectors = (grub_disk_get_size (disk) >>
(crypt->log_sector_size - disk->log_sector_size))
-- - crypt->offset;
-+ - crypt->offset_sectors;
+- crypt->total_sectors = grub_disk_get_size (disk) - crypt->offset;
++ crypt->total_sectors = grub_disk_get_size (disk) -
crypt->offset_sectors;
else
- crypt->total_sectors = grub_strtoull (segment.size, NULL, 10) >>
crypt->log_sector_size;
+ crypt->total_sectors = grub_strtoull (segment.size, NULL, 10);
## include/grub/cryptodisk.h ##
10: 738fe0139 ! 4: cceded198 luks2: Rename source disk variabled named
'disk' to 'source' as in luks.c.
@@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk,
crypt->log_sector_size = sizeof (unsigned int) * 8
- __builtin_clz ((unsigned int) segment.sector_size) - 1;
if (grub_strcmp (segment.size, "dynamic") == 0)
-- crypt->total_sectors = (grub_disk_get_size (disk) >>
(crypt->log_sector_size - disk->log_sector_size))
-+ crypt->total_sectors = (grub_disk_get_size (source) >>
(crypt->log_sector_size - source->log_sector_size))
- - crypt->offset_sectors;
+- crypt->total_sectors = grub_disk_get_size (disk) -
crypt->offset_sectors;
++ crypt->total_sectors = grub_disk_get_size (source) -
crypt->offset_sectors;
else
- crypt->total_sectors = grub_strtoull (segment.size, NULL, 10) >>
crypt->log_sector_size;
+ crypt->total_sectors = grub_strtoull (segment.size, NULL, 10);
- ret = luks2_decrypt_key (candidate_key, disk, crypt, &keyslot,
+ ret = luks2_decrypt_key (candidate_key, source, crypt, &keyslot,
-: --------- > 5: badbd93f8 types: Define GRUB_CHAR_BIT based on compiler
macro instead of using literal.
1: 80dd653c9 ! 6: 4839ead9b luks2: Fix use of incorrect index and some
grub_error() messages.
@@ Metadata
Author: Glenn Washburn <development@efficientek.com>
## Commit message ##
- luks2: Fix use of incorrect index and some grub_error() messages.
+ luks2: Use correct index variable when looping in luks2_get_keyslot.
- When looping over the digests and segments, the loop variable is j,
but the
- variable i is used to index in the the digests and segments json
array. The
- variable i is the keyslot index. Similarly, there are several
grub_error()
- statements using the wrong index in constructing the error string.
+ The loop variable j should be used to index the digests and segments
json
+ array, instead of the variable i, which is the keyslot index.
## grub-core/disk/luks2.c ##
@@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k,
grub_luks2_digest_t *d, grub_luks2_s
@@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k,
grub_luks2_d
if ((d->keyslots & (1 << idx)))
break;
- }
- if (j == size)
-- return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot
%"PRIuGRUB_SIZE);
-+ return grub_error (GRUB_ERR_FILE_NOT_FOUND, "No digest for keyslot
%"PRIuGRUB_SIZE, i);
-
- /* Get segment that matches the digest. */
- if (grub_json_getvalue (&segments, root, "segments") ||
- grub_json_getsize (&size, &segments))
+@@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k,
grub_luks2_digest_t *d, grub_luks2_s
return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not get segments");
-- for (j = 0; j < size; j++)
-+ for (i = j, j = 0; j < size; j++)
+ for (j = 0; j < size; j++)
{
- if (grub_json_getchild (&segment, &segments, i) ||
+ if (grub_json_getchild (&segment, &segments, j) ||
@@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k,
grub_luks2_d
if ((d->segments & (1 << idx)))
break;
- }
- if (j == 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_SIZE, i);
-
- return GRUB_ERR_NONE;
- }
2: e4cf8ab40 < -: --------- luks2: Improve readability in luks2_get_keyslot.
-: --------- > 7: 21b745c11 luks2: Rename variable i to keyslot_idx in
luks2_get_keyslot.
-: --------- > 8: 21a3962b9 luks2: Rename index variable j to i.
-: --------- > 9: b3b361c49 luks2: Split idx into three variables:
keyslot_key, digest_key, segment_key.
-: --------- > 10: 8ac7af919 luks2: Improve error messages in
luks2_get_keyslot.
3: 1f65a04e0 ! 11: 31b9f0b83 luks2: Use more intuitive keyslot key instead
of index when naming keyslot.
@@ Commit message
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
+ 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.
@@ grub-core/disk/luks2.c: typedef struct grub_luks2_header
grub_luks2_header_t;
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-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)))
++ d->slot_key = digest_key;
+ if ((d->keyslots & (1 << k->slot_key)))
break;
}
@@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k,
grub_luks2_d
/* 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,
+@@ grub-core/disk/luks2.c: luks2_get_keyslot (grub_luks2_keyslot_t *k,
grub_luks2_digest_t *d, grub_luks2_s
+ luks2_parse_segment (s, &segment))
+ 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)))
+ break;
+ }
+@@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source,
if (keyslot.priority == 0)
{
@@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk,
+ 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,
+ crypt->offset_sectors = grub_divmod64 (segment.offset,
segment.sector_size, NULL);
+@@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source,
(const grub_uint8_t *) passphrase, grub_strlen
(passphrase));
if (ret)
{
@@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk,
continue;
}
-@@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk,
+@@ grub-core/disk/luks2.c: 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.
*/
-: --------- > 12: 87bafa5d9 cryptodisk: Replace some literals with
constants in grub_cryptodisk_endecrypt.
4: fcb72f70d ! 13: 1bc8c867c luks2: grub_cryptodisk_t->total_length is the
max number of device native sectors
@@ grub-core/disk/luks2.c: luks2_decrypt_key (grub_uint8_t *out_key,
const gcry_md_spec_t *hash;
gcry_err_code_t gcry_ret;
grub_err_t ret;
-@@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk,
+@@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t source,
crypt->log_sector_size = sizeof (unsigned int) * 8
- __builtin_clz ((unsigned int) segment.sector_size) - 1;
if (grub_strcmp (segment.size, "dynamic") == 0)
-- crypt->total_length = grub_disk_get_size (disk) - crypt->offset;
-+ crypt->total_length = (grub_disk_get_size (disk) >>
(crypt->log_sector_size - disk->log_sector_size))
-+ - crypt->offset;
+- crypt->total_sectors = grub_disk_get_size (source) -
crypt->offset_sectors;
++ crypt->total_sectors = (grub_disk_get_size (source) >>
(crypt->log_sector_size - source->log_sector_size))
++ - crypt->offset_sectors;
else
-- crypt->total_length = grub_strtoull (segment.size, NULL, 10);
-+ crypt->total_length = grub_strtoull (segment.size, NULL, 10) >>
crypt->log_sector_size;
+- crypt->total_sectors = grub_strtoull (segment.size, NULL, 10);
++ crypt->total_sectors = grub_strtoull (segment.size, NULL, 10) >>
crypt->log_sector_size;
- ret = luks2_decrypt_key (candidate_key, disk, crypt, &keyslot,
+ ret = luks2_decrypt_key (candidate_key, source, crypt, &keyslot,
(const grub_uint8_t *) passphrase, grub_strlen
(passphrase));
6: 6fe26ba56 ! 14: c3b6adaeb cryptodisk: Properly handle non-512 byte sized
sectors.
@@ 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 *
- iv[1] = grub_cpu_to_le32 (sector >> 32);
- /* FALLTHROUGH */
case GRUB_CRYPTODISK_MODE_IV_PLAIN:
-- iv[0] = grub_cpu_to_le32 (sector & 0xFFFFFFFF);
+- iv[0] = grub_cpu_to_le32 (sector & GRUB_TYPE_U_MAX (iv[0]));
+ /*
+ * The IV is a 32 or 64 bit value of the dm-crypt native sector
+ * number. If using 32 bit IV mode, zero out the most significant
+ * 32 bits.
+ */
+ {
-+ grub_uint64_t *iv64 = (grub_uint64_t *)iv;
++ grub_uint64_t *iv64 = (grub_uint64_t *) iv;
+ *iv64 = grub_cpu_to_le64 (sector << (log_sector_size
+ -
GRUB_CRYPTODISK_IV_LOG_SIZE));
+ if (dev->mode_iv == GRUB_CRYPTODISK_MODE_IV_PLAIN)
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_endecrypt (struct
grub_cryptodisk *
+ }
break;
case GRUB_CRYPTODISK_MODE_IV_BYTECOUNT64:
-- iv[1] = grub_cpu_to_le32 (sector >> (32 - dev->log_sector_size));
++ /* The IV is the 64 bit byte offset of the sector. */
+ iv[1] = grub_cpu_to_le32 (sector >> (GRUB_TYPE_BITS (iv[1])
+- - dev->log_sector_size));
- iv[0] = grub_cpu_to_le32 ((sector << dev->log_sector_size)
-+ iv[1] = grub_cpu_to_le32 (sector >> (32 - log_sector_size));
++ - log_sector_size));
+ iv[0] = grub_cpu_to_le32 ((sector << log_sector_size)
- & 0xFFFFFFFF);
+ & GRUB_TYPE_U_MAX (iv[0]));
break;
case GRUB_CRYPTODISK_MODE_IV_BENBI:
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_endecrypt (struct
grub_cryptodisk *dev,
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_write (grub_disk_t disk,
grub_disk_
## grub-core/disk/luks.c ##
@@ grub-core/disk/luks.c: configure_ciphers (grub_disk_t disk, const char
*check_uuid,
return NULL;
- newdev->offset = grub_be_to_cpu32 (header.payloadOffset);
+ newdev->offset_sectors = grub_be_to_cpu32 (header.payloadOffset);
newdev->source_disk = NULL;
- newdev->log_sector_size = 9;
-+ newdev->log_sector_size = LUKS1_LOG_SECTOR_SIZE;
- newdev->total_length = grub_disk_get_size (disk) - newdev->offset;
++ newdev->log_sector_size = GRUB_LUKS1_LOG_SECTOR_SIZE;
+ newdev->total_sectors = grub_disk_get_size (disk) -
newdev->offset_sectors;
grub_memcpy (newdev->uuid, uuid, sizeof (uuid));
newdev->modname = "luks";
@@ grub-core/disk/luks.c: luks_recover_key (grub_disk_t source,
@@ grub-core/disk/luks.c: luks_recover_key (grub_disk_t source,
- gcry_err = grub_cryptodisk_decrypt (dev, split_key, length, 0);
+ gcry_err = grub_cryptodisk_decrypt (dev, split_key, length, 0,
-+ LUKS1_LOG_SECTOR_SIZE);
++ GRUB_LUKS1_LOG_SECTOR_SIZE);
if (gcry_err)
{
grub_free (split_key);
@@ grub-core/disk/luks2.c: luks2_decrypt_key (grub_uint8_t *out_key,
+ * regardless of encrypted data sector size.
+ */
+ gcry_ret = grub_cryptodisk_decrypt (crypt, split_key, k->area.size, 0,
-+ LUKS1_LOG_SECTOR_SIZE);
++ GRUB_LUKS1_LOG_SECTOR_SIZE);
if (gcry_ret)
{
ret = grub_crypto_gcry_error (gcry_ret);
@@ include/grub/cryptodisk.h: typedef enum
#define GRUB_CRYPTODISK_MAX_UUID_LENGTH 71
+/* LUKS1 specification defines the block size to always be 512 bytes. */
-+#define LUKS1_LOG_SECTOR_SIZE 9
++#define GRUB_LUKS1_LOG_SECTOR_SIZE 9
+
+/* By default dm-crypt increments the IV every 512 bytes. */
+#define GRUB_CRYPTODISK_IV_LOG_SIZE 9
7: 3918a9013 < -: --------- cryptodisk: Replace some literals with
constants in grub_cryptodisk_endecrypt.
-: --------- > 15: f0758a06f luks2: Better error handling when setting up
the cryptodisk.
-: --------- > 16: 6ef5c9965 luks2: Error check segment.sector_size.
--
2.27.0
- [PATCH v4 00/15] Cryptodisk fixes for v2.06 redux,
Glenn Washburn <=