search

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
407 stars 510 forks source link

FIX: Remove Indy/unqualified DID verification on "holder_did" and "entropy" in issuer during credential exchange #2923

Closed swcurran closed 1 month ago

swcurran commented 5 months ago

In the original AnonCreds issue implementation, the holder provided the issuer with a "holder_did" value in the request message of Issue Credential. In working through that in creating the AnonCreds spec, it was realized that the holder_did did not have to be a DID, but rather, just a string, a thing to provide entropy to the issuance process. Further, there is no concept of a "holder_did" in the AnonCreds implementation. As such, the JSON attribute name was changed.

However, in the ACA-Py implementation, the issuer code is still assuming the value is a DID, and in fact requiring that it be an INDY_DID or unqualified DID. The "holder DID" being used is the holder's peer DID in their relationship with the issuer, and that DID is now a qualified DID -- often a did:peer:2/4. There is a verification check that errors out on this.

This is causing errors such as: https://github.com/hyperledger/aries-cloudagent-python/pull/2861#issuecomment-2083162080 where the holder is using a peer:did:2/4.

The minimum answer is that the issuer Issue Credential code receiving the request from the Holder MUST NOT check that the entropy (or holder_did) value is a did, and especially not an INDYDID. The validation should be limited to checking that the value is a string.

Other references and validations on holder_did (which is common in the code), should probably also reviewed to make sure that the INDYDID verification is not happening -- since that will break when using qualified Peer DIDs.

swcurran commented 5 months ago

@ianco @dbluhm -- fyi -- this is another (IMHO) urgent fix for 0.12.1 as it is another problem with unqualified DIDs.

We have covered this before, so @ianco -- I wonder if this is just an issue with the "AnonCreds in W3C VCDM Format" code -- perhaps something missed from another change? Or is it also in the main code. I would expect issuance with Alice using a peer:did:2/4 would uncover this issue.

ianco commented 5 months ago

The error is coming from the new VC_DI schema (credential request) so I don't think there is an impact on the 12.1 release.

swcurran commented 5 months ago

Cool — that’s good!