zalando / skipper

An HTTP router and reverse proxy for service composition, including use cases like Kubernetes Ingress
https://opensource.zalando.com/skipper/
Other
3.07k stars 347 forks source link

combination of oauthOidcUserInfo filter + websocket + python backend does not work #1844

Open abinet opened 3 years ago

abinet commented 3 years ago

This is something I found recently and even not sure whether this is a bug.

I have a python based service running on K8s and covered by skipper oauthOidcUserInfo plugin. It works well until browser is trying to establish a websocket connection to the backend service. On this backend service responds 400 BadRequest to the skipper during proxy upgrade call. Surprisingly it works perfectly with some dummy nodejs service protected by same filter. It even works with python service if request goes through JwtValidation filter (which I am trying to contribute with https://github.com/zalando/skipper/issues/1810 ) instead of oauthOidcUserInfo. So, it is a combination of python + oauthOidcUserInfo + websocket which does not work properly.

On the stackoverflow I found this https://stackoverflow.com/questions/26332322/python-socket-bad-request-400

In tcpdump indeed I do not see Host header on upgrade call when oauthOidcUserInfo is used. Without the filter or with JwtValidation upgrade call does provide Host header in upgrade call and all the magic works.

I've tried to set host header explicitly, but it seems that it gets removed somewhere

szuecs commented 3 years ago

@abinet can you try to use https://opensource.zalando.com/skipper/reference/filters/#preservehost preservehost("true") in your route with the oauthOidcUserInfo filter and check if you get the original host header in the Upgrade call?

abinet commented 3 years ago

Just tried but with preservehost("true") skipper do not properly redirect after the callback response

szuecs commented 3 years ago

Did you try:

preservehost("true") -> oauthOidcUserInfo()

or

oauthOidcUserInfo() -> preservehost("true")
abinet commented 3 years ago

oauthOidcUserInfo() -> preservehost("true")

UPD: tried both variants with same result

szuecs commented 3 years ago

I tested the following which works, so it has to be a problem with the oauthOidc* filter

# proxy
/bin/skipper -inline-routes='r0: * -> "http://127.0.0.1:12345"' -address :9999 -experimental-upgrade

# server
% nc -l 12345
GET / HTTP/1.1
Host: 127.0.0.1:12345               <---
User-Agent: Go-http-client/1.1
Connection: Upgrade
Sec-Websocket-Extensions: permessage-deflate; client_max_window_bits
Sec-Websocket-Key: MTMtMTYzMDUwODA3NDE5MA==
Sec-Websocket-Version: 13
Upgrade: websocket

# client
wscat -c http://127.0.0.1:9999     

with preserveHost:

# proxy
/bin/skipper -inline-routes='r0: * -> preserveHost("true") -> "http://127.0.0.1:12345"' -address :9999 -experimental-upgrade

# server
% nc -l 12345        
GET / HTTP/1.1
Host: 127.0.0.1:9999   <---
User-Agent: Go-http-client/1.1
Connection: Upgrade
Sec-Websocket-Extensions: permessage-deflate; client_max_window_bits
Sec-Websocket-Key: MTMtMTYzMDUwODI0MDg2NA==
Sec-Websocket-Version: 13
Upgrade: websocket

# client
wscat -c http://127.0.0.1:9999     
szuecs commented 3 years ago

Can you try to test a similar example with the oauthOidc* filter and show results here from netcat (server) here?

abinet commented 3 years ago

@szuecs I am not able to reproduce your steps exactly because oidc flow requires initial user interaction to generate oauth cookie, but here is tcpdump from my use-case:

Origin: https://example.org Pragma: no-cache Sec-Websocket-Extensions: permessage-deflate; client_max_window_bits Sec-Websocket-Key: rzZ9IbtoxMyIwcGWhA9TaA== Sec-Websocket-Version: 13 Upgrade: websocket X-Appgw-Trace-Id: 542fef86114fd8cb7f3efcb5ee832f01 X-Forwarded-For: :63164 X-Forwarded-Port: 443 X-Forwarded-Proto: https X-Original-Host: example.org X-Original-Url: /some/url

abinet commented 3 years ago

I think I found the issue. It seems that whatever python library used for websocket communication it reacts badly on skiperOauthOidc-cookie. Adding -> dropHeader("Cookie") solved the problem.

szuecs commented 3 years ago

@abinet interesting, I mean it will work in the websocket case, but it won't be SSO. The cookie is important for the token handling, but if you just have an open websocket, it does not need another validation, because it just streams data in both direction in skipper's point of view. A reconnect will require another redirect login with credentials passing, so maybe not as usable as it should be.