haproxy: Update HAProxy to v2.4.10
- Update haproxy download URL and hash - Updated upstream patches Signed-off-by: Christian Lachner <gladiac@gmail.com>
This commit is contained in:
Christian Lachner
Rosen Penev
parent
daaba8f3ff
commit
8891db4a91
10 changed files with 535 additions and 90 deletions
|
|
@ -10,12 +10,12 @@
|
||||||
include $(TOPDIR)/rules.mk
|
include $(TOPDIR)/rules.mk
|
||||||
|
|
||||||
PKG_NAME:=haproxy
|
PKG_NAME:=haproxy
|
||||||
PKG_VERSION:=2.4.9
|
PKG_VERSION:=2.4.10
|
||||||
PKG_RELEASE:=$(AUTORELEASE)
|
PKG_RELEASE:=$(AUTORELEASE)
|
||||||
|
|
||||||
PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz
|
PKG_SOURCE:=$(PKG_NAME)-$(PKG_VERSION).tar.gz
|
||||||
PKG_SOURCE_URL:=https://www.haproxy.org/download/2.4/src
|
PKG_SOURCE_URL:=https://www.haproxy.org/download/2.4/src
|
||||||
PKG_HASH:=d56c7fe3c5afedd1b9a19e1b7f8f954feaf50a9c2f205f99891043858b72a763
|
PKG_HASH:=4838dcc961a4544ef2d1e1aa7a7624cffdc4dda731d9cb66e46114819a3b1c5c
|
||||||
|
|
||||||
PKG_MAINTAINER:=Thomas Heil <heil@terminal-consulting.de>, \
|
PKG_MAINTAINER:=Thomas Heil <heil@terminal-consulting.de>, \
|
||||||
Christian Lachner <gladiac@gmail.com>
|
Christian Lachner <gladiac@gmail.com>
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,7 @@
|
||||||
#!/bin/sh
|
#!/bin/sh
|
||||||
|
|
||||||
CLONEURL=https://git.haproxy.org/git/haproxy-2.4.git
|
CLONEURL=https://git.haproxy.org/git/haproxy-2.4.git
|
||||||
BASE_TAG=v2.4.9
|
BASE_TAG=v2.4.10
|
||||||
TMP_REPODIR=tmprepo
|
TMP_REPODIR=tmprepo
|
||||||
PATCHESDIR=patches
|
PATCHESDIR=patches
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,93 @@
|
||||||
|
commit 0a55591b2c9140dee31dbedb6126fbb0b2eb3367
|
||||||
|
Author: Willy Tarreau <w@1wt.eu>
|
||||||
|
Date: Fri Dec 24 11:27:53 2021 +0100
|
||||||
|
|
||||||
|
BUG/MEDIUM: backend: fix possible sockaddr leak on redispatch
|
||||||
|
|
||||||
|
A subtle change of target address allocation was introduced with commit
|
||||||
|
68cf3959b ("MINOR: backend: rewrite alloc of stream target address") in
|
||||||
|
2.4. Prior to this patch, a target address was allocated by function
|
||||||
|
assign_server_address() only if none was previously allocated. After
|
||||||
|
the change, the allocation became unconditional. Most of the time it
|
||||||
|
makes no difference, except when we pass multiple times through
|
||||||
|
connect_server() with SF_ADDR_SET cleared.
|
||||||
|
|
||||||
|
The most obvious fix would be to avoid allocating that address there
|
||||||
|
when already set, but the root cause is that since introduction of
|
||||||
|
dynamically allocated addresses, the SF_ADDR_SET flag lies. It can
|
||||||
|
be cleared during redispatch or during a queue redistribution without
|
||||||
|
the address being released.
|
||||||
|
|
||||||
|
This patch instead gives back all its correct meaning to SF_ADDR_SET
|
||||||
|
and guarantees that when not set no address is allocated, by freeing
|
||||||
|
that address at the few places the flag is cleared. The flag could
|
||||||
|
even be removed so that only the address is checked but that would
|
||||||
|
require to touch many areas for no benefit.
|
||||||
|
|
||||||
|
The easiest way to test it is to send requests to a proxy with l7
|
||||||
|
retries enabled, which forwards to a server returning 500:
|
||||||
|
|
||||||
|
defaults
|
||||||
|
mode http
|
||||||
|
timeout client 1s
|
||||||
|
timeout server 1s
|
||||||
|
timeout connect 1s
|
||||||
|
retry-on all-retryable-errors
|
||||||
|
retries 1
|
||||||
|
option redispatch
|
||||||
|
|
||||||
|
listen proxy
|
||||||
|
bind *:5000
|
||||||
|
server app 0.0.0.0:5001
|
||||||
|
|
||||||
|
frontend dummy-app
|
||||||
|
bind :5001
|
||||||
|
http-request return status 500
|
||||||
|
|
||||||
|
Issuing "show pools" on the CLI will show that pool "sockaddr" grows
|
||||||
|
as requests are redispatched, and remains stable with the fix. Even
|
||||||
|
"ps" will show that the process' RSS grows by ~160B per request.
|
||||||
|
|
||||||
|
This fix will need to be backported to 2.4. Note that before 2.5,
|
||||||
|
there's no strm->si[1].dst, strm->target_addr must be used instead.
|
||||||
|
|
||||||
|
This addresses github issue #1499. Special thanks to Daniil Leontiev
|
||||||
|
for providing a well-documented reproducer.
|
||||||
|
|
||||||
|
(cherry picked from commit 266d5405490050adeaf414158f7f4b9bad5298bc)
|
||||||
|
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||||
|
(cherry picked from commit 11f5fdd07202f2d39b02c48e29b437aacd286c4d)
|
||||||
|
[wt: used target_addr]
|
||||||
|
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||||
|
|
||||||
|
--- a/include/haproxy/stream.h
|
||||||
|
+++ b/include/haproxy/stream.h
|
||||||
|
@@ -24,6 +24,7 @@
|
||||||
|
|
||||||
|
#include <haproxy/action-t.h>
|
||||||
|
#include <haproxy/api.h>
|
||||||
|
+#include <haproxy/connection.h>
|
||||||
|
#include <haproxy/fd.h>
|
||||||
|
#include <haproxy/freq_ctr.h>
|
||||||
|
#include <haproxy/obj_type.h>
|
||||||
|
@@ -341,6 +342,7 @@ static inline void stream_choose_redispa
|
||||||
|
if (may_dequeue_tasks(objt_server(s->target), s->be))
|
||||||
|
process_srv_queue(objt_server(s->target), 0);
|
||||||
|
|
||||||
|
+ sockaddr_free(&s->target_addr);
|
||||||
|
s->flags &= ~(SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET);
|
||||||
|
si->state = SI_ST_REQ;
|
||||||
|
} else {
|
||||||
|
--- a/src/queue.c
|
||||||
|
+++ b/src/queue.c
|
||||||
|
@@ -561,6 +561,10 @@ int pendconn_dequeue(struct stream *strm
|
||||||
|
strm->flags &= ~(SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET);
|
||||||
|
strm->flags |= p->strm_flags & (SF_DIRECT | SF_ASSIGNED | SF_ADDR_SET);
|
||||||
|
|
||||||
|
+ /* the entry might have been redistributed to another server */
|
||||||
|
+ if (!(strm->flags & SF_ADDR_SET))
|
||||||
|
+ sockaddr_free(&strm->target_addr);
|
||||||
|
+
|
||||||
|
if (p->target) {
|
||||||
|
/* a server picked this pendconn, it must skip LB */
|
||||||
|
strm->target = &p->target->obj_type;
|
||||||
|
|
@ -1,87 +0,0 @@
|
||||||
commit ea489aa18caacf057880c67b933689c363307a23
|
|
||||||
Author: Amaury Denoyelle <adenoyelle@haproxy.com>
|
|
||||||
Date: Thu Nov 25 10:20:19 2021 +0100
|
|
||||||
|
|
||||||
BUILD/MINOR: server: fix compilation without SSL
|
|
||||||
|
|
||||||
The build without SSL was broken by the following backported commit:
|
|
||||||
8e99b84dba7755e73563093a8fcedb25e70e7b94
|
|
||||||
MEDIUM: server/backend: implement websocket protocol selection
|
|
||||||
|
|
||||||
Fix this by checking the ALPN only if USE_OPENSSL and
|
|
||||||
TLSEXT_TYPE_application_layer_protocol_negotiation are defined. Else
|
|
||||||
always use the server mux-proto.
|
|
||||||
|
|
||||||
No need to backport.
|
|
||||||
|
|
||||||
--- a/src/server.c
|
|
||||||
+++ b/src/server.c
|
|
||||||
@@ -204,7 +204,33 @@ void srv_set_dyncookie(struct server *s)
|
|
||||||
*/
|
|
||||||
int srv_check_reuse_ws(struct server *srv)
|
|
||||||
{
|
|
||||||
- if (srv->mux_proto || srv->use_ssl != 1 || !srv->ssl_ctx.alpn_str) {
|
|
||||||
+#if defined(USE_OPENSSL) && defined(TLSEXT_TYPE_application_layer_protocol_negotiation)
|
|
||||||
+ if (!srv->mux_proto && srv->use_ssl && srv->ssl_ctx.alpn_str) {
|
|
||||||
+ /* ALPN selection.
|
|
||||||
+ * Based on the assumption that only "h2" and "http/1.1" token
|
|
||||||
+ * are used on server ALPN.
|
|
||||||
+ */
|
|
||||||
+ const struct ist alpn = ist2(srv->ssl_ctx.alpn_str,
|
|
||||||
+ srv->ssl_ctx.alpn_len);
|
|
||||||
+
|
|
||||||
+ switch (srv->ws) {
|
|
||||||
+ case SRV_WS_AUTO:
|
|
||||||
+ /* for auto mode, consider reuse as possible if the
|
|
||||||
+ * server uses a single protocol ALPN
|
|
||||||
+ */
|
|
||||||
+ if (!istchr(alpn, ','))
|
|
||||||
+ return 1;
|
|
||||||
+ break;
|
|
||||||
+
|
|
||||||
+ case SRV_WS_H2:
|
|
||||||
+ return isteq(alpn, ist("\x02h2"));
|
|
||||||
+
|
|
||||||
+ case SRV_WS_H1:
|
|
||||||
+ return isteq(alpn, ist("\x08http/1.1"));
|
|
||||||
+ }
|
|
||||||
+ }
|
|
||||||
+ else {
|
|
||||||
+#endif /* defined(OPENSSL) && defined(TLSEXT_TYPE_application_layer_protocol_negotiation) */
|
|
||||||
/* explicit srv.mux_proto or no ALPN : srv.mux_proto is used
|
|
||||||
* for mux selection.
|
|
||||||
*/
|
|
||||||
@@ -232,31 +258,9 @@ int srv_check_reuse_ws(struct server *sr
|
|
||||||
return 1;
|
|
||||||
break;
|
|
||||||
}
|
|
||||||
+#if defined(USE_OPENSSL) && defined(TLSEXT_TYPE_application_layer_protocol_negotiation)
|
|
||||||
}
|
|
||||||
- else {
|
|
||||||
- /* ALPN selection.
|
|
||||||
- * Based on the assumption that only "h2" and "http/1.1" token
|
|
||||||
- * are used on server ALPN.
|
|
||||||
- */
|
|
||||||
- const struct ist alpn = ist2(srv->ssl_ctx.alpn_str,
|
|
||||||
- srv->ssl_ctx.alpn_len);
|
|
||||||
-
|
|
||||||
- switch (srv->ws) {
|
|
||||||
- case SRV_WS_AUTO:
|
|
||||||
- /* for auto mode, consider reuse as possible if the
|
|
||||||
- * server uses a single protocol ALPN
|
|
||||||
- */
|
|
||||||
- if (!istchr(alpn, ','))
|
|
||||||
- return 1;
|
|
||||||
- break;
|
|
||||||
-
|
|
||||||
- case SRV_WS_H2:
|
|
||||||
- return isteq(alpn, ist("\x02h2"));
|
|
||||||
-
|
|
||||||
- case SRV_WS_H1:
|
|
||||||
- return isteq(alpn, ist("\x08http/1.1"));
|
|
||||||
- }
|
|
||||||
- }
|
|
||||||
+#endif /* defined(OPENSSL) && defined(TLSEXT_TYPE_application_layer_protocol_negotiation) */
|
|
||||||
|
|
||||||
return 0;
|
|
||||||
}
|
|
||||||
|
|
@ -0,0 +1,95 @@
|
||||||
|
commit 78bc9d39b0b31d647b35131ae45b21a145112192
|
||||||
|
Author: Willy Tarreau <w@1wt.eu>
|
||||||
|
Date: Fri Nov 12 10:26:18 2021 +0100
|
||||||
|
|
||||||
|
BUG/MINOR: pools: don't mark ourselves as harmless in DEBUG_UAF mode
|
||||||
|
|
||||||
|
When haproxy is built with DEBUG_UAF=1, some particularly slow
|
||||||
|
allocation functions are used for each pool, and it was not uncommon
|
||||||
|
to see the watchdog trigger during performance tests. For this reason
|
||||||
|
the allocation functions were surrounded by a pair of thread_harmless
|
||||||
|
calls to mention that the function was waiting in slow syscalls. The
|
||||||
|
problem is that this also releases functions blocked in thread_isolate()
|
||||||
|
which can then start their work.
|
||||||
|
|
||||||
|
In order to protect against the accidental removal of a shared resource
|
||||||
|
in this situation, in 2.5-dev4 with commit ba3ab7907 ("MEDIUM: servers:
|
||||||
|
make the server deletion code run under full thread isolation") was added
|
||||||
|
thread_isolate_full() for functions which want to be totally protected
|
||||||
|
due to being manipulating some data.
|
||||||
|
|
||||||
|
But this is not sufficient, because there are still places where we
|
||||||
|
can allocate/free (thus sleep) under a lock, such as in long call
|
||||||
|
chains involving the release of an idle connection. In this case, if
|
||||||
|
one thread asks for isolation, one thread might hang in
|
||||||
|
pool_alloc_area_uaf() with a lock held (for example the conns_lock
|
||||||
|
when coming from conn_backend_get()->h1_takeover()->task_new()), with
|
||||||
|
another thread blocked on a lock waiting for that one to release it,
|
||||||
|
both keeping their bit clear in the thread_harmless mask, preventing
|
||||||
|
the first thread from being released, thus causing a deadlock.
|
||||||
|
|
||||||
|
In addition to this, it was already seen that the "show fd" CLI handler
|
||||||
|
could wake up during a pool_free_area_uaf() with an incompletely
|
||||||
|
released memory area while deleting a file descriptor, and be fooled
|
||||||
|
showing bad pointers, or during a pool_alloc() on another thread that
|
||||||
|
was in the process of registering a freshly allocated connection to a
|
||||||
|
new file descriptor.
|
||||||
|
|
||||||
|
One solution could consist in replacing all thread_isolate() calls by
|
||||||
|
thread_isolate_full() but then that makes thread_isolate() useless
|
||||||
|
and only shifts the problem by one slot.
|
||||||
|
|
||||||
|
A better approach could possibly consist in having a way to mark that
|
||||||
|
a thread is entering an extremely slow section. Such sections would
|
||||||
|
be timed so that this is not abused, and the bit would be used to
|
||||||
|
make the watchdog more patient. This would be acceptable as this would
|
||||||
|
only affect debugging.
|
||||||
|
|
||||||
|
The approach used here for now consists in removing the harmless bits
|
||||||
|
around the UAF allocator, thus essentially undoing commit 85b2cae63
|
||||||
|
("MINOR: pools: make the thread harmless during the mmap/munmap
|
||||||
|
syscalls").
|
||||||
|
|
||||||
|
This is marked as minor because nobody is expected to be running with
|
||||||
|
DEBUG_UAF outside of development or serious debugging, so this issue
|
||||||
|
cannot affect regular users. It must be backported to stable branches
|
||||||
|
that have thread_harmless_now() around the mmap() call.
|
||||||
|
|
||||||
|
(cherry picked from commit fdf53b4962229b6cfcc5bc11151356c3d92d7023)
|
||||||
|
[wt: applied to include/pool-os.h]
|
||||||
|
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||||
|
|
||||||
|
--- a/include/haproxy/pool-os.h
|
||||||
|
+++ b/include/haproxy/pool-os.h
|
||||||
|
@@ -65,12 +65,8 @@ static inline void pool_free_area(void *
|
||||||
|
static inline void *pool_alloc_area(size_t size)
|
||||||
|
{
|
||||||
|
size_t pad = (4096 - size) & 0xFF0;
|
||||||
|
- int isolated;
|
||||||
|
void *ret;
|
||||||
|
|
||||||
|
- isolated = thread_isolated();
|
||||||
|
- if (!isolated)
|
||||||
|
- thread_harmless_now();
|
||||||
|
ret = mmap(NULL, (size + 4095) & -4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
|
||||||
|
if (ret != MAP_FAILED) {
|
||||||
|
/* let's dereference the page before returning so that the real
|
||||||
|
@@ -83,8 +79,6 @@ static inline void *pool_alloc_area(size
|
||||||
|
} else {
|
||||||
|
ret = NULL;
|
||||||
|
}
|
||||||
|
- if (!isolated)
|
||||||
|
- thread_harmless_end();
|
||||||
|
return ret;
|
||||||
|
}
|
||||||
|
|
||||||
|
@@ -101,9 +95,7 @@ static inline void pool_free_area(void *
|
||||||
|
if (pad >= sizeof(void *) && *(void **)(area - sizeof(void *)) != area)
|
||||||
|
ABORT_NOW();
|
||||||
|
|
||||||
|
- thread_harmless_now();
|
||||||
|
munmap(area - pad, (size + 4095) & -4096);
|
||||||
|
- thread_harmless_end();
|
||||||
|
}
|
||||||
|
|
||||||
|
#endif /* DEBUG_UAF */
|
||||||
|
|
@ -0,0 +1,42 @@
|
||||||
|
commit b9aac36d8d8d3bb05c785aa8f6630338d078f64a
|
||||||
|
Author: Willy Tarreau <w@1wt.eu>
|
||||||
|
Date: Sat Nov 20 19:17:38 2021 +0100
|
||||||
|
|
||||||
|
BUILD: cli: clear a maybe-unused warning on some older compilers
|
||||||
|
|
||||||
|
The SHOW_TOT() and SHOW_AVG() macros used in cli_io_handler_show_activity()
|
||||||
|
produce a warning on gcc 4.7 on MIPS with threads disabled because the
|
||||||
|
compiler doesn't know that global.nbthread is necessarily non-null, hence
|
||||||
|
that at least one iteration is performed. Let's just change the loop for
|
||||||
|
a do {} while () that lets the compiler know it's always initialized. It
|
||||||
|
also has the tiny benefit of making the code shorter.
|
||||||
|
|
||||||
|
(cherry picked from commit 97b5d07a3e5a33552327bac2e4c9c6a0496f7b5e)
|
||||||
|
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||||
|
|
||||||
|
--- a/src/cli.c
|
||||||
|
+++ b/src/cli.c
|
||||||
|
@@ -1368,8 +1368,10 @@ static int cli_io_handler_show_activity(
|
||||||
|
unsigned int _v[MAX_THREADS]; \
|
||||||
|
unsigned int _tot; \
|
||||||
|
const unsigned int _nbt = global.nbthread; \
|
||||||
|
- for (_tot = t = 0; t < _nbt; t++) \
|
||||||
|
+ _tot = t = 0; \
|
||||||
|
+ do { \
|
||||||
|
_tot += _v[t] = (x); \
|
||||||
|
+ } while (++t < _nbt); \
|
||||||
|
if (_nbt == 1) { \
|
||||||
|
chunk_appendf(&trash, " %u\n", _tot); \
|
||||||
|
break; \
|
||||||
|
@@ -1386,8 +1388,10 @@ static int cli_io_handler_show_activity(
|
||||||
|
unsigned int _v[MAX_THREADS]; \
|
||||||
|
unsigned int _tot; \
|
||||||
|
const unsigned int _nbt = global.nbthread; \
|
||||||
|
- for (_tot = t = 0; t < _nbt; t++) \
|
||||||
|
+ _tot = t = 0; \
|
||||||
|
+ do { \
|
||||||
|
_tot += _v[t] = (x); \
|
||||||
|
+ } while (++t < _nbt); \
|
||||||
|
if (_nbt == 1) { \
|
||||||
|
chunk_appendf(&trash, " %u\n", _tot); \
|
||||||
|
break; \
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
commit f9c40ed93330c9add953310b7ac04a0d4f43ec5c
|
||||||
|
Author: Daniel Jakots <haproxy@chown.me>
|
||||||
|
Date: Tue Dec 7 20:34:39 2021 -0500
|
||||||
|
|
||||||
|
BUILD: ssl: unbreak the build with newer libressl
|
||||||
|
|
||||||
|
In LibreSSL 3.5.0, BIO is going to become opaque, so haproxy's
|
||||||
|
compat macros will no longer work. The functions they substitute
|
||||||
|
have been available since LibreSSL 2.7.0.
|
||||||
|
|
||||||
|
(cherry picked from commit d1a2e2b0d1da0dff726738343fbaed044fb93470)
|
||||||
|
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||||
|
(cherry picked from commit ca0b6e08dcc3686794d37e8bfcc4e4a4b2adb806)
|
||||||
|
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||||
|
|
||||||
|
--- a/include/haproxy/openssl-compat.h
|
||||||
|
+++ b/include/haproxy/openssl-compat.h
|
||||||
|
@@ -386,7 +386,7 @@ static inline void SSL_CTX_up_ref(SSL_CT
|
||||||
|
#define SSL_CTX_get_extra_chain_certs(ctx, chain) do { *(chain) = (ctx)->extra_certs; } while (0)
|
||||||
|
#endif
|
||||||
|
|
||||||
|
-#if HA_OPENSSL_VERSION_NUMBER < 0x10100000L
|
||||||
|
+#if HA_OPENSSL_VERSION_NUMBER < 0x10100000L && (!defined(LIBRESSL_VERSION_NUMBER) || LIBRESSL_VERSION_NUMBER < 0x2070000fL)
|
||||||
|
#define BIO_get_data(b) (b)->ptr
|
||||||
|
#define BIO_set_data(b, v) do { (b)->ptr = (v); } while (0)
|
||||||
|
#define BIO_set_init(b, v) do { (b)->init = (v); } while (0)
|
||||||
|
|
@ -0,0 +1,24 @@
|
||||||
|
commit fc13f79e8f8090018109e5c91e4e4e55a7105146
|
||||||
|
Author: Thierry Fournier <thierry.fournier@ozon.io>
|
||||||
|
Date: Wed Dec 15 19:03:52 2021 +0100
|
||||||
|
|
||||||
|
DOC: fix misspelled keyword "resolve_retries" in resolvers
|
||||||
|
|
||||||
|
"resolve_retries" was spelled "resolve_retires".
|
||||||
|
|
||||||
|
(cherry picked from commit 55c40ea17778b1afa3cf8f0f0a2cc42717c9364a)
|
||||||
|
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||||
|
(cherry picked from commit 8bb445d66b1bfddd77e3cf25973a642212d90880)
|
||||||
|
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||||
|
|
||||||
|
--- a/doc/configuration.txt
|
||||||
|
+++ b/doc/configuration.txt
|
||||||
|
@@ -15145,7 +15145,7 @@ used by HAProxy. The following processin
|
||||||
|
2. When the fallback on the query type was done (or not applicable), HAProxy
|
||||||
|
retries the original DNS query, with the preferred query type.
|
||||||
|
|
||||||
|
- 3. HAProxy retries previous steps <resolve_retires> times. If no valid
|
||||||
|
+ 3. HAProxy retries previous steps <resolve_retries> times. If no valid
|
||||||
|
response is received after that, it stops the DNS resolution and reports
|
||||||
|
the error.
|
||||||
|
|
||||||
|
|
@ -0,0 +1,188 @@
|
||||||
|
commit 6338b7d4a884639f98823e885325a50750f72e04
|
||||||
|
Author: William Lallemand <wlallemand@haproxy.org>
|
||||||
|
Date: Tue Dec 28 18:47:17 2021 +0100
|
||||||
|
|
||||||
|
BUG/MEDIUM: ssl: initialize correctly ssl w/ default-server
|
||||||
|
|
||||||
|
This bug was introduced by d817dc73 ("MEDIUM: ssl: Load client
|
||||||
|
certificates in a ckch for backend servers") in which the creation of
|
||||||
|
the SSL_CTX for a server was moved to the configuration parser when
|
||||||
|
using a "crt" keyword instead of being done in ssl_sock_prepare_srv_ctx().
|
||||||
|
|
||||||
|
The patch 0498fa40 ("BUG/MINOR: ssl: Default-server configuration ignored by
|
||||||
|
server") made it worse by setting the same SSL_CTX for every servers
|
||||||
|
using a default-server. Resulting in any SSL option on a server applied
|
||||||
|
to every server in its backend.
|
||||||
|
|
||||||
|
This patch fixes the issue by reintroducing a string which store the
|
||||||
|
path of certificate inside the server structure, and loading the
|
||||||
|
certificate in ssl_sock_prepare_srv_ctx() again.
|
||||||
|
|
||||||
|
This is a quick fix to backport, a cleaner way can be achieve by always
|
||||||
|
creating the SSL_CTX in ssl_sock_prepare_srv_ctx() and splitting
|
||||||
|
properly the ssl_sock_load_srv_cert() function.
|
||||||
|
|
||||||
|
This patch fixes issue #1488.
|
||||||
|
|
||||||
|
Must be backported as far as 2.4.
|
||||||
|
|
||||||
|
(cherry picked from commit 2c776f1c30c85be11c9ba8ca8d9a7d62690d1a32)
|
||||||
|
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||||
|
(cherry picked from commit 2f3c354b6cdc21ee185e263b5c7422c86ae58c98)
|
||||||
|
[wt: ssl_sock_load_srv_cert() doesn't take the create_if_none arg in 2.4,
|
||||||
|
thus adjust context and make sure ssl_sock_prepare_srv_ctx() matches
|
||||||
|
what srv_parse_crt() used to do]
|
||||||
|
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||||
|
|
||||||
|
--- a/include/haproxy/server-t.h
|
||||||
|
+++ b/include/haproxy/server-t.h
|
||||||
|
@@ -364,6 +364,7 @@ struct server {
|
||||||
|
char *verify_host; /* hostname of certificate must match this host */
|
||||||
|
char *ca_file; /* CAfile to use on verify */
|
||||||
|
char *crl_file; /* CRLfile to use on verify */
|
||||||
|
+ char *client_crt; /* client certificate to send */
|
||||||
|
struct sample_expr *sni; /* sample expression for SNI */
|
||||||
|
#ifdef OPENSSL_NPN_NEGOTIATED
|
||||||
|
char *npn_str; /* NPN protocol string */
|
||||||
|
--- a/reg-tests/ssl/ssl_default_server.vtc
|
||||||
|
+++ b/reg-tests/ssl/ssl_default_server.vtc
|
||||||
|
@@ -15,7 +15,7 @@ feature cmd "$HAPROXY_PROGRAM -cc 'versi
|
||||||
|
feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
|
||||||
|
feature ignore_unknown_macro
|
||||||
|
|
||||||
|
-server s1 -repeat 7 {
|
||||||
|
+server s1 -repeat 10 {
|
||||||
|
rxreq
|
||||||
|
txresp
|
||||||
|
} -start
|
||||||
|
@@ -56,7 +56,10 @@ haproxy h1 -conf {
|
||||||
|
|
||||||
|
backend third_be
|
||||||
|
default-server ssl crt client1.pem ca-file ca-auth.crt verify none
|
||||||
|
- server s1 "${tmpdir}/ssl.sock" crt client2_expired.pem
|
||||||
|
+ server s1 "${tmpdir}/ssl.sock"
|
||||||
|
+ server s2 "${tmpdir}/ssl.sock" crt client2_expired.pem
|
||||||
|
+ server s3 "${tmpdir}/ssl.sock"
|
||||||
|
+ server s4 "${tmpdir}/ssl.sock"
|
||||||
|
|
||||||
|
backend fourth_be
|
||||||
|
default-server ssl crt client1.pem verify none
|
||||||
|
@@ -106,9 +109,25 @@ client c1 -connect ${h1_clearlst_sock} {
|
||||||
|
txreq
|
||||||
|
rxresp
|
||||||
|
expect resp.status == 200
|
||||||
|
+ expect resp.http.x-ssl == "Ok"
|
||||||
|
+} -run
|
||||||
|
+
|
||||||
|
+client c1 -connect ${h1_clearlst_sock} {
|
||||||
|
+ txreq -url "/third"
|
||||||
|
+ txreq
|
||||||
|
+ rxresp
|
||||||
|
+ expect resp.status == 200
|
||||||
|
expect resp.http.x-ssl == "Expired"
|
||||||
|
} -run
|
||||||
|
|
||||||
|
+client c1 -connect ${h1_clearlst_sock} -repeat 2 {
|
||||||
|
+ txreq -url "/third"
|
||||||
|
+ txreq
|
||||||
|
+ rxresp
|
||||||
|
+ expect resp.status == 200
|
||||||
|
+ expect resp.http.x-ssl == "Ok"
|
||||||
|
+} -run
|
||||||
|
+
|
||||||
|
client c1 -connect ${h1_clearlst_sock} {
|
||||||
|
txreq -url "/fourth"
|
||||||
|
txreq
|
||||||
|
--- a/src/cfgparse-ssl.c
|
||||||
|
+++ b/src/cfgparse-ssl.c
|
||||||
|
@@ -1450,25 +1450,17 @@ static int srv_parse_crl_file(char **arg
|
||||||
|
/* parse the "crt" server keyword */
|
||||||
|
static int srv_parse_crt(char **args, int *cur_arg, struct proxy *px, struct server *newsrv, char **err)
|
||||||
|
{
|
||||||
|
- int retval = -1;
|
||||||
|
- char *path = NULL;
|
||||||
|
-
|
||||||
|
if (!*args[*cur_arg + 1]) {
|
||||||
|
memprintf(err, "'%s' : missing certificate file path", args[*cur_arg]);
|
||||||
|
return ERR_ALERT | ERR_FATAL;
|
||||||
|
}
|
||||||
|
|
||||||
|
if ((*args[*cur_arg + 1] != '/') && global_ssl.crt_base)
|
||||||
|
- memprintf(&path, "%s/%s", global_ssl.crt_base, args[*cur_arg + 1]);
|
||||||
|
+ memprintf(&newsrv->ssl_ctx.client_crt, "%s/%s", global_ssl.crt_base, args[*cur_arg + 1]);
|
||||||
|
else
|
||||||
|
- memprintf(&path, "%s", args[*cur_arg + 1]);
|
||||||
|
-
|
||||||
|
- if (path) {
|
||||||
|
- retval = ssl_sock_load_srv_cert(path, newsrv, err);
|
||||||
|
- free(path);
|
||||||
|
- }
|
||||||
|
+ memprintf(&newsrv->ssl_ctx.client_crt, "%s", args[*cur_arg + 1]);
|
||||||
|
|
||||||
|
- return retval;
|
||||||
|
+ return 0;
|
||||||
|
}
|
||||||
|
|
||||||
|
/* parse the "no-check-ssl" server keyword */
|
||||||
|
--- a/src/server.c
|
||||||
|
+++ b/src/server.c
|
||||||
|
@@ -2063,6 +2063,8 @@ static void srv_conn_src_cpy(struct serv
|
||||||
|
static void srv_ssl_settings_cpy(struct server *srv, struct server *src)
|
||||||
|
{
|
||||||
|
/* <src> is the current proxy's default server and SSL is enabled */
|
||||||
|
+ BUG_ON(src->ssl_ctx.ctx != NULL); /* the SSL_CTX must never be initialized in a default-server */
|
||||||
|
+
|
||||||
|
if (src == &srv->proxy->defsrv && src->use_ssl == 1)
|
||||||
|
srv->flags |= SRV_F_DEFSRV_USE_SSL;
|
||||||
|
|
||||||
|
@@ -2070,10 +2072,11 @@ static void srv_ssl_settings_cpy(struct
|
||||||
|
srv->ssl_ctx.ca_file = strdup(src->ssl_ctx.ca_file);
|
||||||
|
if (src->ssl_ctx.crl_file != NULL)
|
||||||
|
srv->ssl_ctx.crl_file = strdup(src->ssl_ctx.crl_file);
|
||||||
|
+ if (src->ssl_ctx.client_crt != NULL)
|
||||||
|
+ srv->ssl_ctx.client_crt = strdup(src->ssl_ctx.client_crt);
|
||||||
|
|
||||||
|
srv->ssl_ctx.verify = src->ssl_ctx.verify;
|
||||||
|
|
||||||
|
- srv->ssl_ctx.ctx = src->ssl_ctx.ctx;
|
||||||
|
|
||||||
|
if (src->ssl_ctx.verify_host != NULL)
|
||||||
|
srv->ssl_ctx.verify_host = strdup(src->ssl_ctx.verify_host);
|
||||||
|
--- a/src/ssl_sock.c
|
||||||
|
+++ b/src/ssl_sock.c
|
||||||
|
@@ -4669,7 +4669,7 @@ int ssl_sock_prepare_srv_ctx(struct serv
|
||||||
|
{
|
||||||
|
struct proxy *curproxy = srv->proxy;
|
||||||
|
int cfgerr = 0;
|
||||||
|
- SSL_CTX *ctx = srv->ssl_ctx.ctx;
|
||||||
|
+ SSL_CTX *ctx;
|
||||||
|
|
||||||
|
/* Make sure openssl opens /dev/urandom before the chroot */
|
||||||
|
if (!ssl_initialize_random()) {
|
||||||
|
@@ -4693,6 +4693,26 @@ int ssl_sock_prepare_srv_ctx(struct serv
|
||||||
|
if (srv->use_ssl == 1)
|
||||||
|
srv->xprt = &ssl_sock;
|
||||||
|
|
||||||
|
+ if (srv->ssl_ctx.client_crt) {
|
||||||
|
+ char *err = NULL;
|
||||||
|
+ int err_code = 0;
|
||||||
|
+
|
||||||
|
+ /* If there is a crt keyword there, the SSL_CTX will be created here. */
|
||||||
|
+ err_code = ssl_sock_load_srv_cert(srv->ssl_ctx.client_crt, srv, &err);
|
||||||
|
+ if (err_code != ERR_NONE) {
|
||||||
|
+ if ((err_code & ERR_WARN) && !(err_code & ERR_ALERT))
|
||||||
|
+ ha_warning("%s", err);
|
||||||
|
+ else
|
||||||
|
+ ha_alert("%s", err);
|
||||||
|
+
|
||||||
|
+ if (err_code & (ERR_FATAL|ERR_ABORT))
|
||||||
|
+ cfgerr++;
|
||||||
|
+ }
|
||||||
|
+ ha_free(&err);
|
||||||
|
+ }
|
||||||
|
+
|
||||||
|
+ ctx = srv->ssl_ctx.ctx;
|
||||||
|
+
|
||||||
|
/* The context will be uninitialized if there wasn't any "cert" option
|
||||||
|
* in the server line. */
|
||||||
|
if (!ctx) {
|
||||||
|
|
@ -0,0 +1,64 @@
|
||||||
|
commit 7c565501e91f3759274dd7cd43ec9de7c4b9a162
|
||||||
|
Author: William Lallemand <wlallemand@haproxy.org>
|
||||||
|
Date: Wed Dec 29 18:16:27 2021 +0100
|
||||||
|
|
||||||
|
REGTESTS: ssl: fix ssl_default_server.vtc
|
||||||
|
|
||||||
|
Patch 2c776f1 ("BUG/MEDIUM: ssl: initialize correctly ssl w/
|
||||||
|
default-server") added tests that are not relevant anymore and broke the
|
||||||
|
reg-test. revert them.
|
||||||
|
|
||||||
|
(cherry picked from commit 0387632ac0db520402550cf19a96d41f8c1661de)
|
||||||
|
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||||
|
(cherry picked from commit 5229b2badbc370ef11ad5c6a7c9529bd24b2de66)
|
||||||
|
Signed-off-by: Willy Tarreau <w@1wt.eu>
|
||||||
|
|
||||||
|
--- a/reg-tests/ssl/ssl_default_server.vtc
|
||||||
|
+++ b/reg-tests/ssl/ssl_default_server.vtc
|
||||||
|
@@ -15,7 +15,7 @@ feature cmd "$HAPROXY_PROGRAM -cc 'versi
|
||||||
|
feature cmd "$HAPROXY_PROGRAM -cc 'feature(OPENSSL)'"
|
||||||
|
feature ignore_unknown_macro
|
||||||
|
|
||||||
|
-server s1 -repeat 10 {
|
||||||
|
+server s1 -repeat 7 {
|
||||||
|
rxreq
|
||||||
|
txresp
|
||||||
|
} -start
|
||||||
|
@@ -56,10 +56,7 @@ haproxy h1 -conf {
|
||||||
|
|
||||||
|
backend third_be
|
||||||
|
default-server ssl crt client1.pem ca-file ca-auth.crt verify none
|
||||||
|
- server s1 "${tmpdir}/ssl.sock"
|
||||||
|
- server s2 "${tmpdir}/ssl.sock" crt client2_expired.pem
|
||||||
|
- server s3 "${tmpdir}/ssl.sock"
|
||||||
|
- server s4 "${tmpdir}/ssl.sock"
|
||||||
|
+ server s1 "${tmpdir}/ssl.sock" crt client2_expired.pem
|
||||||
|
|
||||||
|
backend fourth_be
|
||||||
|
default-server ssl crt client1.pem verify none
|
||||||
|
@@ -109,25 +106,9 @@ client c1 -connect ${h1_clearlst_sock} {
|
||||||
|
txreq
|
||||||
|
rxresp
|
||||||
|
expect resp.status == 200
|
||||||
|
- expect resp.http.x-ssl == "Ok"
|
||||||
|
-} -run
|
||||||
|
-
|
||||||
|
-client c1 -connect ${h1_clearlst_sock} {
|
||||||
|
- txreq -url "/third"
|
||||||
|
- txreq
|
||||||
|
- rxresp
|
||||||
|
- expect resp.status == 200
|
||||||
|
expect resp.http.x-ssl == "Expired"
|
||||||
|
} -run
|
||||||
|
|
||||||
|
-client c1 -connect ${h1_clearlst_sock} -repeat 2 {
|
||||||
|
- txreq -url "/third"
|
||||||
|
- txreq
|
||||||
|
- rxresp
|
||||||
|
- expect resp.status == 200
|
||||||
|
- expect resp.http.x-ssl == "Ok"
|
||||||
|
-} -run
|
||||||
|
-
|
||||||
|
client c1 -connect ${h1_clearlst_sock} {
|
||||||
|
txreq -url "/fourth"
|
||||||
|
txreq
|
||||||
Loading…
Reference in a new issue