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?
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.
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.
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.
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](https://datatracker.ietf.org/doc/bcp14/) sense. If this text said
For elliptic curves not listed elsewhere in section 8, a new hash-to-curve suite can be defined by
You wouldn’t need any of the [BCP 14](https://datatracker.ietf.org/doc/bcp14/) 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 …
armfazh
commented
2 years ago
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.