[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v3 0/9] Cryptodisk fixes for v2.06
From: |
Patrick Steinhardt |
Subject: |
[PATCH v3 0/9] Cryptodisk fixes for v2.06 |
Date: |
Mon, 7 Sep 2020 17:27:27 +0200 |
Hi,
this is the third version of this patchset, collecting various fixes for
LUKS2/cryptodisk for the upcoming release of GRUB v2.06.
Besides my Reviewed-by tag, the only thing that changed is the final
patch by Glenn. Quoting him:
> The main difference with this patch is that sector_size is renamed to
> log_sector_size, grub has enough inaccurate or misleading names.
> Additionally, rename LOG_SECTOR_SIZE to LUKS_LOG_SECTOR_SIZE and
> CRYPT_LOG_SECTOR_SIZE to GRUB_CRYPTODISK_IV_LOG_SIZE and moved to
> cryptodisk.h. Also a comment was reworded for clarity.
The range-diff against v2 is attached below.
Patrick
Glenn Washburn (6):
luks2: Fix use of incorrect index and some error messages
luks2: grub_cryptodisk_t->total_length is the max number of device
native sectors
cryptodisk: Unregister cryptomount command when removing module
cryptodisk: Fix incorrect calculation of start sector
cryptodisk: Fix cipher IV mode 'plain64' always being set as 'plain'
cryptodisk: Properly handle non-512 byte sized sectors
Patrick Steinhardt (3):
json: Remove invalid typedef redefinition
luks: Fix out-of-bounds copy of UUID
luks2: Improve error reporting when decrypting/verifying key
grub-core/disk/cryptodisk.c | 60 +++++++++++++++++++------------------
grub-core/disk/luks.c | 8 +++--
grub-core/disk/luks2.c | 55 ++++++++++++++++++++--------------
grub-core/lib/json/json.h | 9 +++---
include/grub/cryptodisk.h | 9 +++++-
include/grub/disk.h | 7 +++++
6 files changed, 88 insertions(+), 60 deletions(-)
Range-diff against v2:
1: ee402de4d = 1: ee402de4d json: Remove invalid typedef redefinition
2: 5ecb9a4eb = 2: 5ecb9a4eb luks: Fix out-of-bounds copy of UUID
3: f8da5b4b4 = 3: f8da5b4b4 luks2: Fix use of incorrect index and some
error messages
4: 150491a07 ! 4: efbf789e2 luks2: grub_cryptodisk_t->total_length is the
max number of device native sectors
@@ Commit message
segment.size is in bytes which need to be converted to cryptodisk
sectors.
Signed-off-by: Glenn Washburn <development@efficientek.com>
+ Reviewed-by: Patrick Steinhardt <ps@pks.im>
## grub-core/disk/luks2.c ##
@@ grub-core/disk/luks2.c: luks2_decrypt_key (grub_uint8_t *out_key,
5: 7dbfad568 = 5: eb4198a01 luks2: Improve error reporting when
decrypting/verifying key
6: dbf25a0ae = 6: 7ef38470b cryptodisk: Unregister cryptomount command when
removing module
7: 4ee7f8774 = 7: a9765c0f4 cryptodisk: Fix incorrect calculation of start
sector
8: 4aecb174b = 8: 5497b02cc cryptodisk: Fix cipher IV mode 'plain64' always
being set as 'plain'
9: f520aecfa ! 9: 81b8a7f91 cryptodisk: Properly handle non-512 byte sized
sectors
@@ Commit message
Reviewed-by: Patrick Steinhardt <ps@pks.im>
## grub-core/disk/cryptodisk.c ##
-@@
-
- GRUB_MOD_LICENSE ("GPLv3+");
-
-+/* Internally encrypted sectors are 512 bytes regardless of what the
cryptodisk is */
-+#define CRYPT_LOG_SECTOR_SIZE 9
-+
- grub_cryptodisk_dev_t grub_cryptodisk_list;
-
- static const struct grub_arg_option options[] =
@@ grub-core/disk/cryptodisk.c: lrw_xor (const struct lrw_sector *sec,
static gcry_err_code_t
grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev,
grub_uint8_t * data, grub_size_t len,
- grub_disk_addr_t sector, int do_encrypt)
-+ grub_disk_addr_t sector, grub_size_t sector_size,
++ grub_disk_addr_t sector, grub_size_t log_sector_size,
+ int do_encrypt)
{
grub_size_t i;
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_endecrypt (struct
grub_cryptodisk *
: grub_crypto_ecb_decrypt (dev->cipher, data, data, len));
- for (i = 0; i < len; i += (1U << dev->log_sector_size))
-+ for (i = 0; i < len; i += (1U << sector_size))
++ for (i = 0; i < len; i += (1U << log_sector_size))
{
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_uint64_t iv_calc;
++ grub_uint64_t iv_calc = 0;
if (dev->rekey)
{
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_endecrypt (struct
grub_cryptodisk *
return GPG_ERR_OUT_OF_MEMORY;
- tmp = grub_cpu_to_le64 (sector << dev->log_sector_size);
-+ tmp = grub_cpu_to_le64 (sector << sector_size);
++ tmp = grub_cpu_to_le64 (sector << log_sector_size);
dev->iv_hash->init (ctx);
dev->iv_hash->write (ctx, dev->iv_prefix, dev->iv_prefix_len);
dev->iv_hash->write (ctx, &tmp, sizeof (tmp));
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_endecrypt (struct
grub_cryptodisk *
break;
case GRUB_CRYPTODISK_MODE_IV_PLAIN64:
- iv[1] = grub_cpu_to_le32 (sector >> 32);
-+ iv_calc = sector << (sector_size - CRYPT_LOG_SECTOR_SIZE);
++ iv_calc = sector << (log_sector_size - GRUB_CRYPTODISK_IV_LOG_SIZE);
+ iv[1] = grub_cpu_to_le32 (iv_calc >> 32);
/* FALLTHROUGH */
case GRUB_CRYPTODISK_MODE_IV_PLAIN:
- iv[0] = grub_cpu_to_le32 (sector & 0xFFFFFFFF);
-+ iv_calc = sector << (sector_size - CRYPT_LOG_SECTOR_SIZE);
++ iv_calc = sector << (log_sector_size - GRUB_CRYPTODISK_IV_LOG_SIZE);
+ iv[0] = grub_cpu_to_le32 (iv_calc & 0xFFFFFFFF);
break;
case GRUB_CRYPTODISK_MODE_IV_BYTECOUNT64:
- iv[1] = grub_cpu_to_le32 (sector >> (32 - dev->log_sector_size));
- iv[0] = grub_cpu_to_le32 ((sector << dev->log_sector_size)
-+ iv[1] = grub_cpu_to_le32 (sector >> (32 - sector_size));
-+ iv[0] = grub_cpu_to_le32 ((sector << sector_size)
++ iv[1] = grub_cpu_to_le32 (sector >> (32 - log_sector_size));
++ iv[0] = grub_cpu_to_le32 ((sector << log_sector_size)
& 0xFFFFFFFF);
break;
case GRUB_CRYPTODISK_MODE_IV_BENBI:
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_endecrypt (struct
grub_cryptodisk *
if (do_encrypt)
err = grub_crypto_cbc_encrypt (dev->cipher, data + i, data + i,
- (1U << dev->log_sector_size), iv);
-+ (1U << sector_size), iv);
++ (1U << log_sector_size), iv);
else
err = grub_crypto_cbc_decrypt (dev->cipher, data + i, data + i,
- (1U << dev->log_sector_size), iv);
-+ (1U << sector_size), iv);
++ (1U << log_sector_size), iv);
if (err)
return err;
break;
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_endecrypt (struct
grub_cryptodisk *
if (do_encrypt)
err = grub_crypto_pcbc_encrypt (dev->cipher, data + i, data + i,
- (1U << dev->log_sector_size), iv);
-+ (1U << sector_size), iv);
++ (1U << log_sector_size), iv);
else
err = grub_crypto_pcbc_decrypt (dev->cipher, data + i, data + i,
- (1U << dev->log_sector_size), iv);
-+ (1U << sector_size), iv);
++ (1U << log_sector_size), iv);
if (err)
return err;
break;
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_endecrypt (struct
grub_cryptodisk *
return err;
- for (j = 0; j < (1U << dev->log_sector_size);
-+ for (j = 0; j < (1U << sector_size);
++ for (j = 0; j < (1U << log_sector_size);
j += dev->cipher->cipher->blocksize)
{
grub_crypto_xor (data + i + j, data + i + j, iv,
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_endecrypt (struct
grub_cryptodisk *
err = grub_crypto_ecb_encrypt (dev->cipher, data + i,
data + i,
- (1U << dev->log_sector_size));
-+ (1U << sector_size));
++ (1U << log_sector_size));
else
err = grub_crypto_ecb_decrypt (dev->cipher, data + i,
data + i,
- (1U << dev->log_sector_size));
-+ (1U << sector_size));
++ (1U << log_sector_size));
if (err)
return err;
lrw_xor (&sec, dev, data + i);
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_endecrypt (struct
grub_cryptodisk *
if (do_encrypt)
err = grub_crypto_ecb_encrypt (dev->cipher, data + i, data + i,
- (1U << dev->log_sector_size));
-+ (1U << sector_size));
++ (1U << log_sector_size));
else
err = grub_crypto_ecb_decrypt (dev->cipher, data + i, data + i,
- (1U << dev->log_sector_size));
-+ (1U << sector_size));
++ (1U << log_sector_size));
if (err)
return err;
break;
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_endecrypt (struct
grub_cryptodisk *
grub_cryptodisk_decrypt (struct grub_cryptodisk *dev,
grub_uint8_t * data, grub_size_t len,
- grub_disk_addr_t sector)
-+ grub_disk_addr_t sector, grub_size_t sector_size)
++ grub_disk_addr_t sector, grub_size_t log_sector_size)
{
- return grub_cryptodisk_endecrypt (dev, data, len, sector, 0);
-+ return grub_cryptodisk_endecrypt (dev, data, len, sector, sector_size,
0);
++ return grub_cryptodisk_endecrypt (dev, data, len, sector,
log_sector_size, 0);
}
grub_err_t
@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_write (grub_disk_t disk,
grub_disk_
grub_free (tmp);
## grub-core/disk/luks.c ##
-@@
- GRUB_MOD_LICENSE ("GPLv3+");
-
- #define MAX_PASSPHRASE 256
-+#define LOG_SECTOR_SIZE 9
-
- #define LUKS_KEY_ENABLED 0x00AC71F3
-
@@ 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->source_disk = NULL;
- newdev->log_sector_size = 9;
-+ newdev->log_sector_size = LOG_SECTOR_SIZE;
++ newdev->log_sector_size = LUKS_LOG_SECTOR_SIZE;
newdev->total_length = grub_disk_get_size (disk) - newdev->offset;
grub_memcpy (newdev->uuid, uuid, sizeof (uuid));
newdev->modname = "luks";
@@ 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,
LOG_SECTOR_SIZE);
++ gcry_err = grub_cryptodisk_decrypt (dev, split_key, length, 0,
++ LUKS_LOG_SECTOR_SIZE);
if (gcry_err)
{
grub_free (split_key);
@@ grub-core/disk/luks2.c: luks2_decrypt_key (grub_uint8_t *out_key,
- gcry_ret = grub_cryptodisk_decrypt (crypt, split_key, k->area.size, 0);
+ /*
-+ * The encrypted key slots are always with 512byte sectors,
-+ * regardless of encrypted data sector size
++ * The key slots area is always encrypted in 512-byte sectors,
++ * regardless of encrypted data sector size.
+ */
-+ gcry_ret = grub_cryptodisk_decrypt (crypt, split_key, k->area.size, 0,
9);
++ gcry_ret = grub_cryptodisk_decrypt (crypt, split_key, k->area.size, 0,
++ LUKS_LOG_SECTOR_SIZE);
if (gcry_ret)
{
ret = grub_crypto_gcry_error (gcry_ret);
## include/grub/cryptodisk.h ##
+@@ include/grub/cryptodisk.h: typedef enum
+
+ #define GRUB_CRYPTODISK_MAX_UUID_LENGTH 71
+
++#define LUKS_LOG_SECTOR_SIZE 9
++
++/* For the purposes of IV incrementing the sector size is 512 bytes,
unless
++ * otherwise specified.
++ */
++#define GRUB_CRYPTODISK_IV_LOG_SIZE 9
++
+ #define GRUB_CRYPTODISK_GF_LOG_SIZE 7
+ #define GRUB_CRYPTODISK_GF_SIZE (1U << GRUB_CRYPTODISK_GF_LOG_SIZE)
+ #define GRUB_CRYPTODISK_GF_LOG_BYTES (GRUB_CRYPTODISK_GF_LOG_SIZE - 3)
@@ include/grub/cryptodisk.h: grub_cryptodisk_setkey (grub_cryptodisk_t
dev,
gcry_err_code_t
grub_cryptodisk_decrypt (struct grub_cryptodisk *dev,
grub_uint8_t * data, grub_size_t len,
- grub_disk_addr_t sector);
-+ grub_disk_addr_t sector, grub_size_t sector_size);
++ grub_disk_addr_t sector, grub_size_t log_sector_size);
grub_err_t
grub_cryptodisk_insert (grub_cryptodisk_t newdev, const char *name,
grub_disk_t source);
--
2.28.0
signature.asc
Description: PGP signature