TBD54566975 / ssi-service

The Self Sovereign Identity Service
Apache License 2.0
152 stars 55 forks source link

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

Open andresuribe87 opened 1 year ago

andresuribe87 commented 1 year ago

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 commented 1 year ago

I just did steps 1 & 2 with

did:ion:EiDwdKcZXuMP0dnuMOk0PmqkorpukCNRqasIKxUoG1t6UA:eyJkZWx0YSI6eyJwYXRjaGVzIjpbeyJhY3Rpb24iOiJyZXBsYWNlIiwiZG9jdW1lbnQiOnsicHVibGljS2V5cyI6W3siaWQiOiJrZXktMSIsInB1YmxpY0tleUp3ayI6eyJjcnYiOiJzZWNwMjU2azEiLCJrdHkiOiJFQyIsIngiOiJXZ3BwYUJicE1rcVEyeXRPd1VkVXVkZnBGRWg0aHRaN1dEczBoWWROMFdFIiwieSI6ImJKYjZXdE5pZXhrNHljUTFoYVQwbjRjQU5qLUk0OWp4b21od0ZINWlqMWMifSwicHVycG9zZXMiOlsiYXV0aGVudGljYXRpb24iXSwidHlwZSI6IkVjZHNhU2VjcDI1NmsxVmVyaWZpY2F0aW9uS2V5MjAxOSJ9XSwic2VydmljZXMiOlt7ImlkIjoiZG9tYWluLTEiLCJzZXJ2aWNlRW5kcG9pbnQiOiJodHRwczovL2Zvby5leGFtcGxlLmNvbSIsInR5cGUiOiJMaW5rZWREb21haW5zIn1dfX1dLCJ1cGRhdGVDb21taXRtZW50IjoiRWlBbFdRRFBDdWpNU1RLMG5wTzAwN1JPd043UFBhU2tGTlZxR05hYlhVc2NCUSJ9LCJzdWZmaXhEYXRhIjp7ImRlbHRhSGFzaCI6IkVpRDEzNk1sVy1HQXJfTldvNU1JSUVUZlRDVlZqSkQzeDVWS3FwX3FxMWtzd3ciLCJyZWNvdmVyeUNvbW1pdG1lbnQiOiJFaUR1dDZEQnhCbDI5YTRTOHhNLVVVR1hjYldrSWpnNEhPcFBqMDRZUE41VGV3In19

Will post an update once enough time has passed.

decentralgabe commented 1 year ago

@andresuribe87 update here?

andresuribe87 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago
  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:

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 commented 1 year ago

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