Skip to content

Commit

Permalink
fix: ensure request matches offer, if sent
Browse files Browse the repository at this point in the history 
This change implements checking that ld proof credential requests match
their corresponding offer, if an offer was sent.

If using the `--auto-*` flags for issuance, it was possible for the
receiver of the credential offer to change values in the request and
ACA-Py would accept this and issue based off of the request values. The
`--auto-*` flags are debug flags and should not be used in production
which would mean that a controller should have been able to catch this
discrepancy. However, it is still expedient for ACA-Py to check that the
offer and request match to avoid this slipping past the controller as
well.

There is a side affect of this check. We were permitting late binding of
the credential subject ID to the holder in the request. Meaning, on
request, the holder will automatically (when using auto flags) insert a
DID key as the credential subject ID to ensure the holder can actually
present proof of possession later. These changes modify this behavior
such that it only applies iff the credential subject id is not set
already (e.g. in the credential offer). This enables the issuer to bind
the credential to a DID other than the holder's pairwise DID if an
alternate is known to the issuer. If the issuer wants to permit late
binding by the holder still, the credential subject ID should be omitted
in the offer.

So, to summarize, the two modifications implemented here:
- Ensure the request doesn't change the credential unless the offer
  explicitly omits a credential subject ID
- Only override with holder did if the credential subject ID is omitted

Signed-off-by: Daniel Bluhm <[email protected]>
  • Loading branch information
@dbluhm
dbluhm committed Aug 14, 2023
1 parent cdfa488 commit 214a89e
Show file tree
Hide file tree
Showing 2 changed files with 187 additions and 5 deletions.
49 changes: 46 additions & 3 deletions aries_cloudagent/protocols/issue_credential/v2_0/formats/ld_proof/handler.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
from ...models.cred_ex_record import V20CredExRecord
from ...models.detail.ld_proof import V20CredExRecordLDProof
from ..handler import CredFormatAttachment, V20CredFormatError, V20CredFormatHandler
from .models.cred_detail_options import LDProofVCDetailOptions
from .models.cred_detail import LDProofVCDetail, LDProofVCDetailSchema

LOGGER = logging.getLogger(__name__)
Expand Down Expand Up @@ -361,6 +362,8 @@ async def _prepare_detail(
self, detail: LDProofVCDetail, holder_did: str = None
) -> LDProofVCDetail:
# Add BBS context if not present yet
assert detail.options and isinstance(detail.options, LDProofVCDetailOptions)
assert detail.credential and isinstance(detail.credential, VerifiableCredential)
if (
detail.options.proof_type == BbsBlsSignature2020.signature_type
and SECURITY_CONTEXT_BBS_URL not in detail.credential.context_urls
Expand All @@ -373,9 +376,21 @@ async def _prepare_detail(
):
detail.credential.add_context(SECURITY_CONTEXT_ED25519_2020_URL)

# add holder_did as credentialSubject.id (if provided)
if holder_did and holder_did.startswith("did:key"):
detail.credential.credential_subject["id"] = holder_did
# Permit late binding of credential subject:
# IFF credential subject doesn't already have an id, add holder_did as
# credentialSubject.id (if provided)
subject = detail.credential.credential_subject

# TODO if credential subject is a list, we're only binding the first...
# How should this be handled?
if isinstance(subject, list):
subject = subject[0]

if not subject:
raise V20CredFormatError("Credential subject is required")

if holder_did and holder_did.startswith("did:key") and "id" not in subject:
subject["id"] = holder_did

return detail

Expand Down Expand Up @@ -462,6 +477,34 @@ async def receive_request(
self, cred_ex_record: V20CredExRecord, cred_request_message: V20CredRequest
) -> None:
"""Receive linked data proof request."""
# Check that request hasn't substantially changed from offer, if offer sent
if cred_ex_record.cred_offer:
offer_detail_dict = cred_ex_record.cred_offer.attachment(
LDProofCredFormatHandler.format
)
req_detail_dict = cred_request_message.attachment(
LDProofCredFormatHandler.format
)

# If credentialSubject.id in offer, it should be the same in request
offer_id = (
offer_detail_dict["credential"].get("credentialSubject", {}).get("id")
)
request_id = (
req_detail_dict["credential"].get("credentialSubject", {}).get("id")
)
if offer_id and offer_id != request_id:
raise V20CredFormatError(
"Request credentialSubject.id must match offer credentialSubject.id"
)

# Nothing else should be different about the request
if request_id:
offer_detail_dict["credential"].setdefault("credentialSubject", {})[
"id"
] = request_id
if offer_detail_dict != req_detail_dict:
raise V20CredFormatError("Request must match offer if offer is sent")

async def issue_credential(
self,
Expand Down
143 changes: 141 additions & 2 deletions aries_cloudagent/protocols/issue_credential/v2_0/formats/ld_proof/tests/test_handler.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -623,13 +623,152 @@ async def test_create_request_x_no_data(self):
in str(context.exception)
)

async def test_receive_request(self):
async def test_receive_request_no_offer(self):
cred_ex_record = async_mock.MagicMock()
cred_ex_record.cred_offer = None
cred_request_message = async_mock.MagicMock()

# Not much to assert. Receive request doesn't do anything
# Not much to assert. Receive request doesn't do anything if no prior offer
await self.handler.receive_request(cred_ex_record, cred_request_message)

async def test_receive_request_with_offer_no_id(self):
cred_offer = V20CredOffer(
formats=[
V20CredFormat(
attach_id="0",
format_=ATTACHMENT_FORMAT[CRED_20_OFFER][
V20CredFormat.Format.LD_PROOF.api
],
)
],
offers_attach=[AttachDecorator.data_base64(LD_PROOF_VC_DETAIL, ident="0")],
)
cred_ex_record = V20CredExRecord(
cred_ex_id="dummy-id",
state=V20CredExRecord.STATE_OFFER_RECEIVED,
cred_offer=cred_offer,
)
cred_request = V20CredRequest(
formats=[
V20CredFormat(
attach_id="0",
format_=ATTACHMENT_FORMAT[CRED_20_REQUEST][
V20CredFormat.Format.LD_PROOF.api
],
)
],
requests_attach=[
AttachDecorator.data_base64(LD_PROOF_VC_DETAIL, ident="0")
],
)

await self.handler.receive_request(cred_ex_record, cred_request)

async def test_receive_request_with_offer_with_id(self):
detail = deepcopy(LD_PROOF_VC_DETAIL)
detail["credential"]["credentialSubject"]["id"] = "some id"
cred_offer = V20CredOffer(
formats=[
V20CredFormat(
attach_id="0",
format_=ATTACHMENT_FORMAT[CRED_20_OFFER][
V20CredFormat.Format.LD_PROOF.api
],
)
],
offers_attach=[AttachDecorator.data_base64(detail, ident="0")],
)
cred_ex_record = V20CredExRecord(
cred_ex_id="dummy-id",
state=V20CredExRecord.STATE_OFFER_RECEIVED,
cred_offer=cred_offer,
)
cred_request = V20CredRequest(
formats=[
V20CredFormat(
attach_id="0",
format_=ATTACHMENT_FORMAT[CRED_20_REQUEST][
V20CredFormat.Format.LD_PROOF.api
],
)
],
requests_attach=[AttachDecorator.data_base64(detail, ident="0")],
)

await self.handler.receive_request(cred_ex_record, cred_request)

async def test_receive_request_with_offer_with_id_x_mismatch_id(self):
detail = deepcopy(LD_PROOF_VC_DETAIL)
detail["credential"]["credentialSubject"]["id"] = "some id"
cred_offer = V20CredOffer(
formats=[
V20CredFormat(
attach_id="0",
format_=ATTACHMENT_FORMAT[CRED_20_OFFER][
V20CredFormat.Format.LD_PROOF.api
],
)
],
offers_attach=[AttachDecorator.data_base64(detail, ident="0")],
)
cred_ex_record = V20CredExRecord(
cred_ex_id="dummy-id",
state=V20CredExRecord.STATE_OFFER_RECEIVED,
cred_offer=cred_offer,
)
req_detail = deepcopy(detail)
req_detail["credential"]["credentialSubject"]["id"] = "other id"
cred_request = V20CredRequest(
formats=[
V20CredFormat(
attach_id="0",
format_=ATTACHMENT_FORMAT[CRED_20_REQUEST][
V20CredFormat.Format.LD_PROOF.api
],
)
],
requests_attach=[AttachDecorator.data_base64(req_detail, ident="0")],
)

with self.assertRaises(V20CredFormatError) as context:
await self.handler.receive_request(cred_ex_record, cred_request)
assert "must match offer" in str(context.exception)

async def test_receive_request_with_offer_with_id_x_changed_cred(self):
detail = deepcopy(LD_PROOF_VC_DETAIL)
cred_offer = V20CredOffer(
formats=[
V20CredFormat(
attach_id="0",
format_=ATTACHMENT_FORMAT[CRED_20_OFFER][
V20CredFormat.Format.LD_PROOF.api
],
)
],
offers_attach=[AttachDecorator.data_base64(detail, ident="0")],
)
cred_ex_record = V20CredExRecord(
cred_ex_id="dummy-id",
state=V20CredExRecord.STATE_OFFER_RECEIVED,
cred_offer=cred_offer,
)
req_detail = deepcopy(LD_PROOF_VC_DETAIL_ED25519_2020)
cred_request = V20CredRequest(
formats=[
V20CredFormat(
attach_id="0",
format_=ATTACHMENT_FORMAT[CRED_20_REQUEST][
V20CredFormat.Format.LD_PROOF.api
],
)
],
requests_attach=[AttachDecorator.data_base64(req_detail, ident="0")],
)

with self.assertRaises(V20CredFormatError) as context:
await self.handler.receive_request(cred_ex_record, cred_request)
assert "Request must match offer if offer is sent" in str(context.exception)

async def test_issue_credential(self):
cred_request = V20CredRequest(
formats=[
Expand Down

0 comments on commit 214a89e

Please sign in to comment.