Skip to content

secp256k1/ecdsa: Add sign and verify tests.#2908

Merged
davecgh merged 4 commits intodecred:masterfrom
davecgh:secp256k1_ecdsa_add_sign_and_verify_tests
Apr 2, 2022
Merged

secp256k1/ecdsa: Add sign and verify tests.#2908
davecgh merged 4 commits intodecred:masterfrom
davecgh:secp256k1_ecdsa_add_sign_and_verify_tests

Conversation

@davecgh
Copy link
Copy Markdown
Member

@davecgh davecgh commented Mar 25, 2022

Although the ECDSA code has been extensively tested through usage and external fuzz testing, there are not currently any unit tests for the non-compact signing and verification path.

This resolves that by adding signing and verification tests for both the positive and negative paths.

The following is a high-level overview of the changes:

  • Refactors the primary logic for the regular ECDSA signing code to a separate function that accepts an arbitrary nonce to facilitate fully testing both the positive and negative paths and provides more flexibility for potentially using other nonce generation mechanisms in the future should it be necessary
  • Add sign and verify tests with known good test data that is easily reproducible and has been independently verified with the Sage computer algebra system
    • The test data includes the original messages so the resulting hashes are independently verifiable and have a known source
    • The test data includes variations of signing the same data with different keys and nonces, both deterministically generated via RFC6979 and random, and signing different data with the same keys
  • Add tests to ensure the aforementioned test data that was independently verified produces the expected results
  • Add tests which are specifically designed to exercise conditions that lead to invalid signatures
  • Add tests to help ensure the signature verification code fails to verify edge conditions as expected by crafting signatures that are specifically designed to hit them
  • Add test to sign and verify random data
    • Generates random keys and messages with each run from a new random seed and log that seed in the event of failure
    • Signs the rand message with the random key and ensures the produced signature verifies correctly
    • Ensures mutating a random bit in each good signature results in that mutated signature failing to verify the original message
    • Ensures mutating a random bit in each message hash that was originally signed results in the original good signature failing to verify the new mutated message

@davecgh davecgh added the test coverage Discussion and pull requests for improving test coverage. label Mar 25, 2022
@davecgh davecgh added this to the 1.8.0 milestone Mar 25, 2022
Comment thread dcrec/secp256k1/ecdsa/signature.go
Copy link
Copy Markdown
Member

@rstaudt2 rstaudt2 left a comment

Choose a reason for hiding this comment

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

Nice increase in coverage!

Comment thread dcrec/secp256k1/ecdsa/signature_test.go Outdated
Comment thread dcrec/secp256k1/ecdsa/signature_test.go
davecgh added 4 commits March 30, 2022 22:56
This refactors the primary logic for the regular ECDSA signing code to a
separate function that accepts an arbitrary nonce.  The primary
motivation is to facilitate upcoming unit tests which aim to fully test
both the positive and negative paths, however, it is also beneficial in
that it provides more flexibility for potentially using other nonce
generation mechanisms in the future should it be necessary.
Although the ECDSA code has been extensively tested through usage and
external fuzz testing, there are not currently any unit tests for the
non-compact signing and verification path.

This resolves that by adding signing and verification tests for both the
positive and negative paths.  The following is a high-level overview of
the changes:

- Create new known good test data that is easily reproducible and has
  been independently verified with the Sage computer algebra system
  - The test data includes the original messages so the resulting hashes
    are independently verifiable and have a known source
  - The test data includes variations of signing the same data with
    different keys and nonces, both deterministically generated via
    RFC6979 and random, and signing different data with the same keys
- Add tests to ensure the aforementioned test data that was
  independently verified produces the expected results
- Add tests which are specifically designed to exercise conditions that
  lead to invalid signatures
- Add tests to help ensure the signature verification code fails to
  verify edge conditions as expected by crafting signatures that are
  specifically designed to hit them
This adds a test to sign and verify random data in addition to the
existing tests the deal with known good data and edge conditions.

Specifically, the test:

- Generates random keys and messages with each run from a new random
  seed and log that seed in the event of failure
- Signs the rand message with the random key and ensures the produced
  signature verifies correctly
- Ensures mutating a random bit in each good signature results in that
  mutated signature failing to verify the original message
- Ensures mutating a random bit in each message hash that was originally
  signed results in the original good signature failing to verify the
  new mutated message
@davecgh davecgh force-pushed the secp256k1_ecdsa_add_sign_and_verify_tests branch from c49e886 to d9ba0b9 Compare March 31, 2022 03:59
@davecgh davecgh merged commit d9ba0b9 into decred:master Apr 2, 2022
@davecgh davecgh deleted the secp256k1_ecdsa_add_sign_and_verify_tests branch April 2, 2022 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test coverage Discussion and pull requests for improving test coverage.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants