[OAUTH-WG] WGLC review of draft-ietf-oauth-security-topics-13

[Guido Schmitz <[e-mail]>]

Hi,

All of my comments on oauth-security-topics-13 are
remarks/questions/suggestions for clarification in the document, i.e., I
do not have any fundamental objections. Overall, the draft is, in my
opinion, in good shape to be published and as already discussed, open
points can be updated later. I think that it is important that the
document should become a BCP as soon as possible.

* Section 2, Attacker Model: While A1 and A2 are quite clear, A3-A5 seem
to be extensions for A1 and A2 respectively, e.g., we could have
attackers like (A1+A3) or (A2+A4+A5). A reader not fully familiar with
this concept might misunderstand this classification. A sentence to
clarify this would be good.

* Section 3.1, Second Paragraph: The reference [owasp] is quite coarse
as it points to the entry page of the OWASP organization. I suggest
pointing to the OWASP cheat sheet on open redirects at
https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html.

* Section 3.1, Third Paragraph, Section 4.7, and other places throughout
the document: (Please excuse that the following might be a bit
nitpicking.) The term CSRF might be misleading as we are not talking
about "classical" CSRF, where a party is tricked into (implicitly)
believing that a request was caused by its own origin (from a web
browser), but was actually sent cross-origin. Here, we are talking about
requests which (by protocol) are typically intended to be cross-origin.
The point here is not that the request is cross-origin, but that it
originates from an unintended (cross-)origin. Maybe you could mention
that we are talking about an advanced kind of CSRF in the
double-redirection context.

* Section 3.1.2: What about SPAs? Either they use authorization code
grant with the help of some server or use CORS to access the token
endpoint. The token endpoint, however, then needs to support CORS. The
obvious question is then, what is the CORS policy of the token endpoint?
It is probably fine to allow all origins (if this policy only applies to
the token ep itself and not other eps at the same origin) or do I miss
something?

* Section 4.1.3, "fix fragments": The source [fb_fragments] only says
that Facebook adds the fragment #_=_ to some URLs. There is no
explanation or reasoning in the source (as well as in this document) at
all that this technique strips the fragment from redirects. (Mike Jones
also commented on this.)

* Section 4.2.4, Bullet Point "referrer header": Adding the
rel="noreferrer" attribute to links does not protect against leakage via
third-party content. Referrer Policy is the only effective
countermeasure of the mentioned two as it can be used to completely
suppress the Referer header or at least strip it to the origin. Should
we give a concrete example for Referrer Policies? For example, the
header "Referrer-Policy: no-referrer" in the response completely
suppresses the Referer header in all requests originating from the
resulting document.  Also, this measure is easier to implement than
adding rel="noreferrer" to each link. (BTW, there is also an excess
quotation mark in this paragraph.)

* Section 4.3.2: You mention postmessage communication (I think Hans
also pointed to this). Afaik Google uses postmessage in their client
library. Is there any document describing this technique?

* Section 4.4.1: The attack description is based on a very concrete
example. Some details, however, are not relevant for the attack to
succeed. In Step 3, the exact response code could be amended with an "e.g."

* Section 4.5: Add pointers (A1) and (A2) to web or network attacker in
last sentence.

* Section 4.5.3, "Code-bound State": This requires that state is fresh
for each flow. You should mention or even emphasize this.

* Section 4.5.3, "Per-instance client id/secret": This essentially says
that native apps or anything that might be able to obtain and store
client id/secret (e.g., using dynamic registration) could do this. Web
SPAs could do this as well (e.g., using Web Storage). The last sentence
of this paragraph is somehow confusing.

* Section 4.5.3, Second Paragraph after bullet points, "Note on
pre-warmed secrets": What is the context of this note? Does the attacker
start a flow on some device, extract secrets, and then pass this device
to the victim who then completes the OAuth flow? (Mike Jones also
remarked that "pre-warmed secrets" is not explained)

* Section 4.8.1.1, metadata parameter "resource_servers": This is
supposed to be an extension to RFC8414, right? (Also mentioned by Mike
Jones)

* Section 4.8.1.1, return parameter "access_token_resource_server": This
is supposed to be an extension of the token endpoint (as defined in
RFC6749), right? I suggest adding a pointer to its definition.

-Guido