Re: show errors from stats socket

2020-09-29 Thread Willy Tarreau
Hi Elias,

On Mon, Sep 28, 2020 at 12:51:18PM +0200, Elias Abacioglu wrote:
> Hi
> 
> I'm trying to get details about some errors in one of my backends.
> Looking at the stats, there are a bunch of errors per server.
> However, when I try to get details about the errors via the stats socket.
> 
> # echo "show errors -1" | socat stdio /var/lib/haproxy/stats.sock
> Total events captured on [28/Sep/2020:12:44:29.832] : 0
> 
> I get nothing.
> Anyone with a clue on what I might be doing wrong?

That's already a good thing! Errors captured there are unparsable messages
(requests or responses) indicating either attacks from a client, or a
severe bug on a server. What type of errors are you noticing ? It's
possible to have plenty of other types of errors, including closed
connections, resets, timeouts etc, which will not appear there.

Willy



Re: [PATCH 2/2] DOC: crt: advise to move away from cert bundle

2020-09-29 Thread Willy Tarreau
On Mon, Sep 28, 2020 at 02:31:18PM +0200, William Lallemand wrote:
> > diff --git a/doc/management.txt b/doc/management.txt
> > index adbad95d3..42e8ddbca 100644
> > --- a/doc/management.txt
> > +++ b/doc/management.txt
> > @@ -1725,6 +1725,10 @@ new ssl cert 
> >Create a new empty SSL certificate store to be filled with a certificate 
> > and
> >added to a directory or a crt-list. This command should be used in
> >combination with "set ssl cert" and "add ssl crt-list".
> > +  Note that bundle certificates are not supported; it is recommended to use
> > +  `ssl-load-extra-file none` in global config to avoid loading 
> > certificates as
> > +  bundle and then mixing with single certificates in the runtime API. This 
> > will
> > +  avoid confusion, especailly when it comes to the `commit` command.
> >  
> >  prompt
> >Toggle the prompt at the beginning of the line and enter or leave 
> > interactive
> 
> 
> 
> I don't think that's the good approach for 2.3, I replied on the github
> issue: https://github.com/haproxy/haproxy/issues/872

I already picked that doc patch the day before, should we revert it then,
or just part of it maybe ?

Willy



Re: [PATCH 2/2] MINOR: ssl: Add error if a crt-list might be truncated

2020-09-29 Thread Willy Tarreau
Hi Tim,

On Mon, Sep 28, 2020 at 07:02:15PM +0200, Tim Duesterhus wrote:
> see https://github.com/haproxy/haproxy/issues/860#issuecomment-693422936
> see 0354b658f061d00d5ab4b728d7deeff2c8f1503a
> 
> This should be backported as a warning to 2.2.

As a rule of thumb, it would be good to keep in mind to always provide a
bit of context in commit messages so that people working on backports can
judge of the relevance of a patch when reading the "git log" output without
having to start a browser, switch context, or even depend on github's
accessibility.

Links are perfect for reference and to provide the full details but should
not constitute the essence of the description, which should still be there,
even if it can be synthetic. Plus imagine if two years ahead github decides
to start purge old issues, we'd lose all history!

Thanks!
Willy



Re: [PATCH] BUILD: makefile: Update feature flags for FreeBSD

2020-09-29 Thread Willy Tarreau
On Tue, Sep 15, 2020 at 03:10:04AM -0400, Brad Smith wrote:
> This updates the feature flags for FreeBSD.
> 
> FreeBSD 10 adds support for accept4().
> 
> Enable getaddrinfo().
> 
> >From the FreeBSD port / package.

Applied, thanks Brad! And sorry for missing it the first time, it simply
went out of my scrolling area :-)

Willy



Re: [PATCH 1/2] DOC: agent-check: fix typo in "fail" word expected reply

2020-09-27 Thread Willy Tarreau
On Sat, Sep 26, 2020 at 01:35:51PM +0200, William Dauchy wrote:
> `tcpcheck_agent_expect_reply` expects "fail" not "failed"
(...)

both patches applied, thank you William.

willy



Re: [PATCH] BUILD: makefile: Update feature flags for OpenBSD

2020-09-27 Thread Willy Tarreau
On Sat, Sep 26, 2020 at 11:05:25PM -0400, Brad Smith wrote:
> Update the OpenBSD target features being enabled.
> 
> I updated the list of features after noticing
> "BUILD: makefile: disable threads by default on OpenBSD".
> 
> The Makefile utilizing gcc(1) by default resulted in utilizing
> our legacy and obsolete compiler (GCC 4.2.1) instead of the
> proper system compiler (Clang), which does support TLS. With
> "BUILD: makefile: change default value of CC from gcc to cc"
> that is resolved.

Oh, great! Now applied, thank you!
Willy



Re: [PATCH] REGTESTS: replace "which" with POSIX "command"

2020-09-26 Thread Willy Tarreau
On Sat, Sep 26, 2020 at 01:21:22PM +0500,  ??? wrote:
> Fedora docker also comes without "find" utility
> 
> ## Gathering tests to run ##
> ./scripts/run-regtests.sh: line 131: find: command not found
> ## Starting vtest ##

Isn't it worth filing a bug report to whoever maintains that image ?
Because quite frankly, a UNIX-like system without the ubiquitous
"find" command is kind of pointless. It cannot even be a matter of
place at this point, since even busybox provides a working one if
they need.

Willy



Re: [ANNOUNCE] haproxy-2.3-dev5

2020-09-26 Thread Willy Tarreau
On Fri, Sep 25, 2020 at 09:55:46PM +0200, Christopher Faulet wrote:
> Hi,
> 
> HAProxy 2.3-dev5 was released on 2020/09/25. It added 104 new commits
> after version 2.3-dev4.

Thanks for doing this one, Christopher, you definitely saved me quite
some time, allowing me to progress further on the listeners :-)

(...)
> All this description is probably a bit cryptic and it does not do Willy's
> work justice. It was amazingly hard and painful to unmangle. But, it was a
> mandatory step to add the QUIC support. The next changes to come in this
> area are about the way listeners, receivers and proxies are started,
> stopped, paused or resumed.

I'm starting to see the end of the tunnel there (well just a little bit of
light), as well as some stuff that will still cause some trouble but
overall we're soon about to be able to declare a QUIC listener, with
a stream protocol for the upper layers with datagram for the lower ones.
This will also remove a lot of the ugly tricks that were needed for the
log forwarder (such as the fake "bind" lines that silently ignore unknown
keywords).

Among the upcoming changes that I mentioned a while ago that I'd still like
to see done before 2.3, there was:
  - setting log-send-hostname by default
  - enabling tune.fd.edge-triggered by default
  - changing the way "http-reuse safe" works for backend H2 connections
to avoid mixing two clients over the same connection and avoid head
of line blocking

We're already at end of September, we must really finish quickly what's
still in progress and think about stabilizing. I know we've been late on
2.2 but that didn't remove development time on 2.3 since all that was
done before 2.2 was released is still there :-) So let's say that what
is not merged in two weeks by 9th october will go to -next so that we
still have a few weeks left to fix bugs, test and document.

In addition I'd like that for 2.4 we further shorten the merge window,
that's still far too long, as we spend most of the bug-fixing time after
the release instead of before, which is counter-productive. So we'll
need to have pending stuff in -next anyway.

Cheers,
Willy



Re: [PATCH] REGTESTS: replace "which" with POSIX "command"

2020-09-26 Thread Willy Tarreau
Hi Ilya,

On Sat, Sep 26, 2020 at 11:58:48AM +0500,  ??? wrote:
> Hello,
> 
> I've found that "socat" was not properly detected under Fedora docker image.

Thanks, now applied.

> (maybe we should introduce "set -e" as well)

This could be an idea, indeed. But overall any error will be spotted one
way or another.

Willy



Re: Random with Two Choices Load Balancing Algorithm

2020-09-25 Thread Willy Tarreau
Hi Norman,

On Fri, Sep 25, 2020 at 03:58:54PM +, Branitsky, Norman wrote:
> Wondering if this was ever implemented?

Yes it was merged into 2.0, it's used by default when you select
random since it's always slightly better than a naive random, and
you can even configure it to set the number of draws you want.

I also made some experiments to measure the gains :

https://www.haproxy.com/blog/power-of-two-load-balancing/

TL;DR: it's only better than the default random, but isn't as good as
the real leastconn like the one we have which is combined with round
robin hance limits collisions between LB nodes in a distributed system.

Willy



Re: [PATCH] BUILD: trace: include tools.h

2020-09-25 Thread Willy Tarreau
Hi Miroslav,

On Thu, Sep 24, 2020 at 09:43:56AM +0200, Miroslav Zagorac wrote:
> Hello,
> 
> haproxy cannot be compiled on debian 9.13 if the TRACE option is used.

Thanks, applied!

Note below:
> ---
> src/calltrace.o: In function `make_line':
> .../src/calltrace.c:204: undefined reference to `rdtsc'
> src/calltrace.o: In function `calltrace':
> .../src/calltrace.c:277: undefined reference to `rdtsc'
> collect2: error: ld returned 1 exit status
> Makefile:866: recipe for target 'haproxy' failed
> ---

Git uses the "---" marker as an end of commit message, to put your
comments to committers below. So when I applied your patch, it dropped
this part. I've reintroduced it, but I thought you would like to know
in order not to get caught by accident on another patch if you're used
to use this :-)

Oh and by the way it also drops lines starting with "#" which tends
to annoy me as I get caught once in a while when referencing a github
issue at the beginning of a line!

Cheers,
Willy



Re: source algorithm - question.

2020-09-25 Thread Willy Tarreau
On Fri, Sep 25, 2020 at 03:57:58PM +0200, Lukasz Tasz wrote:
> first is opposite to leastconn - first keep load at the beginning,
> leastconn same everywhere.

Indeed.

> but why source is limiting balancing only to one machine?

Because in almost every case people want to use it to maintain affinity.
For example you don't want the control and data connection of an FTP
client to go to different servers.

> would it be possible to remove this limitation?

It's in part done by hash-balance-factor which forces the algorithm
to pick a different server when the one initially choosen has too high
a load.

However removing completely a hashed server from the pool every time it
reaches its limits would have serious consequences in terms of CPU usage.
At high loads a server should constantly run at its limit, which means
that it's constantly added and removed to/from the pool. This can happen
hundreds of thousands of times per second. Adding or removing a hashed
server into the pool means:
  - recomputing the whole farm's hash for static algorithms
  - inserting or removing a large number of instances of this server
from the consistent hash tree when using consistent hashing

Both are very expensive and while they're actually acceptable for the
rare cases where a server goes up or down, they're really not for when
a server reaches a limit or drops below.

Also, at a low dose, queuing is good and can even be faster than
spreading the load on more servers because it makes a more efficient
use of network packets and TCP connections by merging requests with
ACKs, and can deliver a request to a server having the code to handle
it hot in its cache with a CPU already running at top speed. But that's
true for small queue sizes of course.

Before we had hash-balance-factor, some people were doing something
related to what you describe by using two backends: one with the hash
LB algorithm, and another one with RR, random or anything. And they
would choose what backend to use depending on the average queue size
in the first backend, using the avg_queue(backend) sample fetch.

Now with hash-balance-factor, I really think you could reach your
goal, because usually if a server has some requests in queue, either
all of them do, and you don't care what server you pick, or one has
not, and hash-balance-factor will spot it and use it instead. Just
run it with a very low value, e.g. 101, and that should be OK.

If that's not sufficient, I think we could imagine adding a "no-queue"
option to the balance keyword to indicate that when the selected server
is full, instead the request should be queued in the backend. It would
still require significant changes to make sure that such a request can
*first* be attempted on another server before being queued but I think
it might be possible. An alternative would be to have a modifier on the
consistent hash algorithms to try to find a different server when the
selected one has some queue. The problem is, we need to limit the
attempts because we don't want to loop forever if all have some queue.
Maybe that could actually be a variant of the hash-balance-factor to
ask it to systematically ignore servers with queue.

regards,
Willy



Re: [2.0.17] crash with coredump

2020-09-25 Thread Willy Tarreau
On Fri, Sep 25, 2020 at 03:26:47PM +0200, Kirill A. Korinsky wrote:
> > On 25. Sep 2020, at 15:06, Willy Tarreau  wrote:
> > 
> > Till here your analysis is right but:
> >  - the overflow would only be at most the number of extra threads running
> >init_genrand() concurrently, or more precisely the distance between
> >the most upfront to the latest thread, so in the worst case nbthread-1
> >hence there's no way to write a single location into a totally unrelated
> >structure without writing the nbthread-2 words that are between the end
> >of the MT array and the overwritten location ;
> 
> I don't think so.
> 
> We have two threads and steps
> 
> 1. T1: mti < N
> 2. T2: mti < N
> 3. T1:  mtii++
> 4. T2:  mtii++
> 5. T1: mt[mti] = (1812433253UL * (mt[mti-1] ^ (mt[mti-1] >> 30)) + mti);
> 6. T2: mt[mti] = (1812433253UL * (mt[mti-1] ^ (mt[mti-1] >> 30)) + mti);
> 
> Let assume that mti = N - 1 at steps 1 and 2. At this case mti (aka MT->o) is 
> N + 1 that is overflow of mt array (MT->v).
> 
> So, when T1 writes something at step 5 it puts a random value to mti (aka
> MT->i). Here I assume that the sume has dwrod similar that size(long) and MT
> hasn't got any gaps between v and I.
> 
> If so at step 6 we have mti with random value and writes to unpredicted place
> in memory.

Oh indeed you're totally right, I didn't notice that i was stored right after
the array! So yes, the index can be overflown with a random value. However,
the probability that a random 32-bit value used as an offset relative to
the previous array remains valid without crashing the process is extremely
low, and the probability that each time it lands in the exact same field of
another totally unrelated structure is even lower.

> >  - the lua code is not threaded (there's a "big lock" around lua since
> >stacks cannot easily be protected, though Thierry has some ideas for
> >this).
> 
> What does not threaded mean here?

My understanding is that this code is loaded from within Lua. And you
have only one thread running Lua at a time, since Lua itself is not
thread safe (it can be built to use external locks but apparently noone
builds it like this by default so it's not usable this way), this is
protected by hlua_global_lock. So you won't have two competing calls
to lrandom from Lua.

However I agree that the bug you found (which I looked for and didn't
spot last time I checked) is real for threaded applications.

Willy



Re: [2.0.17] crash with coredump

2020-09-25 Thread Willy Tarreau
On Fri, Sep 25, 2020 at 03:26:05PM +0200, Maciej Zdeb wrote:
> > Here I can suggest to implement Yarrow PRGN (that is very simple to
> > implement) with some lua-pure cryptographic hash function.
> 
> We're using lrandom because of the algorithm Mersenne Twister and its well
> known weaknesses and strengths.
> 
> > In fact I know it's possible to call haproxy's internal sample fetch
> > functions from Lua (I never can figure how to do that, I always need
> > to lookup an example for this unfortunately). But once you figure out how
> > to do it, you can call the "rand()" sample fetch that will call the
> > internal thread-safe random number generator.
> 
> Rand() sample fetch cannot be seeded (at the time we checked) so on HAProxy
> servers with nbproc > 1 we got multiple sequences of the same random
> numbers - it was one of the reasons we couldn't use it.

That was fixed long ago in 2.2-dev4 exactly for this reason:

  commit 52bf839394e683eec2fa8aafff5a0dd51d2dd365
  Author: Willy Tarreau 
  Date:   Sun Mar 8 00:42:37 2020 +0100

BUG/MEDIUM: random: implement a thread-safe and process-safe PRNG

This is the replacement of failed attempt to add thread safety and
per-process sequences of random numbers initally tried with commit
1c306aa84d ("BUG/MEDIUM: random: implement per-thread and per-process
random sequences").
(...)
It supports fast jumps allowing to cut the period into smaller
non-overlapping sequences, which we use here to support up to 2^32
processes each having their own, non-overlapping sequence of 2^96
numbers (~7*10^28). This is enough to provide 1 billion randoms per
second and per process for 2200 billion years.

This was backported into 2.0.14 as well. So if you know how to use it
you definitely can right now. But as I mentioned, the thread-unsafety
of lrandom isn't related to your issue at the moment anyway.

> I was mailing outside the list with Willy and Christopher but it's worth
> sharing that the problem occurs even with nbthread = 1. I've managed to
> confirm it today.

Yes, thanks for the info by the way, however being blocked on something
else at the moment I didn't have the opportunity to have a look at it yet.

Regards,
Willy



Re: [2.0.17] crash with coredump

2020-09-25 Thread Willy Tarreau
Hi Kirill,

On Fri, Sep 25, 2020 at 12:34:16PM +0200, Kirill A. Korinsky wrote:
> I've extracted a pice of code from lrandom and put it here:
> https://gist.github.com/catap/bf862cc0d289083fc1ccd38c905e2416
> 
> 
> You can see that object generator contains N words (and here it is 624), and
> I use an assumption that Maciej's code doesn't create a new generator for
> each request and share lrandom.
> 
> Idea of this RNG is initialize each N words via init_genrand and it checking
> that all of them are used, and after one generated a new ones.
> 
> Let assume that we called genrand_int32 at the same moment from two threads.
> If condition at lines 39 and 43 are true we start to initialize the next
> words at both threads.
> 
> You can see that we can easy move outside of v array at line 21 because two
> threads are increasing i field, and put some random number to i field.

Till here your analysis is right but:
  - the overflow would only be at most the number of extra threads running
init_genrand() concurrently, or more precisely the distance between
the most upfront to the latest thread, so in the worst case nbthread-1
hence there's no way to write a single location into a totally unrelated
structure without writing the nbthread-2 words that are between the end
of the MT array and the overwritten location ;

  - the lua code is not threaded (there's a "big lock" around lua since
stacks cannot easily be protected, though Thierry has some ideas for
this).

> Ans when the second thread is going to line 27 and nobody knows where it put 
> 0x

Actually init_genrand() doesn't actually *write* 0x but only writes
*something* of 64 bits size and applies a 32-bit mask over it, so it would
have written 32 bits pseudo-random bits followed by 4 zero bytes.

> How can it be proved / solved?
> 
> I see a few possible options:
> 1. Switch off threads inside haproxy
> 2. Use dedicated lrandom per thread
> 3. Move away from lrandom
> 
> As I understand lrandom is using here because it is very fast and secure, and
> reading from /dev/urandom isn't an option.
> 
> Here I can suggest to implement Yarrow PRGN (that is very simple to
> implement) with some lua-pure cryptographic hash function.

In fact I know it's possible to call haproxy's internal sample fetch
functions from Lua (I never can figure how to do that, I always need
to lookup an example for this unfortunately). But once you figure how
to do it, you can call the "rand()" sample fetch that will call the
internal thread-safe random number generator.

Regards,
Willy



Re: Naming and discoverability of fetches and converters

2020-09-23 Thread Willy Tarreau
On Wed, Sep 23, 2020 at 11:39:57PM +0200, Tim Düsterhus wrote:
> Willy,
> 
> Am 23.09.20 um 23:02 schrieb Tim Düsterhus:
> > Yes, such a categorized list would certainly solve most of the current
> > pain points of the converter documentation. I probably would even leave
> > out the purpose column and instead relying on a descriptive name + the
> > current long form description. That should make maintaining the
> > documentation a bit easier. Most the the purposes you listed there are
> > painfully tautological. I mean ... it's obvious that hash.crc32 will
> > create a CRC-32 hash.
> > 
> > I believe that the short names of the more uncommon converters should be
> > deprecated in the long run, though. This keeps the list nice and tidy
> > and makes the configuration more self-documenting. Especially since the
> > current short names are not ideally named as we established in this
> > conversations.
> > 
> 
> Splitting this off in a separate email to allow discussing this
> separately. I feel it might be helpful having a more organized process
> for documenting those converters that would allow to automatically
> generate those category lists without requiring a human to add the
> converter in all the appropriate locations, while also taking care of
> the alphabetic order.
> 
> As much as I dislike YAML, it feels like an appropriate format here,
> because it allows to mix structured data with arbitrary text. Each
> converter would then be placed into a dedicated YAML file that contains
> everything one needs to know about that converter.
> 
> Here's a simply example script in Python:
> 
> 
> > import sys
> > import textwrap
> > from ruamel.yaml import YAML
> > 
> > yaml = YAML()
> > docs = yaml.load_all("""
> > ---
> > signature: sha2()
> > tags:
> >   - hash
> >   - crypto
> > requirements:
> >   - USE_OPENSSL
> > --- |
> > Converts a binary input sample to a digest in the SHA-2 family. The result 
> > is a binary sample with length of /8 bytes.
> > 
> > Valid values for  are 224, 256, 384, 512, each corresponding to 
> > SHA-. The default value is 256.
> > """)

This is exactly the type of thing I want to avoid. Not only it's unreadable,
in addition it's unwritable, and would make the contribution process a pain
for anyone who doesn't know this part very well. Many of the sample fetches
and converters are added by first-time contributors who currently do a good
job at this and don't face major difficulties nor need to learn any particular
language. In your example above I have no idea how I can enter formatted text
with an example, or a multi-line entry or whatever, and *this* is what can
easily discourage anyone. Right now the main benefit of what we have is that
if it reads fine for you it's OK by definition. Sure it encourages blind
copy-paste but it's no big deal, and it's natural that once in a while we
have to refactor things once they start to accumulate. Another common issue
is that some keywords are not properly sorted, but that's to be expected
since not everyone uses a latin alphabet and perfectly knows the ordering
(and even those using it sometimes get it wrong). Should we care about this?
Clearly no.

Let's keep the contribution a low entry barrier, and have some contributors
like you and me for whom it's easier to occasionally provide fixes do the
necessary janitor work, and that's fine. It's the same with Ilya's typo
fixes. When I spot one in a patch I'm about to merge, I fix it before
merging it, otherwise it will be part of the next typo fixes series and
it's not dramatic.

Now, if you think that some scripts can make it easier for some users to
enter some doc, maybe we can place them into scripts/. A script like yours
could take a few arguments like keyword name, arguments, type, description,
see also, and emit the block of formatted text that can be copy-pasted and
serve as a skeletton if the user wants to improve it before committing.

Willy



Re: Naming and discoverability of fetches and converters

2020-09-23 Thread Willy Tarreau
Hi Tim,

On Wed, Sep 23, 2020 at 11:02:47PM +0200, Tim Düsterhus wrote:
> Willy,
> 
> please excuse my belated reply. First the weekend was there and then
> subsequently I forgot about your email.

Well, you don't need to apologize :-) I prefer when design discussions
take their time than when they're rushed anyway.

> Unfortunately no one else wanted
> to add their 2ct so far :-( I believe this is a topic that could benefit
> from administrator opinions.

It could also be indicative of something we already know, which is that
once users have configured their systems, the first thing they don't
want to do is to reconfigure them!

> Am 18.09.20 um 15:26 schrieb Willy Tarreau:
> >> Then there's quite a few redundant converters. We have `sha1`, then
> >> `sha2` and most recently we have `digest`.
> > 
> > From what I'm reading in the commit message (the doc not being clear
> > on this point), they don't seem to overlap as "digest" seems to return
> > an HMAC. In addition, "sha1" doesn't require openssl so it comes for
> > free. But anyway I get your point and generally agree.
> 
> No, digest() is a plain hash. hmac() is for HMACs :-) Both were added in
> the same commit, though.

OK, but in any case there's no info in the doc about supported algos
beyond "sha256" that is given as an example, so I'm pretty sure both
are mostly unused at the moment :-/

> >> But there's also
> >> a difference in naming. Sometimes the namespace is separated by a dot
> >> (`req.hdr`). Sometime it's an underscore (`ssl_fc`).
> > 
> > Yep. The dot was added later to try to group all those working within
> > the same domain (e.g. the request or the response). We also (re)discovered
> > that the automatic binding making them directly available from Lua is less
> > transparent with the dot as that one has to be turned to an underscore
> > there. But it's less common and the dot still appears much more readable
> > to me.
> 
> Yes, I agree. Maybe it's because the dot feels natural from the member
> access of a struct in C (or as a property access in many other
> programming languages). It feels like a natural namespace separator to me.

Sure, but my first nasty encounters with underscores that made me want
to suppress them was when seeing them not display at the bottom of a
screen connected to a KVM in a rack, making it a real pain to work on
a config. That's when I realized that they're mostly used by developers
to name symbols in code but rarely in config keywords where the dash
usually replaces the space and the dot serves as a namespace delimiter,
and that using underscores in config keywords was awkward and ought to
be avoided.

> >> s.field() instead of field()
> >> s.word() instead of word()
> >> s.json() instead of json()
> > 
> > Maybe, why not for these ones. But then json() alone cannot stay
> > this way and needs to be called json-enc() or something like this
> > since it only encodes and doesn't decode.
> 
> Technically even json_enc() is not entirely appropriate, because the
> only thing it does is encoding a value so that the result is a valid
> contents of a string in JSON. Specifically bool(1),json() is simply `1`,
> not `true`. And str("foo"),json() is `foo`, not `"foo"`.

I think you already explained that to me, and the fact that I don't
remember it indicates that the current naming isn't the best one.

> >> Make 'http.' contain HTTP related converters.
> >>
> >> http.language() instead of language()
> > 
> > This already becomes difficult. http is very wide and touches all
> > areas. For example you'll use http.date() to encode an HTTP date
> > while plenty of users will search for it in "date.something" because
> > it's just one way to represent a date, or "time.something" because a
> > date is first and foremost something related to time.
> 
> I guess for that date() would best be generalized to take the proper
> format as a parameter. Allowing e.g. date(iso8601), initially only
> supporting dates for use in HTTP header.

Yes, possibly.

> I must admit that I never used the math operators for anything before.
> No idea how commonly they are used in real world configurations.

I've mostly seen them used to compute thresholds based on historic values
extracted from stick tables, and used to compute expiration dates to
place in headers.

> >> Make 'sec.' contain security related converters.
> >>
> >> sec.digest() instead of digest()
> >> deprecate sha1, sha2 in favor of digest()
> > 
> > Actually I'd rather have "hash.something" for everything related to
> > hashing, as most of t

Re: Different behavior of multiple track-sc0 in haproxy 2.2

2020-09-23 Thread Willy Tarreau
Hi Reinhard,

On Mon, Sep 21, 2020 at 03:29:17PM +0200, Reinhard Vicinus wrote:
> Hi,
> 
> the following haproxy configuration produces different results with
> haproxy 2.0.16 (and 1.8.20) vs 2.2.3 running with "haproxy -f
> haproxy.conf -d":
> 
> global
>   stats  socket /tmp/haproxy.sock mode 660 user haproxy group haproxy
> level admin
> 
> defaults
>   mode http
> 
> frontend fe-a
>   bind :10001
>   http-request track-sc0 src table be-a
>   http-request track-sc0 src table be-b
> 
> backend be-b
>   stick-table type ip  size 100k  expire 30s  store
> gpc0,http_req_rate(10s),http_err_rate(10s)
> 
> backend be-a
>   stick-table type ip  size 100k  expire 30s  store
> gpc0,http_req_rate(10s),http_err_rate(10s)
> 
> With haproxy 2.0.16 (and 1.8.20) only the table be-a gets updated. With
> haproxy 2.2.3 both table gets updated.

Oh this is weird. I don't even know how this is possible, because the
principle of the action is that once we have the tracker enabled, it
cannot change. Thus I suspect we possibly broke something at the low
level in the action processing maybe. Definitely something we need to
have a look at!

Thanks,
Willy



Re: Naming and discoverability of fetches and converters

2020-09-18 Thread Willy Tarreau
HiTim,

On Thu, Sep 17, 2020 at 07:27:55PM +0200, Tim Düsterhus wrote:
> Hi List,
> 
> I'd like to discuss something that's bothering me for quite some time
> now: The naming, namespacing and discoverability of fetches and converters.
> 
> Frankly, it's a small mess right now. I assume this is due to historic
> growth an features being added over time.

I don't think anyone will disagree with this!

> Let me give a few examples:
> 
> I want to encoded something using base64? I use the `base64` converter.
> Decoding? **Obviously** that's `b64dec` ;)

That's a perfect example of something initially created to do one
thing and taking too generic a name ("base64"). Adding the decoder
years later proved to be difficult. I even think that by then we
didn't yet easily support arguments for converters, so it wasn't
trivial to indicate in base64() what direction to use, plus it
wouldn't have helped much anyway, given that what you write usually
means something in a context and might not be the correct form anyway.

> Hex encoding? Easily done with `hex`. Decoding back to a string? Not
> possible. But you can to `hex2i` to turn a hexadecimal string into an
> integer.

This one was recently discussed as well, I even though we had "unhex"
or something like this. Maybe it appeared in a private discussion with
someone, I don't remember.

> We have `url_dec`. But there isn't an `url_enc` or maybe I am just not
> able to find it.

I'm pretty sure it appeared in a private discussion recently as well,
in which I explained that it does have the exact same issue as url_dec
in that it needs to work differently depending on the part that's being
encoded and as such has to be very flexible.

> Then there's quite a few redundant converters. We have `sha1`, then
> `sha2` and most recently we have `digest`.

>From what I'm reading in the commit message (the doc not being clear
on this point), they don't seem to overlap as "digest" seems to return
an HMAC. In addition, "sha1" doesn't require openssl so it comes for
free. But anyway I get your point and generally agree.

> Then within the description of the converters the variable naming and
> scoping is explained over and over again for each converter that takes
> variable names.

Did you also notice that copy-pasting is quite a common practice in the
doc :-/

> For fetches it's not as bad, many have proper prefixes.

Initially there were very few converters, they were usable and needed
only for stick tables, so these were ipmask(), lower(), upper() and
that was about all. These slowly expanded without any general direction
being observed, making the naming quite not trivial. For fetches it's
different because you have to indicate where you want to retrieve the
information, and actually for some of them, depending on the mode
haproxy was working, very early it was possible to have some confusion
about that, so they needed to be a bit more precise.

> But there's also
> a difference in naming. Sometimes the namespace is separated by a dot
> (`req.hdr`). Sometime it's an underscore (`ssl_fc`).

Yep. The dot was added later to try to group all those working within
the same domain (e.g. the request or the response). We also (re)discovered
that the automatic binding making them directly available from Lua is less
transparent with the dot as that one has to be turned to an underscore
there. But it's less common and the dot still appears much more readable
to me.

> Long story short: Is it feasible to update the canonical names of
> fetches and converters, preserving backwards compatibility with old
> versions and adding the missing "duals" for encoders / decoders or do we
> have to live with the current situation.

We can *try*. Trust me, it is *extremely hard* to correctly name things.
A significant part of the development job is to find correct names for
variables, functions, struct members, and worse, config options! You want
non-confusing names that accurately represent what you're designating with
a very limited risk that it later degenerates into something broader and
confusing again.

> ---
> 
> Let me give a proposal of what makes sense to me:
> 
> Make 's.' contain string related converters.
> 
> s.field() instead of field()
> s.word() instead of word()
> s.json() instead of json()

Maybe, why not for these ones. But then json() alone cannot stay
this way and needs to be called json-enc() or something like this
since it only encodes and doesn't decode.

> Make 'http.' contain HTTP related converters.
> 
> http.language() instead of language()

This already becomes difficult. http is very wide and touches all
areas. For example you'll use http.date() to encode an HTTP date
while plenty of users will search for it in "date.something" because
it's just one way to represent a date, or "time.something" because a
date is first and foremost something related to time.

> Make 'math.' contain mathematics related converters.
> 
> math.add() instead of add()
> math.sub() instead of 

Re: [2.0.17] crash with coredump

2020-09-17 Thread Willy Tarreau
On Thu, Sep 17, 2020 at 10:56:39AM +0200, Maciej Zdeb wrote:
> Hi,
> 
> Our config is quite complex and I'm trying to narrow it down. It is
> occurring only on one production haproxy cluster (which consists of 6
> servers in each of two data centers) with significant load - crashes occurs
> on random servers so I would exclude memory corruption.

When I'm saying "memory corruption" I don't necessarily mean hardware
error, most likely a software error. For example a write after free to
a memory location, or any such thing, which typically happens outside
of the visible code path.

> I'm suspecting SPOE or/and LUA script both are used to send metadata about
> each request to an external endpoint. Yesterday I disabled this feature in
> one datacenter to verify.

Great!

> Our build is done in docker (Ubuntu bionic) with kernel 4.9.184-linuxkit,
> crash is on Ubuntu bionic 4.15.0-55-generic, using:
> haproxy 2.0.17
> openssl 1.1.1f
> pcre 8.44
> lua 5.3.5
> lrandom (PRNG for lua, we're using it for 2 or 3 years without any
> problems, and soon we will drop it from our build)

Never heard of this last one, not that it would make it suspicious at
all, just that it might indicate you're having a slightly different
workload than most common ones and can help spotting directions where
to look for the problem.

> compiled in following way:
(...)

OK, nothing unusual here, thanks for the details.

Let's wait for your new tests to narrow down the issue a little bit more,
then.

Thanks,
Willy



Re: [2.0.17] crash with coredump

2020-09-17 Thread Willy Tarreau
Hi guys,

On Thu, Sep 17, 2020 at 11:05:31AM +1000, Igor Cicimov wrote:
(...)
> > Coredump fragment from thread1:
> > (gdb) bt
> > #0  0x55cbbf6ed64b in h2s_notify_recv (h2s=0x7f65b8b55130) at
> > src/mux_h2.c:783

So the code is this one:

   777  static void __maybe_unused h2s_notify_recv(struct h2s *h2s)
   778  {
   779  struct wait_event *sw;
   780  
   781  if (h2s->recv_wait) {
   782  sw = h2s->recv_wait;
   783  sw->events &= ~SUB_RETRY_RECV;
   784  tasklet_wakeup(sw->tasklet);
   785  h2s->recv_wait = NULL;
   786  }
   787  }

In the trace it's said that sw = 0x. Looking at all places where
h2s->recv_wait() is modified, it's either NULL or a valid pointer to some
structure. We could have imagined that for whatever reason h2s is wrong
here, but this call only happens when its state is still valid, and it
experiences double dereferences before landing here, which tends to
indicate that the h2s pointer is OK. Thus the only hypothesis I can have
for now is memory corruption :-/ That field would get overwritten with
(int)-1 for whatever reason, maybe a wrong cast somewhere, but it's not
as if we had many of these.

> I'm not one of the devs but obviously many of us using v2.0 will be
> interested in the answer. Assuming you do not install from packages can you
> please provide some more background on how you produce the binary, like if
> you compile then what OS and kernel is this compiled on and what OS and
> kernel this crashes on? Again if compiled any other custom compiled
> packages in use, like OpenSSL, lua etc, you might be using or have compiled
> haproxy against etc.?
> 
> Also if this is a bug and you have hit some corner case with your config
> (many are using 2.0 but we have not seen crashes) you should provide a
> stripped down version (not too stripped though just the sensitive data) of
> your config too.

I agree with Igor here, any info to try to narrow down a reproducer, both
in terms of config and operations, would be tremendously helpful!

Thanks,
Willy



Re: Statistics improvement - usage for the json schema

2020-09-16 Thread Willy Tarreau
Hi Amaury,

On Wed, Sep 16, 2020 at 10:13:44AM +0200, Amaury Denoyelle wrote:
> Hi,
> 
> I'm currently working on extending the statistisc capabilities for
> HAProxy. My first goal is to design a small API to be able to quickly
> add new counters.
> 
> I have a question about the cli command "show schema json". I am not a
> json expert and do not know precisely the purpose of a json schema.
> However, it seems that the currently generated schema is not valid, at
> least by checking on this website :
> https://www.jsonschemavalidator.net/
> 
> Is the json schema useful to anyone or can I safely ignore/remove it ?

Just for reference, I've found the commit which added the schema 3.5
years ago:

   6f6bb380e ("MEDIUM: stats: Add show json schema")

And the discussion around this was in this thread:

   https://www.mail-archive.com/haproxy@formilux.org/msg23173.html

The thread is quite long with multiple proposals, you may be
interested in the discussions there.

In the mean time, if anyone (still) uses it, it would be nice to know :-)

Cheers,
Willy



Re: Is adaptive circuit breaking in the roadmap for 2.3/2.4?

2020-09-15 Thread Willy Tarreau
On Tue, Sep 15, 2020 at 09:24:32AM +0200, Willy Tarreau wrote:
(...)
> There's no such ongoing work that I'm aware of but that has always
> been a subject of interest to me (I even wrote down the algorithm to
> compute weights by measured response times using a low-pass filter a
> decade ago but I lost my notes and never felt like doing work again).
> So if anyone is interested in this subject, we can continue this
> conversation till we reach something that looks like a possible design
> roadmap.

By the way there is another aspect that has always interested me around
this, which is that it would simplify the confuguration of a server's
maxconn when it's shared between multiple backends or even multiple LB
nodes. It will certainly lead to oscillations sometimes but that's
manageable by using randomness in increments so that multiple LB nodes
cannot enter into resonance.

Willy



Re: Is adaptive circuit breaking in the roadmap for 2.3/2.4?

2020-09-15 Thread Willy Tarreau
Hi Pavlos!

On Sat, Sep 12, 2020 at 11:45:12AM +0200, Pavlos Parissis wrote:
> Hi old friends!,
> 
> Is in the roadmap the addition of a circuit breaking which adapts its 
> settings using real-time data?
> I believe we discussed this in the last HAProxyConf with a group of people, 
> but I don't remember if there were
> , back then, concrete plans to work on it.
> 
> I know that something similar can  be accomplished by using agent check, but 
> IMHO it less ideal.
> - watch 
> https://www.youtube.com/watch?v=CQvmSXlnyeQ=PLj6h78yzYM2O1wlsM-Ma-RYhfT5LKq0XC=21
> 
> - read 
> https://www.envoyproxy.io/docs/envoy/v1.15.0/configuration/http/http_filters/adaptive_concurrency_filter

Yep as you say we can indeed already do it using agent checks. I know
it's not ideal, but what matters to me is that it indicates we already
have the required core functionalities to do something like this.

The proper way to implement such a mechanism is by using feedback
reaction, similar to how MPPT solar panel controllers work: you never
know if you're sending enough load to keep the servers busy, so you
constantly need to try to send a bit more to see if any service
degradation occurs or not. The degradation happens by response time
suddenly becoming affine to the number of concurrent requests. Then
the decision of where to stick needs to be made based on the choice
of lower latency (bottom of curve) or higher bandwidth (top of curve).
A simpler approach would consist in having a setting indicating how
much extra response time is acceptable compared to the measued optimal
one.

I also think that it remains important to let the user configure lower
and upper bounds that guarantee safe operations. And that's typically
what could be done using the minconn and maxconn values. Instead of
using the backend's fullconn setting we would rely on a response time
measurement.

Or maybe that could be the right opportunity for splitting connection
concurrency from request concurrency, keeping connections for the hard
limits and using request concurrency for softer limits.

There's no such ongoing work that I'm aware of but that has always
been a subject of interest to me (I even wrote down the algorithm to
compute weights by measured response times using a low-pass filter a
decade ago but I lost my notes and never felt like doing work again).
So if anyone is interested in this subject, we can continue this
conversation till we reach something that looks like a possible design
roadmap.

Cheers,
Willy



Re: [PATCH] travis-ci: report asan findings in more intelligent way

2020-09-15 Thread Willy Tarreau
On Tue, Sep 15, 2020 at 11:46:02AM +0500,  ??? wrote:
> ping

Sorry, I missed it, now merged.

Thanks!
Willy



Re: [PATCH] BUILD: makefile: change default value of CC from gcc to cc

2020-09-14 Thread Willy Tarreau
On Mon, Sep 14, 2020 at 11:42:47PM -0400, Brad Smith wrote:
> > It's not about package maintainers as they know to do the right
> > thing. It's
> > about end users that don't know any better. In this case someone
> > submitted
> > a bogus patch which was commited based on using the wrong compiler.
> > 
> > why they do not just use package system ?
> 
> I don't know, but it's beside the point now. All kinds of people for various
> reasons (re)build software outside of pre-built packages.

Well, after sleeping over it, I think your points above are valid. We
used to support exclusively gcc by extensively using GNU extensions,
but over time other compilers like icc, clang, and tcc have adopted a
lot of these GNU extensions and are compatible. And it makes sense not
to ask them to call themselves gcc of course. So I think we can do this,
and that 2.3 is the right version to do it, and it will leave us enough
time to see if any issues are reported. In practice I expect the vast
majority of systems where development packages are installed to have
at least one working "cc".

I'm going to take your patch, thanks!

Willy



Re: [PATCH] BUILD: makefile: change default value of CC from gcc to cc

2020-09-14 Thread Willy Tarreau
Hi Brad,

On Sun, Sep 13, 2020 at 02:53:06AM -0400, Brad Smith wrote:
> Change the default value of CC from gcc to cc to be more appropriate
> for modern day mix of compilers. On GCC based OS's cc -> gcc. On Clang
> based OS's cc -> clang. FreeBSD / OpenBSD have switched to Clang and
> this corrects building with the proper compiler on OS's using Clang
> as the default compiler. This especially matters for the necessity for
> TLS on OpenBSD. I would expect this affects OpenMandriva and other
> Linux OS's using Clang as well.

I don't have a strong opinion on this one, I'd like to get feedback
from distro maintainers first. In any case it's trivial to set CC
while building so it's no big deal for sure.

Thanks,
Willy



Re: [PATCH v2 0/2] Fixed type in sizeof in calloc

2020-09-12 Thread Willy Tarreau
On Sat, Sep 12, 2020 at 08:26:41PM +0200, Tim Duesterhus wrote:
> Willy,
> 
> Am 12.09.20 um 18:52 schrieb Willy Tarreau:
> > Thanks for doing this, I really appreciate it as I too hate having
> > a named type in calloc, as it doesn't survive code changes over time.
> > 
> > I'd rather have one patch to fix the known bug and the rest as a
> > cleanup patch however, so that we can backport the fix wherever
> > needed without risking to break something subtle by accident. Just
> > let me know if that's OK for you, otherwise I'll take care of slicing
> > your patch next week.
> > 
> 
> Okay. I've split it into 2 patches.

Great, now applied, thank you!

> Checking out the blame for the BUG was fun. Apparently it's the
> second time I fixed this exact issue.

I'm not surprised. This type of practice is common and sometimes even
needed (e.g. when you put the output into a void*), so some people tend
to be more used to it than to the other.

> I'm already seeing Ilya suggesting that we add Coccinelle to the CI pipeline
> in response to this :-)

That could be an idea, indeed. I seldom run coccinelle on the code when
I spot an ugly bug that's easy to search at other places, but I don't
think about doing it often. We need to be reasonable so that we don't
have yet-another series of low importance reports to deal with that
require code changes just to silence a low-threshold detector. That's
always the difficult part.

Cheers,
Willy



Re: [PATCH] BUG/MINOR: Do not use a fixed type for 'sizeof' in 'calloc'

2020-09-12 Thread Willy Tarreau
Hi Tim,

On Sat, Sep 12, 2020 at 05:32:03PM +0200, Tim Duesterhus wrote:
> This fixes at least one instance where the wrong type was given to sizeof.
> clang-analyzer complained that in src/cfgparse.c line 3614 
> (newsrv->curr_idle_thr):
> 
> > Result of 'calloc' is converted to a pointer of type 'unsigned int', which 
> > is
> > incompatible with sizeof operand type 'int'
> 
(...)

Thanks for doing this, I really appreciate it as I too hate having
a named type in calloc, as it doesn't survive code changes over time.

I'd rather have one patch to fix the known bug and the rest as a
cleanup patch however, so that we can backport the fix wherever
needed without risking to break something subtle by accident. Just
let me know if that's OK for you, otherwise I'll take care of slicing
your patch next week.

Thanks!
Willy



Re: [PATCH] ci: travis-ci: help coverity to recognize abort

2020-09-12 Thread Willy Tarreau
On Sat, Sep 12, 2020 at 11:31:29AM +0500,  ??? wrote:
> so, it is good time to adjust .gitignore :)
> 
> I also added commit message with explanation. I'm ok if you modify it by
> your will.

That's perfect, all applied with no change now, thank you very much Ilya!
Willy



Re: [PATCH] MINOR: Add either(,) converter

2020-09-11 Thread Willy Tarreau
On Fri, Sep 11, 2020 at 05:19:26PM +0200, Tim Düsterhus, WoltLab GmbH wrote:
> Fun. I didn't receive your reply on company mail. I only got it from the
> list using my personal subscription. I hope this message threads properly.

Yep it does.

> Muscle memory is too strong :-/ I even used search and replace to adjust
> the name in all files, except configuration.txt. I even had to fix the
> 'either' within the GPL license comment.
> 
> Willy, find a patch attached. I've put Miroslav as 'Reported-by'.

Thanks, now merged (just after the dev4 release).

Willy



[ANNOUNCE] haproxy-2.3-dev4

2020-09-11 Thread Willy Tarreau
the pattern expression revision when it is 
pruned

Gilchrist Dadaglo (5):
  BUG/MAJOR: contrib/spoa-server: Fix unhandled python call leading to 
memory leak
  BUG/MINOR: contrib/spoa-server: Ensure ip address references are freed
  BUG/MINOR: contrib/spoa-server: Do not free reference to NULL
  BUG/MINOR: contrib/spoa-server: Updating references to free in case of 
failure
  BUG/MEDIUM: contrib/spoa-server: Fix ipv4_address used instead of 
ipv6_address

Jerome Magnin (1):
  DOC: ssl-load-extra-files only applies to certificates on bind lines

Lukas Tribus (1):
  DOC: overhauling github issue templates

MIZUTA Takeshi (1):
  DOC: add description of pidfile in master-worker mode

Shimi Gersner (2):
  MEDIUM: ssl: Support certificate chaining for certificate generation
  MINOR: ssl: Support SAN extension for certificate generation

Thierry Fournier (1):
  MINOR: hlua: Add error message relative to the Channel manipulation and 
HTTP mode

Tim Duesterhus (11):
  MEDIUM: cfgparse: Emit hard error on truncated lines
  DOC: cache: Use '' instead of '' in error message
  MINOR: cache: Reject duplicate cache names
  MINOR: Commit .gitattributes
  CLEANUP: Update .gitignore
  BUG/MINOR: haproxy: Free uri_auth->scope during deinit
  CLEANUP: Free old_argv on deinit
  CLEANUP: haproxy: Free post_proxy_check_list in deinit()
  CLEANUP: haproxy: Free per_thread_*_list in deinit()
  CLEANUP: haproxy: Free post_check_list in deinit()
  MINOR: sample: Add iif(,) converter

Victor Kislov (1):
  BUG/MINOR: auth: report valid crypto(3) support depending on build options

William Lallemand (3):
  BUG/MEDIUM: ssl: crt-list negative filters don't work
  BUG/MEDIUM: ssl: fix ssl_bind_conf double free w/ wildcards
  BUG/MINOR: startup: haproxy -s cause 100% cpu

Willy Tarreau (48):
  REGTEST: remove stray leading spaces in converteers_ref_cnt_never_dec.vtc
  BUILD: tools: include auxv a bit later
  BUILD: task: work around a bogus warning in gcc 4.7/4.8 at -O1
  MINOR: tcp: don't try to set/clear v6only on inherited sockets
  BUG/MINOR: reload: detect the OS's v6only status before choosing an old 
socket
  MINOR: reload: determine the foreing binding status from the socket
  MEDIUM: reload: stop passing listener options along with FDs
  MEDIUM: fd: replace usages of fd_remove() with fd_stop_both()
  CLEANUP: fd: remove fd_remove() and rename fd_dodelete() to fd_delete()
  MINOR: fd: add a new "exported" flag and use it for all regular listeners
  MEDIUM: reload: pass all exportable FDs, not just listeners
  BUG/MINOR: reload: do not fail when no socket is sent
  REORG: tcp: move TCP actions from proto_tcp.c to tcp_act.c
  CLEANUP: tcp: stop exporting smp_fetch_src()
  REORG: tcp: move TCP sample fetches from proto_tcp.c to tcp_sample.c
  REORG: tcp: move TCP bind/server keywords from proto_tcp.c to 
cfgparse-tcp.c
  REORG: unix: move UNIX bind/server keywords from proto_uxst.c to 
cfgparse-unix.c
  REORG: sock: start to move some generic socket code to sock.c
  MINOR: sock: introduce sock_inet and sock_unix
  MINOR: tcp/udp/unix: make use of proto->addrcmp() to compare addresses
  MINOR: sock_inet: implement sock_inet_get_dst()
  REORG: inet: replace tcp_is_foreign() with sock_inet_is_foreign()
  REORG: sock_inet: move v6only_default from proto_tcp.c to sock_inet.c
  REORG: sock_inet: move default_tcp_maxseg from proto_tcp.c
  REORG: listener: move xfer_sock_list to sock.{c,h}.
  MINOR: sock: add interface and namespace length to xfer_sock_list
  MINOR: sock: implement sock_find_compatible_fd()
  MINOR: sock_inet: move the IPv4/v6 transparent mode code to sock_inet
  REORG: sock: move get_old_sockets() from haproxy.c
  MINOR: sock: do not use LI_O_* in xfer_sock_list anymore
  MINOR: sock: distinguish dgram from stream types when retrieving old 
sockets
  BUILD: sock_unix: fix build issue with isdigit()
  CLEANUP: http: silence a cppcheck warning in get_http_auth()
  REGTEST: increase some short timeouts to make tests more reliable
  BUG/MINOR: threads: work around a libgcc_s issue with chrooting
  BUILD: thread: limit the libgcc_s workaround to glibc only
  MINOR: protocol: do not call proto->bind_all() anymore
  MINOR: protocol: do not call proto->unbind_all() anymore
  CLEANUP: protocol: remove all ->bind_all() and ->unbind_all() functions
  MAJOR: init: start all listeners via protocols and not via proxies anymore
  BUG/MEDIUM: mux-h1: always apply the timeout on half-closed connections
  BUILD: threads: better workaround for late loading of libgcc_s
  BUILD: compiler: reserve the gcc version checks to the gcc compiler
  BUILD: compiler: workaround a glibc madness around __attribute__()
  BUILD: intops: on x86_64, the 

Re: [PATCH] MINOR: Add either(,) converter

2020-09-11 Thread Willy Tarreau
On Fri, Sep 11, 2020 at 04:55:45PM +0200, Tim Düsterhus, WoltLab GmbH wrote:
> I consider 'iif' a bit obscure. It easily looks like a typo. Similar to
> 'iff' for 'if and only if' which tends to generate a number of questions
> as well.

I agree but others possibly know it and we should not consider our own
case as representative, so let's see.

> But I've certainly seen 'iif' being used before and I don't strongly
> care for the naming. I'm happy with anything.
> 
> I've attached an updated patch and leave it up to you whether you want
> to merge it and whether you'd like to wait for more opinions or not :-)

I've just merged it so that it gets better exposure, that's the best way
to receive criticisms and proposals for another name :-)

Thanks!
Willy



Re: [PATCH] MINOR: Add either(,) converter

2020-09-11 Thread Willy Tarreau
Hi guys,

First, I really like the feature, that's a great idea.

On Fri, Sep 11, 2020 at 04:28:31PM +0200, Miroslav Zagorac wrote:
> On 09/11/2020 03:56 PM, Tim Düsterhus, WoltLab GmbH wrote:
> > We've had a bit of discussion regarding the naming of the converter. I
> > wanted to avoid calling it `if`, because then we could have stuff like this:
> > 
> >http-request set-var(txn.foo) bool(1),if(bar,baz)
> > 
> > which can easily be confused with:
> > 
> >http-request set-var(txn.foo) bool(1) if bar
> > 
> > Some other naming suggestions that came up:
> > 
> > - choice (my initial choice)
> > - ifor / if_or
> > - ifelse / if_else
> > - iftrue (with the  argument being optional)
> 
> Maybe something like this would be appropriate (IIF)?

I was thinking about "select" which I've seldom seen, I never heard about
"iif" previously but it has the benefit of being quite short (convenient
for long expressions) and already in use elsewhere. And it also has the
benefit that someone searching for "if" would easily stumble on it.

I'm fine with the patch BTW, so if everyone agrees on "iif", let's suggest
we go with it ?

thanks,
Willy



Re: http2 smuggling

2020-09-11 Thread Willy Tarreau
On Fri, Sep 11, 2020 at 09:56:21AM +0200, Tim Düsterhus wrote:
> Willy,
> 
> Am 11.09.20 um 09:42 schrieb Willy Tarreau:
> > On Fri, Sep 11, 2020 at 09:02:57AM +0200, Tim Düsterhus wrote:
> >> According to the article performing a h2c upgrade via TLS is not valid
> >> according to the spec. HAProxy implements the H2 spec.
> > 
> > "according to the article" :-) There's no such mention in the spec
> > itself from what I remember, it's just that it's usually pointless,
> > but there may be a lot of situations where it's considered better to
> > forward an upgradable connection over TLS to the next intermediary
> > because the intermediary network is not safe.
> 
> I might misunderstand it, but I'd say that RFC 7540#3.3 specifically
> disallows h2c for TLS:
> 
> >HTTP/2 over TLS uses the "h2" protocol identifier.  The "h2c"
> >protocol identifier MUST NOT be sent by a client or selected by a
> >server; the "h2c" protocol identifier describes a protocol that does
> >not use TLS.

Ah OK, got it, this one is about the protocol indicated in ALPN, as
reflected by the preceeding sentence: "A client that makes a request
to an "https" URI uses TLS with the application-layer protocol
negotiation (ALPN) extension".

When this was designed, there were long discussions about whether h2
alone or both h2/h2c had to be registered as protocols, and about
whether h2c was allowed in ALPN and in this case what to do about it.
It was then concluded that the easiest solution was to state that h2c
is the cleartext upgrade version while h2 is the raw one over TLS.

> >> Question 1: Should HAProxy reject requests that set Upgrade: h2c over
> >> TLS? I think it should. Basically the following rule should be applied
> >> automatically to my understanding.
> >>
> >> http-request deny deny_status 400 if { req.hdr(upgrade) h2c } { ssl_fc }
> > 
> > No I disagree. Let's say you have an h2c client on datacenter 1 and
> > an h2c server on datacenter 2. This rule would prevent you from using
> > the local haproxy to secure the connection, while providing zero benefit.
> 
> If I just want to secure the connection I would use 'mode tcp' where
> HAProxy is a dumb pipe and this rule would not apply.

You can't always. Let's say haproxy is your local side car and your
application sends everything to it. You'll route requests to various
places according to their Host and URI. Why is it that for a particular
reason one type of request is considered special enough to deserve being
broken ?

> > By the way, it's fun to see that a discussion started a few days ago
> > regarding the uselessness of h2c and its removal from the next H2 spec
> > because "nobody implemented it yet" :-)  And actually the guy had to
> > implement its own server to find a complying one.
> 
> I believe nginx can do h2c.

Maybe but then probably not by default, given that the guy used it in
his test to forward the Upgrade request. If it would support it by
default, it would have considered that request for itself instead. I'm
pretty sure that nginx does support cleartext h2 (without TLS) however.

By the way if you're interested in the todo list that starts to be
collected for an update of the H2 spec, it's here at the moment:

   https://github.com/martinthomson/http2-spec/issues/

Cheers,
Willy



Re: http2 smuggling

2020-09-11 Thread Willy Tarreau
On Fri, Sep 11, 2020 at 09:02:57AM +0200, Tim Düsterhus wrote:
> According to the article performing a h2c upgrade via TLS is not valid
> according to the spec. HAProxy implements the H2 spec.

"according to the article" :-) There's no such mention in the spec
itself from what I remember, it's just that it's usually pointless,
but there may be a lot of situations where it's considered better to
forward an upgradable connection over TLS to the next intermediary
because the intermediary network is not safe.

> Question 1: Should HAProxy reject requests that set Upgrade: h2c over
> TLS? I think it should. Basically the following rule should be applied
> automatically to my understanding.
> 
> http-request deny deny_status 400 if { req.hdr(upgrade) h2c } { ssl_fc }

No I disagree. Let's say you have an h2c client on datacenter 1 and
an h2c server on datacenter 2. This rule would prevent you from using
the local haproxy to secure the connection, while providing zero benefit.

By the way, it's fun to see that a discussion started a few days ago
regarding the uselessness of h2c and its removal from the next H2 spec
because "nobody implemented it yet" :-)  And actually the guy had to
implement its own server to find a complying one.

> Further the article says that the HTTP2-Settings header is a hop by hop
> header. It should not be forwarded by a proxy. According to the article
> HAProxy *does* forward it.
> 
> Question 2: Should HAProxy automatically strip the HTTP2-Settings header
> when forwarding requests?

Haproxy is not a proxy but a gateway. See it as a transparent cable
capable of staying synchronized with both ends and interacting the least
possible. There's nothing wrong with repeating hop-by-hop header fields
on gateways if you don't break the transport nor semantics, it's exactly
what happens when you use a raw TLS offloader for example, which doesn't
even understand HTTP. Technically speaking we're not "keeping" the
header, we're formulating a new request that places it again since it's
compatible with both sides' capabilities. In fact haproxy produces a
new connection header with all the tokens that it did not use, since
they are by definition for the next hop. "keep-alive" and "close" are
the only two that are of interest to us and that are terminated locally.

For example, the TE header is hop-by-hop. Haproxy doesn't need it but
maintains the transport, messaging and semantics end-to-end so it does
not need to eliminate it and disrupt end-to-end connectivity as long as
it stays synchronized. Really that's not different from a pure TCP proxy
(which is how haproxy started 20 years ago by the way).

Hoping this clarifies the point,
Willy



Re: http2 smuggling

2020-09-11 Thread Willy Tarreau
On Fri, Sep 11, 2020 at 02:52:30AM -0400, John Lauro wrote:
> I could be wrong, but I think he is stating that if you have that
> allowed, it can be used to get a direct connection to the backend
> bypassing any routing or acls you have in the load balancer, so if you
> some endpoints are blocked, or internal only, they could potentially
> be accessed this way.
> For example, if you have something like:
>   acl is_restrict path_sub /.git/
>   http-request deny if is_restrict !is_safe_ip
> 
> The acl could be bypassed by using the method to connect directly to a 
> backend.
> 
> That's not to say it's a security flaw in haproxy, but a potential
> misconfiguration that could allow traffic you thought was blocked by
> the proxy.

We're talking about an upgrade agreement between the client and the
server in order to use a protocol that the LB doesn't speak. This is
typically used for websocket, I remember having seen one terminal
server using this as well, maybe RDP or Citrix, I don't remember.

This is exactly the same as CONNECT+200: both ends agree to upgrade the
HTTP connection to another protocol till it ends. It's not HTTP that
applies once the tunnel is set up, so there are no filtering rules
nor whatever, exactly like after an accepted CONNECT request.

Willy



Re: [*EXT*] Re: http2 smuggling

2020-09-11 Thread Willy Tarreau
Hi Ionel,

On Fri, Sep 11, 2020 at 08:35:58AM +0200, Ionel GARDAIS wrote:
> Hi Willy,
> 
> Being devil's advocate : isn't the point that even if this is a documented,
> standardized and intended behavior, users relying on the reverse proxy for
> security/sanity checks could by tricked by this feature inadvertently ?

The security checks are properly performed during the request for upgrade,
the reporter even mentioned it in his article where he blocks the upgrade
by removing the Upgrade header. Note that the important point is that the
server *must* accept the upgrade for this to work. So there's no real
accident here. It's exactly what's being used for websocket.

Willy



Re: http2 smuggling

2020-09-11 Thread Willy Tarreau
On Fri, Sep 11, 2020 at 08:07:02AM +0200, Willy Tarreau wrote:
> Sadly, as usual after people discover protocols during the summer, some
> journalists will surely want to make noise about this to put some bread
> on their table...
> 
> Thanks for the link anyway I had a partial laugh; partial only because
> it makes useless noise.

And sadly, this one already started to make some noise there about his
recent discovery of a 20-years old standard:

   https://twitter.com/theBumbleSec

Had he asked if we supported 101, we could even have saved him time
in his HTTP discover test by pointing him to the doc:

   
http://git.haproxy.org/?p=haproxy.git;a=blob;f=doc/configuration.txt;h=c1f6f82;hb=HEAD#l332

Probably that next year he will discover that we also support CONNECT.
It's not even funny, the world is really doomed...

Willy



Re: http2 smuggling

2020-09-11 Thread Willy Tarreau
Hi Igor,

On Fri, Sep 11, 2020 at 01:55:10PM +1000, Igor Cicimov wrote:
> Should we be worried?
> 
> https://portswigger.net/daily-swig/http-request-smuggling-http-2-opens-a-new-attack-tunnel

But this stuff is total non-sense. Basically the guy is complaining
that the products he tested work exactly as desired, designed and
documented!

The principle of the upgrade at the gateway level precisely is to say
"OK both the client and the server want to speak another protocol you
agreed upon, let me retract" and let them talk over a tunnel. That's
exactly what is needed to support WebSocket for example. The simple
fact that he found that many proxies/gateways work like this should
ring a bell about the intended behavior!

In addition there is zero smuggling here as there is no desynchronisation.
It's just a tunnel between the client and the server, both agreeing to
do so. It does *exactly* the same as if the client had connected to the
server using a CONNECT method and the server had returned 200. So there
is absolutely no attack nor whatever here, just a connection that remains
dedicated to a client and a server till the end.

Sadly, as usual after people discover protocols during the summer, some
journalists will surely want to make noise about this to put some bread
on their table...

Thanks for the link anyway I had a partial laugh; partial only because
it makes useless noise.

Cheers,
Willy



Re: [PATCH 0/5] More deinit / free stuff

2020-09-10 Thread Willy Tarreau
On Thu, Sep 10, 2020 at 07:46:37PM +0200, Tim Duesterhus wrote:
> Willy,
> 
> this basically is a continuation from my series in July:
> 
> https://www.mail-archive.com/haproxy@formilux.org/msg37808.html
> 
> One more real bugfix and 4 more frees of live allocations for consistency with
> the surrounding code.

Whole series applied, thank you Tim!
willy



Re: [PATCH] ci: travis-ci: help coverity to recognize abort

2020-09-10 Thread Willy Tarreau
Hi Ilya,

On Thu, Sep 10, 2020 at 09:45:08PM +0500,  ??? wrote:
> ping :)

Ah sorry, thanks for the reminedr, I remember reading it and thought it
was merged, but I was wrong. However I'm seeing two mistakes:

  - first patch accidently merged a copy of your libwurfl.a. Don't
worry, I'll edit the patch to get rid of it, that's easy.

  - the second one doesn't explain what the problem was and it's hard to
figure it from just the patch itself. Please keep in mind that the
purpose of the commit message is in the first place to quickly explain
why the patch has to exist in the first place. I can help redact a bit
of the message if you're not comfortable with it but I need a bit
of input.

Thanks!
Willy



Re: Haproxy 2.2.3 source

2020-09-09 Thread Willy Tarreau
On Wed, Sep 09, 2020 at 10:03:29PM +0200, Vincent Bernat wrote:
>  ?  9 septembre 2020 19:31 +02, Willy Tarreau:
> 
> >> Feel free to pick this patch if that helps for your builds, I'm going
> >> to backport it to 2.2 once all platforms are happy.
> >
> > All builds are OK now, the commit was backported to 2.2 and the patch
> > can be retrieved here:
> >
> >   http://git.haproxy.org/?p=haproxy-2.2.git;a=commitdiff_plain;h=10c627ab
> 
> All good on Debian:
>  https://buildd.debian.org/status/package.php?p=haproxy=experimental

Great, thank you Vincent for the quick update!

Willy



Re: Haproxy 2.2.3 source

2020-09-09 Thread Willy Tarreau
On Wed, Sep 09, 2020 at 07:20:17PM +0200, Willy Tarreau wrote:
> Feel free to pick this patch if that helps for your builds, I'm going
> to backport it to 2.2 once all platforms are happy.

All builds are OK now, the commit was backported to 2.2 and the patch
can be retrieved here:

  http://git.haproxy.org/?p=haproxy-2.2.git;a=commitdiff_plain;h=10c627ab

Sorry for the mess :-/

Willy



Re: Haproxy 2.2.3 source

2020-09-09 Thread Willy Tarreau
On Wed, Sep 09, 2020 at 05:49:50PM +0200, Willy Tarreau wrote:
> On Wed, Sep 09, 2020 at 05:40:05PM +0200, Vincent Bernat wrote:
> >  ?  9 septembre 2020 16:58 +02, Willy Tarreau:
> > 
> > > Ah I'm really angry because I tested on many platforms, *including* armhf,
> > > but now I'm not seeing it, so either I failed on one test or it depends
> > > on the compiler combination :-(
> > 
> > I am getting it on Debian Unstable (gcc 10.2.0, glibc 2.31), Ubuntu
> > Focal (gcc 9.3.0, glibc 2.31) and Ubuntu Bionic (gcc 7.5.0, glibc 2.27).
> 
> Thanks. Now I'm trying to work on it but can't reproduce the loading
> problem anymore... That's a heisenbug... The trick I'm trying right now
> is to really create a thread that exits on startup. This way we'll be
> certain pthread_exit() was already called once. It's much uglier than
> the first work around but should be more portable and more reliable.

OK, I've pushed this one into -dev:

   f734ebfac ("BUILD: threads: better workaround for late loading of libgcc_s")

I found a more reliable and safer way to address this, exactly the same
as the one we did for backtrace(), which consists in calling the offending
function once at boot. So what we do now very early during initialization
is that we create a new thread that exits using pthread_exit(). This way
we're certain the function is usable before going further, and we don't
care if libgcc_s or whatever else provides the functionality.

I tested it on armhf, aarch64, x86_64, mipsel, ppc64le, and sparc64 with
success. It was also tested on musl which uses a different threading
library and met not issue. On Travis it's OK on all platforms. On Cirrus
and Cygwin it's still in progrss but Centos is already OK so I'm quite
confident.

Feel free to pick this patch if that helps for your builds, I'm going
to backport it to 2.2 once all platforms are happy.

Willy



Re: `ssl_fc_has_early` fetcher and 0rtt

2020-09-09 Thread Willy Tarreau
On Wed, Sep 09, 2020 at 05:43:08PM +0200, Olivier Houchard wrote:
> > I seem to remember there was one but can't find it, so I may have been
> > confused. With this said, it doesn't provide a big information since
> > once the handshake is completed, it's exactly identical to a regular
> > one. But it can be nice for statistics at least.
> > 
> 
> Pretty sure the original version always returned 1 if there were early
> data, but we changed it so that it would return 0 once the handshake was
> done, as we thought it was more useful, that may be what you're thinking
> about.

That indeed tells me something. However I checked if CO_FL_EARLY_DATA
persists, and apparently we kill it very early once we have everything
so it's not as if we could trivially add a new sample fetch function to
report its past status.

Also I seem to remember that we concluded that depending on the timing
and how data were aggregated and processed in the SSL lib, we couldn't
reliably report the use of early data if those were converted very early.

Willy



Re: Haproxy 2.2.3 source

2020-09-09 Thread Willy Tarreau
On Wed, Sep 09, 2020 at 05:40:05PM +0200, Vincent Bernat wrote:
>  ?  9 septembre 2020 16:58 +02, Willy Tarreau:
> 
> > Ah I'm really angry because I tested on many platforms, *including* armhf,
> > but now I'm not seeing it, so either I failed on one test or it depends
> > on the compiler combination :-(
> 
> I am getting it on Debian Unstable (gcc 10.2.0, glibc 2.31), Ubuntu
> Focal (gcc 9.3.0, glibc 2.31) and Ubuntu Bionic (gcc 7.5.0, glibc 2.27).

Thanks. Now I'm trying to work on it but can't reproduce the loading
problem anymore... That's a heisenbug... The trick I'm trying right now
is to really create a thread that exits on startup. This way we'll be
certain pthread_exit() was already called once. It's much uglier than
the first work around but should be more portable and more reliable.

Willy



Re: `ssl_fc_has_early` fetcher and 0rtt

2020-09-09 Thread Willy Tarreau
On Wed, Sep 09, 2020 at 04:57:58PM +0200, William Dauchy wrote:
> > I think it's not easy to reproduce these tests, you need a high enough
> > latency between haproxy and the client so that the handshake is not
> > already completed when you evaluate the rule, and of course you need
> > to make sure the client sends using early data. I don't remember how
> > Olivier used to run his tests but I remember that it was a bit tricky,
> > so it's very possible that you never fall into the situation where you
> > can see the unvalidated early data yet.
> 
> It means my understanding of this fetcher was wrong indeed.
> For me the protection was here:
>   http-request wait-for-handshake if ! METH_GET
> and the fetcher here to log whether it was a 0rtt request or not.
> In reality, it means all our requests have completed the handshake
> when the rule is evaluated (which is surprising looking at the number
> we have).

That seems strange indeed but looking at the code that's what I'm
seeing. Was your access to ssl_fc_has_early placed before or after the
rule above ? If it's after it must indeed report false.

> So maybe we can possibly work on an alternative fetcher to know
> whether this was a 0rtt request? Or is there another way?

I seem to remember there was one but can't find it, so I may have been
confused. With this said, it doesn't provide a big information since
once the handshake is completed, it's exactly identical to a regular
one. But it can be nice for statistics at least.

Willy



Re: Haproxy 2.2.3 source

2020-09-09 Thread Willy Tarreau
On Tue, Sep 08, 2020 at 11:47:25PM +0200, Vincent Bernat wrote:
>  ?  8 septembre 2020 16:13 -04, Alex Evonosky:
> 
> > Just compiling 2.2.3 and getting this reference:
> >
> >
> > /haproxy-2.2.3/src/thread.c:212: undefined reference to
> > `_Unwind_Find_FDE'
> 
> I am getting the same issue on armhf only. Other platforms don't get
> this issue. On this platform, we only get:
> 
>   w   DF *UND*    GLIBC_2.4   __gnu_Unwind_Find_exidx
> 000165d0 gDF .text  000c  GCC_3.0 _Unwind_DeleteException
> d1f6 gDF .text  0002  GCC_3.0 _Unwind_GetTextRelBase
> 00016e1c gDF .text  0022  GCC_4.3.0   _Unwind_Backtrace
> 00016df8 gDF .text  0022  GCC_3.0 _Unwind_ForcedUnwind
> 00016dd4 gDF .text  0022  GCC_3.3 _Unwind_Resume_or_Rethrow
> d1f0 gDF .text  0006  GCC_3.0 _Unwind_GetDataRelBase
> 0001662c gDF .text  0036  GCC_3.5 _Unwind_VRS_Set
> 00016db0 gDF .text  0022  GCC_3.0 _Unwind_Resume
> 000169d8 gDF .text  02ba  GCC_3.5 _Unwind_VRS_Pop
> 00017178 gDF .text  000a  GCC_3.0 _Unwind_GetRegionStart
> 000165cc gDF .text  0002  GCC_3.5 _Unwind_Complete
> 00017184 gDF .text  0012  GCC_3.0 _Unwind_GetLanguageSpecificData
> 000165dc gDF .text  0036  GCC_3.5 _Unwind_VRS_Get
> 000164f0 gDF .text  0004  GCC_3.3 _Unwind_GetCFA
> 00016d8c gDF .text  0022  GCC_3.0 _Unwind_RaiseException
> 
> So, older symbols are:
> 
> 000165d0 gDF .text  000c  GCC_3.0 _Unwind_DeleteException
> d1f6 gDF .text  0002  GCC_3.0 _Unwind_GetTextRelBase
> 00016df8 gDF .text  0022  GCC_3.0 _Unwind_ForcedUnwind
> d1f0 gDF .text  0006  GCC_3.0 _Unwind_GetDataRelBase
> 00016db0 gDF .text  0022  GCC_3.0 _Unwind_Resume
> 00017178 gDF .text  000a  GCC_3.0 _Unwind_GetRegionStart
> 00017184 gDF .text  0012  GCC_3.0 _Unwind_GetLanguageSpecificData
> 00016d8c gDF .text  0022  GCC_3.0 _Unwind_RaiseException
> 
> Moreover, comment says _Unwind_Find_DFE doesn't take arguments, but the
> signature I have in glibc is:
> 
> fde *
> _Unwind_Find_FDE (void *pc, struct dwarf_eh_bases *bases)

Ah I'm really angry because I tested on many platforms, *including* armhf,
but now I'm not seeing it, so either I failed on one test or it depends
on the compiler combination :-(

The principle was just to force to load libgcc_s that tends to cause
aborts on reload for some users in chroots when threads exit, because
pthread_exit() tends to be the first one to require libgcc_s and it's
quite too late.

In the mean time you can probably get away with this:

diff --git a/src/thread.c b/src/thread.c
index 5eb68e33a..167e26e9d 100644
--- a/src/thread.c
+++ b/src/thread.c
@@ -201,7 +201,7 @@ static void __thread_init(void)
exit(1);
}
 
-#if defined(__GNUC__) && (__GNUC__ >= 3) && defined(__GNU_LIBRARY__) && 
!defined(__clang__)
+#if defined(__GNUC__) && (__GNUC__ >= 3) && defined(__GNU_LIBRARY__) && 
!defined(__clang__) && !defined(__arm__)
/* make sure libgcc_s is already loaded, because pthread_exit() may
 * may need it on exit after the chroot! _Unwind_Find_FDE() is defined
 * there since gcc 3.0, has no side effect, doesn't take any argument


But I'd really like to find a *reliable* way to force libgcc_s to be loaded
if available and required (i.e. not if statically built). I thought about
causing a 64/64 bit divide that usually is done via divsi3 and friend but
on x86_64 (where the problem was encountered) it will not do it.

I'm thinking about something: maybe I can have a look around
pthread_getspecific() and pthread_key_create(). I would be very surprised
if they didn't rely on some compiler-specific help from libgcc_s.

I'll keep testing and will get back to you guys.

Willy



Re: `ssl_fc_has_early` fetcher and 0rtt

2020-09-09 Thread Willy Tarreau
Hi William!

On Wed, Sep 09, 2020 at 12:02:03PM +0200, William Dauchy wrote:
> On Wed, Sep 9, 2020 at 10:48 AM William Dauchy  wrote:
> > I'm trying to understand `ssl_fc_has_early` fetcher behavior as I'm
> > unable to find a single request where it returns 1.
> 
> (sorry, forgot to mention, all of these tests were done on v2.2.x)

If I remember well, the principle consists in detecting whether or not
the request was received using TLS early data (0-rtt) before the handshake
was completed. The problem is that early data may trivially be captured
and replayed, so you don't necessarily want to accept all of them, only
replay-safe requests. Typically a login page that is limited to 3
attempts before blocking should not be allowed, but fetching a favicon
is totally safe.

Once the handshake ends you'll know whether it was safe or not, so you
can actually decide to wait on this function to return false to indicate
that the request is complete and not replayed, or just use it to return
a "425 too early" response for certain sensitive resources.

I think it's not easy to reproduce these tests, you need a high enough
latency between haproxy and the client so that the handshake is not
already completed when you evaluate the rule, and of course you need
to make sure the client sends using early data. I don't remember how
Olivier used to run his tests but I remember that it was a bit tricky,
so it's very possible that you never fall into the situation where you
can see the unvalidated early data yet.

Hoping this helps,
Willy



Re: [PATCH v3 0/4] Add support for if-none-match for cache responses

2020-09-08 Thread Willy Tarreau
On Tue, Sep 08, 2020 at 07:14:10PM +0200, William Lallemand wrote:
> On Tue, Sep 08, 2020 at 06:59:07PM +0200, William Lallemand wrote:
> > On Tue, Sep 08, 2020 at 05:51:31PM +0200, Willy Tarreau wrote:
> > > On Tue, Sep 08, 2020 at 05:21:34PM +0200, William Lallemand wrote:
> > > > Also, when reading the RFC about the 304, I notice that they impose to
> > > > remove some of the entity headers in the case of the weak etag, so the
> > > > output is not exactly the same as the HEAD.
> > > > https://tools.ietf.org/html/rfc2616#section-10.3.5
> > > 
> > > Warning, 2616 is totally outdated and must really die. Please use 7234
> > > for caching, 7231 for semantics and 7230 for messaging.
> > > 
> > > Willy
> > 
> > Sorry, I checked on 7230, but somehow I pasted the wrong one :-)
> > 
> > Thanks,
> > 
> > 
> 
> I definitively checked the wrong one, as the RFC now states a SHOULD
> NOT for this, so that's not a requirement:
> 
> https://tools.ietf.org/html/rfc7232#section-4.1

Actually, among the numerous changes that happened between 2616 and 723x,
an important one was the removal of the distinction between entity header,
message headers, representation headers etc, which were quite confusing
since plenty got added in between. The only thing that remains are the
hop-by-hop headers (as opposed to end-to-end), and these are designated
by being referenced in the Connection header.

Willy



Re: [PATCH v3 0/4] Add support for if-none-match for cache responses

2020-09-08 Thread Willy Tarreau
On Tue, Sep 08, 2020 at 05:21:34PM +0200, William Lallemand wrote:
> Also, when reading the RFC about the 304, I notice that they impose to
> remove some of the entity headers in the case of the weak etag, so the
> output is not exactly the same as the HEAD.
> https://tools.ietf.org/html/rfc2616#section-10.3.5

Warning, 2616 is totally outdated and must really die. Please use 7234
for caching, 7231 for semantics and 7230 for messaging.

Willy



[ANNOUNCE] haproxy-2.2.3

2020-09-08 Thread Willy Tarreau
ing in 
"replace-path" action"
  BUG/MEDIUM: doc: Fix replace-path action description
  MINOR: http-rules: Add set-pathq and replace-pathq actions
  MINOR: http-fetch: Add pathq sample fetch
  REGTEST: Add a test for request path manipulations, with and without the 
QS
  MINOR: arg: Use chunk_destroy() to release string arguments
  BUG/MEDIUM: dns: Don't store additional records in a linked-list
  BUG/MEDIUM: dns: Be sure to renew IP address for already known servers
  MINOR: server: Improve log message sent when server address is updated

Gilchrist Dadaglo (5):
  BUG/MAJOR: contrib/spoa-server: Fix unhandled python call leading to 
memory leak
  BUG/MINOR: contrib/spoa-server: Ensure ip address references are freed
  BUG/MINOR: contrib/spoa-server: Do not free reference to NULL
  BUG/MINOR: contrib/spoa-server: Updating references to free in case of 
failure
  BUG/MEDIUM: contrib/spoa-server: Fix ipv4_address used instead of 
ipv6_address

Jerome Magnin (1):
  DOC: ssl-load-extra-files only applies to certificates on bind lines

Tim Duesterhus (4):
  DOC: cache: Use '' instead of '' in error message
  MINOR: cache: Reject duplicate cache names
  MINOR: Commit .gitattributes
  CLEANUP: Update .gitignore

William Dauchy (2):
  BUG/MINOR: spoa-server: fix size_t format printing
  DOC: spoa-server: fix false friends `actually`

William Lallemand (11):
  BUG/MINOR: ssl: fix memory leak at OCSP loading
  BUG/MEDIUM: ssl: memory leak of ocsp data at SSL_CTX_free()
  BUG/MINOR: snapshots: leak of snapshots on deinit()
  BUG/MEDIUM: ssl: fix the ssl-skip-self-issued-ca option
  BUG/MINOR: ssl: ssl-skip-self-issued-ca requires >= 1.0.2
  BUG/MEDIUM: ssl: never generates the chain from the verify store
  BUG/MEDIUM: ssl: fix ssl_bind_conf double free w/ wildcards
  BUG/MEDIUM: ssl: crt-list negative filters don't work
  BUG/MINOR: startup: haproxy -s cause 100% cpu
  BUG/MEDIUM: ssl: check OCSP calloc in ssl_sock_load_ocsp()
      BUG/MEDIUM: ssl: does not look for all SNIs before chosing a certificate

Willy Tarreau (11):
  SCRIPTS: git-show-backports: make -m most only show the left branch
  SCRIPTS: git-show-backports: emit the shell command to backport a commit
  BUG/MINOR: stats: use strncmp() instead of memcmp() on health states
  BUG/MEDIUM: htx: smp_prefetch_htx() must always validate the direction
  BUG/MINOR: reload: do not fail when no socket is sent
  BUILD: tools: include auxv a bit later
  BUILD: task: work around a bogus warning in gcc 4.7/4.8 at -O1
  BUG/MINOR: threads: work around a libgcc_s issue with chrooting
  BUILD: thread: limit the libgcc_s workaround to glibc only
  CLEANUP: dns: remove 45 "return" statements from 
dns_validate_dns_response()
  BUG/MEDIUM: mux-h1: always apply the timeout on half-closed connections

---



Re: [PATCH] CLEANUP: Update .gitignore

2020-09-06 Thread Willy Tarreau
On Sun, Sep 06, 2020 at 06:34:12PM +0200, Tim Duesterhus wrote:
> This excludes the currently tracked files from being ignored.

Good spot, now merged, thanks!
Willy



Re: [PATCH] MINOR: Commit .gitattributes

2020-09-05 Thread Willy Tarreau
On Sat, Sep 05, 2020 at 12:46:11PM +0200, Tim Duesterhus wrote:
> Commit .gitattributes to the repository to allow GitHub downloads to
> properly fill in the SUBVERS and VERDATE files. It also prevents the
> engineer in charge of the release to forget creating it when a new branch
> is released.
> 
> see https://www.mail-archive.com/haproxy@formilux.org/msg38107.html
> 
> No functional change, may be backported everywhere.

Cool, thank you for this one, Tim! Now applied.
Willy



Re: Large difference between response time as measured by nginx and as measured by HAProxy

2020-08-27 Thread Willy Tarreau
On Thu, Aug 27, 2020 at 05:39:34PM +0200, Tim Düsterhus, WoltLab GmbH wrote:
> Willy and I discussed this in private. My understanding is that this
> issue is caused by an inherent limitation of H2 with regard to head of
> the line blocking.
> 
> In simple terms:
> 
> If the stuff at the beginning of the the backend connection belongs to a
> client that is very slowly or not reading the stuff within its client
> connection then everything that's within that backend connection will
> stall until the slow client finally gets around to reading the stuff
> thats intended for it.
> 
> Basically a very slow client can block *other* clients that are
> unfortunate and share the backend connection.

And in order to complete this, one theorical workaround would consist
in using very small initial windows, but that would not be practical.
A test just with the buffer size didn't help.

> Our resolution is simple: We disable H2 for the backend connections and
> use good old H1. We don't *need* end to end H2.

Also that raised an idea about adding a new "http-reuse" mode or slightly
changing the way "never" works for H2, which would consist in only grouping
together requests from the same session (client connection). It would be
what "never" already does, except that after the last response, instead of
closing the connection, it would be sent back to the idle pool for any
other client to use. It would not be perfect but would keep streams in
the same order for a given client based on their performance.

Willy



Re: Is it possible for a http frontend using a tcp backend?

2020-08-27 Thread Willy Tarreau
On Thu, Aug 27, 2020 at 09:19:24PM +0800, Hongyi Zhao wrote:
> On Thu, Aug 27, 2020 at 9:51 AM Willy Tarreau  wrote:
> >
> > Hi,
> >
> > On Mon, Aug 17, 2020 at 10:09:36PM +0800, Hongyi Zhao wrote:
> > > Hi,
> > >
> > > Is it possible for the following settings in haproxy:
> > >
> > > A http frontend listening on the localhost:18887 which uses a backend
> > > consisting of several remote socks5 servers?
> >
> > Not exactly, however you can use a socks4 encapsulation when reaching a
> > server. That doesn't change the http mode on the backend, it just inserts
> > a socks4 "header" on the connection to the server.
> 
> Are there examples/tutorials for this case?

I think that if you look for "socks4" in the configuration manual you'll
find at least one place indicating the syntax to use on the server line.
>From memory, it should just be one word on the server line to enable this
encapsulation.

Willy



Re: Large difference between response time as measured by nginx and as measured by HAProxy

2020-08-27 Thread Willy Tarreau
On Thu, Aug 27, 2020 at 11:45:13AM +0200, Tim Düsterhus, WoltLab GmbH wrote:
> Willy,
> 
> Am 27.08.20 um 10:29 schrieb Willy Tarreau:
> >> I now have the trace results and my HAProxy log where I can correlate
> >> the slow requests using the timestamp and path. Unfortunately the trace
> >> does not appear to contain the unique-id of the request.
> > 
> > Sadly, due to the way HPACK works we can't decode the headers being sent.
> > There's already a ugly hack being done for the URI and method. I seem to
> > remember we peak them from the HTX's start line, but it could not reliably
> > be done for all headers. I'd still like to improve this :-/
> 
> The unique-id is a property of the the stream (struct stream).unique_id.
> So that one should technically be available, no? I would also be happy
> with using the internal request_counter (the %rt log value), because
> that one is part of my unique ID format anyway.

In the stream output yes, but not in the h1 or h2 output, which do not
necessarily have a stream above them (e.g.: checks). However if we had it
in the stream output, enabling traces at both levels would be enough to
trace them. What I'm missing in both is the conn_stream, which joins the
h1/h2 stream and the application layer stream, but that shouldn't be hard
to add. For the request counter, same problem. I'm trying to think of an
identifier that would be present everywhere but can't think of any. Maybe
we could try to attack the issue the other way around and enable the
production of a trace_id when traces are enabled, and this trace_id would
be passed as much as possible between layers and could also be logged. The
problem with muxed connections is that it would have to be generated on a
new request, and that for a number of issues, that's already too late. Thus
maybe a composite trace ID would help instead, i.e. front connection ID,
front stream ID, back stream ID, back connection ID. Missing some of these
parts would then not be a problem because it would just mean that we didn't
reach that stage.

> > You can see two h2c connections, one is on the frontend (hence the 'F')
> > and the other one is on the backend. The h2s is the H2 stream, unique to
> > a request (though it comes from a pool, so it will be reused for new
> > streams on any connection once this one is finished). Thus bu just
> > filtering on the h2 and correlating with what you see in the logs (with
> > your source ports), and the date of course, you can quite reliably match
> > connections, hence requests.
> 
> Okay, that works sufficiently well for me.

Cool! I do imagine that it has been painful, so thanks for doing it!

> I've extracted the relevant
> parts, redacted any private information and will check with a coworker
> to see if I missed anything. I'll send you the traces + logs in private
> after the contents have been ACKed :-)

No problem.

> There's even a PHP request within there with an 800ms difference between
> nginx and HAProxy.

Ah excellent! We'll have something to study at least. I'm suddenly thinking
about something: there is a known case of head-of-line blocking in H2 when
stream windows are larger than buffers: a slow stream could cause some of
its data to block into the connection's buffer for some time. It could very
well be what's happening to you: merge two clients on the same backend
connection, have one slowly read its data and sometimes the other may
experience some hiccups. This should be significantly reduced by reducing
the number of concurrent streams however. One way to alleviate this is to
reduce the tune.h2.initial-window-size to 16kB but it's used on both sides
so it would also mean very slow posts. The other solution is to increase
the default buffer size to 64kB so that all of an H2 incoming data's stream
window can be flushed into a client's buffer. It may increase your memory
consumption a bit if you're mostly dealing with slow clients. A test could
consist in setting it to 32kB which will generally be enough as you have one
buffer in the mux and another one in the channel ;-)

> > Do not hesitate to let me know if you think of something that can be
> > improved, traces are still quite young and under exploited, I'm quite
> > open to studying potential improvements.
> 
> Nothing else for now. grepping for the pointers worked sufficiently
> well. The bigger issue is identifying the request in question in the
> first place.

Yes I know, that's always the same for me as well. That's the reason
why the unique-id feature was implemented so long ago!

> I was lucky that the file in this case was fairly
> low-trafficked so that I was able to accurately identify it based on the
> timestamp and path.

Usually with patience it does work. There's still a lot of room for
improvement but I often tend to imagine what it would have been without
and that helps me be tolerant :-)

Regards,
Willy



Re: Large difference between response time as measured by nginx and as measured by HAProxy

2020-08-27 Thread Willy Tarreau
Hi Tim,

On Thu, Aug 27, 2020 at 10:05:08AM +0200, Tim Düsterhus, WoltLab GmbH wrote:
> My definition of reproducible is "I can write up a list of steps that
> makes the issue happen somewhat reliably".

OK, that's fine by me :-)

> This morning I re-enabled H2 for the backend communication and then
> plugged in the tracing. In the half of an hour since I reenabled H2 I'm
> seeing 160 static requests taking longer than 45ms, with the worst ones
> being > 800ms.

So there's definitely something related to H2.

> I now have the trace results and my HAProxy log where I can correlate
> the slow requests using the timestamp and path. Unfortunately the trace
> does not appear to contain the unique-id of the request.

Sadly, due to the way HPACK works we can't decode the headers being sent.
There's already a ugly hack being done for the URI and method. I seem to
remember we peak them from the HTX's start line, but it could not reliably
be done for all headers. I'd still like to improve this :-/

It's possible to also log the streams (at level developer) and get the
full requests/responses but they will not necessarily correlate with the
H2 traffic (though it could give you high enough confidence).

> Can I somehow filter down the trace file to just the offending requests
> + possible the requests within the same H2 connection? For privacy
> reasons I would not like to provide the full trace log, even if it's in
> a non-public email.

Of course I understand. You can rely on the "h2c" pointer that appears
in the trace, it's the H2 connection. For example below, I'm forwarding
an H2 request from a frontend to a backend:

  <0>2020-08-27T10:23:26.117936+02:00 [01|h2|0|mux_h2.c:2541] rcvd H2 request  
: h2c=0x7f59f0026880(F,FRH) : [1] H2 REQ: GET http://127.0.0.1:4446/ HTTP/2.0
  <0>2020-08-27T10:23:26.128922+02:00 [01|h2|0|mux_h2.c:5152] sent H2 request : 
h2c=0x7f59f00382a0(B,FRH) h2s=0x7f59f0037fa0(1,IDL) : [1] H2 REQ: GET 
http://mail.google.com/ HTTP/2.0
  <0>2020-08-27T10:23:26.149012+02:00 [01|h2|0|mux_h2.c:2632] rcvd H2 response 
: h2c=0x7f59f00382a0(B,FRH) : [1] H2 RES: HTTP/2.0 301 
  <0>2020-08-27T10:23:26.149041+02:00 [01|h2|0|mux_h2.c:4790] sent H2 response 
: h2c=0x7f59f0026880(F,FRH) h2s=0x7f59f0027dc0(1,HCR) : [1] H2 RES: HTTP/2.0 
301 

You can see two h2c connections, one is on the frontend (hence the 'F')
and the other one is on the backend. The h2s is the H2 stream, unique to
a request (though it comes from a pool, so it will be reused for new
streams on any connection once this one is finished). Thus bu just
filtering on the h2 and correlating with what you see in the logs (with
your source ports), and the date of course, you can quite reliably match
connections, hence requests.

Do not hesitate to let me know if you think of something that can be
improved, traces are still quite young and under exploited, I'm quite
open to studying potential improvements.

Willy



Re: New Open source SNMP MIB and agent for HAProxy:

2020-08-26 Thread Willy Tarreau
Hi Malcolm,

On Wed, Aug 26, 2020 at 02:43:34PM +0100, Malcolm Turnbull wrote:
> One of our best Geeks Peter Statham has put a fair bit of work into a
> new agent and MIB for HAProxy to make the lives of some of our
> customers easier...
> 
> He's somewhat nervous that the community might rip his code to shreds...
> So please be nice with your comments!
> 
> But it would be great to get constructive feedback + any tips and
> advice on the best monitoring/graphing tools to use with HAProxy SNMP
> data:
> 
> https://www.loadbalancer.org/blog/loadbalancer-org-releases-open-source-snmp-mib-and-agent-for-haproxy/

Thanks for sharing this. Peter should have a look at "show stats typed"
which provides a lot of information for each metric, indicating its type,
size, how to aggregate it etc. This typically allows to know if it's a
counter/gauge/max/avg etc. This could significantly simplify the switch/cases
parts. The output is larger however (1 line per metric) but would allow the
code to automatically adapt to new metrics as they are added.

Cheers,
Willy



Re: [*EXT*] http/2 fine tuning

2020-08-26 Thread Willy Tarreau
On Tue, Aug 25, 2020 at 03:32:21PM +0200, Ionel GARDAIS wrote:
> That's improvement ! 
> 
> Are there similar settings in haproxy to get these numbers ? 

It's difficult to respond, because what the article shows is that by
default there was a performance limitation which was essentially due
to how buffering works in nginx. It doesn't work the same way in
haproxy so the corner cases will be different. Typically in our case
the H2 communication with the client is totally independent of the
communication with the server, because that's performed at different
places. The client will gets its WINDOW_UPDATE as payload is consumed
(even incomplete frames).

By default we also use the default 64kB stream window size, but this
can be changed via tune.h2.initial-window-size. The default buffer
size is 16kB but can be increased using bufsize. There's no dynamic
buffer sizing, however on the send side we use a ring of buffers (32
by default). This means that in practice we can read one buffer,
pass it to the other side and continue to read, which effectively
corresponds to having up to 33 times the request buffer even if the
server doesn't read as fast. So the model is a bit different but
could deserve being compared. In any case we're not changing the
buffer size on the fly nor allocating larger buffers for slower
clients.

Regards,
Willy



Re: Server sent fatal alert: decode_error

2020-08-26 Thread Willy Tarreau
Hello Arnall,

On Mon, Aug 17, 2020 at 11:29:32PM +0200, Arnall wrote:
> Hello everyone,
> 
> i've made a tls test on ssllabs, and in the report i can see we have this
> error : "Server sent fatal alert: decode_error" in the hanshake simulation
> part.
> 
> it happens essentially with recent platform : Android 8.1/9.0, Chrome
> 69/70/80, Firefox 73, OpenSSL 1.1.0k/1.1.1c, Safari 12.
> 
> Our configuration concerning tls is :
> 
> Haproxy 2.0.14, certificates are RSA
> 
> global
> 
>     tune.ssl.default-dh-param 2048
>     ssl-default-bind-ciphers 
> ECDH+AESGCM:DH+AESGCM:ECDH+AES256:DH+AES256:ECDH+AES128:DH+AES:ECDH+3DES:DH+3DES:RSA+AESGCM:RSA+AES:RSA+3DES:!aNULL:!MD5:!DSS
>     ssl-default-bind-options no-sslv3 no-tls-tickets
>     tune.ssl.cachesize 90 # default to 2
>     tune.ssl.lifetime 3600 # default to 300 seconds
> 
> frontend web
> 
>     bind *:80
>     bind *:443 ssl crt /etc/haproxy/certs/site1.pem crt
> /etc/haproxy/certs/site2.pem crt /etc/haproxy/certs/site3.pem crt
> /etc/haproxy/certs/site4.pem
> 
> 
> And we effectively have a certain number of :  'web/2: SSL handshake
> failure' in the logs.
> 
> Any advice ?

No particular idea, but have you tried disabling some of the settings
in the global section to see if they have any effect ? We could imagine
that something fails the handshake for any reason. This would allow to
narrow the cause down to a specific setting.

Willy



Re: Is it possible for a http frontend using a tcp backend?

2020-08-26 Thread Willy Tarreau
Hi,

On Mon, Aug 17, 2020 at 10:09:36PM +0800, Hongyi Zhao wrote:
> Hi,
> 
> Is it possible for the following settings in haproxy:
> 
> A http frontend listening on the localhost:18887 which uses a backend
> consisting of several remote socks5 servers?

Not exactly, however you can use a socks4 encapsulation when reaching a
server. That doesn't change the http mode on the backend, it just inserts
a socks4 "header" on the connection to the server. We don't support socks5
however, I seem to remember that it requires more than just a prefix in
each direction.

Hoping this helps,
Willy



Re: [RFC PATCH] MAJOR: ssl: Support for validating backend certificates with URI SANs (subjectAltName)

2020-08-26 Thread Willy Tarreau
Hi Teo,

On Wed, Aug 26, 2020 at 02:44:01PM +0200, Teo Klestrup Röijezon wrote:
> Currently HAProxy only supports DNS-style SANs[0]. This can be a problem when 
> integrating with external systems. For example, systems that use SPIFFE 
> authentication (such as Istio) store SPIFFE IDs[1] in the certificate's SAN, 
> but formatted as a URI.
> 
> As a quick prototype I was able to piggyback onto the existing `verifyhost` 
> keyword, but this has a few drawbacks. In particular:
> 
> 1. HAProxy currently overrides `verifyhost` with any SNI (Server Name 
> Indication) name, if used (not that this behaviour makes sense to me, since 
> this special-casing doesn't actually enable anything)

I don't understand what you mean here in that it does not make sense to
you. Actually it's not even about overriding verifyhost, it's more that
we match that the requested host (if any) is indeed supported by the
presented certificate. The purpose is to make sure that the connection
is not served by a server presenting a valid cert which doesn't match
the authority we're asking for. And if we don't send any servername,
then we can still enforce the check against a hard-coded servername
presented in verifyhost.

> 2. Wildcards would behave differently (if supported at all)
> 3. It is dangerous to allow wildcarding DNS names into URIs (*.haproxy.org 
> probably shouldn't match https://evilcorp.com/.haproxy.org)
> 4. It may be desirable to allow more than one URI (for example, during a 
> migration to a new SPIFFE ID)

How is this used, usually ? I had never heard of such features with these
URIs being advertised instead of server names (but I must not be taken as
a reference about TLS as I'm mostly clueless about it). I'm seeing that
in your patch you're checking a type against GEN_URI, so I guess that there
is an explicit type for such alt names. Thus we could imagine having a new
keyword to enumerate a list of valid URIs to match against. This would
completely eliminate the risk to match against other fields by accident.
But do these have to be hard-coded or may they be determined dynamically
(from SNI or anything else for example) ?

If you could provide an example sequence where this is used to illustrate
the use case, it would be useful for those who, like me, are not aware of
this feature. Also, one thing which is not clear to me is, should this check
be necessary or sufficient to validate a cert ? I.e. if we have a matching
servername, and a configured URI, should we check that this URI is valid
in addition to the servername, or as an alternative ?

> I attached a prototype patch, but because of the previous drawbacks it 
> shouldn't be merged as is. I just wanted a point to jump off of.

Sure, and thanks for providing something to discuss about, it's always
easier to discuss around a piece of code :-)

Thanks,
Willy



Re: Large difference between response time as measured by nginx and as measured by HAProxy

2020-08-26 Thread Willy Tarreau
On Wed, Aug 26, 2020 at 05:38:52PM +0200, Tim Düsterhus, WoltLab GmbH wrote:
> There are even worse ones:
> 
> nginx:
> 
> > Aug 26 09:56:50 *snip* abd21141cdb4[5615]: 5F4631E22E407 "GET *snip*.gif 
> > HTTP/2.0" 200 139 0.000
> 
> HAProxy:
> 
> > Aug 26 09:57:17 *snip* haproxy[3066]: *snip*:57086 
> > [26/Aug/2020:09:56:50.265] https~ *snip*/nginx 0/0/0/27340/27340 200 335 - 
> > -  82/82/17/17/0 0/0 {*snip*|*snip*} {*snip*|} "GET *snip*.gif 
> > HTTP/2.0" 5F4631E22E407
> 
> Yes, that's 27340ms. I don't have any more details regarding the
> connection reuse for that, unfortunately :-(

Ouch!

> FWIW: I disabled HTTP/2 for the backend connection after sending my
> initial email and I am not seeing anything worse than 45ms for static
> files any more.

OK, that already narrows the issue down a bit.

> I just arbitrarily checked 3 PHP requests and they match. The static
> ones *mostly* do as well. It's just that static files taking ages stick
> out like a sore thumb, while PHP tends to be all over the place anyway.
> It basically was a "grep for static file request" and then "halog -pct"
> and wonder why there's a large number at the bottom.

I can understand indeed.

> See above: HTTP/1 did not show the issue within the last 4 hours, so I
> would rule out nginx / the disk being the culprit here.

That's perfect, it definitely rules out that part indeed.

> Static files are served from fast local SSDs with plenty of page cache.
> I don't expect the disk access to bottleneck up to the 3-digit
> millisecond range.

OK. I've been hit in the past by machines swapping out while uploading
some logs over HTTP, and taking ages to even allocate network buffers
after that! They used to slowly recover during the day and start their
madness again next night. It was even causing packet losses causing
retransmissions! That's why I asked :-)

> As I said: It's not really reproducible.

Yeah but we all have a definition of "not really reproducible". As you
consider that it doesn't happen in HTTP/1 after only 4 hours, for me
this means you expect it to usually happen at least once in 4 hours.
If the load is not too high (no more than a few thousands requests per
second) and you have some disk space, the h2 trace method could prove
to be useful once you have isolated a culprit in the logs. You could
even gzip the output on the fly to take less space, they compress very
well.

> I'm mostly seeing it after the
> fact. I can't even tell if HAProxy's measuring or nginx' measuring is
> correct (i.e. which latency the client actually experienced).

Unless there's a bug causing haproxy to report a random value (or a timer
related to the previous request for example and would explain the large
values), I'd be tempted to say that it's likely the client experiences
it :-/

> However tcpdump definitely is not going to be fun, because it's all TLS
> with PFS ciphers.

That's what I was suspecting, sadly, but preferred to ask.

> > Another thing you can try is to artificially limit
> > tune.h2.max-concurrent-streams just in case there is contention in
> > the server's connection buffers. By default it's 100, you can try with
> > much less (e.g. 20) and see if it seems to make any difference at all.
> > 
> 
> The fact that disabling HTTP/2 helps could indicate that something like
> this is the case here. I'll try that tomorrow, thanks.

You're welcome!

Willy



Re: Logging using %HP (path) produce different results with H1 and H2

2020-08-26 Thread Willy Tarreau
On Wed, Aug 26, 2020 at 06:31:21PM +0300, Ciprian Dorin Craciun wrote:
> On Wed, Aug 26, 2020 at 6:29 PM Willy Tarreau  wrote:
> > > Then, still related to logging, might I add a feature request to just
> > > use "raw" lines over UDP or TCP, instead of SysLog?  (Just as we have
> > > now support for `stdout`, but with network support.)
> >
> > You already have it. I'm doing this:
> >
> > log 127.0.0.1:5514 format raw daemon
> > log-format "%[uuid]"
> >
> > And netcat shows me only the uuid with no header nor anything. So
> > technically speaking I don't think you're missing anything for this.
> 
> 
> This is nice.  I'll have to give it a try.  (I thought that the `raw`
> applied only to `stdout`.

I think it used to be the case for quite some time, probably that it's
become available with 2.2 and the rework needed for TCP servers.

> However we are missing the TCP support.  :)

You have it in 2.2. You need to declare a ring buffer, which itself is
chained to the TCP log server. Ah I've just found you an example here:

  https://www.haproxy.com/blog/announcing-haproxy-2-2/#syslog-over-tcp

And in 2.3 we're even starting to receive syslog to forward and load
balance it (including with format conversion and UDP->TCP).

Cheers,
Willy



Re: Logging using %HP (path) produce different results with H1 and H2

2020-08-26 Thread Willy Tarreau
On Wed, Aug 26, 2020 at 06:17:11PM +0300, Ciprian Dorin Craciun wrote:
> On Wed, Aug 26, 2020 at 1:08 PM Willy Tarreau  wrote:
> > > Also what would be extra useful, especially for debugging and perhaps
> > > security, is to add an `http-request|http-response log 
> > >  if ` that would allow adding additional log
> > > lines if a certain condition is met.
> >
> > We cannot realistically "add more lines" to a request, however deciding
> > to emit one line at the instant the rule is processed shouldn't be hard
> > at all, and I definitely see the value there, as you say, both for
> > debugging and for security alerts.
> 
> 
> Yes, you are right, I don't expect such a statement to add a new line
> to the buffer, but to immediately format a log line and submit it.
> 
> (This also aids in debugging, because if the same log line is put in
> multiple places of the `http-request` chain, one could see how a
> certain header has changed, and it could also help in generating a log
> line the moment the request was parsed, as opposed to waiting for the
> response to be processed.)

Actually that's fun, because among the features for which we wanted
to have the log profiles was the ability to have multiple log points,
precisely for request then response typically. It appeared as being a
little bit of a mess and I don't think anyone made any progress in this
area. But with your idea we could possibly delegate this choice to the
user. One hook I wanted to support was the "server connection established",
which will not be covered, but now that we have the after-response rules,
that at least gives request, response and end of transfer, which are the
most valuable ones.

> Then, still related to logging, might I add a feature request to just
> use "raw" lines over UDP or TCP, instead of SysLog?  (Just as we have
> now support for `stdout`, but with network support.)

You already have it. I'm doing this:

log 127.0.0.1:5514 format raw daemon
log-format "%[uuid]"

And netcat shows me only the uuid with no header nor anything. So
technically speaking I don't think you're missing anything for this.

Willy



Re: Large difference between response time as measured by nginx and as measured by HAProxy

2020-08-26 Thread Willy Tarreau
Hi Tim,

On Wed, Aug 26, 2020 at 12:59:55PM +0200, Tim Düsterhus, WoltLab GmbH wrote:
> Within the HAProxy logs we're sometimes seeing largish 'Tr' values for
> static file requests. For example 734ms for the example request below.

That sounds huge. I could have imagined a 200ms time, possibly indicating
that somewhere between the two, an extra MSG_MORE was present and prevented
a request and/or response from leaving the machine. But 734ms doesn't
correspond to anything usual.

> The two requests in nginx' log:
> 
> > Aug 26 10:23:21 *snip* abd21141cdb4[5615]: 5F46381900A0 "GET *snip* 
> > HTTP/2.0" 200 27049 0.070
> > Aug 26 10:23:26 *snip* abd21141cdb4[5615]: 5F46381E00F5 "GET *snip*.gif 
> > HTTP/2.0" 200 6341 0.000
> 
> Note how the timing for the PHP request matches up what HAProxy was
> seeing: Roughly 70ms / 0.070s. The timing for the GIF request is *way*
> off, though.

Stupid question, do the PHP request *always* match ? I'm asking because
one point is that haproxy cannot tell the difference between the two.
However the static one very likely responds instantly, which could justify
a less common code path being taken and a bug to be triggered.

However static files might also cause some disk access, and not knowing
anything about the environment, I'd say it's also possible that the whole
nginx process gets frozen for the time it takes open() to respond, and if
for whatever reason there are lots of I/Os and it takes that long, it might
be possible that it doesn't notice it, depending where the total time is
retrieved.

But it could definitely be a bug in the connection reuse or mux code, or
course. It's just that I have no idea what could cause this for now.

If you see it often enough, it might be useful to enable h2 traces and
watch them on the CLI, or even run a tcpdump between the two to observe
what's going on. The traces might be easier as a first step as you'll
see already decoded stuff. For example:

  (echo "trace h2 sink buf0; trace h2 level user; trace h2 verbosity \
   complete; trace h2 start now;show events buf0 -w -n" ; cat ) | \
  socat - /path/to/socket > /path/to/logs

Once you're confident you got it, you can disable all traces using the
short form "trace 0".

Another thing you can try is to artificially limit
tune.h2.max-concurrent-streams just in case there is contention in
the server's connection buffers. By default it's 100, you can try with
much less (e.g. 20) and see if it seems to make any difference at all.

Hoping this helps,
Willy



Re: Logging using %HP (path) produce different results with H1 and H2

2020-08-26 Thread Willy Tarreau
Hi Ciprian,

On Wed, Aug 26, 2020 at 12:52:52PM +0300, Ciprian Dorin Craciun wrote:
> If we are to talk about logging, please make it easy to have valid
> JSON log lines.

Yep, good point!

> I know there is the `%[something(),json()]` that works as expected;
> however there are many (especially old) log items (say `%ST`) that
> sometimes expand to a number, and sometimes to a `-`, which basically
> means one has to always treat everything as strings, and hope the
> actual value doesn't contain a quote or JSON escape codes...  In fact,
> to keep things simple, I would suggest just adding new functions
> (fetch samples as they are called) that expose all items available in
> the `Custom log format` section  (some are available, some are not
> from what I remember);  these should expand to an empty string if they
> are not available (as opposed to a `-`).

This is exactly what was planned to be done in the background. I even
think I created an issue for this a long time ago, so that anyone can
come in and progressively convert some of these tags to sample fetch
functions that would allow to do this. I must confess that after that
long a time, I mostly remembered about the complaint not to be able
to operate on these values but I almost forgot about the problem of
not being able to actually re-encode them!

> Also what would be extra useful, especially for debugging and perhaps
> security, is to add an `http-request|http-response log 
>  if ` that would allow adding additional log
> lines if a certain condition is met.

We cannot realistically "add more lines" to a request, however deciding
to emit one line at the instant the rule is processed shouldn't be hard
at all, and I definitely see the value there, as you say, both for
debugging and for security alerts.

There's something I've been wanting for a while which would be log profiles
(pre-defined and reusable log-format directives) that would significantly
help with such situations as you probably don't want to duplicate the
exact same format all the time. As such I'd rather first define how we
want to reference them, then make sure that the suggestion above can be
done in a forward compatible way.

These are good ideas that will fuel a long feature request :-)

Thanks!
Willy



Re: Logging using %HP (path) produce different results with H1 and H2

2020-08-25 Thread Willy Tarreau
Hi Pierre,

On Mon, Aug 24, 2020 at 08:17:05AM +, Pierre Cheynier wrote:
> On Fri, Aug 21, 2020 at 8:11 PM William Dauchy  wrote:
> 
> So awesome to get the first response from your direct colleague :)
> 
> > I believe this is expected; this behaviour has changed since v2.1 though.
> 
> Indeed, we don't use this logging variable since a long time, so I'm not 
> really able to confirm if this is so new.
> Anyway, I understand this is related to handling h2 and its specifics, still 
> I think there should be something to fix (in one way or the other) to get 
> back to a consistent/deterministic meaning of %HP (and maybe in other places 
> where this had an impact).
> 
> Willy, any thought about this?

What is certain is that I don't want to start logging a false or misleading
information. The issues stems from browsers using a different way to represent
a resource in a request depending on HTTP versions (and it's not their fault,
it's the recommended way to do it). In HTTP/1.x we used to have :
  - relative URIs made of a path only with a separate Host header
  - absolute URIs made of a scheme + host + path and the Host header
having to be repeated.

In HTTP/2 this has been greatly simplified and only the second full URI
was kept by default. In order to save servers from having to parse these
elements, they are sent already split into:
  - a ":scheme" pseudo header holding the scheme of the URI
  - a ":authority" pseudo header holding the equivalent of the host part
of the URI
  - a ":path" pseudo header holding the path part of the URI

And no Host header is needed anymore.

Thus an HTTP/2 request effectively "looks like" an HTTP/1 request using
an absolute URI. What causes the mess in the logs is that such HTTP/1
requests are rarely used (most only for proxies), but they are perfectly
valid and given that they are often used to take routing decisions, it's
mandatory that they are part of the logs. For example if you decide that
every *url* starting with "/img" has to be routed to the static server
and the rest to the application, you're forgetting that "https://foo/img/;
is valid as well and will be routed to the application. That's what I do
not want to report fake or reconstructed information in the logs.

In 1.8, what happened when we introduced H2 is that it was directly turned
into HTTP/1.1 before being processed and that given that we didn't support
server-side H2, the most seamless way to handle it was to just replace
everything with origin requests (no authority). That remained till 2.0
since it was not really acceptable to imagine that depending on whether
you enabled HTX or not you'd get different logs for the exact same request.
But now we cannot cheat anymore, it had caused too much trouble already

What I understand however is that it's possible that we need to rethink
what we're logging. Maybe instead of logging the URI by default (and missing
the Host in HTTP/1) we ought to instead log the scheme, authority and path
parts. These are not always there (scheme or authority in H1) but we can
log an empty field like "-" in this case.

We cannot realistically do that in the default "httplog" format, but we
can imagine a new default format which would report such info (htxlog?),
or maybe renaming httplog to http-legacy-log and changing the httplog's
default. We should then consider this opportunity to revisit certain
fields that do not make sense anymore, like the "~" in front of the
frontend's name for SSL, the various timers that need to report idle
and probably user-facing time instead of just data, etc.

There was something important I've been wanting for a few versions, which
was to have named log formats that we could declare in a central place and
use everywhere. It would tremendously help here. I know it can be done
using environment variables declared in the global section but I personally
find this ugly.

So I think it's the right place to open such a discussion (what we should
log and whether or not it loses info by default or requires to duplicate
some data while waiting for the response), so that we can reach a better
and more modern solution. I'm open to proposals.

Cheers,
Willy



Re: HAProxy 2.2.2 CE issue report

2020-08-25 Thread Willy Tarreau
On Tue, Aug 25, 2020 at 10:23:27AM +0200, Vincent Bernat wrote:
>  ? 24 août 2020 21:59 +03, Milen Simeonov:
> 
> > frontend fe_main
> > bind 127.0.0.1:443 ssl crt-list /etc/haproxy/certs/websites.crt_list
> 
> I am not able to reproduce. The configuration is missing a path to a
> certificate. Does it also crash if you don't provide a crt-list?

FWIW I couldn't reproduce it either and am also suspecting something
odd in the crt-list.

Willy



Re: [PATCH] MEDIUM: cfgparse: Emit hard error on truncated lines

2020-08-19 Thread Willy Tarreau
On Wed, Aug 19, 2020 at 11:38:51AM +0200, William Lallemand wrote:
> On Wed, Aug 19, 2020 at 11:19:02AM +0200, Willy Tarreau wrote:
> > On Wed, Aug 19, 2020 at 10:56:59AM +0200, Tim Düsterhus wrote:
> > > William,
> > > 
> > > Am 19.08.20 um 10:54 schrieb William Lallemand:
> > > > I think it broke
> > > > reg-tests/stick-table/converteers_ref_cnt_never_dec.vtc:
> > > > 
> > > > https://travis-ci.com/github/haproxy/haproxy/jobs/375236964
> > > > 
> > > > But I can't reproduce localy.
> > 
> > Strangely I didn't reproduce it this morning when applying them, so
> > either it depends on the vtest version or I did something wrong in
> > the test. Here I'm seeing fail 100%.
> > 
> 
> That's surprising because vtest is built from the git during the CI
> run. I tried with the latest vtest commit locally and I can't reproduce.
> :/

I retested at home on my other machine and it also fails 100% (so I messed
up this morning). Maybe a problem with your bash ? (private joke :-)).

Willy



Re: stable-bot: Bugfixes waiting for a release 2.2 (18), 2.1 (13), 2.0 (8), 1.8 (6)

2020-08-19 Thread Willy Tarreau
Hi Aleks,

On Wed, Aug 19, 2020 at 11:32:13AM +0200, Aleksandar Lazic wrote:
> Please can the following patch also be considered to be backported.
> 
> OPTIM: startup: fast unique_id allocation for acl.
> http://git.haproxy.org/?p=haproxy-2.2.git;a=commit;h=f91ac19299fe216a793ba6550dca06b688b31549

Backported to which ones ? It's already in 2.2, 2.1 and 2.0, as well
as the fix that Tim brought later since this patch alone caused some
accidental breakage.

If you're thinking about 1.8, I'm really not fond of backporting
optimizations that far at the risk of breaking well-working setups.
One of the reasons we now have a faster release cycle is to make sure
users have plenty of choice of versions with a very limited risk of
breakage, and that in order to maintain this we also avoid backporting
what is not essential. And if you want my opinion, I think we still
backport too much too far... (or at least too fast).

Regards,
Willy



Re: [PR] Prevent favicon.ico requests errors for stats page.

2020-08-19 Thread Willy Tarreau
Hi,

On Tue, Aug 18, 2020 at 10:23:08AM +0200, PR Bot wrote:
> Patch title(s): 
>Prevent favicon.ico requests for stats page
>Merge pull request #1 from zurikus/zurikus-patch-1

Thank you for this! I didn't even know it was possible to prevent the
browser from requesting this file and it has been annoying me as well.
Tested and confirmed, thus merged :-)

Thanks!
Willy



Re: [PATCH] MEDIUM: cfgparse: Emit hard error on truncated lines

2020-08-19 Thread Willy Tarreau
On Wed, Aug 19, 2020 at 10:56:59AM +0200, Tim Düsterhus wrote:
> William,
> 
> Am 19.08.20 um 10:54 schrieb William Lallemand:
> > I think it broke
> > reg-tests/stick-table/converteers_ref_cnt_never_dec.vtc:
> > 
> > https://travis-ci.com/github/haproxy/haproxy/jobs/375236964
> > 
> > But I can't reproduce localy.

Strangely I didn't reproduce it this morning when applying them, so
either it depends on the vtest version or I did something wrong in
the test. Here I'm seeing fail 100%.

> Looking at the test I would suspect that removing the indentation in
> front of the brace in line 62 fixes the issue. But I did not test this
> either:
> 
> https://github.com/haproxy/haproxy/blob/ff4d86becd658f61e45692815f3b9e7a1e66107f/reg-tests/stick-table/converteers_ref_cnt_never_dec.vtc#L62

Confirmed, the spaces are sent to the output config file and cause the
error. I'm going to fix that and push the fix.

Willy

> 
> Best regards
> Tim Düsterhus



Re: [PATCH] MINOR: cache: Reject duplicate cache names

2020-08-18 Thread Willy Tarreau
Hi Tim,

On Tue, Aug 18, 2020 at 10:20:27PM +0200, Tim Duesterhus wrote:
> Using a duplicate cache name most likely is the result of a misgenerated
> configuration. There is no good reason to allow this, as the duplicate
> caches can't be referred to.
> 
> This commit resolves GitHub issue #820.
> 
> It can be argued whether this is a fix for a bug or not. I'm erring on the
> side of caution and marking this as a "new feature". It can be considered for
> backporting to 2.2, but for other branches the risk of accidentally breaking
> some working (but non-ideal) configuration might be too large.

All 3 patches merged. I also agree for not going too far on this one. In
addition, it reintroduces an O(N^2) lookup during config parsing (we had
some in the past in backend lookups and even N^3 in state file processing)
so while I'm generally not worried, I'd rather stay on the safe side by
making sure nobody accidently hits that by using hundreds of thousands of
cache instances on an older version :-)

Thanks!
Willy



Re: [PATCH] DOC: overhauling github issue templates

2020-08-17 Thread Willy Tarreau
On Mon, Aug 17, 2020 at 08:17:11PM +0200, Lukas Tribus wrote:
> as per the suggestions from Cyril and Willy on the mailing list:
> 
> Message-ID: 
> 
> and with direct contributions from Tim, this changes large parts
> of the bug issue template.
> 
> The Feature template is also updated as well as a new template for
> Code Reports is introduced.
> 
> Co-authored-by: Tim Duesterhus 

Now applied, thanks to everyone involved!

Willy



Re: Lua error message patch

2020-08-17 Thread Willy Tarreau
On Sun, Aug 16, 2020 at 05:28:55PM +0200, Thierry Fournier wrote:
> Hi list,
> 
> A little patch to improve verbosity of some Lua errors.

Applied, thank you Thierry!

Willy



Re: I am unable to un-subscribe from the list, where is the admin?

2020-08-17 Thread Willy Tarreau
Hi Eliezer,

On Fri, Aug 14, 2020 at 10:54:34AM +0300, Eliezer Croitoru wrote:
> I have tried more then once to un-subscribe from the haproxy@formilux.org
>   list and am unable to do so.
(...)

Now done, I don't know why it didn't work, maybe your messages never
reached the bot. You won't receive messages from the list anymore.

Regards,
Willy



Re: github template

2020-08-16 Thread Willy Tarreau
Hi Lukas,

On Sun, Aug 16, 2020 at 10:27:24PM +0200, Lukas Tribus wrote:
> The changes can be seen here:
> 
> https://github.com/lukastribus/hap-issue-trial/issues/new/choose
> 
> If you agree, I will send the patches.

Looks perfect to me, many thanks!

Willy



Re: [ANNOUNCE] haproxy-2.3-dev3

2020-08-15 Thread Willy Tarreau
On Sat, Aug 15, 2020 at 04:09:14PM +0200, William Lallemand wrote:
> On Fri, Aug 14, 2020 at 07:23:36PM +0200, Willy Tarreau wrote:
> > Another long-standing issue was addressed by William today, regarding how
> > filters "work" in crt-lists. When using an exclusion they don't work well
> > because instead of using a list of exclusions, a lookup is performed and
> > the matching entry is skipped. While that might work in certain cases
> > (single entry for a given cert), there are situations where it cannot work
> > like when this is used to exclude certain servernames from certain cert
> > types. Thus William reworked that so that it really does what the doc
> > says and what the syntax suggests. It should not have any visible effect
> > for all those who were not subject to the problem, but might possibly
> > reveal issues in certain broken configs that were working by accident
> > (i.e. the desired cert is broken and not used and might suddenly be
> > exposed). If you're using crt-lists with exlusions, you're welcome to
> > verify that it's still OK for you. After some time this fix will be
> > backported so that users don't get trapped anymore, but we'll have to
> > delay this to avoid bad surprises.
> 
> There is indeed a problem which was identified with the exclusions, but
> the current commits fix another problem (issue #810). The problem lies
> in the lookup of the SNIs, if a SNI with a single name is found, it
> won't try to lookup for a wildcard, which is a problem if you use a
> single name certificate for ECDSA and a wildcard for the RSA
> certificate. This bug was introduced quite recently and backported as
> far as 1.8 in december 2019... 
> 
> But this is not a fix for the negative filters, that should also be
> fixed but need a rework of the filters.

OK, then sorry for confusing the two and thanks for clarifying.

Willy



Re: Haproxy 1.8.26-1~bpo9+1

2020-08-15 Thread Willy Tarreau
On Thu, Aug 13, 2020 at 08:21:28AM +0200, Erwin Schliske wrote:
> > The patch was for the 2.0. It must be adapted for the 1.8. But, it is not
> > necessary because the bug is now fixed in 2.0 and 1.8 :
> >
> >   * 2.0 : http://git.haproxy.org/?p=haproxy-2.0.git;a=commit;h=307f31ec
> >   * 1.8 : http://git.haproxy.org/?p=haproxy-1.8.git;a=commit;h=179d316c
> 
> 
> Thanks. I need the patch until 1.8.27 is released. I found it yesterday and
> tested it successfully. The observed problem is now solved.

Thanks for your feedback. We'll issue another 1.8 shortly then I guess.

Willy



Re: [ANNOUNCE] haproxy-2.3-dev3

2020-08-14 Thread Willy Tarreau
On Fri, Aug 14, 2020 at 08:12:10PM +0200, Tim Düsterhus wrote:
> Willy,
> 
> Am 14.08.20 um 19:23 schrieb Willy Tarreau:
> > HAProxy 2.3-dev3 was released on 2020/08/14. It added 38 new commits
> > after version 2.3-dev2.
> 
> I'm not seeing the tag. Did you forget to 'git push'?

Oops, sorry for that, you guessed well. Now pushed.

Thanks,
willy



[ANNOUNCE] haproxy-2.3-dev3

2020-08-14 Thread Willy Tarreau
strings in trash during args 
validation
  BUG/MINOR: lua: Duplicate lua strings in sample fetches/converters arg 
array
  MEDIUM: lua: Don't filter exported fetches and converters
  MINOR: lua: Add support for userlist as fetches and converters arguments
  MINOR: lua: Add support for regex as fetches and converters arguments
  MINOR: arg: Use chunk_destroy() to release string arguments

David Carlier (1):
  OPTIM: regex: PCRE2 use JIT match when JIT optimisation occured.

Ilya Shipitsin (2):
  BUILD: Makefile: require SSL_LIB, SSL_INC to be explicitly set
  CI: travis-ci: specify SLZ_LIB, SLZ_INC for travis builds

William Dauchy (5):
  BUG/MINOR: spoa-server: fix size_t format printing
  DOC: spoa-server: fix false friends `actually`
  CLEANUP: ssl: ssl_sock_crt2der semicolon and spaces
  MINOR: ssl: add ssl_{c,s}_chain_der fetch methods
  CLEANUP: fix all duplicated semicolons

William Lallemand (9):
  BUG/MINOR: ssl: fix memory leak at OCSP loading
  BUG/MEDIUM: ssl: memory leak of ocsp data at SSL_CTX_free()
  BUG/MINOR: snapshots: leak of snapshots on deinit()
  BUG/MEDIUM: ssl: fix the ssl-skip-self-issued-ca option
  BUG/MINOR: ssl: ssl-skip-self-issued-ca requires >= 1.0.2
  BUG/MINOR: ssl: double free w/ smp_fetch_ssl_x_chain_der()
  BUG/MEDIUM: ssl: never generates the chain from the verify store
  BUG/MEDIUM: ssl: does not look for all SNIs before chosing a certificate
  CLEANUP: ssl: remove poorly readable nested ternary

Willy Tarreau (5):
  SCRIPTS: git-show-backports: make -m most only show the left branch
  SCRIPTS: git-show-backports: emit the shell command to backport a commit
  BUG/MINOR: stats: use strncmp() instead of memcmp() on health states
  BUILD: makefile: don't disable -Wstringop-overflow anymore
  BUG/MEDIUM: htx: smp_prefetch_htx() must always validate the direction

---



Re: [PATCH]: regex: PCRE2 optimisation with JIT

2020-08-13 Thread Willy TARREAU
On Thu, Aug 13, 2020 at 06:17:15PM +0100, David CARLIER wrote:
> To be honest not huge improvement but relatively constant has it is
> set once per regex compilation.

Perfect, thanks, now merged.

Willy



Re: [PATCH]: regex: PCRE2 optimisation with JIT

2020-08-13 Thread Willy TARREAU
On Thu, Aug 13, 2020 at 05:30:49PM +0100, David CARLIER wrote:
> In fact the jit match does less check than the normal match and fits
> better when the regex code had been compiled with JIT. However the
> classic match call still works with JIT only it does more checks.

OK! did you observe any gain in doing so ? Because we're replacing a
direct call with an indirect one. I know it's a detail, but if we're
just replacing a few "if" inside pcre_match() with an indirect call,
it might easily be on tie!

Willy



Re: [PATCH]: regex: PCRE2 optimisation with JIT

2020-08-13 Thread Willy TARREAU
Hi David,

On Thu, Aug 13, 2020 at 03:00:28PM +0100, David CARLIER wrote:
> Subject: [PATCH] CLEANUP/MEDIUM: regex: PCRE2 use JIT match when JIT
>  optimisation occured.
> 
> When a regex had been succesfully compiled by the JIT pass, it is better
>  to use the related match, thanksfully having same signature, for better
>  performance.

I'm not sure I understand well. Does this mean that right now when we
use PCRE/JIT we compile using JIT but we don't match using it ? This
sounds strange to me because I remember that you published numbers a
long time ago showing a real boost with JIT. Or is there only a special
case where JIT is used for the match ? Or only a special case where it
is not used ?

Thanks!
Willy



Re: No access to git.haproxy.org from Travis CI

2020-08-12 Thread Willy Tarreau
Hi Bjoern,

On Wed, Aug 12, 2020 at 06:52:05PM +0200, bjun...@gmail.com wrote:
> Am Sa., 13. Juni 2020 um 22:15 Uhr schrieb Willy Tarreau :
> 
> > Hi William,
> >
> > On Sat, Jun 13, 2020 at 03:13:06PM +0200, William Dauchy wrote:
> > > Hi,
> > >
> > > On Thu, Jun 11, 2020 at 1:10 PM Willy Tarreau  wrote:
> > > > Sure but what I wanted to say was that travis seems to be the only
> > > > point experiencing such difficulties and we don't know how it works
> > > > nor what are the rules in place.
> > >
> > > I don't know whether this is related to the issue described here but I
> > just had:
> > >
> > > $ git pull --rebase
> > > fatal: unable to access 'http://git.haproxy.org/git/haproxy.git/': The
> > > requested URL returned error: 502
> > > $ git pull --rebase
> > > error: The requested URL returned error: 502 (curl_result = 22,
> > > http_code = 502, sha1 = f38175cf6ed4d02132e6b21cbf643f73be5ee000)
> > > error: Unable to find f38175cf6ed4d02132e6b21cbf643f73be5ee000 under
> > > http://git.haproxy.org/git/haproxy.git
> > > Fetching objects: 4529, done.
> > > Cannot obtain needed commit f38175cf6ed4d02132e6b21cbf643f73be5ee000
> > > while processing commit ff0e8a44a4c23ab36b6f67c4052777ac908d4211.
> > > error: fetch failed.
> > >
> > > Third try was ok though :/
> >
> > Thanks for the info, I'll have a look. However the issue reported about
> > travis was a connection error, which likely indicates a routing issue.
> >
> > Cheers,
> > Willy
> >
> 
> Hi,
> 
> fyi, after 2 months and more than 20 emails Travis has fixed it.

Ah, great, thanks for letting us know!

Willy



Re: github template

2020-08-12 Thread Willy Tarreau
Hi again,

On Wed, Aug 12, 2020 at 04:54:17AM +0200, Willy Tarreau wrote:
> > Let me know about 1) and 2) above, and I will send a patch.
> 
> You now get the idea, maybe my wording is not perfect and something is
> more suitable, so feel free to propose anything along these lines :-)

Regarding a potential patch, I think we should really consider having
new type, which is not exactly a bug but not a feature request either.
The ones I'm having in mind are the valgrind and coverity reports, most
of which are in fact 100% harmless but still potential bugs or "unclean"
stuff, I don't really know how to qualify them. Maybe just "automated
reports", which could even cover future tools.

The issue is that it already happened to me twice to miss a real bug
in the list when there were a burst of such reports occupying the
main issues page. By nature they don't work the same way as bugs.
They don't require the formalism of a real bug report, there are less
round trips (i.e. I don't expect to see a "feedback required" status
that often), they generally have no known impact so should be taken
with less precedence than prod-blocking bugs, and can be handled by
more people (especially the coverity reports). In addition, the
valgrind reports present the risk to introduce real bugs when trying to
fix them, just because the person looking at them may not necessarily
know the affected area in deepest details, and it would be much safer
to involve a knowledgeable maintainer.

Also some of them will not be backported so there's an incentive to
quickly act and close them, which is good because none should be
seen rotting for a long time there. Having a special status will
ease their listing and encourage eliminating them more regularly.

Maybe we could add that as "test bot" (but some are really done by
hand using a tool so that doesn't match).

Actually I'm aware there's a slight difference between the coverity
reports and valgrind reports, in that in the case of coverity it's
a suspected issue while with valgrind it's a detected one. In the
former case almost anyone can sort that out. In the second case,
almost only the code's author knows and even sometimes it's hard,
and a mistake done there becomes dramatic for stability. So maybe
they're not exactly the same but in both cases they have no short
term impact and some of them ought even not to be backported (which
is why many of them are expected to be short lived).

I'm open to ideas and suggestions.

Cheers,
Willy



Re: github template

2020-08-11 Thread Willy Tarreau
Hi Lukas,

On Tue, Aug 11, 2020 at 11:25:02PM +0200, Lukas Tribus wrote:
> On Mon, 20 Jul 2020 at 06:35, Willy Tarreau  wrote:
> > > (Another case is when I try to follow the issue reports during vacation)
> > >
> > > I think it could be easier and quicker by only changing the sections order
> > > like this :
> > > 1. Expected behavior
> > > 2. Actual behavior
> > > 3. Steps to reproduce the behavior
> > > 4. Do you have any idea what may have caused this?
> > > 5. Do you have an idea how to solve the issue?
> > > 6. What's the configuration?
> > > 7. Output of haproxy -vv and uname -a
> > >
> > > What do you think about it ?
> >
> > Actually I'm wondering whether we should merge points 1 and 2 above into
> > "detailed description of the problem": sometimes it's easier to mention
> > "I'm seeing this which I find strange" without knowing exactly what the
> > expected behavior should be. We could indicate in the comments for this
> > section "please clearly describe the whole problem, preferably starting
> > with the expected behavior and followed by the actual behavior".
> >
> > And even then, I'm now thinking that most users would probably more
> > naturally first describe what they observed then explain what they
> > would have expected. This would flow more naturally:
> >
> >1) subject of the bug report
> 
> Not sure whether you are just referring to the title of the issue
> (which actually is the subject already) or whether you are proposing
> to add a new section: I feel like that's redundant and would advise
> against it.

I was indeed speaking about the current title.

> >2) more detailed description matching the subject above: "when I
> >   do this, haproxy does that"
> 
> That's what "Actual behavior" is. Are you suggesting we rephrase?

I think that "Detailed description of the problem" can be more natural
than just the actual behavior in that for some people providing a multi-
step description, it could cover both the current and expected behavior.
Quite often the expected behavior is something like "not crash :-)" which
is definitely obvious, and implies that the current behavior is wrong.
Sometimes it's more nuanced and it can be "does this while I'd expect
that". And if the reporter has tested several workarounds, it's harder
to place them under "actual behavior" than "detailed description".

> I agree it should be at the beginning, before "expected behavior".
> 
> 
> >3) expected behavior: explain why what's described above is
> >   considered as wrong and what was expected instead (probably
> >   a mismatch with the doc, can be skipped if it's about a crash)
> >4, 5, 6, 7) unchanged
> >8) haproxy's last output if it crashed, gdb output if a core was
> >   produced
> >9) any additional info that may help (local patches, environment
> >   specificities, unusual workload, observations, coincidences
> >   with events on other components ...)
> 
> Agreed.
> 
> Features.md need's something similar (moving all the boring stuff below).

Yes, probably indeed! By the way, I was a bit worried that the features
entry would serve as unordered wish lists for some groups hoping to use
them to vote for certain things, but for now it remains very reasonable
and that's great.

> Let me know about 1) and 2) above, and I will send a patch.

You now get the idea, maybe my wording is not perfect and something is
more suitable, so feel free to propose anything along these lines :-)

Thanks!
Willy



Re: Can I help with the 2.1 release?

2020-08-08 Thread Willy Tarreau
On Thu, Jul 30, 2020 at 11:10:35PM +0300, Valter Jansons wrote:
> On Thu, Jul 30, 2020 at 10:37 PM Julien Pivotto  
> wrote:
> > I'm with Lukas on this. 2.1 is a strong release, and we should be
> > grateful for everyone which is using that release, as their feedback is
> > valuable for the building the next releases of HAProxy.
> 
> My apologies if the message sounded ungrateful, for rolling out new
> changes and testing. As the latest 2.2.0 release did show just
> recently, there is great benefit in people running upcoming (new)
> changes.

No offense, don't worry :-)

We're used to say that odd versions being maintained for less time, we're
allowed to take more risks with them and we know that most of their users
are those autonomous enough to roll back or switch to another one in case
of trouble. As such, the stability of an odd version can be a bit more
chaotic than the one of an even one, and that's by choice to preserve more
users. Also I'm less reluctant to backport small features to odd versions
than for even ones (it's a give and take, brave users test & report issues
and in exchange they can get a version that better suits their needs). In
other areas of the industry, the terms "early availability" and "general
deployment" exist to designate these different stability statuses, and I
think that they model what we do quite well.

Of course when a new version is issued, it needs a little bit of time to
dry up, and a few surprises are expected. But the point is that there
should be (by design) less risks to upgrade from 2.1.x to 2.2.x than from
2.0.x to 2.1.x two months after the new major release is emitted. Here
we're facing something unusual in that 2.1 appeared to be exceptionally
stable and 2.2 started with some rough edges, so at this point of the
cycle the difference in stability expectations might still be less visible
of course.

Anyway, the point of maintaining long term supported versions is that
anyone is free to use the one that best suits their needs. Anything
between the oldest that supports all needed features, to the latest
stable enough for the use case is fine.

As a rule of thumb, I'd say that it's probably OK to always be late by
one or two stable versions on average. This should help one figure what
branch to deploy: if the latest stable emits one version every two weeks,
it means you need to upgrade your production every two to four weeks. If
an older stable one produces one version every 6 months, it may allow you
not to care about your prod for 6 months to one year. But in any case
there is always the risk of a critical bug requiring an urgent deployment,
so you should see this as a rule of thumb only and not a strict rule.

Hoping this clarifies the process a bit.

Willy



Re: [PATCH] CI: rework SSL_LIB, SSL_INC in more elegant way, improve build speed by switching to stock lib for openssl-1.1.1 builds

2020-08-04 Thread Willy Tarreau
On Tue, Aug 04, 2020 at 03:55:04PM +0500,  ??? wrote:
> I've modified build system again.
> please consider that as a final effort from me. If you are not happy enough
> with it, you are free to improve it.

Thanks Ilya. I think that will be OK this way. I'll add commit
messages and will take them.

Cheers,
Willy



Re: SUBVERS for 2.2 snapshots not correctly filled in?

2020-08-04 Thread Willy Tarreau
On Tue, Aug 04, 2020 at 06:49:40PM +0200, Tim Düsterhus wrote:
> Willy,
> 
> Am 04.08.20 um 18:34 schrieb Willy Tarreau:
> > That's rather strange. I'm still comparing the two repos, they look
> > exactly similar.
> > 
> > Ah found it! We don't just need the "atttributes" file but also the
> > info/attributes one. I don't remember which one is used when. I'm
> > going to update my release check list. We're tackling one release bug
> > at a time for each release, hopefully 2.3 will be perfect!
> > 
> 
> Or you just could commit the .gitattributes file as this would also fix
> the GitHub downloads :-)

Yep, I can (and should) definitely do this one. I guess it didn't exist
initially and we got used to the same old method.

Cheers,
Willy



Re: SUBVERS for 2.2 snapshots not correctly filled in?

2020-08-04 Thread Willy Tarreau
Hi Tim,

On Tue, Aug 04, 2020 at 06:14:41PM +0200, Tim Düsterhus wrote:
> Willy,
> 
> It appears that the SUBVERS file is broken for 2.2 snapshots:
> 
> > [timwolla@/tmp]curl -fsSL 
> > 'http://www.haproxy.org/download/2.2/src/snapshot/haproxy-ss-LATEST.tar.gz' 
> > |tar -O -xvz --wildcards '*/SUBVERS'
> > haproxy-ss-20200801/SUBVERS
> > -$Format:%h$
> 
> It looks good for 2.1:
> 
> > [timwolla@/tmp]curl -fsSL 
> > 'http://www.haproxy.org/download/2.1/src/snapshot/haproxy-ss-LATEST.tar.gz' 
> > |tar -O -xvz --wildcards '*/SUBVERS'
> > haproxy-ss-20200801/SUBVERS
> > -3a5bfcc
> > 

That's rather strange. I'm still comparing the two repos, they look
exactly similar.

Ah found it! We don't just need the "atttributes" file but also the
info/attributes one. I don't remember which one is used when. I'm
going to update my release check list. We're tackling one release bug
at a time for each release, hopefully 2.3 will be perfect!

Thanks!
Willy



Re: VTest issue when built on Ubuntu 20.04

2020-08-01 Thread Willy Tarreau
On Sun, Aug 02, 2020 at 02:28:09AM +0500,  ??? wrote:
> it is /c2.2 subtest
> 
> somehow server decided to reply earlier
> 
> 20.04
> https://travis-ci.com/github/chipitsine/haproxy/jobs/367551643#L642-L644
> 
> 18.04
> https://travis-ci.com/github/chipitsine/haproxy/jobs/367551644#L618-L620

It's not that it replied earlier, it's that they're all progressing in
parallel, and the apparent ordering different you're seeing is only an
artefact of the scheduling. However, I notice that the faulty one uses
significantly smaller chunks, and we cannot rule out that the decoder
fails below certain sizes, or that we've lost data inside (but where
and why is another story).

Willy



Re: [ANNOUNCE] haproxy-1.9.16

2020-07-31 Thread Willy Tarreau
On Fri, Jul 31, 2020 at 03:15:07PM +0200, Tim Düsterhus wrote:
> Christopher,
> Willy,
> 
> Am 31.07.20 um 14:14 schrieb Christopher Faulet:
> > The 1.9 branch is EOL now. Thus, it is the last 1.9 release. No further 
> > release 
> > should be expected. It contains all pending patches marked to be backported 
> > to 
> > the 1.9 to leave it in a proper state. Have a look at the changelog below 
> > for 
> > the complete list of fixes.
> > 
> > You should have no reason to deploy it in a production environment. Use the 
> > 2.0 
> > or above instead. No specific support should no longer be expected on the 
> > 1.9. 
> > This branch will not receive fixes anymore.
> > 
> > Thanks everyone for you help and your contributions !
> 
> Don't forget to update the color in the version table on haproxy.org

Good catch, thanks for the reminder, now done!

Willy



[ANNOUNCE] haproxy-2.3-dev2

2020-07-31 Thread Willy Tarreau
r Faulet (19):
  BUG/MAJOR: dns: Make the do-resolve action thread-safe
  BUG/MEDIUM: dns: Release answer items when a DNS resolution is freed
  MEDIUM: htx: Add a flag on a HTX message when no more data are expected
  BUG/MEDIUM: stream-int: Don't set MSG_MORE flag if no more data are 
expected
  BUG/MEDIUM: http-ana: Only set CF_EXPECT_MORE flag on data filtering
  BUG/MINOR: mux-fcgi: Don't url-decode the QUERY_STRING parameter anymore
  BUG/MINOR: debug: Don't dump the lua stack if it is not initialized
  MEDIUM: lua: Add support for the Lua 5.4
  BUG/MEDIUM: dns: Don't yield in do-resolve action on a final evaluation
  BUG/MINOR: lua: Abort execution of actions that yield on a final 
evaluation
  MINOR: tcp-rules: Return an internal error if an action yields on a final 
eval
  BUG/MINOR: tcp-rules: Preserve the right filter analyser on content eval 
abort
  BUG/MINOR: tcp-rules: Set the inspect-delay when a tcp-response action 
yields
  MEDIUM: tcp-rules: Use a dedicated expiration date for tcp ruleset
  MEDIUM: lua: Set the analyse expiration date with smaller wake_time only
  BUG/MEDIUM: connection: Be sure to always install a mux for sync connect
  MINOR: connection: Preinstall the mux for non-ssl connect
  MINOR: stream-int: Be sure to have a mux to do sends and receives
  BUG/MINOR: lua: Fix a possible null pointer deref on lua ctx

Emeric Brun (1):
  BUG/MEDIUM: resolve: fix init resolving for ring and peers section.

Ilya Shipitsin (5):
  CI : travis-ci : prepare for using stock OpenSSL
  CI: travis-ci : switch to stock openssl when openssl-1.1.1 is used
  CI: travis-ci: use better name for Coverity scan job
  CI: travis-ci: use proper linking flags for SLZ build
  CLEANUP: assorted typo fixes in the code and comments

Jackie Tapia (1):
  DOC: Use gender neutral language

Jerome Magnin (2):
  BUG/MAJOR: dns: fix null pointer dereference in snr_update_srv_status
  BUG/MAJOR: dns: don't treat Authority records as an error

Lukas Tribus (1):
  DOC: ssl: req_ssl_sni needs implicit TLS

William Lallemand (1):
  BUG/MEDIUM: ssl: check OCSP calloc in ssl_sock_load_ocsp()

Willy Tarreau (13):
  BUG/MEDIUM: arg: empty args list must be dropped
  BUG/MAJOR: tasks: don't requeue global tasks into the local queue
  MINOR: tasks/debug: make the thread affinity BUG_ON check a bit stricter
  MINOR: tasks/debug: add a few BUG_ON() to detect use of wrong timer queue
  MINOR: tasks/debug: add a BUG_ON() check to detect requeued task on free
  CLEANUP: dns: remove 45 "return" statements from 
dns_validate_dns_response()
  BUG/MINOR: htx: add two missing HTX_FL_EOI and remove an unexpected one
  SCRIPTS: announce-release: add the link to the wiki in the announce 
messages
  BUG/MEDIUM: backend: always attach the transport before installing the mux
  BUG/MEDIUM: tcp-checks: always attach the transport before installing the 
mux
  MINOR: connection: avoid a useless recvfrom() on outgoing connections
  MINOR: mux-h1: do not even try to receive if the connection is not fully 
set up
  MINOR: mux-h1: do not try to receive on backend before sending a request

---



[ANNOUNCE] haproxy-2.0.17

2020-07-31 Thread Willy Tarreau
Hi,

HAProxy 2.0.17 was released on 2020/07/31. It added 19 new commits
after version 2.0.16. It's not much, nothing is likely to hit anybody who
was not already hit, but we thought it was worth flushing the pipe.

In short (some of them copied from the 2.1 announce):

  - various DNS fixes (do-resolve was not thread-safe, would spin if
called as a final action, and there were memory leaks).

  - spliced transfers could occasionally stall on certain sizes due to 
an FD not always being re-enabled.

  - the memcmp() fix for ebtrees failed to build on libmusl due to a
missing include.

  - better fix for the potential connection freezes on mux-h1 that were
fixed in 2.0.16.

  - risk of possible infinite loop on H2 in legacy mode when transferring
truncated chunked responses. It's been there since 1.9, indicating that
almost nobody uses legacy anymore, which is great.

  - "server" directives in peers and rings wouldn't resolve if an FQDN was
used, because they used to call str2sa_range() with resolve=0 like the
regular servers. Sadly, no error was spotted there so that would only
result in failed connection attempts.

In addition, compatibility support for Lua 5.4 was backported so that those
who prefer to use 2.0 on their latest distros do not experience biuld issues.

Please find the usual URLs below :
   Site index   : http://www.haproxy.org/
   Discourse: http://discourse.haproxy.org/
   Slack channel: https://slack.haproxy.org/
   Issue tracker: https://github.com/haproxy/haproxy/issues
   Wiki : https://github.com/haproxy/wiki/wiki
   Sources  : http://www.haproxy.org/download/2.0/src/
   Git repository   : http://git.haproxy.org/git/haproxy-2.0.git/
   Git Web browsing : http://git.haproxy.org/?p=haproxy-2.0.git
   Changelog: http://www.haproxy.org/download/2.0/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/

Willy
---
Complete changelog :
Christopher Faulet (12):
  REGEST: Add reg tests about error files
  BUG/MEDIUM: mux-h2: Emit an error if the response chunk formatting is 
incomplete
  BUG/MAJOR: dns: Make the do-resolve action thread-safe
  BUG/MEDIUM: dns: Release answer items when a DNS resolution is freed
  BUG/MEDIUM: mux-h1: Wakeup the H1C in h1_rcv_buf() if more data are 
expected
  BUG/MEDIUM: mux-h1: Disable the splicing when nothing is received
  BUG/MINOR: debug: Don't dump the lua stack if it is not initialized
  MEDIUM: lua: Add support for the Lua 5.4
  BUG/MEDIUM: dns: Don't yield in do-resolve action on a final evaluation
  BUG/MINOR: tcp-rules: Set the inspect-delay when a tcp-response action 
yields
  MINOR: connection: Preinstall the mux for non-ssl connect
  MINOR: stream-int: Be sure to have a mux to do sends and receives

Emeric Brun (1):
  BUG/MEDIUM: resolve: fix init resolving for ring and peers section.

Olivier Houchard (1):
  BUG/MINOR: threads: Don't forget to init each thread toremove_lock.

Willy Tarreau (5):
  BUILD: ebtree: fix build on libmusl after recent introduction of 
eb_memcmp()
  MINOR: pools: increase MAX_BASE_POOLS to 64
  BUILD: thread: add parenthesis around values of locking macros
  BUG/MINOR: cfgparse: don't increment linenum on incomplete lines
  SCRIPTS: announce-release: add the link to the wiki in the announce 
messages

---



[ANNOUNCE] haproxy-2.1.8

2020-07-31 Thread Willy Tarreau
bonte.github.io/haproxy-dconv/

Willy
---
Complete changelog :
Anthonin Bonnefoy (1):
  MINOR: http: Add support for http 413 status

Baruch Siach (1):
  BUILD: tools: fix build with static only toolchains

Christopher Faulet (29):
  REGTESTS: Add missing OPENSSL to REQUIRE_OPTIONS for 
compression/lua_validation
  REGTEST: Add a simple script to tests errorfile directives in proxy 
sections
  MINOR: spoe: Don't systematically create new applets if processing rate 
is low
  BUG/MEDIUM: pattern: Add a trailing \0 to match strings only if possible
  BUG/MINOR: mux-h1: Fix the splicing in TUNNEL mode
  BUG/MINOR: mux-h1: Don't read data from a pipe if the mux is unable to 
receive
  BUG/MINOR: mux-h1: Disable splicing only if input data was processed
  BUG/MEDIUM: mux-h1: Disable splicing for the conn-stream if read0 is 
received
  MINOR: mux-h1: Improve traces about the splicing
  BUG/MEDIUM: mux-h1: Subscribe rather than waking up in h1_rcv_buf()
  BUG/MEDIUM: connection: Continue to recv data to a pipe when the FD is 
not ready
  BUG/MINOR: backend: Remove CO_FL_SESS_IDLE if a client remains on the 
last server
  BUG/MEDIUM: mux-h1: Continue to process request when switching in tunnel 
mode
  BUG/MINOR: mux-fcgi: Handle empty STDERR record
  BUG/MINOR: mux-fcgi: Set conn state to RECORD_P when skipping the record 
padding
  BUG/MINOR: mux-fcgi: Set flags on the right stream field for empty 
FCGI_STDOUT
  BUG/MEDIUM: channel: Be aware of SHUTW_NOW flag when output data are 
peeked
  REGEST: Add reg tests about error files
  BUG/MAJOR: dns: Make the do-resolve action thread-safe
  BUG/MEDIUM: dns: Release answer items when a DNS resolution is freed
  BUG/MINOR: mux-fcgi: Don't url-decode the QUERY_STRING parameter anymore
  BUG/MEDIUM: mux-h1: Wakeup the H1C in h1_rcv_buf() if more data are 
expected
  BUG/MEDIUM: mux-h1: Disable the splicing when nothing is received
  BUG/MINOR: debug: Don't dump the lua stack if it is not initialized
  MEDIUM: lua: Add support for the Lua 5.4
  BUG/MEDIUM: dns: Don't yield in do-resolve action on a final evaluation
  BUG/MINOR: tcp-rules: Set the inspect-delay when a tcp-response action 
yields
  MINOR: connection: Preinstall the mux for non-ssl connect
  MINOR: stream-int: Be sure to have a mux to do sends and receives

Emeric Brun (2):
  BUG/MEDIUM: log: issue mixing sampled to not sampled log servers.
  BUG/MEDIUM: resolve: fix init resolving for ring and peers section.

Florian Tham (2):
  MINOR: http: Add 410 to http-request deny
  MINOR: http: Add 404 to http-request deny

Harris Kaufmann (1):
  BUG/MEDIUM: fcgi-app: fix memory leak in fcgi_flt_http_headers

Ilya Shipitsin (1):
  BUG/MEDIUM: server: resolve state file handle leak on reload

Miroslav Zagorac (1):
  BUG/MINOR: spoe: correction of setting bits for analyzer

Olivier Houchard (1):
  BUG/MINOR: threads: Don't forget to init each thread toremove_lock.

Ryan O'Hara (1):
  BUG/MINOR: systemd: Wait for network to be online

Tim Duesterhus (5):
  REGTESTS: Add missing OPENSSL to REQUIRE_OPTIONS for lua/txn_get_priv
  BUG/MEDIUM: fetch: Fix hdr_ip misparsing IPv4 addresses due to missing NUL
  BUG/MINOR: http_act: don't check capture id in backend (2)
  BUG/MINOR: sample: Free str.area in smp_check_const_bool
  BUG/MINOR: sample: Free str.area in smp_check_const_meth

William Lallemand (9):
  BUG/MINOR: ssl: fix ssl-{min,max}-ver with openssl < 1.1.0
  BUG/MEDIUM: ssl: crt-list must continue parsing on ERR_WARN
  BUG/MINOR: mworker/cli: fix the escaping in the master CLI
  BUG/MINOR: mworker/cli: fix semicolon escaping in master CLI
  REGTEST: http-rules: test spaces in ACLs
  REGTEST: http-rules: test spaces in ACLs with master CLI
  REGTEST: ssl: tests the ssl_f_* sample fetches
  REGTEST: ssl: add some ssl_c_* sample fetches test
  DOC: ssl: add "allow-0rtt" and "ciphersuites" in crt-list

Willy Tarreau (27):
  BUG/MEDIUM: log: don't hold the log lock during writev() on a file 
descriptor
  BUG/MEDIUM: pattern: fix thread safety of pattern matching
  BUILD: make dladdr1 depend on glibc version and not __USE_GNU
  BUG/MINOR: http: make smp_fetch_body() report that the contents may change
  BUG/MINOR: tcp-rules: tcp-response must check the buffer's fullness
  BUG/MEDIUM: ebtree: use a byte-per-byte memcmp() to compare memory blocks
  BUG/MINOR: spoe: add missing key length check before checking key names
  MEDIUM: map: make the "clear map" operation yield
  BUG/MINOR: http_ana: clarify connection pointer check on L7 retry
  MINOR: cli: make "show sess" stop at the last known session
  BUG/MINOR: proxy: fix dump_server_state()'s misuse of the trash
  BUG/MINOR: proxy: always initialize the trash in show servers

Re: [PATCH] suppress "return value is not checked" warnings

2020-07-31 Thread Willy Tarreau
On Fri, Jul 31, 2020 at 02:32:02PM +0500,  ??? wrote:
> > Probably we should proceed differently and have something like these
> > for the cases where no return is desired:
> >
> > fcntl_noret()  => unchecked fcntl()
> > setsockopt_noret() => unchecked setsockopt()
> >
> > Both would be just static inline functions wrapping the other ones in a
> > DISGUISE() if needed.
> >
> > Then we could change the calls above to be more explicit about the intent
> > of not having a return, and less ugly to read.
> >
> > Do you want to try to do something like this ?
> >
> 
> those issues are not urgent.
> I will try.

Thank you. Do not hesitate to ask if you need help or if some parts
are not clear yet.

Cheers,
Willy



  1   2   3   4   5   6   7   8   9   10   >