Jacob Champion [Mon, 29 Aug 2016 23:56:21 +0000 (23:56 +0000)]
mpm_winnt: clear OVERLAPPED structs before reuse
MSDN documentation states that
Any unused members of [an OVERLAPPED] structure should always be
initialized to zero before the structure is used in a function call.
Otherwise, the function may fail and return ERROR_INVALID_PARAMETER.
Prior to this patch, the internal state left over from previous
overlapped I/O was passed into the next call. It's unclear what effect
this might have, if any. (I have not personally witnessed an
ERROR_INVALID_PARAMETER myself.)
Jacob Champion [Mon, 29 Aug 2016 23:56:20 +0000 (23:56 +0000)]
mpm_winnt: remove duplication of ap_process_connection
Further follow-up to the previous commit: now that we no longer patch a
network bucket into the brigade, we can revert to calling
ap_process_connection() directly instead of duplicating its logic.
Jacob Champion [Mon, 29 Aug 2016 23:56:18 +0000 (23:56 +0000)]
mpm_winnt: remove the AcceptEx data network bucket
Follow-up to the prior commit: without an incoming data buffer, the
custom network bucket code is now orphaned and we can remove it
entirely. This has the added benefit that we are no longer using the
internal OVERLAPPED.Pointer field, which is discouraged by the MSDN
docs.
Jacob Champion [Mon, 29 Aug 2016 23:56:16 +0000 (23:56 +0000)]
mpm_winnt: remove 'data' AcceptFilter in favor of 'connect'
The 'data' AcceptFilter optimization instructs Windows to wait until
data is received on a connection before completing the AcceptEx
operation. Unfortunately, it seems this isn't performed atomically --
AcceptEx "partially" accepts the incoming connection during the wait for
data, leaving all other incoming connections in the accept queue. This
opens the server to a denial of service.
Since the fix for this requires a substantial rearchitecture (likely
involving multiple outstanding calls to AcceptEx), disable the 'data'
filter for now and replace it with 'connect', which uses the AcceptEx
interface but does not wait for data.
Users running prior releases of httpd on Windows should explicitly move
to a 'connect' AcceptFilter in their configurations if they are
currently using the default 'data' filter.
Many thanks to mludha, Arthur Ramsey, Paul Spangler, and many others for
their assistance in tracking down and diagnosing this issue.
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.
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.
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.
An ap_mmn bump will follow (major, this removes a struct elt)
Eric Covener [Sat, 27 Aug 2016 23:15:11 +0000 (23:15 +0000)]
Stash the cgi PID earlier in mod_cgid
In some cases, a 2nd CGI using the same c->id can get into
the mod_cgid handler before cleanups have been run, causing
the new CGI pid to be used by the first CGI's cleanup function.
Instead of stashing c->id in the request processing thread,
just use it before leaving the handler to get the pid.
May indirectly fix PR57771, but it must have a slightly different
cause because stashing the conn_id slightly differently was
supposed to be sufficient there.
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.
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.
Luca Toscano [Fri, 26 Aug 2016 11:00:44 +0000 (11:00 +0000)]
After a long discussion in dev@ I reviewed my previous commit to only warn
the admins about Last-Modified header violations rather than trying
to interpret datestrings (like the ones not in GMT).
I also added explicit comments to summarize the current assumptions,
so it will be easier for somebody in the future to modify the code.
The following use cases are covered:
1) (F)CGI backend sends a Last-Modified header not in GMT and considered in the future by httpd (like now() in the EU/Paris timezone)
2) (F)CGI backend sends a Last-Modified header not in GMT and not considered in the future by httpd (like now() + 2 hours in the PST timezone)
3) (F)CGI backend sends a Last-Modified header in GMT but with a datetime in the future
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.
Jacob Champion [Mon, 22 Aug 2016 21:27:18 +0000 (21:27 +0000)]
docs: update the "SSL Strong Encryption" how-to
The how-to was a little behind the times. Update to modern ciphersuite
selections, and teach the reader more about *why* certain selections and
settings are chosen. Try to future-proof a little bit by including the
"last-reviewed" date and pointing to Mozilla's recommendation tool.
Joe Orton [Mon, 22 Aug 2016 11:10:58 +0000 (11:10 +0000)]
* modules/ssl/ssl_engine_kernel.c (ssl_callback_SessionTicket): Fail
if RAND_bytes() fails; possible per API, although not in practice
with the OpenSSL implementation.
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.
Stefan Fritsch [Sat, 20 Aug 2016 20:48:13 +0000 (20:48 +0000)]
mpm_event: don't re-use scoreboard slots that are still in use
This causes inconsistent data in the scoreboard (due to async
connections) and makes it difficult to determine what is going on.
Therefore it is not a useful fix for the scoreboard-full issues (PR
53555).
The consent on the dev list is that we should allocate/use more
scoreboard entries instead.
Stefan Fritsch [Sat, 20 Aug 2016 20:21:39 +0000 (20:21 +0000)]
mod_status: note stopping procs in async info table
* add new column "stopping", denoting if a process is shutting down
* add additional "(old gen)", if a process is from before a graceful reload
* add counts of processes and stopping processes to summary line
Delay some memory allocation in order to save 272 bytes in the 'request' memory pool if mod_status handler is triggered and is not able to handle the request
Fix the number of column for 'Async connections'.
There are only 3 columns (writing, keep-alive, closing), not 4.
Try to improve the code layout for it to be more readable.
Each <th> is on its own line so keep the corresponding "colspan" <td> fields grouped together.
r1738628 introduced a new column, 'Slot'.
Add an empty cell for it in the last line of the table, in order to fix the layout of the Totals.
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.
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.
projects / apache / log