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 DIF presentation exchange context and cache document #3093

Merged
merged 3 commits into from
Jul 11, 2024

Conversation

gmulhearn
Copy link
Contributor

Fixes #2634

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
@gmulhearn gmulhearn marked this pull request as ready for review July 9, 2024 04:10
@jamshale jamshale changed the title Draft: Add DIF presentation exchange context and cache document Add DIF presentation exchange context and cache document Jul 9, 2024
Copy link
Contributor

@jamshale jamshale left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@jamshale
Copy link
Contributor

This will need to be updated with main. Not sure why I can't do it with this PR.

@gmulhearn-anonyome
Copy link
Contributor

Thanks @jamshale , done

Copy link

sonarcloud bot commented Jul 10, 2024

@jamshale jamshale merged commit 76f5a71 into hyperledger:main Jul 11, 2024
8 checks passed
@gmulhearn
Copy link
Contributor Author

Hey @jamshale i get the following error trying to load that:

ForbiddenRequest has expired8e22ee14:19021c9bc1d:6e5e7:120e

@jamshale
Copy link
Contributor

Sorry. Here's the stack trace instead.

DEBUG Match info: <MatchInfo {'pres_ex_id': '47a1cc1e-264c-4478-ac63-e40f5fd17885'}: <ResourceRoute [POST] <DynamicResource  /present-proof-2.0/records/{pres_ex_id}/verify-presentation> -> <function present_proof_verify_presentation at 0x7ffff9c68900>>
2024-07-15 18:07:19,498 aries_cloudagent.admin.server DEBUG Body: None
2024-07-15 18:07:19,510 aries_cloudagent.core.dispatcher ERROR Handler error: upgrade_middleware
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/asyncio/tasks.py", line 314, in __step_run_and_handle_result
    result = coro.send(None)
             ^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/admin/server.py", line 199, in upgrade_middleware
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aiohttp_apispec/middlewares.py", line 51, in validation_middleware
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/admin/decorators/auth.py", line 72, in tenant_auth
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/protocols/present_proof/v2_0/routes.py", line 1312, in present_proof_verify_presentation
    pres_ex_record = await pres_manager.verify_pres(pres_ex_record)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/protocols/present_proof/v2_0/manager.py", line 398, in verify_pres
    pres_ex_record = await pres_exch_format.handler(
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/protocols/present_proof/v2_0/formats/dif/handler.py", line 483, in verify_pres
    manager, options = self._get_type_manager_options(dif_proof, pres_request)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/protocols/present_proof/v2_0/formats/dif/handler.py", line 495, in _get_type_manager_options
    if dif_proof["proof"]["type"] == "DataIntegrityProof":
       ~~~~~~~~~^^^^^^^^^
KeyError: 'proof'
2024-07-15 18:07:19,517 aries_cloudagent.admin.server ERROR Handler error with exception: 'proof'

=================
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/admin/server.py", line 141, in ready_middleware
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/admin/server.py", line 212, in debug_middleware
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/admin/server.py", line 359, in setup_context
    return await task
           ^^^^^^^^^^
  File "/usr/local/lib/python3.12/asyncio/futures.py", line 287, in __await__
    yield self  # This tells Task to wait for completion.
    ^^^^^^^^^^
  File "/usr/local/lib/python3.12/asyncio/tasks.py", line 385, in __wakeup
    future.result()
  File "/usr/local/lib/python3.12/asyncio/futures.py", line 203, in result
    raise self._exception.with_traceback(self._exception_tb)
  File "/usr/local/lib/python3.12/asyncio/tasks.py", line 314, in __step_run_and_handle_result
    result = coro.send(None)
             ^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/admin/server.py", line 199, in upgrade_middleware
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aiohttp_apispec/middlewares.py", line 51, in validation_middleware
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/admin/decorators/auth.py", line 72, in tenant_auth
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/protocols/present_proof/v2_0/routes.py", line 1312, in present_proof_verify_presentation
    pres_ex_record = await pres_manager.verify_pres(pres_ex_record)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/protocols/present_proof/v2_0/manager.py", line 398, in verify_pres
    pres_ex_record = await pres_exch_format.handler(
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/protocols/present_proof/v2_0/formats/dif/handler.py", line 483, in verify_pres
    manager, options = self._get_type_manager_options(dif_proof, pres_request)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/protocols/present_proof/v2_0/formats/dif/handler.py", line 495, in _get_type_manager_options
    if dif_proof["proof"]["type"] == "DataIntegrityProof":
       ~~~~~~~~~^^^^^^^^^
KeyError: 'proof'
2024-07-15 18:07:19,519 aiohttp.server ERROR Error handling request
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/site-packages/aiohttp/web_protocol.py", line 452, in _handle_request
    resp = await request_handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aiohttp/web_app.py", line 543, in _handle
    resp = await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aiohttp/web_middlewares.py", line 114, in impl
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/admin/server.py", line 141, in ready_middleware
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/admin/server.py", line 212, in debug_middleware
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/admin/server.py", line 359, in setup_context
    return await task
           ^^^^^^^^^^
  File "/usr/local/lib/python3.12/asyncio/futures.py", line 287, in __await__
    yield self  # This tells Task to wait for completion.
    ^^^^^^^^^^
  File "/usr/local/lib/python3.12/asyncio/tasks.py", line 385, in __wakeup
    future.result()
  File "/usr/local/lib/python3.12/asyncio/futures.py", line 203, in result
    raise self._exception.with_traceback(self._exception_tb)
  File "/usr/local/lib/python3.12/asyncio/tasks.py", line 314, in __step_run_and_handle_result
    result = coro.send(None)
             ^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/admin/server.py", line 199, in upgrade_middleware
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aiohttp_apispec/middlewares.py", line 51, in validation_middleware
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/admin/decorators/auth.py", line 72, in tenant_auth
    return await handler(request)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/protocols/present_proof/v2_0/routes.py", line 1312, in present_proof_verify_presentation
    pres_ex_record = await pres_manager.verify_pres(pres_ex_record)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/protocols/present_proof/v2_0/manager.py", line 398, in verify_pres
    pres_ex_record = await pres_exch_format.handler(
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/protocols/present_proof/v2_0/formats/dif/handler.py", line 483, in verify_pres
    manager, options = self._get_type_manager_options(dif_proof, pres_request)
                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/aries_cloudagent/protocols/present_proof/v2_0/formats/dif/handler.py", line 495, in _get_type_manager_options
    if dif_proof["proof"]["type"] == "DataIntegrityProof":
       ~~~~~~~~~^^^^^^^^^
KeyError: 'proof'

@jamshale
Copy link
Contributor

jamshale commented Jul 15, 2024

Looks like maybe the proof field needs to be mapped over? If it's not a problem with this PR let me know but it looked like the module was changed recently here.

@gmulhearn
Copy link
Contributor Author

Hmm, yea it seems that the VP is missing the "proof" field; prover perhaps sent a VP without an LD proof. Is this just one exception that fails? Or do other similar tests fail? Also is this a private test suite (given that the test suite that ran in CI for this PR succeeded)?

@jamshale
Copy link
Contributor

It's a different test suite. It's public via the AATH tests for interoperability https://github.com/hyperledger/aries-agent-test-harness/actions/workflows/test-harness-acapy.yml. These tests are just acapy to acapy. https://allure.vonx.io/allure-docker-service-ui/projects/acapy. A few similar tests failed.

I thought they would have been covered by the repo's tests as well. We need to work on consolidating all these tests on PR's.

@jamshale
Copy link
Contributor

If you download the logs from the github action you can see the same exception happened 7 times for the same thing, so I believe that's the only problem.

@gmulhearn
Copy link
Contributor Author

just debugging what i've seen so far. it seems that the credential was issued without a credentialSubject.id assigned. So my gut would assume that since there is no credentialSubject ID, the holder (Bob) skips over signing the presentation, and just sends it unsigned (without a proof field).

Here's the VP that fails, note that there is not proof at the base level, and the nested credential does not have a credentialSubject.id:

{
  "@context": [
    "https://www.w3.org/2018/credentials/v1",
    "https://identity.foundation/presentation-exchange/submission/v1"
  ],
  "type": [
    "VerifiablePresentation",
    "PresentationSubmission"
  ],
  "verifiableCredential": [
    {
      "@context": [
        "https://www.w3.org/2018/credentials/v1",
        "https://w3id.org/security/bbs/v1",
        {
          "dl": "http://example.com/drivers-license#",
          "AATHDriversLicense": "dl:AATHDriversLicense",
          "address": "dl:address",
          "DL_number": "dl:DL_number",
          "expiry": "dl:expiry",
          "age": "dl:age"
        }
      ],
      "type": [
        "VerifiableCredential",
        "AATHDriversLicense"
      ],
      "issuer": "did:key:z6MkrMihCwgc54xPRnUTJbrY43Fz3z46esXDMZTdoFWppYdu",
      "issuanceDate": "2024-07-15T02:43:03Z",
      "credentialSubject": {
        "address": "947 this street, Kingston Ontario Canada, K9O 3R5",
        "DL_number": "09385029529385",
        "expiry": "10/12/2022",
        "age": "30"
      },
      "proof": {
        "type": "Ed25519Signature2018",
        "proofPurpose": "assertionMethod",
        "verificationMethod": "did:key:z6MkrMihCwgc54xPRnUTJbrY43Fz3z46esXDMZTdoFWppYdu#z6MkrMihCwgc54xPRnUTJbrY43Fz3z46esXDMZTdoFWppYdu",
        "created": "2024-07-15T02:43:09+00:00",
        "jws": "eyJhbGciOiAiRWREU0EiLCAiYjY0IjogZmFsc2UsICJjcml0IjogWyJiNjQiXX0..u2uL_uloYFjB0BkaHWJqZerPyzXrwUaUzcaNIfoQsSjIUiOhdygGCxkM7IRqdqjbDwjbi94RM0Y5ceodjWgyAg"
      }
    }
  ],
  "presentation_submission": {
    "id": "9e62b6d5-088e-4bbe-9923-0e8b53b25478",
    "definition_id": "5ab84299-c066-4b34-bcc6-d356aa44c826",
    "descriptor_map": [
      {
        "id": "drivers_license_input_1",
        "format": "ldp_vc",
        "path": "$.verifiableCredential[0]"
      }
    ]
  }
}

@gmulhearn
Copy link
Contributor Author

@jamshale I had a look at VPs created in the previous logs of acapy (before my PR), they seem to be missing signatures on their VPs too. My guess so far, is that ACAPY was previously "getting away" with "verifying" VPs that didn't have a proof field. But new changes brought in 3 days ago are now assuming that VPs have that proof.type field (VC DI changes, where the key error happens): a1a5afc#diff-277f226ac763ead09d6bef1596a9c3a8eb0c3127e15cb9d5fd2af7a3d2a406de

@jamshale
Copy link
Contributor

Ohhh... Ok. thanks for your help. This might be related to another PR that was recently merged then. Probably this PR.
#2960.

Sorry. I was wondering what could have happen with yours. I looked fine to me.

@gmulhearn
Copy link
Contributor Author

gmulhearn commented Jul 15, 2024

no worries. just a final note, it may be worth investigating ACApys verification of VPs at AATH. Above i said ACApy was previously "getting away" with it, but i don't think that's true, i think perhaps the AATH was getting away with it. Looking at logs of previous acapy Faber verifiers, many of the presentation exchange objects are logged with having "verified": "false"; likely a symptom of the VPs not having the .proof field, which would previously (before #2960) get caught here:

    # TODO validate presentation structure here
    if "proof" not in presentation:
        raise LinkedDataProofException('presentation must contain "proof"')

(and then caught and turned into verified=false).

So i suppose, if previously the AATH tests were passing (despite verified=false), then might need to be looked at.

Edit: i've taken this to a question in the AATH discord: https://discord.com/channels/905194001349627914/941708308459438121/1262534220467667004

@nodlesh
Copy link

nodlesh commented Jul 16, 2024

it seems that the credential was issued without a credentialSubject.id assigned

I'm a little confused here. ACAPy issued the credential a few steps before the verify in this test scenario. Is ACAPy supposed to create the credentialSubject.id when the credential is issued?

As for the "proof" in the presentation, Is this only required for JSON LD credentials? Indy credentials work without "proof".

@gmulhearn
Copy link
Contributor Author

@nodlesh ignoring special W3C VC types (such as new anoncreds w3c creds), i believe the typical way to bind a W3C VC to a specific holder, is by including credentialSubject.id fields in the issued credential. Then when the holder presents this credential (in the form of a W3C VP), they create a DataIntegrityProof signature over the VP (including the VC), where this signature is performed using a key associated with the credentialSubject.id of the presented VC (thereby proving they are the subject bound to the credential). credentialSubject.id is typically a DID.

The "proof" field within the W3C VC and VP data models are properties of JSON-LD Data Integrity Proofs, when a JSON-LD document is signed, this proof field will be populated with the signature details. A W3C VP does not necessarily require the proof field, however it is what makes it "Verifiable" (ignoring other ways to sign a W3C VP, such as JWTs).

My gut feeling on what is happening here: ACApy is offering/requesting/issuing the W3C VC without a credentialSubject.id. Meaning the credential is not bound to any holder. So then, when ACApy goes to present this VC within a VP, it skips over signing the VP since it cannot work out which key/DID it should sign the VP with (since the VC is not bound to any holder), resulting in a VP without the proof field. Previously ACApy verifiers would handle this gracefully by catching an error and reporting verified=false on the proof exchange, but due to new VC-DI changes, it throws an unexpected error.

Is ACAPy supposed to create the credentialSubject.id when the credential is issued?

good question, i think this is a question for the ACApy AATH backchannel. The typical flow i've seen with Aries W3C VC issuance (ignoring the new VC-DI aries attachment format being implemented) is:

  1. issuer offers VC to holder without any credentialSubject.id
  2. holder sends a request back, requesting the same VC payload, but with their holder DID assigned to the credentialSubject.id
  3. issuer signs this updated VC and sends the final issuance message

This shortcuts the negotiation required for including the credentialSubject.id. ACApy holder APIs support this directly by allowing holders to "accept the offer and request a credentialSubject.id be included" (see the V2 send-request API in ACApy).

So perhaps something ACApy AATH could do around this, is have their /agent/command/issue-credential-v2/send-request AATH backchannel handler call into the ACAPy send-request API with that holder_did (such that the acapy holder request that the w3c VC is bound to their holder DID). e.g.:

{
  "holder_did": "did:key:ahsdkjahsdkjhaskjdhakjshdkajhsdkjahs"
}

@gmulhearn
Copy link
Contributor Author

Alternatively, making sure the credential is bound to a credentialSubject.id DID could be something that is enforced at the AATH level. That might be more difficult though

@gmulhearn
Copy link
Contributor Author

But a related concern, is that i believe the AATH processor is not checking that verified=true from the verifiers POV when verifying the VPs (as raised in discord)

darshilnb pushed a commit to Northern-Block/aries-cloudagent-python that referenced this pull request Sep 5, 2024
…#3093)

* add static cache, and append fields

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

* linting and try load the test doc loader

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>

---------

Signed-off-by: George Mulhearn <gmulhearn@anonyome.com>
Co-authored-by: George Mulhearn <gmulhearn@anonyome.com>
Co-authored-by: gmulhearn-anonyome <90162009+gmulhearn-anonyome@users.noreply.github.com>
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.

Lack of DIF type in json-ld VPs causing dropped/ignored fields when normalized
4 participants