Subject: Re: [OAUTH-WG] AD Review of draft-ietf-oauth-security-topics-24

From: Daniel Fett <[email-masked]>
Date: Thu, 28 Dec 2023 14:35:23 +0100

Hi Roman,

thanks again for your feedback!

I have a few open questions below, but already incorporated most of your 
(and Hannes) feedback in the editor's copy:

- 
https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.html

- Diff to published version: 
https://author-tools.ietf.org/api/iddiff?doc_1=draft-ietf-oauth-security-topics&url_2=https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.txt 
<https://author-tools.ietf.org/api/iddiff?doc_1=draft-ietf-oauth-security-topics&url_2=https://oauthstuff.github.io/draft-ietf-oauth-security-topics/draft-ietf-oauth-security-topics.txt>

I plan to publish the next version once we have resolved the discussion 
points below.

Also looking forward to feedback from the list and the co-editors!

-Daniel

Am 19.12.23 um 00:08 schrieb Roman Danyliw:
> ** Section 2.1
>     When an OAuth client can interact with more than one authorization
>     server, a defense against mix-up attacks (see Section 4.4) is
>     REQUIRED.  To this end, clients SHOULD
>           …
>
>     In the absence of these options, clients MAY instead use ...
>
> The text is clear on saying some defense from a mix-up attack is needed.  What happens if the client cannot use the methods prescribed by the SHOULD and MAY in this text?
>
> ** Section 2.1.1
>     Clients MUST prevent authorization code injection attacks (see
>     Section 4.5) and misuse of authorization codes using one of the
>     following options:
>
> What approach should a confidential client take of PKCE is not used?  It is only recommended in the subsequent bulleted list.

In both of these cases, the most important point is that protections are 
applied (REQUIRED/MUST). We further want to steer implementers towards a 
specific solution.

In the first case, we give options but intentionally don't limit 
implementers, as other solutions may exist. (Custom solutions should not 
bring any interoperability issues here, as it is mostly internal to the 
client. At the same time, we don't want to describe any of the other 
solutions, as we provide robust solutions already.)

In the second case, we list all the options, but give guidance on which 
to use, especially for confidential clients. We can't say that they MUST 
use PKCE, as we want to permit the OpenID nonce approach. If the 
implementer don't implement PKCE ("RECOMMENDED") or OpenID nonce ("MAY") 
they are in violation of the MUST in the sentence before the list.

We (as the working group) spent quite some time refining the wording in 
these paragraphs, in particular in the second case. I think they are 
correct and I have a hard time coming up with a clearer way to express 
the same thing, so I suggest to not change anything.

>
> ** Section 2.2.
>
>     The privileges associated with an access token SHOULD be restricted
>     to the minimum required for the particular application or use case.
>
> Under what circumstances should access tokens not be restricted?  Can this be documented?

Due to the variety in architectures, I guess this would mostly boil down 
to performance reasons and architectural limitations (resource server or 
exact privileges are not known at the time of authorization).

I suggest that we add a sentence to discuss this to Section 4.10 
<https://www.ietf.org/archive/id/draft-ietf-oauth-security-topics-24.html#section-4.10>.

>
> ** Section 2.3
>     In particular, access tokens SHOULD be restricted to certain resource
>     servers (audience restriction), preferably to a single resource
>     server.
>
> Is there anyway to refine this text to make the interplay of the SHOULD and “preferably” clearer?

I propose to modify this as follows:

"In particular, access tokens SHOULD be audience-restricted to a 
specific resource
server, or, if that is not feasible, to a small set of resource servers."

>
> ** Section 2.4
>    and authentication processes that require multiple
>     steps can be hard or impossible.
>
> Is this difficulty due to complexity?  It would be helpful to refine why it is “hard”.

I propose the following text:

"Furthermore, the resource owner password credentials grant is not 
designed to
work with two-factor authentication and authentication processes that 
require
multiple user interaction steps. Authentication with cryptographic 
credentials
(cf. WebCrypto [@W3C.WebCrypto], WebAuthn [@W3C.WebAuthn]) may be impossible
to implement with this grant type, as it is usually bound to a specific 
web origin."

> ** Section 2.6.
>     It is RECOMMENDED to use end-to-end TLS between the client and the
>     resource server.  If TLS traffic needs to be terminated at an
>     intermediary, refer to Section 4.13 for further security advice.
>
> Is there further TLS guidance to provide?  Could BCP 195 be used (RFC9325)?

I would be fine with adding "according to BCP 195" in the first 
sentence. What do you think?

> ** Section 3.
> Implementers MUST take into account all possible
>     types of attackers in the environment in which their OAuth
>     implementations are expected to run.  Previous attacks on OAuth have
>     shown that OAuth deployments SHOULD in particular consider the
>     following, stronger attackers in addition to those listed above:
>
> The mix of MUST in the first sentence and SHOULD in the second is confusing.  The first sentence requires taking all possible attacks into consideration.  However, the second sentence then uses the weaker SHOULD for specific attacks that seem in-scope of the first sentence.

That's right, A3 to A5 are listed as something that implementers 
probably want to consider under the MUST in the first sentence. 
(Depending on the deployment, they may also be irrelevant, but that's 
unlikely.) I'll remove the normative SHOULD from the second sentence and 
highlight that A3 to A5 are listed because they are particularly relevant.

>
> ** Section 4.1.1.  Editorial.  These examples are very helpful.  For the inline URLs, consider putting them in quotes to improve readability.
The URLs will be in quotes in the HTML version, please see my response 
in this thread for details: 
https://mailarchive.ietf.org/arch/msg/oauth/G1c8kp4kwu8tY_TZItMdXK6x_EE/
>
> ** Section 4.8.1
>        the client has set the parameter
>         code_challenge=sha256(abc)
>
> -- Isn’t it base64(sha265(abc))?
> -- Recommend citing that this is S256 per Section 4.2 of RFC7636 otherwise IETF LC or IESG review feedback will likely ask for a SHA256 citation.

Since the details of the hash and the encoding would distract from the 
point of the attack, I modified this as follows:

"...`code_challenge=hash(abc)` as the PKCE code challenge (with the hash 
function and parameter encoding as defined in [@!RFC7636])."

>
> ** Section 4.8.2.  Editorial?
>     Therefore, ASs MUST take precautions against this threat.
>
> Is that the same things as saying this attack MUST be mitigated in some way?

Yes, changed to

"Therefore, authorization servers MUST mitigate this attack."

Does this work for you?

>
> ** Section 4.9.2.
>     If the attacker were able to gain full control, including shell
>     access, all controls can be circumvented and all resources can be
>     accessed.
>
> What is the link to “shell access”?

That is a bit too specific, so I removed it and merged the two 
paragraphs into one:

"An attacker may compromise a resource server to gain access to the
resources of the respective deployment. Such a compromise may range
from partial access to the system, e.g., its log files, to full
control over the respective server, in which case all controls can be
circumvented and all resources can be
accessed. The attacker would also be able to obtain other access
tokens held on the compromised system that would potentially be valid
to access other resource servers."

> ** Section 4.14.2
>     Authorization servers SHOULD determine, based on a risk assessment,
>     whether to issue refresh tokens to a certain client.
>
> How can the decision to issue refresh tokens selectively be ambiguous?  Either they are issued or not.  Why isn’t this a MUST?

That's a good point and I think we should change this to a MUST. In 
particular, accepting the risk and issuing refresh tokens to all clients 
may be a potential outcome of the risk assessment. Does that sound okay 
to you?

>
> ** Section 4.18.2.  Formally cite that these are Javascript (?) examples.

I added that these are JavaScript examples. Do you think we need a 
formal reference to ECMAScript? I don't think this would be very helpful.

Thank you,
Daniel

-- 
Please use my new email address:[email-masked]