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

feat: ensure valid random PrivateKey if no secret is given #392

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

charlie632
Copy link
Contributor

@charlie632 charlie632 commented Jul 13, 2023

fixes: #390

This will loop for an arbitrary time (10 times) until the random private key is valid.

The PrivateKeyImplementation can throw an error if an invalid secret is provided, which can happen if it returns a value bigger than 0xfffffffffffffffffffffffffffffffebaaedce6af48a03bbfd25e8cd0364141 (see https://crypto.stackexchange.com/questions/30269/are-all-possible-ec-private-keys-valid/30272#30272).

This looping mechanism follows a technique used by Apple

@what-the-diff
Copy link

what-the-diff bot commented Jul 13, 2023

PR Summary

  • Enhanced Error Checking in Key Context Creation
    Added a verification step in secp256k1.swift to ensure the context for key creation is correctly initialized.

  • Improved Private Key Generation
    Implemented a loop that tries to generate a random private key 10 times within the initialization function, enhancing the reliability of key generation process.

  • Efficiency Improvement in Key Generation
    Refined the process of private key generation in the loop of the initialization function in secp256k1.swift for better efficiency.

  • Verification of Public Key Serialization
    Included error checking after public key serialization operation to ensure its successful execution.

  • Additional Check in Public Key Encoding
    Added a check following the serialization of the public key in the encode function to improve the trustworthiness of the coding process.

  • Comprehensive Verification for Xonly Public Key Serialization
    Inserted a verification step after serializing the xonly public key in the publicKey computed property. This improves the security and correctness of the public key.

@charlie632 charlie632 force-pushed the charlie632/ensure-valid-random-key-creation branch from 633b449 to f7d3d82 Compare July 13, 2023 04:45
Copy link
Contributor

@csjones csjones left a comment

Choose a reason for hiding this comment

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

This is a great start @charlie632, thank you for the contribution! These changes just update the backing implementation, we still need to update the public API in Asymmetric.swift.

Additionally, we should incorporate a unit test (similar to the one discussed in the issue thread) with a high iteration rate. Maybe 100K?

Lastly, we'd want to update the examples in the README to reflect the API change.

@@ -151,16 +151,14 @@ extension secp256k1 {

/// Backing initialization that creates a random secp256k1 private key for signing
@usableFromInline init(format: secp256k1.Format = .compressed) throws {
Copy link
Contributor

Choose a reason for hiding this comment

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

We will want to update the function signature so a try catch statement isn't needed.

Suggested change
@usableFromInline init(format: secp256k1.Format = .compressed) throws {
@usableFromInline init(format: secp256k1.Format = .compressed) {

@csjones
Copy link
Contributor

csjones commented Sep 11, 2023

Hey @charlie632, just checking in, would you like any help with this PR?

@charlie632
Copy link
Contributor Author

@csjones hey! Sorry, I've been extremely busy with a big deliverable for my company. I have some free time next week, so I can happily finish this PR!

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

Successfully merging this pull request may close these issues.

Is it possible to generate an invalid PrivateKey when no secret it passed?
2 participants