decentralized-identity / JWS-Test-Suite

JsonWebSignature2020 Test Suite
https://identity.foundation/JWS-Test-Suite
Apache License 2.0
9 stars 11 forks source link

Danubetech impl has `challenge` in proof values #43

Closed decentralgabe closed 2 years ago

decentralgabe commented 2 years ago

A few examples:

peacekeeper commented 2 years ago

Yes, and..?

decentralgabe commented 2 years ago

Yes, and..?

https://github.com/decentralized-identity/JWS-Test-Suite/issues/42#issuecomment-1050912490

@OR13

peacekeeper commented 2 years ago

Well it would be very easy for me to remove one line of code, but I don't really understand the desire to make everyone produce the exact same output.

Why do we care about different "created" timestamps or about the ordering of the JWS header properties? I don't think the test suite should enforce that. It should only check if the result is correct, not if all results are byte-for-byte equal.

The argument that there should be no "challenge" if proof purpose is "assertionMethod" makes sense to me, but I'm not aware of any spec text or test suite documentation where that requirement is actually written down.

So strictly speaking I don't think there is sufficient ground for declaring my test result "wrong". Also, this issue here has zero explanation of why it was opened or what it's trying to accomplish.

But of course I could just make the simple change instead of starting a complicated disussion :)

OR13 commented 2 years ago

@msporny ^ do you think that challenge should be allowed in assertionMethod proofs?

I guess since nonce is allowed everywhere in JOSE, @peacekeeper has a good point.

msporny commented 2 years ago

@msporny ^ do you think that challenge should be allowed in assertionMethod proofs?

If we're talking about the Data Integrity spec, you can add any arbitrary value (as long as it's defined in a JSON-LD context somewhere) to any proof and that value will be digitally signed over.

The question really is: "Is there a use case for embedding a challenge in assertionMethod proofs?" (man, I so wish we had just done assertion instead for the term name, but that ship sailed long ago) -- and I do think there is, but a fairly weak one:

You know when people hold up a sign on reddit during an Ask Me Anything (AMA)?

https://www.google.com/search?q=reddit+ama&source=lnms&tbm=isch&sa=X&cshid=1646057214423639&biw=1920&bih=975&dpr=1

Well, that's sort of a self-generated "challenge" (really, it's a "nonce") being included as an assertion (the picture they're holding up)... it's a bit of a stretch, but it is an existence proof of some kind.

The reason no spec says anything about this yet is because, I believe, the jury is still out on whether or not including challenge in an assertionMethod proof is a good idea. It's perfectly fine for specs to stay silent on things that are not clearly good and not clearly bad yet.

Do I personally think challenge should be allowed in assertionMethod proofs? Yeah, allow it for now until it's very clear that we don't want to... I've never heard a very strong/compelling use case for it, though.

msporny commented 2 years ago

@peacekeeper wrote:

Why do we care about different "created" timestamps or about the ordering of the JWS header properties? I don't think the test suite should enforce that. It should only check if the result is correct, not if all results are byte-for-byte equal.

+1 to this, FWIW.

A topic that's up for debate in the Data Integrity work in the upcoming VC2WG is: Should a verifier throw an error if it doesn't understand every single property in a proof block? I'm leaning towards "yes" these days... for the same reasons a Data Integrity library signing JSON-LD should throw and error if one of the terms isn't mapped in the active JSON-LD Context. That rule clearly wouldn't apply to a Data Integrity library signing JSON-only (e.g., JCS + JWS) because, you know, anything goes in JSON.

decentralgabe commented 2 years ago

Why do we care about different "created" timestamps or about the ordering of the JWS header properties? I don't think the test suite should enforce that. It should only check if the result is correct, not if all results are byte-for-byte equal.

Simply, it's a way to validate implementations. Same input = same output. I take your point that signature validation is what's most important, and I agree.

msporny commented 2 years ago

Simply, it's a way to validate implementations. Same input = same output.

I get the desire for simplicity. My concern here is that there is no spec text that says that "JWT Header fields need to be in a specific order." It's usually a bad idea to have test suites for standards that enforce requirements that are not written down anywhere -- and this is one of those situations. I expect implementers to object to "unwritten rules", so either you need to change the VC-JWT specification to say something it doesn't say now, which is "Header fields MUST be canonicalized according to JCS" (or something equivalent), or you need to allow arbitrary fields, with arbitrary semantics, and arbitrary ordering in JWT Headers (which is, sadly, what JWT allows for today).

decentralgabe commented 2 years ago

Thanks for the insight, @msporny. A potential middle ground may be "JWT Headers do not need to be in a specific order, and proofs may allow for additional fields as per their respective specifications. However, for the sake of implementers being able to have deterministic inputs and outputs here are some known values you can test against".

That said, just a nice to have. Signature validation is what matters, and what this project accomplishes. Closing.