-
Notifications
You must be signed in to change notification settings - Fork 18
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
Method 2 #26
Method 2 #26
Conversation
lacks complete update of the new method in the rest of the document. Signed-off-by: Sam Curren <telegramsam@gmail.com>
Signed-off-by: Sam Curren <telegramsam@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am in favor of merging this change. I will let the PR sit for a week to see if community members have feedback.
core.html
Outdated
<p>When Resolving the peer DID into a DID Document, the process is similar to Method 0.</p> | ||
<ul> | ||
<li>Resolve the document as described in Method 0, using the portion of the DID prior to the delimiter.</li> | ||
<li>Decode the key after the delimiter, and insert it into the PublicKey array, using this key as the id fragment.</li> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the "PublickKey" array is not longer part of the did core spec. Need to decide where this goes in the new did core spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was basing this off the did:key spec, which has not been updated. We'll need to make sure we align with the core spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this part is missing now, but for future reference publicKey
has been deprecated in favor of verificationMethod
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@llorllale has submitted pr #27 to update to verificationMethod
. I'll try to merge both PRs together.
core.html
Outdated
"keyAgreement": [{ | ||
"id": "did:key:z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH#zBzoR5sqFgi6q3iFia8JPNfENCpi7RNSTKF7XNXX96SBY4", | ||
"type": "X25519KeyAgreementKey2019", | ||
"controller": "did:key:z6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
did:key
-> did:peer
This PR is strangely focusing on signing and encryption keys instead of being generalized. Furthermore, reserving the first key for "encryption" keys does not seem compatible with the requirement that deltas need to be signed by the inception key. The mechanism for determining the purpose of keys is under defined. How does this scheme codify P256 keys for use in key agreement instead of signatures? There are other details missing here related to the algorithms, such as how to specify the exact key wrapping scheme and so forth. This PR does not solve the problem we have in DIDComm v1 with naked keys. |
Suggestions on how to correct any unsolved issues are welcome. |
Signed-off-by: Sam Curren <telegramsam@gmail.com>
From today's Aries B call:
|
Not yet polished. Signed-off-by: Sam Curren <telegramsam@gmail.com>
Signed-off-by: Sam Curren <telegramsam@gmail.com>
I think I found a way to abuse JSON-LD semantics such that you can produce a self certifying identifier that is based on the entire did document but isn't affected by the The algorithm for this would be. produce a DID Document that looks like the following:
Pass this into an RDF Normalize function which will produce a set of N-Quads that match this
hash those N-Quads to the following hash post inject the id property and an {
"@context": ["https://www.w3.org/ns/did/v1", {"id": null, "@base": "did:example:a0acef8a8a4421b4b4f223adc1f170fcaacd2bbfb8937e3a74ff2d0331436705"}],
"id": "did:example:a0acef8a8a4421b4b4f223adc1f170fcaacd2bbfb8937e3a74ff2d0331436705",
"authentication": [
{
"id": "#z6MkecaLyHuYWkayBDLw5ihndj3T1m6zKTGqau3A51G7RBf3",
"type": "Ed25519VerificationKey2018",
"publicKeyBase58": "AKJP3f7BD6W4iWEQ9jwndVTCBq8ua2Utt8EEjJ6Vxsf"
}
],
"capabilityInvocation": [
{
"id": "#z6MkhdmzFu659ZJ4XKj31vtEDmjvsi5yDZG5L7Caz63oP39k",
"type": "Ed25519VerificationKey2018",
"publicKeyBase58": "4BWwfeqdp1obQptLLMvPNgBw48p7og1ie6Hf9p5nTpNN"
}
],
"capabilityDelegation": [
{
"id": "#z6Mkw94ByR26zMSkNdCUi6FNRsWnc2DFEeDXyBGJ5KTzSWyi",
"type": "Ed25519VerificationKey2018",
"publicKeyBase58": "Hgo9PAmfeoxHG8Mn2XHXamxnnSwPpkyBHAMNF3VyXJCL"
}
],
"assertionMethod": [
{
"id":"#z6MkiukuAuQAE8ozxvmahnQGzApvtW7KT5XXKfojjwbdEomY",
"type": "Ed25519VerificationKey2018",
"publicKeyBase58": "5TVraf9itbKXrRvt2DSS95Gw4vqU3CHAdetoufdcKazA"
}
]
} Now you should have a valid DID Document that is self certifying, but doesn't have massive dids and comes with the added bonus that you can reorder the did document without causing any sort of canonicalization issues (since N-Quads handle that canonicalization for you). Then in order to pass this via a DIDUrl, we could combine it with the intial-state query parameter such that the entire thing could be sent and expandably resolvable, but still verifiable to be connected to the initial state. Even better, we can include any property we want in there (including extension properties we might want in the future)
@OR13 how bad of an idea is this? |
This is pretty cool idea, you can I proposed something similar for sidetree / ipfs a while back... essentially, you delete the id property before hashing the document, and inject it before consumption. Imo its not a terrible idea. |
This might make the URI a little long, but one can also append a |
Strong +1 to using |
@OR13 recommended |
yes, ideally you would do this:
This gives you lots of flexibility and security without needing to add yet another patch type for every new DID Document feature... you just update your schemas to allow that feature. I tried to get sidetree to work like this, but ultiamtly they decided to implement a patch type per did feature, so there are patches for public keys and services.... and no way to provide arbitrary json.... UNLESS you used ietf-json-patch.... which AFAIK, Microsoft still does not support. |
@kdenhartog What you propose sounds nice, but does not really meet the goals of Method 2. Are you proposing it as a replacement, or useful for another purpose? |
A replacement. Which goal is it not addressing? |
It is replacing a ~300 byte DID with a ~2000 byte DID. I don't think it addresses terseness. |
Thanks, I hadn't seen any goals listed in the comments or this PR, so wasn't sure. Could we add a list of them somewhere to align what the problems we're trying to solve are? It's worth noting the initial-state only needs to be passed in the first message and subsequent updating of states later on. Additionally, with the alternative proposal that I mentioned we get to keep state updates at this layer rather than having to rely on the protocol layer above handling did rotation capabilities. In @TelegramSam proposal did document updates don't seem possible which would seem to limit the functionality of this method in similar ways that did:key gets limited. Is that not a goal that needs to be addressed at this point? |
There is already a robust method for updating state (method 1). It calls for exchanging deltas in a formal protocol, and there is a PR that reframes it to use initial-state. Modifying it further to use signed IETF JSON patches would be another nice enhancement. What you are proposing, Kyle, feels like the feature equivalent of that method. Sam's method intended to strike a middle ground -- more sophisticated than did:key (method 0) in that it allows multiple keys and an endpoint, but less fancy than method 1 because it disallows updates. Sam's proposal also attempted to address the requirement that we would communicate these DIDs in messages represented as QR codes. That's where the need for terseness comes from. |
This makes it far more clear to me what's trying to be achieved in this PR. Based on that, I agree that this wouldn't align with the goals well. Thanks for the additional information. I'll move my discussion to the improvements discussed in method 1 rather than detract from the work on method 2 here with the original approach. I'll take a look at it from this new lens as well to see if I might be able to help optimize for the goals as well. Just so I'm understanding correctly the priorities of method 2 are:
Is there any other ones that aren't mentioned above that should be considered? |
|
||
<pre class="example nohighlight" title="Example Multi-key Peer DID" id="multi-key-creation"> | ||
Encoded Encryption Key: .Ez6MkpTHR8VNsBxYAAWHut2Geadd9jSwuBV8xRoAnwWsdvktH | ||
Encoded Signing Key: .VzXwpBnMdCm1cLmKuzgESn29nqnonp1ioqrQMRHNsmjMyppzx8xB2pv7cw8q1PdDacSrdWE3dtB9f7Nxk886mdzNFoPtY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you taken any consideration into how you would encode an uncompressed public key? For example, P-256 keys often include both an x and y coordinate rather than using a compressed form made possible with Ed25519 keys. In the case where the key is 2 coordinate points can we specify how ordering of the points should be handled? Additionally with RSA keys do we want to assume that the exponent ("e" in JWK) would always be "AQAB" and that the encoded value will always be the modulus parameter (n
in a JWK)? Some more info on how to encode different key types would help here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using multi-base multi-codec keys allows us to support any key that can be encoded in those methods. Is that not sufficient?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've not found anywhere that defines how it should work. Can you point me to where you're finding them defining how those questions are specified? Even Ed25519/X25519 keys are not defined well anywhere from what I can find. All of the multicodec bytes are registered, but I can't find anywhere that specifies things for the questions I raised.
No, I think your summary of requirements is excellent. |
From Kyle Co-authored-by: Kyle Den Hartog <kdenhartog@users.noreply.github.com>
I think this PR has been discussed 3 or 4 times on community calls. The conversation stream here has been excellent as well. The open question by @kdenhartog about advanced key properties is an interesting one, but I don't want to hold up the PR for it, so let's move that to an issue. The other proposal that Kyle made is quite interesting, but represents a method with different goals than the Method 2 Sam has proposed. Therefore, I suggest we move that to a new PR. I'm going to go ahead and merge this one. |
This method adds a way to have a different authentication key than key agreement key. It adds a new method identifier (2) and explains how it works. Comments, Suggestions, Corrections, and Ideas welcome.