Please also add these WGLC comments that a Microsoft Azure Active Directory 
(AAD) developer asked me to convey:


  1.  In 4.12, "Authorization servers MUST determine based on their risk 
assessment whether to issue refresh tokens to a certain client [...]" I'm not 
sure what this requirement requires in practice. AAD issues refresh_tokens to 
all clients upon request and user consent and applies different lifetime 
policies to different clients. We also routinely make risk assessments about 
all manner of things. Does AAD thereby comply with this guideline? Reading the 
whole paragraph, I think the paragraph is trying to encourage OAuth clients 
which use a RT when the RT is returned but use auth codes when the RT is not 
returned. That's fine, but the current text comes off as imposing a vague 
requirement on authorization servers. Edits inline - "Authorization servers 
MUST MAY dynamically determine based on their risk assessment whether to issue 
refresh tokens to a certain client.  If the authorization server decides not to 
issue refresh tokens, the client may SHOULD refresh access tokens by utilizing 
other grant types, such as the authorization code grant type.  In such a case, 
the authorization server may utilize cookies and persistent grants to optimize 
the user experience."


  1.  In 4.12, the heading says "Authorization server MUST utilize one of these 
methods to detect refresh token replay for public clients" - however, it 
doesn't quite specify why refresh token replay is bad. The second paragraph 
described the goal more precisely ("If an attacker is able to exfiltrate and 
successfully replay [RTs]") - it's refresh token exfiltration that we MUST 
mitigate, not refresh token replay. Browser based apps that desire multiple 
access token should be able to use sender-constrained refresh tokens on 
multiple simultaneous HTTP requests - and some AAD apps do exactly this today. 
In practice, it's also difficult and undesirable to lock access to MSAL's RT 
cache - individual reads and writes are serialized, but { read, http request, 
write } events are not serialized. Here, just replace "to detect refresh token 
replay" with "to mitigate refresh token exfiltration" and then under "refresh 
token rotation," explain that such ASes using RT rotation to mitigate refresh 
token exfiltration MUST also forbid / prevent RT replay.


  1.  In 4.12, *Sender-constrained refresh tokens:* - sender-constrained tokens 
are absolutely valuable from a security perspective. However, without 
authenticating the sender, it's not necessarily an adequate protection for XSS. 
Some sender-constrained implementations (such as AAD's WIP spec for RTs) use 
un-attested keys and permit silent rebinding. Indeed, I think this will be 
common in browser based apps - attested keys have privacy implications leading 
to undesirable UX and rebinding prompts also lead to undesirable UX. An XSS 
attacker in those scenarios could likely silently rebind to an 
attacker-controlled key. It'd be good for the spec to acknowledge these aspects 
of sender-constrained tokens and make recommendations.
     *   It's these things that make us wary of moving from the implicit flow 
to RTs. The damage from exfiltrated RTs is much more severe, but the 
mitigations are less-specified by the BCPs, and thus seem less 
well-understood.. Frankly, I think each method of mitigation is worth of its 
own subsection with more discussion - 4.12.1, 4.12.2, etc.


  1.  In 4.12, *Refresh token rotation:* - in addition to Mike Jones' review 
comment. "The authorization server cannot determine which party submitted the 
invalid refresh token, but it can revoke the active refresh token" is the 
guidance given around revoking stolen RTs. I think this guidance should use an 
RFC2119 word in place of "can" - and that the guidance should be stronger, e.g. 
"SHOULD" or "MUST", w.r.t revoking the active refresh token. Rotating RTs 
doesn't do much any good as mitigation if the stolen RT remains valid.


  1.  In 4.12, we'd like to see fixed-expiry RTs of ~1 day added as "one of the 
methods to mitigate RT exfiltration for public clients" that ASes MUST utilize 
one of. This is what we're implementing; we think it's sound; and we'd like to 
see the draft say so, to remove all doubt.


                                                       -- Mike


_______________________________________________
OAuth mailing list
OAuth@ietf.org
https://www.ietf.org/mailman/listinfo/oauth

Reply via email to