Re: ssl: crashing since 8d85aa (BUG/MAJOR: map: fix segfault ...)

[Lukas Tribus]

Hi Emeric,


Am 05.07.2017 um 13:58 schrieb Emeric Brun:
>
>> Another bisect (this time with -dM or -DDEBUG_MEMORY), another commit...
>> Now it points to 23e9e931 (MINOR: log: Add logurilen tunable).
>>
>>
> Hi Lukas,
>
> Indeed this commit introduced a regression.
>
> The commit in attachment should fix the issue.
>

Great, thank you.

I guess 23e9e931 also made matching a path unreliable, because with the
crash fixed, the following configuration now works reliably:

use_backend robots if { path /robots.txt }




Thanks!

Lukas

Re: Passing SNI value ( ssl_fc_sni ) to backend's verifyhost.

[Willy Tarreau]

Hi guys,

back to this old discussion.

On Fri, May 12, 2017 at 04:10:20PM +0200, Willy Tarreau wrote:
> On Tue, May 09, 2017 at 12:12:42AM +0200, Lukas Tribus wrote:
> > Haproxy can verify the certificate of backend TLS servers since day 1.
> > 
> > The only thing missing is client SNI based backend certificate
> > verification, which yes - since we can pass client SNI to the TLS server
> > - we need to consider for the certificate verification process as well.
> 
> In fact the cert name is checked, it's just that it can only check against
> a constant in the configuration. I agree that it's a problem when using SNI.
> Furthermore it forces one to completely disable verifyhost in case SNI is
> used.
> 
> I tend to think that the best approach would be to always enable it when
> SNI is involved in fact, because if SNI is used to the server, it really
> means we want to check what cert is provided. This could then possibly be
> explicitly turned off by the "verify none" directive.
> 
> I have absolutely no idea how to do that however, I don't know if we can
> retrieve the previously configured SNI using openssl's API after the
> connection is established.

So after digging I found a way to implement this. The configured SNI value
is indeed stored in the SSL session and can be extracted, more or less
easily depending on the version. I came up with the attached patch that
works for me on all cases below. I'm willing to merge it into 1.8-dev and
later backport to older versions once we're sure it does the right thing
and doesn't break compatibility with openssl forks.

Please give it a try, and if some can test it on libressl/boringssl, it
would be nice.

---
tested with the following server configurations (cert on port 8443 is valid
for "localhost" only) :

# passes checks and traffic (no hostname check)
server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem

# passes checks and traffic (localhost is what the server presents)
server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem verifyhost localhost

# fails checks and traffic (foo not matched on the server)
server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem verifyhost foo

# passes checks and traffic (verify none ignores the host)
server ssl 127.0.0.1:8443 ssl verify none check inter 100 ca-file 
rsa2048.pem verifyhost foo

# passes checks and traffic (localhost is fine)
server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem sni str(localhost) verifyhost localhost

# passes checks and traffic (verifyhost overrides sni)
server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem sni str(foo) verifyhost localhost

# passes checks and traffic (localhost always valid)
server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem sni str(localhost)

# passes checks, and traffic without host or with "host: localhost" and 
fails other hosts.
server ssl 127.0.0.1:8443 ssl verify required check inter 100 ca-file 
rsa2048.pem sni req.hdr(host)
---

Cheers,
Willy
>From 14eea17b6f43f391a2ac7436ce5aafc740e121aa Mon Sep 17 00:00:00 2001
From: Willy Tarreau 
Date: Wed, 5 Jul 2017 18:23:03 +0200
Subject: MINOR: ssl: compare server certificate names to the SNI on outgoing
 connections

When support for passing SNI to the server was added in 1.6-dev3, there
was no way to validate that the certificate presented by the server would
really match the name requested in the SNI, which is quite a problem as
it allows other (valid) certificates to be presented instead (when hitting
the wrong server or due to a man in the middle).

This patch adds the missing check against the value passed in the SNI.
The "verifyhost" value keeps precedence if set. If no SNI is used and
no verifyhost directive is specified, then the certificate name is not
checked (this is unchanged).

In order to extract the SNI value, it was necessary to make use of
SSL_SESSION_get0_hostname(), which appeared in openssl 1.1.0. This is
a trivial function which returns the value of s->tlsext_hostname, so
it was open coded on older versions. I don't know what it does for
libressl / boringssl nor any possible other fork.

After some careful observation period it may make sense to backport
this to 1.7 and 1.6 as some users rightfully consider this limitation
as a bug.
---
 doc/configuration.txt | 23 +++
 src/ssl_sock.c| 22 +-
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 7cac3ca..0f425f4 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -11474,7 +11474,9 @@ sni 
   string and uses the result as the host name sent in the SNI TLS extension to
   the server. A typical 

Re: Creating a health check in Lua?

[Thierry FOURNIER]

On Wed, 5 Jul 2017 11:29:10 +0300
Gil Bahat  wrote:

> Hi,
> 
> I have some lua code I would like to use as a health check. Documentation
> seems to hint this is possible somehow, but I have not been able to either
> direct an applet:send to the health check (nor is it clear how it could
> expect something in return) or formulate the strings on demand via lua,
> which was the other idea I was looking at.


Hi, the integration of Lua in the health check are in my todo list.
I'm very busy by my professional activity and sadly, this development
is not in my priorities.


> as a not unrelated aside, the 'use-service' directive is missing from the
> configuration manual.


Ok, thanks I will check this.


Thierry


> Regards,
> 
> Gil

Re: ssl: crashing since 8d85aa (BUG/MAJOR: map: fix segfault ...)

[Willy Tarreau]

Hi Aleks,

On Wed, Jul 05, 2017 at 02:12:13PM +0200, Aleksandar Lazic wrote:
> Amazing how fast this Open source community is ;-)
> 
> Open : Dienstag, 04. Juli 2017, 21:56:09  (Tue, 4 Jul 2017 21:56:09 +0200)
> Close: Mittwoch, 05. Juli 2017, 14:00:32  (Wed, 5 Jul 2017 14:00:32 +0200)
> 
> Duration: ~16 Hours ( 16 hours, 3 minutes and 51 seconds )
> https://www.timeanddate.com/date/durationresult.html?d1=04=07=2017=05=07=2017=21=56=09=14=00=

You could even count the moment the bug was introduced, it was about 1.5
months ago, which validates the benefit of having an open development
model :

  commit 23e9e931284b44e9d06cca26ab13648873b4029b
  Author: Stéphane Cottin 
  Date:   Thu May 18 08:58:41 2017 +0200

MINOR: log: Add logurilen tunable.

  (...)

Willy

Re: ssl: crashing since 8d85aa (BUG/MAJOR: map: fix segfault ...)

[Willy Tarreau]

On Wed, Jul 05, 2017 at 01:58:03PM +0200, Emeric Brun wrote:
> Indeed this commit introduced a regression.
> 
> The commit in attachment should fix the issue.

Awesome, now merged, thanks! And it was specific to 1.8-dev.

Willy

Re: ssl: crashing since 8d85aa (BUG/MAJOR: map: fix segfault ...)

[Emeric Brun]

On 07/05/2017 12:25 AM, Lukas Tribus wrote:
> 
> Am 04.07.2017 um 23:18 schrieb Willy Tarreau:
>> On Tue, Jul 04, 2017 at 10:57:08PM +0200, Lukas Tribus wrote:
>>> The call trace doesn't really look different when I used -dM or 
>>> -DDEBUG_MEMORY.
>>>
>>> I was able to get a different trace by actually connecting to a backend 
>>> however,
>>> (instead of showing an haproxy internal 403 error):
>> (...)
>>
>> Thank you Lukas, let's hope this will help.
>>
>>
> 
> Another bisect (this time with -dM or -DDEBUG_MEMORY), another commit...
> Now it points to 23e9e931 (MINOR: log: Add logurilen tunable).
> 
> 

Hi Lukas,

Indeed this commit introduced a regression.

The commit in attachment should fix the issue.

R,
Emeric
>From 595396561c380aa100e2c1f80299e5fadd18e663 Mon Sep 17 00:00:00 2001
From: Emeric Brun 
Date: Wed, 5 Jul 2017 13:33:16 +0200
Subject: [PATCH] BUG/MAJOR: http: fix buffer overflow on loguri buffer.

The pool used to log the uri was created with a size of 0 because the
configuration and 'tune.http.logurilen' were parsed too earlier.

The fix consist to postpone the pool_create as it is done for
cookie captures.

Regression introduced with 'MINOR: log: Add logurilen tunable'
---
 src/cfgparse.c   | 2 ++
 src/proto_http.c | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/cfgparse.c b/src/cfgparse.c
index 3706bca..600f273 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -7404,6 +7404,8 @@ int check_config_validity()
 	if (!global.tune.requri_len)
 		global.tune.requri_len = REQURI_LEN;
 
+	pool2_requri = create_pool("requri", global.tune.requri_len , MEM_F_SHARED);
+
 	pool2_capture = create_pool("capture", global.tune.cookie_len, MEM_F_SHARED);
 
 	/* allocate pool of resolution per resolvers */
diff --git a/src/proto_http.c b/src/proto_http.c
index 46cb6ff..7141833 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -459,7 +459,6 @@ void init_proto_http()
 
 	/* memory allocations */
 	pool2_http_txn = create_pool("http_txn", sizeof(struct http_txn), MEM_F_SHARED);
-	pool2_requri = create_pool("requri", global.tune.requri_len , MEM_F_SHARED);
 	pool2_uniqueid = create_pool("uniqueid", UNIQUEID_LEN, MEM_F_SHARED);
 }
 
-- 
2.7.4

dynamic return address and lua creates server

[Aleksandar Lazic]

Hi,

I have here a use case where the haproxy is in between a 
couchdb/oracle/dynamic connect backend.

This means that we have the following flow.

 APP Driver -> haproxy TCP mode -> Backend
   |
   -> Backend send a IP to connect -> App Driver should connect to IP
  over haproxy tcp setup
   
Does anyone had such a request in the past?

I think that could be a nice usecase for lua & server-template?

Idea and pseudo code.

###
global
   lua-load parse_and_analyze_couch.lua
   
frontend tcp
... 

backend couch
  tcp-response inspect-delay 1s
  tcp-response content use-service lua.parse_and_analyze_couch
  ...
###

parse_and_analyze_couch.lua

core.register_service("parse_and_analyze_couch", "tcp", function(???)
   --- some lua code
   --- create server with backend from analyzed proto 
end)


What do you think?

-- 
Best Regards
Aleks

Re: 2x filter + keep-alive regressions (1.7 affected)

[Willy Tarreau]

Hi Lukas,

On Fri, Jun 30, 2017 at 10:09:56PM +0200, Lukas Tribus wrote:
> In v1.7.6 (due to 73d071e BUG/MINOR: http: Fix conditions to clean up a txn 
> and to handle the next request):
> "mode http-server-close":
>   log stalled and keep-alive idle close at "timeout client";
>   ignores "timeout http-keep-alive"
> 
> "mode http-keep-alive":
>   log stalled for "timeout server"; keep-alive stalled for
>   "timeout server (then log) + timeout http-keep-alive"

I'm seeing a 5s timeout in case of keepalive, matching the connect
timeout. That seems to indicate that maybe a timeout is not properly
reset at some point and we're still relying on this one. Just a bold
assumption though. I'm a bit more concerned by the http-server-close
case which accumulates connections :-/

Willy

Creating a health check in Lua?

[Gil Bahat]

Hi,

I have some lua code I would like to use as a health check. Documentation
seems to hint this is possible somehow, but I have not been able to either
direct an applet:send to the health check (nor is it clear how it could
expect something in return) or formulate the strings on demand via lua,
which was the other idea I was looking at.

as a not unrelated aside, the 'use-service' directive is missing from the
configuration manual.

Regards,

Gil