On Wed, May 08, 2019 at 06:26:45PM +0200, Reyk Floeter wrote:
> On Wed, Mar 06, 2019 at 05:36:32PM +0100, Sebastian Benoit wrote:
> > Rivo Nurges(rivo.nur...@smit.ee) on 2019.03.05 22:42:13 +0000:
> > > Hi!
> > >
> > > On 3/5/19 10:36 PM, Claudio Jeker wrote:
> > > > I guess that this would need strcasestr() instead of strcasecmp(),
> > > > since you
> > > > are looking for the substring "Upgrade" in value. Maybe more is needed
> > > > if
> > > > we want to be sure that 'Connection: Upgrade-maybe' does not match.
> > >
> > > You are correct about strcasestr. "Connection: Upgrade-maybe" would need
> > > to have correct "Upgrade: websocket". Anyway, lets be strict.
> > >
> > > Does something like this make sense?
> >
> > i think the seperator list needs to include '\t'
> > because https://tools.ietf.org/html/rfc7230#appendix-B includes HTAB.
> >
> > And i dont think you can mix "," with " \t" seperators,
> > because otherwise "Foo Upgrade, Bar" will match.
> >
> > Something more is needed to parse elements of a header.
> >
>
> I would reshuffle the websocket handling a bit as I don't think that
> we need the http priv struct. We can lookup the parsed headers later.
>
> The attached diff moves it all into one places and introduces a new
> function kv_find_value() that is able to to match headers with
> multiple values (I think we might need it elsewhere. And even if not,
> I would prefer to have this in a function intead of stuffing it into
> relay_read_http).
>
> A minor question is if the lookup should be done before or after
> applying the filters (relay_test - my diff does it afterwards, the
> current code does it before).
>
> Tests? Comments? Ok?
>
I keep on replying to my own diffs... the updated one below uses
strcasecmp instead of strcasestr in kv_find_value(). There is no need
for substring- or fn- matching in this function.
Reyk
Index: usr.sbin/relayd/http.h
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/http.h,v
retrieving revision 1.10
diff -u -p -u -p -r1.10 http.h
--- usr.sbin/relayd/http.h 4 Mar 2019 21:25:03 -0000 1.10
+++ usr.sbin/relayd/http.h 8 May 2019 16:55:59 -0000
@@ -251,10 +251,4 @@ struct http_descriptor {
struct kvtree http_headers;
};
-struct relay_http_priv {
-#define HTTP_CONNECTION_UPGRADE 0x01
-#define HTTP_UPGRADE_WEBSOCKET 0x02
- int http_upgrade_req;
-};
-
#endif /* HTTP_H */
Index: usr.sbin/relayd/relay.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relay.c,v
retrieving revision 1.242
diff -u -p -u -p -r1.242 relay.c
--- usr.sbin/relayd/relay.c 4 Mar 2019 21:25:03 -0000 1.242
+++ usr.sbin/relayd/relay.c 8 May 2019 16:56:00 -0000
@@ -1410,13 +1410,7 @@ relay_session(struct rsession *con)
return;
}
- 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_proto->type != RELAY_PROTO_HTTP) {
if (rlay->rl_conf.fwdmode == FWD_TRANS)
relay_bindanyreq(con, 0, IPPROTO_TCP);
else if (relay_connect(con) == -1) {
Index: usr.sbin/relayd/relay_http.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relay_http.c,v
retrieving revision 1.72
diff -u -p -u -p -r1.72 relay_http.c
--- usr.sbin/relayd/relay_http.c 4 Mar 2019 21:25:03 -0000 1.72
+++ usr.sbin/relayd/relay_http.c 8 May 2019 16:56:01 -0000
@@ -110,17 +110,6 @@ relay_http_init(struct relay *rlay)
}
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)
{
struct http_descriptor *desc;
@@ -163,13 +152,13 @@ relay_read_http(struct bufferevent *bev,
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;
const char *errstr;
size_t size, linelen;
struct kv *hdr = NULL;
+ struct kv *upgrade = NULL, *upgrade_ws = NULL;
getmonotime(&con->se_tv_last);
cre->timedout = 0;
@@ -398,17 +387,6 @@ relay_read_http(struct bufferevent *bev,
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_CONNECTION_UPGRADE;
- 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,
@@ -445,37 +423,34 @@ relay_read_http(struct bufferevent *bev,
return;
}
- /* HTTP 101 Switching Protocols */
- if (cre->dir == RELAY_DIR_REQUEST) {
- if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
- !(proto->httpflags & HTTPFLAG_WEBSOCKETS)) {
+ /*
+ * HTTP 101 Switching Protocols
+ */
+ upgrade = kv_find_value(&desc->http_headers,
+ "Connection", "upgrade", ",");
+ upgrade_ws = kv_find_value(&desc->http_headers,
+ "Upgrade", "websocket", ",");
+ if (cre->dir == RELAY_DIR_REQUEST && upgrade_ws != NULL) {
+ if ((proto->httpflags & HTTPFLAG_WEBSOCKETS) == 0) {
relay_abort_http(con, 403,
"Websocket Forbidden", 0);
return;
- }
- if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
- !(priv->http_upgrade_req & HTTP_CONNECTION_UPGRADE))
- {
+ } else if (upgrade == NULL) {
relay_abort_http(con, 400,
"Bad Websocket Request", 0);
return;
- }
- if ((priv->http_upgrade_req & HTTP_UPGRADE_WEBSOCKET) &&
- (desc->http_method != HTTP_METHOD_GET)) {
+ } else if (desc->http_method != HTTP_METHOD_GET) {
relay_abort_http(con, 405,
"Websocket Method Not Allowed", 0);
return;
}
} else if (cre->dir == RELAY_DIR_RESPONSE &&
desc->http_status == 101) {
- if (((priv->http_upgrade_req &
- (HTTP_CONNECTION_UPGRADE | HTTP_UPGRADE_WEBSOCKET))
- ==
- (HTTP_CONNECTION_UPGRADE | HTTP_UPGRADE_WEBSOCKET))
- && proto->httpflags & HTTPFLAG_WEBSOCKETS) {
+ if (upgrade_ws != NULL && upgrade != NULL &&
+ (proto->httpflags & HTTPFLAG_WEBSOCKETS)) {
cre->dst->toread = TOREAD_UNLIMITED;
cre->dst->bev->readcb = relay_read;
- } else {
+ } else {
relay_abort_http(con, 502,
"Bad Websocket Gateway", 0);
return;
Index: usr.sbin/relayd/relayd.c
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.175
diff -u -p -u -p -r1.175 relayd.c
--- usr.sbin/relayd/relayd.c 24 Apr 2019 19:13:49 -0000 1.175
+++ usr.sbin/relayd/relayd.c 8 May 2019 16:56:02 -0000
@@ -812,6 +812,46 @@ kv_find(struct kvtree *keys, struct kv *
return (match);
}
+struct kv *
+kv_find_value(struct kvtree *keys, char *key, const char *value,
+ const char *delim)
+{
+ struct kv *match, kv;
+ char *val = NULL, *next, *ptr;
+
+ kv.kv_key = key;
+ if ((match = RB_FIND(kvtree, keys, &kv)) == NULL)
+ return (NULL);
+
+ if (match->kv_value == NULL)
+ return (NULL);
+
+ if (delim == NULL) {
+ if (strcasecmp(match->kv_value, value) == 0)
+ goto done;
+ } else {
+ if ((val = strdup(match->kv_value)) == NULL)
+ return (NULL);
+ for (next = ptr = val; ptr != NULL;
+ ptr = strsep(&next, delim)) {
+ /* strip leading whitespace or newlines */
+ ptr += strspn(ptr, " \t\r\n");
+ if (strcasecmp(ptr, value) == 0)
+ goto done;
+ }
+ }
+
+ /* not matched */
+ match = NULL;
+ done:
+#ifdef DEBUG
+ if (match != NULL)
+ DPRINTF("%s: matched %s: %s", __func__, key, value);
+#endif
+ free(val);
+ return (match);
+}
+
int
kv_cmp(struct kv *a, struct kv *b)
{
Index: usr.sbin/relayd/relayd.h
===================================================================
RCS file: /cvs/src/usr.sbin/relayd/relayd.h,v
retrieving revision 1.252
diff -u -p -u -p -r1.252 relayd.h
--- usr.sbin/relayd/relayd.h 4 Mar 2019 21:25:03 -0000 1.252
+++ usr.sbin/relayd/relayd.h 8 May 2019 16:56:02 -0000
@@ -1324,6 +1324,8 @@ void relay_log(struct rsession *, char
int kv_log(struct rsession *, struct kv *, u_int16_t,
enum direction);
struct kv *kv_find(struct kvtree *, struct kv *);
+struct kv *kv_find_value(struct kvtree *, char *, const char *,
+ const char *);
int kv_cmp(struct kv *, struct kv *);
int rule_add(struct protocol *, struct relay_rule *, const char
*);
