Comments from Dan Moore

[yaronf]

• In https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#name-weak-symmetric-keys perhaps you want to mention this open source project which cracks JWTs signed with a weak HMAC key: https://github.com/brendan-rius/c-jwt-cracker

• In https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#name-incorrect-use-and-compositi I'd change
  "However verifiers don't always check that the received JWT is a signed JWS as opposed to an encrypted JWE structure."
  to "However verifiers don't always check that the received JWT is a JWS (a signed JWT) as opposed to a JWE (a JWT with encrypted structure)."
  A JWS is by definition signed, just as a JWE is by definition encrypted, so I thought this was clearer.

• In https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-2.4 I'd change
  "Many encryption algorithms leak information about the length of the plaintext, with a varying amount of leakage depending on the algorithm and mode of operation."
  to "Many encryption algorithms leak information about the length of the plaintext, with a varying amount of leakage depending on the algorithm and mode of operation. JWEs are vulnerable to this leakage."
  Alternatively you could add other language to make it clear this attack is applicable to JWEs, not JWTs in general, just as is done in https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-2.12 . I suppose you could even change the section header to "JWE Plaintext Leakage through Analysis of Ciphertext Length". In general there are a few security issues that are related to encryption and therefore only affect JWEs.

• In https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-2.5
  The Sanso link is busted (expired cert, 404 for https://blogs.adobe.com/security/2017/03/critical-vulnerability-uncovered-in-json-encryption.html ) so you might want to review.

• Is the attack outlined here https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-2.5 the same as
  https://neilmadden.blog/2022/04/19/psychic-signatures-in-java/ ? If so, maybe that is a better link?

• In https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-2.8 I'd change
  "As JWTs are being used by more different protocols in diverse application areas, it becomes increasingly important to prevent cases of JWT tokens that have been issued for one purpose being subverted and used for another."
  To "As JWTs are used by more protocols in diverse ways, it becomes increasingly important to prevent JWT tokens that have been issued for one purpose being used for another."
  Just think it reads cleaner.

• In https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-3.1-2 I'd change
  "In particular, use allowlists for critical parameters such as "alg" instead of blocklists."
  "In particular, libraries SHOULD use allowlists for critical parameters such as "alg" instead of blocklists."
  This makes it a complete sentence.

• In https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-3.2 you quote the RFC which mentions SHOULD but then you follow it up with MUST. I find that confusing. I'd remove the first paragraph or at least this sentence: "Even if a JWS can be successfully validated, unless the algorithm(s) used in the JWS are acceptable to the application, it SHOULD consider the JWS to be invalid." because then you remove the SHOULD/MUST confusion I had.

• In that same section, the semi-colon was awkward. I'd change
  "JWTs using "none" are often used in application contexts in which the content is optionally signed; then, the URL-safe claims representation and processing can be the same in both the signed and unsigned cases."
  to
  "JWTs using "none" are often used in application contexts in which the content is optionally signed. The URL-safe claims representation and processing in this context can be the same in both the signed and unsigned cases."

• In https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-3.3 I'd change
  "Libraries MUST allow the verifier to distinguish between JWS-signed and JWE-encrypted JWTs. This would allow verifiers to easily establish a policy of only accepting JWS-signed JWTs."
  to
  "Libraries MUST allow the verifier to distinguish between signed JWTs (JWSes) and encrypted JWTs (JWEs). This allows verifiers to easily establish a policy of only accepting signed JWTs."
  I found the terms JWS-signed and JWT-encrypted confusing. They felt internally redundant (a JWS is always signed, for example).

• In https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-3.6 I wondered if it made sense to mention JWEs here? Maybe change
  "Compression of data SHOULD NOT be done before encryption, because such compressed data often reveals information about the plaintext."
  to
  "Compression of data SHOULD NOT be used when creating a JWE, because such compressed data often reveals information about the plaintext."
  This distinguishes between JWS and JWE, where the latter is the only one this recommendation applies to.

• In https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-3.7 I'd change
  "Implementations and applications MUST do this and not use or admit the use of other Unicode encodings for these purposes."
  to
  "Implementations and applications MUST do this and not use or allow the use of other Unicode encodings for these purposes."
  Unless "admit" means something special in this context, "allow" is clearer. "allow" seems more common and "admit" is not used elsewhere in this doc.

• In https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-3.9 I worry about changes to the implementation (where you start with one RP/app, then add more). I'd address this by adding a clause about the future:
  "If the same issuer can issue JWTs that are intended for use by more than one relying party or application"
  to
  "If the same issuer can issue JWTs that are intended for use by more than one relying party or application, or may in the future,"

• I'd also change
  "or was substituted by an attacker at an unintended party."
  to
  "or was substituted by an attacker."
  I'm not sure what the "unintended party" phrase adds.

• There's also this sentence:
  "In such cases, the relying party or application MUST validate the audience value, and if the audience value is not present or not associated with the recipient, it MUST reject the JWT."
  The aud claim can be an array https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.3 . Should we mention that here? Maybe something like:
  "In such cases, the relying party or application MUST validate the audience value, and if no audience value is present or none of the values are associated with the recipient, it MUST reject the JWT."

• I found https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-3.11-2 confusing. Here's the text:
  "Per the definition of "typ" in Section 4.1.9 of [RFC7515], it is RECOMMENDED that the "application/" prefix be omitted from the "typ" value. Therefore, for example, the "typ" value used to explicitly include a type for a SET SHOULD be "secevent+jwt". When explicit typing is employed for a JWT, it is RECOMMENDED that a media type name of the format "application/example+jwt" be used, where "example" is replaced by the identifier for the specific kind of JWT."
  As a JWT creator, should I be including "application/" in the typ value? I see recommendations for and against that, unless the media type name refers to something other than the value of the typ header. What am I missing?

• In https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-3.12 I missed the MUST here, as I jumped right to the list. I'd suggest a small formatting change to highlight the MUST. Can we do a newline to make it clearer? That would change:
  "If more than one kind of JWT can be issued by the same issuer, the validation rules for those JWTs MUST be written such that they are mutually exclusive, rejecting JWTs of the wrong kind. To prevent substitution of JWTs from one context into another, application developers may employ a number of strategies:"
  to
  "If more than one kind of JWT can be issued by the same issuer, the validation rules for those JWTs MUST be written such that they are mutually exclusive, rejecting JWTs of the wrong kind. To prevent substitution of JWTs from one context into another, application developers may employ a number of strategies:"

• In https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-3.13
  that 600k OWASP iteration recommendation value changes over time. Do you want this text to be more broadly applicable in the future? (Not sure how this kind of thing is handled in general.) If so, could change this from:
  ""[OWASP-Password-Storage] states that an iteration count of 600,000 is required when using HMAC-SHA-256 to achieve FIPS-140 compliance. Thus, rejecting inputs with a p2c (PBES2 Count) value over 1,200,000 (double that) is RECOMMENDED."
  to
  "[OWASP-Password-Storage] states a specific iteration count (600,000 at time of publishing) is required when using HMAC-SHA-256 to achieve FIPS-140 compliance. Rejecting inputs with a p2c (PBES2 Count) value larger than double the recommended OWASP value is RECOMMENDED."

• In https://www.ietf.org/archive/id/draft-sheffer-oauth-rfc8725bis-01.html#section-3.14 it is unclear to me if this if this is for the JWT as encoded or the decoded value? I think encoded, but am not sure. Do you want to reference base64 url encoding here? as mentioned in 7515 and by https://datatracker.ietf.org/doc/html/rfc4648#section-5

cc: @mooreds