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.

Sign up for GitHub

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

Jump to bottom

scalar: Verify invariants on every entry #1360

Closed
real-or-random opened this issue Jun 27, 2023 · 2 comments · Fixed by #1373
Closed

scalar: Verify invariants on every entry #1360

real-or-random opened this issue Jun 27, 2023 · 2 comments · Fixed by #1373
Labels
assurance

Comments

@real-or-random
Copy link
Contributor

We have secp256k1_ge_verify, secp256k1_gej_verify, and secp256k1_fe_verify functions to the invariants of the respective type. We call them on every entry/exit of a function that operates on a respective element.

We should add a similar function for scalars. I think the only invariant is that scalars a are reduced mod the group order, i.e., secp256k1_scalar_check_overflow(a) == 0.

(see #1184 (comment))

@stratospher Are you interested in working on this?
@stratospher
Copy link
Contributor

yes! i'd be interested in trying this.

@stratospher stratospher mentioned this issue Jul 6, 2023
Merged
@stratospher
Copy link
Contributor

done in #1373!

@real-or-random real-or-random linked a pull request Jul 6, 2023 that will close this issue
Add invariant checking for scalars #1373
Merged
real-or-random added a commit that referenced this issue Aug 18, 2023
@real-or-random
Merge #1373: Add invariant checking for scalars
0ba2b94 
d23da6d use secp256k1_scalar_verify checks (stratospher)
c7d0454 add verification for scalars (stratospher)
ad15215 update max scalar in scalar_cmov_test and fix schnorrsig_verify exhaustive test (stratospher)

Pull request description:

  From #1360. This PR:
  1. adds `secp256k1_scalar_verify` to make sure scalars are reduced mod the group order in VERIFY mode
  2. uses `secp256k1_scalar_verify` in all the scalar functions except `secp256k1_scalar_clear`, `secp256k1_scalar_reduce_512`, `secp256k1_scalar_mul_512` and `secp256k1_scalar_*_var` functions in `scalar_low_impl.h`

ACKs for top commit:
  real-or-random:
    utACK d23da6d
  theStack:
    Code-review ACK d23da6d

Tree-SHA512: a371b319d948198c4038d35c9ea58f4b94de4dc312215e2b78a323c2acd4ae1355d97935c558b388774832d6d0058b97ff8ca50c3aab40b9ede5307760d0a505
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
assurance
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add invariant checking for scalars stratospher/secp256k1
2 participants
@real-or-random @stratospher