mit-plv / fiat-crypto

Cryptographic Primitive Code Generation by Fiat
http://adam.chlipala.net/papers/FiatCryptoSP19/FiatCryptoSP19.pdf
Other
705 stars 147 forks source link

Mask most significant bit in from_bytes #1820

Open 0x0ece opened 7 months ago

0x0ece commented 7 months ago

The function from_bytes, at least for curve25519, doesn't mask the most significant bit: https://github.com/mit-plv/fiat-crypto/blob/9eb0f5ec4e2a1a700d1d5f7985e970a2e59a2cb7/fiat-c/src/curve25519_64.c#L802

In practice, this function is often used as part of point decompression, where the most significant bit is set to determine the sign of the other coordinate, so it'd be nice to "disregard" the msb.

The fix is straightforward:

x1 = ((uint64_t)(arg1[31] & 0x7F) << 44);
                          ^^^^^^

In addition, right now this limitation forces the caller to either make the input buffer writeable (to zero the bit), or to copy the buffer (example in dalek). Both seem undesirable.

andres-erbsen commented 7 months ago

The proposed change is compatible with the current spec which already assumes truncated input. I don't have a clear intuition about where to apply it though. Do you think the same concept should also be applied to other finite fields, or is this a curve25519 special?

0x0ece commented 7 months ago

To my knowledge that's a typical way to encode points, not just for curve25519, but also for secp, secp256k1, etc. Also, it shouldn't hurt even if one is just using the field.