Rev sd

[armfazh]

This PR cover largely the comments and observations by Spencer D.
It's better to do review commit-by-commit so the diff changes get lets noisy.

I'm only vaguely aware of how this stuff works, so please keep that in mind, when reading my comments. I hope they are somewhat helpful.

In this text from the Introduction,

Unfortunately for implementors, the precise hash function that is suitable for a given protocol implemented using a given elliptic curve is often unclear from the protocol's description. Meanwhile, an incorrect choice of hash function can have disastrous consequences for security.

I’m not sure I understand (at this point in the document) what the problem is (“why it’s not OK to just pick a hash function”), other than “if you do that, bad things will happen”). Is there a reference you could include here, or even a forward pointer if there's a good place to point to in the draft, so that us non-experts can follow along?

1. Not sure how to resolve this one.
   Two paragraphs below the cited text there is a pointer to Section 8 for those who want to quickly jump in. I consider this is enough as a forward pointer.

I learned a lot from googling “rejection sampling methods” while reading this text

This document does not cover rejection sampling methods, sometimes referred to as "try-and-increment" or "hunt-and-peck,"

But the text didn’t tell me enough to understand rejection sampling methods. Perhaps a half-sentence explanation, or a reference, would be helpful.

2. See commit 95b21a5

This is nit-ish, but it confused me.

5.1. Security considerations, is only for section 5, is that right? There’s another Security Considerations - section 10 - which starts with these two sentences:

Section 3.1 describes considerations related to domain separation. See Section 10.4 for further discussion.

Section 5 describes considerations for uniformly hashing to field elements; see Section 10.2 and Section 10.3 for further discussion.

I found myself wondering why some security considerations seem to be in Section 3.1 (which isn’t called Security considerations), and others seem to be in Section 5 (shouldn’t the second sentence refer to Section 5.1, which IS called Security considerations?), and these considerations outside Section 10 aren’t complete. If I was looking for all the Security considerations, I’d expect to find them together, and probably in Section 10.

Do the right thing, of course.

3. See commits 86f9947 and 11744a1

I’m not picking on BCP 14 language in most of the text, but in Section 7,

Note that in this case scalar multiplication by the cofactor h does not generally give the same result as the fast method, and SHOULD NOT be used.

I’m not understanding why this is not a MUST - when is it OK to use scalar multiplication, if it usually gives a different result?

4. See commits a78c19a

I have roughly the same question in Section 8.5,

This section defines ciphersuites for curve25519 and edwards25519 [RFC7748]. Note that these ciphersuites SHOULD NOT be used when hashing to ristretto255 [I-D.irtf-cfrg-ristretto255-decaf448]. See Appendix B for information on how to hash to that group.

What if I DO use these ciphersuites inappropriately?

Very similar text is in 8.6, so I have a very similar question.

This section defines ciphersuites for curve448 and edwards448 [RFC7748]. Note that these ciphersuites SHOULD NOT be used when hashing to decaf448 [I-D.irtf-cfrg-ristretto255-decaf448]. See Appendix C for information on how to hash to that group.

Do the right thing, of course.

5. See commits 3825baa

In section 8.9,

The RECOMMENDED way to define a new hash-to-curve suite is:

When hashing to an elliptic curve not listed in this section, corresponding hash-to-curve suites SHOULD be fully specified as described above.

As a nit, “not listed in this section” might reasonably be read as “not listed in section 8.9”. I think you might better say “not listed elsewhere in section 8”.

But beyond that, I don’t think you mean “RECOMMENDED” in the BCP 14 sense. If this text said

For elliptic curves not listed elsewhere in section 8, a new hash-to-curve suite can be defined by
<steps 1-8 as they appear in the draft>

You wouldn’t need any of the BCP 14 language in this section, with the attendant “why is this not a MUST”, “in what cases would you not do this”, and “if you don’t do this, what happens?” questions that reviewers can’t help asking …

6. See commits 11055a3

[chris-wood]

Thanks! This looks good to me, pending some suggestions.

[armfazh]

Applied requested changes.