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

Current approach to passing token in channels is unsafe #5778

Open
mspanc opened this issue Apr 11, 2024 · 3 comments
Open

Current approach to passing token in channels is unsafe #5778

mspanc opened this issue Apr 11, 2024 · 3 comments

Comments

@mspanc
Copy link

mspanc commented Apr 11, 2024

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
Copy link
Contributor

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
Copy link
Member

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants