Make the value of the base context normative. #1158
Conversation
msporny
requested review from
OR13,
selfissued,
decentralgabe and
awoie
as code owners
msporny
requested review from
TallTed,
brentzundel,
Sakurann,
iherman and
andresuribe87
There was a problem hiding this comment.
I am fine with the changes proposed in this PR, hence my approval.
However, with regard to your comment:
One editorial fix the Editor's might make after this PR is to move the Base Context section from the appendix into the main specification (given that having a normative appendix section is a bit strange).
I am a -1 to that. There is a difference between saying that the URL of the context file is "normative" (also reinforced through the hash value) and listing the content of the context file in the specification, which makes every line of the context normative as well. This leads to the sort of confusion that I have described, for example, in #1103 (comment):
If we relied on
@context[…] as the source of that normative information, then we hit a bunch of additional questions like: what is the status of the mapping ofnoncetohttps://w3id.org/security#noncev.a.v. the security specification? How do we account for mappings likenametohttps://schema.org/name? In essence: which part of the@contextis normative and which one is not? Etc.
My proposal is not to include the context file in the specification at all. The URL of the context file is de-referencable, and we can also be sure that one will download the right file if the hash value is checked. By keeping only the reference in the spec, i.e., by being silent about the normativeness of the content of the @context file, we avoid all these issues, we avoid the set of open PR-s that resulted from that confusion, etc.
My 2 cents...
| <h3>Base Context</h3> | ||
|
|
||
| <p class="issue" title="(AT RISK) Hash values might change during Candidate Recommendation"> |
There was a problem hiding this comment.
is there an issue we can tag to tack this?
There was a problem hiding this comment.
I don't think it's necessary to create an issue to track this statement. It creates more work for everyone w/o really having any upside. What's the issue going to say other than "the hash values might change" -- there's nothing to be discussed/debated there -- it's just a statement of fact.
Not every issue marker needs to map to an issue. This is just guidance to readers and implementers.
|
I agree that having a normative appendix is weird. Could this language be added instead to the |
Yes, excellent idea... that's probably the ideal location for this text. I will update the PR to move the text to that location. |
index.html
Outdated
| Implementations MUST ensure that the base context value, located at | ||
| <code>https://www.w3.org/ns/credentials/v2</code>, matches the following | ||
| SHA-256 digest of the value: | ||
| <strong><code>dc28cf86d8a3199beb9d14e29d1da662346b8660abf2edca873098c88ca2a457</code></strong>. |
There was a problem hiding this comment.
Use the same approach for data integrity and context integrity see #1140
There was a problem hiding this comment.
There was a problem hiding this comment.
Rational for "why" alignment matters.
Implementers are required to load dependencies to support normative statements, when implementing the specification.
It should be a design goal of this working group to reduce the amount of vendor specific libraries (non standard implementations) that an implementation must use to implement this standard.
If implementers are already required to load a library that supports multibase (even a trivial library implemented by a startup), then they should be able to use that library to implement support for normative requirements the same way, everywhere they show up.
Here is a counter example (showing what not to do):
- v2 core context secured with sha512 hex encoded.
- resourceIntegrity feature secured with sha384 (base64url encoded)
- proofValue in data integrity secured with multibase (base58btc encoded)
There was a problem hiding this comment.
Rational for "why" alignment matters.
Implementers are required to load dependencies to support normative statements, when implementing the specification.
It should be a design goal of this working group to reduce the amount of vendor specific libraries (non standard implementations) that an implementation must use to implement this standard.
If implementers are already required to load a library that supports multibase (even a trivial library implemented by a startup), then they should be able to use that library to implement support for normative requirements the same way, everywhere they show up.
Here is a counter example (showing what not to do):
- v2 core context secured with sha512 hex encoded.
- resourceIntegrity feature secured with sha384 (base64url encoded)
- proofValue in data integrity secured with multibase (base58btc encoded)
i agree in principle and think that there is a desire to have this pr (or a follow on to it) to align with #1140 and vice versa. This would mean that our primary normative doc (the spec itself) is consistent.
regarding the reference to proof value (i believe only a reference from core data perspective in the context) i think that this should be handled separately (e.g. what goes in the context since we agreed it is normative?)
Broadly, i think the questions of encodings for data integrity and what is done under the data integrity spec belong there.
There was a problem hiding this comment.
(e.g. what goes in the context since we agreed it is normative?)
I agree, but unfortunately, we are making the context normative with logs of things already in it... so your solution does not work.
Broadly, i think the questions of encodings for data integrity and what is done under the data integrity spec belong there.
I also agree with this, but the working group disagreed, when I attempted to separate things.
So now we have to make them consistent, or someone else needs to try to separate them in a way so that they can diverge without causing further implementer burden.
There was a problem hiding this comment.
@OR13 — s/logs of things already in it/lots of things already in it/ (that is, logs->lots)?
There was a problem hiding this comment.
@msporny minor adjustments suggested here to bring things in line with SRI encoding of digest values and examples in integrity metadata - also brings this in line with latest updates (3ecd1b8) on #1140
I'm fine w/ the changes and will approve when @OR13 accepts these changes. |
|
I assume the base64 vs base64 url is correct? |
|
I will approve if changes are applied. |
|
We should express the hash in a way that's compatible with #1140 (to make it easier on developers who have implemented that mechanism). Notably, we could express the hash in multiple ways or add new ways editorially in the future as needed -- and this doesn't cause implementation burden for anyone. |
index.html
Outdated
| Implementations MUST ensure that the base context value, located at | ||
| <code>https://www.w3.org/ns/credentials/v2</code>, matches the following | ||
| SHA-256 digest of the value: |
There was a problem hiding this comment.
What are implementations supposed to do if they don't match?
There was a problem hiding this comment.
For the base context... almost certainly throw an error (if you are not using a cached copy, which all VC processors are expected to use).
@jyasskin, we did have approval to merge this on a call today, would you prefer that we don't and address this particular concern before we merge this PR? Or would you be ok w/ an issue marker that tracks the concern?
There was a problem hiding this comment.
I don't have any opinion about the timing of fixing this (i.e. feel free to merge and just queue the issue), but I think the spec will need to eventually tell the processors what to do instead of just saying they need to check. This might need a change to 5.2.5 in https://www.w3.org/TR/json-ld-api/#algorithm to make it possible to check contexts against a hash value when fetching them, or you could possibly require that all implementations treat this URL as "previously dereferenced" (step 5.2.4) with a particular value, in which case you don't need the hash at all.
There was a problem hiding this comment.
If the URL is previously dereferenced, it doesn't need to be a URL, it could be a URN.
Context needs to dereference to the same bytes... Context identifier does not actually matter...
Using a URN, or some other IRI would also protect W3C from being forced to serve the JSON-LD file that is always assumed to be previously dereferenced over and over again.
Making the value a URL encourages traffic, and implies it will be resolvable forever... It's not just a security thing, there is a cost / sustainability component to how this is made normative as well.
There was a problem hiding this comment.
If the URL is previously dereferenced, it doesn't need to be a URL, it could be a URN.
We've had this discussion before (setting aside also that a "URN" is now also a "URL" per WHATWG). To increase interoperability in the linked data ecosystem, it should not be a URN. Only the people who come, read, and implement the VCDM spec will be able to (and should be required to, IMO) treat the context as already dereferenced. We should not block other processors written by people who haven't read the spec from being able to consume VC data as general linked data.
Co-authored-by: Mike Prorock <mprorock@users.noreply.github.com>
|
The issue was discussed in a meeting on 2023-06-27
View the transcript1.1. Make the value of the base context normative. (pr vc-data-model#1158)See github pull request vc-data-model#1158. Michael Prorock: Suggest a quick diversion to 1158 since its related to 1140. There are a couple change I suggested to bring the language into alignment. If we are talking about hashing. Kristina Yasuda: lets talk about 1158. Manu Sporny: fine with these changes. Concern that it sounds like an argument is constructed based on these changes that puts us on a slippery slope. Kristina Yasuda: How is this a slippery slope. Manu Sporny: The next argument could make that all DI specification need to follow the SRI encoding format.
Kristina Yasuda: clarify that the scope is only the VC DM. Would not apply to data integrity.
Michael Prorock: This PR is solely to bring it up to 384 and encode the digest according to the SRI spec. this is focused just on the core data model.
Orie Steele: agree with mike prorock. When we look at resource integrity for context, context is a resource. we found a way to make the context values normative.
Kristina Yasuda: PR 1158 we have agreement to align with 1140.
Kristina Yasuda: are we OK to bring up 1100 and 1101? Brent Zundel: Can we get a sense of where each of the PRs are? we have 15 open PRs. Can we go through each of the PRs briefly? Kristina Yasuda: WIll take up PRs where we have authors on the call. |
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
|
Normative, multiple reviews, changes requested and made (issues raised for unresolved issues), no objections, merging. |
This PR implements two resolutions made during the last special topic call:
Namely:
Resolution #1: The v2 context URL will remain normative (https://www.w3.org/ns/credentials/v2), its value will be made normative through the use of a hash digest.
Resolution #2: Add issue markers saying that the value of the hash digest for the v2 context may change before PR and that’s expected.
One editorial fix the Editor's might make after this PR is to move the Base Context section from the appendix into the main specification (given that having a normative appendix section is a bit strange).
Preview | Diff