Re: [Cfrg] Review of draft-irtf-cfrg-argon2-02

["Stanislav V. Smyshlyaev" <[e-mail]>]

Document: draft-irtf-cfrg-argon2-02
Reviewer: Stanislav Smyshlyaev
Review Date: 2017-06-27


Summary: Minor revision needed


The draft defines a family of Argon2 functions, Argon2i, Argon2d and
Argon2id, which are designed to be memory-hard functions for password
hashing and PoW.


The function design is reasonable and well-thought. The security analysis
is deep enough - a number of adequate adversary models are considered (see
https://www.cryptolux.org/images/0/0d/Argon2.pdf).


Nevertheless, in my opinion, more words (not in the I-D but in the cited
design reference) about comparison of results for Argon2 with most recent
results for Balloon and sCrypt would be very helpful: for instance,
considering the model and results in Eurocrypt 2017 best paper "Scrypt is
Maximally Memory-Hard", or results in the pROM model. Are there any
complete results (existing or planned for future research) in the pROM
model?


Also some (at least, formal) words should be said about security analysis
of recommended Argon2id (I understand that results for Argon2i lead to
corollaries for Argon2id - but it should be said explicitly, I think).


Another major concern is the following: I'd prefer the number of variants
to be reduced.

Now there are 3 functions with several parameters. It is great that the
Section 9.4 with recommendations to use Argon2id with t=1 and maximum
available memory has been added in -02 version - but do we need the whole
set of alternative options in the document?

It seems to me that it is not a good idea to have a large set of
parameterized functions standardized and I'd like it to be reduced in the
future RFC - for example, by getting rid of Argon2i and Argon2d, leaving
only Argon2id.


Comments:

1) General comments about notation:

- Section 2 defines “^”, but later "XOR” is used in the formulas below
(sections 3.2, 3.5). Please select one notation, and if “XOR” is chosen,
then it makes sense to use “^” instead of “**”.

- It will be better to put spaces for arithmetic operations in a unified
way. For example, put them before and after all binary operations  (/, -,
*, ^ (or XOR), …) and do not put them in case of unary operations (**).

- "Kibibytes", "GiB" and "GB" are used. It would be better to select one
measurement system.

2) Please use figure numbers and captions for the illustrations.

3) Notation in Section 2:

- Symbol “a” is used to denote integers and strings without any comments.
It will be good to clarify the type of the arguments for defined functions
and operations.

- The function extract(a, i) should be defined more clearly: e.g. which set
is the 1-st one for a?

- Such simple things as “*”, “-”, “/”, “I(j)” are defined (do we really
need it?) but there are no words about “-P->”, “->”, “ceil”. In particular,
on a first encounter one may read string “G: (X, Y) -> R = X XOR Y -P-> Q
-P-> Z -P-> Z XOR R” in an incorrect way.

4) Transformation rules (from int to str and back) are not always provided
in the draft: in Section 3.4.1 J_1, J_2 deal with strings; in Section 3.4.2
J_1, J_2 deal with integers.

Think about defining int() and str() functions to avoid repeating the same
phrases.

5) Function H is BLAKE2b function, which takes 4 arguments as an input
(d[0..dd-1], ll, kk, nn). But it takes 14 arguments in Section 3.2 and 1
argument in Section 3.3. Please, clarify it.

6) Section 3.2.  The link [I-D.saarinen-blake2] is for a version of draft,
but now there is RFC 7693.

7) Section 3.2. "Establish H_0 as the 64-bit value as shown in the figure
below."

The nearest figure is “Single-pass Argon2 with p lanes and 4 slices” - and
H_0 is described in the formula (string) below.

8) Section 3.2. "G and H' are described in later section."

The number of the section here would look better than just "later".

9) Section 3.3 specifies function H’ with one argument X, but H’ uses 3
arguments in Section 3.3. (B[i][0] = H'(H_0, 0, i)).

10) Section 3.3: "in our case H_x is BLAKE2b, which supports x between 1
and 64 inclusive"

It would be better to clarify that x is "nn" in terms of BLAKE2b.

11)

- Section 3.3. "Here integers are padded to 4 bytes and encoded in little
endian."

The question "Padded with what?" can be raised. Maybe it will be better
just to say “encoded in little-endian as 4-byte integer” .

- Section 3.3. "little-endian as 32-bit integer". "4 bytes" instead of "32
bits" looks better to me here.

12) Section 3.3. "The block indices i' and j' are determined differently
for Argon2d, Argon2i, and Argon2id."

Please, insert the link to a section that determines this.

13) In Section 3.4.2 and Section 3.5 “R” is used for different purposes

14) In Section 3.4.1.2 and Section 3.4.2 “x” is used for different purposes

15) Notation looks too cumbersome for an RFC. For some readers it can be
hard to understand the meaning of the string “J_1 in [0, 2**32) -> |R|(1 -
J_1**2 / 2**64)”.

16) Section 3.5. Maybe it would be better to use “v” instead of "\ /".

17) Section 6.3. Extra spaces in the first string.

18) Lines “Password[32]:…”, “Pre-hashing digest…", ", "Tag: 0d 64 0d f5 …"
go off the edge of the page.

19) Section 3.5. G: (X, Y) -> R = X XOR Y -P-> Q -P-> Z -P-> Z XOR R

Is the third “-P->” needed here?



However, the document is very important and, in my opinion, it should be
published after resolving the comments.


Of course I'll be happy to discuss the mentioned concerns with the authors.

Best regards,

Stanislav


2017-06-24 0:28 GMT+03:00 Russ Housley <[e-mail]>:

> I meant to say:
>
> Once the comments below are resolved, I think that the IRSG should
> publish the document.
>
> Russ
>
>
>
> > On Jun 23, 2017, at 5:19 PM, Russ Housley <[e-mail]> wrote:
> >
> > Document: draft-irtf-cfrg-argon2-02
> > Reviewer: Russ Housley
> > Review Date: 2017-06-23
> >
> > Summary: Almost Ready
> >
> > Once the comments below are resolved, I think that the ISE should
> > publish the document.
> >
> >
> > Major Concerns:
> >
> > Section 3.3: The pseudo-code includes this line:
> >
> >               V_{r+1} = H_{T-32*r}(V_{r})
> >
> > Up to this point, I thought that the Argon2 construction could work with
> > many different hash functions, but at this point I realized that Argon2
> > depends the ability of Blake2b to produce an output between 1 and 64
> > bytes.  Please add a sentence or two to the introduction so that others
> > can avoid this misunderstanding.
> >
> > Section 3.5: This section defines the compression function G.  It makes
> > use of the BLAKE2b round function P, which is explained in Section 3.6.
> > Section 3.6 defines another function G, which is different than the
> > compression function.  I realize that both G functions are discussed in
> > other documents ([I-D.saarinen-blake2] and [ARGON2]), but there ought to
> > be a way to avoid this name collision.
> >
> > Section 5: I tried to compile the code, and it does not work with gcc.
> > Is there a missing include file?
> >
> >
> > Minor Concerns:
> >
> >
> > Section 2 defines "E_f" as a variable E with subscript index f; however,
> > in Section 3.3, defines "H_x" as a hash function with x-byte output.
> > Please use a different notation for variables and functions.
> >
> > Section 5: A main function would make it easier to use this reference
> > code.  Can you include something simple?  Perhaps something that prints
> > the tags associated with the test vectors in Section 6.
> >
> >
> > Nits:
> >
> > Section 1: s/implementer oriented/implementer-oriented/
> >
> > Section 1: s/data-depending memory access/data-dependent memory access/
> >
> > Section 2: Please add a descriptions for the floor and ceil functions.
> >
> > Section 2: Please add a description for length(P), which always returns
> > 32 bits.
> >
> > Section 2 defines "a ^ b", but Sections 3.2 and 3.5 use "a XOR b".
> > Pick one.
> >
> > Most of the document uses "=" for assignment, but Section 3.6 uses "<-".
> > Please use "=" throughout.
> >
> > Several places, the document tells us that "integers are padded to 4
> > bytes and encoded in little endian".  Maybe a "uint32" function should
> > be specified to avoid repeating this phrase.
> >
> > Section 3.2: s/described in later section./described in later sections./
> >
> > Section 6.3: Please correct the formatting.
> >
> > Section 9.1: s/calls to Blake2b is needed/calls to Blake2b are needed/
> >
> > Section 9.2: s/number t of passes/the number of passes, t,/
> >
> > Section 9.3: s/for Argon2i 3 passes/for Argon2i, 3 passes/
> >
> > Section 9.3: s/for Argon2d and Argon2id 1 pass/for Argon2d and Argon2id,
> 1 pass/
> >
> >
> > Personal preference:
> >
> > Section 3.5: s/applied rowwise, and then columnwise/
> >              /applied to each row, and then to each column/
> >
> > _______________________________________________
> > Cfrg mailing list
> > [e-mail]
> > https://www.irtf.org/mailman/listinfo/cfrg
>
> _______________________________________________
> Cfrg mailing list
> [e-mail]
> https://www.irtf.org/mailman/listinfo/cfrg
>