]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
10 years agoMEDIUM: checks: provide environment variables to the external checks
Cyril Bonté [Tue, 2 Dec 2014 20:21:36 +0000 (21:21 +0100)] 
MEDIUM: checks: provide environment variables to the external checks

The external command accepted 4 arguments, some with the value "NOT_USED" when
not applicable. In order to make the exernal command more generic, this patch
also provides the values in environment variables. This allows to provide more
information.

Currently, the supported environment variables are :

PATH, as previously provided.

HAPROXY_PROXY_NAME, the backend name
HAPROXY_PROXY_ID, the backend id
HAPROXY_PROXY_ADDR, the first bind address if available (or empty)
HAPROXY_PROXY_PORT, the first bind port if available (or empty)

HAPROXY_SERVER_NAME, the server name
HAPROXY_SERVER_ID, the server id
HAPROXY_SERVER_ADDR, the server address
HAPROXY_SERVER_PORT, the server port if available (or empty)
HAPROXY_SERVER_MAXCONN, the server max connections
HAPROXY_SERVER_CURCONN, the current number of connections on the server

10 years agoMINOR: checks: allow external checks in backend sections
Cyril Bonté [Tue, 2 Dec 2014 20:21:35 +0000 (21:21 +0100)] 
MINOR: checks: allow external checks in backend sections

Previously, external checks required to find at least one listener in order to
pass the <proxy_address> and <proxy_port> arguments to the external script.
It prevented from declaring external checks in backend sections and haproxy
rejected the configuration.

The listener is now optional and values "NOT_USED" are passed if no listener is
found. For instance, this is the case with a backend section.

This is specific to the 1.6 branch.

10 years agoBUG/MEDIUM: payload: ensure that a request channel is available
Willy Tarreau [Wed, 26 Nov 2014 12:24:24 +0000 (13:24 +0100)] 
BUG/MEDIUM: payload: ensure that a request channel is available

Denys Fedoryshchenko reported a segfault when using certain
sample fetch functions in the "tcp-request connection" rulesets
despite the warnings. This is because some tests for the existence
of the channel were missing.

The fetches which were fixed are :
  - req.ssl_hello_type
  - rep.ssl_hello_type
  - req.ssl_sni

This fix must be backported to 1.5.

10 years agoBUG/MEDIUM: patterns: previous fix was incomplete
Willy Tarreau [Wed, 26 Nov 2014 12:17:03 +0000 (13:17 +0100)] 
BUG/MEDIUM: patterns: previous fix was incomplete

Dmitry Sivachenko <trtrmitya@gmail.com> reported that commit 315ec42
("BUG/MEDIUM: pattern: don't load more than once a pattern list.")
relies on an uninitialised variable in the stack. While it used to
work fine during the tests, if the uninitialized variable is non-null,
some patterns may be aggregated if loaded multiple times, resulting in
slower processing, which was the original issue it tried to address.

The fix needs to be backported to 1.5.

10 years agoBUG/MAJOR: sessions: unlink session from list on out of memory
Willy Tarreau [Tue, 25 Nov 2014 16:10:33 +0000 (17:10 +0100)] 
BUG/MAJOR: sessions: unlink session from list on out of memory

Since embryonic sessions were introduced in 1.5-dev12 with commit
2542b53 ("MAJOR: session: introduce embryonic sessions"), a major
bug remained present. If haproxy cannot allocate memory during
session_complete() (for example, no more buffers), it will not
unlink the new session from the sessions list. This will cause
memory corruptions if the memory area from the session is reused
for anything else, and may also cause bogus output on "show sess"
on the CLI.

This fix must be backported to 1.5.

10 years agoMINOR: samples: add the word converter.
Emeric Brun [Tue, 25 Nov 2014 13:09:01 +0000 (14:09 +0100)] 
MINOR: samples: add the word converter.

word(<index>,<delimiters>)
  Extracts the nth word considering given delimiters from an input string.
  Indexes start at 1 and delimiters are a string formatted list of chars.

10 years agoDEBUG: pools: apply poisonning on every allocated pool
Willy Tarreau [Tue, 25 Nov 2014 12:45:16 +0000 (13:45 +0100)] 
DEBUG: pools: apply poisonning on every allocated pool

Till now, when memory poisonning was enabled, it used to be done only
after a calloc(). But sometimes it's not enough to detect unexpected
sharing, so let's ensure that we now poison every allocation once it's
in place. Note that enabling poisonning significantly hurts performance
(it can typically half the overall performance).

10 years agoMINOR: samples: adds the field converter.
Emeric Brun [Mon, 3 Nov 2014 16:07:03 +0000 (17:07 +0100)] 
MINOR: samples: adds the field converter.

field(<index>,<delimiters>)
  Extracts the substring at the given index considering given delimiters from
  an input string. Indexes start at 1 and delimiters are a string formatted
  list of chars.

10 years agoMINOR: samples: adds the bytes converter.
Emeric Brun [Mon, 3 Nov 2014 14:32:43 +0000 (15:32 +0100)] 
MINOR: samples: adds the bytes converter.

bytes(<offset>[,<length>])
  Extracts a some bytes from an input binary sample. The result is a
  binary sample starting at an offset (in bytes) of the original sample
  and optionnaly truncated at the given length.

10 years agoMINOR: sample: add a few basic internal fetches (nbproc, proc, stopping)
Willy Tarreau [Mon, 24 Nov 2014 15:02:05 +0000 (16:02 +0100)] 
MINOR: sample: add a few basic internal fetches (nbproc, proc, stopping)

Sometimes, either for debugging or for logging we'd like to have a bit
of information about the running process. Here are 3 new fetches for this :

nbproc : integer
  Returns an integer value corresponding to the number of processes that were
  started (it equals the global "nbproc" setting). This is useful for logging
  and debugging purposes.

proc : integer
  Returns an integer value corresponding to the position of the process calling
  the function, between 1 and global.nbproc. This is useful for logging and
  debugging purposes.

stopping : boolean
  Returns TRUE if the process calling the function is currently stopping. This
  can be useful for logging, or for relaxing certain checks or helping close
  certain connections upon graceful shutdown.

10 years agoBUG/MINOR: samples: fix unnecessary memcopy converting binary to string.
Emeric Brun [Mon, 3 Nov 2014 17:17:10 +0000 (18:17 +0100)] 
BUG/MINOR: samples: fix unnecessary memcopy converting binary to string.

10 years agoBUG/MINOR: peers: the buffer size is global.tune.bufsize, not trash.size
Willy Tarreau [Mon, 24 Nov 2014 09:47:35 +0000 (10:47 +0100)] 
BUG/MINOR: peers: the buffer size is global.tune.bufsize, not trash.size

Currently this is harmless since trash.size is copied from
global.tune.bufsize, but this may soon change when buffers become
more dynamic.

At least for consistency it should be backported to 1.5.

10 years agoBUG/MEDIUM: pattern: don't load more than once a pattern list.
Thierry FOURNIER [Mon, 24 Nov 2014 10:14:42 +0000 (11:14 +0100)] 
BUG/MEDIUM: pattern: don't load more than once a pattern list.

A memory optimization can use the same pattern expression for many
equal pattern list (same parse method, index method and index_smp
method).

The pattern expression is returned by "pattern_new_expr", but this
function dont indicate if the returned pattern is already in use.

So, the caller function reload the list of patterns in addition with
the existing patterns. This behavior is not a problem with tree indexed
pattern, but it grows the lists indexed patterns.

This fix add a "reuse" flag in return of the function "pattern_new_expr".
If the flag is set, I suppose that the patterns are already loaded.

This fix must be backported into 1.5.

10 years agoDOC: fix typo in the body parser documentation for msg.sov
Willy Tarreau [Thu, 14 Aug 2014 17:02:46 +0000 (19:02 +0200)] 
DOC: fix typo in the body parser documentation for msg.sov

"positive" ... "null or positive". The second one is "null or negative".

10 years agoMAJOR: polling: centralize calls to I/O callbacks
Willy Tarreau [Wed, 19 Nov 2014 18:43:05 +0000 (19:43 +0100)] 
MAJOR: polling: centralize calls to I/O callbacks

In order for HTTP/2 not to eat too much memory, we'll have to support
on-the-fly buffer allocation, since most streams will have an empty
request buffer at some point. Supporting allocation on the fly means
being able to sleep inside I/O callbacks if a buffer is not available.

Till now, the I/O callbacks were called from two locations :
  - when processing the cached events
  - when processing the polled events from the poller

This change cleans up the design a bit further than what was started in
1.5. It now ensures that we never call any iocb from the poller itself
and that instead, events learned by the poller are put into the cache.
The benefit is important in terms of stability : we don't have to care
anymore about the risk that new events are added into the poller while
processing its events, and we're certain that updates are processed at
a single location.

To achieve this, we now modify all the fd_* functions so that instead of
creating updates, they add/remove the fd to/from the cache depending on
its state, and only create an update when the polling status reaches a
state where it will have to change. Since the pollers make use of these
functions to notify readiness (using fd_may_recv/fd_may_send), the cache
is always up to date with the poller.

Creating updates only when the polling status needs to change saves a
significant amount of work for the pollers : a benchmark showed that on
a typical TCP proxy test, the amount of updates per connection dropped
from 11 to 1 on average. This also means that the update list is smaller
and has more chances of not thrashing too many CPU cache lines. The first
observed benefit is a net 2% performance gain on the connection rate.

A second benefit is that when a connection is accepted, it's only when
we're processing the cache, and the recv event is automatically added
into the cache *after* the current one, resulting in this event to be
processed immediately during the same loop. Previously we used to have
a second run over the updates to detect if new events were added to
catch them before waking up tasks.

The next gain will be offered by the next steps on this subject consisting
in implementing an I/O queue containing all cached events ordered by priority
just like the run queue, and to be able to leave some events pending there
as long as needed. That will allow us *not* to perform some FD processing
if it's not the proper time for this (typically keep waiting for a buffer
to be allocated if none is available for an recv()). And by only processing
a small bunch of them, we'll allow priorities to take place even at the I/O
level.

As a result of this change, functions fd_alloc_or_release_cache_entry()
and fd_process_polled_events() have disappeared, and the code dedicated
to checking for new fd events after the callback during the poll() loop
was removed as well. Despite the patch looking large, it's mostly a
change of what function is falled upon fd_*() and almost nothing was
added.

10 years agoBUG/MINOR: stats: correctly set the request/response analysers
Willy Tarreau [Thu, 20 Nov 2014 21:23:10 +0000 (22:23 +0100)] 
BUG/MINOR: stats: correctly set the request/response analysers

When enabling stats, response analysers were set on the request
analyser list, which 1) has no effect, and 2) means we don't have
the response analysers properly set.

In practice these response analysers are set when the connection
to the server or applet is established so we don't need/must not
set them here.

Fortunately this bug had no impact since the flags are distinct,
but it definitely is confusing.

It should be backported to 1.5.

10 years agoMAJOR: namespace: add Linux network namespace support
KOVACS Krisztian [Mon, 17 Nov 2014 14:11:45 +0000 (15:11 +0100)] 
MAJOR: namespace: add Linux network namespace support

This patch makes it possible to create binds and servers in separate
namespaces.  This can be used to proxy between multiple completely independent
virtual networks (with possibly overlapping IP addresses) and a
non-namespace-aware proxy implementation that supports the proxy protocol (v2).

The setup is something like this:

net1 on VLAN 1 (namespace 1) -\
net2 on VLAN 2 (namespace 2) -- haproxy ==== proxy (namespace 0)
net3 on VLAN 3 (namespace 3) -/

The proxy is configured to make server connections through haproxy and sending
the expected source/target addresses to haproxy using the proxy protocol.

The network namespace setup on the haproxy node is something like this:

= 8< =
$ cat setup.sh
ip netns add 1
ip link add link eth1 type vlan id 1
ip link set eth1.1 netns 1
ip netns exec 1 ip addr add 192.168.91.2/24 dev eth1.1
ip netns exec 1 ip link set eth1.$id up
...
= 8< =

= 8< =
$ cat haproxy.cfg
frontend clients
  bind 127.0.0.1:50022 namespace 1 transparent
  default_backend scb

backend server
  mode tcp
  server server1 192.168.122.4:2222 namespace 2 send-proxy-v2
= 8< =

A bind line creates the listener in the specified namespace, and connections
originating from that listener also have their network namespace set to
that of the listener.

A server line either forces the connection to be made in a specified
namespace or may use the namespace from the client-side connection if that
was set.

For more documentation please read the documentation included in the patch
itself.

Signed-off-by: KOVACS Tamas <ktamas@balabit.com>
Signed-off-by: Sarkozi Laszlo <laszlo.sarkozi@balabit.com>
Signed-off-by: KOVACS Krisztian <hidden@balabit.com>
10 years agoBUG/MEDIUM: connection: sanitize PPv2 header length before parsing address information
KOVACS Krisztian [Wed, 19 Nov 2014 09:53:20 +0000 (10:53 +0100)] 
BUG/MEDIUM: connection: sanitize PPv2 header length before parsing address information

Previously, if hdr_v2->len was less than the length of the protocol
specific address information we could have read after the end of the
buffer and initialize the sockaddr structure with junk.

Signed-off-by: KOVACS Krisztian <hidden@balabit.com>
[WT: this is only tagged medium since proxy protocol is only used from
 trusted sources]

This must be backported to 1.5.

10 years agoBUG/MAJOR: frontend: initialize capture pointers earlier
Willy Tarreau [Tue, 18 Nov 2014 17:49:19 +0000 (18:49 +0100)] 
BUG/MAJOR: frontend: initialize capture pointers earlier

Denys Fedoryshchenko reported and diagnosed a nasty bug caused by TCP
captures, introduced in late 1.5-dev by commit 18bf01e ("MEDIUM: tcp:
add a new tcp-request capture directive"). The problem is that we're
using the array of capture pointers initially designed for HTTP usage
only, and that this array was only reset when starting to process an
HTTP request. In a tcp-only frontend, the pointers are not reset, and
if the capture pool is shared, we can very well point to whatever other
memory location, resulting in random crashes when tcp-request content
captures are processed.

The fix simply consists in initializing these pointers when the pools
are prepared.

A workaround for existing versions consists in either disabling TCP
captures in tcp-only frontends, or in forcing the frontends to work in
HTTP mode.

Thanks to Denys for the amount of testing and detailed reports.

This fix must be backported to 1.5.

10 years agoBUG/MINOR: config: don't inherit the default balance algorithm in frontends
Willy Tarreau [Tue, 18 Nov 2014 14:04:29 +0000 (15:04 +0100)] 
BUG/MINOR: config: don't inherit the default balance algorithm in frontends

Tom Limoncelli from Stack Exchange reported a minor bug : the frontend
inherits the LB parameters from the defaults sections. The impact is
that if a "balance" directive uses any L7 parameter in the defaults
sections and the frontend is in TCP mode, a warning is emitted about
their incompatibility. The warning is harmless but a valid, sane config
should never cause any warning to be reported.

This fix should be backported into 1.5 and possibly 1.4.

10 years agoMEDIUM: regex: Use pcre_study always when PCRE is used, regardless of JIT
Christian Ruppert [Tue, 18 Nov 2014 12:03:58 +0000 (13:03 +0100)] 
MEDIUM: regex: Use pcre_study always when PCRE is used, regardless of JIT

pcre_study() has been around long before JIT has been added. It also seems to
affect the performance in some cases (positive).

Below I've attached some test restults. The test is based on
http://sljit.sourceforge.net/regex_perf.html (see bottom). It has been modified
to just test pcre_study vs. no pcre_study. Note: This test does not try to
match specific header it's instead run over a larger text with more and less
complex patterns to make the differences more clear.

% ./runtest
'mark.txt' loaded. (Length: 19665221 bytes)
-----------------
Regex: 'Twain'
[pcre-nostudy] time:    14 ms (2388 matches)
[pcre-study] time:    21 ms (2388 matches)
-----------------
Regex: '^Twain'
[pcre-nostudy] time:   109 ms (100 matches)
[pcre-study] time:   109 ms (100 matches)
-----------------
Regex: 'Twain$'
[pcre-nostudy] time:    14 ms (127 matches)
[pcre-study] time:    16 ms (127 matches)
-----------------
Regex: 'Huck[a-zA-Z]+|Finn[a-zA-Z]+'
[pcre-nostudy] time:   695 ms (83 matches)
[pcre-study] time:    26 ms (83 matches)
-----------------
Regex: 'a[^x]{20}b'
[pcre-nostudy] time:    90 ms (12495 matches)
[pcre-study] time:    91 ms (12495 matches)
-----------------
Regex: 'Tom|Sawyer|Huckleberry|Finn'
[pcre-nostudy] time:  1236 ms (3015 matches)
[pcre-study] time:    34 ms (3015 matches)
-----------------
Regex: '.{0,3}(Tom|Sawyer|Huckleberry|Finn)'
[pcre-nostudy] time:  5696 ms (3015 matches)
[pcre-study] time:  5655 ms (3015 matches)
-----------------
Regex: '[a-zA-Z]+ing'
[pcre-nostudy] time:  1290 ms (95863 matches)
[pcre-study] time:  1167 ms (95863 matches)
-----------------
Regex: '^[a-zA-Z]{0,4}ing[^a-zA-Z]'
[pcre-nostudy] time:   136 ms (4507 matches)
[pcre-study] time:   134 ms (4507 matches)
-----------------
Regex: '[a-zA-Z]+ing$'
[pcre-nostudy] time:  1334 ms (5360 matches)
[pcre-study] time:  1214 ms (5360 matches)
-----------------
Regex: '^[a-zA-Z ]{5,}$'
[pcre-nostudy] time:   198 ms (26236 matches)
[pcre-study] time:   197 ms (26236 matches)
-----------------
Regex: '^.{16,20}$'
[pcre-nostudy] time:   173 ms (4902 matches)
[pcre-study] time:   175 ms (4902 matches)
-----------------
Regex: '([a-f](.[d-m].){0,2}[h-n]){2}'
[pcre-nostudy] time:  1242 ms (68621 matches)
[pcre-study] time:   690 ms (68621 matches)
-----------------
Regex: '([A-Za-z]awyer|[A-Za-z]inn)[^a-zA-Z]'
[pcre-nostudy] time:  1215 ms (675 matches)
[pcre-study] time:   952 ms (675 matches)
-----------------
Regex: '"[^"]{0,30}[?!\.]"'
[pcre-nostudy] time:    27 ms (5972 matches)
[pcre-study] time:    28 ms (5972 matches)
-----------------
Regex: 'Tom.{10,25}river|river.{10,25}Tom'
[pcre-nostudy] time:   705 ms (2 matches)
[pcre-study] time:    68 ms (2 matches)

In some cases it's more or less the same but when it's faster than by a huge margin.
It always depends on the pattern, the string(s) to match against etc.

Signed-off-by: Christian Ruppert <c.ruppert@babiel.com>
10 years agoBUG/MEDIUM: checks: fix conflicts between agent checks and ssl healthchecks
Cyril Bonté [Sat, 15 Nov 2014 21:41:27 +0000 (22:41 +0100)] 
BUG/MEDIUM: checks: fix conflicts between agent checks and ssl healthchecks

Lasse Birnbaum Jensen reported an issue when agent checks are used at the same
time as standard healthchecks when SSL is enabled on the server side.

The symptom is that agent checks try to communicate in SSL while it should
manage raw data. This happens because the transport layer is shared between all
kind of checks.

To fix the issue, the transport layer is now stored in each check type,
allowing to use SSL healthchecks when required, while an agent check should
always use the raw_sock implementation.

The fix must be backported to 1.5.

10 years agoMINOR: task: release the task pool when stopping
Willy Tarreau [Thu, 13 Nov 2014 15:57:19 +0000 (16:57 +0100)] 
MINOR: task: release the task pool when stopping

When we're stopping, we're not going to create new tasks anymore, so
let's release the task pool upon each task_free() in order to reduce
memory fragmentation.

10 years agoMINOR: session: release a few other pools when stopping
Willy Tarreau [Thu, 13 Nov 2014 15:46:28 +0000 (16:46 +0100)] 
MINOR: session: release a few other pools when stopping

We currently release all pools when a proxy is stopped, except the
connection, pendconn, and pipe pools. Doing so can improve further
reduce memory usage of old processes, eventhough the connection struct
is quite small, but there are a lot and they can participate to memory
fragmentation. The pipe pool is very small and limited, and not exported
so it's not done here.

10 years agoMEDIUM: ssl: add support for smaller SSL records
Willy Tarreau [Thu, 13 Nov 2014 13:06:52 +0000 (14:06 +0100)] 
MEDIUM: ssl: add support for smaller SSL records

There's a very common openssl patch on the net meant to significantly
reduce openssl's memory usage. This patch has been provided for many
versions now, and it makes sense to add support for it given that it
is very simple. It only requires to add an extra SSL_MODE flag. Just
like for other flags, if the flag is unknown, it's unset. About 44kB
of memory may be saved per SSL session with the patch.

10 years agoBUG/MEDIUM: ssl: force a full GC in case of memory shortage
Willy Tarreau [Thu, 13 Nov 2014 12:48:58 +0000 (13:48 +0100)] 
BUG/MEDIUM: ssl: force a full GC in case of memory shortage

When memory becomes scarce and openssl refuses to allocate a new SSL
session, it is worth freeing the pools and trying again instead of
rejecting all incoming SSL connection. This can happen when some
memory usage limits have been assigned to the haproxy process using
-m or with ulimit -m/-v.

This is mostly an enhancement of previous fix and is worth backporting
to 1.5.

10 years agoBUG/MEDIUM: ssl: fix bad ssl context init can cause segfault in case of OOM.
Emeric Brun [Wed, 12 Nov 2014 16:35:37 +0000 (17:35 +0100)] 
BUG/MEDIUM: ssl: fix bad ssl context init can cause segfault in case of OOM.

Some SSL context's init functions errors were not handled and
can cause a segfault due to an incomplete SSL context
initialization.

This fix must be backported to 1.5.

10 years agoBUILD: fix "make install" to support spaces in the install dirs
Arcadiy Ivanov [Tue, 4 Nov 2014 12:06:13 +0000 (07:06 -0500)] 
BUILD: fix "make install" to support spaces in the install dirs

Makefile is unable to install into directories containing spaces.

10 years agoBUG/MAJOR: buffer: check the space left is enough or not when input data in a buffer...
Godbach [Fri, 31 Oct 2014 05:16:37 +0000 (13:16 +0800)] 
BUG/MAJOR: buffer: check the space left is enough or not when input data in a buffer is wrapped

HAProxy will crash with the following configuration:

global
    ...
    tune.bufsize 1024
    tune.maxrewrite 0
frontend xxx
    ...
backend  yyy
   ...
   cookie cookie insert maxidle 300s

If client sends a request of which object size is more than tune.bufsize (1024
bytes), HAProxy will crash.

After doing some debugging, the crash was caused by http_header_add_tail2() ->
buffer_insert_line2() while inserting cookie at the end of response header.
Part codes of buffer_insert_line2() are as below:

int buffer_insert_line2(struct buffer *b, char *pos, const char *str, int len)
{
    int delta;

    delta = len + 2;

    if (bi_end(b) + delta >= b->data + b->size)
        return 0;  /* no space left */

    /* first, protect the end of the buffer */
    memmove(pos + delta, pos, bi_end(b) - pos);
    ...
}

Since tune.maxrewrite is 0, HAProxy can receive 1024 bytes once which is equals
to full buffer size.  Under such condition, the buffer is full and bi_end(b)
will be wrapped to the start of buffer which pointed to b->data. As a result,
though there is no space left in buffer, the check condition
if (bi_end(b) + delta >= b->data + b->size)
will be true, then memmove() is called, and (pos + delta) will exceed the end
of buffer (b->data + b->size), HAProxy crashes

Just take buffer_replace2() as a reference, the other check when input data in
a buffer is wrapped should be also added into buffer_insert_line2().

This fix must be backported to 1.5.

Signed-off-by: Godbach <nylzhaowei@gmail.com>
10 years agoBUG/BUILD: revert accidental change in the makefile from latest SSL fix
Willy Tarreau [Fri, 31 Oct 2014 06:36:51 +0000 (07:36 +0100)] 
BUG/BUILD: revert accidental change in the makefile from latest SSL fix

Commit 0bed994 ("BUG/MINOR: ssl: correctly initialize ssl ctx for
invalid certificates") accidently left a change in the Makefile
resulting in -ldl being appended to the LDFLAGS. As reported by
Dmitry Sivachenko, this will break build on systems without libdl
such as FreeBSD.

This fix must be backported to 1.5.

10 years agoBUG/MINOR: ssl: correctly initialize ssl ctx for invalid certificates
Emeric Brun [Thu, 30 Oct 2014 18:25:24 +0000 (19:25 +0100)] 
BUG/MINOR: ssl: correctly initialize ssl ctx for invalid certificates

Bug reported by John Leach: no-sslv3 does not work using some certificates.

It appears that ssl ctx is not updated with configured options if the
CommonName of the certificate's subject is not found.

It applies only on the first cerificate of a configured bind line.

There is no security impact, because only invalid nameless certficates
are concerned.

This fix must be backported to 1.5

10 years agoMINOR: ssl: add statement to force some ssl options in global.
Emeric Brun [Thu, 30 Oct 2014 14:56:50 +0000 (15:56 +0100)] 
MINOR: ssl: add statement to force some ssl options in global.

Adds global statements 'ssl-default-server-options' and
'ssl-default-bind-options' to force on 'server' and 'bind' lines
some ssl options.

Currently available options are 'no-sslv3', 'no-tlsv10', 'no-tlsv11',
'no-tlsv12', 'force-sslv3', 'force-tlsv10', 'force-tlsv11',
'force-tlsv12', and 'no-tls-tickets'.

Example:
      global
        ssl-default-server-options no-sslv3
        ssl-default-bind-options no-sslv3

10 years agoBUG/MEDIUM: tcp: don't use SO_ORIGINAL_DST on non-AF_INET sockets
Willy Tarreau [Wed, 29 Oct 2014 20:46:01 +0000 (21:46 +0100)] 
BUG/MEDIUM: tcp: don't use SO_ORIGINAL_DST on non-AF_INET sockets

There's an issue when using SO_ORIGINAL_DST to retrieve the original
destination of a connection's address before being translated by
Netfilter's DNAT/REDIRECT or the old TPROXY. SO_ORIGINAL_DST is
able to retrieve an IPv4 address when the original destination was
IPv4 mapped into IPv6. At first glance it's not a big deal, but it
is for logging and for the proxy protocol, because we then have
two different address families for the source and destination. In
this case, the proxy protocol correctly detects the issue and emits
"UNKNOWN".

In order to fix this, we perform getsockname() first, and only if
the address family is AF_INET, then we perform the getsockopt() call.

This fix must be backported to 1.5, and probably even to 1.4 and 1.3.

10 years agoMINOR: add fetchs 'ssl_c_der' and 'ssl_f_der' to return DER formatted certs
Emeric Brun [Wed, 29 Oct 2014 18:03:26 +0000 (19:03 +0100)] 
MINOR: add fetchs 'ssl_c_der' and 'ssl_f_der' to return DER formatted certs

ssl_c_der : binary
  Returns the DER formatted certificate presented by the client when the
  incoming connection was made over an SSL/TLS transport layer. When used for
  an ACL, the value(s) to match against can be passed in hexadecimal form.

ssl_f_der : binary
  Returns the DER formatted certificate presented by the frontend when the
  incoming connection was made over an SSL/TLS transport layer. When used for
  an ACL, the value(s) to match against can be passed in hexadecimal form.

10 years agoBUG/MEDIUM: regex: fix pcre_study error handling
Christian Ruppert [Wed, 29 Oct 2014 16:05:53 +0000 (17:05 +0100)] 
BUG/MEDIUM: regex: fix pcre_study error handling

pcre_study() may return NULL even though it succeeded. In this case error is
NULL otherwise error is not NULL. Also see man 3 pcre_study.

Previously a ACL pattern of e.g. ".*" would cause error because pcre_study did
not found anything to speed up matching and returned regex->extra = NULL and
error = NULL which in this case was a false-positive. That happend only when
PCRE_JIT was enabled for HAProxy but libpcre has been built without JIT.

Signed-off-by: Christian Ruppert <c.ruppert@babiel.com>
[wt: this needs to be backported to 1.5 as well]

10 years agoBUILD/MINOR: ssl: de-constify "ciphers" to avoid a warning on openssl-0.9.8
Willy Tarreau [Sun, 26 Oct 2014 05:49:19 +0000 (06:49 +0100)] 
BUILD/MINOR: ssl: de-constify "ciphers" to avoid a warning on openssl-0.9.8

When building on openssl-0.9.8, since commit 23d5d37 ("MINOR: ssl: use
SSL_get_ciphers() instead of directly accessing the cipher list.") we get
the following warning :

src/ssl_sock.c: In function 'ssl_sock_prepare_ctx':
src/ssl_sock.c:1592: warning: passing argument 1 of 'SSL_CIPHER_description' discards qualifiers from pointer target type

This is because the openssl API has changed between 0.9.8 and 1.0.1 :

0.9.8: char *SSL_CIPHER_description(SSL_CIPHER *,char *buf,int size);
1.0.1: char *SSL_CIPHER_description(const SSL_CIPHER *,char *buf,int size);

So let's remove the "const" type qualifier to satisfy both versions.
Note that the fix above was backported to 1.5, so this one should as well.

10 years agoMINOR: sample: add "json" converter
Thierry FOURNIER [Tue, 12 Aug 2014 08:20:47 +0000 (10:20 +0200)] 
MINOR: sample: add "json" converter

This converter escapes string to use it as json/ascii escaped string.
It can read UTF-8 with differents behavior on errors and encode it in
json/ascii.

json([<input-code>])
  Escapes the input string and produces an ASCII ouput string ready to use as a
  JSON string. The converter tries to decode the input string according to the
  <input-code> parameter. It can be "ascii", "utf8", "utf8s", "utf8"" or
  "utf8ps". The "ascii" decoder never fails. The "utf8" decoder detects 3 types
  of errors:
   - bad UTF-8 sequence (lone continuation byte, bad number of continuation
     bytes, ...)
   - invalid range (the decoded value is within a UTF-8 prohibited range),
   - code overlong (the value is encoded with more bytes than necessary).

  The UTF-8 JSON encoding can produce a "too long value" error when the UTF-8
  character is greater than 0xffff because the JSON string escape specification
  only authorizes 4 hex digits for the value encoding. The UTF-8 decoder exists
  in 4 variants designated by a combination of two suffix letters : "p" for
  "permissive" and "s" for "silently ignore". The behaviors of the decoders
  are :
   - "ascii"  : never fails ;
   - "utf8"   : fails on any detected errors ;
   - "utf8s"  : never fails, but removes characters corresponding to errors ;
   - "utf8p"  : accepts and fixes the overlong errors, but fails on any other
                error ;
   - "utf8ps" : never fails, accepts and fixes the overlong errors, but removes
                characters corresponding to the other errors.

  This converter is particularly useful for building properly escaped JSON for
  logging to servers which consume JSON-formated traffic logs.

  Example:
     capture request header user-agent len 150
     capture request header Host len 15
     log-format {"ip":"%[src]","user-agent":"%[capture.req.hdr(1),json]"}

  Input request from client 127.0.0.1:
     GET / HTTP/1.0
     User-Agent: Very "Ugly" UA 1/2

  Output log:
     {"ip":"127.0.0.1","user-agent":"Very \"Ugly\" UA 1\/2"}

10 years agoBUG/MEDIUM: tcp: fix outgoing polling based on proxy protocol
Willy Tarreau [Fri, 24 Oct 2014 10:02:24 +0000 (12:02 +0200)] 
BUG/MEDIUM: tcp: fix outgoing polling based on proxy protocol

During a tcp connection setup in tcp_connect_server(), we check if
there are pending data to start polling for writes immediately. We
also use the same test to know if we can disable the quick ack and
merge the first data packet with the connection's ACK. This last
case is also valid for the proxy protocol.

The problem lies in the way it's done, as the "data" variable is
improperly completed with the presence of the proxy protocol, resulting
in the connection being polled for data writes if the proxy protocol is
enabled. It's not a big issue per se, except that the proxy protocol
uses the fact that we're polling for data to know if it can use MSG_MORE.

This causes no problem on HTTP/HTTPS, but with banner protocols, it
introduces a 200ms delay if the server waits for the PROXY header.

This has been caused by the connection management changes introduced in
1.5-dev12, specifically commit a1a7474 ("MEDIUM: proxy-proto: don't use
buffer flags in conn_si_send_proxy()"), so this fix must be backported
to 1.5.

10 years agoBUG/MINOR: log: fix request flags when keep-alive is enabled
Cyril Bonté [Wed, 22 Oct 2014 20:30:13 +0000 (22:30 +0200)] 
BUG/MINOR: log: fix request flags when keep-alive is enabled

Colin Ingarfield reported some unexplainable flags in the logs.
For example, a "LR" termination state was set on a request which was forwarded
to a server, where "LR" means that the request should have been handled
internally by haproxy.

This case happens when at least client side keep-alive is enabled. Next
requests in the connection will inherit the flags from the previous request.

2 fields are impacted : "termination_state" and "Tt" in the timing events,
where a "+" can be added, when a previous request was redispatched.

This is not critical for the service itself but can confuse troubleshooting.

The fix must be backported to 1.5 and 1.4.

10 years agoBUG/MAJOR: cli: explicitly call cli_release_handler() upon error
Willy Tarreau [Wed, 22 Oct 2014 17:06:31 +0000 (19:06 +0200)] 
BUG/MAJOR: cli: explicitly call cli_release_handler() upon error

Dmitry Sivachenko reported an embarrassing problem where haproxy
would sometimes segfault upon reload. After careful analysis and
code inspection, what happens is related to the "show sess" command
on the CLI, and it is not limited to reload operations only.

When a "show sess" is running, once the output buffer is full, the
stats applet grabs a reference to the session being dumped in order
for the current pointer to be able to advance by itself should this
session disappear while the buffer is full. The applet also uses a
release handler that is called when the applet terminates to release
such references.

The problem is that upon error, the command line parser sets the
applet state to STAT_CLI_O_END indicating it wants to terminate the
processing. Unfortunately, the release handler which is called later
to clean everything up relies on the applet's state to know what
operations were in progress, and as such it does not release the
reference. A later "show sess" or the completion of the task being
watched lead to a LIST_DEL() on the task's list which point to a
location that does not match the applet's reference list anymore
and the process dies.

One solution to this would be to add a flag to the current applet's
state mentionning it must leave, without affecting the state indicating
the current operation. It's a bit invasive but could be the long term
solution. The short term solution simply consists in calling the
release handler just before changing the state to STAT_CLI_O_END.
That way everything that must be released is released in time.

Note that the probability to encounter this issue is very low.
It requires a lot of "show sess" or "show sess all" calls, and
that one of them dies before being completed. That can happen
if "show sess" is run in scripts which truncate the output (eg:
"echo show sess|socat|head"). This could be the worst case as it
almost ensures that haproxy fills a buffer, grabs a reference and
detects the error on the socket.

There's no config-based workaround to this issue, except refraining
from issuing "show sess" on large connection counts or "show sess all".
If that's not possible to block everyone, restricting permissions on
the stats socket ensures only authorized tools can connect.

This fix must be backported to 1.5 and to 1.4 (with some changes in
1.4 since the release function does not exist so the LIST_DEL sequence
must be open-coded).

Special thanks to Dmitry for the fairly complete report.

10 years agoBUG/MEDIUM: http: don't dump debug headers on MSG_ERROR
Willy Tarreau [Tue, 21 Oct 2014 17:36:09 +0000 (19:36 +0200)] 
BUG/MEDIUM: http: don't dump debug headers on MSG_ERROR

When the HTTP parser is in state HTTP_MSG_ERROR, we don't know if it was
already initialized or not. If the error happens before HTTP_MSG_RQBEFORE,
random offsets might be present and we don't want to display such random
strings in debug mode.

While it's theorically possible to randomly crash the process when running
in debug mode here, this bug was not tagged MAJOR because it would not
make sense to run in debug mode in production.

This fix must be backported to 1.5 and 1.4.

10 years agoMINOR: ssl: use SSL_get_ciphers() instead of directly accessing the cipher list.
Remi Gacogne [Fri, 10 Oct 2014 15:04:26 +0000 (17:04 +0200)] 
MINOR: ssl: use SSL_get_ciphers() instead of directly accessing the cipher list.

10 years agoBUG/MEDIUM: backend: fix URI hash when a query string is present
Willy Tarreau [Fri, 17 Oct 2014 10:11:50 +0000 (12:11 +0200)] 
BUG/MEDIUM: backend: fix URI hash when a query string is present

Commit 98634f0 ("MEDIUM: backend: Enhance hash-type directive with an
algorithm options") cleaned up the hashing code by using a centralized
function. A bug appeared in get_server_uh() which is the URI hashing
function. Prior to the patch, the function would stop hashing on the
question mark, or on the trailing slash of a maximum directory count.
Consecutive to the patch, this last character is included into the
hash computation. This means that :

    GET /0
    GET /0?

Are not hashed similarly. The following configuration reproduces it :

    mode http
    balance uri
    server s1 0.0.0.0:1234 redir /s1
    server s2 0.0.0.0:1234 redir /s2

Many thanks to Vedran Furac for reporting this issue. The fix must
be backported to 1.5.

10 years agoBUG/MINOR: config: do not accept more track-sc than configured
Willy Tarreau [Fri, 17 Oct 2014 09:53:05 +0000 (11:53 +0200)] 
BUG/MINOR: config: do not accept more track-sc than configured

MAX_SESS_STKCTR allows one to define the number of stick counters that can
be used in parallel in track-sc* rules. The naming of this macro creates
some confusion because the value there is sometimes used as a max instead
of a count, and the config parser accepts values from 0 to MAX_SESS_STKCTR
and the processing ignores anything tracked on the last one. This means
that by default, track-sc3 is allowed and ignored.

This fix must be backported to 1.5 where the problem there only affects
TCP rules.

10 years agoMINOR: systemd: Check configuration before start
Kristoffer Grönlund [Thu, 9 Oct 2014 14:51:29 +0000 (16:51 +0200)] 
MINOR: systemd: Check configuration before start

Adds a configuration check before starting the haproxy service.

10 years agoBUG/MEDIUM: config: avoid skipping disabled proxies
Willy Tarreau [Fri, 10 Oct 2014 12:54:25 +0000 (14:54 +0200)] 
BUG/MEDIUM: config: avoid skipping disabled proxies

Paul Taylor and Bryan Talbot found that after commit 419ead8 ("MEDIUM:
config: compute the exact bind-process before listener's maxaccept"),
a backend marked "disabled" would cause the next backend to be skipped
and if it was the last one it would cause a segfault.

The reason is that the commit above changed the "while" loop for a "for"
loop but a "continue" statement still incrementing the current proxy was
left in the code for disabled proxies, causing the next one to be skipped
as well and the last one to try to dereference NULL when seeking ->next.

The quick workaround consists in not disabling backends, or adding an
empty dummy one after a disabled section.

This fix must be backported to 1.5.

10 years agoBUG/MEDIUM: systemd: set KillMode to 'mixed'
Apollon Oikonomopoulos [Wed, 8 Oct 2014 12:14:41 +0000 (15:14 +0300)] 
BUG/MEDIUM: systemd: set KillMode to 'mixed'

By default systemd will send SIGTERM to all processes in the service's
control group. In our case, this includes the wrapper, the master
process and all worker processes.

Since commit c54bdd2a the wrapper actually catches SIGTERM and survives
to see the master process getting killed by systemd and regard this as
an error, placing the unit in a failed state during "systemctl stop".

Since the wrapper now handles SIGTERM by itself, we switch the kill mode
to 'mixed', which means that systemd will deliver the initial SIGTERM to
the wrapper only, and if the actual haproxy processes don't exit after a
given amount of time (default: 90s), a SIGKILL is sent to all remaining
processes in the control group. See systemd.kill(5) for more
information.

This should also be backported to 1.5.

10 years agoDOC: indicate that weight zero is reported as DRAIN
Willy Tarreau [Tue, 7 Oct 2014 13:27:33 +0000 (15:27 +0200)] 
DOC: indicate that weight zero is reported as DRAIN

It's not the same state but reported as such.

10 years agoDOC: Address issue where documentation is excluded due to a gitignore rule.
Andrew Latham [Tue, 7 Oct 2014 12:45:54 +0000 (07:45 -0500)] 
DOC: Address issue where documentation is excluded due to a gitignore rule.

10 years agoBUG/MINOR: config: don't propagate process binding for dynamic use_backend
Cyril Bonté [Thu, 2 Oct 2014 17:56:25 +0000 (19:56 +0200)] 
BUG/MINOR: config: don't propagate process binding for dynamic use_backend

A segfault was reported with the introduction of the propagate_processes()
function. It was caused when a use_backend rule was declared with a dynamic
name, using a log-format string. The backend is not resolved during the
configuration, which lead to the segfault.

The patch prevents the process binding propagation for such dynamic rules, it
should also be backported to 1.5.

10 years agoBUG/MINOR: tcp-check: report the correct failed step in the status
Willy Tarreau [Thu, 2 Oct 2014 12:51:02 +0000 (14:51 +0200)] 
BUG/MINOR: tcp-check: report the correct failed step in the status

The step number was reported by checking only last_started_step, which
was not set in case of error during the initial connection phase, and
caused "step 1" to be returned with an invalid check type (typically
SEND). So now we first verify that a test was started before returning
this.

In addition to this, the indication of the test type was taken from
current_step instead of last_started_step, so the error description
was matching the next action instead of the one reported in the step
ID. Thus we could get the confusing "step 1 (send)" report below :

      tcp-check connect
      tcp-check send foo

In order to ease debugging, when the port number is known for a connect,
it is indicated in the error report.

Note that this only affects asynchronous error messages, synchronous ones
are correct.

This fix must be backported to 1.5.

10 years agoBUG/MEDIUM: check: rule-less tcp-check must detect connect failures
Willy Tarreau [Thu, 2 Oct 2014 12:30:14 +0000 (14:30 +0200)] 
BUG/MEDIUM: check: rule-less tcp-check must detect connect failures

When "option tcp-check" is specified without any tcp-check rules, the
documentation says that it's the same as the default check method. But
the code path is a bit different, and we used to consider that since
the end of rules was reached, the check is always successful regardless
of the connection status.

This patch reorganizes the error detection, and considers the special
case where there's no tcp-check rule as a real L4 check. It also avoids
dereferencing the rule list head as a rule by itself.

While fixing this bug, another one related to the output messages'
accuracy was noticed, it will be fixed in a separate commit and is
much less important.

This bug is also present in 1.5, so this fix must be backported.

10 years agoBUG/MINOR: config: don't propagate process binding on fatal errors.
Willy Tarreau [Wed, 1 Oct 2014 18:50:17 +0000 (20:50 +0200)] 
BUG/MINOR: config: don't propagate process binding on fatal errors.

propagate_processes() must not be called with unresolved proxies, but
nothing prevents it from being called in check_config_validity(). The
resulting effect is that an unresolved proxy can cause a recursion
loop if called in such a situation, ending with a segfault after the
fatal error report. There's no side effect beyond this.

This patch refrains from calling the function when any error was met.

This bug also affects 1.5, it should be backported.

10 years agoBUG/MEDIUM: http: adjust close mode when switching to backend
Willy Tarreau [Tue, 30 Sep 2014 16:44:22 +0000 (18:44 +0200)] 
BUG/MEDIUM: http: adjust close mode when switching to backend

Commit 179085c ("MEDIUM: http: move Connection header processing earlier")
introduced a regression : the backend's HTTP mode is not considered anymore
when setting the session's HTTP mode, because wait_for_request() is only
called once, when the frontend receives the request (or when the frontend
is in TCP mode, when the backend receives the request).

The net effect is that in some situations when the frontend and the backend
do not work in the same mode (eg: keep-alive vs close), the backend's mode
is ignored.

This patch moves all that processing to a dedicated function, which is
called from the original place, as well as from session_set_backend()
when switching from an HTTP frontend to an HTTP backend in different
modes.

This fix must be backported to 1.5.

10 years agoBUG/MEDIUM: remove debugging code from systemd-wrapper
Willy Tarreau [Wed, 24 Sep 2014 10:59:25 +0000 (12:59 +0200)] 
BUG/MEDIUM: remove debugging code from systemd-wrapper

Kristoffer Grönlund reported that after my recent update to the
systemd-wrapper, I accidentely left the debugging code which
consists in disabling the fork :-(

The fix needs to be backported to 1.5 as well since I pushed it
there as well.

10 years agoMEDIUM: systemd-wrapper: support multiple executable versions and names
Willy Tarreau [Fri, 19 Sep 2014 13:42:30 +0000 (15:42 +0200)] 
MEDIUM: systemd-wrapper: support multiple executable versions and names

Having to use a hard-coded "haproxy" executable name next to the systemd
wrapper is not always convenient, as it's sometimes desirable to run with
multiple versions in parallel.

Thus this patch performs a minor change to the wrapper : if the name ends
with "-systemd-wrapper", then it trims that part off and what remains
becomes the target haproxy executable. That makes it easy to have for
example :

     haproxy-1.5.4-systemd-wrapper      haproxy-1.5.4
     haproxy-1.5.3-systemd-wrapper      haproxy-1.5.3

and so on, in a same directory.

This patch also fixes a rare bug caused by readlink() not adding the
trailing zero and leaving possible existing contents, including possibly
a randomly placed "/" which would make it unable to locate the correct
binary. This case is not totally unlikely as I got a \177 a few times
at the end of the executable names, so I could have got a '/' as well.

Back-porting to 1.5 is desirable.

10 years agoMINOR: config: detect the case where a tcp-request content rule has no inspect-delay
Willy Tarreau [Tue, 16 Sep 2014 14:21:19 +0000 (16:21 +0200)] 
MINOR: config: detect the case where a tcp-request content rule has no inspect-delay

If a frontend has any tcp-request content rule relying on request contents
without any inspect delay, we now emit a warning as this will randomly match.

This can be backported to 1.5 as it reduces the support effort.

10 years agoDOC: indicate in the doc that track-sc* can wait if data are missing
Willy Tarreau [Tue, 16 Sep 2014 13:48:15 +0000 (15:48 +0200)] 
DOC: indicate in the doc that track-sc* can wait if data are missing

Since commit 1b71eb5 ("BUG/MEDIUM: counters: fix track-sc* to wait on
unstable contents"), we don't need the "if HTTP" anymore. But the doc
was not updated to reflect this.

Since this change was backported to 1.5, this doc update should be
backported as well.

10 years agoMEDIUM: config: report it when tcp-request rules are misplaced
Willy Tarreau [Tue, 16 Sep 2014 13:39:51 +0000 (15:39 +0200)] 
MEDIUM: config: report it when tcp-request rules are misplaced

A config where a tcp-request rule appears after an http-request rule
might seem valid but it is not. So let's report a warning about this
since this case is hard to detect by the naked eye.

10 years agoMEDIUM: config: only warn if stats are attached to multi-process bind directives
Willy Tarreau [Tue, 16 Sep 2014 13:11:04 +0000 (15:11 +0200)] 
MEDIUM: config: only warn if stats are attached to multi-process bind directives

Some users want to have a stats frontend with one line per process, but while
100% valid and safe, the config parser emits a warning. Relax this check to
ensure that the warning is only emitted if at least one of the listeners is
bound to multiple processes, or if the directive is placed in a backend called
from multiple processes (since in this case we don't know if it's safe).

10 years agoMEDIUM: config: compute the exact bind-process before listener's maxaccept
Willy Tarreau [Tue, 16 Sep 2014 11:41:21 +0000 (13:41 +0200)] 
MEDIUM: config: compute the exact bind-process before listener's maxaccept

This is a continuation of previous patch, the listener's maxaccept is divided
by the number of processes, so it's best if we can swap the two blocks so that
the number of processes is already known when computing the maxaccept value.

10 years agoMEDIUM: config: make the frontends automatically bind to the listeners' processes
Willy Tarreau [Tue, 16 Sep 2014 11:21:03 +0000 (13:21 +0200)] 
MEDIUM: config: make the frontends automatically bind to the listeners' processes

When a frontend does not have any bind-process directive, make it
automatically bind to the union of all of its listeners' processes
instead of binding to all processes. That will make it possible to
have the expected behaviour without having to explicitly specify a
bind-process directive.

Note that if the listeners are not bound to a specific process, the
default is still to bind to all processes.

This change could be backported to 1.5 as it simplifies process
management, and was planned to be done during the 1.5 development phase.

10 years agoMEDIUM: config: properly propagate process binding between proxies
Willy Tarreau [Tue, 16 Sep 2014 10:17:36 +0000 (12:17 +0200)] 
MEDIUM: config: properly propagate process binding between proxies

We now recursively propagate the bind-process values between frontends
and backends instead of doing it during name resolving. This ensures
that we're able to properly propagate all the bind-process directives
even across "listen" instances, which are not perfectly covered at the
moment, depending on the declaration order.

10 years agoBUG/MEDIUM: config: propagate frontend to backend process binding again.
Willy Tarreau [Tue, 16 Sep 2014 09:31:31 +0000 (11:31 +0200)] 
BUG/MEDIUM: config: propagate frontend to backend process binding again.

This basically reverts 3507d5d ("MEDIUM: proxy: only adjust the backend's
bind-process when already set"). It was needed during the transition to
the new process binding method but is causing trouble now because frontend
to backend binding is not properly propagated.

This fix should be backported to 1.5.

10 years agoMEDIUM: http: enable header manipulation for 101 responses
Willy Tarreau [Tue, 16 Sep 2014 08:40:38 +0000 (10:40 +0200)] 
MEDIUM: http: enable header manipulation for 101 responses

Ryan Brock reported that server stickiness did not work for WebSocket
because the cookies and headers are not modified on 1xx responses. He
found that his browser correctly presents the cookies learned on 101
responses, which was not specifically defined in the WebSocket spec,
nor in the cookie spec. 101 is a very special case. Being part of 1xx,
it's an interim response. But within 1xx, it's special because it's
the last HTTP/1 response that transits on the wire, which is different
from 100 or 102 which may appear multiple times. So in that sense, we
can consider it as a final response regarding HTTP/1, and it makes
sense to allow header processing there. Note that we still ensure not
to mangle the Connection header, which is critical for HTTP upgrade to
continue to work smoothly with agents that are a bit picky about what
tokens are found there.

The rspadd rules are now processed for 101 responses as well, but the
cache-control checks are not performed (since no body is delivered).

Ryan confirmed that this patch works for him.

It would make sense to backport it to 1.5 given that it improves end
user experience on WebSocket servers.

10 years agoMINOR: Also accept SIGHUP/SIGTERM in systemd-wrapper
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. :)

10 years agoMINOR: stats: fix minor typo fix in stats_dump_errors_to_buffer()
Olivier Doucet [Mon, 8 Sep 2014 09:23:00 +0000 (11:23 +0200)] 
MINOR: stats: fix minor typo fix in stats_dump_errors_to_buffer()

Remove the space before the colon to match the format used in the frontend.

10 years agoDOC: missing track-sc* in http-request rules
Baptiste Assmann [Wed, 3 Sep 2014 16:29:47 +0000 (18:29 +0200)] 
DOC: missing track-sc* in http-request rules

track-sc is well defined in http-request rules, but not listed in
option list.
This patch fix this miss.

10 years agoDOC: clearly state that the "show sess" output format is not fixed
Olivier [Fri, 5 Sep 2014 16:49:10 +0000 (18:49 +0200)] 
DOC: clearly state that the "show sess" output format is not fixed

It requires to look at the code (src/dumpstats.c) since the format may
change at any moment.

10 years agoMINOR: deinit: fix memory leak
Sárközi, László [Fri, 5 Sep 2014 08:08:23 +0000 (10:08 +0200)] 
MINOR: deinit: fix memory leak

deinit() did not free the conf.file member of server objects.

10 years agoBUG/CRITICAL: http: don't update msg->sov once data start to leave the buffer
Willy Tarreau [Mon, 1 Sep 2014 18:35:55 +0000 (20:35 +0200)] 
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.

This fix must be backported to haproxy 1.5.

10 years agoBUG/MEDIUM: config: userlists should ensure that encrypted passwords are supported
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.

This patch should be backported to 1.4 and 1.5.

10 years agoBUG/MEDIUM: auth: fix segfault with http-auth and a configuration with an unknown...
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.

The fix should be backported to 1.4 and 1.5.

10 years agoCLEANUP: acl: cleanup some of the redundancy and spaghetti after last fix
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.

10 years agoBUG/MEDIUM: acl: correctly compute the output type when a converter is used
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.

10 years agoBUG/MINOR: pattern: remove useless allocation of unused trash in pat_parse_reg()
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.

10 years agoBUG/MEDIUM: http: fix improper parsing of HTTP methods for use with ACLs
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.

10 years agoBUG/MEDIUM: http: fix inverted condition in pat_match_meth()
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.

10 years agoMINOR: log: add a new field "%lc" to implement a per-frontend log counter
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.

10 years agoOPTIM/MINOR: proxy: reduce struct proxy by 48 bytes on 64-bit archs
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.

10 years agoMEDIUM: connection: add new bit in Proxy Protocol V2
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]

10 years agoBUG/MEDIUM: http: tarpit timeout is reset
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.

This should be backported in 1.5.

10 years agoMINOR: ssl: don't use boringssl's cipher_list
Lukas Tribus [Sun, 17 Aug 2014 22:56:33 +0000 (00:56 +0200)] 
MINOR: ssl: don't use boringssl's cipher_list

Google's boringssl has a different cipher_list, we cannot use it as
in OpenSSL. This is due to the "Equal preference cipher groups" feature:

https://boringssl.googlesource.com/boringssl/+/858a88daf27975f67d9f63e18f95645be2886bfb^!/

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.

Signed-off-by: Lukas Tribus <luky-37@hotmail.com>
10 years agoBUILD: ssl: don't call get_rfc2409_prime when using boringssl
Lukas Tribus [Sun, 17 Aug 2014 22:56:32 +0000 (00:56 +0200)] 
BUILD: ssl: don't call get_rfc2409_prime when using boringssl

get_rfc2409_prime_1024() and friends are not available in Google's
boringssl, so use the fallback in that case.

Signed-off-by: Lukas Tribus <luky-37@hotmail.com>
10 years agoBUILD: ssl: disable OCSP when using boringssl
Lukas Tribus [Sun, 17 Aug 2014 22:56:31 +0000 (00:56 +0200)] 
BUILD: ssl: disable OCSP when using boringssl

Google's boringssl doesn't currently support OCSP, so
disable it if detected.

OCSP support may be reintroduced as per:
https://code.google.com/p/chromium/issues/detail?id=398677

In that case we can simply revert this commit.

Signed-off-by: Lukas Tribus <luky-37@hotmail.com>
10 years agoBUILD: ssl: handle boringssl in openssl version detection
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.

Signed-off-by: Lukas Tribus <luky-37@hotmail.com>
10 years agoBUG: config: error in http-response replace-header number of arguments
Baptiste Assmann [Fri, 8 Aug 2014 15:29:06 +0000 (17:29 +0200)] 
BUG: config: error in http-response replace-header number of arguments

A couple of typo fixed in 'http-response replace-header':
- an error when counting the number of arguments
- a typo in the alert message

This should be backported to 1.5.

10 years agoBUG/MINOR: checks: external checks shouldn't wait for timeout to return the result
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.

This fix is specific to the 1.6 branch.

10 years agoBUG/MEDIUM: checks: segfault with external checks in a backend section
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.

This fix is specific to the 1.6 branch.

10 years agoBUG/MEDIUM: checks: external checks can't change server status to UP
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.

This bug is specific to the 1.6 branch.

10 years agoBUG/MAJOR: tcp: fix a possible busy spinning loop in content track-sc*
Willy Tarreau [Wed, 30 Jul 2014 06:56:35 +0000 (08:56 +0200)] 
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.

10 years agoMEDIUM: Improve signal handling in systemd wrapper.
Conrad Hoffmann [Mon, 28 Jul 2014 21:52:20 +0000 (23:52 +0200)] 
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.

Signed-off-by: Conrad Hoffmann <conrad@soundcloud.com>
10 years agoBUG/MINOR: Fix search for -p argument in systemd wrapper.
Conrad Hoffmann [Mon, 28 Jul 2014 21:22:43 +0000 (23:22 +0200)] 
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.

Signed-off-by: Conrad Hoffmann <conrad@soundcloud.com>
10 years agoBUG/MINOR: server: move the directive #endif to the end of file
Godbach [Mon, 28 Jul 2014 09:31:57 +0000 (17:31 +0800)] 
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.

Signed-off-by: Godbach <nylzhaowei@gmail.com>
11 years agoBUG/MEDIUM: connection: fix proxy v2 header again!
Willy Tarreau [Sat, 19 Jul 2014 04:37:33 +0000 (06:37 +0200)] 
BUG/MEDIUM: connection: fix proxy v2 header again!

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.

11 years agoBUG/MEDIUM: connection: fix memory corruption when building a proxy v2 header
Dave McCowan [Thu, 17 Jul 2014 18:34:01 +0000 (14:34 -0400)] 
BUG/MEDIUM: connection: fix memory corruption when building a proxy v2 header

Use temporary trash chunk, instead of global trash chunk in
make_proxy_line_v2() to avoid memory overwrite.

This fix must also be backported to 1.5.

11 years agoMEDIUM: http: add the track-sc* actions to http-request rules
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.

11 years agoCLEANUP: session: move the stick counters declarations to stick_table.h
Willy Tarreau [Wed, 25 Jun 2014 16:30:01 +0000 (18:30 +0200)] 
CLEANUP: session: move the stick counters declarations to stick_table.h

They're really not appropriate in session.h as they always require a
stick table, and I'm having a hard time finding them each time I need
to.

11 years agoBUILD: report commit ID in git versions as well
Willy Tarreau [Wed, 16 Jul 2014 09:38:52 +0000 (11:38 +0200)] 
BUILD: report commit ID in git versions as well

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.

11 years agoMINOR: sample: allow integers to cast to binary
Willy Tarreau [Tue, 15 Jul 2014 19:19:08 +0000 (21:19 +0200)] 
MINOR: sample: allow integers to cast to binary

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.