From: Yann Ylavic Date: Fri, 17 Apr 2020 16:47:42 +0000 (+0000) Subject: core, h2: common ap_parse_request_line() and ap_check_request_header() code. X-Git-Tag: 2.5.0-alpha2-ci-test-only~1512 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=22b3d7cec44d9910862c643659f78b90d153a320;p=thirdparty%2Fapache%2Fhttpd.git core, h2: common ap_parse_request_line() and ap_check_request_header() code. Extract parsing/validation code from read_request_line() and ap_read_request() into ap_parse_request_line() and ap_check_request_header() helpers such that mod_http2 can validate its HTTP/1 request with the same/configured policy. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1876674 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/include/ap_mmn.h b/include/ap_mmn.h index 4ce8ac51b18..24ac648ac94 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -626,6 +626,8 @@ * ap_request_core_filter_handle. * 20200331.1 (2.5.1-dev) Add flushed:1 to request_rec * 20200331.2 (2.5.1-dev) Add ap_proxy_should_override to mod_proxy.h + * 20200331.3 (2.5.1-dev) Add ap_parse_request_line() and + * ap_check_request_header() */ #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */ @@ -633,7 +635,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20200331 #endif -#define MODULE_MAGIC_NUMBER_MINOR 2 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 3 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a diff --git a/include/http_protocol.h b/include/http_protocol.h index 841f2966ce4..b7953d0a6b1 100644 --- a/include/http_protocol.h +++ b/include/http_protocol.h @@ -67,6 +67,20 @@ AP_DECLARE(request_rec *) ap_create_request(conn_rec *c); */ request_rec *ap_read_request(conn_rec *c); +/** + * Parse and validate the request line. + * @param r The current request + * @return 1 on success, 0 on failure + */ +AP_DECLARE(int) ap_parse_request_line(request_rec *r); + +/** + * Validate the request header and select vhost. + * @param r The current request + * @return 1 on success, 0 on failure + */ +AP_DECLARE(int) ap_check_request_header(request_rec *r); + /** * Read the mime-encoded headers. * @param r The current request diff --git a/modules/http2/h2_request.c b/modules/http2/h2_request.c index d8a1b7b4d26..a3d3696704c 100644 --- a/modules/http2/h2_request.c +++ b/modules/http2/h2_request.c @@ -266,9 +266,7 @@ static request_rec *my_ap_create_request(conn_rec *c) request_rec *h2_request_create_rec(const h2_request *req, conn_rec *c) { - int access_status = HTTP_OK; - const char *rpath = req->path ? req->path : ""; - const char *s; + int access_status; #if AP_MODULE_MAGIC_AT_LEAST(20150222, 13) request_rec *r = ap_create_request(c); @@ -280,33 +278,17 @@ request_rec *h2_request_create_rec(const h2_request *req, conn_rec *c) /* Time to populate r with the data we have. */ r->request_time = req->request_time; - r->method = apr_pstrdup(r->pool, req->method); - /* Provide quick information about the request method as soon as known */ - r->method_number = ap_method_number_of(r->method); - if (r->method_number == M_GET && r->method[0] == 'H') { - r->header_only = 1; - } - r->proto_num = HTTP_VERSION(2, 0); - r->protocol = apr_pstrdup(r->pool, "HTTP/2.0"); - r->the_request = apr_psprintf(r->pool, "%s %s %s", - r->method, rpath, r->protocol); + 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_parse_uri(r, rpath); - if (r->status != HTTP_OK) { - access_status = r->status; - r->status = HTTP_OK; - goto die; - } - - /* update what we think the virtual host is based on the headers we've - * now read. may update status. - * Leave r->hostname empty, vhost will parse if form our Host: header, - * otherwise we get complains about port numbers. + /* Start with r->hostname = NULL, ap_check_request_header() will get it + * form Host: header, otherwise we get complains about port numbers. */ r->hostname = NULL; - ap_update_vhost_from_headers(r); - if (r->status != HTTP_OK) { + + /* Validate HTTP/1 request and select vhost. */ + if (!ap_parse_request_line(r) || !ap_check_request_header(r)) { access_status = r->status; r->status = HTTP_OK; goto die; @@ -314,17 +296,6 @@ request_rec *h2_request_create_rec(const h2_request *req, conn_rec *c) /* we may have switched to another server */ r->per_dir_config = r->server->lookup_defaults; - - s = apr_table_get(r->headers_in, "Expect"); - if (s && s[0]) { - if (ap_cstr_casecmp(s, "100-continue") == 0) { - r->expecting_100 = 1; - } - else { - access_status = HTTP_EXPECTATION_FAILED; - goto die; - } - } /* * Add the HTTP_IN filter here to ensure that ap_discard_request_body diff --git a/server/protocol.c b/server/protocol.c index d8e4d0ae45d..57c696aba4a 100644 --- a/server/protocol.c +++ b/server/protocol.c @@ -676,13 +676,6 @@ static int field_name_len(const char *field) static int read_request_line(request_rec *r, apr_bucket_brigade *bb) { - enum { - rrl_none, rrl_badmethod, rrl_badwhitespace, rrl_excesswhitespace, - rrl_missinguri, rrl_baduri, rrl_badprotocol, rrl_trailingtext, - rrl_badmethod09, rrl_reject09 - } deferred_error = rrl_none; - char *ll; - char *uri; apr_size_t len; int num_blank_lines = DEFAULT_LIMIT_BLANK_LINES; core_server_config *conf = ap_get_core_module_config(r->server->module_config); @@ -745,6 +738,20 @@ static int read_request_line(request_rec *r, apr_bucket_brigade *bb) } r->request_time = apr_time_now(); + return 1; +} + +AP_DECLARE(int) ap_parse_request_line(request_rec *r) +{ + core_server_config *conf = ap_get_core_module_config(r->server->module_config); + int strict = (conf->http_conformance != AP_HTTP_CONFORMANCE_UNSAFE); + enum { + rrl_none, rrl_badmethod, rrl_badwhitespace, rrl_excesswhitespace, + rrl_missinguri, rrl_baduri, rrl_badprotocol, rrl_trailingtext, + rrl_badmethod09, rrl_reject09 + } deferred_error = rrl_none; + apr_size_t len = 0; + char *uri, *ll; r->method = r->the_request; @@ -776,7 +783,6 @@ static int read_request_line(request_rec *r, apr_bucket_brigade *bb) if (deferred_error == rrl_none) deferred_error = rrl_missinguri; r->protocol = uri = ""; - len = 0; goto rrl_done; } else if (strict && ll[0] && apr_isspace(ll[1]) @@ -807,7 +813,6 @@ static int read_request_line(request_rec *r, apr_bucket_brigade *bb) /* Verify URI terminated with a single SP, or mark as specific error */ if (!ll) { r->protocol = ""; - len = 0; goto rrl_done; } else if (strict && ll[0] && apr_isspace(ll[1]) @@ -1007,6 +1012,87 @@ rrl_failed: return 0; } +AP_DECLARE(int) ap_check_request_header(request_rec *r) +{ + core_server_config *conf; + int strict_host_check; + const char *expect; + int access_status; + + conf = ap_get_core_module_config(r->server->module_config); + + /* update what we think the virtual host is based on the headers we've + * now read. may update status. + */ + strict_host_check = (conf->strict_host_check == AP_CORE_CONFIG_ON); + access_status = ap_update_vhost_from_headers_ex(r, strict_host_check); + if (strict_host_check && access_status != HTTP_OK) { + if (r->server == ap_server_conf) { + ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(10156) + "Requested hostname '%s' did not match any ServerName/ServerAlias " + "in the global server configuration ", r->hostname); + } + else { + ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(10157) + "Requested hostname '%s' did not match any ServerName/ServerAlias " + "in the matching virtual host (default vhost for " + "current connection is %s:%u)", + r->hostname, r->server->defn_name, r->server->defn_line_number); + } + r->status = access_status; + } + if (r->status != HTTP_OK) { + return 0; + } + + if ((!r->hostname && (r->proto_num >= HTTP_VERSION(1, 1))) + || ((r->proto_num == HTTP_VERSION(1, 1)) + && !apr_table_get(r->headers_in, "Host"))) { + /* + * Client sent us an HTTP/1.1 or later request without telling us the + * hostname, either with a full URL or a Host: header. We therefore + * need to (as per the 1.1 spec) send an error. As a special case, + * HTTP/1.1 mentions twice (S9, S14.23) that a request MUST contain + * a Host: header, and the server MUST respond with 400 if it doesn't. + */ + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00569) + "client sent HTTP/1.1 request without hostname " + "(see RFC2616 section 14.23): %s", r->uri); + r->status = HTTP_BAD_REQUEST; + return 0; + } + + /* we may have switched to another server */ + conf = ap_get_core_module_config(r->server->module_config); + + if (((expect = apr_table_get(r->headers_in, "Expect")) != NULL) + && (expect[0] != '\0')) { + /* + * The Expect header field was added to HTTP/1.1 after RFC 2068 + * as a means to signal when a 100 response is desired and, + * unfortunately, to signal a poor man's mandatory extension that + * the server must understand or return 417 Expectation Failed. + */ + if (ap_cstr_casecmp(expect, "100-continue") == 0) { + r->expecting_100 = 1; + } + else if (conf->http_expect_strict == AP_HTTP_EXPECT_STRICT_DISABLE) { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02595) + "client sent an unrecognized expectation value " + "of Expect (not fatal): %s", expect); + } + else { + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00570) + "client sent an unrecognized expectation value " + "of Expect: %s", expect); + r->status = HTTP_EXPECTATION_FAILED; + return 0; + } + } + + return 1; +} + static int table_do_fn_check_lengths(void *r_, const char *key, const char *value) { @@ -1346,23 +1432,19 @@ AP_DECLARE(request_rec *) ap_create_request(conn_rec *conn) request_rec *ap_read_request(conn_rec *conn) { - const char *expect; int access_status; apr_bucket_brigade *tmp_bb; apr_socket_t *csd; apr_interval_time_t cur_timeout; - core_server_config *conf; - int strict_host_check; request_rec *r = ap_create_request(conn); - conf = ap_get_core_module_config(r->server->module_config); tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc); ap_run_pre_read_request(r, conn); /* Get the request... */ - if (!read_request_line(r, tmp_bb)) { + if (!read_request_line(r, tmp_bb) || !ap_parse_request_line(r)) { apr_brigade_cleanup(tmp_bb); switch (r->status) { case HTTP_REQUEST_URI_TOO_LARGE: @@ -1444,50 +1526,13 @@ request_rec *ap_read_request(conn_rec *conn) } } - /* update what we think the virtual host is based on the headers we've - * now read. may update status. - */ - strict_host_check = (conf->strict_host_check == AP_CORE_CONFIG_ON); - access_status = ap_update_vhost_from_headers_ex(r, strict_host_check); - if (strict_host_check && access_status != HTTP_OK) { - if (r->server == ap_server_conf) { - ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(10156) - "Requested hostname '%s' did not match any ServerName/ServerAlias " - "in the global server configuration ", r->hostname); - } else { - ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(10157) - "Requested hostname '%s' did not match any ServerName/ServerAlias " - "in the matching virtual host (default vhost for " - "current connection is %s:%u)", - r->hostname, r->server->defn_name, r->server->defn_line_number); - } - goto die_early; - } - if (r->status != HTTP_OK) { + if (!ap_check_request_header(r)) { access_status = r->status; goto die_early; } - if ((!r->hostname && (r->proto_num >= HTTP_VERSION(1, 1))) - || ((r->proto_num == HTTP_VERSION(1, 1)) - && !apr_table_get(r->headers_in, "Host"))) { - /* - * Client sent us an HTTP/1.1 or later request without telling us the - * hostname, either with a full URL or a Host: header. We therefore - * need to (as per the 1.1 spec) send an error. As a special case, - * HTTP/1.1 mentions twice (S9, S14.23) that a request MUST contain - * a Host: header, and the server MUST respond with 400 if it doesn't. - */ - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00569) - "client sent HTTP/1.1 request without hostname " - "(see RFC2616 section 14.23): %s", r->uri); - access_status = HTTP_BAD_REQUEST; - goto die_early; - } - /* we may have switched to another server */ r->per_dir_config = r->server->lookup_defaults; - conf = ap_get_core_module_config(r->server->module_config); /* Toggle to the Host:-based vhost's timeout mode to fetch the * request body and send the response body, if needed. @@ -1510,33 +1555,6 @@ request_rec *ap_read_request(conn_rec *conn) goto die; } - if (((expect = apr_table_get(r->headers_in, "Expect")) != NULL) - && (expect[0] != '\0')) { - /* - * The Expect header field was added to HTTP/1.1 after RFC 2068 - * as a means to signal when a 100 response is desired and, - * unfortunately, to signal a poor man's mandatory extension that - * the server must understand or return 417 Expectation Failed. - */ - if (ap_cstr_casecmp(expect, "100-continue") == 0) { - r->expecting_100 = 1; - } - else { - if (conf->http_expect_strict != AP_HTTP_EXPECT_STRICT_DISABLE) { - ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00570) - "client sent an unrecognized expectation value " - "of Expect: %s", expect); - access_status = HTTP_EXPECTATION_FAILED; - goto die; - } - else { - ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02595) - "client sent an unrecognized expectation value " - "of Expect (not fatal): %s", expect); - } - } - } - AP_READ_REQUEST_SUCCESS((uintptr_t)r, (char *)r->method, (char *)r->uri, (char *)r->server->defn_name, r->status);