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

Invalid BIP-38 for Compressed Pubkeys #32

Open
praeluceo opened this issue Dec 3, 2017 · 10 comments
Open

Invalid BIP-38 for Compressed Pubkeys #32

praeluceo opened this issue Dec 3, 2017 · 10 comments

Comments

@praeluceo
Copy link

When using the wallet to generate a BIP38 encrypted paper wall against a compressed pubkey, the decrypted key does not match the input.

For example, the compressed private key for "17bbQL696yc37SxbXwAs9Zi6yCwzBwhFH" is "L452QJZK6rn2imxzQrJiBArX25reWqwvdBuKzmv11YDQb7cgGHJf" which is compressed, when encrypted with "bitcoinpaperwallet" yields a BIP-38 wallet of "6PRJAp2oK4NXHe6dvPjazggtcSt6Sr737D79UA2K4isba4GGtfJWSrq3Hx".

However, decrypting the above wallet in bitcoinpaperwallet yields a public key of "1JpuWDAUoKHFVNzL3cmuhR2psac7JqfNfC" instead, with a private key of "5KKW9Ay1gF2hzjGsx8pQcZW8q24tvXXesrv5eMwKkwRWkN6eCSF".

There appears to be a bug with how bitcoinpaperwallet is encrypting compressed wallets.

@cantonbecker
Copy link
Owner

cantonbecker commented Dec 4, 2017 via email

@praeluceo
Copy link
Author

praeluceo commented Dec 4, 2017

Apologies, of course. Steps to Reproduce:

  1. Go to https://iancoleman.io/bip39/ and generate a random 12-word seed
  2. Scroll down, ensuring you're at the BIP44 tab and select any private key you want to encrypt, note that these private keys are all presented as a compressed private key, and begin with either an L or K.
  3. Go to https://bitcoinpaperwallet.com/bitcoinpaperwallet/generate-wallet.html and skip the entropy generator, clicking straight through to the "Print Front" tab.
  4. Take the private key, click on the "enter my own key" button on the generator, and paste it into the text box.
  5. Hit "apply", and verify that the reported public key matches the private key you copied.
  6. Check the "BIP38 Encrypt" box, and enter any encryption passhrase you want, then click on "TURN ON BIP38 ENCRYPTION".
  7. Wait for your key to be encrypted, upon completion copy the BIP38 private key starting with "6P" from the wallet generator page, make note of your public key.
  8. Navigate to the "Validate or Decrypt" tab, paste in your BIP38 encrypted private key, type in your passphrase, click on "Decrypt BIP38".
  9. Note that the displayed public key no-longer matches the desired public key, and that the private key does not match the private key from step 2.

I haven't done a lot of digging yet, but it appears to be an issue where the BIP38 encryption is expecting an uncompressed pubkey/privkey combination (starting with a 5). My next step was going to be to uncompress the same private key used to reproduce the failure, but I haven't looked heavily into how to uncompress them yet.

@praeluceo
Copy link
Author

Got a little closer, I just went to https://www.bitaddress.org/ (where this was forked from) and the defect exists there too. I can generate a bip38 compressed private key on bitaddress.org, and switch over to the verify tab, and it gives me a different public key than it originally displayed. Tested that "6P" address on here, and got the identical wrong address.

I then went to bitaddress and generated an uncompressed BIP38 encrypted private key, and it -did- decrypt to the original public key. And it did on bitcoinpaperwallet as well.

@cantonbecker
Copy link
Owner

cantonbecker commented Dec 4, 2017 via email

@praeluceo
Copy link
Author

OK, so that worked! But now I'm a little confused. So revealing the compressed keys displays the original public key as expected, but the behavior still doesn't match. For example, when scanning the private key to sweep into Mycelium or Samourai, the displayed public key is the Public uncompressed address. But the public key printed on the wallet itself, is the public compressed address. And if the public compressed address is funded by scanning the paper wallet, and if the private key is swept by a mobile wallet, then the balance on the recipient's phone is empty. For instance:

  1. Generate a new compressed keypair: 1JTgVXzMCgnh2S5rowvAcuZRDj65eTxYtZ & L2hv26igH5nSN9bVd4mjKQm3yRc54dYLu3YyTESMTtF6nsM2kPfs
  2. Go to Bitcoinpaperwallet and input "L2hv26igH5nSN9bVd4mjKQm3yRc54dYLu3YyTESMTtF6nsM2kPfs", press "apply", confirm that it represents the private key to "1JT...tZ"
  3. Check BIP38 encrypt with the passphrase of "test"
  4. Verify that the public key has remained "1JT...tZ" and the new private key is "6PRPVo5DSs1bGMFmBxpb1AxstEShPGyUq27sjAr8FubXMWWFrDzsVDLGt1"
  5. Hypothetically, load 1 BTC onto "1JT...tZ"
  6. Go to "Validate or Decrypt" tab, enter BIP38 private key & passphrase, verify that the displayed Public Key is now, "1DMjz5d324YFdpEeKhMqRWWr8uiru8KWGN" instead of "1JT...tZ" with a private key of "5K4NytbN8isApqcuhoAgVe6uL8PSqrWkvw3iLY91ZNaZ8EFURic".
  7. Click on the "Display compressed format keys" button and verify that the correct public/private keypair is shown as expected (no data loss, hooray!)
  8. However, now Using Mycelium scan the paper wallet to add a new private key. Enter the passphrase, select BTC, save the wallet, and note that the displayed public key is "1DMjz5...u8KWGN" and is unfunded. The 1 BTC balance remains on the compressed key.
  9. Open Samourai and confirm that the sweep behaves the same.
  10. Go back to bitcoinpaperwallet, and scan the newly revealed private key from the "display compressed format keys" entry, and tada, you have your "1JTgVX...eTxYtZ" public key!

In the end, what I believe, is that we are seeing a mix of compressed and uncompressed keys used inconsistently on the wallet generator. If "1JTgVXzMCgnh2S5rowvAcuZRDj65eTxYtZ" is shown as the public key, then "L2hv26igH5nSN9bVd4mjKQm3yRc54dYLu3YyTESMTtF6nsM2kPfs" should be used for the BIP38 encrypted but if the private key decrypts to "5K4NytbN8isApqcuhoAgVe6uL8PSqrWkvw3iLY91ZNaZ8EFURic" then the public key -has- to reflect that as being "1DMjz5d324YFdpEeKhMqRWWr8uiru8KWGN".

So recovery is possible, but is not straight-forward from either wallets I tested with. Additionally, the confusion with compressed keys (which was confusing even for me!) is that you end up with new public keys from the uncompressed BIP38 decrypted wallet. I was using an HD-wallet to generate dozens of keypairs, entering them into bitcoinpaperwallet, and funding them in bulk. So when I did a test sweep and it failed, it was unsettling because the public/private keypair it did generate, do not appear in my Derivation Path from my BIP44 wallet.

@cantonbecker
Copy link
Owner

cantonbecker commented Dec 4, 2017 via email

@praeluceo
Copy link
Author

When you take those steps, on the paper wallet you just printed, does it show a private key of 6PRPVo5DSs1bGMFmBxpb1AxstEShPGyUq27sjAr8FubXMWWFrDzsVDLGt1 and if so, does it show the QR code for the public key of "1JTgVXzMCgnh2S5rowvAcuZRDj65eTxYtZ" on the outside of the wallet, or does it show the public key of "1DMjz5d324YFdpEeKhMqRWWr8uiru8KWGN"?

Because in my scenario, I see a QR code for the public key of 1TJ... and a QR code for the private key of 6PRP..., so if I funded the wallet by scanning the outer QR Code, and if I attempted to sweep the wallet with Mycelium or Samourai, I would not sweep the balance displayed. I'll retest when I get home to make sure I'm seeing what I think I'm seeing, but just by using the website, that's how it appears to me.

@praeluceo
Copy link
Author

So I guess this bug is a duplicate of pointbiz#114 really, I've worked a temporary solution where I modified a single line to display uncompressed keys:

-                       var flagByte = compressed ? 0xe0 : 0xc0;
+                       var flagByte = uncompressed ? 0xe0 : 0xc0;

This gets me closer to what I need where when the private key is scanned by a wallet, the public key that shows up on the software wallet is the same one as shows on the physical paper wallet. I know there are issues with BIP32 and uncompressed wallets, but I'm not sure what those are. My next step is to get the ability to generate uncompressed public keys, and feed pre-encrypted private/public key pairs into the generator (so I don't have to have access to the private key).

@xioustic
Copy link

xioustic commented Jul 3, 2019

This is still an issue today, which sucks because I think this is the best paper wallet generator for Bitcoin. I suppose paper wallets have become deprecated and an obscurity these days.

I have fixed it with a few lines of code but according to the README the author is not accepting pull requests? If so, are there any actively maintained forks? Or trusted alternatives?

I'm away from the computer for a week or two, but the fix is essentially (apologies for any mistakes):

  1. Modify "GenerateNewWallet" function on 11956
  2. Insert "var compressed = (new Bitcoin.ECKey(addressSeed.wifKey)).compressed" at line 11963 after the check if the option for encryption was enabled.
  3. Make sure "compressed" is passed into the third argument instead of "false" in the call on 11965 BIP38PrivateKeyToEncryptedKeyAsync.

I have not thoroughly tested this fix but it appears to have worked using a few manual test cases when generating wallets.

Per the fix, the mistake was simply not indicating whether the key was compressed to the BIP38 encrypting function and instead just passing in "false", causing the returned BIP38 to always wrap the uncompressed version of the private key.

This is a problem, as per standard I've noticed among wallets:

  • given a compressed private key, unlock and use the compressed address
  • given an uncompressed private key, unlock and use the uncompressed address
  • on unlocking a BIP38, use the returned private key per whatever format it wrapped (uncompressed or compressed)
  • thus, a BIP38 encryption scheme should wrap the compressed private key if given the compressed one, and wrap the uncompressed private key if given the uncompressed one

For reference, the current version of bitaddress.org (project that bitcoinpaperwallet was forked from) does not have this bug as far as I can tell. Perhaps it did in the past though.

As this issue is old, I will provide a verbose summary in addition to the fix above with hopes it will help others find this solution:

I've gone to recover one of my paper wallets in the past and when unlocked in a popular wallet like Mycelium it yields a completely different address (the uncompressed one). This is because BitcoinPaperWallet converts whatever private key it is given into the uncompressed one before wrapping it in BIP38. The unlocking wallet then naturally assumes the user wants the uncompressed key corresponding to the uncompressed address.

Worse yet, the user's expected public key (the compressed address) is displayed on the generated wallet so there is no indication that the conversion to the uncompressed private key occured until you go to unlock the wallet to retrieve the funds. To someone who does not understand the intricacies of Bitcoin address/key formats, they'd think the Bitcoin is then irretrievable since the paper wallet unlocks a completely different address.

The funds can be recovered by unlocking the BIP38 on bitaddress.org or in the validate feature of bitcoinpaperwallet and also clicking the "show compressed" link on that tab. The correct raw compressed private key will be next to the correct
compressed address, which can then be scanned by a standard wallet.

This issue can only happen on addresses you import into bitcoinpaperwallet yourself that you generated elsewhere. Left to its own devices, I think bitcoinpaperwallet will only ever generate uncompressed private keys which skirts the issue entirely. This makes the bug relatively obscure (only occuring for a specific use case) but no less catastrophic to the unfortunate and misinformed.

@linuxman21
Copy link

Unfortunately cantonbecker has sold his paperwallet projects to a new owner a little over a year ago and they have not done any updates since taking ownership.

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

No branches or pull requests

4 participants