Seems to be vulnerable to some forms of browser swapping attacks

[tbroyer]

See https://mailarchive.ietf.org/arch/msg/oauth/K8Wnw08GzPstyAQAh0JmSB47pOQ/, https://datatracker.ietf.org/doc/slides-124-oauth-sessa-browser-swapping/ and https://openid.net/specs/fapi-security-profile-2_0-final.html#name-browser-swapping-attacks

Specifically:

• the openidc_authorization_response function, in case of state, iss or client_id (?) mismatch, errors out without calling the token endpoint, so the code is still valid (in case it's been somehow intercepted or exfiltrated by an attacker) and can be injected into another session
• in the sample code in the README for how to use the module, in case of error, an error HTTP response is sent back, but this doesn't account for the specific error of a missing code, which can happen in a response mode downgrade attack where the response mode has been changed somehow by an attacker to fragment; in this case, the code might still be present in the URL, accessible in the browser, and exfiltrable.

[oldium]

What is the reproduction workflow/attack scenario with proxy use case like lua-resty-openidc running on Nginx?

[tbroyer]

From the tests I implemented in my own Java library (here and there):

This scenario assumes the attacker has a near real-time access to access logs or is a member of the company's support team (see discussion in IETF OAuth WG mailing list, this is not as far-fetched as it could seem); this also requires using response_mode=query, which is the default:

1. Attacker starts an authentication flow and notes the URL they're being redirected to at the IdP
2. Attacker tricks the victim in opening said URL
3. Victim authenticates at the IdP (if needed, this could also be transparent if already authenticated), and is being redirected to the application (nginx+lua-resty-openidc) at the callback endpoint. Because the victim does not have a session, this will fail with "state from argument does not match state restored from session", but the token endpoint won't have been called so the code is still valid
4. Attacker gets the callback endpoint URL (either from access logs, or by tricking the victim to share it during a support call) and opens it in their browser: this now goes through as the browser has the required session, and the attacker is now logged in with the victim's identity.

This scenario assumes some XSS vulnerability (or a rogue browser extension, where the IdP would be protected from direct keylogging with a strict CSP):

1. Attacker starts an authentication flow and notes the URL they're being redirected to at the IdP
2. Attacker changes the response_mode to fragment in said URL
3. Attacker tricks the victim in opening said modified URL
4. Victim authenticates at the IdP (if needed, this could also be transparent if already authenticated), and is being redirected to the application (nginx+lua-resty-openidc) at the callback endpoint. Because of the response mode downgrade to fragment, lua-resty-openidc receives no code or state, and fails with "unhandled request to the redirect_uri: ". Following the sample code in the README, the integration of lua-resty-openidc returns an error page (without any redirection), and because the code was never received, it couldn't have been redeemed at the token endpoint so it's still valid
5. Attacker's injected code exfiltrates the URL
6. Attacker receives the URL, changes the fragment into a query string and opens said modified URL in their browser: this now goes through as the browser has the required session and the server receives all the parameters, and the attacker is now logged in with the victim's identity.

[tbroyer]

The server (nginx=lua-restry-openidc) should:

• call the token endpoint whenever it receives a code, even if it knows that call will error out (e.g. no session, so no code verifier); this should be enough to counter the first attack scenario above
• the sample integration code in the README should handle the case where no code (or an explicit error!) was received by redirecting with an explicit fragment, to replace the possible fragment containing the code. I'd suggest redirecting to the same callback endpoint with an explicit error. This should be enough to counter the second attack scenario above.

[oldium]

The pull request does not mention that mitigation. The mitigation mentioned there is actually to allow enforcing POST requests (form_post) to exchange the authorization code on the IdP side by allowing the developer to set it. But in case of a support member, he could change that too. I have not read the discussion yet, but it would be good to have some official recommendations.

[tbroyer]

The first part of the mitigation is implicit: the PR talks about letting the code unused, so the idea is to use it (there's still a risk that attacker uses it first, but if the IdP respects OAuth then it should be a single-use code, so using it again in the victim's request should revoke the attacker's session). The second part of the mitigation is more explicit in the PR: “To solve this, clients SHOULD sanitize the URL, especially in error cases, e.g., by redirecting to a dedicated error page.”

You can have a look at these messages in the conversation, where I explicitly asked about those mitigations: https://mailarchive.ietf.org/arch/msg/oauth/juzJwavYWL-ju677dNWrGFXL0zc/, https://mailarchive.ietf.org/arch/msg/oauth/HZRa28ZenGGmwJTe_IyEX3WZ4s4/ and https://mailarchive.ietf.org/arch/msg/oauth/3H0Q9eHm1LDC03nxdr8Bf90iucc/