Re: [PATCH v2 0/6] Support Argon2 KDF in LUKS2

[Leif Lindholm]

Hi Patrick,

On Thu, Feb 20, 2020 at 19:00:48 +0100, Patrick Steinhardt wrote:
> this is the second version of my patchset to add support for Argon2
> encryption keys for LUKS2.
> 
> The most important change is that I've now verbosely imported the argon2
> code from the official reference implementation instead of from the
> cryptsetup project. The diff between both isn't that big in the end, and
> including from crypsetup's upstream seems a bit cleaner to me. There
> were several transformations required to use GRUB's types and functions
> as well as stripping of unused stuff, which I've now documented the dev
> manual. This also fixes my previously mistaken license headers.
> 
> One thing I'm not sure about here is whether it's fine to declare the
> argon2 mod's license as GPLv3. The code is licensed under CC0/Apache
> 2.0, where the latter is compatible with GPLv3. But I don't know whether
> it's legit to just say "Yeah, this mod is a GPLv3 one".
> 
> I didn't address the comment made by Leif yet with regards to grabbing
> memory. I ain't got much of a clue of GRUB's memory subsystem, so I'd
> gladly accept help there. Otherwise I'll have to dig a bit deeper.

That's fair enough. I think we could do something halfway clever to
resolve that, or we could do something quick and simple, but either
would be better than moving to reserving 50%.

So could you reply to my email on that thread with some info with
regards to the specific memory requirements, and whether they are
precise or "this much seems to always work"?

Regards,

Leif

> The range diff compared to the previous version of this patch set is
> attached to this mail.
> 
> Patrick
> 
> 
> Patrick Steinhardt (6):
>   efi: Allocate half of available memory by default
>   types.h: add UINT-related macros needed for Argon2
>   argon2: Import Argon2 from cryptsetup
>   luks2: Add missing newline to debug message
>   luks2: Discern Argon2i and Argon2id
>   luks2: Support key derival via Argon2
> 
>  Makefile.util.def                             |   6 +-
>  docs/grub-dev.texi                            |  64 +++
>  grub-core/Makefile.core.def                   |  10 +-
>  grub-core/disk/luks2.c                        |  28 +-
>  grub-core/kern/efi/mm.c                       |   4 +-
>  grub-core/lib/argon2/argon2.c                 | 232 ++++++++
>  grub-core/lib/argon2/argon2.h                 | 264 +++++++++
>  grub-core/lib/argon2/blake2/blake2-impl.h     | 151 +++++
>  grub-core/lib/argon2/blake2/blake2.h          |  89 +++
>  grub-core/lib/argon2/blake2/blake2b.c         | 388 +++++++++++++
>  .../lib/argon2/blake2/blamka-round-ref.h      |  56 ++
>  grub-core/lib/argon2/core.c                   | 525 ++++++++++++++++++
>  grub-core/lib/argon2/core.h                   | 228 ++++++++
>  grub-core/lib/argon2/ref.c                    | 190 +++++++
>  include/grub/types.h                          |   8 +
>  15 files changed, 2231 insertions(+), 12 deletions(-)
>  create mode 100644 grub-core/lib/argon2/argon2.c
>  create mode 100644 grub-core/lib/argon2/argon2.h
>  create mode 100644 grub-core/lib/argon2/blake2/blake2-impl.h
>  create mode 100644 grub-core/lib/argon2/blake2/blake2.h
>  create mode 100644 grub-core/lib/argon2/blake2/blake2b.c
>  create mode 100644 grub-core/lib/argon2/blake2/blamka-round-ref.h
>  create mode 100644 grub-core/lib/argon2/core.c
>  create mode 100644 grub-core/lib/argon2/core.h
>  create mode 100644 grub-core/lib/argon2/ref.c
> 
> Range-diff against v1:
> 1:  53cdfdc27 = 1:  15bdf830e efi: Allocate half of available memory by 
> default
> 2:  c55946ca5 < -:  --------- argon2: Import Argon2 from cryptsetup
> -:  --------- > 2:  e81db7d95 types.h: add UINT-related macros needed for 
> Argon2
> -:  --------- > 3:  50aff9670 argon2: Import Argon2 from cryptsetup
> 3:  c17cd2197 ! 4:  af3f85665 disk: luks2: Add missing newline to debug 
> message
>     @@ Metadata
>      Author: Patrick Steinhardt <address@hidden>
>      
>       ## Commit message ##
>     -    disk: luks2: Add missing newline to debug message
>     +    luks2: Add missing newline to debug message
>      
>          The debug message printed when decryption with a keyslot fails is
>          missing its trailing newline. Add it to avoid mangling it with
>          subsequent output.
>      
>          Signed-off-by: Patrick Steinhardt <address@hidden>
>     +    Reviewed-by: Daniel Kiper <address@hidden>
>      
>       ## grub-core/disk/luks2.c ##
>      @@ grub-core/disk/luks2.c: luks2_recover_key (grub_disk_t disk,
> 4:  390728cea ! 5:  89abe827b disk: luks2: Discern Argon2i and Argon2id
>     @@ Metadata
>      Author: Patrick Steinhardt <address@hidden>
>      
>       ## Commit message ##
>     -    disk: luks2: Discern Argon2i and Argon2id
>     +    luks2: Discern Argon2i and Argon2id
>      
>          While GRUB is already able to parse both Argon2i and Argon2id 
> parameters
>          from the LUKS2 header, it doesn't discern both types. This commit
> 5:  ec4389627 ! 6:  70a354e0b disk: luks2: Support key derival via Argon2
>     @@ Metadata
>      Author: Patrick Steinhardt <address@hidden>
>      
>       ## Commit message ##
>     -    disk: luks2: Support key derival via Argon2
>     +    luks2: Support key derival via Argon2
>      
>          One addition with LUKS2 was support of the key derival function 
> Argon2
>          in addition to the previously supported PBKDF2 algortihm. In order to
>     @@ Makefile.util.def: library = {
>         common = grub-core/kern/partition.c;
>         common = grub-core/lib/crypto.c;
>      +  common = grub-core/lib/argon2/argon2.c;
>     ++  common = grub-core/lib/argon2/core.c;
>     ++  common = grub-core/lib/argon2/ref.c;
>      +  common = grub-core/lib/argon2/blake2/blake2b.c;
>         common = grub-core/lib/json/json.c;
>         common = grub-core/disk/luks.c;
>     @@ grub-core/disk/luks2.c: luks2_decrypt_key (grub_uint8_t *out_key,
>             case LUKS2_KDF_TYPE_ARGON2ID:
>      -        ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Argon2 not 
> supported");
>      -        goto err;
>     -+        ret = grub_crypto_argon2 (passphrase, passphraselen, salt, 
> saltlen,
>     -+                                  k->kdf.u.argon2.time, 
> k->kdf.u.argon2.memory, k->kdf.u.argon2.cpus,
>     -+                                  k->kdf.type == LUKS2_KDF_TYPE_ARGON2I 
> ? GRUB_ARGON2_I : GRUB_ARGON2_ID,
>     -+                                  GRUB_ARGON2_VERSION_NUMBER,
>     -+                                  area_key, k->area.key_size);
>     ++        ret = argon2_hash (k->kdf.u.argon2.time, 
> k->kdf.u.argon2.memory, k->kdf.u.argon2.cpus,
>     ++                           passphrase, passphraselen, salt, saltlen, 
> area_key, k->area.key_size,
>     ++                           k->kdf.type == LUKS2_KDF_TYPE_ARGON2I ? 
> Argon2_i : Argon2_id,
>     ++                           ARGON2_VERSION_NUMBER);
>      +        if (ret)
>      +          {
>     -+            grub_dprintf ("luks2", "Argon2 failed: %s\n", grub_errmsg);
>     ++            grub_dprintf ("luks2", "Argon2 failed: %s\n", 
> argon2_error_message (ret));
>      +            goto err;
>      +          }
>      +        break;
> -- 
> 2.25.1
>