Re: [PATCH 3/7] cryptomount luks allow multiple passphrase attempts

[John Lane]

On 17/03/18 11:10, TJ wrote:
> On 14/03/18 09:45, John Lane wrote:
>> --- a/grub-core/disk/luks.c
>> +++ b/grub-core/disk/luks.c
>> @@ -321,10 +321,10 @@ configure_ciphers (grub_disk_t disk, const char 
>> *check_uuid,
>>  
>>  static grub_err_t
>>  luks_recover_key (grub_disk_t source,
>> -              grub_cryptodisk_t dev,
>> -              grub_file_t hdr,
>> -              grub_uint8_t *keyfile_bytes,
>> -              grub_size_t keyfile_bytes_size)
>> +                  grub_cryptodisk_t dev,
>> +                  grub_file_t hdr,
>> +                  grub_uint8_t *keyfile_bytes,
>> +                  grub_size_t keyfile_bytes_size)
> 
> ---8-<--- snip
> 
> Much of this patch is moving existing code around, could it be
> refactored to avoid that so as to make the new code stand out?
> 

The code that was moved was just indented into a while loop.
Most of it pre-existed prior to my patches (#357-464), a small part was
added by patch#2. I'm not sure how I would refactor it - any change
would result in 100-ish lines changing position and/or indent and would
lead to a similarly sized patch.

The way the patch presents the changes is confusing but I am not sure
how to control that.

If it helps, all this patch did was add a while loop around the
passphrase reading code to allow the user 3 attempts. i.e

  while (attempts)
    {

      <existing code which returns if successful>

      grub_printf_ (N_("Failed to decrypt master key.\n"));
      if (--attempts) grub_printf_ (N_("%u attempt%s remaining.\n"),
attempts,
                                    (attempts==1) ? "" : "s");
    }

    <existing code returns access denied>