Hi!
I performed an AD review of draft-ietf-oauth-security-topics-24. Thank you for
taking the time to document many years of operational deployment experience.
My feedback is below:
From idnits:
** All documents that are called out as being updated in the meta-data need to
be mentioned in the abstract:
-- The draft header indicates that this document updates RFC6750, but the
abstract doesn't seem to mention this, which it should.
-- The draft header indicates that this document updates RFC6749, but the
abstract doesn't seem to mention this, which it should.
-- The draft header indicates that this document updates RFC6819, but the
abstract doesn't seem to mention this, which it should.
** Replaced RFC7231 with RFC9110
-- Obsolete informational reference (is this intentional?): RFC 7231
(Obsoleted by RFC 9110)
** I haven’t done the detailed review, but how much of this document applies to
OAuth 2.1? If it is significant, should it be mentioned?
** I struggled to understand what was mandatory in the mix of RFC2119 keywords.
(a) Section 1 says “Nonetheless, it is RECOMMENDED that implementers upgrade
their implementations and ecosystems when feasible.”
(b) Section 3 says “OAuth MUST ensure that the authorization of the resource
owner (RO) (with a user agent) at the authorization server (AS) and the
subsequent usage of the access token at the resource server (RS) is protected
at least against the following attackers:”
(c) Section 3 later says “Implementers MUST take into account all possible
types of attackers in the environment in which their OAuth implementations
are expected to run.”
(b) and (c) seem to be making strong statements mandatory requirements for
high-level threats. However, (a) seems to suggest that conformance is only
recommended. Furthermore, Section 4.* helpfully provides very extensive
guidance on threat mitigation but there are a many “SHOULDs” with few
rationales on why a given practice is not mandatory. I recommend reviewing the
“SHOULDs” in Section 4.* and confirming they can’t be MUSTs or explaining why
the not.
** Section 1. Editorial.
OAuth 2.0 ("OAuth"
in the following)
This parenthetical is a fragment. Should it be “referred to as simply “OAuth”
in this document”?
** 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.
** 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?
** 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?
** 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”.
** 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)?
** Section 3.
It must also be assumed that web attackers can lure the user to
open arbitrary attacker-chosen URIs at any time.
Is “open” needed here? Isn’t it just arbitrary URIs?
** 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.
** Section 4.1
Several
successful attacks exploiting flaws in the pattern matching
implementation or concrete configurations have been observed in the
wild .
Could these be cited?
** Section 4.1.1. Editorial. These examples are very helpful. For the inline
URLs, consider putting them in quotes to improve readability.
** Section 4.5.3. Editorial.
There are two good technical solutions to achieve this goal, outlined
in the following.
Recommend restating the goal.
** Section 4.7.1
* If state is used for carrying application state, and integrity of
its contents is a concern, clients MUST protect state against
tampering and swapping. This can be achieved by binding the
contents of state to the browser session and/or signed/encrypted
state values as discussed in the now-expired draft
[I-D.bradley-oauth-jwt-encoded-state].
Is [I-D.bradley-oauth-jwt-encoded-state] the only way this can be achieved? If
not, s/This can be achieved/An example of how this can be achieved/.
Otherwise, there is a normative dependency created on an expired draft.
** Section 4.7.1. Editorial. s/but AS MAY/but the AS MAY/
** 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.
** 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?
** 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”?
** Section 4.9.2.
* The resource server MUST treat access tokens like any other
credentials. It is considered good practice to not log them and
not store them in plain text.
Consider tightening up the language here. There are a multitude of credentials
with varied practices. Recommend being specific on the desired OAuth token
behavior.
** Section 4.10.2.
The proposed mechanism [RFC8707] could be used or by
encoding the information in the scope value.
-- What is “proposed” about RFC8707?
-- Please provide a reference to the scope value (Section 3.3 of RFC6749)
** Section 4.12. Typo. s/unambigiously/unambiguously/
** 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?
** Section 4.18.2. Formally cite that these are Javascript (?) examples.
Regards,
Roman
_______________________________________________
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth
