aries-cloudagent-python icon indicating copy to clipboard operation
aries-cloudagent-python copied to clipboard

Alice/Faber Demo oddity in JSON-LD VC issuance -- holder always overrides credentialSubject.id with BLS did:key

Open swcurran opened this issue 3 years ago • 9 comments

I was ran the demo AliceWantsAJsonCredential.md using an Ed25519Signature2081, I noticed two things that I wonder about.

First, when I put in Alice's did:key(in my case: did:key:z6MksTBMW4EeA9hQUYcXSDYmzaKV4fD972b9VL5pGBBPcf2G) in the id field of subjectCredential, in the issued credential (held by Alice) the id was changed to: did:key:zUC72B8bP2CGx8BqPer6zg5wA4bU6cUySp6Dc8aZFs62thfkfXW1q9c5oDXn6jn1Gcg97QwRmUfJ7j7Ye1hi7RGms7rQmQKnojGjCxqyWnVLfncXhiJabcs3BD7Ryns94ECeHUq.

Second, if I leave off the id field in subjectCredential, the id field was the same did:key:zUC72B8bP2CGx8BqPer6zg5wA4bU6cUySp6Dc8aZFs62thfkfXW1q9c5oDXn6jn1Gcg97QwRmUfJ7j7Ye1hi7RGms7rQmQKnojGjCxqyWnVLfncXhiJabcs3BD7Ryns94ECeHUq in the issued credential.

Questions:

  1. What transformation occurs such that the did:key value seems to change? Is that supposed to happen?
  2. If the id field is left out of the issuer's send payload, does the id get filled in by the Holder automagically (perhaps via a proposal message?)? If so, what did:key is used? In my case, there was just one.

swcurran avatar Nov 09 '22 22:11 swcurran

@TimoGlastra -- do you recall this? Any ideas on the intended behaviour? @ianco any ideas? I'm wondering if the seemingly duplicated did:key (the long one) is a default value in the controller?

swcurran avatar Nov 09 '22 22:11 swcurran

Also -- I notice that using the Universal Resolver, I can resolve Alice's did:key that was created in the wallet, but I can't resolve the long did:key that is in the stored credential (in the "subjects_id" array, and "id" in the credential itself). As such, I don't think it is a valid key.

swcurran avatar Nov 09 '22 22:11 swcurran

@ianco any ideas?

I'm not sure, I know there have been a bunch of (or some) did:key updates recently, not sure how they've affected the json credentials. (The aca-py version issue with the mediator is also did:key related I think)

ianco avatar Nov 16 '22 20:11 ianco

@swcurran that did:key looks like a BLS key. I found recently that in order to fulfill the is_holder constraint with BBS+ credentials, the subject ID must be a BLS key or the verification fails. It does seem like the controller might be overwriting values in order to do a BBS+ signature.

dbluhm avatar Nov 29 '22 18:11 dbluhm

That's weird -- there should be no BLS keys involved in what I did. I generated an ed25519 key for Alice (holder), and used it in one example and still got the BLS key in the VC. As well, I did a credential with no ID for also and still got that BLS key. Sounds like a bug to me. Perhaps the controller is overdoing things -- it would be very weird for that to happen in ACA-Py itself.

swcurran avatar Nov 29 '22 18:11 swcurran

Also -- I notice that using the Universal Resolver, I can resolve Alice's did:key that was created in the wallet, but I can't resolve the long did:key that is in the stored credential (in the "subjects_id" array, and "id" in the credential itself). As such, I don't think it is a valid key.

I think the universal resolver doesn't support bls did:keys. If I resolve it using the transmute did key resolver it works: https://did.key.transmute.industries/did:key:zUC72B8bP2CGx8BqPer6zg5wA4bU6cUySp6Dc8aZFs62thfkfXW1q9c5oDXn6jn1Gcg97QwRmUfJ7j7Ye1hi7RGms7rQmQKnojGjCxqyWnVLfncXhiJabcs3BD7Ryns94ECeHUq

Re the inclusion of the bls key, that is done by the demo. If an offer is received it will create a did on the holder side and put it in the request. The credentialSubject did can be any did and doesn't have to be related to the crypto suite of the credential. The demo should probably check whether a credentialSubject.id is already defined though, and not overwrite it in that case. When I present this credential I would need to short proof of control over that did. Normally the issuer would have to verify this control over the did before issuing the credential, I don't think that's done in the demo.

            elif message["by_format"]["cred_offer"].get("ld_proof"):
                holder_did = await self.admin_POST(
                    "/wallet/did/create",
                    {"method": "key", "options": {"key_type": "bls12381g2"}},
                )
                data = {"holder_did": holder_did["result"]["did"]}
                await self.admin_POST(
                    f"/issue-credential-2.0/records/{cred_ex_id}/send-request", data
                )

TimoGlastra avatar Nov 30 '22 09:11 TimoGlastra

Thanks @TimoGlastra -- that helps. with both parts of the issue.

I suppose the demo can do whatever it wants (it's the business logic) so good to know it is not a bug in ACA-Py, and good to know that the did:key type is BLS.

In the demo, it looks like the issuer puts the holder's DID into the credential in constructing the cred_offer (at /send time). That's a bit odd, as the issuer has to know the holder's DID at that time, which would better added by the holder (with a proof of control) at cred_request time. However, in looking at RFC 00593 JSON-LD Credential Attachment, I don't think there is a place for the proof in the attachment.

I think the following are the improvements to the demo -- does that make sense to you?

  1. In getting the cred_offer (the code identified by Timo above), Alice checks to see if there is a subject ID in the message and if so, to see if that DID exists in Alice's wallet (that she controls).
  2. If it does, don't change the DID in the credential.
  3. If it does not exist, see if there is a DID (any DID) in the wallet, and use that, and,
  4. if there are no DIDs, create one (as is done now) and replace the one in the credential with the new DID.
  5. In getting the cred_offer (same code), if there is no subject ID in the message, add one to the message from Alice's wallet, creating one if needed.

I think the above represents what a "generic" holder would do. Of course, any holder can do what they want.

Aside: This is where the opinionated element of AnonCreds has helped a lot. We've never had to deal with issues and options like this with AnonCreds, so the behaviour of the different parties is well defined (and the crypto requires it). Here, there are different options at the controller level and all possibilities must be handled.

If this works, I suggest that the demo be updated.

swcurran avatar Nov 30 '22 15:11 swcurran

However, in looking at RFC 00593 JSON-LD Credential Attachment, I don't think there is a place for the proof in the attachment.

No that's correct. We discussed this during the Aries WG call a while ago and landed on a generic did proof of control protocol. @TelegramSam made a draft of it, not sure where it's at right now?

TimoGlastra avatar Nov 30 '22 16:11 TimoGlastra

I recently adjusted the behavior of the credential subject ID override in #2341. If the issuer knows the appropriate ID to use for the holder, it can set the ID and this cannot be overridden by the holder. If the ID is omitted by the issuer, it can be overridden by the holder during the credential request as before.

dbluhm avatar Aug 22 '23 14:08 dbluhm