cfrg / draft-irtf-cfrg-frost

Other
22 stars 9 forks source link

RGLC editorial issues #329

Closed chris-wood closed 1 year ago

chris-wood commented 1 year ago

From @cjpatton:

Section 3.2: nit: "FROST requires the use of a cryptographically secure hash function, generically written as H, which functions effectively as a random oracle." I would reword this as "... secure hash function, ..., which is modeled as a random oracle in current security proofs."

editorial: "FROST assumes that the Coordinator and the set of signer participants, are chosen externally to the protocol." Omit the comma.

commit() in Section 5.1: I might have called the output "nonce" the "ephmeral_secret" or something, to emphasize that it is secret (nonces generally aren't) and that it is used only once.

Editorial: Many of the algorithm specs seem to be too long to fit into the column limit. Consider wrapping lines so that the reader doesn't have to scroll.


From Paul Bottinelli:

  1. Introduction

    • "[RFC8032]" is only referred to as such (namely, by reference to the RFC), and never as Edwards-Curve Digital Signature Algorithm (EdDSA), which could help readers. In comparison, the Ristretto specification states "Curve25519 [RFC7748]" when talking about "[RFC7748]" for the first time.
  2. Conventions and Definitions

    • "len(l): Outputs the length of input list l, e.g., len([1,2,3]) = 3)." -> There's an extra closing parenthesis at the end. Also consider changing to "len(l): Outputs the length of the list ..." (i.e., dropping "input" and adding "the") for consistency with the other bullet points.

3.1. Prime-Order Group

* "element addition or scalar addition, depending on types of the" -> "element addition or scalar addition, depending on *the* type of the"; The type being pluralized make it sounds like the operands may be of different types, in my opinion.

* "equivalent to the repeated application of the group operation B with" -> "equivalent to the repeated application of the group operation *on* B with"

* "addition, negation, and equality comparisons can be efficiently" -> either pluralize addition, negation, or change "comparisons" to "comparison".

* Consider adding a comma after i.e. in "(i.e. p)" and "(i.e.  I)" for consistency with the rest of the document. Also, there's an extra whitespace in the latter.

* "ScalarMult(A, k): Output" -> "ScalarMult(A, k): Outputs"

* "ScalarBaseMult(k): Output" -> "ScalarBaseMult(k): Outputs"

* "raise an error if deserialization fails or A is the identity" -> "raise an error if deserialization fails or *if* A is the identity"
  1. Helper Functions

    • "Signature binding Section 4.4, group commitment Section 4.5, and challenge computation Section 4.6." -> should these have their dedicated bullet points?

    • "These sections describes these operations in more detail." -> "describes" should be "describe". Also, nitpicking, but the repetition of "these" could be avoided by writing "The following sections describe".

4.3. List Operations

* "list of participant commitments into a bytestring for use in the" -> replace with "byte string" to be consistent with the rest of the document

5.2. Round Two - Signature Share Generation

* "input messages are also required to also perform relevant" -> repetition of "also", delete one.

5.3. Signature Share Verification and Aggregation

* "generated in round one from the ith participant" I am noticing now that "ith" is also sometimes spelled "i-th" in this document. I personally prefer the latter and would suggest performing a search and replace over the entire text.

* "commitment will be with respect to a different signing set than the the aggregated response." -> remove the superfluous "the".
  1. Ciphersuites

    • "The RECOMMENDED ciphersuite is (ristretto255, SHA-512) Section 6.2." Is there a word missing here, such as "The RECOMMENDED ciphersuite is (ristretto255, SHA-512) as described in Section 6.2."

    • "these functions are described for each the ciphersuites below." -> "for each of the"

  2. Security Considerations

    • "unforgeability assuming at most (MIN_PARTICIPANTS-1) corrupted" why is "MIN_PARTICIPANTS-1" in parentheses?

    • "Existential Unforgeability Under Chosen Message Attack (EUF-CMA) attacks," I think the second "attacks" could be dropped, but I would understand wanting to keep it.

    • "Post quantum security." consider hyphenating "Post-quantum".

7.4. Input Message Hashing

* "security level commensurate with the security in inherent to the" -> delete "in"

From @armfazh:

chris-wood commented 1 year ago

I'm going to tackle this last, after all other issues are resolved, to minimize possible conflicts.