Last Call Review of draft-ietf-oauth-browser-based-apps-22
review-ietf-oauth-browser-based-apps-22-secdir-lc-ladd-2025-01-28-00

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

The summary of the review is Has Issues. I wish it didn't have to be: it's a
thoroughly written, and a model in many respects for carefully analyzing the
security properties of a number of choices. However, I think that given the
importance of the document it needs to be more accessible to people who are not
experts, and there's a number of analysis choices I don't understand. I'm also
not sure the title is quite right: it sounds like a big replacement, rather
than a detailed guide to the security of OAuth in browser apps.

As a reader I did not understand much of the scenario being discussed until the
very end: the resource server is not on the same domain as the application is
being retrieved from, and the oauth server might be a third domain from both. I
had to wait until Section 7 to figure that out. For those of us unused to Oauth
it might be worth spelling that out: typically my interactions with it are SSO
where the resource and application are the same. Certain key acronyms like PCKE
are not defined when first introduced, and used for quite some time, which also
makes it difficult to understand some sections until getting to them. I think
these can be straightened out without too much text.

I do have some concerns about the analysis. The attacks seem to be centered
around Javascript injection. This gives the attacker incredible power. However,
the capabilities of the attacker are then centered on 5 specific attacks. It is
not clear to me why this is justified. Similarly, the presence of the relaying
server does simplify the story around Oauth token management, but introduces a
deputy that can be confused. While there are a bunch of security considerations
added in 6.1.3, this one isn't there. There's a lot about cookies, but little
about session fixation or other attacks that confuse the link between clients.
This also affects the token mediating backend.

Overall I think this is in pretty good shape.

Sincerely,
Watson Ladd