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

Issue with how the BC Wallet responds with non-revocable credential attributes to a presentation request containing a non-revocation interval predicate #762

Closed
WadeBarnes opened this issue Nov 30, 2022 · 15 comments
Labels
bug Something isn't working spike

Comments

@WadeBarnes
Copy link
Member

The A2A (ACM) team ran into an issue authenticating with ACM using the new accredited-lawyer-bcpc-dev presentation request.

The issue has to do with how the BC Wallet responds to presentation requests that contain a non-revocation interval predicate using attributes from a non-revocable credential; the wallet is not providing an appropriate and expected non-revocation interval for the non-revocable credential. This steams from, to quote Emiliano, "the fact that wallets (traditionally) do not cope well when validating non-revocable credentials with a non-revocation interval in the proof". Below are steps to reproduce for both the failure and success scenarios.

Summary

A user should be able to respond to a presentation request containing a non-revocation interval predicate using a non-revocable credential, provided that credential also satisfies the request.

Steps to reproduce - failure scenario:

  • Acquire an invite for a (revocable) Member Certificate through the A2A Issuer Admin pages.
    • Acquire a (revocable) Member Certificate through the A2A - Trust Over IP invite link. When filling out the form, make sure you use the same First Name and Last Name on this and the Open Verified Person credential you'll get later.
  • Acquire a (non-revocable) Open Verified Person Credential from CANdy - Unverified Person Issuer. Remember to use the same First Name and Last Name as you did with your (revocable) Member Certificate.
  • Browse to the Access to Court Materials dev site. URL can be provided on request.
  • Click Proceed to website
  • Scan the QR code.
  • View the Proof Request in the wallet. You'll see all attributes have been filled out with the attributes from the two credentials.
  • Click the share button in the wallet.

Observed behavior:

  • You get the following Unsuccessful Proof message from ACM.
    image
  • Clicking on the details you get the following info:
    {
      "name": "Accredited Lawyer with BC Person Credential",
      "names": null,
      "version": "1.0.0",
      "nonce": "937353492356337386129612",
      "requested_attributes": {
        "fcf6d204-fada-47cf-9a46-d73da8d62b4f": {
          "names": [
            "PPID",
            "Given Name",
            "Surname",
            "Member Status",
            "Member Status Code"
          ],
          "restrictions": [
            {
              "schema_name": "Member Certificate",
              "schema_version": "0.5.0",
              "issuer_did": "RznYFPVhHpYZgsn4Hu3StV"
            },
            {
              "schema_name": "Member Certificate",
              "schema_version": "1.0.1",
              "issuer_did": "RznYFPVhHpYZgsn4Hu3StV"
            },
            {
              "schema_name": "Member Card",
              "schema_version": "1.5.1",
              "issuer_did": "UUHA3oknprvKrpa7a6sncK"
            }
          ]
        },
        "1c4565b5-4b7d-491c-854e-7a8e2d00bd4e": {
          "names": [
            "family_name",
            "given_names"
          ],
          "restrictions": [
            {
              "schema_name": "Person",
              "schema_version": "1.0",
              "issuer_did": "XpgeQa93eZvGSZBZef3PHn"
            },
            {
              "schema_name": "Person",
              "schema_version": "1.0",
              "issuer_did": "7xjfawcnyTUcduWVysLww5"
            },
            {
              "schema_name": "unverified_person",
              "schema_version": "0.1.0",
              "issuer_did": "Ui6HA36FvN83cEtmYYHxrn"
            },
            {
              "schema_name": "unverified_person",
              "schema_version": "0.4.0",
              "issuer_did": "NCwGwDrzbZEqesYQummzWW"
            }
          ]
        }
      },
      "requested_predicates": {},
      "non_revoked": {
        "from": 0,
        "to": 1669840658
      }
    }
    
  • The vc-authn agent reports the following error due to how the wallet has responded to that portion of the presentation request, it has not provided the expected non-revocation interval for the non-revocable credential.
    aries_cloudagent.indy.sdk.verifier ERROR Presentation on nonce=937353492356337386129612 cannot be validated: Timestamp on sub-proof #1 is missing vs. requested attribute group 1c4565b5-4b7d-491c-854e-7a8e2d00bd4e
    

Expected behavior:

  • The wallet to provide an appropriate response to the non-revocation interval for the non-revocable credential.
  • The login to succeed because the proof satisfies the request even though some of the attributes are from a non-revocable credential.

Steps to reproduce - success scenario:

  • Delete the non-revocable Open Verified Person credential from your wallet.
  • Acquire a (revocable) Open Verified Person Credential from Unverified Person Issuer (Revocable). Remember to use the same First Name and Last Name as you did with your (revocable) Member Certificate
  • Browse to the Access to Court Materials dev site. URL can be provided on request.
  • Click Proceed to website
  • Scan the QR code.
  • View the Proof Request in the wallet. You'll see all attributes have been filled out with the attributes from the two credentials.
  • Click the share button in the wallet.

Observed behavior:

  • Login succeeds and you are presented with the following screen:
    image
  • The proof is valid because a non-revocation interval was provided for both credentials, only because both are revocable.
@WadeBarnes
Copy link
Member Author

WadeBarnes commented Nov 30, 2022

This is actually a long standing bug seen in most wallets.

We have actually had to implement and deploy workarounds for these scenarios. We have two instances of vc-authn to deal with this issue. One for revocable credentials and one for non-revocable credentials. In this case the issue was discovered because of the use of a mix of revokable and non-revokable credentials.

@WadeBarnes WadeBarnes added the bug Something isn't working label Nov 30, 2022
@jleach
Copy link
Member

jleach commented Nov 30, 2022

@WadeBarnes I may be conflating two different issues here but I think the wallet / AFJ works correctly. The BCLS is not providing the correct interval. We patch the wallet to work with BCLS in #697.

  "non_revoked": {
    "from": 0,
    "to": 1669840658
  }

As noted here

The verifier MUST specify, as current INDY-HIPE 11 notes, the same integer EPOCH time for both ends of the interval, or else omit the "from" key and value. In effect, where the presentation request specifies a non-revocation interval, the verifier MUST request a non-revocation instant.

The expected interval is:

  "non_revoked": {
    "from": 1669840658,
    "to": 1669840658
  }

@amanji Thoughts?

@WadeBarnes
Copy link
Member Author

WadeBarnes commented Nov 30, 2022

@jleach, The issue being reported is related to the use of the non-revokable Open Verified Person credential to respond to the presentation request which contains a non-revocation interval predicate. The LSBC Member certificate is not an issue in this scenario, it's just along for the ride.

@WadeBarnes
Copy link
Member Author

@esune, Any comments?

@esune
Copy link
Member

esune commented Nov 30, 2022

@WadeBarnes I think your explanation is pretty clear.

@swcurran
Copy link

swcurran commented Dec 1, 2022

To determine where the bug is, we have to look at the response back from the wallet. I'm not sure the bug is not in vc-authn-oidc in evaluating the presentation. The wallet should treat any proof request that includes a non-revocation range as not having that range if responding with a non-revocable credential. That is all the wallet can do -- there is no revocation information to include. The question is what vc-authn-oidc does when receiving the credential, and the information above is sufficient to know that.

@WadeBarnes
Copy link
Member Author

Related issue with additional info; openwallet-foundation/acapy#1651

@jleach
Copy link
Member

jleach commented Dec 8, 2022

Noted by Stephen in his Jun 3 comment "Any fixes needed to happen at the AnonCreds level -- below ACA-Py. Ideally". Moving to blocked and monitoring related issues.

@cvarjao
Copy link
Member

cvarjao commented Mar 2, 2023

The fix is probably at AFJ level, but for now we will create a test scenario to cover this use case and monitor

@jleach jleach assigned jleach and unassigned jleach Mar 7, 2023
@amanji amanji assigned amanji and unassigned amanji Mar 16, 2023
@cvarjao
Copy link
Member

cvarjao commented Apr 13, 2023

@swcurran , do you have any comment on this issue? We might need to update RFC for some clarity

@swcurran
Copy link

My understanding is that this can be worked around in the Presentation Request (so in this case vc-authn-oidc and the A2A level). An overall fix that avoids the workaround is needed at the AnonCreds implementation level.

A summary:

  • A non-revocable credential MUST be able to be used with a Presentation Request (PR) that includes a revocation instance, and vice-versa.
    • A verifier that accepts a credential from multiple issuers may not know that if all issuers use revovcation or not.
  • There is currently an issue in the AnonCreds implementations (AFAIK each of indy-sdk, indy-shared-rs and anoncreds-rs) such that if the non-revocation interval in the PR is specified at the REQUEST level, the verification will fail when a non-revocable credential is used in the presentation. However, if the non-revocation interval is specified at the ATTRIBUTE level, the verification will (as expected) pass when a non-revocable credential is used in the presentation.

So, the workaround needs to be done at the A2A deployment of vc-authn-oidc by either:

  • If the use of revocation is known for the precise credentials being used, take advantage of that information.
  • Set the non-revocation interval at the ATTRIBUTE (aka referent) level, not at the REQUEST level.

The latter should work before and after the fix to AnonCreds implementations, and regardless of whether the Issuer changes to using revocation.

The other guidance about PRs outlined in RFC 0441 Present Proof Best Practices should also be followed — notably, not to use 0 as the from value in the non-revocation interval.

@esune
Copy link
Member

esune commented Apr 13, 2023

So, the workaround needs to be done at the A2A deployment of vc-authn-oidc by either:

* If the use of revocation is known for the precise credentials being used, take advantage of that information.

* Set the non-revocation interval at the **ATTRIBUTE** (aka referent) level, not at the **REQUEST** level.

vc-authn 2.0 sets non-revocation intervals at the attribute level to work around the issue, as described by @swcurran.
It also changes the to/from to match the RFC - in the 1.0 version we HAD to use 0 as value for the starting interval as proofs would fail otherwise (see this for context: bcgov/vc-authn-oidc#172).

@cvarjao
Copy link
Member

cvarjao commented Apr 27, 2023

@swcurran , is there a better place we could move this issue? or would should we keep in bc-wallet?

@swcurran
Copy link

swcurran commented May 5, 2023

As long as the BC Wallet allows for responding to Proof Requests that have a revocation interval with satisfying credentials that are not revocable, and vice versa, this issue can be closed. If that is not the case — e.g. the wallet prevents sending a presentation in those scenarios, I would recommend opening another issue to address that. Either way, close this one.

Does that make sense?

@cvarjao
Copy link
Member

cvarjao commented Jul 11, 2023

@swcurran, @WadeBarnes , we don't have a use case for non-revocation credentials. If this is still impact, please open an issue with AFJ

@cvarjao cvarjao closed this as completed Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working spike
Projects
None yet
Development

No branches or pull requests

6 participants