OIDC login final redirect to callback URL fails with missing nonce

[jmullo]

Title: OIDC login final redirect to callback URI fails with missing nonce

Description:
Error: "state query param does not contain url or nonce"

URL in browser: https://our.site.com/cb?code=uuid-code&state=url%3Dhttps%3A%2F%2Four.site.com%2F

Repro steps:
OIDC login flow with Cognito as identity provider.

[jmullo]

Apparently there are two different ways Cognito can mess up here. It either drops the nonce from state and loses it completely. Or it drops the nonce from state and puts the nonce to callback URL as a separate query parameter.

For these kind of scenarios an option to disable the whole nonce validation in Envoy would be needed.

It's possible that Cognito doesn't like state being URL-encoded, and could work as base64-encoded(?)

[nezdolik]

@jmullo which envoy features do you use for this flow?

[jmullo]

@jmullo which envoy features do you use for this flow?

OIDC Authentication security policy for HTTPRoutes. Also JWT Authentication policy.

[steeni]

Root issue here is that cognito login page doesn't handle state value properly and the nonce doesn't stay in the state but gets raised directly to the url parameter and when coming back to callback nonce has disappeared from the state. In their documentation they do not support urlencoded json state values (and I guess this extends to any format, json just so common that it was mentioned). If we would ask them to fix it they probably just refer that section of their documentation. https://docs.aws.amazon.com/cognito/latest/developerguide/authorization-endpoint.html#get-authorize-request-parameters

Implementing the point 3 here #36276 would probably help because then nonce would not be part of the state. Alternatively if the state value would be base64 urlencoded (base64 but with / and + changed to _ and - and no padding =) there would be less chance that it could break. Doing both would probably give most flexibility to handle with different oidc gateways. In the meantime I would recommend adding an option to disable nonce checking.

[nezdolik]

cc @mattklein123 @TAOXUY @tyxia @derekargueta

[zhaohuabing]

Apparently there are two different ways Cognito can mess up here. It either drops the nonce from state and loses it completely. Or it drops the nonce from state and puts the nonce to callback URL as a separate query parameter.

For these kind of scenarios an option to disable the whole nonce validation in Envoy would be needed.

It's possible that Cognito doesn't like state being URL-encoded, and could work as base64-encoded(?)

nonce is supposed to be transparent to the OIDC provider, I didn't realize that some OIDC providers would inspect the state parameter and remove the nonce outside of it.

[zhaohuabing]

Implementing the point 3 here #36276 would probably help because then nonce would not be part of the state.

I'm not sure if this the right way to go. Nonce is an optional paramete in the standard. The oauth2 filter will need to know whether the provider supports the nonce parameter or not and supports both the nonce in the state and the standalone nonce, which complicate the implementation.

[steeni]

Implementing the point 3 here #36276 would probably help because then nonce would not be part of the state.

I'm not sure if this the right way to go. Nonce is an optional paramete in the standard. The oauth2 filter will need to know whether the provider supports the nonce parameter or not and supports both the nonce in the state and the standalone nonce, which complicate the implementation.

Yeah, I understand. I notice that there is already an implementation of base64url encode/decode here https://github.com/envoyproxy/envoy/blob/main/source/common/common/base64.cc#L245 Maybe it could be used instead of percent-encoding in filter.cc. In theory, that should fix the immediate issue.

[zhaohuabing]

Implementing the point 3 here #36276 would probably help because then nonce would not be part of the state.

Report back here:

the nonce in the state paramter and the nonce parameter are two differenent things: the nonce in the state parameter is used for csrf prevention and is applicable for both oauth2 and oidc. Please note that the OIDC spec defines a seperate nonce parameter ,which is specifically designed to prevent replay attacks and is unique to OIDC. More discussion about state and nonce parameters can be found in this comment: #37050 (comment)

I've gone ahead to raise #37473 in envoy to use base64url encoding to fix this issue.