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 context integrity approach for VC-JWTs #90

Closed
wants to merge 4 commits into from

Conversation

mprorock
Copy link
Contributor

@mprorock mprorock commented May 30, 2023

With verifiable credentials, or any signed JSON-LD it is often a desirable property to know that the context you are working with is the same on both ends, or if there has been a change. This PR adds an optional ability to provide the hash of contexts used at issuance time.


Preview | Diff

@mprorock mprorock requested review from selfissued and OR13 May 30, 2023 13:17
@andresuribe87
Copy link
Contributor

What are the benefits of putting this in vc-jwt, as opposed to putting them in the VCDM?

@mprorock
Copy link
Contributor Author

What are the benefits of putting this in vc-jwt, as opposed to putting them in the VCDM?

this absolutely could go in the core data model if it is desired there - at minimum, having this in VC-JWT to address the security considerations notes around contexts in JSON-LD etc will be required.

index.html Outdated Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
@mprorock
Copy link
Contributor Author

thanks @andresuribe87 - i added a few things based on your careful reading. very very much appreciated.

Copy link
Collaborator

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

This is not at all JWT-like. For instance, it uses a long-form timestamp rather than a numeric time value.

I also believe that, should we decide that we want this functionality, it belongs in the data model rather than VC-JWT, because that's where the context stuff is. We shouldn't be doing anything to add to context semantics in this spec.

index.html Outdated
<section>
<h2>Context Integrity</h2>
<p>
In some cases is is desireable to know that the contents of the
Copy link
Collaborator

Choose a reason for hiding this comment

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

In some cases, it is desirable

index.html Outdated
<h2>Context Integrity</h2>
<p>
In some cases is is desireable to know that the contents of the
context(s) utilized in the verifiable credential is the same as
Copy link
Collaborator

Choose a reason for hiding this comment

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

are the same

@mprorock
Copy link
Contributor Author

mprorock commented Jun 1, 2023

This is not at all JWT-like. For instance, it uses a long-form timestamp rather than a numeric time value.

I also believe that, should we decide that we want this functionality, it belongs in the data model rather than VC-JWT, because that's where the context stuff is. We shouldn't be doing anything to add to context semantics in this spec.

I think the core data model is likely a better place for this.... Will want bring it up on the next VCWG call @Sakurann @brentzundel

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@OR13
Copy link
Contributor

OR13 commented Jun 1, 2023

This is not at all JWT-like. For instance, it uses a long-form timestamp rather than a numeric time value.

This is because this is a core data model "extension" that just happens to be being defined here... which... seems wrong.

This should be handled like all the other "core data model extensions", in the core spec.

And I would object there a well, unless a similar bar to what has been met for renderMethod is achieved.

Copy link
Contributor

@OR13 OR13 left a comment

Choose a reason for hiding this comment

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

Core data model extension points belong in the core data model.

Co-authored-by: Orie Steele <orie@or13.io>
@mprorock
Copy link
Contributor Author

mprorock commented Jun 2, 2023

opened this pr on core data model that would permit us to not handle this in vc-jwt as it is really across teh board an issue for VC securing formats and should be handled consistently

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.

Tweaks for clarity

Comment on lines +390 to +391
As noted in [[rfc7515]] when utilizing JSON [[rfc7159]], strict validation is a security
requirement. If malformed JSON is received, it may be impossible to reliably interpret the
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
As noted in [[rfc7515]] when utilizing JSON [[rfc7159]], strict validation is a security
requirement. If malformed JSON is received, it may be impossible to reliably interpret the
As noted in [[rfc7515]], strict validation is a security requirement when using
JSON [[rfc7159]]. If malformed JSON is received, it may be impossible to reliably interpret the

producer's intent, potentially leading to ambiguous or exploitable situations.
To prevent these risks, it is essential to use a JSON parser that strictly validates the syntax
of all input data. It is essential that any JSON inputs that do not conform to the JSON-text syntax defined in [[rfc7159]]
be rejected in their entirety by JSON parsers. Failure to reject invalid input could compromise
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
be rejected in their entirety by JSON parsers. Failure to reject invalid input could compromise
be rejected in their entirety by JSON parsers. Failure to reject such invalid input could compromise

<h2>Context Integrity</h2>
<p>
In some cases it is desirable to know that the contents of the
context(s) utilized in the verifiable credential are the same as
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
context(s) utilized in the verifiable credential are the same as
context(s) used in the verifiable credential are the same as

Follow all security considerations outlined in [[rfc7515]] and [[rfc7519]].
</p>
<p>
When utilizing JSON-LD, take special care around remote retrieval of contexts and follow the
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
When utilizing JSON-LD, take special care around remote retrieval of contexts and follow the
When using JSON-LD, take special care around remote retrieval of contexts and follow the

</p>
<p>
To validate that a context included in a Verifiable Credential is
the same at verification time as at issuing time an implementer
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
the same at verification time as at issuing time an implementer
the same at verification time as at issuing time, an implementer

Comment on lines +419 to +423
the URL to the context named <code>context</code>, a
<code>timestamp</code>
that indicates the time at which the hash was computed, the
<code>hash</code>
of the context, and the <code>method</code> which indicates what
Copy link
Member

Choose a reason for hiding this comment

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

It might be better to use an <ul> for these. If making that switch, there should be no punctuation at the end of any list item (i.e., delete the semicolons here as well as the . after Registry).

Suggested change
the URL to the context named <code>context</code>, a
<code>timestamp</code>
that indicates the time at which the hash was computed, the
<code>hash</code>
of the context, and the <code>method</code> which indicates what
the URL to the context named <code>context</code>; a
<code>timestamp</code>
that indicates the time at which the hash was computed; the
<code>hash</code>
of the context; and the <code>method</code> which indicates what

href="https://www.iana.org/assignments/named-information/named-information.xhtml">IANA
Named Information Hash Algorithm Registry</a> to ensure that they
are chosing a current and reliable hash algorithm. At the time of
this writing `sha-256` should be considered the minimum strength
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
this writing `sha-256` should be considered the minimum strength
this writing, `sha-256` is considered the minimum strength

Comment on lines +441 to +447
If at a later date subresource integrity as defined in [[SRI]] is
adopted into the [[JSON-LD]] specification as noted in that
specifications <a
href="https://www.w3.org/TR/json-ld11/#security">current security
considerations</a> of that specification, this hash in the VC can
serve as an additional check towards ensuring that a cached
context used when issuing the VC matches the remote resource.
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
If at a later date subresource integrity as defined in [[SRI]] is
adopted into the [[JSON-LD]] specification as noted in that
specifications <a
href="https://www.w3.org/TR/json-ld11/#security">current security
considerations</a> of that specification, this hash in the VC can
serve as an additional check towards ensuring that a cached
context used when issuing the VC matches the remote resource.
If subresource integrity as defined in [[SRI]] is adopted into
the [[JSON-LD]] specification at a later date, as noted in the
<a href="https://www.w3.org/TR/json-ld11/#security">current security
considerations</a> of that specification, this hash in the VC can
serve as an additional check towards ensuring that a cached
context used when issuing the VC matches the remote resource.

@OR13
Copy link
Contributor

OR13 commented Jun 6, 2023

This PR should be closed, because w3c/vc-data-model#1140 has been opened.

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