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

From: Julia Hesse <[email-masked]>
Date: Tue, 7 Sep 2021 10:41:35 +0200

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 
<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.

// - 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?)

// - 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.

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.

---

From: Watson Ladd <[email-masked]>
Date: Wed, 8 Sep 2021 16:39:24 -0400

On Tue, Sep 7, 2021 at 4:41 AM Julia Hesse <[email-masked]> 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
> // - 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?
> // - 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
>
> 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

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

---

From: Colin Perkins <[email-masked]>
Date: Wed, 8 Sep 2021 23:17:08 +0100

Hi,

Thanks, Julia, for the review!

> On 8 Sep 2021, at 21:39, Watson Ladd <[email-masked]> wrote:
...
> not I should be able to fix these all pretty promptly.

I believe this was the final review, although I’ll let the RG chairs confirm in case I missed anything.

Colin

---

From: "Stanislav V. Smyshlyaev" <[email-masked]>
Date: Thu, 9 Sep 2021 09:53:03 +0300

>>  I believe this was the final review, although I’ll let the RG chairs
confirm in case I missed anything.
Yes, that was the only review that we were waiting for (thanks a lot,
Julia!).

Regards,
Stanislav

On Thu, 9 Sept 2021 at 01:17, Colin Perkins <[email-masked]> wrote:

> _______________________________________________
> Crypto-panel mailing list
> [email-masked]
> https://www.irtf.org/mailman/listinfo/crypto-panel
>

---

From: Julia Hesse <[email-masked]>
Date: Fri, 10 Sep 2021 20:04:15 +0200

Dear Watson,

> addressed by using this variant.

Ah, apparently I got confused here: the draft is referring to SPAKE2 as 
a symmetric PAKE, and to the case of M=N as its symmetric variant. So 
you are completely right, the sentence in question should stay, but I 
would recommend referring to the case of M=N as "parallel variant" (or 
something else) instead of assigning two different meanings to the word 
"symmetric".

> initiating a request and the other receiving. Happy to add clarifying
Ack to all. It would actually be good to clarify this in the draft: the 
M != N variant is for an initiator-responder variant, the M=N for a 
parallel variant that MUST (?) be used whenever the applicatoin cannot 
assign initiator and responder role to the parties. My concern was from 
a security perspective, from which we should avoid situation such as the 
following: a user generates pA wrt M, but then receives pB also 
generated wrt M, suddenly realizes that it is the responder, and 
*additionally* sends pA wrt N. This voids the security proof, so we need 
to be pretty clear here.

Best regards,
Julia

---

From: Watson Ladd <[email-masked]>
Date: Tue, 14 Sep 2021 11:45:32 -0700

I've opened https://github.com/kaduk/spake2/pull/24 which I think
addresses all the comments from this review.  If this is so, I can
merge it and upload a new copy tonight.

---

From: Julia Hesse <[email-masked]>
Date: Tue, 14 Sep 2021 23:11:26 +0200

Hi Watson,

looks good to me, thank you!

Julia

---

From: Watson Ladd <[email-masked]>
Date: Sat, 18 Sep 2021 08:33:21 -0700

Draft 22 is now uploaded.

---

From: "Stanislav V. Smyshlyaev" <[email-masked]>
Date: Mon, 27 Sep 2021 08:55:26 +0300

Hi all,

Thanks to Watson, the concerns raised in Julia's review are now addressed
in the draft (many thanks again to Julia Hesse for her review, by the way!).

Colin,

>> I believe this was the final review, although I’ll let the RG chairs
confirm in case I missed anything.

That was the final review, and the concerns raised in that review have been
addressed - therefore, nothing prevents the draft from moving to the IRSG
ballot.

Regards,
Stanislav (on behalf of CFRG chairs)

On Sat, 18 Sept 2021 at 18:33, Watson Ladd <[email-masked]> wrote:

> > >>>>       * Resolved. I suggest to remove "to have a symmetric variant"
> - it is not needed and might cause confusion (variant of what?)
> > >>>> // - 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.
> > >>>> // - 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
> > >>>>
> > >>>> 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