Skip to content

Support P-256 (secp256r1) #1083

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

Closed
wants to merge 1 commit into from
Closed

Support P-256 (secp256r1) #1083

wants to merge 1 commit into from

Conversation

uint
Copy link
Contributor

@uint uint commented Sep 8, 2021

Closes #1058.

I started working on it and hit a little bit of a blocker. I'm not a cryptography pro. Open questions I have:

  • The p256 crate doesn't seem to provide a way to recover a public key like the k256 one does. I assume we're not providing that functionality?
  • (This is the blocker.) It doesn't look like it's possible to normalize a p256 signature like it is for k256. This should be possible, right? Is this something to raise with p256 authors? Edit: Normalize NistP256 signature RustCrypto/signatures#341
  • Are p256 signatures also going to be 64 bytes? I get 71 or 72 using online tools or openssl, but I'm assuming this is before they're normalized?

@maurolacy
Copy link
Contributor

maurolacy commented Sep 8, 2021

I started working on it and hit a little bit of a blocker. I'm not a cryptography pro. Open questions I have:

  • The p256 crate doesn't seem to provide a way to recover a public key like the k256 one does. I assume we're not providing that functionality?

I assume so. Good to raise an issue / ask the crate maintainers about it.

Good find. This is pretty recent, and from the comments it seems they are going to add this normalization soon. I would just put a TODO here as a reminder, instead of implementing this normalization ourselves. In any case, if implementing let's try adding it / pushing it into the p256 crate.

  • Are p256 signatures also going to be 64 bytes? I get 71 or 72 using online tools or openssl, but I'm assuming this is before they're normalized?

AFAIK, normalizing wouldn't change the signature length. I have no idea about the signature lengths for secp256r1. This depends a lot on the formats we're using for the input data, and the specific scheme / impl we want to support / be compatible with. @webmaster128 (or @pluscubed perhaps; issue #1058 creator) might be able to help you better here.

@webmaster128
Copy link
Member

Are p256 signatures also going to be 64 bytes? I get 71 or 72 using online tools or openssl, but I'm assuming this is before they're normalized?

70/71/72 bytes indicate a DER encoding of variable length integers (r, s) instead of fixed length encoding 32 bytes r and 32 bytes s. See https://github.com/cosmos/cosmjs/blob/v0.26.0/packages/crypto/src/secp256k1signature.spec.ts#L102-L133

@uint
Copy link
Contributor Author

uint commented Sep 8, 2021

70/71/72 bytes indicate a DER encoding of variable length integers (r, s) instead of fixed length encoding 32 bytes r and 32 bytes s. See https://github.com/cosmos/cosmjs/blob/v0.26.0/packages/crypto/src/secp256k1signature.spec.ts#L102-L133

Oh, good! That's a clue. Thanks.

@webmaster128
Copy link
Member

In the Cosmos ecosystem 2x32 bytes fixed length encoding is predominant. And if I recall correctly Bitcoin regrets to have picked DER for historic reasons.

@webmaster128
Copy link
Member

The p256 crate doesn't seem to provide a way to recover a public key like the k256 one does. I assume we're not providing that functionality?

Recovering the public key from signature + message seems to be an algorithm that works for ECDSA over all curves. At least this is stated here: https://bitcointalk.org/index.php?topic=5183471.msg52417717#msg52417717. The algorithm described here also makes no restrictions on the curve: https://crypto.stackexchange.com/a/18106. So it should be possible to implement and is worth a feature request.

If we need it is a different story. In Cosmos, the public key is always stored on chain. For Ethereum this is not the case. There only 20 byte addresses are stored instead of 33 byte pubkeys. In order to verify a transaction signature you need to recover the pubkey from signature+message first before the actual verification can be done. But Ethereum does not have secp256r1.

(This is the blocker.) It doesn't look like it's possible to normalize a p256 signature like it is for k256. This should be possible, right? Is this something to raise with p256 authors? Edit: Normalize NistP256 signature RustCrypto/signatures#341

Great insight, thanks. I consider this feature blocked because of that.

@ethanfrey
Copy link
Member

Note, I saw the linked issue "Normalize NistP256 signature". Tony, who answered the questions, is relatively well known in Cosmos, and works with Zaki. I hit him up with crypto questions from time to time on discord. If you want an intro, I can make one.

@ethanfrey
Copy link
Member

Also, Regen is adding secp256r1 support to the Cosmos SDK (maybe they already did in 0.44?). I would focus on compatibility with their design unless there is another strong driving force behind this.

@webmaster128
Copy link
Member

Currently not planned. Maybe we can re-use this work later. Cosing for now. Thanks!

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

Successfully merging this pull request may close these issues.

Support Secp256r1 signature verification
4 participants