EOSIO / eosjs-ecc

Elliptic curve cryptography functions: Private Key, Public Key, Signature, AES, Encryption, Decryption
288 stars 119 forks source link

PrivateKey.fromWif not support compressed WIF format #16

Closed zhjjj closed 6 years ago

zhjjj commented 6 years ago

It seems that PrivateKey.fromWif doesn't support WIF with compressed format. When I use a WIF private key with compressed format, some warning shows up: WARN: Expecting 32 bytes, instead got 33, stack trace: Error And I can get a result from the function. However, the result is not correct.

So I read the souce code. PrivateKey.fromWif will generate a BigInteger from buffer generated from WIF format. So if I provide a compressed WIF which has an extra compressed flag byte, it will use the 33 bytes data to generate the private key. But it is not my original private key.

jcalfee commented 6 years ago

I see what your talking about in the Bitcoin spec.. I'll look into it.

jcalfee commented 6 years ago

So far, we have used WIFs and compressed "public keys" without adding this 0x01 byte on the end of the private key WIF. This "compressed" flag applies to the public key though (not the private key). It seems to be more like a friendly / helpful indicator (to the best of my knowledge).

The 0x01 compression flag is under the checksum and the length is of the private key matches so I think it is safe to remove this and create the correct private key. It is reasonable that a library would create a WIF and add this 0x01 compression flag, this is just a change in how keys are being created in the libraries so far.

@chris-allnutt what do you think: I can just remove and ignore this compression byte if i see it (no warning no error).

jcalfee commented 6 years ago

published eosjs-ecc@4.0.1

RayMetz100 commented 6 years ago

Three BP key checker tools accepted compressed K/L private keys and gave a consistent private key with this 0x01 bug that didn't match the registered public key after EOS launch. When I enter my same prelaunch private key to cleos now, it gives me a different public key and my funds are stranded on the registered public key.

Was this 0x01 issue applied before or after the SHA256 hash? If it was applied before, then it may be possible to take my private key and convert it to a different private key that will match the patched software. If the extra byte issue was after the hash, then there's no way to create a matching private key and the public key is a burn address. which side of the SHA256 hash is this bug on?

jcalfee commented 6 years ago

This change is very safe. It allowed additional valid private keys (more spec like) to get parsed. There is no burn address possible.

RayMetz100 commented 6 years ago

I have four compressed private keys I generated with iancoleman/bip39. I put them in the pre-launch key checker tools and had the tools hash me matching public keys. I registered the public keys with 170 eos. Pre launch. Today I can still put my same private keys into the key checker tools and generate the same matching public keys that I registered, because the key checkers still have the 0x01 bug. I can see the migrated EOS balance now on those registered public keys.

Because I generated public keys having the bug, and cleos does not have the bug, when I put my same compressed private keys into cleos, I get different public keys and no access to my funds.

Is there a way to convert my four private keys to patched private keys that can be entered into cleos and hash to my registered public keys?

Repro steps... Generate any test compressed private key with leading K or L. Put it into any key checker tool and generate a matching bugged public key. Transform the private key somehow to adjust for the 0x01 bug. Enter the revised private key into cleos. Cleos gives the matching public key that the bugged key checkers did.

How can I transform my private keys to get around the bug?

jcalfee commented 6 years ago

Do you have a web tool for this?

Generate any test compressed private key with leading K or L.

Please tell me which one your using:

Put it into any key checker tool and generate a matching bugged public key.

RayMetz100 commented 6 years ago

All of these still have the same bug https://github.com/eoscafe/eos-key or https://github.com/webdigi/EOS-Offline-Private-key-check or https://github.com/eosamsterdam/eos-keypair-check

This will give you lots of test private keys with leading K or L. Try importing them into cleos and the key checker tools above to see the different public keys generated for the same private keys. https://iancoleman.io/bip39/?

If you want to convert the same private key to compressed leading K or L vs. uncompressed leading 5 use bit address.org and choose "Wallet Details" option.

Example: https://www.bitaddress.org/bitaddress.org-v3.3.0-SHA256-dec17c07685e1870960903d8f58090475b25af946fe95a734f88408cef4aa194.html

jcalfee commented 6 years ago

Thanks for the links .. I am just looking for 1 case.. All private keys start with "5" .. I'm not sure what your saying.. This one gives some test keys but it imports and converts just fine:

https://github.com/webdigi/EOS-Offline-Private-key-check

$ 5JS9bTWMc52HWmMC8v58hdfePTxPV5dd5fcxq92xUzbfmafeeRo
$ cleos wallet import -n pkcheck
private key: imported private key for: EOS8HuvjfQeUS7tMdHPPrkTFMnEP7nr6oivvuJyNcvW9Sx5MxJSkZ
RayMetz100 commented 6 years ago

Both cleos and all three private key checkers accept the K/L formatted compressed private keys too. Notice when you use any K/L private key, the public key between cleos and the key checkers becomes different. I thought this was the 0x01 bug you were describing, but maybe not.

Importing any uncompressed leading 5 private key gives the same public key in both the key checkers and cleos so there is no issue. The bug is only when compressed K/L private keys are given to the three key checkers.

How did I know that the key checkers are wrong and cleos is right? Because I used bitaddress.org to convert the same private key to both private key formats and tested those. Cleos takes the same private key in both compressed and uncompressed formats and hashes them to the same public key. The three private key checker tools give a public key matching cleos when uncompressed/5 style private keys are used, but different public keys when compressed K/L private keys are used. The bug is only in the three BP key checkers.

jcalfee commented 6 years ago

@RayMetz100 I was able to recreate the issue. The public key is not necessarily a burn address. I'm still researching this and we will not know for sure until you can change your keys or transfer the funds.

Recent versions of eosjs-ecc are fixed (after the 0x01 commit above).


I was able to get the correct public key in recent versions of eosjs and cleos using a test key:

$ cleos wallet import -n compressedpk --private-key L5TCkLizyYqjvKSy6jg1XM3Lc4uTDwwvHS2BYatyXSyoS8T5kC2z
imported private key for: EOS59DAgjLtnPY34ezGspqdezuNaES2HE99dcsay3uRjk557F2Hd9

image

As of today, I see EOS-Offline-Private-key-check does use a version of eosjs-ecc with this bug: image

Prior to this change above (the check for 0x01) eosjs displayed a warning but returned the wrong public key. This is shown in the console log. So, if you see this WARN then eosjs-ecc is creating the wrong private key for compressed K/L keys: image

This is the same bug in code:

> ecc.PrivateKey('L5TCkLizyYqjvKSy6jg1XM3Lc4uTDwwvHS2BYatyXSyoS8T5kC2z').toPublic().toString()
WARN: Expecting 32 bytes, instead got 33, stack trace: Error
    at Function.PrivateKey.fromBuffer (/home/james/eosjs/ecc/src/key_private.js:168:90)
    at parseKey (/home/james/eosjs/ecc/src/key_private.js:146:35)
    at Function.PrivateKey.fromString (/home/james/eosjs/ecc/src/key_private.js:226:12)
    at Object.PrivateKey (/home/james/eosjs/ecc/src/key_private.js:29:27)
    at repl:1:5
    at Script.runInThisContext (vm.js:91:20)
    at REPLServer.defaultEval (repl.js:321:29)
    at bound (domain.js:396:14)
    at REPLServer.runBound [as eval] (domain.js:409:12)
    at REPLServer.onLine (repl.js:619:10)
'EOS6ig7VSsHLtt8BDkVV39HYAQsdNUwwhY1SPJBUBEA4NK8Zpkz2y'

This is how to apply the fix for EOS-Offline-Private-key-check:

~/EOS-Offline-Private-key-check-master$ wget https://cdn.jsdelivr.net/npm/eosjs-ecc@4.0.2/lib/eosjs-ecc.js -O ecc.js

# Check the hash here -> https://github.com/EOSIO/eosjs-ecc/releases/tag/v4.0.2
$ npx srisum ecc.js 
npx: installed 53 in 1.286s
sha512-HCSw4GhpVw/3DS2n9Hdq7hVAw3nheZzYddAJsKEK7wk57LsfnI9oNHHNHzG+e+BWqWKxKPCM+XMxBTV+TE2QYw== ecc.js

# The reference eos_ecc changed to eosjs_ecc .. This needs to be changed: 
$ egrep eos_ecc . -r
./index.js:  var {PrivateKey, PublicKey} = eos_ecc

# note: sed may not have a -i (in-place replace) option
$ sed -i 's/eos_ecc/eosjs_ecc/g' ./index.js

# now it should look like this:
$ egrep eosjs_ecc . -or
./ecc.js:eosjs_ecc
./index.js:eosjs_ecc

NOTE: I used the non-minimized version for ecc.js only because that matches the version out there now. The minimized version could be used too, the srisum hash will be different.

After the change, the correct public key is shown: image

I'll continue to work on a tool. If all goes well, we can use that private key in a different way to make a passing signature.

jcalfee commented 6 years ago

I reported the issue to: https://nodesecurity.io .. Hopefully they will make one of those npm audit reports soon.

jcalfee commented 6 years ago

When upgrading, please use the script integrity hash attribute.. In new releases, hashes are published here: https://github.com/EOSIO/eosjs-ecc/releases.

Examples:

<script src="./eosjs-ecc.min.js"
  integrity="sha512-5esVQ4sQT8XLWek5KUEIQbFfxu2EZdet8DNDkIdchp4Y1e0b+xkMozwUdFii5bJa3Zk/vgr1ZP235Uf9ULCrkA=="
  crossorigin="anonymous"></script>
<script src="https://cdn.jsdelivr.net/npm/eosjs-ecc@4.0.2/lib/eosjs-ecc.min.js"
  integrity="sha512-5esVQ4sQT8XLWek5KUEIQbFfxu2EZdet8DNDkIdchp4Y1e0b+xkMozwUdFii5bJa3Zk/vgr1ZP235Uf9ULCrkA=="
  crossorigin="anonymous"></script>
RayMetz100 commented 6 years ago

Thanks a lot James. I'll hold on to my old private keys in case they are useful someday. Please make sure all three key checkers from the three BPs get your update script integrity logic.

All of these still have the same bug https://github.com/eoscafe/eos-key https://github.com/webdigi/EOS-Offline-Private-key-check https://github.com/eosamsterdam/eos-keypair-check

There are likely others. I'm not sure if I ever heard of another user like me who used Bitcoin private keys rather than EOS keys. I had an airgap machine with iancoleman/bip39 already and thought this would be more secure. So secure I was locked from my funds, lol.

jcalfee commented 6 years ago

I have built a tool that tries to recover an account like yours on the testnet. I had partial success validating a signature with one of these keys but it only worked in eosjs-ecc. The signature is still being rejected by the testnet.

https://github.com/jcalfee/eosjs-ecc-compression-tweak

RayMetz100 commented 6 years ago

I'm trying to understand and isolate a solution with pseudo-code..

Bugged code: Accept compressed private key Uncompress private key Bug modifies private key Hash modified key Hash function outputs public key that does not match cleos for same private key input.

What I want is the modified uncompressed private key, from just before the hash.

Is there a way to find the hash function in the bugged code, then just before the hash function, output the hash function input to the debug console?

If I input my private key, and run the above, I could see a converted bugged private key output just before it hashes. The public key would be the same as what I registered pre launch, but the debug output would be a corrected different matching private key.

Next I would see if I can either use that converted private key directly in cleos, or if it needs to be converted to the standard leading 5 uncompressed format somehow. In the end, I'd like to make a private key compatible with cleos directly and not have to send transactions through a new tool.

I'm not sure if that's possible or if I must send transactions through a new tool. Any way I can access my funds and maybe help others access theirs is awesome.

jcalfee commented 6 years ago

All that happens here: https://github.com/jcalfee/eosjs-ecc-compression-tweak/blob/1da4b1f4a67b8a836be3524a6f00a2394a4ded39/index.html#L66-L74

RayMetz100 commented 6 years ago

Would this give me a private key that i can import into cleos and cleos will show the same public key that I registered? I'm not home at the moment, but I will give it a try with a new bugged example keypair first.

wif = 'L5TCkLizyYqjvKSy6jg1XM3Lc4uTDwwvHS2BYatyXSyoS8T5kC2z'

ecc = Eos.modules.ecc

unintendedPrivate = ecc.PrivateKey(privateCompressionTweak(wif))

console.info('Corrected Private Key', unintendedPrivate)

RayMetz100 commented 6 years ago

This is the code I modified to turn my bad private key and registered public key, into a working private key with same registered public key. I haven't tested moving real funds yet, but it looks good.

  1. download and unzip https://github.com/jcalfee/eosjs-ecc-compression-tweak/tree/1da4b1f4a67b8a836be3524a6f00a2394a4ded39

  2. copy the below html into a new file named showkeys.html, save in the same folder as eos.js and index.js.

  3. open showkeys.html and press F12 in windows to view the console output.

  4. test the keys with the old bugged key checkers. When you are ready, modify showkeys.html (the code below) with your real bad private key and attempt to get a working private key that will unlock your funds. I'm trying these steps myself now and will post if it works or not.

edit: this did not work. when I put my LK compressed private key into a bugged key checker, it does give me the same unintended public key as the code below. what I want is another "debugged" private key that gives the same public key output as the bugged one did.

question: Do all LK compressed private keys give "bugged" public keys when running in the old bugged eosjs-ecc software? Do all 5-uncompressed private keys give "good" public keys when running in the old bugged eosjs-ecc software?

<html>
<meta charset="utf-8">
<title>Eos Private Key Compression Tweak</title>
<head>

<script src="./eos.js"></script>
<script src="./index.js"></script>

<script>
inputKey = 'L5TCkLizyYqjvKSy6jg1XM3Lc4uTDwwvHS2BYatyXSyoS8T5kC2z'
ecc = Eos.modules.ecc

unintendedPrivate = ecc.PrivateKey(privateCompressionTweak(inputKey))
displayUnintendedPrivate = unintendedPrivate.toString()
console.info('Unintended Private Key', displayUnintendedPrivate)

unintendedPublic = unintendedPrivate.toPublic().toString()
console.info('Unintended Public Key', unintendedPublic)

intendedPrivate = ecc.PrivateKey(inputKey)
displayIntendedPrivate = intendedPrivate.toString()
console.info('Intended Private Key', displayIntendedPrivate)

intendedPublic = intendedPrivate.toPublic().toString()
console.info('Intended Public Key', intendedPublic)
</script>

</head>
<body>
  Test running..  Open the Web Inspector (Console) to view the output..
</body>
</html>
RayMetz100 commented 6 years ago

ecc.PrivateKey(privateCompressionTweak(inputKey)) returns the same input private key.

Can we make a new NewMatchedPrivateKey() function that will accept privateCompressionTweak(inputKey), but return a working non bugged private key that matches the the same public key as unintendedPrivate.toPublic()?

I need some time to understand the below code and see if there are any clues on how to generate a non-bugged private key that would have the same public key as my bugged public key.

Thanks for all your help. I feel its getting really close. Before I had lost all hope other than the BPs and 911 being nice to me.

function privateCompressionTweak(wif) { const buffer = Buffer.from(bs58.decode(wif))

const checksum = buffer.slice(-4) const versionKey = buffer.slice(0, -4)

const sha256 = data => crypto.createHash('sha256').update(data).digest() const newCheck = sha256(sha256(versionKey)).slice(0, 4)

if (checksum.toString() !== newCheck.toString()) { throw new Error('Invalid checksum, ' + ${checksum.toString('hex')} != ${newCheck.toString('hex')} ) }

jcalfee commented 6 years ago

question: Do all LK compressed private keys give "bugged" public keys when running in the old bugged eosjs-ecc software? Do all 5-uncompressed private keys give "good" public keys when running in the old bugged eosjs-ecc software? yes and yes

There is probably no point in getting an unattended private key out, it is not valid anywhere else.

The only way I see to fix this, is to create a signature that will get accepted by the network. That is going to involve getting into the signature generation and checking algorithms. I did have an open PR in eosjs-ecc that helped but did not get me far enough to have the signature accepted. Maybe someone that knows this better than I can help or it is going to take me some time to understand it..

You can't fix it anymore with key formats, you have to get into the ecc math and signature generation and signature checking.

jcalfee commented 6 years ago

I'll be happy to help you with the BPs and 911. With the tweak you can prove you own the account. I was able to do a recovery in eosjs-ecc using the unintended key. We could probably use that so you could prove ownership by your signing with the buggy key.

It is better that they change the keys rather than do a fund raiser because a fix could come out later where this unintended key might actually work.

We let several people know all about this issue so they can be on the lookout other people that may have been affected.

RayMetz100 commented 6 years ago

I registered new keypairs with EOS911 to these 4 pre-launch accounts:

https://etherscan.io/address/0x8ce4f9605510b965c13da14b58b3885f24e777b1 https://etherscan.io/address/0x0768fdb45bf5299e1573713ddebbb34fa8e0eabd https://etherscan.io/address/0x782579f066badf7a3cf943b51f8b79bf696e17cc https://etherscan.io/address/0xf1cb1fb0e61d526747504d6d29f9e7ffa6bed2c6

My new EOS keypars all have leading 5s and were generated from cleos. Let me know if this is sufficient to prove my ownership or if I need to do something with my original bad KL compressed private keys too.

jcalfee commented 6 years ago

I was able to recover the account on the junglenet .. The account updated and the keys were changed to the intended "non-buggy" public key.

The signing algorithm has a nonce counter that creases potentially several signatures then stops at the first one it likes. I added that code and changed the nonce so it started with zero instead of one. I was going to try nonce values by hand for a while to see if I could find one that would work. The very first try, nonce=0 worked. ;) .. So you might want to give this a try. If the nonce=0 does not work you can run updateAuths() in the web console to try again with a higher nonce or look at the log and see the last "nonce" it tried then take that value +1 and add a hash nonce eq value like this: ../index.html#nonce=?

https://github.com/jcalfee/eosjs-ecc-compression-tweak/blob/4f8506b8269f5f50f5d79fa57dd7beef302b3b23/index.html#L94-L95

RayMetz100 commented 6 years ago

Awesome. I'll give it a try.

RayMetz100 commented 6 years ago

Thanks so much. I just recovered this account on mainnet. https://eosflare.io/account/g43donzqgmge It worked first time and I didn't have to adjust the nonce.

I will try my other three.

RayMetz100 commented 6 years ago

All my accounts worked first try. I can log into all of them with Scatter. All airdrops are there. Since I rebought on Binance, now I have double the EOS. woot! Thank you so much!!!