digitalbazaar / did-method-key

A did-io driver for the DID "key" method
Other
25 stars 10 forks source link

Add support for secp256k1 #15

Open OR13 opened 4 years ago

OR13 commented 4 years ago

@dlongley @dmitrizagidulin I assume you may prefer for this to actually be added to crypto-ld. I'm eager to get this into DID Key, any quick advice that I might be able to follow to get a PR started on this.

We use https://github.com/bitauth/bitcoin-ts WASM module for browser support.

There are also:

dmitrizagidulin commented 4 years ago

@OR13 - there's this repo, https://github.com/digitalbazaar/secp256k1-key-pair, which works with crypto-ld - would that help?

dmitrizagidulin commented 4 years ago

@OR13 so, unfortunately the fingerprint() function is not yet implemented - https://github.com/digitalbazaar/secp256k1-key-pair/blob/master/lib/index.js#L117 :) But at least the stub is there. (Same API as crypto-ld).

OR13 commented 4 years ago

Seems like it uses: elliptic

Sounds like crypto-ld is ready to go (minus the fingerprint).... If I convert the key to jwk... I can calculate the fingerprint correctly... there is no standard for calculating a fingerprint on a base58 encoded key...

should I use the kid JOSE implementation for the fingerprint despite the fact that its using a base58?

Any chance I can use jwk formatted keys for secp256k1, use that as the fingerprint and just support publicKeyBase58 as an additional property?

Obviously, keyAgreement won't be included here.

dmitrizagidulin commented 4 years ago

I don't really have a strong opinion on this. @dlongley ?

dlongley commented 4 years ago

Note that a "fingerprint" in this case is the full public key or it won't work with did:key. If that's the fingerprint we're talking about here, I don't really care how it's calculated and prefer a standard or defacto standard that everyone is already using -- so long as it's fully compatible with the multifactor/multibase stuff, etc.

OR13 commented 4 years ago

Thats not correct ATM i think...

https://github.com/digitalbazaar/crypto-ld/blob/master/lib/Ed25519KeyPair.js#L363

Note that ByHnpUCFb1vAfh9CFZ8ZkmUZguURW8nSw889hy6rD8L7 != z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V

and similarly:

DmgBSHMqaZiYqwNMEJJuxWzsGGC8jUYADrfSdBrC6L8s !== zCDGPtoUB2xJzywzLodnst7ggAWY16JB9f7jxUUGebCaF5

{
  "@context": [
    "https://w3id.org/did/v0.11"
  ],
  "id": "did:key:z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V",
  "publicKey": [
    {
      "id": "did:key:z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V#z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V",
      "type": "Ed25519VerificationKey2018",
      "controller": "did:key:z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V",
      "publicKeyBase58": "ByHnpUCFb1vAfh9CFZ8ZkmUZguURW8nSw889hy6rD8L7"
    }
  ],
  "authentication": [
    "did:key:z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V#z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V"
  ],
  "assertionMethod": [
    "did:key:z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V#z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V"
  ],
  "capabilityDelegation": [
    "did:key:z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V#z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V"
  ],
  "capabilityInvocation": [
    "did:key:z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V#z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V"
  ],
  "keyAgreement": [
    {
      "id": "did:key:z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V#zCDGPtoUB2xJzywzLodnst7ggAWY16JB9f7jxUUGebCaF5",
      "type": "X25519KeyAgreementKey2019",
      "controller": "did:key:z6MkqRYqQiSgvZQdnBytw86Qbs2ZWUkGv22od935YF4s8M7V",
      "publicKeyBase58": "DmgBSHMqaZiYqwNMEJJuxWzsGGC8jUYADrfSdBrC6L8s"
    }
  ]
}

This fingerprint implementation is custom (multibase / multicodec), its probably possible to implement it for secp256k1 and use it, we should just be really clear about how it works... it should be mentioned directly in the did key spec

dlongley commented 4 years ago

Sorry, to clarify, the fingerprint value doesn't need to match the base58 public key value byte for byte, it's just that the "fingerprint" (or value after did:key:) needs to be the multibase+multicodec encoded public key. If a hash of the public key were put there, the resolver could not produce a DID Document from it.

OR13 commented 4 years ago

yes, i understand now after reading the source code. We should update the method spec to define the fingerprint behavior, and note that it matches: https://w3c-ccg.github.io/did-method-key/#format

OR13 commented 4 years ago

Created an issue here: https://github.com/w3c-ccg/did-method-key/issues/10

OR13 commented 4 years ago

Whats the significance of the 01 part of (multicodec ed25519-pub 0xed01 + key bytes) ?

https://github.com/digitalbazaar/crypto-ld/blob/master/lib/Ed25519KeyPair.js#L352

will this be the same for secp256k1?

OR13 commented 4 years ago

Blocked by https://github.com/digitalbazaar/secp256k1-key-pair/pull/3

dmitrizagidulin commented 4 years ago

Whats the significance of the 01 part of (multicodec ed25519-pub 0xed01 + key bytes)?

It has to do with how multicodec encodes multi-byte varints, if I recall.

dmitrizagidulin commented 4 years ago

@OR13 I don't remember if it'll be the same for secp256k. Basically it's whatever the npm multicodec package will return for multicodec.addPrefix(), for the key type.

OR13 commented 4 years ago
const multicodec = require('multicodec');
const buffer = Buffer.from('');
console.log(multicodec.addPrefix('ed25519-pub', buffer))
// <Buffer ed 01>

console.log(multicodec.addPrefix('secp256k1-pub', buffer))
// ???

No way to tell currently since the 'secp256k1-pub' change is not published yet....

dmitrizagidulin commented 4 years ago

@OR13 ah, boo, ok. I need to dig into https://github.com/multiformats/unsigned-varint carefully and figure it out, then.

dmitrizagidulin commented 4 years ago

@OR13 Ok, confirmed:

const varint = require('varint')
varint.encode(0xe7)
[ 231, 1 ]

which is the same as your implementation in https://github.com/digitalbazaar/secp256k1-key-pair/pull/3/files

buffer[0] = 0xe7
buffer[1] = 0x01

So, all good.

OR13 commented 4 years ago

We still need to publish a version of https://github.com/digitalbazaar/secp256k1-key-pair before we can implement support in did key.

dmitrizagidulin commented 4 years ago

@OR13 secp256k1-key-pair npm package v1.1.0 is now published.

OR13 commented 4 years ago

Blocked by: https://github.com/digitalbazaar/crypto-ld/pull/41

OR13 commented 4 years ago

Blocked by: https://github.com/digitalbazaar/secp256k1-key-pair/pull/6

dlongley commented 1 year ago

To close this issue, we should add an example to the README. The new code base would not include anything directly.

wip-abramson commented 10 months ago

I would also like to use this library with secp256k1. But struggling to get it working using https://github.com/digitalbazaar/ecdsa-secp256k1-verification-key-2019

I am attempting this:

didKeyDriverSecp.use({
    multibaseMultikeyHeader: 'zQ3s',
    fromMultibase: createFromMultibase(Secp256k1KeyPair)
});

// This fails
const secpDidController = await didKeyDriverSecp.fromKeyPair({
    verificationKeyPair: secpKeyPair,
});

The above code fails. And every time I fix the current error, another one bubbles up.

The latest one I am stuck on is: Error: Cannot derive key agreement key from verification key type "EcdsaSecp256k1VerificationKey2019".

So basically wondering if anyone has an example of this working?