[ANNOUNCE] haproxy-1.8.7

2018-04-06 Thread Willy Tarreau
Subject: [ANNOUNCE] haproxy-1.8.7
To: haproxy@formilux.org

Hi,

HAProxy 1.8.7 was released on 2018/04/07. It added 2 new commits
after version 1.8.6.

I know it's very short after 1.8.6, but Thierry found that the cache
issue would still occasionally strike so I really wanted to knock it
down once for all. Finally, it can cause a crash when it cannot save
a response into the cache (eg due to a conflict), because of an
uninitialized field in this specific case, which explains why the
crash is much less frequent.

I preferred to issue a new version before everyone deploys 1.8.6 to
reduce the number of operations in production.

If you have already upgraded to 1.8.6 and are not using the cache,
you don't need to upgrade again.

Please find the usual URLs below :
   Site index   : http://www.haproxy.org/
   Discourse: http://discourse.haproxy.org/
   Sources  : http://www.haproxy.org/download/1.8/src/
   Git repository   : http://git.haproxy.org/git/haproxy-1.8.git/
   Git Web browsing : http://git.haproxy.org/?p=haproxy-1.8.git
   Changelog: http://www.haproxy.org/download/1.8/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/

Have all a nice week-end,
Willy
---
Complete changelog :
Thierry Fournier (1):
  MINOR: servers: Support alphanumeric characters for the server templates 
names

Willy Tarreau (1):
  BUG/MAJOR: cache: always initialize newly created objects

---



Re: Haproxy 1.8.4 crashing workers and increased memory usage

2018-04-06 Thread Willy Tarreau
Hi Robin,

On Fri, Apr 06, 2018 at 03:52:33PM +0200, Robin Geuze wrote:
> Hey Willy,
> 
> I was actually the one that had the hunch to disable compression. I
> suspected that this was the issue because there was a bunch of "abort" calls
> in include/common/hathreads.h" which is used by the compression stuff.
> However I just noticed those aborts are actually only there if DEBUG_THREAD
> is defined which it doesn't seem to be for our build. So basically, I have
> no clue whatsoever why disabling compression fixes the bug.

At least I don't feel alone :-)

> I can see next week if we can make a build with slz instead of zlib (we seem
> to be linked against zlib/libz atm).

Thank you, I appreciate it!

Cheers,
Willy



Re: [PATCH] server-template and allowed chars in the name

2018-04-06 Thread Willy Tarreau
On Mon, Mar 26, 2018 at 03:42:32PM +0200, Frederic Lecaille wrote:
> I have no objection to accept your patch (have a look to the comments).

OK now merged, thanks guys.

Willy



Re: Fix building haproxy 1.8.5 with LibreSSL 2.6.4

2018-04-06 Thread Willy Tarreau
Hi Andy,

On Sat, Mar 31, 2018 at 05:43:55PM +0300, Andy Postnikov wrote:
> I used to rework previous patch from Alpinelinux to build with latest
> stable libressl
> But found no way to run tests with openssl which is primary library as I see
> Is it possible to accept the patch upstream or get review on it?

It is probably correct, though I remember that some parts that you changed
used to be tricky with certain openssl versions, thus I'd like that Emeric
takes a look before merging this. And possibly Manu who uses BoringSSL,
which comes with its own set of incompatibilities :-)

CCing them now.

Thanks,
Willy



Re: Segfault on haproxy 1.7.10 with state file and slowstart

2018-04-06 Thread Willy Tarreau
Hi Raghu,

On Wed, Mar 28, 2018 at 08:30:39PM +, Raghu Udiyar wrote:
> Hi Willy, Baptiste
> 
> It looks like this was the intended behaviour as per the documentation. But
> it looks to be simple to enable the init-addr behaviour for IP addresses as
> well. Can you please review the attached patch, this is against devel.

Baptiste, I missed this one. Could you please take a look ?

Thanks,
Willy



Re: resolvers - resolv.conf fallback

2018-04-06 Thread Willy Tarreau
On Fri, Apr 06, 2018 at 04:50:54PM +0200, Lukas Tribus wrote:
> > Well, sometimes when you're debugging a configuration, it's nice to be
> > able to disable some elements. Same for those manipulating/building
> > configs by assembling elements and iteratively pass them through
> > "haproxy -c". That's exactly the reason why we relaxed a few checks in
> > the past, like accepting a frontend with no bind line or accepting a
> > backend with a "cookie" directive with no cookie on server lines. In
> > fact we could simply emit a warning when a resolvers section has no
> > resolver nor resolv.conf enabled, but at least accept to start.
> 
> Understood; however in this specific case I would argue one would
> remove the "resolver" directive from the server-line(s), instead of
> dropping the nameservers from the global nameserver declaration.

No, because in order to do this, you also have to remove all references
on all "server" lines, which is quite a pain, and error-prone when you
want to reactivate them.

> Maybe a config warning would be a compromise for this case?

Yes, that's what I mentionned above, I'm all in favor of this given that
we can't objectively find a valid use case for an empty resolvers section
in production.

Cheers,
Willy



Re: resolvers - resolv.conf fallback

2018-04-06 Thread Lukas Tribus
Hello Willy,


On 6 April 2018 at 14:14, Willy Tarreau  wrote:
>> The confusion often arises because haproxy accepts a resolver
>> configuration where no resolvers are configured. Maybe we should
>> reject the configuration when a resolver is referred to in the servers
>> lines, but no actual resolvers are configured (AND resolv.conf parsing
>> is not enabled in future).
>
> Well, sometimes when you're debugging a configuration, it's nice to be
> able to disable some elements. Same for those manipulating/building
> configs by assembling elements and iteratively pass them through
> "haproxy -c". That's exactly the reason why we relaxed a few checks in
> the past, like accepting a frontend with no bind line or accepting a
> backend with a "cookie" directive with no cookie on server lines. In
> fact we could simply emit a warning when a resolvers section has no
> resolver nor resolv.conf enabled, but at least accept to start.

Understood; however in this specific case I would argue one would
remove the "resolver" directive from the server-line(s), instead of
dropping the nameservers from the global nameserver declaration.

Maybe a config warning would be a compromise for this case?



Regards,

Lukas



Re: Cookies, load balancing, stick tables.

2018-04-06 Thread Moemen MHEDHBI
Hi Franks,


On 28/03/2018 14:11, Franks Andy (IT Technical Architecture Manager) wrote:
>
> Hi all,
>
>   Hopefully an easy one, but I can’t really find the solution.
>
> We’ve come up with a control system for haproxy, where we manually can
> clear stick table entries from a GUI. We’re also using a cookie to set
> the server in a backend as we’re expecting to deal with clients behind
> a nat device.
>
>  
>
> It’s the customers (just internal IT in another dept) request that
> they should be able to close down a stick table entry and have the
> client not be able to go to that stick-table selected server AT ALL,
> even when presenting a cookie.
>
> It seems to me that HA is designed to allow these cookie selected
> server connections irrespective of the stick table entries, so there
> are two ways to continue to me:
>
>  
>
> 1)  Have the application remove the separate cookie we insert when
> the application gets logged off or times out (timeout happens at 15
> minutes of app idle time).
>
> 2)  We get HAProxy to control the expiry time of the cookie we
> send over, and refresh that expiry each time a transaction happens.
>
> 3)  Live with the imbalance of clients from NATted source ip
> addresses and ditch the cookie insertion.
>
>  
>
> We would all prefer #2, since the devs don’t want to spend time
> redeveloping, and HAProxy can seemingly do just about anything! #3
> would work, but removing entries from the stick table during testing
> or certain maintenance may well remove more than just the intended target.
>
>  
>
> Any ideas?
>
> Thanks
>
> Andy
>
For solution #2 you can use the "maxlife" param of the "cookie"
directive:
http://cbonte.github.io/haproxy-dconv/1.8/configuration.html#4.2-cookie
When the date in maxlife has expired the cookie will be ignored which
means haproxy will choose a different server but there is no clean way
to refresh the expiry date without updating the date in the cookie with
the "replace-header" action. This won't be easy because the date is an
internal haproxy format.

So if you don't want to spend time redeveloping the application you can
still go with solution #1 by removing the persistence cookie in haproxy
using something like (  http-request replace-header Cookie SRV=[^;]*;? '
' if ACL )

-- 
Moemen MHEDHBI



Re: [PATCH] MEDIUM: cli: Add multi-line mode support

2018-04-06 Thread Willy Tarreau
On Fri, Apr 06, 2018 at 03:59:59PM +0200, Aurélien Nephtali wrote:
> I tried to be the less intrusive as
> possible into the CLI parser so I tried to reuse what was already
> done.
> Maybe that's not the best approach but with all the legacy behind I
> REALLY didn't want to break something, even if it lands on a
> development branch :).

It's not a problem to heavily modify what exists as long as it ultimately
works. That always happens anyway. The problem by being too conservative
is that the code gets totally glued together and when someone wants to
rework it, he realizes it's too late, so prefers to add further crap on
top of it and this never ends, until something really breaks and we regret
it.

Obviously, changes for changes are not always welcome and changes for a
good reason are welcome, which is the purpose of the discussions on the
list :-)

> Using a special pattern is a good idea. Let's start with "<<" and
> tweak it later if it collides with what can be found in a payload.

Great.

> > The benefit here is that it's still the parser which knows whether or
> > not a payload follows, regardless of the command, and can simply pass
> > a flag to the command saying "you have a payload to read".
> 
> Yes, in fact that's exactly what I did at first but I really thought
> it was too ambitious and subject to compatibility-breaking (again due
> to my lack of background on the CLI/HAProxy usage).

No problem.

> I realize I wanted to be too generic to be sure I would not break
> anything but I ended doing something too light.

It's fine (and often desirable) to be generic for the long term. There's
no problem being restricted for the short term if you have some visibility
over the next steps to reach the long term. Sometimes you may also want to
do some "disposable" code. Experience taught us that such code remains
almost forever (haproxy itself started like this). But sometimes it helps
getting a first version of something. It often makes it very hard to work
around this however.

> Ok, I'll do that in a way it will be easy to enhance.

Great, thank you.

Thanks for your understanding. Do not hesitate to ping an insist when
you need some inputs that are not coming fast enough, I know it's never
fun to have to redo some work differently after reviews. Hint: don't
worry about updating the doc during design reviews ;-)

Thanks,
Willy



Re: [PATCH] MEDIUM: cli: Add multi-line mode support

2018-04-06 Thread Aurélien Nephtali
Hello Willy,

On Fri, Apr 6, 2018 at 12:02 PM, Willy Tarreau  wrote:
>
> Hi Aurélien,
>
> I finally found some time to review your patch, really sorry for the
> long delay, but bug fixing passes first when they are sensitive :-/

No problem. Adding features over fixing bugs would be quite disturbing. :)

> > Examples:
> > $ echo -e "multiline;set ssl ocsp-response $(base64 ocsp.der)\n" | socat
> > /tmp/sock1 -
> > $ echo -e "multiline ; show\nstat\n" | socat /tmp/sock1 -
>
> I must confess I feel a bit embarrassed by this. I think it isn't a good
> idea for the long term to mix arguments and payload. And the example you
> put below in the doc scares me a little bit :
>
> > set
> + server TEST/A
> + state
> + ready
> +
>
> It's a bit complicated for me to express why I'm not thrilled, I just
> feel it's not right, without having a strong argument. Among the things
> I'm thinking about is the fact that the command line arguments are
> delimited by spaces, so above, "server" and "TEST/A" are two different
> arguments. Then I don't see how we can properly process a *real* payload
> containing spaces, like a PEM formated certificate, without starting to
> run into dirty hacks.

Well, in that case the dirty hack would be what I did in
ocsp-response: treat everything from args[3] as being the payload,
with one argument = one line.
I admit that's quite hackish; I tried to be the less intrusive as
possible into the CLI parser so I tried to reuse what was already
done.
Maybe that's not the best approach but with all the legacy behind I
REALLY didn't want to break something, even if it lands on a
development branch :).

>
> Also, I'm pretty sure that most, if not all, of the commands consuming a
> payload will know how to process it based on the command line arguments.
> So making the payload appear as an argument (or mixed with them) will
> make it harder to conditionally process them.

Yes, at first I designed a solution that involved parsers cooperation
but there were ambiguous cases, especially when the command takes
optional arguments.
It's totally doable but at that time I thought that would make too big
a change that could cause issues or break use cases I didn't think
about.

>
> Another possibly more problematic point is that if you pass a payload as
> an argument, it will be processed at once, thus will always be limited
> to a buffer size, so we won't be able to use this to update maps or ACLs
> for example.

That is very true and in fact I had to increase MAX_STATS_ARGS to an
arbitrary value in my SSL on-the-fly patch.

> I agree with the point you made about keeping the parser independant of
> the commands so that it doesn't have to know which one takes a payload
> and which one not. But here I fear that we'll end up causing the opposite
> problem by forcing all commands to run into tricks to distinguish their
> args and the payload. For example, let's say we use it to add a map
> between HTTP status codes and reasons :
>
>   > add map http_status
>   + 200 OK
>   + 206 Partial Content
>   + 302 Found
>   + 303 See Other
>   + 304 Not Modified
>   ...
>
> If it's cut along words, you can as well enter it this way :
>
>   > add map
>   + http_status
>   + 200 OK
>   + 206 Partial Content
>   + 302 Found
>   + 303 See Other
>   + 304 Not Modified
>   ...
>
> Or even this way :
>
>   > add map
>   + http_status 200 OK
>   + 206 Partial Content
>   + 302 Found
>   + 303 See Other
>   + 304 Not Modified
>   ...
>
> Or this way as well :
>
>   > add map
>   + http_status 200 OK 206 Partial Content 302 Found 303 See Other 304 Not 
> Modified
>   ...
>
> You probably see the problem : there's no way to cut the arguments anymore.

Indeed, I didn't think about this use case.

> By having arguments and payload separate, both can continue to use their
> respective, non-colliding syntaxes, so that the command's arguments are
> delimited by spaces and only spaces, and are all present once the command
> is started, and that the payload's syntax solely depends on the command
> and is a byte stream ending on an empty line (ie \n\n). This way commands
> consuming multiple entries at once (like maps, acls, crtlists) can easily
> keep the on-disk file format and consider that one line equals one entry,
> and other commands taking more complex formats (like certificates) can use
> a different assumption, including the option to pass the whole stream to
> an underlying framework (typically openssl).
>
> Given that there are very few commands consuming a payload, I think we
> should not use a sticky mode. The multiline prefix should only apply to
> the next command so that there is no need to do this :
>
>   > multiline;add map http_status
>   + 200 OK
>   + 206 Partial Content
>   + 302 Found
>   + 303 See Other
>   + 304 Not Modified
>   +
>   > multiline
>
> It could thus be shortened (eg: "ml") or even be a different character,
> though I don't really see which one.
>
> Or probably much 

Re: Haproxy 1.8.4 crashing workers and increased memory usage

2018-04-06 Thread Robin Geuze

Hey Willy,

I was actually the one that had the hunch to disable compression. I 
suspected that this was the issue because there was a bunch of "abort" 
calls in include/common/hathreads.h" which is used by the compression 
stuff. However I just noticed those aborts are actually only there if 
DEBUG_THREAD is defined which it doesn't seem to be for our build. So 
basically, I have no clue whatsoever why disabling compression fixes the 
bug.


I can see next week if we can make a build with slz instead of zlib (we 
seem to be linked against zlib/libz atm).


Regards,

Robin Geuze


On 4/6/2018 14:18, Willy Tarreau wrote:

Hi Frank,

On Fri, Apr 06, 2018 at 10:53:36AM +, Frank Schreuder wrote:

We tested haproxy 1.8.6 with compression enabled today, within the first few 
hours it already went wrong:
[ALERT] 095/120526 (12989) : Current worker 5241 exited with code 134

OK thanks, and sorry for that.


Our other balancer running haproxy 1.8.5 with compression disabled is still
running fine after 2 days with the same workload.
So there seems to be a locking issue when compression is enabled.

Well, an issue with compression, but I'm really not seeing what makes
you speak about locking since :
   - you don't seem to have threads enabled
   - locking issues generally cause deadlocks, not aborts

The other problem is that we noticed already that there are very few
abort() calls in haproxy and none of them in this area. So it's very
possible that it comes from another layer detecting an issue provoked
by compression. Typically the libc's malloc/free can stop the program
using abort() if they detect a corruption.

It would really help to know where this abort() happens, at least to
get a backtrace.

By the way, area you using zlib or slz ? zlib uses a tricky allocator.
I checked it again yesterday and it was made thread safe. But we couldn't
rule out an issue there. slz doesn't need memory however. If you're on
zlib, switching to slz could also indicate if the problem is related to
these memory allocations or not.

Thanks,
Willy






Re: format warning cleanup (patch attached)

2018-04-06 Thread Илья Шипицин
well, I was not sure whether to split into several files or not.

ok, will split next time :)

2018-04-06 17:32 GMT+05:00 Willy Tarreau :

> Hi Ilya,
>
> On Fri, Apr 06, 2018 at 05:08:20PM +0500,  ??? wrote:
> > Hello,
> >
> > please review the attached patch
>
> thank you, but in the future, you should definitely split that into
> several patches when it touches different areas since different people
> will be involved. For halog and hpack I think it's OK. For the other
> ones I'll let their respective authors and maintainers check.
>
> Thanks!
> Willy
>


Re: format warning cleanup (patch attached)

2018-04-06 Thread Willy Tarreau
Hi Ilya,

On Fri, Apr 06, 2018 at 05:08:20PM +0500,  ??? wrote:
> Hello,
> 
> please review the attached patch

thank you, but in the future, you should definitely split that into
several patches when it touches different areas since different people
will be involved. For halog and hpack I think it's OK. For the other
ones I'll let their respective authors and maintainers check.

Thanks!
Willy



Re: Haproxy 1.8.4 crashing workers and increased memory usage

2018-04-06 Thread Willy Tarreau
Hi Frank,

On Fri, Apr 06, 2018 at 10:53:36AM +, Frank Schreuder wrote:
> We tested haproxy 1.8.6 with compression enabled today, within the first few 
> hours it already went wrong:
> [ALERT] 095/120526 (12989) : Current worker 5241 exited with code 134

OK thanks, and sorry for that.

> Our other balancer running haproxy 1.8.5 with compression disabled is still
> running fine after 2 days with the same workload.
> So there seems to be a locking issue when compression is enabled.

Well, an issue with compression, but I'm really not seeing what makes
you speak about locking since :
  - you don't seem to have threads enabled
  - locking issues generally cause deadlocks, not aborts

The other problem is that we noticed already that there are very few
abort() calls in haproxy and none of them in this area. So it's very
possible that it comes from another layer detecting an issue provoked
by compression. Typically the libc's malloc/free can stop the program
using abort() if they detect a corruption.

It would really help to know where this abort() happens, at least to
get a backtrace.

By the way, area you using zlib or slz ? zlib uses a tricky allocator.
I checked it again yesterday and it was made thread safe. But we couldn't
rule out an issue there. slz doesn't need memory however. If you're on
zlib, switching to slz could also indicate if the problem is related to
these memory allocations or not.

Thanks,
Willy



Re: resolvers - resolv.conf fallback

2018-04-06 Thread Willy Tarreau
Hi Lukas,

On Fri, Apr 06, 2018 at 12:33:14PM +0200, Lukas Tribus wrote:
> On 6 April 2018 at 11:14, Willy Tarreau  wrote:
> >> I don't think we need a new config know.
> >
> > Just thinking, is the goal *not to have to* configure "resolve" on
> > server lines in this case, or to avoid having to pre-configure the
> > resolvers themselves when they're the same as the system's ?
> 
> The latter is the goal.

OK thanks for confirming.

> > If the former, that would mean always enabling DNS resolving at runtime
> > which doesn't sound like a good idea at all to me. If the latter, then
> > why not have a special directive in the resolvers section to indicate
> > that it should use resolv.conf instead ? That could avoid some surprizes
> > when you simply comment your all your resolvers and that suddenly the
> > behaviour changes. I'd even say that this directive could serve to
> > populate the resolvers section from resolv.conf (thus possibly several
> > resolvers) which will ensure the exclusivity between the two mechanisms.
> 
> Yes, that's a good point. In fact, I don't see why the fallback has to
> be implicit.
> 
> The confusion often arises because haproxy accepts a resolver
> configuration where no resolvers are configured. Maybe we should
> reject the configuration when a resolver is referred to in the servers
> lines, but no actual resolvers are configured (AND resolv.conf parsing
> is not enabled in future).

Well, sometimes when you're debugging a configuration, it's nice to be
able to disable some elements. Same for those manipulating/building
configs by assembling elements and iteratively pass them through
"haproxy -c". That's exactly the reason why we relaxed a few checks in
the past, like accepting a frontend with no bind line or accepting a
backend with a "cookie" directive with no cookie on server lines. In
fact we could simply emit a warning when a resolvers section has no
resolver nor resolv.conf enabled, but at least accept to start.

Just my two cents,

Cheers,
Willy



format warning cleanup (patch attached)

2018-04-06 Thread Илья Шипицин
Hello,

please review the attached patch

Ilya Shipitsin
From a7f2e5a064c40a5842decc7e053a6e4a4291df33 Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Fri, 6 Apr 2018 17:06:10 +0500
Subject: [PATCH] MINOR: format warnings cleanup

issue detected by cppcheck

[contrib/base64/base64rev-gen.c:57]: (warning) %u in format string (no. 1) requires 'unsigned int' but the argument type is 'signed int'.
[contrib/halog/fgets2.c:259]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1106]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1106]: (warning) %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1106]: (warning) %d in format string (no. 3) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1106]: (warning) %d in format string (no. 4) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1106]: (warning) %d in format string (no. 6) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1106]: (warning) %d in format string (no. 7) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1165]: (warning) %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1194]: (warning) %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1217]: (warning) %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1217]: (warning) %d in format string (no. 3) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1217]: (warning) %d in format string (no. 4) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1217]: (warning) %d in format string (no. 5) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1217]: (warning) %d in format string (no. 6) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1217]: (warning) %d in format string (no. 7) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1217]: (warning) %d in format string (no. 9) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1234]: (warning) %d in format string (no. 3) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1290]: (warning) %d in format string (no. 1) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1290]: (warning) %d in format string (no. 2) requires 'int' but the argument type is 'unsigned int'.
[contrib/halog/halog.c:1290]: (warning) %Ld in format string (no. 3) requires 'long long' but the argument type is 'unsigned long long'.
[contrib/halog/halog.c:1290]: (warning) %Ld in format string (no. 4) requires 'long long' but the argument type is 'unsigned long long'.
[contrib/halog/halog.c:1290]: (warning) %Ld in format string (no. 5) requires 'long long' but the argument type is 'unsigned long long'.
[contrib/halog/halog.c:1290]: (warning) %Ld in format string (no. 7) requires 'long long' but the argument type is 'unsigned long long'.
[contrib/halog/halog.c:1290]: (warning) %Ld in format string (no. 8) requires 'long long' but the argument type is 'unsigned long long'.
[contrib/mod_defender/spoa.c:409]: (warning) %u in format string (no. 5) requires 'unsigned int' but the argument type is 'signed int'.
[contrib/mod_defender/spoa.c:869]: (warning) %u in format string (no. 5) requires 'unsigned int' but the argument type is 'signed int'.
[contrib/mod_defender/spoa.c:1344]: (warning) %u in format string (no. 4) requires 'unsigned int' but the argument type is 'signed int'.
[contrib/mod_defender/spoa.c:1693]: (warning) %u in format string (no. 2) requires 'unsigned int' but the argument type is 'signed int'.
[contrib/modsecurity/spoa.c:414]: (warning) %u in format string (no. 5) requires 'unsigned int' but the argument type is 'signed int'.
[contrib/modsecurity/spoa.c:874]: (warning) %u in format string (no. 5) requires 'unsigned int' but the argument type is 'signed int'.
[contrib/modsecurity/spoa.c:1373]: (warning) %u in format string (no. 4) requires 'unsigned int' but the argument type is 'signed int'.
[contrib/modsecurity/spoa.c:1722]: (warning) %u in format string (no. 2) requires 'unsigned int' but the argument type is 'signed int'.
[contrib/spoa_example/spoa.c:463]: (warning) %u in format string (no. 5) requires 'unsigned int' but the argument type is 'signed int'.
[contrib/spoa_example/spoa.c:923]: (warning) %u in format string (no. 5) requires 'unsigned int' but the argument type is 'signed int'.
[contrib/spoa_example/spoa.c:1366]: (warning) %u in format string (no. 4) requires 'unsigned int' but the argument type is 'signed int'.
[contrib/spoa_example/spoa.c:1715]: 

Re: Haproxy 1.8.4 crashing workers and increased memory usage

2018-04-06 Thread Frank Schreuder
Hi Willy,

>>  There are very few abort() calls in the code :
>>    - some in the thread debugging code to detect recursive locks ;
>>    - one in the cache applet which triggers on an impossible case very
>>      likely resulting from cache corruption (hence a bug)
>>    - a few inside the Lua library
>>    - a few in the HPACK decompressor, detecting a few possible bugs there
>> 
>> After playing around with some config changes we managed to not have haproxy
>> throw the "worker  exited with code 134" error for at least a day. Which
>> is a long time as before we had this error at least 5 times a day...
>
> Great!
>
>> The line we removed from our config to get this result was:
>> compression algo gzip
>
> Hmmm interesting.
>
>> Could it be a locking issue in the compression code? I'm going to run a few
>> more days without compression enabled, but for now this looks promising!
>
> In fact, the locking is totally disabled when not using compression, so
> it cannot be an option. Also, most of the recently fixed bugs may only
> be triggered with H2 or threads, none of which you're using. I rechecked
> the compression code to try to spot anything obvious, but nothing popped
> out :-/
>
> All I can strongly recommend if you retry with compression enabled is to
> do it with latest 1.8 release. I'm currently checking that I didn't miss
> anything to issue 1.8.6 hopefully today. If it still dies, this will at
> least rule out the possible side effects of a few of the bugs we've fixed
> since, all of which were really tricky.

We tested haproxy 1.8.6 with compression enabled today, within the first few 
hours it already went wrong:
[ALERT] 095/120526 (12989) : Current worker 5241 exited with code 134

Our other balancer running haproxy 1.8.5 with compression disabled is still 
running fine after 2 days with the same workload.
So there seems to be a locking issue when compression is enabled.

Thanks,
Frank


Re: resolvers - resolv.conf fallback

2018-04-06 Thread Lukas Tribus
Hi Willy,



On 6 April 2018 at 11:14, Willy Tarreau  wrote:
>> I don't think we need a new config know.
>
> Just thinking, is the goal *not to have to* configure "resolve" on
> server lines in this case, or to avoid having to pre-configure the
> resolvers themselves when they're the same as the system's ?

The latter is the goal.



> If the former, that would mean always enabling DNS resolving at runtime
> which doesn't sound like a good idea at all to me. If the latter, then
> why not have a special directive in the resolvers section to indicate
> that it should use resolv.conf instead ? That could avoid some surprizes
> when you simply comment your all your resolvers and that suddenly the
> behaviour changes. I'd even say that this directive could serve to
> populate the resolvers section from resolv.conf (thus possibly several
> resolvers) which will ensure the exclusivity between the two mechanisms.

Yes, that's a good point. In fact, I don't see why the fallback has to
be implicit.

The confusion often arises because haproxy accepts a resolver
configuration where no resolvers are configured. Maybe we should
reject the configuration when a resolver is referred to in the servers
lines, but no actual resolvers are configured (AND resolv.conf parsing
is not enabled in future).



Lukas



Re: [PATCH] MEDIUM: cli: Add multi-line mode support

2018-04-06 Thread Willy Tarreau
Hi Aurélien,

I finally found some time to review your patch, really sorry for the
long delay, but bug fixing passes first when they are sensitive :-/

On Sat, Mar 24, 2018 at 11:16:03PM +0100, Aurélien Nephtali wrote:
> From 53356f83dab6483512d2db1fae87abd24beca4c0 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Aur=C3=A9lien=20Nephtali?= 
> Date: Thu, 15 Mar 2018 15:44:19 +0100
> Subject: [PATCH] MEDIUM: cli: Add multi-line mode support
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Add a new command to toggle multi-line mode: multiline.
> Once activated, all commands must end with an empty line.
> 
> The multi-line mode enables line feeds to be used anywhere in the
> request as long it stays syntactically correct once reassembled.
> This mode should be particularly useful for scripts or when inputing
> data that can contain line feeds.
> 
> Examples:
> $ echo -e "multiline;set ssl ocsp-response $(base64 ocsp.der)\n" | socat
> /tmp/sock1 -
> $ echo -e "multiline ; show\nstat\n" | socat /tmp/sock1 -

I must confess I feel a bit embarrassed by this. I think it isn't a good
idea for the long term to mix arguments and payload. And the example you
put below in the doc scares me a little bit :

> set
+ server TEST/A
+ state
+ ready
+

It's a bit complicated for me to express why I'm not thrilled, I just
feel it's not right, without having a strong argument. Among the things
I'm thinking about is the fact that the command line arguments are
delimited by spaces, so above, "server" and "TEST/A" are two different
arguments. Then I don't see how we can properly process a *real* payload
containing spaces, like a PEM formated certificate, without starting to
run into dirty hacks.

Also, I'm pretty sure that most, if not all, of the commands consuming a
payload will know how to process it based on the command line arguments.
So making the payload appear as an argument (or mixed with them) will
make it harder to conditionally process them.

Another possibly more problematic point is that if you pass a payload as
an argument, it will be processed at once, thus will always be limited
to a buffer size, so we won't be able to use this to update maps or ACLs
for example.

I agree with the point you made about keeping the parser independant of
the commands so that it doesn't have to know which one takes a payload
and which one not. But here I fear that we'll end up causing the opposite
problem by forcing all commands to run into tricks to distinguish their
args and the payload. For example, let's say we use it to add a map
between HTTP status codes and reasons :

  > add map http_status
  + 200 OK
  + 206 Partial Content
  + 302 Found
  + 303 See Other
  + 304 Not Modified
  ...

If it's cut along words, you can as well enter it this way :

  > add map
  + http_status
  + 200 OK
  + 206 Partial Content
  + 302 Found
  + 303 See Other
  + 304 Not Modified
  ...

Or even this way :

  > add map
  + http_status 200 OK
  + 206 Partial Content
  + 302 Found
  + 303 See Other
  + 304 Not Modified
  ...

Or this way as well :

  > add map
  + http_status 200 OK 206 Partial Content 302 Found 303 See Other 304 Not 
Modified
  ...

You probably see the problem : there's no way to cut the arguments anymore.
By having arguments and payload separate, both can continue to use their
respective, non-colliding syntaxes, so that the command's arguments are
delimited by spaces and only spaces, and are all present once the command
is started, and that the payload's syntax solely depends on the command
and is a byte stream ending on an empty line (ie \n\n). This way commands
consuming multiple entries at once (like maps, acls, crtlists) can easily
keep the on-disk file format and consider that one line equals one entry,
and other commands taking more complex formats (like certificates) can use
a different assumption, including the option to pass the whole stream to
an underlying framework (typically openssl).

Given that there are very few commands consuming a payload, I think we
should not use a sticky mode. The multiline prefix should only apply to
the next command so that there is no need to do this :

  > multiline;add map http_status
  + 200 OK
  + 206 Partial Content
  + 302 Found
  + 303 See Other
  + 304 Not Modified
  +
  > multiline

It could thus be shortened (eg: "ml") or even be a different character,
though I don't really see which one.

Or probably much better, we could end a command with a character indicating
that a payload follows, a bit a-la "<< EOF" we're used to see in shell.
Maybe just using "<<" at the end of a line could be sufficient to indicate
that a payload follows and that it must be ended by an empty line. Eg:

  > add map http_status <<
  + 200 OK
  + 206 Partial Content
  + 302 Found
  + 303 See Other
  + 304 Not Modified
  +
  >

The benefit here is that it's still the parser which knows whether or
not a payload 

Why is it that the summary line of the backend at the bottom says limit 1?

2018-04-06 Thread Pieter Vogelaar
Hi,

I use a configuration with global maxconn 8000 and defaults maxconn 8000.

This affects the frontends. On the stats page I see the servers of backend have 
a “-” in the limit field. Which means there is no limit.
But why is it that the summary line of the backend at the bottom says limit 1?

Best regards,
Pieter Vogelaar

Freelance DevOps engineer / PHP developer

pie...@pietervogelaar.nl
pietervogelaar.nl


Re: resolvers - resolv.conf fallback

2018-04-06 Thread Willy Tarreau
Hi guys,

On Wed, Apr 04, 2018 at 12:32:41PM +0200, Lukas Tribus wrote:
> Hello Baptiste,
> 
> 
> > - (for Lukas) what do you think is better, a configuration option to trigger
> > parsing of resolv.conf or as proposed, if no nameserver are found, we use
> > resolv.conf as a failback?
> 
> 
> I don't think we need a config knob for this; currently we don't do
> anything when no nameservers are configured; that behavior combined
> with the by-default enabled libc resolver at startup can cause some
> confusion.
> 
> Transitioning to a resolv.conf fallback is the correct thing to do here imho.
> 
> 
> Just:
> - only fallback if no resolvers are configured in haproxy configuration
> - don't fallback if configured resolvers are unresponsive
> - update the documentation at the same time
> 
> 
> I don't think we need a new config know.

Just thinking, is the goal *not to have to* configure "resolve" on
server lines in this case, or to avoid having to pre-configure the
resolvers themselves when they're the same as the system's ?

If the former, that would mean always enabling DNS resolving at runtime
which doesn't sound like a good idea at all to me. If the latter, then
why not have a special directive in the resolvers section to indicate
that it should use resolv.conf instead ? That could avoid some surprizes
when you simply comment your all your resolvers and that suddenly the
behaviour changes. I'd even say that this directive could serve to
populate the resolvers section from resolv.conf (thus possibly several
resolvers) which will ensure the exclusivity between the two mechanisms.

Cheers,
Willy



Re: DNS resolver and mixed case responses

2018-04-06 Thread Dennis Jacobfeuerborn
On 04.04.2018 16:30, Tim Düsterhus wrote:
> Dale,
> 
> Am 03.04.2018 um 16:17 schrieb Dale Smith:
>> I'm trying to understand what system is at fault here; the DNS server for
>> not responding with the same case as the query, or HAProxy which
>> should be
>> performing a case insensitive match.
> 
> This is left unspecified in the standards, but on the other hand there
> is this Internet Draft:
> https://tools.ietf.org/html/draft-vixie-dnsext-dns0x20-00 which wants to
> mandate case preserval to make DNS spoofing harder by introducing more
> entropy in the DNS request.
> 
> I recommend to fix your internal DNS server, because case preserving
> behaviour seems to be somewhat expected according to a quick Google search.

There is this:

Domain Name System (DNS) Case Insensitivity Clarification:
https://tools.ietf.org/html/rfc4343#section-3.1

In section 3 it says this:

3.  Name Lookup, Label Types, and CLASS

   According to the original DNS design decision, comparisons on name
   lookup for DNS queries should be case insensitive [STD13].  That is
   to say, a lookup string octet with a value in the inclusive range
   from 0x41 to 0x5A, the uppercase ASCII letters, MUST match the
   identical value and also match the corresponding value in the
   inclusive range from 0x61 to 0x7A, the lowercase ASCII letters.  A
   lookup string octet with a lowercase ASCII letter value MUST
   similarly match the identical value and also match the corresponding
   value in the uppercase ASCII letter range.

   (Historical note: The terms "uppercase" and "lowercase" were invented
   after movable type.  The terms originally referred to the two font
   trays for storing, in partitioned areas, the different physical type
   elements.  Before movable type, the nearest equivalent terms were
   "majuscule" and "minuscule".)

This reads to me like HAProxy should match characters in the ranges 0x41
to 0x5A and 0x61 to 0x7A insensitively as long as the label type is ASCII.

Section 4.1 "DNS Output Case Preservation" mentions this: "No "case
conversion" or "case folding" is done during such output operations,
thus "preserving" case."

Regrads,
  Dennis



Re: No enabled listener found and reloads triggered an inconsistent state.

2018-04-06 Thread Willy Tarreau
Hi Pierre,

On Wed, Apr 04, 2018 at 08:12:50AM +, Pierre Cheynier wrote:
> Hi there,
> 
> We had an issue recently, using 1.8.5. For some reason we ended up entering
> in the "No enabled listener found" state (I guess the config file was
> incomplete, being written at that time, something like that).
> 
> Here are the logs:
> 
> Apr 03 17:51:49 hostname systemd[1]: Reloaded HAProxy Load Balancer.
> Apr 03 17:54:22 hostname haproxy[27090]: [WARNING] 092/175149 (27090) : 
> Reexecuting Master process
> Apr 03 17:54:22 hostname haproxy[27090]: [ALERT] 092/175422 (27090) : 
> [/usr/sbin/haproxy.main()] No enabled listener found (check for 'b
> Apr 03 17:54:22 hostname haproxy[27090]: [WARNING] 092/175422 (27090) : 
> Reexecuting Master process in waitpid mode
> Apr 03 17:54:22 hostname haproxy[27090]: [WARNING] 092/175422 (27090) : 
> Reexecuting Master process
> Apr 03 17:54:22 hostname systemd[1]: Reloaded HAProxy Load Balancer.
> 
> Subsequent reloads were OK.
> 
> The issue here is that it lets HAProxy in an inconsistent state: connections
> are almost always in timeout, including the one on the Unix socket, the
> healtcheck endpoint set using monitor-uri, etc., until a real stop/start is
> done (meaning the parent/worker is restarted itself).
> Doing reloads doesn't fix here.

At the moment I have really no idea about this, it should be handled
like any config parsing error in theory, since after all, it's just
that. The master-worker mechanism is a bit complex as it takes care of
many things, and I'm not much at ease with it yet. William's coming
back next week, let's check with him if he has an idea about what can
happen.

Cheers,
Willy