Re: HAProxy 1.8.3 SSL caching regression

[Willy Tarreau]

On Thu, Jan 04, 2018 at 02:14:41PM -0500, Jeffrey J. Persch wrote:
> Hi William,
> 
> Verified.
> 
> Thanks for the quick fix,

Great, patch now merged. Thanks!
Willy

Re: HAProxy 1.8.3 SSL caching regression

[Jeffrey J. Persch]

Hi William,

Verified.

Thanks for the quick fix,
Jeffrey J. Persch


On Wed, Jan 3, 2018 at 2:02 PM, Jeffrey J. Persch 
wrote:

> Hi William,
>
> The test case now works. I will do load testing with the patch today.
>
> Thanks,
> Jeffrey J. Persch
>
> On Wed, Jan 3, 2018 at 1:25 PM, William Lallemand 
> wrote:
>
>> On Wed, Jan 03, 2018 at 06:41:01PM +0100, William Lallemand wrote:
>> > I'm able to reproduce the problem thanks to your detailed example, it
>> looks
>> > like a regression in the code.
>> >
>> > I will check the code to see what's going on.
>>
>> I found the issue, would you mind trying the attached patch?
>>
>> Thanks.
>>
>> --
>> William Lallemand
>>
>
>

Re: HAProxy 1.8.3 SSL caching regression

[Jeffrey J. Persch]

Hi William,

The test case now works. I will do load testing with the patch today.

Thanks,
Jeffrey J. Persch

On Wed, Jan 3, 2018 at 1:25 PM, William Lallemand 
wrote:

> On Wed, Jan 03, 2018 at 06:41:01PM +0100, William Lallemand wrote:
> > I'm able to reproduce the problem thanks to your detailed example, it
> looks
> > like a regression in the code.
> >
> > I will check the code to see what's going on.
>
> I found the issue, would you mind trying the attached patch?
>
> Thanks.
>
> --
> William Lallemand
>

Re: HAProxy 1.8.3 SSL caching regression

[William Lallemand]

On Wed, Jan 03, 2018 at 06:41:01PM +0100, William Lallemand wrote:
> I'm able to reproduce the problem thanks to your detailed example, it looks
> like a regression in the code.
> 
> I will check the code to see what's going on.

I found the issue, would you mind trying the attached patch?

Thanks.

-- 
William Lallemand
>From da786103ff39a0bed8efbde120808b2ee2ec Mon Sep 17 00:00:00 2001
From: William Lallemand 
Date: Wed, 3 Jan 2018 19:15:51 +0100
Subject: [PATCH] BUG/MEDIUM: ssl: cache doesn't release shctx blocks

Since the rework of the shctx with the hot list system, the ssl cache
was putting session inside the hot list, without removing them.
Once all block were used, they were all locked in the hot list, which
was forbiding to reuse them for new sessions.

Bug introduced by 4f45bb9 ("MEDIUM: shctx: separate ssl and shctx")

Thanks to Jeffrey J. Persch for reporting this bug.

Must be backported to 1.8.
---
 src/ssl_sock.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index f9d5f2567..322b05409 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -3849,8 +3849,12 @@ static int sh_ssl_sess_store(unsigned char *s_id, unsigned char *data, int data_
 		first->len = sizeof(struct sh_ssl_sess_hdr);
 	}
 
-	if (shctx_row_data_append(ssl_shctx, first, data, data_len) < 0)
+	if (shctx_row_data_append(ssl_shctx, first, data, data_len) < 0) {
+		shctx_row_dec_hot(ssl_shctx, first);
 		return 0;
+	}
+
+	shctx_row_dec_hot(ssl_shctx, first);
 
 	return 1;
 }
-- 
2.13.6

Re: HAProxy 1.8.3 SSL caching regression

[William Lallemand]

On Wed, Jan 03, 2018 at 12:04:50PM -0500, Jeffrey J. Persch wrote:
> Greetings,
> 

Hi Jeffrey,

> We have been load testing 1.8.3 and noticed SSL caching was broken in 1.8
> during the shctx refactoring.
> 
> New SSL connections will cache up until tune.ssl.cachesize, then no
> connections will ever be cached again.
> 
> In haproxy 1.7 and before, the SSL cache works correctly as a LRU cache.
> 
> 
> [...] 
> 
> This appears to independent of target & openssl version, we have reproduced
> on linux2628 openssl 1.0.1k-fips and osx openssl 1.0.2n.
> 
> Any insights appreciated.
> 

I'm able to reproduce the problem thanks to your detailed example, it looks
like a regression in the code.

I will check the code to see what's going on.

-- 
William Lallemand

HAProxy 1.8.3 SSL caching regression

[Jeffrey J. Persch]

Greetings,

We have been load testing 1.8.3 and noticed SSL caching was broken in 1.8
during the shctx refactoring.

New SSL connections will cache up until tune.ssl.cachesize, then no
connections will ever be cached again.

In haproxy 1.7 and before, the SSL cache works correctly as a LRU cache.


Example configuration file, haproxy-ssl-cache.cfg, with cachesize set to 3
to easily reproduce:

global
ssl-default-bind-ciphers HIGH:!aNULL:!MD5
ssl-default-bind-options no-sslv3 no-tls-tickets
tune.ssl.default-dh-param 2048
tune.ssl.cachesize 3
tune.ssl.lifetime 60

defaults
stats enable
stats uri /haproxy/stats

frontend some-frontend
bind :8443 ssl crt self-signed.pem
mode http
timeout client 15s
timeout http-request 15s
use_backend some-backend

backend some-backend
mode http
timeout connect 1s
timeout queue 0s
timeout server 1s
server some-server 127.0.0.1:8091 check


Example script to build and test on macosx:

srcdir=haproxy-1.8

# Install openssl library
brew install openssl

# Build HAProxy with OpenSSL support
make -C $srcdir TARGET=osx USE_OPENSSL=1
SSL_INC=/usr/local/opt/openssl/include
SSL_LIB=/usr/local/opt/openssl/lib USE_ZLIB=1

# Generate self signed cert
openssl req -newkey rsa:2048 -nodes -keyout self-signed.key -x509
-days 365 -out self-signed.crt -subj
"/C=US/ST=Pennsylvania/L=Philadelphia/O=HAProxy/OU=QA/CN=localhost"
cat self-signed.crt self-signed.key >>self-signed.pem

# Run HAProxy
$srcdir/haproxy -f haproxy-ssl-cache.cfg &

# Demonstrate failure to cache new sessions after cache fills
openssl s_client -connect localhost:8443 -reconnect -no_ticket
verify.err | egrep 'New|Reused' # PASS: 1 New, 5 Reused
openssl s_client -connect localhost:8443 -reconnect -no_ticket
verify.err | egrep 'New|Reused' # PASS: 1 New, 5 Reused
openssl s_client -connect localhost:8443 -reconnect -no_ticket
verify.err | egrep 'New|Reused' # PASS: 1 New, 5 Reused
openssl s_client -connect localhost:8443 -reconnect -no_ticket
verify.err | egrep 'New|Reused' # FAIL: 6 New

# Demonstrate failure to evict old entries from cache
sleep 65
openssl s_client -connect localhost:8443 -reconnect -no_ticket
verify.err | egrep 'New|Reused' # FAIL: 6 New


This appears to independent of target & openssl version, we have reproduced
on linux2628 openssl 1.0.1k-fips and osx openssl 1.0.2n.

Any insights appreciated.

Thanks,
Jeffrey J. Persch