Re: TCP mode and ultra short lived connection
Hi Maksim, On Thu, Feb 11, 2021 at 01:20:04PM +0300, ?? ? wrote: > Thank you very much, Willy! > > Turning off abortonclose (it was enabled globally) for this particular > session really helped :) Fantastic, one less bug to chase :-) Cheers, Willy
Re: Segfault in liblua-5.3.so
Hi Sachin, On Thu, Feb 11, 2021 at 03:11:09AM +0530, Sachin Shetty wrote: > Hi, > > We have a lua block that connects to memcache when a request arrives > > """ > function get_from_gds(host, port, key)local sock = core.tcp() > sock:settimeout(20)local result = DOMAIN_NOT_FOUNDlocal > status, error = sock:connect(host, port)if not status then > core.Alert(GDS_LOG_PREFIX .. "GDS_ERROR: Error in connecting:" .. key > .. ":" .. port .. ":" .. error)return GDS_ERROR, "Error: " .. > errorendsock:send(key .. "\r\n")while true dolocal > s, status, partial = sock:receive("*l")if not s then > core.Alert(GDS_LOG_PREFIX .. "GDS_ERROR: Error reading:" .. key .. > ":" .. port .. ":" .. status)return GDS_ERROR, status > endif s == "END" then break endresult = send > sock:close()return resultend > > -- Comment: get_proxy calls get_from_gds > > core.register_action("get_proxy", { "http-req" }, get_proxy) > """ > The value is cached in a haproxy map so we don't make a memcache > connection for every request. > > At peak traffic if we reload haproxy, that invalidates the map and the > surge causes > quite a few memcache connections to fail. Error returned is "Can't connect" > > We see the following messages in dmesg > > [ +0.006924] haproxy[14258]: segfault at 0 ip 7f117fba94c4 sp > 7f1179eefe08 error 4 in liblua-5.3.so[7f117fba1000+37000] > > HA-Proxy version 2.0.18-be8b761 2020/09/30 - https://haproxy.org/ Unfortunately, this is not enough to figure the cause, you'll need to enable core dumps and to pass it through gdb to figure a more exploitable backtrace. Please take this opportunity for updating, as I'm seeing 117 patches merged into 2.0 after your version, some of which affect Lua and others related to thread safety. One of them is even related to Lua+maps. Note, if that's not urgent on your side, we do have a few more fixes pending to be backported to 2.0 that will warrant yet another version. However none of them seem related to your issue (but if you're willing to retest with the latest 2.0 snapshot you're welcome of course). Willy
Re: [PATCH] introduce guard for SCTL openssl specific functions
I changed macro name, new patch attached сб, 13 февр. 2021 г. в 03:41, William Lallemand : > On Sat, Feb 13, 2021 at 12:21:56AM +0500, Илья Шипицин wrote: > > Hello, > > > > let as switch to feature macro instead of HA_OPENSSL_VERSION. > > > > Ilya > > Hello Ilya, > > For more concistency with the other macros I'd rather use > "HAVE_SSL_SCTL" instead of "HAVE_OPENSSL_SCTL". > > Regards, > > -- > William Lallemand > From 48fda0400d94c354f3eee937896eae5c22e67705 Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Sat, 13 Feb 2021 11:45:33 +0500 Subject: [PATCH] BUILD: ssl: introduce fine guard for OpenSSL specific SCTL functions SCTL (signed certificate timestamp list) specified in RFC6962 was implemented in c74ce24cd22e8c683ba0e5353c0762f8616e597d, let us introduce macro HAVE_SSL_SCTL for the HAVE_SSL_SCTL sake, which in turn is based on SN_ct_cert_scts, which comes in the same commit --- include/haproxy/openssl-compat.h | 4 src/ssl_ckch.c | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/include/haproxy/openssl-compat.h b/include/haproxy/openssl-compat.h index 3fe58be40..b5f05d1ae 100644 --- a/include/haproxy/openssl-compat.h +++ b/include/haproxy/openssl-compat.h @@ -57,6 +57,10 @@ #define HAVE_SSL_CTX_get0_privatekey #endif +#if (defined(SN_ct_cert_scts) && !defined(OPENSSL_NO_TLSEXT)) +#define HAVE_SSL_SCTL +#endif + #if (HA_OPENSSL_VERSION_NUMBER < 0x0090800fL) /* Functions present in OpenSSL 0.9.8, older not tested */ static inline const unsigned char *SSL_SESSION_get_id(const SSL_SESSION *sess, unsigned int *sid_length) diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c index f654b4b52..8aa29bd22 100644 --- a/src/ssl_ckch.c +++ b/src/ssl_ckch.c @@ -320,7 +320,7 @@ int ssl_sock_load_files_into_ckch(const char *path, struct cert_key_and_chain *c goto end; } -#if (HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT && !defined OPENSSL_IS_BORINGSSL) +#ifdef HAVE_SSL_SCTL /* try to load the sctl file */ if (global_ssl.extra_files & SSL_GF_SCTL) { struct stat st; @@ -939,7 +939,7 @@ enum { CERT_TYPE_OCSP, #endif CERT_TYPE_ISSUER, -#if (HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT && !defined OPENSSL_IS_BORINGSSL) +#ifdef HAVE_SSL_SCTL CERT_TYPE_SCTL, #endif CERT_TYPE_MAX, @@ -956,7 +956,7 @@ struct { #if ((defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined OPENSSL_NO_OCSP) || defined OPENSSL_IS_BORINGSSL) [CERT_TYPE_OCSP] = { "ocsp",CERT_TYPE_OCSP, &ssl_sock_load_ocsp_response_from_file }, #endif -#if (HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT && !defined OPENSSL_IS_BORINGSSL) +#ifdef HAVE_SSL_SCTL [CERT_TYPE_SCTL] = { "sctl",CERT_TYPE_SCTL, &ssl_sock_load_sctl_from_file }, #endif [CERT_TYPE_ISSUER] = { "issuer", CERT_TYPE_ISSUER, &ssl_sock_load_issuer_file_into_ckch }, -- 2.29.2
Re: [PATCH] introduce guard for SCTL openssl specific functions
On Sat, Feb 13, 2021 at 12:21:56AM +0500, Илья Шипицин wrote: > Hello, > > let as switch to feature macro instead of HA_OPENSSL_VERSION. > > Ilya Hello Ilya, For more concistency with the other macros I'd rather use "HAVE_SSL_SCTL" instead of "HAVE_OPENSSL_SCTL". Regards, -- William Lallemand
[PATCH] introduce guard for SCTL openssl specific functions
Hello, let as switch to feature macro instead of HA_OPENSSL_VERSION. Ilya From ddae23ca3503f29416cb04dc5689282be67df087 Mon Sep 17 00:00:00 2001 From: Ilya Shipitsin Date: Sat, 13 Feb 2021 00:16:58 +0500 Subject: [PATCH] BUILD: ssl: introduce fine guard for OpenSSL specific SCTL functions SCTL (signed certificate timestamp list) specified in RFC6962 was implemented in c74ce24cd22e8c683ba0e5353c0762f8616e597d, let us introduce macro HAVE_OPENSSL_SCTL for the HAVE_OPENSSL_SCTL sake, which in turn is based on SN_ct_cert_scts, which comes in the same commit --- include/haproxy/openssl-compat.h | 4 src/ssl_ckch.c | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/include/haproxy/openssl-compat.h b/include/haproxy/openssl-compat.h index 3fe58be40..f3044228a 100644 --- a/include/haproxy/openssl-compat.h +++ b/include/haproxy/openssl-compat.h @@ -57,6 +57,10 @@ #define HAVE_SSL_CTX_get0_privatekey #endif +#if (defined(SN_ct_cert_scts) && !defined(OPENSSL_NO_TLSEXT)) +#define HAVE_OPENSSL_SCTL +#endif + #if (HA_OPENSSL_VERSION_NUMBER < 0x0090800fL) /* Functions present in OpenSSL 0.9.8, older not tested */ static inline const unsigned char *SSL_SESSION_get_id(const SSL_SESSION *sess, unsigned int *sid_length) diff --git a/src/ssl_ckch.c b/src/ssl_ckch.c index f654b4b52..d0cc562c0 100644 --- a/src/ssl_ckch.c +++ b/src/ssl_ckch.c @@ -320,7 +320,7 @@ int ssl_sock_load_files_into_ckch(const char *path, struct cert_key_and_chain *c goto end; } -#if (HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT && !defined OPENSSL_IS_BORINGSSL) +#ifdef HAVE_OPENSSL_SCTL /* try to load the sctl file */ if (global_ssl.extra_files & SSL_GF_SCTL) { struct stat st; @@ -939,7 +939,7 @@ enum { CERT_TYPE_OCSP, #endif CERT_TYPE_ISSUER, -#if (HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT && !defined OPENSSL_IS_BORINGSSL) +#ifdef HAVE_OPENSSL_SCTL CERT_TYPE_SCTL, #endif CERT_TYPE_MAX, @@ -956,7 +956,7 @@ struct { #if ((defined SSL_CTRL_SET_TLSEXT_STATUS_REQ_CB && !defined OPENSSL_NO_OCSP) || defined OPENSSL_IS_BORINGSSL) [CERT_TYPE_OCSP] = { "ocsp",CERT_TYPE_OCSP, &ssl_sock_load_ocsp_response_from_file }, #endif -#if (HA_OPENSSL_VERSION_NUMBER >= 0x1000200fL && !defined OPENSSL_NO_TLSEXT && !defined OPENSSL_IS_BORINGSSL) +#ifdef HAVE_OPENSSL_SCTL [CERT_TYPE_SCTL] = { "sctl",CERT_TYPE_SCTL, &ssl_sock_load_sctl_from_file }, #endif [CERT_TYPE_ISSUER] = { "issuer", CERT_TYPE_ISSUER, &ssl_sock_load_issuer_file_into_ckch }, -- 2.29.2
Re: [PATCH] fix freebsd ci, update freebsd image
Le 11/02/2021 à 19:35, Илья Шипицин a écrit : Hello, attached patch fix freebsd builds. Ilya Thanks, now merged ! -- Christopher Faulet
[PATCH] DOC: tune: explain the origin of block size for ssl.cachesize
A user could eventually ask himself where those 200 bytes block size are coming from. This patch tries to better explain the origin in case people are curious or want to double check the reality. Signed-off-by: William Dauchy --- doc/configuration.txt | 21 +++-- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/doc/configuration.txt b/doc/configuration.txt index 391e074a7..b21c56091 100644 --- a/doc/configuration.txt +++ b/doc/configuration.txt @@ -2451,16 +2451,17 @@ tune.sndbuf.server tune.ssl.cachesize Sets the size of the global SSL session cache, in a number of blocks. A block - is large enough to contain an encoded session without peer certificate. - An encoded session with peer certificate is stored in multiple blocks - depending on the size of the peer certificate. A block uses approximately - 200 bytes of memory. The default value may be forced at build time, otherwise - defaults to 2. When the cache is full, the most idle entries are purged - and reassigned. Higher values reduce the occurrence of such a purge, hence - the number of CPU-intensive SSL handshakes by ensuring that all users keep - their session as long as possible. All entries are pre-allocated upon startup - and are shared between all processes if "nbproc" is greater than 1. Setting - this value to 0 disables the SSL session cache. + is large enough to contain an encoded session without peer certificate. An + encoded session with peer certificate is stored in multiple blocks depending + on the size of the peer certificate. A block uses approximately 200 bytes of + memory (based on `sizeof(struct sh_ssl_sess_hdr) + SHSESS_BLOCK_MIN_SIZE` + calculation used for `shctx_init` function). The default value may be forced + at build time, otherwise defaults to 2. When the cache is full, the most + idle entries are purged and reassigned. Higher values reduce the occurrence + of such a purge, hence the number of CPU-intensive SSL handshakes by ensuring + that all users keep their session as long as possible. All entries are + pre-allocated upon startup and are shared between all processes if "nbproc" + is greater than 1. Setting this value to 0 disables the SSL session cache. tune.ssl.force-private-cache This option disables SSL session cache sharing between all processes. It -- 2.30.0
Re: [PATCH v3 0/5] cli commands for checks and agent
On Fri, Feb 12, 2021 at 3:06 PM Christopher Faulet wrote: > I just slightly amended the 3rd patch to handle the v2 in > apply_server_state(). > There is a test on the version when a state-file is local to a proxy. Just a > minor change. ok, thanks for that. > And in the last one, I removed the "chunk_appendf(msg, "\n");" to > move the LF in ha_warning() calls. yes ok it is probably clearer that way. Glad to see the end of this series, now I'm ready to receive related bug reports ;) -- William
Re: [PATCH v3 0/5] cli commands for checks and agent
Le 11/02/2021 à 22:51, William Dauchy a écrit : Hello Christopher, Here is some work to finish you week. I believe I addressed all the points raised: - warning are no longer emitted when we have "0" or "-" values - I enhanced the warning output as well, and understood my mistake for my previous CLEANUP patch removing a space... so I removed this patch as well. - Fixed all the chunk_appendf issues. - I was a bit lazy to address the partial vs complete failure in parsing as I was a bit puzzled about the approach to take. I think it would be sad to duplicate the code for pre validation, but on the other hand I agree it was clear to assume the whole line was not applied at all. I however concluded it was acceptable to simply know the line was not fully applied. I believe in that case the user should worry. We can probably add a way to show where it stopped, but I felt discouraged with the srv resolution, in the middle of srv port, where it is harder to have a kinda generic way to know where we stopped. William Dauchy (5): MEDIUM: cli: add check-addr command MEDIUM: cli: add agent-port command MEDIUM: server: add server-states version 2 MEDIUM: server: support {check,agent}_addr, agent_port in server state MINOR: server: enhance error precision when applying server state Ok, it is good for me. I will push it soon. Thanks William! And don't be too worry about the loading of server-state files. This part is a mess and should be refactored, at least to be readable and also to fix bugs on error path. I'm tempted to do so and a bit afraid too. I just slightly amended the 3rd patch to handle the v2 in apply_server_state(). There is a test on the version when a state-file is local to a proxy. Just a minor change. And in the last one, I removed the "chunk_appendf(msg, "\n");" to move the LF in ha_warning() calls. -- Christopher Faulet
Re: stats / "show servers conn" looses counter after reload
On 2021-02-12 12:06, William Dauchy wrote: Hi Christian, On Fri, Feb 12, 2021 at 11:59 AM Christian Ruppert wrote: Is this a bug? Can you confirm this behavior? Is there any other way I could figure out whether a backend is currently in use? unfortunately reload does not recover stats values; it is a known problem; see also https://github.com/haproxy/haproxy/issues/954 Thanks, William! I just commented on that issue. -- Regards, Christian Ruppert
Re: [PATCH] fix freebsd ci, update freebsd image
Willy, Am 11.02.21 um 19:35 schrieb Илья Шипицин: > attached patch fix freebsd builds. This one looks good to me. Please take it. Best regards Tim Düsterhus
Re: stats / "show servers conn" looses counter after reload
Hi Christian, On Fri, Feb 12, 2021 at 11:59 AM Christian Ruppert wrote: > Is this a bug? Can you confirm this behavior? Is there any other way I > could figure out whether a backend is currently in use? unfortunately reload does not recover stats values; it is a known problem; see also https://github.com/haproxy/haproxy/issues/954 -- William
stats / "show servers conn" looses counter after reload
Hi list, I'm not sure if that is intended, to me it looks like a bug. I was trying to figure out if a backend is in use or not so I was looking for the used_cur: echo "show servers conn somebackend_rtmp" | socat stdio /var/run/haproxy.stat # bkname/svname bkid/svid addr port - purge_delay used_cur used_max need_est unsafe_nb safe_nb idle_lim idle_cur idle_per_thr[48] somebackend_rtmp/localhost 615/1 127.0.0.1 50643 - 5000 0 0 0 0 0 -1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 I then noticed, that those values doesn't match with what tcpdump says. I tracked it down to be caused by a reload. A testcase: Create a large file with dd or throttle your browsers connection to like modem or something to just keep the connection open Do a "show servers conn" and make sure used_cur is > 0 Reload HAProxy Notice it's 0 even though your download / session continues I tested it with tcp as well as http mode. I also have "expose-fd listeners" in use, in case it matters. Affected are at least 2.2.9 as well as 2.3.5. The stats backend also looses its counter values. Is this a bug? Can you confirm this behavior? Is there any other way I could figure out whether a backend is currently in use? -- Regards, Christian Ruppert