Subject: [secdir] Secdir last call review of draft-ietf-oauth-access-token-jwt-11

From: Joseph Salowey via Datatracker <[email-masked]>
Date: Sun, 07 Feb 2021 10:48:19 -0800

Reviewer: Joseph Salowey
Review result: Has Issues

I have reviewed this document as part of the security directorate's
ongoing effort to review all IETF documents being processed by the
IESG.  These comments were written primarily for the benefit of the
security area directors.  Document editors and WG chairs should treat
these comments just like any other last call comments.

The summary of the review is the document has issues.

1.  (Editorial) What is the relationship between this document and RFC 7523. 
They are using JWT for different purposes, but I think it would be useful to
clarify this in the introduction.

2.  (Issue) The specification does not specify any mandatory to implement for
the recommended asymmetric algorithms.  This will not help interop.  Perhaps
specify one or both of  "RS256" and "ES256".

3. (Question) Is it currently possible to use the JWT access token in a mode
other than a bearer token?  For example is there a way to bind the JWT to a
verifiable key or identifier.  If there is, there should be some discussion of
this in the security considerations.  If not, do the authors know if there is
any work planned in this area?

4. Genart review pointed out a nit that should be fixed.

---

From: Vittorio Bertocci <[email-masked]>
Date: Sat, 20 Feb 2021 08:42:13 +0000

Thank you Joseph for your comments!

>    clarify this in the introduction.
Good point, I agree it would be good to preempt doubts on this. I am suggesting to append the following language to the current introduction (pardon the XML).

Please note: although both this document and <xref target="RFC7523"/> both use JSON Web Tokens in the context of the OAuth2 framework, the two specifications differ in both intent and mechanics. Whereas <xref target=" RFC7523"/> defines how a JWT Bearer Token can be used to request an access token, this documents describes how to encode access tokens in JWT format.

>    specify one or both of  "RS256" and "ES256".
The current text doesn’t mandate a format for two main reasons: 
- thanks to the AS metadata the negotiation between a RS and an AS is very easy to perform, hence the likelihood of interop issues at runtime is very low
- worries that as crypto advances, what we mandate at this point might no longer be suitable.
That said, if you feel strongly about this please do let me know, and I will be OK with adding RS256 as a mandatory supported alg for both AS and RS.
From a cursory check, ES256 doesn’t seem to enjoy as widespread support at the moment (see https://jwt.io/ libraries list) hence I would be hesitant to make it mandatory. 

>    any work planned in this area?
At this time, the main mechanisms that can work with non-bearer ATs are dPOP and MTLS. In both cases, the binding mechanism relies on a cnf claim (or reference to it) - which can be added to a JWT access token without changing any of the guidance in this specification. There's a quick primer for both approaches in https://auth0.com/blog/identity-unlocked-explained-episode-1/. Given that there are no changes in the AT format and verification, and MTLS/DPOP would be purely additive, any language here would likely be functionally equivalent to "You should be aware that MTLS exists, and the tokens defined here do not break it" (DPOP isn’t mentioned because it's still in progress) but my intuition is that the compatibility should be assumed, with a note being required when it's not the case and the reader should be aware of potential problems. What do you think?  

On 2/7/21, 10:48, "Joseph Salowey via Datatracker" <[email-masked]> wrote:

    Reviewer: Joseph Salowey

---

From: Joseph Salowey <[email-masked]>
Date: Wed, 24 Feb 2021 18:10:39 -0800

[Joe] Thanks for your response, comments inline below:

> Good point, I agree it would be good to preempt doubts on this. I am
> suggesting to append the following language to the current introduction
> (pardon the XML).
>
> Please note: although both this document and <xref target="RFC7523"/> both
> use JSON Web Tokens in the context of the OAuth2 framework, the two
> specifications differ in both intent and mechanics. Whereas <xref target="
> RFC7523"/> defines how a JWT Bearer Token can be used to request an access
> token, this documents describes how to encode access tokens in JWT format.
>
>
[Joe] Yes, that looks good to me.

> - thanks to the AS metadata the negotiation between a RS and an AS is very
> easy to perform, hence the likelihood of interop issues at runtime is very
> low
> That said, if you feel strongly about this please do let me know, and I
> will be OK with adding RS256 as a mandatory supported alg for both AS and
> RS.
> From a cursory check, ES256 doesn’t seem to enjoy as widespread support at
> the moment (see https://jwt.io/ libraries list) hence I would be hesitant
> to make it mandatory.
>
>
[Joe] I think you should add RS256 as mandatory to implement.  Since RFC
7518 does not mandate an asymmetric algorithm it's possible that two
implementations make different choices on which one to support.

> At this time, the main mechanisms that can work with non-bearer ATs are
> dPOP and MTLS. In both cases, the binding mechanism relies on a cnf claim
> (or reference to it) - which can be added to a JWT access token without
> changing any of the guidance in this specification. There's a quick primer
> for both approaches in
> https://auth0.com/blog/identity-unlocked-explained-episode-1/. Given that
> there are no changes in the AT format and verification, and MTLS/DPOP would
> be purely additive, any language here would likely be functionally
> equivalent to "You should be aware that MTLS exists, and the tokens defined
> here do not break it" (DPOP isn’t mentioned because it's still in progress)
> but my intuition is that the compatibility should be assumed, with a note
> being required when it's not the case and the reader should be aware of
> potential problems. What do you think?
>
>
[Joe] Thanks for the explanation. I don't think there needs to be anything
added to the document for this.