w3c / vc-data-model

W3C Verifiable Credentials v2.0 Specification
https://w3c.github.io/vc-data-model/
Other
293 stars 106 forks source link

Add `relatedResource` to `VerifiablePresentation` #1265

Closed OR13 closed 11 months ago

OR13 commented 1 year ago

Originally discussed in:

This would allow a VerifiablePresentation or VerifiableCredential to point to "other graph nodes by id", instead of pointing to "VerifiableCredentialGraph" containers.

See also the discussion on the different between the two here:

https://github.com/w3c/vc-data-model/issues/1240

msporny commented 1 year ago

+1, this feels like a useful thing to do.

I have since changed my mind based on the conversations at W3C TPAC; -1, new feature, not clear what the use case is.

dmitrizagidulin commented 1 year ago

+1 excellent idea.

iherman commented 1 year ago

I am of course fine with adding this property, which is indeed missing. But there is more, see below

Here is what I think must be done (see also https://github.com/w3c/vc-data-model/issues/1263#issuecomment-1704206033):

However, it turns out that the VCDM spec includes two more properties:

These two properties are not only missing from the vocabulary, but also missing from the context!

Note that, for the mediaType, an alternative would be to use the term encodingFormat instead, referring to http://schema.org/encodingFormat. Do not reinvent the wheel, and all that... That would mean just add an entry to the context file, and this property is oblivious to the vocabulary.

I am happy to create a PR with all these changes, if we have an agreement.

@dlongley @msporny

jandrieu commented 1 year ago

This feels like a new feature. Am I missing something?

Sakurann commented 1 year ago

I think one question raised during TPAC WG mtg was "what use-cases relatedResource in presentations are not addressed by relatedResouce in credentials"

To validate that a resource referenced by a verifiable credential is the same at verification time as it is at issuing time, an implementer MAY include a property named relatedResource that stores an array of objects that describe additional integrity metadata about each resource referenced by the verifiable credential. If relatedResource is present, there MUST be an object in the array for each remote resource for each context used in the verifiable credential.

iherman commented 1 year ago

The issue was discussed in a meeting on 2023-09-14

View the transcript #### 2.2. Add `relatedResource` to `VerifiablePresentation` (issue vc-data-model#1265) _See github issue [vc-data-model#1265](https://github.com/w3c/vc-data-model/issues/1265)._ **Brent Zundel:** Raised by Orie, who isn't able to attend today. � Is it a good idea? Why? **Manu Sporny:** we. have the concept of relatedResource. � It allows us to refer to files outside a VC using a cryptographic hash. � E.g. instead of embedding an image, you can refer to it with a cryptohash. � We have that capability in VC, we don't have it for VPs. � Unclear to me what the use case is for this. But sounds like a generally useful thing to have. � I don't think it causes any harm having that feature. I'd be interested in hearing from the group on whether it's useful to have. **Joe Andrieu:** Sounds like a new feature after the feature freeze. There are other ways of doing this by adding it to a presented VC. **Sebastian Crane:** I would like to raise a potential issue that this means that a VP cannot be understood fully offline. > *Joe Andrieu:* just make a VC that says here's a related resource. **Manu Sporny:** that's correct. This also happens if you haven't cached the context. > *Joe Andrieu:* holders can issue VCw. > *Joe Andrieu:* VCs. > *Brent Zundel:* +1 that handling this differently than the way additional data about the VP is handled would be inconsistent. **Manu Sporny:** realted to what JoeAndrieu said, we don't have a way for the holder to do that. **Brent Zundel:** we agreed on the mechanism in which the holder can make statements about the presentations. **Manu Sporny:** yes, that's a good point. Had forgotten about that. � that's a mark against this feature. **Ivan Herman:** We have an open issue to look at all the properties that we have in the VC, to see which ones can be applied to VPs as well. This is related. � I just translated what is in the spec to the vocab. **Brent Zundel:** We'll get to it as some point today. **Joe Andrieu:** If we had a good use case, it would be a different convo. Agree with seabass that we need a way to comm images. Will be tough with feature freeze. **Brent Zundel:** Wrap up, thanks all. Back in 30 min. > *Eric Siow:* Thanks. I was about to ask. **Kristina Yasuda:** I looked at the issue. Its a bit different than what we talked about before break. � there's a broader application of a common feature, then that's viable. **Manu Sporny:** looking at it again, I think Kristina is right. The intention is to do something different from what linkedResource does in DIDs. � this is badly titled. � This points to graphs by id. So that's problematic. � still struggling to understand the use case. > *Kristina Yasuda:* +1 to ask Orie for clarification. **Dmitri Zagidulin:** use case-wise I think I understand. � not sure about the graph node. its the ability to include non VC resources in a VP. � like here is PDF that is a part of something. � and bind that to the same authentication. � the same thing that linkedResources does. **Manu Sporny:** each time I re-read the issue, my understanding changes. � What you said Dimitri suggests it is related to relatedResources. > *Dmitri Zagidulin:* yes! attachments! **Brent Zundel:** another interpretation? Gen has been discussing how to add attachments to credentials and presentations. � the primary response is that we need more clarity from Orie. **Manu Sporny:** plus one for the attachment use case. � we can do that withthe VC. > *Dmitri Zagidulin:* the binding time is different, for a VC vs VP. **Joe Andrieu:** I think the attachment use case is more problematic w/ feature freeze, we should table this for the next version. **Brent Zundel:** any objection to adding this as future. > *jose-gataca:* jose-gataca has joined #vcwg. **Brent Zundel:** chairs will have a conversation about feature freeze.
jandrieu commented 1 year ago

I think it's likely a mistake to add arbitrary resources to a presentation. If the verifier didn't request it, it's going to be weird.

If the verifier did request it, I think the likely path is as a VC, e.g., requesting a VC with a photo of the user and getting that VC is much more appropriate than arbitrarily shoving an image in the presentation. And how do request protocols handle requests for arbitrary data?

Doing it outside the VC flow raises questions. How does the verifier ask for the right asset? What happens if you don't have it? These augmentations are potentially interesting, but right now, we haven't explored how we support it.

I think this is too much of a new feature that isn't just a replication of what happens with VCs, it creates new questions and problems that need additional analysis and consideration.

iherman commented 1 year ago

As also agreed at TPAC, I will submit soonish a PR taking care of https://github.com/w3c/vc-data-model/issues/1265#issuecomment-1705009825. Reading through the comments, I do not think there is a consensus on adding VP as a possible domain for related resource, so I propose to keep it for VC only for now. (It is easy to change that later if we decide to do so.)

dlongley commented 1 year ago

@jandrieu,

I think it's likely a mistake to add arbitrary resources to a presentation. If the verifier didn't request it, it's going to be weird.

If the verifier did request it, I think the likely path is as a VC, e.g., requesting a VC with a photo of the user and getting that VC is much more appropriate than arbitrarily shoving an image in the presentation. And how do request protocols handle requests for arbitrary data?

I think you may have missed some of the other discussions around this. The verifiableCredential property (of VPs) only supports values that are isolated graphs of information (to separate them cleanly from the VP) expressed using the VCDM as JSON-LD. VCs that are secured using VC-JOSE-COSE do not work -- as they add an envelope around the VCDM and change the data format from the VCDM / JSON-LD to instead be a base64-encoded-dot-delimited string. We need a place for people to express these different data structures in VPs and relatedResource was what we turned to.

So the expectation is that protocols such as OID4VP will be expected to request VCs that are secured using VC-JOSE-COSE to be presented as values in the relatedResource property. IOW, it should not be weird for those verifiers, but expected.

msporny commented 11 months ago

Removing ready for PR as it doesn't seem like there is consensus on adding the feature.

OR13 commented 11 months ago

suggest closing and tacking no action

iherman commented 11 months ago

The issue was discussed in a meeting on 2023-11-15

View the transcript #### 3.3. Add `relatedResource` to `VerifiablePresentation` (issue vc-data-model#1265) _See github issue [vc-data-model#1265](https://github.com/w3c/vc-data-model/issues/1265)._ **Brent Zundel:** I don't believe we have consensus on this, so let's discuss it now. **Ivan Herman:** I don't believe we'll get to agreement on this issue, so I would suggest marking it 'prepare to close'. **Dave Longley:** There is no way to put a VC that's secured with JOSE into a Verifiable Presentation. … I'm concerned that if we don't have a property for it, implementers will invent new ones. **Orie Steele:** When you see nested JSON structures it's easy to seem them as a whole, but the RDF graph is clear that they are separate entities. … If the N-quads don't reflect reality, the solution is to update the JSON-LD context. > *Dave Longley:* Orie: using a URL in `verifiableCredential` will just drop it. **Dmitri Zagidulin:** I think it's surprising to many that you can't include JOSE-secured credentials in a Verifiable Presentation. I agree with dlongley that if we don't include our own mechanism, either people will invent new methods or use JOSE version of VCs. We know that the ecosystem is divided between JSON-LD support and JOSE support. > *Orie Steele:* to be clear, you can.... go ahead and test it. > *Orie Steele:* whether the information is "correct" with respect to RDF data model is a separate question. > *Orie Steele:* dlongley: not in the security format we use. > *Manu Sporny:* +1 to "property that lets you do that", the more specific the better. **Joe Andrieu:** The fact that we can't include JOSE-secured credentials in a VP is an issue. I think we could create a new property for 'external proof' so that there's a semantic link. **Dave Longley:** I would prefer there to be a specific property for it too, but that didn't get consensus. Therefore 'related resource' was used as a fallback. > *Orie Steele:* You can use related resource to serve a context, that doesn't drop credentials by reference from the graph. **Dave Longley:** If we added a new property, we would need much more effort and more properties to support it. I think we need Related Resource anyway, and if we need a property for JOSE support add that too. > *Dmitri Zagidulin:* what are the objections to adding 'relatedResource' to a VP? > *Orie Steele:* +1 ivan. > *Dave Longley:* "relatedResource" was proposed by Mike Prorock / Orie to express context hashes in VCs. > *Dave Longley:* VP can also have custom contexts. **Ivan Herman:** The Related Resource is very loosely defined at the moment for external data with integrity, and I think it would be a mistake to use that for the specific purpose of supporting JOSE-secured VCs in VPs. > *Orie Steele:* its different thing to secure context, or include credentials by reference. > *Manu Sporny:* +1 to well-defined property for the use case we're trying to solve... relatedResource doesn't sound like it? > *Dmitri Zagidulin:* @manu - we still need relatedResources to hash-secure @contexts on the VP. > *Dave Longley:* "relatedResource" was good enough for context hash information in VCs, why not in VPs? ... if we want yet another property for JOSE-secured/other-envelope/external-secured VCs, that's good too. **Ivan Herman:** I would support a specific new structure for JOSE support. I recall that Joe had strong opposition to Related Resource. > *Manu Sporny:* dmitriz, who is doing that right now? What implementer is shipping that feature? > *Orie Steele:* including properties that are specific to a securing format, is a layer violation imo. > *Dave Longley:* VC-JOSE-COSE could add context hashes to its format. > *Dave Longley:* Orie^. **Dmitri Zagidulin:** First question we have is what to do with JOSE-secured credentials. The second question though is what to do with SRI digests. … What are the objections to adding the property to the VP. > *Dmitri Zagidulin:* I did specifically highlight that it IS two separate issues. > *Dave Longley:* +1 to dmitri. > *Orie Steele:* dlongley: security best practices seems to imply securing bytes, when they are JSON-LD bytes representing a conforming document, thats all that is needed, imo. > *Dave Longley:* Orie: non-three-party model best securing practices do that. **Manu Sporny:** I believe that the two use-cases can be described as 1: support for JOSE-secured VCs in VPs, and 2: to support secured contexts. > *Orie Steele:* +1 manu. > *Ivan Herman:* +1 manu. **Manu Sporny:** If we can use the same property for both, that would be great. **Joe Andrieu:** I agree with what manu said. I don't believe we need an arbitrary integrity facility, and that could open up vulnerabilities with unknown binary content. **Brent Zundel:** I'll close the Related Resource issue so two new issues can replace it. > *Dmitri Zagidulin:* @JoeAndrieu - what vulnerabilities are you referring to? digest hashing /anything/ is safe. > *Joe Andrieu:* @dmitriz yes, but you are trusting the untrustable. This isn't about trusting an issuer, it's trusting the holder. > *Manu Sporny:* +1 to what JoeAndrieu said above -- that's the problem w/ relatedResource in presentations. > *Manu Sporny:* (well, one of them) :).
msporny commented 11 months ago

I have since changed my mind based on the conversations at W3C TPAC; -1, new feature, not clear what the use case is.

After some debate with @dlongley, I've changed my mind (yet again) on this issue and propose a concrete use case for this feature:

That is, the argument for this feature is the same as for including it in verifiableCredential.

To be clear, I don't think that relatedResource should be used to "embed externally secured VCs", that should be done via the proposal in issue #1352.

I remain dubious to the use of relatedResource. The feature is so general that implementations are going to end up implementing custom logic that processes special things in the field, and developers are going to start using it as a dumping ground for cryptographic hashing of anything they deem important (and implementations are going to ignore throwing errors if hashes don't match). I remain a -1 on relatedResource in general, but will not formally object if it remains in the specification. The strongest reason I'm not suggesting a FO is because there seemed to be threats of formal objections if we didn't include the feature from the JOSE/COSE folks.

I expect that @jandrieu will object to this proposal because of what he said above, and will suggest we provide something like a securedContext or integrityProtectedContext property that looks exactly like relatedResource but is fit for purpose. digestMultibase or digestSRI (and mediatType) can be used for everything else, but I defer to him on the details (or, maybe my read on his statements during the last meeting were wrong). :)

To summarize:

Option A

Define relatedResource on VerifiablePresentation for the purposes of securing JSON-LD context files.

Option B

Define digestSRI/digestMultibase and define mediaType for general use in any object w/ an id property and rename relatedResource to contextSecurity (or some other bikesheded term). Allow contextSecurity on any object that can expresses a @context value, including VerifiableCredential and VerifiablePresentation. This approach still enables all of the use cases provided by the relatedResource property in the spec today, but is more targeted about context security.

iherman commented 11 months ago

@msporny, just a technical comment:

Define digestSRI/digestMultibase and define mediaType for general use in any object w/ an id property and rename

The term definition in the spec as well as the vocabulary item have been defined without any domain statement. I think that was intentional to make them usable on any object. I.e., this wish in option B has already been fulfilled 😀

msporny commented 11 months ago

@iherman wrote:

I think that was intentional to make them usable on any object. I.e., this wish in option B has already been fulfilled 😀

Not quite... :) more below.

Define digestSRI/digestMultibase and define mediaType for general use in any object w/ an id property and rename

We don't have mediaType defined generally, which can only be used on relatedResource, and would need to be usable on any resource (and the definition in the spec is a bit dodgy as well).

Hrm, looks like we also don't have digestMultibase defined at all in the v2 context; we need define this in order to see which form implementers implement, to match the statements in the spec, (DB plans to implement digestMultibase, btw), otherwise we bias heavily towards digestSRI.

iherman commented 11 months ago

Yeah, I am very narrow-minded, and was looking at the vocabulary only. The JSON context is your baby :-)

dmitrizagidulin commented 11 months ago

During the 2023-11-15 meeting (see above), the group requested that this issue be closed, and the discussion to be continued in two separate issues (extracted from this one). These issues have now been created:

  1. https://github.com/w3c/vc-data-model/issues/1352
  2. https://github.com/w3c/vc-data-model/issues/1360

So presumably this issue can be marked 'pending closed'.

iherman commented 11 months ago

@dmitrizagidulin the issue has been marked as Pending Close last week. I would not consider Thanksgiving week as 'complete', so I would think the required pause should end next week, at which time @brentzundel can close the issue.

brentzundel commented 11 months ago

Closing this issue in preference of #1352 and #1360