w3c / did-core

W3C Decentralized Identifier Specification v1.0
https://www.w3.org/TR/did-core/
Other
405 stars 94 forks source link

Public key "id" and "type" members duplicate JWK "kid" and "kty" members #170

Open selfissued opened 4 years ago

selfissued commented 4 years ago

Section 6.3 on Public Keys https://w3c.github.io/did-core/#public-keys requires that id and type members be present for public keys. These unnecessarily duplicate the functionality of the JWK members kid and kty.

Please revise this section to say that id and type MUST NOT be present for keys represented as JWKs and that kid and kty MUST be present in the JWK.

OR13 commented 4 years ago

See https://medium.com/transmute-techtalk/linked-data-proofs-vs-jose-why-not-both-1594393418cc

Its necessary for interoperability between linked data signatures and "vanilla jose"... small price to pay, and it should only need to be paid once now.

OR13 commented 4 years ago

also... we defintly don't want to remove id because the value might not be equivalent to kid... and kid might not be a valid URI... and also publicKeys might not be JWKs....

Here is an example: https://lds.jsld.org/example/didDoc.json

OR13 commented 4 years ago

If we want to represent publicKey as vanilla JWKs, we can do so by creating and registering a property jwks in the core registry... not all keys in the publicKey object will be JWKs.

msporny commented 4 years ago

Public Keys https://w3c.github.io/did-core/#public-keys requires that id and type members be present for public keys.

To put a finer point on what @OR13 says above... id is the identifier for the verification method and type is the type of verification method. A verification method may be a public key and that public key may be expressed using a JWK. However, that does not mean that id === jwk.kid and type === jwk.kty.

The spec text should be updated to make this more clear... there was a PR that just went in that was a step in the right direction, but this is still confusing based on the text in the spec. The spec should be updated to more precisely point out the difference between a verification method and cryptographic key material (JWK).

OR13 commented 4 years ago

I think we need to provide better examples of DID Methods that use JWKs... this will make this clearer.

OR13 commented 4 years ago

Here is an example in the did core registry: https://w3c.github.io/did-core-registry/#ecdsasecp256k1verificationkey2019

selfissued commented 4 years ago

Per #171 - having examples using JWKs will help us reason about the relationships among the different fields.

selfissued commented 4 years ago

On the 12-May-20 regular call we agreed that we need another special call on key representation to consider this and related key representation issues.

OR13 commented 4 years ago

related to JsonWebKey2020: https://github.com/w3c-ccg/lds-jws2020#supported-jws-algs

We need id and type for keys that are not JWKs... but we should do our best to align these values with kty and crv when thats possible to do so.

OR13 commented 4 years ago

recommend closing, we have discussed this many times. we need both id and type and publicKeyJwk / publicKeyBase58...

OR13 commented 4 years ago

type is describing a "VerificationMethod"... JWKs are only one part of that... and its the responsibility of the suite author to define if they are related, and how.

OR13 commented 4 years ago

for example: https://github.com/w3c-ccg/lds-jws2020#supported-jose-algorithms

selfissued commented 4 years ago

I understand that the group has decided to duplicate the key type for JWKs by including both "type" and "kty" - albeit, using different strings that effectively mean the same thing. The question that hasn't been answered is whether a JWK should/MUST include a "kid" and if so, what its value should/MUST be.

OR13 commented 4 years ago

@selfissued publicKeyJwk is json, and kid is optional per https://tools.ietf.org/html/rfc7517#section-4.5 its optional, however, I suggest we recommend that kid not be present in publicKeyJwk, and rely on the outer id property.... although there could be reasons why you might want to have 2 identifiers for the same verification material.

Here are some recommendations to working with JOSE and DIDs:

https://github.com/decentralized-identity/did-jose-extensions/blob/master/options.md#kid-in-jwk-and-jwajwe

kdenhartog commented 4 years ago

@selfissued publicKeyJwk is json, and kid is optional per https://tools.ietf.org/html/rfc7517#section-4.5 its optional, however, I suggest we recommend that kid not be present in publicKeyJwk, and rely on the outer id property.... although there could be reasons why you might want to have 2 identifiers for the same verification material.

Here are some recommendations to working with JOSE and DIDs:

https://github.com/decentralized-identity/did-jose-extensions/blob/master/options.md#kid-in-jwk-and-jwajwe

Even though it's optional in the JWK spec, might it be helpful to us to further constrain the representation in the did document? I'd think we'd still want to do this with a SHOULD though so we're not forcing duplication of the DID URL in VDRs where space is a concern.

OR13 commented 4 years ago

@kdenhartog agreed, if we say anything its probably a SHOULD here.... however the DIF DID-JOSE-Extensions Spec can be bossier if we get agreement that its helpful.

OR13 commented 3 years ago

I suggest we close this issue, we need id and type not everyone uses JOSE / JWK.

msporny commented 3 years ago

This issue will be closed in 7 days unless there are objections.

selfissued commented 3 years ago

Object to closing this issue, as it has not been resolved.

msporny commented 3 years ago

Object to closing this issue, as it has not been resolved.

Let me try to explain why there seems to be consensus in the group to close the issue:

Section 6.3 on Public Keys https://w3c.github.io/did-core/#public-keys requires that id and type members be present for public keys. These unnecessarily duplicate the functionality of the JWK members kid and kty.

The group has discussed this many, many times over the past year. Verification relationships point to verification methods in the specification. Verification methods have a id and type. We have consensus in the group on that point... there isn't a single implementer that we know of that is doing anything other than that.

Please revise this section to say that id and type MUST NOT be present for keys represented as JWKs and that kid and kty MUST be present in the JWK.

Removing id and type from Verification Methods is unlikely to achieve consensus -- there will be multiple -1s to that proposal. kid and kty may be used in publicKeyJwk as expressed in numerous examples in the specification:

https://w3c.github.io/did-core/#example-15-various-public-keys

The group, and @OR13 in particular, has put a lot of work into supporting JWK in the specification via the JsonWebKey2020 Verification Method.

In summary:

1) The original assertion that 'id' and 'type' duplicate 'kid' and 'kty' is provably false at this point (per the examples in the specification). 2) The proposal to to mandate the use of kid and kty is a JsonWebKey2020 spec responsbility, not a DID Core spec responsibility.

As such, there is nothing for this group to do. Thus the issue is pending closure. If you continue to object, please propose a concrete proposal or PR that would achieve consensus.

OR13 commented 3 years ago

I agree with @msporny and having spent a lot of time integrating JWK and verificationMethods, I don't see any reason to leave this open... the spec contains examples which clearly demonstrate why this separation is needed.

See https://w3c.github.io/did-core/#example-35-did-document-with-many-different-verification-methods

Microsoft has already shipped support for this data structure in did:ion, and they cannot change the interface now, without forcing another round of breaking changes on consumers of ION.... I expect @csuwildcat would formally object to any changes to DID Core that caused ION / Sidetree to no longer be conformant, which would include any further changes to verificationMethods....

Also, since JOSE and DIDs integration is not defined anywhere, we have this DIF proposal fro trying to make a clearer set of recommendations:

https://github.com/decentralized-identity/did-jose-extensions

I agree this issue should be closed.

msporny commented 3 years ago

The following specification text achieved consensus as in the specification now:

In the case where a verification method is a public key, the value of the id property MAY be structured as a compound key. This is especially useful for integrating with existing key management systems and key formats such as JWK [RFC7517]. It is RECOMMENDED that JWK kid values are set to the public key fingerprint [RFC7638]. It is RECOMMENDED that verification methods that use JWKs to represent their public keys utilize the value of kid as their fragment identifier. See the first key in Example 15 for an example of a public key with a compound key identifier.

As far as two of the Editors of DID Core and DID Specification Registries are concerned, the existing specification text addresses this issue. I am putting the pending close label back on this issue.

@selfissued, if you would like this issue to remain open you will have to propose some concrete specification text changes and/or a PR. Due to the holidays, we'll delay closing this issue until January 10th 2021 -- three weeks from now.

msporny commented 3 years ago

The spec was updated as a result of this issue and currently says this:

https://w3c.github.io/did-core/#verification-methods

In the case where a verification method is a public key, the value of the id property MAY be structured as a compound key. This is especially useful for integrating with existing key management systems and key formats such as JWK [RFC7517]. It is RECOMMENDED that JWK kid values are set to the public key fingerprint [RFC7638]. It is RECOMMENDED that verification methods that use JWKs to represent their public keys utilize the value of kid as their fragment identifier. See the first key in Example 16 for an example of a public key with a compound key identifier.

TallTed commented 3 years ago

Teeny tiny language (verb tense) tweak (tl;dr: kid values are set -> kid values be set)...

It is RECOMMENDED that JWK kid values are set to the public key fingerprint [RFC7638].

-- (which talks about the present state of the values) should be changed to --

It is RECOMMENDED that JWK kid values be set to the public key fingerprint [RFC7638].

-- (which talks about the recommended future state of the kid values, since we're guiding the people who will be doing the setting).

brentzundel commented 3 years ago

No objections raised since marked pending close. Closing.

msporny commented 3 years ago

Apologies @brentzundel -- on a recent WG call, @selfissued objected to closing this until he writes a PR. It seems like we didn't record that in the issue. I've re-opened the issue because closing it would most likely lead to an objection from @selfissued. The issue will be deferred or closed if a PR isn't written by a yet-to-be determined deadline before CR... but until then, we're waiting for a PR from @selfissued.

veikkoeeva commented 2 years ago

I would add that it seem confusing that in the examples here in did-core there is f.ex.

"verificationMethod": [{
    "id": "did:example:123#_Qq0UL2Fq651Q0Fjd6TvnYE-faHiOpRlPVQcY_-tA4A",
    "type": "JsonWebKey2020", 
    "controller": "did:example:123",
    "publicKeyJwk": {
      "crv": "Ed25519", 
      "x": "VCpo2LMLhn6iWku8MKvSLg2ZAoC-nlOyPVQaO3FxVeQ", 
      "kty": "OKP", 
      "kid": "_Qq0UL2Fq651Q0Fjd6TvnYE-faHiOpRlPVQcY_-tA4A" 
    }
  }

that is kid field is present (Example 13). Whereas when one looks at https://w3c-ccg.github.io/lds-jws2020/ there is no kid field is present.

Further, as a second note, in the common scenario of did:key the examples at https://w3c-ccg.github.io/did-method-key/ do have the id encoded as multibase base58-btc. While it may be clear for did:key this how it's done, but together with that mentioned kid thing in did-core and what's in https://w3c-ccg.github.io/lds-jws2020/, it becomes less clear what should be included/not included and used in these cases when reading only the specifications.

OR13 commented 2 years ago

In general, JSON processors should ignore fields they don't understand.

kid is a string or URI, and it's optional in a JWK.

id, type and controller are required in a verification method, id MUST be a DID URL.

If you are using a verification method with a key in a different format, you are pretty much responsible for explaining how the key and a related signature are supposed to be linked.

For example... how do verifiers know how to use publicKeyMultibase to verify a JWS ... or any other signature serialization?

TLDR; convert it to a JWK and then use any off the shelf library to verify... or role your own and document it.

The most confusing part of this issue is the desperate need to acknowledge that key and signature formats other than JWK and JWS exist..... here are some quick examples:

obviously kid in headers or in JWK is not relevant to those key formats... and therefore this issue does not address those use cases... eliminating those use cases seems like not a good idea... but we can continue to debate if DIDs should support ONLY 1 key format per representation.