Matt Robenolt [Thu, 11 Sep 2014 05:19:30 +0000 (05:19 +0000)]
MINOR: Also accept SIGHUP/SIGTERM in systemd-wrapper
My proposal is to let haproxy-systemd-wrapper also accept normal
SIGHUP/SIGTERM signals to play nicely with other process managers
besides just systemd. In my use case, this will be for using with
runit which has to ability to change the signal used for a
"reload" or "stop" command. It also might be worth renaming this
bin to just haproxy-wrapper or something of that sort to separate
itself away from systemd. But that's a different discussion. :)
BUG/CRITICAL: http: don't update msg->sov once data start to leave the buffer
Commit bb2e669 ("BUG/MAJOR: http: correctly rewind the request body
after start of forwarding") was incorrect/incomplete. It used to rely on
CF_READ_ATTACHED to stop updating msg->sov once data start to leave the
buffer, but this is unreliable because since commit a6eebb3 ("[BUG]
session: clear BF_READ_ATTACHED before next I/O") merged in 1.5-dev1,
this flag is only ephemeral and is cleared once all analysers have
seen it. So we can start updating msg->sov again each time we pass
through this place with new data. With a sufficiently large amount of
data, it is possible to make msg->sov wrap and validate the if()
condition at the top, causing the buffer to advance by about 2GB and
crash the process.
Note that the offset cannot be controlled by the attacker because it is
a sum of millions of small random sizes depending on how many bytes were
read by the server and how many were left in the buffer, only because
of the speed difference between reading and writing. Also, nothing is
written, the invalid pointer resulting from this operation is only read.
Many thanks to James Dempsey for reporting this bug and to Chris Forbes for
narrowing down the faulty area enough to make its root cause analysable.
Cyril Bonté [Fri, 29 Aug 2014 18:20:02 +0000 (20:20 +0200)]
BUG/MEDIUM: config: userlists should ensure that encrypted passwords are supported
When an unknown encryption algorithm is used in userlists or the password is
not pasted correctly in the configuration, http authentication silently fails.
An initial check is now performed during the configuration parsing, in order to
verify that the encrypted password is supported. An unsupported password will
fail with a fatal error.
Cyril Bonté [Fri, 29 Aug 2014 18:20:01 +0000 (20:20 +0200)]
BUG/MEDIUM: auth: fix segfault with http-auth and a configuration with an unknown encryption algorithm
Grégoire Morpain reported a segfault when a secured password is used for http
authentication. It was caused by the use of an unsupported encryption algorithm
with libcrypto. In this case, crypt() returns a NULL pointer.
Willy Tarreau [Fri, 29 Aug 2014 17:09:48 +0000 (19:09 +0200)]
CLEANUP: acl: cleanup some of the redundancy and spaghetti after last fix
This code aims at clearing up the ACL parsing code a bit to make it
more obvious what happens in the case of an ACL keyword and what happens
in the case of a sample expression. Variables prev_type and cur_type were
merged, and ACL keyword processing functions are checked only once.
This patch could be backported into 1.5 after the previous patch, in order
to keep the code more maintainable.
Willy Tarreau [Fri, 29 Aug 2014 15:36:40 +0000 (17:36 +0200)]
BUG/MEDIUM: acl: correctly compute the output type when a converter is used
Sample expressions involving converters in expression simply do not work
if the converter changes the sample type from the original keyword. Either
the keyword is a sample fetch keyword and its own type is used, or it's an
ACL keyword, and the keyword's parse/index/match functions are used despite
the converters. Thus it causes such a stupid error :
redirect location / if { date,ltime(%a) -i Fri }
[ALERT] 240/171746 (29127) : parsing [bug-conv.cfg:35] : error detected in proxy 'svc' while parsing redirect rule : error in condition: 'Fri' is not a number.
In fact now in ACLs, the case where the ACL keyword is alone is an exception
(eventhough the most common one). It's an exception to the sample expression
parsing rules since ACLs allow to redefine alternate parsing functions.
This fix does two things :
- it voids any references to the ACL keyword when a converter is involved
since we certainly not want to enforce a parser for a wrong data type ;
- it ensures that for all other cases (sample expressions), the type of
the expression is used instead of the type of the fetch keyword.
A significant cleanup of the code should be done, but this patch only aims
fixing the bug.
The fix should be backported into 1.5 since this appeared along the redesign
of the acl/pattern processing.
Willy Tarreau [Fri, 29 Aug 2014 13:19:33 +0000 (15:19 +0200)]
BUG/MINOR: pattern: remove useless allocation of unused trash in pat_parse_reg()
Just like previous patch, this is a remains of an early implementation. Also
fix the outdated comments above. The fix may be backported to 1.5 though the
bug cannot be triggerred, thus it's just a matter of keeping the code clean.
Willy Tarreau [Fri, 29 Aug 2014 13:15:50 +0000 (15:15 +0200)]
BUG/MEDIUM: http: fix improper parsing of HTTP methods for use with ACLs
pat_parse_meth() had some remains of an early implementation attempt for
the patterns, it initialises a trash and never sets the pattern value there.
The result is that a non-standard method cannot be matched anymore. The bug
appeared during the pattern rework in 1.5, so this fix must be backported
there. Thanks to Joe Williams of GitHub for reporting the bug.
Willy Tarreau [Thu, 28 Aug 2014 18:42:57 +0000 (20:42 +0200)]
BUG/MEDIUM: http: fix inverted condition in pat_match_meth()
This results in a string-based HTTP method match returning true when
it doesn't match and conversely. This bug was reported by Joe Williams.
The fix must be backported to 1.5, though it still doesn't work because
of at least 3-4 other bugs in the long path which leads to building this
pattern list.
Willy Tarreau [Thu, 28 Aug 2014 13:03:15 +0000 (15:03 +0200)]
MINOR: log: add a new field "%lc" to implement a per-frontend log counter
Sometimes it would be convenient to have a log counter so that from a log
server we know whether some logs were lost or not. The frontend's log counter
serves exactly this purpose. It's incremented each time a traffic log is
produced. If a log is disabled using "http-request set-log-level silent",
the counter will not be incremented. However, admin logs are not accounted
for. Also, if logs are filtered out before being sent to the server because
of a minimum level set on the log line, the counter will be increased anyway.
The counter is 32-bit, so it will wrap, but that's not an issue considering
that 4 billion logs are rarely in the same file, let alone close to each
other.
Willy Tarreau [Thu, 28 Aug 2014 12:36:36 +0000 (14:36 +0200)]
OPTIM/MINOR: proxy: reduce struct proxy by 48 bytes on 64-bit archs
Just by moving a few struct members around, we can avoid 32-bit holes
between 64-bit pointers and shrink the struct size by 48 bytes. That's
not huge but that's for free, so let's do it.
Dave McCowan [Wed, 30 Jul 2014 14:39:13 +0000 (10:39 -0400)]
MEDIUM: connection: add new bit in Proxy Protocol V2
There are two sample commands to get information about the presence of a
client certificate.
ssl_fc_has_crt is true if there is a certificate present in the current
connection
ssl_c_used is true if there is a certificate present in the session.
If a session has stopped and resumed, then ssl_c_used could be true, while
ssl_fc_has_crt is false.
In the client byte of the TLS TLV of Proxy Protocol V2, there is only one
bit to indicate whether a certificate is present on the connection. The
attached patch adds a second bit to indicate the presence for the session.
This maintains backward compatibility.
[wt: this should be backported to 1.5 to help maintain compatibility
between versions]
Thierry FOURNIER [Fri, 22 Aug 2014 04:55:26 +0000 (06:55 +0200)]
BUG/MEDIUM: http: tarpit timeout is reset
Before the commit bbba2a8ecc35daf99317aaff7015c1931779c33b
(1.5-dev24-8), the tarpit section set timeout and return, after this
commit, the tarpit section set the timeout, and go to the "done" label
which reset the timeout.
Thanks Bryan Talbot for the bug report and analysis.
also see:
https://www.imperialviolet.org/2014/02/27/tlssymmetriccrypto.html
cipher_list is used in haproxy since commit f46cd6e4ec3a ("MEDIUM: ssl:
Add the option to use standardized DH parameters >= 1024 bits") to
check if DHE ciphers are used.
So, if boringssl is used, the patch just assumes that there is some
DHE cipher enabled. This will lead to false positives, but thats better
than compiler warnings and crashes.
This may be replaced one day by properly implementing the the new style
cipher_list, in the meantime this workaround allows to build and use
boringssl.
Lukas Tribus [Sun, 17 Aug 2014 22:56:30 +0000 (00:56 +0200)]
BUILD: ssl: handle boringssl in openssl version detection
Google's boringssl doesn't have OPENSSL_VERSION_TEXT, SSLeay_version()
or SSLEAY_VERSION, in fact, it doesn't have any real versioning, its
just git-based.
So in case we build against boringssl, we can't access those values.
Instead, we just inform the user that HAProxy was build against
boringssl.
Cyril Bonté [Wed, 6 Aug 2014 23:55:39 +0000 (01:55 +0200)]
BUG/MINOR: checks: external checks shouldn't wait for timeout to return the result
When the child process terminates, it should wake up the associated task to
process the result immediately, otherwise it will be available only when the
task expires.
Cyril Bonté [Wed, 6 Aug 2014 23:55:38 +0000 (01:55 +0200)]
BUG/MEDIUM: checks: segfault with external checks in a backend section
The documentation indicates that external checks can be used in a backend
section, but the code requires a listener to provide information in the script
arguments.
External checks were initialized lately, during the first check, leaving some
variables uninitialized in such a scenario, which trigger the segfault when
accessed to collect errors information.
To prevent the segfault, currently we should initialize the external checks
earlier, during the process initialiation itself and quit if the error occurs.
Cyril Bonté [Wed, 6 Aug 2014 23:55:37 +0000 (01:55 +0200)]
BUG/MEDIUM: checks: external checks can't change server status to UP
Mark Brooks reported an issue with external healthchecks, where servers are
never marked as UP. This is due to a typo, which flags a successful check as
CHK_RES_FAILED instead of CHK_RES_PASSED.
BUG/MAJOR: tcp: fix a possible busy spinning loop in content track-sc*
As a consequence of various recent changes on the sample conversion,
a corner case has emerged where it is possible to wait forever for a
sample in track-sc*.
The issue is caused by the fact that functions relying on sample_process()
don't all exactly work the same regarding the SMP_F_MAY_CHANGE flag and
the output result. Here it was possible to wait forever for an output
sample from stktable_fetch_key() without checking the SMP_OPT_FINAL flag.
As a result, if the client connects and closes without sending the data
and haproxy expects a sample which is capable of coming, it will ignore
this impossible case and will continue to wait.
This change adds control for SMP_OPT_FINAL before waiting for extra data.
The various relevant functions have been better documented regarding their
output values.
This fix must be backported to 1.5 since it appeared there.
MEDIUM: Improve signal handling in systemd wrapper.
Move all code out of the signal handlers, since this is potentially
dangerous. To make sure the signal handlers behave as expected, use
sigaction() instead of signal(). That also obsoletes messing with
the signal mask after restart.
BUG/MINOR: Fix search for -p argument in systemd wrapper.
Searching for the pid file in the list of arguments did not
take flags without parameters into account, like e.g. -de. Because
of this, the wrapper would use a different pid file than haproxy
if such an argument was specified before -p.
The new version can still yield a false positive for some crazy
situations, like your config file name starting with "-p", but
I think this is as good as it gets without using getopt or some
library.
BUG/MINOR: server: move the directive #endif to the end of file
If a source file includes proto/server.h twice or more, redefinition errors will
be triggered for such inline functions as server_throttle_rate(),
server_is_draining(), srv_adm_set_maint() and so on. Just move #endif directive
to the end of file to solve this issue.
Last commit 77d1f01 ("BUG/MEDIUM: connection: fix memory corruption
when building a proxy v2 header") was wrong, using &cn_trash instead
of cn_trash resulting in a warning and the client's SSL cert CN not
being stored at the proper location.
Thanks to Lukas Tribus for spotting this quickly.
This should be backported to 1.5 after the patch above is backported.
Willy Tarreau [Wed, 25 Jun 2014 16:12:15 +0000 (18:12 +0200)]
MEDIUM: http: add the track-sc* actions to http-request rules
Add support for http-request track-sc, similar to what is done in
tcp-request for backends. A new act_prm field was added to HTTP
request rules to store the track params (table, counter). Just
like for TCP rules, the table is resolved while checking for
config validity. The code was mostly copied from the TCP code
with the exception that here we also count the HTTP request count
and rate by hand. Probably that something could be factored out in
the future.
It seems like tracking flags should be improved to mark each hook
which tracks a key so that we can have some check points where to
increase counters of the past if not done yet, a bit like is done
for TRACK_BACKEND.
Currently, the commit ID appears in the sub-version in snapshots, but
when people use the git repository, we only have the commits count,
and not the last commit ID, which requires to count commits when
troubleshooting. This change ensures that unreleased versions also
report the commit ID before the commit number, such as :
1.6-dev0-bbfd1a-50
Tagged versions will not have this, since the post-release commit count
is zero.
Doing so finally allows to apply the hex converter to integers as well.
Note that all integers are represented in 32-bit, big endian so that their
conversion remains human readable and portable. A later improvement to the
hex converter could be to make it trim leading zeroes, and/or to only report
a number of least significant bytes.
From time to time it's useful to hash input data (scramble input, or
reduce the space needed in a stick table). This patch provides 3 simple
converters allowing use of the available hash functions to hash input
data. The output is an unsigned integer which can be passed into a header,
a log or used as an index for a stick table. One nice usage is to scramble
source IP addresses before logging when there are requirements to hide them.
IP addresses are a perfect example of fixed size data which we could
cast to binary, still it was not allowed by lack of cast function,
eventhough the opposite was allowed in ACLs. Make that possible both
in sample expressions and in stick tables.
BUG/MINOR: http: base32+src should use the big endian version of base32
We're using the internal memory representation of base32 here, which is
wrong since these data might be exported to headers for logs or be used
to stick to a server and replicated to other peers. Let's convert base32
to big endian (network representation) when building the binary block.
This mistake is also present in 1.5, it would be better to backport it.
MEDIUM: stick-table: make it easier to register extra data types
Some users want to add their own data types to stick tables. We don't
want to use a linked list here for performance reasons, so we need to
continue to use an indexed array. This patch allows one to reserve a
compile-time-defined number of extra data types by setting the new
macro STKTABLE_EXTRA_DATA_TYPES to anything greater than zero, keeping
in mind that anything larger will slightly inflate the memory consumed
by stick tables (not per entry though).
Then calling stktable_register_data_store() with the new keyword will
either register a new keyword or fail if the desired entry was already
taken or the keyword already registered.
Note that this patch does not dictate how the data will be used, it only
offers the possibility to create new keywords and have an index to
reference them in the config and in the tables. The caller will not be
able to use stktable_data_cast() and will have to explicitly cast the
stable pointers to the expected types. It can be used for experimentation
as well.
BUG/MEDIUM: ssl: Fix a memory leak in DHE key exchange
OpenSSL does not free the DH * value returned by the callback specified with SSL_CTX_set_tmp_dh_callback(),
leading to a memory leak for SSL/TLS connections using Diffie Hellman Ephemeral key exchange.
This patch fixes the leak by allocating the DH * structs holding the DH parameters once, at configuration time.
Konstantin Romanenko reported a typo in the HTML documentation. The typo is
already present in the raw text version : the "shutdown sessions" command
should be "shutdown sessions server".
DOC: mention that "compression offload" is ignored in defaults section
This one is not inherited from defaults into frontends nor backends
because it would create a confusion situation where it would be hard
to disable it (since both frontend and backend would enable it).
DOC: remove references to CPU=native in the README
Certain compilers running in virtualized environments may produce code
that the same processor cannot execute with -march=native, either because
of hypervisor bugs reporting wrong CPU features, or because of compiler
bugs forgetting to check CPU features. So better stop recommending this
combination so that users don't get trapped anymore.
BUG/MAJOR: http: correctly rewind the request body after start of forwarding
Daniel Dubovik reported an interesting bug showing that the request body
processing was still not 100% fixed. If a POST request contained short
enough data to be forwarded at once before trying to establish the
connection to the server, we had no way to correctly rewind the body.
The first visible case is that balancing on a header does not always work
on such POST requests since the header cannot be found. But there are even
nastier implications which are that http-send-name-header would apply to
the wrong location and possibly even affect part of the request's body
due to an incorrect rewinding.
There are two options to fix the problem :
- first one is to force the HTTP_MSG_F_WAIT_CONN flag on all hash-based
balancing algorithms and http-send-name-header, but there's always a
risk that any new algorithm forgets to set it ;
- the second option is to account for the amount of skipped data before
the connection establishes so that we always know the position of the
request's body relative to the buffer's origin.
The second option is much more reliable and fits very well in the spirit
of the past changes to fix forwarding. Indeed, at the moment we have
msg->sov which points to the start of the body before headers are forwarded
and which equals zero afterwards (so it still points to the start of the
body before forwarding data). A minor change consists in always making it
point to the start of the body even after data have been forwarded. It means
that it can get a negative value (so we need to change its type to signed)..
In order to avoid wrapping, we only do this as long as the other side of
the buffer is not connected yet.
Doing this definitely fixes the issues above for the requests. Since the
response cannot be rewound we don't need to perform any change there.
This bug was introduced/remained unfixed in 1.5-dev23 so the fix must be
backported to 1.5.
MEDIUM: stick-table: add new converters to fetch table data
These new converters make it possible to look up any sample expression
in a table, and check whether an equivalent key exists or not, and if it
exists, to retrieve the associated data (eg: gpc0, request rate, etc...).
Till now it was only possible using tracking, but sometimes tracking is
not suited to only retrieving such counters, either because it's done too
early or because too many items need to be checked without necessarily
being tracked.
These converters all take a string on input, and then convert it again to
the table's type. This means that if an input sample is of type IPv4 and
the table is of type IP, it will first be converted to a string, then back
to an IP address. This is a limitation of the current design which does not
allow converters to declare that "any" type is supported on input. Since
strings are the only types which can be cast to any other one, this method
always works.
MEDIUM: stick-table: implement lookup from a sample fetch
Currently we have stktable_fetch_key() which fetches a sample according
to an expression and returns a stick table key, but we also need a function
which does only the second half of it from a known sample. So let's cut the
function in two and introduce smp_to_stkey() to perform this lookup. The
first function was adapted to make use of it in order to avoid code
duplication.
Dan Dubovik [Tue, 8 Jul 2014 15:51:03 +0000 (08:51 -0700)]
BUG/MEDIUM: backend: Update hash to use unsigned int throughout
When we were generating a hash, it was done using an unsigned long. When the hash was used
to select a backend, it was sent as an unsigned int. This made it difficult to predict which
backend would be selected.
This patch updates get_hash, and the hash methods to use an unsigned int, to remain consistent
throughout the codebase.
This fix should be backported to 1.5 and probably in part to 1.4.
DOC: explicitly mention the limits of abstract namespace sockets
Listening to an abstract namespace socket is quite convenient but
comes with some drawbacks that must be clearly understood when the
socket is being listened to by multiple processes. The trouble is
that the socket cannot be rebound if a new process attempts a soft
restart and fails, so only one of the initially bound processes
will still be bound to it, the other ones will fail to rebind. For
most situations it's not an issue but it needs to be indicated.
BUG/MEDIUM: unix: completely unbind abstract sockets during a pause()
Abstract namespace sockets ignore the shutdown() call and do not make
it possible to temporarily stop listening. The issue it causes is that
during a soft reload, the new process cannot bind, complaining that the
address is already in use.
This change registers a new pause() function for unix sockets and
completely unbinds the abstract ones since it's possible to rebind
them later. It requires the two previous patches as well as preceeding
fixes.
This fix should be backported into 1.5 since the issue apperas there.
MEDIUM: listener: support rebinding during resume()
When a listener resumes operations, supporting a full rebind makes it
possible to perform a full stop as a pause(). This will be used for
pausing abstract namespace unix sockets.
MEDIUM: listener: implement a per-protocol pause() function
In order to fix the abstact socket pause mechanism during soft restarts,
we'll need to proceed differently depending on the socket protocol. The
pause_listener() function already supports some protocol-specific handling
for the TCP case.
This commit makes this cleaner by adding a new ->pause() function to the
protocol struct, which, if defined, may be used to pause a listener of a
given protocol.
For now, only TCP has been adapted, with the specific code moved from
pause_listener() to tcp_pause_listener().
BUG/MEDIUM: unix: failed abstract socket binding is retryable
Jan Seda noticed that abstract sockets are incompatible with soft reload,
because the new process cannot bind and immediately fails. This patch marks
the binding as retryable and not fatal so that the new process can try to
bind again after sending a signal to the old process.
Note that this fix is not enough to completely solve the problem, but it
is necessary. This patch should be backported to 1.5.
BUG/MINOR: listener: set the listener's fd to -1 after deletion
This is currently harmless, but when stopping a listener, its fd is
closed but not set to -1, so it is not possible to re-open it again.
Currently this has no impact but can have after the abstract sockets
are modified to perform a complete close on soft-reload.
The fix can be backported to 1.5 and may even apply to 1.4 (protocols.c).
BUILD: http: fix isdigit & isspace warnings on Solaris
As usual, when touching any is* function, Solaris complains about the
type of the element being checked. Better backport this to 1.5 since
nobody knows what the emitted code looks like since macros are used
instead of functions.
Jan Seda [Thu, 26 Jun 2014 18:44:05 +0000 (20:44 +0200)]
BUG/MEDIUM: unix: do not unlink() abstract namespace sockets upon failure.
When bind() fails (function uxst_bind_listener()), the fail path doesn't
consider the abstract namespace and tries to unlink paths held in
uninitiliazed memory (tempname and backname). See the strace excerpt;
the strings still hold the path from test1.
===============================================================================================
23722 bind(5, {sa_family=AF_FILE, path=@"test2"}, 110) = -1 EADDRINUSE (Address already in use)
23722 unlink("/tmp/test1.sock.23722.tmp") = -1 ENOENT (No such file or directory)
23722 close(5) = 0
23722 unlink("/tmp/test1.sock.23722.bak") = -1 ENOENT (No such file or directory)
===============================================================================================
Willy Tarreau [Fri, 27 Jun 2014 16:10:07 +0000 (18:10 +0200)]
MEDIUM: log: support a user-configurable max log line length
With all the goodies supported by logformat, people find that the limit
of 1024 chars for log lines is too short. Some servers do not support
larger lines and can simply drop them, so changing the default value is
not always the best choice.
This patch takes a different approach. Log line length is specified per
log server on the "log" line, with a value between 80 and 65535. That
way it's possibly to satisfy all needs, even with some fat local servers
and small remote ones.
Willy Tarreau [Fri, 27 Jun 2014 16:08:49 +0000 (18:08 +0200)]
MINOR: log: make MAX_SYSLOG_LEN overridable at build time
This value was set in log.h without any #ifndef around, so when one
wanted to change it, a patch was needed. Let's move it to defaults.h
with the usual #ifndef so that it's easier to change it.
Willy Tarreau [Wed, 25 Jun 2014 15:01:56 +0000 (17:01 +0200)]
BUG/MEDIUM: counters: fix track-sc* to wait on unstable contents
I've been facing multiple configurations which involved track-sc* rules
in tcp-request content without the "if ..." to force it to wait for the
contents, resulting in random behaviour with contents sometimes retrieved
and sometimes not.
Reading the doc doesn't make it clear either that the tracking will be
performed only if data are already there and that waiting on an ACL is
the only way to avoid this.
Since this behaviour is not natural and we now have the ability to fix
it, this patch ensures that if input data are still moving, instead of
silently dropping them, we naturally wait for them to stabilize up to
the inspect-delay. This way it's not needed anymore to implement an
ACL-based condition to force to wait for data, eventhough the behaviour
is not changed for when an ACL is present.
The most obvious usage will be when track-sc is followed by any HTTP
sample expression, there's no need anymore for adding "if HTTP".
It's probably worth backporting this to 1.5 to avoid further configuration
issues. Note that it requires previous patch.
Willy Tarreau [Wed, 25 Jun 2014 14:20:53 +0000 (16:20 +0200)]
MINOR: stick-table: make stktable_fetch_key() indicate why it failed
stktable_fetch_key() does not indicate whether it returns NULL because
the input sample was not found or because it's unstable. It causes trouble
with track-sc* rules. Just like with sample_fetch_string(), we want it to
be able to give more information to the caller about what it found. Thus,
now we use the pointer to a sample passed by the caller, and fill it with
the information we have about the sample. That way, even if we return NULL,
the caller has the ability to check whether a sample was found and if it is
still changing or not.
Willy Tarreau [Wed, 25 Jun 2014 14:56:41 +0000 (16:56 +0200)]
BUG/MAJOR: sample: correctly reinitialize sample fetch context before calling sample_process()
We used to only clear flags when reusing the static sample before calling
sample_process(), but that's not enough because there's a context in samples
that can be used by some fetch functions such as auth, headers and cookies,
and not reinitializing it risks that a pointer of a different type is used
in the wrong context.
An example configuration which triggers the case consists in mixing hdr()
and http_auth_group() which both make use of contexts :
Willy Tarreau [Wed, 25 Jun 2014 13:36:04 +0000 (15:36 +0200)]
BUG/MINOR: counters: do not untrack counters before logging
Baptiste Assmann reported a corner case in the releasing of stick-counters:
we release content-aware counters before logging. In the past it was not a
problem, but since now we can log them it, it prevents one from logging
their value. Simply switching the log production and the release of the
counter fixes the issue.
Emeric Brun [Tue, 24 Jun 2014 16:26:41 +0000 (18:26 +0200)]
BUG/MINOR: ssl: Fix external function in order not to return a pointer on an internal trash buffer.
'ssl_sock_get_common_name' applied to a connection was also renamed
'ssl_sock_get_remote_common_name'. Currently, this function is only used
with protocol PROXYv2 to retrieve the client certificate's common name.
A further usage could be to retrieve the server certificate's common name
on an outgoing connection.
Willy Tarreau [Tue, 24 Jun 2014 15:27:02 +0000 (17:27 +0200)]
BUG/MEDIUM: http: fetch "base" is not compatible with set-header
The sample fetch function "base" makes use of the trash which is also
used by set-header/add-header etc... everything which builds a formated
line. So we end up with some junk in the header if base is in use. Let's
fix this as all other fetches by using a trash chunk instead.
This bug was reported by Baptiste Assmann, and also affects 1.5.
Willy Tarreau [Mon, 23 Jun 2014 16:07:15 +0000 (18:07 +0200)]
BUG/MINOR: logs: properly initialize and count log sockets
Commit 81ae195 ("[MEDIUM] add support for logging via a UNIX socket")
merged in 1.3.14 introduced a few minor issues with log sockets. All
of them happen only when a failure is encountered when trying to set
up the logging socket (eg: socket family is not available or is
temporarily short in resources).
The first socket which experiences an error causes the socket setup
loop to abort, possibly preventing any log from being sent if it was
the first logger. The second issue is that if this socket finally
succeeds after a second attempt, errors are reported for the wrong
logger (eg: logger #1 failed instead of #2). The last point is that
we now have multiple loggers, and it's a waste of time to walk over
their list for every log while they're almost always properly set up.
So in order to fix all this, let's merge the two lists. If a logger
experiences an error, it simply sends an alert and skips to the next
one. That way they don't prevent messages from being sent and are
all properly accounted for.
Willy Tarreau [Mon, 23 Jun 2014 13:22:31 +0000 (15:22 +0200)]
BUG/MAJOR: session: revert all the crappy client-side timeout changes
This is the 3rd regression caused by the changes below. The latest to
date was reported by Finn Arne Gangstad. If a server responds with no
content-length and the client's FIN is never received, either we leak
the client-side FD or we spin at 100% CPU if timeout client-fin is set.
Enough is enough. The amount of tricks needed to cover these side-effects
starts to look like used toilet paper stacked over a chocolate cake. I
don't want to eat that cake anymore!
All this to avoid reporting a server-side timeout when a client stops
uploading data and haproxy expires faster than the server... A lot of
"ifs" resulting in a technically valid log that doesn't always please
users, and whose alternative causes that many issues for all others
users.
If a cleaner AND SAFER way to do something equivalent in 1.6-dev, we *might*
consider backporting it to 1.5, but given the vicious bugs that have surfaced
since, I doubt it will happen any time soon.
Fortunately, that crap never made it into 1.4 so no backport is needed.
Simon Horman [Fri, 20 Jun 2014 03:29:47 +0000 (12:29 +0900)]
BUG/MEDIUM: Consistently use 'check' in process_chk
I am not entirely sure that this is a bug, but it seems
to me that it may cause a problem if there agent-check is
configured and there is some kind of error making a connection for it.
Willy Tarreau [Thu, 19 Jun 2014 19:01:06 +0000 (21:01 +0200)]
[RELEASE] Released version 1.5.0
Released version 1.5.0 with the following main changes :
- MEDIUM: ssl: ignored file names ending as '.issuer' or '.ocsp'.
- MEDIUM: ssl: basic OCSP stapling support.
- MINOR: ssl/cli: Fix unapropriate comment in code on 'set ssl ocsp-response'
- MEDIUM: ssl: add 300s supported time skew on OCSP response update.
- MINOR: checks: mysql-check: Add support for v4.1+ authentication
- MEDIUM: ssl: Add the option to use standardized DH parameters >= 1024 bits
- MEDIUM: ssl: fix detection of ephemeral diffie-hellman key exchange by using the cipher description.
- MEDIUM: http: add actions "replace-header" and "replace-values" in http-req/resp
- MEDIUM: Break out check establishment into connect_chk()
- MEDIUM: Add port_to_str helper
- BUG/MEDIUM: fix ignored values for half-closed timeouts (client-fin and server-fin) in defaults section.
- BUG/MEDIUM: Fix unhandled connections problem with systemd daemon mode and SO_REUSEPORT.
- MINOR: regex: fix a little configuration memory leak.
- MINOR: regex: Create JIT compatible function that return match strings
- MEDIUM: regex: replace all standard regex function by own functions
- MEDIUM: regex: Remove null terminated strings.
- MINOR: regex: Use native PCRE API.
- MINOR: missing regex.h include
- DOC: Add Exim as Proxy Protocol implementer.
- BUILD: don't use type "uint" which is not portable
- BUILD: stats: workaround stupid and bogus -Werror=format-security behaviour
- BUG/MEDIUM: http: clear CF_READ_NOEXP when preparing a new transaction
- CLEANUP: http: don't clear CF_READ_NOEXP twice
- DOC: fix proxy protocol v2 decoder example
- DOC: fix remaining occurrences of "pattern extraction"
- MINOR: log: allow the HTTP status code to be logged even in TCP frontends
- MINOR: logs: don't limit HTTP header captures to HTTP frontends
- MINOR: sample: improve sample_fetch_string() to report partial contents
- MINOR: capture: extend the captures to support non-header keys
- MINOR: tcp: prepare support for the "capture" action
- MEDIUM: tcp: add a new tcp-request capture directive
- MEDIUM: session: allow shorter retry delay if timeout connect is small
- MEDIUM: session: don't apply the retry delay when redispatching
- MEDIUM: session: redispatch earlier when possible
- MINOR: config: warn when tcp-check rules are used without option tcp-check
- BUG/MINOR: connection: make proxy protocol v1 support the UNKNOWN protocol
- DOC: proxy protocol example parser was still wrong
- DOC: minor updates to the proxy protocol doc
- CLEANUP: connection: merge proxy proto v2 header and address block
- MEDIUM: connection: add support for proxy protocol v2 in accept-proxy
- MINOR: tools: add new functions to quote-encode strings
- DOC: clarify the CSV format
- MEDIUM: stats: report the last check and last agent's output on the CSV status
- MINOR: freq_ctr: introduce a new averaging method
- MEDIUM: session: maintain per-backend and per-server time statistics
- MEDIUM: stats: report per-backend and per-server time stats in HTML and CSV outputs
- BUG/MINOR: http: fix typos in previous patch
- DOC: remove the ultra-obsolete TODO file
- DOC: update roadmap
- DOC: minor updates to the README
- DOC: mention the maxconn limitations with the select poller
- DOC: commit a few old design thoughts files
Willy Tarreau [Thu, 19 Jun 2014 14:03:41 +0000 (16:03 +0200)]
DOC: commit a few old design thoughts files
These ones were design notes and ideas collected during the 1.5
development phase lying on my development machine. There might still
be some value in keeping them for future reference since they mention
certain corner cases.
Willy Tarreau [Thu, 19 Jun 2014 13:31:25 +0000 (15:31 +0200)]
DOC: mention the maxconn limitations with the select poller
Select()'s safe area is limited to 1024 FDs, and anything higher
than this will report "select: FAILED" on startup in debug mode,
so better document it.
Emeric Brun [Mon, 16 Jun 2014 16:36:30 +0000 (18:36 +0200)]
MEDIUM: ssl: basic OCSP stapling support.
The support is all based on static responses. This doesn't add any
request / response logic to HAProxy, but allows a way to update
information through the socket interface.
Currently certificates specified using "crt" or "crt-list" on "bind" lines
are loaded as PEM files.
For each PEM file, haproxy checks for the presence of file at the same path
suffixed by ".ocsp". If such file is found, support for the TLS Certificate
Status Request extension (also known as "OCSP stapling") is automatically
enabled. The content of this file is optional. If not empty, it must contain
a valid OCSP Response in DER format. In order to be valid an OCSP Response
must comply with the following rules: it has to indicate a good status,
it has to be a single response for the certificate of the PEM file, and it
has to be valid at the moment of addition. If these rules are not respected
the OCSP Response is ignored and a warning is emitted. In order to identify
which certificate an OCSP Response applies to, the issuer's certificate is
necessary. If the issuer's certificate is not found in the PEM file, it will
be loaded from a file at the same path as the PEM file suffixed by ".issuer"
if it exists otherwise it will fail with an error.
It is possible to update an OCSP Response from the unix socket using:
set ssl ocsp-response <response>
This command is used to update an OCSP Response for a certificate (see "crt"
on "bind" lines). Same controls are performed as during the initial loading of
the response. The <response> must be passed as a base64 encoded string of the
DER encoded response from the OCSP server.
Thierry FOURNIER [Wed, 18 Jun 2014 09:50:51 +0000 (11:50 +0200)]
MINOR: regex: Use native PCRE API.
The pcreposix layer (in the pcre projetc) execute strlen to find
thlength of the string. When we are using the function "regex_exex*2",
the length is used to add a final \0, when pcreposix is executed a
strlen is executed to compute the length.
If we are using a native PCRE api, the length is provided as an
argument, and these operations disappear.
This is useful because PCRE regex are more used than POSIC regex.
Thierry FOURNIER [Wed, 18 Jun 2014 09:53:08 +0000 (11:53 +0200)]
MEDIUM: regex: Remove null terminated strings.
The new regex function can use string and length. The HAproxy buffer are
not null-terminated, and the use of the regex_exec* functions implies
the add of this null character. This patch replace these function by the
functions which takes a string and length as input.
Just the file "proto_http.c" is change because this one is more executed
than other. The file "checks.c" have a very low usage, and it is not
interesting to change it. Furthermore, the buffer used by "checks.c" are
null-terminated.
Thierry FOURNIER [Wed, 11 Jun 2014 11:59:05 +0000 (13:59 +0200)]
MINOR: regex: Create JIT compatible function that return match strings
This patchs rename the "regex_exec" to "regex_exec2". It add a new
"regex_exec", "regex_exec_match" and "regex_exec_match2" function. This
function can match regex and return array containing matching parts.
Otherwise, this function use the compiled method (JIT or PCRE or POSIX).
JIT require a subject with length. PCREPOSIX and native POSIX regex
require a null terminted subject. The regex_exec* function are splited
in two version. The first version take a null terminated string, but it
execute strlen() on the subject if it is compiled with JIT. The second
version (terminated by "2") take the subject and the length. This
version adds a null character in the subject if it is compiled with
PCREPOSIX or native POSIX functions.
The documentation of posix regex and pcreposix says that the function
returns 0 if the string matche otherwise it returns REG_NOMATCH. The
REG_NOMATCH macro take the value 1 with posix regex and the value 17
with the pcreposix. The documentaion of the native pcre API (used with
JIT) returns a negative number if no match, otherwise, it returns 0 or a
positive number.
This patch fix also the return codes of the regex_exec* functions. Now,
these function returns true if the string match, otherwise it returns
false.