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))
>
