Invalid proof presentation when requesting revocable and non-revocable credentials

[mmoramartinez]

When requesting a proof presentation that contains two attribute groups, one from a revocable credential definition and one from a non-revocable definition, if the check for revocation is requested for all attributes, the proof is always invalid, even if the revocable credential is still active.

"presentation_request": {
        "nonce": "1104330525903017641791888",
        "name": "twoCredCheck",
        "version": "1.0",
        "requested_attributes": {
          "attrGrp_0": {
            "names": [
              "one",
              "two"
            ],
            "restrictions": [
              {
                "cred_def_id": "5vT6qtd26Rc9huD4tL2cXm:3:CL:8055:rev"
              }
            ]
          },
          "attrGrp_1": {
            "names": [
              "one",
              "two"
            ],
            "restrictions": [
              {
                "cred_def_id": "5vT6qtd26Rc9huD4tL2cXm:3:CL:8055:non"
              }
            ]
          }
        },
        "requested_predicates": {},
        "non_revoked": {
          "to": 1646301471
        }
      },
      "state": "request_sent"

During verification, “non_revoc_intervals” is not empty, but there is no timestamp for the non revocable attributes (indy/verifier.py).
if (timestamp is not None) ^ bool(non_revoc_intervals.get(uuid))
Error:
aries_cloudagent.indy.sdk.verifier ERROR Presentation on nonce=79516549657105118470684 cannot be validated: Timestamp on sub-proof #1 is missing vs. requested attribute group attrGrp_1

"results": [
    {
      "updated_at": "2022-03-03T10:00:04.820234Z",
      "connection_id": "e144d8a6-7996-446c-b61a-e80a0b440de5",
      "auto_present": false,
      "thread_id": "e3ac25cf-59e0-4e0d-90e3-0d85253143f5",
      "created_at": "2022-03-03T09:57:51.268109Z",
      "presentation_request": {
        "nonce": "1104330525903017641791888",
        "name": "twoCredCheck",
        "version": "1.0",
        "requested_attributes": {
          "attrGrp_0": {
            "names": [
              "one",
              "two"
            ],
            "restrictions": [
              {
                "cred_def_id": "5vT6qtd26Rc9huD4tL2cXm:3:CL:8055:rev"
              }
            ]
          },
          "attrGrp_1": {
            "names": [
              "one",
              "two"
            ],
            "restrictions": [
              {
                "cred_def_id": "5vT6qtd26Rc9huD4tL2cXm:3:CL:8055:non"
              }
            ]
          }
        },
        "requested_predicates": {},
        "non_revoked": {
          "to": 1646301471
        }
      },
      "state": "verified",
      "verified": "false",
...

In the case of a proof containing only non-revocable credentials, including the non revocation interval does not cause the proof to be invalid, so the same behaviour would be expected in the first case and the interval should be ignored when checking non-revocable attributes.

[swcurran]

I think we need to get this to work. IMHO, we should require that all presentation requests include a revocation interval and that we make sure that we can handle a proofs sourced from (1) only revocable credentials, (2) only non-revocable credentials and (3) a combination of revocable and non-revocable credentials. This would require a verifier to include a revocation interval, even if they don't care about revocation, but I think that is reasonable. If someone really wants to work around that, they can.

[swcurran]

@shaangill025 -- can you please look at this when you finish the current PQ work? Perhaps you can get a bit of help (if needed) from @ianco to setup test cases for this.

[swcurran]

Hmm...requiring a revocation interval in a proof request is a breaking change, so presumably that is not something we should do without careful consideration. I suppose we could do it at the ACA-Py level -- adding the revocation interval if not included. A challenge with this is getting the wallets (other frameworks) to handle this consistently. That was a challenge in the past with Aries Framework .NET. Probably we need to get these tests added to AATH to see how the interop is working.

• three tests (as above) with the proof request having a revocation interval
• three tests (as above) with the proof request not having a revocation interval

All should be possible.

[ianco]

I've added an integration test that demonstrates this issue, see PR #1672

I'll take a look at what aca-py is doing ... the error message the aca-py logs is:

2022-03-17 17:14:45,831 aries_cloudagent.indy.sdk.verifier ERROR Presentation on nonce=838013515937407266142254 cannot be validated: Timestamp on sub-proof #1 is missing vs. requested attribute health_attrs

(health_attrs is from a non-revocable credential)

[ianco]

@swcurran @mmoramartinez @andrewwhitehead

I've updated aca-py and added an integration test (see my PR referenced above) however now the proof is getting rejected by Indy (or Askar). Can someone please verify that I'm not doing something wrong in either the integration test or my aca-py updates?

[ianco]

OK Upon some research there are a couple of options for how to request proof of non-revocation in a proof request:

• non_revoked can be specified for the entire proof request
• non_revoked can be specified for individual requested attributes or predicates

See https://hyperledger-indy.readthedocs.io/projects/sdk/en/latest/docs/design/002-anoncreds/README.html

So, if you ask for:

"presentation_request": {
    "requested_attributes": {
        "attrGrp_0": {
            "names": ["one","two"],
            "restrictions": [{...}]
        },
        "attrGrp_1": {
            "names": ["three","four"],
            "restrictions": [{...}]
        },
    }
    "requested_predicates": {},
    "non_revoked": {"to": 1646301471}
}

... then both attribute groups need to prove non-revocation. I don't think a non-revocable credential can be presented in this scenario.

However you can ask for:

"presentation_request": {
    "requested_attributes": {
        "attrGrp_0": {
            "names": ["one","two"],
            "restrictions": [{...}]
            "non_revoked": {"to": 1646301471}
        },
        "attrGrp_1": {
            "names": ["three","four"],
            "restrictions": [{...}]
        },
    }
    "requested_predicates": {}
}

... and then you only need to prove non-revocation for the first group and not the second.

(You can also specify non_revoked at both the global and attribute levels, and the latter will override the former.)

So ... this is all great in theory, but I don't think aca-py supports all these scenarios. I've done some ad-hoc local testing and didn't get the expected results in some scenarios.

I'll update the integration test in this PR to be a bit more flexible to test the different non_revoked possibilities.

ALSO I think the current approach in aca-py is a bit problematic - we validate the received proof and "massage" it if it's not quite what we're expecting. I suggest a better approach is - if we don't like the received presentation, we kick back a problem report to the prover, and we always try to verify the exact presentation received.

See the code here, which does all the pre-verify verifying and "massaging": https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/indy/verifier.py

[swcurran]

The challenge I see if that the person creating the Proof Request does not know in the general case if the person will respond with claims from revokable or non-revokable credentials. IMHO, the proof validation process should still work.

The ideal I think is the verifier always ask for a revocation time, and if the user responds with a revokable credential, they must supply a proof-of-non-revocation, and if the user responds with a non-revokable credential, they don't supply the proof-of-non-revocation and it still verifies.

If the user responds without a proof-of-non-revocation for a revokable credential, the verification should fail.

I think that is what the manipulation by ACA-Py is trying to accomplish, because Indy/lower levels don't handle that. If the verifier requests a revocation time, and the holder responds with a non-revocable credential, the presentation request is adjusted to remove the revocation time so AnonCreds is happy.

[swcurran]

I get what you mean about not changing it in ACA-Py, and instead reporting an error to the Verifier, but that doesn't help, as we don't know there is an issue until after we receive the Proof and see the revocation time/non-revokable credential quandary. I think manipulating the proof-request in ACA-Py is the right way to go -- at least until the underlying AnonCreds function deals with the issue properly.

Now to get the rest of the community to do that as well :-).

FYI - the Aries RFC 0441 Indy Present Proof Best Practices might cover this -- I didn't read it thoroughly, but I believe it documents what Mr. Klump implemented in ACA-Py.

[swcurran]

Lots of good info in this issue. I think that the issue itself is resolved / understood and so I'm going to close this. Any fixes needed to happen at the AnonCreds level -- below ACA-Py. Ideally -- a fix would be to ignore the revocation interval info when the source credential for a presentation is non-revocable.