envoy icon indicating copy to clipboard operation
envoy copied to clipboard

OAuth2 filter fails a request with Non-ASCII characters

Open Alexcei88 opened this issue 3 years ago • 2 comments

There is a listener with Oauth2 filter.

User is not yet authorized. User sends a request with non-ASCII characters to Envoy like this

https%3A%2F%2Freg-portal.ca.dev.kontur.ru%3A4433%2FRC%2FSearch%2FLocal?stateGroup%3D0%26typeGroup%3D0%26isFromForm%3DFalse%26Query%3D%D0%B2%D0%B2%D1%81

Once user authorize (login and password has been inputed), IDP server calls a callback url according our configuration that looks like

https://reg-portal.ca.dev.kontur.ru:4433/oauth2/callback?state=https%3A%2F%2Freg-portal.ca.dev.kontur.ru%3A4433%2FRC%2FSearch%2FLocal?stateGroup%3D0%26typeGroup%3D0%26isFromForm%3DFalse%26Query%3D%D0%B2%D0%B2%D1%81&session_state=2f0c1f44-6726-47ca-ae04-9b27ab470403&code=d4245176-0970-4250-b536-62e5eb8cc6c9.2f0c1f44-6726-47ca-ae04-9b27ab470403.bac706d3-bede-49cf-b52d-a38a44c98d3c

If we'll decode this url we get

https://reg-portal.ca.dev.kontur.ru:4433/oauth2/callback?state=https://reg-portal.ca.dev.kontur.ru:4433/RC/Search/Local?stateGroup=0&typeGroup=0&isFromForm=False&Query=ввс&session_state=2f0c1f44-6726-47ca-ae04-9b27ab470403&code=d4245176-0970-4250-b536-62e5eb8cc6c9.2f0c1f44-6726-47ca-ae04-9b27ab470403.bac706d3-bede-49cf-b52d-a38a44c98d3c

Most important here is that a state query parameter is a equals our source url and contains Non-ASCII characters (Query=ввс)

The problem is that Envoy cannot initialize Http::Utility::Url for provided url.

https://github.com/envoyproxy/envoy/blob/7461266aae623c7a0c9858fe9055aaf37aed95b6/source/extensions/filters/http/oauth2/filter.cc#L347-L353

As result I have the error

OAuth flow failed

instead of successfull flow.

Alexcei88 avatar Sep 19 '22 18:09 Alexcei88

cc @derekargueta @snowp as codeowners

adisuissa avatar Sep 19 '22 18:09 adisuissa

The problem is in http-parser library. The library doesn't work properly for urls with Non-ASCII characters. I have checked my url using by GURL library, it's ok. So I would like to send PR to replace http_parser library to GURL library exactly in this filter. I see that it's going to unify URL parsing with GURL but it's a critical bug for me.

@derekargueta , @snowp , what do you think about it?

Alexcei88 avatar Sep 20 '22 12:09 Alexcei88

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.

github-actions[bot] avatar Oct 21 '22 16:10 github-actions[bot]

This issue has been automatically closed because it has not had activity in the last 37 days. If this issue is still valid, please ping a maintainer and ask them to label it as "help wanted" or "no stalebot". Thank you for your contributions.

github-actions[bot] avatar Oct 28 '22 20:10 github-actions[bot]