Sebastian Benoit([email protected]) on 2019.03.02 00:13:20 +0100: > Hi, > > Alexander Bluhm([email protected]) 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([email protected]) 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&m=152510591921674 > > > > - https://marc.info/?l=openbsd-misc&m=152558941423236 > > > > - https://marc.info/?l=openbsd-misc&m=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 kvtree http_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 <v.string> STRING %token <v.number> NUMBER %type <v.string> 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..0dca0cb72bd 100644 --- usr.sbin/relayd/relay_http.c +++ usr.sbin/relayd/relay_http.c @@ -109,6 +109,17 @@ relay_http_init(struct relay *rlay) relay_calc_skip_steps(&rlay->rl_proto->rules); } +int +relay_http_priv_init(struct rsession *con) +{ + struct relay_http_priv *p; + if ((p = calloc(1, sizeof(struct relay_http_priv))) == NULL) + return (-1); + + con->se_priv = p; + return (0); +} + int relay_httpdesc_init(struct ctl_relay_event *cre) { @@ -152,6 +163,7 @@ relay_read_http(struct bufferevent *bev, void *arg) struct relay *rlay = con->se_relay; struct protocol *proto = rlay->rl_proto; struct evbuffer *src = EVBUFFER_INPUT(bev); + struct relay_http_priv *priv = con->se_priv; char *line = NULL, *key, *value; char *urlproto, *host, *path; int action, unique, ret; @@ -386,6 +398,17 @@ relay_read_http(struct bufferevent *bev, void *arg) unique = 0; if (cre->line != 1) { + if (cre->dir == RELAY_DIR_REQUEST) { + if (strcasecmp("Connection", key) == 0 && + strcasecmp("Upgrade", value) == 0) + priv->http_upgrade_req |= + HTTP_UPGRADE_CONN; + if (strcasecmp("Upgrade", key) == 0 && + strcasecmp("websocket", value) == 0) + priv->http_upgrade_req |= + HTTP_UPGRADE_WEBSOCKET; + } + if ((hdr = kv_add(&desc->http_headers, key, value, unique)) == NULL) { relay_abort_http(con, 400, @@ -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)) { + relay_abort_http(con, 403, + "Forbidden", 0); + return; + } + } else if (cre->dir == RELAY_DIR_RESPONSE && + desc->http_status == 101) { + if ((priv->http_upgrade_req & + (HTTP_UPGRADE_CONN | HTTP_UPGRADE_WEBSOCKET)) && + proto->httpflags & HTTPFLAG_WEBSOCKETS) { + cre->dst->toread = TOREAD_UNLIMITED; + cre->dst->bev->readcb = relay_read; + } else { + relay_abort_http(con, 502, "Bad Gateway", 0); + return; + } + } + switch (desc->http_method) { case HTTP_METHOD_CONNECT: /* Data stream */ diff --git usr.sbin/relayd/relayd.conf.5 usr.sbin/relayd/relayd.conf.5 index d43ffa06627..5a651784060 100644 --- usr.sbin/relayd/relayd.conf.5 +++ usr.sbin/relayd/relayd.conf.5 @@ -1006,6 +1006,10 @@ Valid options are: .It Ic headerlen Ar number Set the maximum size of all HTTP headers in bytes. The default value is 8192 and it is limited to a maximum of 131072. +.It Ic websockets +Allow connection upgrade to websocket protocol. +The default is +.Ic no websockets . .El .El .Sh FILTER RULES diff --git usr.sbin/relayd/relayd.h usr.sbin/relayd/relayd.h index fe55c3a8478..121aa5bd13e 100644 --- usr.sbin/relayd/relayd.h +++ usr.sbin/relayd/relayd.h @@ -702,6 +702,8 @@ struct relay_ticket_key { }; #define TLS_SESSION_LIFETIME (2 * 3600) +#define HTTPFLAG_WEBSOCKETS 0x01 + struct protocol { objid_t id; u_int32_t flags; @@ -711,6 +713,7 @@ struct protocol { u_int8_t tcpipttl; u_int8_t tcpipminttl; size_t httpheaderlen; + int httpflags; u_int8_t tlsflags; char tlsciphers[768]; char tlsdhparams[128]; @@ -1227,6 +1230,7 @@ const char *relay_httpmethod_byid(u_int); const char *relay_httperror_byid(u_int); +int relay_http_priv_init(struct rsession *); int relay_httpdesc_init(struct ctl_relay_event *); ssize_t relay_http_time(time_t, char *, size_t);
