Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Check that the secret key is non-zero modulo the order of the elliptic curve #8829

Merged
merged 16 commits into from
Aug 17, 2023

Conversation

mosmartin
Copy link
Contributor

What was the problem?

This PR resolves #8783

How was it solved?

Added the isSecretKeyNonZeroModEC function to check that the secret key generated using the SecretKey.fromBytes function is non-zero modulo the order of the elliptic curve.

How was it tested?

Added unit tests

Ensure that the secret key generated using the `SecretKey.fromBytes` function is non-zero modulo the order of the elliptic curve.
@mosmartin mosmartin self-assigned this Aug 9, 2023
@codecov
Copy link

codecov bot commented Aug 9, 2023

Codecov Report

Merging #8829 (98b14dc) into release/6.0.0 (6e47eea) will increase coverage by 0.00%.
The diff coverage is 94.44%.

❗ Current head 98b14dc differs from pull request most recent head c9a37b4. Consider uploading reports for the commit c9a37b4 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff               @@
##           release/6.0.0    #8829   +/-   ##
==============================================
  Coverage          83.41%   83.42%           
==============================================
  Files                601      601           
  Lines              22467    22479   +12     
  Branches            3318     3321    +3     
==============================================
+ Hits               18740    18752   +12     
  Misses              3727     3727           
Files Changed Coverage Δ
elements/lisk-cryptography/src/bls_lib/lib.ts 98.48% <94.44%> (-1.52%) ⬇️

... and 1 file with indirect coverage changes

@mosmartin mosmartin requested a review from shuse2 August 9, 2023 15:55
Copy link
Collaborator

@shuse2 shuse2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than correctness of the value used, LGTM

elements/lisk-cryptography/src/bls_lib/lib.ts Outdated Show resolved Hide resolved
@shuse2 shuse2 changed the title Check that the secret key is non-zero modulo the order of the elliptic curve. Check that the secret key is non-zero modulo the order of the elliptic curve Aug 11, 2023
Case for a secret key that is non-zero but zero modulo the group order must hold
- Update the blsSign function
- Add and update unit tests
@mosmartin mosmartin requested a review from shuse2 August 16, 2023 09:07
elements/lisk-cryptography/src/bls_lib/lib.ts Outdated Show resolved Hide resolved
elements/lisk-cryptography/src/bls_lib/lib.ts Outdated Show resolved Hide resolved
elements/lisk-cryptography/src/bls_lib/lib.ts Outdated Show resolved Hide resolved
elements/lisk-cryptography/src/bls_lib/lib.ts Outdated Show resolved Hide resolved
elements/lisk-cryptography/src/bls_lib/lib.ts Outdated Show resolved Hide resolved
elements/lisk-cryptography/test/bls_lib/lib.spec.ts Outdated Show resolved Hide resolved
elements/lisk-cryptography/test/bls_lib/lib.spec.ts Outdated Show resolved Hide resolved
elements/lisk-cryptography/test/bls_lib/lib.spec.ts Outdated Show resolved Hide resolved
elements/lisk-cryptography/test/bls_lib/lib.spec.ts Outdated Show resolved Hide resolved
elements/lisk-cryptography/test/bls_lib/lib.spec.ts Outdated Show resolved Hide resolved
mosmartin and others added 6 commits August 17, 2023 11:45
Remove unnecessary checks

Co-authored-by: AndreasKendziorra <40799768+AndreasKendziorra@users.noreply.github.com>
Co-authored-by: AndreasKendziorra <40799768+AndreasKendziorra@users.noreply.github.com>
Co-authored-by: AndreasKendziorra <40799768+AndreasKendziorra@users.noreply.github.com>
Co-authored-by: AndreasKendziorra <40799768+AndreasKendziorra@users.noreply.github.com>
Test that `blsPopProve` function throws an an error when the sk is zero
@shuse2 shuse2 enabled auto-merge (squash) August 17, 2023 14:19
@shuse2 shuse2 merged commit 7e8a33b into release/6.0.0 Aug 17, 2023
8 checks passed
@shuse2 shuse2 deleted the 8783-bls-sk-checks branch August 17, 2023 14:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants