]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
10 years agoMAJOR: session: implement a wait-queue for sessions who need a buffer
Willy Tarreau [Tue, 25 Nov 2014 20:10:35 +0000 (21:10 +0100)] 
MAJOR: session: implement a wait-queue for sessions who need a buffer

When a session_alloc_buffers() fails to allocate one or two buffers,
it subscribes the session to buffer_wq, and waits for another session
to release buffers. It's then removed from the queue and woken up with
TASK_WAKE_RES, and can attempt its allocation again.

We decide to try to wake as many waiters as we release buffers so
that if we release 2 and two waiters need only once, they both have
their chance. We must never come to the situation where we don't wake
enough tasks up.

It's common to release buffers after the completion of an I/O callback,
which can happen even if the I/O could not be performed due to half a
failure on memory allocation. In this situation, we don't want to move
out of the wait queue the session that was just added, otherwise it
will never get any buffer. Thus, we only force ourselves out of the
queue when freeing the session.

Note: at the moment, since session_alloc_buffers() is not used, no task
is subscribed to the wait queue.

10 years agoMEDIUM: session: implement a basic atomic buffer allocator
Willy Tarreau [Tue, 25 Nov 2014 18:46:36 +0000 (19:46 +0100)] 
MEDIUM: session: implement a basic atomic buffer allocator

This patch introduces session_alloc_recv_buffer(), session_alloc_buffers()
and session_release_buffers() whose purpose will be to allocate missing
buffers and release unneeded ones around the process_session() and during
I/O operations.

I/O callbacks only need a single buffer for recv operations and none
for send. However we still want to ensure that we don't pick the last
buffer. That's what session_alloc_recv_buffer() is for.

This allocator is atomic in that it always ensures we can get 2 buffers
or fails. Here, if any of the buffers is not ready and cannot be
allocated, the operation is cancelled. The purpose is to guarantee that
we don't enter into the deadlock where all buffers are allocated by the
same size of all sessions.

A queue will have to be implemented for failed allocations. For now
they're just reported as failures.

10 years agoMEDIUM: buffer: implement b_alloc_margin()
Willy Tarreau [Tue, 2 Dec 2014 12:54:01 +0000 (13:54 +0100)] 
MEDIUM: buffer: implement b_alloc_margin()

This function is used to allocate a buffer and ensure that we leave
some margin after it in the pool. The function is not obvious. While
we allocate only one buffer, we want to ensure that at least two remain
available after our allocation. The purpose is to ensure we'll never
enter a deadlock where all sessions allocate exactly one buffer, and
none of them will be able to allocate the second buffer needed to build
a response in order to release the first one.

We also take care of remaining fast in the the fast path by first
checking whether or not there is enough margin, in which case we only
rely on b_alloc_fast() which is guaranteed to succeed. Otherwise we
take the slow path using pool_refill_alloc().

10 years agoMINOR: buffer: implement b_alloc_fast()
Willy Tarreau [Mon, 8 Dec 2014 15:37:26 +0000 (16:37 +0100)] 
MINOR: buffer: implement b_alloc_fast()

This function allocates a buffer and replaces *buf with this buffer. If
no memory is available, &buf_wanted is used instead. No control is made
to check if *buf already pointed to another buffer. The allocated buffer
is returned, or NULL in case no memory is available. The difference with
b_alloc() is that this function only picks from the pool and never calls
malloc(), so it can fail even if some memory is available. It is the
caller's job to refill the buffer pool if needed.

10 years agoMINOR: session: group buffer allocations together
Willy Tarreau [Tue, 25 Nov 2014 18:54:11 +0000 (19:54 +0100)] 
MINOR: session: group buffer allocations together

We'll soon want to release buffers together upon failure so we need to
allocate them after the channels. Let's change this now. There's no
impact on the behaviour, only the error path is unrolled slightly
differently. The same was done in peers.

10 years agoMEDIUM: channel: do not report full when buf_empty is present on a channel
Willy Tarreau [Fri, 28 Nov 2014 19:54:13 +0000 (20:54 +0100)] 
MEDIUM: channel: do not report full when buf_empty is present on a channel

Till now we'd consider a buffer full even if it had size==0 due to pointing
to buf.size. Now we change this : if buf_wanted is present, it means that we
have already tried to allocate a buffer but failed. Thus the buffer must be
considered full so that we stop trying to poll for reads on it. Otherwise if
it's empty, it's buf_empty and we report !full since we may allocate it on
the fly.

10 years agoMEDIUM: buffer: add a new buf_wanted dummy buffer to report failed allocations
Willy Tarreau [Mon, 24 Nov 2014 10:55:08 +0000 (11:55 +0100)] 
MEDIUM: buffer: add a new buf_wanted dummy buffer to report failed allocations

Doing so ensures that even when no memory is available, we leave the
channel in a sane condition. There's a special case in proto_http.c
regarding the compression, we simply pre-allocate the tmpbuf to point
to the dummy buffer. Not reusing &buf_empty for this allows the rest
of the code to differenciate an empty buffer that's not used from an
empty buffer that results from a failed allocation which has the same
semantics as a buffer full.

10 years agoMEDIUM: buffer: always assign a dummy empty buffer to channels
Willy Tarreau [Mon, 24 Nov 2014 10:39:34 +0000 (11:39 +0100)] 
MEDIUM: buffer: always assign a dummy empty buffer to channels

Channels are now created with a valid pointer to a buffer before the
buffer is allocated. This buffer is a global one called "buf_empty" and
of size zero. Thus it prevents any activity from being performed on
the buffer and still ensures that chn->buf may always be dereferenced.
b_free() also resets the buffer to &buf_empty, and was split into
b_drop() which does not reset the buffer.

10 years agoMINOR: buffer: only use b_free to release buffers
Willy Tarreau [Tue, 25 Nov 2014 18:45:11 +0000 (19:45 +0100)] 
MINOR: buffer: only use b_free to release buffers

We don't call pool_free2(pool2_buffers) anymore, we only call b_free()
to do the job. This ensures that we can start to centralize the releasing
of buffers.

10 years agoMINOR: buffer: move buffer initialization after channel initialization
Willy Tarreau [Mon, 24 Nov 2014 10:36:57 +0000 (11:36 +0100)] 
MINOR: buffer: move buffer initialization after channel initialization

It's not clean to initialize the buffer before the channel since it
dereferences one pointer in the channel. Also we'll want to let the
channel pre-initialize the buffer, so let's ensure that the channel
is always initialized prior to the buffers.

10 years agoMEDIUM: buffer: use b_alloc() to allocate and initialize a buffer
Willy Tarreau [Mon, 24 Nov 2014 10:30:16 +0000 (11:30 +0100)] 
MEDIUM: buffer: use b_alloc() to allocate and initialize a buffer

b_alloc() now allocates a buffer and initializes it to the size specified
in the pool minus the size of the struct buffer itself. This ensures that
callers do not need to care about buffer details anymore. Also this never
applies memory poisonning, which is slow and useless on buffers.

10 years agoMINOR: buffer: reset a buffer in b_reset() and not channel_init()
Willy Tarreau [Mon, 24 Nov 2014 09:54:47 +0000 (10:54 +0100)] 
MINOR: buffer: reset a buffer in b_reset() and not channel_init()

We'll soon need to be able to switch buffers without touching the
channel, so let's move buffer initialization out of channel_init().
We had the same in compressoin.c.

10 years agoMINOR: stream-int: retrieve session pointer from stream-int
Willy Tarreau [Fri, 28 Nov 2014 17:30:25 +0000 (18:30 +0100)] 
MINOR: stream-int: retrieve session pointer from stream-int

sess_from_si() does this via the owner (struct task). It works because
all stream ints belong to a task nowadays.

10 years agoMEDIUM: memory: improve pool_refill_alloc() to pass a refill count
Willy Tarreau [Wed, 3 Dec 2014 14:25:28 +0000 (15:25 +0100)] 
MEDIUM: memory: improve pool_refill_alloc() to pass a refill count

Till now this function would only allocate one entry at a time. But with
dynamic buffers we'll like to allocate the number of missing entries to
properly refill the pool.

Let's modify it to take a minimum amount of available entries. This means
that when we know we need at least a number of available entries, we can
ask to allocate all of them at once. It also ensures that we don't move
the pointers back and forth between the caller and the pool, and that we
don't call pool_gc2() for each failed malloc. Instead, it's called only
once and the malloc is only allowed to fail once.

10 years agoMINOR: memory: cut pool allocator in 3 layers
Willy Tarreau [Mon, 8 Dec 2014 15:35:23 +0000 (16:35 +0100)] 
MINOR: memory: cut pool allocator in 3 layers

pool_alloc2() used to pick the entry from the pool, fall back to
pool_refill_alloc(), and to perform the poisonning itself, which
pool_refill_alloc() was also doing. While this led to optimal
code size, it imposes memory poisonning on the buffers as well,
which is extremely slow on large buffers.

This patch cuts the allocator in 3 layers :
  - a layer to pick the first entry from the pool without falling back to
    pool_refill_alloc() : pool_get_first()
  - a layer to allocate a dirty area by falling back to pool_refill_alloc()
    but never performing the poisonning : pool_alloc_dirty()
  - pool_alloc2() which calls the latter and optionally poisons the area

No functional changes were made.

10 years agoCLEANUP: memory: replace macros pool_alloc2/pool_free2 with functions
Willy Tarreau [Tue, 23 Dec 2014 13:13:16 +0000 (14:13 +0100)] 
CLEANUP: memory: replace macros pool_alloc2/pool_free2 with functions

Using inline functions here makes the code more readable and reduces its
size by about 2 kB.

10 years agoCLEANUP: memory: remove dead code
Willy Tarreau [Tue, 23 Dec 2014 12:51:28 +0000 (13:51 +0100)] 
CLEANUP: memory: remove dead code

The very old pool managment code has not been used for the last 7 years
and is still polluting the file. Get rid of it now.

10 years agoCLEANUP: lists: remove dead code
Willy Tarreau [Tue, 23 Dec 2014 12:58:43 +0000 (13:58 +0100)] 
CLEANUP: lists: remove dead code

Remove the code dealing with the old dual-linked lists imported from
librt that has remained unused for the last 8 years. Now everything
uses the linux-like circular lists instead.

10 years agoBUG/MEDIUM: compression: correctly report zlib_mem
Willy Tarreau [Wed, 24 Dec 2014 17:07:55 +0000 (18:07 +0100)] 
BUG/MEDIUM: compression: correctly report zlib_mem

In zlib we track memory usage. The problem is that the way alloc_zlib()
and free_zlib() account for memory is different, resulting in variations
that can lead to negative zlib_mem being reported. The alloc() function
uses the requested size while the free() function uses the pool size. The
difference can happen when pools are shared with other pools of similar
size. The net effect is that zlib_mem can be reported negative with a
slowly decreasing count, and over the long term the limit will not be
enforced anymore.

The fix is simple : let's use the pool size in both cases, which is also
the exact value when it comes to memory usage.

This fix must be backported to 1.5.

10 years agoBUG/MAJOR: namespaces: conn->target is not necessarily a server
Willy Tarreau [Wed, 24 Dec 2014 12:47:55 +0000 (13:47 +0100)] 
BUG/MAJOR: namespaces: conn->target is not necessarily a server

create_server_socket() used to dereference objt_server(conn->target),
but if the target is not a server (eg: a proxy) then it's NULL and we
get a segfault. This can be reproduced with a proxy using "dispatch"
with no server, even when namespaces are disabled, because that code
is not #ifdef'd. The fix consists in first checking if the target is
a server.

This fix does not need to be backported, this is 1.6-only.

10 years agoBUG/MEDIUM: memory: fix freeing logic in pool_gc2()
Willy Tarreau [Mon, 22 Dec 2014 20:40:55 +0000 (21:40 +0100)] 
BUG/MEDIUM: memory: fix freeing logic in pool_gc2()

There's a long-standing bug in pool_gc2(). It tries to protect the pool
against releasing of too many entries but the formula is wrong as it
compares allocated to minavail instead of (allocated-used) to minavail.
Under memory contention, it ends up releasing more than what is granted
by minavail and causes trouble to the dynamic buffer allocator.

This bug is in fact major by itself, but since minavail has never been
used till now, there is no impact at least in mainline. A backport to
1.5 is desired anyway in case any future backport or out-of-tree patch
relies on this.

10 years agoBUG/MAJOR: stream-int: properly check the memory allocation return
Willy Tarreau [Mon, 22 Dec 2014 18:34:00 +0000 (19:34 +0100)] 
BUG/MAJOR: stream-int: properly check the memory allocation return

In stream_int_register_handler(), we call si_alloc_appctx(si) but as a
mistake, instead of checking the return value for a NULL, we test <si>.
This bug was discovered under extreme memory contention (memory for only
two buffers with 500 connections waiting) and after 3 million failed
connections. While it was very hard to produce it, the fix is tagged
major because in theory it could happen when haproxy runs with a very
low "-m" setting preventing from allocating just the few bytes needed
for an appctx. But most users will never be able to trigger it. The
fix was confirmed to address the bug.

This fix must be backported to 1.5.

10 years agoBUG/MAJOR: ns: HAProxy segfault if the cli_conn is not from a network connection
Thierry FOURNIER [Fri, 19 Dec 2014 12:37:11 +0000 (13:37 +0100)] 
BUG/MAJOR: ns: HAProxy segfault if the cli_conn is not from a network connection

The path "MAJOR: namespace: add Linux network namespace support" doesn't
permit to use internal data producer like a "peers synchronisation"
system. The result is a segfault when the internal application starts.

This patch fix the commit b3e54fe387c7c1ea750f39d3029672d640c499f9
It is introduced in 1.6dev version, it doesn't need to be backported.

10 years agoMINOR: map/acl/dumpstats: remove the "Done." message
Thierry FOURNIER [Thu, 18 Dec 2014 14:28:01 +0000 (15:28 +0100)] 
MINOR: map/acl/dumpstats: remove the "Done." message

By convention, the HAProxy CLI doesn't return message if the opration
is sucessfully done. The MAP and ACL returns the "Done." message, an
its noise the output during big MAP or ACL injection.

10 years agoBUG/MEDIUM: config: do not propagate processes between stopped processes
Willy Tarreau [Thu, 18 Dec 2014 13:00:43 +0000 (14:00 +0100)] 
BUG/MEDIUM: config: do not propagate processes between stopped processes

Immo Goltz reported a case of segfault while parsing the config where
we try to propagate processes across stopped frontends (those with a
"disabled" statement). The fix is trivial. The workaround consists in
commenting out these frontends, although not always easy.

This fix must be backported to 1.5.

10 years agoBUG/MINOR: config: fix typo in condition when propagating process binding
Willy Tarreau [Thu, 18 Dec 2014 12:56:26 +0000 (13:56 +0100)] 
BUG/MINOR: config: fix typo in condition when propagating process binding

propagate_processes() has a typo in a condition :

if (!from->cap & PR_CAP_FE)
return;

The return is never taken because each proxy has at least one capability
so !from->cap always evaluates to zero. Most of the time the caller already
checks that <from> is a frontend. In the cases where it's not tested
(use_backend, reqsetbe), the rules have been checked for the context to
be a frontend as well, so in the end it had no nasty side effect.

This should be backported to 1.5.

10 years agoBUG/MINOR: parse: refer curproxy instead of proxy
Godbach [Thu, 18 Dec 2014 07:44:58 +0000 (15:44 +0800)] 
BUG/MINOR: parse: refer curproxy instead of proxy

Since during parsing stage, curproxy always represents a proxy to be operated,
it should be a mistake by referring proxy.

Signed-off-by: Godbach <nylzhaowei@gmail.com>
10 years agoBUG/MINOR: http: fix typo: "401 Unauthorized" => "407 Unauthorized"
Godbach [Wed, 17 Dec 2014 08:32:05 +0000 (16:32 +0800)] 
BUG/MINOR: http: fix typo: "401 Unauthorized" => "407 Unauthorized"

401 Unauthorized => 407 Unauthorized

Signed-off-by: Godbach <nylzhaowei@gmail.com>
10 years agoCLEANUP: epoll: epoll_events should be allocated according to global.tune.maxpollevents
Godbach [Wed, 17 Dec 2014 08:14:26 +0000 (16:14 +0800)] 
CLEANUP: epoll: epoll_events should be allocated according to global.tune.maxpollevents

Willy: commit f2e8ee2b introduced an optimization in the old speculative epoll
code, which implemented its own event cache. It was needed to store that many
events (it was bound to maxsock/4 btw). Now the event cache lives on its own
and we don't need this anymore. And since events are allocated on the kernel
side, we only need to allocate the events we want to return.

As a result, absmaxevents will be not used anymore. Just remove the definition
and the comment of it, replace it with global.tune.maxpollevents. It is also an
optimization of memory usage for large amounts of sockets.

Signed-off-by: Godbach <nylzhaowei@gmail.com>
10 years agoDOC: httplog does not support 'no'
PiBa-NL [Thu, 11 Dec 2014 20:31:54 +0000 (21:31 +0100)] 
DOC: httplog does not support 'no'

10 years agoBUG/MEDIUM: sample: fix random number upper-bound
Vincent Bernat [Wed, 10 Dec 2014 09:31:37 +0000 (10:31 +0100)] 
BUG/MEDIUM: sample: fix random number upper-bound

random() will generate a number between 0 and RAND_MAX. POSIX mandates
RAND_MAX to be at least 32767. GNU libc uses (1<<31 - 1) as
RAND_MAX.

In smp_fetch_rand(), a reduction is done with a multiply and shift to
avoid skewing the results. However, the shift was always 32 and hence
the numbers were not distributed uniformly in the specified range. We
fix that by dividing by RAND_MAX+1. gcc is smart enough to turn that
into a shift:

    0x000000000046ecc8 <+40>:    shr    $0x1f,%rax

10 years agoDOC: fix a few typos
Godbach [Wed, 10 Dec 2014 02:21:30 +0000 (10:21 +0800)] 
DOC: fix a few typos

include/types/proto_http.h: hwen -> when
include/types/server.h: SRV_ST_DOWN -> SRV_ST_STOPPED
src/backend.c: prefer-current-server -> prefer-last-server

Signed-off-by: Godbach <nylzhaowei@gmail.com>
10 years agoBUILD: ssl: use OPENSSL_NO_OCSP to detect OCSP support
Lukas Tribus [Tue, 9 Dec 2014 15:32:51 +0000 (16:32 +0100)] 
BUILD: ssl: use OPENSSL_NO_OCSP to detect OCSP support

Since commit 656c5fa7e859 ("BUILD: ssl: disable OCSP when using
boringssl) the OCSP code is bypassed when OPENSSL_IS_BORINGSSL
is defined. The correct thing to do here is to use OPENSSL_NO_OCSP
instead, which is defined for this exact purpose in
openssl/opensslfeatures.h.

This makes haproxy forward compatible if boringssl ever introduces
full OCSP support with the additional benefit that it links fine
against a OCSP-disabled openssl.

Signed-off-by: Lukas Tribus <luky-37@hotmail.com>
10 years agoBUG/MEDIUM: tcp-checks: disable quick-ack unless next rule is an expect
Willy Tarreau [Mon, 8 Dec 2014 11:11:28 +0000 (12:11 +0100)] 
BUG/MEDIUM: tcp-checks: disable quick-ack unless next rule is an expect

Using "option tcp-checks" without any rule is different from not using
it at all in that checks are sent with the TCP quick ack mode enabled,
causing servers to log incoming port probes.

This commit fixes this behaviour by disabling quick-ack on tcp-checks
unless the next rule exists and is an expect. All combinations were
tested and now the behaviour is as expected : basic port probes are
now doing a SYN-SYN/ACK-RST sequence.

This fix must be backported to 1.5.

10 years agoBUG/MEDIUM: tcp-check: don't rely on random memory contents
Willy Tarreau [Mon, 8 Dec 2014 10:52:28 +0000 (11:52 +0100)] 
BUG/MEDIUM: tcp-check: don't rely on random memory contents

If "option tcp-check" is used and no "tcp-check" rule is specified, we
only look at rule->action which dereferences the proxy's memory and which
can randomly match TCPCHK_ACT_CONNECT or whatever else, causing a check
to fail. This bug is the result of an incorrect fix attempted in commit
f621bea ("BUG/MINOR: tcpcheck connect wrong behavior").

This fix must be backported into 1.5.

10 years agoBUG/MINOR: tcp-check: don't condition data polling on check type
Willy Tarreau [Mon, 8 Dec 2014 10:26:08 +0000 (11:26 +0100)] 
BUG/MINOR: tcp-check: don't condition data polling on check type

tcp_check_main() would condition the polling for writes on check->type,
but this is absurd given that check->type == PR_O2_TCPCHK_CHK since this
is the only way we can get there! This patch removes this confusing test.

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.