Re: [OAUTH-WG] a token review of draft-ietf-oauth-access-token-jwt-01/-02

[Aaron Parecki <[e-mail]>]

Some notes and minor corrections:

> Abstract

"OAuth2" should be "OAuth 2.0"

> 1. Introduction

"many important scenario" should be "scenarios"
"OAuth2" should be "OAuth 2.0"

> 2.1. Header
> ... use of asymmetric algorithms is RECOMMENDED

Has anyone implemented or does anyone have plans to implement
symmetric algorithms here? Fewer options is generally better and will
lead to better library support and interoperability, so I'd love to
see this require asymmetric algorithms if possible.

> 2.2.1. Identity Claims
> Commercial authorization servers will...

"Commercial" seems like an odd distinction here, since that doesn't
really describe anything about the architecture, and there could be
cases of non-commercial authorization servers with this same feature.
How about "External or standalone authorization servers..."?

> 3. Requesting a JWT Access Token
> GET /as/authorization.oauth2?response_type=token

should be response_type=code because the description below then says
"Once redeemed, the code obtained from the request above..."

> 4. Validating JWT Access Tokens

There are several instances of "Openid connect" which should be
"OpenID Connect".

"OAuth2 bearer token usage" should be "OAuth 2.0 Bearer Token Usage"
and cite RFC6750.

> If the JWT access token is encrypted...

This is the first mention of the possibility of an encrypted JWT. I
was under the impression this spec is limited to unencrypted JWTs, not
also encompassing rfc7516 JWE. If the intent is to also allow JWEs,
this should be specified much higher up in the document as well.

"...JWT access token according..." should be "access tokens"

"represented by the exp Claim" no need to capitalize "claim"

> 6. Privacy Considerations
> the authorization server should take explicit steps to prevent it as described above.

What steps exactly are described above? The only thing I see is using
opaque tokens. Is that the intent? If so, probably better to just say
"should use opaque tokens" instead. Or this could refer to the steps
described later in this section, in which case should say "below".

> Possible measure include
should be "measures"

This may also read better as a bullet list, but not sure if you want
to take up that much space.

----
Aaron Parecki
aaronparecki.com
@aaronpk


On Wed, Jul 24, 2019 at 5:46 PM Vittorio Bertocci
<Vittorio=[e-mail]> wrote:
>
> Thank you Brian for the thorough and insightful review!
>
> Comments:
>
> > On authenticated encryption.
> I did chat with Neil about his draft, but as you mention I didn't reference it given that it hasn't bee picked up (yet?).
> On referencing JWE RFC7516 and more JWA RFC7518, I am reluctant. My rationale is that this profile attempts to capture current practices or practices that are close enough to the capabilities of existing systems to have a reasonable chance of being adopted. None of the products/services surveyed used authenticated encryption, and the requirement to acquire a symmetric key out of band throws  a big wrench in the otherwise clear-cut process of preparing your RS to handle JWT ATs. I will bring the matter up as open issue on Friday, and if the consensus will be to include symmetric authenticated encryption I'll do that; but my personal preference would be to preserve the simplicity of a concrete, nothing-of-substance-left-as-exercise-to-the-reader solution that can be achieved when the key material can be advertised via metadata.
> Typo fixed.
>
>
> >Connect doesn't say anything about checking the "typ" header of id tokens [..] this document probably shouldn't overstate what typing the JWT can accomplish.  [..]  'nonce' in ID Tokens delivered via the front channel [..] to be interpreted as id_tokens.
>
> This is a very fair point, the current language is misleading. I rephrased to position the header typing as a hint.
> About the nonce mitigation you mention. Do you think this means that I should explicitly forbid the presence of a "nonce" claim in JWT ATs?
>
>
> >I think you want to say here that the scope claim in the token has to correlate to the scope which was approved. Not necessarily what what requested. The authorization request might ask for scope of a+b+c, for example, while the user only approves b. Or any other variation on things where what was asked for isn't what was gotten..
>
> Here I am trying not to get into the details of what's inside the scope claim, more requesting that if "scope" was in the request, the issued token should express a delegation and a symptom of that should be the presence of the scope claim. Paradoxically that scope claim might be empty if the user only consented to scopes that have effect on the presence of other claims in the token (say a functional equivalent of profile). Yes, it does sound odd, but that's a side effect of the prohibition of sending id_tokens to API that creates the requirement to create functional equivalents.
>
>
> > https://tools.ietf.org/html/draft-ietf-oauth-resource-indicators has an "invalid_target" error code that is intended for this kind of thing. So that should probably be allowed. If not suggested. Probably not required though.
>
> Wonderful, that is a better fit- love it. I added it as possible alternative to "invalid_request".
>
>
> >I personally think the SHOULD is too strong here because it puts the onus on the resource server to know about (via some config option or something) and enforce on every transaction a setup/policy thing that the AS is responsible for which isn't about the integrity of the authorization data in the token. A MAY or even removing the list item would be preferred.
>
> I borrowed this language straight from the OpenID Connect validation steps for idtokens. Given that JWT ATs can carry identity info, the requirement seemed appropriate... also, I think it is reasonable to expect the resource server to know about its own registration- just like it must know about the expected aud value and what key should be used to decrypt messages, it should know whether encryption was requested or not. In fact, the fact that such option was selected at registration time is likely the reflection of a policy of the RS itself, something the RS itself would want to ensure has been respected. WDYT?
>
>
> >multi value audiences
>
> I see the issue. I definitely don't want to redefine the semantic of aud, but I also would like to be as crisp as possible on the audience-scope correlation and prevent scopes from being misinterpreted as applying to the wrong resource. The aud validation will likely happen in some middleware in front of the API, but authorization checks might happen in the body of the API itself when the actual access is being attempted, and having an OR list in the aud claim might lead to false positives. RSes correctly written should not suffer from this issue (the authorization logic should receive only the value from the aud collection that is an actual match) but I have seen enough sloppy implementations to be skeptical about this.
> As a result, I would be inclined to  take out the sentence "or if it contains additional audiences that are not known aliases of the resource indicator of the current resource server", effectively restricting to single value aud and eliminating this issue.
>
>
> >I think it'd be worthwhile to explicitly disallow the use of the "none" algorithm here.
>
> Good point. I am all for doing everything we can to defuse that FUD. I added language that explicitly disallow RS to accept JWTs with "alg":"none"
>
>
> >Little 'o' in TOken -> Token
> fixed
>
>
> > A link directly to SCIM alone from the JSON Web Token Claims registry might be rather confusing.
>
> Added a reference as you suggested in all the entries. I am always confused by forward references whenever the syntax requires the use of an RFC number that isn't available, but the [[]] trick is a good one.
> I didn't follow the "subcompact" considerations, I'll bug you offline for clarifications. Thx in advance
>
>
> >Ping<blank>identity
>
> Fixed! Sorry for not having done in 01, I think you mentioned this already.
>
> On Wed, Jul 24, 2019 at 12:59 PM Brian Campbell <bcampbell=[e-mail]> wrote:
>>
>>
>> 2.1.  Header
>>>
>>>    NOTE: there were discussions about adding a reference to
>>>    authenticated encyption methods as well, but there's no internet
>>>    draft specifying interoperable public key methods at this time
>>
>> Well, Neil did write this up a while back https://tools.ietf.org/html/draft-madden-jose-ecdh-1pu-01 which is certainly interesting and I wonder if it's something that this group (or some other group?) should look at working on. But it is only an individual draft (with all the authority that brings to bear) and thus I do agree that it isn't appropriate or useful for this document to reference.
>>
>> However JWE RFC7516 and more JWA RFC7518 do have authenticated encryption with symmetric keys that, IMHO, can work very nicely in some cases for JWT access tokens by providing both integrity protection and confidentiality. And the size and performance benefits thereof can be sufficiently useful so as to justify the need to do an out-of-band exchange of the symmetric key.
>>
>> Also encyption should be encryption.
>>
>>
>> 2.1.  Header
>>>
>>>    The typ header parameter for a JWT access token MUST be at+jwt.  See
>>>    the security considerations section for details on the importance of
>>>    preventing JWT access tokens to be interpreted as id_tokens.
>>
>> 5.  Security Considerations
>>>
>>>    The JWT access token data layout described here is very similar to
>>>    the one of the id_token as defined by [OpenID.Core].  Without the
>>>    explicit typing required in this profile, in line with the
>>>    recommendations in [JWT..BestPractices] there would be the risk of
>>>    attackers using JWT access tokens in lieu of id_tokens.
>>
>> Connect doesn't say anything about checking the "typ" header of id tokens so typing ATs with "at+jwt" doesn't actually do anything to prevent JWT access tokens being used as id tokens. It can prevent misuse in the other direction. But this document probably shouldn't overstate what typing the JWT can accomplish.
>>
>> BTW, there's been some discussion and agreement that the requirement around 'nonce' in ID Tokens delivered via the front channel (the only time connect is subject to that kind of token swapping) is sufficient to protect against JWT access tokens (and other types of JWTs) to be interpreted as id_tokens. So I think the risk is acceptable but just think the text in the draft shouldn't make claims which are untrue.
>>
>>
>> 2.2.2.  Authorization Claims:
>>>
>>>    If an authorization request includes a scope parameter, the
>>>    corresponding issued JWT access token
>>>
>>>   MUST in -01/SHOULD in -02 include a scope claim
>>>
>>> as defined in
>>
>> I think you want to say here that the scope claim in the token has to correlate to the scope which was approved. Not necessarily what what requested. The authorization request might ask for scope of a+b+c, for example, while the user only approves b. Or any other variation on things where what was asked for isn't what was gotten..
>>
>>
>> 3.  Requesting a JWT Access Token
>>>
>>>    If it receives a request for an access token containing more than one
>>>    resource parameter, an authorization server issuing JWT access tokens
>>>    MUST reject the request and fail with invalid_request as described in
>>>    section 4.1.2.1 of [RFC6749].
>>
>> https://tools.ietf.org/html/draft-ietf-oauth-resource-indicators has an "invalid_target" error code that is intended for this kind of thing. So that should probably be allowed. If not suggested. Probably not required though.
>>
>> 4.  Validating JWT Access Tokens
>>>
>>>       If encryption was negotiated with the
>>>        authorization server at registration time and the incoming JWT
>>>        access token is not encrypted, the resource server SHOULD reject
>>>        it.
>>
>> I personally think the SHOULD is too strong here because it puts the onus on the resource server to know about (via some config option or something) and enforce on every transaction a setup/policy thing that the AS is responsible for which isn't about the integrity of the authorization data in the token. A MAY or even removing the list item would be preferred.
>>
>> 4.  Validating JWT Access Tokens
>>>
>>>      The JWT access token MUST be
>>>        rejected if aud does not list the resource indicator of the
>>>        current resource server as a valid audience, or if it contains
>>>        additional audiences that are not known aliases of the resource
>>>        indicator of the current resource server.
>>
>> 5.  Security Considerations
>>>
>>>    This profile explicitly forbids the use of multi value aud claim when
>>>    the individual values refer to different resources, as that would
>>>    introduce confusion about what scopes apply to which resource-
>>>    possibly opening up avenues for elevation of delegated privileges
>>>    attacks.  Alternative techniques to prevent scope confusion include
>>>    "scope stuffing", imposing to every individual scope string to
>>>    include a reference to the resource they are meant to be applied to,
>>>    but its application is problematic (scope opacity violations, size
>>>    inflation, more error conditions become possible when the combination
>>>    of requested scopes and resource indicators is invalid) and the
>>>    observed frequency of the scenario doesn't warrant complicating the
>>>    more common cases.
>>
>> I do think I see the simplification you're aiming at with this stuff but I'd like to offer the perspective of how this is likely more complicated for standards based JWT library implementations. The semantics of aud in RFC 7519 basically say that the recipient has to identify with at least one of the aud values. It's effectively a big OR. While the text in this draft changes that semantic to say that the recipient has to identify with at every one of the aud values. Effectively making it a big AND. Normal JWT libraries will need nonstandard one-off functionality or adaptations to get this right.
>>
>>
>>  4.  Validating JWT Access Tokens
>>>
>>>       The resource server MUST validate the signature of all incoming
>>>        JWT access token according to [RFC7515] using the algorithm
>>>        specified in the JWT alg Header Parameter.  The resource server
>>>        MUST use the keys provided by the authorization server.
>>
>> I think it'd be worthwhile to explicitly disallow the use of the "none" algorithm here. I think/suspect that it is implied already. But at least some of the JWT hate out there seems to stem from or focus on statements like this one and conclude that words like "MUST validate ... according to [the] algorithm specified in the JWT alg Header Parameter" means that to be spec compliant one has to accept "alg":"none" as valid. That's more of a logical leap than I think is reasonable but I'd like to avoid it anyway if possible. And it's not a bad thing to remind folks not to accept "none" here.
>>
>> 7.2.  Claims Registration
>>>
>>>     claims in the JSON Web TOken (JWT) IANA
>>
>> Little 'o' in TOken -> Token
>>
>>
>> 7.2.1.  Registry Contents
>> |   Specification Document(s): section 4.1.2 of [RFC7643]
>> Ultimately this is what will show up in the "References" column in the registry https://www.iana.org/assignments/jwt/jwt.xhtml and thus I'd suggest that it also include something like "Section 2.2.2.1 of [[this specification]]" there too so someone who starts from the registry has the link to and context of this document. A link directly to SCIM alone from the JSON Web Token Claims registry might be rather confusing.
>> For the folks that will have to review and act on these registrations at some point, it might be nice to give em a little better grouping/formatting. I'm thinking along the lines of what is done here https://tools.ietf.org/html/draft-ietf-oauth-token-exchange-18#section-7.4.1, which you can corice xml2rfc into doing with <?rfc subcompact="yes"?> and <?rfc subcompact="no"?> as in the src at https://github.com/oauth-token-exchange/spec/blob/master/draft-ietf-oauth-token-exchange.xml#L1191
>>
>> Appendix A.  Acknowledgements
>>>
>>>     Brian Campbell (PingIdentity)
>>
>> My corporate overloads will be happier if you put a space between Ping and Identity.
>>
>>
>> CONFIDENTIALITY NOTICE: This email may contain confidential and privileged material for the sole use of the intended recipient(s). Any review, use, distribution or disclosure by others is strictly prohibited...  If you have received this communication in error, please notify the sender immediately by e-mail and delete the message and any file attachments from your computer. Thank you._______________________________________________
>> OAuth mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/oauth
>
> _______________________________________________
> OAuth mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/oauth