RustCrypto / traits

Collection of cryptography-related traits
579 stars 189 forks source link

elliptic-curve: support scalar modulus larger than base field size #1304

Open halvors opened 1 year ago

halvors commented 1 year ago

secp224k1 has a modulus that is larger than the base field size. This will probably be a big breaking change

tarcieri commented 1 year ago

Right now the size of the base and scalar fields is defined by Curve::FieldBytesSize, and the serialization of both uses FieldBytes<Curve>.

This has worked with every curve up until now.

Supporting this will involve splitting up all of these types. We'd need Curve::BaseFieldSize and Curve::ScalarFieldSize, as well as BaseFieldBytes and ScalarFieldBytes.

This will also particularly complicate the implementation of ECDSA (and similar algorithms like SM2DSA), which need to reduce an element of the base field into the scalar field.

tarcieri commented 11 months ago

Note that in the next breaking release of elliptic-curve we'll be migrating from generic-array to hybrid-array, and I'm a bit wary to try to tackle addressing this issue at the same time as that migration.

Fixing this is going to cause breakage/upgrade pain even though it's irrelevant to most users, so I'm a bit worried about trying to slip it into a release which is otherwise making major breaking changes to the same types.

halvors commented 8 months ago

Note that in the next breaking release of elliptic-curve we'll be migrating from generic-array to hybrid-array, and I'm a bit wary to try to tackle addressing this issue at the same time as that migration.

Fixing this is going to cause breakage/upgrade pain even though it's irrelevant to most users, so I'm a bit worried about trying to slip it into a release which is otherwise making major breaking changes to the same types.

What is the status with the generic-array to hybrid-array, is it complete?

tarcieri commented 8 months ago

As it's a migration which impacts hundreds of crates throughout this project, I'd say we're about halfway through, but there's still a lot of work to go.

The latest prereleases of e.g. p256 make use of it: https://crates.io/crates/p256/0.14.0-pre.0

halvors commented 1 month ago

How is this progress with migration going? Is this issue still blocked by the migration?

tarcieri commented 1 month ago

We will hopefully be able to start cutting stable releases either around EOY or the start of next. Perhaps closer to release time we can debate whether or not this change should make it in, if a workable PR has been opened.

I still worry the hybrid-array changes are going to be a lot for people to handle, and trying to split the types for base and scalar field elements will make the migration that much more difficult.

tarcieri commented 1 month ago

The main thing that needs to be decided to implement this is a set of new names for serialized field elements.

They're currently FieldBytes and FieldBytesSize which aren't great names to begin with for serialized field elements, and we still need to differentiate between base and scalar fields, however ScalarFieldElementBytes is a bit unwieldy.

There's also the FieldBytesEncoding trait which will need to be modified to support the two potentially differently sized fields.