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

Elliptic curve point compression #32

Closed
clehner opened this issue Mar 4, 2021 · 17 comments
Closed

Elliptic curve point compression #32

clehner opened this issue Mar 4, 2021 · 17 comments

Comments

@clehner
Copy link
Member

clehner commented Mar 4, 2021

The public keys in the test vectors for Secp256k1 are in compressed form (33 bytes), but for P-256 they are uncompressed (64 bytes, untagged). For Ed25519 they are compressed (32 bytes).

  • Should P-256 use compressed form instead? The smaller size in a URI may be desirable, and this would be consistent with the use of Secp256k1 and Ed25519.
  • Should did:key allow either form - compressed or uncompressed - for these key types? Or only a single form is allowed? Or either form allowed but one is preferred?
@OR13
Copy link

OR13 commented Apr 27, 2021

imo, did-key method spec should probably stick to compressed only... the main reason the NIST curves are uncompressed is that they are commonly represented uncompressed... I did the first version of them and now @troyronda and SecureKey have support for them as well...

I am not sure how hard it would be to convert did key for nist curves to compressed, but if we can, we should IMO.

@msporny
Copy link
Contributor

msporny commented Apr 27, 2021

  • Should P-256 use compressed form instead? The smaller size in a URI may be desirable, and this would be consistent with the use of Secp256k1 and Ed25519.

Yes, because the smaller size URI is a desirable feature.

  • Should did:key allow either form - compressed or uncompressed - for these key types? Or only a single form is allowed? Or either form allowed but one is preferred?

Let's just stick to compressed form, I don't see a need to support both.

@dlongley
Copy link
Contributor

dlongley commented Apr 27, 2021

Some links on P-256 compressed form:

https://tools.ietf.org/html/rfc5480
https://tools.ietf.org/id/draft-jivsov-ecc-compact-05.html
https://stackoverflow.com/a/30431547/399274

Looks like you simply add the last bit of the Y coordinate to 0x02 (used as the prefix value) such that the prefix will be either 0x02 or 0x03 and then you follow that with the X coordinate and that's it (the rest of the Y coordinate is dropped).

@OR13
Copy link

OR13 commented Apr 27, 2021

One issue we have is that we only support did key variants that have some JWK representation, so we won't implement support for this until we can easily convert back and forth.... because we like interoperability with off the shelf JOSE libraries.

I will need to take a look at bi-directional transformations to be sure its possible to support.

@bshambaugh
Copy link
Contributor

bshambaugh commented Apr 27, 2021

+1 Orie. I'm presently contemplating building a function that takes both the compressed and uncompressed versions and produces a JWK representation. It would be extra code to compress, send over the wire, then decompress but perhaps that could be better.

edit: I found a few links both today and yesterday: https://crypto.stackexchange.com/questions/8914/ecdsa-compressed-public-key-point-back-to-uncompressed-public-key-point , https://bitcoin.stackexchange.com/questions/69315/how-are-compressed-pubkeys-generated , https://stackoverflow.com/questions/53629537/how-to-generate-33-byte-compressed-nist-p-256-public-key ,https://stackoverflow.com/questions/6665353/is-there-a-standardized-fixed-length-encoding-for-ec-public-keys

I believe this contains the definition of the P-256 and other NIST curves: https://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.186-4.pdf

edit (5/02/2021): https://bitcoin.stackexchange.com/questions/92680/what-are-the-der-signature-and-sec-format , https://en.wikipedia.org/wiki/Endianness, https://tools.ietf.org/html/rfc5480 , https://www.secg.org/sec1-v2.pdf (see: 2.3.3) , https://www.secg.org/sec2-v2.pdf

@oed
Copy link

oed commented Apr 29, 2021

You can use a library such as elliptic to convert keys back and forth between compressed and uncompressed. Although, it's a big dependency that would be nice to avoid if possible!

let ec = new require('elliptic').ec('p256')
undefined
> ec.genKeyPair().getPublic(true, 'hex')
'03413029cb9a5a4a0b087a9b8a060116d0d32bb22d14aebf7778215744811bb6ce'
> pub = ec.keyFromPublic('03413029cb9a5a4a0b087a9b8a060116d0d32bb22d14aebf7778215744811bb6ce', 'hex').getPublic(false, 'hex')
'04413029cb9a5a4a0b087a9b8a060116d0d32bb22d14aebf7778215744811bb6ce40780d7bb9e2e068879f443e05b21b8fc0b62c9c811008064d988856077e35e7'

@bshambaugh
Copy link
Contributor

I have some code over here that supports raw public keys, compressed public keys, and uncompressed public keys for P-256. ceramicnetwork/js-ceramic#1418

@OR13
Copy link

OR13 commented Jun 1, 2021

Anyone on this thread want to take a stab at updating the test vectors in this repo to use the compressed from for P-256, P-384, P-521?

just edits to the JSON files.

@bshambaugh
Copy link
Contributor

bshambaugh commented Jun 1, 2021

I will look at it.

I can use (or slightly modify) this [1] to create test vectors (given the raw or uncompressed public key input):
[1] https://github.com/bshambaugh/key-did-provider-secp256r1/blob/main/src/index.ts + source extra curves from :
https://www.npmjs.com/package/elliptic or the existing test vectors.

By changing about 3 lines of code I believe that this could be written for P-384 and P-521.
https://github.com/bshambaugh/js-ceramic/blob/develop/packages/key-did-resolver/src/secp256r1.ts

@clehner
Copy link
Member Author

clehner commented Jun 2, 2021

I get the following for the P-256 one:

did:key:zDnaerx9CtbPJ1q36T5Ln5wYt3MQYeGRG5ehnPAmxcf5mDZpv

@bshambaugh
Copy link
Contributor

bshambaugh commented Jun 2, 2021

did:key:zDnaeUKTWUXc1HDpGfKbEK31nKLN19yX5aunFd7VK1CUMeyJu
did:key:zDnaerx9CtbPJ1q36T5Ln5wYt3MQYeGRG5ehnPAmxcf5mDZpv

Yes. I had one that looked similar in my tests. The length after the key: is 49 for both yours and mine.

@bshambaugh
Copy link
Contributor

bshambaugh commented Jun 2, 2021

Okay. I started a pull request: #36 . I'll look at this again tomorrow when more awake.

@bshambaugh
Copy link
Contributor

I get the following for the P-256 one:

did:key:zDnaerx9CtbPJ1q36T5Ln5wYt3MQYeGRG5ehnPAmxcf5mDZpv

Awesome. Thanks. I am got the same thing.

@OR13
Copy link

OR13 commented Sep 23, 2021

I think this is done, recommend closing.

@oed
Copy link

oed commented Sep 23, 2021

👍
Note that multicodec table also have been updated to explicitly state that keys should be compressed: https://github.com/multiformats/multicodec/blob/master/table.csv#L136-L138

@bshambaugh
Copy link
Contributor

bshambaugh commented Sep 23, 2021 via email

@OR13
Copy link

OR13 commented Sep 28, 2021

@bshambaugh please consider raising a separate issue for that if you find anything wrong with P-521...

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

No branches or pull requests

6 participants