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@

diff --git a/regress/usr.sbin/relayd/args-http-host.pl b/regress/usr.sbin/relayd/args-http-host.pl
new file mode 100644 (file)
index 0000000..a77e852
--- /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 (file)
index 0000000..b34a580
--- /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 (file)
index 0000000..38bf6ca
--- /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 (file)
index 0000000..11787ef
--- /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 3d4066b..bab2135 100644 (file)
--- 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 <reyk@openbsd.org>
@@ -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 8fdedcb..65c7ee2 100644 (file)
--- 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 <reyk@openbsd.org>
@@ -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 2430617..e051b8e 100644 (file)
--- 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 <reyk@openbsd.org>
@@ -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 *);