w3c-ccg / did-method-key

DID (Decentralized Identifier) did:key method for embedding keys in DID urls
https://w3c-ccg.github.io/did-method-key
Other
18 stars 12 forks source link

PublicKey id length #24

Open oed opened 3 years ago

oed commented 3 years ago

Currently the id, specifically what comes after #, is the entire encoded public key. Is this really needed? Can they be made shorter by just using the first few characters of the pubkey?

OR13 commented 2 years ago

this is how the method works currently... it has advantages, IMO its not worth changing.

oed commented 2 years ago

What's the advantages of using the full key?

OR13 commented 2 years ago

@oed makes string comparisons for keys possible.

I could live with making the id the JWK thumbprint... but I expect folks would object to that...

oed commented 2 years ago

makes string comparisons for keys possible.

Can you elaborate on what this means? Comparing would still work if it's only the first 15 chars of the key?

OR13 commented 2 years ago

@oed https://datatracker.ietf.org/doc/html/rfc7638

TLDR ... not safe to just compare the first few character of a string in order to implement a binary quality check.

oed commented 2 years ago

@OR13 should be safe if you use DID + id?

OR13 commented 2 years ago

@oed not if you use relative ref ids...

{
  "@context": [
    "https://www.w3.org/ns/did/v1",
    "https://w3id.org/security/suites/jws-2020/v1"
  ],
  "id": "did:key:z6Mkff5wEYRgDuuPzg4u5FYhvCzGHQDGGCHL8C9YNaEs2nwF",
  "verificationMethod": [
    {
      "id": "#z6Mk...",
      "type": "JsonWebKey2020",
      "controller": "did:key:z6Mkff5wEYRgDuuPzg4u5FYhvCzGHQDGGCHL8C9YNaEs2nwF",
      "publicKeyJwk": {
        "kty": "OKP",
        "crv": "Ed25519",
        "x": "EeM5xZGniNFEdHXYX6YhzDY9MEaVhdna_9c-HFaSqcY"
      }
    },
OR13 commented 2 years ago

FWIW I feel the same way as you do about this... But i think these are the only options:

1. use the full multiform for the fragment value 
`z6Mkff5wEYRgDuuPzg4u5FYhvCzGHQDGGCHL8C9YNaEs2nwF` (base58 encoded)

2. use a hash of the key for the fragment value 
`NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs` (base64url encoded)

3. last few characters of did  value for the fragment 
(`#s2nwF`)

4. use integer index for fragment 
(`#key-0`)
oed commented 2 years ago

not if you use relative ref ids...

If you see a relative ref, wouldn't it make sense to concat it with the DID?

I would modify 3. above to use the last n chars (as to not include multicodec byte).

I would be fine with using integer index (4) since this DID is not mutable.

oed commented 2 years ago

@OR13 how would you feel about a PR that introduces #0 and #1 etc for ids?

The old ids could be kept for backward compatibility. I'm mainly worried about protected JWS headers becoming super long with the current ids, e.g.:

eyJhbGciOiJFZERTQSIsImNhcCI6ImlwZnM6Ly9iYWZ5cmVpY3JqZnF4eGNoZGFweGFkMmo2N3RlM2x0bGdjdW8zbnl1d2F4NmoyaXp4bWJpZmZxYXdkaSIsImtpZCI6ImRpZDprZXk6ejZNa3ZRaXpMeXprcVRSeVhCN2JmNjdQN3ZWQVl1NjFVektOWkRueUs2SGFGNWp1I3o2TWt2UWl6THl6a3FUUnlYQjdiZjY3UDd2VkFZdTYxVXpLTlpEbnlLNkhhRjVqdSJ9
OR13 commented 2 years ago

@oed

Thanks for the header, you are proposing:

{
  "alg": "EdDSA",
  "cap": "ipfs://bafyreicrjfqxxchdapxad2j67te3ltlgcuo3nyuwax6j2izxmbiffqawdi",
  "kid": "did:key:z6MkvQizLyzkqTRyXB7bf67P7vVAYu61UzKNZDnyK6HaF5ju#z6MkvQizLyzkqTRyXB7bf67P7vVAYu61UzKNZDnyK6HaF5ju"
}

be changed to:

{
  "alg": "EdDSA",
  "cap": "ipfs://bafyreicrjfqxxchdapxad2j67te3ltlgcuo3nyuwax6j2izxmbiffqawdi",
  "kid": "did:key:z6MkvQizLyzkqTRyXB7bf67P7vVAYu61UzKNZDnyK6HaF5ju#0"
}

I am open to such a PR.

It would be a major breaking change, and we would have to make a bunch of updates, but we are willing to do that to make the identifiers that end up in credentials shorter.

However, we are not the only implementers of did:key.

It's one of the most popular, if not the most popular and interoperable did method.

So breaking changes may take a long time to negotiate.

cc @msporny @kimdhamilton @mavarley @mprorock ... please add others to help with the discussion.

oed commented 2 years ago

Note that it would be possible to keep the old id around in generated DID documents. Perhaps this could be added as a Backwards compatibility note?

msporny commented 2 years ago

It would be a major breaking change, and we would have to make a bunch of updates, but we are willing to do that to make the identifiers that end up in credentials shorter.

Digital Bazaar would be a -1 on this change for the following reasons:

Just signalling very early that we will be opposed to this change for architectural layering and security design reasons.

baha-ai commented 2 years ago

I agree with @msporny

Adding to his argument:

Sending this (from @OR13's examples above) base64 encoded string: ewogICJhbGciOiAiRWREU0EiLAogICJjYXAiOiAiaXBmczovL2JhZnlyZWljcmpmcXh4Y2hkYXB4YWQyajY3dGUzbHRs \ Z2N1bzNueXV3YXg2ajJpenhtYmlmZnFhd2RpIiwKICAia2lkIjogImRpZDprZXk6ejZNa3ZRaXpMeXprcVRSeVhCN2JmN \ jdQN3ZWQVl1NjFVektOWkRueUs2SGFGNWp1I3o2TWt2UWl6THl6a3FUUnlYQjdiZjY3UDd2VkFZdTYxVXpLTlpEbnlL \ NkhhRjVqdSIKfQ== compared to the suggested encoding (with #0 suffix in the kid): ewogICJhbGciOiAiRWREU0EiLAogICJjYXAiOiAiaXBmczovL2JhZnlyZWljcmpmcXh4Y2hkYXB4YWQyajY3dGUzbHR \ sZ2N1bzNueXV3YXg2ajJpenhtYmlmZnFhd2RpIiwKICAia2lkIjogImRpZDprZXk6ejZNa3ZRaXpMeXprcVRSeVhCN2J \ mNjdQN3ZWQVl1NjFVektOWkRueUs2SGFGNWp1IzAiCn0=

saves you about 64 bytes. We're not saving much here. Using CBOR is probably a better option for saving bytes than JSON based messages.

Also a didKey is a special kind of DID document without a service bloc. Its kid field is a representation of the public key, not an index of the key in the document as it has no VMs. Using the index #0 would imply having VMs in the DID doc, but it's not the case for DIDkeys. On the other hand using the full key multibase encoding as a suffix of the kid indicates that this is a didKey document. Finally this DIDkey has a full kid value and cannot be referenced by a relative id (ie #0 is meaningless in a didkey while #z6MkvQizLyzkqTRyXB7bf67P7vVAYu61UzKNZDnyK6HaF5ju represents the public key).