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 <developm...@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 <developm...@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 <developm...@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 <developm...@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 <developm...@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
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
