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

Clarify how issuer validation occurs. #1393

Merged
merged 10 commits into from
Dec 26, 2023
Merged

Clarify how issuer validation occurs. #1393

merged 10 commits into from
Dec 26, 2023

Conversation

msporny
Copy link
Member

@msporny msporny commented Dec 17, 2023

This PR is an attempt to address issue #1386 by clarifying how issuer validation could occur. It establishes an interface between the VCDM and securing specifications for the securing mechanism verification algorithms. It also adds more detail to the issuer validation section in the specification.

NOTE: The algorithms haven't been updated to use the new interface (that will be done in a separate PR). What's important in this PR is that the controller and controller document is being returned by the securing mechanism verification algorithm such that a check between the issuer property and the controller is possible.

/cc @jyasskin (this PR starts to define the interface between the Securing Mechanisms and the VCDM)


Preview | Diff

@msporny msporny changed the base branch from main to msporny-safe-processing December 18, 2023 01:09
@msporny msporny marked this pull request as ready for review December 18, 2023 02:03
index.html Outdated
Comment on lines 2009 to 2026
<li>
MUST document normative algorithms that provide content integrity protection
for <a>conforming documents</a>. The algorithms MAY be general in nature and
MAY be used to secure data other than <a>conforming documents</a>.
</li>
<li>
MUST provide a verification mechanism that returns only the information in the
<a>conforming document</a> that has been secured. It MAY provide an
additional mechanism to receive other information that might be helpful
during validation or for debugging purposes. The concrete interface that is
expected to be provided is detailed in the
<a href="#verify-securing-mechanism">securing mechanism verification
algorithm</a> section.
</li>
<li>
MUST document post-processing considerations for data returned from verification
processes. This includes, but is not limited to, what information can and cannot
be used for decision making, for what period of time after verification, when
additional information is required, and when care is necessary even when the
returned data was properly secured.
</li>
<li>
SHOULD provide integrity protection for any information referenced by a URL that
is critical to validation. Mechanisms that can achieve this protection are
discussed in Section <a href="#integrity-of-related-resources"></a> and Section
Copy link
Member Author

@msporny msporny Dec 18, 2023

Choose a reason for hiding this comment

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

The text above hasn't been deleted, but has instead been rewritten below in non-list form.

index.html Outdated
Comment on lines 2018 to 2021
that might be helpful during validation or for debugging purposes. A securing
mechanism's verification algorithm MUST provide an interface that
receives an sequence of bytes ([=byte sequence=] |inputBytes|) and a media type
([=string=] |inputMediaType|) as inputs and returns a verification result with
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it must be also possible for securing mechanisms to be valid if they accept a JSON document / parsed JSON document (Map/dictionary?) instead of bytes and an input media type. Many implementations will be written that way for embedded securing mechanisms (and already are), based on common software pipeline practices that will have JSON already being parsed into a document / object before it hits this API.

We should allow for either of these approaches to be used, especially if we are requiring these algorithms to be normative in the securing mechanism specifications.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 02c3744.

index.html Outdated
Comment on lines 2015 to 2017
Securing mechanism specifications MUST provide a verification mechanism that
returns only the information in the <a>conforming document</a> that has been
secured. It MAY provide an additional mechanism to receive other information
Copy link
Contributor

Choose a reason for hiding this comment

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

How does this sentence apply when Data Integrity is used? Does it mean that the "proof" member must not be returned in the secured information? That would be my interpretation but this could be clearer.

Copy link
Member Author

@msporny msporny Dec 19, 2023

Choose a reason for hiding this comment

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

All current Data Integrity specifications secure all the information in the proof value. There is no such thing as "unprotected" in the Data Integrity cryptosuites that exist today.

That said, we defer what to do to the securing mechanism specifications. It is their remit wrt. what is returned. For example, vc-jose-cose might choose to strip out proof values that were not verified during the securing process (but by doing that, it'll make it impossible for upstream processors to verify those proofs). So, there will need to be work done in vc-jose-cose to determine what to do there.

For Data Integrity, I expect all the current cryptosuites we have to return proof in the returned data because all information in the proof values ARE protected and verified as a part of the securing mechanism verification process.

Copy link
Contributor

Choose a reason for hiding this comment

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

In DI, all verified proofs are secured -- so DI securing mechanisms can return all proofs there were verified using the verifier's accepted cryptosuites from the base verification algorithm.

Copy link
Contributor

Choose a reason for hiding this comment

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

The plain meaning of the text above implies that proof is used to secure the secured data and is not in the secured data. Please be clear that proof, if present, must be removed by this algorithm.

Copy link
Member Author

Choose a reason for hiding this comment

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

@selfissued fixed in 8c16fec.

Copy link
Contributor

@selfissued selfissued left a comment

Choose a reason for hiding this comment

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

I will approve this PR after my suggestion about not returning proof as part of the secured data is accepted.

@iherman
Copy link
Member

iherman commented Dec 20, 2023

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

  • no resolutions were taken
View the transcript

2.4. Clarify how issuer validation occurs. (pr vc-data-model#1393)

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

Brent Zundel: "Clarify how issuer validation occurs".

Manu Sporny: This PR is an attempt at addressing Oliver's concern where Oliver is saying that the spec needs to be clear about how you verify that a VC came from a particular issuer.
… Some people in the group would argue that's validation.
… And that we don't do that and don't normatively specify that. So what I tried to do is ... we're now specifying an interface between the securing mechanisms and the VCDM verify algorithm -- and it now returns the controller and controller document. That gives you info you could use to check against the issuer.
… However, that's not the only way to do validation, you could also have a trust list with some keys and you could just trust that -- including the statements in the VC, including the issuer field.
… In that case the issuer isn't directly tied to the verification method and controller, etc. -- so I tried to address Oliver's concern to ensure the information is available if the issuer validation works one way.
… I added some non-normative language as well to help address Oliver's concerns. I don't know if these changes together address his concerns.

Brent Zundel: Any comments on this PR?
… If all of these, if it's possible to find consensus, if the language needs tweaks or not, that is our goal. If you have comments to make please do or say the PR is great, etc. Let us know what we need to change for you if you need it.

index.html Outdated
Comment on lines 6446 to 6453
Relevant metadata that is related to the <code>issuer</code> <a>property</a>
is available to the <a>verifier</a> through the
<a href="#verify-securing-mechanism">securing mechanism verification
algorithm</a> as defined in Section <a href="#securing-mechanisms"></a>. This
information includes the controller, which is typically the `issuer`, of the
verification method used by the securing mechanism to secure each <a>verifiable
credential</a>. When verifying a <a>verifiable presentation</a>, the controller
of the verification method is typically the `holder`.
Copy link
Member

Choose a reason for hiding this comment

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

I hope this edit makes this (single) paragraph somewhat clearer.

I wonder whether the first sentence of this paragraph should discuss metadata related to the `holder` property in addition to metadata related to the `issuer` property.

It might be better to cover verifiable credentials (and issuers) in one paragraph, and verifiable presentations (and holders) in another.

Suggested change
Relevant metadata that is related to the <code>issuer</code> <a>property</a>
is available to the <a>verifier</a> through the
<a href="#verify-securing-mechanism">securing mechanism verification
algorithm</a> as defined in Section <a href="#securing-mechanisms"></a>. This
information includes the controller, which is typically the `issuer`, of the
verification method used by the securing mechanism to secure each <a>verifiable
credential</a>. When verifying a <a>verifiable presentation</a>, the controller
of the verification method is typically the `holder`.
Metadata related to the `issuer` <a>property</a>
is available to the <a>verifier</a> through the
<a href="#verify-securing-mechanism">verification algorithm of the securing
mechanism</a> as defined in Section <a href="#securing-mechanisms"></a>.
This metadata includes identification of the controller of the verification
method used by the securing mechanism to secure each <a>verifiable credential</a>
or <a>verifiable presentation</a>, of which the controller is typically the
respective `issuer` or `holder`.

Copy link
Member Author

Choose a reason for hiding this comment

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

Applied in af27128.

Copy link
Contributor

@awoie awoie left a comment

Choose a reason for hiding this comment

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

I assume the securing mechanism verifies the returned controller is the controller of the securing mechanism?

index.html Outdated Show resolved Hide resolved
@msporny
Copy link
Member Author

msporny commented Dec 21, 2023

@awoie, @selfissued, and @TallTed, I have applied each of your requested changes, requesting re-review from each of you.

index.html Outdated Show resolved Hide resolved
@msporny msporny changed the base branch from msporny-safe-processing to main December 26, 2023 16:06
@msporny
Copy link
Member Author

msporny commented Dec 26, 2023

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

@msporny msporny merged commit 8d81752 into main Dec 26, 2023
1 check passed
@msporny msporny deleted the msporny-issuer-validation branch December 26, 2023 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants