Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problem with websocket requests access check via oathkeeper #76

Closed
4 of 6 tasks
rwos opened this issue Apr 7, 2022 · 6 comments · Fixed by ory/oathkeeper#952
Closed
4 of 6 tasks

Problem with websocket requests access check via oathkeeper #76

rwos opened this issue Apr 7, 2022 · 6 comments · Fixed by ory/oathkeeper#952
Labels
bug Something is not working.

Comments

@rwos
Copy link

rwos commented Apr 7, 2022

Preflight checklist

Describe the bug

We're using an oathkeeper (v0.38.19-beta.1-17-g74c2581 at the moment) with a check_session_url into our ory cloud kratos. Since the cloud update with the URL changes (I think, might not be related) we're having problems with websocket connections.

We're using a similar setup to this one: ory/oathkeeper#896 (comment), except with a bearer_token instead of cookie_session.

That used to work, and non-websocket requests still work, but for ws we're now getting this in the oathkeeper logs:
level=warning msg=Access request denied audience=application error=map[debug: message:Access credentials are not sufficient to access this resource reason:Get "https://CUSTOM_URL_POINTING_TO_ORY_CLOUD_PROJECT/sessions/whoami": context canceled status:Forbidden status_code:403] granted=false http_host=HOST http_method=GET http_url=https://HOST/path http_user_agent=... service_name=ORY Oathkeeper service_version=v0.38.19-beta.1-17-g74c2581, after some time (the connection just hangs there for a bit).

Any ideas for how to debug this further would be welcome :)

Reproducing the bug

oathkeeper config something like:

authenticators:
  bearer_token:
    enabled: true
    config:
      check_session_url: https://ORY_CLOUD_PROJECT_URL/sessions/whoami
      subject_from: '@this.identity.traits.username'
      token_from:
        header: Authorization

oathkeeper rule

- id: ory:foo
  upstream:
    preserve_host: false
    url: http://foo:1234
  match:
    url: "<{https,wss}>://ws.endpoint.example.com/<**>"
    methods:
      - GET
  authenticators:
    - handler: bearer_token
  authorizer:
    handler: allow
  mutators:
    - handler: noop

and the client request is then something like
curl -v --include --no-buffer --http1.1 -H "Connection: Upgrade" -H"Upgrade: websocket" -H "Origin: http://example.com:80" -H "Sec-WebSocket-Key: SGVsbG8sIHdvcmxkIQ==" -H "Sec-WebSocket-Version: 13" -H 'Authorization: bearer SOME_TOKEN' 'https://ws.endpoint.example.com/'
(we're not really using curl, but this seems to display the same problem at least)

Relevant log output

No response

Relevant configuration

No response

Version

cloud

On which operating system are you observing this issue?

Ory Cloud

In which environment are you deploying?

Ory Cloud

Additional Context

No response

@rwos rwos added the bug Something is not working. label Apr 7, 2022
@aeneasr
Copy link
Member

aeneasr commented Apr 9, 2022

Sorry to hear that you’re having trouble! From the looks of it it appears that everything should be in order. Oathkeeper unfortunately is known to have issues with websockets sometimes. Context canceled usually means that you’re hitting some type of timeout somewhere, not sire if that info is helpful?

@rwos
Copy link
Author

rwos commented Apr 12, 2022

So after some debugging: the good news is that oathkeeper is fine, it's relaying the incoming request to the session store, something like this:

GET / HTTP/1.1
Host: session-store.example.com
Accept-Encoding: gzip
Authorization: bearer wvAUrXXXXXXXXXXXXXXXXXXaC
Connection: Upgrade
Sec-Websocket-Key: gnXM/V545apioaFAHRas/w==
Sec-Websocket-Version: 13
Upgrade: websocket
User-Agent: Go-http-client/1.1
X-Forwarded-Host: oathkeeper.example.com
X-Forwarded-Port: 443
X-Forwarded-Proto: https
X-Forwarded-Server: 799844ef0ed6

It seems that either Kratos itself, or something in front of it in your cloud offering, takes the Upgrade header at face value and switches to a websocket connection - which then stalls until some sort of timeout; which oathkeeper then (rightly) treats as access denied.

A simple patch against oathkeeper current (84c15a6257685fd161c50b79e32323493950b4bf) master that fixes that would be:

diff --git a/pipeline/authn/authenticator_cookie_session.go b/pipeline/authn/authenticator_cookie_session.go
index 8b974e3..09e0980 100644
--- a/pipeline/authn/authenticator_cookie_session.go
+++ b/pipeline/authn/authenticator_cookie_session.go
@@ -155,6 +155,10 @@ func forwardRequestToSessionStore(r *http.Request, checkSessionURL string, prese
 
        // We need to make a COPY of the header, not modify r.Header!
        for k, v := range r.Header {
+               // remove websocket-related headers to not confuse reverse proxies
+               if k == "Upgrade" || k == "Connection" || k == "Sec-Websocket-Key" || k == "Sec-Websocket-Version" {
+                       continue
+               }
                req.Header[k] = v
        }

I could PR that (or you can just copy it) but maybe that should rather be part of a config option - sort of like authenticators.bearer_token.force_method, just for headers.

@aeneasr
Copy link
Member

aeneasr commented Apr 12, 2022

Oh, nice find! That’s the problem for sure!

@aeneasr
Copy link
Member

aeneasr commented Apr 12, 2022

PRa welcomed - it would be great if you could also add a test :)

rwos added a commit to hostwithquantum/ory-oathkeeper-fork that referenced this issue Apr 13, 2022
details at ory/network#76

tl;dr: When Oathkeeper is used to secure the initial connection request
to a websocket endpoint, that doesn't mean we also want to open a
websocket connection to the session store. With this commit, the
"Connection: Upgrade" and related headers are removed now, so the
session store receives a "normal" HTTP request.
@Benehiko
Copy link

I think what happened here is that we added websocket support to our cloud infrastructure not too long ago. Before it would just ignore the Upgrade header 😅

@till
Copy link

till commented Apr 22, 2022

I think what happened here is that we added websocket support to our cloud infrastructure not too long ago. Before it would just ignore the Upgrade header 😅

We were thinking both our PRs are Cloudflare related. Is that what you're thinking as well?

aeneasr added a commit to ory/oathkeeper that referenced this issue Jun 23, 2022
BREAKING CHANGE: From now on, the `bearer_token` and `cookie_session` handlers pass only the needed header (`Authorization`, `Cookie`) to the check URL. To pass additional headers, use the `forward_http_headers` configuration key.

Closes #954
Closes ory/network#76

Co-authored-by: hackerman <3372410+aeneasr@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants