Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
swcurran opened this issue Nov 9, 2022 · 9 comments
Assignees

Comments

@swcurran
Copy link
Contributor

swcurran commented Nov 9, 2022

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
Copy link
Contributor Author

swcurran commented Nov 9, 2022

@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
Copy link
Contributor Author

swcurran commented Nov 9, 2022

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.

@ianco
Copy link
Contributor

ianco commented Nov 16, 2022

@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 ianco self-assigned this Nov 24, 2022
@dbluhm
Copy link
Contributor

dbluhm commented Nov 29, 2022

@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.

@swcurran
Copy link
Contributor Author

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.

@TimoGlastra
Copy link
Contributor

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
                )

@swcurran
Copy link
Contributor Author

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 swcurran changed the title In /issue-credential-v2/send for a W3C format VC, does the issuer have to know the subject id? Alice/Faber Demo oddity in JSON-LD VC issuance -- holder always overrides credentialSubject.id with BLS did:key Nov 30, 2022
@TimoGlastra
Copy link
Contributor

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?

@dbluhm
Copy link
Contributor

dbluhm commented Aug 22, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants