Require digest verification for related resources.

[msporny]

This PR is an attempt to address w3c/vc-data-integrity#272 by requiring digest verification for related resources.

---

Preview | Diff

[iherman]

@msporny I believe there is a github manipulation error. The 'diff' file shows over 1000 changes, mostly editorials, which makes it difficult to review. I presume the subject of this PR is only §5.3 on the Integrity of Related Resources...

[msporny]

@msporny I believe there is a github manipulation error. The 'diff' file shows over 1000 changes, mostly editorials, which makes it difficult to review. I presume the subject of this PR is only §5.3 on the Integrity of Related Resources...

This is what I see when I view the diff on Github:

There was an issue initially, which created the issue you mention, but I fixed that last night. I expect you probably hit a cache somewhere that gave you the old version. Try a hard refresh?

[iherman]

It may be there is a cache issue, but not mine... I have made a clean reload, and I have also looked at the diff file from browsers that I have never used for this purpose. I still see changes in the abstract or the introduction, for example:

Anyway. I can concentrate on the part that you highlight, maybe this exchange is good enough for other reviewers to disregard the rest...

[iherman]

B.t.w., if I look at the "Files changed" tab, then I get what you really changed only. Something with the preview cache...

[iherman]

Just being picky...

Suggested change

	the `id` of an object inside a [=conforming document=] with a corresponding
	cryptographic digest MUST check the digest against the retrieved resource. If
	the `id` of an object inside a [=conforming document=] with a corresponding
	cryptographic digest appearing in a `relatedResource` object value MUST check the digest against the retrieved resource. If

It is pickiness. I will a accept either way.

[dlongley]

@iherman,

Hmm, this is in the related resource section. The id is in the relatedResource object, that's the object that is being referred to here, i.e., relatedResource.id, so I don't think your suggestion is correct and / or it is redundant in a strange way. IMO, if we need to, we can say "the id of a relatedResource object" instead of "the id of an object".

Note the sentence right before the above paragraph says:

Any objects for which selective disclosure or unlinkable disclosure is desired SHOULD NOT be included as an object in the relatedResource array.

This "an object" ^ is the same one referred to in the paragraph above.

[TallTed]

Should this end something like "MUST produce an error that SHOULD indicate type _____"?

Suggested change

	A [=conforming verifier implementation=] that makes use of a resource based on
	the `id` of an object inside a [=conforming document=] with a corresponding
	cryptographic digest MUST check the digest against the retrieved resource. If
	the digest of the retrieved resource does not match the one provided by the
	[=issuer=], the [=conforming verifier implementation=] MUST produce an error.
	A [=conforming verifier implementation=] that makes use of a resource based on
	the `id` of an object inside a [=conforming document=] with a corresponding
	cryptographic digest MUST compute the digest of the retrieved resource. If the
	digest provided by the [=issuer=] does not match the digest computed for the
	retrieved resource, the [=conforming verifier implementation=] MUST produce
	an error.

[TallTed]

Revised based on conversation between @dlongley and @iherman above. Still wondering whether this should end with something like "MUST produce an error that SHOULD indicate type _____"?

Suggested change

	A [=conforming verifier implementation=] that makes use of a resource based on
	the `id` of an object inside a [=conforming document=] with a corresponding
	cryptographic digest MUST check the digest against the retrieved resource. If
	the digest of the retrieved resource does not match the one provided by the
	[=issuer=], the [=conforming verifier implementation=] MUST produce an error.
	A [=conforming verifier implementation=] that makes use of a resource based on
	the `id` of a `relatedResource` object inside a [=conforming document=] with a
	corresponding cryptographic digest appearing in a `relatedResource` object value
	MUST compute the digest of the retrieved resource. If the digest provided by the
	[=issuer=] does not match the digest computed for the retrieved resource, the
	[=conforming verifier implementation=] MUST produce an error.

[dlongley]

@iherman,

Hmm, this is in the related resource section. The id is in the relatedResource object, that's the object that is being referred to here, i.e., relatedResource.id, so I don't think your suggestion is correct and / or it is redundant in a strange way. IMO, if we need to, we can say "the id of a relatedResource object" instead of "the id of an object".

Note the sentence right before the above paragraph says:

Any objects for which selective disclosure or unlinkable disclosure is desired SHOULD NOT be included as an object in the relatedResource array.

This "an object" ^ is the same one referred to in the paragraph above.

[dlongley]

If necessary for clarity, here's an alternative suggestion to what Ivan suggests below:

Suggested change

	the `id` of an object inside a [=conforming document=] with a corresponding
	the `id` of a `relatedResource` object inside a [=conforming document=] with a corresponding

[iherman]

Yep, that works and is mathematically more precise.

👍

[longpd]

The changes make the clarity of the language much more explicit and I approve this PR

[iherman]

The issue was discussed in a meeting on 2024-10-09

• no resolutions were taken

View the transcript

2.1. Require digest verification for related resources. (pr vc-data-model#1567)

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

Manu Sporny: I took an action to raise a PR in VCDM 2.0 for requiring digest verification if it's provided, there has been some discussion on it, I wanted the group to see that this is out there, there are some suggested changes, I will process this and merge it by the end of the week, please get in there and provide commentary, hopefully the PR reflects consensus in the group. I plan to close the DI issue based on the merge of requiring digest verification if it's provideds provided.