aries-cloudagent-python
aries-cloudagent-python copied to clipboard
hyperledger
Unrevealed attributes cause the verification to fail
What is currently happening?
If the verifier is ok in having a flexible proof presentation and would allow the prover to proove only some attributes, currently there isn't a great method to either indicate attributes as optional or to negotiate which attributes to include in a proof. If the prover simply doesn't reveal some of the requested attributes the verification of the proof will automatically fail.
Example case
If the prover doesn't want to reveal a specific attribute (the "surname" attribute in this example) but wants to follow through with the rest and using the Open API calls the send-presentation endpoint with the following data:
send-presentation call
{
"requested_attributes": {
"0_name_uuid": {
"cred_id": "referent",
"revealed": True
},
"0_id_number_uuid": {
"cred_id": "referent",
"revealed": True
},
"0_surname_uuid": {
"cred_id": "referent",
"revealed": False
},
},
"requested_predicates": {
},
"self_attested_attributes":{ },
"trace": False
}
This message has some attributes set as revealed True and the "surname" to False.
The message send from the prover to the verifier is the following:
Presentation Message
{
"connection_id": "89ad56f3-05c4-40e1-9456-c6623aac9c07",
"trace": false,
"state": "verified",
"initiator": "self",
"presentation_request_dict": {
"@type": "did:sov:BzCbsNYhMrjHiqZDTUASHg;spec/present-proof/1.0/request-presentation",
"@id": "d4075e9d-5467-4d4f-9098-e244e6f4e15f",
"request_presentations~attach": [
{
"@id": "libindy-request-presentation-0",
"mime-type": "application/json",
"data": {
"base64": "eyJuYW1lIjogIlByb29mIG9mIElEIiwgInZlcnNpb24iOiAiMS4wIiwgInJlcXVlc3RlZF9hdHRyaWJ1dGVzIjogeyIwX25hbWVfdXVpZCI6IHsibmFtZSI6ICJuYW1lIiwgInJlc3RyaWN0aW9ucyI6IFt7ImNyZWRfZGVmX2lkIjogIjRkdGdkc0h1dmoxcDNueXF6dlFqS246MzpDTDo2MjpwZXJzb25hbElkIn1dfSwgIjBfc3VybmFtZV91dWlkIjogeyJuYW1lIjogInN1cm5hbWUiLCAicmVzdHJpY3Rpb25zIjogW3siY3JlZF9kZWZfaWQiOiAiNGR0Z2RzSHV2ajFwM255cXp2UWpLbjozOkNMOjYyOnBlcnNvbmFsSWQifV19LCAiMF9pZF9udW1iZXJfdXVpZCI6IHsibmFtZSI6ICJpZF9udW1iZXIiLCAicmVzdHJpY3Rpb25zIjogW3siY3JlZF9kZWZfaWQiOiAiNGR0Z2RzSHV2ajFwM255cXp2UWpLbjozOkNMOjYyOnBlcnNvbmFsSWQifV19LCAiMF9zZWxmX2F0dGVzdGVkX3RoaW5nX3V1aWQiOiB7Im5hbWUiOiAic2VsZl9hdHRlc3RlZF90aGluZyJ9fSwgInJlcXVlc3RlZF9wcmVkaWNhdGVzIjoge30sICJub25jZSI6ICIxMDUwMDYyNDQ3OTQ5NTkxNTUwNjI4MDg3In0="
}
}
],
"comment": "Alice should prove these attributes"
},
"presentation_exchange_id": "31719d5e-474a-4ef1-aaae-b2bdeca4f51c",
"role": "verifier",
"presentation_request": {
"nonce": "1050062447949591550628087",
"name": "Proof of ID",
"version": "1.0",
"requested_attributes": {
"0_name_uuid": {
"name": "name",
"restrictions": [
{
"cred_def_id": "4dtgdsHuvj1p3nyqzvQjKn:3:CL:62:personalId"
}
]
},
"0_surname_uuid": {
"name": "surname",
"restrictions": [
{
"cred_def_id": "4dtgdsHuvj1p3nyqzvQjKn:3:CL:62:personalId"
}
]
},
"0_id_number_uuid": {
"name": "id_number",
"restrictions": [
{
"cred_def_id": "4dtgdsHuvj1p3nyqzvQjKn:3:CL:62:personalId"
}
]
}
},
"requested_predicates": {}
},
"presentation": {
"proof": {
"proofs": [
{
"primary_proof": {
"eq_proof": {
"revealed_attrs": {
"id_number": "115180327462699894979562043104106501556488886182032957666468162020401194019730",
"name": "27034640024117331033063128044004318218486816931520886405535659934417438781507"
},
"a_prime": "67197175726728447749221983425256242162703806854350225169729512074217922903236980851020874695317996693508878332122157168191520240903517234969419296671985979688834490892324697419663284785723069955841660617395849571313647486243405583195998910243746087297978908743611066452580926095984871231637373691367795163296097142964070524786987645517008298750185526974792636099181485479276922215166336927808319613185542558677057376005592056355305150753684825349947920645853687477837725784921985594878480624167184725170682286931428102308732862613031484827423114422826092719661417389485717235254439260172124537763168697990653968688675",
"e": "91381387665698868227662198268166242406619148606161817458840595652360065601349332435157180396782810157865906398372616749518755194731005344",
"v": "398226143239317593817064245540601368469811834935178622986828067368336925623727140582713260089172517569177567925653015531345500526715238625146604240821224024915249839897373572302203876955249630075182706050276651173416565921085337704278624940532876680045259692010612393309984044622124113420398824763661824422908357720569099775877848050913808550365619154411522794059046823128721420439997089537338427873604946712687114741702719925280934364350168886496263842279851215726756415417095343347446729880444836282457673171434193799554066004655986019232991107472566831830410609212623456126744545722768942783121073075509709954101073883496172768590592019811211208604159568765132143980963916054088003291207401162913676781193922567270055967503424738100339309861840576687830327394544492636785899866699978985600312038210883073216719454041050761491832760327377633909713830223220173039542149820629761703314511950751781828903643291159527471599",
"m": {
"surname": "5948724803051118632131326800199348052871099901252899467846313142885659522779376472217427660023568731562336040964062115214029959419502712657997078209851146278546549617549226833928",
"master_secret": "5158880197274689153573377005890760992106519474315096258720133690703071420905267787687755938393206852136568440841062300694541874186781578781138626681139885147584666924605867284067"
},
"m2": "13086352799721374888215276319342702115565602236684797840471361335340255999106254554645058339023550501008254358258805702273795502862565384609416220107808122366418869809850513967933"
},
"ge_proofs": []
}
}
],
"aggregated_proof": {
"c_hash": "55036692055909262949240081123272451959634401177917882467516918734110285933018",
"c_list": [
[
2,5 ...
]
]
}
},
"requested_proof": {
"revealed_attrs": {
"0_id_number_uuid": {
"raw": "00000000A",
"encoded": "115180327462699894979562043104106501556488886182032957666468162020401194019730",
"sub_proof_index": 0
},
"0_name_uuid": {
"raw": "Alice",
"encoded": "27034640024117331033063128044004318218486816931520886405535659934417438781507",
"sub_proof_index": 0
}
},
"self_attested_attrs": {},
"unrevealed_attrs": {
"0_surname_uuid": {
"sub_proof_index": 0
}
},
"predicates": {}
},
"identifiers": [
{
"schema_id": "4dtgdsHuvj1p3nyqzvQjKn:2:personalId:18.57",
"cred_def_id": "4dtgdsHuvj1p3nyqzvQjKn:3:CL:62:personalId"
}
]
},
"created_at": "2022-08-08T10:17:39.755373Z",
"auto_present": false,
"updated_at": "2022-08-08T10:17:42.079163Z",
"verified": "false",
"thread_id": "d4075e9d-5467-4d4f-9098-e244e6f4e15f"
}
This message contains information about the unrevealed attribute but not the encoded or raw value but rather the encripted value necessary to verify the proof. You can find this following the path: presentation.proof.proofs.primary_proof.m.surname
When ACA-Py tries to verify the proof it fails while doing some checks both in the "check_timestamps" and "pre_verify" functions found here.
Nevertheless if I remove these 2 checks from ACA-Py then the verifier function "indy.anoncreds.verifier_verify_proof" found here returns true. From what I understand even though the indy sdk considers this a correctly validated proof ACA-Py considers this as not valid.
Question
I have not ben able to find any definitive information on how not revealing attributes affects the verification of the proof in the Aries RFCs or in the indy sdk documentation. Should a proof be considered valid if the information to verify the signature is present and available but the verifier doesn't get all the encoded/raw values he requested?
If this functionality is open to interpretation for the verifier and he should choose what to do when this edge case arises can I just ammend the pre-check functions to fulfill my needs or can there be other unknown issues when adapting the check to my needs. i e, can I just deploy the cloudagent with the code modified so that it accepts these proofs?
Proposal
If its decided that the proof should be valid even if there are unrevealed attributes, the pre-check funcions should be ammended so that they take into consideration that some attributes might be in the "unrevealed_attrs" section.
If its not clear if the proof should be considered valid or not maybe it would be possible to add more values than just "verified", there could be another value like "validated" or something else that would only check the correctness of the mathematical signatures of the proof.
I think we should just remove the code from aca-py (that checks for unrevealed attribute values) and just use the anoncreds verification to set the "verified" flag. If there are unrevealed attributes we should set the verification to "true" (assuming the anoncreds verification verifies) and then let the controller decide whether to accept the proof (based on whether attributes are revealed or not or whatever other logic they decide).
Note that AnonCreds behaviour, in this scenario, is to include the unrevealed attribute in "unrevealed_attributes" and provide neither the encoded nor raw value, for example:
"requested_proof": {
"revealed_attrs": {
"0_date_uuid": {
"sub_proof_index": 0,
"raw": "2018-05-28",
"encoded": "23402637423876324098256519317695433196813217785795317220680415812348801086586"
},
"0_degree_uuid": {
"sub_proof_index": 0,
"raw": "Maths",
"encoded": "460273229220408542178729328948548235132905393400001582342944147813984660772"
}
},
"self_attested_attrs": {},
"unrevealed_attrs": {
"0_name_uuid": {
"sub_proof_index": 0
}
},
"predicates": {
"0_birthdate_dateint_GE_uuid": {
"sub_proof_index": 0
}
}
}
In the above example 0_name_uuid is not revealed and the proof is validated (as far as AnonCreds is concerned). However aca-py rejects this scenario, since 0_name_uuid was requested and not provided.
Aca-py does the following pre-validation on received proofs:
- Checks for presence of and validity of revocation intervals
- In some cases the proof is rejected (for example if a non-revoked credential is requested and no revocation interval is provided then the proof is rejected)
- In some cases the proof is "fixed" (for example if a revocation interval is provided on a non-revocable credential the non-revocation timestamp is dropped)
- checks overall validity of the proof (encoded values match raw, requested attributes are present, requested predicates are present)
The revocation interval validation has had quite a bit of testing, and a few bugs have been fixed. I'm reasonably confident that this logic is correct (and prevents potential malicious proofs). There are some scenarios that AnonCreds will accept that aca-py will reject (for example if a prover leaves out a non-revocation interval in a proof I believe AnonCreds will accept it - aca-py rejects this scenario as a proof of non-revocation was requested).
The "overall validity of the proof" hasn't had as many eyes on it. The "revealed" vs "unrevealed" attributes is an error in aca-py (I believe), the question of whether aca-py should hi-light this scenario is an outstanding question.
Ah -- I get it. Thanks for the clarification. I figured my previous thinking on that had to be flawed, since the "encoded" value was not always encoded in normal use -- e.g. in the case of integers.
OK -- that is even better for some scenarios, such as what we were trying to do for the case where we (the verifier) is willing to accept credentials from one of N credentials that the person might have. If the prover is able to reveal the attributes from the credential they have, and put into the "unrevealed_attrs" the attributes from credentials they don't have, then we have a working solution for that 1 of N problem.
My suggestion for the proper answer to this is:
-
verifiedresult is whatever AnonCreds says is the right answer - We add a new item to the result, something like
incompleteset to true/false if we want the controller to know the proof was not complete wrt to the proof request - As needed, we add an "incomplete_reason", perhaps as an array, with why the
incompleteflag is false.
I have a couple of questions/remarks here:
-
What about rfc 0347 proof negotiation? To quote it:
The goal of this is an approach to make proof presentation more flexible, allowing attributes to be required or optional as well as allowing a choose-from-a-list scenario.
-
I think the issue here is mainly with semantics as the protocol has no way of specifying if an attribute must be revealed or not. From the perspective of anoncreds it should not matter if an attribute is revealed or not as long as the cryptography checks out. From the verifiers/controllers perspective it matters very much. Because if the controller needs the revealed values it can only set the exchange to failed in its local state after acapy is done, but now the verifier is stuck as there is no way to tell the holder to do something different in the next exchange. The only way I could imagine would be to switch the exchange to manual, and send a problem report before calling the /verify-presentation endpoint.
-
A workaround for 2 would be to always imply that any requested attribute must be revealed and if the holder does not want to do so it would send a counter proposal. But this raises two problems 1) it breaks other wallets and b) This would mean that there is a way to link a proposal to an existing (already ongoing) exchange. I think this is specified in rfc 0037, but not implemented.
-
Another workaround for 2 could be to always imply that all requested attributes need to be revealed and to use predicates for everything that is optional.
Generally I agree that the exchange should be valid if an attribute is not revealed, but I also think that the protocol needs to be extended so that the verifier has a way of communicating to the holder what attributes must be revealed.
I agree with your general opinion about extending "the protocol" to allow a verifier to be more clear about what it means. The question is what protocol should be extended? At minimum, it should be at the Aries RFC level, I think it would be best done at the AnonCreds level (however, that will take a while). I do not think it should be done unilaterally by ACA-Py,
Looking at RFC 0347, I think that if we want to go there, we should switch to the DIF Presentation Exchange (at a quick glance, 0347 look quite similar). However, that is a pretty heavy weight change for a relatively subtle change. There may be other reasons for going to DIF PE, but I don't think this is the driver.
Agree that the big question is 2. We've been marking these proofs as "not verified", and we are now proposing a change to that. Further, the AnonCreds behaviour is not well known. It also means that verifiers should begin the practice of checking what they are getting from a proof. Any verifier that does not know what Issuer to might get a proof from has to do that, to decide if they trust the issuer (e.g. an education credential).
I don't agree that 3 is the right answer for a couple of reasons. I think that not revealing something (perhaps because you don't have a credential to populate the attributes) is a viable response, and doing so saves having to extend the in-flight credential and lots of tricky logic in negotiating. I think verifiers would be easier to implement if they send a problem report to holders if the don't accept the data was provided (for whatever reason...), and then allowing for retrying. I can't imagine what a verifier would do on receipt of a message from the holder saying "Hey, how about this instead?". I'm sure most would throw up their hands and send back a problem report.
I don't agree that 4 is the right way as in many cases predicates can't be used. They only work with sortable integers so that the expressions are meaningful with the encoded values.
My guess is that this has not been happening much because (a) most wallets don't support letting the user not reveal attributes (I recall one does), and (b) no wallets support responding to a proof for which you don't have all the attributes (which could/should be possible, I think). I also don't think really bad things would happen if we removed this restriction as existing controllers that need required data would error off when the attribute does not have an associated value. Not ideal, but we could publish the change as breaking.
Discussed on the Aries WG call 2022.08.17. @ianco -- please go ahead with the change to NOT alter the AnonCreds verification when unrevealed attributes are included, and please add a new boolean and string (reason code) to indicate to the controller that the proof was cryptographically correct, but not complete.
Thanks!
I tested my previous use case and now it works correctly thanks a lot for this.