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

Parse elliptic curve "nid" from secp256k1 public key. #21

Open
georgepadayatti opened this issue May 24, 2022 · 5 comments
Open

Parse elliptic curve "nid" from secp256k1 public key. #21

georgepadayatti opened this issue May 24, 2022 · 5 comments

Comments

@georgepadayatti
Copy link

Here there is a check to make sure the public key curve "nid" is matching - https://github.com/georgepadayatti/sslcrypto/blob/22c3bb311c291b2d1398b0b68d9e638f2ba81f59/sslcrypto/_ecc.py#L359

Question, is there a public documentation for why this is done so ? Wanted to check if there is a compatibility issue between sslcrypto and eccrypto (https://github.com/bitchan/eccrypto).

@purplesyringa
Copy link
Owner

purplesyringa commented May 25, 2022

Here there is a check to make sure the public key curve "nid" is matching

This seems like an important check. Otherwise, a maliciously crafted public key could lead to the usage of an insecure verification protocol or something. I'm not sure if there's any actual vulnerability, but I wouldn't count on it either way.

Wanted to check if there is a compatibility issue between sslcrypto and eccrypto

Why do you think there is an incompatibility? Given that sslcrypto often uses OpenSSL underhood, was tested on actual data generated and accepted by OpenSSL, and was cross-checked with some other cryptography libraries (though those checks aren't in the source tree sadly), I think it should be compatible.

@georgepadayatti
Copy link
Author

georgepadayatti commented May 25, 2022

Thanks for the reply. "nid" value returned for a secp256k1 public key came as "1158" where as it should have been "714" and therefore resulted in wrong curve exception. I will check on my end, if this is an issue with the implementation of secp256k1 public keys that I received from a 3rd party.

@georgepadayatti
Copy link
Author

@imachug

I have recreated the issue using following snippets.

Node.js code snippet to generate secp256k1 (public, private) key pair:

var eccrypto = require("eccrypto");
var EC = require("elliptic").ec;
var ec = new EC("secp256k1");

var privateKeyA = eccrypto.generatePrivate();

var pk = ec.keyFromPrivate(privateKeyA).getPublic("hex")

console.log(pk)

Output:

~/nodejs-projects/eccrypto-snippets node index.js

04f8830f92f0eb103137f18109c92da663fb38309ae0f9046a0b62fcae04d74a2dc51ecf2e5eb0c3f8ad0bf3554268f744dcc6840042088cdf108364ed73fc9803

Python code snippet to calculate nid of the public key:

from coincurve import PublicKey
import struct

pk_hex = "04f8830f92f0eb103137f18109c92da663fb38309ae0f9046a0b62fcae04d74a2dc51ecf2e5eb0c3f8ad0bf3554268f744dcc6840042088cdf108364ed73fc9803"
pk_bytes = bytes.fromhex(pk_hex)

pk = PublicKey(pk_bytes)

nid, = struct.unpack("!H", pk_bytes[0:2])

print(nid)

Output:

(venv) ➜  ~/PythonProjects/sslcrypto-snippets python main.py 

1272

The nid is not equal to 714.

Any ideas ?

@georgepadayatti
Copy link
Author

Raised the same in eccrypto repository as well. bitchan/eccrypto#88

@purplesyringa
Copy link
Owner

Wait, actually, the problem is different.

Basically, there are two common formats of public keys.

The first format is ASN. This thing includes NID as well as everything else required to specify the curve and the public key.

The second format is a format which probably doesn't have a name, is used when the exact curve is already known, and is mostly common in secp256k1 world. It's a single byte 0x04 followed by the X and Y coordinates taking 32 bytes each, thus occupying 65 bytes in total. Or occasionally it's 0x02 or 0x03 followed by the X coordinate, so 33 bytes in total.

The problem is that the public key you're using is in the second format (so pk_bytes[0:2] is not NID but 0x04 followed by the first byte of the X coordinate), and sslcrypto is trying to interpret it as if it was in ASN format. So there's no problem regarding NIDs in other libraries, there must be another reason.

According to the code, the only place where _decode_public_key_openssl is used (this method assumes ASN) is when decrypting ECIES-encoded data. It would be very useful to know which library was used to encrypt the data in the first place. If it is indeed confirmed that the format of the ciphertext is <iv><raw ephemeral public key><encrypted data> rather than <iv><ASN ephemeral public key><encrypted data>, I'll be able to add support for that in a minute.

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

2 participants