w3c / vc-di-eddsa

EdDSA Data Integrity Cryptosuites Specification
https://w3c.github.io/vc-di-eddsa/
Other
12 stars 10 forks source link

Is the hashing formulation inconsistent? #92

Closed iherman closed 2 months ago

iherman commented 3 months ago

§3.2.4 says, in bullet entry (3):

Let hashData be the result of joining proofConfigHash (the first hash) with transformedDocumentHash (the second hash).

However, proofConfigHash is defined in bullet item (2), and transformedDocumentHash is defined in bullet item (1). Ie, the item should either be:

Let hashData be the result of joining transformedDocumentHash (the first hash) with proofConfigHash (the second hash).

or

Let hashData be the result of joining proofConfigHash (the second hash) with transformedDocumentHash (the first hash).

Either way is fine, although the first possibility would make all current implementation invalid 😒.

Of course, a third solution would be to exchange bullet items (1) and (2).

(I did not check whether the same error occurs elsewhere in the same document as well as in other cryptosuites.)

dlongley commented 3 months ago

The meaning of "first" and "second", I believe, had to do with indicating the order that the concatenation or "joining" happens, not the order in which the hashes might have been created. I believe Ted? may have wanted those parentheticals to clarify the concatenation order? I can't recall now.

So if we need it to say "the first hash when concatenating" (or something like that), we can do that.

iherman commented 3 months ago

The meaning of "first" and "second", I believe, had to do with indicating the order that the concatenation or "joining" happens, not the order in which the hashes might have been created.

Which is fine with me, but it would be better to then order the bullet lists the same way to avoid any kind of misunderstandings (this is my third option in the issue)

I believe Ted? may have wanted those parentheticals to clarify the concatenation order? I can't recall now.

So if we need it to say "the first hash when concatenating" (or something like that), we can do that.

iherman commented 3 months ago

Note that §3.2.4 has a slightly different formulation which seems to be clear and without ambiguities (and, indeed, reflecting the order of the final hashing). For the sake of consistency, I think we should use the same formulation in this section as well.

msporny commented 3 months ago

I believe Ted? may have wanted those parentheticals to clarify the concatenation order? I can't recall now.

Yes, that is why the text is the way it is now.

I took a look at https://www.w3.org/TR/vc-di-ecdsa/#hashing-ecdsa-rdfc-2019 and the formulation is almost exactly the same as https://www.w3.org/TR/vc-di-eddsa/#hashing-eddsa-rdfc-2022. The only difference I can find, other than the obvious differences in key sizes in the ECDSA algorithm, is the word "Respective".

image

What am I missing? Did someone fix this in a previous PR?

TallTed commented 3 months ago

I don't think you're missing anything, @msporny.

The word "respective" is only relevant if there are two possible curves and hence hashing algorithms and hash sizes, which 2019 had but 2022 does not; hence, the lack of this word in 2022.

The reason for the "{first|second} hash" in 2019 was to ensure that proofConfigHash and transformedDocumentHash were "joined" (what does "join" mean? apparently, "concatenate") in the correct order to produce hashData. This concatenation doesn't happen in 2023; instead, the hashes are their own attributes (proofHash and mandatoryHash) on the hashData object.

msporny commented 3 months ago

Ok, thanks @TallTed, thought I was going crazy there for a second.

@iherman do you agree. If so, should we close this issue?

iherman commented 3 months ago

@iherman do you agree. If so, should we close this issue?

Sorry, this is irrelevant to my comment. I maintain that bullet entry (3) is still confusing. I understand what the intention is: the "first"/"second" refers to the way the hashes are concatenated. But what I am saying is that these two adjectives may be misunderstood by referring to transformedDocumenHash and proofConfigHash respectively. Which would yield the wrong order.

iherman commented 3 months ago

I raised #94, proposing a possible way to handle that.

msporny commented 3 months ago

@iherman wrote:

Sorry, this is irrelevant to my comment. I maintain that bullet entry (3) is still confusing. I understand what the intention is: the "first"/"second" refers to the way the hashes are concatenated. But what I am saying is that these two adjectives may be misunderstood by referring to transformedDocumenHash and proofConfigHash respectively. Which would yield the wrong order.

Oh! Ok, now I understand what you are saying, and agree that #94 would address that concern.

msporny commented 2 months ago

PR #94 has been merged, closing.