Skip to content
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

Add multikey 2021 #2174

Merged
merged 2 commits into from
May 25, 2021
Merged

Add multikey 2021 #2174

merged 2 commits into from
May 25, 2021

Conversation

OR13
Copy link
Contributor

@OR13 OR13 commented May 20, 2021

This PR is sadly necessary to handle cases where there is no JSON-LD support for standard NIST curves like P256.

I did my best not to trigger you @msporny but we can't possible support did:key without this, hopefully its acceptable.

My sense is that the LD Sec WG Suite imp guide will have a large section on best practices in this regard, and we can elaborate on the pro's and con's of type there.

@davidlehn
Copy link
Collaborator

Should this be merged? This is ok as far as w3id procedures are concerned. @msporny was there some issue that needed to be resolved first? I'm not sure how this is being used, but the name "multibase-2021" seems off since the context looks to be a collection of key and signature types, not various base encodings. (Is there somewhere better to discuss technical details of this?)

security/.htaccess Outdated Show resolved Hide resolved
@msporny
Copy link
Collaborator

msporny commented May 21, 2021

This PR is sadly necessary to handle cases where there is no JSON-LD support for standard NIST curves like P256.

Isn't the only thing necessary there is to create an EcdsaSignature2021 suite (or similar) and a Secp256r1VerificationKey2021 (or similar)? We don't have to support all variations, do we?

we can't possible support did:key without this, hopefully its acceptable.

I don't understand this statement... isn't this conflating multibase/multicodec encoding w/ cryptosuite naming?

Co-authored-by: Manu Sporny <msporny@digitalbazaar.com>
@OR13
Copy link
Contributor Author

OR13 commented May 22, 2021

@msporny I took your proposed name change, the issue is a general one, for all these keys: https://www.iana.org/assignments/jose/jose.xhtml#web-key-elliptic-curve
which we support with JWK, we want to support did:key as well.

You are right that I could create a linked data suite per key type, but thats not how JOSE really works, and it's also not how JsonWebKey2020 works.

IMO we need to show that all variations can be supported, but we don't need to actually use all of them.

This is part of showing that we are improving things, its hard to argue that a subset of IANA / JWK is better than a superset, especially when it doesn't cover NIST / FIPs crypto.

@OR13 OR13 changed the title Add multibase 2021 Add multikey 2021 May 22, 2021
@OR13
Copy link
Contributor Author

OR13 commented May 22, 2021

Implemented the needed change on our side: transmute-industries/ns.did.ai@43a5cd3

This PR is blocking our ability to register our did key implementation in the did core test suite, and our ability to update the dicd core examples.

Related PR: transmute-industries/did-core#20

We can register without the w3id.org redirect, but it feels like bad form to do so, so we are considering it blocked.

@OR13
Copy link
Contributor Author

OR13 commented May 24, 2021

@msporny this PR is blocking our did test suite registration but only for JSON-LD.

@OR13
Copy link
Contributor Author

OR13 commented May 25, 2021

@davidlehn @msporny eta on merge for this? or are there more changes requested?

@msporny
Copy link
Collaborator

msporny commented May 25, 2021

@davidlehn @msporny eta on merge for this? or are there more changes requested?

Apologies @OR13 -- I'm not getting these direct pings in my email for some reason. @davidlehn ping'd me. Looking now.

@msporny msporny merged commit 762ebf2 into perma-id:master May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants