babelouest / ulfius

Web Framework to build REST APIs, Webservices or any HTTP endpoint in C language. Can stream large amount of data, integrate JSON data with Jansson, and create websocket services
https://babelouest.github.io/ulfius
GNU Lesser General Public License v2.1
1.07k stars 183 forks source link

Erroneous "Origin" header in client websocket connection #209

Closed mvduin closed 2 years ago

mvduin commented 2 years ago

The Origin HTTP header is used by web browsers and browser-like clients to caution the server that the websocket connection is being established at the request of a website, and optionally to identify that website to the server (Origin: null indicates the connection is at the request of an undisclosed website). Non-browser clients that establish a websocket connection on their own volition are supposed to omit the header, indicating the client is not acting on behalf of a website.

Adding an Origin header by default like ulfius_websocket_connection_handshake() currently does is therefore incorrect. Worse yet, the origin it specifies (ws://HOST or wss://HOST) is completely bogus since an origin cannot be a websocket connection, and in our case it causes connection failure as the server rejects the invalid scheme.

The correct behaviour is to send no Origin header at all. Optionally it may be useful for some users to have the option to include an Origin header with a user-specified value. The main purpose of that would be as a workaround for badly implemented servers that erroneously reject connections that lack an Origin header. (The purpose of the Origin header is not to protect against malicious clients, which it does not and cannot do, it is to protect against honest browsers acting on behalf of malicious websites. Non-browser clients have the ability to add whatever headers they please, and requiring them to pretend to be a browser does not enhance anyone's security.)

babelouest commented 2 years ago

Hello @mvduin ,

If I summarize, you are referring to the Origin header sent in the websocket client framework, which currently send the same value as the server address.

Indeed that's not correct and thanks for pointing out.

The Websocket RFC states "Servers that are not intended to process input from any web page but only for certain sites SHOULD verify the |Origin| field is an origin they expect.", which, as I understand it says that the Origin header sent by the client should be specified by the program calling.

I can make the following change, tell me what you think about that:

struct _u_request request;
ulfius_init_request(&request);

u_map_put(request.map_header, "Origin", "http://client.example.com");
ulfius_set_websocket_request(&request, "ws://server.example.com/ws", "protocol", "permessage-deflate");
ulfius_open_websocket_client_connection(&request, &websocket_manager_callback, websocket_user_data, &websocket_incoming_message_callback, websocket_user_data, &websocket_onclose_callback, websocket_user_data, &websocket_client_handler, &response);
mvduin commented 2 years ago

It also says The request MUST include a header field with the name |Origin| [RFC6454] if the request is coming from a browser client. If the connection is from a non-browser client, the request MAY include this header field if the semantics of that client match the use-case described here for browser clients. and similarly elsewhere This header field is sent by browser clients; for non-browser clients, this header field may be sent if it makes sense in the context of those clients. and Additionally, if the client is a web browser, it supplies /origin/. ... I wish it spelled out more explicitly that an Origin-check should only be performed if an Origin header is present, but unfortunately it doesn't, although it does say that A connection attempt lacking this header field SHOULD NOT be interpreted as coming from a browser client. and it also explains the Origin-check is only a useful protection mechanism in the case of browser clients. I'm pretty sure I've seen it spelled out more explicitly somewhere but I don't remember where.

Regardless, if a particular Origin header is required for some websocket server, it'll still need to be up to the user to specify what it needs to be, there's no reliable way to guess it.

Your proprosal sounds fine.

BTW, don't forget that conversely, a websocket server should almost certainly check that all origins listed by an Origin header (if present) are trusted, either by matching against a listed of trusted origins specified by the user or using a callback to validate each origin listed (an Origin header is allowed to list multiple). If the user fails to specify any trusted origins, then only connections lacking an Origin header should be accepted. Failing to check Origin is almost certainly a security vulnerability, since web browsers allow any website to establish a connection to any websocket server (on any address, including on the user's local network, and any port, apart from the obnoxious restriction that only https websites can establish wss connections while http websites can establish both ws and wss connections). This is how the websocket protocol was designed: it makes very very sure that the server is a websocket server and understands the protocol it's negotiating (using Sec-WebSocket-Accept) since it implies consent to receive connections from any website in the world. The websocket server is expected to check Origin to prevent untrusted websites from connecting, and the browser fully relies on that.

babelouest commented 2 years ago

Si do you think my solution above is correct, or do you have another solution to discuss?

babelouest commented 2 years ago

Your proprosal sounds fine.

Sorry I didn't read that line at first, early in the morning... I'll make the change then

babelouest commented 2 years ago

Hello @mvduin ,

Can you test with the following commit: https://github.com/babelouest/ulfius/commit/be116c8176f9f2f82cd4375549531f88ebdf3c1c and see if it fits your needs and the specs?

mvduin commented 2 years ago

Looks good to me