From fbbef3a4d72240710459691de512eb43e66c2e5e Mon Sep 17 00:00:00 2001 From: jamshale Date: Thu, 20 Jun 2024 09:53:47 -0700 Subject: [PATCH] Fix anoncreds legacy indy revocation rotate without tails server / Refactor Signed-off-by: jamshale --- aries_cloudagent/anoncreds/issuer.py | 2 +- aries_cloudagent/anoncreds/revocation.py | 246 ++++++++++-------- .../anoncreds/revocation_setup.py | 13 +- .../anoncreds/tests/test_issuer.py | 2 +- .../anoncreds/tests/test_revocation.py | 38 +-- aries_cloudagent/ledger/indy_vdr.py | 19 +- .../endorse_transaction/v1_0/manager.py | 6 +- demo/runners/support/agent.py | 4 +- 8 files changed, 192 insertions(+), 138 deletions(-) diff --git a/aries_cloudagent/anoncreds/issuer.py b/aries_cloudagent/anoncreds/issuer.py index 45c018de2b..78bc65f0d2 100644 --- a/aries_cloudagent/anoncreds/issuer.py +++ b/aries_cloudagent/anoncreds/issuer.py @@ -310,7 +310,7 @@ async def create_and_register_credential_definition( if not isinstance(support_revocation, bool): raise ValueError("support_revocation must be a boolean") - max_cred_num = options.get("max_cred_num", DEFAULT_MAX_CRED_NUM) + max_cred_num = options.get("revocation_registry_size", DEFAULT_MAX_CRED_NUM) if not isinstance(max_cred_num, int): raise ValueError("max_cred_num must be an integer") diff --git a/aries_cloudagent/anoncreds/revocation.py b/aries_cloudagent/anoncreds/revocation.py index a5d49e0dbb..3a65a5f773 100644 --- a/aries_cloudagent/anoncreds/revocation.py +++ b/aries_cloudagent/anoncreds/revocation.py @@ -8,7 +8,7 @@ import os import time from pathlib import Path -from typing import List, NamedTuple, Optional, Sequence, Tuple, Union +from typing import List, Mapping, NamedTuple, Optional, Sequence, Tuple, Union from urllib.parse import urlparse import base58 @@ -21,6 +21,7 @@ RevocationStatusList, W3cCredential, ) +from aries_askar import Entry from aries_askar.error import AskarError from requests import RequestException, Session from uuid_utils import uuid4 @@ -43,6 +44,7 @@ from .models.anoncreds_revocation import ( RevList, RevListResult, + RevListState, RevRegDef, RevRegDefResult, RevRegDefState, @@ -459,7 +461,9 @@ async def create_and_register_revocation_list( self.profile, rev_reg_def, RevList.from_native(rev_list), options ) - # TODO Handle `failed` state + if options.get("failed_to_upload", False): + result.revocation_list_state.state = RevListState.STATE_FAILED + await self.store_revocation_registry_list(result) return result @@ -481,9 +485,10 @@ async def store_revocation_registry_list(self, result: RevListResult): identifier, value_json={ "rev_list": rev_list.serialize(), - "pending": None, - # TODO THIS IS A HACK; this fixes ACA-Py expecting 1-based indexes # noqa: E501 + # Anoncreds uses the 0 index internally + # and can't be used for a credential "next_index": 1, + "pending": None, }, tags={ "state": result.revocation_list_state.state, @@ -800,9 +805,9 @@ async def handle_full_registry(self, rev_reg_def_id: str): tag=str(uuid4()), max_cred_num=active_rev_reg_def.value_json["value"]["maxCredNum"], ) - LOGGER.info(f"previous rev_reg_def_id = {rev_reg_def_id}") - LOGGER.info(f"current rev_reg_def_id = {backup_rev_reg_def_id}") - LOGGER.info(f"backup reg = {backup_reg}") + LOGGER.info(f"Previous rev_reg_def_id = {rev_reg_def_id}") + LOGGER.info(f"Current rev_reg_def_id = {backup_rev_reg_def_id}") + LOGGER.info(f"Backup reg = {backup_reg.rev_reg_def_id}") async def decommission_registry(self, cred_def_id: str): """Decommission post-init registries and start the next registry generation.""" @@ -856,9 +861,9 @@ async def decommission_registry(self, cred_def_id: str): max_cred_num=active_reg.rev_reg_def.value.max_cred_num, ) - LOGGER.info(f"new reg = {new_reg}") - LOGGER.info(f"backup reg = {backup_reg}") - LOGGER.info(f"decommissioned regs = {recs}") + LOGGER.info(f"New registry = {new_reg}") + LOGGER.info(f"Backup registry = {backup_reg}") + LOGGER.debug(f"Decommissioned registries = {recs}") return recs async def get_or_create_active_registry(self, cred_def_id: str) -> RevRegDefResult: @@ -874,7 +879,6 @@ async def get_or_create_active_registry(self, cred_def_id: str) -> RevRegDefResu ) if not rev_reg_defs: - # TODO Create a registry if none available raise AnonCredsRevocationError("No active registry") entry = rev_reg_defs[0] @@ -922,6 +926,45 @@ async def create_credential_w3c( retries=retries, ) + async def _get_cred_def_objects( + self, credential_definition_id: str + ) -> tuple[Entry, Entry]: + try: + async with self.profile.session() as session: + cred_def = await session.handle.fetch( + CATEGORY_CRED_DEF, credential_definition_id + ) + cred_def_private = await session.handle.fetch( + CATEGORY_CRED_DEF_PRIVATE, credential_definition_id + ) + except AskarError as err: + raise AnonCredsRevocationError( + "Error retrieving credential definition" + ) from err + if not cred_def or not cred_def_private: + raise AnonCredsRevocationError( + "Credential definition not found for credential issuance" + ) + return cred_def, cred_def_private + + def _check_and_get_attribute_raw_values( + self, schema_attributes: List[str], credential_values: dict + ) -> Mapping[str, str]: + raw_values = {} + for attribute in schema_attributes: + # Ensure every attribute present in schema to be set. + # Extraneous attribute names are ignored. + try: + credential_value = credential_values[attribute] + except KeyError: + raise AnonCredsRevocationError( + "Provided credential values are missing a value " + f"for the schema attribute '{attribute}'" + ) + + raw_values[attribute] = str(credential_value) + return raw_values + async def _create_credential( self, credential_definition_id: str, @@ -949,93 +992,79 @@ async def _create_credential( A tuple of created credential and revocation ID """ - try: - async with self.profile.session() as session: - cred_def = await session.handle.fetch( - CATEGORY_CRED_DEF, credential_definition_id + + def _handle_missing_entries( + rev_list: Entry, rev_reg_def: Entry, rev_key: Entry + ): + if not rev_list: + raise AnonCredsRevocationError("Revocation registry list not found") + if not rev_reg_def: + raise AnonCredsRevocationError( + "Revocation registry definition not found" ) - cred_def_private = await session.handle.fetch( - CATEGORY_CRED_DEF_PRIVATE, credential_definition_id + if not rev_key: + raise AnonCredsRevocationError( + "Revocation registry definition private data not found" ) - except AskarError as err: - raise AnonCredsRevocationError( - "Error retrieving credential definition" - ) from err - if not cred_def or not cred_def_private: - raise AnonCredsRevocationError( - "Credential definition not found for credential issuance" - ) - raw_values = {} - for attribute in schema_attributes: - # Ensure every attribute present in schema to be set. - # Extraneous attribute names are ignored. - try: - credential_value = credential_values[attribute] - except KeyError: - raise AnonCredsRevocationError( - "Provided credential values are missing a value " - f"for the schema attribute '{attribute}'" + def _has_required_id_and_tails_path(): + return rev_reg_def_id and tails_file_path + + revoc = None + credential_revocation_id = None + rev_list = None + + if _has_required_id_and_tails_path(): + async with self.profile.session() as session: + rev_reg_def = await session.handle.fetch( + CATEGORY_REV_REG_DEF, rev_reg_def_id + ) + rev_list = await session.handle.fetch(CATEGORY_REV_LIST, rev_reg_def_id) + rev_key = await session.handle.fetch( + CATEGORY_REV_REG_DEF_PRIVATE, rev_reg_def_id ) - raw_values[attribute] = str(credential_value) + _handle_missing_entries(rev_list, rev_reg_def, rev_key) + + rev_info = rev_list.value_json + rev_info_tags = rev_list.tags - if rev_reg_def_id and tails_file_path: + # If the state is failed then the tails file was never uploaded + # try to upload it now and finish the revocation list + if rev_info_tags.get("state") == RevListState.STATE_FAILED: + await self.upload_tails_file( + RevRegDef.deserialize(rev_reg_def.value_json) + ) + rev_info_tags["state"] = RevListState.STATE_FINISHED + + rev_reg_index = rev_info["next_index"] try: - async with self.profile.transaction() as txn: - rev_list = await txn.handle.fetch(CATEGORY_REV_LIST, rev_reg_def_id) - rev_reg_def = await txn.handle.fetch( - CATEGORY_REV_REG_DEF, rev_reg_def_id - ) - rev_key = await txn.handle.fetch( - CATEGORY_REV_REG_DEF_PRIVATE, rev_reg_def_id - ) - if not rev_list: - raise AnonCredsRevocationError("Revocation registry not found") - if not rev_reg_def: - raise AnonCredsRevocationError( - "Revocation registry definition not found" - ) - if not rev_key: - raise AnonCredsRevocationError( - "Revocation registry definition private data not found" - ) - # NOTE: we increment the index ahead of time to keep the - # transaction short. The revocation registry itself will NOT - # be updated because we always use ISSUANCE_BY_DEFAULT. - # If something goes wrong later, the index will be skipped. - # FIXME - double check issuance type in case of upgraded wallet? - rev_info = rev_list.value_json - rev_info_tags = rev_list.tags - rev_reg_index = rev_info["next_index"] - try: - rev_reg_def = RevocationRegistryDefinition.load( - rev_reg_def.raw_value - ) - rev_list = RevocationStatusList.load(rev_info["rev_list"]) - except AnoncredsError as err: - raise AnonCredsRevocationError( - "Error loading revocation registry definition" - ) from err - if rev_reg_index > rev_reg_def.max_cred_num: - raise AnonCredsRevocationRegistryFullError( - "Revocation registry is full" - ) - rev_info["next_index"] = rev_reg_index + 1 - await txn.handle.replace( - CATEGORY_REV_LIST, - rev_reg_def_id, - value_json=rev_info, - tags=rev_info_tags, - ) - await txn.commit() - except AskarError as err: + rev_reg_def = RevocationRegistryDefinition.load(rev_reg_def.raw_value) + rev_list = RevocationStatusList.load(rev_info["rev_list"]) + except AnoncredsError as err: raise AnonCredsRevocationError( - "Error updating revocation registry index" + "Error loading revocation registry" ) from err - # rev_info["next_index"] is 1 based but getting from - # rev_list is zero based... + # NOTE: we increment the index ahead of time to keep the + # transaction short. The revocation registry itself will NOT + # be updated because we always use ISSUANCE_BY_DEFAULT. + # If something goes wrong later, the index will be skipped. + # FIXME - double check issuance type in case of upgraded wallet? + if rev_reg_index > rev_reg_def.max_cred_num: + raise AnonCredsRevocationRegistryFullError( + "Revocation registry is full" + ) + rev_info["next_index"] = rev_reg_index + 1 + async with self.profile.transaction() as txn: + await txn.handle.replace( + CATEGORY_REV_LIST, + rev_reg_def_id, + value_json=rev_info, + tags=rev_info_tags, + ) + await txn.commit() + revoc = CredentialRevocationConfig( rev_reg_def, rev_key.raw_value, @@ -1043,10 +1072,10 @@ async def _create_credential( rev_reg_index, ) credential_revocation_id = str(rev_reg_index) - else: - revoc = None - credential_revocation_id = None - rev_list = None + + cred_def, cred_def_private = await self._get_cred_def_objects( + credential_definition_id + ) try: credential = await asyncio.get_event_loop().run_in_executor( @@ -1056,7 +1085,9 @@ async def _create_credential( cred_def_private=cred_def_private.raw_value, cred_offer=credential_offer, cred_request=credential_request, - attr_raw_values=raw_values, + attr_raw_values=self._check_and_get_attribute_raw_values( + schema_attributes, credential_values + ), revocation_config=revoc, ), ) @@ -1161,24 +1192,30 @@ async def _create_credential_helper( rev_reg_def_id, tails_file_path, ) - except AnonCredsRevocationRegistryFullError: - # unlucky, another instance filled the registry first + except AnonCredsRevocationError as err: + LOGGER.warning(f"Failed to create credential: {err.message}, retrying") continue - # cred rev id is zero based - # max cred num is one based - # however, if we wait until max cred num is reached, we are too late. - if rev_reg_def_result: - if ( + def _is_full_registry( + rev_reg_def_result: RevRegDefResult, cred_rev_id: str + ) -> bool: + # cred rev id is zero based + # max cred num is one based + # however, if we wait until max cred num is reached, we are too late. + return ( rev_reg_def_result.rev_reg_def.value.max_cred_num <= int(cred_rev_id) + 1 - ): - await self.handle_full_registry(rev_reg_def_id) + ) + + if rev_reg_def_result and _is_full_registry( + rev_reg_def_result, cred_rev_id + ): + await self.handle_full_registry(rev_reg_def_id) return cred_json, cred_rev_id, rev_reg_def_id raise AnonCredsRevocationError( - f"Cred def '{cred_def_id}' has no active revocation registry" + f"Cred def '{cred_def_id}' revocation registry or list is in a bad state" ) async def revoke_pending_credentials( @@ -1342,8 +1379,7 @@ async def revoke_pending_credentials( ) if not rev_info_upd: LOGGER.warning( - "Revocation registry missing, skipping update: {}", - revoc_reg_id, + f"Revocation registry missing, skipping update: {revoc_reg_id}" # noqa: E501 ) updated_list = None break diff --git a/aries_cloudagent/anoncreds/revocation_setup.py b/aries_cloudagent/anoncreds/revocation_setup.py index c12b9060a8..f24e59bad0 100644 --- a/aries_cloudagent/anoncreds/revocation_setup.py +++ b/aries_cloudagent/anoncreds/revocation_setup.py @@ -5,7 +5,7 @@ from aries_cloudagent.protocols.endorse_transaction.v1_0.util import is_author_role -from ..anoncreds.revocation import AnonCredsRevocation +from ..anoncreds.revocation import AnonCredsRevocation, AnonCredsRevocationError from ..core.event_bus import EventBus from ..core.profile import Profile from ..revocation.util import notify_revocation_published_event @@ -95,7 +95,16 @@ async def on_rev_reg_def(self, profile: Profile, event: RevRegDefFinishedEvent): if auto_create_revocation: revoc = AnonCredsRevocation(profile) - await revoc.upload_tails_file(payload.rev_reg_def) + failed_to_upload_tails = False + try: + await revoc.upload_tails_file(payload.rev_reg_def) + except AnonCredsRevocationError as err: + LOGGER.warning(f"Failed to upload tails file: {err}") + failed_to_upload_tails = True + + if failed_to_upload_tails: + payload.options["failed_to_upload"] = True + await revoc.create_and_register_revocation_list( payload.rev_reg_def_id, payload.options ) diff --git a/aries_cloudagent/anoncreds/tests/test_issuer.py b/aries_cloudagent/anoncreds/tests/test_issuer.py index b1ba1517a3..673dfe462e 100644 --- a/aries_cloudagent/anoncreds/tests/test_issuer.py +++ b/aries_cloudagent/anoncreds/tests/test_issuer.py @@ -471,7 +471,7 @@ async def test_create_and_register_credential_definition_invalid_options_raises_ issuer_id="issuer-id", schema_id="schema-id", signature_type="CL", - options={"max_cred_num": "100"}, # requires integer + options={"support_revocation": "100"}, # requires integer ) @mock.patch.object(test_module.AnonCredsIssuer, "notify") diff --git a/aries_cloudagent/anoncreds/tests/test_revocation.py b/aries_cloudagent/anoncreds/tests/test_revocation.py index f4a3a12ad6..3cedc1cc20 100644 --- a/aries_cloudagent/anoncreds/tests/test_revocation.py +++ b/aries_cloudagent/anoncreds/tests/test_revocation.py @@ -11,10 +11,11 @@ RevocationRegistryDefinitionPrivate, RevocationStatusList, Schema, - # AnoncredsError, - # W3cCredential, - # CredentialRevocationConfig, ) + +# AnoncredsError, +# W3cCredential, +# CredentialRevocationConfig, from aries_askar import AskarError, AskarErrorCode from requests import RequestException, Session @@ -78,6 +79,8 @@ class MockRevRegDefEntry: def __init__(self, name="name"): self.name = name + rev_reg_def_id = "test-rev-reg-def-id" + tags = { "state": RevRegDefState.STATE_ACTION, } @@ -890,7 +893,7 @@ async def test_upload_tails_file(self): @mock.patch.object( test_module.AnonCredsRevocation, "create_and_register_revocation_registry_definition", - return_value="backup", + return_value=MockRevRegDefEntry(), ) async def test_handle_full_registry( self, mock_create_and_register, mock_set_active_registry, mock_handle @@ -1067,14 +1070,14 @@ async def test_create_credential_private_no_rev_reg_or_tails( credential_type=Credential, ) - @mock.patch.object(InMemoryProfileSession, "handle") @mock.patch.object( RevocationRegistryDefinition, "load", return_value=rev_reg_def.value ) @mock.patch("aries_cloudagent.anoncreds.revocation.CredentialRevocationConfig") + @mock.patch.object(InMemoryProfileSession, "handle") @mock.patch.object(Credential, "create", return_value=mock.MagicMock()) async def test_create_credential_private_with_rev_reg_and_tails( - self, mock_create, mock_config, mock_load_rev_reg_def, mock_handle + self, mock_create, mock_handle, *_ ): async def call_test_func(): await self.revocation._create_credential( @@ -1096,21 +1099,21 @@ async def call_test_func(): credential_type=Credential, ) - # missing rev list + # missing rev def mock_handle.fetch = mock.CoroutineMock( - side_effect=[MockEntry(), MockEntry(), None, MockEntry(), MockEntry()] + side_effect=[None, MockEntry(), MockEntry()] ) with self.assertRaises(test_module.AnonCredsRevocationError): await call_test_func() - # missing rev def + # missing rev list mock_handle.fetch = mock.CoroutineMock( - side_effect=[MockEntry(), MockEntry(), MockEntry(), None, MockEntry()] + side_effect=[MockEntry(), None, MockEntry()] ) with self.assertRaises(test_module.AnonCredsRevocationError): await call_test_func() # missing rev key mock_handle.fetch = mock.CoroutineMock( - side_effect=[MockEntry(), MockEntry(), MockEntry(), MockEntry(), None] + side_effect=[MockEntry(), MockEntry(), None] ) with self.assertRaises(test_module.AnonCredsRevocationError): await call_test_func() @@ -1119,35 +1122,34 @@ async def call_test_func(): mock_handle.replace = mock.CoroutineMock(return_value=None) mock_handle.fetch = mock.CoroutineMock( side_effect=[ - MockEntry(), - MockEntry(), + MockEntry(raw_value=rev_reg_def.serialize()), MockEntry( value_json={ "rev_list": rev_list.serialize(), "next_index": 0, } ), - MockEntry(raw_value=rev_reg_def.serialize()), + MockEntry(), + MockEntry(), MockEntry(), ] ) await call_test_func() assert mock_create.called assert mock_handle.replace.called - assert mock_handle.fetch.call_count == 5 # revocation registry is full mock_handle.fetch = mock.CoroutineMock( side_effect=[ - MockEntry(), - MockEntry(), + MockEntry(raw_value=rev_reg_def.serialize()), MockEntry( value_json={ "rev_list": rev_list.serialize(), "next_index": 101, } ), - MockEntry(raw_value=rev_reg_def.serialize()), + MockEntry(), + MockEntry(), MockEntry(), ] ) diff --git a/aries_cloudagent/ledger/indy_vdr.py b/aries_cloudagent/ledger/indy_vdr.py index 3e8c2dbb2b..fb3aa537f8 100644 --- a/aries_cloudagent/ledger/indy_vdr.py +++ b/aries_cloudagent/ledger/indy_vdr.py @@ -1169,13 +1169,18 @@ async def send_revoc_reg_def( rev_reg_def_req.set_endorser(endorser_did) legacy_indy_registry = LegacyIndyRegistry() - resp = await legacy_indy_registry.txn_submit( - self, - rev_reg_def_req, - sign=True, - sign_did=did_info, - write_ledger=write_ledger, - ) + try: + resp = await legacy_indy_registry.txn_submit( + self, + rev_reg_def_req, + sign=True, + sign_did=did_info, + write_ledger=write_ledger, + ) + except LedgerError as err: + raise LedgerError( + "Exception when sending revocation registry definition" + ) from err if not write_ledger: return revoc_reg_def["id"], {"signed_txn": resp} diff --git a/aries_cloudagent/protocols/endorse_transaction/v1_0/manager.py b/aries_cloudagent/protocols/endorse_transaction/v1_0/manager.py index 9fca6aec4b..167fd842cc 100644 --- a/aries_cloudagent/protocols/endorse_transaction/v1_0/manager.py +++ b/aries_cloudagent/protocols/endorse_transaction/v1_0/manager.py @@ -425,8 +425,10 @@ async def complete_transaction( ) legacy_indy_registry = LegacyIndyRegistry() - ledger_response_json = await legacy_indy_registry.txn_submit( - ledger, ledger_transaction, sign=False, taa_accept=False + ledger_response_json = await shield( + legacy_indy_registry.txn_submit( + ledger, ledger_transaction, sign=False, taa_accept=False + ) ) else: async with ledger: diff --git a/demo/runners/support/agent.py b/demo/runners/support/agent.py index ae03cde81d..606d87d0f7 100644 --- a/demo/runners/support/agent.py +++ b/demo/runners/support/agent.py @@ -7,8 +7,8 @@ import subprocess import sys from concurrent.futures import ThreadPoolExecutor -from timeit import default_timer from secrets import token_hex +from timeit import default_timer import asyncpg import yaml @@ -464,7 +464,7 @@ async def register_schema_and_creddef_anoncreds( }, "options": { "support_revocation": support_revocation, - "max_cred_num": max_cred_num, + "revocation_registry_size": max_cred_num, }, } credential_definition_response = await self.admin_POST(