From: Patrick Steinhardt <ps@pks.im>
To: grub-devel@gnu.org
Cc: Denis GNUtoo Carikli <GNUtoo@cyberdimension.org>,
Glenn Washburn <development@efficientek.com>,
Daniel Kiper <daniel.kiper@oracle.com>
Subject: [PATCH v3 0/9] Cryptodisk fixes for v2.06
Date: Mon, 7 Sep 2020 17:27:27 +0200 [thread overview]
Message-ID: <cover.1599492346.git.ps@pks.im> (raw)
In-Reply-To: <cover.1598179677.git.ps@pks.im>
[-- Attachment #1: Type: text/plain, Size: 12933 bytes --]
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
next prev parent reply other threads:[~2020-09-07 15:27 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-23 10:59 [PATCH 0/9] Cryptodisk fixes for v2.06 Patrick Steinhardt
2020-08-23 10:59 ` [PATCH 1/9] json: Remove invalid typedef redefinition Patrick Steinhardt
2020-08-23 10:59 ` [PATCH 2/9] luks: Fix out-of-bounds copy of UUID Patrick Steinhardt
2020-08-23 21:34 ` Denis 'GNUtoo' Carikli
2020-08-26 7:18 ` Patrick Steinhardt
2020-08-23 11:03 ` [PATCH 3/9] luks2: Fix use of incorrect index and some error messages Patrick Steinhardt
2020-08-24 6:30 ` Glenn Washburn
2020-08-24 6:33 ` Patrick Steinhardt
2020-08-23 11:03 ` [PATCH 4/9] luks2: grub_cryptodisk_t->total_length is the max number of device native sectors Patrick Steinhardt
2020-08-23 11:03 ` [PATCH 5/9] luks2: Improve error reporting when decrypting/verifying key Patrick Steinhardt
2020-08-23 11:03 ` [PATCH 6/9] cryptodisk: Unregister cryptomount command when removing module Patrick Steinhardt
2020-08-23 11:04 ` [PATCH 7/9] cryptodisk: Incorrect calculation of start sector for grub_disk_read in grub_cryptodisk_read Patrick Steinhardt
2020-08-23 11:04 ` [PATCH 8/9] cryptodisk: Fix cipher IV mode 'plain64' always being set as 'plain' Patrick Steinhardt
2020-08-23 11:04 ` [PATCH 9/9] cryptodisk: Properly handle non-512 byte sized sectors Patrick Steinhardt
2020-08-24 6:22 ` [PATCH 0/9] Cryptodisk fixes for v2.06 Glenn Washburn
2020-08-24 6:31 ` Patrick Steinhardt
2020-08-26 8:13 ` [PATCH v2 " Patrick Steinhardt
2020-08-26 8:13 ` [PATCH v2 1/9] json: Remove invalid typedef redefinition Patrick Steinhardt
2020-08-26 8:13 ` [PATCH v2 2/9] luks: Fix out-of-bounds copy of UUID Patrick Steinhardt
2020-08-26 8:13 ` [PATCH v2 3/9] luks2: Fix use of incorrect index and some error messages Patrick Steinhardt
2020-08-26 8:13 ` [PATCH v2 4/9] luks2: grub_cryptodisk_t->total_length is the max number of device native sectors Patrick Steinhardt
2020-08-26 8:13 ` [PATCH v2 5/9] luks2: Improve error reporting when decrypting/verifying key Patrick Steinhardt
2020-08-26 8:13 ` [PATCH v2 6/9] cryptodisk: Unregister cryptomount command when removing module Patrick Steinhardt
2020-08-26 23:44 ` [PATCH] cryptodisk: Incorrect calculation of sector in grub_cryptodisk_read/write Glenn Washburn
2020-08-26 23:50 ` Glenn Washburn
2020-08-28 7:12 ` Patrick Steinhardt
2020-08-26 8:13 ` [PATCH v2 7/9] cryptodisk: Fix incorrect calculation of start sector Patrick Steinhardt
2020-08-26 8:13 ` [PATCH v2 8/9] cryptodisk: Fix cipher IV mode 'plain64' always being set as 'plain' Patrick Steinhardt
2020-08-26 8:14 ` [PATCH v2 9/9] cryptodisk: Properly handle non-512 byte sized sectors Patrick Steinhardt
2020-08-31 18:43 ` Glenn Washburn
2020-09-01 15:28 ` Patrick Steinhardt
2020-09-01 23:21 ` [PATCH] " Glenn Washburn
2020-09-02 0:01 ` Glenn Washburn
2020-09-07 15:28 ` Patrick Steinhardt
2020-08-26 22:16 ` [PATCH v2 0/9] Cryptodisk fixes for v2.06 Glenn Washburn
2020-08-28 7:17 ` Patrick Steinhardt
2020-09-07 15:27 ` Patrick Steinhardt [this message]
2020-09-07 15:27 ` [PATCH v3 1/9] json: Remove invalid typedef redefinition Patrick Steinhardt
2020-09-07 15:27 ` [PATCH v3 2/9] luks: Fix out-of-bounds copy of UUID Patrick Steinhardt
2020-09-07 15:27 ` [PATCH v3 3/9] luks2: Fix use of incorrect index and some error messages Patrick Steinhardt
2020-09-08 12:58 ` Daniel Kiper
2020-09-21 6:45 ` Glenn Washburn
2020-09-21 11:24 ` Daniel Kiper
2020-09-07 15:27 ` [PATCH v3 4/9] luks2: grub_cryptodisk_t->total_length is the max number of device native sectors Patrick Steinhardt
2020-09-08 13:21 ` Daniel Kiper
2020-09-21 6:28 ` Glenn Washburn
2020-09-21 11:23 ` Daniel Kiper
2020-10-03 5:42 ` Glenn Washburn
2020-10-27 19:11 ` Daniel Kiper
2020-10-29 19:53 ` Glenn Washburn
2020-10-30 12:49 ` Daniel Kiper
2020-11-03 20:21 ` Glenn Washburn
2020-11-04 13:15 ` Daniel Kiper
2020-11-06 6:41 ` Glenn Washburn
2020-09-07 15:27 ` [PATCH v3 5/9] luks2: Improve error reporting when decrypting/verifying key Patrick Steinhardt
2020-09-07 15:27 ` [PATCH v3 6/9] cryptodisk: Unregister cryptomount command when removing module Patrick Steinhardt
2020-09-08 13:28 ` Daniel Kiper
2020-09-21 6:45 ` Glenn Washburn
2020-09-21 11:25 ` Daniel Kiper
2020-09-07 15:27 ` [PATCH v3 7/9] cryptodisk: Fix incorrect calculation of start sector Patrick Steinhardt
2020-09-07 15:28 ` [PATCH v3 8/9] cryptodisk: Fix cipher IV mode 'plain64' always being set as 'plain' Patrick Steinhardt
2020-09-08 13:42 ` Daniel Kiper
2020-09-07 15:28 ` [PATCH v3 9/9] cryptodisk: Properly handle non-512 byte sized sectors Patrick Steinhardt
2020-09-09 11:21 ` Daniel Kiper
2020-09-21 5:58 ` Glenn Washburn
2020-09-21 11:16 ` Daniel Kiper
2020-09-09 11:28 ` [PATCH v3 0/9] Cryptodisk fixes for v2.06 Daniel Kiper
2020-09-17 14:14 ` Patrick Steinhardt
2020-09-21 4:54 Glenn Washburn
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1599492346.git.ps@pks.im \
--to=ps@pks.im \
--cc=GNUtoo@cyberdimension.org \
--cc=daniel.kiper@oracle.com \
--cc=development@efficientek.com \
--cc=grub-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.