-
Notifications
You must be signed in to change notification settings - Fork 97
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
CR Feedback: Support of publicKeyMultibase
#707
Comments
@msporny and I have also worked on publicKeyMultibase.... https://w3c-ccg.github.io/lds-ed25519-2020/ However, we fought for a very long time about this, and settled on IMO the best thing we could do would be replace |
100% in agreement on this! |
With my Editor hat off and company rep hat on -- Digital Bazaar is also strongly in favor of replacing With my Editor hat on -- DAMMIT, PEOPLE! You want to do this NOW!? :P Doing a change like this at this point would delay entering Candidate Recommendation (which no one wants to do -- we're already 4 months late going into CR), or require us to go through another Candidate Recommendation round (add an extra 1-2 months to our dangerously short timeline). We are almost certainly going to need to go through at least one extra CR... so my suggestion would be to acknowledge this issue, run the first CR for two months and then if there is agreement in the WG to make this change (and the tests to support it), run another CR. EDIT: Wait, no, I'm totally wrong. I forgot that we marked that as an issue at risk... so it wouldn't cause us to go through another CR. Nevermind! We're all good here. |
An alternative consideration: Leave this up to the LDP suites to define (since that's where this should be handled anyways - the LDP work made a good architectural split let's use it) and then leave things as they are defined by the LDP suites which should ultimately should be making this decision about which encoding term is used. Then in DID Core we can place additional constraints to maintain interoperability by saying "LDP suites that are defining public key data model representations SHOULD use publicKeyJwk or publicKeyMultibase". I'd argue for a MUST here, but by using SHOULD we allow for deprecated publicKeyBase58 usage keeping the selection narrow for implementors. I'm open to upgrading that to a MUST though if others feel that's the right way to handle it. |
3Box Labs team also strongly favors |
I don't really have a strong opinion either way, but want to mention that during a number of Special Topic Calls we settled on At one point in time, the idea was to support I think you could also argue that But things evolve of course, so no objection here... Of course @msporny - in one of his typical sneaky PRs #613 - already managed to change the Simple Example to use |
Note that, editorially, that example should be "annotated" like, for example, Example 13 making it clear that cc @rhiaro |
I think the Simple Example should use |
The change was intended primarily to telegraph that (at least my hope is) that we're going to upgrade to The biggest remaining problem in Ed25519Signature2018 is the JWS encoding -- we had done this as a compromise with the JOSE community, but it just created complexity w/o much upside. Now that @OR13 has created a proper JOSE cryptosuite, we don't need to support JWS/JWK anywhere else but in the JOSE cryptosuite... which means other cryptosuites can move on w/o all the JOSE base64 encoding cruft. One of the advantages of I'm afraid that if we go with what's common today ( All that said... all of the examples are non-normative... we can change them at any point during CR. I just wanted to get an example into the spec that demonstrated that we had contemplated this before we went into CR (along with the at risk marker that's in the spec now about We can make the change, it won't make us go through another CR... the only thing we need is implementer feedback on what implementers would like to do. |
PublicKeyMultibase
publicKeyMultibase
publicKeyMultibase
publicKeyMultibase
Seems like more private key entries will be necessary for equivalent representation support to JWK... the only multibase private key code is for ed25519... https://github.com/multiformats/multicodec/blob/master/table.csv#L128 |
@OR13 You're now talking about multicodec, but we are not proposing to adopt that, are we? This issue is only about adopting multibase, not multicodec? |
sorry, I was not paying attention when I wrote that. multibase is all we need to update publicKeyBase58 and privateKeyBase58 to publicKeyMultibase and privateKeyMultibase... both would start with The multicodec stuff is not relevant to this discussion, I have just been dealing with it recently and got confused by that entry... Its worth noting that when you use a key representation like multibasee, without multicodec, you end up transmitting key type in other places, like
to a JWK format, you will need to pass: type, publicKeyMultibase => jwk. |
Worth considering the intersection of this with w3c/did-extensions#264 |
Something to note about publicKeyMultibase is that while the multicodec and multibase aspects are well defined there's a lack of definition around how the public key material should be serialized. As an example, when using publicKeyMultibase with a P-256 key are we expecting that the key MUST be compressed, or is it possible for the key to be uncompressed X and Y coordinate points. If it is X and Y coordinates would we expect the X to come before the Y or vice versa? Similarly, if we encode an RSA key do we need to include all of the public components for the key that are passed and in what way are we encoding them? I expect that this isn't something we should be defining, but rather we should be working with @jbenet and others working on multiformat to get that work done there. From what I can tell, this may be the intent of what multikey is for. |
Doesn't actually define how x, and y are handled... but we are all esssentially assuming [x,y] @kdenhartog just let RSA die. |
Yeah we should go and define that because it could be assumed they need to be compressed as well.
I hope Schnorr finally put the nail in that coffin and only included it for the complexity it would bring. |
Reading this thread again... I don't have a strong opinion here, and I do find
Is this whole issue even still relevant if we decide to split out the cryptosuite terms into their own contexts as suggested in w3c/did-extensions#277? (Which I think is a good idea). |
The issue was discussed in a meeting on 2021-03-30
View the transcript5.2. CR Feedback: Support of publicKeyMultibaseSee github issue #707. Brent Zundel: if you identify some other ones, please mention them
Orie Steele: I'm one of the folks who commented on this. I'm very supportive of this issue. Brent Zundel: I am adding you as an assignee to the issue. Manu Sporny: +1 to what Orie_ said. I believe on purpose we put language in the spec that would allow us to make this substantive change during CR. Kyle Den Hartog: Maybe venturing into "take it into the comments" ... but does it make sense for us to define it here, or better in the linked data suites work? Manu Sporny: good question, it should have been done years ago in that work. But because it did not go standards track first, we are in the awkward position of having to define this stuff. |
As a developer, I'm strongly against supporting the usage of publicKeyMultibase as a MUST in the DID Core specifications. The Multibase Algorithms Registry is expanding; there are many encodings in draft and candidate status. A brief look at the currently linked implementations shows us many of the libraries are already missing some of the default encodings, E.g., Java, Python, Haskell, .NET, Elixir are missing the base16upper, and C++ is missing most of the defaults as far as I can tell. Developers will need to ensure the used libraries are up-to-date and supporting all the encodings or implement/maintain theirs. I believe this is too much to ask for so little to gain and can lead to many interoperability issues. There are many benefits of supporting publicKeyMultibase, that's for sure. But it shouldn't be enforced, or required base-encodings should be known/limited for the sake of implementers. Leaving the limitation of base-encodings to crypto-suites, as @msporny suggests, might be the solution. |
well, good news is that at least there is tooling for handling JWKs... I suppose the same is true for multibase now, thanks for IPFS... its less true for base58. |
Been looking around and reading the specs, trying to decide upon which encoding to use for the public key. I wanted to drop supporting multiple and just go with one, I already was doing Hex and Jwk, but figured I would migrate to only do base58. Then I found this thread (I was thinking of submitting an issue) and lots of good content here. From what I understand of multibase, it's simply a flexible value that uses a single character prefix to pick the encoding? That would make it fairly easy to use together with most crypto-libraries for blockchains that already exists, which a lot of them does base58. It's even possible to not use the multibase library for implementations that produce documents, I should be able to simply prefix with "z" and use the base58 implementation I already got. So for what it's worth, I'm supportive of publicKeyMultibase! |
I see the need for smaller strings, but also the concern of added complexity of As an implementer I have preferred I wonder if using COSE with |
It's not said often enough, but this is the operating assumption. Just because we're supporting publicKeyMultibase doesn't mean we want to support ALL base-encodings (that would be a bit insane). What we're trying to do is future-proof all key expressions so that we can switch base-encoding formats in the future w/o having to invent publicKeyBase72 (for example). At present, it seems like MOST of the publicKeyMultibase encodings are going to be base58, with some choosing base16 (lowercase).
I strongly disagree with "potential source of confusion". I suggest that most developers have no idea what each component of the keys they're expressing are... and there have been countless failures with developers publishing private keys with their public keys or constructing the cryptographic variables in ways that cause security failures in their applications. We shouldn't allow developers that don't know what inputs to cryptographic algorithms do to be able to change those inputs for the same reason that we don't allow drivers to easily change the fuel enrichment settings on their gas-powered cars, or the voltage output settings on their electric vehicles. Instead, the people that know about the cryptography should decide on the byte layout for cryptographic algorithm input and expose that as an opaque blob to the rest of the developer community... because the rest of the developer community doesn't need to tweak those settings. This was one of the strongest arguments behind why ed25519 was designed the way it was... they noticed that developers were screwing the selection of the input parameters up and they were like... "Ok, how do we make it so that the only thing you need to get right is to feed a random number in, and then we take care of the rest." The nice thing about ed25519 is that there is very little there to mess up. That cannot be said for the JOSE stack.
We could start with two problems... and then do You're coming at this as a "developer in the know"... try to think about the developers that will be using this stuff, that don't know nearly as much as you do... how would you protect them from themselves? Would you add more switches and toggles? Or would you take some switches and toggles away? I suggest that simpler systems tend to be easier to analyze, easier to maintain, and more secure. |
Alright, looks like we have a decent bit of support for moving to
We have an example of a DID Document that uses this new format here, if folks are curious: https://github.com/w3c/did-test-suite/blob/main/packages/did-core-test-server/suites/implementations/did-key-2020-db.json#L15-L18 Since we marked Any objections to executing on that plan in a week or two? |
Although the conclusion is the same, the premise is not really: I believe that (Oh, and just to be picky: we do not need another Candidate Recommendation Snapshot in the process newspeak.) One more point to add:
|
@iherman wrote:
From the spec 5.2.1 Verification Material:
We can't define |
Very exciting @msporny, FWIW your plan of action above looks great to me! |
You are right, I missed that. Which also means that, technically, it is a mistake to list this property in, e.g.: https://github.com/w3c/did-spec-registries/blob/main/vocabs/v1/vocab.ttl, which is supposed to define the normative vocabulary, and the normative vocabulary only... Which is one more reason, if there was a need for one, to review the vocabulary files if and when we go for |
The issue was discussed in a meeting on 2021-05-04
View the transcript7.3. Support of
|
I don't have a strong opinion on this topic, but I do want to point out that there were some arguments against publicKeyMultibase e.g. in #707 (comment). And I have to say I haven't really heard a good explanation why people want publicKeyMultibase either:
So what is the argument for publicKeyMultibase? It is that we can support multiple encodings in a property value without having to introduce new properties. But then instead of (or in addition to) relying on our own DID Spec Registries for new properties, we would now be relying on an external registry that 1. can change at any time, and 2. is not equally supported by all implementations, as explained in #707 (comment). I'm also surprised to hear the following by @msporny:
So we would somehow have a separate (cryptosuite-specific?) registry that will tell us which base encodings of the external multibase registry we support? This feels like a lot of dependencies. But I do buy the argument that re-using the external multibase registry has some advantages over defining potentially lots of new DID document properties for every encoding, and that therefore publicKeyMultibase is indeed useful. Did I get all of this right? I just think this should be explained well, considering that we had multiple special topic calls on key formats, and the vast majority of DID methods and DID documents today use the simpler publicKeyBase58. |
This IMO is the primary value of The same is true for Sure, you can make up your own values and never register them, but most folks who you interoperate with will eventually complain / help you register things properly... and when you do, it won't be just for "DIDs".... thats a big win for the whole community IMO. |
PRs w3c-ccg/security-vocab#103, w3c/did-extensions#299, and #731 have been raised to address this issue based on the plan put forth in #707 (comment) and given that there were no strong objections to that plan. This issue will be closed once each of the aforementioned PRs have been merged. |
PRs w3c-ccg/security-vocab#103, w3c/did-extensions#299, and #731 have been merged. This addresses the current issue. @carsonfarmer please confirm that these changes address the concerns you raised in this issue. You have 7 days to respond until we mark the issue as resolved. |
I have taken a look at all linked PRs/discussions, and honestly I couldn't be happier! This is great, and I really think it is going to bump up interoperability across the ecosystem in a really positive way. Thanks to the core maintainers and developers for a swift, balanced discussion and implementation. This was one of the most effective open source discussions I've seen in a while... kudos to the community 💪 . |
What is the protocol here? Shall I close this issue myself, or should I leave that up to maintainers? Happy to close, as I consider this issue "resolved". |
That's all we needed, thanks @carsonfarmer. Closing with explicit confirmation from issue submitter that their concern has been addressed. |
According to https://w3c.github.io/did-core/#verification-material, the "DID Working Group is seeking implementer feedback on the preference of the ecosystem with respect to using
publicKeyBase58
orpublicKeyMultibase
. I would like to strongly signal preference for inclusion ofpublicKeyMultibase
in the spec.As stated in the did-core document,
publicKeyMultibase
supports more base-representation formats and provides a more future proof path. It additionally supports publicKeyBase58 quite easily, and there are now lots of libraries and implementations of the "standard" out in the wild, most of which are fully compatible and stable. See for example: https://github.com/multiformats/multibase#implementations (linking for those less familiar with multiformats and multibase encoding). In the interest of limiting the number of formats for expressing public key material in a DID Document to the fewest possible, and to increase the likelihood of interoperability, I would suggest thatpublicKeyMultibase
be the preferred encoding format.Our team (@textileio) has already begun implementing a Go-lang version of https://github.com/decentralized-identity/did-resolver where we are explicitly supporting
publicKeyMultibase
for compatibility with the wider multibase/IPLD/IPFS ecosystem. I have also cc'ed a few other contributors/implementors that might be interested in voicing their opinion here as well.Thank you, and apologies if this is not the right venue to signal support!
cc @sanderpick @oed
The text was updated successfully, but these errors were encountered: