Amos Jeffries [Sat, 6 Aug 2016 04:49:55 +0000 (16:49 +1200)]
Fix clang errors with global InstanceId initialization
Move static member Last into change() method to avoid initialization order
errors when a caller uses a global InstanceId object before the library
instantiating its template is initialized.
Also, convert the Prefix static member to a constant string in a prefix()
method to avoid the same issue there.
Many web servers do not have complete certificate chains. Many browsers use
certificate extensions of the server certificate and download the missing
intermediate certificates automatically from the Internet.
This patch add this feature to Squid.
The information for missing issuer certificates provided by the Authority
Information Access X509 extension. This describes the format and the location
of additional information provided by the issuer of the certificate.
This patch:
- Implements a class Downloader as an independet AsyncJob class. This new
class can be used by internal squid subsystems to download objects from
the network.
- Modify Ssl::PeerConnector class to use new Downloader class to
retrieve missing certificates from the net. The URIs of missing
certificates from the Authority Information Access X509 extension.
- Implements a new basic certificates parser based on openSSL for the
TLS handshake messages parser.
- Modify the Ssl::ServerBio class to:
* Buffer the Server Hello message and not pass it to the openSSL library
until downloading missing certificates, if any, is finished.
* Extract server certificates from server hello message.
This is required to check if there are missing certificates, and if yes
give the chance to squid to download missing certificates and complete
certificate chains before pass them for processing to openSSL
TODO:
- Add support for certs-only CMS message.
From RFC 4325:
"Where the information is available via HTTP or FTP, accessLocation
MUST be a uniformResourceIdentifier and the URI MUST point to either
a single DER encoded certificate as specified in [RFC2585] or a
collection of certificates in a BER or DER encoded "certs-only" CMS
message as specified in [RFC2797]. "
...
"Conforming applications that support HTTP or FTP for accessing
certificates MUST be able to accept individual DER encoded
certificates and SHOULD be able to accept "certs-only" CMS messages."
Squid segfault via Ftp::Client::readControlReply().
Ftp::Client::scheduleReadControlReply(), which may called from the
asynchronous start() or readControlReply()/handleControlReply()
handlers, does not check whether the control connection is still usable
before using it.
Certain server certificates with a large chain and/or large certificates
(i.e. due to excessive amount of SAN entries) are producing helper requests and
replies which are larger than the 32KB limit set in src/helper.cc.
The major problem for squid is that the helper response should fit in the
helper read buffer. Currently squid starts with 4K request buffer and if
required may increase the buffer size up to 32K.
This patch:
- Uses a constant-size read buffer for helpers and accumulates the helper
response to Helper::Reply object.
- Changes the HelperServerBase::requests list to hold list of the new
Helper::Xaction class which holds pairs of Helper::Request and
Helper::Reply objects
- Modifies the Helper::Reply class to accumulate data and restricts the
required memory allocations
... also address Bug 4311 and partially address Bug 2833 and Bug 4471.
Extend collapsed_forwarding functionality to internal revalidation
requests. This implementation does not support Vary-controlled cache
objects and is limited to SMP-unaware caching environments, where each
Squid worker knows nothing about requests and caches handled by other
workers. However, it also lays critical groundwork for future SMP-aware
collapsed revalidation support.
Prior to these changes, multiple concurrent HTTP requests for the same
stale cached object always resulted in multiple internal revalidation
requests sent by Squid to the origin server. Those internal requests
were likely to result in multiple competing Squid cache updates, causing
cache misses and/or more internal revalidation requests, negating
collapsed forwarding savings.
Internal cache revalidation requests are collapsed if and only if
collapsed_forwarding is enabled. There is no option to control just
revalidation collapsing because there is no known use case for it.
* Public revalidation keys
Each Store entry has a unique key. Keys are used to find entries in the
Store (both already cached/swapped_out entries and not). Public keys are
normally tied to the request method and target URI. Same request
properties normally lead to the same public key, making cache hits
possible. If we were to calculate a public key for an internal
revalidation request, it would have been the same as the public key of
the stale cache entry being revalidated. Adding a revalidation response
to Store would have purged that being-revalidated cached entry, even if
the revalidation response itself was not cachable.
To avoid purging being-revalidated cached entries, the old code used
private keys for internal revalidation requests. Private keys are always
unique and cannot accidentally purge a public entry. On the other hand,
for concurrent [revalidation] requests to find the store entry to
collapse on, that store entry has to have a public key!
We resolved this conflict by adding "scope" to public keys:
* Regular/old public keys have default empty scope (that does not affect
key generation). The code not dealing with collapsed revalidation
continues to work as before. All entries stored in caches continue to
have the same keys (with an empty scope).
* When collapsed forwarding is enabled, collapsable internal
revalidation requests get public keys with a "revalidation" scope
(that is fed to the MD5 hash when the key is generated). Such a
revalidation request can find a matching store entry created by
another revalidation request (and collapse on it), but cannot clash
with the entry being revalidated (because that entry key is using a
different [empty] scope).
This change not only enables collapsing of internal revalidation
requests within one worker, but opens the way for SMP-aware workers to
share information about collapsed revalidation requests, similar to how
those workers already share information about being-swapped-out cache
entries.
After receiving the revalidation response, each collapsed revalidation
request may call updateOnNotModified() to update the stale entry [with
the same revalidation response!]. Concurrent entry updates would have
wasted many resources, especially for disk-cached entries that support
header updates, and may have purged being-revalidated entries due to
locking conflicts among updating transactions. To minimize these
problems, we adjusted header and entry metadata updating logic to skip
the update if nothing have changed since the last update.
Also fixed Bug 4311: Collapsed forwarding deadlocks for SMP Squids using
SMP-unaware caches. Collapsed transactions stalled without getting a
response because they were waiting for the shared "transients" table
updates. The table was created by Store::Controller but then abandoned (not
updated) by SMP-unaware caches. Now, the transients table is not created
at all unless SMP-aware caches are present. This fix should also address
complaints about shared memory being used for Squid instances without
SMP-aware caches.
A combination of SMP-aware and SMP-unaware caches is still not supported
and is likely to stall collapsed transactions if they are enabled. Note
that, by default, the memory cache is SMP-aware in SMP environments.
There are situations when Squid logs nothing to access.log after an
[abnormal] transaction termination. Such "stealthy" transactions may be
a security risk and an accounting problem.
ClientHttpRequest is responsible for logging most transactions but that
object is created only after the HTTP request headers are successfully
parsed. Request header parsing errors may be detected and logged
appropriately, but the job handling the incoming transaction may
terminate for reasons outside the parsing code control (e.g., a job-
killing exception thrown when there are no request headers to start
parsing yet or when the job waits for more request headers to finishing
parsing).
This change adds access logging for three cases:
1. accept(2) system call errors (before ConnStateData job is created).
2. Unexpected ConnStateData job termination, when there is no
ClientHttpRequest to log the failure.
3. Connections which send no bytes: these connections drain Squid
resources and, hence, should be logged.
TODO: make this behavior configurable because some browsers are known to
routinely create such connections(and, hence, logging them may create
too much noise in some environments).
* rename initializeSsl() method to initializeTls()
* use SessionPointer instead of SessionPtr
* return value is best used in boolean expressions, so make it bool
this also resolves some template issues with SessionPointer return type.
* rename the session parameter to serverSession which better documents what
it is than 'ssl', and distinguishes it from clientSession in child methods
which have to deal with both.
- Add/fix basic documentation for Downloader and DownloaderContext classes
- Move DownloaderContext class definition to Downloader.cc file
- fix cbdata leaks inside DownloaderCotnext destructor, caused by simple typo
mistake.
- Do not cbdatalock the ClientHttpRequest object inside DownloaderContext.
This class is responsible to hold the main pointer and finaly delete the
ClientHttpRequest object.
LockingPointer is now a stand-alone class that is understood (and
documented) as a typical shared pointer with OpenSSL-friendly object
importing methods. Replaced its TidyPointer::reset() abuse with an
explicit resetWithoutLocking() method after reviewing that all such
calls needed no locking indeed.
Patrick Welche [Wed, 29 Jun 2016 17:24:24 +0000 (11:24 -0600)]
Bug 4376: clang cannot build Squid eCAP code
This change is not needed for libecap v1.0 that uses tr1::shared_ptr.
However, libecap is being updated to use std::shared_ptr. Once that
update is complete, Squid eCAP code will no longer compile because the
implicit conversion of shared_ptr to bool in tr1 was deemed dangerous
and, hence, made "explicit" when shared_ptr became standard. Unlike "if"
statements, "return" statements do not trigger an explicit conversion.
HTTP/1.1: Update all stored headers on 304 revalidation.
According to RFC 7234 section 4.3.4. when a proxy receives 304 (Not Modified)
response, it should update every header, except Warning headers (which need
special processing). Also RFC 7232 section 4.1. does not prohibit origin servers
from sending any headers in 304 responses. That means that Squid now should not
ignore headers, which earlier were considered as malicious/faulty.
Nathan Hoad [Fri, 24 Jun 2016 05:29:44 +0000 (17:29 +1200)]
Convert Http::Stream::reqbuf to a MemBlob, making it configurable at runtime.
This also makes many other auxilary changes:
* Increases the size of Http::Stream::requestBuffer to match that of
read_ahead_gap. Previously this was a 4kb fixed size buffer. As a result,
the overhead for a single client connection has been increased by 12 KB in
the default configuration, but Squid will no longer artifically slow down
client responses in this situation by fragmenting the read(2)/write(2)
calls.
* Improves the performance of large uncacheable replies. This was achieved by
increasing the buffer size to 16 KB as mentioned above, but it is worth
mentioning separately. Specifically, for a server, client and proxy all
running on my local machine, this patch increases throughput on a 5 GB file
from ~110 MB/s to ~340 MB/s.
* Documents the influence that read_ahead_gap had on the size of read(2) calls
for HTTP, and now the size of write(2) calls.
* Prevent read_ahead_gap from being set to 0. Previously this would result in
hung requests.
This work is submitted on behalf of Bloomberg L.P.
The Ftp::Server::stopWaitingForOrigin() notification may come after
Ftp::Server (or an external force) has started closing the control
connection but before the Ftp::Server job became unreachable for
notifications. Writing a response in this state leads to assertions.
Other, currently unknown paths may lead to the same write-after-close
problems. This change protects all asynchronous notification methods
(except the connection closure handler itself) from exposing underlying
code to a closing control connection. This is very similar to checking
for ERR_CLOSING in Comm handlers.
Alex Rousskov [Wed, 15 Jun 2016 15:37:44 +0000 (09:37 -0600)]
Do not make bogus recvmsg(2) calls when closing UDS sockets.
comm_empty_os_read_buffers() assumes that all non-blocking
FD_READ_METHODs can read into an opaque buffer filled with random
characters. That assumption is wrong for UDS sockets that require an
initialized msghdr structure. Feeding random data to recvmsg(2) leads to
confusing errors, at best. Squid does not log those errors, but they
are visible in, for example, strace:
recvmsg(17, 0x7fffbb, MSG_DONTWAIT) = -1 EMSGSIZE (Message too long)
comm_empty_os_read_buffers() is meant to prevent TCP RST packets. The
function now ignores UDS sockets that are not used for TCP.
TODO: Useless reads may also exist for UDP and some TCP sockets.
Alex Rousskov [Tue, 14 Jun 2016 16:54:23 +0000 (04:54 +1200)]
Fixed Server::maybeMakeSpaceAvailable() logic.
This change fixes logic bugs that mostly affect performance: In micro-
tests, this change gives 10% performance improvement for intercepted
"fast peek at SNI and splice" SslBump configurations. Similar
improvement is expected for future plain HTTP/2 parsers.
maybeMakeSpaceAvailable() is called with an essentially random inBuf.
The method must prepare inBuf for the next network read. The old code
was not doing that [well enough], leading to performance problems.
In some environments, inBuf often ends up having tiny space exceeding 2
bytes (e.g., 6 bytes). This happens, for example, when Squid creates and
parses a fake CONNECT request. The old code often left such tiny inBufs
"as is" because we tried to ensure that we have at least 2 bytes to read
instead of trying to provide a reasonable number of buffer space for the
next network read. Tiny buffers naturally result in tiny network reads,
which are very inefficient, especially for non-incremental parsers.
I have removed the explicit "2 byte" space checks: Both the new and the
old code do not _guarantee_ that at least 2 bytes of buffer space are
always available, and the caller does not check that condition either.
If some other code relies on it, more fixes will be needed (but this
change is not breaking that guarantee -- either it was broken earlier or
was never fully enforced). In practice, only buffers approaching
Config.maxRequestBufferSize limit may violate this guarantee AFAICT, and
those buffers ought to be rare, so the bug, if any, remains unnoticed.
Another subtle maybeMakeSpaceAvailable() problem was that the code
contained its own buffer capacity increase algorithm (n^2 growth).
However, increasing buffer capacity exponentially does not make much
sense because network read sizes are not going to increase
exponentially. Also, memAllocStringmemAllocate() overwrites n^2 growth
with its own logic. Besides, it is buffer _space_, not the total
capacity that should be increased. More work is needed to better match
Squid buffer size for from-user network reads with the TCP stack buffers
and traffic patterns.
Both the old and the new code reallocate inBuf MemBlobs. However, the
new code leaves "reallocate or memmove" decision to the new
SBuf::reserve(), opening the possibility for future memmove
optimizations that SBuf/MemBlob do not currently support.
It is probably wrong that inBuf points to an essentially random MemBlob
outside Server control but this change does not attempt to fix that.
This patch add support for mimicking TLS Authority Key Identifier certificate
extension in Squid generated TLS certificates: If the origin server certificate
has that extension, the generated certificate (via the ssl_crtd daemon or
internally) should have the same extension, with the same set of fields if
possible.
projects / thirdparty / squid.git / log
