change error handling for bad resp headers
- avoid looping between ap_die and the http filter
- remove the header that failed the check
- keep calling apr_table_do until our fn stops matching
This is still not great. We get the original body, a 500 status
code and status line.
(r1773285 + fix for first return from check_headers)
Follow up to r1773293.
When check_headers() fails, clear anything (headers and body) from original/errorneous
response before returning 500.
Follow up to r1773761: don't check_headers() more than once.
Follow up to r1773761: don't recurse on internal redirects.
Follow up to r1773761: don't recurse on ap_send_error_response() either.
Follow up to r1773761: we need to check both ap_send_error_response() and internal redirect recursions.
Follow up to r1773761: improved recursion detection.
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict@1775669 13f79535-47bb-0310-9956-ffa450edef68
Drop C-L header and message-body from HTTP 204 responses.
The C-L header can be set in a fcgi/cgi backend or in other
filters like ap_content_length_filter (with the value of 0),
meanwhile the message-body can be returned incorrectly
by any backend. The idea is to remove unnecessary bytes
from a HTTP 204 response.
PR 51350
Note: merged to avoid manual conflicts, became a depedendency of the HTTP
strict in trunk.
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict@1775668 13f79535-47bb-0310-9956-ffa450edef68
are pstrdup'ed. Note that r->protocol = "" is not in a return path.
Simplify the garbage-in protocol handling without consideration to 'strict'
settings. It is expected to be caused by an invalid raw SP in the URL.
Backports: r1773159 (with pstrdup enhancement)
Submitted by: rpluem, wrowe
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict@1773161 13f79535-47bb-0310-9956-ffa450edef68
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
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict@1770868 13f79535-47bb-0310-9956-ffa450edef68
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
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict@1770789 13f79535-47bb-0310-9956-ffa450edef68
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
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict@1769672 13f79535-47bb-0310-9956-ffa450edef68
Submitted by: jailletc36
Backport: r1756862
Introduce StrictURI|UnsafeURI for RFC3986 enforcement
Submitted by: wrowe
Backport: r1756959
Surpress noise about syntax
Submitted by: wrowe
Backport: r1756978
Yann is correct, % is distinct from reserved and unreserved
Submitted by: wrowe
Backport: r1757062
As commented, ensure we don't flag a request as a rejected 0.9 request
if we identified any other parsing errors and handle all 0.9 request
errors as 400 BAD REQUEST, presuming HTTP/1.0 to deliver the error details.
Do not report 0.9 issues as 505 INVALID PROTOCOL because the client apparently
specified no protocol, and 505 post-dates the simple HTTP request mechanism.
Submitted by: wrowe
Backport: r1757065
Rename LenientWhitespace to UnsafeWhitespace and change StrictWhitespace
to the default behavior, after discussion with fielding et al about the
purpose of section 3.5. Update the documentation to clarify this.
This patch removes whitespace considerations from the Strict|Unsafe toggle
and consolidates them all in the StrictWhitespace|UnsafeWhitespace toggle.
Added a bunch of logic comments to read_request_line parsing.
Dropped the badwhitespace list for an all-or-nothing toggle in rrl.
Leading space before the method is optimized to be evaluated only once.
Toggled the request from HTTP/0.9 to HTTP/1.0 for more BAD_REQUEST cases.
Moved s/[\n\v\f\r]/ / cleanup logic earlier in the cycle, to operate on
each individual line read, and catch bad whitespace errors earlier.
This changes the obs-fold to more efficiently condense whitespace and
forces concatinatination with a single SP, always. Overrides are not
necessary since obs-fold is clearly deprecated.
Submitted by: wrowe
Backport: r1757589
Also catch invalid spaces between the URI <> Protocol in StrictWhitespace mode.
(matching the test for the Method <> URI)
Submitted by: wrowe
Backport: r1757593
Correct RFC reference text (link was right)
Submitted by: wrowe
Backport: r1757711
First survey results, all intrinsicly bad input will be logged at the debug
level, no louder. This patch intentionally dodges the Limit* constrained tests
since administrators may shoot themselves in the foot, or be confronted with
impossibly long cookie values, etc.
Adjust the documentation to match.
Submitted by: wrowe
Backport: r1757920
Correct URL failure reporting.
Drop the second reporting of HEAD over HTTP/0.9 requests, we short-circuit
this early now in read_request_line() when presented anything other than
the sole "GET" method permitted by spec.
Revert to the correct APLOGNO ID for this case
Submitted by: wrowe
Backport: r1757921, r1757924
Folding StrictWhitespace into the Strict ruleset of RFC7230, per dev@ poll.
This choice is unanimous, although StrictURI (a different RFC) still hasn't
found absolute concensus.
Submitted by: wrowe
Backport: r1758226
Correct the parser construction for several optimizations,
based on the fact that bad whitespace shall not be permitted
or corrected in any operating mode, while preserving the
ability to extract bad method/uri/proto for later reporting
and diagnostics.
This change causes badwhitespace in the request line or any
request field line to always fail, and not honor the setting
of the HttpProtocolOptions Unsafe option. Mult SP characters
or trailing SP characters in the request line are still
permitted in Unsafe mode.
Adjusted several error message emits to match these changes.
Submitted by: wrowe
Backport: r1758263
Clarify documentation based on concensus decisions discussed on dev@
and reflecting the current implementation, clean up stray <p>
Submitted by: wrowe
Backport: r1758265, r1758266
New optional flag to enforce <CR><LF> line delimiters in ap_[r]getline,
created by overloading 'int fold' (1 or 0) as 'int flags', with the same
value 1 for AP_GETLINE_FOLD (which httpd doesn't use), and a new value
2 for AP_GETLINE_CRLF
Enforce CRLF when HttpProtocolOptions Strict is in force.
Correctly introduces a new t/TEST fail.
Submitted by: wrowe
Backport: r1758304
Calm some overly agressive crlf handling, and clarify
Submitted by: wrowe
Backport: r1758305, r1758313
Review of IE 11, Firefox 48 and Chrome 53 all indicate that ';' URI characters
are transmitted unencoded, per RFC3986 section 3.3 grammer. Correct httpd's
behavior to not encode ';' in proxied URI's or Location: response headers.
Submitted by: wrowe
Backport: r1760444
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict@1769669 13f79535-47bb-0310-9956-ffa450edef68
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
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict@1769664 13f79535-47bb-0310-9956-ffa450edef68
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
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict@1769662 13f79535-47bb-0310-9956-ffa450edef68
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
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict@1769649 13f79535-47bb-0310-9956-ffa450edef68
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
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict@1768036 13f79535-47bb-0310-9956-ffa450edef68
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
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x-merge-http-strict@1768035 13f79535-47bb-0310-9956-ffa450edef68
mod_remoteip: Use r->useragent_addr as the root trusted address for verifying.
This fixes issue resulting in setting of bad useragent_ip when internal
redirection has been generated as response to the request (typically as
result of "ErrorDocument 40x").
In this case, the original request has been handled by mod_remoteip and its
useragent_ip has been changed properly, but when internal redirection
to ErrorDocument has been generated later, the mod_remoteip's handler has been
executed again with *the same* c->client_addr as in the original request. If
c->client_addr IP is trusted, this results in bad useragent_ip being set.
When using r->useragent_addr as the root trusted address instead of
c->client_addr, the internal redirection uses the first non-trusted
IP in this particular case, so it won't change the r->useragent_ip during
the internal redirection to ErrorDocument.
Submitted by: jkaluza
Reviewed/backported by: jim
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1767483 13f79535-47bb-0310-9956-ffa450edef68
