Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

elliptic_curve::NonZeroScalar non-zeroness guarantee #1607

Open
kwantam opened this issue Jul 5, 2024 · 1 comment
Open

elliptic_curve::NonZeroScalar non-zeroness guarantee #1607

kwantam opened this issue Jul 5, 2024 · 1 comment

Comments

@kwantam
Copy link

kwantam commented Jul 5, 2024

Thanks a lot for the great libraries! Apologies if this has already been asked---I looked briefly and didn't see anything.

It looks like in release builds it's quite easy to construct a NonZeroScalar that equals zero (debug builds will panic).

let sk = SecretKey::new(ScalarPrimitive::ZERO);
let nzs = sk.to_nonzero_scalar();

I could easily be missing something, and of course it's easy enough to avoid with a careful check. But I think probably SecretKey::new wants to be fallible since ScalarPrimitive can be zero, while SecretKey and NonZeroScalar presumably want to be nonzero.

@kwantam kwantam changed the title NonZeroScalar non-zeroness guarantee elliptic_curve::NonZeroScalar non-zeroness guarantee Jul 5, 2024
@tarcieri
Copy link
Member

tarcieri commented Jul 5, 2024

Indeed you have found a way around the NonZeroScalar invariant!

Either SecretKey::new needs to be changed to be fallible and reject an all-zero scalar, or to_nonzero_scalar needs to be changed to be fallible.

There are various opinions as to what should be allowable as secret keys. However I'd note that elliptic_curve::PublicKey already maintains an invariant that the inner curve point is a non-identity point, so a fallible constructor for SecretKey would make sense for symmetry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants