Luca Toscano [Tue, 19 Jun 2018 22:28:24 +0000 (22:28 +0000)]
mod_ratelimit: fix behavior with proxied content
mod_ratelimit works by splitting data in "chunks"
to send to the client, sleeping a predefined amount
of time between them (200ms). So for example,
a rate-limit 40 value would correspond to a chunk size
of 8192 bytes, flushed to the client every 200ms.
The idea works fine when httpd directly serves the
content, since the filter will be called once with
a single bucket brigade. In the context of a proxied
content though the filter is likely to be called multiple
times, with a bucket brigade size that corresponds to
the maximum allowed buffer size. If this value is lower
or higher than the chunk size, the filter will not
properly rate limit the data going to the client.
This patch solves the problem with two fix:
1) do_sleep is now stored in the ctx context struct,
so if the filter is invoked multiple times it
will still sleep when needed. For example, say
that the chunk_size is 8192 and the bucket brigate
len is 10240: the filter will flush 8192 bytes
on the first invocation, sleep 200ms, flush the
remaining bytes and then finish. The next invocation
will do the same, clearly not leading to the
correct "sleeping pattern".
2) The example above highlights also another issue:
mod_ratelimit should flush only chunk_size bytes
at the time (I am now excluding the burst calculation
from the picture), and buffer between invocations
unless the brigade contains EOS.
The change has been tested with various scenarios and
it looks working as expected, but of course more
feedback/testing is welcome.
The original patch was written by me and then Yann
refactored the code to be more precise and efficient,
basically transforming an axe in a wonderful Japanese
katana sword, so credits to him for this work.
Eric Covener [Tue, 19 Jun 2018 12:07:19 +0000 (12:07 +0000)]
add server_rec to log.c fatal startup errors
Not strictly necessary for trunk, but in 2.4.x if the main ErrorLog is
using syslog, these messages are lost. In trunk, the low-level logging
routines reach up and find the syslog provider when no server_rec is provided
but that backport is stalled.
Yann Ylavic [Fri, 15 Jun 2018 14:35:31 +0000 (14:35 +0000)]
mod_ssl: build with LibreSSL.
LibreSSL seems to be openssl-1.1 API compatible only in version 2.8 (master).
So use that for MODSSL_USE_OPENSSL_PRE_1_1_API instead of 2.7, the two 2.7
compatibility-exceptions are handled explicitely but overall it's simpler.
Regarding CRYPTO_malloc_init vs OPENSSL_malloc_init, libreSSL uses none, the
former used to be a no-op but depends is LIBRESSL_INTERNAL in latest versions,
while the latter has never been (and will never be) defined. So don't call any
with LibreSSL.
Yann Ylavic [Fri, 15 Jun 2018 11:12:19 +0000 (11:12 +0000)]
mod_ssl: disable check for client initiated renegotiations with TLS 1.3.
This is already forbidden by the protocol, enforced by OpenSSL, and the
current logic can't work (ssl_callback_Info() may be called multiple times
with TLS 1.3).
Yann Ylavic [Wed, 13 Jun 2018 09:54:16 +0000 (09:54 +0000)]
Follow up to r1833368: share openssl between modules.
Both libapr[-util], the core PRNG, mod_ssl, mod_crypto and mod_session_crypto
can use the same crypto library (e.g. openssl), use the new APR crypto loading
API so that they can work together and initialize/terminate the lib either once
for all or on demand and reusable by the others.
apr_pcalloc can be turned into apr_palloc (the allocated memory is fully initialized by the subsequent memcpy/strcpy) and '(int)strlen(p)' can be replaced by 'plen - 1' to save some cycles.
Joe Orton [Thu, 7 Jun 2018 13:17:27 +0000 (13:17 +0000)]
* configure.in, Makefile.in: Handle no-test-suite case through
check-no rule. Only regenerate the test suite on repeated
"make check" run if a header file has changed.
Yann Ylavic [Wed, 6 Jun 2018 21:04:21 +0000 (21:04 +0000)]
Avoid cyclic dependency by moving ap_set_etag() from module http to core.
This function, along with ap_make_etag(), is used by the default_handler in
core.c, and in several modules other than builtin mod_http, breaking static
linking and httpdunit tests build.
The move is done by "svn move modules/http/http_etag.c server/util_etag.c".
MMN major bumped, not backportable (as is) to 2.4.x.
Joe Orton [Wed, 6 Jun 2018 11:39:33 +0000 (11:39 +0000)]
* modules/http/http_request.c (ap_process_request_after_handler,
ap_process_request): Cache and retrieve the brigade structure used
to send EOR and FLUSH between requests in c->pool userdata, to avoid
allocating a brigade structure per-request out of c->pool.
Stefan Eissing [Wed, 6 Jun 2018 09:56:00 +0000 (09:56 +0000)]
mod_ssl: after code review, changed:
* eliminated SSLPolicyRec as name no longer used
* eliminated some left over parameters in internal functions due to policy def removal
* reverted a NULL test, necessary before
If several directories are given in a UserDir directive, only files in the first existing one are checked. If the file is not found there, the other possible directories are not checked. The doc clearly states that they will be checked one by one, until a match is found or an external redirect is performed.
PR 59636.
While at it, add some debug messages to better understand what is performed.
mod_proxy_hcheck: add some hyperlinks, improve syntax highlight, add some missing trailing dot, be more consistent with other modules in the way 'Syntax' lines are displayed
In 'ap_proxy_cookie_reverse_map', iterate over each token of the 'Set-Cookie' header field in order to avoid updating the wrong one.
This could happen if the header field has something like 'fakepath=foo;path=bar". In this case fakepath would be updated instead of path.
We don't need regex anymore in order to parse the field values and 'ap_proxy_strmatch_domain' and 'ap_proxy_strmatch_path' are now useless. (and should be axed IMHO)
Luca Toscano [Wed, 23 May 2018 11:22:34 +0000 (11:22 +0000)]
http_protocol.c: avoid duplicate headers when using
ap_send_error_response
While debugging PR 61860 I found a chain of events
that leads to duplicate headers in the HTTP response
if Header always set is used in the httpd's config.
As far as can understand, mod_headers uses err_headers_out
when dealing with the 'always' setting, since it is expected
to be preserved even on error. The current code seems
to correctly preserve the important headers like Location
(happening before the bit of code that changed), but then
it swaps headers_out with err_headers_out and clears
err_headers_out. My understanding is that this will cause
mod_headers, if called again, to re-insert the headers
set via 'always' again in err_headers_out, leading to a
duplication in the response. So far I managed to reproduce this
only with the specific use case outlined by the PR, but
there might be more.
r1831585 was added to the test suite for PR 61860,
(marked as TODO), and now it seems to pass as expected.
Since this part of the codebase has been working fine
for years I was reluctant to change it, but it seems
the right change to me. I didn't run into any regression
while testing this change (including running the test suite),
but as always I'd be glad to get feedback from a more expert
eye. If I missed a clear regression I'll make sure to
add it to the test suite as part of the follow up.
Joe Orton [Tue, 22 May 2018 08:32:32 +0000 (08:32 +0000)]
Fix "make check" to be able to find ab, creating a "bin" inside the
dummy install root at $builddir/check.
* Makefile.in (check-binaries): New target.
(check/bin/apxs): Moved target inside check/bin.
(check/build/config_vars.mk): Override bindir as well.
(check): Use apxs at new location.
Yann Ylavic [Sun, 20 May 2018 13:08:13 +0000 (13:08 +0000)]
mod_proxy_balancer: clear slotmem slots' inuse flags from persisted files.
Otherwise, when BalancerPersist is enabled, we might fail (re)starting with a
saved slotmem if a BalancerMember was renamed (e.g. PR 62308's port change).
This is because the renamed member is considered a new one, while the old name
still reserves a slot, so we end up missing one room. The overall number of
members does not change so the sizes check succeeds thus the persisted slotmem
is not invalidated. Yet the slomem is not really compatible.
By clearing inuse flags, we still allow for slots to be reused if their index
did not change (thanks to ap_proxy_find_balancershm() and slotmem_fgrab() at
the given index), but will also invalidate renamed slots which don't match the
index. We now have the correct behaviour by slot and the server (re)starts in
any case.
Yann Ylavic [Fri, 18 May 2018 17:05:18 +0000 (17:05 +0000)]
mod_slotmem_shm: follow up to r1831869 (check persistent files).
Since persistent files are also reused on stop/start, we must ensure that
they match the same descriptor when reused on the next startup, so add it
to integrity metadata.
Also, the descriptor being the first field in the SHM, we don't need to
copy on the stack it in several places, and can handle it as a pointer.
Yann Ylavic [Fri, 18 May 2018 16:48:34 +0000 (16:48 +0000)]
mod_slotmem_shm: follow up to r1831869.
Don't try to attach SHMs on startup, they might be from a previous crash and
prevent the server to start (on failure to reuse it).
On mpm_winnt though, the pre/post_config() phases are re-run in child process,
and the parent process already created the SMs, so they must be attached there.
Yann Ylavic [Fri, 18 May 2018 16:33:28 +0000 (16:33 +0000)]
mod_slotmem_shm: use a generation number for SHM filename on all platforms.
Successive generations can't share the same SHMs because restarts may modify
them under the terminating children, while SHMs are not extensible when all
slots are in use.
This effectively restores r1822341 which was reverted by r1822505.
Yann Ylavic [Fri, 11 May 2018 09:44:38 +0000 (09:44 +0000)]
Revert r1831218, the API garantees slotmem_attach() is called in child_init().
r1831394 is the right follow to r1830800 to preserve "inherited" slotmems in
children processes.
While at it, comment on the expectations from mod_proxy_balancer w.r.t.
slotmem_attach() implementations, and eventually how we could improve the API
later (w/o backporting to 2.4.x).
Yann Ylavic [Wed, 9 May 2018 01:23:59 +0000 (01:23 +0000)]
mod_proxy_balancer: follow up to r1830800.
Don't call slotmem_attach() if the slot is already initialized/reused, that
used to work previously because the returned error status is ignored, but
r1830800 changed the function to forcibly reset the returned slot pointer to
NULL first.
There is no point to call slotmem_attach() in this case, the slot is already
initialized because it's either inherited/fork()ed on Unixes, or on Windows
the attachment happened in post_config/slotmem_create() already.
One case where slotmem_attach() would fail is when balancers are defined
globally (main server) and some virtualhosts don't have/use any mod_proxy
configuration/directive. In this case their proxy server config is a pointer
copy of the main server thus their slots are already initialized in both
post_config and child_init hooks. While the case was already handled in
the balancer_post_config(), it did not in balancer_child_init() where we
relied on slotmem_attach() to be a noop.
Use apr_pstrmemdup instead of apr_pstrndup when possible.
Avoid scanning the first 2 bytes when looking for the | delimiter. it is known to be "${".
Avoid comma separated statements, it is not that usual.
Joe Orton [Fri, 4 May 2018 17:56:32 +0000 (17:56 +0000)]
Simplify the ssl_asn1_table API, remove abstraction (it is used only
to cache serialized EVP_PKEYs not any char * blobs), and document.
* modules/ssl/ssl_util.c (ssl_asn1_table_set): Take the EVP_PKEY and
serialize internally. Use ap_realloc. Return the ssl_asn1_t *
pointer. Don't call apr_hash_set() for unchanged pointer case.
* modules/ssl/ssl_engine_pphrase.c (ssl_load_encrypted_pkey):
Adjust for the above.
* modules/ssl/ssl_private.h: Adjust as above, add docs.
Joe Orton [Thu, 3 May 2018 13:06:46 +0000 (13:06 +0000)]
mod_ssl: Add support for loading private keys from ENGINEs. Support
for PKCS#11 URIs only, and PIN entry is not threaded through
SSLPassPhraseDialog config yet.
* modules/ssl/ssl_util.c (modssl_is_engine_key): New function.
* modules/ssl/ssl_engine_config.c (ssl_cmd_SSLCertificateKeyFile):
Use it, skip check for file existence for engine keys.
* modules/ssl/ssl_engine_pphrase.c (modssl_load_engine_pkey):
New function.
* modules/ssl/ssl_engine_init.c (ssl_init_server_certs):
For engine keys, load via modssl_load_engine_pkey.
Submitted by: Anderson Sasaki <ansasaki redhat.com>, jorton
Yann Ylavic [Thu, 3 May 2018 08:32:42 +0000 (08:32 +0000)]
mod_slomem_shm: Handle a generation number when the slotmem size changes.
Modifying the number of proxy balancers or balancer members on restart
could have prevented the server to load, notably on Windows. PR 62308.
The generation number integrated in the SHM filename allows to create a
new/resized SHM while the previous is still in use by previous generation
gracefully shutting down (Windows prevents SHM/file to be removed in this
case, but even on Unix(es) an unlinked file might not be re-openable while
an inode exists). The generation number is added/incremented only if the
size requirement changed, such that unrelated restarts continue to share
SHMs between generations.
The cleanup handling is also simplified because both the parent process and
the Windows child process need to cleanup everything on exit. This translates
to cleanup_slotmem() being always registered but in the dry load state
(AP_SQ_MS_CREATE_PRE_CONFIG), for both cases still.