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

ECC uncompressed keys are impossible to create due to faulty interface implementation #102

Open
TheQuantumPhysicist opened this issue Apr 4, 2018 · 0 comments
Labels

Comments

@TheQuantumPhysicist
Copy link
Collaborator

TheQuantumPhysicist commented Apr 4, 2018

Investigation of the code in key.cpp has revealed a bug in the code that:

  1. Makes it not possible to create uncompressed keys
  2. Makes the conversion from compressed to uncompressed keys impossible

The issue is in the class CKey, which binds directly to OpenSSL for handling private keys.

First, we have to emphasize that a private key is just an integer while a public key is a point on the elliptic curve. The notion of a compressed private key only means that the corresponding public key is going to appear compressed. Following up from Bitcoin definition:

uncompressed: 0x80 + [32-byte secret] + [4 bytes of Hash() of previous 33 bytes], base58 encoded
compressed: 0x80 + [32-byte secret] + 0x01 + [4 bytes of Hash() previous 34 bytes], base58 encoded
neblio uses 0xb5 instead of 0x80

And we notice here there's compression is just a flag. The relation of this with the following issue is still unclear. OpenSSL seems to privde a way to use a key as compressed/uncompressed EC_KEY_set_conv_form. We should mention that Bitcoin Core doesn't use this method anymore.

Taking a look at the method that creates a new key and the method that does the conversion in the current state:

void CKey::MakeNewKey(bool fCompressed)
{
    if (!EC_KEY_generate_key(pkey))
        throw key_error("CKey::MakeNewKey() : EC_KEY_generate_key failed");
    if (fCompressed)
        SetCompressedPubKey();
    fSet = true;
}
void CKey::SetCompressedPubKey(bool fCompressed)
{
    EC_KEY_set_conv_form(pkey, fCompressed ? POINT_CONVERSION_COMPRESSED : POINT_CONVERSION_UNCOMPRESSED);
    fCompressedPubKey = true;
}

The interface of MakeNewKey() looks correct as it takes a boolean that decides whether the key is compressed or not. First observation is that this boolean is not passed to SetCompressedKey, which makes it always take the default value true. Second observation is that SetCompressedKey seems to ignore the parameter fCompressed when setting the member variable fCompressedPubKey. Finally, all this works because no call in the whole code is made with SetCompressedPubKey(false), except in script.cpp, which in turn is not used anywhere.

Related:
https://en.bitcoin.it/wiki/List_of_address_prefixes
https://en.bitcoin.it/wiki/Wallet_import_format
https://www.openssl.org/docs/man1.1.0/crypto/EC_KEY_set_conv_form.html
https://www.openssl.org/docs/man1.1.0/crypto/EC_POINT_new.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant