Hi, On Wed, Jan 25, 2017 at 9:17 AM, Rashmi Srinivasan <rashmisrinivasan2007@xxxxxxxxx> wrote: > We are trying to port the fix for CVE (CVE-2016-8743) to 2.4.18. Tried > checking the revision on git for the list of files fixed for this CVE. > There are lots of changes related to RFC7320 and was difficult to figure out > the files changed for this CVE as We couldnt find the CVE-2016-8743 in the > log either. The branch [1] collects all the related changes between versions 2.4.25 (latest) and 2.4.23 (previous). Attached is the output of: $ svn diff -x-p https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@r1767912 https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict >httpd-2.4.23-CVE-2016-8743.patch It should apply cleanly to 2.4.23, though it may not to 2.4.18 (possibly more work needed...). Hope this helps. Regards, Yann. [1] https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict
Index: docs/manual/mod/core.xml =================================================================== --- docs/manual/mod/core.xml (.../2.4.x) (revision 1767912) +++ docs/manual/mod/core.xml (.../2.4.x-merge-http-strict) (revision 1780175) @@ -1239,6 +1239,82 @@ EnableSendfile On </directivesynopsis> <directivesynopsis> +<name>HttpProtocolOptions</name> +<description>Modify restrictions on HTTP Request Messages</description> +<syntax>HttpProtocolOptions [Strict|Unsafe] [RegisteredMethods|LenientMethods] + [Allow0.9|Require1.0]</syntax> +<default>HttpProtocolOptions Strict LenientMethods Allow0.9</default> +<contextlist><context>server config</context> +<context>virtual host</context></contextlist> +<compatibility>2.2.32 or 2.4.24 and later</compatibility> + +<usage> + <p>This directive changes the rules applied to the HTTP Request Line + (<a href="https://tools.ietf.org/html/rfc7230#section-3.1.1" + >RFC 7230 §3.1.1</a>) and the HTTP Request Header Fields + (<a href="https://tools.ietf.org/html/rfc7230#section-3.2" + >RFC 7230 §3.2</a>), which are now applied by default or using + the <code>Strict</code> option. Due to legacy modules, applications or + custom user-agents which must be deperecated the <code>Unsafe</code> + option has been added to revert to the legacy behaviors. These rules + are applied prior to request processing, so must be configured at the + global or default (first) matching virtual host section, by IP/port + interface (and not by name) to be honored.</p> + + <p>Prior to the introduction of this directive, the Apache HTTP Server + request message parsers were tolerant of a number of forms of input + which did not conform to the protocol. + <a href="https://tools.ietf.org/html/rfc7230#section-9.4" + >RFC 7230 §9.4 Request Splitting</a> and + <a href="https://tools.ietf.org/html/rfc7230#section-9.5" + >§9.5 Response Smuggling</a> call out only two of the potential + risks of accepting non-conformant request messages, while + <a href="https://tools.ietf.org/html/rfc7230#section-3.5" + >RFC 7230 §3.5</a> "Message Parsing Robustness" identify the + risks of accepting obscure whitespace and request message formatting. + As of the introduction of this directive, all grammer rules of the + specification are enforced in the default <code>Strict</code> operating + mode, and the strict whitespace suggested by section 3.5 is enforced + and cannot be relaxed.</p> + + <p>Users are strongly cautioned against toggling the <code>Unsafe</code> + mode of operation, particularly on outward-facing, publicly accessible + server deployments. If an interface is required for faulty monitoring + or other custom service consumers running on an intranet, users should + toggle the Unsafe option only on a specific virtual host configured + to service their internal private network.</p> + + <p>Reviewing the messages logged to the <directive>ErrorLog</directive>, + configured with <directive>LogLevel</directive> <code>debug</code> level, + can help identify such faulty requests along with their origin. + Users should pay particular attention to the 400 responses in the access + log for invalid requests which were unexpectedly rejected.</p> + + <p><a href="https://tools.ietf.org/html/rfc7231#section-4.1" + >RFC 7231 §4.1</a> "Request Methods" "Overview" requires that + origin servers shall respond with an error when an unsupported method + is encountered in the request line. This already happens when the + <code>LenientMethods</code> option is used, but administrators may wish + to toggle the <code>RegisteredMethods</code> option and register any + non-standard methods using the <directive>RegisterHttpMethod</directive> + directive, particularly if the <code>Unsafe</code> option has been toggled. + The <code>RegisteredMethods</code> option should <strong>not</strong> + be toggled for forward proxy hosts, as the methods supported by the + origin servers are unknown to the proxy server.</p> + + <p><a href="https://tools.ietf.org/html/rfc2616#section-19.6" + >RFC 2616 §19.6</a> "Compatibility With Previous Versions" had + encouraged HTTP servers to support legacy HTTP/0.9 requests. RFC 7230 + superceeds this with "The expectation to support HTTP/0.9 requests has + been removed" and offers additional comments in + <a href="https://tools.ietf.org/html/rfc7230#appendix-A" + >RFC 7230 Appendix A</a>. The <code>Require1.0</code> option allows + the user to remove support of the default <code>Allow0.9</code> option's + behavior.</p> +</usage> +</directivesynopsis> + +<directivesynopsis> <name>Error</name> <description>Abort configuration parsing with a custom error message</description> <syntax>Error <var>message</var></syntax> @@ -4766,5 +4842,20 @@ as if 'QualifyRedirectURL ON' was configured.</com </directivesynopsis> +<directivesynopsis> +<name>RegisterHttpMethod</name> +<description>Register non-standard HTTP methods</description> +<syntax>RegisterHttpMethod <var>method</var> [<var>method</var> [...]]</syntax> +<contextlist><context>server config</context></contextlist> +<usage> +<p>HTTP Methods that are not conforming to the relvant RFCs are normally +rejected by request processing in Apache HTTPD. To avoid this, modules +can register non-standard HTTP methods they support. +The <directive>RegisterHttpMethod</directive> allows to register such +methods manually. This can be useful for if such methods are forwared +for external processing, e.g. to a CGI script.</p> +</usage> +</directivesynopsis> + </modulesynopsis> Index: docs/manual =================================================================== --- docs/manual (.../2.4.x) (revision 1767912) +++ docs/manual (.../2.4.x-merge-http-strict) (revision 1780175) Property changes on: docs/manual ___________________________________________________________________ Modified: svn:mergeinfo ## -0,0 +0,1 ## Merged /httpd/httpd/trunk/docs/manual:r1407599,1756540,1756649,1756729,1756862,1756959,1757589,1757711,1757920,1758265-1758266,1764961 Index: server/gen_test_char.c =================================================================== --- server/gen_test_char.c (.../2.4.x) (revision 1767912) +++ server/gen_test_char.c (.../2.4.x-merge-http-strict) (revision 1780175) @@ -16,11 +16,11 @@ #ifdef CROSS_COMPILE +#include <ctype.h> #define apr_isalnum(c) (isalnum(((unsigned char)(c)))) #define apr_isalpha(c) (isalpha(((unsigned char)(c)))) #define apr_iscntrl(c) (iscntrl(((unsigned char)(c)))) #define apr_isprint(c) (isprint(((unsigned char)(c)))) -#include <ctype.h> #define APR_HAVE_STDIO_H 1 #define APR_HAVE_STRING_H 1 @@ -52,11 +52,13 @@ #define T_ESCAPE_LOGITEM (0x10) #define T_ESCAPE_FORENSIC (0x20) #define T_ESCAPE_URLENCODED (0x40) +#define T_HTTP_CTRLS (0x80) +#define T_VCHAR_OBSTEXT (0x100) int main(int argc, char *argv[]) { unsigned c; - unsigned char flags; + unsigned short flags; printf("/* this file is automatically generated by gen_test_char, " "do not edit */\n" @@ -67,8 +69,10 @@ int main(int argc, char *argv[]) "#define T_ESCAPE_LOGITEM (%u)\n" "#define T_ESCAPE_FORENSIC (%u)\n" "#define T_ESCAPE_URLENCODED (%u)\n" + "#define T_HTTP_CTRLS (%u)\n" + "#define T_VCHAR_OBSTEXT (%u)\n" "\n" - "static const unsigned char test_char_table[256] = {", + "static const unsigned short test_char_table[256] = {", T_ESCAPE_SHELL_CMD, T_ESCAPE_PATH_SEGMENT, T_OS_ESCAPE_PATH, @@ -75,11 +79,13 @@ int main(int argc, char *argv[]) T_HTTP_TOKEN_STOP, T_ESCAPE_LOGITEM, T_ESCAPE_FORENSIC, - T_ESCAPE_URLENCODED); + T_ESCAPE_URLENCODED, + T_HTTP_CTRLS, + T_VCHAR_OBSTEXT); for (c = 0; c < 256; ++c) { flags = 0; - if (c % 20 == 0) + if (c % 8 == 0) printf("\n "); /* escape_shell_cmd */ @@ -107,7 +113,7 @@ int main(int argc, char *argv[]) flags |= T_ESCAPE_PATH_SEGMENT; } - if (!apr_isalnum(c) && !strchr("$-_.+!*'(),:@&=/~", c)) { + if (!apr_isalnum(c) && !strchr("$-_.+!*'(),:;@&=/~", c)) { flags |= T_OS_ESCAPE_PATH; } @@ -115,11 +121,32 @@ int main(int argc, char *argv[]) flags |= T_ESCAPE_URLENCODED; } - /* these are the "tspecials" (RFC2068) or "separators" (RFC2616) */ - if (c && (apr_iscntrl(c) || strchr(" \t()<>@,;:\\\"/[]?={}", c))) { + /* Stop for any non-'token' character, including ctrls, obs-text, + * and "tspecials" (RFC2068) a.k.a. "separators" (RFC2616), which + * is easer to express as characters remaining in the ASCII token set + */ + if (!c || !(apr_isalnum(c) || strchr("!#$%&'*+-.^_`|~", c))) { flags |= T_HTTP_TOKEN_STOP; } + /* Catch CTRLs other than VCHAR, HT and SP, and obs-text (RFC7230 3.2) + * This includes only the C0 plane, not C1 (which is obs-text itself.) + * XXX: We should verify that all ASCII C0 ctrls/DEL corresponding to + * the current EBCDIC translation are captured, and ASCII C1 ctrls + * corresponding are all permitted (as they fall under obs-text rule) + */ + if (!c || (apr_iscntrl(c) && c != '\t')) { + flags |= T_HTTP_CTRLS; + } + + /* From RFC3986, the specific sets of gen-delims, sub-delims (2.2), + * and unreserved (2.3) that are possible somewhere within a URI. + * Spec requires all others to be %XX encoded, including obs-text. + */ + if (c && !apr_iscntrl(c) && c != ' ') { + flags |= T_VCHAR_OBSTEXT; + } + /* For logging, escape all control characters, * double quotes (because they delimit the request in the log file) * backslashes (because we use backslash for escaping) @@ -137,7 +164,7 @@ int main(int argc, char *argv[]) flags |= T_ESCAPE_FORENSIC; } - printf("%u%c", flags, (c < 255) ? ',' : ' '); + printf("0x%03x%c", flags, (c < 255) ? ',' : ' '); } printf("\n};\n"); Index: server/util.c =================================================================== --- server/util.c (.../2.4.x) (revision 1767912) +++ server/util.c (.../2.4.x-merge-http-strict) (revision 1780175) @@ -79,7 +79,7 @@ * char in here and get it to work, because if char is signed then it * will first be sign extended. */ -#define TEST_CHAR(c, f) (test_char_table[(unsigned)(c)] & (f)) +#define TEST_CHAR(c, f) (test_char_table[(unsigned char)(c)] & (f)) /* Win32/NetWare/OS2 need to check for both forward and back slashes * in ap_getparents() and ap_escape_url. @@ -1525,7 +1525,7 @@ AP_DECLARE(const char *) ap_parse_token_list_stric while (!string_end) { const unsigned char c = (unsigned char)*cur; - if (!TEST_CHAR(c, T_HTTP_TOKEN_STOP) && c != '\0') { + if (!TEST_CHAR(c, T_HTTP_TOKEN_STOP)) { /* Non-separator character; we are finished with leading * whitespace. We must never have encountered any trailing * whitespace before the delimiter (comma) */ @@ -1593,6 +1593,37 @@ AP_DECLARE(const char *) ap_parse_token_list_stric return NULL; } +/* Scan a string for HTTP VCHAR/obs-text characters including HT and SP + * (as used in header values, for example, in RFC 7230 section 3.2) + * returning the pointer to the first non-HT ASCII ctrl character. + */ +AP_DECLARE(const char *) ap_scan_http_field_content(const char *ptr) +{ + for ( ; !TEST_CHAR(*ptr, T_HTTP_CTRLS); ++ptr) ; + + return ptr; +} + +/* Scan a string for HTTP token characters, returning the pointer to + * the first non-token character. + */ +AP_DECLARE(const char *) ap_scan_http_token(const char *ptr) +{ + for ( ; !TEST_CHAR(*ptr, T_HTTP_TOKEN_STOP); ++ptr) ; + + return ptr; +} + +/* Scan a string for visible ASCII (0x21-0x7E) or obstext (0x80+) + * and return a pointer to the first ctrl/space character encountered. + */ +AP_DECLARE(const char *) ap_scan_vchar_obstext(const char *ptr) +{ + for ( ; TEST_CHAR(*ptr, T_VCHAR_OBSTEXT); ++ptr) ; + + return ptr; +} + /* Retrieve a token, spacing over it and returning a pointer to * the first non-white byte afterwards. Note that these tokens * are delimited by semis and commas; and can also be delimited Index: server/core.c =================================================================== --- server/core.c (.../2.4.x) (revision 1767912) +++ server/core.c (.../2.4.x-merge-http-strict) (revision 1780175) @@ -519,6 +519,15 @@ static void *merge_core_server_configs(apr_pool_t if (virt->trace_enable != AP_TRACE_UNSET) conf->trace_enable = virt->trace_enable; + if (virt->http09_enable != AP_HTTP09_UNSET) + conf->http09_enable = virt->http09_enable; + + if (virt->http_conformance != AP_HTTP_CONFORMANCE_UNSET) + conf->http_conformance = virt->http_conformance; + + if (virt->http_methods != AP_HTTP_METHODS_UNSET) + conf->http_methods = virt->http_methods; + /* no action for virt->accf_map, not allowed per-vhost */ if (virt->protocol) @@ -3895,6 +3904,57 @@ static const char *set_protocols_honor_order(cmd_p return NULL; } +static const char *set_http_protocol_options(cmd_parms *cmd, void *dummy, + const char *arg) +{ + core_server_config *conf = + ap_get_core_module_config(cmd->server->module_config); + + if (strcasecmp(arg, "allow0.9") == 0) + conf->http09_enable |= AP_HTTP09_ENABLE; + else if (strcasecmp(arg, "require1.0") == 0) + conf->http09_enable |= AP_HTTP09_DISABLE; + else if (strcasecmp(arg, "strict") == 0) + conf->http_conformance |= AP_HTTP_CONFORMANCE_STRICT; + else if (strcasecmp(arg, "unsafe") == 0) + conf->http_conformance |= AP_HTTP_CONFORMANCE_UNSAFE; + else if (strcasecmp(arg, "registeredmethods") == 0) + conf->http_methods |= AP_HTTP_METHODS_REGISTERED; + else if (strcasecmp(arg, "lenientmethods") == 0) + conf->http_methods |= AP_HTTP_METHODS_LENIENT; + else + return "HttpProtocolOptions accepts " + "'Unsafe' or 'Strict' (default), " + "'RegisteredMethods' or 'LenientMethods' (default), and " + "'Require1.0' or 'Allow0.9' (default)"; + + if ((conf->http09_enable & AP_HTTP09_ENABLE) + && (conf->http09_enable & AP_HTTP09_DISABLE)) + return "HttpProtocolOptions 'Allow0.9' and 'Require1.0'" + " are mutually exclusive"; + + if ((conf->http_conformance & AP_HTTP_CONFORMANCE_STRICT) + && (conf->http_conformance & AP_HTTP_CONFORMANCE_UNSAFE)) + return "HttpProtocolOptions 'Strict' and 'Unsafe'" + " are mutually exclusive"; + + if ((conf->http_methods & AP_HTTP_METHODS_REGISTERED) + && (conf->http_methods & AP_HTTP_METHODS_LENIENT)) + return "HttpProtocolOptions 'RegisteredMethods' and 'LenientMethods'" + " are mutually exclusive"; + + return NULL; +} + +static const char *set_http_method(cmd_parms *cmd, void *conf, const char *arg) +{ + const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY); + if (err != NULL) + return err; + ap_method_register(cmd->pool, arg); + return NULL; +} + static apr_hash_t *errorlog_hash; static int log_constant_item(const ap_errorlog_info *info, const char *arg, @@ -4419,6 +4479,12 @@ AP_INIT_ITERATE("Protocols", set_protocols, NULL, AP_INIT_TAKE1("ProtocolsHonorOrder", set_protocols_honor_order, NULL, RSRC_CONF, "'off' (default) or 'on' to respect given order of protocols, " "by default the client specified order determines selection"), +AP_INIT_ITERATE("HttpProtocolOptions", set_http_protocol_options, NULL, RSRC_CONF, + "'Allow0.9' or 'Require1.0' (default); " + "'RegisteredMethods' or 'LenientMethods' (default); " + "'Unsafe' or 'Strict' (default). Sets HTTP acceptance rules"), +AP_INIT_ITERATE("RegisterHttpMethod", set_http_method, NULL, RSRC_CONF, + "Registers non-standard HTTP methods"), { NULL } }; Index: server/protocol.c =================================================================== --- server/protocol.c (.../2.4.x) (revision 1767912) +++ server/protocol.c (.../2.4.x-merge-http-strict) (revision 1780175) @@ -191,6 +191,10 @@ AP_DECLARE(apr_time_t) ap_rationalize_mtime(reques /* Get a line of protocol input, including any continuation lines * caused by MIME folding (or broken clients) if fold != 0, and place it * in the buffer s, of size n bytes, without the ending newline. + * + * Pulls from r->proto_input_filters instead of r->input_filters for + * stricter protocol adherence and better input filter behavior during + * chunked trailer processing (for http). * * If s is NULL, ap_rgetline_core will allocate necessary memory from r->pool. * @@ -200,7 +204,7 @@ AP_DECLARE(apr_time_t) ap_rationalize_mtime(reques * APR_ENOSPC is returned if there is not enough buffer space. * Other errors may be returned on other errors. * - * The LF is *not* returned in the buffer. Therefore, a *read of 0 + * The [CR]LF are *not* returned in the buffer. Therefore, a *read of 0 * indicates that an empty line was read. * * Notes: Because the buffer uses 1 char for NUL, the most we can return is @@ -211,7 +215,7 @@ AP_DECLARE(apr_time_t) ap_rationalize_mtime(reques */ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n, apr_size_t *read, request_rec *r, - int fold, apr_bucket_brigade *bb) + int flags, apr_bucket_brigade *bb) { apr_status_t rv; apr_bucket *e; @@ -218,6 +222,8 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s apr_size_t bytes_handled = 0, current_alloc = 0; char *pos, *last_char = *s; int do_alloc = (*s == NULL), saw_eos = 0; + int fold = flags & AP_GETLINE_FOLD; + int crlf = flags & AP_GETLINE_CRLF; /* * Initialize last_char as otherwise a random value will be compared @@ -229,13 +235,15 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s for (;;) { apr_brigade_cleanup(bb); - rv = ap_get_brigade(r->input_filters, bb, AP_MODE_GETLINE, + rv = ap_get_brigade(r->proto_input_filters, bb, AP_MODE_GETLINE, APR_BLOCK_READ, 0); if (rv != APR_SUCCESS) { return rv; } - /* Something horribly wrong happened. Someone didn't block! */ + /* Something horribly wrong happened. Someone didn't block! + * (this also happens at the end of each keepalive connection) + */ if (APR_BRIGADE_EMPTY(bb)) { return APR_EGENERAL; } @@ -321,6 +329,13 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s } } + if (crlf && (last_char <= *s || last_char[-1] != APR_ASCII_CR)) { + *last_char = '\0'; + bytes_handled = last_char - *s; + *read = bytes_handled; + return APR_EINVAL; + } + /* Now NUL-terminate the string at the end of the line; * if the last-but-one character is a CR, terminate there */ if (last_char > *s && last_char[-1] == APR_ASCII_CR) { @@ -343,7 +358,7 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s apr_brigade_cleanup(bb); /* We only care about the first byte. */ - rv = ap_get_brigade(r->input_filters, bb, AP_MODE_SPECULATIVE, + rv = ap_get_brigade(r->proto_input_filters, bb, AP_MODE_SPECULATIVE, APR_BLOCK_READ, 1); if (rv != APR_SUCCESS) { return rv; @@ -394,7 +409,8 @@ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s */ if (do_alloc) { tmp = NULL; - } else { + } + else { /* We're null terminated. */ tmp = last_char; } @@ -464,7 +480,7 @@ AP_DECLARE(apr_status_t) ap_rgetline(char **s, apr } #endif -AP_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold) +AP_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int flags) { char *tmp_s = s; apr_status_t rv; @@ -472,7 +488,7 @@ AP_DECLARE(apr_status_t) ap_rgetline(char **s, apr apr_bucket_brigade *tmp_bb; tmp_bb = apr_brigade_create(r->pool, r->connection->bucket_alloc); - rv = ap_rgetline(&tmp_s, n, &len, r, fold, tmp_bb); + rv = ap_rgetline(&tmp_s, n, &len, r, flags, tmp_bb); apr_brigade_destroy(tmp_bb); /* Map the out-of-space condition to the old API. */ @@ -552,16 +568,29 @@ AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r, } } +/* get the length of the field name for logging, but no more than 80 bytes */ +#define LOG_NAME_MAX_LEN 80 +static int field_name_len(const char *field) +{ + const char *end = ap_strchr_c(field, ':'); + if (end == NULL || end - field > LOG_NAME_MAX_LEN) + return LOG_NAME_MAX_LEN; + return end - field; +} + static int read_request_line(request_rec *r, apr_bucket_brigade *bb) { - const char *ll; - const char *uri; - const char *pro; - - unsigned int major = 1, minor = 0; /* Assume HTTP/1.0 if non-"HTTP" protocol */ - char http[5]; + 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); + int strict = (conf->http_conformance != AP_HTTP_CONFORMANCE_UNSAFE); /* Read past empty lines until we get a real request line, * a read error, the connection closes (EOF), or we timeout. @@ -586,7 +615,7 @@ static int read_request_line(request_rec *r, apr_b */ r->the_request = NULL; rv = ap_rgetline(&(r->the_request), (apr_size_t)(r->server->limit_req_line + 2), - &len, r, 0, bb); + &len, r, strict ? AP_GETLINE_CRLF : 0, bb); if (rv != APR_SUCCESS) { r->request_time = apr_time_now(); @@ -596,7 +625,7 @@ static int read_request_line(request_rec *r, apr_b * happen if it exceeds the configured limit for a request-line. */ if (APR_STATUS_IS_ENOSPC(rv)) { - r->status = HTTP_REQUEST_URI_TOO_LARGE; + r->status = HTTP_REQUEST_URI_TOO_LARGE; } else if (APR_STATUS_IS_TIMEUP(rv)) { r->status = HTTP_REQUEST_TIME_OUT; @@ -617,58 +646,266 @@ static int read_request_line(request_rec *r, apr_b } r->request_time = apr_time_now(); - ll = r->the_request; - r->method = ap_getword_white(r->pool, &ll); - uri = ap_getword_white(r->pool, &ll); + r->method = r->the_request; - if (!*r->method || !*uri) { - r->status = HTTP_BAD_REQUEST; - r->proto_num = HTTP_VERSION(1,0); - r->protocol = apr_pstrdup(r->pool, "HTTP/1.0"); - return 0; + /* If there is whitespace before a method, skip it and mark in error */ + if (apr_isspace(*r->method)) { + deferred_error = rrl_badwhitespace; + for ( ; apr_isspace(*r->method); ++r->method) + ; } - /* Provide quick information about the request method as soon as known */ + /* Scan the method up to the next whitespace, ensure it contains only + * valid http-token characters, otherwise mark in error + */ + if (strict) { + ll = (char*) ap_scan_http_token(r->method); + } + else { + ll = (char*) ap_scan_vchar_obstext(r->method); + } + if (((ll == r->method) || (*ll && !apr_isspace(*ll))) + && deferred_error == rrl_none) { + deferred_error = rrl_badmethod; + ll = strpbrk(ll, "\t\n\v\f\r "); + } + + /* Verify method terminated with a single SP, or mark as specific error */ + if (!ll) { + 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]) + && deferred_error == rrl_none) { + deferred_error = rrl_excesswhitespace; + } + + /* Advance uri pointer over leading whitespace, NUL terminate the method + * If non-SP whitespace is encountered, mark as specific error + */ + for (uri = ll; apr_isspace(*uri); ++uri) + if (*uri != ' ' && deferred_error == rrl_none) + deferred_error = rrl_badwhitespace; + *ll = '\0'; + + if (!*uri && deferred_error == rrl_none) + deferred_error = rrl_missinguri; + + /* Scan the URI up to the next whitespace, ensure it contains no raw + * control characters, otherwise mark in error + */ + ll = (char*) ap_scan_vchar_obstext(uri); + if (ll == uri || (*ll && !apr_isspace(*ll))) { + deferred_error = rrl_baduri; + ll = strpbrk(ll, "\t\n\v\f\r "); + } + + /* 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]) + && deferred_error == rrl_none) { + deferred_error = rrl_excesswhitespace; + } + + /* Advance protocol pointer over leading whitespace, NUL terminate the uri + * If non-SP whitespace is encountered, mark as specific error + */ + for (r->protocol = ll; apr_isspace(*r->protocol); ++r->protocol) + if (*r->protocol != ' ' && deferred_error == rrl_none) + deferred_error = rrl_badwhitespace; + *ll = '\0'; + + /* Scan the protocol up to the next whitespace, validation comes later */ + if (!(ll = (char*) ap_scan_vchar_obstext(r->protocol))) { + len = strlen(r->protocol); + goto rrl_done; + } + len = ll - r->protocol; + + /* Advance over trailing whitespace, if found mark in error, + * determine if trailing text is found, unconditionally mark in error, + * finally NUL terminate the protocol string + */ + if (*ll && !apr_isspace(*ll)) { + deferred_error = rrl_badprotocol; + } + else if (strict && *ll) { + deferred_error = rrl_excesswhitespace; + } + else { + for ( ; apr_isspace(*ll); ++ll) + if (*ll != ' ' && deferred_error == rrl_none) + deferred_error = rrl_badwhitespace; + if (*ll && deferred_error == rrl_none) + deferred_error = rrl_trailingtext; + } + *((char *)r->protocol + len) = '\0'; + +rrl_done: + /* For internal integrety and palloc efficiency, reconstruct the_request + * in one palloc, using only single SP characters, per spec. + */ + r->the_request = apr_pstrcat(r->pool, r->method, *uri ? " " : NULL, uri, + *r->protocol ? " " : NULL, r->protocol, NULL); + + if (len == 8 + && r->protocol[0] == 'H' && r->protocol[1] == 'T' + && r->protocol[2] == 'T' && r->protocol[3] == 'P' + && r->protocol[4] == '/' && apr_isdigit(r->protocol[5]) + && r->protocol[6] == '.' && apr_isdigit(r->protocol[7]) + && r->protocol[5] != '0') { + r->assbackwards = 0; + r->proto_num = HTTP_VERSION(r->protocol[5] - '0', r->protocol[7] - '0'); + } + else if (len == 8 + && (r->protocol[0] == 'H' || r->protocol[0] == 'h') + && (r->protocol[1] == 'T' || r->protocol[1] == 't') + && (r->protocol[2] == 'T' || r->protocol[2] == 't') + && (r->protocol[3] == 'P' || r->protocol[3] == 'p') + && r->protocol[4] == '/' && apr_isdigit(r->protocol[5]) + && r->protocol[6] == '.' && apr_isdigit(r->protocol[7]) + && r->protocol[5] != '0') { + r->assbackwards = 0; + r->proto_num = HTTP_VERSION(r->protocol[5] - '0', r->protocol[7] - '0'); + if (strict && deferred_error == rrl_none) + deferred_error = rrl_badprotocol; + else + memcpy((char*)r->protocol, "HTTP", 4); + } + else if (r->protocol[0]) { + r->proto_num = HTTP_VERSION(0, 9); + /* Defer setting the r->protocol string till error msg is composed */ + if (deferred_error == rrl_none) + deferred_error = rrl_badprotocol; + } + else { + r->assbackwards = 1; + r->protocol = apr_pstrdup(r->pool, "HTTP/0.9"); + r->proto_num = HTTP_VERSION(0, 9); + } + + /* Determine the method_number and parse the uri prior to invoking error + * handling, such that these fields are available for subsitution + */ r->method_number = ap_method_number_of(r->method); - if (r->method_number == M_GET && r->method[0] == 'H') { + if (r->method_number == M_GET && r->method[0] == 'H') r->header_only = 1; + + ap_parse_uri(r, uri); + + /* With the request understood, we can consider HTTP/0.9 specific errors */ + if (r->proto_num == HTTP_VERSION(0, 9) && deferred_error == rrl_none) { + if (conf->http09_enable == AP_HTTP09_DISABLE) + deferred_error = rrl_reject09; + else if (strict && (r->method_number != M_GET || r->header_only)) + deferred_error = rrl_badmethod09; } - ap_parse_uri(r, uri); - if (r->status != HTTP_OK) { - r->proto_num = HTTP_VERSION(1,0); - r->protocol = apr_pstrdup(r->pool, "HTTP/1.0"); + /* Now that the method, uri and protocol are all processed, + * we can safely resume any deferred error reporting + */ + if (deferred_error != rrl_none) { + if (deferred_error == rrl_badmethod) + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03445) + "HTTP Request Line; Invalid method token: '%.*s'", + field_name_len(r->method), r->method); + else if (deferred_error == rrl_badmethod09) + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03444) + "HTTP Request Line; Invalid method token: '%.*s'" + " (only GET is allowed for HTTP/0.9 requests)", + field_name_len(r->method), r->method); + else if (deferred_error == rrl_missinguri) + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03446) + "HTTP Request Line; Missing URI"); + else if (deferred_error == rrl_baduri) + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03454) + "HTTP Request Line; URI incorrectly encoded: '%.*s'", + field_name_len(r->uri), r->uri); + else if (deferred_error == rrl_badwhitespace) + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03447) + "HTTP Request Line; Invalid whitespace"); + else if (deferred_error == rrl_excesswhitespace) + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03448) + "HTTP Request Line; Excess whitespace " + "(disallowed by HttpProtocolOptions Strict"); + else if (deferred_error == rrl_trailingtext) + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03449) + "HTTP Request Line; Extraneous text found '%.*s' " + "(perhaps whitespace was injected?)", + field_name_len(ll), ll); + else if (deferred_error == rrl_reject09) + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02401) + "HTTP Request Line; Rejected HTTP/0.9 request"); + else if (deferred_error == rrl_badprotocol) + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02418) + "HTTP Request Line; Unrecognized protocol '%.*s' " + "(perhaps whitespace was injected?)", + field_name_len(r->protocol), r->protocol); + r->status = HTTP_BAD_REQUEST; + goto rrl_failed; + } + + if (conf->http_methods == AP_HTTP_METHODS_REGISTERED + && r->method_number == M_INVALID) { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02423) + "HTTP Request Line; Unrecognized HTTP method: '%.*s' " + "(disallowed by RegisteredMethods)", + field_name_len(r->method), r->method); + r->status = HTTP_NOT_IMPLEMENTED; + /* This can't happen in an HTTP/0.9 request, we verified GET above */ return 0; } - if (ll[0]) { - r->assbackwards = 0; - pro = ll; - len = strlen(ll); - } else { - r->assbackwards = 1; - pro = "HTTP/0.9"; - len = 8; + if (r->status != HTTP_OK) { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03450) + "HTTP Request Line; Unable to parse URI: '%.*s'", + field_name_len(r->uri), r->uri); + goto rrl_failed; } - r->protocol = apr_pstrmemdup(r->pool, pro, len); - /* Avoid sscanf in the common case */ - if (len == 8 - && pro[0] == 'H' && pro[1] == 'T' && pro[2] == 'T' && pro[3] == 'P' - && pro[4] == '/' && apr_isdigit(pro[5]) && pro[6] == '.' - && apr_isdigit(pro[7])) { - r->proto_num = HTTP_VERSION(pro[5] - '0', pro[7] - '0'); + if (strict) { + if (r->parsed_uri.fragment) { + /* RFC3986 3.5: no fragment */ + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02421) + "HTTP Request Line; URI must not contain a fragment"); + r->status = HTTP_BAD_REQUEST; + goto rrl_failed; + } + if (r->parsed_uri.user || r->parsed_uri.password) { + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02422) + "HTTP Request Line; URI must not contain a " + "username/password"); + r->status = HTTP_BAD_REQUEST; + goto rrl_failed; + } } - else if (3 == sscanf(r->protocol, "%4s/%u.%u", http, &major, &minor) - && (strcasecmp("http", http) == 0) - && (minor < HTTP_VERSION(1, 0)) ) /* don't allow HTTP/0.1000 */ - r->proto_num = HTTP_VERSION(major, minor); - else - r->proto_num = HTTP_VERSION(1, 0); return 1; + +rrl_failed: + if (r->proto_num == HTTP_VERSION(0, 9)) { + /* Send all parsing and protocol error response with 1.x behavior, + * and reserve 505 errors for actual HTTP protocols presented. + * As called out in RFC7230 3.5, any errors parsing the protocol + * from the request line are nearly always misencoded HTTP/1.x + * requests. Only a valid 0.9 request with no parsing errors + * at all may be treated as a simple request, if allowed. + */ + r->assbackwards = 0; + r->connection->keepalive = AP_CONN_CLOSE; + r->proto_num = HTTP_VERSION(1, 0); + r->protocol = apr_pstrdup(r->pool, "HTTP/1.0"); + } + return 0; } static int table_do_fn_check_lengths(void *r_, const char *key, @@ -680,26 +917,13 @@ static int table_do_fn_check_lengths(void *r_, con r->status = HTTP_BAD_REQUEST; apr_table_setn(r->notes, "error-notes", - apr_pstrcat(r->pool, "Size of a request header field " - "after merging exceeds server limit.<br />" - "\n<pre>\n", - ap_escape_html(r->pool, key), - "</pre>\n", NULL)); - ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00560) "Request header " - "exceeds LimitRequestFieldSize after merging: %s", key); + "Size of a request header field exceeds server limit."); + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00560) "Request " + "header exceeds LimitRequestFieldSize after merging: %.*s", + field_name_len(key), key); return 0; } -/* get the length of the field name for logging, but no more than 80 bytes */ -#define LOG_NAME_MAX_LEN 80 -static int field_name_len(const char *field) -{ - const char *end = ap_strchr_c(field, ':'); - if (end == NULL || end - field > LOG_NAME_MAX_LEN) - return LOG_NAME_MAX_LEN; - return end - field; -} - AP_DECLARE(void) ap_get_mime_headers_core(request_rec *r, apr_bucket_brigade *bb) { char *last_field = NULL; @@ -710,6 +934,8 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_ apr_size_t len; int fields_read = 0; char *tmp_field; + core_server_config *conf = ap_get_core_module_config(r->server->module_config); + int strict = (conf->http_conformance != AP_HTTP_CONFORMANCE_UNSAFE); /* * Read header lines until we get the empty separator line, a read error, @@ -717,11 +943,10 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_ */ while(1) { apr_status_t rv; - int folded = 0; field = NULL; rv = ap_rgetline(&field, r->server->limit_req_fieldsize + 2, - &len, r, 0, bb); + &len, r, strict ? AP_GETLINE_CRLF : 0, bb); if (rv != APR_SUCCESS) { if (APR_STATUS_IS_TIMEUP(rv)) { @@ -738,104 +963,125 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_ * exceeds the configured limit for a field size. */ if (rv == APR_ENOSPC) { - const char *field_escaped; - if (field && len) { - /* ensure ap_escape_html will terminate correctly */ - field[len - 1] = '\0'; - field_escaped = ap_escape_html(r->pool, field); - } - else { - field_escaped = field = ""; - } - apr_table_setn(r->notes, "error-notes", - apr_psprintf(r->pool, - "Size of a request header field " - "exceeds server limit.<br />\n" - "<pre>\n%.*s\n</pre>\n", - field_name_len(field_escaped), - field_escaped)); + "Size of a request header field " + "exceeds server limit."); ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00561) "Request header exceeds LimitRequestFieldSize%s" "%.*s", - *field ? ": " : "", - field_name_len(field), field); + (field && *field) ? ": " : "", + (field) ? field_name_len(field) : 0, + (field) ? field : ""); } return; } - if (last_field != NULL) { - if ((len > 0) && ((*field == '\t') || *field == ' ')) { - /* This line is a continuation of the preceding line(s), - * so append it to the line that we've set aside. - * Note: this uses a power-of-two allocator to avoid - * doing O(n) allocs and using O(n^2) space for - * continuations that span many many lines. - */ - apr_size_t fold_len = last_len + len + 1; /* trailing null */ + /* For all header values, and all obs-fold lines, the presence of + * additional whitespace is a no-op, so collapse trailing whitespace + * to save buffer allocation and optimize copy operations. + * Do not remove the last single whitespace under any condition. + */ + while (len > 1 && (field[len-1] == '\t' || field[len-1] == ' ')) { + field[--len] = '\0'; + } - if (fold_len >= (apr_size_t)(r->server->limit_req_fieldsize)) { - const char *field_escaped; + if (*field == '\t' || *field == ' ') { - r->status = HTTP_BAD_REQUEST; - /* report what we have accumulated so far before the - * overflow (last_field) as the field with the problem - */ - field_escaped = ap_escape_html(r->pool, last_field); - apr_table_setn(r->notes, "error-notes", - apr_psprintf(r->pool, - "Size of a request header field " - "after folding " - "exceeds server limit.<br />\n" - "<pre>\n%.*s\n</pre>\n", - field_name_len(field_escaped), - field_escaped)); - ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00562) - "Request header exceeds LimitRequestFieldSize " - "after folding: %.*s", - field_name_len(last_field), last_field); - return; - } + /* Append any newly-read obs-fold line onto the preceding + * last_field line we are processing + */ + apr_size_t fold_len; + if (last_field == NULL) { + r->status = HTTP_BAD_REQUEST; + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03442) + "Line folding encountered before first" + " header line"); + return; + } + + if (field[1] == '\0') { + r->status = HTTP_BAD_REQUEST; + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03443) + "Empty folded line encountered"); + return; + } + + /* Leading whitespace on an obs-fold line can be + * similarly discarded */ + while (field[1] == '\t' || field[1] == ' ') { + ++field; --len; + } + + /* This line is a continuation of the preceding line(s), + * so append it to the line that we've set aside. + * Note: this uses a power-of-two allocator to avoid + * doing O(n) allocs and using O(n^2) space for + * continuations that span many many lines. + */ + fold_len = last_len + len + 1; /* trailing null */ + + if (fold_len >= (apr_size_t)(r->server->limit_req_fieldsize)) { + r->status = HTTP_BAD_REQUEST; + /* report what we have accumulated so far before the + * overflow (last_field) as the field with the problem + */ + apr_table_setn(r->notes, "error-notes", + "Size of a request header field " + "exceeds server limit."); + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00562) + "Request header exceeds LimitRequestFieldSize " + "after folding: %.*s", + field_name_len(last_field), last_field); + return; + } + + if (fold_len > alloc_len) { + char *fold_buf; + alloc_len += alloc_len; if (fold_len > alloc_len) { - char *fold_buf; - alloc_len += alloc_len; - if (fold_len > alloc_len) { - alloc_len = fold_len; - } - fold_buf = (char *)apr_palloc(r->pool, alloc_len); - memcpy(fold_buf, last_field, last_len); - last_field = fold_buf; + alloc_len = fold_len; } - memcpy(last_field + last_len, field, len +1); /* +1 for nul */ - last_len += len; - folded = 1; + fold_buf = (char *)apr_palloc(r->pool, alloc_len); + memcpy(fold_buf, last_field, last_len); + last_field = fold_buf; } - else /* not a continuation line */ { + memcpy(last_field + last_len, field, len +1); /* +1 for nul */ + /* Replace obs-fold w/ SP per RFC 7230 3.2.4 */ + last_field[last_len] = ' '; + last_len += len; - if (r->server->limit_req_fields + /* We've appended this obs-fold line to last_len, proceed to + * read the next input line + */ + continue; + } + else if (last_field != NULL) { + + /* Process the previous last_field header line with all obs-folded + * segments already concatinated (this is not operating on the + * most recently read input line). + */ + + if (r->server->limit_req_fields && (++fields_read > r->server->limit_req_fields)) { - r->status = HTTP_BAD_REQUEST; - apr_table_setn(r->notes, "error-notes", - "The number of request header fields " - "exceeds this server's limit."); - ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00563) - "Number of request headers exceeds " - "LimitRequestFields"); - return; - } + r->status = HTTP_BAD_REQUEST; + apr_table_setn(r->notes, "error-notes", + "The number of request header fields " + "exceeds this server's limit."); + ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00563) + "Number of request headers exceeds " + "LimitRequestFields"); + return; + } - if (!(value = strchr(last_field, ':'))) { /* Find ':' or */ - r->status = HTTP_BAD_REQUEST; /* abort bad request */ - apr_table_setn(r->notes, "error-notes", - apr_psprintf(r->pool, - "Request header field is " - "missing ':' separator.<br />\n" - "<pre>\n%.*s</pre>\n", - (int)LOG_NAME_MAX_LEN, - ap_escape_html(r->pool, - last_field))); - ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00564) + if (!strict) + { + /* Not Strict ('Unsafe' mode), using the legacy parser */ + + if (!(value = strchr(last_field, ':'))) { /* Find ':' or */ + r->status = HTTP_BAD_REQUEST; /* abort bad request */ + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00564) "Request header field is missing ':' " "separator: %.*s", (int)LOG_NAME_MAX_LEN, last_field); @@ -842,52 +1088,93 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_ return; } - tmp_field = value - 1; /* last character of field-name */ + /* last character of field-name */ + tmp_field = value - (value > last_field ? 1 : 0); *value++ = '\0'; /* NUL-terminate at colon */ + if (strpbrk(last_field, "\t\n\v\f\r ")) { + r->status = HTTP_BAD_REQUEST; + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03452) + "Request header field name presented" + " invalid whitespace"); + return; + } + while (*value == ' ' || *value == '\t') { - ++value; /* Skip to start of value */ + ++value; /* Skip to start of value */ } - /* Strip LWS after field-name: */ - while (tmp_field > last_field - && (*tmp_field == ' ' || *tmp_field == '\t')) { - *tmp_field-- = '\0'; + if (strpbrk(value, "\n\v\f\r")) { + r->status = HTTP_BAD_REQUEST; + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03451) + "Request header field value presented" + " bad whitespace"); + return; } - /* Strip LWS after field-value: */ - tmp_field = last_field + last_len - 1; - while (tmp_field > value - && (*tmp_field == ' ' || *tmp_field == '\t')) { - *tmp_field-- = '\0'; + if (tmp_field == last_field) { + r->status = HTTP_BAD_REQUEST; + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(03453) + "Request header field name was empty"); + return; } + } + else /* Using strict RFC7230 parsing */ + { + /* Ensure valid token chars before ':' per RFC 7230 3.2.4 */ + value = (char *)ap_scan_http_token(last_field); + if ((value == last_field) || *value != ':') { + r->status = HTTP_BAD_REQUEST; + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02426) + "Request header field name is malformed: " + "%.*s", (int)LOG_NAME_MAX_LEN, last_field); + return; + } - apr_table_addn(r->headers_in, last_field, value); + *value++ = '\0'; /* NUL-terminate last_field name at ':' */ - /* reset the alloc_len so that we'll allocate a new - * buffer if we have to do any more folding: we can't - * use the previous buffer because its contents are - * now part of r->headers_in + while (*value == ' ' || *value == '\t') { + ++value; /* Skip LWS of value */ + } + + /* Find invalid, non-HT ctrl char, or the trailing NULL */ + tmp_field = (char *)ap_scan_http_field_content(value); + + /* Reject value for all garbage input (CTRLs excluding HT) + * e.g. only VCHAR / SP / HT / obs-text are allowed per + * RFC7230 3.2.6 - leave all more explicit rule enforcement + * for specific header handler logic later in the cycle */ - alloc_len = 0; + if (*tmp_field != '\0') { + r->status = HTTP_BAD_REQUEST; + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02427) + "Request header value is malformed: " + "%.*s", (int)LOG_NAME_MAX_LEN, value); + return; + } + } - } /* end if current line is not a continuation starting with tab */ + apr_table_addn(r->headers_in, last_field, value); + + /* This last_field header is now stored in headers_in, + * resume processing of the current input line. + */ } - /* Found a blank line, stop. */ + /* Found the terminating empty end-of-headers line, stop. */ if (len == 0) { break; } - /* Keep track of this line so that we can parse it on - * the next loop iteration. (In the folded case, last_field - * has been updated already.) + /* Keep track of this new header line so that we can extend it across + * any obs-fold or parse it on the next loop iteration. We referenced + * our previously allocated buffer in r->headers_in, + * so allocate a fresh buffer if required. */ - if (!folded) { - last_field = field; - last_len = len; - } + alloc_len = 0; + last_field = field; + last_len = len; } /* Combine multiple message-header fields with the same @@ -912,7 +1199,7 @@ request_rec *ap_read_request(conn_rec *conn) request_rec *r; apr_pool_t *p; const char *expect; - int access_status = HTTP_OK; + int access_status; apr_bucket_brigade *tmp_bb; apr_socket_t *csd; apr_interval_time_t cur_timeout; @@ -971,8 +1258,11 @@ request_rec *ap_read_request(conn_rec *conn) /* Get the request... */ if (!read_request_line(r, tmp_bb)) { - if (r->status == HTTP_REQUEST_URI_TOO_LARGE - || r->status == HTTP_BAD_REQUEST) { + switch (r->status) { + case HTTP_REQUEST_URI_TOO_LARGE: + case HTTP_BAD_REQUEST: + case HTTP_VERSION_NOT_SUPPORTED: + case HTTP_NOT_IMPLEMENTED: if (r->status == HTTP_REQUEST_URI_TOO_LARGE) { ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00565) "request failed: client's request-line exceeds LimitRequestLine (longer than %d)", @@ -979,8 +1269,8 @@ request_rec *ap_read_request(conn_rec *conn) r->server->limit_req_line); } else if (r->method == NULL) { - ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00566) - "request failed: invalid characters in URI"); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00566) + "request failed: malformed request line"); } access_status = r->status; r->status = HTTP_OK; @@ -990,19 +1280,17 @@ request_rec *ap_read_request(conn_rec *conn) r = NULL; apr_brigade_destroy(tmp_bb); goto traceout; - } - else if (r->status == HTTP_REQUEST_TIME_OUT) { + case HTTP_REQUEST_TIME_OUT: ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, NULL); - if (!r->connection->keepalives) { + if (!r->connection->keepalives) ap_run_log_transaction(r); - } apr_brigade_destroy(tmp_bb); goto traceout; + default: + apr_brigade_destroy(tmp_bb); + r = NULL; + goto traceout; } - - apr_brigade_destroy(tmp_bb); - r = NULL; - goto traceout; } /* We may have been in keep_alive_timeout mode, so toggle back @@ -1021,7 +1309,7 @@ request_rec *ap_read_request(conn_rec *conn) ap_get_mime_headers_core(r, tmp_bb); if (r->status != HTTP_OK) { - ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00567) + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00567) "request failed: error reading the headers"); ap_send_error_response(r, 0); ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r); @@ -1040,7 +1328,7 @@ request_rec *ap_read_request(conn_rec *conn) */ if (!(strcasecmp(tenc, "chunked") == 0 /* fast path */ || ap_find_last_token(r->pool, tenc, "chunked"))) { - ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(02539) + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02539) "client sent unknown Transfer-Encoding " "(%s): %s", tenc, r->uri); r->status = HTTP_BAD_REQUEST; @@ -1061,25 +1349,6 @@ request_rec *ap_read_request(conn_rec *conn) apr_table_unset(r->headers_in, "Content-Length"); } } - else { - if (r->header_only) { - /* - * Client asked for headers only with HTTP/0.9, which doesn't send - * headers! Have to dink things just to make sure the error message - * comes through... - */ - ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00568) - "client sent invalid HTTP/0.9 request: HEAD %s", - r->uri); - r->header_only = 0; - r->status = HTTP_BAD_REQUEST; - ap_send_error_response(r, 0); - ap_update_child_status(conn->sbh, SERVER_BUSY_LOG, r); - ap_run_log_transaction(r); - apr_brigade_destroy(tmp_bb); - goto traceout; - } - } apr_brigade_destroy(tmp_bb); @@ -1087,6 +1356,7 @@ request_rec *ap_read_request(conn_rec *conn) * now read. may update status. */ ap_update_vhost_from_headers(r); + access_status = r->status; /* Toggle to the Host:-based vhost's timeout mode to fetch the * request body and send the response body, if needed. @@ -1110,7 +1380,7 @@ request_rec *ap_read_request(conn_rec *conn) * a Host: header, and the server MUST respond with 400 if it doesn't. */ access_status = HTTP_BAD_REQUEST; - ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00569) + 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); } Index: server/vhost.c =================================================================== --- server/vhost.c (.../2.4.x) (revision 1767912) +++ server/vhost.c (.../2.4.x-merge-http-strict) (revision 1780175) @@ -687,6 +687,116 @@ static int vhost_check_config(apr_pool_t *p, apr_p * run-time vhost matching functions */ +static apr_status_t fix_hostname_v6_literal(request_rec *r, char *host) +{ + char *dst; + int double_colon = 0; + + for (dst = host; *dst; dst++) { + if (apr_isxdigit(*dst)) { + if (apr_isupper(*dst)) { + *dst = apr_tolower(*dst); + } + } + else if (*dst == ':') { + if (*(dst + 1) == ':') { + if (double_colon) + return APR_EINVAL; + double_colon = 1; + } + else if (*(dst + 1) == '.') { + return APR_EINVAL; + } + } + else if (*dst == '.') { + /* For IPv4-mapped IPv6 addresses like ::FFFF:129.144.52.38 */ + if (*(dst + 1) == ':' || *(dst + 1) == '.') + return APR_EINVAL; + } + else { + return APR_EINVAL; + } + } + return APR_SUCCESS; +} + +static apr_status_t fix_hostname_non_v6(request_rec *r, char *host) +{ + char *dst; + + for (dst = host; *dst; dst++) { + if (apr_islower(*dst)) { + /* leave char unchanged */ + } + else if (*dst == '.') { + if (*(dst + 1) == '.') { + return APR_EINVAL; + } + } + else if (apr_isupper(*dst)) { + *dst = apr_tolower(*dst); + } + else if (*dst == '/' || *dst == '\\') { + return APR_EINVAL; + } + } + /* strip trailing gubbins */ + if (dst > host && dst[-1] == '.') { + dst[-1] = '\0'; + } + return APR_SUCCESS; +} + +/* + * If strict mode ever becomes the default, this should be folded into + * fix_hostname_non_v6() + */ +static apr_status_t strict_hostname_check(request_rec *r, char *host) +{ + char *ch; + int is_dotted_decimal = 1, leading_zeroes = 0, dots = 0; + + for (ch = host; *ch; ch++) { + if (!apr_isascii(*ch)) { + goto bad; + } + else if (apr_isalpha(*ch) || *ch == '-') { + is_dotted_decimal = 0; + } + else if (ch[0] == '.') { + dots++; + if (ch[1] == '0' && apr_isdigit(ch[2])) + leading_zeroes = 1; + } + else if (!apr_isdigit(*ch)) { + /* also takes care of multiple Host headers by denying commas */ + goto bad; + } + } + if (is_dotted_decimal) { + if (host[0] == '.' || (host[0] == '0' && apr_isdigit(host[1]))) + leading_zeroes = 1; + if (leading_zeroes || dots != 3) { + /* RFC 3986 7.4 */ + goto bad; + } + } + else { + /* The top-level domain must start with a letter (RFC 1123 2.1) */ + while (ch > host && *ch != '.') + ch--; + if (ch[0] == '.' && ch[1] != '\0' && !apr_isalpha(ch[1])) + goto bad; + } + return APR_SUCCESS; + +bad: + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02415) + "[strict] Invalid host name '%s'%s%.6s", + host, *ch ? ", problem near: " : "", ch); + return APR_EINVAL; +} + /* Lowercase and remove any trailing dot and/or :port from the hostname, * and check that it is sane. * @@ -700,79 +810,90 @@ static int vhost_check_config(apr_pool_t *p, apr_p * Instead we just check for filesystem metacharacters: directory * separators / and \ and sequences of more than one dot. */ -static void fix_hostname(request_rec *r) +static int fix_hostname(request_rec *r, const char *host_header, + unsigned http_conformance) { + const char *src; char *host, *scope_id; - char *dst; apr_port_t port; apr_status_t rv; const char *c; + int is_v6literal = 0; + int strict = (http_conformance != AP_HTTP_CONFORMANCE_UNSAFE); - /* According to RFC 2616, Host header field CAN be blank. */ - if (!*r->hostname) { - return; + src = host_header ? host_header : r->hostname; + + /* According to RFC 2616, Host header field CAN be blank */ + if (!*src) { + return is_v6literal; } /* apr_parse_addr_port will interpret a bare integer as a port * which is incorrect in this context. So treat it separately. */ - for (c = r->hostname; apr_isdigit(*c); ++c); - if (!*c) { /* pure integer */ - return; + for (c = src; apr_isdigit(*c); ++c); + if (!*c) { + /* pure integer */ + if (strict) { + /* RFC 3986 7.4 */ + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02416) + "[strict] purely numeric host names not allowed: %s", + src); + goto bad_nolog; + } + r->hostname = src; + return is_v6literal; } - rv = apr_parse_addr_port(&host, &scope_id, &port, r->hostname, r->pool); - if (rv != APR_SUCCESS || scope_id) { - goto bad; + if (host_header) { + rv = apr_parse_addr_port(&host, &scope_id, &port, src, r->pool); + if (rv != APR_SUCCESS || scope_id) + goto bad; + if (port) { + /* Don't throw the Host: header's port number away: + save it in parsed_uri -- ap_get_server_port() needs it! */ + /* @@@ XXX there should be a better way to pass the port. + * Like r->hostname, there should be a r->portno + */ + r->parsed_uri.port = port; + r->parsed_uri.port_str = apr_itoa(r->pool, (int)port); + } + if (host_header[0] == '[') + is_v6literal = 1; } - - if (port) { - /* Don't throw the Host: header's port number away: - save it in parsed_uri -- ap_get_server_port() needs it! */ - /* @@@ XXX there should be a better way to pass the port. - * Like r->hostname, there should be a r->portno + else { + /* + * Already parsed, surrounding [ ] (if IPv6 literal) and :port have + * already been removed. */ - r->parsed_uri.port = port; - r->parsed_uri.port_str = apr_itoa(r->pool, (int)port); + host = apr_pstrdup(r->pool, r->hostname); + if (ap_strchr(host, ':') != NULL) + is_v6literal = 1; } - /* if the hostname is an IPv6 numeric address string, it was validated - * already; otherwise, further validation is needed - */ - if (r->hostname[0] != '[') { - for (dst = host; *dst; dst++) { - if (apr_islower(*dst)) { - /* leave char unchanged */ - } - else if (*dst == '.') { - if (*(dst + 1) == '.') { - goto bad; - } - } - else if (apr_isupper(*dst)) { - *dst = apr_tolower(*dst); - } - else if (*dst == '/' || *dst == '\\') { - goto bad; - } - } - /* strip trailing gubbins */ - if (dst > host && dst[-1] == '.') { - dst[-1] = '\0'; - } + if (is_v6literal) { + rv = fix_hostname_v6_literal(r, host); } + else { + rv = fix_hostname_non_v6(r, host); + if (strict && rv == APR_SUCCESS) + rv = strict_hostname_check(r, host); + } + if (rv != APR_SUCCESS) + goto bad; + r->hostname = host; - return; + return is_v6literal; bad: - r->status = HTTP_BAD_REQUEST; ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00550) "Client sent malformed Host header: %s", - r->hostname); - return; + src); +bad_nolog: + r->status = HTTP_BAD_REQUEST; + return is_v6literal; } - /* return 1 if host matches ServerName or ServerAliases */ static int matches_aliases(server_rec *s, const char *host) { @@ -982,15 +1103,76 @@ static void check_serverpath(request_rec *r) } } +static APR_INLINE const char *construct_host_header(request_rec *r, + int is_v6literal) +{ + struct iovec iov[5]; + apr_size_t nvec = 0; + /* + * We cannot use ap_get_server_name/port here, because we must + * ignore UseCanonicalName/Port. + */ + if (is_v6literal) { + iov[nvec].iov_base = "["; + iov[nvec].iov_len = 1; + nvec++; + } + iov[nvec].iov_base = (void *)r->hostname; + iov[nvec].iov_len = strlen(r->hostname); + nvec++; + if (is_v6literal) { + iov[nvec].iov_base = "]"; + iov[nvec].iov_len = 1; + nvec++; + } + if (r->parsed_uri.port_str) { + iov[nvec].iov_base = ":"; + iov[nvec].iov_len = 1; + nvec++; + iov[nvec].iov_base = r->parsed_uri.port_str; + iov[nvec].iov_len = strlen(r->parsed_uri.port_str); + nvec++; + } + return apr_pstrcatv(r->pool, iov, nvec, NULL); +} AP_DECLARE(void) ap_update_vhost_from_headers(request_rec *r) { - /* must set this for HTTP/1.1 support */ - if (r->hostname || (r->hostname = apr_table_get(r->headers_in, "Host"))) { - fix_hostname(r); - if (r->status != HTTP_OK) - return; + core_server_config *conf = ap_get_core_module_config(r->server->module_config); + const char *host_header = apr_table_get(r->headers_in, "Host"); + int is_v6literal = 0; + int have_hostname_from_url = 0; + + if (r->hostname) { + /* + * If there was a host part in the Request-URI, ignore the 'Host' + * header. + */ + have_hostname_from_url = 1; + is_v6literal = fix_hostname(r, NULL, conf->http_conformance); } + else if (host_header != NULL) { + is_v6literal = fix_hostname(r, host_header, conf->http_conformance); + } + if (r->status != HTTP_OK) + return; + + if (conf->http_conformance != AP_HTTP_CONFORMANCE_UNSAFE) { + /* + * If we have both hostname from an absoluteURI and a Host header, + * we must ignore the Host header (RFC 2616 5.2). + * To enforce this, we reset the Host header to the value from the + * request line. + */ + if (have_hostname_from_url && host_header != NULL) { + const char *repl = construct_host_header(r, is_v6literal); + apr_table_set(r->headers_in, "Host", repl); + ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02417) + "Replacing host header '%s' with host '%s' given " + "in the request uri", host_header, repl); + } + } + /* check if we tucked away a name_chain */ if (r->connection->vhost_lookup_data) { if (r->hostname) Index: modules/http/http_filters.c =================================================================== --- modules/http/http_filters.c (.../2.4.x) (revision 1767912) +++ modules/http/http_filters.c (.../2.4.x-merge-http-strict) (revision 1780175) @@ -126,14 +126,15 @@ static apr_status_t bail_out_on_error(http_ctx_t * /** * Parse a chunk line with optional extension, detect overflow. - * There are two error cases: - * 1) If the conversion would require too many bits, APR_EGENERAL is returned. - * 2) If the conversion used the correct number of bits, but an overflow + * There are several error cases: + * 1) If the chunk link is misformatted, APR_EINVAL is returned. + * 2) If the conversion would require too many bits, APR_EGENERAL is returned. + * 3) If the conversion used the correct number of bits, but an overflow * caused only the sign bit to flip, then APR_ENOSPC is returned. - * In general, any negative number can be considered an overflow error. + * A negative chunk length always indicates an overflow error. */ static apr_status_t parse_chunk_size(http_ctx_t *ctx, const char *buffer, - apr_size_t len, int linelimit) + apr_size_t len, int linelimit, int strict) { apr_size_t i = 0; @@ -146,6 +147,12 @@ static apr_status_t parse_chunk_size(http_ctx_t *c if (ctx->state == BODY_CHUNK_END || ctx->state == BODY_CHUNK_END_LF) { if (c == LF) { + if (strict && (ctx->state != BODY_CHUNK_END_LF)) { + /* + * CR missing before LF. + */ + return APR_EINVAL; + } ctx->state = BODY_CHUNK; } else if (c == CR && ctx->state == BODY_CHUNK_END) { @@ -153,7 +160,7 @@ static apr_status_t parse_chunk_size(http_ctx_t *c } else { /* - * LF expected. + * CRLF expected. */ return APR_EINVAL; } @@ -180,6 +187,12 @@ static apr_status_t parse_chunk_size(http_ctx_t *c } if (c == LF) { + if (strict && (ctx->state != BODY_CHUNK_LF)) { + /* + * CR missing before LF. + */ + return APR_EINVAL; + } if (ctx->remaining) { ctx->state = BODY_CHUNK_DATA; } @@ -201,7 +214,8 @@ static apr_status_t parse_chunk_size(http_ctx_t *c } else if (ctx->state == BODY_CHUNK_EXT) { /* - * Control chars (but tabs) are invalid. + * Control chars (excluding tabs) are invalid. + * TODO: more precisely limit input */ if (c != '\t' && apr_iscntrl(c)) { return APR_EINVAL; @@ -208,7 +222,9 @@ static apr_status_t parse_chunk_size(http_ctx_t *c } } else if (c == ' ' || c == '\t') { - /* Be lenient up to 10 BWS (term from rfc7230 - 3.2.3). + /* Be lenient up to 10 implied *LWS, a legacy of RFC 2616, + * and noted as errata to RFC7230; + * https://www.rfc-editor.org/errata_search.php?rfc=7230&eid=4667 */ ctx->state = BODY_CHUNK_CR; if (++ctx->chunk_bws > 10) { @@ -324,7 +340,10 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bu ap_input_mode_t mode, apr_read_type_e block, apr_off_t readbytes) { - core_server_config *conf; + core_server_config *conf = + (core_server_config *) ap_get_module_config(f->r->server->module_config, + &core_module); + int strict = (conf->http_conformance != AP_HTTP_CONFORMANCE_UNSAFE); apr_bucket *e; http_ctx_t *ctx = f->ctx; apr_status_t rv; @@ -332,9 +351,6 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bu apr_bucket_brigade *bb; int again; - conf = (core_server_config *) - ap_get_module_config(f->r->server->module_config, &core_module); - /* just get out of the way of things we don't want. */ if (mode != AP_MODE_READBYTES && mode != AP_MODE_GETLINE) { return ap_get_brigade(f->next, b, mode, block, readbytes); @@ -526,7 +542,7 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bu if (rv == APR_SUCCESS) { parsing = 1; rv = parse_chunk_size(ctx, buffer, len, - f->r->server->limit_req_fieldsize); + f->r->server->limit_req_fieldsize, strict); } if (rv != APR_SUCCESS) { ap_log_rerror(APLOG_MARK, APLOG_INFO, rv, f->r, APLOGNO(01590) @@ -668,6 +684,114 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bu return APR_SUCCESS; } +struct check_header_ctx { + request_rec *r; + int strict; +}; + +/* check a single header, to be used with apr_table_do() */ +static int check_header(struct check_header_ctx *ctx, + const char *name, const char **val) +{ + const char *pos, *end; + char *dst = NULL; + + if (name[0] == '\0') { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02428) + "Empty response header name, aborting request"); + return 0; + } + + if (ctx->strict) { + end = ap_scan_http_token(name); + } + else { + end = ap_scan_vchar_obstext(name); + } + if (*end) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02429) + "Response header name '%s' contains invalid " + "characters, aborting request", + name); + return 0; + } + + for (pos = *val; *pos; pos = end) { + end = ap_scan_http_field_content(pos); + if (*end) { + if (end[0] != CR || end[1] != LF || (end[2] != ' ' && + end[2] != '\t')) { + ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02430) + "Response header '%s' value of '%s' contains " + "invalid characters, aborting request", + name, pos); + return 0; + } + if (!dst) { + *val = dst = apr_palloc(ctx->r->pool, strlen(*val) + 1); + } + } + if (dst) { + memcpy(dst, pos, end - pos); + dst += end - pos; + if (*end) { + /* skip folding and replace with a single space */ + end += 3 + strspn(end + 3, "\t "); + *dst++ = ' '; + } + } + } + if (dst) { + *dst = '\0'; + } + return 1; +} + +static int check_headers_table(apr_table_t *t, struct check_header_ctx *ctx) +{ + const apr_array_header_t *headers = apr_table_elts(t); + apr_table_entry_t *header; + int i; + + for (i = 0; i < headers->nelts; ++i) { + header = &APR_ARRAY_IDX(headers, i, apr_table_entry_t); + if (!header->key) { + continue; + } + if (!check_header(ctx, header->key, (const char **)&header->val)) { + return 0; + } + } + return 1; +} + +/** + * Check headers for HTTP conformance + * @return 1 if ok, 0 if bad + */ +static APR_INLINE int check_headers(request_rec *r) +{ + struct check_header_ctx ctx; + core_server_config *conf = + ap_get_core_module_config(r->server->module_config); + + ctx.r = r; + ctx.strict = (conf->http_conformance != AP_HTTP_CONFORMANCE_UNSAFE); + return check_headers_table(r->headers_out, &ctx) && + check_headers_table(r->err_headers_out, &ctx); +} + +static int check_headers_recursion(request_rec *r) +{ + void *check = NULL; + apr_pool_userdata_get(&check, "check_headers_recursion", r->pool); + if (check) { + return 1; + } + apr_pool_userdata_setn("true", "check_headers_recursion", NULL, r->pool); + return 0; +} + typedef struct header_struct { apr_pool_t *pool; apr_bucket_brigade *bb; @@ -674,8 +798,7 @@ typedef struct header_struct { } header_struct; /* Send a single HTTP header field to the client. Note that this function - * is used in calls to table_do(), so their interfaces are co-dependent. - * In other words, don't change this one without checking table_do in alloc.c. + * is used in calls to apr_table_do(), so don't change its interface. * It returns true unless there was a write error of some kind. */ static int form_header_field(header_struct *h, @@ -1160,6 +1283,7 @@ AP_DECLARE_NONSTD(int) ap_send_http_trace(request_ typedef struct header_filter_ctx { int headers_sent; + int headers_error; } header_filter_ctx; AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_header_filter(ap_filter_t *f, @@ -1175,19 +1299,23 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade header_filter_ctx *ctx = f->ctx; const char *ctype; ap_bucket_error *eb = NULL; + apr_bucket *eos = NULL; AP_DEBUG_ASSERT(!r->main); - if (r->header_only) { - if (!ctx) { - ctx = f->ctx = apr_pcalloc(r->pool, sizeof(header_filter_ctx)); - } - else if (ctx->headers_sent) { + if (!ctx) { + ctx = f->ctx = apr_pcalloc(r->pool, sizeof(header_filter_ctx)); + } + if (ctx->headers_sent) { + /* Eat body if response must not have one. */ + if (r->header_only || r->status == HTTP_NO_CONTENT) { apr_brigade_cleanup(b); - return OK; + return APR_SUCCESS; } } - + else if (!ctx->headers_error && !check_headers(r)) { + ctx->headers_error = 1; + } for (e = APR_BRIGADE_FIRST(b); e != APR_BRIGADE_SENTINEL(b); e = APR_BUCKET_NEXT(e)) @@ -1204,10 +1332,44 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade ap_remove_output_filter(f); return ap_pass_brigade(f->next, b); } + if (ctx->headers_error && APR_BUCKET_IS_EOS(e)) { + eos = e; + } } - if (eb) { + if (ctx->headers_error) { + if (!eos) { + /* Eat body until EOS */ + apr_brigade_cleanup(b); + return APR_SUCCESS; + } + + /* We may come back here from ap_die() below, + * so clear anything from this response. + */ + ctx->headers_error = 0; + apr_table_clear(r->headers_out); + apr_table_clear(r->err_headers_out); + + /* Don't recall ap_die() if we come back here (from its own internal + * redirect or error response), otherwise we can end up in infinite + * recursion; better fall through with 500, minimal headers and an + * empty body (EOS only). + */ + if (!check_headers_recursion(r)) { + apr_brigade_cleanup(b); + ap_die(HTTP_INTERNAL_SERVER_ERROR, r); + return AP_FILTER_ERROR; + } + APR_BUCKET_REMOVE(eos); + apr_brigade_cleanup(b); + APR_BRIGADE_INSERT_TAIL(b, eos); + r->status = HTTP_INTERNAL_SERVER_ERROR; + r->content_type = r->content_encoding = NULL; + r->content_languages = NULL; + ap_set_content_length(r, 0); + } + else if (eb) { int status; - status = eb->status; apr_brigade_cleanup(b); ap_die(status, r); @@ -1264,6 +1426,10 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade apr_table_unset(r->headers_out, "Content-Length"); } + if (r->status == HTTP_NO_CONTENT) { + apr_table_unset(r->headers_out, "Content-Length"); + } + ctype = ap_make_content_type(r, r->content_type); if (ctype) { apr_table_setn(r->headers_out, "Content-Type", ctype); @@ -1352,11 +1518,11 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_heade terminate_header(b2); ap_pass_brigade(f->next, b2); + ctx->headers_sent = 1; - if (r->header_only) { + if (r->header_only || r->status == HTTP_NO_CONTENT) { apr_brigade_cleanup(b); - ctx->headers_sent = 1; - return OK; + return APR_SUCCESS; } r->sent_bodyct = 1; /* Whatever follows is real body stuff... */ Index: CHANGES =================================================================== --- CHANGES (.../2.4.x) (revision 1767912) +++ CHANGES (.../2.4.x-merge-http-strict) (revision 1780175) @@ -1,7 +1,27 @@ -*- coding: utf-8 -*- Changes with Apache 2.4.24 + *) core: Drop Content-Length header and message-body from HTTP 204 responses. + PR 51350 [Luca Toscano] + *) Enforce http request grammer corresponding to RFC7230 for request lines + and request headers [William Rowe, Stefan Fritsch] + + *) core: New directive HttpProtocolOptions to control httpd enforcement + of various RFC7230 requirements. [Stefan Fritsch, William Rowe] + + *) core: Permit unencoded ';' characters to appear in proxy requests and + Location: response headers. Corresponds to modern browser behavior. + [William Rowe] + + *) core: ap_rgetline_core now pulls from r->proto_input_filters. + + *) core: Correctly parse an IPv6 literal host specification in an absolute + URL in the request line. [Stefan Fritsch] + + *) core: New directive RegisterHttpMethod for registering non-standard + HTTP methods. [Stefan Fritsch] + *) mod_http2: unannounced and multiple interim responses (status code < 200) are parsed and forwarded to client until a final response arrives. [Stefan Eissing] Index: include/httpd.h =================================================================== --- include/httpd.h (.../2.4.x) (revision 1767912) +++ include/httpd.h (.../2.4.x-merge-http-strict) (revision 1780175) @@ -1585,6 +1585,28 @@ AP_DECLARE(int) ap_find_etag_weak(apr_pool_t *p, c */ AP_DECLARE(int) ap_find_etag_strong(apr_pool_t *p, const char *line, const char *tok); +/* Scan a string for field content chars, as defined by RFC7230 section 3.2 + * including VCHAR/obs-text, as well as HT and SP + * @param ptr The string to scan + * @return A pointer to the first (non-HT) ASCII ctrl character. + * @note lws and trailing whitespace are scanned, the caller is responsible + * for trimming leading and trailing whitespace + */ +AP_DECLARE(const char *) ap_scan_http_field_content(const char *ptr); + +/* Scan a string for token characters, as defined by RFC7230 section 3.2.6 + * @param ptr The string to scan + * @return A pointer to the first non-token character. + */ +AP_DECLARE(const char *) ap_scan_http_token(const char *ptr); + +/* Scan a string for visible ASCII (0x21-0x7E) or obstext (0x80+) + * and return a pointer to the first SP/CTL/NUL character encountered. + * @param ptr The string to scan + * @return A pointer to the first SP/CTL character. + */ +AP_DECLARE(const char *) ap_scan_vchar_obstext(const char *ptr); + /** * Retrieve an array of tokens in the format "1#token" defined in RFC2616. Only * accepts ',' as a delimiter, does not accept quoted strings, and errors on Index: include/http_core.h =================================================================== --- include/http_core.h (.../2.4.x) (revision 1767912) +++ include/http_core.h (.../2.4.x-merge-http-strict) (revision 1780175) @@ -723,10 +723,24 @@ typedef struct { #define AP_MERGE_TRAILERS_DISABLE 2 int merge_trailers; + apr_array_header_t *protocols; + int protocols_honor_order; +#define AP_HTTP09_UNSET 0 +#define AP_HTTP09_ENABLE 1 +#define AP_HTTP09_DISABLE 2 + char http09_enable; - apr_array_header_t *protocols; - int protocols_honor_order; +#define AP_HTTP_CONFORMANCE_UNSET 0 +#define AP_HTTP_CONFORMANCE_UNSAFE 1 +#define AP_HTTP_CONFORMANCE_STRICT 2 + char http_conformance; + +#define AP_HTTP_METHODS_UNSET 0 +#define AP_HTTP_METHODS_LENIENT 1 +#define AP_HTTP_METHODS_REGISTERED 2 + char http_methods; + } core_server_config; /* for AddOutputFiltersByType in core.c */ Index: include/http_protocol.h =================================================================== --- include/http_protocol.h (.../2.4.x) (revision 1767912) +++ include/http_protocol.h (.../2.4.x-merge-http-strict) (revision 1780175) @@ -582,17 +582,22 @@ AP_DECLARE(int) ap_get_basic_auth_pw(request_rec * */ AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r, const char *uri); +#define AP_GETLINE_FOLD 1 /* Whether to merge continuation lines */ +#define AP_GETLINE_CRLF 2 /*Whether line ends must be in the form CR LF */ + /** * Get the next line of input for the request * @param s The buffer into which to read the line * @param n The size of the buffer * @param r The request - * @param fold Whether to merge continuation lines + * @param flags Bit flag of multiple parsing options + * AP_GETLINE_FOLD Whether to merge continuation lines + * AP_GETLINE_CRLF Whether line ends must be in the form CR LF * @return The length of the line, if successful * n, if the line is too big to fit in the buffer * -1 for miscellaneous errors */ -AP_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold); +AP_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int flags); /** * Get the next line of input for the request @@ -610,7 +615,9 @@ AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r, * @param n The size of the buffer * @param read The length of the line. * @param r The request - * @param fold Whether to merge continuation lines + * @param flags Bit flag of multiple parsing options + * AP_GETLINE_FOLD Whether to merge continuation lines + * AP_GETLINE_CRLF Whether line ends must be in the form CR LF * @param bb Working brigade to use when reading buckets * @return APR_SUCCESS, if successful * APR_ENOSPC, if the line is too big to fit in the buffer @@ -619,7 +626,7 @@ AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r, #if APR_CHARSET_EBCDIC AP_DECLARE(apr_status_t) ap_rgetline(char **s, apr_size_t n, apr_size_t *read, - request_rec *r, int fold, + request_rec *r, int flags, apr_bucket_brigade *bb); #else /* ASCII box */ #define ap_rgetline(s, n, read, r, fold, bb) \ @@ -629,7 +636,7 @@ AP_DECLARE(apr_status_t) ap_rgetline(char **s, apr /** @see ap_rgetline */ AP_DECLARE(apr_status_t) ap_rgetline_core(char **s, apr_size_t n, apr_size_t *read, - request_rec *r, int fold, + request_rec *r, int flags, apr_bucket_brigade *bb); /** Index: include/ap_mmn.h =================================================================== --- include/ap_mmn.h (.../2.4.x) (revision 1767912) +++ include/ap_mmn.h (.../2.4.x-merge-http-strict) (revision 1780175) @@ -487,6 +487,13 @@ * 20120211.65 (2.4.24-dev) Add ap_check_pipeline(). * 20120211.66 (2.4.24-dev) Rename ap_proxy_check_backend() to * ap_proxy_check_connection(). + * 20120211.67 (2.4.24-dev) Add http09_enable, http_conformance, and + * http_methods to core_server_config + * Add ap_scan_http_field_token(), + * ap_scan_http_field_content(), + * and ap_scan_vchar_obstext() + * Replaced fold boolean with with multiple bit flags + * to ap_[r]getline() */ #define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */ @@ -494,7 +501,7 @@ #ifndef MODULE_MAGIC_NUMBER_MAJOR #define MODULE_MAGIC_NUMBER_MAJOR 20120211 #endif -#define MODULE_MAGIC_NUMBER_MINOR 66 /* 0...n */ +#define MODULE_MAGIC_NUMBER_MINOR 67 /* 0...n */ /** * Determine if the server's current MODULE_MAGIC_NUMBER is at least a Index: . =================================================================== --- . (.../2.4.x) (revision 1767912) +++ . (.../2.4.x-merge-http-strict) (revision 1780175) Property changes on: . ___________________________________________________________________ Modified: svn:mergeinfo ## -0,0 +0,1 ## Merged /httpd/httpd/trunk:r1406719,1407599,1407643,1425366,1426827,1426877,1426879,1426988,1426992,1428145,1436457,1446421,1610383,1664576,1687642-1687643,1754536,1754538-1754541,1754544,1754547-1754548,1754555-1754556,1754568-1754570,1754577,1754579,1755123-1755126,1755233-1755236,1755263-1755264,1755343,1755744,1756540,1756555,1756649,1756729,1756821,1756823-1756824,1756847,1756849,1756862,1756934,1756937,1756946,1756959,1756978,1757062,1757065,1757589,1757593,1757711,1757920-1757921,1757924,1758226,1758263,1758265-1758266,1758304-1758305,1758313,1760444,1764961,1765112-1765115,1765451,1769965,1770786,1770817,1770867,1770869,1771690,1772418,1773159,1773162,1773293,1773346,1773761,1773779,1773812,1773861-1773862,1773865,1774286,1777460,1777672
--------------------------------------------------------------------- To unsubscribe, e-mail: users-unsubscribe@xxxxxxxxxxxxxxxx For additional commands, e-mail: users-help@xxxxxxxxxxxxxxxx