Re: [OAUTH-WG] Initial JSON Web Token Best Current Practices Draft

[Yaron Sheffer <[e-mail]>]

Hi Neil,

Thank you again for your review and the follow up. Please see my 
comments in-line.

	‏Yaron

> 
> Hi Mike,
> 
> I sent this originally back in June last year, I can see some of these points have been addressed in -01, but not others, so I will include further comments in-line below. (Apologies if I missed replies - I?ve realised a few messages from this WG have ended up in my spam folder).
> 
> As a general point, the BCP is for JWT, but some of the advice (and my points below) apply more generally to other JOSE objects (as pointed out in the introduction). Should this be reflected in the title?
> 
> 
>> On 4 Jun 2017, at 15:11, Neil Madden <[e-mail]> wrote:
>>
>> I originally set this message just to the BCP authors. As requested by Mike Jones, I am sending it here too:
>>
>> Hi,
>>
>> I've just seen this draft best-practice guide for JWTs pop up. I have a number of suggestions for improvements. Mostly, I think the advice is good but should be spelt out a bit more clearly. Here are some suggestions:
>>
>> 1. Explicitly spell out the ECDH-ES public key validation routines from NIST. I have a blog post summarising them: https://neilmadden.wordpress.com/2017/05/17/so-how-do-you-validate-nist-ecdh-public-keys/
> 
> To be clear here, I think section 3.4 (Validate Cryptographic Inputs) should have text along the lines of the following added:
> 
> ?ECDH-ES ephemeral public key (epk) inputs should be validated according to the recipient?s chosen elliptic curve. For NIST prime-order curves P-256, P-384 and P-521, validation MUST be performed according to Section 5.6.2.3.4 ?ECC Partial Public-Key Validation Routine? of NIST Special Publication 800-56A revision 3 [1]. Where the X25519 or X448 curves of RFC 8037 [2] are used, then the implementation MAY reject any ephemeral public key that produces an all-zero ECDH shared secret as per RFC 7748 Section 6 [3]."
> 
> [1] https://nvlpubs.nist.gov/nistpubs/SpecialPublications/NIST.SP.800-56Ar3.pdf
> [2] https://tools.ietf.org/html/rfc8037
> [3] https://tools.ietf.org/html/rfc7748#section-6
> 
> I should note that the final sentence about validating X25519/X448 public keys is not without some controversy. See https://moderncrypto.org/mail-archive/curves/2017/000896.html for a rabbit-hole discussion together with associated links.
> 
> I do not feel confident to say what the right answer is for those curves - whether to validate at all, and if so whether to blacklist bad public keys or check for an all-zero shared secret. Perhaps we should get CFRG input on this point? Or should we take RFC 7748 as being the considered CFRG opinion on this topic (and so copy the ?MAY? wording)?

I think a MAY check is useless in practice, and also, the link you 
mention argues quite convincingly against the 25519 check. So I propose 
to adopt the first part of your text.

> 
>> 2. Recommend that (despite the -ES) ECDH is safest when *both* keys are ephemeral (eg you use some initial step to retrieve a fresh authenticated ephemeral key from the other party).
> 
> I think this comment is not necessary in hindsight. While fully-ephemeral ECDH has some advantages (as does static-static in some cases, e.g. NaCl crypto_box), it?s a more complicated discussion. For this BCP it is best to concentrate on ECDH-ES as specified and ensure that implementations perform proper validation of ephemeral public keys.
> 

Agree.

>> 3. Spell out how to authenticate ECDH ephemeral keys. For instance, include an inner signed JWT that repeats the epk and possibly the apu/apv claims too.
> 
> This is probably a more general point that none of the public-key encryption modes in JOSE are authenticated. If sender authentication is required then nested signed-and-encrypted objects should be used.
> 

Yes, and it's not up to the BCP to add new authenticated modes.

>> 4. Recommend that the apu and apv claims as a bare minimum include a hash of both public keys and SHOULD include any other known identifiers.

I'm not clear about "apu" in the general use case for JWT, where the 
identity of the recipient of a JWT is not known in advance to the sender.

Also, RFC 7518 uses "Alice" and "Bob" in these claims, and does not 
include any hashes. (And see my next comment).

> 
> Section 5.8.2 of NIST SP.800-56A rev 3 (referenced in point 1 above) gives some advice on the kinds of things that should be included here. Appendix B of the same document provides some rationale for why this should be done, as does RFC 7748 Section 7 (Security Considerations). RFC 7518 (JWA) Section 4.6.2 already normatively references NIST SP.800-56A, but only for ?applications wishing to conform to [NIST.800-56A]?. In my opinion, this should be strengthened in this BCP to a SHOULD/RECOMMENDED.
> 
>> 5. Recommend that the receiving party recalculates the apu and apv claims from known inputs to check they match. (Or leave these out of the JWT and require the other party to recalculate them).

But this would require normative language on the contents of these 
claims, which does not appear in existing RFCs. I would hesitate to 
include such language in a BCP. There must be a very good reason for a 
BCP to make existing implementations non-conformant.

>> 6. Provide explicit key lifetime requirements. E.g., for AES GCM this should not exceed 2^32 messages for randomly-generated IVs, and not exceed 2^64 *blocks* in total (so unless you restrict JWT lengths to less than 2^32 blocks and use a message counter as IV then this also limits to 2^32 messages). For CBC it should not exceed 2^48 messages.
> 
> I withdraw this comment, RFC 7518 Section 8.2 already addresses this point.
> 
>> 7. Require that the "alg" and "enc" claims are ONLY used to reject unexpected algorithms, NEVER to select an algorithm (even amongst a "supported" set). Cryptographic agility should be achieved by using "kid" claims that reference one of a set of known keys and the key should specify the algorithm (either explicitly or by the key parameters/size).
> 
> I think this is now adequately addressed by section 3.1.
> 
>> 8. Deprecate or strongly recommend against all of the RSA encryption modes except OAEP-256.

Yes. What about RSA with/without PSS?

>> 9. Strongly discourage any use of compression with encrypted JWEs - it is too easy to leak sensitive information.
>> 10. Recommend that if a JWE is used to encrypt a password or other value for which knowing the length may be a weakness, that such fields are explicitly padded by the application or at least use CBC mode to reduce the amount of length information leaked to a multiple of the AES block size.
> 
> These last two points are related: encryption hides everything apart from the length of a plaintext. Compression varies the length based on the content. The combination of the two weakens the security of the encryption, but the length may already be sensitive. Applications should consider what information might be leaked in these cases and make informed decisions about whether to pad or compress content before encryption.

Agree.

> 
>> 11. Require constant-time comparisons of HMAC tags.
> 
> This point is already covered by RFC 7518 Section 3.2.
> 
>> 12. Recommend using deterministic ECDSA signatures as described in RFC 6979 to minimise the risk of leaking the private key.

I'm not sure how this would interact with JWE. I suspect these 
signatures would have to be defined as a new algorithm (so out of scope 
for the BCP).

>> 13. Recommend that if the ECDH calculation produces an all-zero shared secret that this is rejected.
> 
> This is covered in my suggestion in point 1.
> 
>> 14. Never produce a signed JWT containing a "sub" claim unless you are explicitly vouching for the identity of that subject. It is far too easy to accidentally produce valid OIDC id tokens from unrelated code!
> 
> I think this is now covered by the section on explicit typing.
> 
> Explicit typing is a welcome addition. A complementary approach would be to recommend using different cryptographic keys for different types of JWTs. One way to achieve this for symmetric algorithms or ECDH would be to use a key-derivation function to derive unique keys based on some domain separation string.
> 
> I find the discussion in section 3.2 about the ?none? algorithm unnecessary for the BCP. I would say using ?none? should not be a recommended best practice.
> 
> A colleague recently shared this pen-tester JWT cheat-sheet with me: https://assets.pentesterlab.com/jwt_security_cheatsheet/jwt_security_cheatsheet.pdf which hints at a few other points worth mentioning (I can draft text for these if you wish):
> 
> - If the ?kid? header is used to perform a lookup in a database then there is a risk of SQL/LDAP injection if the value is not validated first or appropriate interfaces used (e.g. prepared statements).
> - The ?jwk? header should not be directly trusted, but only ever used to match a known key.

I agree about kid and jku, but I'm not familiar with all valid use cases 
of "jwk" so would appreciate more opinions regarding this statement.

  Likewise, I?d say the ?jku? header should really be matched against a 
known whitelist before being trusted, as it might additionally lead to 
SSRF attacks if the server can be tricked into loading arbitrary URLs.
> 
> Kind regards,
> 
> Neil
> 
> 
> ---