Mix-up mitigation (defence in depth)

[Dave Tonge]

Currently some of the approaches in FAPI require the RP to validate certain parameters to detect mix-up and man-in-the-browser attacks.

For example:

• the RP must validate the s_hash
• the RP must check nonces
• the RP must validate the iss in the id_token

Another mitigation for mix-up attacks (described in https://datatracker.ietf.org/doc/draft-ietf-oauth-security-topics) is code-bound state. This is where the RP sends the state param to the token endpoint when exchanging the auth code for an access token.

I think we should consider adding this requirement in, as it allows the OP to detect some attacks even in the event of an RP that is not properly validating the above.

For what its worth, this OpenID Connect Certified Relying Party implementation (https://github.com/panva/node-openid-client) already sends state to the token endpoint.

[Tom Jones]

I don't see that this adds any security benefit over checking the nonce (other than length.)

[Dave Tonge]

It doesn't give any more security benefit to the overall protocol, but it does allow the OP to detect an attack in the event that an RP is not properly verifying the nonce.

[Tom Jones]

I see, so your assumption is that the token ep is co-located with the authority. I haven't seen ukob docs that assume it, or anything else about the op.

[Nat Sakimura]

PKCE is the recommended way for the server side detection. Perhaps we can add it to the security considering. It should be purely optional. More discussion needed.

[Joseph Heenan]

As I mentioned on the call - we should decide who PKCE is optional for - ie. can a conformant AS insist that clients use PKCE?

[Dave Tonge]

@josephheenan adding this in is for the benefit of the AS. So if we make it optional for the AS and they decide to implement it, then I think the client will have to implement.

[Joseph Heenan]

Okay, I guess we're looking at a new clause along the lines of:

The authorisation server: <...> may require all clients to use PKCE

I think that counts as a breaking change for clients so we would need a new implementer's draft to add it.

[Nat Sakimura]

Currently some of the approaches in FAPI require the RP to validate certain parameters to detect mix-up and man-in-the-browser attacks.

For example:

• the RP must validate the s_hash
• the RP must check nonces
• the RP must validate the iss in the id_token

Another mitigation for mix-up attacks (described in https://datatracker.ietf.org/doc/draft-ietf-oauth-security-topics) is code-bound state. This is where the RP sends the state param to the token endpoint when exchanging the auth code for an access token.

I think we should consider adding this requirement in, as it allows the OP to detect some attacks even in the event of an RP that is not properly validating the above.

For what its worth, this OpenID Connect Certified Relying Party implementation (https://github.com/panva/node-openid-client) already sends state to the token endpoint.

[Dave Tonge]

So I propose that we add this text to Part 2, 5.2.2:

1. may require all clients to use [RFC7636],

NOTE this is overrides [FAPI1] 5.2.2.7. While this document provides appropriate methods for the Client to detect mix-up and authorization response tampering, PKCE allows the Authorisation Server to detect some of these attacks without relying on the client. For this reason some Authorisation Servers may require Clients to use PKCE

[Dave Tonge]

It would be good to discuss the above text. If we go ahead with this change, then arguably we need a metadata param for the AS to advertise the requirement to use PKCE.

If we add something like the above clause for the AS, then we will also need to add the provision to the client. However I think we need to agree the principle first, and whether we need a metadata param.

[Dave Tonge]

We discussed this on the call and had a general agreement that it would be better to mandate PKCE for all situations.

This is a breaking change, so we will seek feedback from the WG.

We discussed that this change would go into the next implementers draft and that the conformance suite would have different tests for implementers draft 1 and implementers draft 2.

I will propose some updated wording, as I suggest we explain why we are requiring multiple methods of detecting the same attacks.

[Dave Tonge]

I suggest this issue is closed - breaking changes should go into 2.0

[Daniel Fett]

FAPI 2 makes PKCE mandatory.

A decision on the mix-up mitigation is still open (see https://bitbucket.org/openid/fapi/pull-requests/163); there’s also an effort in the OAuth WG to create a new draft to define a mix-up mitigation technique.

[Daniel Fett]

For FAPI 2 and mix-up mitigation, I increasingly see iss as the only robust solution. I found a security problem when *not* using iss but relying on per-issuer redirect URIs:
https://mailarchive.ietf.org/arch/msg/oauth/RjbSwFRmLsk0EgAY2Ter-nw66EY/

I adapted the Baseline profile to use iss only, currently with a reference to the security BCP. There is also a new draft for the iss parameter (https://tools.ietf.org/html/draft-meyerzuselhausen-oauth-iss-auth-resp). If this draft proceeds to a working group draft, we shall reference it in FAPI 2 as well (it provides more/better explanation for using iss).

I think we can close this issue for now.