Commit f8eec8bb authored by William A. Rowe Jr's avatar William A. Rowe Jr
Browse files

Fix syntax 

Submitted by: jailletc36
Backport: r1756862

Introduce StrictURI|UnsafeURI for RFC3986 enforcement
Submitted by: wrowe
Backport: r1756959

Surpress noise about syntax
Submitted by: wrowe
Backport: r1756978

Yann is correct, % is distinct from reserved and unreserved
Submitted by: wrowe
Backport: r1757062

As commented, ensure we don't flag a request as a rejected 0.9 request
if we identified any other parsing errors and handle all 0.9 request
errors as 400 BAD REQUEST, presuming HTTP/1.0 to deliver the error details.
Do not report 0.9 issues as 505 INVALID PROTOCOL because the client apparently
specified no protocol, and 505 post-dates the simple HTTP request mechanism.
Submitted by: wrowe
Backport: r1757065

Rename LenientWhitespace to UnsafeWhitespace and change StrictWhitespace
to the default behavior, after discussion with fielding et al about the
purpose of section 3.5. Update the documentation to clarify this.

This patch removes whitespace considerations from the Strict|Unsafe toggle
and consolidates them all in the StrictWhitespace|UnsafeWhitespace toggle.

Added a bunch of logic comments to read_request_line parsing.

Dropped the badwhitespace list for an all-or-nothing toggle in rrl.

Leading space before the method is optimized to be evaluated only once.

Toggled the request from HTTP/0.9 to HTTP/1.0 for more BAD_REQUEST cases.

Moved s/[\n\v\f\r]/ / cleanup logic earlier in the cycle, to operate on
each individual line read, and catch bad whitespace errors earlier.
This changes the obs-fold to more efficiently condense whitespace and
forces concatinatination with a single SP, always. Overrides are not
necessary since obs-fold is clearly deprecated.
Submitted by: wrowe
Backport: r1757589

Also catch invalid spaces between the URI <> Protocol in StrictWhitespace mode.
(matching the test for the Method <> URI)
Submitted by: wrowe
Backport: r1757593

Correct RFC reference text (link was right)
Submitted by: wrowe
Backport: r1757711

First survey results, all intrinsicly bad input will be logged at the debug
level, no louder. This patch intentionally dodges the Limit* constrained tests
since administrators may shoot themselves in the foot, or be confronted with
impossibly long cookie values, etc.

Adjust the documentation to match.
Submitted by: wrowe
Backport: r1757920

Correct URL failure reporting.

Drop the second reporting of HEAD over HTTP/0.9 requests, we short-circuit
this early now in read_request_line() when presented anything other than
the sole "GET" method permitted by spec.
Revert to the correct APLOGNO ID for this case	
Submitted by: wrowe
Backport: r1757921, r1757924

Folding StrictWhitespace into the Strict ruleset of RFC7230, per dev@ poll.
This choice is unanimous, although StrictURI (a different RFC) still hasn't
found absolute concensus.
Submitted by: wrowe
Backport: r1758226

Correct the parser construction for several optimizations,
based on the fact that bad whitespace shall not be permitted
or corrected in any operating mode, while preserving the 
ability to extract bad method/uri/proto for later reporting
and diagnostics.

This change causes badwhitespace in the request line or any
request field line to always fail, and not honor the setting
of the HttpProtocolOptions Unsafe option. Mult SP characters
or trailing SP characters in the request line are still 
permitted in Unsafe mode.

Adjusted several error message emits to match these changes.
Submitted by: wrowe
Backport: r1758263

Clarify documentation based on concensus decisions discussed on dev@
and reflecting the current implementation, clean up stray <p>
Submitted by: wrowe
Backport: r1758265, r1758266

New optional flag to enforce <CR><LF> line delimiters in ap_[r]getline,
created by overloading 'int fold' (1 or 0) as 'int flags', with the same
value 1 for AP_GETLINE_FOLD (which httpd doesn't use), and a new value
2 for AP_GETLINE_CRLF

Enforce CRLF when HttpProtocolOptions Strict is in force.

Correctly introduces a new t/TEST fail.
Submitted by: wrowe
Backport: r1758304

Calm some overly agressive crlf handling, and clarify
Submitted by: wrowe
Backport: r1758305, r1758313

Review of IE 11, Firefox 48 and Chrome 53 all indicate that ';' URI characters
are transmitted unencoded, per RFC3986 section 3.3 grammer. Correct httpd's
behavior to not encode ';' in proxied URI's or Location: response headers.
Submitted by: wrowe
Backport: r1760444




git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict@1769669 13f79535-47bb-0310-9956-ffa450edef68
parent 62f8456e

CHANGES

+4 −0
Original line number Diff line number Diff line
@@ -2,6 +2,10 @@ 


Changes with Apache 2.4.24



  *) 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, http: Extend HttpProtocol with an option to enforce stricter HTTP

docs/manual/mod/core.xml

+49 −45
Original line number Diff line number Diff line
@@ -1239,12 +1239,11 @@ EnableSendfile On 
</directivesynopsis>



<directivesynopsis>

<name>HTTPProtocolOptions</name>

<name>HttpProtocolOptions</name>

<description>Modify restrictions on HTTP Request Messages</description>

<syntax>HTTPProtocolOptions [Strict|Unsafe] [Allow0.9|Require1.0] 

[StrictWhitespace|LenientWhitespace] [RegisteredMethods|LenientMethods]</syntax>

<default>HTTPProtocolOptions Strict Allow0.9 LenientWhitespace 

LenientMethods</default>

<syntax>HttpProtocolOptions [Strict|Unsafe] [StrictURL|UnsafeURL]

 [RegisteredMethods|LenientMethods] [Allow0.9|Require1.0]</syntax>

<default>HttpProtocolOptions Strict StrictURL 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>
@@ -1256,11 +1255,11 @@ LenientMethods</default> 
    (<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>

    custom user-agents which must be deperecated, <code>Unsafe</code>

    and <code>UnsafeURL</code> options have 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
@@ -1269,53 +1268,58 @@ LenientMethods</default> 
      >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 default <code>Strict</code> operating mode.</p>

    risks of accepting non-conformant request messages, while

    <a href="https://tools.ietf.org/html/rfc7230#section-3.5"

         >RFC 7230 &sect;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><a href="https://tools.ietf.org/html/rfc3986#section-2.2"

         >RFC 3986 &sect;2.2 and 2.3</a> define "Reserved Characters" and

    "Unreserved Characters". All other character octets are required to

    be %XX encoded under this spec, and RFC7230 defers to these requirements.

    By default the <code>StrictURI</code> option will reject all requests 

    containing invalid characters. This rule can be relaxed with the

    <code>UnsafeURI</code> option to support badly written user-agents.</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>, configured with

    <directive>LogLevel</directive> <code>info</code> level or below,

    or <code>UnsafeURI</code> modes 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 only those Unsafe

    options which are necessary, and only on a specific virtual host configured

    to service only 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 any 400 responses in the access

    log for indiciations that valid requests are unexpectedly 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 default <code>Allow0.9</code> option's

    behavior.</p>



    <p><a href="https://tools.ietf.org/html/rfc7230#section-3.5"

         >RFC 7230 &sect;3.5</a> "Message Parsing Robustness" permits, and

    identifies potential risks of parsing messages containing non-space

    character whitespace. While the spec defines that exactly one space

    seperates the URI from the method, and the protocol from the URI, the

    Apache HTTP Server has traditionally been lenient in accepting other

    whitespace including one or more horizontal-tab or space characters.

    The default <code>LenientWhitespace</code> continues to accept such

    requests from non-conforming user-agents, but the administrator may toggle

    the <code>StrictWhitespace</code> option to insist on precisely two spaces

    in the request line. Other whitespace including vertical-tab, form-feed,

    and carriage-return characters are rejected and cannot be supported.</p>

    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 &sect;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 all

    permitted method tokens using the <directive>RegisterHttpMethod</directive>

    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 &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 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>

include/ap_mmn.h

+4 −4
Original line number Diff line number Diff line
@@ -487,12 +487,12 @@ 
 * 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.5.0-dev)  Add http09_enable to core_server_config

 *                          Add http_conformance to core_server_config

 * 20120211.67 (2.4.24-dev) Add http09_enable, http_conformance, and

 *                          http_methods to core_server_config

 *                          Add ap_has_cntrl(), ap_get_http_token()

 *                          Add ap_scan_http_field_[content|token]()

 *                          Added http_whitespace and http_methods to

 *                          core_server_config

 *                          Replaced fold boolean with with multiple bit flags

 *                          to ap_[r]getline()

 */



#define MODULE_MAGIC_COOKIE 0x41503234UL /* "AP24" */

include/http_core.h

+5 −5
Original line number Diff line number Diff line
@@ -736,16 +736,16 @@ typedef struct { 
#define AP_HTTP_CONFORMANCE_STRICT    2

    char http_conformance;



#define AP_HTTP_WHITESPACE_UNSET      0

#define AP_HTTP_WHITESPACE_LENIENT    1

#define AP_HTTP_WHITESPACE_STRICT     2

    char http_whitespace;



#define AP_HTTP_METHODS_UNSET         0

#define AP_HTTP_METHODS_LENIENT       1

#define AP_HTTP_METHODS_REGISTERED    2

    char http_methods;



#define AP_HTTP_URI_UNSET             0

#define AP_HTTP_URI_UNSAFE            1

#define AP_HTTP_URI_STRICT            2

    char http_stricturi;



} core_server_config;



/* for AddOutputFiltersByType in core.c */

include/http_protocol.h

+12 −5
Original line number Diff line number Diff line
@@ -582,17 +582,22 @@ AP_DECLARE(int) ap_get_basic_auth_pw(request_rec *r, const char **pw); 
 */

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_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold); 
 * @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_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold); 
#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_size_t n, 
/** @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);



/**