Merge pull request #1913 from ianco/fix/proof-unrevealed-attrs

Remove aca-py check for unrevealed revealed attrs on proof validation
openwallet-foundation · Aug 29, 2022 · 5a636d6 · 5a636d6
2 parents cbd5cee + 79692de
commit 5a636d6
Show file tree

Hide file tree

Showing 14 changed files with 263 additions and 53 deletions.
diff --git a/AnoncredsProofValidation.md b/AnoncredsProofValidation.md
@@ -0,0 +1,84 @@
+# Anoncreds Proof Validation in Aca-Py
+
+Aca-Py does some pre-validation when verifying Anoncreds presentations (proofs), some scenarios are rejected (things that are indicative of tampering, for example) and some attributes are removed before running the anoncreds validation (for example removing superfluous non-revocation timestamps).  Any Aca-Py validations or presentation modifications are indicated by the "verify_msgs" attribute in the final presentation exchange object
+
+The list of possible verification messages is [here](https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/indy/verifier.py#L24), and consists of:
+
+```
+class PresVerifyMsg(str, Enum):
+    """Credential verification codes."""
+
+    RMV_REFERENT_NON_REVOC_INTERVAL = "RMV_RFNT_NRI"
+    RMV_GLOBAL_NON_REVOC_INTERVAL = "RMV_GLB_NRI"
+    TSTMP_OUT_NON_REVOC_INTRVAL = "TS_OUT_NRI"
+    CT_UNREVEALED_ATTRIBUTES = "UNRVL_ATTR"
+    PRES_VALUE_ERROR = "VALUE_ERROR"
+    PRES_VERIFY_ERROR = "VERIFY_ERROR"
+```
+
+If there is additional information, it will be included like this:  `TS_OUT_NRI::19_uuid` (which means the attribute identified by `19_uuid` contained a timestamp outside of the non-revocation interval (which is just a warning)).
+
+A presentation verification may include multiple messages, for example:
+
+```
+    ...
+    "verified": "true",
+    "verified_msgs": [
+        "TS_OUT_NRI::18_uuid",
+        "TS_OUT_NRI::18_id_GE_uuid",
+        "TS_OUT_NRI::18_busid_GE_uuid"
+    ],
+    ...
+```
+
+... or it may include a single message, for example:
+
+```
+    ...
+    "verified": "false",
+    "verified_msgs": [
+        "VALUE_ERROR::Encoded representation mismatch for 'Preferred Name'"
+    ],
+    ...
+```
+
+... or the `verified_msgs` may be null or an empty array.
+
+## Presentation Modifications and Warnings
+
+The following modifications/warnings may be done by Aca-Py which shouldn't affect the verification of the received proof):
+
+- "RMV_RFNT_NRI":  Referent contains a non-revocation interval for a non-revocable credential (timestamp is removed)
+- "RMV_GLB_NRI":  Presentation contains a global interval for a non-revocable credential (timestamp is removed)
+- "TS_OUT_NRI":  Presentation contains a non-revocation timestamp outside of the requested non-revocation interval (warning)
+- "UNRVL_ATTR":  Presentation contains attributes with unrevealed values (warning)
+
+## Presentation Pre-validation Errors
+
+The following pre-verification checks are done, which will fail the proof (before calling anoncreds) and will result in the following message:
+
+```
+VALUE_ERROR::<description of the failed validation>
+```
+
+These validations are all done within the [Indy verifier class](https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/indy/verifier.py) - to see the detailed validation just look for anywhere a `raise ValueError(...)` appears in the code.
+
+A summary of the possible errors is:
+
+- information missing in presentation exchange record
+- timestamp provided for irrevocable credential
+- referenced revocation registry not found on ledger
+- timestamp outside of reasonable range (future date or pre-dates revocation registry)
+- mis-match between provided and requested timestamps for non-revocation
+- mis-match between requested and provided attributes or predicates
+- self-attested attribute is provided for a requested attribute with restrictions
+- encoded value doesn't match raw value
+
+## Anoncreds Verification Exceptions
+
+Typically when you call the anoncreds `verifier_verify_proof()` method, it will return a `True` or `False` based on whether the presentation cryptographically verifies.  However in the case where anoncreds throws an exception, the exception text will be included in a verification message as follows:
+
+```
+VERIFY_ERROR::<the exception text>
+```
+
diff --git a/aries_cloudagent/indy/credx/verifier.py b/aries_cloudagent/indy/credx/verifier.py
@@ -7,7 +7,7 @@
 
 from ...core.profile import Profile
 
-from ..verifier import IndyVerifier
+from ..verifier import IndyVerifier, PresVerifyMsg
 
 LOGGER = logging.getLogger(__name__)
 
@@ -33,7 +33,7 @@ async def verify_presentation(
         credential_definitions,
         rev_reg_defs,
         rev_reg_entries,
-    ) -> bool:
+    ) -> (bool, list):
         """
         Verify a presentation.
 
@@ -46,16 +46,21 @@ async def verify_presentation(
             rev_reg_entries: revocation registry entries
         """
 
+        msgs = []
         try:
-            self.non_revoc_intervals(pres_req, pres, credential_definitions)
-            await self.check_timestamps(self.profile, pres_req, pres, rev_reg_defs)
-            await self.pre_verify(pres_req, pres)
+            msgs += self.non_revoc_intervals(pres_req, pres, credential_definitions)
+            msgs += await self.check_timestamps(
+                self.profile, pres_req, pres, rev_reg_defs
+            )
+            msgs += await self.pre_verify(pres_req, pres)
         except ValueError as err:
+            s = str(err)
+            msgs.append(f"{PresVerifyMsg.PRES_VALUE_ERROR.value}::{s}")
             LOGGER.error(
                 f"Presentation on nonce={pres_req['nonce']} "
                 f"cannot be validated: {str(err)}"
             )
-            return False
+            return (False, msgs)
 
         try:
             presentation = Presentation.load(pres)
@@ -68,11 +73,13 @@ async def verify_presentation(
                 rev_reg_defs.values(),
                 rev_reg_entries,
             )
-        except CredxError:
+        except CredxError as err:
+            s = str(err)
+            msgs.append(f"{PresVerifyMsg.PRES_VERIFY_ERROR.value}::{s}")
             LOGGER.exception(
                 f"Validation of presentation on nonce={pres_req['nonce']} "
                 "failed with error"
             )
             verified = False
 
-        return verified
+        return (verified, msgs)
diff --git a/aries_cloudagent/indy/sdk/tests/test_verifier.py b/aries_cloudagent/indy/sdk/tests/test_verifier.py
@@ -336,7 +336,7 @@ async def test_verify_presentation(self, mock_verify):
         ) as mock_get_ledger:
             mock_get_ledger.return_value = (None, self.ledger)
             INDY_PROOF_REQ_X = deepcopy(INDY_PROOF_REQ_PRED_NAMES)
-            verified = await self.verifier.verify_presentation(
+            (verified, msgs) = await self.verifier.verify_presentation(
                 INDY_PROOF_REQ_X,
                 INDY_PROOF_PRED_NAMES,
                 "schemas",
@@ -370,7 +370,7 @@ async def test_verify_presentation_x_indy(self, mock_verify):
             IndyLedgerRequestsExecutor, "get_ledger_for_identifier"
         ) as mock_get_ledger:
             mock_get_ledger.return_value = ("test", self.ledger)
-            verified = await self.verifier.verify_presentation(
+            (verified, msgs) = await self.verifier.verify_presentation(
                 INDY_PROOF_REQ_NAME,
                 INDY_PROOF_NAME,
                 "schemas",
@@ -397,7 +397,7 @@ async def test_check_encoding_attr(self, mock_verify):
         ) as mock_get_ledger:
             mock_get_ledger.return_value = (None, self.ledger)
             mock_verify.return_value = True
-            verified = await self.verifier.verify_presentation(
+            (verified, msgs) = await self.verifier.verify_presentation(
                 INDY_PROOF_REQ_NAME,
                 INDY_PROOF_NAME,
                 "schemas",
@@ -415,6 +415,8 @@ async def test_check_encoding_attr(self, mock_verify):
             json.dumps("rev_reg_entries"),
         )
         assert verified is True
+        assert len(msgs) == 1
+        assert "TS_OUT_NRI::19_uuid" in msgs
 
     @async_mock.patch("indy.anoncreds.verifier_verify_proof")
     async def test_check_encoding_attr_tamper_raw(self, mock_verify):
@@ -426,7 +428,7 @@ async def test_check_encoding_attr_tamper_raw(self, mock_verify):
             IndyLedgerRequestsExecutor, "get_ledger_for_identifier"
         ) as mock_get_ledger:
             mock_get_ledger.return_value = ("test", self.ledger)
-            verified = await self.verifier.verify_presentation(
+            (verified, msgs) = await self.verifier.verify_presentation(
                 INDY_PROOF_REQ_NAME,
                 INDY_PROOF_X,
                 "schemas",
@@ -438,6 +440,11 @@ async def test_check_encoding_attr_tamper_raw(self, mock_verify):
         mock_verify.assert_not_called()
 
         assert verified is False
+        assert len(msgs) == 2
+        assert "TS_OUT_NRI::19_uuid" in msgs
+        assert (
+            "VALUE_ERROR::Encoded representation mismatch for 'Preferred Name'" in msgs
+        )
 
     @async_mock.patch("indy.anoncreds.verifier_verify_proof")
     async def test_check_encoding_attr_tamper_encoded(self, mock_verify):
@@ -449,7 +456,7 @@ async def test_check_encoding_attr_tamper_encoded(self, mock_verify):
             IndyLedgerRequestsExecutor, "get_ledger_for_identifier"
         ) as mock_get_ledger:
             mock_get_ledger.return_value = (None, self.ledger)
-            verified = await self.verifier.verify_presentation(
+            (verified, msgs) = await self.verifier.verify_presentation(
                 INDY_PROOF_REQ_NAME,
                 INDY_PROOF_X,
                 "schemas",
@@ -461,6 +468,11 @@ async def test_check_encoding_attr_tamper_encoded(self, mock_verify):
         mock_verify.assert_not_called()
 
         assert verified is False
+        assert len(msgs) == 2
+        assert "TS_OUT_NRI::19_uuid" in msgs
+        assert (
+            "VALUE_ERROR::Encoded representation mismatch for 'Preferred Name'" in msgs
+        )
 
     @async_mock.patch("indy.anoncreds.verifier_verify_proof")
     async def test_check_pred_names(self, mock_verify):
@@ -470,7 +482,7 @@ async def test_check_pred_names(self, mock_verify):
             mock_get_ledger.return_value = ("test", self.ledger)
             mock_verify.return_value = True
             INDY_PROOF_REQ_X = deepcopy(INDY_PROOF_REQ_PRED_NAMES)
-            verified = await self.verifier.verify_presentation(
+            (verified, msgs) = await self.verifier.verify_presentation(
                 INDY_PROOF_REQ_X,
                 INDY_PROOF_PRED_NAMES,
                 "schemas",
@@ -491,6 +503,10 @@ async def test_check_pred_names(self, mock_verify):
         )
 
         assert verified is True
+        assert len(msgs) == 3
+        assert "TS_OUT_NRI::18_uuid" in msgs
+        assert "TS_OUT_NRI::18_id_GE_uuid" in msgs
+        assert "TS_OUT_NRI::18_busid_GE_uuid" in msgs
 
     @async_mock.patch("indy.anoncreds.verifier_verify_proof")
     async def test_check_pred_names_tamper_pred_value(self, mock_verify):
@@ -502,7 +518,7 @@ async def test_check_pred_names_tamper_pred_value(self, mock_verify):
             IndyLedgerRequestsExecutor, "get_ledger_for_identifier"
         ) as mock_get_ledger:
             mock_get_ledger.return_value = (None, self.ledger)
-            verified = await self.verifier.verify_presentation(
+            (verified, msgs) = await self.verifier.verify_presentation(
                 deepcopy(INDY_PROOF_REQ_PRED_NAMES),
                 INDY_PROOF_X,
                 "schemas",
@@ -514,6 +530,14 @@ async def test_check_pred_names_tamper_pred_value(self, mock_verify):
         mock_verify.assert_not_called()
 
         assert verified is False
+        assert len(msgs) == 4
+        assert "RMV_RFNT_NRI::18_uuid" in msgs
+        assert "RMV_RFNT_NRI::18_busid_GE_uuid" in msgs
+        assert "RMV_RFNT_NRI::18_id_GE_uuid" in msgs
+        assert (
+            "VALUE_ERROR::Timestamp on sub-proof #0 is superfluous vs. requested attribute group 18_uuid"
+            in msgs
+        )
 
     @async_mock.patch("indy.anoncreds.verifier_verify_proof")
     async def test_check_pred_names_tamper_pred_req_attr(self, mock_verify):
@@ -523,7 +547,7 @@ async def test_check_pred_names_tamper_pred_req_attr(self, mock_verify):
             IndyLedgerRequestsExecutor, "get_ledger_for_identifier"
         ) as mock_get_ledger:
             mock_get_ledger.return_value = (None, self.ledger)
-            verified = await self.verifier.verify_presentation(
+            (verified, msgs) = await self.verifier.verify_presentation(
                 INDY_PROOF_REQ_X,
                 INDY_PROOF_PRED_NAMES,
                 "schemas",
@@ -535,6 +559,14 @@ async def test_check_pred_names_tamper_pred_req_attr(self, mock_verify):
         mock_verify.assert_not_called()
 
         assert verified is False
+        assert len(msgs) == 4
+        assert "RMV_RFNT_NRI::18_uuid" in msgs
+        assert "RMV_RFNT_NRI::18_busid_GE_uuid" in msgs
+        assert "RMV_RFNT_NRI::18_id_GE_uuid" in msgs
+        assert (
+            "VALUE_ERROR::Timestamp on sub-proof #0 is superfluous vs. requested attribute group 18_uuid"
+            in msgs
+        )
 
     @async_mock.patch("indy.anoncreds.verifier_verify_proof")
     async def test_check_pred_names_tamper_attr_groups(self, mock_verify):
@@ -546,7 +578,7 @@ async def test_check_pred_names_tamper_attr_groups(self, mock_verify):
             IndyLedgerRequestsExecutor, "get_ledger_for_identifier"
         ) as mock_get_ledger:
             mock_get_ledger.return_value = ("test", self.ledger)
-            verified = await self.verifier.verify_presentation(
+            (verified, msgs) = await self.verifier.verify_presentation(
                 deepcopy(INDY_PROOF_REQ_PRED_NAMES),
                 INDY_PROOF_X,
                 "schemas",
@@ -558,3 +590,7 @@ async def test_check_pred_names_tamper_attr_groups(self, mock_verify):
         mock_verify.assert_not_called()
 
         assert verified is False
+        assert len(msgs) == 3
+        assert "RMV_RFNT_NRI::18_busid_GE_uuid" in msgs
+        assert "RMV_RFNT_NRI::18_id_GE_uuid" in msgs
+        assert "VALUE_ERROR::Missing requested attribute group 18_uuid" in msgs
diff --git a/aries_cloudagent/indy/sdk/verifier.py b/aries_cloudagent/indy/sdk/verifier.py
@@ -8,7 +8,7 @@
 
 from ...core.profile import Profile
 
-from ..verifier import IndyVerifier
+from ..verifier import IndyVerifier, PresVerifyMsg
 
 LOGGER = logging.getLogger(__name__)
 
@@ -34,7 +34,7 @@ async def verify_presentation(
         credential_definitions,
         rev_reg_defs,
         rev_reg_entries,
-    ) -> bool:
+    ) -> (bool, list):
         """
         Verify a presentation.
 
@@ -49,16 +49,21 @@ async def verify_presentation(
 
         LOGGER.debug(f">>> received presentation: {pres}")
         LOGGER.debug(f">>> for pres_req: {pres_req}")
+        msgs = []
         try:
-            self.non_revoc_intervals(pres_req, pres, credential_definitions)
-            await self.check_timestamps(self.profile, pres_req, pres, rev_reg_defs)
-            await self.pre_verify(pres_req, pres)
+            msgs += self.non_revoc_intervals(pres_req, pres, credential_definitions)
+            msgs += await self.check_timestamps(
+                self.profile, pres_req, pres, rev_reg_defs
+            )
+            msgs += await self.pre_verify(pres_req, pres)
         except ValueError as err:
+            s = str(err)
+            msgs.append(f"{PresVerifyMsg.PRES_VALUE_ERROR.value}::{s}")
             LOGGER.error(
                 f"Presentation on nonce={pres_req['nonce']} "
                 f"cannot be validated: {str(err)}"
             )
-            return False
+            return (False, msgs)
 
         LOGGER.debug(f">>> verifying presentation: {pres}")
         LOGGER.debug(f">>> for pres_req: {pres_req}")
@@ -71,11 +76,13 @@ async def verify_presentation(
                 json.dumps(rev_reg_defs),
                 json.dumps(rev_reg_entries),
             )
-        except IndyError:
+        except IndyError as err:
+            s = str(err)
+            msgs.append(f"{PresVerifyMsg.PRES_VERIFY_ERROR.value}::{s}")
             LOGGER.exception(
                 f"Validation of presentation on nonce={pres_req['nonce']} "
                 "failed with error"
             )
             verified = False
 
-        return verified
+        return (verified, msgs)