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
-- strict thread matches above, loose matches on Subject: below --
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox