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

Add interoperable way for holder-asserted claims in a VP #1186

Merged
merged 18 commits into from
Jul 23, 2023

Conversation

brentzundel
Copy link
Member

@brentzundel brentzundel commented Jul 5, 2023

This PR addresses #860 by introducing normative guidance for a holder to include self-asserted claims inside of a VP.
It is based on the conversation in #860 where it was suggested that the holder should construct a VC with the self-asserted claims and include that in the VP.


Preview | Diff

@David-Chadwick
Copy link
Contributor

Surely @context is also mandatory to include in the self-asserted VC

@brentzundel
Copy link
Member Author

Surely @context is also mandatory to include in the self-asserted VC

Yes, of course.
I should probably add a line like: "The following are required for self-attested VCs _in addition to the normative requirements in Sections 4 and 5 _

Copy link
Contributor

@andresuribe87 andresuribe87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would favor removing the special casing the SelfAssertedCredential. Specifically, removing the text that reads

These self-asserted claims are considered to be
secured by the same mechanism that secures the verifiable presentation in
which they are included.

and the one that says

The type property is required. The value MUST be
VerifiableCredential and SelfAssertedCredential.

Removing them would simplify implementations of the VCDM. Curious why this is the approach suggested?

@brentzundel
Copy link
Member Author

brentzundel commented Jul 6, 2023

@andresuribe87 The PR proposes a type for a self-asserted credential so that every implementer can use the same mechanism for indicating that a VC in a VP is self-asserted. I'm open to suggestions for another mechanism that would accomplish the same thing.

On the securing language: I could probably add a line indicating that a self-asserted VC shouldn't have a proof property because the proof for a self-asserted VC is the same as the proof for the VP.

edited for clarity

@andresuribe87
Copy link
Contributor

I'm open to suggestions for another mechanism that would accomplish the same thing.

I'm arguing for not introducing such mechanism. Verifiers could determine this by comparing the issuer of the proof for the VP vs. the issuer of the proof of a VC. When they match, then the VC can be considered self-asserted. Does that make sense?

@TallTed
Copy link
Member

TallTed commented Jul 7, 2023

[@brentzundel] On the securing language: I could probably add a line indicating that a self-asserted VC shouldn't have a proof to make it more clear that the proof for the self-asserted VC is the same as the proof for the VP.

Saying that "a self-asserted VC shouldn't have a proof" is saying that a self-asserted VC is not V.

On the other hand, saying that "the proof for [a] self-asserted VC is the same as the proof for the VP [containing that VC]" seems clear, and leaves the V intact.

@brentzundel
Copy link
Member Author

brentzundel commented Jul 7, 2023

I'm arguing for not introducing such mechanism. Verifiers could determine this by comparing the issuer of the proof for the VP vs. the issuer of the proof of a VC. When they match, then the VC can be considered self-asserted. Does that make sense?

Currently, the holder property is optional, so it can't be relied on to be present in the verifiable presentation for comparison to the issuer field of the self-asserted VC (which means I need to tweak the language in this PR).

Maybe a VP that contains a self-asserted VC MUST have aholder property?

@brentzundel
Copy link
Member Author

Saying that "a self-asserted VC shouldn't have a proof" is saying that a self-asserted VC is not V.

I was not clear (and have edited the comment for clarity). I meant to say "a self-asserted VC shouldn't have a proof property, because the proof for a self-asserted VC is the same as the proof for the VP"

@andresuribe87
Copy link
Contributor

I'm arguing for not introducing such mechanism. Verifiers could determine this by comparing the issuer of the proof for the VP vs. the issuer of the proof of a VC. When they match, then the VC can be considered self-asserted. Does that make sense?

Currently, the holder property is optional, so it can't be relied on to be present in the verifiable presentation for comparison to the issuer field of the self-asserted VC (which means I need to tweak the language in this PR).

Maybe a VP that contains a self-asserted VC MUST have aholder property?

I think that's better. Yet I still feel that we'd be better off by not adding a the SelfAssertedCredential type. Another reason why I would like to not add it is because Verifiers should define the way in which they decide whether a VC is self-asserted. That is very much business logic, and should be up to them to define how it is done.

@brentzundel
Copy link
Member Author

Verifiers should define the way in which they decide whether a VC is self-asserted. That is very much business logic, and should be up to them to define how it is done.

I agree that a verifier should be able to determine what business logic they apply. It is not clear to me what that has to do with a holder being given the ability to make clear that their claims are self-asserted. A verifier remains free to accept additional ways, or to ignore the SelfAssertedCredential type.
The whole point of the PR is to provide a single common way that this can be done. That doesn't prohibit the myriad other ways a verifier might prefer to do it.

I believe the interop story is much stronger (and much more testable) with the inclusion of a new type than with any combination of other properties from which a verifier can infer that a VC is self-attested.
If having the new type plus matching the holder/issuer fields is too complex, I'd much rather remove the requirement that the holder/issuer fields match and rely solely on the type, especially considering that some securing methods for VPs won't require the holder property be present in the VP at all.

@andresuribe87
Copy link
Contributor

I believe the interop story is much stronger (and much more testable) with the inclusion of a new type than with any combination of other properties from which a verifier can infer that a VC is self-attested.

I agree that it's much more testable, but there are also drawbacks. Imagine a verifier that relies on multiple services for validation. When the verifier sends over the SelfAssertedCredential VC to the validator, then the validator would need the whole VP in order to be able to independently verify the contents of such VC. Such a scenario makes interoperability difficult, as both will need to understand and implement support for the SelfAssertedCredential type.

@brentzundel
Copy link
Member Author

When the verifier sends over the SelfAssertedCredential VC to the validator, then the validator would need the whole VP in order to be able to independently verify the contents of such VC.

Wouldn't this already be the case, as the self-asserted VC relies on the VP's securing mechanism for verifiablility.

@andresuribe87
Copy link
Contributor

When the verifier sends over the SelfAssertedCredential VC to the validator, then the validator would need the whole VP in order to be able to independently verify the contents of such VC.

Wouldn't this already be the case, as the self-asserted VC relies on the VP's securing mechanism for verifiablility.

My understanding is that the self-asserted VC is a VC thats secured independently from the VP.

@brentzundel
Copy link
Member Author

My understanding is that the self-asserted VC is a VC thats secured independently from the VP.

I understand that that's what you want this PR to say. That's not what it says and not what I believe reflects the previous conversation on this topic held in Issue #860
(particularly #860 (comment)).

I believe that this PR is the simplest mechanism for normative inclusion of a self-asserted VC inside of a VP.

As it is clear that you and I do not agree, I ask @Sakurann @jandrieu and @David-Chadwick to indicate whether this PR is acceptable to them so we can get a better sense of where consensus may lie.

@Sakurann
Copy link
Contributor

Sakurann commented Jul 8, 2023

I honestly think both options should be mentioned: a) self-attested claims being secured by a VP and b) self-attested claims included in a holder-signed VC.

For option a), what is the benefit of formatting self-attested claims in an unsigned verifiable credential? it can be pretty confusing. Why not include self-attested claims as top level claims alongside VP's type etc?

I think option b) above is beneficial because there is a possibility that self-signed VC is sent by itself without any other VC/VP, or sent alongside other VP(s) (that contain non-Holder signed VC(s)), in which case I am not sure what is the benefit of mandating nesting VC in a VP.

I do not think there are enough implementation experience around holder-signed VCs to choose a) vs b) and I think VCDM is extensible enough to support both without major interop issues.

index.html Outdated
Comment on lines 2056 to 2058
<code>VerifiableCredential</code> and <code>SelfAssertedCredential</code>.For
example,<br>
<code>"type": ["VerifiableCredential", "SelfAssertedCredential"]</code>
Copy link
Member

@msporny msporny Jul 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I agree with @andresuribe87 here -- it /could/ be SelfAssertedCredential, or it could be just about any other type of VC. IOW, the thing that makes a VC "self-asserted" is that it's issued by the holder.

So, there is probably some sort of interplay here with VP.holder and the issuer for the self-asserted VC.

EDIT: I now see that there is a documented interplay below; good.

index.html Outdated
Comment on lines 2062 to 2063
The <code>issuer</code> <a>property</a> is required. The value MUST be identical
to the <code>holder</code> <a>property</a> of the <a>verifiable presentation</a>.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes to this.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 to this as well.

index.html Outdated
"holder": "did:example:12345678",
"verifiableCredential": [{
"@context": "https://www.w3.org/ns/credentials/v2",
"type": ["VerifiableCredential", "SelfAssertedCredential"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"type": ["VerifiableCredential", "SelfAssertedCredential"],
"type": ["VerifiableCredential", "ExampleFoodPreferenceCredential"],

I'm unsure about this suggestion. I think it's the required type of SelfAssertedCredential that's throwing me off.

Given that we want to create a level playing field (anyone can issue VCs about anything, holders included), it feels like we shouldn't create a distinction between "self-asserted" vs. "officially-asserted". These are all just VCs and you either trust the issuer to make statements, or you don't. One of those issuers that you choose to trust might be the holder, like when you ask them about their favorite cheese... and you need to have them on record that they (the holder) expressed what their favorite cheese was to you... perhaps due to some sort of really important cheese census taking effort (which I expect they take seriously in countries where a significant portion of their commerce is related to cheese).

So, in this case, they'd probably be issuing a VC with a type of FoodPreferenceCredential, not SelfAssertedCredential.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have modified the example to include FoodPreferenceCredential, but I am not yet convinced that removing SelfAssertedCredential is the best path forward.

@msporny
Copy link
Member

msporny commented Jul 8, 2023

@andresuribe87 wrote:

I still feel that we'd be better off by not adding a the SelfAssertedCredential type.

This resonates with me. The VC type could be anything, given that the VC architecture allows anyone to say anything about anything... so best to not establish a VC type for self-asserted claims. I tried to elaborate more about this concern here: #1186 (review)

@Sakurann wrote:

I honestly think both options should be mentioned: a) self-attested claims being secured by a VP and b) self-attested claims included in a holder-signed VC.

Yes, agree with @Sakurann. Here are some use cases we might base each mechanism on:

a) self-attested claims being secured by a VP

Expressing the terms of use for a particular presentation (such as, "Use only for the purposes of this transaction", or "Do not cache", or "Do not monetize"). The unfortunate truth here is that we don't have any good terms of use expression languages for VCs (beyond ODRL, which doesn't seem to be getting much uptake).

b) self-attested claims included in a holder-signed VC.

This is what this PR does, so I think we have this use case covered.

To be clear, I'm supportive of this PR and there is a variation of it that is useful and I think can gain consensus. Waiting on feedback from @jandrieu and @David-Chadwick to read their thoughts and then will try to provide some alterations that might align and get us to consensus.

@dlongley
Copy link
Contributor

dlongley commented Jul 9, 2023

@andresuribe87 wrote:

I still feel that we'd be better off by not adding a the SelfAssertedCredential type.

This resonates with me. The VC type could be anything, given that the VC architecture allows anyone to say anything about anything... so best to not establish a VC type for self-asserted claims. I tried to elaborate more about this concern here: #1186 (review)

+1 ... I agree with the other things said about this as well.

@Sakurann wrote:

I honestly think both options should be mentioned: a) self-attested claims being secured by a VP and b) self-attested claims included in a holder-signed VC.

a) self-attested claims being secured by a VP

I think this is really just a VC without an assertion method proof, i.e., it is only secured using the authentication proof associated with the VP. Whether or not it's "self-attested" doesn't seem like a requirement for putting the components together this way, though it may be the most common use case for doing this sort of thing.

So, it seems we should just be clear that a VP may have a VC in it that doesn't carry its own proof (e.g., no proof clearly marked as signed for the purpose of making an assertion... if using data integrity proofs). Really, it could be more than just one, allowing for the holder to reuse the full breadth of credential vocabularies out there just like any other issuer. Such a VC can't be separated from the VP without losing the ability to verify any cryptography associated with it -- which has advantages and disadvantages.

It's not clear how easy it will be to build general purpose verification services (that are separable from the business logic component in a system) to handle this case -- except in the cases that holder matches issuer in the VCs that lack such proofs. So, I expect, in practice, those will be the only sorts of VCs that could pass verification by such software -- but I don't think we need to add any language to the data model spec restricting other possibilities.

We should just say that a "self-attested" VC in a VP is one where the VP has a holder property and its value matches the issuer value in the VC. I think that keeps it simple and probably allows for general purpose verification services to be easily implemented.

b) self-attested claims included in a holder-signed VC.

I think this is already covered generally and is just a case where the issuer of a VC happens to also be the party presenting.

Copy link
Contributor

@dlongley dlongley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've left some suggestions that match my earlier comments and that seem like they would be inline with what others have suggested.

index.html Outdated Show resolved Hide resolved
index.html Outdated
Comment on lines 2047 to 2065
A self-asserted <a>verifiable credential</a> that has been included in the value
of the <code>verifiableCredential</code> <a>property</a> of a
<a>verifiable presentation</a> MUST include the following properties:
</p>

<dl>
<dt><var>type</var></dt>
<dd>
The <code>type</code> <a>property</a> is required. The value MUST be
<code>VerifiableCredential</code> and <code>SelfAssertedCredential</code>.For
example,<br>
<code>"type": ["VerifiableCredential", "SelfAssertedCredential"]</code>
</dd>
<dt><var>issuer</var></dt>
<dd>
The <code>issuer</code> <a>property</a> is required. The value MUST be identical
to the <code>holder</code> <a>property</a> of the <a>verifiable presentation</a>.
</dd>
</dl>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
A self-asserted <a>verifiable credential</a> that has been included in the value
of the <code>verifiableCredential</code> <a>property</a> of a
<a>verifiable presentation</a> MUST include the following properties:
</p>
<dl>
<dt><var>type</var></dt>
<dd>
The <code>type</code> <a>property</a> is required. The value MUST be
<code>VerifiableCredential</code> and <code>SelfAssertedCredential</code>.For
example,<br>
<code>"type": ["VerifiableCredential", "SelfAssertedCredential"]</code>
</dd>
<dt><var>issuer</var></dt>
<dd>
The <code>issuer</code> <a>property</a> is required. The value MUST be identical
to the <code>holder</code> <a>property</a> of the <a>verifiable presentation</a>.
</dd>
</dl>
A <a>verifiable presentation</a> that includes a self-asserted
<a>verifiable credential</a> that is only secured using the same mechanism
as the <a>verifiable presentation</a> MUST include a <code>holder</code>
<a>property</a>. The value of the <code>holder</code> <a>property</a>
MUST be identical to the value of the <code>issuer</code> <a>property</a>
for every such self-asserted <a>verifiable credential</a>.
</p>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not accept this suggestion because, in addition to clarifying requirements for a self-asserted VC that is secured using the VP mechanism, it removes the type requirements. I believe that there is real value in having a unified type for self-asserted credentials, so rather than removing it entirely have changed the requirement from a MUST to RECOMMENDED.

index.html Outdated
"issuer": "did:example:12345678",
"credentialSubject": {
<span class="highlight">"id": "http://example.com/presentation/5223"</span>,
"validUntil": "2024-01-01T19:23:24Z"
Copy link
Member

@msporny msporny Jul 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"validUntil": "2024-01-01T19:23:24Z"
"validUntil": "2024-01-01T19:23:24Z"

Huh... this is valid, and proposes the question: Why not just put validUntil on the presentation itself? We might want to have an example of someone putting a self-asserted claim on the VP itself... possibly in another PR, we don't want to complicate this one further.

IOW, do we want to enable validFrom and validUntil onto the Presentation too? I think we do want to enable that. At present, it's only allowed on a VerifiableCredential. Historically, this was because presentations were expected to always be short lived... but the short-lived-ness being determined by the verifier and the created/iat date on the proof, not explicitly stated by the holder.

This comment is non-blocking.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@msporny I think you're probably right, but I don't want that conversation to derail this PR. I think a better example is probably be what's needed here.

This example came out of my attempt to address @jandrieu 's desire for a normative way for a holder to make claims about the VP. As I constructed the self-asserted VC, I had a hard time coming up with a claim the holder would want to make about the VP.
This is just to say that there is probably a better example of the sort of claims a holder would want to make about the VP, but I couldn't think one up. Looking to @jandrieu for suggestions.

Copy link
Member

@msporny msporny Jul 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, we don't want to derail this PR with trying to find the perfect example. I defer to @jandrieu to suggest a use case. I'll try to think of a use case (with markup) as well (for a future PR).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the example based on Joe's comment here: https://github.com/w3c/vc-data-model/pull/1186/files#r1268305835

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Thanks Ted!

Co-authored-by: Ted Thibodeau Jr <[email protected]>
Comment on lines +2067 to +2070
mechanism as the <a>verifiable presentation</a>, the value of the
<code>issuer</code> <a>property</a> of the <a>verifiable credential</a>
MUST be identical to the <code>holder</code> <a>property</a> of the
<a>verifiable presentation</a>.
Copy link
Contributor

@dlongley dlongley Jul 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that we could have a situation where the VC uses an issuer property that expresses an object with the issuer's identifier in the id property (typically to express other properties about the issuer) -- and a VP uses a holder property that directly expresses the identifier of the holder. To account for this, I think we should indicate that it's the identifiers that must match and just reference those sections for how they could be expressed:

Suggested change
mechanism as the <a>verifiable presentation</a>, the value of the
<code>issuer</code> <a>property</a> of the <a>verifiable credential</a>
MUST be identical to the <code>holder</code> <a>property</a> of the
<a>verifiable presentation</a>.
mechanism as the <a>verifiable presentation</a>, the identifier of the
<a>issuer</a> of the <a>verifiable credential</a> (expressed according
to Section <a href="#issuer"></a>) MUST be identical to the identifier of
the <a>holder</a> of the <a>verifiable presentation</a> (expressed
according to Section <a href="#presentations-0"></a>).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jandrieu ^ this was the intention of citing the normative definitions related to graph objects, in the "holder" validation section PR, can you review here as well?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like we can perhaps remove that text from validation, if this section repeats it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the language already present is simpler and is sufficient.
The holder is the entity constructing both the VP and the self-asserted VC, so they can make sure those values are identical, whether they choose to use an object with an id or just a URL.

Plus, we already have this line above, which was introduced specifically to avoid pointing back to each normative section of the spec that also applies: "All of the normative requirements defined for verifiable credentials apply to self-asserted verifiable credentials."

I would prefer to not make this change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not going to block this PR if the change isn't made either.

Copy link
Contributor

@jandrieu jandrieu Jul 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting.

This doesn't say anything about how the holder makes a VC about the current presentation.

I was expecting something like

It is possible for the holder to express statements about a Verifiable Presentation in that presentation itself, e.g., documenting that "the this presentation was submitted by the subject as evidence of their legal right to drive." In this situation, the holder simply creates a Verifiable Credential with claims with a subject id that matches the "id" of the Verifiable Presentation. Those claims are understood to be about the presentation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as an aside, presentations and credentials don't require id, but when one is present, the claims are bound to the id.

Copy link
Contributor

@OR13 OR13 Jul 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this suggestion should be applied, we can clean up when the dust settles.

@brentzundel
Copy link
Member Author

@selfissued and @TallTed I believe I have made the changes you've requested. Could you please re-review?

@msporny msporny requested review from jandrieu and TallTed July 19, 2023 19:09
@David-Chadwick
Copy link
Contributor

"As it is clear that you and I do not agree, I ask @Sakurann @jandrieu and @David-Chadwick to indicate whether this PR is acceptable to them so we can get a better sense of where consensus may lie."
This is not the approach I would have taken. As I said earlier, a simpler approach is for the holder to put the assertions in the VP itself, rather than one level down in an embedded VC. Then it is obvious that the holder is making these assertions about this VP. But given that the majority of folk seem to prefer a self signed VC embedded in the VP, then I will not strongly object. I just think it is technically a worse solution for the reasons I stated earlier, and for some raised here by others

@OR13
Copy link
Contributor

OR13 commented Jul 20, 2023

+1 to this:

a simpler approach is for the holder to put the assertions in the VP itself, rather than one level down in an embedded VC.

However the reason people seem not to be doing this is that issuer and holder are treated differently than credentialSubject.

Despite all being RDF graph nodes... This seems to be a convention not a normative requirement.

In a different universe we could have had this:

{
type: VC,
issuer: Alice,
credentials: [{
type: VC
issuer: Bob,
subject: { id: Charlie }
}]
}

No need for VP or holder.

Because they are both just special cases of VC and issuer.

@iherman
Copy link
Member

iherman commented Jul 20, 2023

The issue was discussed in a meeting on 2023-07-19

  • no resolutions were taken
View the transcript

1.2. Add interoperable way for holder-asserted claims in a VP (pr vc-data-model#1186)

See github pull request vc-data-model#1186.

Manu Sporny: 1186 is about 'Holder Asserted Claims'. Brent has been processing feedback about it; JoeAndrieu, TallTed are you OK with this PR?

Joe Andrieu: The language doesn't address what I was referring to. I have some language which would fit, but the PR is rather tangential now.

Brent Zundel: I think I addressed it in a part of the PR you were not tagged for.

Ted Thibodeau Jr.: I'll re-review 1186 and 1149 (and any others currently awaiting my input, if I can have a list of them) after this call.

Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go with it

@msporny
Copy link
Member

msporny commented Jul 23, 2023

Normative, multiple reviews, changes requested and made, no objections, merging.

@msporny msporny merged commit cad8bae into w3c:main Jul 23, 2023
Copy link
Contributor

@jandrieu jandrieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good. My last comments are effectively editorial. I think we're trying to say something correct, but saying it badly.

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

Successfully merging this pull request may close these issues.