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

[Bug] Longform DID resolution of published DID returns incorrect resolution Result #648

Open
andresuribe87 opened this issue Aug 10, 2023 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@andresuribe87
Copy link
Contributor

Describe the bug
When doing resolution for a long form ION DID, the network is never checked to see if the DID has been published. This results in did document within the resolution results that's incorrect according to 15.1.2 of https://identity.foundation/sidetree/spec/#resolution.

To Reproduce

  1. Create a DID ION outside of ssi-service. Take note of the LongFormDID.
  2. Update the DID ION outside of ssi-service.
  3. Wait for 20 minutes.
  4. Let longFormResolution be the result of GET localhost:3000/v1/dids/resolver/did:ion:<xxx>:<yyy>.
  5. Let shortFormResolution be the result of GET localhost:3000/v1/dids/resolver/did:ion:<xxx>.
  6. Note that shortFormResolution !== longFormResolution.

Expected behavior
Both forms of resolution should be equal.

Supporting Material

If record of the DID being published has been observed, proceed to Step 3. If there is no observed record of the DID being published, skip all remaining Operation Compilation steps and process the DID as follows:

From https://identity.foundation/sidetree/spec/#resolution

@andresuribe87 andresuribe87 added the bug Something isn't working label Aug 10, 2023
@andresuribe87
Copy link
Contributor Author

I just did steps 1 & 2 with

did:ion:EiDwdKcZXuMP0dnuMOk0PmqkorpukCNRqasIKxUoG1t6UA:eyJkZWx0YSI6eyJwYXRjaGVzIjpbeyJhY3Rpb24iOiJyZXBsYWNlIiwiZG9jdW1lbnQiOnsicHVibGljS2V5cyI6W3siaWQiOiJrZXktMSIsInB1YmxpY0tleUp3ayI6eyJjcnYiOiJzZWNwMjU2azEiLCJrdHkiOiJFQyIsIngiOiJXZ3BwYUJicE1rcVEyeXRPd1VkVXVkZnBGRWg0aHRaN1dEczBoWWROMFdFIiwieSI6ImJKYjZXdE5pZXhrNHljUTFoYVQwbjRjQU5qLUk0OWp4b21od0ZINWlqMWMifSwicHVycG9zZXMiOlsiYXV0aGVudGljYXRpb24iXSwidHlwZSI6IkVjZHNhU2VjcDI1NmsxVmVyaWZpY2F0aW9uS2V5MjAxOSJ9XSwic2VydmljZXMiOlt7ImlkIjoiZG9tYWluLTEiLCJzZXJ2aWNlRW5kcG9pbnQiOiJodHRwczovL2Zvby5leGFtcGxlLmNvbSIsInR5cGUiOiJMaW5rZWREb21haW5zIn1dfX1dLCJ1cGRhdGVDb21taXRtZW50IjoiRWlBbFdRRFBDdWpNU1RLMG5wTzAwN1JPd043UFBhU2tGTlZxR05hYlhVc2NCUSJ9LCJzdWZmaXhEYXRhIjp7ImRlbHRhSGFzaCI6IkVpRDEzNk1sVy1HQXJfTldvNU1JSUVUZlRDVlZqSkQzeDVWS3FwX3FxMWtzd3ciLCJyZWNvdmVyeUNvbW1pdG1lbnQiOiJFaUR1dDZEQnhCbDI5YTRTOHhNLVVVR1hjYldrSWpnNEhPcFBqMDRZUE41VGV3In19

Will post an update once enough time has passed.

@decentralgabe
Copy link
Member

@andresuribe87 update here?

@andresuribe87
Copy link
Contributor Author

The result of Step 3

curl -X 'GET' \
  'https://ssi.tbddev.org/v1/dids/resolver/did%3Aion%3AEiDwdKcZXuMP0dnuMOk0PmqkorpukCNRqasIKxUoG1t6UA%3AeyJkZWx0YSI6eyJwYXRjaGVzIjpbeyJhY3Rpb24iOiJyZXBsYWNlIiwiZG9jdW1lbnQiOnsicHVibGljS2V5cyI6W3siaWQiOiJrZXktMSIsInB1YmxpY0tleUp3ayI6eyJjcnYiOiJzZWNwMjU2azEiLCJrdHkiOiJFQyIsIngiOiJXZ3BwYUJicE1rcVEyeXRPd1VkVXVkZnBGRWg0aHRaN1dEczBoWWROMFdFIiwieSI6ImJKYjZXdE5pZXhrNHljUTFoYVQwbjRjQU5qLUk0OWp4b21od0ZINWlqMWMifSwicHVycG9zZXMiOlsiYXV0aGVudGljYXRpb24iXSwidHlwZSI6IkVjZHNhU2VjcDI1NmsxVmVyaWZpY2F0aW9uS2V5MjAxOSJ9XSwic2VydmljZXMiOlt7ImlkIjoiZG9tYWluLTEiLCJzZXJ2aWNlRW5kcG9pbnQiOiJodHRwczovL2Zvby5leGFtcGxlLmNvbSIsInR5cGUiOiJMaW5rZWREb21haW5zIn1dfX1dLCJ1cGRhdGVDb21taXRtZW50IjoiRWlBbFdRRFBDdWpNU1RLMG5wTzAwN1JPd043UFBhU2tGTlZxR05hYlhVc2NCUSJ9LCJzdWZmaXhEYXRhIjp7ImRlbHRhSGFzaCI6IkVpRDEzNk1sVy1HQXJfTldvNU1JSUVUZlRDVlZqSkQzeDVWS3FwX3FxMWtzd3ciLCJyZWNvdmVyeUNvbW1pdG1lbnQiOiJFaUR1dDZEQnhCbDI5YTRTOHhNLVVVR1hjYldrSWpnNEhPcFBqMDRZUE41VGV3In19' \
  -H 'accept: application/json'

is

{
  "didResolutionMetadata": {},
  "didDocument": {
    "@context": [
      "https://www.w3.org/ns/did/v1",
      {
        "@base": "did:ion:EiDwdKcZXuMP0dnuMOk0PmqkorpukCNRqasIKxUoG1t6UA:eyJkZWx0YSI6eyJwYXRjaGVzIjpbeyJhY3Rpb24iOiJyZXBsYWNlIiwiZG9jdW1lbnQiOnsicHVibGljS2V5cyI6W3siaWQiOiJrZXktMSIsInB1YmxpY0tleUp3ayI6eyJjcnYiOiJzZWNwMjU2azEiLCJrdHkiOiJFQyIsIngiOiJXZ3BwYUJicE1rcVEyeXRPd1VkVXVkZnBGRWg0aHRaN1dEczBoWWROMFdFIiwieSI6ImJKYjZXdE5pZXhrNHljUTFoYVQwbjRjQU5qLUk0OWp4b21od0ZINWlqMWMifSwicHVycG9zZXMiOlsiYXV0aGVudGljYXRpb24iXSwidHlwZSI6IkVjZHNhU2VjcDI1NmsxVmVyaWZpY2F0aW9uS2V5MjAxOSJ9XSwic2VydmljZXMiOlt7ImlkIjoiZG9tYWluLTEiLCJzZXJ2aWNlRW5kcG9pbnQiOiJodHRwczovL2Zvby5leGFtcGxlLmNvbSIsInR5cGUiOiJMaW5rZWREb21haW5zIn1dfX1dLCJ1cGRhdGVDb21taXRtZW50IjoiRWlBbFdRRFBDdWpNU1RLMG5wTzAwN1JPd043UFBhU2tGTlZxR05hYlhVc2NCUSJ9LCJzdWZmaXhEYXRhIjp7ImRlbHRhSGFzaCI6IkVpRDEzNk1sVy1HQXJfTldvNU1JSUVUZlRDVlZqSkQzeDVWS3FwX3FxMWtzd3ciLCJyZWNvdmVyeUNvbW1pdG1lbnQiOiJFaUR1dDZEQnhCbDI5YTRTOHhNLVVVR1hjYldrSWpnNEhPcFBqMDRZUE41VGV3In19"
      }
    ],
    "id": "did:ion:EiDwdKcZXuMP0dnuMOk0PmqkorpukCNRqasIKxUoG1t6UA:eyJkZWx0YSI6eyJwYXRjaGVzIjpbeyJhY3Rpb24iOiJyZXBsYWNlIiwiZG9jdW1lbnQiOnsicHVibGljS2V5cyI6W3siaWQiOiJrZXktMSIsInB1YmxpY0tleUp3ayI6eyJjcnYiOiJzZWNwMjU2azEiLCJrdHkiOiJFQyIsIngiOiJXZ3BwYUJicE1rcVEyeXRPd1VkVXVkZnBGRWg0aHRaN1dEczBoWWROMFdFIiwieSI6ImJKYjZXdE5pZXhrNHljUTFoYVQwbjRjQU5qLUk0OWp4b21od0ZINWlqMWMifSwicHVycG9zZXMiOlsiYXV0aGVudGljYXRpb24iXSwidHlwZSI6IkVjZHNhU2VjcDI1NmsxVmVyaWZpY2F0aW9uS2V5MjAxOSJ9XSwic2VydmljZXMiOlt7ImlkIjoiZG9tYWluLTEiLCJzZXJ2aWNlRW5kcG9pbnQiOiJodHRwczovL2Zvby5leGFtcGxlLmNvbSIsInR5cGUiOiJMaW5rZWREb21haW5zIn1dfX1dLCJ1cGRhdGVDb21taXRtZW50IjoiRWlBbFdRRFBDdWpNU1RLMG5wTzAwN1JPd043UFBhU2tGTlZxR05hYlhVc2NCUSJ9LCJzdWZmaXhEYXRhIjp7ImRlbHRhSGFzaCI6IkVpRDEzNk1sVy1HQXJfTldvNU1JSUVUZlRDVlZqSkQzeDVWS3FwX3FxMWtzd3ciLCJyZWNvdmVyeUNvbW1pdG1lbnQiOiJFaUR1dDZEQnhCbDI5YTRTOHhNLVVVR1hjYldrSWpnNEhPcFBqMDRZUE41VGV3In19",
    "verificationMethod": [
      {
        "id": "#key-1",
        "type": "EcdsaSecp256k1VerificationKey2019",
        "controller": "did:ion:EiDwdKcZXuMP0dnuMOk0PmqkorpukCNRqasIKxUoG1t6UA:eyJkZWx0YSI6eyJwYXRjaGVzIjpbeyJhY3Rpb24iOiJyZXBsYWNlIiwiZG9jdW1lbnQiOnsicHVibGljS2V5cyI6W3siaWQiOiJrZXktMSIsInB1YmxpY0tleUp3ayI6eyJjcnYiOiJzZWNwMjU2azEiLCJrdHkiOiJFQyIsIngiOiJXZ3BwYUJicE1rcVEyeXRPd1VkVXVkZnBGRWg0aHRaN1dEczBoWWROMFdFIiwieSI6ImJKYjZXdE5pZXhrNHljUTFoYVQwbjRjQU5qLUk0OWp4b21od0ZINWlqMWMifSwicHVycG9zZXMiOlsiYXV0aGVudGljYXRpb24iXSwidHlwZSI6IkVjZHNhU2VjcDI1NmsxVmVyaWZpY2F0aW9uS2V5MjAxOSJ9XSwic2VydmljZXMiOlt7ImlkIjoiZG9tYWluLTEiLCJzZXJ2aWNlRW5kcG9pbnQiOiJodHRwczovL2Zvby5leGFtcGxlLmNvbSIsInR5cGUiOiJMaW5rZWREb21haW5zIn1dfX1dLCJ1cGRhdGVDb21taXRtZW50IjoiRWlBbFdRRFBDdWpNU1RLMG5wTzAwN1JPd043UFBhU2tGTlZxR05hYlhVc2NCUSJ9LCJzdWZmaXhEYXRhIjp7ImRlbHRhSGFzaCI6IkVpRDEzNk1sVy1HQXJfTldvNU1JSUVUZlRDVlZqSkQzeDVWS3FwX3FxMWtzd3ciLCJyZWNvdmVyeUNvbW1pdG1lbnQiOiJFaUR1dDZEQnhCbDI5YTRTOHhNLVVVR1hjYldrSWpnNEhPcFBqMDRZUE41VGV3In19",
        "publicKeyJwk": {
          "kty": "EC",
          "crv": "secp256k1",
          "x": "WgppaBbpMkqQ2ytOwUdUudfpFEh4htZ7WDs0hYdN0WE",
          "y": "bJb6WtNiexk4ycQ1haT0n4cANj-I49jxomhwFH5ij1c"
        }
      }
    ],
    "authentication": [
      "#key-1"
    ],
    "service": [
      {
        "id": "#domain-1",
        "type": "LinkedDomains",
        "serviceEndpoint": "https://foo.example.com"
      }
    ]
  }
}

The result of Step 4, when doing

curl -X 'GET' \
  'https://ssi.tbddev.org/v1/dids/resolver/did%3Aion%3AEiDwdKcZXuMP0dnuMOk0PmqkorpukCNRqasIKxUoG1t6UA' \
  -H 'accept: application/json'

is the following resolution

{
  "didResolutionMetadata": {},
  "didDocument": {
    "@context": [
      "https://www.w3.org/ns/did/v1",
      {
        "@base": "did:ion:EiDwdKcZXuMP0dnuMOk0PmqkorpukCNRqasIKxUoG1t6UA"
      }
    ],
    "id": "did:ion:EiDwdKcZXuMP0dnuMOk0PmqkorpukCNRqasIKxUoG1t6UA",
    "verificationMethod": [
      {
        "id": "#key-2",
        "type": "EcdsaSecp256k1VerificationKey2019",
        "controller": "did:ion:EiDwdKcZXuMP0dnuMOk0PmqkorpukCNRqasIKxUoG1t6UA",
        "publicKeyJwk": {
          "kty": "EC",
          "crv": "secp256k1",
          "x": "qDXLOP6zSXdyNLb9Pp9cA0oI1tC_0BiNOyDJNFTH36M",
          "y": "x7TR47dVvmSj_-liLMuSJeOhP_PTvR30tEvfqnL8tuU"
        }
      }
    ],
    "authentication": [
      "#key-2"
    ],
    "service": [
      {
        "id": "#domain-1",
        "type": "LinkedDomains",
        "serviceEndpoint": "https://foo.example.com"
      },
      {
        "id": "#some-service-2",
        "type": "SomeServiceType",
        "serviceEndpoint": "http://www.example.com"
      }
    ]
  }
}

@decentralgabe
Copy link
Member

I don't see this as an error. The DID that's being resolved (long form) is fundamentally different than a short form which has guarantees about the latest state of the document.

We could add a flag in the response such as shortFormAvailable to note that the DID has been anchored and is resolvable as a short form

@andresuribe87
Copy link
Contributor Author

The current implementation doesn't follow the sidetree spec section on resolution. We can have a v1/dids/resolver be an endpoint that does not do the same resolution, but that would result in confusion to developers.

I think we should ensure that our implementation follows the spec.

@decentralgabe
Copy link
Member

we have no need to follow the sidetree spec on resolution since we're not a sidetree node

we need to ask ourselves: when someone resolves a long form DID, do they want the decoded long form DID, or the latest version of the DID

since we support the latter already I can see value in supporting the former (which is what's implemented) today

@andresuribe87
Copy link
Contributor Author

I agree we have no need, but as I mentioned, diverging will cause confusion. A developer using ssi-service will find it very confusing that universal resolver resolves a did to something that's different from what our v1/dids/resolver endpoint resolves it to.

when someone resolves a long form DID, do they want the decoded long form DID, or the latest version of the DID

The answer, to me, is that they want neither. They want what the DID should resolve to. They shouldn't have to worry about implementation details.

@decentralgabe
Copy link
Member

the main thing I'm concerned about is...

  1. Verifier gets a credential with a LFD, signed by kid #key-1
  2. ION DID is anchored, and updated to remove #key-1
  3. resolution on LFD which returns the latest state (SFD without #key-1)
  4. the data cannot be verified

until ION supports historical resolution it's much safer to resolve the exact state data was signed with. if that's a LFD, we should return it. if it's a SFD we cannot control whether a key was removed and we're out of luck

@andresuribe87
Copy link
Contributor Author

A couple of things.

  1. I originally thought the long form DIDs carry the current state. Turns out I was incorrect. They're only meant to contain the initial state of the DID Document. @csuwildcat can elaborate more on the reasons behind that.
  2. Not being able to verify the VC when it's signed with a key that was removed from the DID document is the behavior I expect. Can you elaborate why it's safer to be able to verify it?

@decentralgabe
Copy link
Member

  1. we've talked about extending this functionality to encode any current state + 1, it's not yet been implemented
  2. it's my assertion that any DID method that doesn't allow historical state resolution is broken (yes, that means all DID methods we support today are functionally broken)

The reason for (2) is that it increases confusion on what it means for a data payload to be valid. It is confusing to not distinguish between "valid but no longer actively using that key", "no longer valid," and "not able to be verified." It also conflates verification method rotation with verification method revocation.

Knowing that a piece of data is valid but a key is no longer in use OR was valid at a given point in time is useful information, especially considering that key rotation is a good security practice, and does not mean that a key was compromised (rotate), or the data is no longer valid (status change).

I believe to address this issue sidetree should:

  • Update spec text to require implementers to support historical resolution
  • Support rotation of keys by maintaining the rotated kid (e.g. #key-1 would stay in the verification methods section with the actual pub key removed)
  • Support revocation of keys by removing the kid from the latest state

This would allow us to be able to determine:

  1. A document authored by this DID is valid at present, and the key is still active in the document
  2. A document authored by this DID is valid at present, and the key is no longer active in the document
  3. A document authored by this DID was once valid, but the key has been revoked

Status could still amend all three cases.


Without the changes suggested above all we know is "able to be verified" or "not able to be verified" based on the presence of a key in the document. LFD offer us a slight improvement which for some set of data allows us to determine "was valid at a point in time" which is still useful information.

I suppose without clear spec text "was valid at a point in time" cannot, with certainty, be claimed as "still valid even though we're no longer using that key" -- so I'm supportive of fixing this issue with your suggestion, even though I still view sidetree as functionally lacking.

@csuwildcat
Copy link

@decentralgabe @andresuribe87 let's talk about this together instead of having three separate conversations. There are some assumptions that look off in these responses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
No open projects
Status: No status
Development

No branches or pull requests

4 participants