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

proposal: add subprotocol for token-authenticated websockets #1407

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

minrk
Copy link
Contributor

@minrk minrk commented Mar 14, 2024

follows kubernetes' example of smuggling the token in the subprotocol itself. Maybe not great, but solves our problem and is simple and appears to work.

To authenticate websockets via new mechanism, clients should:

ws = new WebSocket(wss://..., ['v1.token.websocket.jupyter.org', `v1.token.websocket.jupyter.org.${token}`, ...])

which sets the header:

Sec-WebSocket-Protocol: v1.token.websocket.jupyter.org, v1.token.websocket.jupyter.org.abc123

The response will have the header:

Sec-WebSocket-Protocol: v1.token.websocket.jupyter.org

This is transparently ignored if unrecognized, so fully backward compatible for both clients and servers, and fully compatible with specifying other subprotocols, etc.

This is transparently ignored if unrecognized by servers and most clients, but at least Chrome checks that the server's response includes one of the requested subprotocols if any are specified. For backward-compatibility, a client must try with subprotocols, then retry without them. This is the reason for the v1.token.websocket.juptyer.org subprotocol in addition to the protocol including the token itself.

This seems like the simplest path to not sending the token in URL parameters. The alternative of using a handshake message is more complex and protocol-breaking, and further allows clients that never authenticate to hold open connections indefinitely.

Notable difference: kubernetes base64-encodes the token on the end. I haven't done that because negotiating url-safe-base64-without-padding seems like a pain because:

  • neither Python nor Javascript actually implement the required unpadded urlsafe-base64 in the standard library
  • base64 encoding only makes an already-valid string longer
  • While we don't technically specify that tokens must be safe strings in header fields like this, it is always true in practice, so we could without any practical loss. There's a good chance we already have this requirement elsewhere by accident (e.g. we don't escape tokens when passed in URL params).

I think the only question that remains is whether and how servers advertise support for this.

via

follows kubernetes' example of smuggling the token in the subprotocol itself
@minrk
Copy link
Contributor Author

minrk commented Mar 14, 2024

Unfortunately, upon further testing this is trivially backward-compatible for clients except on Chrome, where requesting a subprotocol that is not granted results in a dropped connection. So for a client to be backward-compatible, it would have to try with the subprotocol, then fallback to without the subprotocol if it doesn't have another way to determine whether the subprotocol is supported.

Since we use subprotocols for the kernel already which already present the same issue, JupyterLab already has the necessary logic to try connecting with subprotocols, then fallback without them.

@davidbrochart
Copy link
Contributor

Not sure if it's relevant, but we use subprotocols in the kernel protocol over WebSocket, maybe this helps: #657 (comment).

Chrome doesn't accept websocket connections if protocol is requested but not provided
@minrk minrk force-pushed the subprotocol-token branch from 7f309b3 to aef6348 Compare March 14, 2024 13:46
@minrk
Copy link
Contributor Author

minrk commented Mar 14, 2024

@davidbrochart Thanks for linking to the discussion. I did mention the JupyterLab implementation of subprotocol fallback for that, so I think we have everything we need for this, at least in JupyterLab, and have precedent.

A kernel request would have

subprotocols = ["v1.kernel.websocket.jupyter.org", "v1.token.websocket.jupyter.org", "v1.token.websocket.jupyter.org.abc123"]

and the response would be unchanged, selecting v1.kernel.websocket.jupyter.org

@minrk
Copy link
Contributor Author

minrk commented Mar 26, 2024

Started JEP process for this, if that makes sense to folks: jupyter/enhancement-proposals#119

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

Successfully merging this pull request may close these issues.

2 participants