Astronomical issue with private keys to sign tx.

[chrisforrester]

secret seed is "erupt consider beyond twist bike enroll you salute weasel emerge divert hundred"

with keystore password "password"
with hdPath = "m/44'/60'/0'"

the private key that is returned is a 62 character hex value.

example code:
(using lightwallet.keystore.deriveKeyFromPassword(password, function(err, pwDerivedKey))

var keystore = new lightwallet.keystore(secretSeed, pwDerivedKey);
var hdPath = "m/44'/60'/0'"; //as defined in SLIP44

keystore.addHdDerivationPath(hdPath, pwDerivedKey, {curve: 'secp256k1', purpose: 'sign'});
keystore.generateNewAddress(pwDerivedKey, 1, hdPath); //Generate a new address

var incompleteAddress = keystore.getAddresses(hdPath)[0];
keystore.setDefaultHdDerivationPath(hdPath);
var hexSeedETH = keystore.exportPrivateKey(incompleteAddress, pwDerivedKey);

..

hexSeedETH.length = 62.

something like a:

function pad(num, size) {
var s = num+"";
while (s.length < size) {
s = "0" + s;
}
return s;
}

with a:

hexSeedETH = pad(hexSeedETH, 64);

returns valid private keys that can sign for this address for the ethereumjs-tx module, and this library's signtx function.

[coder5876]

Wow, that's not good, thanks for the catch! Might be related to converting from Uint8Array to hex string, will check it out right now.

[coder5876]

Hi @chrisforrester, I did a test and was able to generate the same address from the truncated (31 byte) private key as from the padded (32 byte) private key. So this is good and means that logically the system treats both keys the same. I think the error occurs in the conversion between base64 and hex, I'll implement a padding scheme.

[chrisforrester]

@christianlundkvist yep :) it was a longshot to have caught it this early.

[coder5876]

@chrisforrester: Yeah, thanks so much for the help! 😄 Very bad bug, I was extremely concerned because it would have been extremely bad if it had generated a different address from the truncated private key, basically causing people to send their Ether into a black hole.

This incident is actually a good reason for switching to using a scheme like Buffer or Uint8Array for keys.

[coder5876]

@chrisforrester: Interestingly the 31 byte private key is generated by the bitcore HD tree library, so there doesn't seem to be an error in the base64 or Uint8Array conversion as I originally thought. So this is not something that had crept in in the latest refactoring as I thought, this has been there from the start.

So logically the private key is correct, and when I use the elliptic module to import the private key like this

  var keyPair = ec.genKeyPair();
  keyPair._importPrivate(privKey, 'hex');

it correctly infers that it is a private key and generates the correct public key and address. But when the same private key is used in the signTx function, then I get

  var sig = ecdsa.signSync(msgHash, privateKey)
                  ^
RangeError: secret key length is invalid

from ethereumjs-tx. So the correct place to put the padding is when the key is returned from the bitcore library. Maybe I'll even submit a pull request to the bitcore library and see if they want to make sure that their keys are always 32 bytes.

[chrisforrester]

neat! so I'm curious, if you're good with the math, what is the chance that
this issue would appear.

On Wed, Feb 24, 2016 at 10:41 PM, Christian Lundkvist <
notifications@github.com> wrote:

@chrisforrester https://github.com/chrisforrester: Interestingly the 31
byte private key is generated by the bitcore HD tree library, so there
doesn't seem to be an error in the base64 or Uint8Array conversion as I
originally thought. So this is not something that had crept in in the
latest refactoring as I thought, this has been there from the start.

So logically the private key is correct, and when I use the elliptic
module to import the private key like this

var keyPair = ec.genKeyPair();
keyPair._importPrivate(privKey, 'hex');

it correctly infers that it is a private key and generates the correct
public key and address. But when the same private key is used in the
signTx function, then I get

var sig = ecdsa.signSync(msgHash, privateKey)
^
RangeError: secret key length is invalid

from ethereumjs-tx. So the correct place to put the padding is when the
key is returned from the bitcore library. Maybe I'll even submit a pull
request to the bitcore library and see if they want to make sure that
their keys are always 32 bytes.

—
Reply to this email directly or view it on GitHub
#60 (comment)
.

[coder5876]

@chrisforrester: This should occur roughly once every 256 private keys. So it's strange that we've never seen it before. It could also be that ethereumjs-tx recently introduced stricter checks for this, it might have zero-padded private keys before potentially which would have made things work earlier. Anyway, this was a good find, I'll add it to the test suite after I add a padding in the _generatePrivKeys() function.

[coder5876]

Fixed in 97985df.

[grayl0921]

secret seed is "erupt consider beyond twist bike enroll you salute weasel emerge divert hundred"

with keystore password "password" with hdPath = "m/44'/60'/0'"

the private key that is returned is a 62 character hex value.

example code: (using lightwallet.keystore.deriveKeyFromPassword(password, function(err, pwDerivedKey))

var keystore = new lightwallet.keystore(secretSeed, pwDerivedKey); var hdPath = "m/44'/60'/0'"; //as defined in SLIP44

keystore.addHdDerivationPath(hdPath, pwDerivedKey, {curve: 'secp256k1', purpose: 'sign'}); keystore.generateNewAddress(pwDerivedKey, 1, hdPath); //Generate a new address

var incompleteAddress = keystore.getAddresses(hdPath)[0]; keystore.setDefaultHdDerivationPath(hdPath); var hexSeedETH = keystore.exportPrivateKey(incompleteAddress, pwDerivedKey);

..

hexSeedETH.length = 62.

something like a:

function pad(num, size) { var s = num+""; while (s.length < size) { s = "0" + s; } return s; }

with a:

hexSeedETH = pad(hexSeedETH, 64);

returns valid private keys that can sign for this address for the ethereumjs-tx module, and this library's signtx function.

Bitcoin main chain