feat(key-did-resolver): secp256r1 support

[oed]

To be merged when we want to upgrade to ES2020.

• commit a temporary file called backup

• rebase to april 9th, and bring in desireable changes to may19th

• remove tests for null and related functions because they are not needed

• Add "bigint-mod-arith": "^3.0.0" to key-did-resolver package.json

• add a try and finally blocks for pubKeyBytesToXY add tests to show errors for functions that are dependencies

• removed try and finally blocks from secp256r1.ts

• refactored code to get linter to pass

• simplify TypeError conditional in pubKeyBytesToXY since null loosely equals undefined

• add additional tests for malformed did:keys , add comments to secp256r1.ts code

• move test for functions in the test file to the end. Clean up and add additional tests to null. Assume this covers undefined.

• add tests for undefined case for functions declared for use in test file. Fix functions to allow undefined cases to pass.

• modify js-ceramic tsconfig.json to include target of ES2020 to allow for support of BigInt in package key-did-resolver

• remove old target for ES2018 js-ceramic in main tsconfig.json, and replace with ES2020 (removing duplicates)

• remove target: ES2020 from key-did-resolver package, since it is redundant with parent js-ceramic tsconfig.json

• first run at adding comments to src/secp256r1.ts . Incomplete.

• added documentation annotations for src/secp256r1.ts. May rewrite after forcing functions to throw custom errors for null and undefined

• updated code and tests

• updated the tests and code for more specific error responses. This is a work in progress.

• update tests and code for publicKeyIntToUint8ArrayPointPair function

• updated tests to pass

• updated publicKeyIntToUint8ArrayPointPair and publicKeyIntToXY parameter description since redundant with error message

• add function with tests to test to see if input is a Uint8Array : testUint8Array

• update testUint8Array with typing info to pass linter test

• added function to test for hex string

• update publicKeyHexToUint8ArrayPointPair with better error checking

• updated publicKeyToXY to handle only hex strings and throw and error otherwise

• update pubKeyBytesToHex to only allow for Uint8Arrays and throw and error if otherwise

• Add error checking for ECPointDecompress so it only allows Uint8Arrays

• Add error checking for publicKeyBytesToXY so it only allows Uint8Arrays

• Refactor publicKeyIntToXY and publicKeyIntToUint8ArrayPointPair in sec/secp256r1.ts

• refactored logic in secp256r1.ts NOT(A AND B) = NOT(A) OR NOT(B)

[bshambaugh]

The other option to avoid ES2020 I think is to use a bigger library that was a polyfill before ES2020
: https://github.com/peterolson/BigInteger.js .

[bshambaugh]

The other option to avoid ES2020 I think is to use a bigger library: https://github.com/peterolson/BigInteger.js .

However, I feel shooting for forward compatibility is better than reaching back. Anyway, there are other things I can look at for now.

[bshambaugh]

The other option to avoid ES2020 I think is to use a bigger library: https://github.com/peterolson/BigInteger.js .

However, I feel shooting for forward compatibility is better than reaching back. Anyway, there are other things I can look at for now.

I'm still looking into it because of w3c-ccg/did-method-key#36 and w3c-ccg/did-method-key#32 . I may be able to avoid having official ceramic support for it for awhile, at which point ES2020 may be supported.

[oed]

Thanks @bshambaugh, yeah hopefully we can update to ES2020 soon. Trying to be careful here, not to cause issues upstream :)

[bshambaugh]

I'm refactoring*. P-256 and P-384 seem to work, but P-521 runs into some issues for the Y coordinate when decompressed. I'm investigating this. I will add this to the PR when finished.

• https://github.com/bshambaugh/js-ceramic/tree/new-nist/packages/key-did-resolver/src

[bshambaugh]

I have been considering abandoning committing code for P-521, and just leaving a framework in place so it can be plugged in later. it looks like P-256 and P-384 can be put in order.

[bshambaugh]

I noticed that https://github.com/bshambaugh/js-ceramic/blob/new-nist/packages/key-did-resolver/src/secp384r1.ts#L103-L145 and https://github.com/bshambaugh/js-ceramic/blob/new-nist/packages/key-did-resolver/src/secp256r1.ts#L105-L145 for P-384 and P-256 have code that considers raw public keys (without a prefix), uncompressed public keys with a '04' prefix, and compressed public keys. This works fine if the keys stay the same length. However, P-521 varies by a byte which makes my comparisons useless. What if I had a raw public key that started with 04 by chance? How would I tell the difference between this and a 04 prefix placed with intentional regularity?

Here are the sources I checked out:

"It uses 521-bit private keys (encoded as 65-66 bytes, 130-132 hex digits) and 1042-bit public keys (uncompressed, encoded as 130-131 bytes, 260-261 hex digits)."
https://cryptobook.nakov.com/digital-signatures/exercises-secp256k1-sign-verify

For P-256 and P-384:
"If it's a P-256 key then the next 32 bytes (256 bits) are the x value and the remaining 32 bytes are the y value. For P-384 length of each is 48 bytes (384 bits)."
https://coolaj86.com/articles/the-ssh-public-key-format/

https://tools.ietf.org/id/draft-jivsov-ecc-compact-05.html#rfc.section.3

[oed]

I would be fine with dropping P-521 for now. Don't think that many applications use it.

[bshambaugh]

I am starting to get this error when using Jest with BigInt. interledger/rafiki#138 (comment) . I'm investigating.

One person pointed this out: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt#usage_recommendations .

[oed]

What if I had a raw public key that started with 04 by chance? How would I tell the difference between this and a 04 prefix placed with intentional regularity?

Do we really want to support both? What does the DID Key spec use? Would be easier if we only supported one of the two options!

[bshambaugh]

It would be easier to code for only compressed P-521 keys due to the key length variance problem mentioned above. I don't even know how to code for other cases with P-521.

I liked using 64 byte P-256 keys (raw, un-tagged) so I kept them there even though some argue for the spec to not include them: w3c-ccg/did-method-key#32.

P-384 was going to be kept like P-256.

I'll have to go back to revisit the merits of so much support. Currently, I'm still looking at:
P-256: raw (un-tagged), uncompressed, compressed
P-384: raw(un-tagged), uncompressed, compressed
P-521: compressed

[oed]

Please refer to the multicodec table, we should only deal with compressed keys imo. Supporting uncompressed and raw ones seems confusing for consumers of this package.

[bshambaugh]

I'll look at the code. A lot of this code was developed when discussions were starting, had just started, etc and uncompressed test vectors were still implemented in the w3c ccg specification and compressed test vectors did not exist. I had it working nicely for all cases. I'm not sure how much work it will be to un-support it.

I'll still need to have the code to compress the keys personally somewhere as my hardware only gives me uncompressed. I'll need to separate this into another library to deal with all of the compression and point all of the users there who want to replicate my use case.

[bshambaugh]

I don't know. I'll have to look at it. P-384 and P-521 was in there to be nice to the community. I only need P-256.
Well correction. keys were always compressed elsewhere with my code and not in the js-ceramic PR. which only resolves. One benefit I see of maintaining both is if users take a uncompressed NIST key, like is frequently seen in the wild, and try to convert it to a did key without compression, and then try to resolve, they won't be disappointed. I like code that is smart and can make sense of stuff if it can.

[oed]

Ok, I mean I guess it doesn't hurt to keep it.

[bshambaugh]

What gives?

I guess I need a test that looks at both sides of the logical OR.

I am not sure what options.accept means. I tried to trace it to an interface. I assume DID_JSON is used.

[oed]

I wouldn't worry about testing that. It's a super simple statement so should be fine.

[bshambaugh]

See updates here: #1884

[oed]

Should we close this PR in favor of the new one?

[bshambaugh]

Closing this pull request in favor of #1884 sounds reasonable.

[ukstv]

Hey @bshambaugh Read through the PR, here is what I noticed:

1. multibase package is being deprecated in favour of multiformats. Please, use muliformats.
2. Please, .js prefix: import * as secp256r1 from './secp256r1.js'
3. Make sure you could import key-did-resolver as ESM. On my machine bigint-mod-arith refuses to be imported as ESM.
4. Please, resolve merge conflicts.

[bshambaugh]

@ukstv , thanks for looking into this. #1884 should have all of these changes with the exception of bigint-mod-arith as an ESM. I'll do work over there, if that's okay.

[ukstv]

Thanks for letting me know @bshambaugh !

[ukstv]

Closing this PR: it is no longer relevant. #1884 , that is a continuation of this work, has been merged.

[bshambaugh]

Thanks for catching this @ukstv .