RustCrypto / elliptic-curves

Collection of pure Rust elliptic curve implementations: NIST P-224, P-256, P-384, P-521, secp256k1, SM2
683 stars 191 forks source link

k256: type safety for non-normalized field elements #531

Open tarcieri opened 2 years ago

tarcieri commented 2 years ago

The k256 crate uses lazy normalization of field elements. While not a user-facing concern as we deliberately encapsulate FieldElement, there is a potential for bugs in code in k256 itself in the event one "forgets" to normalize a field element prior to returning it as the result of some computation. See #530 as an example (although there have been others).

One potential way to eliminate this class of bugs is to use separate types for normalized vs non-normalized field elements. For example, we could introduce something like LazyFieldElement on which all of the arithmetic operations are defined, while retaining FieldElement for the normalized form and defining all serialization operations on that.

LazyFieldElement::normalize could return FieldElement, and we could additionally have bidirectional From conversions between the two.

Things would be a bit tricky in regard to the the ff::Field traits. I imagine for compatibility reasons they would need to be impl'd on FieldElement, converting to a LazyFieldElement to perform the arithmetic, and then calling normalize() to get a FieldElement again as a result, which would mean that the performance benefits of lazy normalization wouldn't be available under such an API. However, that is the only safe usage pattern since the traits are designed to abstract over different fields/field implementations, and not all of them have lazy normalization.

cc @fjarri

fjarri commented 2 years ago

I think it's a great idea for preventing errors, but yes, trait stuff might get difficult.