]> granicus.if.org Git - apache/commitdiff
------------------------------------------------------------------------
authorJim Jagielski <jim@apache.org>
Mon, 5 Dec 2016 14:34:29 +0000 (14:34 +0000)
committerJim Jagielski <jim@apache.org>
Mon, 5 Dec 2016 14:34:29 +0000 (14:34 +0000)
r1772419 | covener | 2016-12-02 19:10:53 -0500 (Fri, 02 Dec 2016) | 7 lines

Merge r1772418 from trunk:

loop in checking response headers

w/ HTTPProtocolOptions Unsafe

------------------------------------------------------------------------
r1772236 | wrowe | 2016-12-01 11:29:27 -0500 (Thu, 01 Dec 2016) | 8 lines

Appears we cannot disallow this whitespace, since the chunk BNF coexisted
with the implied *LWS rule, before RFC7230 eliminated the later. Whether
this is actually OWS or BWS is an editorial decision beyond our pay grade.

Backports: r1765475
Submitted by: wrowe

------------------------------------------------------------------------
r1771697 | rpluem | 2016-11-28 04:59:00 -0500 (Mon, 28 Nov 2016) | 4 lines

Merge r1771690 from trunk:

* Fix numbers count in comment.

------------------------------------------------------------------------
r1771696 | rpluem | 2016-11-28 04:56:42 -0500 (Mon, 28 Nov 2016) | 1 line

* Revert 1771372: As Bill points out correctly. Only backport trunk revisions to this branch.
------------------------------------------------------------------------
r1771372 | rpluem | 2016-11-25 14:55:18 -0500 (Fri, 25 Nov 2016) | 1 line

* Fix numbers count in comment.
------------------------------------------------------------------------
r1770870 | wrowe | 2016-11-22 13:44:21 -0500 (Tue, 22 Nov 2016) | 3 lines

Optimize away one more strchr.
Backports: 1770869

------------------------------------------------------------------------
r1770868 | wrowe | 2016-11-22 13:34:25 -0500 (Tue, 22 Nov 2016) | 8 lines

List discussion resulted in rejecting all but SP characters in the request
line, but in the strict mode prioritize excessive space testing over bad
space testing (which is captured later) and make both more efficient
(at this test ll[0] is already whitespace or \0 char). Also correct a comment.

Backports: r1770867
Submitted by: wrowe

------------------------------------------------------------------------
r1770846 | covener | 2016-11-22 09:32:45 -0500 (Tue, 22 Nov 2016) | 5 lines

Merge r1770817 from trunk:

Removing unused warning after r1764961 changes.

------------------------------------------------------------------------
r1770789 | covener | 2016-11-21 20:58:06 -0500 (Mon, 21 Nov 2016) | 25 lines

Merge r1770786 from trunk:

remove Location: header checks for absolute URL

https://tools.ietf.org/html/rfc7231#section-7.1.2

   The "Location" header field is used in some responses to refer to a
   specific resource in relation to the response.  The type of
   relationship is defined by the combination of request method and
   status code semantics.

     Location = URI-reference

   The field value consists of a single URI-reference.  When it has the
   form of a relative reference ([RFC3986], Section 4.2), the final
   value is computed by resolving it against the effective request URI
   ([RFC3986], Section 5).

There is even an example with no scheme:

     Location: /People.html#tim

------------------------------------------------------------------------
r1770386 | wrowe | 2016-11-18 09:45:32 -0500 (Fri, 18 Nov 2016) | 6 lines

Backport: r1769965
Submitted by: wrowe, rpluem

Actually cause the Host header to be overridden, as noted by rpluem,
and simplify now that there isn't a log-only mode.

------------------------------------------------------------------------
r1770173 | wrowe | 2016-11-17 07:09:32 -0500 (Thu, 17 Nov 2016) | 1 line

Merge of r1765451 did not apply cleanly, drop unneeded prototype.
------------------------------------------------------------------------
r1769675 | wrowe | 2016-11-14 13:57:12 -0500 (Mon, 14 Nov 2016) | 1 line

Add an entry about RFC strictness
------------------------------------------------------------------------
r1769674 | wrowe | 2016-11-14 13:54:42 -0500 (Mon, 14 Nov 2016) | 1 line

Clean up CHANGES for clarity
------------------------------------------------------------------------
r1769672 | wrowe | 2016-11-14 13:15:07 -0500 (Mon, 14 Nov 2016) | 31 lines

Dropped the never-released ap_has_cntrls() as it had very limited
and inefficient application at that, added ap_scan_vchar_obstext()
to accomplish a similar purpose.

Dropped HttpProtocolOptions StrictURL option, this will be better
handled in the future with a specific directive and perhaps multiple
levels of scrutiny, use ap_scan_vchar_obstext() to simply ensure there
are no control characters or whitespace within the URI.

Changed the scanning of the response header table by check_headers()
to follow the same rulesets as reading request headers. Disallow any
CTL character within a response header value, and any CTL or whitespace
in response header field name, even in strict mode.

Apply HttpProtocolOptions Strict to chunk header parsing, invalid
whitespace is invalid, line termination must follow CRLF convention.
Submitted by: wrowe
Backport: r1764961,1765112-1765115

When redrawing the parser, ap_get_http_token looked to be useful, but there's
no application for this yet in httpd, so hold off adding this function when
we backport the enhancements. ap_scan_http_token was entirely sufficient.
If the community wants this new function, we can add it when backporting
work is complete.

This patch, and the earlier patches Friday actually demanded an mmn major
bump due to struct member changes. In any final backport, new members must
be added to the end of the struct to retain an mmn minor designation.
Submitted by: wrowe
Backport: r1765451

------------------------------------------------------------------------
r1769669 | wrowe | 2016-11-14 12:59:10 -0500 (Mon, 14 Nov 2016) | 124 lines

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

------------------------------------------------------------------------
r1769664 | wrowe | 2016-11-14 12:07:40 -0500 (Mon, 14 Nov 2016) | 48 lines

Drop unused, previously sscanf() target variables
Submitted by: wrowe
Backport: r1756821

Drop redundant == --rrl_none evaluation
Submitted by: rpluem
Backport: r1756823

server/protocol.c (read_request_line): Fix compiler warnings with GCC.
Submitted by: jorton
Backport: r1756824

Correct request header handling of whitespace with the new possible config of
HttpProtocolOptions Unsafe StrictWhitespace

I have elected not to preserve any significance to excess whitespace in the
now-deprecated obs-fold code path, that's certainly open for discussion.

This can be reviewed by tweaking t/conf/extra.conf to switch Strict to Unsafe.
Submitted by: wrowe
Backport: r1756847

A band-aid to resolve an immediate IBM MVS'ism
Submitted by: wrowe
Backport: r1756849

Resolve Netware (and other arch) build error for non-portable isascii()
Submitted by: wrowe
Backport: r1756934

Generally, the cart comes before the horse, this mirrors apr_lib.h
Submitted by: wrowe
Backport: r1756937

After lengthy investigation with covener's assistance, it seems we cannot
use a static table. We cannot change this to dynamic use of the local iconv
without build changes to avoid such use on cross-platform builds.

I'm satisfied if we trust iscntrl to at least catch all the most lethal
C0 Ctrls (we are promised it catches bad carriage control/line endings)
and leave this in the short term with an XXX to revisit at a future time.

The token stop never needed this table, because we can use the affirmative
list of token characters to define it.
Submitted by: wrowe, covener
Backport: r1756946

------------------------------------------------------------------------
r1769662 | wrowe | 2016-11-14 12:01:20 -0500 (Mon, 14 Nov 2016) | 46 lines

Rename the previously undocumented HTTPProtocol directive
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.
Submitted by: wrowe
Backport: r1756540

Correct AP_HTTP_CONFORMANCE_ flags
Submitted by: wrowe
Backport: r1756555

Renaming this directive to HttpProtocolOptions after discussion on dev@
Submitted by: wrowe
Backport: r1756649

Perform correct, strict parsing of the request line, handling the
http protocol tag, url and method appropriately, and attempting
to extract values even in the presence of unusual whitespace in
keeping with section 3.5, prior to responding with whatever
error reply is needed. Conforms to RFC7230 in all respects,
the section 3.5 optional behavior can be disabled by the user
with a new HttpProtocolOptions StrictWhitespace flag. In all
cases, the_request is regenerated from the parsed components
with exactly two space characters.

Shift sf's 'strict' method check from the Strict behavior because
it violates forward proxy logic, adding a new RegisteredMethods
flag, as it will certainly be useful to some.
Submitted by: wrowe
Backport: r1756729

------------------------------------------------------------------------
r1769649 | wrowe | 2016-11-14 10:29:20 -0500 (Mon, 14 Nov 2016) | 124 lines

Improve legibility of reviewing the generated table, using hex rather than dec
Submitted by: wrowe
Backport: r1754536

Correct T_HTTP_TOKEN_STOP per RFC2068 (2.2) - RFC7230 (3.2.6),
which has always defined 'token' as CHAR or VCHAR - visible USASCII only.
NUL char is also a stop, end of parsing.
Submitted by: wrowe
Backport: r1754538

Be more explicit about NUL in case iscntrl is inconsistent
Submitted by: wrowe
Backport: r1754539

Introduce T_HTTP_CTRLS for efficiently finding non-text chars
Submitted by: wrowe
Backport: r1754540

Introduce ap_scan_http_field_content, ap_scan_http_token
and ap_get_http_token [later reverted] for more efficient
string handling.
Submitted by: wrowe
Backport: r1754541

With NUL as a TOKEN_STOP, this code is more efficient
Submitted by: wrowe
Backport: r1754544

We arrive here for more than one cause; offer a more general statement
Submitted by: wrowe
Backport: r1754547

Strictly observe spec on obs-fold
Submitted by: wrowe
Backport: r1754548

Leave an emphatic TODO per Jeff's observations
Submitted by: trawick
Backport: r1754555

Introduce ap_scan_http_token / ap_scan_http_field_content for a much
more efficient pass through the header text; rather than reparsing
the strings over and over under the HTTP_CONFORMANCE_STRICT fules.

Improve logic and legibility by eliminating multiple repetitive tests
of the STRICT flag, and simply reorder 'classic' behavior first and
this new parser second to simplify the diff. Because of the whitespace
change (which I had wished to dodge), reading this --ignore-all-space
is a whole lot easier. Particularly against 2.4.x branch, which is now
identical in the 'classic' logic flow. Both of which I'll share with dev@
Submitted by: wrowe
Backport: r1754556

Friendly catch by RĂ¼diger, restore line mis-removed by the previous commit
Submitted by: rpluem
Backport: r1754568

Clean up doubled-'{'
Correct usage for ap_scan_http_token (had used _get_ syntax)
Correct logic, detect no 'token' chars, or missing ':'
Submitted by: wrowe, rpluem
Backport: r1754569,r1754570,r1754577

Replacement solution to identify VCHAR/ASCII symbols, even in EBCDIC.
Looking for someone with an EBCDIC environment to post the output of
the test_char.h generated file for verification.
Submitted by: wrowe
Backport: r1754579

Clean up an edge case where obs-fold continuation preceeds the first header,
as with r1755098, but this time ensure the previous header processing logic
ensures there was a previous header as identified by jchampion.

This patch restructures the loop for legibility with a loop continuation,
allowing us to flatten all of this hard-to-follow code. The subsequent
patch will be a whitespace-only change for formatting.

Testing len > 0 is redundant when *field is a "\0" and mismatches here,
folded flag was a no-op, unused once we added continue; logic.
Fix these as initially attempted in r1755114.

Improve comments and reflow whitespace.
Submitted by: wrowe
Backport: r1755123,r1755124,r1755125,r1755126

As promised, reduce this logic by net 9 code lines, shifting the burden
of killing trailing whitespace to the purpose-agnostic read logic.

Whitespace before or after an obs-fold, and before or after a field value
have no semantic purpose at all. Because we are building a buffer for all
folded values, reducing the size of the newly allocated buffer is always
to our advantage.
Submitted by: wrowe
Backport: r1755233

Treat empty obs-fold line as a noop, eliminate all intra-obs-fold excess
whitespace, and observe the 1 SP per obs-folding per spec.
Submitted by: wrowe
Backport: r1755234,r1755235,r1755236

Treat empty obs-fold line as abusive traffic.
Submitted by: wrowe
Backport: r1755263

Stop reflecting irrelevant data to the request error notes, particularly
for abusive and malformed traffic the non-technical consumer of a user-agent
has no control over.

Simply take note where the administrator-configured limits have been exceeded,
that administrator can find details in the error log if desired.
Submitted by: wrowe
Backport: r1755264

Follow up to r1755264.
Don't crash when ap_rgetline() returns a NULL field on ENOSPC.
Submitted by: ylavic
Backport: r1755343

Follow on to r1755264, for the case of merged header length exceptions,
and ensure the field header name is truncated to a sane log width.
Submitted by: wrowe
Backport: r1755744

------------------------------------------------------------------------
r1769454 | wrowe | 2016-11-12 18:47:29 -0500 (Sat, 12 Nov 2016) | 2 lines

Partial Backport of r1746884, no-op changes that introduce patch conflicts.

------------------------------------------------------------------------
r1768978 | wrowe | 2016-11-09 09:39:05 -0500 (Wed, 09 Nov 2016) | 5 lines

Backports: r1687643
Submitted by: covener

be less weird in comment

------------------------------------------------------------------------
r1768977 | wrowe | 2016-11-09 09:37:34 -0500 (Wed, 09 Nov 2016) | 5 lines

Backports: r1687642
Submitted by: covener
elaborate on a misleading comment

------------------------------------------------------------------------
r1768971 | wrowe | 2016-11-09 09:32:09 -0500 (Wed, 09 Nov 2016) | 8 lines

core: Follow up to r1664205 (previously backported)
Don't let invalid r->proto_num/protocol out of read_request_line() reach
the output filters (when responding with 400 Bad Request).
Suggested by: rpluem
Backports: r1664576

------------------------------------------------------------------------
r1768969 | wrowe | 2016-11-09 09:23:00 -0500 (Wed, 09 Nov 2016) | 10 lines

Backport: r1610383
Submitted by: jailletc36
Simplify code.

Cases where 'loc' doesn't have any ':' or is  starting with ':' are already
handled by 'ap_is_url()'
Calling 'apr_isascii()' seems useless.

------------------------------------------------------------------------
r1768968 | wrowe | 2016-11-09 09:20:45 -0500 (Wed, 09 Nov 2016) | 4 lines

Backport: r1546860
Submitted by: jailletc36
Fix missing space in message of protocol.c (other r1546860 changes ignored)

------------------------------------------------------------------------
r1768093 | wrowe | 2016-11-04 16:50:45 -0400 (Fri, 04 Nov 2016) | 7 lines

ap_rgetline_core() now pulls from r->proto_input_filters
for better input filtering behavior during chunked trailer
processing by ap_http_filter().
Backports: r1446421
Submitted by: joes

------------------------------------------------------------------------
r1768090 | wrowe | 2016-11-04 16:47:00 -0400 (Fri, 04 Nov 2016) | 7 lines

Stupid CodeWarrior compiler cant take vars with struct inits.
Ensure that is_v6literal is always initialized

Backports: r1428145, r1436457
Submitted by: fuankg, rpluem

------------------------------------------------------------------------
r1768036 | wrowe | 2016-11-04 10:20:16 -0400 (Fri, 04 Nov 2016) | 40 lines

Add an option to enforce stricter HTTP conformance

This is a first stab, the checks will likely have to be revised.
For now, we check

 * if the request line contains control characters
 * if the request uri has fragment or username/password
 * that the request method is standard or registered with RegisterHttpMethod
 * that the request protocol is of the form HTTP/[1-9]+.[0-9]+,
   or missing for 0.9
 * if there is garbage in the request line after the protocol
 * if any request header contains control characters
 * if any request header has an empty name
 * for the host name in the URL or Host header:
   - if an IPv4 dotted decimal address: Reject octal or hex values, require
     exactly four parts
   - if a DNS host name: Reject non-alphanumeric characters besides '.' and
     '-'. As a side effect, this rejects multiple Host headers.
 * if any response header contains control characters
 * if any response header has an empty name
 * that the Location response header (if present) has a valid scheme and is
   absolute

If we have a host name both from the URL and the Host header, we replace the
Host header with the value from the URL to enforce RFC conformance.

There is a log-only mode, but the loglevels of the logged messages need some
thought/work. Currently, the  checks for incoming data log for 'core' and the
checks for outgoing data log for 'http'. Maybe we need a way to configure the
loglevels separately from the core/http loglevels.

change protocol number parsing in strict mode according to HTTPbis draft
- only accept single digit version components
- don't accept white-space after protocol specification

Clean up comment, fix log tags.
Submitted by: sf
Backports: r1426877, r1426879, r1426988, r1426992

------------------------------------------------------------------------
r1768035 | wrowe | 2016-11-04 10:14:59 -0400 (Fri, 04 Nov 2016) | 14 lines

Correctly parse an IPv6 literal host specification in an absolute URL
in the request line.

- Fix handling of brackets [ ] surrounding the IPv6 address.
- Skip parsing r->hostname again if not necessary.
- Do some checks that the IPv6 address is sane. This is not done by
  apr_parse_addr_port().

log client error at level debug, log broken Host header value

Backports: r1407006, r1426827
Submitted by: sf

------------------------------------------------------------------------
r1767942 | wrowe | 2016-11-03 14:01:23 -0400 (Thu, 03 Nov 2016) | 5 lines

Expose ap_method_register() to the admin with a new RegisterHttpMethod
directive.
Backports: r1407599
Submitted by: sf

------------------------------------------------------------------------
r1767941 | wrowe | 2016-11-03 13:57:50 -0400 (Thu, 03 Nov 2016) | 9 lines

New directive HttpProtocol which allows to disable HTTP/0.9 support
with min=0.9|1.0 syntax.

A tighter restriction off the version in the request line is still
possible with <If "%{SERVER_PROTOCOL_NUM} ..."> .
Submitted by: sf
Backports: r1406719, r1407643, r1425366

------------------------------------------------------------------------
r1767912 | wrowe | 2016-11-03 11:55:18 -0400 (Thu, 03 Nov 2016) | 1 line

Branch to bring http protocol parsing in 2.4.x in sync with trunk
------------------------------------------------------------------------

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1772678 13f79535-47bb-0310-9956-ffa450edef68

13 files changed:
CHANGES
STATUS
docs/manual/mod/core.xml
include/ap_mmn.h
include/http_core.h
include/http_protocol.h
include/httpd.h
modules/http/http_filters.c
server/core.c
server/gen_test_char.c
server/protocol.c
server/util.c
server/vhost.c

diff --git a/CHANGES b/CHANGES
index 404dbf8c90b38e64da0bd85d02e5fd91cbf2fe6a..7fc1fd06fa4b1de6a654064ba275ba4b75f9ea3d 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -65,6 +65,24 @@ Changes with Apache 2.4.24
      the same PID (e.g. in container).  PR 60261.
      [Val <valentin.bremond gmail.com>, Yann Ylavic]
 
+  *) 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]
diff --git a/STATUS b/STATUS
index 0a9e10df4ca022a9609b7b1507f1c45afd9b289e..d8aff5a61a32bac553b3bf7e42c35d5e9f646500 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -117,16 +117,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) Propose default strict RFC7230 behavior, and HttpProtocolOptions directive
-     to relax or further constrain some behaviors.
-     trunk patches: too numerous to list, see 
-       svn log --stop-on-copy http://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict/
-     2.4.x patch: see
-       svn diff -r1767912:HEAD http://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict/
-       Last updated: 1 Dec with a missing element of http_filters.c
-     When testing, revert;
-       http://svn.apache.org/r1772339 
-     +1: wrowe, covener, jim
 
 
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
index 5d179249745a397cf3dd1b1dca28d6b6bca2985a..471cb0ad893ce0e7d5de3321c38657a342e83c9b 100644 (file)
@@ -1237,6 +1237,82 @@ EnableSendfile On
 </usage>
 </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 &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 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 &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, 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>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 &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 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>
+
 <directivesynopsis>
 <name>Error</name>
 <description>Abort configuration parsing with a custom error message</description>
@@ -4765,5 +4841,20 @@ as if 'QualifyRedirectURL ON' was configured.</compatibility>
 </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 a4feabc144660eedb7a7f48b042b1d467b15f027..124057ca7d688d2b8866e727a493622cad1e7885 100644 (file)
  * 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" */
 #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 2590b22d36f919559d61339b0eec339773d3a20f..35df5dc96014883fff3da71634ca67473b8ee184 100644 (file)
@@ -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;
+
+#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 447e3a8c0308931e56cdb1298bf17a3d434f854a..a9e09904bdc32bd6ca853eafd6f5c7868f1a4997 100644 (file)
@@ -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);
 
 /**
index e02bcc09fbfbd436e3090499f83b66814aac987a..61ab2e6ed62042b5a3ce8588951f46ca0c49b8df 100644 (file)
@@ -1585,6 +1585,28 @@ AP_DECLARE(int) ap_find_etag_weak(apr_pool_t *p, const char *line, const char *t
  */
 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 b4ca437edd6267f05374954722e99831466e4463..45c1d81198840c83f38214bbb502e72f51f311e3 100644 (file)
@@ -126,14 +126,15 @@ static apr_status_t bail_out_on_error(http_ctx_t *ctx,
 
 /**
  * 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 *ctx, const char *buffer,
         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 *ctx, const char *buffer,
             }
             else {
                 /*
-                 * LF expected.
+                 * CRLF expected.
                  */
                 return APR_EINVAL;
             }
@@ -180,6 +187,12 @@ static apr_status_t parse_chunk_size(http_ctx_t *ctx, const char *buffer,
         }
 
         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,14 +214,17 @@ static apr_status_t parse_chunk_size(http_ctx_t *ctx, const char *buffer,
         }
         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;
             }
         }
         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_bucket_brigade *b,
                             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_bucket_brigade *b,
     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_bucket_brigade *b,
                     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,14 +684,83 @@ apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b,
     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(void *arg, const char *name, const char *val)
+{
+    struct check_header_ctx *ctx = arg;
+    const char *test;
+
+    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) { 
+        test = ap_scan_http_token(name);
+    }
+    else {
+        test = ap_scan_vchar_obstext(name);
+    }
+    if (*test) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02429)
+                      "Response header name '%s' contains invalid "
+                      "characters, aborting request",
+                      name);
+        return 0;
+    }
+
+    if (ctx->strict) { 
+        test = ap_scan_http_field_content(val);
+    }
+    else {
+        /* Simply terminate scanning on a CTL char, allowing whitespace */
+        test = val;
+        do {
+            while (*test == ' ' || *test == '\t') test++;
+            test = ap_scan_vchar_obstext(test);
+        } while (*test == ' ' || *test == '\t');
+    }
+    if (*test) {
+        ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, ctx->r, APLOGNO(02430)
+                      "Response header '%s' value of '%s' contains invalid "
+                      "characters, aborting request",
+                      name, 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);
+    if (!apr_table_do(check_header, &ctx, r->headers_out, NULL))
+        return 0; /* problem has been logged by check_header() */
+
+    return 1;
+}
+
 typedef struct header_struct {
     apr_pool_t *pool;
     apr_bucket_brigade *bb;
 } 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,
@@ -1229,6 +1314,11 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_header_filter(ap_filter_t *f,
                                            r->headers_out);
     }
 
+    if (!check_headers(r)) {
+        ap_die(HTTP_INTERNAL_SERVER_ERROR, r);
+        return AP_FILTER_ERROR;
+    }
+
     /*
      * Remove the 'Vary' header field if the client can't handle it.
      * Since this will have nasty effects on HTTP/1.1 caches, force
index 120c9bd034a4baa2642ee67397846fcd827393da..20fa583bdbb283d74144e19f687897566d74f147 100644 (file)
@@ -519,6 +519,15 @@ static void *merge_core_server_configs(apr_pool_t *p, void *basev, void *virtv)
     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_parms *cmd, void *dummy,
     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, RSRC_CONF,
 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 9cace3e4ee7c7b8cd48b208c6b9cb28ee2c7faea..48ae6f47d02dfc46f4ace8c01eee2c0bf0520312 100644 (file)
 
 #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
 
 #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,19 +69,23 @@ 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,
            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 88d0f992517091e42f021f6b573a1a37528d1e48..7bc292cb162479fde6504a0d14e2a363018ee7f8 100644 (file)
@@ -191,6 +191,10 @@ AP_DECLARE(apr_time_t) ap_rationalize_mtime(request_rec *r, apr_time_t mtime)
 /* 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(request_rec *r, apr_time_t mtime)
  * 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,13 +215,15 @@ AP_DECLARE(apr_time_t) ap_rationalize_mtime(request_rec *r, apr_time_t mtime)
  */
 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;
     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, apr_size_t n,
 
     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, apr_size_t n,
         }
     }
 
+    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_size_t n,
             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, apr_size_t n,
                      */
                     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_size_t n,
 }
 #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(int) ap_getline(char *s, int n, request_rec *r, int fold)
     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, const char *uri)
     }
 }
 
-static int read_request_line(request_rec *r, apr_bucket_brigade *bb)
+/* 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 *ll;
-    const char *uri;
-    const char *pro;
+    const char *end = ap_strchr_c(field, ':');
+    if (end == NULL || end - field > LOG_NAME_MAX_LEN)
+        return LOG_NAME_MAX_LEN;
+    return end - field;
+}
 
-    unsigned int major = 1, minor = 0;   /* Assume HTTP/1.0 if non-"HTTP" protocol */
-    char http[5];
+static int read_request_line(request_rec *r, apr_bucket_brigade *bb)
+{
+    enum {
+        rrl_none, rrl_badmethod, rrl_badwhitespace, rrl_excesswhitespace,
+        rrl_missinguri, rrl_baduri, rrl_badprotocol, rrl_trailingtext,
+        rrl_badmethod09, rrl_reject09
+    } deferred_error = rrl_none;
+    char *ll;
+    char *uri;
     apr_size_t len;
     int num_blank_lines = DEFAULT_LIMIT_BLANK_LINES;
+    core_server_config *conf = ap_get_core_module_config(r->server->module_config);
+    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_bucket_brigade *bb)
          */
         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_bucket_brigade *bb)
              * 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,269 @@ static int read_request_line(request_rec *r, apr_bucket_brigade *bb)
     }
 
     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)
+            ; 
+    }
+
+    /* 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';
 
-    /* Provide quick information about the request method as soon as known */
+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->assbackwards = 0;
+        r->proto_num = HTTP_VERSION(1,0);
+        /* Defer setting the r->protocol string till error msg is composed */
+        if (strict && deferred_error == rrl_none)
+            deferred_error = rrl_badprotocol;
+        else
+            r->protocol  = "HTTP/1.0";
+    }
+    else {
+        r->assbackwards = 1;
+        r->protocol = "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);
-    if (r->status != HTTP_OK) {
-        r->proto_num = HTTP_VERSION(1,0);
-        r->protocol  = apr_pstrdup(r->pool, "HTTP/1.0");
+
+    /* 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;
+    }
+
+    /* 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');
-    }
-    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);
+    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;
+        }
+    }
 
     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  = "HTTP/1.0";
+    }
+    return 0;
 }
 
 static int table_do_fn_check_lengths(void *r_, const char *key,
@@ -680,26 +920,13 @@ static int table_do_fn_check_lengths(void *r_, const char *key,
 
     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 +937,8 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_rec *r, apr_bucket_brigade *bb
     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 +946,10 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_rec *r, apr_bucket_brigade *bb
      */
     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,156 +966,218 @@ AP_DECLARE(void) ap_get_mime_headers_core(request_rec *r, apr_bucket_brigade *bb
              * 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;
+
+            /* 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
+            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 (!strict)
+            {
+                /* Not Strict ('Unsafe' mode), using the legacy parser */
 
-                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 (!(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);
                     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;
+                }
+
+                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;
                 }
 
-                /* Strip LWS after field-value: */
-                tmp_field = last_field + last_len - 1;
-                while (tmp_field > value
-                       && (*tmp_field == ' ' || *tmp_field == '\t')) {
-                    *tmp_field-- = '\0';
+                *value++ = '\0'; /* NUL-terminate last_field name at ':' */
+
+                while (*value == ' ' || *value == '\t') {
+                    ++value;     /* Skip LWS of value */
                 }
 
-                apr_table_addn(r->headers_in, last_field, value);
+                /* Find invalid, non-HT ctrl char, or the trailing NULL */
+                tmp_field = (char *)ap_scan_http_field_content(value);
 
-                /* 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
+                /* 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 +1202,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,16 +1261,19 @@ 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)",
                               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 +1283,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 +1312,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 +1331,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 +1352,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 +1359,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 +1383,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 06da789718bd8a9ea196dc5539164d8dcfa4fa54..fba34bde1a9b318681fca6a3a9d455e4745359f7 100644 (file)
@@ -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_strict(apr_pool_t *p,
     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_strict(apr_pool_t *p,
     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 67572c8a31f8aa1e931cf42cb8d6611b7e336e5a..1896595653598a2dba758c54df86717f02854527 100644 (file)
@@ -687,6 +687,116 @@ static int vhost_check_config(apr_pool_t *p, apr_pool_t *plog,
  * 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_pool_t *plog,
  * 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)