From: Stefan Eissing Date: Thu, 10 Feb 2022 10:59:08 +0000 (+0000) Subject: *) mod_http2: :scheme pseudo-header values, not matching the X-Git-Tag: 2.5.0-alpha2-ci-test-only~499 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b90220157d4ef25d2ecbbb96ee8b2e99b661f8d5;p=thirdparty%2Fapache%2Fhttpd.git *) mod_http2: :scheme pseudo-header values, not matching the connection scheme, are forwarded via absolute uris to the http protocol processing to preserve semantics of the request. Checks on combinations of pseudo-headers values/absence have been added as described in RFC 7540. Fixes . git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1897940 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/changes-entries/http2_request_scheme.txt b/changes-entries/http2_request_scheme.txt index 93a6d6348b3..36744decdaa 100644 --- a/changes-entries/http2_request_scheme.txt +++ b/changes-entries/http2_request_scheme.txt @@ -1,3 +1,7 @@ - *) mod_http2: when a h2 request carries a ':scheme' pseudoheader, - it gives a 400 response if the scheme does not match the - connection. Fixes . + *) mod_http2: :scheme pseudo-header values, not matching the + connection scheme, are forwarded via absolute uris to the + http protocol processing to preserve semantics of the request. + Checks on combinations of pseudo-headers values/absence + have been added as described in RFC 7540. + Fixes . + [Stefan Eissing] diff --git a/modules/http2/h2_c2.c b/modules/http2/h2_c2.c index 4e689c7c11c..00a783d28e7 100644 --- a/modules/http2/h2_c2.c +++ b/modules/http2/h2_c2.c @@ -574,8 +574,8 @@ static apr_status_t c2_process(h2_conn_ctx_t *conn_ctx, conn_rec *c) } ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c, - "h2_c2(%s-%d): created request_rec", - conn_ctx->id, conn_ctx->stream_id); + "h2_c2(%s-%d): created request_rec for %s", + conn_ctx->id, conn_ctx->stream_id, r->the_request); conn_ctx->server = r->server; /* the request_rec->server carries the timeout value that applies */ diff --git a/modules/http2/h2_request.c b/modules/http2/h2_request.c index 54721c45af1..e759c2e4558 100644 --- a/modules/http2/h2_request.c +++ b/modules/http2/h2_request.c @@ -16,7 +16,11 @@ #include -#include +#include "apr.h" +#include "apr_strings.h" +#include "apr_lib.h" +#include "apr_strmatch.h" + #include #include @@ -25,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -170,7 +175,7 @@ apr_status_t h2_request_add_header(h2_request *req, apr_pool_t *pool, apr_status_t h2_request_end_headers(h2_request *req, apr_pool_t *pool, int eos, size_t raw_bytes) { const char *s; - + /* rfc7540, ch. 8.1.2.3: * - if we have :authority, it overrides any Host header * - :authority MUST be omitted when converting h1->h2, so we @@ -295,9 +300,30 @@ request_rec *h2_create_request_rec(const h2_request *req, conn_rec *c) /* Time to populate r with the data we have. */ r->request_time = req->request_time; - r->the_request = apr_psprintf(r->pool, "%s %s HTTP/2.0", - req->method, req->path ? req->path : ""); - r->headers_in = apr_table_clone(r->pool, req->headers); + AP_DEBUG_ASSERT(req->authority); + if (req->scheme && (ap_cstr_casecmp(req->scheme, + ap_ssl_conn_is_ssl(c->master? c->master : c)? "https" : "http") + || !ap_cstr_casecmp("CONNECT", req->method))) { + /* Client sent a non-matching ':scheme' pseudo header. Forward this + * via an absolute URI in the request line. + */ + r->the_request = apr_psprintf(r->pool, "%s %s://%s%s HTTP/2.0", + req->method, req->scheme, req->authority, + req->path ? req->path : ""); + } + else if (req->path) { + r->the_request = apr_psprintf(r->pool, "%s %s HTTP/2.0", + req->method, req->path); + } + else { + /* We should only come here on a request that is errored already. + * create a request line that passes parsing, we'll die anyway. + */ + AP_DEBUG_ASSERT(req->http_status != H2_HTTP_STATUS_UNSET); + r->the_request = apr_psprintf(r->pool, "%s / HTTP/2.0", req->method); + } + + r->headers_in = apr_table_copy(r->pool, req->headers); /* Start with r->hostname = NULL, ap_check_request_header() will get it * form Host: header, otherwise we get complains about port numbers. @@ -399,6 +425,8 @@ request_rec *h2_create_request_rec(const h2_request *req, conn_rec *c) return r; die: + ap_log_cerror(APLOG_MARK, APLOG_TRACE2, 0, c, + "ap_die(%d) for %s", access_status, r->the_request); ap_die(access_status, r); /* ap_die() sent the response through the output filters, we must now diff --git a/modules/http2/h2_stream.c b/modules/http2/h2_stream.c index 725f6a7bb47..406465112bf 100644 --- a/modules/http2/h2_stream.c +++ b/modules/http2/h2_stream.c @@ -17,7 +17,10 @@ #include #include -#include +#include "apr.h" +#include "apr_strings.h" +#include "apr_lib.h" +#include "apr_strmatch.h" #include #include @@ -770,33 +773,97 @@ apr_status_t h2_stream_end_headers(h2_stream *stream, int eos, size_t raw_bytes) { apr_status_t status; val_len_check_ctx ctx; - - status = h2_request_end_headers(stream->rtmp, stream->pool, eos, raw_bytes); - if (APR_SUCCESS == status) { - set_policy_for(stream, stream->rtmp); + int is_http_or_https; + h2_request *req = stream->rtmp; - ctx.maxlen = stream->session->s->limit_req_fieldsize; - ctx.failed_key = NULL; - apr_table_do(table_check_val_len, &ctx, stream->rtmp->headers, NULL); - if (ctx.failed_key) { - if (!h2_stream_is_ready(stream)) { - ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, stream->session->c1, - H2_STRM_LOG(APLOGNO(10230), stream,"Request header exceeds " - "LimitRequestFieldSize: %.*s"), - (int)H2MIN(strlen(ctx.failed_key), 80), ctx.failed_key); - } - set_error_response(stream, HTTP_REQUEST_HEADER_FIELDS_TOO_LARGE); - /* keep on returning APR_SUCCESS, so that we send a HTTP response and - * do not RST the stream. */ + status = h2_request_end_headers(req, stream->pool, eos, raw_bytes); + if (APR_SUCCESS != status || req->http_status != H2_HTTP_STATUS_UNSET) goto cleanup; + + /* keep on returning APR_SUCCESS for error responses, so that we + * send it and do not RST the stream. + */ + set_policy_for(stream, req); + + ctx.maxlen = stream->session->s->limit_req_fieldsize; + ctx.failed_key = NULL; + apr_table_do(table_check_val_len, &ctx, req->headers, NULL); + if (ctx.failed_key) { + if (!h2_stream_is_ready(stream)) { + ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, stream->session->c1, + H2_STRM_LOG(APLOGNO(10230), stream,"Request header exceeds " + "LimitRequestFieldSize: %.*s"), + (int)H2MIN(strlen(ctx.failed_key), 80), ctx.failed_key); } - if (stream->rtmp->scheme && strcasecmp(stream->rtmp->scheme, - ap_ssl_conn_is_ssl(stream->session->c1)? "https" : "http")) { - ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, stream->session->c1, - H2_STRM_LOG(APLOGNO(10379), stream,"Request :scheme '%s' and " - "connection do not match."), stream->rtmp->scheme); + set_error_response(stream, HTTP_REQUEST_HEADER_FIELDS_TOO_LARGE); + goto cleanup; + } + + /* http(s) scheme. rfc7540, ch. 8.1.2.3: + * This [:path] pseudo-header field MUST NOT be empty for "http" or "https" + * URIs; "http" or "https" URIs that do not contain a path component + * MUST include a value of '/'. The exception to this rule is an + * OPTIONS request for an "http" or "https" URI that does not include + * a path component; these MUST include a ":path" pseudo-header field + * with a value of '*' + * + * All HTTP/2 requests MUST include exactly one valid value for the + * ":method", ":scheme", and ":path" pseudo-header fields, unless it is + * a CONNECT request. + */ + is_http_or_https = (!req->scheme + || !(ap_cstr_casecmpn(req->scheme, "http", 4) != 0 + || (req->scheme[4] != '\0' + && (apr_tolower(req->scheme[4]) != 's' + || req->scheme[5] != '\0')))); + + /* CONNECT. rfc7540, ch. 8.3: + * In HTTP/2, the CONNECT method is used to establish a tunnel over a + * single HTTP/2 stream to a remote host for similar purposes. The HTTP + * header field mapping works as defined in Section 8.1.2.3 ("Request + * Pseudo-Header Fields"), with a few differences. Specifically: + * o The ":method" pseudo-header field is set to "CONNECT". + * o The ":scheme" and ":path" pseudo-header fields MUST be omitted. + * o The ":authority" pseudo-header field contains the host and port to + * connect to (equivalent to the authority-form of the request-target + * of CONNECT requests (see [RFC7230], Section 5.3)). + */ + if (!ap_cstr_casecmp(req->method, "CONNECT")) { + if (req->scheme || req->path) { + ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, stream->session->c1, + H2_STRM_LOG(APLOGNO(), stream, "Request to CONNECT " + "with :scheme or :path specified, sending 400 answer")); set_error_response(stream, HTTP_BAD_REQUEST); + goto cleanup; } - stream->request = stream->rtmp; + } + else if (is_http_or_https) { + if (!req->path) { + ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, stream->session->c1, + H2_STRM_LOG(APLOGNO(), stream, "Request for http(s) " + "resource without :path, sending 400 answer")); + set_error_response(stream, HTTP_BAD_REQUEST); + goto cleanup; + } + if (!req->scheme) { + req->scheme = ap_ssl_conn_is_ssl(stream->session->c1)? "https" : "http"; + } + } + + if (req->scheme && (req->path && req->path[0] != '/')) { + /* We still have a scheme, which means we need to pass an absolute URI into + * our HTTP protocol handling and the missing '/' at the start will prevent + * us from doing so (as it then confuses path and authority). */ + ap_log_cerror(APLOG_MARK, APLOG_INFO, 0, stream->session->c1, + H2_STRM_LOG(APLOGNO(10379), stream, "Request :scheme '%s' and " + "path '%s' do not allow creating an absolute URL. Failing " + "request with 400."), req->scheme, req->path); + set_error_response(stream, HTTP_BAD_REQUEST); + goto cleanup; + } + +cleanup: + if (APR_SUCCESS == status) { + stream->request = req; stream->rtmp = NULL; } return status; @@ -1108,6 +1175,7 @@ const h2_priority *h2_stream_get_priority(h2_stream *stream, int h2_stream_is_ready(h2_stream *stream) { + /* Have we sent a response or do we have the response in our buffer? */ if (stream->response) { return 1; } diff --git a/test/modules/http2/test_003_get.py b/test/modules/http2/test_003_get.py index 09ea31167de..ccef11fbdeb 100644 --- a/test/modules/http2/test_003_get.py +++ b/test/modules/http2/test_003_get.py @@ -215,7 +215,15 @@ content-type: text/html # use an invalid scheme def test_h2_003_51(self, env): url = env.mkurl("https", "cgi", "/") - opt = ["-H:scheme: http"] + opt = ["-H:scheme: invalid"] r = env.nghttp().get(url, options=opt) assert r.exit_code == 0, r assert r.response['status'] == 400 + + # use an differing scheme, but one that is acceptable + def test_h2_003_52(self, env): + url = env.mkurl("https", "cgi", "/") + opt = ["-H:scheme: http"] + r = env.nghttp().get(url, options=opt) + assert r.exit_code == 0, r + assert r.response['status'] == 200