grub-devel
[Top][All Lists]
Advanced

[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

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]