On Mon, Sep 07, 2020 at 05:28:08PM +0200, Patrick Steinhardt wrote:
> From: Glenn Washburn <developm...@efficientek.com>
>
> 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
> that when using a larger sector size, the IV is incremented every sector.
> However, the amount the IV is incremented is the number of 512 byte blocks
> in a sector (ie 8 for 4K sectors). Confusingly the IV does not corespond to
> the number of, for example, 4K sectors. So each cipher block in the fifth
> 4K sector will be encrypted with an IV equal to 32, as opposed to 32-39 for
s/32-39/32-9/?
> each sequential 512 byte block or an IV of 4 for each cipher block in the
> sector.
>
> There are some encryption utilities which do it the intuitive way and have
> the IV equal to the sector number regardless of sector size (ie. the fifth
> sector would have an IV of 4 for each cipher block). And this is supported
> by dm-crypt with the iv_large_sectors option and also cryptsetup as of 2.3.3
> with the --iv-large-sectors, though not with LUKS headers (only with --type
> plain). However, support for this has not been included as grub does not
> support plain devices right now.
>
> One gotcha here is that the encrypted split keys are encrypted with a hard-
> coded 512-byte sector size. So even if your data is encrypted with 4K sector
> sizes, the split key encrypted area must be decrypted with a block size of
> 512 (ie the IV increments every 512 bytes). This made these changes less
> aestetically pleasing than desired.
>
> Signed-off-by: Glenn Washburn <developm...@efficientek.com>
> Reviewed-by: Patrick Steinhardt <p...@pks.im>
> ---
> grub-core/disk/cryptodisk.c | 44 ++++++++++++++++++++-----------------
> grub-core/disk/luks.c | 5 +++--
> grub-core/disk/luks2.c | 7 +++++-
> include/grub/cryptodisk.h | 9 +++++++-
> 4 files changed, 41 insertions(+), 24 deletions(-)
>
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index 0b63b7d96..6319f3164 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -224,7 +224,8 @@ 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 log_sector_size,
> + int do_encrypt)
> {
> grub_size_t i;
> gcry_err_code_t err;
> @@ -237,12 +238,13 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev,
> return (do_encrypt ? grub_crypto_ecb_encrypt (dev->cipher, data, data,
> len)
> : 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 << 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 = 0;
>
> if (dev->rekey)
> {
> @@ -270,7 +272,7 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev,
> if (!ctx)
> return GPG_ERR_OUT_OF_MEMORY;
>
> - tmp = grub_cpu_to_le64 (sector << dev->log_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));
> @@ -281,14 +283,16 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev,
> }
> break;
> case GRUB_CRYPTODISK_MODE_IV_PLAIN64:
> - iv[1] = grub_cpu_to_le32 (sector >> 32);
> + iv_calc = sector << (log_sector_size - GRUB_CRYPTODISK_IV_LOG_SIZE);
> + iv[1] = grub_cpu_to_le32 (iv_calc >> 32);
Why 32? Could you use a constant or add a comment here?
> /* FALLTHROUGH */
> case GRUB_CRYPTODISK_MODE_IV_PLAIN:
> - iv[0] = grub_cpu_to_le32 (sector & 0xFFFFFFFF);
> + 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));
Ditto?
> - iv[0] = grub_cpu_to_le32 ((sector << dev->log_sector_size)
> + iv[1] = grub_cpu_to_le32 (sector >> (32 - log_sector_size));
Ditto?
[...]
> diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c
> index 59702067a..2e1347b13 100644
> --- a/grub-core/disk/luks.c
> +++ b/grub-core/disk/luks.c
> @@ -124,7 +124,7 @@ 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 = LUKS_LOG_SECTOR_SIZE;
> newdev->total_length = grub_disk_get_size (disk) - newdev->offset;
> grub_memcpy (newdev->uuid, uuid, sizeof (uuid));
> newdev->modname = "luks";
> @@ -247,7 +247,8 @@ luks_recover_key (grub_disk_t source,
> return err;
> }
>
> - gcry_err = grub_cryptodisk_decrypt (dev, split_key, length, 0);
> + gcry_err = grub_cryptodisk_decrypt (dev, split_key, length, 0,
> + LUKS_LOG_SECTOR_SIZE);
> if (gcry_err)
> {
> grub_free (split_key);
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index 26e1126b1..eb64a0596 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -492,7 +492,12 @@ luks2_decrypt_key (grub_uint8_t *out_key,
> goto err;
> }
>
> - gcry_ret = grub_cryptodisk_decrypt (crypt, split_key, k->area.size, 0);
> + /*
> + * 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,
> + LUKS_LOG_SECTOR_SIZE);
s/LUKS_LOG_SECTOR_SIZE/GRUB_CRYPTODISK_IV_LOG_SIZE/?
> if (gcry_ret)
> {
> ret = grub_crypto_gcry_error (gcry_ret);
> diff --git a/include/grub/cryptodisk.h b/include/grub/cryptodisk.h
> index e1b21e785..ecb37ba43 100644
> --- a/include/grub/cryptodisk.h
> +++ b/include/grub/cryptodisk.h
> @@ -48,6 +48,13 @@ 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)
> @@ -139,7 +146,7 @@ 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 log_sector_size);
> grub_err_t
> grub_cryptodisk_insert (grub_cryptodisk_t newdev, const char *name,
> grub_disk_t source);
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
