Willy Tarreau [Mon, 25 Jan 2016 00:09:11 +0000 (01:09 +0100)]
BUG/MEDIUM: channel: fix miscalculation of available buffer space.
The function channel_recv_limit() relies on channel_reserved() which
itself relies on channel_in_transit(). Individually they're OK but
combined they're doing the wrong thing.
The problem is that we refrain from filling buffers while to_forward
is even much larger than the buffer because of a semantic issue along
the call chain. This is particularly visible when offloading SSL on
moderately large files (1 MB), though it is also visible on clear text.
Twice the number of recv() calls are made compared to what is needed,
and the typical performance drops by 15-20% in SSL in 1.6 and later,
and no directly measurable drop in 1.5 except when using strace.
There's no need for all these intermediate functions, so let's get
rid of them and reimplement channel_recv_limit() from scratch in a
safer way.
This fix needs to be backported to 1.6 and 1.5 (at least). Note that in
1.5 the function is called buffer_recv_limit() and it may differ a bit.
BUG/MEDIUM: sample: http_date() doesn't provide the right day of the week
Gregor KovaÄ\8d reported that http_date() did not return the right day of the
week. For example "Sat, 22 Jan 2016 17:43:38 GMT" instead of "Fri, 22 Jan
2016 17:43:38 GMT". Indeed, gmtime() returns a 'struct tm' result, where
tm_wday begins on Sunday, whereas the code assumed it began on Monday.
This patch must be backported to haproxy 1.5 and 1.6.
Baptiste Assmann [Wed, 20 Jan 2016 23:17:09 +0000 (00:17 +0100)]
BUG/MAJOR: servers state: server port is erased when dns resolution is enabled on a server
Servers state function save and apply server IP when DNS resolution is
enabled on a server.
Purpose is to prevent switching traffic from one server to an other one
when multiple IPs are returned by the DNS server for the A or AAAA
record.
That said, a bug in current code lead to erase the service port while
copying the IP found in the file into the server structure in HAProxy's
memory.
This patch fix this bug.
The bug was reported on the ML by Robert Samuel Newson and fix proposed
by Nenad Merdanovic.
Thank you both!!!
Baptiste Assmann [Wed, 20 Jan 2016 23:59:46 +0000 (00:59 +0100)]
BUG/MEDIUM: dns: no DNS resolution happens if no ports provided to the nameserver
Erez reported a bug on discourse.haproxy.org about DNS resolution not
occuring when no port is specified on the nameserver directive.
This patch prevent this behavior by returning an error explaining this
issue when parsing the configuration file.
That said, later, we may want to force port 53 when client did not
provide any.
CLEANUP: 51d: Aligned if statements with HAProxy best practices and removed casts from malloc.
Changes to if statements do not affect code operation, just layout of
the code. Type casts from malloc returns have been removed as this cast
happens automatically from the void* type.
BUG/MINOR: 51d: Aligns Pattern cache implementation with HAProxy best practices.
Malloc continues to be used for the creation of cache entries. The
implementation has been enhanced ready for production deployment. A new
method to free cache entries created in 51d.c has been added to ensure
memory is released correctly.
MINOR: lru: new function to delete <nb> least recently used keys
Introduction of a new function in the LRU cache source file.
Purpose of this function is to be used to delete a number of entries in
the cache. 'number' is defined by the caller and the key removed are
taken at the tail of the tree
Willy Tarreau [Fri, 8 Jan 2016 09:04:08 +0000 (10:04 +0100)]
MINOR: tools: make csv_enc_append() always start at the first byte of the chunk
csv_enc_append() returns a pointer to the beginning of the encoded
string, which makes it convenient to use in printf(). However it's not
convenient for use in chunks as it may leave an unused byte at the
beginning depending on the automatic quoting. Let's modify it to work
in two passes. First it looks for a character that requires escaping
using strpbrk(), and second it encodes the string. This way it
guarantees to always start at the first available byte of the chunk.
Additionally it made the code quite simpler.
Willy Tarreau [Wed, 6 Jan 2016 17:07:04 +0000 (18:07 +0100)]
MEDIUM: tools: add csv_enc_append() to preserve the original chunk
We have csv_enc() but there's no way to append some CSV-encoded data
to an existing chunk, so here we modify the existing function for this
and create an inlined version of csv_enc() which first resets the output
chunk. It will be handy to append data to an existing chunk without
having to use an extra temporary chunk, or to encode multiple strings
into a single chunk with chunk_newstr().
The patch is quite small, in fact most changes are typo fixes in the
comments.
Willy Tarreau [Wed, 6 Jan 2016 19:45:03 +0000 (20:45 +0100)]
MINOR: chunk: make chunk_initstr() take a const string
chunk_initstr() prepares a read-only chunk from a string of
fixed length. Thus it must be prepared to accept a read-only
string on the input, otherwise the caller has to force-cast
some const char* and that's not a good idea.
Willy Tarreau [Mon, 4 Jan 2016 19:13:55 +0000 (20:13 +0100)]
MINOR: chunks: add chunk_strcat() and chunk_newstr()
These two new functions will make it easier to manipulate small strings
from within functions, because at many places, multiple short strings
are needed which do not deserve a malloc() nor a free(), and alloca()
is often discouraged. Since we already have trash chunks, it's convenient
to be able to allocate substrings from a chunk and use them later since
our functions already perform all the length checks. chunk_newstr() adds
a trailing zero at the end of a chunk and returns the pointer to the next
character, which can be used as an independant string. chunk_strcat()
does what it says.
Willy Tarreau [Mon, 4 Jan 2016 19:21:33 +0000 (20:21 +0100)]
MINOR: chunks: ensure that chunk_strcpy() adds a trailing zero
Since thus function bears the name of a well-known string function, it
must at least promise compatible semantics. Here it means always adding
the trailing zero so that anyone willing to use chunk->str as a regular
string can do it. Of course the zero is not counted in the chunk's length.
Willy Tarreau [Mon, 4 Jan 2016 19:36:59 +0000 (20:36 +0100)]
BUG/MINOR: chunk: make chunk_dup() always check and set dst->size
chunk_dup() was affected by two bugs at once related to dst->size :
- first, it didn't check dst->size to know if it could free(dst->str),
so using it on a statically allocated chunk would cause a free(constant)
and crash the process ;
- second, it didn't properly set dst->size, possibly causing smaller
strings not to be properly reported in a chunk that was previously
used for something else.
Fortunately, neither of these situations ever happened since the function
is rarely used.
In the process of doing this, we even allocate one more byte for a
trailing zero if the input chunk was not full, so that the copied
string can safely be reused by standard string functions.
The bug was introduced in 1.3.4 nine years ago with this commit :
0f77253 ("[MINOR] store HTTP error messages into a chunk array")
It's better to backport this fix in case a future fix relies on it.
MINOR: filters/http: Use a wrapper function instead of stream_int_retnclose
The function http_reply_and_close has been added in proto_http.c to wrap calls
to stream_int_retnclose. This functions will be modified when the filters will
be added.
BUG/MINOR: http: Be sure to process all the data received from a server
When the response body is forwarded, if the server closes the input before the
end, an error is thrown. But if the data processing is too slow, all data could
already be received and pending in the input buffer. So this is a bug to stop
processing in this context. The server doesn't really closed the input before
the end.
As an example, this could happen when HAProxy is configured to do compression
offloading. If the server closes the connection explicitly after the response
(keep-alive disabled by the server) and if HAProxy receives the data faster than
they are compressed, then the response could be truncated.
This patch fixes the bug by checking if some pending data remain in the input
buffer before returning an error. If yes, the processing continues.
Willy Tarreau [Sun, 27 Dec 2015 13:51:01 +0000 (14:51 +0100)]
BUG/MINOR: http: fix several off-by-one errors in the url_param parser
Several cases of "<=" instead of "<" were found in the url_param parser,
mostly affecting the case where the parameter is wrapping. They shouldn't
affect header operations, just body parsing in a wrapped pipelined request.
The code is a bit complicated with certain operations done multiple times
in multiple functions, so it's not sure others are not left. This code
must be re-audited.
It should only be backported to 1.6 once carefully tested, because it is
possible that other bugs relied on these ones.
Thierry FOURNIER [Fri, 25 Dec 2015 00:33:18 +0000 (01:33 +0100)]
MINOR: lua: add set/get priv for applets
The applet can't have access to the session private data. This patch
fix this problem. Now an applet can use private data stored by actions
and fecthes.
Willy Tarreau [Sun, 20 Dec 2015 22:33:18 +0000 (23:33 +0100)]
[RELEASE] Released version 1.7-dev1
Released version 1.7-dev1 with the following main changes :
- DOC: specify that stats socket doc (section 9.2) is in management
- BUILD: install only relevant and existing documentation
- CLEANUP: don't ignore debian/ directory if present
- BUG/MINOR: dns: parsing error of some DNS response
- BUG/MEDIUM: namespaces: don't fail if no namespace is used
- BUG/MAJOR: ssl: free the generated SSL_CTX if the LRU cache is disabled
- MEDIUM: dns: Don't use the ANY query type
- BUILD: ssl: fix build error introduced in commit 7969a3 with OpenSSL < 1.0.0
- DOC: fix a typo for a "deviceatlas" keyword
- FIX: small typo in an example using the "Referer" header
- MINOR: cli: ability to set per-server maxconn
- DEBUG/MINOR: memory: add a build option to disable memory pools sharing
- DEBUG/MEDIUM: memory: optionally protect free data in pools
- DEBUG/MEDIUM: memory: add optional control pool memory operations
- MEDIUM: memory: add accounting for failed allocations
- BUG/MEDIUM: config: count memory limits on 64 bits, not 32
- BUG/MAJOR: dns: first DNS response packet not matching queried hostname may lead to a loop
- BUG/MINOR: dns: unable to parse CNAMEs response
- BUG/MINOR: examples/haproxy.init: missing brace in quiet_check()
- DOC: deviceatlas: more example use cases.
- MINOR: config: allow IPv6 bracketed literals
- BUG/BUILD: replace haproxy-systemd-wrapper with $(EXTRA) in install-bin.
- BUILD: add Haiku as supported target.
- BUG/MAJOR: http: don't requeue an idle connection that is already queued
- DOC: typo on capture.res.hdr and capture.req.hdr
- BUG/MINOR: dns: check for duplicate nameserver id in a resolvers section was missing
- CLEANUP: use direction names in place of numeric values
- BUG/MEDIUM: lua: sample fetches based on response doesn't work
- MINOR: check: add agent-send server parameter
- BUG/MINOR: http rule: http capture 'id' rule points to a non existing id
- BUG/MINOR: server: check return value of fgets() in apply_server_state()
- BUG/MINOR: acl: don't use record layer in req_ssl_ver
- BUILD: freebsd: double declaration
- BUG/MEDIUM: lua: clean output buffer
- BUILD: check for libressl to be able to build against it
- DOC: lua-api/index.rst small example fixes, spelling correction.
- DOC: lua: architecture and first steps
- DOC: relation between timeout http-request and option http-buffer-request
- BUILD: Make deviceatlas require PCRE
- BUG: http: do not abort keep-alive connections on server timeout
- BUG/MEDIUM: http: switch the request channel to no-delay once done.
- BUG/MINOR: lua: don't force-sslv3 LUA's SSL socket
- BUILD/MINOR: http: proto_http.h needs sample.h
- BUG/MEDIUM: http: don't enable auto-close on the response side
- BUG/MEDIUM: stream: fix half-closed timeout handling
- CLEANUP: compression: don't allocate DEFAULT_MAXZLIBMEM without USE_ZLIB
- BUG/MEDIUM: cli: changing compression rate-limiting must require admin level
- BUG/MEDIUM: sample: urlp can't match an empty value
- BUILD: dumpstats: silencing warning for printf format specifier / time_t
- CLEANUP: proxy: calloc call inverted arguments
- MINOR: da: silent logging by default and displaying DeviceAtlas support if built.
- BUG/MEDIUM: da: stop DeviceAtlas processing in the convertor if there is no input.
- DOC: Edited 51Degrees section of README/
- BUG/MEDIUM: checks: email-alert not working when declared in defaults
- BUG/MINOR: checks: email-alert causes a segfault when an unknown mailers section is configured
- BUG/MINOR: checks: typo in an email-alert error message
- BUG/MINOR: tcpcheck: conf parsing error when no port configured on server and last rule is a CONNECT with no port
- BUG/MINOR: tcpcheck: conf parsing error when no port configured on server and first rule(s) is (are) COMMENT
- BUG/MEDIUM: http: fix http-reuse when frontend and backend differ
- DOC: prefer using http-request/response over reqXXX/rspXXX directives
- CLEANUP: haproxy: using _GNU_SOURCE instead of __USE_GNU macro.
- MINOR: ssl: Added cert_key_and_chain struct
- MEDIUM: ssl: Added support for creating SSL_CTX with multiple certs
- MINOR: ssl: Added multi cert support for crt-list config keyword
- MEDIUM: ssl: Added multi cert support for loading crt directories
- MEDIUM: ssl: Added support for Multi-Cert OCSP Stapling
- BUILD: ssl: set SSL_SOCK_NUM_KEYTYPES with openssl < 1.0.2
- MINOR: config: make tune.recv_enough configurable
- BUG/MEDIUM: config: properly adjust maxconn with nbproc when memmax is forced
- DOC: ssl: Adding docs for Multi-Cert bundling
- BUG/MEDIUM: peers: table entries learned from a remote are pushed to others after a random delay.
- BUG/MEDIUM: peers: old stick table updates could be repushed.
- MINOR: lua: service/applet can have access to the HTTP headers when a POST is received
- REORG/MINOR: lua: convert boolean "int" to bitfield
- BUG/MEDIUM: lua: Lua applets must not fetch samples using http_txn
- BUG/MINOR: lua: Lua applets must not use http_txn
- BUG/MEDIUM: lua: Forbid HTTP applets from being called from tcp rulesets
- BUG/MAJOR: lua: Do not force the HTTP analysers in use-services
- CLEANUP: lua: bad error messages
- CONTRIB: initiate a debugging suite to make debugging easier
Willy Tarreau [Sun, 20 Dec 2015 22:21:57 +0000 (23:21 +0100)]
CONTRIB: initiate a debugging suite to make debugging easier
The goal is to have a collection of quick-n-dirty utilities that make
debugging easier and that can easily be modified when needed. The first
utility in this series is called "flags". For a given numeric argument,
it reports the various known combinations of flags for channels, streams
and so on. This way it's easy to copy-paste values from the CLI or from
gdb and immediately know what state a stream-interface or connection is
in.
Thierry FOURNIER [Sun, 20 Dec 2015 18:14:35 +0000 (19:14 +0100)]
BUG/MAJOR: lua: Do not force the HTTP analysers in use-services
INNER and XFERBODY analyzer were set in order to support HTTP applets
from TCP rulesets, but this does not work (cf previous patch).
Other cases already provides theses analyzers, so their addition is
not needed. Furthermore if INNER was set it could cause some headers
to be rewritten (ex: connection) after headers were already forwarded,
resulting in a crash in buffer_insert_line2().
Special thanks to Bernd Helm for providing very detailed information,
captures and stack traces making it possible to spot the root cause
here.
Thierry FOURNIER [Sun, 20 Dec 2015 19:13:14 +0000 (20:13 +0100)]
BUG/MEDIUM: lua: Forbid HTTP applets from being called from tcp rulesets
HTTP applets request requires everything initilized by
"http_process_request" (analyzer flag AN_REQ_HTTP_INNER).
The applet will be immediately initilized, but its before
the call of this analyzer.
Due to this problem HTTP applets could be called with uncompletely
initialized http_txn.
Thierry FOURNIER [Sun, 20 Dec 2015 18:14:52 +0000 (19:14 +0100)]
BUG/MINOR: lua: Lua applets must not use http_txn
In certain circumstances (eg: Lua HTTP applet called from a
TCP ruleset before http_process_request()), the HTTP TXN is not
yet fully initialized so some information it contains cannot be
relied on. Such information include the HTTP version, the state
of the expect: 100-continue header, the connection header and
the transfer-encoding header.
Here the bug only turns something which already doesn't work
into something wrong, but better avoid any references to the
http_txn from the Lua code to avoid future mistakes.
This patch should be backported into 1.6 for code consistency.
Thierry FOURNIER [Sun, 20 Dec 2015 17:43:03 +0000 (18:43 +0100)]
BUG/MEDIUM: lua: Lua applets must not fetch samples using http_txn
If a sample fetch needing http_txn is called from an HTTP Lua applet,
the result will be invalid and may even cause a crash because some HTTP
data can be forwarded and the HTTP txn is no longer valid.
Here the solution is to ensure that a fetch called from Lua never
needs http_txn. This is done thanks to a new flag HLUA_F_MAY_USE_HTTP
which indicates whether or not it is safe to call a fetch which needs
HTTP.
Thierry FOURNIER [Fri, 11 Dec 2015 16:10:09 +0000 (17:10 +0100)]
MINOR: lua: service/applet can have access to the HTTP headers when a POST is received
When a POST is processed by a Lua service, the HTTP header are
potentially gone. So, we cannot retrieve their content using
the standard "hdr" sample fetchs (which will soon become invalid
anyway) from an applet.
This patch add an entry "headers" to the object applet_http. This
entry is an array containing all the headers. It permits to use the
HTTP headers during the processing of the service.
Many thanks to Jan Bruder for reporting this issue with enough
details to reproduce it.
This patch will have to be backported to 1.6 since it will be the
only way to access headers from Lua applets.
Emeric Brun [Wed, 16 Dec 2015 14:16:46 +0000 (15:16 +0100)]
BUG/MEDIUM: peers: table entries learned from a remote are pushed to others after a random delay.
New sticktable entries learned from a remote peer can be pushed to others after
a random delay because they are not inserted at the right position in the updates
tree.
Willy Tarreau [Mon, 14 Dec 2015 11:46:07 +0000 (12:46 +0100)]
BUG/MEDIUM: config: properly adjust maxconn with nbproc when memmax is forced
When memmax is forced using "-m", the per-process memory limit is enforced
using setrlimit(), but this value is not used to compute the automatic
maxconn limit. In addition, the per-process memory limit didn't consider
the fact that the shared SSL cache only needs to be accounted once.
The doc was also fixed to clearly state that "-m" is global and not per
process. It makes sense because people who use -m want to protect the
system's resources regardless of whatever appears in the configuration.
Willy Tarreau [Mon, 14 Dec 2015 11:04:35 +0000 (12:04 +0100)]
MINOR: config: make tune.recv_enough configurable
This setting used to be assigned to a variable tunable from a constant
and for an unknown reason never made its way into the config parser.
tune.recv_enough <number>
Haproxy uses some hints to detect that a short read indicates the end of the
socket buffers. One of them is that a read returns more than <recv_enough>
bytes, which defaults to 10136 (7 segments of 1448 each). This default value
may be changed by this setting to better deal with workloads involving lots
of short messages such as telnet or SSH sessions.
yanbzhu [Wed, 2 Dec 2015 18:54:14 +0000 (13:54 -0500)]
MINOR: ssl: Added multi cert support for crt-list config keyword
Added support for loading mutiple certs into shared contexts when they
are specified in a crt-list
Note that it's not practical to support SNI filters with multicerts, so
any SNI filters that's provided to the crt-list is ignored if a
multi-cert opertion is used.
yanbzhu [Wed, 2 Dec 2015 18:01:29 +0000 (13:01 -0500)]
MEDIUM: ssl: Added support for creating SSL_CTX with multiple certs
Added ability for users to specify multiple certificates that all relate
a single server. Users do this by specifying certificate "cert_name.pem"
but having "cert_name.pem.rsa", "cert_name.pem.dsa" and/or
"cert_name.pem.ecdsa" in the directory.
HAProxy will now intelligently search for those 3 files and try combine
them into as few SSL_CTX's as possible based on CN/SAN. This will allow
HAProxy to support multiple ciphersuite key algorithms off a single
SSL_CTX.
This change integrates into the existing architecture of SNI lookup and
multiple SNI's can point to the same SSL_CTX, which can support multiple
key_types.
yanbzhu [Tue, 1 Dec 2015 20:16:07 +0000 (15:16 -0500)]
MINOR: ssl: Added cert_key_and_chain struct
Added cert_key_and_chain struct to ssl. This struct will store the
contents of a crt path (from the config file) into memory. This will
allow us to use the data stored in memory instead of reading the file
multiple times.
This will be used to support a later commit to load multiple pkeys/certs
into a single SSL_CTX
David Carlier [Tue, 8 Dec 2015 21:43:09 +0000 (21:43 +0000)]
CLEANUP: haproxy: using _GNU_SOURCE instead of __USE_GNU macro.
In order to properly enable sched_setaffinity, in some versions of Linux,
it is rather _GNU_SOURCE than __USE_GNU (spotted on Alpine Linux for instance),
also for the sake of consistency as __USE_GNU seems not used across the code and
for last, it seems on Linux it is the best way to enable non portable code.
On Linux glibc's based versions, it seems _GNU_SOURCE defines __USE_GNU
it should be safe enough.
backend private-backend
mode http
http-reuse always
server bck 127.0.0.1:8888
The reason for this is that in http_end_txn_clean_session() we check the
stream's backend backend's http-reuse option before deciding whether the
backend connection should be moved back to the server's pool or not. But
since we're doing this after the call to http_reset_txn(), the backend is
reset to match the frontend, which doesn't have the option. However it
will work fine in a setup involving a "listen" section.
We just need to keep a pointer to the current backend before calling
http_reset_txn(). The code does that and replaces the few remaining
references to s->be inside the same function so that if any part of
code were to be moved later, this trap doesn't happen again.
BUG/MINOR: tcpcheck: conf parsing error when no port configured on server and first rule(s) is (are) COMMENT
A small configuration parsing error exists when no port is setup on the
server IP:port statement and the server's parameter 'port' is not set
and if the first tcp-check rule is a comment, like in the example below:
backend b
option tcp-check
tcp-check comment blah
tcp-check connect 8444
server s 127.0.0.1 check
In such case, an ALERT is improperly returned, despite this
configuration is valid and works.
The new code move the pointer to the first tcp-check rule which isn't a
comment before checking the presence of the port.
BUG/MINOR: tcpcheck: conf parsing error when no port configured on server and last rule is a CONNECT with no port
Current configuration parsing is permissive in such situation:
A server in a backend with no port conigured on the IP address
statement, no 'port' parameter configured and last rule of a tcp-check
is a CONNECT with no port.
The current code currently parses all the rules to validate a port is
well available, but it misses the last one, which means such
configuration is valid:
backend b
option tcp-check
tcp-check connect port 8444
tcp-check connect
server s 127.0.0.1 check
the second connect tentative is sent to port '0'...
Current patch fixes this by parsing the list the right way, including
the last rule.
Cyril Bonté [Fri, 4 Dec 2015 02:07:07 +0000 (03:07 +0100)]
BUG/MINOR: checks: email-alert causes a segfault when an unknown mailers section is configured
A segfault can occur during at the initialization phase, when an unknown
"mailers" name is configured. This happens when "email-alert myhostname" is not
set, where a direct pointer to an array is used instead of copying the string,
causing the segfault when haproxy tries to free the memory.
This is a minor issue because the configuration is invalid and a fatal error
will remain, but it should be fixed to prevent reload issues.
Example of minimal configuration to reproduce the bug :
backend example
email-alert mailers NOT_FOUND
email-alert from foo@localhost
email-alert to bar@localhost
Cyril Bonté [Fri, 4 Dec 2015 02:07:06 +0000 (03:07 +0100)]
BUG/MEDIUM: checks: email-alert not working when declared in defaults
Tommy Atkinson and Sylvain Faivre reported that email alerts didn't work when
they were declared in the defaults section. This is due to the use of an
internal attribute which is set once an email-alert is at least partially
configured. But this attribute was not propagated to the current proxy during
the configuration parsing.
Not that the issue doesn't occur if "email-alert myhostname" is configured in
the defaults section.
Cyril Bonté [Thu, 26 Nov 2015 20:39:56 +0000 (21:39 +0100)]
BUG/MEDIUM: sample: urlp can't match an empty value
Currently urlp fetching samples were able to find parameters with an empty
value, but the return code depended on the value length. The final result was
that acls using urlp couldn't match empty values.
Example of acl which always returned "false":
acl MATCH_EMPTY urlp(foo) -m len 0
The fix consists in unconditionally return 1 when the parameter is found.
Willy Tarreau [Thu, 26 Nov 2015 15:34:56 +0000 (16:34 +0100)]
CLEANUP: compression: don't allocate DEFAULT_MAXZLIBMEM without USE_ZLIB
It's pointless to reserve this amount of memory when zlib is not used.
Adding the condition will make build scripts easier to manage. This may
be backported to 1.6.
client-fin and server-fin are bogus. They are applied on the write
side after a SHUTR was seen. The immediate effect is that sometimes
if a SHUTR was seen after a SHUTW on the same side, the timeout is
enabled again regardless of the fact that the output is already
closed. This results in the timeout event not to be processed and
a busy poll loop to happen until another timeout on the stream gets
rid of it. Note that haproxy continues its job during this, it's just
that it eats all the CPU trying to handle an event that it ignores.
An reproducible case consists in having a client stop reading data from
a server to ensure data remain in the response buffer, then the client
sends a shutdown(write). If abortonclose is enabled on haproxy, the
shutdown is passed to the server side and the server responds with a
SHUTR that cannot immediately be forwarded to the client since the
buffer is full. During this time the event is ignored and the task is
woken again in loops.
It is worth noting that the timeout handling since 1.5 is a bit fragile
and that it might be possible that other similar conditions still exist,
so the timeout handling should be audited regarding this issue.
Many thanks to BaiYang for providing detailed information showing the
problem in action.
This bug also affects 1.5 thus the fix must be backported.
Willy Tarreau [Wed, 25 Nov 2015 19:11:11 +0000 (20:11 +0100)]
BUG/MEDIUM: http: don't enable auto-close on the response side
There is a bug where "option http-keep-alive" doesn't force a response
to stay in keep-alive if the server sends the FIN along with the response
on the second or subsequent response. The reason is that the auto-close
was forced enabled when recycling the HTTP transaction and it's never
disabled along the response processing chain before the SHUTR gets a
chance to be forwarded to the client side. The MSG_DONE state of the
HTTP response properly disables it but too late.
There's no more reason for enabling auto-close here, because either it
doesn't matter in non-keep-alive modes because the connection is closed,
or it is automatically enabled by process_stream() when it sees there's
no analyser on the stream.
This bug also affects 1.5 so a backport is desired.
Willy Tarreau [Wed, 25 Nov 2015 10:29:47 +0000 (11:29 +0100)]
BUILD/MINOR: http: proto_http.h needs sample.h
Since commit fd7edd3 ("MINOR: Move http method enum from proto_http to sample")
proto_http.h needs to include sample.h. This can be backported to 1.6 though
it doesn't affect existing code.
Sander Klein reported an error messages about SSLv3 not
being supported on Debian 8, although he didn't force-sslv3.
Vincent Bernat tracked this down to the LUA initialization, which
actually does force-sslv3.
This patch removes force-sslv3 from the LUA initialization, so
the LUA SSL socket can actually use TLS and doesn't trigger
warnings when SSLv3 is not supported by libssl (such as in
Debian 8).
Willy Tarreau [Wed, 18 Nov 2015 10:59:55 +0000 (11:59 +0100)]
BUG/MEDIUM: http: switch the request channel to no-delay once done.
There's an issue when sending POST data that came in a second packet,
the CF_NEVER_WAIT flag is not always set on the request channel, while
the server is waiting for the request. We must always set this flag in
this case since we're not going to shut down after sending, contrary
to the response side.
Note that option http-no-delay works around this issue.
lsenta [Fri, 13 Nov 2015 09:44:22 +0000 (10:44 +0100)]
BUG: http: do not abort keep-alive connections on server timeout
When a server timeout is detected on the second or nth request of a keep-alive
connection, HAProxy closes the connection without writing a response.
Some clients would fail with a remote disconnected exception and some
others would retry potentially unsafe requests.
This patch removes the special case and makes sure a 504 timeout is
written back whenever a server timeout is handled.
David CARLIER [Fri, 6 Nov 2015 15:13:06 +0000 (15:13 +0000)]
BUILD: Make deviceatlas require PCRE
Makefile deviceatlas throwing an error if the necessary pcre flag
is not passed avoiding surprising bunch of 'undefined reference'
for the user. Plus a tiny typo in OPENSSL area.
Baptiste Assmann [Wed, 28 Oct 2015 12:49:01 +0000 (13:49 +0100)]
DOC: relation between timeout http-request and option http-buffer-request
The documentation missed the explanation and relation between the
timeout http-request and option http-buffer-request.
Combined together, it helps protecting against slow POST types of
attacks.
When the txn.done() fiunction is called, the ouput buffer is cleaned,
but the associated relative pointer on the HTTP requests elements
is not reseted.
This patch remove this cleanup, because the output buffer may contain
data to forward.
Lukas Tribus [Thu, 5 Nov 2015 12:59:30 +0000 (13:59 +0100)]
BUG/MINOR: acl: don't use record layer in req_ssl_ver
The initial record layer version in a SSL handshake may be set to TLSv1.0
or similar for compatibility reasons, this is allowed as per RFC5246
Appendix E.1 [1]. Some implementations are Openssl [2] and NSS [3].
A related issue has been fixed some time ago in commit 57d229747
("BUG/MINOR: acl: req_ssl_sni fails with SSLv3 record version").
Fix this by using the real client hello version instead of the record
layer version.
This was reported by Julien Vehent and analyzed by Cyril Bonté.
The initial patch is from Julien Vehent as well.
This should be backported to stable series, the req_ssl_ver keyword was
first introduced in 1.3.16.
Dragan Dosen [Wed, 4 Nov 2015 22:03:26 +0000 (23:03 +0100)]
BUG/MINOR: server: check return value of fgets() in apply_server_state()
fgets() can return NULL on error or when EOF occurs. This patch adds a
check of fgets() return value and displays a warning if the first line of
the server state file can not be read. Additionally, we make sure to close
the previously opened file descriptor.
James Brown [Thu, 22 Oct 2015 01:19:05 +0000 (18:19 -0700)]
MINOR: check: add agent-send server parameter
Causes HAProxy to emit a static string to the agent on every check,
so that you can independently control multiple services running
behind a single agent port.
BUG/MEDIUM: lua: sample fetches based on response doesn't work
The direction (request or response) is not propagated in the
sample fecthes called throught Lua. This patch adds the direction
status in some structs (hlua_txn and hlua_smp) to make sure that
the sample fetches will be called with all the information.
The converters can not access to a TXN object, so there are not
impacted the direction. However, the samples used as input of the
Lua converter wrapper are initiliazed with the direction. Thereby,
the struct smp stay consistent.
[wt: needs to be backported to 1.6]
CLEANUP: use direction names in place of numeric values
This patch cleanups the direction names. It replaces numeric values,
by the associated defines. It ensure the compliance with values found
somwhere else in HAProxy.
It is required by the bugfix patch which is following.
[wt: needs to be backported to 1.6]
BUG/MINOR: dns: check for duplicate nameserver id in a resolvers section was missing
Current resolvers section parsing function is permissive on nameserver
id and two nameservers may have the same id.
It's a shame, since we don't know for example, whose statistics belong
to which nameserver...
From now, configuration with duplicated nameserver id in a resolvers
section are considered as broken and returns a fatal error when parsing.
Willy Tarreau [Mon, 2 Nov 2015 19:20:11 +0000 (20:20 +0100)]
BUG/MAJOR: http: don't requeue an idle connection that is already queued
Cyril Bonté reported a reproduceable sequence which can lead to a crash
when using backend connection reuse. The problem comes from the fact that
we systematically add the server connection to an idle pool at the end of
the HTTP transaction regardless of the fact that it might already be there.
This is possible for example when processing a request which doesn't use
a server connection (typically a redirect) after a request which used a
connection. Then after the first request, the connection was already in
the idle queue and we're putting it a second time at the end of the second
request, causing a corruption of the idle pool.
Interestingly, the memory debugger in 1.7 immediately detected a suspicious
double free on the connection, leading to a very early detection of the
cause instead of its consequences.
Thanks to Cyril for quickly providing a working reproducer.
This fix must be backported to 1.6 since connection reuse was introduced
there.