Re: [PATCH v6 6/6] disk: Implement support for LUKS2

[Daniel Kiper]

On Tue, Dec 10, 2019 at 10:26:21AM +0100, Patrick Steinhardt wrote:
> With cryptsetup 2.0, a new version of LUKS was introduced that breaks
> compatibility with the previous version due to various reasons. GRUB
> currently lacks any support for LUKS2, making it impossible to decrypt
> disks encrypted with that version. This commit implements support for
> this new format.
>
> Note that LUKS1 and LUKS2 are quite different data formats. While they
> do share the same disk signature in the first few bytes, representation
> of encryption parameters is completely different between both versions.
> While the former version one relied on a single binary header, only,
> LUKS2 uses the binary header only in order to locate the actual metadata
> which is encoded in JSON. Furthermore, the new data format is a lot more
> complex to allow for more flexible setups, like e.g. having multiple
> encrypted segments and other features that weren't previously possible.
> Because of this, it was decided that it doesn't make sense to keep both
> LUKS1 and LUKS2 support in the same module and instead to implement it
> in two different modules "luks" and "luks2".
>
> The proposed support for LUKS2 is able to make use of the metadata to
> decrypt such disks. Note though that in the current version, only the
> PBKDF2 key derival function is supported. This can mostly attributed to
> the fact that the libgcrypt library currently has no support for either
> Argon2i or Argon2id, which are the remaining KDFs supported by LUKS2. It
> wouldn't have been much of a problem to bundle those algorithms with
> GRUB itself, but it was decided against that in order to keep down the
> number of patches required for initial LUKS2 support. Adding it in the
> future would be trivial, given that the code structure is already in
> place.
>
> Signed-off-by: Patrick Steinhardt <address@hidden>

In general Reviewed-by: Daniel Kiper <address@hidden>...
However, one question below...

[...]

> +static grub_err_t
> +luks2_recover_key (grub_disk_t disk,
> +                grub_cryptodisk_t crypt)
> +{
> +  grub_uint8_t candidate_key[GRUB_CRYPTODISK_MAX_KEYLEN];
> +  char passphrase[MAX_PASSPHRASE], cipher[32];
> +  char *json_header = NULL, *part = NULL, *ptr;
> +  grub_size_t candidate_key_len = 0, i, size;
> +  grub_luks2_header_t header;
> +  grub_luks2_keyslot_t keyslot;
> +  grub_luks2_digest_t digest;
> +  grub_luks2_segment_t segment;
> +  gcry_err_code_t gcry_ret;
> +  grub_json_t *json = NULL, keyslots;
> +  grub_err_t ret;
> +
> +  ret = luks2_read_header (disk, &header);
> +  if (ret)
> +    return ret;
> +
> +  json_header = grub_zalloc (grub_be_to_cpu64 (header.hdr_size) - sizeof 
> (header));
> +  if (!json_header)
> +      return GRUB_ERR_OUT_OF_MEMORY;
> +
> +  /* Read the JSON area. */
> +  ret = grub_disk_read (disk, 0, grub_be_to_cpu64 (header.hdr_offset) + 
> sizeof (header),
> +                     grub_be_to_cpu64 (header.hdr_size) - sizeof (header), 
> json_header);
> +  if (ret)
> +      goto err;
> +
> +  ptr = grub_memchr (json_header, 0, grub_be_to_cpu64 (header.hdr_size) - 
> sizeof (header));
> +  if (!ptr)
> +    goto err;
> +
> +  ret = grub_json_parse (&json, json_header, grub_be_to_cpu64 
> (header.hdr_size));
> +  if (ret)
> +    {
> +      ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid LUKS2 JSON header");
> +      goto err;
> +    }
> +
> +  /* Get the passphrase from the user. */
> +  if (disk->partition)
> +    part = grub_partition_get_name (disk->partition);
> +  grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), disk->name,
> +             disk->partition ? "," : "", part ? : "",
> +             crypt->uuid);

Why do you use grub_printf() instead of grub_printf()?

> +  if (!grub_password_get (passphrase, MAX_PASSPHRASE))
> +    {
> +      ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not supplied");
> +      goto err;
> +    }
> +
> +  if (grub_json_getvalue (&keyslots, json, "keyslots") ||
> +      grub_json_getsize (&size, &keyslots))
> +    {
> +      ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not get keyslots");
> +      goto err;
> +    }
> +
> +  /* Try all keyslot */
> +  for (i = 0; i < size; i++)
> +    {
> +      ret = luks2_get_keyslot (&keyslot, &digest, &segment, json, i);
> +      if (ret)
> +     goto err;
> +
> +      if (keyslot.priority == 0)
> +     {
> +       grub_dprintf ("luks2", "Ignoring keyslot %"PRIuGRUB_SIZE" due to 
> priority\n", i);
> +       continue;
> +        }
> +
> +      grub_dprintf ("luks2", "Trying keyslot %"PRIuGRUB_SIZE"\n", i);
> +
> +      /* Set up disk according to keyslot's segment. */
> +      crypt->offset = segment.offset / segment.sector_size;
> +      crypt->log_sector_size = sizeof (unsigned int) * 8
> +             - __builtin_clz ((unsigned int) segment.sector_size) - 1;
> +      if (grub_strcmp (segment.size, "dynamic") == 0)
> +     crypt->total_length = grub_disk_get_size (disk) - crypt->offset;
> +      else
> +     crypt->total_length = grub_strtoull (segment.size, NULL, 10);
> +
> +      ret = luks2_decrypt_key (candidate_key, disk, crypt, &keyslot,
> +                            (const grub_uint8_t *) passphrase, grub_strlen 
> (passphrase));
> +      if (ret)
> +     {
> +       grub_dprintf ("luks2", "Decryption with keyslot %"PRIuGRUB_SIZE" 
> failed", i);
> +       continue;
> +     }
> +
> +      ret = luks2_verify_key (&digest, candidate_key, keyslot.key_size);
> +      if (ret)
> +     {
> +       grub_dprintf ("luks2", "Could not open keyslot %"PRIuGRUB_SIZE"\n", 
> i);
> +       continue;
> +     }
> +
> +      /*
> +       * TRANSLATORS: It's a cryptographic key slot: one element of an array
> +       * where each element is either empty or holds a key.
> +       */
> +      grub_printf_ (N_("Slot %"PRIuGRUB_SIZE" opened\n"), i);

Ditto?

Daniel