12110 Commits

Author SHA1 Message Date
9719b31a45 2.0.11-alt1
- 2.0.11
2019-12-16 11:30:40 +03:00
2acf60e8fe HAProxy 2.0.11
-----BEGIN PGP SIGNATURE-----
 
 iQIcBAABAgAGBQJd8R1uAAoJEE44bZycYXAvIo0P/1d/nHAdPSL7Lch0whM2epdb
 xbnSA/vswCXjhxSEbB+rHK2pcaahLskpo6vsUcTk8x8Hf7P9ZMP3mcW7D7Pl3Km5
 4AomnYZW/VFMcIjzr/NP5d8X/K1JY8eK99CEQqWSnX/20ewvD4mcK86xLtU6DR0s
 Sjo2SDTnzPBraPaNbiqP8CvsS+gPckCGSUtfJmJ+9QK2n/oZYy9Q7fcDWtVqa+Qr
 DQdCgjzkdi+14VpSLzFz0dihchwfeG33ur3G2d3qNmY3VWUG3SmbHLbCG9LQUY0q
 XupCXrBT5FZO67nEMBR8fEnxiSHu0o199HHNsh9Lmw3dD2ft2+nba2QaUGlo77wB
 xlqqgBvm6+y9/sfZV/Vvngz6EfFufxeVEYWX/hw+ag2rFzLXBfu9PxdV2DVqAaPr
 dlfbw8vIFtgxkLlrxV0dy/VszFVFIJONT0f+gBpVoL0TlanRNNWxzR/VgJfWYx8i
 zT2K03ECKLtG0wqTA5P3bdkzvTQ/on195C2Sdg9EMsY3VPRhvQvwhQRU16+TnD5M
 kh2Z0MC/dMuaklyMiMqpEEcFSVAh5M3pktGoKJMUBTnF8XMRE796DHBJDxYHlgfX
 KKvTtj58YiqleRJhlzeZYTSeLFoW71B3IecMGZ/RZMbP9nYxkiY++SWONHU4nVvH
 m+n2ItSAilLfHxcJrF5r
 =q/xQ
 -----END PGP SIGNATURE-----

Merge tag 'v2.0.11'

HAProxy 2.0.11
2019-12-16 11:26:16 +03:00
Willy Tarreau
fd8594d1da [RELEASE] Released version 2.0.11
Released version 2.0.11 with the following main changes :
    - BUG/MINOR: stream: init variables when the list is empty
    - BUG/MINOR: contrib/prometheus-exporter: Use HTX errors and not legacy ones
    - BUG/MINOR: contrib/prometheus-exporter: decode parameter and value only
    - BUG/MINOR: http-htx: Don't make http_find_header() fail if the value is empty
    - DOC: Clarify behavior of server maxconn in HTTP mode
    - DOC: clarify matching strings on binary fetches
    - DOC: move the "group" keyword at the right place
    - BUG/MEDIUM: stream-int: don't subscribed for recv when we're trying to flush data
    - BUG/MINOR: stream-int: avoid calling rcv_buf() when splicing is still possible
    - BUG/MEDIUM: listener/thread: fix a race when pausing a listener
    - BUG/MINOR: ssl: certificate choice can be unexpected with openssl >= 1.1.1
    - BUG/MEDIUM: mux-h1: Never reuse H1 connection if a shutw is pending
    - BUG/MINOR: mux-h1: Don't rely on CO_FL_SOCK_RD_SH to set H1C_F_CS_SHUTDOWN
    - BUG/MINOR: mux-h1: Fix conditions to know whether or not we may receive data
    - BUG/MEDIUM: tasks: Make sure we switch wait queues in task_set_affinity().
    - BUG/MEDIUM: checks: Make sure we set the task affinity just before connecting.
    - BUG/MINOR: mux-h1: Be sure to set CS_FL_WANT_ROOM when EOM can't be added
    - BUG/MINOR: proxy: make soft_stop() also close FDs in LI_PAUSED state
    - BUG/MINOR: listener/threads: always use atomic ops to clear the FD events
    - BUG/MINOR: listener: also clear the error flag on a paused listener
    - BUG/MEDIUM: listener/threads: fix a remaining race in the listener's accept()
    - DOC: document the listener state transitions
    - BUG/MAJOR: dns: add minimalist error processing on the Rx path
    - BUG/MEDIUM: proto_udp/threads: recv() and send() must not be exclusive.
    - BUG/MEDIUM: kqueue: Make sure we report read events even when no data.
    - DOC: listeners: add a few missing transitions
    - BUG/MINOR: tasks: only requeue a task if it was already in the queue
    - DOC: proxies: HAProxy only supports 3 connection modes
    - BUILD/MINOR: ssl: shut up a build warning about format truncation
    - BUILD/MINOR: tools: shut up the format truncation warning in get_gmt_offset()
    - BUILD: do not disable -Wformat-truncation anymore
    - DOC: remove references to the outdated architecture.txt
    - BUG/MINOR: log: fix minor resource leaks on logformat error path
    - BUG/MINOR: mworker: properly pass SIGTTOU/SIGTTIN to workers
    - BUG/MINOR: listener: do not immediately resume on transient error
    - BUG/MINOR: server: make "agent-addr" work on default-server line
    - BUG/MINOR: listener: fix off-by-one in state name check
    - BUILD/MINOR: unix sockets: silence an absurd gcc warning about strncpy()
2019-12-11 17:46:38 +01:00
Willy Tarreau
d668dcfa97 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.

(cherry picked from commit 719e07c989be48a69fcfcaa404d12d7478de8a1b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit b5085dfe892bef654841103173079434a32c2a24)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-11 16:37:06 +01:00
Willy Tarreau
4a1cfa7fc1 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.

(cherry picked from commit fec56c6a76463d40be3e15eee297aa8d2b67362a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit b23644370b9a5ed727e50b01bfa390078c7462c1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-11 15:54:09 +01:00
Willy Tarreau
fb38956bef 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.

(cherry picked from commit 2444108f16868ccde928d97ffa3db847ddad89fb)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 3834754e654fa41491916a2e23c5fbe9b4ba5d82)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-11 15:47:58 +01:00
Willy Tarreau
5de8d1fc35 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.

(cherry picked from commit cdcba115b8a6d3773d5bd3c0fe6f8c239d356eab)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 81a1ad0f526e5e4647e5603acac57f1fc0fd5184)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-11 15:20:36 +01:00
Willy Tarreau
708c244026 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.

(cherry picked from commit d26c9f9465de24d2414f4a46653fc20fd2871ac4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit b97a22aa43aaebca207e52c8c80a6b85de4a25d4)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-11 14:36:26 +01:00
Willy Tarreau
1a9f27f681 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.

(cherry picked from commit 51013e82d4931c4f0ce6f7fc99788a39cc6960ed)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit f58f3a0fb818903610432cdb8f98060cffd9af29)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-11 12:07:49 +01:00
Willy Tarreau
a9330e91b8 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.

(cherry picked from commit 9ef75ecea129e3f77aaeb38cf5c163d0eb73742e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 55a92393572eeff4cee4d4de738078df83e42f98)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-11 11:58:05 +01:00
Willy Tarreau
08b9180d55 BUILD: do not disable -Wformat-truncation anymore
There were very few entries to fix and this warning, while often
wrong, can actually spot future issues. If it can help developers
adjust their code in the future to make it more robust, it's not
necessarily that bad. Let's re-enable it and see how it goes.

(cherry picked from commit c1b16734c017118ce6582b16ea724ec6095e5fc9)
[wt: this is not required but helpful to detect mistakes in backports]
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-11 11:37:48 +01:00
Willy Tarreau
bf5eeb60ab BUILD/MINOR: tools: shut up the format truncation warning in get_gmt_offset()
The gcc warning about format truncation in get_gmt_offset() is annoying
since we always call it with a valid time thus it cannot fail. However
it's true that nothing guarantees that future code reuses this function
incorrectly in the future, so better enforce the modulus on one day and
shut the warning.

(cherry picked from commit e112c8a64be8ba2f7d8efb3221ec87a6ec119c54)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-11 11:36:11 +01:00
Willy Tarreau
5b8a865f0b BUILD/MINOR: ssl: shut up a build warning about format truncation
Actually gcc believes it has detected a possible truncation but it
cannot since the output string is necessarily at least one char
shorter than what it expects. However addressing it is easy and
removes the need for an intermediate copy so let's do it.

(cherry picked from commit 0580052bb6f9c924daedfe62d779eade68677adf)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-11 11:31:00 +01:00
Julien Pivotto
599788ecf6 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>
(cherry picked from commit 21ad31531601299eb52a56b50f90f491f46e4a88)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 0363fd302b8c48d86b7acb97e53c41cb58f6dd10)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-11 10:18:13 +01:00
Willy Tarreau
fcd7401b3e 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.

(cherry picked from commit 440d09b2448fcddea6877054300a95ba5b55dac7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 5f9b548c65dceff150154ed1152b35e4ff94dc1f)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-11 09:46:05 +01:00
Willy Tarreau
66b5d7414e 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.

(cherry picked from commit 4ac36d691ab40a29ec4a1fb43a3270f31114feda)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 8b3f8dbf031f7dd05e7b717dc65704baf45d38db)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-11 09:46:05 +01:00
Olivier Houchard
7b869bf019 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.

(cherry picked from commit eaefc3c5032506e89cceb6ad5fdd1c5955c4ea66)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 66c7b64e20c3b9e93675fee50f245e5257e393ea)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-11 06:08:38 +01:00
Willy Tarreau
8eb61c6eb8 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.

(cherry picked from commit d7f76a0a50f4ac6b32d2317c675b3752133ef6d2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 8e4a62508c4e2f6ba0ae01f552a13208a8018875)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-10 19:22:28 +01:00
Willy Tarreau
1899dbf7e0 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).

(cherry picked from commit 1c759956112996245eaccbf20e2506b9c9cbceb2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 3c0c292b491c9024bdb3fabaef90b3bb45aef273)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-10 19:22:28 +01:00
Willy Tarreau
890d5a2e21 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.

(cherry picked from commit 977afab3f824b8322853e27abbaaa752f601a504)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 8b135c000dc4e5d75f5a3764e93776af0c1cb861)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-10 19:17:42 +01:00
Willy Tarreau
07e1322cc8 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.

(cherry picked from commit 92079934a913a330a57e6d84eba3dca68c0cde8e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 5d5baf06574cfe9892bf8215633f6981852c3238)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-10 19:17:42 +01:00
Willy Tarreau
bbee29cb07 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.

(cherry picked from commit 20aeb1c7cd38907d704a4d769695b9ea264fa4c0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit f10a920b20d310a87c55ce50e91bd8081024d810)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-10 19:17:42 +01:00
Willy Tarreau
174c2f2651 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.

(cherry picked from commit 7cdeb61701729ef7b4d2ed97e590860478ad718d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 6b0244049d92e960805f46c9fbdd57d5508c56f0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-10 19:17:42 +01:00
Willy Tarreau
0d5b5c2ac5 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.

(cherry picked from commit 67878d7bdcb88683bdbf6fba851901a31f348eb8)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 6967d100b6fe32bbe23c3e5d078a529380d54c37)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-10 19:17:42 +01:00
Christopher Faulet
a882a46eef 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.

(cherry picked from commit 7aae858001f99dd4a80e3f533284cda5702d501a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 6f6886e021828a24295271bd83b33a9917d70323)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2019-12-09 10:47:05 +01:00
Olivier Houchard
68123bc4dd 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.

(cherry picked from commit aebeff74fc7eaef12728b1fc15b2d42d93a7767a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit e77b108cde2c9b47f4e7c6f932310e1a5ac674d7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-06 10:17:49 +01:00
Olivier Houchard
d5d0fc90ef 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.

(cherry picked from commit 0742c314c35c2c96b72e42076c76d6a6786045ba)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 279698ebdef2c00e7a01544af0321438b69ea7d1)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-06 10:17:49 +01:00
Christopher Faulet
7caf150a6c 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.

(cherry picked from commit 2545a0b352ffb49e68e8945c9b8ce7e633d7e8b0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 67ff2112bf139b2e92c4a3a5fd4cfc75791a127a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2019-12-05 15:08:32 +01:00
Christopher Faulet
f36e72b535 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.

(cherry picked from commit 7b109f2f8b9cb493d9f6c01f1613bc54a6f71ba3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 70512eb718ff97e840d797bbef689ace69fc8df8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2019-12-05 15:07:20 +01:00
Christopher Faulet
e31a0f1c40 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.

(cherry picked from commit aaa67bcef299486f1cdb75ef28b3ec6c39713ae6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit cb52e073a09bad884226083ccb9be4e4076b9937)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

[Cf: H1C_F_CS_WAIT_CONN value has been changed because it was in conflict with
    H1C_F_CS_IDLE. H1C_F_CS_WAIT_CONN does not exist in 2.1.]
2019-12-05 15:02:37 +01:00
Emmanuel Hocdet
cc957c3f53 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.

(cherry picked from commit 3777e3ad14f2ce54b6662fd0db56413dde9ec9fa)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
(cherry picked from commit 7126994b35f001ba86429345a68c0b955bdacd78)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2019-12-05 14:58:32 +01:00
Willy Tarreau
7911610b97 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+.

(cherry picked from commit 4c044e274c16fde42863c476449895b0fd603818)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 8d30120a21cc22138395c153930da6f81ebca013)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-05 08:03:12 +01:00
Willy Tarreau
791b18ba51 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.

(cherry picked from commit c640ef1a7de5d13504599f85ca3cf3c282128a05)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit dd2efd48d5c408ebc66e195c867378444acb4f4a)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-05 08:03:12 +01:00
Willy Tarreau
9dc6772eb7 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.

(cherry picked from commit 1ac5f208042ff571c9341aed0380ca52c084a261)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit c4fcb6b3644b87a46fb8735a387b9bd9bf61ffaf)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-05 08:03:12 +01:00
Willy Tarreau
8b85246b95 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.

(cherry picked from commit 11770ce64ba29b91178e61ce6dc11c7713b7469d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit a54c297c5008a1cce1716e28116272cfddd19d8e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-05 08:03:11 +01:00
Mathias Weiersmueller
b2fe22313a 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.

(cherry picked from commit cb250fc9843a335fffe44ed6b15570e5b7cd2a35)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 31f3f5e5fecc6dcec08497b06df8267aad62e0ae)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-05 08:03:11 +01:00
Tim Duesterhus
50cfb315e0 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.

(cherry picked from commit cefbbd98116cc97b43711b784638789c5557e7e6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 6ac493c41908856077aa74b8324b1d0aaff591e7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-12-05 08:03:11 +01:00
Christopher Faulet
6d9a455da1 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.

(cherry picked from commit f3ad62996fa3bab907429ce25a2ada10186b4242)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit c4c499b812ab306c7ecd0d2fcc9eb7e0a3175c41)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2019-11-29 11:54:37 +01:00
William Dauchy
200c6215fe 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>
(cherry picked from commit c65f656d75091db3087a752dbc956159392fc8f2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 8ec21c5fef89f13fea2ac9be55d55215d4b9104a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2019-11-27 14:29:38 +01:00
Christopher Faulet
3a00e5fcc1 BUG/MINOR: contrib/prometheus-exporter: Use HTX errors and not legacy ones
This bug was introduced when the commit 32d634f1 ("MINOR:
contrib/prometheus-exporter: filter exported metrics by scope") was backported
to 2.0.

In 2.0, Error chunks exist in raw format (http_err_chunks[]) and in HTX format
(htx_err_chunks[]). Prometheus exported only supports the HTX. So error must not
be reported using the raw chunks.

This fix is specific to 2.0. No backport needed.
2019-11-27 11:27:05 +01:00
Jerome Magnin
54948f3fd3 BUG/MINOR: stream: init variables when the list is empty
We need to call vars_init() when the list is empty otherwise we
can't use variables in the response scope. This regression was
introduced by cda7f3f5 (MINOR: stream: don't prune variables if
the list is empty).

The following config reproduces the issue:

 defaults
   mode http

 frontend in
   bind *:11223
   http-request set-var(req.foo) str("foo")  if { path /bar }
   http-request set-header bar %[var(req.foo)]  if { var(req.foo) -m found }
   http-response set-var(res.bar) str("bar")
   http-response set-header foo %[var(res.bar)] if { var(res.bar) -m found }
   use_backend out

 backend out
   server s1 127.0.0.1:11224

 listen back
   bind *:11224
   http-request deny deny_status 200

 > GET /ba HTTP/1.1
 > Host: localhost:11223
 > User-Agent: curl/7.66.0
 > Accept: */*
 >
 < HTTP/1.0 200 OK
 < Cache-Control: no-cache
 < Content-Type: text/html

 > GET /bar HTTP/1.1
 > Host: localhost:11223
 > User-Agent: curl/7.66.0
 > Accept: */*
 >
 < HTTP/1.0 200 OK
 < Cache-Control: no-cache
 < Content-Type: text/html
 < foo: bar

This must be backported as far as 1.9.

(cherry picked from commit 2f44e8843a553ef0f9c53c9b27899727de097777)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-11-25 16:32:02 +01:00
Willy Tarreau
ac198b92d4 [RELEASE] Released version 2.0.10
Released version 2.0.10 with the following main changes :
    - BUG/MINOR: init: fix set-dumpable when using uid/gid
    - 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.
    - BUG/MINOR: ssl: fix crt-list neg filter for openssl < 1.1.1
    - 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
    - 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
    - MINOR: freq_ctr: Make the sliding window sums thread-safe
    - MINOR: stream: Remove the lock on the proxy to update time stats
    - MINOR: counters: Add fields to store the max observed for {q,c,d,t}_time
    - MINOR: contrib/prometheus-exporter: Report metrics about max times for sessions
    - BUG/MINOR: contrib/prometheus-exporter: Rename some metrics
    - MINOR: contrib/prometheus-exporter: report the number of idle conns per server
    - MINOR: contrib/prometheus-exporter: filter exported metrics by scope
    - MINOR: contrib/prometheus-exporter: Add a param to ignore servers in maintenance
    - BUG/MINOR: stream-int: Fix si_cs_recv() return value
    - MINOR: stats: Report max times in addition of the averages for sessions
    - REGTEST: vtest can now enable mcli with its own flag
    - MEDIUM: mux-h1: Add the support of headers adjustment for bogus HTTP/1 apps
    - BUG/MINOR: mux-h1: Fix a UAF in cfg_h1_headers_case_adjust_postparser()
    - BUG/MINOR: mux-h1: Adjust header case when chunked encoding is add to a message
    - DOC: Add missing stats fields in the management manual
    - DOC: Add documentation about the use-service action
    - BUG/MINOR: cli: fix out of bounds in -S parser
    - BUG/MINOR: ssl: fix curve setup with LibreSSL
    - 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
    - SCRIPTS: create-release: show the correct origin name in suggested commands
    - SCRIPTS: git-show-backports: add "-s" to proposed cherry-pick commands
2019-11-25 15:55:45 +01:00
Willy Tarreau
224b80885a 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!

(cherry picked from commit 1a3af7830202323c0ef306e4a3a57f21ac24a09b)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-11-25 15:52:33 +01:00
Willy Tarreau
d1a4778c14 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.

(cherry picked from commit ff0c8424c87f1d702d98cdb593e6b701c7e61b15)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-11-25 15:51:14 +01:00
Willy Tarreau
85ee6e8343 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().

(cherry picked from commit 57a1816faec652d9c7fbd9be48ce25c832be20d9)
[wt: moved the code into h2_process_demux()]
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-11-25 11:37:35 +01:00
Willy Tarreau
5eaeec588d BUG/MAJOR: h2: make header field name filtering stronger
Tim Dsterhus 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.

(cherry picked from commit 146f53ae7e97dbfe496d0445c2802dd0a30b0878)
[wt: check added to h2_make_h1_request() and h2_make_h1_trailers()]
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-11-25 11:32:35 +01:00
Willy Tarreau
c36c6789d1 BUG/MAJOR: h2: reject header values containing invalid chars
Tim Dsterhus 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.

(cherry picked from commit 54f53ef7ce4102be596130b44c768d1818570344)
[wt: checks added to h2_make_h1_request() and h2_make_h1_trailers()]
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-11-25 11:29:16 +01:00
Willy Tarreau
658e0ddd64 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).

(cherry picked from commit 8f3ce06f14e13719c9353794d60001eab8d43717)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-11-25 10:58:51 +01:00
Lukas Tribus
d13e925213 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").

(cherry picked from commit d14b49c128af76870d64214694adbb38057932e0)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2019-11-25 10:45:34 +01:00
William Lallemand
b4f8786d51 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.

(cherry picked from commit 2e945c8ee7d5cb358d98c4333a66b0d6631cefa6)
Signed-off-by: William Lallemand <wlallemand@haproxy.org>
2019-11-25 10:20:20 +01:00