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

WebSocket: missing Sec-WebSocket-Protocol in response header when it is provided in request #375

Open
acieroid opened this issue Jan 1, 2025 · 1 comment

Comments

@acieroid
Copy link
Contributor

acieroid commented Jan 1, 2025

Hi!

I stumbled upon this one (on Dream 1.0.0~alpha8) implementing a WebSocket server with authentication through the Sec-WebSocket-Protocol (see it explained e.g., here).
In short: because WebSocket does not allow for extra headers, some people use the header Sec-WebSocket-Protocol to pass authentication tokens (instead of using HTTP's Authorization header).
This can easily be done with Dream, for example as follows (adapted from example/k-websocket):

let home =
  <html>
  <body>
    <script>

    const token = "super-secret-token";
    const socket = new WebSocket("ws://" + window.location.host + "/websocket", ["Authorization", token]);

    socket.onopen = function () {
      socket.send("Hello?");
    };

    socket.onmessage = function (e) {
      alert(e.data);
    };

    </script>
  </body>
  </html>

let () =
  Dream.run
  @@ Dream.logger
  @@ Dream.router [

    Dream.get "/"
      (fun _ ->
        Dream.html home);

    Dream.get "/websocket"
      (fun request ->
        match Dream.header request "Sec-WebSocket-Protocol" with
          | Some "Authorization, super-secret-token" ->
            (* Client is successfully authenticated now *)
            Dream.websocket (fun websocket ->
                match%lwt Dream.receive websocket with
                | Some m ->
                  Dream.send websocket m
                | _ ->
                  Dream.close_websocket websocket)
          | Some _ -> failwith "Not authorized"
          | None -> failwith "Expected Sec-WebSocket-Protocol header")
  ]

This works perfectly fine in Firefox. In Chrome however, the connection is closed upon receiving the 101 reply from the server. As a developer perspective, the only information we see is this in Chrome's console:

(index):6 WebSocket connection to 'ws://localhost:8080/websocket' failed:

From Dream, we see the request correctly logged. Looking in more details at what happens (through Wireshark, as I didn't manage to see much details using Chrome's debug tools), we can arrive at the conclusion that this is because the reply is malformed according to the WebSocket protocol. From MDN:

In a response it specifies the sub-protocol selected by the server. This must be the first sub-protocol that the server supports from the list provided in the request header.

It seems that Firefox is lenient with the response, while Chrome is not.
The current solution is to add a header to the response:

    Dream.get "/websocket"
      (fun request ->
        match Dream.header request "Sec-WebSocket-Protocol" with
          | Some "Authorization, super-secret-token" ->
            (* Client is successfully authenticated now *)
            let%lwt response = Dream.websocket (fun websocket ->
                match%lwt Dream.receive websocket with
                | Some m ->
                  Dream.send websocket m
                | _ ->
                  Dream.close_websocket websocket) in
            Dream.add_header response "Sec-WebSocket-Protocol" "Authorization";
            Lwt.return response
          | Some _ -> failwith "Not authorized"
          | None -> failwith "Expected Sec-WebSocket-Protocol header")

Now, is this something that should be handled by Dream or by the application? I'm not sure what's best. But clearly, I spent quite some time figuring that out, until I looked at the headers through a Wireshark capture (because I didn't manage to see the headers of the response through Chrome's debug tools), and compared Dream behavior with the one from node's ws module.
For comparison, the following Node server transparently adds the Sec-WebSocket-Protocol to the response when it is provided as part of the request. It selects the first element of the comma-separated list, adding Sec-WebSocket-Protocol: Authorization to the headers (just as I did in the example above).

const WebSocket = require('ws');
const wss = new WebSocket.Server({ port: 8080 });

wss.on('connection', (ws) => {
    ws.send('Hello from WebSocket server!');
    ws.on('message', (message) => {
        ws.send(message)
    });
});

Even if you prefer not to add it as the default behavior, this ticket would serve as useful documentation for anyone trying to do the same.

@yawaramin
Copy link
Contributor

Interesting, thanks for the write-up. Looking at the header documentation: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Sec-WebSocket-Protocol#websocket_opening_handshake

It seems it is meant to select a supported WebSocket subprotocol:

GET /chat HTTP/1.1
Upgrade: websocket
...
Sec-WebSocket-Protocol: soap, wamp

Another example is in Dream's GraphQL handler, where it looks for the graphql-transport-ws subprotocol: https://github.com/aantron/dream/blob/27ff43177b57a9c64abb39e72750fbd5f6a47199/src/graphql/graphql.ml#L277C44-L277C53

My first thought here is that inside the implementation of val websocket, we don't have access to the request, so we don't know what headers were sent. It could be doable in a middleware though: if there is a request header matching Sec-WebSocket-Protocol: Authorization, something and a response header Upgrade: websocket, then add a new response header Sec-WebSocket-Protocol: Authorization.

Not sure if the Dream API is the right home for such a middleware, especially as this 'auth' method itself seems kinda non-standard, and we would have to cross-reference it from all WebSocket documentation.

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

2 participants