Re: [CFRG] IRSG review draft-irtf-cfrg-spake2-20

[Watson Ladd <[e-mail]>]

On Tue, Sep 7, 2021 at 4:41 AM Julia Hesse <[e-mail]> wrote:
>
> Dear all,
>
> below is a review of the latest SPAKE2 draft. I did not verify changes in test vectors. The // - questions were posted by Chris Wood here https://gist.github.com/chris-wood/2205c8e79b309adec5a785470d31226e
> and my * comments are about how they were addressed.
>
> Best regards,
> Julia
>
>
> // - Should we include a definition of UKS attacks inline, rather than cite draft-ietf-mmusic-sdp-uks?
>     * Resolved. Maybe substitute "over who is authenticated" by "over whom the key is shared with"? Because this is tied to the key.
>
> // - Should SPAKE2 require that the output length of Hash is at least 256-bits? (It's output is split in half to derive Ke and Ka, and we probably want those to have at least 128 bits.)
>     * Watson said he will add the requirement, but I cannot seem to find it. However, all ciphersuites use at least SHA-256 and it is very clear from the draft that the key length is half of the hash output. So I do not see the need to edit further in this regard.

I think it's worth adding "Keys MUST be at least 128 bits in length"
to the first paragraph in section 4.

>
> // - What does it mean to exchange messages symmetrically? (In the per-user M and N section)
>     * Resolved. I suggest to remove "to have a symmetric variant" - it is not needed and might cause confusion (variant of what?)

It is needed: Magic Wormhole uses this variant. And the concern about
who uses M and who uses N in cases where it isn't clear is best
addressed by using this variant.

>
> // - Beyond scalar multiplication being constant time, are there any other constant time considerations we should include?
>     * Resolved
>
> // - Why is Ke not included in the test vectors? It may be redundant, but it seems useful as an additional sanity check.
> // - There are currently no test vectors that include AAD -- should we add some?
>     * I have not verified these two.
>
> // - Why is len() a little-endian output?
>     * I agree with Watson to be consistent with Kerberos here
>
> // - Should we clarify that the transcript assumes a particular point encoding scheme, and that is to be defined by the calling application or implementation?
>     * Resolved
>
> Some more comments, not related to the IRSG review, below.
>
> I was missing a minimal list of data that A and B can start SPAKE2 from: ciphersuite, pw, A, B, M, N - did I forget something? What about the output key length? This can be easily included in 3.2, replacing "A and B are provisioned with information such as..."
>
> The document does not specify who uses M and who uses N. In the M\neq N variant of the protocol, which is mostly described in this draft, this is relevant, since no analysis was conducted in case A sends both xM and xN (a solution that implementors are not unlikely to come up with). I would include an instruction in the RFC, e.g., that the application must ensure that M and N are assigned to A and B, e.g., by having them apply some lexicographically-first-... rule.

I'm not sure I follow the issue. The sentence says 'A picks x randomly
and uniformly from the integers in [0,p), and calculates X=x*P and
S=w*M+X, then transmits pA=S to B.' Alice is hence the initiatior. If
it's impossible to do this easily then one of the M=N variants should
be used. Most protocols have an asymmetry here where one side is
initiating a request and the other receiving. Happy to add clarifying
text.

>
> p.2
> The KDF has only 3 inputs, but a fourth (key length L) is described in the text
>
> p.3
> servers -> serves
> flow figure should include "verify cA" and "verify cB", since it is a MUST.
>
> p.4
> remove unnecessary variables T and S? E.g., directly set pB=w*N+Y.

These editorial comments seem worthwile to fix. Side note to Colin: is
there another set of reviews I should be waiting for at this stage? If
not I should be able to fix these all pretty promptly.

Sincerely,
Watson Ladd


--
Astra mortemque praestare gradatim