From c9822c566a2e2077c872b465772a179d1c6dae10 Mon Sep 17 00:00:00 2001 From: reyk Date: Wed, 27 Jul 2016 06:55:44 +0000 Subject: [PATCH] Improve parsing of the Host by following RFC 7230 Section 5.4 more strictly: - Respond with a 400 (Bad Request) if there is more than one Host: header to prevent ambiguities. - Make sure that the host in the optional absolute form of request-target (eg. GET http://www.target.com/ HTTP/1.1) matches the Host: value. Proxies are supposed to ignore the Host: value if the request-target exists, but relayd used to ignore the absolute request-target form instead. In HTTP terminology, relayd is a gateway and not a proxy, but it has to make sure that the host is validated consistently. OK benno@ bluhm@ --- regress/usr.sbin/relayd/args-http-host.pl | 37 +++++++++++ regress/usr.sbin/relayd/args-http-host2.pl | 34 ++++++++++ regress/usr.sbin/relayd/args-http-host3.pl | 36 +++++++++++ regress/usr.sbin/relayd/args-http-host4.pl | 37 +++++++++++ usr.sbin/relayd/relay_http.c | 37 +++++++++-- usr.sbin/relayd/relayd.c | 75 ++++++++++++++++++---- usr.sbin/relayd/relayd.h | 5 +- 7 files changed, 241 insertions(+), 20 deletions(-) create mode 100644 regress/usr.sbin/relayd/args-http-host.pl create mode 100644 regress/usr.sbin/relayd/args-http-host2.pl create mode 100644 regress/usr.sbin/relayd/args-http-host3.pl create mode 100644 regress/usr.sbin/relayd/args-http-host4.pl diff --git a/regress/usr.sbin/relayd/args-http-host.pl b/regress/usr.sbin/relayd/args-http-host.pl new file mode 100644 index 00000000000..a77e852ee08 --- /dev/null +++ b/regress/usr.sbin/relayd/args-http-host.pl @@ -0,0 +1,37 @@ +use strict; +use warnings; + +our %args = ( + client => { + func => sub { + my $self = shift; + print <<'EOF'; +GET /1 HTTP/1.1 +Host: www.foo.com +Host: www.bar.com + +EOF + # no http_response($self, 1); + }, + http_vers => ["1.1"], + nocheck => 1, + method => "GET", + }, + relayd => { + protocol => [ "http", + "match request header log Host", + ], + loggrep => { + qr/, malformed header$/ => 1, + qr/\[Host: www.foo.com\] GET/ => 0, + qr/\[Host: www.bar.com\] GET/ => 0, + }, + }, + server => { + func => \&http_server, + noserver => 1, + nocheck => 1, + } +); + +1; diff --git a/regress/usr.sbin/relayd/args-http-host2.pl b/regress/usr.sbin/relayd/args-http-host2.pl new file mode 100644 index 00000000000..b34a580665f --- /dev/null +++ b/regress/usr.sbin/relayd/args-http-host2.pl @@ -0,0 +1,34 @@ +use strict; +use warnings; + +our %args = ( + client => { + func => sub { + my $self = shift; + print <<'EOF'; +GET http://www.foo.com/1 HTTP/1.1 + +EOF + http_response($self, 1); + }, + http_vers => ["1.1"], + nocheck => 1, + method => "GET", + }, + relayd => { + protocol => [ "http", + "match request header log Host", + "match request path log \"*\"", + ], + loggrep => { + qr/, malformed header$/ => 0, + qr/\[http:\/\/www.foo.com\/1\] GET/ => 1, + }, + }, + server => { + func => \&http_server, + nocheck => 1, + } +); + +1; diff --git a/regress/usr.sbin/relayd/args-http-host3.pl b/regress/usr.sbin/relayd/args-http-host3.pl new file mode 100644 index 00000000000..38bf6ca089f --- /dev/null +++ b/regress/usr.sbin/relayd/args-http-host3.pl @@ -0,0 +1,36 @@ +use strict; +use warnings; + +our %args = ( + client => { + func => sub { + my $self = shift; + print <<'EOF'; +GET http://www.foo.com/1 HTTP/1.1 +Host: www.foo.com + +EOF + http_response($self, 1); + }, + http_vers => ["1.1"], + nocheck => 1, + method => "GET", + }, + relayd => { + protocol => [ "http", + "match request header log Host", + "match request path log \"*\"", + ], + loggrep => { + qr/, malformed host$/ => 0, + qr/\[http:\/\/www.foo.com\/1\] GET/ => 1, + qr/\[Host: www.foo.com\]/ => 1, + }, + }, + server => { + func => \&http_server, + nocheck => 1 + }, +); + +1; diff --git a/regress/usr.sbin/relayd/args-http-host4.pl b/regress/usr.sbin/relayd/args-http-host4.pl new file mode 100644 index 00000000000..11787efe990 --- /dev/null +++ b/regress/usr.sbin/relayd/args-http-host4.pl @@ -0,0 +1,37 @@ +use strict; +use warnings; + +our %args = ( + client => { + func => sub { + my $self = shift; + print <<'EOF'; +GET http://www.foo.com/1 HTTP/1.1 +Host: www.bar.com + +EOF + # no http_response($self, 1); + }, + http_vers => ["1.1"], + nocheck => 1, + method => "GET", + }, + relayd => { + protocol => [ "http", + "match request header log Host", + "match request path log \"*\"", + ], + loggrep => { + qr/, malformed host$/ => 1, + qr/\[http:\/\/www.foo.com\/1\] GET/ => 0, + qr/\[Host: www.bar.com\]/ => 0, + }, + }, + server => { + func => \&http_server, + noserver => 1, + nocheck => 1, + } +); + +1; diff --git a/usr.sbin/relayd/relay_http.c b/usr.sbin/relayd/relay_http.c index 3d4066b71cd..bab21355b2d 100644 --- a/usr.sbin/relayd/relay_http.c +++ b/usr.sbin/relayd/relay_http.c @@ -1,4 +1,4 @@ -/* $OpenBSD: relay_http.c,v 1.56 2016/07/22 09:30:36 benno Exp $ */ +/* $OpenBSD: relay_http.c,v 1.57 2016/07/27 06:55:44 reyk Exp $ */ /* * Copyright (c) 2006 - 2015 Reyk Floeter @@ -150,7 +150,8 @@ relay_read_http(struct bufferevent *bev, void *arg) struct protocol *proto = rlay->rl_proto; struct evbuffer *src = EVBUFFER_INPUT(bev); char *line = NULL, *key, *value; - int action; + char *urlproto, *host, *path; + int action, unique, ret; const char *errstr; size_t size, linelen; struct kv *hdr = NULL; @@ -331,11 +332,35 @@ relay_read_http(struct bufferevent *bev, void *arg) strcasecmp("chunked", value) == 0) desc->http_chunked = 1; + /* The following header should only occur once */ + if (strcasecmp("Host", key) == 0) { + unique = 1; + + /* + * The path may contain a URL. The host in the + * URL has to match the Host: value. + */ + if (parse_url(desc->http_path, + &urlproto, &host, &path) == 0) { + ret = strcasecmp(host, value); + free(urlproto); + free(host); + free(path); + if (ret != 0) { + relay_abort_http(con, 400, + "malformed host", 0); + goto abort; + } + } + } else + unique = 0; + if (cre->line != 1) { if ((hdr = kv_add(&desc->http_headers, key, - value)) == NULL) { - free(line); - goto fail; + value, unique)) == NULL) { + relay_abort_http(con, 400, + "malformed header", 0); + goto abort; } desc->http_lastheader = hdr; } @@ -1537,7 +1562,7 @@ relay_apply_actions(struct ctl_relay_event *cre, struct kvlist *actions) if (addkv && kv->kv_matchtree != NULL) { /* Add new entry to the list (eg. new HTTP header) */ if ((match = kv_add(kv->kv_matchtree, kp->kv_key, - kp->kv_value)) == NULL) + kp->kv_value, 0)) == NULL) goto fail; match->kv_option = kp->kv_option; match->kv_type = kp->kv_type; diff --git a/usr.sbin/relayd/relayd.c b/usr.sbin/relayd/relayd.c index 8fdedcbfbfa..65c7ee20b85 100644 --- a/usr.sbin/relayd/relayd.c +++ b/usr.sbin/relayd/relayd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: relayd.c,v 1.153 2016/02/02 17:51:11 sthen Exp $ */ +/* $OpenBSD: relayd.c,v 1.154 2016/07/27 06:55:44 reyk Exp $ */ /* * Copyright (c) 2007 - 2014 Reyk Floeter @@ -645,9 +645,8 @@ purge_relay(struct relayd *env, struct relay *rlay) free(rlay); } - struct kv * -kv_add(struct kvtree *keys, char *key, char *value) +kv_add(struct kvtree *keys, char *key, char *value, int unique) { struct kv *kv, *oldkv; @@ -655,24 +654,30 @@ kv_add(struct kvtree *keys, char *key, char *value) return (NULL); if ((kv = calloc(1, sizeof(*kv))) == NULL) return (NULL); - if ((kv->kv_key = strdup(key)) == NULL) { - free(kv); - return (NULL); - } + if ((kv->kv_key = strdup(key)) == NULL) + goto fail; if (value != NULL && - (kv->kv_value = strdup(value)) == NULL) { - free(kv->kv_key); - free(kv); - return (NULL); - } + (kv->kv_value = strdup(value)) == NULL) + goto fail; TAILQ_INIT(&kv->kv_children); if ((oldkv = RB_INSERT(kvtree, keys, kv)) != NULL) { + /* + * return error if the key should occur only once, + * or add it to a list attached to the key's node. + */ + if (unique) + goto fail; TAILQ_INSERT_TAIL(&oldkv->kv_children, kv, kv_entry); kv->kv_parent = oldkv; } return (kv); + fail: + free(kv->kv_key); + free(kv->kv_value); + free(kv); + return (NULL); } int @@ -1381,6 +1386,52 @@ canonicalize_host(const char *host, char *name, size_t len) return (NULL); } +int +parse_url(const char *url, char **protoptr, char **hostptr, char **pathptr) +{ + char *p, *proto = NULL, *host = NULL, *path = NULL; + + /* return error if it is not a URL */ + if ((p = strstr(url, ":/")) == NULL || + (strcspn(url, ":/") != (size_t)(p - url))) + return (-1); + + /* get protocol */ + if ((proto = strdup(url)) == NULL) + goto fail; + p = proto + (p - url); + + /* get host */ + p += strspn(p, ":/"); + if (*p == '\0' || (host = strdup(p)) == NULL) + goto fail; + *p = '\0'; + + /* find and copy path or default to "/" */ + if ((p = strchr(host, '/')) == NULL) + p = "/"; + if ((path = strdup(p)) == NULL) + goto fail; + + /* strip path after host */ + host[strcspn(host, "/")] = '\0'; + + DPRINTF("%s: %s proto %s, host %s, path %s", __func__, + url, proto, host, path); + + *protoptr = proto; + *hostptr = host; + *pathptr = path; + + return (0); + + fail: + free(proto); + free(host); + free(path); + return (-1); +} + int bindany(struct ctl_bindany *bnd) { diff --git a/usr.sbin/relayd/relayd.h b/usr.sbin/relayd/relayd.h index 2430617a16f..e051b8ee798 100644 --- a/usr.sbin/relayd/relayd.h +++ b/usr.sbin/relayd/relayd.h @@ -1,4 +1,4 @@ -/* $OpenBSD: relayd.h,v 1.223 2016/07/22 09:30:36 benno Exp $ */ +/* $OpenBSD: relayd.h,v 1.224 2016/07/27 06:55:44 reyk Exp $ */ /* * Copyright (c) 2006 - 2015 Reyk Floeter @@ -1268,6 +1268,7 @@ void purge_table(struct relayd *, struct tablelist *, void purge_relay(struct relayd *, struct relay *); char *digeststr(enum digest_type, const u_int8_t *, size_t, char *); const char *canonicalize_host(const char *, char *, size_t); +int parse_url(const char *, char **, char **, char **); int map6to4(struct sockaddr_storage *); int map4to6(struct sockaddr_storage *, struct sockaddr_storage *); void imsg_event_add(struct imsgev *); @@ -1281,7 +1282,7 @@ struct in6_addr *prefixlen2mask6(u_int8_t, u_int32_t *); u_int32_t prefixlen2mask(u_int8_t); int accept_reserve(int, struct sockaddr *, socklen_t *, int, volatile int *); -struct kv *kv_add(struct kvtree *, char *, char *); +struct kv *kv_add(struct kvtree *, char *, char *, int); int kv_set(struct kv *, char *, ...); int kv_setkey(struct kv *, char *, ...); void kv_delete(struct kvtree *, struct kv *); -- 2.20.1