hyperledger / aries-cloudagent-python

Hyperledger Aries Cloud Agent Python (ACA-Py) is a foundation for building decentralized identity applications and services running in non-mobile environments.
https://wiki.hyperledger.org/display/aries
Apache License 2.0
404 stars 511 forks source link

Unrevealed attribute group always marked as verified=false #2306

Open gmulhearn-anonyome opened 1 year ago

gmulhearn-anonyome commented 1 year ago

Scenario:

  1. proof request the following requested attributes:

    "requested_attributes": {
          "1": {
            "names": ["gpa","year"],
            "restrictions": [{ "cred_def_id": "MNskjCH2i6UUDvzUCUBM5L:3:CL:8417:SUDO_EA_RS_CRED_DEF1" }]
          }
        },
  2. present using anoncreds-rs, with revealed = false

  3. aca-py requested_proof like so:

    "requested_proof": {
          "predicates": {},
          "revealed_attrs": {},
          "self_attested_attrs": {},
          "unrevealed_attrs": {
            "1": {"sub_proof_index": 0}
    }}
  4. aca-py reports verified=false, and gives the warning/error: "VALUE_ERROR::Missing requested attribute group 1"

looking in verifier.py check_timestamps, it looks like in the case of names (i.e. group of attributes was requested), if the referent ("1" in this case) is not listed under requested_proof.revealed_attr_groups, then it automatically throws that missing value error, and it never checks in requested_proof.unrevealed_attrs for the referent.

in the case of name, the implementation checks unrevealed_attrs as you'd expect, and then adds a warning message if the referent is present: PresVerifyMsg.CT_UNREVEALED_ATTRIBUTES.value}::" f"{uuid}".

I believe that presenting an attribute group as unrevealed is a cryptographically valid use case, so i think the aca-py impl should allow this case too

swcurran commented 1 year ago

What version of the ACA-Py are you looking at? This PR was merged some time ago — https://github.com/hyperledger/aries-cloudagent-python/pull/1913 — that updated the unrevealed attribute handling.

gmulhearn commented 1 year ago

Encountered this on ACA-py 0.8.1 (haven't been able to try 0.8.2 due to my current environment limitation), however the code I linked indicates that the issue is still present on main.

My understanding is that #1913 added support for checking if name referents (i.e. single attribute referents) are unrevealed, but it does not seem to address the case where a names referent (i.e. multi attributes under a referent) is unrevealed.

swcurran commented 1 year ago

Arrggh….

Thanks. That makes sense (doesn’t make sense…). The whole mess of name vs names is such a pain. Name should have been deprecated and eliminated long ago. But if there are two places, both should have been fixed in #1913.

OK — I’ll keep this in mind to get fixed. Won’t be done immediately, I’m afraid, but I’ll keep it near the top.