]> granicus.if.org Git - apache/commitdiff
Rename the previously undocumented HTTPProtocol directive
authorWilliam A. Rowe Jr <wrowe@apache.org>
Tue, 16 Aug 2016 18:11:14 +0000 (18:11 +0000)
committerWilliam A. Rowe Jr <wrowe@apache.org>
Tue, 16 Aug 2016 18:11:14 +0000 (18:11 +0000)
to EnforceHTTPProtocol, and invert the default behavior
to strictly observe RFC 7230 unless otherwise configured.
And Document This.

The relaxation option is renamed 'Unsafe'. 'Strict' is no
longer case sensitive. 'min=0.9|1.0' is now the verbose
'Allow0.9' or 'Require1.0' case-insenstive grammer. The
exclusivity tests have been modified to detect conflicts.

The 'strict,log' option failed to enforce strict conformance,
and has been removed. Unsafe, informational logging is possible
in any loadable module, after the request data is unsafely
accepted.

This triggers a group of failures in t/apache/headers.t as
expected since those patterns violated RFC 7230 section 3.2.4.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1756540 13f79535-47bb-0310-9956-ffa450edef68

docs/manual/mod/core.xml
modules/http/http_filters.c
server/core.c
server/protocol.c
server/vhost.c

index 27d24a243d53895e096a10a8448b64dbff813ce1..2bc11c04717809f445e2f12b8a08b398727861c4 100644 (file)
@@ -1249,6 +1249,60 @@ EnableSendfile On
 </usage>
 </directivesynopsis>
 
+<directivesynopsis>
+<name>EnforceHTTPProtocol</name>
+<description>Modify restrictions on HTTP Request Messages</description>
+<syntax>EnforceHTTPProtocol [Strict|Unsafe] [Allow0.9|Require1.0]</syntax>
+<default>EnforceHTTPProtocol Strict 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 &sect;3.1.1</a>) and the HTTP Request Header Fields
+    (<a href="https://tools.ietf.org/html/rfc7230#section-3.2"
+      >RFC 7230 &sect;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, an <code>Unsafe</code>
+    option has been added to revert to the legacy behavior. These rules are
+    applied prior to request processing, so must be configured at the global
+    or default (first) matching virtual host section, by 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 &sect;9.4 Request Splitting</a> and
+    <a href="https://tools.ietf.org/html/rfc7230#section-9.5"
+      >&sect;9.5 Response Smuggling</a> call out only two of the potential
+    risks of accepting non-conformant request messages. As of the introduction
+    of this directive, all grammer rules of the specification are enforced in
+    the <code>Strict</code> operating mode.</p>
+
+    <p>Users are strongly cautioned against toggling the <code>Unsafe</code>
+    mode of operation for these reasons, most especially on outward-facing,
+    publicly accessible server deployments. Reviewing the messages within the
+    <directive>ErrorLog</directive> in the <code>info</code>
+    <directive>LogLevel</directive> or below can help identify such faulty
+    requests, along with their origin. Users should pay particular attention
+    to any 400 responses in the access log for indiciations that these requests 
+    are being correctly rejected.</p>
+
+    <p><a href="https://tools.ietf.org/html/rfc2616#section-19.6"
+         >RFC 2616 &sect;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 2616 Appendix A</a>. The <code>Require1.0</code> option allows
+    the user to remove support of the <code>Allow0.9</code> default option's
+    behavior.</p>
+</usage>
+</directivesynopsis>
+
 <directivesynopsis>
 <name>Error</name>
 <description>Abort configuration parsing with a custom error message</description>
index 892b699a546da04dbc588d3fecb47b0d91266f25..059cc247c2b0c782b15cc54e7bea3dcc8b2c98c0 100644 (file)
@@ -1246,9 +1246,9 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_header_filter(ap_filter_t *f,
     }
 
     conf = ap_get_core_module_config(r->server->module_config);
-    if (conf->http_conformance & AP_HTTP_CONFORMANCE_STRICT) {
+    if (conf->http_conformance != AP_HTTP_CONFORMANCE_UNSAFE) {
         int ok = check_headers(r);
-        if (!ok && !(conf->http_conformance & AP_HTTP_CONFORMANCE_LOGONLY)) {
+        if (!ok) {
             ap_die(HTTP_INTERNAL_SERVER_ERROR, r);
             return AP_FILTER_ERROR;
         }
index 8e2c1f4b168b1806709f46870865d855c6dec204..5fbb1803ed3834754a04a05ccf25d916cf062268 100644 (file)
@@ -4011,37 +4011,39 @@ static const char *set_protocols_honor_order(cmd_parms *cmd, void *dummy,
     return NULL;
 }
 
-static const char *set_http_protocol(cmd_parms *cmd, void *dummy,
-                                     const char *arg)
+static const char *set_enforce_http_protocol(cmd_parms *cmd, void *dummy,
+                                             const char *arg)
 {
     core_server_config *conf =
         ap_get_core_module_config(cmd->server->module_config);
 
-    if (strncmp(arg, "min=", 4) == 0) {
-        arg += 4;
-        if (strcmp(arg, "0.9") == 0)
-            conf->http09_enable = AP_HTTP09_ENABLE;
-        else if (strcmp(arg, "1.0") == 0)
-            conf->http09_enable = AP_HTTP09_DISABLE;
-        else
-            return "HttpProtocol 'min' must be one of '0.9' and '1.0'";
-        return NULL;
+    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 {
+        return "EnforceHttpProtocol accepts 'Allow0.9' (default), 'Require1.0',"
+               " 'Unsafe', or 'Strict' (default)";
     }
 
-    if (strcmp(arg, "strict") == 0)
-        conf->http_conformance = AP_HTTP_CONFORMANCE_STRICT;
-    else if (strcmp(arg, "strict,log-only") == 0)
-        conf->http_conformance = AP_HTTP_CONFORMANCE_STRICT|
-                                 AP_HTTP_CONFORMANCE_LOGONLY;
-    else if (strcmp(arg, "liberal") == 0)
-        conf->http_conformance = AP_HTTP_CONFORMANCE_LIBERAL;
-    else
-        return "HttpProtocol accepts 'min=0.9', 'min=1.0', 'liberal', "
-               "'strict', 'strict,log-only'";
+    if ((conf->http09_enable & AP_HTTP09_ENABLE) &&
+        (conf->http09_enable & AP_HTTP09_DISABLE)) {
+        return "EnforceHttpProtocol 'Allow0.9' and 'Require1.0'"
+               " are mutually exclusive";
+    }
 
     if ((conf->http_conformance & AP_HTTP_CONFORMANCE_STRICT) &&
-        (conf->http_conformance & AP_HTTP_CONFORMANCE_LIBERAL)) {
-        return "HttpProtocol 'strict' and 'liberal' are mutually exclusive";
+        (conf->http_conformance & AP_HTTP_CONFORMANCE_UNSAFE)) {
+        return "EnforceHttpProtocol 'Strict' and 'Unsafe'"
+               " are mutually exclusive";
     }
 
     return NULL;
@@ -4682,9 +4684,9 @@ AP_INIT_TAKE1("TraceEnable", set_trace_enable, NULL, RSRC_CONF,
               "'on' (default), 'off' or 'extended' to trace request body content"),
 AP_INIT_FLAG("MergeTrailers", set_merge_trailers, NULL, RSRC_CONF,
               "merge request trailers into request headers or not"),
-AP_INIT_ITERATE("HttpProtocol", set_http_protocol, NULL, RSRC_CONF,
-              "'min=0.9' (default) or 'min=1.0' to allow/deny HTTP/0.9; "
-              "'liberal', 'strict', 'strict,log-only'"),
+AP_INIT_ITERATE("EnforceHttpProtocol", set_enforce_http_protocol, NULL, RSRC_CONF,
+              "'Allow0.9' or 'Require1.0' (default) to allow or deny HTTP/0.9; "
+              "'Unsafe' or 'Strict' (default) to process incorrect requests"),
 AP_INIT_ITERATE("RegisterHttpMethod", set_http_method, NULL, RSRC_CONF,
                 "Registers non-standard HTTP methods"),
 AP_INIT_FLAG("HttpContentLengthHeadZero", set_cl_head_zero, NULL, OR_OPTIONS,
index 57efda917940639d05da68b21068d5ac05e6b592..a03ecf177df3ee722bdf3ec6908f205f29b082cb 100644 (file)
@@ -570,8 +570,7 @@ static int read_request_line(request_rec *r, apr_bucket_brigade *bb)
     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_STRICT;
-    int enforce_strict = !(conf->http_conformance & AP_HTTP_CONFORMANCE_LOGONLY);
+    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.
@@ -687,13 +686,11 @@ static int read_request_line(request_rec *r, apr_bucket_brigade *bb)
         if (strict) {
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02418)
                           "Invalid protocol '%s'", r->protocol);
-            if (enforce_strict) {
-                r->proto_num = HTTP_VERSION(1,0);
-                r->protocol  = "HTTP/1.0";
-                r->connection->keepalive = AP_CONN_CLOSE;
-                r->status = HTTP_BAD_REQUEST;
-                return 0;
-            }
+            r->proto_num = HTTP_VERSION(1,0);
+            r->protocol  = "HTTP/1.0";
+            r->connection->keepalive = AP_CONN_CLOSE;
+            r->status = HTTP_BAD_REQUEST;
+            return 0;
         }
         if (3 == sscanf(r->protocol, "%4s/%u.%u", http, &major, &minor)
             && (ap_cstr_casecmp("http", http) == 0)
@@ -706,36 +703,35 @@ static int read_request_line(request_rec *r, apr_bucket_brigade *bb)
     }
 
     if (strict) {
-        int err = 0;
         if (ap_has_cntrl(r->the_request)) {
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02420)
                           "Request line must not contain control characters");
-            err = HTTP_BAD_REQUEST;
+            r->status = HTTP_BAD_REQUEST;
+            return 0;
         }
         if (r->parsed_uri.fragment) {
             /* RFC3986 3.5: no fragment */
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02421)
                           "URI must not contain a fragment");
-            err = HTTP_BAD_REQUEST;
+            r->status = HTTP_BAD_REQUEST;
+            return 0;
         }
         else if (r->parsed_uri.user || r->parsed_uri.password) {
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02422)
                           "URI must not contain a username/password");
-            err = HTTP_BAD_REQUEST;
+            r->status = HTTP_BAD_REQUEST;
+            return 0;
         }
         else if (r->method_number == M_INVALID) {
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02423)
                           "Invalid HTTP method string: %s", r->method);
-            err = HTTP_NOT_IMPLEMENTED;
+            r->status = HTTP_NOT_IMPLEMENTED;
+            return 0;
         }
         else if (r->assbackwards == 0 && r->proto_num < HTTP_VERSION(1, 0)) {
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02424)
                           "HTTP/0.x does not take a protocol");
-            err = HTTP_BAD_REQUEST;
-        }
-
-        if (err && enforce_strict) {
-            r->status = err;
+            r->status = HTTP_BAD_REQUEST;
             return 0;
         }
     }
@@ -780,6 +776,7 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_rec *r, apr_bucket_brigade *bb
     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,
@@ -890,7 +887,7 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_rec *r, apr_bucket_brigade *bb
             }
             memcpy(last_field + last_len, field, len +1); /* +1 for nul */
             /* Replace obs-fold w/ SP per RFC 7230 3.2.4 */
-            if (conf->http_conformance & AP_HTTP_CONFORMANCE_STRICT) {
+            if (strict) {
                 last_field[last_len] = ' ';
             }
             last_len += len;
@@ -919,9 +916,8 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_rec *r, apr_bucket_brigade *bb
                 return;
             }
 
-            if (!(conf->http_conformance & AP_HTTP_CONFORMANCE_STRICT))
-            {
-                /* Not Strict, using the legacy parser */
+            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 */
index 393e7ab4a814ac915b70fcd04f45c1a3451468a0..ebf1a995aa84789abf3e587439290a71d457bcc8 100644 (file)
@@ -751,8 +751,7 @@ static apr_status_t fix_hostname_non_v6(request_rec *r, char *host)
  * 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,
-                                          int logonly)
+static apr_status_t strict_hostname_check(request_rec *r, char *host)
 {
     char *ch;
     int is_dotted_decimal = 1, leading_zeroes = 0, dots = 0;
@@ -795,8 +794,6 @@ bad:
     ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02415)
                   "[strict] Invalid host name '%s'%s%.6s",
                   host, *ch ? ", problem near: " : "", ch);
-    if (logonly)
-        return APR_SUCCESS;
     return APR_EINVAL;
 }
 
@@ -822,8 +819,7 @@ static int fix_hostname(request_rec *r, const char *host_header,
     apr_status_t rv;
     const char *c;
     int is_v6literal = 0;
-    int strict = http_conformance & AP_HTTP_CONFORMANCE_STRICT;
-    int strict_logonly = http_conformance & AP_HTTP_CONFORMANCE_LOGONLY;
+    int strict = (http_conformance != AP_HTTP_CONFORMANCE_UNSAFE);
 
     src = host_header ? host_header : r->hostname;
 
@@ -843,8 +839,7 @@ static int fix_hostname(request_rec *r, const char *host_header,
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02416)
                          "[strict] purely numeric host names not allowed: %s",
                          src);
-            if (!strict_logonly)
-                goto bad_nolog;
+            goto bad_nolog;
         }
         r->hostname = src;
         return is_v6literal;
@@ -882,7 +877,7 @@ static int fix_hostname(request_rec *r, const char *host_header,
     else {
         rv = fix_hostname_non_v6(r, host);
         if (strict && rv == APR_SUCCESS)
-            rv = strict_hostname_check(r, host, strict_logonly);
+            rv = strict_hostname_check(r, host);
     }
     if (rv != APR_SUCCESS)
         goto bad;
@@ -1162,7 +1157,7 @@ AP_DECLARE(void) ap_update_vhost_from_headers(request_rec *r)
     if (r->status != HTTP_OK)
         return;
 
-    if (conf->http_conformance & AP_HTTP_CONFORMANCE_STRICT) {
+    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).
@@ -1172,10 +1167,8 @@ AP_DECLARE(void) ap_update_vhost_from_headers(request_rec *r)
         if (have_hostname_from_url && host_header != NULL) {
             const char *info = "Would replace";
             const char *new = construct_host_header(r, is_v6literal);
-            if (!(conf->http_conformance & AP_HTTP_CONFORMANCE_LOGONLY)) {
-                apr_table_set(r->headers_in, "Host", r->hostname);
-                info = "Replacing";
-            }
+            apr_table_set(r->headers_in, "Host", r->hostname);
+            info = "Replacing";
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(02417)
                           "%s Host header '%s' with host from request uri: "
                           "'%s'", info, host_header, new);