cfrg / draft-irtf-cfrg-hash-to-curve

Hashing to Elliptic Curves
Other
78 stars 27 forks source link

RGLC feedback #328

Closed chris-wood closed 2 years ago

chris-wood commented 2 years ago

section 4.1 sgn0 function the definition is also dependent on how elements in the base/prime field are represented. for example for the field GF(5), sgn0(-3) = -3 mod 2 = 1 = sgn0(3) which is not the desired effect (but then sgn0(2) = 0). maybe this could lead to u,y pairs whose signs disagree in a clumsy implementation that doesn't always represent prime field elements as non-negative ints.

section 5.3 hash_to_field function step 2 may fail as expand_message_xmd aborts when len_in_bytes exceeds a certain size

section 5.4.1 expand_message_xmd second paragraph recommends use of sha-2 and sha-3. consider providing a table of input block sizes for these functions rather than mentioning the value for sha-512 and sha3-512 in the subsequent paragraph

the bound 255*b_in_bytes is checked in step 2 whereas the bound 2^16-1 is not checked

unclear why there is a bound on len_in_bytes for expand_message_xmd but not for expand_message_xof

section 6.6.1 Shallue-van de Woestjine method step 6 multiplies tv4 by -1 if sgn(tv4)==1. is this really necessary as the values -/+ tv5 are used in steps 9 and 10?

kwantam commented 2 years ago

section 4.1 sgn0 function the definition is also dependent on how elements in the base/prime field are represented.

Isn't this explicitly stated? I don't seen an obvious way to make this not the case for extension fields.

for example for the field GF(5), sgn0(-3) = -3 mod 2 = 1 = sgn0(3) which is not the desired effect (but then sgn0(2) = 0). maybe this could lead to u,y pairs whose signs disagree in a clumsy implementation that doesn't always represent prime field elements as non-negative ints.

This seems false. For GF(5), -3 mod 5 = 2, so sgn0(-3) != sgn0(3). I don't know why mod 2 is discussed here, but it seems like that's the source of the confusion. Unless I'm missing something, no correct implementation of sgn0 as defined in the document can have this issue.

kwantam commented 2 years ago

section 5.3 hash_to_field function step 2 may fail as expand_message_xmd aborts when len_in_bytes exceeds a certain size

I suppose we could add a failure check, but it seems pretty silly to do so in pseudocode. It seems 100% clear what should happen if a subroutine call aborts in this context.

section 5.4.1 expand_message_xmd second paragraph recommends use of sha-2 and sha-3. consider providing a table of input block sizes for these functions rather than mentioning the value for sha-512 and sha3-512 in the subsequent paragraph

Listing facts about hash functions does not seem related to the core function of this document. I'm pretty strongly against adding this table for this reason.

(Giving examples for clarity is a different matter, of course.)

the bound 255*b_in_bytes is checked in step 2 whereas the bound 2^16-1 is not checked

Sure, we can add this check on line 2.

unclear why there is a bound on len_in_bytes for expand_message_xmd but not for expand_message_xof

expand_message_xof is bounded by what the XOF can do. The bound in _xmd comes from the construction. I'm not sure what more can be clarified here.

section 6.6.1 Shallue-van de Woestjine method step 6 multiplies tv4 by -1 if sgn(tv4)==1. is this really necessary as the values -/+ tv5 are used in steps 9 and 10?

I haven't looked at this in detail. Seems like the suggestion could be an optimization; I'd prefer clarity if they're in tension.