Subject: [OAUTH-WG] authorization code injection - draft-ietf-oauth-security-topics-13

From: Aaron Parecki <[email-masked]>
Date: Thu, 14 Nov 2019 12:10:38 -0800

I read through the authorization code injection section
https://tools.ietf.org/html/draft-ietf-oauth-security-topics-13#section-4.5
but didn't see a mention of this explicitly...

The guidance in 3.1.1 recommends using PKCE for confidential clients.
Section 3.1 says that  clients may use PKCE instead of "state" for
CSRF protection, in which case "'state' may be used again for its
original purpose".

My worry is that without using a unique state value per request, it
becomes less obvious that clients need to maintain state to avoid
being tricked into exchanging an attacker's authorization code.

If the client is using PKCE, then this isn't an issue of exchanging a
different valid authorization code. However it's an issue that the
client may leave itself open to attackers tricking it into exchanging
garbage authorization codes at the AS, possibly triggering rate limits
or security flags at the AS.

There is a partial sentence that hits on the point in 3.1, "clients
MUST only process redirect responses ... from the same user agent this
authorization request initiated with". My worry is that this isn't
enough guidance anymore if people are using non-random state values.
If clients are required to use a random state value, then they are
forced into a situation where they need to maintain state between
requests, which then prevents this problem.

Any thoughts on this? I am not sure exactly what the resolution should
be. I suppose either still requiring a random value in the "state"
parameter, or adding some guidance on how to link a redirect response
with an authorization request.

----
Aaron Parecki
aaronparecki.com

---

From: Torsten Lodderstedt <[email-masked]>
Date: Fri, 15 Nov 2019 16:51:52 +0100

Hi Aaron,

> On 14. Nov 2019, at 21:10, Aaron Parecki <[email-masked]> wrote:
> 

It’s a MUST for any client, not only confidential clients. 

"Clients utilizing the authorization grant type MUST use PKCE …"

> being tricked into exchanging an attacker's authorization code.

The PKCE verifier is a unique per request random value that needs to maintained in the state of the app as well. So beside it’s application for sender authentication and code replay detection, it also serves the role of state to detect CSRF. It just works differently in that the AS determines the response and the PCKE verifier don’t match and if that happens refuses to issue tokens.

> or security flags at the AS.

You mean because it may send a PKCE verifier valid for the current transaction along with an infected code?

> enough guidance anymore if people are using non-random state values.

What is the relationship to the state value? The paragraph you cite is about mix up prevention and requires the client to memorize the AS it sent the user to. 

> requests, which then prevents this problem.

Same holds true for PKCE.

> with an authorization request.

Potentially it is not obvious enough, but the linking is (indirectly) implemented using the PCKE verifier.  

best regards,
Torsten. 

> 
> ----
> Aaron Parecki
> aaronparecki.com
> 
> _______________________________________________
> OAuth mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/oauth

---

From: Aaron Parecki <[email-masked]>
Date: Sat, 16 Nov 2019 21:28:36 +0800

Thanks for the reply. You're right, PKCE requires maintaining
application state as well, and does solve the main worry I have.

However I think there is still something more. I guess my concern is
around the specific wording:

> in this case, 'state' may be used again for its original purpose...

My concern is if people see this recommendation, (even though it's
clarified that it only applies if the authorization server supports
PKCE), they may revert back to static "state" values *regardless* of
whether the server supports PKCE, opening up a security hole again.
This is especially problematic because of the way PKCE works where
clients have no indication as to whether the server supports PKCE if
the whole flow is successful.

I guess I just would rather not mention the possibility of using
static state values again.

----
Aaron Parecki
aaronparecki.com

----
Aaron Parecki
aaronparecki.com
@aaronpk

---

From: Daniel Fett <[email-masked]>
Date: Mon, 25 Nov 2019 16:12:14 +0100

> static state values again.
I would like to keep this note. We could change the wording, however.
How about this:

"If PKCE [RFC7636] is used by the client and the client *has ensured*
that the authorization server supports PKCE, the client MAY opt to not
use "state" for CSRF protection".

-Daniel

---

From: Aaron Parecki <[email-masked]>
Date: Mon, 25 Nov 2019 07:21:57 -0800

Works for me, sounds like that clears up the confusion I am worried about.

Aaron

On Mon, Nov 25, 2019 at 7:12 AM Daniel Fett <[email-masked]> wrote:

> I would like to keep this note. We could change the wording, however. How
> about this:
>
>
> that the authorization server supports PKCE, the client MAY opt to not use
> "state" for CSRF protection".
>
>
> -Daniel
-- 
----
Aaron Parecki
aaronparecki.com
@aaronpk <http://twitter.com/aaronpk>