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 + Reviewed-by: Patrick Steinhardt ## 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 ## 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