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@

7 files changed, 241 insertions, 20 deletions

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 <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 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 <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
@@ -1382,6 +1387,52 @@ canonicalize_host(const char *host, char *name, size_t len)
 }
 
 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)
 {
 	int	s, v;
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 <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 *);