Re: TCP mode and ultra short lived connection

2021-02-12 Thread Willy Tarreau
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

2021-02-12 Thread Willy Tarreau
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

2021-02-12 Thread Илья Шипицин
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

2021-02-12 Thread 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



[PATCH] introduce guard for SCTL openssl specific functions

2021-02-12 Thread Илья Шипицин
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

2021-02-12 Thread Christopher Faulet

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

2021-02-12 Thread William Dauchy
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

2021-02-12 Thread William Dauchy
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

2021-02-12 Thread Christopher Faulet

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

2021-02-12 Thread Christian Ruppert

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

2021-02-12 Thread Tim Düsterhus
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

2021-02-12 Thread William Dauchy
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

2021-02-12 Thread Christian Ruppert

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