]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
7 years agoBUG/MINOR: lua: Segfaults with wrong usage of types.
Frédéric Lécaille [Fri, 15 Jun 2018 11:56:04 +0000 (13:56 +0200)] 
BUG/MINOR: lua: Segfaults with wrong usage of types.

Patrick reported that this simple configuration made haproxy segfaults:

    global
        lua-load /tmp/haproxy.lua

    frontend f1
        mode http
        bind :8000
        default_backend b1

        http-request lua.foo

    backend b1
        mode http
        server s1 127.0.0.1:8080

with this '/tmp/haproxy.lua' script:

    core.register_action("foo", { "http-req" }, function(txn)
        txn.sc:ipmask(txn.f:src(), 24, 112)
    end)

This is due to missing initialization of the array of arguments
passed to hlua_lua2arg_check() which makes it enter code with
corrupted arguments.

Thanks a lot to Patrick Hemmer for having reported this issue.

Must be backported to 1.8, 1.7 and 1.6.

7 years agoBUG/MINOR: tasklets: Just make sure we don't pass a tasklet to the handler.
Olivier Houchard [Thu, 14 Jun 2018 13:40:47 +0000 (15:40 +0200)] 
BUG/MINOR: tasklets: Just make sure we don't pass a tasklet to the handler.

We can't just set t to NULL if it's a tasklet, or we'd have a hard time
accessing to t->process, so just make sure we pass NULL as the first parameter
of t->process if it's a tasklet.
This should be a non-issue at this point, as tasklets aren't used yet.

7 years agoMINOR: tasks: Make sure we correctly init and deinit a tasklet.
Olivier Houchard [Fri, 8 Jun 2018 15:08:19 +0000 (17:08 +0200)] 
MINOR: tasks: Make sure we correctly init and deinit a tasklet.

Up until now, a tasklet couldn't be free'd while it was in the list, it is
no longer the case, so make sure we remove it from the list before freeing it.
To do so, we have to make sure we correctly initialize it, so use LIST_INIT,
instead of setting the pointers to NULL.

7 years agoDOC: regression testing: Add a short starting guide.
Frédéric Lécaille [Thu, 7 Jun 2018 12:57:30 +0000 (14:57 +0200)] 
DOC: regression testing: Add a short starting guide.

This documentation describes how to write varnish test case (VTC)
files to reg test haproxy.

7 years agoBUG/MAJOR: map: fix a segfault when using http-request set-map
William Lallemand [Mon, 11 Jun 2018 08:53:46 +0000 (10:53 +0200)] 
BUG/MAJOR: map: fix a segfault when using http-request set-map

The bug happens with an existing entry, when you try to overwrite the
value with wrong data, for example, a string when the type is INT.

The code path was not secure and tried to set *err and *merr while
err = merr = NULL when performing an http action.

Must be backported in 1.6, 1.7, 1.8.

7 years agoBUG/MINOR: signals: ha_sigmask macro for multithreading
William Lallemand [Thu, 7 Jun 2018 09:23:40 +0000 (11:23 +0200)] 
BUG/MINOR: signals: ha_sigmask macro for multithreading

The behavior of sigprocmask in an multithreaded environment is
undefined.

The new macro ha_sigmask() calls either pthreads_sigmask() or
sigprocmask() if haproxy was built with thread support or not.

This should be backported to 1.8.

7 years agoBUG/MINOR: don't ignore SIG{BUS,FPE,ILL,SEGV} during signal processing
William Lallemand [Thu, 7 Jun 2018 07:49:04 +0000 (09:49 +0200)] 
BUG/MINOR: don't ignore SIG{BUS,FPE,ILL,SEGV} during signal processing

We don't have any reason of blocking those signals.

If SIGBUS, SIGFPE, SIGILL, or SIGSEGV are generated while they are blocked, the
result is undefined, unless the signal was generated by kill(2), sigqueue(3), or
raise(3).

This should be backported to 1.8.

7 years agoBUG/MEDIUM: threads: handle signal queue only in thread 0
William Lallemand [Thu, 7 Jun 2018 07:46:01 +0000 (09:46 +0200)] 
BUG/MEDIUM: threads: handle signal queue only in thread 0

Signals were handled in all threads which caused some signals to be lost
from time to time. To avoid complicated lock system (threads+signals),
we prefer handling the signals in one thread avoiding concurrent access.

The side effect of this bug was that some process were not leaving from
time to time during a reload.

This patch must be backported in 1.8.

7 years agoMINOR: lua: Increase debug information
Thierry FOURNIER [Thu, 7 Jun 2018 12:40:48 +0000 (14:40 +0200)] 
MINOR: lua: Increase debug information

When an unrecoverable error raises, the user receive poor information
for the trouble shooting. For example:

   [ALERT] 157/143755 (21212) : Lua function 'hello-world': runtime error: memory allocation error: block too big.

Unfortunately, the memory allocation error can be throwed by many
function, and we have no informatio to reach the original cause.
This patch add the list of function called from the entry point to
the function in error, like this:

   [ALERT] 157/143755 (21212) : Lua function 'hello-world': runtime error: memory allocation error: block too big from [C] method 'req_get_headers', bug35.lua:2 global 'ee', bug35.lua:6 global 'ff', bug35.lua:10 C function line 9.

7 years agoBUG/MINOR: unix: Make sure we can transfer abns sockets on seamless reload.
Olivier Houchard [Wed, 6 Jun 2018 16:34:34 +0000 (18:34 +0200)] 
BUG/MINOR: unix: Make sure we can transfer abns sockets on seamless reload.

When checking if a socket we got from the parent is suitable for a listener,
we just checked that the path matched sockname.tmp, however this is
unsuitable for abns sockets, where we don't have to create a temporary
file and rename it later.
To detect that, check that the first character of the sun_path is 0 for
both, and if so, that &sun_path[1] is the same too.

This should be backported to 1.8.

7 years agoMINOR: tasks: Don't define rqueue if we're building without threads.
Olivier Houchard [Wed, 6 Jun 2018 12:22:03 +0000 (14:22 +0200)] 
MINOR: tasks: Don't define rqueue if we're building without threads.

To make sure we don't inadvertently insert task in the global runqueue,
while only the local runqueue is used without threads, make its definition
and usage conditional on USE_THREAD.

7 years agoBUG/MEDIUM: tasks: Use the local runqueue when building without threads.
Olivier Houchard [Wed, 6 Jun 2018 12:01:08 +0000 (14:01 +0200)] 
BUG/MEDIUM: tasks: Use the local runqueue when building without threads.

When building without threads enabled, instead of just using the global
runqueue, just use the local runqueue associated with the only thread, as
that's what is now expected for a single thread in prcoess_runnable_tasks().
This should fix haproxy when built without threads.

7 years agoMINOR: task: Fix compiler warning.
David Carlier [Tue, 5 Jun 2018 10:41:03 +0000 (10:41 +0000)] 
MINOR: task: Fix compiler warning.

Waking up task, when checking if it is a valid entry.
Similarly to commit caa8a37ffe5922efda7fd7b882e96964b40d7135,
casting explicitally to void pointer as HA_ATOMIC_CAS needs.

7 years agoMINOR: applet: assign the same nice value to a new appctx as its owner task
Willy Tarreau [Thu, 31 May 2018 12:44:25 +0000 (14:44 +0200)] 
MINOR: applet: assign the same nice value to a new appctx as its owner task

When an applet is created, let's assign it the same nice value as the task
of the stream which owns it. It ensures that fairness is properly propagated
to applets, and that the CLI can regain a low latency behaviour again. Huge
differences have been seen under extreme loads, with the CLI being called
every 200 microseconds instead of 11 milliseconds.

7 years agoMINOR: stats: also report the nice and number of calls for applets
Willy Tarreau [Thu, 31 May 2018 12:40:19 +0000 (14:40 +0200)] 
MINOR: stats: also report the nice and number of calls for applets

Since applets are now part of the main scheduler, it's useful to report
their nice value and the number of calls to the applet handler, to see
where the CPU is spent.

7 years agoMINOR: task: Fix a compiler warning by adding a cast.
David Carlier [Fri, 1 Jun 2018 12:32:39 +0000 (14:32 +0200)] 
MINOR: task: Fix a compiler warning by adding a cast.

When calling HA_ATOMIC_CAS with a pointer as the target, the compiler
expects a pointer as the new value, so give it one by casting 0x1 to
(void *).

7 years agoBUG/MINOR: contrib/modsecurity: update pointer on the end of the frame
Dragan Dosen [Fri, 1 Jun 2018 13:50:57 +0000 (15:50 +0200)] 
BUG/MINOR: contrib/modsecurity: update pointer on the end of the frame

Similar to commit 94bb4c6 ("BUG/MINOR: spoa: Update pointer on the end of
the frame when a reply is encoded").

This patch should be backported to 1.8.

7 years agoBUG/MINOR: contrib/mod_defender: update pointer on the end of the frame
Dragan Dosen [Fri, 1 Jun 2018 13:42:12 +0000 (15:42 +0200)] 
BUG/MINOR: contrib/mod_defender: update pointer on the end of the frame

Similar to commit 94bb4c6 ("BUG/MINOR: spoa: Update pointer on the end of
the frame when a reply is encoded").

This patch should be backported to 1.8.

7 years agoBUG/MINOR: contrib/modsecurity: Don't reset the status code during disconnect
Christopher Faulet [Thu, 31 May 2018 14:05:21 +0000 (16:05 +0200)] 
BUG/MINOR: contrib/modsecurity: Don't reset the status code during disconnect

When the connection is closed by HAProxy, the status code provided in the
DISCONNECT frame is lost. By retransmitting it in the agent's reply, we are sure
to have it in the SPOE logs.

This patch may be backported in 1.8.

7 years agoBUG/MINOR: contrib/mod_defender: Don't reset the status code during disconnect
Christopher Faulet [Thu, 31 May 2018 14:04:53 +0000 (16:04 +0200)] 
BUG/MINOR: contrib/mod_defender: Don't reset the status code during disconnect

When the connection is closed by HAProxy, the status code provided in the
DISCONNECT frame is lost. By retransmitting it in the agent's reply, we are sure
to have it in the SPOE logs.

This patch may be backported in 1.8.

7 years agoBUG/MINOR: contrib/spoa_example: Don't reset the status code during disconnect
Christopher Faulet [Thu, 31 May 2018 13:59:32 +0000 (15:59 +0200)] 
BUG/MINOR: contrib/spoa_example: Don't reset the status code during disconnect

When the connection is closed by HAProxy, the status code provided in the
DISCONNECT frame is lost. By retransmitting it in the agent's reply, we are sure
to have it in the SPOE logs.

This patch may be backported in 1.8.

7 years agoMAJOR: spoe: upgrade the SPOP version to 2.0 and remove the support for 1.0
Christopher Faulet [Thu, 31 May 2018 12:56:42 +0000 (14:56 +0200)] 
MAJOR: spoe: upgrade the SPOP version to 2.0 and remove the support for 1.0

The commit c4dcaff3 ("BUG/MEDIUM: spoe: Flags are not encoded in network order")
introduced an incompatibility with older agents. So the major version of the
SPOP is increased to make the situation unambiguous. And because before the fix,
the protocol is buggy, the support of the version 1.0 is removed to be sure to
not continue to support buggy agents.

The agents in the contrib folder (spoa_example, modsecurity and mod_defender)
are also updated to announce the SPOP version 2.0.

So, to be clear, from the patch, connections to agents announcing the SPOP
version 1.0 will be rejected.

This patch must be backported in 1.8.

7 years agoDOC: SPOE.txt: fix a typo
Kevin Zhu [Fri, 1 Jun 2018 03:38:00 +0000 (05:38 +0200)] 
DOC: SPOE.txt: fix a typo

7 years agoDOC: contrib/modsecurity: few typo fixes
David Carlier [Thu, 31 May 2018 15:42:03 +0000 (16:42 +0100)] 
DOC: contrib/modsecurity: few typo fixes

Few typo fixes.

7 years agoBUG/MEDIUM: lua/socket: Buffer error, may segfault
Thierry FOURNIER [Sat, 26 May 2018 23:14:47 +0000 (01:14 +0200)] 
BUG/MEDIUM: lua/socket: Buffer error, may segfault

The buffer pointer is already updated. It is again updated
when it is given to the function ci_putblk().

This patch must be backported in 1.6, 1.7 and 1.8

7 years agoBUG/MEDIUM: lua/socket: Sheduling error on write: may dead-lock
Thierry FOURNIER [Sat, 26 May 2018 23:27:40 +0000 (01:27 +0200)] 
BUG/MEDIUM: lua/socket: Sheduling error on write: may dead-lock

When we write data, we risk to encounter a dead-loack. The
function "stream_int_notify()" cannot be called the the
cosocket because the caller acquire a lock and when the socket
is closed, the cleanup function try to acquire the same lock.,
so a dead-lock raises.

In other way, the function stream_int_update_applet() can't
be called because it schedumes the applet only if some activity
in the buffers were detected. It is not always the case. We
replace this function by appctx_wakeup() which wake up the
applet inconditionnaly.

The last part of the fix is setting right signals. the applet
call the stream_int_update() function if the output buffer si
not empty, and ask for put data if some rite signals are
registered.

This patch must be backported in 1.6, 1.7 and 1.8. Note that it requires
patch "MINOR: task/notification: Is notifications registered" to be
applied.

7 years agoBUG/MEDIUM: lua/socket: Notification error
Thierry FOURNIER [Sat, 26 May 2018 22:59:48 +0000 (00:59 +0200)] 
BUG/MEDIUM: lua/socket: Notification error

Each time the send function yields, a notification must be registered.
Without this notification, the task is never wakeup when data arrives.

Today, the notification is registered only if the buffer is not available.
Other cases like the buffer is too small for all data are not processed.

This patch must be backported in 1.6, 1.7 and 1.8

7 years agoBUG/MAJOR: lua: Dead lock with sockets
Thierry FOURNIER [Fri, 25 May 2018 13:03:50 +0000 (15:03 +0200)] 
BUG/MAJOR: lua: Dead lock with sockets

In some cases, when we are waiting for data and the socket
timeout expires, we have a dead lock. The Lua socket locks
the applet socket, and call for a notify. The notify
immediately executes code and try to acquire the same lock,
so ... dead lock.

stream_int_notify() cant be used because it wakeup the applet
task only if the stream have changes. The changes are forces
by Lua, but not repported on the stream.

stream_int_update_applet() cant be used because the deadlock.

So, I inconditionnaly wakeup the applet. This wake is performed
asynchronously, and will call a stream_int_notify().

This patch must be backported in 1.6, 1.7 and 1.8

7 years agoBUG/MEDIUM: lua/socket: wrong scheduling for sockets
Thierry FOURNIER [Fri, 25 May 2018 12:38:57 +0000 (14:38 +0200)] 
BUG/MEDIUM: lua/socket: wrong scheduling for sockets

The appctx pointer is given from any variable which are wrong.
This implies the wakeup of wrong applet, and the socket are no
longer responsive.

This behavior is hidden by another inherited error which is
fixed in the next patch.

This patch remove all wrong appctx affectations.

This patch must be backported in 1.6, 1.7 and 1.8

7 years agoMINOR: task/notification: Is notifications registered ?
Thierry FOURNIER [Wed, 30 May 2018 09:40:08 +0000 (11:40 +0200)] 
MINOR: task/notification: Is notifications registered ?

This function returns true is some notifications are registered.

This function is usefull for the following patch

   BUG/MEDIUM: lua/socket: Sheduling error on write: may dead-lock

It should be backported in 1.6, 1.7 and 1.8

7 years agoBUG/MEDIUM: spoe: Return an error when the wrong ACK is received in sync mode
Christopher Faulet [Fri, 25 May 2018 08:42:37 +0000 (10:42 +0200)] 
BUG/MEDIUM: spoe: Return an error when the wrong ACK is received in sync mode

This is required to let a message processing timed out. Because, when it
happens, there is no more context attached to the SPOE applet that sent the
NOTIFY frame. So when the ACK is received, it is too late. This is the same
situation when we receive the wrong ACK. It is invalid in sync mode. Otherwise,
the SPOE applet remains in the state "WAITING_SYNC_ACK" until the idle timeout
is reached. In such case, the applet is seen as busy and it is unusable. If this
happens too often, more and more applets will be created because some others are
blocked. If there is a maxconn on the SPOE backend, all processings will be
drastically slowdown.

Returning an error in such cases, in sync mode, allow us to terminate the SPOE
applet. Because it means the agent is unresponsive or too slow.

Note this bug exists only if the sync mode is used.

This patch must be backported in 1.8.

7 years agoMINOR: dns: Implement `parse-resolv-conf` directive
Ben Draut [Tue, 29 May 2018 21:40:08 +0000 (15:40 -0600)] 
MINOR: dns: Implement `parse-resolv-conf` directive

This introduces a new directive for the `resolvers` section:
`parse-resolv-conf`. When present, it will attempt to add any
nameservers in `/etc/resolv.conf` to the list of nameservers
for the current `resolvers` section.

[Mailing list thread][1].

[1]: https://www.mail-archive.com/haproxy@formilux.org/msg29600.html

7 years agoMINOR: task: Also consider the task list size when getting global tasks.
Olivier Houchard [Mon, 28 May 2018 12:53:08 +0000 (14:53 +0200)] 
MINOR: task: Also consider the task list size when getting global tasks.

We're taking tasks from the global runqueue based on the number of tasks
the thread already have in its local runqueue, but now that we have a task
list, we also have to take that into account.

7 years agoBUG/MEDIUM: task: Don't forget to decrement max_processed after each task.
Olivier Houchard [Mon, 28 May 2018 12:54:49 +0000 (14:54 +0200)] 
BUG/MEDIUM: task: Don't forget to decrement max_processed after each task.

When the task list was introduced, we bogusly lost max_processed--, that means
we would execute as much tasks as present in the list, and we would never
set active_tasks_mask, so the thread would go to sleep even if more tasks were
to be executed.

1.9-dev only, no backport is needed.

7 years agoBUG/MEDIUM: tasks: Don't forget to increase/decrease tasks_run_queue.
Olivier Houchard [Mon, 28 May 2018 11:51:06 +0000 (13:51 +0200)] 
BUG/MEDIUM: tasks: Don't forget to increase/decrease tasks_run_queue.

Don't forget to increase tasks_run_queue when we're adding a task to the
tasklet list, and to decrease it when we remove a task from a runqueue,
or its value won't be accurate, and could lead to tasks not being executed
when put in the global run queue.

1.9-dev only, no backport is needed.

7 years agoMINOR: stats: also report the failed header rewrites warnings on the stats page
Willy Tarreau [Mon, 28 May 2018 13:12:40 +0000 (15:12 +0200)] 
MINOR: stats: also report the failed header rewrites warnings on the stats page

These ones concern the warnings detected during header addition/insertion.
They are visible in the tooltip reporting the per-status codes stats. The
frontend and backend contain a total of request+response warnings, while
server only has the response warnings.

7 years agoDOC: management: add the new wrew stats column
Willy Tarreau [Mon, 28 May 2018 13:15:43 +0000 (15:15 +0200)] 
DOC: management: add the new wrew stats column

This is the number of failed rewrite warnings, per front/listener/back/server.

7 years agoMINOR: http: Log warning if (add|set)-header fails
Tim Duesterhus [Sun, 27 May 2018 18:35:08 +0000 (20:35 +0200)] 
MINOR: http: Log warning if (add|set)-header fails

This patch adds a warning if an http-(request|reponse) (add|set)-header
rewrite fails to change the respective header in a request or response.

This usually happens when tune.maxrewrite is not sufficient to hold all
the headers that should be added.

7 years agoBUG/MEDIUM: stick-tables: Decrement ref_cnt in table_* converters
Daniel Corbett [Sun, 27 May 2018 13:47:12 +0000 (09:47 -0400)] 
BUG/MEDIUM: stick-tables: Decrement ref_cnt in table_* converters

When using table_* converters ref_cnt was incremented
and never decremented causing entries to not expire.

The root cause appears to be that stktable_lookup_key()
was called within all sample_conv_table_* functions which was
incrementing ref_cnt and not decrementing after completion.

Added stktable_release() to the end of each sample_conv_table_*
function and reworked the end logic to ensure that ref_cnt is
always decremented after use.

This should be backported to 1.8

7 years agoMAJOR: applets: Use tasks, instead of rolling our own scheduler.
Olivier Houchard [Fri, 25 May 2018 14:58:52 +0000 (16:58 +0200)] 
MAJOR: applets: Use tasks, instead of rolling our own scheduler.

There's no real reason to have a specific scheduler for applets anymore, so
nuke it and just use tasks. This comes with some benefits, the first one
being that applets cannot induce high latencies anymore since they share
nice values with other tasks. Later it will be possible to configure the
applets' nice value. The second benefit is that the applet scheduler was
not very thread-friendly, having a big lock around it in prevision of this
change. Thus applet-intensive workloads should now scale much better with
threads.

Some more improvement is possible now : some applets also use a task to
handle timers and timeouts. These ones could now be simplified to use only
one task.

7 years agoMINOR: tasks: Make the number of tasks to run at once configurable.
Olivier Houchard [Thu, 24 May 2018 16:59:04 +0000 (18:59 +0200)] 
MINOR: tasks: Make the number of tasks to run at once configurable.

Instead of hardcoding 200, make the number of tasks to be run configurable
using tune.runqueue-depth. 200 is still the default.

7 years agoMAJOR: tasks: Introduce tasklets.
Olivier Houchard [Fri, 18 May 2018 16:45:28 +0000 (18:45 +0200)] 
MAJOR: tasks: Introduce tasklets.

Introduce tasklets, lightweight tasks. They have no notion of priority,
they are just run as soon as possible, and will probably be used for I/O
later.

For the moment they're used to replace the temporary thread-local list
that was used in the scheduler. The first part of the struct is common
with tasks so that tasks can be cast to tasklets and queued in this list.
Once a task is in the tasklet list, it has its leaf_p set to 0x1 so that
it cannot accidently be confused as not in the queue.

Pure tasklets are identifiable by their nice value of -32768 (which is
normally not possible).

7 years agoMAJOR: tasks: Create a per-thread runqueue.
Olivier Houchard [Fri, 18 May 2018 16:38:23 +0000 (18:38 +0200)] 
MAJOR: tasks: Create a per-thread runqueue.

A lot of tasks are run on one thread only, so instead of having them all
in the global runqueue, create a per-thread runqueue which doesn't require
any locking, and add all tasks belonging to only one thread to the
corresponding runqueue.

The global runqueue is still used for non-local tasks, and is visited
by each thread when checking its own runqueue. The nice parameter is
thus used both in the global runqueue and in the local ones. The rare
tasks that are bound to multiple threads will have their nice value
used twice (once for the global queue, once for the thread-local one).

7 years agoMINOR: tasks: Change the task API so that the callback takes 3 arguments.
Olivier Houchard [Fri, 25 May 2018 12:04:04 +0000 (14:04 +0200)] 
MINOR: tasks: Change the task API so that the callback takes 3 arguments.

In preparation for thread-specific runqueues, change the task API so that
the callback takes 3 arguments, the task itself, the context, and the state,
those were retrieved from the task before. This will allow these elements to
change atomically in the scheduler while the application uses the copied
value, and even to have NULL tasks later.

7 years agoBUG/MEDIUM: lua/socket: Length required read doesn't work
Thierry FOURNIER [Fri, 25 May 2018 14:27:44 +0000 (16:27 +0200)] 
BUG/MEDIUM: lua/socket: Length required read doesn't work

The limit of data read works only if all the data is in the
input buffer. Otherwise (if the data arrive in chunks), the
total amount of data is not taken in acount.

Only the current read data are compared to the expected amout
of data.

This patch must be backported from 1.9 to 1.6

7 years agoBUG/MEDIUM: servers: Add srv_addr default placeholder to the state file
Daniel Corbett [Sat, 19 May 2018 23:43:24 +0000 (19:43 -0400)] 
BUG/MEDIUM: servers: Add srv_addr default placeholder to the state file

When creating a state file using "show servers state" an empty field is
created in the srv_addr column if the server is from the socket family
AF_UNIX.  This leads to a warning on start up when using
"load-server-state-from-file". This patch defaults srv_addr to "-" if
the socket family is not covered.

This patch should be backported to 1.8.

7 years agoBUG/BUILD: threads: unbreak build without threads
Willy Tarreau [Wed, 23 May 2018 17:54:43 +0000 (19:54 +0200)] 
BUG/BUILD: threads: unbreak build without threads

A few users reported that building without threads was accidently broken
after commit 6b96f72 ("BUG/MEDIUM: pollers: Use a global list for fd
shared between threads.") due to all_threads_mask not being defined.
It's OK to set it to zero as other code parts do when threads are
enabled but only one thread is used.

This needs to be backported to 1.8.

7 years agoBUG/MEDIUM: dns: Delay the attempt to run a DNS resolution on check failure.
Olivier Houchard [Tue, 22 May 2018 16:40:07 +0000 (18:40 +0200)] 
BUG/MEDIUM: dns: Delay the attempt to run a DNS resolution on check failure.

When checks fail, the code tries to run a dns resolution, in case the IP
changed.
The old way of doing that was to check, in case the last dns resolution
hadn't expired yet, if there were an applicable IP, which should be useless,
because it has already be done when the resolution was first done, or to
run a new resolution.
Both are a locking nightmare, and lead to deadlocks, so instead, just wake the
resolvers task, that should do the trick.

This should be backported to 1.8.

7 years agoMINOR: ssl: set SSL_OP_PRIORITIZE_CHACHA
Lukas Tribus [Fri, 18 May 2018 15:55:57 +0000 (17:55 +0200)] 
MINOR: ssl: set SSL_OP_PRIORITIZE_CHACHA

Sets OpenSSL 1.1.1's SSL_OP_PRIORITIZE_CHACHA unconditionally, as per [1]:

When SSL_OP_CIPHER_SERVER_PREFERENCE is set, temporarily reprioritize
ChaCha20-Poly1305 ciphers to the top of the server cipher list if a
ChaCha20-Poly1305 cipher is at the top of the client cipher list. This
helps those clients (e.g. mobile) use ChaCha20-Poly1305 if that cipher
is anywhere in the server cipher list; but still allows other clients to
use AES and other ciphers. Requires SSL_OP_CIPHER_SERVER_PREFERENCE.

[1] https://www.openssl.org/docs/man1.1.1/man3/SSL_CTX_clear_options.html

7 years agoBUG/MEDIUM: cache: don't cache when an Authorization header is present
William Lallemand [Tue, 22 May 2018 09:04:33 +0000 (11:04 +0200)] 
BUG/MEDIUM: cache: don't cache when an Authorization header is present

RFC 7234 says:

A cache MUST NOT store a response to any request, unless:
[...] the Authorization header field (see Section 4.2 of [RFC7235]) does
      not appear in the request, if the cache is shared, unless the
      response explicitly allows it (see Section 3.2), [...]

In this patch we completely disable the cache upon the receipt of an
Authorization header in the request. In this case it's not possible to
either use the cache or store into the cache anymore.

Thanks to Adam Eijdenberg of Digital Transformation Agency for raising
this issue.

This patch must be backported to 1.8.

7 years agoMINOR: lua: Improve error message
Thierry Fournier [Mon, 21 May 2018 17:42:47 +0000 (19:42 +0200)] 
MINOR: lua: Improve error message

The function hlua_ctx_resume return less text message and more error
code. These error code allow the caller to return appropriate
message to the user.

7 years agoBUG/MINOR: ssl/lua: prevent lua from affecting automatic maxconn computation
Willy Tarreau [Fri, 18 May 2018 15:08:28 +0000 (17:08 +0200)] 
BUG/MINOR: ssl/lua: prevent lua from affecting automatic maxconn computation

Since commit 36d1374 ("BUG/MINOR: lua: Fix SSL initialisation") in 1.6, the
Lua code always initializes an SSL server. It caused a small visible side
effect which is that by calling ssl_sock_prepare_srv_ctx(), it forces
global.ssl_used_backend to 1 and makes the initialization code believe that
there are some SSL servers in certain backends. This detection is used to
figure how to set the global maxconn value when only the memory usage is
limited. As such, even a configuration with no SSL at all will have a very
conservative maxconn.

The configuration below exhibits this :

   global
        ssl-server-verify none
        stats socket /tmp/sock1 mode 666 level admin
        tune.bufsize 16384

   listen  px
        timeout client  5s
        timeout server  5s
        timeout connect 5s
        bind :4445
        #bind :4443 ssl crt rsa+dh2048.pem
        #server s1 127.0.0.1:8003 ssl

Starting it with "-m 200" to limit it to 200 MB of RAM reports 1500 for
Maxconn, the same when uncommenting the "server" line, and 1300 when
uncommenting the "bind" line, regardless of the "server" line's status.

In practice it doesn't make sense to consider that Lua's server template
counts for one regular SSL server, because even if used for SSL, it will
not take large connection counts, compared to a backend relaying traffic.
Thus the solution consists in resetting the ssl_used_backend to its
previous value after creating the server_ctx from the Lua code. With the
fix, the same config with the same parameters now show :
  - maxconn=5700 when neither side uses SSL
  - maxconn=1500 when only one side uses SSL
  - maxconn=1300 when both sides use SSL

This fix can be backported to versions 1.6 and beyond.

7 years agoDOC: add some description of the pending rework of the buffer structure
Willy Tarreau [Fri, 18 May 2018 14:18:17 +0000 (16:18 +0200)] 
DOC: add some description of the pending rework of the buffer structure

The "struct buffer" needs to be reworked, this new doc lists the changes
and steps to do this.

7 years agoBUG/MEDIUM: contrib/modsecurity: Use network order to encode/decode flags
Christopher Faulet [Fri, 18 May 2018 12:46:32 +0000 (14:46 +0200)] 
BUG/MEDIUM: contrib/modsecurity: Use network order to encode/decode flags

A recent fix on the SPOE revealed a mismatch between the SPOE specification and
the modsecurity implementation on the way flags are encoded or decoded. They
must be exchanged using the network bytes order and not the host one.

Be careful though, this patch breaks the compatiblity with HAProxy SPOE before
commit c4dcaff3 ("BUG/MEDIUM: spoe: Flags are not encoded in network order").

7 years agoBUG/MEDIUM: contrib/mod_defender: Use network order to encode/decode flags
Christopher Faulet [Fri, 18 May 2018 12:38:56 +0000 (14:38 +0200)] 
BUG/MEDIUM: contrib/mod_defender: Use network order to encode/decode flags

A recent fix on the SPOE revealed a mismatch between the SPOE specification and
the mod_defender implementation on the way flags are encoded or decoded. They
must be exchanged using the network bytes order and not the host one.

Be careful though, this patch breaks the compatiblity with HAProxy SPOE before
commit c4dcaff3 ("BUG/MEDIUM: spoe: Flags are not encoded in network order").

7 years agoDOC: spoe: fix a typo
Christopher Faulet [Thu, 26 Apr 2018 12:25:43 +0000 (14:25 +0200)] 
DOC: spoe: fix a typo

s/STATUC/STATUS/

7 years agoCLEANUP: spoe: Remove unused variables the agent structure
Christopher Faulet [Fri, 6 Apr 2018 09:34:12 +0000 (11:34 +0200)] 
CLEANUP: spoe: Remove unused variables the agent structure

applets_act and applets_idle were used for debugging purpose. Now, these values
are part of the agent's counters.

7 years agoBUG/MEDIUM: spoe: Flags are not encoded in network order
Thierry FOURNIER [Fri, 18 May 2018 10:25:39 +0000 (12:25 +0200)] 
BUG/MEDIUM: spoe: Flags are not encoded in network order

The flags are direct copy of the "unsigned int" in the network stream,
so the stream contains a 32 bits field encoded with the host endian.
 - This is not reliable for stream betwen different architecture host
 - For x86, the bits doesn't correspond to the documentation.

This patch add some precision in the documentation and put the bitfield
in the stream usig network butes order.

Warning: this patch can break compatibility with existing agents.

This patch should be backported in all version supporing SPOE

Original network capture:

   12:28:16.181343 IP 127.0.0.1.46782 > 127.0.0.1.12345: Flags [P.], seq 134:168, ack 59, win 342, options [nop,nop,TS val 2855241281 ecr 2855241281], length 34
           0x0000:  4500 0056 6b94 4000 4006 d10b 7f00 0001  E..Vk.@.@.......
           0x0010:  7f00 0001 b6be 3039 a3d1 ee54 7d61 d6f7  ......09...T}a..
           0x0020:  8018 0156 fe4a 0000 0101 080a aa2f 8641  ...V.J......./.A
           0x0030:  aa2f 8641 0000 001e 0301 0000 0000 010f  ./.A............
                                          ^^^^^^^^^^
           0x0040:  6368 6563 6b2d 636c 6965 6e74 2d69 7001  check-client-ip.
           0x0050:  0006 7f00 0001                           ......

Fixed network capture:

   12:24:26.948165 IP 127.0.0.1.46706 > 127.0.0.1.12345: Flags [P.], seq 4066280627:4066280661, ack 3148908096, win 342, options [nop,nop,TS val 2855183972 ecr 2855177690], length 34
           0x0000:  4500 0056 0538 4000 4006 3768 7f00 0001  E..V.8@.@.7h....
           0x0010:  7f00 0001 b672 3039 f25e 84b3 bbb0 8640  .....r09.^.....@
           0x0020:  8018 0156 fe4a 0000 0101 080a aa2e a664  ...V.J.........d
           0x0030:  aa2e 8dda 0000 001e 0300 0000 0114 010f  ................
                                          ^^^^^^^^^^
           0x0040:  6368 6563 6b2d 636c 6965 6e74 2d69 7001  check-client-ip.
           0x0050:  0006 7f00 0001                           ......

7 years agoBUG/MINOR: spoe: Mistake in error message about SPOE configuration
Thierry FOURNIER [Thu, 10 May 2018 14:41:26 +0000 (16:41 +0200)] 
BUG/MINOR: spoe: Mistake in error message about SPOE configuration

The announced accepted chars are "[a-zA-Z_-.]", but
the real accepted alphabet is "[a-zA-Z0-9_.]".

Numbers are supported and "-" is not supported.

This patch should be backported to 1.8 and 1.7

7 years agoBUG/MINOR: lua: Socket.send threw runtime error: 'close' needs 1 arguments.
sada [Fri, 11 May 2018 18:48:18 +0000 (11:48 -0700)] 
BUG/MINOR: lua: Socket.send threw runtime error: 'close' needs 1 arguments.

Function `hlua_socket_close` expected exactly one argument on the Lua stack.
But when `hlua_socket_close` was called from `hlua_socket_write_yield`,
Lua stack had 3 arguments. So `hlua_socket_close` threw the exception with
message "'close' needs 1 arguments".

Introduced new helper function `hlua_socket_close_helper`, which removed the
Lua stack argument count check and only checked if the first argument was
a socket.

This fix should be backported to 1.8, 1.7 and 1.6.

7 years agoBUG/MEDIUM: ssl: properly protect SSL cert generation
Willy Tarreau [Thu, 17 May 2018 08:56:47 +0000 (10:56 +0200)] 
BUG/MEDIUM: ssl: properly protect SSL cert generation

Commit 821bb9b ("MAJOR: threads/ssl: Make SSL part thread-safe") added
insufficient locking to the cert lookup and generation code : it uses
lru64_lookup(), which will automatically remove and add a list element
to the LRU list. It cannot be simply read-locked.

A long-term improvement should consist in using a lockless mechanism
in lru64_lookup() to safely move the list element at the head. For now
let's simply use a write lock during the lookup. The effect will be
minimal since it's used only in conjunction with automatically generated
certificates, which are much more expensive and rarely used.

This fix must be backported to 1.8.

7 years agoBUG/MEDIUM: http: don't always abort transfers on CF_SHUTR
Willy Tarreau [Wed, 16 May 2018 09:35:05 +0000 (11:35 +0200)] 
BUG/MEDIUM: http: don't always abort transfers on CF_SHUTR

Pawel Karoluk reported on Discourse[1] that HTTP/2 breaks url_param.

Christopher managed to track it down to the HTTP_MSGF_WAIT_CONN flag
which is set there to ensure the connection is validated before sending
the headers, as we may need to rewind the stream and hash again upon
redispatch. What happens is that in the forwarding code we refrain
from forwarding when this flag is set and the connection is not yet
established, and for this we go through the missing_data_or_waiting
path. This exit path was initially designed only to wait for data
from the client, so it rightfully checks whether or not the client
has already closed since in that case it must not wait for more data.
But it also has the side effect of aborting such a transfer if the
client has closed after the request, which is exactly what happens
in H2.

A study on the code reveals that this whole combined check should
be revisited : while it used to be true that waiting had the same
error conditions as missing data, it's not true anymore. Some other
corner cases were identified, such as the risk to report a server
close instead of a client timeout when waiting for the client to
read the last chunk of data if the shutr is already present, or
the risk to fail a redispatch when a client uploads some data and
closes before the connection establishes. The compression seems to
be at risk of rare issues there if a write to a full buffer is not
yet possible but a shutr is already queued.

At the moment these risks are extremely unlikely but they do exist,
and their impact is very minor since it mostly concerns an issue not
being optimally handled, and the fixes risk to cause more serious
issues. Thus this patch only focuses on how the HTTP_MSGF_WAIT_CONN
is handled and leaves the rest untouched.

This patch needs to be backported to 1.8, and could be backported to
earlier versions to properly take care of HTTP/1 requests passing via
url_param which are closed immediately after the headers, though this
is unlikely as this behaviour is only exhibited by scripts.

[1] https://discourse.haproxy.org/t/haproxy-1-8-x-url-param-issue-in-http2/2482/13

7 years agoBUG/MINOR: cli: don't stop cli_gen_usage_msg() when kw->usage == NULL
William Lallemand [Tue, 15 May 2018 09:50:04 +0000 (11:50 +0200)] 
BUG/MINOR: cli: don't stop cli_gen_usage_msg() when kw->usage == NULL

In commit abbf607 ("MEDIUM: cli: Add payload support") some cli keywords
without usage message have been added at the beginning of the keywords
array.

cli_gen_usage_usage_msg() use the kw->usage == NULL to stop generating
the usage message for the current keywords array. With those keywords at
the beginning, the whole array in cli.c was ignored in the usage message
generation.

This patch now checks the keyword itself, allowing a keyword without
usage message anywhere in the array.

7 years agoBUG/MEDIUM: pollers/kqueue: use incremented position in event list
PiBa-NL [Wed, 9 May 2018 23:01:28 +0000 (01:01 +0200)] 
BUG/MEDIUM: pollers/kqueue: use incremented position in event list

When composing the event list for subscribe to kqueue events, the index
where the new event is added must be after the previous events, as such
the changes counter should continue counting.

This caused haproxy to accept connections but not try read and process
the incoming data.

This patch is for 1.9 only

7 years agoBUG/MINOR: lua: ensure large proxy IDs can be represented
Willy Tarreau [Sun, 6 May 2018 12:50:09 +0000 (14:50 +0200)] 
BUG/MINOR: lua: ensure large proxy IDs can be represented

In function hlua_fcn_new_proxy() too small a buffer was passed to
snprintf(), resulting in large proxy or listener IDs to make
snprintf() fail. It is unlikely to meet this case but let's fix it
anyway.

This fix must be backported to all stable branches where it applies.

7 years agoBUG/MINOR: lua: schedule socket task upon lua connect()
PiBa-NL [Sat, 5 May 2018 21:51:42 +0000 (23:51 +0200)] 
BUG/MINOR: lua: schedule socket task upon lua connect()

The parameters like server-address, port and timeout should be set before
process_stream task is called to avoid the stream being 'closed' before it
got initialized properly. This is most clearly visible when running with
tune.lua.forced-yield=1.. So scheduling the task should not be done when
creating the lua socket, but when connect is called. The error
"socket: not yet initialised, you can't set timeouts." would then appear.

Below code for example also shows this issue, as the sleep will
yield the lua code:
  local con = core.tcp()
  core.sleep(1)
  con:settimeout(10)

7 years agoMINOR: pollers: move polled_mask outside of struct fdtab.
Olivier Houchard [Thu, 26 Apr 2018 12:23:07 +0000 (14:23 +0200)] 
MINOR: pollers: move polled_mask outside of struct fdtab.

The polled_mask is only used in the pollers, and removing it from the
struct fdtab makes it fit in one 64B cacheline again, on a 64bits machine,
so make it a separate array.

7 years agoBUG/MEDIUM: pollers: Use a global list for fd shared between threads.
Olivier Houchard [Wed, 25 Apr 2018 14:58:25 +0000 (16:58 +0200)] 
BUG/MEDIUM: pollers: Use a global list for fd shared between threads.

With the old model, any fd shared by multiple threads, such as listeners
or dns sockets, would only be updated on one threads, so that could lead
to missed event, or spurious wakeups.
To avoid this, add a global list for fd that are shared, using the same
implementation as the fd cache, and only remove entries from this list
when every thread as updated its poller.

[wt: this will need to be backported to 1.8 but differently so this patch
 must not be backported as-is]

7 years agoMINOR: fd: Make the lockless fd list work with multiple lists.
Olivier Houchard [Wed, 25 Apr 2018 13:10:30 +0000 (15:10 +0200)] 
MINOR: fd: Make the lockless fd list work with multiple lists.

Modify fd_add_to_fd_list() and fd_rm_from_fd_list() so that they take an
offset in the fdtab to the list entry, instead of hardcoding the fd cache,
so we can use them with other lists.

7 years agoBUG/MEDIUM: task: Don't free a task that is about to be run.
Olivier Houchard [Fri, 4 May 2018 13:46:16 +0000 (15:46 +0200)] 
BUG/MEDIUM: task: Don't free a task that is about to be run.

While running a task, we may try to delete and free a task that is about to
be run, because it's part of the local tasks list, or because rq_next points
to it.
So flag any task that is in the local tasks list to be deleted, instead of
run, by setting t->process to NULL, and re-make rq_next a global,
thread-local variable, that is modified if we attempt to delete that task.

Many thanks to PiBa-NL for reporting this and analysing the problem.

This should be backported to 1.8.

7 years agoBUG/MINOR: map: correctly track reference to the last ref_elt being dumped
Dragan Dosen [Fri, 4 May 2018 14:27:15 +0000 (16:27 +0200)] 
BUG/MINOR: map: correctly track reference to the last ref_elt being dumped

The bug was introduced in the commit 8d85aa4 ("BUG/MAJOR: map: fix
segfault during 'show map/acl' on cli").

This patch should be backported to 1.8, 1.7 and 1.6.

7 years agoMINOR: lua: add get_maxconn and set_maxconn to LUA Server class.
Patrick Hemmer [Sun, 29 Apr 2018 18:25:46 +0000 (14:25 -0400)] 
MINOR: lua: add get_maxconn and set_maxconn to LUA Server class.

7 years agoMINOR: lua: Add server name & puid to LUA Server class.
Patrick Hemmer [Sun, 29 Apr 2018 18:23:48 +0000 (14:23 -0400)] 
MINOR: lua: Add server name & puid to LUA Server class.

7 years agoDOC/MINOR: clean up LUA documentation re: servers & array/table.
Patrick Hemmer [Wed, 2 May 2018 01:30:41 +0000 (21:30 -0400)] 
DOC/MINOR: clean up LUA documentation re: servers & array/table.

* A few typos
* Fix definitions of values which are tables, not arrays.
* Consistent US English naming for "server" instead of "serveur".

[tfo: should be backported to 1.6 and higher]

7 years agoMINOR: backend: implement random-based load balancing
Willy Tarreau [Thu, 3 May 2018 05:20:40 +0000 (07:20 +0200)] 
MINOR: backend: implement random-based load balancing

For large farms where servers are regularly added or removed, picking
a random server from the pool can ensure faster load transitions than
when using round-robin and less traffic surges on the newly added
servers than when using leastconn.

This commit introduces "balance random". It internally uses a random as
the key to the consistent hashing mechanism, thus all features available
in consistent hashing such as weights and bounded load via hash-balance-
factor are usable. It is extremely convenient because one common concern
when using random is what happens when a server is hammered a bit too
much. Here that can trivially be avoided, like in the configuration below :

    backend bk0
        balance random
        hash-balance-factor 110
        server-template s 1-100 127.0.0.1:8000 check inter 1s

Note that while "balance random" internally relies on a hash algorithm,
it holds the same properties as round-robin and as such is compatible with
reusing an existing server connection with "option prefer-last-server".

7 years agoBUG/MINOR, BUG/MINOR: lua: Put tasks to sleep when waiting for data
PiBa-NL [Wed, 2 May 2018 20:27:14 +0000 (22:27 +0200)] 
BUG/MINOR, BUG/MINOR: lua: Put tasks to sleep when waiting for data

If a lua socket is waiting for data it currently spins at 100% cpu usage.
This because the TICK_ETERNITY returned by the socket is ignored when
setting the 'expire' time of the task.

Fixed by removing the check for yields that return TICK_ETERNITY.

This should be backported to at least 1.8.

7 years agoBUG/MEDIUM: threads: Fix the sync point for more than 32 threads
Christopher Faulet [Wed, 2 May 2018 14:58:40 +0000 (16:58 +0200)] 
BUG/MEDIUM: threads: Fix the sync point for more than 32 threads

In the sync point, to know if a thread has requested a synchronization, we call
the function thread_need_sync(). It should return 1 if yes, otherwise it should
return 0. It is intended to return a signed integer.

But internally, instead of returning 0 or 1, it returns 0 or tid_bit
(threads_want_sync & tid_bit). So, tid_bit is casted in integer. For the first
32 threads, it's ok, because we always check if thread_need_sync() returns
something else than 0. But this is a problem if HAProxy is started with more
than 32 threads, because for threads 33 to 64 (so for tid 32 to 63), their
tid_bit casted to integer are evaluated to 0. So the sync point does not work for
more than 32 threads.

Now, the function thread_need_sync() respects its contract, returning 0 or
1. the function thread_no_sync() has also been updated to avoid any ambiguities.

This patch must be backported in HAProxy 1.8.

7 years agoBUG/MINOR: checks: Fix check->health computation for flapping servers
Christopher Faulet [Wed, 2 May 2018 10:12:45 +0000 (12:12 +0200)] 
BUG/MINOR: checks: Fix check->health computation for flapping servers

This patch fixes an old bug introduced in the commit 7b1d47ce ("MAJOR: checks:
move health checks changes to set_server_check_status()"). When a DOWN server is
flapping, everytime a check succeds, check->health is incremented. But when a
check fails, it is decremented only when it is higher than the rise value. So if
only one check succeds for a DOWN server, check->health will remain set to 1 for
all subsequent failing checks.

So, at first glance, it seems not that terrible because the server remains
DOWN. But it is reported in the transitional state "DOWN server, going up". And
it will remain in this state until it is UP again. And there is also an
insidious side effect. If a DOWN server is flapping time to time, It will end to
be considered UP after a uniq successful check, , regardless the rise threshold,
because check->health will be increased slowly and never decreased.

To fix the bug, we just need to reset check->health to 0 when a check fails for
a DOWN server. To do so, we just need to relax the condition to handle a failure
in the function set_server_check_status.

This patch must be backported to haproxy 1.5 and newer.

7 years agoMINOR: ssl: add fetch 'ssl_fc_session_key' and 'ssl_bc_session_key'
Patrick Hemmer [Sat, 28 Apr 2018 23:15:51 +0000 (19:15 -0400)] 
MINOR: ssl: add fetch 'ssl_fc_session_key' and 'ssl_bc_session_key'

These fetches return the SSL master key of the front/back connection.
This is useful to decrypt traffic encrypted with ephemeral ciphers.

7 years agoMINOR: ssl: disable SSL sample fetches when unsupported
Patrick Hemmer [Sat, 28 Apr 2018 23:15:48 +0000 (19:15 -0400)] 
MINOR: ssl: disable SSL sample fetches when unsupported

Previously these fetches would return empty results when HAProxy was
compiled
without the requisite SSL support. This results in confusion and problem
reports from people who unexpectedly encounter the behavior.

7 years agoBUG/MINOR: config: disable http-reuse on TCP proxies
Willy Tarreau [Sat, 28 Apr 2018 05:18:15 +0000 (07:18 +0200)] 
BUG/MINOR: config: disable http-reuse on TCP proxies

Louis Chanouha reported an inappropriate warning when http-reuse is
present in a defaults section while a TCP proxy accidently inherits
it and finds a conflict with other options like the use of the PROXY
protocol. To fix this patch removes the http-reuse option for TCP
proxies.

This fix needs to be backported to 1.8, 1.7 and possibly 1.6.

7 years agoMINOR: http: Add support for 421 Misdirected Request
Tim Duesterhus [Fri, 27 Apr 2018 19:18:46 +0000 (21:18 +0200)] 
MINOR: http: Add support for 421 Misdirected Request

This makes haproxy aware of HTTP 421 Misdirected Request, which
is defined in RFC 7540, section 9.1.2.

7 years agoMINOR: sample: Add strcmp sample converter
Tim Duesterhus [Fri, 27 Apr 2018 19:18:45 +0000 (21:18 +0200)] 
MINOR: sample: Add strcmp sample converter

This converter supplements the existing string matching by allowing
strings to be converted to a variable.

Example usage:

  http-request set-var(txn.host) hdr(host)
  # Check whether the client is attempting domain fronting.
  acl ssl_sni_http_host_match ssl_fc_sni,strcmp(txn.host) eq 0

7 years agoBUG/MINOR: lua/threads: Make lua's tasks sticky to the current thread
Christopher Faulet [Wed, 25 Apr 2018 08:34:45 +0000 (10:34 +0200)] 
BUG/MINOR: lua/threads: Make lua's tasks sticky to the current thread

PiBa-NL reported a bug with tasks registered in lua when HAProxy is started with
serveral threads. These tasks have not specific affinity with threads so they
can be woken up on any threads. So, it is impossbile for these tasks to handled
cosockets or applets, because cosockets and applets are sticky on the thread
which created them. It is forbbiden to manipulate a cosocket from another
thread.

So to fix the bug, tasks registered in lua are now sticky to the current
thread. Because these tasks can be registered before threads creation, the
affinity is set the first time a lua's task is processed.

This patch must be backported in HAProxy 1.8.

7 years agoMINOR: ssl: Add payload support to "set ssl ocsp-response"
Aurélien Nephtali [Wed, 18 Apr 2018 12:04:58 +0000 (14:04 +0200)] 
MINOR: ssl: Add payload support to "set ssl ocsp-response"

It is now possible to use a payload with the "set ssl ocsp-response"
command.  These syntaxes will work the same way:

 # echo "set ssl ocsp-response $(base64 -w 10000 ocsp.der)" | \
     socat /tmp/sock1 -

 # echo -e "set ssl ocsp-response <<\n$(base64 ocsp.der)\n" | \
     socat /tmp/sock1 -

Signed-off-by: Aurélien Nephtali <aurelien.nephtali@corp.ovh.com>
7 years agoMINOR: map: Add payload support to "add map"
Aurélien Nephtali [Wed, 18 Apr 2018 12:04:47 +0000 (14:04 +0200)] 
MINOR: map: Add payload support to "add map"

It is now possible to use a payload with the "add map" command.
These syntaxes will work the same way:

 # echo "add map #-1 key value" | socat /tmp/sock1 -

 # echo -e "add map #-1 <<\n$(cat data)\n" | socat /tmp/sock1 -

with

 # cat data
 key1 value1 with spaces
 key2 value2
 key3 value3 also with spaces

Signed-off-by: Aurélien Nephtali <aurelien.nephtali@corp.ovh.com>
7 years agoMEDIUM: cli: Add payload support
Aurélien Nephtali [Wed, 18 Apr 2018 11:26:46 +0000 (13:26 +0200)] 
MEDIUM: cli: Add payload support

In order to use arbitrary data in the CLI (multiple lines or group of words
that must be considered as a whole, for example), it is now possible to add a
payload to the commands. To do so, the first line needs to end with a special
pattern: <<\n. Everything that follows will be left untouched by the CLI parser
and will be passed to the commands parsers.

Per-command support will need to be added to take advantage of this
feature.

Signed-off-by: Aurélien Nephtali <aurelien.nephtali@corp.ovh.com>
7 years agoBUG/MINOR: spoe: Fix parsing of dontlog-normal option
Christopher Faulet [Thu, 26 Apr 2018 09:36:34 +0000 (11:36 +0200)] 
BUG/MINOR: spoe: Fix parsing of dontlog-normal option

A missing goto led to a parsing error when line "option dontlog-normal" was
parsed.

7 years agoBUG/MINOR: spoe: Fix counters update when processing is interrupted
Christopher Faulet [Thu, 26 Apr 2018 09:33:44 +0000 (11:33 +0200)] 
BUG/MINOR: spoe: Fix counters update when processing is interrupted

When the processing is interrupted, because of a typo, <nb_sending> was
incremented instead of decremented.

7 years agoBUG/MEDIUM: h2: implement missing support for chunked encoded uploads
Willy Tarreau [Wed, 25 Apr 2018 18:44:22 +0000 (20:44 +0200)] 
BUG/MEDIUM: h2: implement missing support for chunked encoded uploads

Upload requests not carrying a content-length nor tunnelling data must
be sent chunked-encoded over HTTP/1. The code was planned but for some
reason forgotten during the implementation, leading to such payloads to
be sent as tunnelled data.

Browsers always emit a content length in uploads so this problem doesn't
happen for most sites. However some applications may send data frames
after a request without indicating it earlier.

The only way to detect that a client will need to send data is that the
HEADERS frame doesn't hold the ES bit. In this case it's wise to look
for the content-length header. If it's not there, either we're in tunnel
(CONNECT method) or chunked-encoding (other methods).

This patch implements this.

The following request is sent using content-length :

    curl --http2 -sk https://127.0.0.1:4443/s2 -XPOST -T /large/file

and these ones using chunked-encoding :

    curl --http2 -sk https://127.0.0.1:4443/s2 -XPUT -T /large/file
    curl --http2 -sk https://127.0.0.1:4443/s2 -XPUT -T - < /dev/urandom

Thanks to Robert Samuel Newson for raising this issue with details.
This fix must be backported to 1.8.

7 years agoMINOR: h2: detect presence of CONNECT and/or content-length
Willy Tarreau [Wed, 25 Apr 2018 16:13:58 +0000 (18:13 +0200)] 
MINOR: h2: detect presence of CONNECT and/or content-length

We'll need this in order to support uploading chunks. The h2 to h1
converter checks for the presence of the content-length header field
as well as the CONNECT method and returns these information to the
caller. The caller indicates whether or not a body is detected for
the message (presence of END_STREAM or not). No transfer-encoding
header is emitted yet.

7 years agoBUG/MEDIUM: lua: Fix segmentation fault if a Lua task exits
Tim Duesterhus [Tue, 24 Apr 2018 11:56:01 +0000 (13:56 +0200)] 
BUG/MEDIUM: lua: Fix segmentation fault if a Lua task exits

PiBa-NL reported that haproxy crashes with a segmentation fault
if a function registered using `core.register_task` returns.

An example Lua script that reproduces the bug is:

  mytask = function()
   core.Info("Stopping task")
  end
  core.register_task(mytask)

The Valgrind output is as follows:

  ==6759== Process terminating with default action of signal 11 (SIGSEGV)
  ==6759==  Access not within mapped region at address 0x20
  ==6759==    at 0x5B60AA9: lua_sethook (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==6759==    by 0x430264: hlua_ctx_resume (hlua.c:1009)
  ==6759==    by 0x43BB68: hlua_process_task (hlua.c:5525)
  ==6759==    by 0x4FED0A: process_runnable_tasks (task.c:231)
  ==6759==    by 0x4B2256: run_poll_loop (haproxy.c:2397)
  ==6759==    by 0x4B2256: run_thread_poll_loop (haproxy.c:2459)
  ==6759==    by 0x41A7E4: main (haproxy.c:3049)

Add the missing `task = NULL` for the `HLUA_E_OK` case. The error cases
have been fixed as of 253e53e661c49fb9723535319cf511152bf09bc7 which
first was included in haproxy v1.8-dev3. This bugfix should be backported
to haproxy 1.8.

7 years agoBUG/MINOR: log: t_idle (%Ti) is not set for some requests
Rian McGuire [Tue, 24 Apr 2018 14:19:21 +0000 (11:19 -0300)] 
BUG/MINOR: log: t_idle (%Ti) is not set for some requests

If TCP content inspection is used, msg_state can be >= HTTP_MSG_ERROR
the first time http_wait_for_request is called. t_idle was being left
unset in that case.

In the example below :
     stick-table type string len 64 size 100k expire 60s
     tcp-request inspect-delay 1s
     tcp-request content track-sc1 hdr(X-Session)

%Ti will always be -1, because the msg_state is already at HTTP_MSG_BODY
when http_wait_for_request is called for the first time.

This patch should backported to 1.8 and 1.7.

7 years agoBUG/MAJOR: channel: Fix crash when trying to read from a closed socket
Tim Duesterhus [Tue, 24 Apr 2018 17:20:43 +0000 (19:20 +0200)] 
BUG/MAJOR: channel: Fix crash when trying to read from a closed socket

When haproxy is compiled using GCC <= 3.x or >= 5.x the `unlikely`
macro performs a comparison with zero: `(x) != 0`, thus returning
either 0 or 1.

In `int co_getline_nc()` this macro was accidentally applied to
the variable `retcode` itself, instead of the result of the
comparison `retcode <= 0`. As a result any negative `retcode`
is converted to `1` for purposes of the comparison.
Thus never taking the branch (and exiting the function) for
negative values.

This in turn leads to reads of uninitialized memory in the for-loop
below:

  ==12141== Conditional jump or move depends on uninitialised value(s)
  ==12141==    at 0x4EB6B4: co_getline_nc (channel.c:346)
  ==12141==    by 0x421CA4: hlua_socket_receive_yield (hlua.c:1713)
  ==12141==    by 0x421F6F: hlua_socket_receive (hlua.c:1896)
  ==12141==    by 0x529B08F: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x52A7EFC: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529B497: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529711A: lua_pcallk (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x52ABDF0: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529B08F: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x52A7EFC: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529A9F1: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529B523: lua_resume (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==
  ==12141== Use of uninitialised value of size 8
  ==12141==    at 0x4EB6B9: co_getline_nc (channel.c:346)
  ==12141==    by 0x421CA4: hlua_socket_receive_yield (hlua.c:1713)
  ==12141==    by 0x421F6F: hlua_socket_receive (hlua.c:1896)
  ==12141==    by 0x529B08F: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x52A7EFC: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529B497: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529711A: lua_pcallk (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x52ABDF0: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529B08F: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x52A7EFC: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529A9F1: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529B523: lua_resume (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==
  ==12141== Invalid read of size 1
  ==12141==    at 0x4EB6B9: co_getline_nc (channel.c:346)
  ==12141==    by 0x421CA4: hlua_socket_receive_yield (hlua.c:1713)
  ==12141==    by 0x421F6F: hlua_socket_receive (hlua.c:1896)
  ==12141==    by 0x529B08F: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x52A7EFC: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529B497: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529711A: lua_pcallk (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x52ABDF0: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529B08F: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x52A7EFC: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529A9F1: ??? (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==    by 0x529B523: lua_resume (in /usr/lib/x86_64-linux-gnu/liblua5.3.so.0.0.0)
  ==12141==  Address 0x8637171e928bb500 is not stack'd, malloc'd or (recently) free'd

Fix this bug by correctly applying the `unlikely` macro to the result of the comparison.

This bug exists as of commit ca16b038132444dea06e6d83953034128a812bce
which is the first commit adding this function.

v1.6-dev1 is the first tag containing this commit, the fix should
be backported to haproxy 1.6 and newer.

7 years agoBUG/MINOR: pattern: Add a missing HA_SPIN_INIT() in pat_ref_newid()
Aurélien Nephtali [Thu, 19 Apr 2018 14:56:07 +0000 (16:56 +0200)] 
BUG/MINOR: pattern: Add a missing HA_SPIN_INIT() in pat_ref_newid()

pat_ref_newid() is lacking a spinlock init. It was probably forgotten
in b5997f740b ("MAJOR: threads/map: Make acls/maps thread safe").

Signed-off-by: Aurélien Nephtali <aurelien.nephtali@corp.ovh.com>
7 years agoDOC: lua: update the links to the config and Lua API
Willy Tarreau [Thu, 19 Apr 2018 13:12:26 +0000 (15:12 +0200)] 
DOC: lua: update the links to the config and Lua API

The links were still stuck to version 1.6. Let's update them.

The patch needs to be carefully backported to 1.8 and 1.7 after
editing the respective version (replace 1.9dev with 1.8 or 1.7).

7 years agoBUG/CRITICAL: h2: fix incorrect frame length check
Willy Tarreau [Tue, 17 Apr 2018 08:28:27 +0000 (10:28 +0200)] 
BUG/CRITICAL: h2: fix incorrect frame length check

The incoming H2 frame length was checked against the max_frame_size
setting instead of being checked against the bufsize. The max_frame_size
only applies to outgoing traffic and not to incoming one, so if a large
enough frame size is advertised in the SETTINGS frame, a wrapped frame
will be defragmented into a temporary allocated buffer where the second
fragment my overflow the heap by up to 16 kB.

It is very unlikely that this can be exploited for code execution given
that buffers are very short lived and their address not realistically
predictable in production, but the likeliness of an immediate crash is
absolutely certain.

This fix must be backported to 1.8.

Many thanks to Jordan Zebor from F5 Networks for reporting this issue
in a responsible way.

7 years agoBUILD: sample: avoid build warning in sample.c
Willy Tarreau [Thu, 19 Apr 2018 08:33:28 +0000 (10:33 +0200)] 
BUILD: sample: avoid build warning in sample.c

Recent commit 9631a28 ("MEDIUM: sample: Extend functionality for field/word
converters") introduced this minor build warning that this patch addresses :

 src/sample.c: In function 'sample_conv_word':
 src/sample.c:2108:8: warning: suggest explicit braces to avoid ambiguous 'else' [-Wparentheses]
 src/sample.c:2137:8: warning: suggest explicit braces to avoid ambiguous 'else' [-Wparentheses]

No backport is needed.

7 years agoBUG/MEDIUM: kqueue: When adding new events, provide an output to get errors.
Olivier Houchard [Mon, 16 Apr 2018 11:24:48 +0000 (13:24 +0200)] 
BUG/MEDIUM: kqueue: When adding new events, provide an output to get errors.

When adding new events using kevent(), if there's an error, because we're
trying to delete an event that wasn't there, or because the fd has already
been closed, kevent() will either add an event in the eventlist array if
there's enough room for it, and keep on handling other events, or stop and
return -1.
We want it to process all the events, so give it a large-enough array to
store any error.

Special thanks to PiBa-NL for diagnosing the root cause of this bug.

This should be backported to 1.8.

7 years agoMINOR: export localpeer as an environment variable
William Lallemand [Tue, 17 Apr 2018 14:46:13 +0000 (16:46 +0200)] 
MINOR: export localpeer as an environment variable

Export localpeer as the environment variable $HAPROXY_LOCALPEER,
allowing to use this variable in the configuration file.

It's useful to use this variable in the case of synchronized
configuration between peers.