LoupVaillant / Monocypher

An easy to use, easy to deploy crypto library
https://monocypher.org
Other
614 stars 80 forks source link

Fix Inverse square root and Elligator edge case #231

Closed LoupVaillant closed 2 years ago

LoupVaillant commented 2 years ago

(Giving this issue both tags "bug" and "enhancement", because I'm not sure how it should be considered. I knew of his discrepancy from the start and clearly documented it in the source code.)

Elligator 2 maps the point (0, 0) to the representative zero. Monocypher however, does not. Likewise, Monocypher fails to serialize the (0, 1) EdDSA public key, which is on the curve (though we don't really care, because it has low order, and legitimate EdDSA public keys all have prime order).

This is a by-product of how the inverse square root function works: it returns true not when its operand is a square, but when it's a non-zero square. Except we use it to determine whether something is a square period, which is slightly incorrect. Though it currently does not affect security (legitimate ephemeral keys are never zero), full conformity is more elegant, and could conceivably matter for some future unanticipated exotic use case. I see 3 compliance levels:

  1. Don't bother, it's okay to reject the point (u, v) when u is zero. This is what we're doing.
  2. Don't reject the point (u, v) when u is zero, without looking at the sign of v. This is what we'd get if invsqrt() returned true when its operand is zero.
  3. Don't reject the point (0, 0), but do reject any point (0, v) when v is not zero.

I personally believe level (2) is worth reaching. Level (3) however is probably not worth worrying about, for two reasons.

I believe the proper fix is to have invsqrt() return true when its operand is zero.

fscoto commented 2 years ago

This leans towards "bug" for me since it's already enough of a pain to get any kind of interoperability in this space.

Having said that, I would be surprised if there are any protocols that require this – if anything, I wonder if this is a way to re-introduce issues about having to check contributory behavior and thus mandate a thick warning in the documentation in hopes that people will read it.