Re: Patch: relayd support for HTTP 101 Switching Protocols
Hi,
thanks for bringing this to my attention, i've commited
my latest diff.
/Benno
Daniel Lamando(d...@danopia.net) on 2019.02.28 21:09:35 -0800:
> Hi all,
>
> I noticed that relayd doesn't support Websocket connections.
> When a Websocket request is forwarded through relayd,
> the handshake completes ok, but the backend never
> receives further packets sent from the client.
>
> I found several messages in the openbsd-misc archive
> mentioning websockets not working with relayd:
> - https://marc.info/?l=openbsd-misc=152510591921674
> - https://marc.info/?l=openbsd-misc=152558941423236
> - https://marc.info/?l=openbsd-misc=153997265632162
>
> After examining conversations and the relayd source I've traced
> this to the fact that Websockets negotiate using the GET method.
> relayd does not currently forward any request body upstream
> after forwarding a GET message from the client:
>
> case HTTP_METHOD_GET:
> cre->toread = 0;
>
> I found that relayd already supports bidirectional client<->server
> communication when the HTTP CONNECT method is sent by
> the client. Websockets are signalled slightly differently.
> The client includes a 'Connection: Upgrade' header, and
> the server returns an HTTP 101 response body.
> There are also other websocket-specific headers but they
> do not concern us as a proxy server.
>
> The Websocket handshake is completed by the server sending:
> HTTP/1.1 101 Switching Protocols
> According to RFC 2616's section on HTTP 101:
> The server will switch protocols to those defined by the response's
> Upgrade header field immediately after the empty line which
> terminates the 101 response.
>
> I've attached a small patch for relayd which re-enables
> client->server forwarding when an HTTP 101 is passed down.
> Applying this patch over OpenBSD 6.5-beta allows relayd to
> pass bidirectional websocket data, fixing my chat app :)
>
> PS: I???ve also tested relayd by relaying the example server from here:
> https://www.websocket.org/echo.html
> If you try that, note that the server is strict about the `Connection???
> request header being preserved. Otherwise it will 400.
>
>
> Index: relay_http.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 relay_http.c
> --- relay_http.c 6 Aug 2018 17:31:31 - 1.71
> +++ relay_http.c 1 Mar 2019 04:52:26 -
> @@ -276,6 +276,13 @@ relay_read_http(struct bufferevent *bev,
> DPRINTF("http_version %s http_rescode %s "
> "http_resmesg %s", desc->http_version,
> desc->http_rescode, desc->http_resmesg);
> +
> + /* HTTP 101 Switching Protocols */
> + if (desc->http_status == 101) {
> + cre->dst->toread = TOREAD_UNLIMITED;
> + cre->dst->bev->readcb = relay_read;
> + }
> +
> goto lookup;
> } else if (cre->line == 1 && cre->dir == RELAY_DIR_REQUEST) {
> if ((desc->http_method = relay_httpmethod_byname(key))
>
Re: Patch: relayd support for HTTP 101 Switching Protocols
On Mon, Mar 04, 2019 at 07:53:04PM +0100, Sebastian Benoit wrote:
> > The RFC says it must be a GET request. We should check at least
> > this. If we check more, an attacker can create less dubious states.
>
> thx, I was looking for something like that and could not find it.
> Where?
RFC 6455, The WebSocket Protocol, Page 16
2. The method of the request MUST be GET, and the HTTP version MUST
be at least 1.1.
There are a lot of other MUST, but let's only do the obvious and
easy ones now. Can be extended later.
> (relayd_101Switching_policy4.diff)
OK bluhm@
> diff --git usr.sbin/relayd/http.h usr.sbin/relayd/http.h
> index 052bc0ce326..135ca5bbcb7 100644
> --- usr.sbin/relayd/http.h
> +++ usr.sbin/relayd/http.h
> @@ -251,4 +251,10 @@ struct http_descriptor {
> struct kvtreehttp_headers;
> };
>
> +struct relay_http_priv {
> +#define HTTP_CONNECTION_UPGRADE 0x01
> +#define HTTP_UPGRADE_WEBSOCKET 0x02
> + int http_upgrade_req;
> +};
> +
> #endif /* HTTP_H */
> diff --git usr.sbin/relayd/parse.y usr.sbin/relayd/parse.y
> index 9875973fd80..66a568d5e62 100644
> --- usr.sbin/relayd/parse.y
> +++ usr.sbin/relayd/parse.y
> @@ -176,6 +176,7 @@ typedef struct {
> %token TO ROUTER RTLABEL TRANSPARENT TRAP UPDATES URL VIRTUAL WITH TTL
> RTABLE
> %token MATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE
> PASSWORD ECDHE
> %token EDH TICKETS CONNECTION CONNECTIONS ERRORS STATE CHANGES CHECKS
> +%token WEBSOCKETS
> %token STRING
> %token NUMBER
> %type hostname interface table value optstring
> @@ -1064,8 +1065,20 @@ protoptsl : ssltls tlsflags
> | ssltls '{' tlsflags_l '}'
> | TCP tcpflags
> | TCP '{' tcpflags_l '}'
> - | HTTP httpflags
> - | HTTP '{' httpflags_l '}'
> + | HTTP {
> + if (proto->type != RELAY_PROTO_HTTP) {
> + yyerror("can set http options only for "
> + "http protocol");
> + YYERROR;
> + }
> + } httpflags
> + | HTTP {
> + if (proto->type != RELAY_PROTO_HTTP) {
> + yyerror("can set http options only for "
> + "http protocol");
> + YYERROR;
> + }
> + } '{' httpflags_l '}'
> | RETURN ERROR opteflags{ proto->flags |= F_RETURN; }
> | RETURN ERROR '{' eflags_l '}' { proto->flags |= F_RETURN; }
> | filterrule
> @@ -1078,17 +1091,14 @@ httpflags_l : httpflags comma httpflags_l
> ;
>
> httpflags: HEADERLEN NUMBER {
> - if (proto->type != RELAY_PROTO_HTTP) {
> - yyerror("can set http options only for "
> - "http protocol");
> - YYERROR;
> - }
> if ($2 < 0 || $2 > RELAY_MAXHEADERLENGTH) {
> yyerror("invalid headerlen: %d", $2);
> YYERROR;
> }
> proto->httpheaderlen = $2;
> }
> + | WEBSOCKETS{ proto->httpflags |= HTTPFLAG_WEBSOCKETS; }
> + | NO WEBSOCKETS { proto->httpflags &= ~HTTPFLAG_WEBSOCKETS; }
> ;
>
> tcpflags_l : tcpflags comma tcpflags_l
> @@ -2338,6 +2348,7 @@ lookup(char *s)
> { "url",URL },
> { "value", VALUE },
> { "virtual",VIRTUAL },
> + { "websockets", WEBSOCKETS },
> { "with", WITH }
> };
> const struct keywords *p;
> diff --git usr.sbin/relayd/relay.c usr.sbin/relayd/relay.c
> index 739c9226b65..bf662b9a1df 100644
> --- usr.sbin/relayd/relay.c
> +++ usr.sbin/relayd/relay.c
> @@ -1410,7 +1410,13 @@ relay_session(struct rsession *con)
> return;
> }
>
> - if (rlay->rl_proto->type != RELAY_PROTO_HTTP) {
> + if (rlay->rl_proto->type == RELAY_PROTO_HTTP) {
> + if (relay_http_priv_init(con) == -1) {
> + relay_close(con,
> + "failed to allocate http session data", 1);
> + return;
> + }
> + } else {
> if (rlay->rl_conf.fwdmode == FWD_TRANS)
> relay_bindanyreq(con, 0, IPPROTO_TCP);
> else if (relay_connect(con) == -1) {
> diff --git usr.sbin/relayd/relay_http.c usr.sbin/relayd/relay_http.c
> index a9d27bfe605..e431d2b8f44 100644
> --- usr.sbin/relayd/relay_http.c
> +++ usr.sbin/relayd/relay_http.c
> @@ -109,6 +109,17 @@ relay_http_init(struct relay *rlay)
>
Re: Patch: relayd support for HTTP 101 Switching Protocols
Alexander Bluhm(alexander.bl...@gmx.net) on 2019.03.04 17:44:08 +0100:
> On Sat, Mar 02, 2019 at 12:13:20AM +0100, Sebastian Benoit wrote:
> > --- usr.sbin/relayd/parse.y
> > +++ usr.sbin/relayd/parse.y
> > @@ -176,6 +176,7 @@ typedef struct {
> > %token TO ROUTER RTLABEL TRANSPARENT TRAP UPDATES URL VIRTUAL WITH TTL
> > RTABLE
> > %token MATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE
> > PASSWORD ECDHE
> > %token EDH TICKETS CONNECTION CONNECTIONS ERRORS STATE CHANGES CHECKS
> > +%token WEBSOCKETS
> > %token STRING
> > %token NUMBER
> > %typehostname interface table value optstring
>
> Here are some tab vs space incosistencies.
fixed.
> > + if (strcasecmp("Connection", key) == 0 &&
> > + strcasecmp("Upgrade", value) == 0)
> > + priv->http_upgrade_req |=
> > + HTTP_UPGRADE_CONN;
>
> Would the name HTTP_CONNECTION_UPGRADE be better? Perhaps we want
> to do something spcific with connection keep-alive or close some
> day.
changed.
> > @@ -422,6 +445,28 @@ relay_read_http(struct bufferevent *bev, void *arg)
> > return;
> > }
> >
> > + /* HTTP 101 Switching Protocols */
> > + if (cre->dir == RELAY_DIR_REQUEST) {
> > + if (!(proto->httpflags & HTTPFLAG_WEBSOCKETS) &&
> > + (priv->http_upgrade_req &
> > + HTTP_UPGRADE_WEBSOCKET)) {
>
> Should we check for both upgrade and websocket?
i am now checking:
1. 'Upgrade: websocket' hdr AND NOT "http { websockets }" --> 403
2. 'Upgrade: websocket' hdr AND NO 'Connection: upgrade' hdr --> 400
3. 'Upgrade: websocket' hdr AND method NOT "GET" --> 405
> > + relay_abort_http(con, 403,
> > + "Forbidden", 0);
>
> A more specific error message like "Websocket Forbidden" would make
> debugging much easier.
done in all cases
> The RFC says it must be a GET request. We should check at least
> this. If we check more, an attacker can create less dubious states.
thx, I was looking for something like that and could not find it.
Where?
Implemented.
> > + return;
> > + }
> > + } else if (cre->dir == RELAY_DIR_RESPONSE &&
> > + desc->http_status == 101) {
> > + if ((priv->http_upgrade_req &
> > + (HTTP_UPGRADE_CONN | HTTP_UPGRADE_WEBSOCKET)) &&
>
> Both flags must be present.
oops. fixed.
>
> > + proto->httpflags & HTTPFLAG_WEBSOCKETS) {
> > + cre->dst->toread = TOREAD_UNLIMITED;
> > + cre->dst->bev->readcb = relay_read;
> > + } else {
> > + relay_abort_http(con, 502, "Bad Gateway", 0);
>
> Could we have a more specific message like "Bad Websocket Gateway"?
>
> > + return;
> > + }
> > + }
> > +
> > switch (desc->http_method) {
> > case HTTP_METHOD_CONNECT:
> > /* Data stream */
>
> bluhm
>
(relayd_101Switching_policy4.diff)
diff --git usr.sbin/relayd/http.h usr.sbin/relayd/http.h
index 052bc0ce326..135ca5bbcb7 100644
--- usr.sbin/relayd/http.h
+++ usr.sbin/relayd/http.h
@@ -251,4 +251,10 @@ struct http_descriptor {
struct kvtreehttp_headers;
};
+struct relay_http_priv {
+#define HTTP_CONNECTION_UPGRADE0x01
+#define HTTP_UPGRADE_WEBSOCKET 0x02
+ int http_upgrade_req;
+};
+
#endif /* HTTP_H */
diff --git usr.sbin/relayd/parse.y usr.sbin/relayd/parse.y
index 9875973fd80..66a568d5e62 100644
--- usr.sbin/relayd/parse.y
+++ usr.sbin/relayd/parse.y
@@ -176,6 +176,7 @@ typedef struct {
%token TO ROUTER RTLABEL TRANSPARENT TRAP UPDATES URL VIRTUAL WITH TTL RTABLE
%token MATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE PASSWORD ECDHE
%token EDH TICKETS CONNECTION CONNECTIONS ERRORS STATE CHANGES CHECKS
+%token WEBSOCKETS
%token STRING
%token NUMBER
%typehostname interface table value optstring
@@ -1064,8 +1065,20 @@ protoptsl: ssltls tlsflags
| ssltls '{' tlsflags_l '}'
| TCP tcpflags
| TCP '{' tcpflags_l '}'
- | HTTP httpflags
- | HTTP '{' httpflags_l '}'
+ | HTTP {
+ if (proto->type != RELAY_PROTO_HTTP) {
+ yyerror("can set http options only for "
+ "http protocol");
+ YYERROR;
+ }
+ } httpflags
+ | HTTP {
+ if (proto->type != RELAY_PROTO_HTTP) {
+ yyerror("can set http options only for "
+
Re: Patch: relayd support for HTTP 101 Switching Protocols
On Sat, Mar 02, 2019 at 12:13:20AM +0100, Sebastian Benoit wrote:
> --- usr.sbin/relayd/parse.y
> +++ usr.sbin/relayd/parse.y
> @@ -176,6 +176,7 @@ typedef struct {
> %token TO ROUTER RTLABEL TRANSPARENT TRAP UPDATES URL VIRTUAL WITH TTL
> RTABLE
> %token MATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE
> PASSWORD ECDHE
> %token EDH TICKETS CONNECTION CONNECTIONS ERRORS STATE CHANGES CHECKS
> +%token WEBSOCKETS
> %token STRING
> %token NUMBER
> %type hostname interface table value optstring
Here are some tab vs space incosistencies.
> + if (strcasecmp("Connection", key) == 0 &&
> + strcasecmp("Upgrade", value) == 0)
> + priv->http_upgrade_req |=
> + HTTP_UPGRADE_CONN;
Would the name HTTP_CONNECTION_UPGRADE be better? Perhaps we want
to do something spcific with connection keep-alive or close some
day.
> @@ -422,6 +445,28 @@ relay_read_http(struct bufferevent *bev, void *arg)
> return;
> }
>
> + /* HTTP 101 Switching Protocols */
> + if (cre->dir == RELAY_DIR_REQUEST) {
> + if (!(proto->httpflags & HTTPFLAG_WEBSOCKETS) &&
> + (priv->http_upgrade_req &
> + HTTP_UPGRADE_WEBSOCKET)) {
Should we check for both upgrade and websocket?
> + relay_abort_http(con, 403,
> + "Forbidden", 0);
A more specific error message like "Websocket Forbidden" would make
debugging much easier.
The RFC says it must be a GET request. We should check at least
this. If we check more, an attacker can create less dubious states.
> + return;
> + }
> + } else if (cre->dir == RELAY_DIR_RESPONSE &&
> + desc->http_status == 101) {
> + if ((priv->http_upgrade_req &
> + (HTTP_UPGRADE_CONN | HTTP_UPGRADE_WEBSOCKET)) &&
Both flags must be present.
> + proto->httpflags & HTTPFLAG_WEBSOCKETS) {
> + cre->dst->toread = TOREAD_UNLIMITED;
> + cre->dst->bev->readcb = relay_read;
> + } else {
> + relay_abort_http(con, 502, "Bad Gateway", 0);
Could we have a more specific message like "Bad Websocket Gateway"?
> + return;
> + }
> + }
> +
> switch (desc->http_method) {
> case HTTP_METHOD_CONNECT:
> /* Data stream */
bluhm
Re: Patch: relayd support for HTTP 101 Switching Protocols
Sebastian Benoit(be...@openbsd.org) on 2019.03.02 00:13:20 +0100:
> Hi,
>
> Alexander Bluhm(alexander.bl...@gmx.net) on 2019.03.01 11:40:05 +0100:
> > On Fri, Mar 01, 2019 at 09:37:42AM +0100, Sebastian Benoit wrote:
> > > i think its ok to add this, and i would like to commit. Maybe we would
> > > want
> > > some filter option to disallow this?
> >
> > I am not sure whether enabling websocket by default is a good idea.
> > Our commercial firewall removes the upgrade command by default.
> > Only if the admin explicitly enables the feature, websocket traffic
> > is allowed. I don't know enough relayd use cases to decide what
> > should be implemented here.
>
> This diff adds an option
>
> protocol ...
> http { [no] websockets }
>
> with which websockets are allowd. The default is no websockets.
>
> > Regarding the patch, I think we should not look only at the server
> > response. Only if both the client requests "Connection: Upgrade"
> > and the server responds "101 Switching Protocols", do the switch
> > to relay_read.
>
> In addition, this diff checks if the client sends
>
> Connection: Upgrade
> Upgrade: websocket
>
> If it does, and "http { websockets }" has not been set, it will
> respond to the Client with 403 Forbidden.
>
> If a Server responds with Status Code 101 and the client did not send
> Upgrade: websocket, relayd will send a "502 Bad Gateway" to the
> client.
>
> One could do more checks, for example check the validity of the
> Sec-WebSocket-Key and ec-WebSocket-Accept headers. I'm not doing that yet.
>
> Comments, Oks?
>
> /Benno
I forgot to mention, i have tested this against
https://www.websocket.org/echo.html
and run the regression tests.
/B.
> > If we want to disable websockets properly, we should filter the
> > client header and server reponse.
> >
> > bluhm
> >
> > > Daniel Lamando(d...@danopia.net) on 2019.02.28 21:09:35 -0800:
> > > > Hi all,
> > > >
> > > > I noticed that relayd doesn't support Websocket connections.
> > > > When a Websocket request is forwarded through relayd,
> > > > the handshake completes ok, but the backend never
> > > > receives further packets sent from the client.
> > > >
> > > > I found several messages in the openbsd-misc archive
> > > > mentioning websockets not working with relayd:
> > > > - https://marc.info/?l=openbsd-misc=152510591921674
> > > > - https://marc.info/?l=openbsd-misc=152558941423236
> > > > - https://marc.info/?l=openbsd-misc=153997265632162
> > > >
> > > > After examining conversations and the relayd source I've traced
> > > > this to the fact that Websockets negotiate using the GET method.
> > > > relayd does not currently forward any request body upstream
> > > > after forwarding a GET message from the client:
> > > >
> > > > case HTTP_METHOD_GET:
> > > > cre->toread = 0;
> > > >
> > > > I found that relayd already supports bidirectional client<->server
> > > > communication when the HTTP CONNECT method is sent by
> > > > the client. Websockets are signalled slightly differently.
> > > > The client includes a 'Connection: Upgrade' header, and
> > > > the server returns an HTTP 101 response body.
> > > > There are also other websocket-specific headers but they
> > > > do not concern us as a proxy server.
> > > >
> > > > The Websocket handshake is completed by the server sending:
> > > > HTTP/1.1 101 Switching Protocols
> > > > According to RFC 2616's section on HTTP 101:
> > > > The server will switch protocols to those defined by the
> > > > response's
> > > > Upgrade header field immediately after the empty line which
> > > > terminates the 101 response.
> > > >
> > > > I've attached a small patch for relayd which re-enables
> > > > client->server forwarding when an HTTP 101 is passed down.
> > > > Applying this patch over OpenBSD 6.5-beta allows relayd to
> > > > pass bidirectional websocket data, fixing my chat app :)
> > > >
> > > > PS: I???ve also tested relayd by relaying the example server from here:
> > > > https://www.websocket.org/echo.html
> > > > If you try that, note that the server is strict about the `Connection???
> > > > request header being preserved. Otherwise it will 400.
>
(relayd_101Switching_policy3.diff)
diff --git usr.sbin/relayd/http.h usr.sbin/relayd/http.h
index 052bc0ce326..8be43c9a9cc 100644
--- usr.sbin/relayd/http.h
+++ usr.sbin/relayd/http.h
@@ -251,4 +251,11 @@ struct http_descriptor {
struct kvtreehttp_headers;
};
+struct relay_http_priv {
+#define HTTP_UPGRADE_CONN 0x01
+#define HTTP_UPGRADE_WEBSOCKET 0x02
+ int http_upgrade_req;
+ int http_upgrade_resp;
+};
+
#endif /* HTTP_H */
diff --git usr.sbin/relayd/parse.y usr.sbin/relayd/parse.y
index 9875973fd80..b7d7058266a 100644
--- usr.sbin/relayd/parse.y
+++ usr.sbin/relayd/parse.y
@@ -176,6 +176,7 @@ typedef struct {
%token TO ROUTER RTLABEL TRANSPARENT TRAP
Re: Patch: relayd support for HTTP 101 Switching Protocols
Hi,
Alexander Bluhm(alexander.bl...@gmx.net) on 2019.03.01 11:40:05 +0100:
> On Fri, Mar 01, 2019 at 09:37:42AM +0100, Sebastian Benoit wrote:
> > i think its ok to add this, and i would like to commit. Maybe we would want
> > some filter option to disallow this?
>
> I am not sure whether enabling websocket by default is a good idea.
> Our commercial firewall removes the upgrade command by default.
> Only if the admin explicitly enables the feature, websocket traffic
> is allowed. I don't know enough relayd use cases to decide what
> should be implemented here.
This diff adds an option
protocol ...
http { [no] websockets }
with which websockets are allowd. The default is no websockets.
> Regarding the patch, I think we should not look only at the server
> response. Only if both the client requests "Connection: Upgrade"
> and the server responds "101 Switching Protocols", do the switch
> to relay_read.
In addition, this diff checks if the client sends
Connection: Upgrade
Upgrade: websocket
If it does, and "http { websockets }" has not been set, it will
respond to the Client with 403 Forbidden.
If a Server responds with Status Code 101 and the client did not send
Upgrade: websocket, relayd will send a "502 Bad Gateway" to the
client.
One could do more checks, for example check the validity of the
Sec-WebSocket-Key and ec-WebSocket-Accept headers. I'm not doing that yet.
Comments, Oks?
/Benno
> If we want to disable websockets properly, we should filter the
> client header and server reponse.
>
> bluhm
>
> > Daniel Lamando(d...@danopia.net) on 2019.02.28 21:09:35 -0800:
> > > Hi all,
> > >
> > > I noticed that relayd doesn't support Websocket connections.
> > > When a Websocket request is forwarded through relayd,
> > > the handshake completes ok, but the backend never
> > > receives further packets sent from the client.
> > >
> > > I found several messages in the openbsd-misc archive
> > > mentioning websockets not working with relayd:
> > > - https://marc.info/?l=openbsd-misc=152510591921674
> > > - https://marc.info/?l=openbsd-misc=152558941423236
> > > - https://marc.info/?l=openbsd-misc=153997265632162
> > >
> > > After examining conversations and the relayd source I've traced
> > > this to the fact that Websockets negotiate using the GET method.
> > > relayd does not currently forward any request body upstream
> > > after forwarding a GET message from the client:
> > >
> > > case HTTP_METHOD_GET:
> > > cre->toread = 0;
> > >
> > > I found that relayd already supports bidirectional client<->server
> > > communication when the HTTP CONNECT method is sent by
> > > the client. Websockets are signalled slightly differently.
> > > The client includes a 'Connection: Upgrade' header, and
> > > the server returns an HTTP 101 response body.
> > > There are also other websocket-specific headers but they
> > > do not concern us as a proxy server.
> > >
> > > The Websocket handshake is completed by the server sending:
> > > HTTP/1.1 101 Switching Protocols
> > > According to RFC 2616's section on HTTP 101:
> > > The server will switch protocols to those defined by the response's
> > > Upgrade header field immediately after the empty line which
> > > terminates the 101 response.
> > >
> > > I've attached a small patch for relayd which re-enables
> > > client->server forwarding when an HTTP 101 is passed down.
> > > Applying this patch over OpenBSD 6.5-beta allows relayd to
> > > pass bidirectional websocket data, fixing my chat app :)
> > >
> > > PS: I???ve also tested relayd by relaying the example server from here:
> > > https://www.websocket.org/echo.html
> > > If you try that, note that the server is strict about the `Connection???
> > > request header being preserved. Otherwise it will 400.
(relayd_101Switching_policy3.diff)
diff --git usr.sbin/relayd/http.h usr.sbin/relayd/http.h
index 052bc0ce326..8be43c9a9cc 100644
--- usr.sbin/relayd/http.h
+++ usr.sbin/relayd/http.h
@@ -251,4 +251,11 @@ struct http_descriptor {
struct kvtreehttp_headers;
};
+struct relay_http_priv {
+#define HTTP_UPGRADE_CONN 0x01
+#define HTTP_UPGRADE_WEBSOCKET 0x02
+ int http_upgrade_req;
+ int http_upgrade_resp;
+};
+
#endif /* HTTP_H */
diff --git usr.sbin/relayd/parse.y usr.sbin/relayd/parse.y
index 9875973fd80..b7d7058266a 100644
--- usr.sbin/relayd/parse.y
+++ usr.sbin/relayd/parse.y
@@ -176,6 +176,7 @@ typedef struct {
%token TO ROUTER RTLABEL TRANSPARENT TRAP UPDATES URL VIRTUAL WITH TTL RTABLE
%token MATCH PARAMS RANDOM LEASTSTATES SRCHASH KEY CERTIFICATE PASSWORD ECDHE
%token EDH TICKETS CONNECTION CONNECTIONS ERRORS STATE CHANGES CHECKS
+%token WEBSOCKETS
%token STRING
%token NUMBER
%typehostname interface table value optstring
@@ -1064,8 +1065,20 @@ protoptsl: ssltls tlsflags
| ssltls '{' tlsflags_l '}'
| TCP tcpflags
Re: Patch: relayd support for HTTP 101 Switching Protocols
On Fri, Mar 01, 2019 at 09:37:42AM +0100, Sebastian Benoit wrote:
> i think its ok to add this, and i would like to commit. Maybe we would want
> some filter option to disallow this?
I am not sure whether enabling websocket by default is a good idea.
Our commercial firewall removes the upgrade command by default.
Only if the admin explicitly enables the feature, websocket traffic
is allowed. I don't know enough relayd use cases to decide what
should be implemented here.
Regarding the patch, I think we should not look only at the server
response. Only if both the client requests "Connection: Upgrade"
and the server responds "101 Switching Protocols", do the switch
to relay_read.
If we want to disable websockets properly, we should filter the
client header and server reponse.
bluhm
> Daniel Lamando(d...@danopia.net) on 2019.02.28 21:09:35 -0800:
> > Hi all,
> >
> > I noticed that relayd doesn't support Websocket connections.
> > When a Websocket request is forwarded through relayd,
> > the handshake completes ok, but the backend never
> > receives further packets sent from the client.
> >
> > I found several messages in the openbsd-misc archive
> > mentioning websockets not working with relayd:
> > - https://marc.info/?l=openbsd-misc=152510591921674
> > - https://marc.info/?l=openbsd-misc=152558941423236
> > - https://marc.info/?l=openbsd-misc=153997265632162
> >
> > After examining conversations and the relayd source I've traced
> > this to the fact that Websockets negotiate using the GET method.
> > relayd does not currently forward any request body upstream
> > after forwarding a GET message from the client:
> >
> > case HTTP_METHOD_GET:
> > cre->toread = 0;
> >
> > I found that relayd already supports bidirectional client<->server
> > communication when the HTTP CONNECT method is sent by
> > the client. Websockets are signalled slightly differently.
> > The client includes a 'Connection: Upgrade' header, and
> > the server returns an HTTP 101 response body.
> > There are also other websocket-specific headers but they
> > do not concern us as a proxy server.
> >
> > The Websocket handshake is completed by the server sending:
> > HTTP/1.1 101 Switching Protocols
> > According to RFC 2616's section on HTTP 101:
> > The server will switch protocols to those defined by the response's
> > Upgrade header field immediately after the empty line which
> > terminates the 101 response.
> >
> > I've attached a small patch for relayd which re-enables
> > client->server forwarding when an HTTP 101 is passed down.
> > Applying this patch over OpenBSD 6.5-beta allows relayd to
> > pass bidirectional websocket data, fixing my chat app :)
> >
> > PS: I???ve also tested relayd by relaying the example server from here:
> > https://www.websocket.org/echo.html
> > If you try that, note that the server is strict about the `Connection???
> > request header being preserved. Otherwise it will 400.
> >
> >
> > Index: relay_http.c
> > ===
> > RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
> > retrieving revision 1.71
> > diff -u -p -r1.71 relay_http.c
> > --- relay_http.c6 Aug 2018 17:31:31 - 1.71
> > +++ relay_http.c1 Mar 2019 04:52:26 -
> > @@ -276,6 +276,13 @@ relay_read_http(struct bufferevent *bev,
> > DPRINTF("http_version %s http_rescode %s "
> > "http_resmesg %s", desc->http_version,
> > desc->http_rescode, desc->http_resmesg);
> > +
> > + /* HTTP 101 Switching Protocols */
> > + if (desc->http_status == 101) {
> > + cre->dst->toread = TOREAD_UNLIMITED;
> > + cre->dst->bev->readcb = relay_read;
> > + }
> > +
> > goto lookup;
> > } else if (cre->line == 1 && cre->dir == RELAY_DIR_REQUEST) {
> > if ((desc->http_method = relay_httpmethod_byname(key))
> >
Re: Patch: relayd support for HTTP 101 Switching Protocols
On Fri, Mar 01, 2019 at 09:37:42AM +0100, Sebastian Benoit wrote:
> Hi,
>
> i think its ok to add this, and i would like to commit. Maybe we would want
> some filter option to disallow this?
I agree to both. OK claudio@ if the relayd regress suite still passes with
it.
> Daniel Lamando(d...@danopia.net) on 2019.02.28 21:09:35 -0800:
> > Hi all,
> >
> > I noticed that relayd doesn't support Websocket connections.
> > When a Websocket request is forwarded through relayd,
> > the handshake completes ok, but the backend never
> > receives further packets sent from the client.
> >
> > I found several messages in the openbsd-misc archive
> > mentioning websockets not working with relayd:
> > - https://marc.info/?l=openbsd-misc=152510591921674
> > - https://marc.info/?l=openbsd-misc=152558941423236
> > - https://marc.info/?l=openbsd-misc=153997265632162
> >
> > After examining conversations and the relayd source I've traced
> > this to the fact that Websockets negotiate using the GET method.
> > relayd does not currently forward any request body upstream
> > after forwarding a GET message from the client:
> >
> > case HTTP_METHOD_GET:
> > cre->toread = 0;
> >
> > I found that relayd already supports bidirectional client<->server
> > communication when the HTTP CONNECT method is sent by
> > the client. Websockets are signalled slightly differently.
> > The client includes a 'Connection: Upgrade' header, and
> > the server returns an HTTP 101 response body.
> > There are also other websocket-specific headers but they
> > do not concern us as a proxy server.
> >
> > The Websocket handshake is completed by the server sending:
> > HTTP/1.1 101 Switching Protocols
> > According to RFC 2616's section on HTTP 101:
> > The server will switch protocols to those defined by the response's
> > Upgrade header field immediately after the empty line which
> > terminates the 101 response.
> >
> > I've attached a small patch for relayd which re-enables
> > client->server forwarding when an HTTP 101 is passed down.
> > Applying this patch over OpenBSD 6.5-beta allows relayd to
> > pass bidirectional websocket data, fixing my chat app :)
> >
> > PS: I???ve also tested relayd by relaying the example server from here:
> > https://www.websocket.org/echo.html
> > If you try that, note that the server is strict about the `Connection???
> > request header being preserved. Otherwise it will 400.
> >
> >
> > Index: relay_http.c
> > ===
> > RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
> > retrieving revision 1.71
> > diff -u -p -r1.71 relay_http.c
> > --- relay_http.c6 Aug 2018 17:31:31 - 1.71
> > +++ relay_http.c1 Mar 2019 04:52:26 -
> > @@ -276,6 +276,13 @@ relay_read_http(struct bufferevent *bev,
> > DPRINTF("http_version %s http_rescode %s "
> > "http_resmesg %s", desc->http_version,
> > desc->http_rescode, desc->http_resmesg);
> > +
> > + /* HTTP 101 Switching Protocols */
> > + if (desc->http_status == 101) {
> > + cre->dst->toread = TOREAD_UNLIMITED;
> > + cre->dst->bev->readcb = relay_read;
> > + }
> > +
> > goto lookup;
> > } else if (cre->line == 1 && cre->dir == RELAY_DIR_REQUEST) {
> > if ((desc->http_method = relay_httpmethod_byname(key))
> >
--
:wq Claudio
Re: Patch: relayd support for HTTP 101 Switching Protocols
Hi,
i think its ok to add this, and i would like to commit. Maybe we would want
some filter option to disallow this?
/Benno
Daniel Lamando(d...@danopia.net) on 2019.02.28 21:09:35 -0800:
> Hi all,
>
> I noticed that relayd doesn't support Websocket connections.
> When a Websocket request is forwarded through relayd,
> the handshake completes ok, but the backend never
> receives further packets sent from the client.
>
> I found several messages in the openbsd-misc archive
> mentioning websockets not working with relayd:
> - https://marc.info/?l=openbsd-misc=152510591921674
> - https://marc.info/?l=openbsd-misc=152558941423236
> - https://marc.info/?l=openbsd-misc=153997265632162
>
> After examining conversations and the relayd source I've traced
> this to the fact that Websockets negotiate using the GET method.
> relayd does not currently forward any request body upstream
> after forwarding a GET message from the client:
>
> case HTTP_METHOD_GET:
> cre->toread = 0;
>
> I found that relayd already supports bidirectional client<->server
> communication when the HTTP CONNECT method is sent by
> the client. Websockets are signalled slightly differently.
> The client includes a 'Connection: Upgrade' header, and
> the server returns an HTTP 101 response body.
> There are also other websocket-specific headers but they
> do not concern us as a proxy server.
>
> The Websocket handshake is completed by the server sending:
> HTTP/1.1 101 Switching Protocols
> According to RFC 2616's section on HTTP 101:
> The server will switch protocols to those defined by the response's
> Upgrade header field immediately after the empty line which
> terminates the 101 response.
>
> I've attached a small patch for relayd which re-enables
> client->server forwarding when an HTTP 101 is passed down.
> Applying this patch over OpenBSD 6.5-beta allows relayd to
> pass bidirectional websocket data, fixing my chat app :)
>
> PS: I???ve also tested relayd by relaying the example server from here:
> https://www.websocket.org/echo.html
> If you try that, note that the server is strict about the `Connection???
> request header being preserved. Otherwise it will 400.
>
>
> Index: relay_http.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
> retrieving revision 1.71
> diff -u -p -r1.71 relay_http.c
> --- relay_http.c 6 Aug 2018 17:31:31 - 1.71
> +++ relay_http.c 1 Mar 2019 04:52:26 -
> @@ -276,6 +276,13 @@ relay_read_http(struct bufferevent *bev,
> DPRINTF("http_version %s http_rescode %s "
> "http_resmesg %s", desc->http_version,
> desc->http_rescode, desc->http_resmesg);
> +
> + /* HTTP 101 Switching Protocols */
> + if (desc->http_status == 101) {
> + cre->dst->toread = TOREAD_UNLIMITED;
> + cre->dst->bev->readcb = relay_read;
> + }
> +
> goto lookup;
> } else if (cre->line == 1 && cre->dir == RELAY_DIR_REQUEST) {
> if ((desc->http_method = relay_httpmethod_byname(key))
>
Patch: relayd support for HTTP 101 Switching Protocols
Hi all,
I noticed that relayd doesn't support Websocket connections.
When a Websocket request is forwarded through relayd,
the handshake completes ok, but the backend never
receives further packets sent from the client.
I found several messages in the openbsd-misc archive
mentioning websockets not working with relayd:
- https://marc.info/?l=openbsd-misc=152510591921674
- https://marc.info/?l=openbsd-misc=152558941423236
- https://marc.info/?l=openbsd-misc=153997265632162
After examining conversations and the relayd source I've traced
this to the fact that Websockets negotiate using the GET method.
relayd does not currently forward any request body upstream
after forwarding a GET message from the client:
case HTTP_METHOD_GET:
cre->toread = 0;
I found that relayd already supports bidirectional client<->server
communication when the HTTP CONNECT method is sent by
the client. Websockets are signalled slightly differently.
The client includes a 'Connection: Upgrade' header, and
the server returns an HTTP 101 response body.
There are also other websocket-specific headers but they
do not concern us as a proxy server.
The Websocket handshake is completed by the server sending:
HTTP/1.1 101 Switching Protocols
According to RFC 2616's section on HTTP 101:
The server will switch protocols to those defined by the response's
Upgrade header field immediately after the empty line which
terminates the 101 response.
I've attached a small patch for relayd which re-enables
client->server forwarding when an HTTP 101 is passed down.
Applying this patch over OpenBSD 6.5-beta allows relayd to
pass bidirectional websocket data, fixing my chat app :)
PS: I’ve also tested relayd by relaying the example server from here:
https://www.websocket.org/echo.html
If you try that, note that the server is strict about the `Connection’
request header being preserved. Otherwise it will 400.
Index: relay_http.c
===
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.71
diff -u -p -r1.71 relay_http.c
--- relay_http.c6 Aug 2018 17:31:31 - 1.71
+++ relay_http.c1 Mar 2019 04:52:26 -
@@ -276,6 +276,13 @@ relay_read_http(struct bufferevent *bev,
DPRINTF("http_version %s http_rescode %s "
"http_resmesg %s", desc->http_version,
desc->http_rescode, desc->http_resmesg);
+
+ /* HTTP 101 Switching Protocols */
+ if (desc->http_status == 101) {
+ cre->dst->toread = TOREAD_UNLIMITED;
+ cre->dst->bev->readcb = relay_read;
+ }
+
goto lookup;
} else if (cre->line == 1 && cre->dir == RELAY_DIR_REQUEST) {
if ((desc->http_method = relay_httpmethod_byname(key))
