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

Added did:web support to holder for JSON-LD proof presentation #3138

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

aritroCoder
Copy link

@aritroCoder aritroCoder commented Jul 31, 2024

Problem description

When the holder uses a did:web in a credential, it is unable to present proofs for that credential. The /present-proof-2.0/records/{pres_ex_id}/send-presentation API endpoint terminates with the message: Unable to retrieve verification method for did.

Where the problem occurs

When the send-presentation endpoint is called, it builds the VP by calling the following functions:

  • Function present_proof_send_presentation at aries_cloudagent/protocols/present_proof/v2_0/routes.py
  • create_pres at aries_cloudagent/protocols/present_proof/v2_0/manager.py
  • create_pres at aries_cloudagent/protocols/present_proof/v2_0/formats/dif/handler.py
  • create_vp at aries_cloudagent/protocols/present_proof/dif/pres_exch_handler.py
  • _get_issue_suite at aries_cloudagent/protocols/present_proof/dif/pres_exch_handler.py
  • get_verification_method_id_for_did at aries_cloudagent/wallet/default_verification_key_strategy.py. This is where the error occurs.

How we are fixing it:

  • As per current observation we need to add a verification key derivation strategy for did:web in the function default_verification_key_strategy.
  • To do this, we can resolve the did:web using ACA-py's resolve method, look at the services, and use the verification key for a service.
  • We need to do this because the function assumes a did:key or did:sov is used as the holder id in the credential subject ID.

Interoperability and performance:

  • The implementation works for preliminary testing of did:web credential issuance and verification flow.
  • The unit tests that came with ACA-py also pass.
  • The implementation has the performance overhead of a did:web resolve call which is essentially dependent on the network of the user.

NOTE

When checking for verification method id for did:web, we resolve the DID and take the first key available in the DID document. There might be better approaches to this which the contributors can suggest.


Signed-off-by: aritroCoder <[email protected]>
@dbluhm
Copy link
Contributor

dbluhm commented Aug 1, 2024

I think these changes are acceptable but agree that there might be better approaches to determining which verification method ID to use based on the DID. For this particular scenario, we should know the VM id because the credential proof should contain a VM ID.

@PatStLouis might be a good idea to keep this scenario in mind as you work on DataIntegrity and VCDM 2.0 updates.

@dbluhm
Copy link
Contributor

dbluhm commented Aug 1, 2024

Would it make sense to use the first VM referenced or embedded in the assertionMethod verification relationship?

@PatStLouis
Copy link
Contributor

I'm currently not convinced by this solution as this assumes that the first verification method corresponds to the keypair used to sign the document. I would rather see a field for user provided verificationMethod like it's the case for issuing credentials. I believe this fix would break the current issuance model where provides the verificaitonMethod through the issuance options.

@aritroCoder could you expand on the issue you are having and how I can reproduce it? What is the /create-presentation endpoint? Afaik this isn't an existing endpoint in aca-py.

@PatStLouis
Copy link
Contributor

@dbluhm wasn't there a way to bind a verificationMethod to a did in the didcomm-v2 PR? I think this is a much better approach, when a client creates a did, they can optionally provide a default verificationMethod

@aritroCoder
Copy link
Author

aritroCoder commented Aug 1, 2024

I'm currently not convinced by this solution as this assumes that the first verification method corresponds to the keypair used to sign the document. I would rather see a field for user provided verificationMethod like it's the case for issuing credentials. I believe this fix would break the current issuance model where provides the verificaitonMethod through the issuance options.

@aritroCoder could you expand on the issue you are having and how I can reproduce it? What is the /create-presentation endpoint? Afaik this isn't an existing endpoint in aca-py.

Yes, that was a typo from me and I corrected it. It should be /present-proof-2.0/records/{pres_ex_id}/send-presentation endpoint. For a credential whose subject contains the holder DID which is not did:sov or did:key, then this API endpoint fails to run with error message: Unable to retrieve verification method for did

Steps to reproduce:

  • Use a Credential offer body (/issue-credential-2.0/send-offer):
{
  "auto_issue": false,
  "auto_remove": false,
  "comment": "Credential from minimal example",
  "connection_id": "df82e639-c15c-4374-8e9a-f820be2a1c6f",    #
  "filter": {
    "ld_proof": {
      "credential": {
        "@context": [
          "https://www.w3.org/2018/credentials/v1",
          "https://w3id.org/citizenship/v1"
        ],
        "credentialSubject": {
          "birthCountry": "Bahamas",
          "birthDate": "1958-07-17",
          "familyName": "Builder",
          "gender": "Male",
          "givenName": "Bob",
          "id": "did:web:aritrocoder.github.io:didwebholder",   #
          "type": [
            "PermanentResident"
          ]
        },
        "issuanceDate": "2024-05-24",
        "issuer": "did:web:aritra-issuer-faber.nborbit.ca",   #
        "type": [
          "VerifiableCredential",
          "PermanentResident"
        ]
      },
      "options": {
        "proofType": "Ed25519Signature2018",
        "proofPurpose": "assertionMethod", 
        "verificationMethod": "did:web:aritra-issuer-faber.nborbit.ca#key-1"      #
      }
    }
  },
  "trace": false
}

In the above body, replace fields marked # with the information relevant to your agent (just make sure all the DID and verification methods are for did:web).

  • In the holder agent, send the issuer a request for the credential using /issue-credential​-2.0/records​/{cred_ex_id}​/send-request
  • In the issuer, issue the credential using /issue-credential-2.0/records/{cred_ex_id}/issue
  • In holder, save the credential using /issue-credential-2.0/records/{cred_ex_id}/store
  • In the verifier, send a proof request to holder containing the body (using /present-proof-2.0/send-request)
{
  "auto_verify": false,
  "comment": "Presentation request from minimal",
  "connection_id": "6a54d6e2-f371-403a-84ab-0e633b4939f3",     # get from conn id
  "presentation_request": {
    "dif": {
      "options": {
        "challenge": "bdf0c9e7-2d2b-45f7-aae6-961329835d85",
        "domain": "test-degree"
      },
      "presentation_definition": {
        "format": {
          "ldp_vp": {
            "proof_type": [
              "Ed25519Signature2018"
            ]
          }
        },
        "id": "a5487d6d-50e6-4bfe-bb94-52c2deb8d2a2",
        "input_descriptors": [
          {
            "constraints": {
              "fields": [
                {
                  "filter": {
                    "const": "Builder"
                  },
                  "id": "1f44d55f-f161-4938-a659-f8026467f126",
                  "path": [
                    "$.credentialSubject.familyName"
                  ],
                  "purpose": "The claim must be from one of the specified issuers"
                },
                {
                  "path": [
                    "$.credentialSubject.givenName"
                  ],
                  "purpose": "The claim must be from one of the specified issuers"
                }
              ],
              "is_holder": [
                {
                  "directive": "required",
                  "field_id": [
                    "1f44d55f-f161-4938-a659-f8026467f126"
                  ]
                }
              ]
            },
            "id": "citizenship_input_1",
            "name": "EU Driver's License",
            "schema": [
              {
                "uri": "https://www.w3.org/2018/credentials#VerifiableCredential"
              },
              {
                "uri": "https://w3id.org/citizenship#PermanentResident"
              }
            ]
          }
        ]
      }
    }
  },
  "trace": false
}
  • In the holder, use /present-proof-2.0/records/{pres_ex_id}/send-presentation with the body:
{
  "dif": {
    "presentation_definition": {
      "format": {
        "ldp_vp": {
          "proof_type": [
            "Ed25519Signature2018"
          ]
        }
      },
      "id": "a5487d6d-50e6-4bfe-bb94-52c2deb8d2a2",  
      "input_descriptors": [
        {
          "constraints": {
            "fields": [
              {
                "filter": {
                  "const": "Builder"
                },
                "id": "1f44d55f-f161-4938-a659-f8026467f126", 
                "path": [
                  "$.credentialSubject.familyName"
                ],
                "purpose": "The claim must be from one of the specified issuers"
              },
              {
                "path": [
                  "$.credentialSubject.givenName"
                ],
                "purpose": "The claim must be from one of the specified issuers"
              }
            ],
            "is_holder": [
              {
                "directive": "required",
                "field_id": [
                  "1f44d55f-f161-4938-a659-f8026467f126" 
                ]
              }
            ]
          },
          "id": "citizenship_input_1",
          "name": "EU Driver's License",
          "schema": [
            {
              "uri": "https://www.w3.org/2018/credentials#VerifiableCredential"
            },
            {
              "uri": "https://w3id.org/citizenship#PermanentResident"
            }
          ]
        }
      ]
    }
  }
}

At this step the error occurs

@PatStLouis
Copy link
Contributor

Thanks for clarifying. One way to create a binding is to ensure that the verificationMethod matches at least one of the ldp_vp.proof_type. In your example, you use the Ed25519Signature2018 proof_type. So you could create a mapping of proof types and pick the first verificationMethod that matches the proof_type you will use for the signature.

Here's so pseudo code:

key_types_mapping = {
    "Ed25519Signature2018": "Ed25519VerificationKey2018",
    "Ed25519Signature2020": "Ed25519VerificationKey2020",
}
proof_type = "Ed25519Signature2018"
verification_method_type = key_types_mapping[proof_type]
verification_method_list = did_document.get("verificationMethod", {})
verification_method_id = next(
    (
        verification_method["id"]
        for verification_method in verification_method_list
        if verification_method["type"] == verification_method_type
    ),
    None,
)

@PatStLouis
Copy link
Contributor

PatStLouis commented Aug 1, 2024

In the current state, this feature is undocumented and is inserting a behavior which could be problematic in some cases.

But this is definitely something that needs to be addressed and is a valid error/issue to raise

@aritroCoder
Copy link
Author

@PatStLouis I implemented the change you suggested, you can have a look at it. I did the preliminary testing and it was working.

Copy link
Contributor

@PatStLouis PatStLouis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make verification method types an array to support multikey

@PatStLouis
Copy link
Contributor

This should do the trick, however I still want to point out I'm not convinced by this solution as it makes a lot of opinions and is currently undocumented. Let's wait before 1.0 release before looking at merging this. I would also strongly consider alternatives such as binding a verificationMethod to a key pair or providing an explicit verificationMethod as part of the presentation creation step.

@dbluhm I think the issue here is that this isn't a verification but the holder looking to sign the presentation, so it's unrelated to the proof of the VC. With the issuance we can provide a verificationMethod.id through the options but here, since this is a call made during the present-proof exchange, we don't have a way to automate it.

We could add a verificationMethod field to the dif filter:

{
  "dif": {
    "presentation_definition": {},
    "options": {
      "verificationMethod": "did:web:...#key"
    }
  }
}

This would serve a similar purpose, providing the holder the ability to select which keypair to use for the presentation proof.

aritroCoder and others added 3 commits August 6, 2024 18:18
added multikey support

Co-authored-by: Patrick St-Louis <[email protected]>
Signed-off-by: Aritra Bhaduri <[email protected]>
added multikey support

Co-authored-by: Patrick St-Louis <[email protected]>
Signed-off-by: Aritra Bhaduri <[email protected]>
added multikey support

Co-authored-by: Patrick St-Louis <[email protected]>
Signed-off-by: Aritra Bhaduri <[email protected]>
@aritroCoder
Copy link
Author

aritroCoder commented Aug 14, 2024

Hi @PatStLouis you can take your time and suggest best way this can be done according to you, and I can implement it. I can also add the verification method with the issuance object if that looks better. We can finalize it after a discussion with the community.
As for sending verification method with issuance, it requires modifying the ACA-py credential issuance schemas which may break compatibility with older systems

@swcurran
Copy link
Contributor

I thought I heard on one of the calls that this might be better as a plugin. Is that the approach that should be taken?

Copy link

sonarcloud bot commented Aug 14, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
28.6% Coverage on New Code (required ≥ 80%)
E Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@PatStLouis
Copy link
Contributor

@aritroCoder it's not the issuance, but the presentation request. Issuance already supports passing the verification method. The issue is that the step where the holder signs the credential is midway through the protocol and cannot be set deterministically, if the protocol is automated unless the verifier knows in advance which verification method to request.

I'm assuming you are doing a business to business use case since the dif presentation exchange implementation was designed around did:key / did:sov where each did is self resolvable to 1 verification method only. Did web introduces verification method entropy. I think your suggestion is the best current way to automate this, unless we make it a requirement for did:web to have the protocol request a "manual" presentation step, requiring the holder to declare which verification method to use.

This being said, if you have a use case that you need to leverage present-proof-v2 where the holder is using did:web (highly correlatable, no privacy), this is the simplest way I can think of achieving this.

The other option is to have a "default" verificationMethod associated with a given did.

If you are willing to spend some more time with this, I would look at adding an optional field to the did:web registration call to provide a default verificationMethod. This option should only be enabled for specific did methods, did:web only for now.

{
  "method": "web",
  "options": {
    "did": "did:web:example.com",
    "verification_method": "did:web:example.com#verkey",
    "key_type": "ed25519"
  }
}

@dbluhm how do you feel about this approach? This would require a modificaiton to the didInfo class, adding a default_verificaiton_method attribute. How does this conflict/overlap with the verificationMethod binding approach from the did peer work?

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.

4 participants