]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
5 years agoMEDIUM: h1-htx: Add HTX EOM block when the message is in H1_MSG_DONE state
Christopher Faulet [Tue, 10 Dec 2019 10:47:22 +0000 (11:47 +0100)] 
MEDIUM: h1-htx: Add HTX EOM block when the message is in H1_MSG_DONE state

During H1 parsing, the HTX EOM block is added before switching the message state
to H1_MSG_DONE. It is an exception in the way to convert an H1 message to
HTX. Except for this block, the message is first switched to the right state
before starting to add the corresponding HTX blocks. For instance, the message
is switched in H1_MSG_DATA state and then the HTX DATA blocks are added.

With this patch, the message is switched to the H1_MSG_DONE state when all data
blocks or trailers were processed. It is the caller responsibility to call
h1_parse_msg_eom() when the H1_MSG_DONE state is reached. This way, it is far
easier to catch failures when the HTX buffer is full.

The H1 and FCGI muxes have been updated accordingly.

This patch may eventually be backported to 2.1 if it helps other backports.

5 years agoBUILD/MINOR: unix sockets: silence an absurd gcc warning about strncpy()
Willy Tarreau [Wed, 11 Dec 2019 15:29:10 +0000 (16:29 +0100)] 
BUILD/MINOR: unix sockets: silence an absurd gcc warning about strncpy()

Apparently gcc developers decided that strncpy() semantics are no longer
valid and now deserve a warning, especially if used exactly as designed.
This results in issue #304. Let's just remove one to the target size to
please her majesty gcc, the God of C Compilers, who tries hard to make
users completely eliminate any use of string.h and reimplement it by
themselves at much higher risks. Pfff....

This can be backported to stable version, the fix is harmless since it
ignores the last zero that is already set on next line.

5 years agoBUG/MINOR: listener: fix off-by-one in state name check
Willy Tarreau [Wed, 11 Dec 2019 14:51:37 +0000 (15:51 +0100)] 
BUG/MINOR: listener: fix off-by-one in state name check

As reported in issue #380, the state check in listener_state_str() is
invalid as it allows state value 9 to report crap. We don't use such
a state value so the issue should never happen unless the memory is
already corrupted, but better clean this now while it's harmless.

This should be backported to all maintained branches.

5 years agoBUG/MINOR: server: make "agent-addr" work on default-server line
Willy Tarreau [Wed, 11 Dec 2019 14:43:45 +0000 (15:43 +0100)] 
BUG/MINOR: server: make "agent-addr" work on default-server line

As reported in issue #408, "agent-addr" doesn't work on default-server
lines. This is due to the transcription of the old "addr" option in commit
6e5e0d8f9e ("MINOR: server: Make 'default-server' support 'addr' keyword.")
which correctly assigns it to the check.addr and agent.addr fields, but
which also copies the default check.addr into both the check's and the
agent's addr fields. Thus the default agent's address is never used.

This fix makes sure to copy the check from the check and the agent from
the agent. However it's worth noting that if "addr" is specified on the
server line, it will still overwrite both the check and the agent's
addresses.

This must be backported as far as 1.8.

5 years agoBUG/MINOR: listener: do not immediately resume on transient error
Willy Tarreau [Wed, 11 Dec 2019 14:06:30 +0000 (15:06 +0100)] 
BUG/MINOR: listener: do not immediately resume on transient error

The listener supports a "transient error" situation, which corresponds
to those situations where accept fails badly but poll() reports an event.
This happens for example when a listener is paused, or on out of FD. The
same mechanism is used when facing a maxconn or maxsessrate limitation.
When this happens, the listener is disabled for up to 100ms and put back
into the global listener queue so that it automatically wakes up again
as soon as the conditions change from an existing connection releasing
one resource, or the system recovers from a transient issue.

The listener_accept() function has a bug in its exit path causing a
freshly limited listener to be immediately enabled again because all
the conditions are met (connection count < max). It doesn't take into
account the fact that the listener might have been queued and must
first wait for the timeout to expire before doing so. The impact is
that upon certain errors, the faulty process will busy loop on the
accept code without sleeping. This is the scenario reported and
diagnosed by @hedong0411 in issue #382.

This commit fixes it by verifying that the global queue's delay is
at least expired before deciding to resume the listener. Another
approach could consist in having an extra state like LI_DELAY for
situations where only a delay is acceptable, but this would probably
not bring anything except more complex code.

This issue was introduced with the lock-free listener accept code
(commits 3f0d02b and 82c9789a) that were backported to 1.8.20+ and
1.9.7+, so this fix must be backported to the relevant branches.

5 years agoBUG/MINOR: mworker: properly pass SIGTTOU/SIGTTIN to workers
Willy Tarreau [Wed, 11 Dec 2019 13:24:07 +0000 (14:24 +0100)] 
BUG/MINOR: mworker: properly pass SIGTTOU/SIGTTIN to workers

If a new process is started with -sf and it fails to bind, it may send
a SIGTTOU to the master process in hope that it will temporarily unbind.
Unfortunately this one doesn't catch it and stops to background instead
of forwarding the signal to the workers. The same is true for SIGTTIN.

This commit simply implements an extra signal handler for the master to
deal with such signals that must be passed down to the workers. It must
be backported as far as 1.8, though there the code differs in that it's
entirely in haproxy.c and doesn't require an extra sig handler.

5 years agoBUG/MINOR: log: fix minor resource leaks on logformat error path
Willy Tarreau [Wed, 11 Dec 2019 11:05:39 +0000 (12:05 +0100)] 
BUG/MINOR: log: fix minor resource leaks on logformat error path

As reported by Ilya in issue #392, Coverity found that we're leaking
allocated strings on error paths in parse_logformat(). Let's use a
proper exit label for failures instead of seeding return 0 everywhere.

This should be backported to all supported versions.

5 years agoDOC: remove references to the outdated architecture.txt
Willy Tarreau [Wed, 11 Dec 2019 10:55:52 +0000 (11:55 +0100)] 
DOC: remove references to the outdated architecture.txt

As mentionned in bug #405 we continue to reference architecture.txt from
places in the doc despite this file not being packaged for many years.
Better drop the reference if it's confusing.

5 years agoDOC: proxies: HAProxy only supports 3 connection modes
Julien Pivotto [Tue, 10 Dec 2019 12:11:17 +0000 (13:11 +0100)] 
DOC: proxies: HAProxy only supports 3 connection modes

The 4th one (forceclose) has been deprecated and deleted from the
documentation in 10c6c16cde0b0b473a1ab16e958a7d6b61ed36fc

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
5 years agoMINOR: tasks: split wake_expired_tasks() in two parts to avoid useless wakeups
Willy Tarreau [Wed, 11 Dec 2019 07:12:23 +0000 (08:12 +0100)] 
MINOR: tasks: split wake_expired_tasks() in two parts to avoid useless wakeups

We used to have wake_expired_tasks() wake up tasks and return the next
expiration delay. The problem this causes is that we have to call it just
before poll() in order to consider latest timers, but this also means that
we don't wake up all newly expired tasks upon return from poll(), which
thus systematically requires a second poll() round.

This is visible when running any scheduled task like a health check, as there
are systematically two poll() calls, one with the interval, nothing is done
after it, and another one with a zero delay, and the task is called:

  listen test
    bind *:8001
    server s1 127.0.0.1:1111 check

  09:37:38.200959 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=8696843}) = 0
  09:37:38.200967 epoll_wait(3, [], 200, 1000) = 0
  09:37:39.202459 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=8712467}) = 0
>> nothing run here, as the expired task was not woken up yet.
  09:37:39.202497 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=8715766}) = 0
  09:37:39.202505 epoll_wait(3, [], 200, 0) = 0
  09:37:39.202513 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=8719064}) = 0
>> now the expired task was woken up
  09:37:39.202522 socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 7
  09:37:39.202537 fcntl(7, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
  09:37:39.202565 setsockopt(7, SOL_TCP, TCP_NODELAY, [1], 4) = 0
  09:37:39.202577 setsockopt(7, SOL_TCP, TCP_QUICKACK, [0], 4) = 0
  09:37:39.202585 connect(7, {sa_family=AF_INET, sin_port=htons(1111), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in progress)
  09:37:39.202659 epoll_ctl(3, EPOLL_CTL_ADD, 7, {EPOLLOUT, {u32=7, u64=7}}) = 0
  09:37:39.202673 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=8814713}) = 0
  09:37:39.202683 epoll_wait(3, [{EPOLLOUT|EPOLLERR|EPOLLHUP, {u32=7, u64=7}}], 200, 1000) = 1
  09:37:39.202693 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=8818617}) = 0
  09:37:39.202701 getsockopt(7, SOL_SOCKET, SO_ERROR, [111], [4]) = 0
  09:37:39.202715 close(7)                = 0

Let's instead split the function in two parts:
  - the first part, wake_expired_tasks(), called just before
    process_runnable_tasks(), wakes up all expired tasks; it doesn't
    compute any timeout.
  - the second part, next_timer_expiry(), called just before poll(),
    only computes the next timeout for the current thread.

Thanks to this, all expired tasks are properly woken up when leaving
poll, and each poll call's timeout remains up to date:

  09:41:16.270449 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=10223556}) = 0
  09:41:16.270457 epoll_wait(3, [], 200, 999) = 0
  09:41:17.270130 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=10238572}) = 0
  09:41:17.270157 socket(AF_INET, SOCK_STREAM, IPPROTO_TCP) = 7
  09:41:17.270194 fcntl(7, F_SETFL, O_RDONLY|O_NONBLOCK) = 0
  09:41:17.270204 setsockopt(7, SOL_TCP, TCP_NODELAY, [1], 4) = 0
  09:41:17.270216 setsockopt(7, SOL_TCP, TCP_QUICKACK, [0], 4) = 0
  09:41:17.270224 connect(7, {sa_family=AF_INET, sin_port=htons(1111), sin_addr=inet_addr("127.0.0.1")}, 16) = -1 EINPROGRESS (Operation now in progress)
  09:41:17.270299 epoll_ctl(3, EPOLL_CTL_ADD, 7, {EPOLLOUT, {u32=7, u64=7}}) = 0
  09:41:17.270314 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=10337841}) = 0
  09:41:17.270323 epoll_wait(3, [{EPOLLOUT|EPOLLERR|EPOLLHUP, {u32=7, u64=7}}], 200, 1000) = 1
  09:41:17.270332 clock_gettime(CLOCK_THREAD_CPUTIME_ID, {tv_sec=0, tv_nsec=10341860}) = 0
  09:41:17.270340 getsockopt(7, SOL_SOCKET, SO_ERROR, [111], [4]) = 0
  09:41:17.270367 close(7)                = 0

This may be backported to 2.1 and 2.0 though it's unlikely to bring any
user-visible improvement except to clarify debugging.

5 years agoBUG/MINOR: tasks: only requeue a task if it was already in the queue
Willy Tarreau [Wed, 11 Dec 2019 08:11:58 +0000 (09:11 +0100)] 
BUG/MINOR: tasks: only requeue a task if it was already in the queue

Commit 0742c314c3 ("BUG/MEDIUM: tasks: Make sure we switch wait queues
in task_set_affinity().") had a slight side effect on expired timeouts,
which is that when used before a timeout is updated, it will cause an
existing task to be requeued earlier than its expected timeout when done
before being updated, resulting in the next poll wakup timeout too early
or even instantly if the previous wake up was done on a timeout. This is
visible in strace when health checks are enabled because there are two
poll calls, one of which has a short or zero delay. The correct solution
is to only requeue a task if it was already in the queue.

This can be backported to all branches having the fix above.

5 years agoDOC: listeners: add a few missing transitions
Willy Tarreau [Wed, 11 Dec 2019 05:38:14 +0000 (06:38 +0100)] 
DOC: listeners: add a few missing transitions

Some disable() transitions were missing, and the distinction between
multi-threaded and single-threaded transitions was not mentioned.

5 years agoBUG/MEDIUM: proto_udp/threads: recv() and send() must not be exclusive.
Willy Tarreau [Tue, 10 Dec 2019 17:12:04 +0000 (18:12 +0100)] 
BUG/MEDIUM: proto_udp/threads: recv() and send() must not be exclusive.

This is a complement to previous fix for bug #399. The exclusion between
the recv() and send() calls prevents send handlers from being called if
rx readiness is reported. The DNS code can trigger this situations with
threads where the fd_recv_ready() flag disappears between the test in
dgram_fd_handler() and the second test in dns_resolve_recv() while a
thread calls fd_cant_recv(), and this situation can sustain itself for
a while. With 8 threads and an error in the socket queue, placing a
printf on the return statement in dns_resolve_recv() scrolls very fast.

Simply removing the "else" in dgram_fd_handler() addresses the issue.

This fix must be backported as far as 1.6.

5 years agoBUG/MAJOR: dns: add minimalist error processing on the Rx path
Willy Tarreau [Tue, 10 Dec 2019 17:38:09 +0000 (18:38 +0100)] 
BUG/MAJOR: dns: add minimalist error processing on the Rx path

It was reported in bug #399 that the DNS sometimes enters endless loops
after hours working fine. The issue is caused by a lack of error
processing in the DNS's recv() path combined with an exclusive recv OR
send in the UDP layer, resulting in some errors causing CPU loops that
will never stop until the process is restarted.

The basic cause is that the FD_POLL_ERR and FD_POLL_HUP flags are sticky
on the FD, and contrary to a stream socket, receiving an error on a
datagram socket doesn't indicate that this socket cannot be used anymore.
Thus the Rx code must at least handle this situation and flush the error
otherwise it will constantly be reported. In theory this should not be a
big issue but in practise it is due to another bug in the UDP datagram
handler which prevents the send() callback from being called when Rx
readiness was reported, so the situation cannot go away. It happens way
more easily with threads enabled, so that there is no dead time between
the moment the FD is disabled and another recv() is called, such as in
the example below where the request was sent to a closed port on the
loopback provoking an ICMP unreachable to be sent back:

  [pid 20888] 18:26:57.826408 sendto(29, ";\340\1\0\0\1\0\0\0\0\0\1\0031wt\2eu\0\0\34\0\1\0\0)\2\0\0\0\0\0\0\0", 35, 0, NULL, >
  [pid 20893] 18:26:57.826566 recvfrom(29, 0x7f97c54ef2f0, 513, 0, NULL, NULL) = -1 ECONNREFUSED (Connection refused)
  [pid 20889] 18:26:57.826601 recvfrom(29, 0x7f97c76182f0, 513, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
  [pid 20892] 18:26:57.826630 recvfrom(29, 0x7f97c5cf02f0, 513, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
  [pid 20891] 18:26:57.826684 recvfrom(29, 0x7f97c66162f0, 513, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
  [pid 20895] 18:26:57.826716 recvfrom(29, 0x7f97bffda2f0, 513, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
  [pid 20894] 18:26:57.826747 recvfrom(29, 0x7f97c4cee2f0, 513, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
  [pid 20888] 18:26:58.419838 recvfrom(29, 0x7ffcc8712c20, 513, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
  [pid 20893] 18:26:58.419900 recvfrom(29, 0x7f97c54ef2f0, 513, 0, NULL, NULL) = -1 EAGAIN (Resource temporarily unavailable)
  (... hundreds before next sendto() ...)

This situation was handled by clearing HUP and ERR when recv()
returns <0.

A second case was handled, there was a control for a missing dgram
handler, but it does nothing, causing the FD to ring again if this
situation ever happens. After looking at the rest of the code, it
doesn't seem possible to face such a situation because these handlers
are registered during startup, but at least we need to handle it
properly.

A third case was handled, that's mainly a small optimization. With
threads and massive responses, due to the large lock around the loop,
it's likely that some threads will have seen fd_recv_ready() and will
wait at the lock(). But if they wait here, chances are that other
threads will have eliminated pending data and issued fd_cant_recv().
In this case, better re-check fd_recv_ready() before performing the
recv() call to avoid the huge amounts of syscalls that happen on
massively threaded setups.

This patch must be backported as far as 1.6 (the atomic AND just
needs to be turned to a regular AND).

5 years agoBUG/MEDIUM: kqueue: Make sure we report read events even when no data.
Olivier Houchard [Tue, 10 Dec 2019 17:22:55 +0000 (18:22 +0100)] 
BUG/MEDIUM: kqueue: Make sure we report read events even when no data.

When we have a EVFILT_READ event, an optimization was made, and the FD was
not reported as ready to receive if there were no data available. That way,
if the socket was closed by our peer (the EV8EOF flag was set), and there were
no remaining data to read, we would just close(), and avoid doing a recv().
However, it may be fine for TCP socket, but it is not for UDP.
If we send data via UDP, and we receive an error, the only way to detect it
is to attempt a recv(). However, in this case, kevent() will report a read
event, but with no data, so we'd just ignore that read event, nothing would be
done about it, and the poller would be woken up by it over and over.
To fix this, report read events if either we have data, or the EV_EOF flag
is not set.

This should be backported to 2.1, 2.0, 1.9 and 1.8.

5 years agoDOC: document the listener state transitions
Willy Tarreau [Tue, 10 Dec 2019 15:06:53 +0000 (16:06 +0100)] 
DOC: document the listener state transitions

This was done by reading all the code affecting a listener's state,
hopefully it will save some time in the future.

5 years agoREORG: listener: move the global listener queue code to listener.c
Willy Tarreau [Tue, 10 Dec 2019 10:18:41 +0000 (11:18 +0100)] 
REORG: listener: move the global listener queue code to listener.c

The global listener queue code and declarations were still lying in
haproxy.c while not needed there anymore at all. This complicates
the code for no reason. As a result, the global_listener_queue_task
and the global_listener_queue were made static.

5 years agoMINOR: listener: split dequeue_all_listener() in two
Willy Tarreau [Tue, 10 Dec 2019 13:10:52 +0000 (14:10 +0100)] 
MINOR: listener: split dequeue_all_listener() in two

We use it half times for the global_listener_queue and half times
for a proxy's queue and this requires the callers to take care of
these. Let's split it in two versions, the current one working only
on the global queue and another one dedicated to proxies for the
per-proxy queues. This cleans up quite a bit of code.

5 years agoMINOR: listener: make the wait paths cleaner and more reliable
Willy Tarreau [Tue, 10 Dec 2019 11:01:21 +0000 (12:01 +0100)] 
MINOR: listener: make the wait paths cleaner and more reliable

In listener_accept() there are several situations where we have to wait
for an event or a delay. These ones all implement their own call to
limit_listener() and the associated task_schedule(). In addition to
being ugly and confusing, one expire date computation is even wrong as
it doesn't take in account the fact that we're using threads and that
the value might change in the middle. Fortunately task_schedule() gets
it right for us.

This patch creates two jump locations, one for the global queue and
one for the proxy queue, allowing the rest of the code to only compute
the expire delay and jump to the right location.

5 years agoBUG/MEDIUM: listener/threads: fix a remaining race in the listener's accept()
Willy Tarreau [Tue, 10 Dec 2019 08:30:05 +0000 (09:30 +0100)] 
BUG/MEDIUM: listener/threads: fix a remaining race in the listener's accept()

Recent fix 4c044e274c ("BUG/MEDIUM: listener/thread: fix a race when
pausing a listener") is insufficient and moves the race slightly farther.
What now happens is that if we're limiting a listener due to a transient
error such as an accept() error for example, or because the proxy's
maxconn was reached, another thread might in the mean time have switched
again to LI_READY and at the end of the function we'll disable polling on
this FD, resulting in a listener that never accepts anything anymore. It
can more easily happen when sending SIGTTOU/SIGTTIN to temporarily pause
the listeners to let another process bind next to them.

What this patch does instead is to move all enable/disable operations at
the end of the function and condition them to the state. The listener's
state is checked under the lock and the FD's polling state adjusted
accordingly so that the listener's state and the FD always remain 100%
synchronized. It was verified with 16 threads that the cost of taking
that lock is not measurable so that's fine.

This should be backported to the same branches the patch above is
backported to.

5 years agoBUG/MINOR: listener: also clear the error flag on a paused listener
Willy Tarreau [Tue, 10 Dec 2019 07:42:21 +0000 (08:42 +0100)] 
BUG/MINOR: listener: also clear the error flag on a paused listener

When accept() fails because a listener is temporarily paused, the
FD might have both FD_POLL_HUP and FD_POLL_ERR bits set. While we do
not exploit FD_POLL_ERR here it's better to clear it because it is
reported on "show fd" and is confusing.

This may be backported to all versions.

5 years agoBUG/MINOR: listener/threads: always use atomic ops to clear the FD events
Willy Tarreau [Tue, 10 Dec 2019 07:37:04 +0000 (08:37 +0100)] 
BUG/MINOR: listener/threads: always use atomic ops to clear the FD events

There was a leftover of the single-threaded era when removing the
FD_POLL_HUP flag from the listeners. By not using an atomic operation
to clear the flag, another thread acting on the same listener might
have lost some events, though this would have resulted in that thread
to reprocess them immediately on the next loop pass.

This should be backported as far as 1.8.

5 years agoBUG/MINOR: proxy: make soft_stop() also close FDs in LI_PAUSED state
Willy Tarreau [Tue, 10 Dec 2019 06:11:35 +0000 (07:11 +0100)] 
BUG/MINOR: proxy: make soft_stop() also close FDs in LI_PAUSED state

The proxies' soft_stop() function closes the FDs in all opened states
except LI_PAUSED. This means that a transient error on a listener might
cause it to turn back to the READY state if it happens exactly when a
reload signal is received.

This must be backported to all supported versions.

5 years agoBUG/MEDIUM: mux-fcgi: Handle cases where the HTX EOM block cannot be inserted
Christopher Faulet [Fri, 6 Dec 2019 15:20:49 +0000 (16:20 +0100)] 
BUG/MEDIUM: mux-fcgi: Handle cases where the HTX EOM block cannot be inserted

During the HTTP response parsing, if there is not enough space in the channel's
buffer, it is possible to fail to add the HTX EOM block while all data in the
rxbuf were consumed. As for the h1 mux, we must notify the conn-stream the
buffer is full to have a chance to add the HTX EOM block later. In this case, we
must also be carefull to not report a server abort by setting too early the
CS_FL_EOS flag on the conn-stream.

To do so, the FCGI_SF_APPEND_EOM flag must be set on the FCGI stream to know the
HTX EOM block is missing.

This patch must be backported to 2.1.

5 years agoBUG/MINOR: mux-h1: Be sure to set CS_FL_WANT_ROOM when EOM can't be added
Christopher Faulet [Fri, 6 Dec 2019 14:59:05 +0000 (15:59 +0100)] 
BUG/MINOR: mux-h1: Be sure to set CS_FL_WANT_ROOM when EOM can't be added

During the message parsing, when the HTX buffer is full and only the HTX EOM
block cannot be added, it is important to notify the conn-stream that some
processing must still be done but it is blocked because there is not enough room
in the buffer. The way to do so is to set the CS_FL_WANT_ROOM flag on the
conn-stream. Otherwise, because all data are received and consumed, the mux is
not called anymore to add this last block, leaving the message unfinished from
the HAProxy point of view. The only way to unblock it is to receive a shutdown
for reads or to hit a timeout.

This patch must be backported to 2.1 and 2.0. The 1.9 does not seem to be
affected.

5 years agoMEDIUM: init: set NO_NEW_PRIVS by default when supported
Willy Tarreau [Fri, 6 Dec 2019 15:31:45 +0000 (16:31 +0100)] 
MEDIUM: init: set NO_NEW_PRIVS by default when supported

HAProxy doesn't need to call executables at run time (except when using
external checks which are strongly recommended against), and is even expected
to isolate itself into an empty chroot. As such, there basically is no valid
reason to allow a setuid executable to be called without the user being fully
aware of the risks. In a situation where haproxy would need to call external
checks and/or disable chroot, exploiting a vulnerability in a library or in
haproxy itself could lead to the execution of an external program. On Linux
it is possible to lock the process so that any setuid bit present on such an
executable is ignored. This significantly reduces the risk of privilege
escalation in such a situation. This is what haproxy does by default. In case
this causes a problem to an external check (for example one which would need
the "ping" command), then it is possible to disable this protection by
explicitly adding this directive in the global section. If enabled, it is
possible to turn it back off by prefixing it with the "no" keyword.

Before the option:

  $ socat - /tmp/sock1 <<< "expert-mode on; debug dev exec sudo /bin/id"
  uid=0(root) gid=0(root) groups=0(root

After the option:
  $ socat - /tmp/sock1 <<< "expert-mode on; debug dev exec sudo /bin/id"
  sudo: effective uid is not 0, is /usr/bin/sudo on a file system with the
        'nosuid' option set or an NFS file system without root privileges?

5 years agoMINOR: debug: replace popen() with pipe+fork() in "debug dev exec"
Willy Tarreau [Fri, 6 Dec 2019 16:18:28 +0000 (17:18 +0100)] 
MINOR: debug: replace popen() with pipe+fork() in "debug dev exec"

popen() is annoying because it doesn't catch stderr. The command was
implemented using it just by pure laziness, let's just redo it a bit
cleaner using normal syscalls. Note that this command is only enabled
when built with -DDEBUG_DEV.

5 years agoBUG/MEDIUM: checks: Make sure we set the task affinity just before connecting.
Olivier Houchard [Fri, 29 Nov 2019 15:18:51 +0000 (16:18 +0100)] 
BUG/MEDIUM: checks: Make sure we set the task affinity just before connecting.

In process_chk_conn(), make sure we set the task affinity to the current
thread as soon as we're attempting a connection (and reset the affinity to
"any thread" if we detect a failure).
We used to only set the task affinity if connect_conn_chk() returned
SF_ERR_NONE, however for TCP checks, SF_ERR_UP is returned, so for those
checks, the task could still run on any thread, and this could lead to a
race condition where the connection runs on one thread, while the task runs
on another one, which could create random memory corruption and/or crashes.
This may fix github issue #369.

This should be backported to 2.1, 2.0 and 1.9.

5 years agoBUG/MEDIUM: tasks: Make sure we switch wait queues in task_set_affinity().
Olivier Houchard [Thu, 5 Dec 2019 14:11:19 +0000 (15:11 +0100)] 
BUG/MEDIUM: tasks: Make sure we switch wait queues in task_set_affinity().

In task_set_affinity(), leave the wait_queue if any before changing the
affinity, and re-enter a wait queue once it is done. If we don't do that,
the task may stay in the wait queue of another thread, and we later may
end up modifying that wait queue while holding no lock, which could lead
to memory corruption.

THis should be backported to 2.1, 2.0 and 1.9.

5 years agoBUG/MINOR: mux-h1: Fix conditions to know whether or not we may receive data
Christopher Faulet [Thu, 5 Dec 2019 11:30:55 +0000 (12:30 +0100)] 
BUG/MINOR: mux-h1: Fix conditions to know whether or not we may receive data

The h1_recv_allowed() function is inherited from the h2 multiplexer. But for the
h1, conditions to know if we may receive data are less complex because there is
no multiplexing and because data are not parsed when received. So now, following
rules are respected :

 * if an error or a shutdown for reads was detected on the connection we must
   not attempt to receive
 * if the input buffer failed to be allocated or is full, we must not try to
   receive
 * if the input processing is busy waiting for the output side, we may attempt
   to receive
 * otherwise must may not attempt to receive

This patch must be backported as far as 1.9.

5 years agoBUG/MINOR: mux-h1: Don't rely on CO_FL_SOCK_RD_SH to set H1C_F_CS_SHUTDOWN
Christopher Faulet [Thu, 5 Dec 2019 10:18:31 +0000 (11:18 +0100)] 
BUG/MINOR: mux-h1: Don't rely on CO_FL_SOCK_RD_SH to set H1C_F_CS_SHUTDOWN

The CO_FL_SOCK_RD_SH flag is only set when a read0 is received. So we must not
rely on it to set the H1 connection in shutdown state (H1C_F_CS_SHUTDOWN). In
fact, it is suffisant to set the connection in shutdown state when the shutdown
for writes is forwared to the sock layer.

This patch must be backported as far as 1.9.

5 years agoBUG/MEDIUM: mux-h1: Never reuse H1 connection if a shutw is pending
Christopher Faulet [Thu, 5 Dec 2019 09:23:37 +0000 (10:23 +0100)] 
BUG/MEDIUM: mux-h1: Never reuse H1 connection if a shutw is pending

On the server side, when a H1 stream is detached from the connection, if the
connection is not reusable but some outgoing data remain, the connection is not
immediatly released. In this case, the connection is not inserted in any idle
connection list. But it is still attached to the session. Because of that, it
can be erroneously reused. h1_avail_streams() always report a free slot if no
stream is attached to the connection, independently on the connection's
state. It is obviously a bug. If a second request is handled by the same session
(it happens with H2 connections on the client side), this connection is reused
before we close it.

There is small window to hit the bug, but it may lead to very strange
behaviors. For instance, if a first h2 request is quickly aborted by the client
while it is blocked in the mux on the server side (so before any response is
received), a second request can be processed and sent to the server. Because the
connection was not closed, the possible reply to the first request will be
interpreted as a reply to the second one. It is probably the bug described by
Peter Fröhlich in the issue #290.

To fix the bug, a new flag has been added to know if an H1 connection is idle or
not. So now, H1C_F_CS_IDLE is set when a connection is idle and useable to
handle a new request. If it is set, we try to add the connection in an idle
connection list. And h1_avail_streams() only relies on this flag
now. Concretely, this flag is set when a K/A stream is detached and both the
request and the response are in DONE state. It is exclusive to other H1C_F_CS
flags.

This patch must be backported as far as 1.9.

5 years agoBUG/MINOR: ssl: certificate choice can be unexpected with openssl >= 1.1.1
Emmanuel Hocdet [Wed, 6 Nov 2019 15:05:34 +0000 (16:05 +0100)] 
BUG/MINOR: ssl: certificate choice can be unexpected with openssl >= 1.1.1

It's regression from 9f9b0c6 "BUG/MEDIUM: ECC cert should work with
TLS < v1.2 and openssl >= 1.1.1". Wilcard EC certifcate could be selected
at the expense of specific RSA certificate.
In any case, specific certificate should always selected first, next wildcard.
Reflect this rule in a loop to avoid any bug in certificate selection changes.

Fix issue #394.

It should be backported as far as 1.8.

5 years agoBUG/MEDIUM: listener/thread: fix a race when pausing a listener
Willy Tarreau [Thu, 5 Dec 2019 06:40:32 +0000 (07:40 +0100)] 
BUG/MEDIUM: listener/thread: fix a race when pausing a listener

There exists a race in the listener code where a thread might disable
receipt on a listener's FD then turn it to LI_PAUSED while at the same
time another one faces EAGAIN on accept() and enables it again via
fd_cant_recv(). The result is that the FD is in LI_PAUSED state with
its polling still enabled. listener_accept() does not do anything then
and doesn't disable the FD either, resulting in a thread eating all the
CPU as reported in issue #358. A solution would be to take the listener's
lock to perform the fd_cant_recv() call and do it only if the FD is still
in LI_READY state, but this would be totally overkill while in practice
the issue only happens during shutdown.

Instead what is done here is that when leaving we recheck the state and
disable polling if the listener is not in LI_READY state, which never
happens except when being limited. In the worst case there could be one
extra check per thread for the time required to converge, which is
absolutely nothing.

This fix was successfully tested, and should be backported to all
versions using the lock-free listeners, which means all those containing
commit 3f0d02bb ("MAJOR: listener: do not hold the listener lock in
listener_accept()"), hence 2.1, 2.0, 1.9.7+, 1.8.20+.

5 years agoBUG/MINOR: ssl/cli: don't overwrite the filters variable
William Lallemand [Wed, 4 Dec 2019 14:33:01 +0000 (15:33 +0100)] 
BUG/MINOR: ssl/cli: don't overwrite the filters variable

When a crt-list line using an already used ckch_store does not contain
filters, it will overwrite the ckchs->filters variable with 0.
This problem will generate all sni_ctx of this ckch_store without
filters. Filters generation mustn't be allowed in any case.

Must be backported in 2.1.

5 years agoBUG/MINOR: stream-int: avoid calling rcv_buf() when splicing is still possible
Willy Tarreau [Tue, 3 Dec 2019 17:13:04 +0000 (18:13 +0100)] 
BUG/MINOR: stream-int: avoid calling rcv_buf() when splicing is still possible

In si_cs_recv(), we can end up with a partial splice() call that will be
followed by an attempt to us rcv_buf(). Sometimes this works and places
data into the buffer, which then prevent splicing from being used, and
this causes splice() and recvfrom() calls to alternate. Better simply
refrain from calling rcv_buf() when there are data in the pipe and still
data to be forwarded. Usually this indicates that we've ate everything
available and that we still want to use splice() on subsequent calls.

This should be backported to 2.1 and 2.0.

5 years agoBUG/MEDIUM: stream-int: don't subscribed for recv when we're trying to flush data
Willy Tarreau [Tue, 3 Dec 2019 17:08:45 +0000 (18:08 +0100)] 
BUG/MEDIUM: stream-int: don't subscribed for recv when we're trying to flush data

If we cannot splice incoming data using rcv_pipe() due to remaining data
in the buffer, we must not subscribe to the mux but instead tag the
stream-int as blocked on missing Rx room. Otherwise when data are
flushed, calling si_chk_rcv() will have no effect because the WAIT_EP
flag remains present, and we'll end in an rx timeout. This case is very
hard to reproduce, and requires an inversion of the polling side in the
middle of a transfer. This can only happen when the client and the server
are using similar links and when splicing is enabled. It typically takes
hundreds of MB to GB for the problem to happen, and tends to be magnified
by the use of option contstats which causes process_stream() to be called
every 5s and to try again to recv.

This fix must be backported to 2.1, 2.0, and possibly 1.9.

5 years agoBUG/MINOR: ssl/cli: 'ssl cert' cmd only usable w/ admin rights
William Lallemand [Tue, 3 Dec 2019 12:32:54 +0000 (13:32 +0100)] 
BUG/MINOR: ssl/cli: 'ssl cert' cmd only usable w/ admin rights

The 3 commands 'set ssl cert', 'abort ssl cert' and 'commit ssl cert'
must be only usable with admin rights over the CLI.

Must be backported in 2.1.

5 years agoMEDIUM: init: prevent process and thread creation at runtime
Willy Tarreau [Tue, 3 Dec 2019 06:07:36 +0000 (07:07 +0100)] 
MEDIUM: init: prevent process and thread creation at runtime

Some concerns are regularly raised about the risk to inherit some Lua
files which make use of a fork (e.g. via os.execute()) as well as
whether or not some of bugs we fix might or not be exploitable to run
some code. Given that haproxy is event-driven, any foreground activity
completely stops processing and is easy to detect, but background
activity is a different story. A Lua script could very well discretely
fork a sub-process connecting to a remote location and taking commands,
and some injected code could also try to hide its activity by creating
a process or a thread without blocking the rest of the processing. While
such activities should be extremely limited when run in an empty chroot
without any permission, it would be better to get a higher assurance
they cannot happen.

This patch introduces something very simple: it limits the number of
processes and threads to zero in the workers after the last thread was
created. By doing so, it effectively instructs the system to fail on
any fork() or clone() syscall. Thus any undesired activity has to happen
in the foreground and is way easier to detect.

This will obviously break external checks (whose concept is already
totally insecure), and for this reason a new option
"insecure-fork-wanted" was added to disable this protection, and it
is suggested in the fork() error report from the checks. It is
obviously recommended not to use it and to reconsider the reasons
leading to it being enabled in the first place.

If for any reason we fail to disable forks, we still start because it
could be imaginable that some operating systems refuse to set this
limit to zero, but in this case we emit a warning, that may or may not
be reported since we're after the fork point. Ideally over the long
term it should be conditionned by strict-limits and cause a hard fail.

5 years agoDOC: move the "group" keyword at the right place
Willy Tarreau [Tue, 3 Dec 2019 07:29:22 +0000 (08:29 +0100)] 
DOC: move the "group" keyword at the right place

It looks like "hard-stop-after", "h1-case-adjust" and "h1-case-adjust-file"
were added before "group", breaking alphabetical ordering.

5 years agoDOC: Fix ordered list in summary
Julien Pivotto [Wed, 27 Nov 2019 14:49:54 +0000 (15:49 +0100)] 
DOC: Fix ordered list in summary

Section 6 about the cache was placed between 7 and 8. This should
be backported to 2.1.

Signed-off-by: Julien Pivotto <roidelapluie@inuits.eu>
5 years agoDOC: clarify matching strings on binary fetches
Mathias Weiersmueller [Mon, 2 Dec 2019 08:43:40 +0000 (09:43 +0100)] 
DOC: clarify matching strings on binary fetches

Add clarification and example to string matching on binary samples,
as comparison stops at first null byte due to strncmp behaviour.

Backporting all the way down to 1.5 is suggested as it might save
from headaches.

5 years agoBUG/MINOR: ssl: fix X509 compatibility for openssl < 1.1.0
Emmanuel Hocdet [Mon, 2 Dec 2019 10:41:23 +0000 (11:41 +0100)] 
BUG/MINOR: ssl: fix X509 compatibility for openssl < 1.1.0

Commit d4f9a60e "MINOR: ssl: deduplicate ca-file" uses undeclared X509
functions when build with openssl < 1.1.0. Introduce this functions
in openssl-compat.h .

Fix issue #385.

5 years agoBUG/MINOR: stats: Fix HTML output for the frontends heading
Christopher Faulet [Mon, 2 Dec 2019 10:29:04 +0000 (11:29 +0100)] 
BUG/MINOR: stats: Fix HTML output for the frontends heading

Since the flag STAT_SHOWADMIN was removed, the frontends heading in the HTML
output appears unaligned because the space reserved for the checkbox (not
displayed for frontends) is not inserted.

This patch fixes the issue #390. It must be backported to 2.1.

5 years agoBUG/MINOR: fcgi-app: Make the directive pass-header case insensitive
Christopher Faulet [Mon, 2 Dec 2019 09:33:31 +0000 (10:33 +0100)] 
BUG/MINOR: fcgi-app: Make the directive pass-header case insensitive

The header name configured by the directive "pass-header", in the "fcgi-app"
section, must be case insensitive. For now, it must be in lowercase to match an
header. Internally, header names are in lowercase but there is no reason to
impose this syntax in the configuration.

This patch must be backported to 2.1.

5 years agoBUG/MINOR: ssl: fix SSL_CTX_set1_chain compatibility for openssl < 1.0.2
Emmanuel Hocdet [Thu, 24 Oct 2019 16:33:10 +0000 (18:33 +0200)] 
BUG/MINOR: ssl: fix SSL_CTX_set1_chain compatibility for openssl < 1.0.2

Commit 1c65fdd5 "MINOR: ssl: add extra chain compatibility" really implement
SSL_CTX_set0_chain. Since ckch can be used to init more than one ctx with
openssl < 1.0.2 (commit 89f58073 for X509_chain_up_ref compatibility),
SSL_CTX_set1_chain compatibility is required.

This patch must be backported to 2.1.

5 years agoDOC: ssl/cli: set/commit/abort ssl cert
William Lallemand [Fri, 29 Nov 2019 15:48:43 +0000 (16:48 +0100)] 
DOC: ssl/cli: set/commit/abort ssl cert

Document the "set/commit/abort ssl cert" CLI commands in management.txt.

Must be backported in 2.1.

5 years agoBUG/MINOR: http-htx: Don't make http_find_header() fail if the value is empty
Christopher Faulet [Fri, 29 Nov 2019 10:18:45 +0000 (11:18 +0100)] 
BUG/MINOR: http-htx: Don't make http_find_header() fail if the value is empty

http_find_header() is used to find the next occurrence of a header matching on
its name. When found, the matching header is returned with the corresponding
value. This value may be empty. Unfortunatly, because of a bug, an empty value
make the function fail.

This patch must be backported to 2.1, 2.0 and 1.9.

5 years agoCLEANUP: dns: resolution can never be null
William Dauchy [Wed, 27 Nov 2019 22:32:41 +0000 (23:32 +0100)] 
CLEANUP: dns: resolution can never be null

`eb` being tested above, `res` cannot be null, so the condition is
not needed and introduces potential dead code.

also fix a typo in associated comment

This should fix issue #349

Reported-by: Ð\98лÑ\8cÑ\8f ШипиÑ\86ин <chipitsine@gmail.com>
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
5 years agoMINOR: ssl: deduplicate crl-file
Emmanuel Hocdet [Thu, 21 Nov 2019 18:09:31 +0000 (19:09 +0100)] 
MINOR: ssl: deduplicate crl-file

Load file for crl or ca-cert is realy done with the same function in OpenSSL,
via X509_STORE_load_locations. Accordingly, deduplicate crl-file and ca-file
can share the same function.

5 years agoMINOR: ssl: compute ca-list from deduplicate ca-file
Emmanuel Hocdet [Thu, 24 Oct 2019 16:08:51 +0000 (18:08 +0200)] 
MINOR: ssl: compute ca-list from deduplicate ca-file

ca-list can be extracted from ca-file already loaded in memory.
This patch set ca-list from deduplicated ca-file when needed
and share it in ca-file tree.

As a corollary, this will prevent file access for ca-list when
updating a certificate via CLI.

5 years agoMINOR: ssl: deduplicate ca-file
Emmanuel Hocdet [Thu, 24 Oct 2019 09:32:47 +0000 (11:32 +0200)] 
MINOR: ssl: deduplicate ca-file

Typically server line like:
'server-template srv 1-1000 *:443 ssl ca-file ca-certificates.crt'
load ca-certificates.crt 1000 times and stay duplicated in memory.
Same case for bind line: ca-file is loaded for each certificate.
Same 'ca-file' can be load one time only and stay deduplicated in
memory.

As a corollary, this will prevent file access for ca-file when
updating a certificate via CLI.

5 years agoDOC: Clarify behavior of server maxconn in HTTP mode
Tim Duesterhus [Wed, 27 Nov 2019 21:35:27 +0000 (22:35 +0100)] 
DOC: Clarify behavior of server maxconn in HTTP mode

In HTTP mode the number of concurrent requests is limited, not the
number of actual connections.

5 years agoBUILD/MINOR: trace: fix use of long type in a few printf format strings
Willy Tarreau [Wed, 27 Nov 2019 14:41:31 +0000 (15:41 +0100)] 
BUILD/MINOR: trace: fix use of long type in a few printf format strings

Building on a 32-bit platform produces these warnings in trace code:

src/stream.c: In function 'strm_trace':
src/stream.c:226:29: warning: format '%lu' expects argument of type 'long unsigned int', but argument 9 has type 'size_t {aka const unsigned int}' [-Wformat=]
   chunk_appendf(&trace_buf, " req=(%p .fl=0x%08x .ana=0x%08x .exp(r,w,a)=(%u,%u,%u) .o=%lu .tot=%llu .to_fwd=%u)",
                             ^
src/stream.c:229:29: warning: format '%lu' expects argument of type 'long unsigned int', but argument 9 has type 'size_t {aka const unsigned int}' [-Wformat=]
   chunk_appendf(&trace_buf, " res=(%p .fl=0x%08x .ana=0x%08x .exp(r,w,a)=(%u,%u,%u) .o=%lu .tot=%llu .to_fwd=%u)",
                             ^
src/mux_fcgi.c: In function 'fcgi_trace':
src/mux_fcgi.c:443:29: warning: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t {aka const unsigned int}' [-Wformat=]
   chunk_appendf(&trace_buf, " - VAL=%lu", *val);
                             ^
src/mux_h1.c: In function 'h1_trace':
src/mux_h1.c:290:29: warning: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t {aka const unsigned int}' [-Wformat=]
   chunk_appendf(&trace_buf, " - VAL=%lu", *val);
                             ^

Let's just cast the type to long. This should be backported to 2.1.

5 years agoBUG/MINOR: h1: Don't test the host header during response parsing
Christopher Faulet [Wed, 27 Nov 2019 13:00:51 +0000 (14:00 +0100)] 
BUG/MINOR: h1: Don't test the host header during response parsing

During the H1 message parsing, the host header is tested to be sure it matches
the request's authority, if defined. When there are multiple host headers, we
also take care they are all the same. Of course, these tests must only be
performed on the requests. A host header in a response has no special meaning.

This patch must be backported to 2.1.

5 years agoBUG/MINOR: contrib/prometheus-exporter: decode parameter and value only
William Dauchy [Tue, 26 Nov 2019 11:56:26 +0000 (12:56 +0100)] 
BUG/MINOR: contrib/prometheus-exporter: decode parameter and value only

we were decoding all substring and then parsing; this could lead to
consider & and = in decoding result as delimiters where it should not.
this patch reverses the order by first parsing and then decoding each key
and value separately.

we also stop parsing after number sign (#).

This patch should be backported to 2.1 and 2.0

Signed-off-by: William Dauchy <w.dauchy@criteo.com>
5 years agoCLEANUP: ssl: Clean up error handling
Tim Duesterhus [Sat, 23 Nov 2019 22:45:10 +0000 (23:45 +0100)] 
CLEANUP: ssl: Clean up error handling

This commit removes the explicit checks for `if (err)` before
passing `err` to `memprintf`. `memprintf` already checks itself
whether the `**out*` parameter is `NULL` before doing anything.
This reduces the indentation depth and makes the code more readable,
before there is less boilerplate code.

Instead move the check into the ternary conditional when the error
message should be appended to a previous message. This is consistent
with the rest of ssl_sock.c and with the rest of HAProxy.

Thus this patch is the arguably cleaner fix for issue #374 and builds
upon
5f1fa7db86c53827c97f8a8c3f5fa75bfcb5be9a and
8b453912ce9a4e1a3b1329efb2af04d1e470852e

Additionally it fixes a few places where the check *still* was missing.

5 years agoSCRIPTS: update create-release to fix the changelog on new branches
Willy Tarreau [Mon, 25 Nov 2019 19:40:52 +0000 (20:40 +0100)] 
SCRIPTS: update create-release to fix the changelog on new branches

The changelog is empty when creating a dev0 version and this confuses
the commit message, let's clearly mention the exact copy when there are
no changes.

5 years agoMINOR: version: this is development again, update the status
Willy Tarreau [Mon, 25 Nov 2019 19:38:32 +0000 (20:38 +0100)] 
MINOR: version: this is development again, update the status

It's basically a revert of commit 9ca7f8cea.

5 years agoDOC: this is development again
Willy Tarreau [Mon, 25 Nov 2019 19:37:49 +0000 (20:37 +0100)] 
DOC: this is development again

This is basically a revert of commit eb1a3ee5 ("DOC: mention in INSTALL
haproxy 2.1 is a stable stable version").

5 years ago[RELEASE] Released version 2.2-dev0 v2.2-dev0
Willy Tarreau [Mon, 25 Nov 2019 19:36:16 +0000 (20:36 +0100)] 
[RELEASE] Released version 2.2-dev0

Released version 2.2-dev0 with the following main changes :
    - exact copy of 2.1.0

5 years ago[RELEASE] Released version 2.1.0 v2.1.0
Willy Tarreau [Mon, 25 Nov 2019 18:47:40 +0000 (19:47 +0100)] 
[RELEASE] Released version 2.1.0

Released version 2.1.0 with the following main changes :
    - BUG/MINOR: init: fix set-dumpable when using uid/gid
    - MINOR: init: avoid code duplication while setting identify
    - BUG/MINOR: ssl: ssl_pkey_info_index ex_data can store a dereferenced pointer
    - BUG/MINOR: ssl: fix crt-list neg filter for openssl < 1.1.1
    - MINOR: peers: Alway show the table info for disconnected peers.
    - MINOR: peers: Add TX/RX heartbeat counters.
    - MINOR: peers: Add debugging information to "show peers".
    - BUG/MINOR: peers: Wrong null "server_name" data field handling.
    - MINOR: ssl/cli: 'abort ssl cert' deletes an on-going transaction
    - BUG/MEDIUM: mworker: don't fill the -sf argument with -1 during the reexec
    - BUG/MINOR: peers: "peer alive" flag not reset when deconnecting.
    - BUILD/MINOR: ssl: fix compiler warning about useless statement
    - BUG/MEDIUM: stream-int: Don't loose events on the CS when an EOS is reported
    - MINOR: contrib/prometheus-exporter: filter exported metrics by scope
    - MINOR: contrib/prometheus-exporter: Add a param to ignore servers in maintenance
    - BUILD: debug: Avoid warnings in dev mode with -02 because of some BUG_ON tests
    - BUG/MINOR: mux-h1: Fix tunnel mode detection on the response path
    - BUG/MINOR: http-ana: Properly catch aborts during the payload forwarding
    - DOC: Update http-buffer-request description to remove the part about chunks
    - BUG/MINOR: stream-int: Fix si_cs_recv() return value
    - DOC: internal: document the init calls
    - MEDIUM: dns: Add resolve-opts "ignore-weight"
    - MINOR: ssl: ssl_sock_prepare_ctx() return an error code
    - MEDIUM: ssl/cli: apply SSL configuration on SSL_CTX during commit
    - MINOR: ssl/cli: display warning during 'commit ssl cert'
    - MINOR: version: report the version status in "haproxy -v"
    - MINOR: version: emit the link to the known bugs in output of "haproxy -v"
    - DOC: Add documentation about the use-service action
    - MINOR: ssl: fix possible null dereference in error handling
    - BUG/MINOR: ssl: fix curve setup with LibreSSL
    - BUG/MINOR: ssl: Stop passing dynamic strings as format arguments
    - CLEANUP: ssl: check if a transaction exists once before setting it
    - BUG/MINOR: cli: fix out of bounds in -S parser
    - MINOR: ist: add ist_find_ctl()
    - BUG/MAJOR: h2: reject header values containing invalid chars
    - BUG/MAJOR: h2: make header field name filtering stronger
    - BUG/MAJOR: mux-h2: don't try to decode a response HEADERS frame in idle state
    - MINOR: h2: add a function to report H2 error codes as strings
    - MINOR: mux-h2/trace: report the connection and/or stream error code
    - SCRIPTS: create-release: show the correct origin name in suggested commands
    - SCRIPTS: git-show-backports: add "-s" to proposed cherry-pick commands
    - BUG/MEDIUM: trace: fix a typo causing an incorrect startup error
    - BUILD: reorder the objects in the makefile
    - DOC: mention in INSTALL haproxy 2.1 is a stable stable version
    - MINOR: version: indicate that this version is stable

5 years agoMINOR: version: indicate that this version is stable
Willy Tarreau [Mon, 25 Nov 2019 18:27:55 +0000 (19:27 +0100)] 
MINOR: version: indicate that this version is stable

Also indicate that it will get fixes till ~Q1 2021.

5 years agoDOC: mention in INSTALL haproxy 2.1 is a stable stable version
Willy Tarreau [Mon, 25 Nov 2019 18:19:24 +0000 (19:19 +0100)] 
DOC: mention in INSTALL haproxy 2.1 is a stable stable version

Let's switch back to the stable wording now.

5 years agoBUILD: reorder the objects in the makefile
Willy Tarreau [Mon, 25 Nov 2019 18:03:59 +0000 (19:03 +0100)] 
BUILD: reorder the objects in the makefile

After a number of reorganization, addition of fcgi and the removal of
the legacy mode, some late files ended up being slow to build and were
slowing down the parallel build. Let's reorder them based on the build
time. Full build went down from 8.3-9.2s to 6.8s.

5 years agoBUG/MEDIUM: trace: fix a typo causing an incorrect startup error
Willy Tarreau [Mon, 25 Nov 2019 18:43:31 +0000 (19:43 +0100)] 
BUG/MEDIUM: trace: fix a typo causing an incorrect startup error

Since commit 88ebd40 ("MINOR: trace: add allocation of buffer-sized
trace buffers") we have a trace buffer allocated at boot time. But
there was a copy-paste error there making the test verify that the
trash was allocated instead of the trace buffer. The result is that
depending on the link order either the test will succeed or fail,
preventing haproxy from starting at all.

No backport is needed.

5 years agoSCRIPTS: git-show-backports: add "-s" to proposed cherry-pick commands
Willy Tarreau [Mon, 25 Nov 2019 14:51:47 +0000 (15:51 +0100)] 
SCRIPTS: git-show-backports: add "-s" to proposed cherry-pick commands

Since we're using signed-off-by tags for backports, let's add -s to
the command so that we can finally copy-paste it!

5 years agoSCRIPTS: create-release: show the correct origin name in suggested commands
Willy Tarreau [Mon, 25 Nov 2019 14:49:31 +0000 (15:49 +0100)] 
SCRIPTS: create-release: show the correct origin name in suggested commands

create-release shows the next steps at the end and suggest to use
"git push origin master" but on my machine it's not "origin" so let's
determine it using git config and only use origin as a fall back.

5 years agoMINOR: mux-h2/trace: report the connection and/or stream error code
Willy Tarreau [Sun, 24 Nov 2019 13:57:00 +0000 (14:57 +0100)] 
MINOR: mux-h2/trace: report the connection and/or stream error code

We were missing the error code when tracing a call to h2s_error() or
h2c_error(), let's report it when it's set.

5 years agoMINOR: h2: add a function to report H2 error codes as strings
Willy Tarreau [Sun, 24 Nov 2019 13:56:03 +0000 (14:56 +0100)] 
MINOR: h2: add a function to report H2 error codes as strings

Just like we have frame type to string, let's have error to string to
improve debugging and traces.

5 years agoBUG/MAJOR: mux-h2: don't try to decode a response HEADERS frame in idle state
Willy Tarreau [Sun, 24 Nov 2019 13:57:53 +0000 (14:57 +0100)] 
BUG/MAJOR: mux-h2: don't try to decode a response HEADERS frame in idle state

Christopher found another issue in the H2 backend implementation that
results from a miss in the H2 spec: the processing of a HEADERS frame
is always permitted in IDLE state, but this doesn't make sense on the
response path! And here when facing such a frame, we try to decode it
while we didn't allocate any stream, so we end up trying to fill the
idle stream's buffer (read-only) and crash.

What we're doing here is that if we get a HEADERS frame in IDLE state
from a server, we terminate the connection with a PROTOCOL_ERROR. No
such transition seems to be permitted by the spec but it seems to be
the only sane solution.

This fix must be backported as far as 1.9. Note that in 2.0 and earlier
there's no h2_frame_check_vs_state() function, instead the check is
inlined in h2_process_demux().

5 years agoBUG/MAJOR: h2: make header field name filtering stronger
Willy Tarreau [Sun, 24 Nov 2019 09:34:39 +0000 (10:34 +0100)] 
BUG/MAJOR: h2: make header field name filtering stronger

Tim Düsterhus found that the amount of sanitization we perform on HTTP
header field names received in H2 is insufficient. Currently we reject
upper case letters as mandated by RFC7540#8.1.2, but section 10.3 also
requires that intermediaries translating streams to HTTP/1 further
refine the filtering to also reject invalid names (which means any name
that doesn't match a token). There is a small trick here which is that
the colon character used to start pseudo-header names doesn't match a
token, so pseudo-header names fall into that category, thus we have to
swap the pseudo-header name lookup with this check so that we only check
from the second character (past the ':') in case of pseudo-header names.

Another possibility could have been to perform this check only in the
HTX-to-H1 trancoder but doing would still expose the configured rules
and logs to such header names.

This fix must be backported as far as 1.8 since this bug could be
exploited and serve as the base for an attack. In 2.0 and earlier,
functions h2_make_h1_request() and h2_make_h1_trailers() must also
be adapted to sanitize requests coming in legacy mode.

5 years agoBUG/MAJOR: h2: reject header values containing invalid chars
Willy Tarreau [Fri, 22 Nov 2019 15:02:43 +0000 (16:02 +0100)] 
BUG/MAJOR: h2: reject header values containing invalid chars

Tim Düsterhus reported an annoying problem in the H2 decoder related to
an ambiguity in the H2 spec. The spec says in section 10.3 that HTTP/2
allows header field values that are not valid (since they're binary) and
at the same time that an H2 to H1 gateway must be careful to reject headers
whose values contain \0, \r or \n.

Till now, and for the sake of the ability to maintain end-to-end binary
transparency in H2-to-H2, the H2 mux wouldn't reject this since it does
not know what version will be used on the other side.

In theory we should in fact perform such a check when converting an HTX
header to H1. But this causes a problem as it means that all our rule sets,
sample fetches, captures, logs or redirects may still find an LF in a header
coming from H2. Also in 2.0 and older in legacy mode, the frames are instantly
converted to H1 and HTX couldn't help there. So this means that in practice
we must refrain from delivering such a header upwards, regardless of any
outgoing protocol consideration.

Applying such a lookup on all headers leaving the mux comes with a
significant performance hit, especially for large ones. A first attempt
was made at placing this into the HPACK decoder to refrain from learning
invalid literals but error reporting becomes more complicated. Additional
tests show that doing this within the HTX transcoding loop benefits from
the hot L1 cache, and that by skipping up to 8 bytes per iteration the
CPU cost remains within noise margin, around ~0.5%.

This patch must be backported as far as 1.8 since this bug could be
exploited and serve as the base for an attack. In 2.0 and earlier the
fix must also be added to functions h2_make_h1_request() and
h2_make_h1_trailers() to handle legacy mode. It relies on previous patch
"MINOR: ist: add ist_find_ctl()" to speed up the control bytes lookup.

All credits go to Tim for his detailed bug report and his initial patch.

5 years agoMINOR: ist: add ist_find_ctl()
Willy Tarreau [Fri, 22 Nov 2019 14:58:53 +0000 (15:58 +0100)] 
MINOR: ist: add ist_find_ctl()

This new function looks for the first control character in a string (a
char whose value is between 0x00 and 0x1F included) and returns it, or
NULL if there is none. It is optimized for quickly evicting non-matching
strings and scans ~0.43 bytes per cycle. It can be used as an accelerator
when it's needed to look up several of these characters (e.g. CR/LF/NUL).

5 years agoBUG/MINOR: cli: fix out of bounds in -S parser
William Lallemand [Mon, 25 Nov 2019 08:58:37 +0000 (09:58 +0100)] 
BUG/MINOR: cli: fix out of bounds in -S parser

Out of bounds when the number or arguments is greater than
MAX_LINE_ARGS.

Fix issue #377.

Must be backported in 2.0 and 1.9.

5 years agoCLEANUP: ssl: check if a transaction exists once before setting it
William Dauchy [Sun, 24 Nov 2019 14:04:20 +0000 (15:04 +0100)] 
CLEANUP: ssl: check if a transaction exists once before setting it

trivial patch to fix issue #351

Fixes: bc6ca7ccaa72 ("MINOR: ssl/cli: rework 'set ssl cert' as 'set/commit'")
Reported-by: Илья Шипицин <chipitsine@gmail.com>
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
5 years agoBUG/MINOR: ssl: Stop passing dynamic strings as format arguments
Tim Duesterhus [Sat, 23 Nov 2019 22:52:30 +0000 (23:52 +0100)] 
BUG/MINOR: ssl: Stop passing dynamic strings as format arguments

gcc complains rightfully:

src/ssl_sock.c: In function ‘ssl_sock_prepare_all_ctx’:
src/ssl_sock.c:5507:3: warning: format not a string literal and no format arguments [-Wformat-security]
   ha_warning(errmsg);
   ^
src/ssl_sock.c:5509:3: warning: format not a string literal and no format arguments [-Wformat-security]
   ha_alert(errmsg);
   ^
src/ssl_sock.c: In function ‘cli_io_handler_commit_cert’:
src/ssl_sock.c:10208:3: warning: format not a string literal and no format arguments [-Wformat-security]
   chunk_appendf(trash, err);

Introduced in 8b453912ce9a4e1a3b1329efb2af04d1e470852e.

5 years agoBUG/MINOR: ssl: fix curve setup with LibreSSL
Lukas Tribus [Sun, 24 Nov 2019 17:20:40 +0000 (18:20 +0100)] 
BUG/MINOR: ssl: fix curve setup with LibreSSL

Since commit 9a1ab08 ("CLEANUP: ssl-sock: use HA_OPENSSL_VERSION_NUMBER
instead of OPENSSL_VERSION_NUMBER") we restrict LibreSSL to the OpenSSL
1.0.1 API, to avoid breaking LibreSSL every minute. We set
HA_OPENSSL_VERSION_NUMBER to 0x1000107fL if LibreSSL is detected and
only allow curves to be configured if HA_OPENSSL_VERSION_NUMBER is at
least 0x1000200fL.

However all relevant LibreSSL releases actually support settings curves,
which is now broken. Fix this by always allowing curve configuration when
using LibreSSL.

Reported on GitHub in issue #366.

Fixes: 9a1ab08 ("CLEANUP: ssl-sock: use HA_OPENSSL_VERSION_NUMBER instead
of OPENSSL_VERSION_NUMBER").

5 years agoMINOR: ssl: fix possible null dereference in error handling
William Dauchy [Sat, 23 Nov 2019 20:14:33 +0000 (21:14 +0100)] 
MINOR: ssl: fix possible null dereference in error handling

recent commit 8b453912ce9a ("MINOR: ssl: ssl_sock_prepare_ctx() return an error code")
converted all errors handling; in this patch we always test `err`, but
three of them are missing. I did not found a plausible explanation about
it.

this should fix issue #374

Fixes: 8b453912ce9a ("MINOR: ssl: ssl_sock_prepare_ctx() return an error code")
Reported-by: Илья Шипицин <chipitsine@gmail.com>
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
5 years agoDOC: Add documentation about the use-service action
Christopher Faulet [Fri, 22 Nov 2019 14:34:17 +0000 (15:34 +0100)] 
DOC: Add documentation about the use-service action

The use-service action may be used in tcp-request and http-request rules. It was
added to customize HAproxy reply to a client using an applet (initially a lua
applet). But the documentation was missing.

This patch may be backported as far as 1.6.

5 years agoMINOR: version: emit the link to the known bugs in output of "haproxy -v"
Willy Tarreau [Thu, 21 Nov 2019 17:48:20 +0000 (18:48 +0100)] 
MINOR: version: emit the link to the known bugs in output of "haproxy -v"

The link to the known bugs page for the current version is built and
reported there. When it is a development version (less than 2 dots),
instead a link to github open issues is reported as there's no way to
be sure about the current situation in this case and it's better that
users report their trouble there.

5 years agoMINOR: version: report the version status in "haproxy -v"
Willy Tarreau [Thu, 21 Nov 2019 17:07:30 +0000 (18:07 +0100)] 
MINOR: version: report the version status in "haproxy -v"

As discussed on Discourse here:

    https://discourse.haproxy.org/t/haproxy-branch-support-lifetime/4466

it's not always easy for end users to know the lifecycle of the version
they are using. This patch introduces a "Status" line in the output of
"haproxy -vv" indicating whether it's a development, stable, long-term
supported version, possibly with an estimated end of life for the branch
when it can be anticipated (e.g. for stable versions). This field should
be adjusted when creating a major release to reflect the new status.

It may make sense to backport this to other branches to clarify the
situation.

5 years agoMINOR: ssl/cli: display warning during 'commit ssl cert'
William Lallemand [Thu, 21 Nov 2019 15:41:07 +0000 (16:41 +0100)] 
MINOR: ssl/cli: display warning during 'commit ssl cert'

Display the warnings on the CLI during a commit of the certificates.

5 years agoMEDIUM: ssl/cli: apply SSL configuration on SSL_CTX during commit
William Lallemand [Thu, 21 Nov 2019 15:30:34 +0000 (16:30 +0100)] 
MEDIUM: ssl/cli: apply SSL configuration on SSL_CTX during commit

Apply the configuration of the ssl_bind_conf on the generated SSL_CTX.
It's a little bit hacky at the moment because the ssl_sock_prepare_ctx()
function was made for the configuration parsing, not for being using at
runtime. Only the 'verify' bind keyword seems to cause a file access so
we prevent it before calling the function.

5 years agoMINOR: ssl: ssl_sock_prepare_ctx() return an error code
William Lallemand [Thu, 21 Nov 2019 14:48:10 +0000 (15:48 +0100)] 
MINOR: ssl: ssl_sock_prepare_ctx() return an error code

Rework ssl_sock_prepare_ctx() so it fills a buffer with the error
messages instead of using ha_alert()/ha_warning(). Also returns an error
code (ERR_*) instead of the number of errors.

5 years agoMEDIUM: dns: Add resolve-opts "ignore-weight"
Daniel Corbett [Sun, 17 Nov 2019 14:48:56 +0000 (09:48 -0500)] 
MEDIUM: dns: Add resolve-opts "ignore-weight"

It was noted in #48 that there are times when a configuration
may use the server-template directive with SRV records and
simultaneously want to control weights using an agent-check or
through the runtime api.  This patch adds a new option
"ignore-weight" to the "resolve-opts" directive.

When specified, any weight indicated within an SRV record will
be ignored.  This is for both initial resolution and ongoing
resolution.

5 years agoDOC: internal: document the init calls
Willy Tarreau [Wed, 20 Nov 2019 15:45:15 +0000 (16:45 +0100)] 
DOC: internal: document the init calls

INITCALLs are used a lot in the code now and were not documented, resulting
in each user having to grep for functions in other files. This doc is not
perfect but aims at improving the situation. It documents what's been
available since 1.9 and may be backported there if it helps though it's
unlikely to be needed as it's mostly aimed at developers.

5 years agoBUG/MINOR: stream-int: Fix si_cs_recv() return value
Christopher Faulet [Wed, 20 Nov 2019 15:42:00 +0000 (16:42 +0100)] 
BUG/MINOR: stream-int: Fix si_cs_recv() return value

The previous patch on this function (36b536d6c "BUG/MEDIUM: stream-int: Don't
loose events on the CS when an EOS is reported") contains a bug. The return
value is based on the conn-stream's flags. But it may be reset if the CS is
closed. Ironically it was exactly the purpose of this patch...

This patch must be backported to 2.0 and 1.9.

5 years agoDOC: Update http-buffer-request description to remove the part about chunks
Christopher Faulet [Tue, 19 Nov 2019 15:27:25 +0000 (16:27 +0100)] 
DOC: Update http-buffer-request description to remove the part about chunks

The limitation on the first chunk for chunked requests was true for the legacy
HTTP mode. But, it does not exist with the HTX. Becaue, the legacy HTTP mode was
removed in 2.1, this limitation does not exist anymore.

5 years agoBUG/MINOR: http-ana: Properly catch aborts during the payload forwarding
Christopher Faulet [Fri, 15 Nov 2019 10:29:40 +0000 (11:29 +0100)] 
BUG/MINOR: http-ana: Properly catch aborts during the payload forwarding

When no data filter are registered on a channel, if the message length is known,
the HTTP payload is infinitely forwarded to save calls to process_stream(). When
we finally fall back again in XFER_BODY analyzers, we detect the end of the
message by checking channel flags. If CF_EOI or CF_SHUTR is set, we switch the
message in DONE state. For CF_EOI, it is relevant. But not for CF_SHUTR. a
shutdown for reads without the end of input must be interpreted as an abort for
messages with a known length.

Because of this bug, some aborts are not properly handled and reported. Instead,
we interpret it as a legitimate shutdown.

This patch must be backported to 2.0.

5 years agoBUG/MINOR: mux-h1: Fix tunnel mode detection on the response path
Christopher Faulet [Fri, 15 Nov 2019 10:14:23 +0000 (11:14 +0100)] 
BUG/MINOR: mux-h1: Fix tunnel mode detection on the response path

There are two issues with the way tunnel mode is detected on the response
path. First, when a response with an unknown content length is handled, the
request is also switched in tunnel mode. It is obviously wrong. Because it was
done on the server side only (so not during the request parsing), it is no
noticeable effects.

The second issue is about the way protocol upgrades are handled. The request is
switched in tunnel mode from the time the 101 response is processed. So an
unfinished request may be switched in tunnel mode too early. It is not a common
use, but a protocol upgrade on a POST is allowed. Thus, parsing of the payload
may be hijacked. It is especially bad for chunked payloads.

Now, conditions to switch the request in tunnel mode reflect what should be
done. Especially for the second issue. We wait the end of the request to switch
it in tunnel mode.

This patch must be backported to 2.0 and 1.9. Note that these versions are only
affected by the second issue but the patch cannot be easily splitted.

5 years agoBUILD: debug: Avoid warnings in dev mode with -02 because of some BUG_ON tests
Christopher Faulet [Mon, 18 Nov 2019 14:50:25 +0000 (15:50 +0100)] 
BUILD: debug: Avoid warnings in dev mode with -02 because of some BUG_ON tests

Some BUG_ON() tests emit a warning because of a potential null pointer
dereference on an HTX block. In fact, it should never happen, but now, GCC is
happy.

This patch must be backported to 2.0.

5 years agoMINOR: contrib/prometheus-exporter: Add a param to ignore servers in maintenance
Christopher Faulet [Tue, 19 Nov 2019 13:18:24 +0000 (14:18 +0100)] 
MINOR: contrib/prometheus-exporter: Add a param to ignore servers in maintenance

By passing the parameter "no-maint" in the query-string, it is now possible to
ignore servers in maintenance. It means that the metrics for servers in this
state will not be exported.

5 years agoMINOR: contrib/prometheus-exporter: filter exported metrics by scope
Christopher Faulet [Mon, 18 Nov 2019 13:47:08 +0000 (14:47 +0100)] 
MINOR: contrib/prometheus-exporter: filter exported metrics by scope

Now, the prometheus exporter parses the HTTP query-string to filter or to adapt
the exported metrics. In this first version, it is only possible select the
scopes of metrics to export. To do so, one or more parameters with "scope" as
name must be passed in the query-string, with one of those values: global,
frontend, backend, server or '*' (means all). A scope parameter with no value
means to filter out all scopes (nothing is returned). The scope parameters are
parsed in their appearance order in the query-string. So an empty scope will
reset all scopes already parsed. But it can be overridden by following scope
parameters in the query-string. By default everything is exported.

The filtering can also be done on prometheus scraping configuration, but general
aim is to optimise the source of data to improve load and scraping time. This is
particularly true for huge configuration with thousands of backends and servers.
Also note that this configuration was possible on the previous official haproxy
exporter but with even more parameters to select the needed metrics. Here we
thought it was sufficient to simply avoid a given type of metric. However, more
filters are still possible.

Thanks to William Dauchy. This patch is based on his work.

5 years agoBUG/MEDIUM: stream-int: Don't loose events on the CS when an EOS is reported
Christopher Faulet [Wed, 20 Nov 2019 10:56:33 +0000 (11:56 +0100)] 
BUG/MEDIUM: stream-int: Don't loose events on the CS when an EOS is reported

In si_cs_recv(), when a shutdown for reads is handled, the conn-stream may be
closed. It happens when the ouput channel is closed for writes or if
SI_FL_NOHALF is set on the stream-interface. In this case, conn-stream's flags
are reset. Thus, if an error (CS_FL_ERROR) or an end of input (CS_FL_EOI) is
reported by the mux, the event is lost. si_cs_recv() does not report these
events by itself. It relies on si_cs_process() to report them to the
stream-interface and/or the channel.

For instance, if CS_FL_EOS and CS_FL_EOI are set by the H1 multiplexer during a
call to si_cs_recv() on the server side, if the conn-stream is closed (read0 +
SI_FL_NOHALF), the CS_FL_EOI flag is lost. Thus, this may lead the stream to
interpret it as a server abort.

Now, conn-stream's flags are processed at the end of si_cs_recv(). The function
is responsible to set the right flags on the stream-interface and/or the
channel. Due to this patch, the function is now almost linear. Except some early
checks at the beginning, there is only one return statement. It also fixes a
potential bug because of an inconsistency between the splicing and the buffered
receipt. On the first case, CS_FL_EOS if handled before errors on the connection
or the conn-stream. On the second one, it is the opposite.

This patch must be backported to 2.0 and 1.9.

5 years agoBUILD/MINOR: ssl: fix compiler warning about useless statement
Eric Salama [Wed, 20 Nov 2019 10:33:40 +0000 (11:33 +0100)] 
BUILD/MINOR: ssl: fix compiler warning about useless statement

There is a compiler warning after commit a9363eb6 ("BUG/MEDIUM: ssl:
'tune.ssl.default-dh-param' value ignored with openssl > 1.1.1"):

src/ssl_sock.c: In function 'ssl_sock_prepare_ctx':
src/ssl_sock.c:4481:4: error: statement with no effect [-Werror=unused-value]

Fix it by adding a (void)

5 years agoBUG/MINOR: peers: "peer alive" flag not reset when deconnecting.
Frédéric Lécaille [Wed, 20 Nov 2019 10:17:30 +0000 (11:17 +0100)] 
BUG/MINOR: peers: "peer alive" flag not reset when deconnecting.

The peer flags (->flags member of peer struct) are reset by __peer_session_deinit()
function. PEER_F_ALIVE flag which is used by the heartbeat part of the peer protocol
to mark a peer as being alive was not reset by this function. This simple patch adds
add the statement to this.

Note that, at this time, there was no identified issue due to this missing reset.

Must be backported to 2.0.

5 years agoBUG/MEDIUM: mworker: don't fill the -sf argument with -1 during the reexec
William Lallemand [Tue, 19 Nov 2019 16:04:18 +0000 (17:04 +0100)] 
BUG/MEDIUM: mworker: don't fill the -sf argument with -1 during the reexec

Upon a reexec_on_failure, if the process tried to exit after the
initialization of the process structure but before it was filled with a
PID, the PID in the mworker_proc structure is set to -1.

In this particular case the -sf argument is filled with -1 and haproxy
will exit with the usage message because of that argument.

Should be backported in 2.0.

5 years agoMINOR: ssl/cli: 'abort ssl cert' deletes an on-going transaction
William Lallemand [Tue, 19 Nov 2019 14:51:51 +0000 (15:51 +0100)] 
MINOR: ssl/cli: 'abort ssl cert' deletes an on-going transaction

This patch introduces the new CLI command 'abort ssl cert' which abort
an on-going transaction and free its content.

This command takes the name of the filename of the transaction as an
argument.

5 years agoBUG/MINOR: peers: Wrong null "server_name" data field handling.
Frédéric Lécaille [Wed, 13 Nov 2019 16:50:34 +0000 (17:50 +0100)] 
BUG/MINOR: peers: Wrong null "server_name" data field handling.

As the peers protocol expects to parse at least one encoded integer value for
each stick-table data field even when not configured on the local side,
about the "server_name" data field we must emit something even if it has
not been set (no server was configured for instance).
As this data field is made of first one encoded integer which is the length
of the remaining data (the dictionary cache entry), we encode the length 0
when emitting such an absent dictionary cache entry.
On the remote side, when we decode such an integer with 0 as value, we stop
parsing the data field and that's it.

Must be backported to 2.0.