Re: [PATCH 1/2] luks2: Improve error reporting when decrypting/verifying key

[Daniel Kiper]

On Tue, Apr 07, 2020 at 06:02:23PM +0200, Patrick Steinhardt wrote:
> While we already set up error messages in both `luks2_verify_key()` and
> `luks2_decrypt_key()`, we do not ever print them. This makes it really
> hard to discover why a given key actually failed to decrypt a disk.
>
> Improve this by including the error message in the user-visible output.
> ---
>  grub-core/disk/luks2.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index 65c4f0aac..58ac7bae1 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -487,7 +487,7 @@ luks2_decrypt_key (grub_uint8_t *out_key,
>    ret = grub_disk_read (disk, 0, k->area.offset, k->area.size, split_key);
>    if (ret)
>      {
> -      grub_dprintf ("luks2", "Read error: %s\n", grub_errmsg);
> +      grub_error (GRUB_ERR_IO, "luks2", "Read error: %s\n", grub_errmsg);

I think that you should drop "luks2" here.

>        goto err;
>      }
>
> @@ -610,14 +610,16 @@ luks2_recover_key (grub_disk_t disk,
>                              (const grub_uint8_t *) passphrase, grub_strlen 
> (passphrase));
>        if (ret)
>       {
> -       grub_dprintf ("luks2", "Decryption with keyslot %"PRIuGRUB_SIZE" 
> failed\n", i);
> +       grub_dprintf ("luks2", "Decryption with keyslot %"PRIuGRUB_SIZE" 
> failed: %s\n",
> +                     i, grub_errmsg);
>         continue;
>       }
>
>        ret = luks2_verify_key (&digest, candidate_key, keyslot.key_size);
>        if (ret)
>       {
> -       grub_dprintf ("luks2", "Could not open keyslot %"PRIuGRUB_SIZE"\n", 
> i);
> +       grub_dprintf ("luks2", "Could not open keyslot %"PRIuGRUB_SIZE": 
> %s\n",
> +                     i, grub_errmsg);

This messages will be printed only if debugging is enabled. Is it what
you expect?

Daniel