-
Notifications
You must be signed in to change notification settings - Fork 37.8k
Check consistency of private keys #782
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
Conversation
Reported by onlineproof on IRC: Bitcoin does not verify whether private keys and public keys correspond, when loading a wallet.
Hi, this is onelineproof from IRC. I think you need to do a bit more than just that...If you're first setting pubkey to be vchpubkey, then obviously they will be equal. But eventually, I can test it to make sure... If you wanna peek at my code that implements such a function in C, take a look at the function priv_to_pub in https://github.com/piratelinux/cwallet/blob/master/src/util.h |
@piratelinux: CKey::GetPubKey extracts the public key from the OpenSSL Key structure, which is constructed at the time of the CKey::SetPrivKey() call. The information present there from the CKey::SetPubKey() is only used to know whether the key is compressed. |
Why not check wkeys as well? |
@TheBlueMatt do they exist? |
Look at the block starting at line 868. |
wkeys are dead code that should be removed, if I recall correctly. |
Check consistency of private keys
wkeys were used by very, very old clients instead of keys iirc, No reason to remove support for opening old wallets. Gavin Andresen reply@reply.github.com wrote: wkeys are dead code that should be removed, if I recall correctly. Reply to this email directly or view it on GitHub: |
I still don't think it will work, because I think what SetPrivKey does is take the public key part of the full private key, but if the secret part (usually 32 bytes) is corrupted, it wouldn't know. I did a rough test, but maybe if you could send me a release that I can compile and you're confident works, then I can do a more thorough test. The master branch of bitcoin is not compiling for me. |
Check consistency of private keys
c6b6b8f Merge bitcoin#830: Rip out non-endomorphism code + dependencies c582aba Consistency improvements to the comments 63c6b71 Reorder comments/function around scalar_split_lambda 2edc514 WNAF of lambda_split output has max size 129 4232e5b Rip out non-endomorphism code ebad841 Check correctness of lambda split without -DVERIFY fe7fc1f Make lambda constant accessible 9d2f2b4 Add tests to exercise lambda split near bounds 9aca2f7 Add secp256k1_split_lambda_verify acab934 Detailed comments for secp256k1_scalar_split_lambda 76ed922 Increase precision of g1 and g2 6173839 Switch to our own memcmp function 63150ab Merge bitcoin#827: Rename testrand functions to have test in name c5257ae Merge bitcoin#821: travis: Explicitly set --with-valgrind bb1f542 Merge bitcoin#818: Add static assertion that uint32_t is unsigned int or wider a45c1fa Rename testrand functions to have test in name 5006895 Merge bitcoin#808: Exhaustive test improvements + exhaustive schnorrsig tests 4eecb4d travis: VALGRIND->RUN_VALGRIND to avoid confusion with WITH_VALGRIND 66a765c travis: Explicitly set --with-valgrind d7838ba Merge bitcoin#813: Enable configuring Valgrind support 7ceb0b7 Merge bitcoin#819: Enable -Wundef warning 8b7dcdd Add exhaustive test for extrakeys and schnorrsig 08d7d89 Make pubkey parsing test whether points are in the correct subgroup 87af00b Abstract out challenge computation in schnorrsig 63e1b2a Disable output buffering in tests_exhaustive.c 39f67dd Support splitting exhaustive tests across cores e99b26f Give exhaustive_tests count and seed cmdline inputs 49e6630 refactor: move RNG seeding to testrand b110c10 Change exhaustive test groups so they have a point with X=1 cec7b18 Select exhaustive lambda in function of order 78f6cdf Make the curve B constant a secp256k1_fe d7f39ae Delete gej_is_valid_var: unused outside tests 8bcd78c Make secp256k1_scalar_b32 detect overflow in scalar_low c498366 Move exhaustive tests for recovery to module be31791 Make group order purely compile-time in exhaustive tests e73ff30 Enable -Wundef warning c0041b5 Add static assertion that uint32_t is unsigned int or wider 4ad408f Merge bitcoin#782: Check if variable=yes instead of if var is set in travis.sh 412bf87 configure: Allow specifying --with[out]-valgrind explicitly 34debf7 Modify .travis.yml to explictly pass no in env vars instead of setting to nothing a0e99fc Merge bitcoin#814: tests: Initialize random group elements fully 5738e86 tests: Initialize random group elements fully c9939ba Merge bitcoin#812: travis: run bench_schnorrsig a51f2af travis: run bench_schnorrsig ef37761 Change travis.sh to check if variables are equal to yes instead of not-empty. Before this, setting `VALGRIND=wat` was considered as true, and to make it evaluate as false you had to unset the variable `VALGRIND=` but not it checks if `VALGRIND=yes` and if it's not `yes` then it's evaluated to false git-subtree-dir: src/secp256k1 git-subtree-split: c6b6b8f
Reported by onlineproof on IRC: Bitcoin does not verify whether private
keys and public keys correspond, when loading a wallet.