chris-wood / draft-bar-cfrg-spake2plus

Other
5 stars 1 forks source link

JP review comments #4

Closed chris-wood closed 2 years ago

chris-wood commented 2 years ago
  • In 3.1: "We fix two elements M and N in the prime-order subgroup": It should say how these are generated, and most importantly whether their discrete log must be known or not. If not, a hash-to-curve algorithm might be necessary.

Per the security analysis, it's required that these are random group elements for which the DL is unknown. Appendix A describes the try-and-increment algorithm for generating these values, but its presentation could be improved. I don't think we should add a hash-to-curve dependency since (a) TAI is much simpler and (b) generation is not timing sensitive, but I'm open to hearing counterarguments.

  • In 3.3: "If both identities A and B are absent, i.e. len(A) = len(B) = 0, then the length prefixes are omitted as in Section 3.1.". Would be clearer to just say: "If both identities are absent, then w0s || w1s = PBKDF(pw)". Risk of misunderstanding/misimplementation otherwise.

Nit acknowledged.

  • In 3.3: The mod p reductions (as in " w0 = w0s mod p") can introduce a small statistical bias, as they discuss briefly. At the very end the draft further says "The choices of random numbers MUST BE uniform.". But nowhere does it say why, and what's the risk.

This is done for alignment with the security analysis, which models the output of this as a random oracle. Any bias in the output would (I think) break that assumption. We can clarify this in the security considerations, with a forward pointer to match.

And just writing the specification as "w0 = w0s mod p" is misleading when you say later that in fact w0 might have to be computed differently from w0s. I think this part needs clarification.

Good point. We should make requirements for computing w0 and w1 more clear, and give a reasonable recommendation for computing them in the default case. We could fold in the bias explanation from the previous comment here, too.

  • It would greatly help implementers to include examples of "reasonable" parameters for Scrypt and Argon2 when used as PBKDFs.

This is a fine suggestion. We can include some recommended parameters for both Scrypt and Argon2 in an appendix.

  • In 5 (ciphersuites): strictly speaking, "HMAC" is not a MAC, guess they mean HMAC-SHA256 (with a key of some specific length).

Nit acknowledged -- we'll fix the ciphersuite column.

Also, why doesn't it include PBKDFs?

Unless I'm missing something, choice of PBKDF and its parameters are not needed for interoperability, so they don't need to be in the suite.

cc @ttaubert, @veorq