Current approach to passing token in channels is unsafe

[mspanc]

Actual behavior

As of Phoenix 1.7 the recommended approach to passing tokens while authenticating in channels results in appending the token to the request URL.

Given that such a request is GET in many server environments such URL, including the token will be exposed in the logs. Even if the app itself won't log the token, it can be done on some ingress/routing layer that is handling incoming requests. This is clearly leading to the situation where access tokens can be easily leaked. It can be used to impersonating users by some malicious actor who will get access to the logs.

In the ideal world, the access to the logs is very limited to the trusted personnel, in practice you often need to give wide access to many developers for sake of debugging. Moreover, the logs are often passed to some third party log processing services where you lose control over security and can be leaked.

Expected behavior

The authentication should happen in the WebSocket layer. For example, the initial connection params should be sent as a first message after establishing connection. If such message does not happen within certain time window, connection should be dropped. From the API perspective it can appear to be identical synchronous connect callback identical to the current one.

I understand that requires modyfying the protocol, but current approach is unsafe.

[Exadra37]

As someone who has worked previously on API security, I can confirm that this approach of using a token in a URL is unsafe, and its use may induce others to consider it a best practice.

I would recommend addressing this security issue as soon as possible, despite it not being a high-severity one.

@chrismccord @josevalim Can I help with addressing this issue?

[josevalim]

I don’t believe Phoenix uses the token by default. So It is a matter of updating the guides and docs.

In particular, Phoenix supports session in websockets, which has been the encouraged approach for a while. Then we should mention the token approach, as long as it is short lived, and then also passing the token on join, which is what this issue recommends. PRs to docs are welcome.

[josevalim]

Ping if you folks are still interested in a PR to the docs. Ping me if you do!

[satoren]

The template uses token. In other words, it can be said that token is used by default.

[josevalim]

That's a good point. To recap, our options are:

1. Passing via params (as today, not encouraged)
2. Passing via headers
3. Passing it on join

I think we can also make the headers one work without much hassle. Thoughts?

[mspanc]

There is no method in the JavaScript WebSockets API for specifying additional headers for the client/browser to send.

Here’s nice summary of the workarounds: https://stackoverflow.com/a/77060459

The cleanest from the architectural standpoint seems to pass it on join.

[josevalim]

Sorry, I should have been more precise and said it should be via the Sec-WebSocket-Protocol header. Although the transport would need a way to make it uniform.

[rhcarvalho]

3. Passing it on join

Passing the token on join has the known issue of allowing unauthenticated connections (item 2 of the StackOverflow link above).

2. Passing via headers (Sec-WebSocket-Protocol)

I checked that this is still present in the Kubernetes API Server (via item 5 of same SO link, all credit to @mspanc for sharing it)

https://github.com/kubernetes/apiserver/blob/c7fb780f6be8071963152a43c94c2191779f3ae1/pkg/authentication/request/websocket/protocol.go