Review of verification algorithms

jyasskin commented 7 months ago

While reviewing https://github.com/w3c/vc-jose-cose/pull/190, I had to understand some of the SD-JWT verification algorithms. I noticed some issues, which I've listed here. I haven't filed individual issues yet because I'm not sure how you'll want to group them. Thanks to @OR13 for doing a first pass; any remaining mistakes are probably because I didn't take his advice.

5.1. Issuer-signed JWT Payload

[x] This should follow https://www.rfc-editor.org/rfc/rfc7515.html#section-7 by specifying "what serialization and serialization features are used". I see that 1.1. Feature Summary mentions the compact serialization, but that doesn't clearly say that this embedded JWT uses it.

5.2.1. Disclosures for Object Properties

[x] "The claim name, or key, as it would be used in a regular JWT payload. The value MUST be a string." <- I assume the "value" is the key, and not the key's value in the JSON object?
[x] Why is 128 bits only recommended and not required for the salt?

5.3. Key Binding JWT

[x] This should follow https://www.rfc-editor.org/rfc/rfc7515.html#section-7 by specifying "what serialization and serialization features are used". I see that 1.1. Feature Summary mentions the compact serialization, but that doesn't clearly say that this embedded JWT uses it.

5.3.1. Integrity Protection of the Presentation

[x] This mentions "bytes preceding the KB-JWT in the Presentation", but several other places mention "Key Binding is provided by means not defined in this specification", in which case the relevant bytes don't precede the KB-JWT. These should be consistent.

8.1. Verification of the SD-JWT

[x] Are "Upon receiving an SD-JWT, a Holder or a Verifier MUST ensure that" and "The Holder or the Verifier MUST perform the following (or equivalent) steps when receiving an SD-JWT" accomplishing the same thing?
[x] "Separate the SD-JWT" should cite an algorithm for doing that. I think it's "5. SD-JWT Data Formats" which implies it's just "split on '~'"?
[x] "Ensure that a signing algorithm was used that was deemed secure" <- This should probably be an input to the algorithm … although I think there are algorithm confusion attacks that imply that the algorithm should actually be picked based on the key the verifier has cached for the issuer rather than the claim in the JWT. I know this hasn't changed compared to JWS, but we know more about how JWS and other cryptography fails, so it'd be good to incorporate modern best practices in the interfaces here.
[x] "Validate the signature over the Issuer-signed JWT" should cite an algorithm for doing that. Is that https://www.rfc-editor.org/rfc/rfc7515.html#section-5.2?
[x] How should the application "Validate the Issuer and that the signing key belongs to this Issuer"? I see that the details of distribution are out of scope (Section 11.9), but this should say something about the application having a map from (issuer ID, key ID) pairs to public keys, and which fields of the Issuer-signed JWT are used to look up keys in this map.
[x] How should the application "Check that the _sd_alg claim value is understood"? Where should it find the _sd_alg, in the JWT's header or payload? 5.1.1. Hash Function Claim answers this, so you could just reference that, but it would be even better to accept a set of acceptable algorithms as an argument, look up the particular field in the payload object, and save that in a variable if it's in the input set.
[x] What's a "claim"? https://www.rfc-editor.org/rfc/rfc7519#section-4 says the claims are the members of the JWT's payload, but the algorithm here also uses "claim" to describe nested object members and array elements.
[x] I would probably have written the algorithm to build a map of hash->disclosure and then call a recursive element-processing-and-replacing function, but this isn't too bad.
[x] "Remove all array elements for which the digest was not found" probably won't confuse anyone for long, but my first reading said that an array in the payload like [1, 2, 3] would wind up empty since no digests were found for the elements.
[x] "Check that the SD-JWT is valid using claims such as nbf, iat, and exp" should say where those claims are defined and what part of which object they appear on. (https://www.iana.org/assignments/jwt/jwt.xhtml#claims, and the top level of the payload, not the header, right?)
[x] The "required validity-controlling claim"s should probably be an input parameter to this algorithm. Possibly all the IANA-registered JWT claim handling should be passed in, or this algorithm should return all the claims for the caller to deal with.

8.2. Processing by the Holder

[x] "create a Key Binding JWT" <- refer to the section that says how to do this.
[x] "Assemble the SD-JWT for Presentation" <- This should at least refer to "5. SD-JWT Data Formats" and possibly just write out the algorithm of "make an array of the Issuer-signed JWT, the selected Disclosures and, if applicable, the Key Binding JWT, and join the array with '~'."

8.3. Verification by the Verifier

[x] This is clearer that "Verifiers MUST ensure that" is accomplished by the steps in the following algorithm, but I don't like the redundant MUST. You can just say "Verifiers need to ensure that" and then leave the normative requirements to the algorithm.
[x] "according to the Verifier's policy" should be an input parameter to this algorithm, that the verifier provides along with the bytes of the presentation.
[x] "verify the Key Binding according to the method used" <- how? This might be https://www.rfc-editor.org/rfc/rfc9449.html#section-6, but you should cite the details.
[x] "Determine the public key for the Holder" <- how? This might be the 'cnf' claim, but you should say so.
[x] "Ensure that a signing algorithm was used that was deemed secure" <- as above, this should be an input parameter to this algorithm or computed from the public key.
[x] "Validate the signature" <- again, cite how to do this.
[x] "the typ of the Key Binding JWT" <- is that in the header or payload? (cite 5.3. Key Binding JWT and possibly https://www.rfc-editor.org/rfc/rfc8725.html#name-use-explicit-typing)
[x] "within an acceptable window." <- should be an input parameter
[x] "validating nonce and aud claims" <- The instructions for doing this should be an input parameter to this algorithm.
[x] "Check that the Key Binding JWT is valid in all other respects" should call a particular algorithm from the other specs that it cites.