BUG/MINOR: h1: Replace authority validation to conform RFC3986

Except for CONNECT method, where a normalization is performed, we expected
to have an exact match between the authority and the host header value.
However it was too strict. Indeed, default port must be handled and the
matching must respect the RFC3986.

There is already a scheme based normalizeation performed on the URI later,
on the HTX message. And we cannot normalize the URI during H1 parsing to be
able to report proper errors on the original raw buffer. And a systematic
read-only normalization to validate the authority will consume CPU for only
few requests. At the end, we decided to perform extra-checks when the exact
match fails. Now, following authority/host are considered as equivalent:

  http:  domain.com <=> domain.com:80  <=> domain.com:
  https: domain.com <=> domain.com:443 <=> domain.com:

This patch depends on:

  * MINOR: h1: Consider empty port as invalid in authority for CONNECT
  * MINOR: http: Considere empty ports as valid default ports

It is a bug regarding the RFC3986. Technically, I may be backported as far
as 2.4. However, this must be discussed first. If it is backported, the
commits above must be backported too.
This commit is contained in:
Christopher Faulet 2022-11-22 10:04:16 +01:00
parent e5dfe1169d
commit e16ffb0383
1 changed files with 69 additions and 9 deletions

View File

@ -203,6 +203,63 @@ static int h1_validate_connect_authority(struct ist authority, struct ist *host_
return -2; return -2;
} }
/* Validate the authority and the host header value for non-CONNECT method, when
* an absolute-URI is detected but when it does not exactly match the host
* value. The idea is to detect default port (http or https). authority and host
* are defined here. 0 is returned on success, -1 if the host is does not match
* the authority.
*/
static int h1_validate_mismatch_authority(struct ist scheme, struct ist authority, struct ist host_hdr)
{
struct ist uri_host, uri_port, host, host_port;
if (!isttest(scheme))
goto mismatch;
uri_host = authority;
uri_port = http_get_host_port(authority);
if (isttest(uri_port))
uri_host.len -= (istlen(uri_port) + 1);
host = host_hdr;
host_port = http_get_host_port(host_hdr);
if (isttest(host_port))
host.len -= (istlen(host_port) + 1);
if (!isttest(uri_port) && !isttest(host_port)) {
/* No port on both: we already know the authority does not match
* the host value
*/
goto mismatch;
}
else if (isttest(uri_port) && !http_is_default_port(scheme, uri_port)) {
/* here there is no port for the host value and the port for the
* authority is not the default one
*/
goto mismatch;
}
else if (isttest(host_port) && !http_is_default_port(scheme, host_port)) {
/* here there is no port for the authority and the port for the
* host value is not the default one
*/
goto mismatch;
}
else {
/* the authority or the host value contain a default port and
* there is no port on the other value
*/
if (!isteqi(uri_host, host))
goto mismatch;
}
return 0;
mismatch:
return -1;
}
/* Parse the Connection: header of an HTTP/1 request, looking for "close", /* Parse the Connection: header of an HTTP/1 request, looking for "close",
* "keep-alive", and "upgrade" values, and updating h1m->flags according to * "keep-alive", and "upgrade" values, and updating h1m->flags according to
* what was found there. Note that flags are only added, not removed, so the * what was found there. Note that flags are only added, not removed, so the
@ -1004,12 +1061,13 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
if (!(h1m->flags & (H1_MF_HDRS_ONLY|H1_MF_RESP))) { if (!(h1m->flags & (H1_MF_HDRS_ONLY|H1_MF_RESP))) {
struct http_uri_parser parser = http_uri_parser_init(sl.rq.u); struct http_uri_parser parser = http_uri_parser_init(sl.rq.u);
struct ist authority; struct ist scheme, authority;
int ret;
scheme = http_parse_scheme(&parser);
authority = http_parse_authority(&parser, 1); authority = http_parse_authority(&parser, 1);
if (sl.rq.meth == HTTP_METH_CONNECT) { if (sl.rq.meth == HTTP_METH_CONNECT) {
struct ist *host = ((host_idx != -1) ? &hdr[host_idx].v : NULL); struct ist *host = ((host_idx != -1) ? &hdr[host_idx].v : NULL);
int ret;
ret = h1_validate_connect_authority(authority, host); ret = h1_validate_connect_authority(authority, host);
if (ret < 0) { if (ret < 0) {
@ -1032,15 +1090,17 @@ int h1_headers_to_hdr_list(char *start, const char *stop,
/* For non-CONNECT method, the authority must match the host header value */ /* For non-CONNECT method, the authority must match the host header value */
if (!isteqi(authority, host)) { if (!isteqi(authority, host)) {
if (h1m->err_pos < -1) { ret = h1_validate_mismatch_authority(scheme, authority, host);
state = H1_MSG_LAST_LF; if (ret < 0) {
ptr = host.ptr; /* Set ptr on the error */ if (h1m->err_pos < -1) {
goto http_msg_invalid; state = H1_MSG_LAST_LF;
ptr = host.ptr; /* Set ptr on the error */
goto http_msg_invalid;
}
if (h1m->err_pos == -1) /* capture the error pointer */
h1m->err_pos = v.ptr - start + skip; /* >= 0 now */
} }
if (h1m->err_pos == -1) /* capture the error pointer */
h1m->err_pos = v.ptr - start + skip; /* >= 0 now */
} }
} }
} }