BUG/MEDIUM: h1: build a missing Host header from the authority of the URI

An HTTP/1 request without a Host header could be accepted as-is, while
host-based rules require a host to apply to the real target. This concerns
the two request-target forms that carry an authority:

  - the authority-form, used by CONNECT requests (RFC9112#3.2.3), where the
    authority is the request target and must not contain userinfo, path or
    query;

  - the absolute-form (RFC9112#3.2.2), where the authority of the absolute
    URI is what an H1 server must use as host, and what is sent as
    :authority to H2/H3 servers.

The H1 parser only checked the consistency between the URI's authority and
the Host header when the latter was present. Now, when there is no Host
header, one is added with the authority's value (userinfo removed), so that
host-based rules apply to the real target and H1 servers receive a Host
header matching the request target, as RFC9112#3.2 requires for
HTTP/1.1. For the authority-form, it is only added once the authority was
validated, and for both forms it is not added when the authority contains
chars that are forbidden in the Host header, which is only possible when
unsafe violations are accepted.

A default port is removed from the synthesized value, like it is done when a
Host header with a default port is present, so that the rules see the same
value with and without the Host header: for the authority-form, both ports
80 and 443 are considered default since there is no scheme (CONNECT), and
for the absolute-form the default port depends on the URI's scheme (80 for
http, 443 for https).

The Host header is inserted before the terminating empty header, so that it
is accounted for in the header list.

The reg-tests h1_absolute_uri_no_host and h1_connect_no_host_no_body cover
this.

This is long-standing and should be backported to all supported versions.

diff --git a/reg-tests/http-messaging/h1_absolute_uri_no_host.vtc b/reg-tests/http-messaging/h1_absolute_uri_no_host.vtc
new file mode 100644 (file)
index 0000000..1679338
--- /dev/null
+++ b/reg-tests/http-messaging/h1_absolute_uri_no_host.vtc
@@ -0,0 +1,40 @@
+varnishtest "H1 absolute-form request without Host header"
+
+# When an absolute-form request has no Host header, the authority of the URI
+# is the target. A Host header must be built from it so that host-based rules
+# apply to the actual target, like for H2/H3.
+
+feature ignore_unknown_macro
+
+haproxy h1 -conf {
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    listen li1
+        bind "fd@${li1}"
+        http-request deny if { req.hdr(host) -m str admin.internal }
+        http-request return status 200 hdr x-host "%[req.hdr(host)]"
+} -start
+
+client c1 -connect ${h1_li1_sock} {
+       txreq -url "http://admin.internal/secret" -nohost
+       rxresp
+       expect resp.status == 403
+} -run
+
+client c2 -connect ${h1_li1_sock} {
+       txreq -url "http://u@public.example:8080/" -nohost
+       rxresp
+       expect resp.status == 200
+       expect resp.http.x-host == "public.example:8080"
+} -run
+
+client c3 -connect ${h1_li1_sock} {
+       txreq -url "/" -hdr "Host: public.example"
+       rxresp
+       expect resp.status == 200
+       expect resp.http.x-host == "public.example"
+} -run

diff --git a/reg-tests/http-messaging/h1_connect_no_host_no_body.vtc b/reg-tests/http-messaging/h1_connect_no_host_no_body.vtc
new file mode 100644 (file)
index 0000000..677802c
--- /dev/null
+++ b/reg-tests/http-messaging/h1_connect_no_host_no_body.vtc
@@ -0,0 +1,39 @@
+varnishtest "H1 CONNECT requests without Host header"
+
+# A CONNECT request without Host header must get a Host header built from its
+# authority so that host-based rules apply to it.
+
+feature ignore_unknown_macro
+
+haproxy h1 -conf {
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    frontend fe1
+        bind "fd@${fe1}"
+        http-request deny if { req.hdr(host) -i -m beg admin.internal }
+        http-request return status 200 hdr x-host "%[req.hdr(host)]"
+} -start
+
+client c1 -connect ${h1_fe1_sock} {
+       txreq -req CONNECT -url "admin.internal:22" -nohost
+       rxresp
+       expect resp.status == 403
+} -run
+
+client c2 -connect ${h1_fe1_sock} {
+       txreq -req CONNECT -url "www.example.com:443" -nohost
+       rxresp
+       expect resp.status == 200
+       expect resp.http.x-host == "www.example.com"
+} -run
+
+client c6 -connect ${h1_fe1_sock} {
+       txreq -req CONNECT -url "www.example.com:8443" -nohost
+       rxresp
+       expect resp.status == 200
+       expect resp.http.x-host == "www.example.com:8443"
+} -run

diff --git a/src/h1.c b/src/h1.c
index 2a91407abac149b2be7e88185ee876394369ea61..38d0cccbe840fbf7262dd930ceabfa7dda42b772 100644 (file)
--- a/src/h1.c
+++ b/src/h1.c
@@ -1115,7 +1115,8 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
                if (!(h1m->flags & (H1_MF_HDRS_ONLY|H1_MF_RESP))) {
                        struct http_uri_parser parser = http_uri_parser_init(sl.rq.u);
                        struct ist scheme, authority = IST_NULL;
-                       int ret;
+                       int ret = 0;
+                       int bad_authority = 0;
 
                        /* WT: gcc seems to see a path where sl.rq.u.ptr was used
                         * uninitialized, but it doesn't know that the function is
@@ -1141,7 +1142,8 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
                        case URI_PARSER_FORMAT_ABSURI_OR_AUTHORITY:
                                scheme = http_parse_scheme(&parser);
                                authority = http_parse_authority(&parser, 1);
-                               if (http_authority_has_forbidden_char(authority)) {
+                               bad_authority = http_authority_has_forbidden_char(authority);
+                               if (bad_authority) {
                                        if (h1m->err_pos < -1) {
                                                state = H1_MSG_LAST_LF;
                                                /* WT: gcc seems to see a path where sl.rq.u.ptr was used
@@ -1216,6 +1218,26 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
                                                }
                                        }
                                }
+
+                               /* No Host header: the authority of the request target is
+                                * the host (RFC9112#3.2.2 and 3.2.3). Add a Host header
+                                * with its value A default port is removed.
+                                */
+                               if (!ret && host_idx == -1 && istlen(authority) && hdr && !skip_update &&
+                                   !bad_authority) {
+                                       struct ist new_host = authority;
+                                       struct ist port = http_get_host_port(authority);
+
+                                       if (unlikely(hdr_count >= hdr_num)) {
+                                               state = H1_MSG_LAST_LF;
+                                               goto http_output_full;
+                                       }
+                                       if (isttest(port) && http_is_default_port(scheme, port))
+                                               new_host.len -= istlen(port) + 1;
+                                       hdr[hdr_count] = hdr[hdr_count - 1];
+                                       http_set_hdr(&hdr[hdr_count - 1], ist("host"), new_host);
+                                       hdr_count++;
+                               }
                                break;
 
                        default: