Re: [PATCH 0/7] Coccinelle

2021-09-17 Thread Willy Tarreau
Hi Tim,

On Wed, Sep 15, 2021 at 01:58:42PM +0200, Tim Duesterhus wrote:
> Willy,
> 
> not sure about the "DOC" tag for the coccinelle patches and the placement
> within the directory structure. Feel free to adjust.

Good idea. I even remember that I was about to store some of my few
coccinelle patches when I started to split contrib/ apart a while ago
but I forgot to merge them. I'll have to blow the dust off them and
insert the relevant ones as well.

I've created dev/coccinelle and merged them there, since dev/ is where
the developer tools go now.

Thanks!
Willy



Re: [PATCH] BUG/???: lua: Add missing call to RESET_SAFE_LJMP in hlua_filter_new()

2021-09-14 Thread Willy Tarreau
On Tue, Sep 14, 2021 at 02:00:16PM +0200, Thierry Fournier wrote:
(...)
> So, I guess this ommit is not a great bug, but the experieence learn
> when we play with longjmp, MEDIUM is the right level for a patch.

Thanks Thierry for the detailed analysis!

Willy



Re: [ANNOUNCE] haproxy-2.5-dev7

2021-09-12 Thread Willy Tarreau
Hi Dmitry,

On Sun, Sep 12, 2021 at 05:54:33PM +0300, Dmitry Sivachenko wrote:
> there is a new warning in -dev branch (on FreeBSD):
> 
> admin/halog/fgets2.c:38:30: warning: '__GLIBC__' is not defined, evaluates to 
> 0 [-Wundef]
> #if defined(__x86_64__) &&  (__GLIBC__ > 2 || (__GLIBC__ == 2 && 
> __GLIBC_MINOR__ >= 15))
>  ^
> admin/halog/fgets2.c:38:48: warning: '__GLIBC__' is not defined, evaluates to 
> 0 [-Wundef]
> #if defined(__x86_64__) &&  (__GLIBC__ > 2 || (__GLIBC__ == 2 && 
> __GLIBC_MINOR__ >= 15))
> 
> Looks like Linux-specific condition.

Ah thank you! Another one I missed. I'll fix it, it's caused by the addition
of -Wundef.

Willy



[ANNOUNCE] haproxy-2.5-dev7

2021-09-12 Thread Willy Tarreau
Hi,

HAProxy 2.5-dev7 was released on 2021/09/12. It added 39 new commits
after version 2.5-dev6.

This version is essentially released to flush the pipe of pending fixes
for 2.5-dev. It contains the fix for CVE-2021-40346, plus a few other
ones related to option abortonclose.

The infamous global names array for the variables was finally eliminated
(which led me to break OpenTracing but Miroslav fixed it by temporarily
disabling the support for variables there). Now the "ifexist" restrictions
in Lua or SPOE only apply to the "proc" scope, so that all ephemeral
variables are not affected by this restriction and are easiler to deal
with. Variables under the scope "proc" that are declared in the config
are marked "permanent" so that they continue to work like before and do
not need to be explicitly created first. This leads me to think that the
"ifexist" argument of the Lua's set_var() could possibly be turned on by
default so that existing code using variables is made safe by default
without having to be modified, but could accept an explicit zero in the
argument to enforce creation of random names under the "proc" scope. But
I could be wrong, I think that those using them know better than me. Thanks
to these cleanups and a few other ones that allowed not to take the
variables lock when not needed, the cost of variables manipulation has
significantly dropped to the point that the request rate on a 16-thread
machine using 12 variables almost doubled.

A new "grace" global keyword was added to replace the per-proxy one that
was removed in 2.5. Some users needed something to maintain the process
alive for a few extra seconds after signal delivery, for the very same
reason that drove this keyword to be added a long time ago (i.e. no reload,
process is always totally stopped but watched by an external agent). It's
a good compromise in my opinion and even does the job better than before
without the previous trouble of half-closed listeners.

And the rest are mostly cleanups.

As a reminder, if you have sensitive changes pending please post them
before the 15th so that we can get all the tricky stuff reviewed and
merged before the 30th. I'm aware that some developers will possibly be
busy preparing their talk for the conference that comes in two months,
so I expect a bit less bandwidth for reviews and fixes in the upcoming
weeks. By the way, by "sensitive changes", I mean anything that may
significantly affect build or stability of non-experimental stuff, as
well as a change of configuration. The variables stuff I just merged
qualifies, for example. I'll try to get some minimalistic thread-group
support by then, but with absolutely no guarantees given all the stuff
that remains to be done.

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.5/src/
   Git repository   : http://git.haproxy.org/git/haproxy.git/
   Git Web browsing : http://git.haproxy.org/?p=haproxy.git
   Changelog: http://www.haproxy.org/download/2.5/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/

Willy
---
Complete changelog :
Christopher Faulet (7):
  Revert "BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn 
may receive"
  BUG/MEDIUM: mux-h1: Remove "Upgrade:" header for requests with payload
  MINOR: htx: Skip headers with no value when adding a header list to a 
message
  CLEANUP: mux-h1: Remove condition rejecting upgrade requests with payload
  BUG/MEDIUM: stream-int: Don't block SI on a channel policy if EOI is 
reached
  BUG/MEDIUM: http-ana: Reset channels analysers when returning an error
  BUG/MINOR: filters: Set right FLT_END analyser depending on channel

Miroslav Zagorac (5):
  BUILD: opentracing: exclude the use of haproxy variables for the 
OpenTracing context
  BUG/MINOR: opentracing: enable the use of http headers without a set value
  CLEANUP: opentracing: use the haproxy function to generate uuid
  MINOR: opentracing: change the scope of the variable 'ot.uuid' from 
'sess' to 'txn'
  CLEANUP: opentracing: simplify the condition on the empty header

Tim Duesterhus (3):
  CLEANUP: Add haproxy/xxhash.h to avoid modifying import/xxhash.h
  CLEANUP: Move XXH3 macro from haproxy/compat.h to haproxy/xxhash.h
  BUG/MEDIUM lua: Add missing call to RESET_SAFE_LJMP in hlua_filter_new()

Tim Düsterhus (1):
  CLEANUP: ebmbtree: Replace always-taken elseif by else

Willy Tarreau (22):
  BUG/MINOR: config: reject configs using HTTP with bufsize >= 256 MB
  CLEANUP: htx: remove comments about "must be < 2

Re: [PATCH] BUG/???: lua: Add missing call to RESET_SAFE_LJMP in hlua_filter_new()

2021-09-12 Thread Willy Tarreau
On Sat, Sep 11, 2021 at 11:17:25PM +0200, Tim Duesterhus wrote:
> In one case before exiting leaving the function the panic handler was not
> reset.
> 
> Introduced in 69c581a09271e91d306e7b9080502a36abdc415e, which is 2.5+.
> No backport required.

Good catch, applied as medium since it seems none of us can clearly
predict the effect :-)

Thanks!
Willy



Re: [PATCH 2/4] BUG/MINOR: opentracing: enable the use of http headers without a set value

2021-09-12 Thread Willy Tarreau
On Sat, Sep 11, 2021 at 12:27:30AM +0200, Miroslav Zagorac wrote:
> On 09/11/2021 12:05 AM, Miroslav Zagorac wrote:
> > Hello all,
> > 
> > the second patch from the last series of patches has been redesigned
> > here, the ist() function is used to set an empty string instead of
> > working directly with the string pointer.
> > 
> > I thank Tim Düsterhus for his advice.
> 
> The operation comment has been slightly corrected.
> Sorry to bother you.  :)

Bah, I discovered that one after merging the first series :-/ I've
applied it on top as a cleanup with your description above as the
commit message since it's really what it's about.

Thanks,
Willy



Re: [PATCH] BUILD: opentracing: excluded use of haproxy variables for, OpenTracing context

2021-09-12 Thread Willy Tarreau
Hi guys,

thanks for working on fixing this, it's now merged. I've added a
tiny change to make sure that text_map is always initialized in
flt_ot_scope_run() because that made clang rightfully upset,
re-enabled OT in the CI since it's now OK.

Cheers,
Willy



Re: [PATCH] CLEANUP: Move XXH3 macro from haproxy/compat.h to haproxy/xxhash.h

2021-09-11 Thread Willy Tarreau
On Sat, Sep 11, 2021 at 08:29:46PM +0200, Tim Duesterhus wrote:
> This moves all the xxhash functionality into a single location.

Wow that was fast, now merged, thanks!
Willy



Re: [PATCH] CLEANUP: Add haproxy/xxhash.h to avoid modifying import/xxhash.h

2021-09-11 Thread Willy Tarreau
On Sat, Sep 11, 2021 at 08:06:01PM +0200, Dragan Dosen wrote:
> Hi Tim,
> 
> On 11. 09. 2021. 17:51, Tim Duesterhus wrote:
> > This solves setting XXH_INLINE_ALL in a cleaner way, because the imported
> > header is not modified, easing future updates.
> > 
> > see 6f7cc11e6dd0f01b437fba893da2edd2362660a2
> > ---
> > (...)
> 
> Thanks!
> 
> We might also want to move the "XXH3" defined in include/haproxy/compat.h to
> include/haproxy/xxhash.h; see 967e7e79a ("MEDIUM: xxhash: use the XXH3
> functions to generate 64-bit hashes"). Just to have all of our xxHash
> related defines in a single file.

That would indeed be cleaner now that there's a dedicated file.

Willy



Re: [PATCH] CLEANUP: Add haproxy/xxhash.h to avoid modifying import/xxhash.h

2021-09-11 Thread Willy Tarreau
Hi Tim,

On Sat, Sep 11, 2021 at 05:51:13PM +0200, Tim Duesterhus wrote:
> This solves setting XXH_INLINE_ALL in a cleaner way, because the imported
> header is not modified, easing future updates.

Excellent idea, I like this. It's indeed much cleaner and more logical
this way. This may also serve as an example of how to better integrate
external components in the future if needed.

Now applied, thanks!
Willy



Re: [ANNOUNCE] HTX vulnerability from 2.0 to 2.5-dev

2021-09-09 Thread Willy Tarreau
Hi Bjoern,

On Thu, Sep 09, 2021 at 08:18:24PM +0200, bjun...@gmail.com wrote:
> Hi,
> 
> is HAProxy 2.0.x with "no option http-use-htx" also affected by
> this vulnerability?

No it's not. I thought I mentioned it but it's possible that I forgot
it in the end.

Regards,
Willy



Re: I just broke opentracing :-(

2021-09-09 Thread Willy Tarreau
Hi Miroslav,

On Wed, Sep 08, 2021 at 08:02:35PM +0200, Miroslav Zagorac wrote:
> On 09/08/2021 07:57 PM, Miroslav Zagorac wrote:
> > On 09/08/2021 07:42 PM, Willy Tarreau wrote:
> > > No rush on this one, I'll let you think about it, just let me know if we
> > > need to temporarily disable it from the CI, or if you just want a day or
> > > two of checking before knowing if you can get a quick solution or if it
> > > will take more time.
> > > 
> > 
> > Thank you for understanding. For a start it would be good to exclude
> > ot filter from the compilation process so as not to interfere with the
> > development and testing of other code.

OK I can do that.

> > Next, I could throw out the part related to context transfer via
> > variables with #ifdef/#endif blocks, so that the ot filter code is
> > compiled without it.
> > 
> > After that the problem with variables could be solved without
> > interfering with the rest of the coding/compilation process and the
> > ot filter will be functional (except for the part with context transfer
> > via variables, but the question is whether anyone uses it).

OK. Given that I really have no idea what it's used for (from the
user's perspective), I cannot judge. But maybe we can start with
this and ask for testers.

> > The variable 'sess.ot.uuid' is just set, it is not used anywhere (as
> > far as memory serves me). I imagined that this could be used if the
> > user needs it, but it is not necessary.
> > 
> 
> OK, it is still used but again only as an example that it can be used
> and not that it should be used.
> 
> In our examples uuid is set as one of the data from the opentracing
> 'baggage' data group:
> 
> test/ctx/ot.cfg:
> ---
>baggage "haproxy_id" var(sess.ot.uuid)
> ---

Yes, that's where I found it but didn't know what its use was. A few
points however:
  - sess.ot.uuid will be shared by all requests on the same session,
which is probably not what you want (e.g. for requests coming in
H2, the last one setting it will replace all other ones' value
while they're working with it). I think you should change its
scope to be limited to the transaction only (txn.ot.uuid). It's
also used in debugging outputs where it will be per-request and
may not match what the requests will log because of this.

  - I finally found its definition here in filter.h:
  #define FTL_OT_VAR_UUID   "sess", "ot", "uuid"

Please note the typo in the macro name (FTL instead of FLT)

  - the comment in struct flt_ot_runtime_context for uuid says
"Randomly generated UUID". We already have a function to produce a
random UUID (ha_generate_uuid()) but flt_ot_runtime_context_init()
makes its own from elements that at first glance look like they're
filled from other locations (uuid.node, uuid.clock_seq etc), but in
fact are simply aliased on the random ints. In this case better use
the existing function and avoid duplicating the painful output format
generation. If you need to keep a copy of the two randoms, just
modify the existing function to pass an optional array to store
the randoms. At first glance they're not used though.

  - there's quite a bunch of functions with no description at all of
their purpose, input values, return values etc. I know pretty well
how painful it is to document that, especially since every single
time we have to go through this we find bugs due to the caller and
callee disagreeing on input domain or output values (typically the
type of thing Coverity loves to look at). This is also why it's
important. Hint: at least fill the description entry and explain
limits/restrictions if any. The locations of empty descriptions
can be found with this:

  $ pcregrep -nMo1 '^ \* DESCRIPTION\n \*   -\n' addons/ot/src/*.c

I'm not asking you to add all of them immediately, but I'm willing
to apply cleanup patches that routinely add a few of them when you
have a bit of time to work on this. And of course bugfixes as well
if you find bugs while documenting.

Thanks,
Willy



Re: I just broke opentracing :-(

2021-09-08 Thread Willy Tarreau
On Wed, Sep 08, 2021 at 07:23:59PM +0200, Miroslav Zagorac wrote:
> On 09/08/2021 06:46 PM, Willy Tarreau wrote:
> > I have no doubt about this, what I mean is that once such needs are
> > identified, as much as possible we should try to move these parts into
> > the proper location (vars), or if really needed, that must be clearly
> > explained around the code so that someone else may try to figure what
> > to change there. For example I've seen a function that parses scope
> > names and turns them list heads, that's the perfect example of something
> > that ought to have been adapted into vars.c if what was provided was not
> > sufficient, because since then new scopes were added without anyone
> > noticing they were listed there, so that function was not updated (there
> > I doubt anyone cares but conceptually it's a problem).
> 
> It is clear to me.. at the time when the code for the ot filter was
> being developed, it was easier for me to write the functions I was
> missing (ie the content I was missing in the existing functions) and
> then see what to do with it .. and it remained like that after
> completion.  At the time, I didn't think that something more
> significant could change in working with variables one day.

I know how we end up like this, I'm not saying that in bad terms, just
as something to be careful about in the future. The codebase is huge
nowadays and no single person knows even 10% of it anymore, so if we
continue to create hacks or temporary stuff in various corners it's
unmaintainable and breaks all over every time someone tries to fix a
design problem. And that's already what happened with these shitty
variables that have been causing us some gray hair for a few years
now.

> I was wrong, obviously.

No worries.

> > > The moment we write the variables we know their
> > > names but at the moment we read them we don't know how many there are
> > > and their complete name, rather we take all the variables of a
> > > particular prefix.
> > 
> > Hmmm that may become quite problematic then since we don't have that
> > name anymore :-/  Are these variables used only for internal stuff in
> > OT or are they used to exchange with the rest of the configuration ?
> 
> In opentracing there is a context of data, it is a group of data that
> is transferred from one place where they are used to another place.
> 
> This data is defined as a key:value, where the use of haproxy variables
> suited me perfectly.  The problem is that it is not known in advance
> what the keys of this data are and how much of this data is, this is
> set by the user in the opentracing configuration (which has nothing to
> do with haproxy).

OK thanks for explaining! Now I understand indeed why you reused variables
for this.

> The operations that handle this data are 'inject' and 'extract', the
> first writes all the data from the opentracing context to the haproxy
> variable and the second extracts the data from the haproxy variable to
> the opentracing context.
> 
> During the 'inject' operation, it is known which are the variables (or
> their names) because they are generated from the 'key' part of the
> opentracing data.
> 
> During the 'extract' operation this is not known, ie only the prefix of
> the variable under which one of the 'inject' operations of that
> variable is written is known.

Got it. It's just as if you'd serialize a blob that it extracted on
the other side, in some sort.

> > Do you have a concrete example of a use case for such prefixes on
> > receipt ? For example if you're dumping all variables from your current
> > request, maybe that's enough. Or if you need prefixes for dumping all
> > variables of a particular scope, we could proceed differently.
> 
> Context transfer is conceived in two ways, one is via variables (if
> transferred within the same haproxy process) and the other is via http
> header (if transferred between different processes, there does not have
> to be haproxy as one of the processes).
> 
> An example of this is the configuration from the test/ctx directory,
> more precisely the test/ctx/ot.cfg file.

The only explicit variable I'm seeing there is sess.ot.uuid, so I
guess that the rest is totally transparent, and that if needed we can
arrange so that sess.ot.uuid is always created on new sessions.

> An example of context transfer via http header is in the test/fe and
> test/be directories where data is transferred from the frontend process
> via the http header to the backend process, which can then add
> opentracing data to the trace initiated in the frontend process.
> 
> In principle, context transfer via haproxy variables can be omitted
> because this data can also be transferred via 

Re: I just broke opentracing :-(

2021-09-08 Thread Willy Tarreau
On Wed, Sep 08, 2021 at 06:30:15PM +0200, Miroslav Zagorac wrote:
> there is a reason why i used some functions related to
> reading/setting/searching variables.  If I could use the original haproxy
> functions, I wouldn't write these because duplicating the
> code doesn't make sense.

I have no doubt about this, what I mean is that once such needs are
identified, as much as possible we should try to move these parts into
the proper location (vars), or if really needed, that must be clearly
explained around the code so that someone else may try to figure what
to change there. For example I've seen a function that parses scope
names and turns them list heads, that's the perfect example of something
that ought to have been adapted into vars.c if what was provided was not
sufficient, because since then new scopes were added without anyone
noticing they were listed there, so that function was not updated (there
I doubt anyone cares but conceptually it's a problem).

> What I have seen so far is that the 'name' member of the vars structure
> has actually been deleted and that the var_clear() function has been
> given another parameter.

Yep, previously, there was a "name" member pointing to a pre-allocated
entry into the names table, that contained the part after the scope. The
goal was to allocate names only once at boot time when variables couldn't
be added at runtime. Now instead we don't keep this name and we only keep
a 64-bit hash of that same part.

> Generally, in the ot filter it is possible to transfer data via a group
> of variables with a common prefix, so that in one place we set all the
> variables and their values and in another we read all the variables
> with a given prefix.

OK I see, thanks for explaining!

> The moment we write the variables we know their
> names but at the moment we read them we don't know how many there are
> and their complete name, rather we take all the variables of a
> particular prefix.

Hmmm that may become quite problematic then since we don't have that
name anymore :-/  Are these variables used only for internal stuff in
OT or are they used to exchange with the rest of the configuration ?
Do you have a concrete example of a use case for such prefixes on
receipt ? For example if you're dumping all variables from your current
request, maybe that's enough. Or if you need prefixes for dumping all
variables of a particular scope, we could proceed differently.

> I need to take a closer look at how ot filter source can be redesigned
> to retain functionality and use the new structure definition to store
> variables.

Just thinking loud, if you absolutely need to enumerate a list of
variables when setting them, maybe it could be possible to enumerate
their names into another well-known one and look up that one first on
the other side?

Thanks!
Willy



I just broke opentracing :-(

2021-09-08 Thread Willy Tarreau
Hi Miroslav,

I just discovered that the changes I've made to the totally broken
variables API now broke opentracing because apparently it's using
some of the variable code's internals for its own use:

   https://github.com/haproxy/haproxy/runs/3545475810

That's annoying because unbreaking the API was already extremely
complicated due to the global name table limitation that became a
feature on its own, but now I see that some upper level code outside
of vars/ is also making intimate use of that broken API (e.g. in
flt_ot_vars_unset), and I have no idea what it tries to do nor why.

Most of the breakage comes from all the undocumented functions
flt_ot_vars_* but I don't even understand why this does not use
the vars_* API that Lua, CLI and SPOE already use. Note that it
doesn't seem that the breakage is large, so most likely we can
figure together a few changes to perform to fix it, but reversing
all that is quite a pain, and even if I went through the process
of trying to build it I wouldn't know if what I'm doing works or
not :-/

The changes that broke are in the following commit range which tries
hard to minimize the visible changes:

   10080716b..55f8a830d

I would highly appreciate it if you could have a look, figure what needs
to be done, if possible using the public vars API, otherwise that we
discuss what needs to be extended, and if you could document these
functions, because, to be honest, the following doesn't help me much:

 /***
  * NAME
  *   flt_ot_vars_get -
  *
  * ARGUMENTS
  *   s  -
  *   scope  -
  *   prefix -
  *   opt-
  *   err-
  *
  * DESCRIPTION
  *   -
  *
  * RETURN VALUE
  *   -
  */
 struct otc_text_map *flt_ot_vars_get(struct stream *s, const char *scope, 
const char *prefix, uint opt, char **err)

Thanks,
Willy



Re: BoringSSL commit dddb60e breaks compilation of HAProxy

2021-09-08 Thread Willy Tarreau
On Wed, Sep 08, 2021 at 12:34:49PM +0200, Aleksandar Lazic wrote:
> On 08.09.21 11:07, Willy Tarreau wrote:
> > On Wed, Sep 08, 2021 at 01:58:00PM +0500,  ??? wrote:
> > > ??, 8 . 2021 ?. ? 13:54, Willy Tarreau :
> > > 
> > > > On Wed, Sep 08, 2021 at 12:05:23PM +0500,  ??? wrote:
> > > > > Hello, Bob
> > > > > 
> > > > > I tracked an issue  https://github.com/haproxy/haproxy/issues/1386
> > > > > 
> > > > > 
> > > > > let's track activity there
> > > > 
> > > > Quite frankly, I'm seriously wondering how long we'll want to keep
> > > > supporting that constantly breaking library. Does it still provide
> > > > 
> > > 
> > > by "let us track activity" I do not mean that we are going to maintain
> > > BoringSSL :)
> > > 
> > > people will come from time to time with BoringSSL support request. 
> > > Existing
> > > github issue is good to redirect them to.
> > 
> > Oh this is how I understood it as well, I just think that you and a
> > handful of others have already spent a lot of energy on that lib and
> > I was only encouraging you not to spend way more than what you find
> > reasonable after this issue is created :-)
> 
> Is there another library which have the quic stuff implemented which
> can be used for quic development?

Fred told me that he manages to build it using the fork of openssl that
contains the proper stuff. Hopefully it should get merged soon. But with
BoringSSL the problem is that something that works on monday suddenly
fails to build on tuesday and there's no stable branch so you're just
forced to change your API to adapt to it on the fly. I don't want to
blame them too much because they always warned against this. It's been
convenient to start on QUIC but as soon as we can avoid this pain it
will be better to get rid of it.

Willy



Re: BoringSSL commit dddb60e breaks compilation of HAProxy

2021-09-08 Thread Willy Tarreau
On Wed, Sep 08, 2021 at 01:58:00PM +0500,  ??? wrote:
> ??, 8 . 2021 ?. ? 13:54, Willy Tarreau :
> 
> > On Wed, Sep 08, 2021 at 12:05:23PM +0500,  ??? wrote:
> > > Hello, Bob
> > >
> > > I tracked an issue  https://github.com/haproxy/haproxy/issues/1386
> > >
> > >
> > > let's track activity there
> >
> > Quite frankly, I'm seriously wondering how long we'll want to keep
> > supporting that constantly breaking library. Does it still provide
> >
> 
> by "let us track activity" I do not mean that we are going to maintain
> BoringSSL :)
> 
> people will come from time to time with BoringSSL support request. Existing
> github issue is good to redirect them to.

Oh this is how I understood it as well, I just think that you and a
handful of others have already spent a lot of energy on that lib and
I was only encouraging you not to spend way more than what you find
reasonable after this issue is created :-)

Willy



Re: BoringSSL commit dddb60e breaks compilation of HAProxy

2021-09-08 Thread Willy Tarreau
On Wed, Sep 08, 2021 at 12:05:23PM +0500,  ??? wrote:
> Hello, Bob
> 
> I tracked an issue  https://github.com/haproxy/haproxy/issues/1386
> 
> 
> let's track activity there

Quite frankly, I'm seriously wondering how long we'll want to keep
supporting that constantly breaking library. Does it still provide
any value that I'm not aware of ? It's not even possible to have
the same version at the beginning and at the end of a maintenance
branch, it's designed to constantly change and documented as such.

I'd be strongly in favor for dropping it once it becomes too painful
to deal with, and I sense that we're starting to get closer to that
point now.

Willy



Re: [ANNOUNCE] HTX vulnerability from 2.0 to 2.5-dev

2021-09-07 Thread Willy Tarreau
On Tue, Sep 07, 2021 at 09:39:41PM +0200, Vincent Bernat wrote:
>  ?  7 September 2021 17:27 +02, Willy Tarreau:
> 
> > I'd like to thank the usual distro maintainers for having accepted to
> > produce yet another version of their packages in a short time. Hopefully
> > now we can all get back to development!
> 
> For Debian/Ubuntu, the fixed versions are:
> 
> 2.4.3-2
> 2.4.3-2~bpo10+1
> 2.4.3-2~bpo11+1
> 2.4.3-2ppa2~bionic
> 2.4.3-2ppa1~focal
> 
> 2.3.13-2
> 2.3.13-2~bpo10+1
> 2.3.13-2ppa1~bionic
> 2.3.13-2ppa1~focal
> 
> 2.2.16-3
> 2.2.16-3~bpo9+1
> 2.2.16-3~bpo10+1
> 2.2.16-3ppa1~bionic
> 2.2.16-3ppa1~focal
> 2.2.9-2+deb11u2 (to be released pretty soon)
> 2.2.9-2+deb11u2~bpo10+1 (not released yet)
> 2.2.9-1ubuntu0.2 (not 100% sure as it is not released yet)
> 
> 2.0.24-1
> 2.0.24-1~bpo9+1
> 2.0.24-1~bpo10+1
> 2.0.24-1ppa1~xenial
> 2.0.24-1ppa1~bionic
> 2.0.24-1ppa1~focal
> 2.0.13-2ubuntu0.3 (not 100% sure as it is not released yet)

Thank you Vincent!
Willy



[ANNOUNCE] haproxy-2.0.25

2021-09-07 Thread Willy Tarreau
Hi,

HAProxy 2.0.25 was released on 2021/09/07. It added 16 new commits
after version 2.0.24.

This version essentially aims at fixing the HTX header encoding issue
mentioned in a previous message, and that may lead to a request smuggling
attack. All users must update.

Another important fix for some users is the relaxed double-slash rule in
the H2 parser, because the previous H2 fixes would (rightfully) block
requests starting by "//" due to a bug in the H2 spec itself! The nice
thing is that it allowed to spot and fix a bug in the spec :-)

A recent fix for "option abortonclose" has resulted in an issue for a
user who sometimes sees some streams looping. The fix was reverted for
now as the situation is worse than before, and the issue is still under
investigation.

A failed backport of a recent fix in 2.2.16 for early connection failures
was better addressed this time. It would manifest itself by high CPU usage
on certain threads, with the poller reporting the same FDs all the time.

The remaining fixes are less important:
  - use thread-safe versions of localtime()/gmtime() in the ltime/utime
converters; previously it was theoretically possible to occasionally
retrieve a bad date under high thread contention

  - fix for incorrect output size check in the base64dec/base64urldec
converters that could write up to 2 extra bytes, but normally they're
always used with outputs having sufficient room so I can't figure a
case where it could have represented a practical problem.

  - tune.bufsize is now checked for being smaller than 256 MB in HTX mode
(that's the hard limit).

  - Lua's initialization of sample converters now uses strlcpy2() and not
strncpy(), as this last one used to fill the entire buffer with zeroes,
resulting in a measurable startup time when using large buffers (a
second or so with 1 MB buffers).

  - the sc-set-gpt* action parser was off by one argument and was ignoring
one word before the "if" condition, forcing to write garbage there (or
a second "if").

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 (2):
  Revert "BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn 
may receive"
  MINOR: action: Use a generic function to check validity of an action rule 
list

Dragan Dosen (1):
  BUG/MEDIUM: base64: check output boundaries within base64{dec,urldec}

Tim Duesterhus (3):
  BUG/MINOR threads: Use get_(local|gm)time instead of (local|gm)time
  BUG/MINOR: tools: Fix loop condition in dump_text()
  CLEANUP: Add missing include guard to signal.h

Willy Tarreau (10):
  BUG/MEDIUM: sock: really fix detection of early connection failures in 
for 2.3-
  REGTESTS: abortonclose: after retries, 503 is expected, not close
  MINOR: compiler: implement an ONLY_ONCE() macro
  BUG/MINOR: lua: use strlcpy2() not strncpy() to copy sample keywords
  BUG/MINOR: ebtree: remove dependency on incorrect macro for bits per long
  DOC: configuration: remove wrong tcp-request examples in tcp-response
  BUG/MINOR: config: reject configs using HTTP with bufsize >= 256 MB
  CLEANUP: htx: remove comments about "must be < 256 MB"
  BUG/MAJOR: htx: fix missing header name length check in 
htx_add_header/trailer
  REGTESTS: mark http_abortonclose as broken

---



[ANNOUNCE] haproxy-2.2.17

2021-09-07 Thread Willy Tarreau
Hi,

HAProxy 2.2.17 was released on 2021/09/07. It added 18 new commits
after version 2.2.16.

This version essentially aims at fixing the HTX header encoding issue
mentioned in a previous message, and that may lead to a request smuggling
attack. All users must update.

Another important fix for some users is the relaxed double-slash rule in
the H2 parser, because the previous H2 fixes would (rightfully) block
requests starting by "//" due to a bug in the H2 spec itself! The nice
thing is that it allowed to spot and fix a bug in the spec :-)

A recent fix for "option abortonclose" has resulted in an issue for a
user who sometimes sees some streams looping. The fix was reverted for
now as the situation is worse than before, and the issue is still under
investigation.

A failed backport of a recent fix in 2.2.16 for early connection failures
was better addressed this time. It would manifest itself by high CPU usage
on certain threads, with the poller reporting the same FDs all the time.

The remaining fixes are less important:
  - use thread-safe versions of localtime()/gmtime() in the ltime/utime
converters; previously it was theoretically possible to occasionally
retrieve a bad date under high thread contention

  - fix for incorrect output size check in the base64dec/base64urldec
converters that could write up to 2 extra bytes, but normally they're
always used with outputs having sufficient room so I can't figure a
case where it could have represented a practical problem.

  - tune.bufsize is now checked for being smaller than 256 MB in HTX mode
(that's the hard limit).

  - Lua's initialization of sample converters now uses strlcpy2() and not
strncpy(), as this last one used to fill the entire buffer with zeroes,
resulting in a measurable startup time when using large buffers (a
second or so with 1 MB buffers).

  - the sc-set-gpt* action parser was off by one argument and was ignoring
one word before the "if" condition, forcing to write garbage there (or
a second "if").

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.2/src/
   Git repository   : http://git.haproxy.org/git/haproxy-2.2.git/
   Git Web browsing : http://git.haproxy.org/?p=haproxy-2.2.git
   Changelog: http://www.haproxy.org/download/2.2/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/

Willy
---
Complete changelog :
Christopher Faulet (2):
  Revert "BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn 
may receive"
  MINOR: action: Use a generic function to check validity of an action rule 
list

Dragan Dosen (1):
  BUG/MEDIUM: base64: check output boundaries within base64{dec,urldec}

Tim Duesterhus (3):
  BUG/MINOR threads: Use get_(local|gm)time instead of (local|gm)time
  BUG/MINOR: tools: Fix loop condition in dump_text()
  CLEANUP: Add missing include guard to signal.h

Willy Tarreau (12):
  BUG/MEDIUM: h2: match absolute-path not path-absolute for :path
  BUG/MEDIUM: sock: really fix detection of early connection failures in 
for 2.3-
  REGTESTS: abortonclose: after retries, 503 is expected, not close
  BUG/MINOR: stick-table: fix the sc-set-gpt* parser when using expressions
  MINOR: compiler: implement an ONLY_ONCE() macro
  BUG/MINOR: lua: use strlcpy2() not strncpy() to copy sample keywords
  BUG/MINOR: ebtree: remove dependency on incorrect macro for bits per long
  DOC: configuration: remove wrong tcp-request examples in tcp-response
  BUG/MINOR: config: reject configs using HTTP with bufsize >= 256 MB
  CLEANUP: htx: remove comments about "must be < 256 MB"
  BUG/MAJOR: htx: fix missing header name length check in 
htx_add_header/trailer
  REGTESTS: mark http_abortonclose as broken

---



[ANNOUNCE] haproxy-2.3.14

2021-09-07 Thread Willy Tarreau
Hi,

HAProxy 2.3.14 was released on 2021/09/07. It added 19 new commits
after version 2.3.13.

This version essentially aims at fixing the HTX header encoding issue
mentioned in a previous message, and that may lead to a request smuggling
attack. All users must update.

Another important fix for some users is the relaxed double-slash rule in
the H2 parser, because the previous H2 fixes would (rightfully) block
requests starting by "//" due to a bug in the H2 spec itself! The nice
thing is that it allowed to spot and fix a bug in the spec :-)

A recent fix for "option abortonclose" has resulted in an issue for a
user who sometimes sees some streams looping. The fix was reverted for
now as the situation is worse than before, and the issue is still under
investigation.

A failed backport of a recent fix in 2.3.13 for early connection failures
was better addressed this time. It would manifest itself by high CPU usage
on certain threads, with the poller reporting the same FDs all the time.

The remaining fixes are less important:
  - use thread-safe versions of localtime()/gmtime() in the ltime/utime
converters; previously it was theoretically possible to occasionally
retrieve a bad date under high thread contention

  - fix for incorrect output size check in the base64dec/base64urldec
converters that could write up to 2 extra bytes, but normally they're
always used with outputs having sufficient room so I can't figure a
case where it could have represented a practical problem.

  - tune.bufsize is now checked for being smaller than 256 MB in HTX mode
(that's the hard limit).

  - Lua's initialization of sample converters now uses strlcpy2() and not
strncpy(), as this last one used to fill the entire buffer with zeroes,
resulting in a measurable startup time when using large buffers (a
second or so with 1 MB buffers).

  - the sc-set-gpt* action parser was off by one argument and was ignoring
one word before the "if" condition, forcing to write garbage there (or
a second "if").

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.3/src/
   Git repository   : http://git.haproxy.org/git/haproxy-2.3.git/
   Git Web browsing : http://git.haproxy.org/?p=haproxy-2.3.git
   Changelog: http://www.haproxy.org/download/2.3/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/

Willy
---
Complete changelog :
Christopher Faulet (2):
  Revert "BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn 
may receive"
  MINOR: action: Use a generic function to check validity of an action rule 
list

Dragan Dosen (1):
  BUG/MEDIUM: base64: check output boundaries within base64{dec,urldec}

Tim Duesterhus (3):
  BUG/MINOR threads: Use get_(local|gm)time instead of (local|gm)time
  BUG/MINOR: tools: Fix loop condition in dump_text()
  CLEANUP: Add missing include guard to signal.h

Willy Tarreau (13):
  BUG/MEDIUM: h2: match absolute-path not path-absolute for :path
  BUG/MEDIUM: sock: really fix detection of early connection failures in 
for 2.3-
  REGTESTS: abortonclose: after retries, 503 is expected, not close
  BUG/MINOR: stick-table: fix the sc-set-gpt* parser when using expressions
  MINOR: compiler: implement an ONLY_ONCE() macro
  BUG/MINOR: lua: use strlcpy2() not strncpy() to copy sample keywords
  BUG/MINOR: ebtree: remove dependency on incorrect macro for bits per long
  BUG/MINOR: vars: fix set-var/unset-var exclusivity in the keyword parser
  DOC: configuration: remove wrong tcp-request examples in tcp-response
  BUG/MINOR: config: reject configs using HTTP with bufsize >= 256 MB
  CLEANUP: htx: remove comments about "must be < 256 MB"
  BUG/MAJOR: htx: fix missing header name length check in 
htx_add_header/trailer
  REGTESTS: mark http_abortonclose as broken

---



[ANNOUNCE] haproxy-2.4.4

2021-09-07 Thread Willy Tarreau
Hi,

HAProxy 2.4.4 was released on 2021/09/07. It added 21 new commits
after version 2.4.3.

This version essentially aims at fixing the HTX header encoding issue
mentioned in a previous message, and that may lead to a request smuggling
attack. All users must update.

Another important fix for some users is the relaxed double-slash rule in
the H2 parser, because the previous H2 fixes would (rightfully) block
requests starting by "//" due to a bug in the H2 spec itself! The nice
thing is that it allowed to spot and fix a bug in the spec :-)

A recent fix for "option abortonclose" has resulted in an issue for a
user who sometimes sees some streams looping. The fix was reverted for
now as the situation is worse than before, and the issue is still under
investigation.

The remaining fixes are less important:
  - use thread-safe versions of localtime()/gmtime() in the ltime/utime
converters; previously it was theoretically possible to occasionally
retrieve a bad date under high thread contention

  - fix for incorrect output size check in the base64dec/base64urldec
converters that could write up to 2 extra bytes, but normally they're
always used with outputs having sufficient room so I can't figure a
case where it could have represented a practical problem.

  - tune.bufsize is now checked for being smaller than 256 MB in HTX mode
(that's the hard limit).

  - Lua's initialization of sample converters now uses strlcpy2() and not
strncpy(), as this last one used to fill the entire buffer with zeroes,
resulting in a measurable startup time when using large buffers (a
second or so with 1 MB buffers).

  - the sc-set-gpt* action parser was off by one argument and was ignoring
one word before the "if" condition, forcing to write garbage there (or
a second "if").

  - the idle time computation would overflow for sleep times greater than
42 seconds and could mistakenly report low idle values under low load.
The idle value reported in the stats will now also reflect the process
wide value and not the randomly picked thread's value.

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.4/src/
   Git repository   : http://git.haproxy.org/git/haproxy-2.4.git/
   Git Web browsing : http://git.haproxy.org/?p=haproxy-2.4.git
   Changelog: http://www.haproxy.org/download/2.4/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/

Willy
---
Complete changelog :
Christopher Faulet (1):
  Revert "BUG/MINOR: stream-int: Don't block reads in si_update_rx() if chn 
may receive"

Dragan Dosen (2):
  BUG/MEDIUM: base64: check output boundaries within base64{dec,urldec}
  BUG/MINOR: base64: base64urldec() ignores padding in output size check

Tim Duesterhus (3):
  BUG/MINOR threads: Use get_(local|gm)time instead of (local|gm)time
  BUG/MINOR: tools: Fix loop condition in dump_text()
  CLEANUP: Add missing include guard to signal.h

Willy Tarreau (15):
  BUG/MEDIUM: h2: match absolute-path not path-absolute for :path
  REGTESTS: http_upgrade: fix incorrect expectation on TCP->H1->H2
  REGTESTS: abortonclose: after retries, 503 is expected, not close
  MINOR: hlua: take the global Lua lock inside a global function
  BUG/MINOR: stick-table: fix the sc-set-gpt* parser when using expressions
  MINOR: compiler: implement an ONLY_ONCE() macro
  BUG/MINOR: lua: use strlcpy2() not strncpy() to copy sample keywords
  BUG/MINOR: time: fix idle time computation for long sleeps
  MINOR: time: add report_idle() to report process-wide idle time
  BUG/MINOR: ebtree: remove dependency on incorrect macro for bits per long
  BUG/MINOR: vars: fix set-var/unset-var exclusivity in the keyword parser
  DOC: configuration: remove wrong tcp-request examples in tcp-response
  BUG/MINOR: config: reject configs using HTTP with bufsize >= 256 MB
  CLEANUP: htx: remove comments about "must be < 256 MB"
  BUG/MAJOR: htx: fix missing header name length check in 
htx_add_header/trailer

---



[ANNOUNCE] HTX vulnerability from 2.0 to 2.5-dev

2021-09-07 Thread Willy Tarreau
Hi everyone,   

Right after the previous announce of HTTP/2 vulnerabilities, a group
of security researchers from JFrog Security have been looking for the
possibility of remaining issues around the same topic. While there was
nothing directly exploitable, Ori Hollander found a bug in the HTTP
header name length encoding in the HTX representation by which the most
significant bit of the name's length can slip into the value's least
significant bit, and figured he could craft a valid request that could
inject a dummy content-length on input that would be produced on output
in addition to the other one, resulting in the possibility of a blind
request smuggling attack ("blind" because the response never gets back
to the attacker). Quite honestly they've done an excellent job at
spotting this one because it's not every day that you manage to turn
a single-bit overflow into an extra request, and figuring this required
to dig deeply into the layers! It's likely that they'll publish something
shortly about their finding.

CVE-2021-40346 was assigned to this issue, which affects versions 2.0
and above. I'm going to emit new maintenance releases for 2.0, 2.2, 2.3
and 2.4 (2.5 still being in development, it will be released a bit later).

A possible workaround for those who cannot upgrade is to block requests
and responses featuring more than one content-length header after the
overflow occured; these ones are always invalid because they're always
resolved during the parsing phase, hence this condition never reaches
the HTTP layer:

   http-request  deny if { req.hdr_cnt(content-length) gt 1 }
   http-response deny if { res.hdr_cnt(content-length) gt 1 }

I'd like to thank the usual distro maintainers for having accepted to
produce yet another version of their packages in a short time. Hopefully
now we can all get back to development!

Thanks,
Willy



Re: [PATCH]: dragonflybsd build fix

2021-09-04 Thread Willy TARREAU
Hi David,

On Sat, Sep 04, 2021 at 09:01:11AM +0100, David CARLIER wrote:
> Hi here a little fix proposal for this platform.
> 
> Cheers.

> From 6cfa1fce839504e04584d1bfedee188bc21c32b1 Mon Sep 17 00:00:00 2001
> From: DC 
> Date: Sat, 4 Sep 2021 09:58:57 +0100
> Subject: [PATCH] BUILD/MINOR: dragonfly build fix.
> 
> build failure since __read_mostly is undefined.

This seems to contradict the previous patch that touched this area:

commit d272b409d73f71b1a85dcc78b380a6188d194329
Author: Amaury Denoyelle 
Date:   Fri Apr 23 16:35:13 2021 +0200

BUILD: compiler: do not use already defined __read_mostly on dragonfly

DragonflyBSD already has an attribute __read_mostly which serves the
same purpose as the one in compiler.h.

As such I suspect that it might depend on the version and that we ought
to avoid relying on the OS but on the root cause of the problem, i.e.
that the macro is already defined. Thus for me the right fix would be
this:

- #if !defined(__DragonFly__)
+ #if !defined(__read_mostly)
  #define __read_mostly   HA_SECTION("read_mostly")
  #endif

Willy



[ANNOUNCE] haproxy-2.5-dev6

2021-09-03 Thread Willy Tarreau
http://git.haproxy.org/git/haproxy.git/
   Git Web browsing : http://git.haproxy.org/?p=haproxy.git
   Changelog: http://www.haproxy.org/download/2.5/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/

Willy
---
Complete changelog :
Remi Tricot-Le Breton (5):
  MINOR: log: Remove log-error-via-logformat option
  MINOR: log: Add new "error-log-format" option
  MINOR: ssl: Add new ssl_bc_hsk_err sample fetch
  MINOR: connection: Add a connection error code sample fetch for backend 
side
  REGTESTS: ssl: Add tests for bc_conn_err and ssl_bc_hsk_err sample fetches

Tim Duesterhus (3):
  BUG/MINOR threads: Use get_(local|gm)time instead of (local|gm)time
  BUG/MINOR: tools: Fix loop condition in dump_text()
  CLEANUP: Add missing include guard to signal.h

Willy Tarreau (24):
  BUILD: ssl: next round of build warnings on LIBRESSL_VERSION_NUMBER
  BUILD: ssl: fix two remaining occurrences of #if USE_OPENSSL
  BUILD: tools: properly guard __GLIBC__ with defined()
  BUILD: globally enable -Wundef
  BUG/MAJOR: queue: better protect a pendconn being picked from the proxy
  MINOR: http-rules: add a new "ignore-empty" option to redirects.
  CI: Github Actions: temporarily disable BoringSSL builds
  BUG/MINOR: vars: fix set-var/unset-var exclusivity in the keyword parser
  BUG/MINOR: vars: improve accuracy of the rules used to check expression 
validity
  MINOR: sample: add missing ARGC_ entries
  BUG/MINOR: vars: properly set the argument parsing context in the 
expression
  DOC: configuration: remove wrong tcp-request examples in tcp-response
  MEDIUM: vars: add a new "set-var-fmt" action
  BUG/MEDIUM: vars: run over the correct list in release_store_rules()
  BUG/MINOR: vars: truncate the variable name in error reports about scope.
  BUG/MINOR: vars: do not talk about global section in CLI errors for 
set-var
  CLEANUP: vars: name the temporary proxy "CFG" instead of "CLI" for global 
vars
  MINOR: log: make log-format expressions completely usable outside of 
req/resp
  MINOR: vars: add a "set-var-fmt" directive to the global section
  MEDIUM: vars: also support format strings in CLI's "set var" command
  CLEANUP: vars: factor out common code from vars_get_by_{desc,name}
  MINOR: vars: make vars_get_by_* support an optional default value
  MINOR: vars: make the vars() sample fetch function support a default value
  BUILD: ot: add argument for default value to vars_get_by_name()

---



Re: Server current weight

2021-09-02 Thread Willy Tarreau
Hello,

On Sun, Aug 29, 2021 at 11:28:43AM +, Prytoegrian wrote:
> I think I found a bug in Haproxy but I first want to be sure it's a real one.
> It might just be a semantic misunderstanding of mine.
> 
> I described a backend with two servers, one of them with "slowstart" option.
> When I set it in maintenance then ready again, CLI `get weight` doesn't show
> current weight, like the stat page could displays, but the user defined one.
> 
> The documentation in the associated function states
> ```c
> // cli_parse_get_weight
> /* return server's effective weight at the moment */
> ```
> effective weight having a precise meaning in haproxy.

The CLI's documentation (management.txt) is correct and says:

 get weight /
  Report the current weight and the initial weight of server  in
  backend  or an error if either doesn't exist. The initial weight is
  the one that appears in the configuration file. Both are normally equal
  unless the current weight has been changed. Both the backend and the server
  may be specified either by their name or by their numeric ID, prefixed with a
  sharp ('#').

We wanted the "get" commands to perform the opposite of "set" so that
scripts can use "get" on a value, recompute it and send the new one
with "set".

Regarding servers' weights, there are 3 values:
  - the initial one (that would rather be called the configured one),
it is the weight that was initially configured with the server and
that is used for percentages (e.g. if you change your weight to 50%,
it's 50% of this one).

  - the user weight, which is the currently configured one, as the
result of a "set weight" command, either using an absolute value
or a relative one. It *used* to be called the effective one a long
time ago (which explains the comment you found in the source file
above).

  - the effective weight, which is the one applied to the server at the
moment you're watching, and which is the user weight modulated by
the slowstart ratio. This one is reported in the stats so that it's
visible why a server only takes a small amount of charge (some users
used to have very long slowstart periods and were confused without
it).

Lesson learned: "never call something effective as you might still find
a new way to post-process it down the chain later"!

We could imagine to add the effective one to the "get weight" output
as well, but then we'd need to be extremely careful not to break
possibly existing scripts that rely on dumping $3 or last field etc
so that's not as easy as it seems.

> Did I understand the expected behavior right ?

Both yes and no :-)  There was some confusion induced by the feature
having been extended over time to include a third value.

Hoping this helps,
Willy



Re: [PATCH] CLEANUP: Add missing include guard to signal.h

2021-09-01 Thread Willy Tarreau
On Wed, Sep 01, 2021 at 09:23:25PM +0200, Tim Duesterhus wrote:
> Willy,
> 
> this also affects include/import/lru.h, include/import/xxhash.h, and
> include/import/sha1.h. But I did not touch these, as they are within import/*

Yes, good point, I also prefer to really limit the amount of changes
there and only modify them if there is a confirmed problem.

Applied now, thank you!
Willy



Re: [PATCH] variables cleanup/fixup

2021-08-31 Thread Willy Tarreau
On Tue, Aug 31, 2021 at 06:37:45PM +0200, Willy Tarreau wrote:
> > As such: Your patches LGTM, thanks. Please proceed :-)
> 
> Will do, and reference the issue above and update the doc regarding ifexist,
> just mentioning that it's now ignored for legacy compatibility.

I'll finally wait for Christopher next week to confirm that we can get
rid of the equivalent in SPOE. By default it protects itself against
registration of random variable names and there's a force-set-var option
to bypass that for trusted agents. In my opinion this can go since we do
not have this problem anymore, but let's confirm before risking to break
setups.

Cheers,
Willy



Re: [PATCH] variables cleanup/fixup

2021-08-31 Thread Willy Tarreau
On Tue, Aug 31, 2021 at 04:41:16PM +0200, Tim Düsterhus wrote:
> Willy,
> 
> On 8/31/21 9:07 AM, Willy Tarreau wrote:
> > I've finally implemented the replacement of the global variables table
> 
> Okay, please refer to issue #624 in the commit:
> https://github.com/haproxy/haproxy/issues/624. I believe it should be
> resolved afterwards.

Ah yes, thank you, I forgot about this one!

> > with a hash instead. However it now obviously breaks the "ifexist"
> > argument that you added to Lua's set_var() that was designed to work
> > around the growing table problem.
> > 
> > Given that the limitation used to be made on the existence of a similarly
> > named variable anywhere in the process (and not of the variable in the
> > current list), I conclude that it was only used to preserve precious
> > resources and never to conditionally set a variable in one's context.
> > As such we could simply remove this test and silently ignore the ifexist
> > argument now. Do you agree with this ? I'd really like it if we could
> > definitely get rid of this old mess!
> 
> For the record this is my the repository I care about:
> 
> https://github.com/TimWolla/haproxy-auth-request
> 
> It includes tests based on VTest. I just ran the tests with patches applied:
> 
> > $ vtest -Dhaproxy_version=2.5.0 -q -k -t 10 -C test/*.vtc
> > #top  TEST test/no_variable_leak.vtc FAILED (0.203) exit=2
> > 1 tests failed, 0 tests skipped, 19 tests passed
> 
> Naturally the no_variable_leak.vtc test 
> (https://github.com/TimWolla/haproxy-auth-request/blob/main/test/no_variable_leak.vtc)
> fails now, as it specifically tests that my detection of the ifexist
> parameter works as expected. I can simply exclude HAProxy 2.5 from that test
> and all is well.

Yeah that's the same now in 2.5-dev with the dedicated test.

> In my case I only care about not bloating HAProxy's memory usage infinitely,
> in case the backend sends sends headers with randomly generated names (these
> are exposed as req.auth_response_header.*). The fact that variables are
> unavailable to Lua is a side-effect of this, not a feature.

That's what I remembered as well, thanks for confirming!

> It would certainly be preferable for the user if they could simply use the
> variables from Lua, without needing to reference them in the config.

... and without eating all the memory :-)

> As such: Your patches LGTM, thanks. Please proceed :-)

Will do, and reference the issue above and update the doc regarding ifexist,
just mentioning that it's now ignored for legacy compatibility.

Thanks!
Willy



[PATCH] variables cleanup/fixup

2021-08-31 Thread Willy Tarreau
Hi Tim,

I've finally implemented the replacement of the global variables table
with a hash instead. However it now obviously breaks the "ifexist"
argument that you added to Lua's set_var() that was designed to work
around the growing table problem.

Given that the limitation used to be made on the existence of a similarly
named variable anywhere in the process (and not of the variable in the
current list), I conclude that it was only used to preserve precious
resources and never to conditionally set a variable in one's context.
As such we could simply remove this test and silently ignore the ifexist
argument now. Do you agree with this ? I'd really like it if we could
definitely get rid of this old mess!

For reference I'm appending the current patch series.

Thanks!
Willy
>From 6abb2e9fd745311c091029933a86fe363d09a7fb Mon Sep 17 00:00:00 2001
From: Willy Tarreau 
Date: Tue, 31 Aug 2021 08:13:25 +0200
Subject: MINOR: vars: rename vars_init() to vars_init_head()

The vars_init() name is particularly confusing as it does not initialize
the variables code but the head of a list of variables passed in
arguments. And we'll soon need to have proper initialization code, so
let's rename it now.
---
 include/haproxy/vars.h | 2 +-
 src/haproxy.c  | 2 +-
 src/http_ana.c | 6 +++---
 src/session.c  | 2 +-
 src/stream.c   | 6 +++---
 src/tcpcheck.c | 2 +-
 src/vars.c | 4 ++--
 7 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/haproxy/vars.h b/include/haproxy/vars.h
index f809c62d5..fedc8ca15 100644
--- a/include/haproxy/vars.h
+++ b/include/haproxy/vars.h
@@ -29,7 +29,7 @@
 
 extern struct vars proc_vars;
 
-void vars_init(struct vars *vars, enum vars_scope scope);
+void vars_init_head(struct vars *vars, enum vars_scope scope);
 void var_accounting_diff(struct vars *vars, struct session *sess, struct 
stream *strm, int size);
 unsigned int var_clear(struct var *var);
 void vars_prune(struct vars *vars, struct session *sess, struct stream *strm);
diff --git a/src/haproxy.c b/src/haproxy.c
index db957256e..32a08441d 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -1529,7 +1529,7 @@ static void init(int argc, char **argv)
hlua_init();
 
/* Initialize process vars */
-   vars_init(_vars, SCOPE_PROC);
+   vars_init_head(_vars, SCOPE_PROC);
 
global.tune.options |= GTUNE_USE_SELECT;  /* select() is always 
available */
 #if defined(USE_POLL)
diff --git a/src/http_ana.c b/src/http_ana.c
index b36054088..3443f7ec4 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -2955,7 +2955,7 @@ int http_eval_after_res_rules(struct stream *s)
if (s->vars_reqres.scope != SCOPE_RES) {
if (!LIST_ISEMPTY(>vars_reqres.head))
vars_prune(>vars_reqres, s->sess, s);
-   vars_init(>vars_reqres, SCOPE_RES);
+   vars_init_head(>vars_reqres, SCOPE_RES);
}
 
ret = http_res_get_intercept_rule(s->be, >be->http_after_res_rules, 
s);
@@ -5083,8 +5083,8 @@ struct http_txn *http_create_txn(struct stream *s)
 
txn->auth.method = HTTP_AUTH_UNKNOWN;
 
-   vars_init(>vars_txn,SCOPE_TXN);
-   vars_init(>vars_reqres, SCOPE_REQ);
+   vars_init_head(>vars_txn,SCOPE_TXN);
+   vars_init_head(>vars_reqres, SCOPE_REQ);
 
return txn;
 }
diff --git a/src/session.c b/src/session.c
index 92d03eaed..7ad5cbb3e 100644
--- a/src/session.c
+++ b/src/session.c
@@ -47,7 +47,7 @@ struct session *session_new(struct proxy *fe, struct listener 
*li, enum obj_type
sess->accept_date = date; /* user-visible date for logging */
sess->tv_accept   = now;  /* corrected date for internal use */
memset(sess->stkctr, 0, sizeof(sess->stkctr));
-   vars_init(>vars, SCOPE_SESS);
+   vars_init_head(>vars, SCOPE_SESS);
sess->task = NULL;
sess->t_handshake = -1; /* handshake not done yet */
sess->t_idle = -1;
diff --git a/src/stream.c b/src/stream.c
index 132ee3abd..27062ea4b 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -451,8 +451,8 @@ struct stream *stream_new(struct session *sess, enum 
obj_type *origin, struct bu
/* Initialise all the variables contexts even if not used.
 * This permits to prune these contexts without errors.
 */
-   vars_init(>vars_txn,SCOPE_TXN);
-   vars_init(>vars_reqres, SCOPE_REQ);
+   vars_init_head(>vars_txn,SCOPE_TXN);
+   vars_init_head(>vars_reqres, SCOPE_REQ);
 
/* this part should be common with other protocols */
if (si_reset(>si[0]) < 0)
@@ -2201,7 +2201,7 @@ struct task *process_stream(struct task *t, void 
*context, unsigned int state)
if (s->vars_reqres.scope != SCOPE_RES) {
if

Re: [PATCH] BUG/???: threads: Use get_(local|gm)time instead of (local|gm)time

2021-08-29 Thread Willy Tarreau
On Sat, Aug 28, 2021 at 11:57:01PM +0200, Tim Duesterhus wrote:
> Willy,
> 
> please fill in the severity yourself.

Good catch, I didn't notice we still had those. Applied as minor as
I don't think anyone really noticed it (it would require different
arguments in different converters to be used in parallel in different
threads for the problem to be noticeable).

Thanks,
Willy



Re: [PATCH] BUG/MINOR: tools: Fix loop condition in dump_text()

2021-08-29 Thread Willy Tarreau
On Sun, Aug 29, 2021 at 12:58:22AM +0200, Tim Duesterhus wrote:
> The condition should first check whether `bsize` is reached, before
> dereferencing the offset. Even if this always works fine, due to the
> string being null-terminated, this certainly looks odd.

Applied as well, thank you!
Willy



[ANNOUNCE] haproxy-2.5-dev5

2021-08-28 Thread Willy Tarreau
: ssl: Fix compilation with OpenSSL 1.0.2

Tim Duesterhus (2):
  REGTESTS: Use `feature cmd` for 2.5+ tests
  REGTESTS: Remove REQUIRE_VERSION=1.5 from all tests

William Lallemand (21):
  MINOR: httpclient: initialize the proxy
  MINOR: httpclient: implement a simple HTTP Client API
  MINOR: httpclient/cli: implement a simple client over the CLI
  MINOR: httpclient/cli: change the User-Agent to "HAProxy"
  MINOR: server: check if srv is NULL in free_server()
  MINOR: proxy: check if p is NULL in free_proxy()
  BUG/MINOR: httpclient: fix uninitialized sl variable
  BUG/MINOR: httpclient/cli: change the appctx test in the callbacks
  BUG/MINOR: httpclient: check if hdr_num is not 0
  MINOR: httpclient: cleanup the include files
  BUG/MINOR: systemd: ExecStartPre must use -Ws
  MINOR: systemd: remove the ExecStartPre line in the unit file
  MINOR: ssl: add an openssl version string parser
  MINOR: cfgcond: implements openssl_version_atleast and 
openssl_version_before
  CLEANUP: ssl: remove useless check on p in openssl_version_parser()
  BUG/MINOR: httpclient: remove deinit of the httpclient
  MINOR: httpclient: set verify none on the https server
  MINOR: httpclient: add the server to the proxy
  BUG/MINOR: httpclient: fix Host header
  BUILD: httpclient: fix build without OpenSSL
  BUG/MINOR: proxy: don't dump servers of internal proxies

Willy Tarreau (19):
  BUG/MEDIUM: h2: match absolute-path not path-absolute for :path
  BUG/MEDIUM: cfgparse: do not allocate IDs to automatic internal proxies
  BUG/MINOR: http_client: make sure to preset the proxy's default settings
  REGTESTS: http_upgrade: fix incorrect expectation on TCP->H1->H2
  REGTESTS: abortonclose: after retries, 503 is expected, not close
  REGTESTS: server: fix agent-check syntax and expectation
  MINOR: hlua: take the global Lua lock inside a global function
  BUG/MINOR: stick-table: fix the sc-set-gpt* parser when using expressions
  CI: github-actions: remove obsolete options
  MINOR: compiler: implement an ONLY_ONCE() macro
  BUG/MINOR: lua: use strlcpy2() not strncpy() to copy sample keywords
  BUG/MINOR: time: fix idle time computation for long sleeps
  MINOR: time: add report_idle() to report process-wide idle time
  BUG/MINOR: ebtree: remove dependency on incorrect macro for bits per long
  BUILD: activity: use #ifdef not #if on USE_MEMORY_PROFILING
  BUILD/MINOR: defaults: eliminate warning on MAXHOSTNAMELEN with -Wundef
  BUILD/MINOR: ssl: avoid a build warning on LIBRESSL_VERSION with -Wundef
  IMPORT: slz: silence a build warning with -Wundef
  BUILD/MINOR: regex: avoid a build warning on USE_PCRE2 with -Wundef

devne...@gmail.com (2):
  MINOR: tools: add FreeBSD support to get_exec_path()
  MINOR: proc: setting the process to produce a core dump on FreeBSD.

---



Re: [PATCH] JA3 TLS Fingerprinting (take 2)

2021-08-26 Thread Willy Tarreau
Hi Marcin,

On Thu, Aug 26, 2021 at 06:56:20PM +0200, Marcin Deranek wrote:
> Hi Willy,
(...)
> No worries. Hopefully soon this will get merged. Attaching latest patches
> with all modification included.

Thanks for detailing all the points. I trust that you did them as you
said, in the worst case it will just lead to bug fixes, that's fine. I
quickly glanced over all of it a last time and think it's OK, so I've
pushed it.

I just wish I had pushed into a temporary branch first to see if the CI
complains on certain openssl versions, but fortunately it went fine :-)

I changed the level of the last patch from MINOR to MEDIUM as it deprecates
a config setting and will result in emitting a new warning for some users,
thus it will have a visible impact.

And that's all!

Thank you!
Willy



Re: [PATCH] REGTESTS: Remove REQUIRE_VERSION=1.5 from all tests

2021-08-25 Thread Willy Tarreau
On Wed, Aug 25, 2021 at 07:17:28PM +0200, Tim Duesterhus wrote:
> HAProxy 1.5 is EOL, thus this always matches.
> 
> 1.6 / 1.7 were already removed in:
> d8be0018fe85b5f815d59cdf1e0400274a99a9b1 (1.6)
> 1b095cac9468d0c3eeb157e9b1a2947487bd3c83 (1.7)

Good catch. Both patches applied (this one and feature cmd).

Thanks,
Willy



Re: [PATCH] prepare scripts/build-ssl.sh for OpenSSL-3.0.0beta2

2021-08-24 Thread Willy Tarreau
On Sat, Aug 21, 2021 at 04:06:59PM +0500,  ??? wrote:
> hello,
> 
> starting with 3.0.0beta2 we need to specify libdir.

Thanks Ilya, now applied!
Willy



Re: [PATCH]: MINOR: proc: making the process able to produce ore dump on FreeBSD

2021-08-24 Thread Willy Tarreau
Now applied, thanks to you both.
Willy



Re: [PATCH] spell fixes

2021-08-24 Thread Willy Tarreau
On Sun, Aug 22, 2021 at 10:22:38PM +0500,  ??? wrote:
> hello,
> 
> yet another spell fixes.

Applied, thanks Ilya!
Willy



Re: [PATCH] JA3 TLS Fingerprinting (take 2)

2021-08-24 Thread Willy Tarreau
Hi Marcin,

I'm finally back to your patch set! Overall that looks fine, but I have
some comments, mostly cosmetic.

> From b3a254b41411f22307a622250a6e95ac39fefee8 Mon Sep 17 00:00:00 2001
> From: Marcin Deranek 
> Date: Mon, 12 Jul 2021 14:16:55 +0200
> Subject: [PATCH 1/5] MEDIUM: ssl: Capture more info from Client Hello
(...)

> diff --git a/include/haproxy/ssl_sock-t.h b/include/haproxy/ssl_sock-t.h
> index 5acedcfc5..4b993894a 100644
> --- a/include/haproxy/ssl_sock-t.h
> +++ b/include/haproxy/ssl_sock-t.h
> @@ -202,8 +202,16 @@ struct ssl_sock_msg_callback {
>  /* This memory pool is used for capturing clienthello parameters. */
>  struct ssl_capture {
>   unsigned long long int xxh64;
> - unsigned char ciphersuite_len;
> - char ciphersuite[VAR_ARRAY];
> + unsigned short int protocol_version;
> + unsigned short int ciphersuite_len;
> + unsigned short int extensions_len;
> + unsigned short int ec_len;

No need to mention "int" after "short" or "long", that's implicit. I
guess you did it because of the "unsigned long long int" above but that
is not needed. Since 2.4 we have shorter type names like "ushort", "uint",
"uchar" etc that we're progressively trying to standardize the new code
to, but they haven't slipped into older code yet. This could make sense
here since the whole structure is being replaced. But that's not an
obligation :-)

> diff --git a/src/ssl_sock.c b/src/ssl_sock.c
> index ba61243ac..2965a1446 100644
> --- a/src/ssl_sock.c
> +++ b/src/ssl_sock.c
> @@ -1652,7 +1652,16 @@ static void ssl_sock_parse_clienthello(struct 
> connection *conn, int write_p, int
>   struct ssl_capture *capture;
>   unsigned char *msg;
>   unsigned char *end;
> - size_t rec_len;
> + unsigned char *extensions_end;
> + unsigned char *ec_start = NULL;
> + unsigned char *ec_formats_start = NULL;
> + unsigned char *list_end;
> + unsigned short int protocol_version;
> + unsigned short int extension_id;
> + unsigned short int ec_len = 0;

Same here for the type names.

> @@ -1747,10 +1763,106 @@ static void ssl_sock_parse_clienthello(struct 
> connection *conn, int write_p, int
>   capture->xxh64 = XXH64(msg, rec_len, 0);
>  
>   /* Capture the ciphersuite. */
> - capture->ciphersuite_len = (global_ssl.capture_cipherlist < rec_len) ?
> - global_ssl.capture_cipherlist : rec_len;
> - memcpy(capture->ciphersuite, msg, capture->ciphersuite_len);
> + capture->ciphersuite_len = MIN(global_ssl.capture_cipherlist, rec_len);
> + capture->ciphersuite_offset = 0;
> + memcpy(capture->data, msg, capture->ciphersuite_len);
> + msg += rec_len;
> + offset += capture->ciphersuite_len;
> +
> + /* Initialize other data */
> + capture->protocol_version = protocol_version;
> + capture->extensions_len = 0;
> + capture->extensions_offset = 0;
> + capture->ec_len = 0;
> + capture->ec_offset = 0;
> + capture->ec_formats_len = 0;
> + capture->ec_formats_offset = 0;

Given the number of fields to preset to zero, you should instead replace
the previous pool_alloc() call with a pool_zalloc(), it would be more
future-proof and safer against the risk of accidentally missing one
such field when more are added.

> + /* Expect 2 bytes extension id + 2 bytes extension size */
> + msg += 2 + 2;
> + if (msg + rec_len > extensions_end || msg + rec_len < msg)
> + goto store_capture;
> + if (extension_id == 0x000a) {
> + /* Elliptic Curves */

Could you put a link here to the registry where you found this definition ?
I guess it comes from an RFC or anything, but it does help when some
extensions are needed or when dealing with interoperability issues, to
figure if any other values need to be considered.

> + else if (extension_id == 0x000b) {
> + /* Elliptic Curves Point Formats */

Ditto.

> + if (ec_start) {
> + rec_len = MIN(global_ssl.capture_cipherlist - offset, ec_len);

I'm not fond of using MIN() here as you're dealing with two different
types (one signed int due to the subtract, and an unsigned short). It
actually does the right thing but it's fragile. What about this:

rec_len = ec_len;
if (offset + rec_len > global_ssl.capture_cipherlist)
rec_len = global_ssl.capture_cipherlist - offset;

> + memcpy(capture->data + offset, ec_start, rec_len);
> + capture->ec_offset = offset;
> + capture->ec_len = rec_len;
> + offset += rec_len;
> + }
> + if (ec_formats_start) {
> + rec_len = MIN(global_ssl.capture_cipherlist - offset, 
> ec_formats_len);

Same here.


> +ssl_fc_cipherlist_bin([]) : binary
> +  Returns the binary form of the client hello cipher list. Setting
> +   to 1 will exclude GREASE (RFC8701) values from the output.

I find that 

Re: BUILD: tools: get the absolute path on FreeBSD

2021-08-20 Thread Willy Tarreau
Hi David,

On Tue, Aug 17, 2021 at 12:58:29PM +0100, David CARLIER wrote:
> Hi,
> 
> same as earlier but for FreeBSD this time.

Applied after rewording a bit the commit message.
Thanks,
Willy



Re: [2.4.2] Thread .. is about to kill the process - Lua-involved

2021-08-20 Thread Willy Tarreau
On Fri, Aug 20, 2021 at 03:10:05PM +0200, Willy Tarreau wrote:
> BTW maybe we should arrange to take the Lua lock inside an externally
> visible function that could be resolved. It would more easily show up in
> case of trouble so that the issue becomes more obvious.

And it works pretty well!

-  perf top

  86.61%  haproxy   [.] lua_take_global_lock
   9.69%  haproxy   [.] luaV_execute
   2.72%  haproxy   [.] luaG_traceexec
   0.17%  [kernel]  [k] native_io_delay
   0.07%  [i2c_i801][k] 0x0cf2
   0.03%  [kernel]  [k] acpi_os_read_port
   0.02%  perf  [.] rb_next
   0.02%  libc-2.30.so  [.] __strcmp_avx2
   0.01%  perf  [.] __symbols__insert
   0.01%  perf  [.] dso__find_symbol
   0.01%  [kernel]  [k] kallsyms_expand_symbol.constprop.0
   0.01%  libc-2.30.so  [.] _int_malloc

- "show threads"

 >Thread 4 : id=0x7f561b677700 act=1 glob=0 wq=1 rq=0 tl=1 tlsz=0 rqsz=1
 stuck=1 prof=1 harmless=0 wantrdv=0
 cpu_ns: poll=43427720070 now=43835242784 diff=407522714
 curr_task=0x7f560c02e9c0 (task) calls=1 last=408413622 ns ago
   fct=0x4d3b20(process_stream) ctx=(nil)

 call trace(13):
 |   0x5a0c3b [85 c0 75 19 48 81 c4 c8]: 
ha_dump_backtrace+0x2b/0x299
 |   0x5a209f [48 83 c4 38 5b 5d 41 5c]: 
ha_thread_dump+0x22f/0x281
 |   0x5a2166 [48 8b 05 2b 2c 20 00 48]: 
debug_handler+0x66/0x10b
 | 0x7f5621ef6690 [48 c7 c0 0f 00 00 00 0f]: libpthread:+0x13690
 |   0x46341a [eb cc 0f 1f 40 00 48 b8]: 
lua_take_global_lock+0x4a/0x4c
 |   0x46987d [64 48 8b 04 25 00 00 00]: 
hlua_ctx_destroy+0xcd/0x35a
 |   0x4d77ea [49 83 bf f0 00 00 00 00]: 
process_stream+0x3cca/0x4e18

- panic:

 >Thread 8 : id=0x7f8ec57fa700 act=1 glob=0 wq=1 rq=0 tl=0 tlsz=0 rqsz=0
 stuck=1 prof=0 harmless=0 wantrdv=0
 cpu_ns: poll=611690 now=1994658802 diff=1994047112
 curr_task=0x7f8eb402e9c0 (task) calls=1 last=0
   fct=0x4d3b20(process_stream) ctx=0x7f8eb402e550
 strm=0x7f8eb402e550 src=127.0.0.1 fe=fe4 be=fe4 dst=unknown
 rqf=40d08002 rqa=30 rpf=8000 rpa=0 sif=EST,200020 sib=INI,30
 af=(nil),0 csf=0x7f8eb402e500,4000
 ab=(nil),0 csb=(nil),0
 cof=0x7f8eb802a2c0,80001300:H1(0x7f8eb4026240)/RAW((nil))/tcpv4(40)
 cob=(nil),0:NONE((nil))/NONE((nil))/NONE(0)
 Current executing Lua from a stream analyser -- 
 call trace(20):
 |   0x5a0c3b [85 c0 75 19 48 81 c4 c8]: 
ha_dump_backtrace+0x2b/0x299
 |   0x5a209f [48 83 c4 38 5b 5d 41 5c]: 
ha_thread_dump+0x22f/0x281
 |   0x5a2166 [48 8b 05 2b 2c 20 00 48]: 
debug_handler+0x66/0x10b
 | 0x7f8ece7d3690 [48 c7 c0 0f 00 00 00 0f]: libpthread:+0x13690
 |   0x46341a [eb cc 0f 1f 40 00 48 b8]: 
lua_take_global_lock+0x4a/0x4c
 |   0x466c17 [e9 fe fe ff ff 0f 1f 40]: 
hlua_ctx_init+0x147/0x1cd
 |   0x469592 [85 c0 74 3a 48 8b 44 24]: main+0x3dc92
 |   0x51dacb [85 c0 74 61 48 8b 5d 20]: 
sample_process+0x4b/0xf7
 |   0x51e55c [48 85 c0 74 3f 64 48 63]: 
sample_fetch_as_type+0x3c/0x9b
 |   0x525613 [48 89 c6 48 85 c0 0f 84]: 
sess_build_logline+0x2443/0x3cae
 |   0x4af0be [4c 63 e8 4c 03 6d 10 4c]: 
http_apply_redirect_rule+0xbfe/0xdf8
 |   0x4af523 [83 f8 01 19 c0 83 e0 03]: main+0x83c23
 |   0x4b2326 [83 f8 07 0f 87 99 00 00]: 
http_process_req_common+0xf6/0x15f6
 |   0x4d5b30 [85 c0 0f 85 9f f5 ff ff]: 
process_stream+0x2010/0x4e18

And from what I'm seeing the forceyield setting doesn't help against this
because the system is really not making any visible progress and non-lua
stuff is blocked (the CLI is unresponsive on short loops).

Thus I'm going to commit this. It doesn't show any measurable performance
change considering how long that lock is usually taken!

For reference, the Lua code here does this:

  core.register_fetches("loop", function(txn) for i=1,1 do end return 
"/" end)

And the haproxy conf is this:

  global
 stats socket /tmp/sock1 level admin
 # tune.lua.forced-yield 10
 # lua-load-per-thread loop.lua
 lua-load loop.lua

   defaults
 mode http
 timeout client 10s
 timeout server 10s
 timeout connect 10s

   frontend fe4
 bind :9003
 http-request redirect location %[lua.loop]

Willy



Re: [2.4.2] Thread .. is about to kill the process - Lua-involved

2021-08-20 Thread Willy Tarreau
Hi Robin,

sorry for the delay, we've been quite busy these last days :-/

On Mon, Aug 09, 2021 at 09:06:36PM +, Robin H. Johnson wrote:
> After months searching, at work we stumbled onto an internally usable-only
> reproduction case using a tool we wrote that made millions of requests: 
> Turning
> it up around ~6K RPS w/ lots of the headers being processed by our Lua code
> triggered the issue, running on a single-sock EPYC 7702P system.

OK!

> We also found a surprising mitigation: enabling multithreaded Lua w/
> "lua-load-per-thread" made the problem go away entirely (and gave a modest 10%
> performance boost, we are mostly limited by backend servers, not HAProxy or
> Lua).

Then I'm sorry that it was not spotted earlier, because this was a known
limitation of Lua in the pre-2.4 versions: the Lua code runs on a single
threaded stack, and by default as there is a single stack, when you have
too many threads, some are waiting ages to try to get access to the CPU,
to the point of possibly spinning more than 2 seconds there (which is an
eternity for a CPU).

BTW maybe we should arrange to take the Lua lock inside an externally
visible function that could be resolved. It would more easily show up in
case of trouble so that the issue becomes more obvious.

And that's exactly why lua-load-per-thread was introduced. It creates one
independent stack per thread so that there is no more locking. I suspect
that limiting the number of Lua instructions executed in a call could
have reduced the probability to keep Lua on a thread for too long, but
that equates to playing Russian roulette, and if you could switch to
lua-load-per-thread it's way better. I had started some work a few months
ago to implement latency-bounded locks that avoid the trouble of NUMA
systems (and even any system with a non-totally-uniform L3 cache) where
some groups of threads can hinder other groups for a while. When I'm done
with this I guess the Lua lock will be a good candidate for it!

> The Lua script was described in the previous script, and only does complex
> string parsing, used for variables, and driving some applets. It doesn't do 
> any
> blocking operations, sockets, files or rely on globals. It got a few cleanups
> for multi-threaded usage (forcing more variables to be explicitly local), but
> has no other significant changes relevant to this discussion (it had some
> business logic changes to string handling used to compute stick table keys, 
> but
> not really functionality changes).

I'm really glad that you managed to make it thread-safe with limited
changes, as that was our hope when we designed it like this with Thierry!

> The full errors are attached along with decoded core dump, with some details
> redacted per $work security team requirements.
> Repeated the error twice and both attempts are attached, 4 files in total.
> I'll repeat the short form here for interest from just one of the occurrences:

Many thanks for sharing all this, it will certainly help others.
(...)

Thanks!
Willy



Re: double // after domain causes ERR_HTTP2_PROTOCOL_ERROR after upgrade to 2.4.3

2021-08-20 Thread Willy Tarreau
Hi all,

On Fri, Aug 20, 2021 at 02:52:32PM +0300, Jarno Huuskonen wrote:
> Hi,
> 
> On 8/20/21 2:20 PM, Lukas Tribus wrote:
> > On Fri, 20 Aug 2021 at 13:08,  ???  wrote:
> > > 
> > > double slashes behaviour is changed in BUG/MEDIUM:
> > > h2: match absolute-path not path-absolute for :path · 
> > > haproxy/haproxy@46b7dff (github.com)
> > 
> > Actually, I think the patch you are referring to would *fix* this
> > particular issue, as it was committed AFTER the last releases:
> > 
> > https://github.com/haproxy/haproxy/commit/46b7dff8f08cb6c5c3004d8874d6c5bc689a4c51
> > 
> > It was this fix that probably caused the issue:
> > https://github.com/haproxy/haproxy/commit/4b8852c70d8c4b7e225e24eb58258a15eb54c26e
> > 
> > 
> > Using the latest git, applying the patch manually or running a
> > 20210820 snapshot would fix this.
> > 
> 
> Yes, 2.4.3+"BUG/MEDIUM: h2: match absolute-path not path-absolute for :path"
> and https://www.example.com// appears to work again.

Yes that's definitely it. The bug this time was in the HTTP/2 spec itself,
so when we're too lax regarding the spec it opens vulnerabilities and when
we respect it, it causes trouble. After some deep analysis I brought the
topic on the HTTP working group and the consensus was that indeed the bug
is in the spec, which was fixed, and I modified haproxy accordingly after
that.

Given that till now I got only one report from a hosting platform, I
considered that it was probably not common at all and that upgrades were
not likely needed right now and could wait a bit for more important fixes.

However, if it's really too much hassle for some to apply just this fix, we
could emit a new set of versions (2.4, 2.3, 2.2). Please just keep in mind
that it's always some extra work especially for the distro maintainers.
Contrary to what I wrote in the commit message, 2.0 is OK.

Please advise!

Thanks,
Willy



Re: [ANNOUNCE] HTTP/2 vulnerabilities from 2.0 to 2.5-dev

2021-08-18 Thread Willy Tarreau
Hi James,

On Wed, Aug 18, 2021 at 04:53:09PM -0700, James Brown wrote:
> Are there CVE numbers coming for these vulnerabilities?

Yes, for what it's worth, Robert Frohl from SuSE got 3 assigned to this:

  - CVE-2021-39240: -> Domain parts in ":scheme" and ":path"   
  - CVE-2021-39241: -> Spaces in the ":method" field
  - CVE-2021-39242: -> Mismatch between ":authority" and "Host"

Regards,
Willy



Re: [ANNOUNCE] HTTP/2 vulnerabilities from 2.0 to 2.5-dev

2021-08-17 Thread Willy Tarreau
On Tue, Aug 17, 2021 at 06:57:28PM +0200, Tim Düsterhus wrote:
> Hi Willy, Everyone,
> 
> On 8/17/21 5:13 PM, Willy Tarreau wrote:
> > 2) Domain parts in ":scheme" and ":path"
> > 
> > [...] As such HTTP/1 servers are safe and only HTTP/2 servers are exposed.
> 
> I'd like to clarify that the above statement is not true. The issue also
> affects H2->HAProxy->H1 connections. It allows to forward a different 'host'
> header than the one HAProxy sees to the backend.

So to be more precise, based on the output of your test, both will see
the same Host header field, however the server will receive a different
authority than the Host header field. While this is not a valid request
we know that some servers are more willing to accept that than the
poorly formatted requests. However those which do most often only use
the Host (which is why they don't check the authority).

I'm not saying this to try to dismiss the problem, it's in order to
help admins analyze strange logs that they may encounter before
upgrading or deploying workarounds.

> The 'http-request set-uri %[url]' workaround mentioned at the bottom of
> Willy's email also fixes the issue for HTTP/1 backends.
> 
> In any case I recommend to upgrade as soon as possible. That way you don't
> have to think whether your setup requires a workaround or not.

I agree on both points!

Thanks for reporting that one.
Willy



Re: [ANNOUNCE] HTTP/2 vulnerabilities from 2.0 to 2.5-dev

2021-08-17 Thread Willy Tarreau
On Tue, Aug 17, 2021 at 05:56:15PM +0200, Tim Düsterhus wrote:
> Vincent,
> 
> On 8/17/21 5:49 PM, Vincent Bernat wrote:
> > For users of haproxy.debian.net or Launchpad PPA, the vulnerabilities
> > are fixed by patching the previous versions. Launchpad PPA builders are
> > still running but it should be available in the next hour. I will upload
> > the new versions later this week.
> 
> As a consumer of your packages: A big thank you for your work.
> 
> After I noticed Willy's announcement I checked whether any new 2.4.x
> packages were available and indeed they were already there, saving me the
> work from compiling a custom version / adjusting the config.

I second that! And I'm well aware of the amount of work Vincent had to
do behind the curtains to deal with this since the end of last week to
provide us with all this just in time.

Thanks a lot Vincent!
Willy



[ANNOUNCE] haproxy-2.0.24

2021-08-17 Thread Willy Tarreau
Hi,

HAProxy 2.0.24 was released on 2021/08/17. It added 18 new commits
after version 2.0.23.

This version contains the fixes for the H2 vulnerabilities reported by
Tim that were described in previous message, and which allows to abuse
the H2 ":method" pseudo-header to forge some malformed HTTP/1 messages
that some vulnerable servers might possibly accept to parse (though
we're not aware of any among the usual mainstream ones).

All users of 2.0 which skipped previous updates *MUST* program an update
to this one. In the mean time, the previous message about the issue
suggests several possible workarounds.

Aside these, the following issues were addressed in this version:

  - there was yet another case where a partial H2 frame could leave an H2
connection in a stuck state. This time it's okay (famous last words).

  - checking a config with -W could cause an attempt to re-execute the
process and crash. It does not bring anything to use -W during a
config check but it usually remains from hard-coded command line
arguments in scripts. And actually that was also missing from the
systemd unit file and was added there.

  - SPOE was fixed regarding the connection close strategy in multi-threading
so that there are always available connections for each active thread.

  - a previous fix for an issue in tcp-checks where a dead connection could
occasionally be dereferenced was incomplete in the 2.0 backport since
the newer code had been refactored. It was completely addressed here.

  - muxes were not respecting "dontlognull" when dealing with H2 prefaces
followed by a close, but this happens often with ALPN when clients
tentatively set up multiple connections for the case where H2 will not
be available. This was fixed.

  - support for "option disable-h2-upgrade" was backported from 2.2 to
help users forcefully disable H1->H2 upgrades when desired.

  - a run-time check on integer wrapping was added upon startup to make
sure haproxy is not accidentally built with incorrect CFLAGS which
cause incorrect/insecure code to be emitted. If the error happens on
startup, haproxy will indicate what to do (i.e. rebuild without
dropping critical options from CFLAGS). There is no soft-fail possible
here as this can only be a runtime check and once the executable code
is damaged there's nothing you can do to make it run reliably again.
Nobody will face this unless they were using a bogus binary without
knowing it.

  - the last_change field of a server was not properly updated when the
server got out of maintenance, resulting in wrong values in the stats,
and accelerated slowstarts.

  - plus a handful of very minor ones

And that's about all. A few lower importance fixes were left pending for
a future version to make sure the upgrade to this one is totally safe.

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 :
Amaury Denoyelle (1):
  REGTESTS: add a test to prevent h2 desync attacks

Christopher Faulet (9):
  BUG/MEDIUM: tcp-check: Do not dereference inexisting connection
  BUG/MINOR: mux-h2: Obey dontlognull option during the preface
  BUG/MEDIUM: mux-h2: Handle remaining read0 cases on partial frames
  MINOR: spoe: Add a pointer on the filter config in the spoe_agent 
structure
  BUG/MEDIUM: spoe: Create a SPOE applet if necessary when the last one is 
released
  BUG/MEDIUM: spoe: Fix policy to close applets when SPOE connections are 
queued
  DOC: Improve the lua documentation
  DOC: config: Fix 'http-response send-spoe-group' documentation
  MINOR: mux-h1/proxy: Add a proxy option to disable clear h2 upgrade

Jonathon Lacher (1):
  DOC/MINOR: fix typo in management document

Remi Tricot-Le Breton (1):
  BUG/MINOR: connection: Add missing error labels to conn_err_code_str

William Lallemand (1):
  BUG/MINOR: systemd: must check the configuration using -Ws

Willy Tarreau (5):
  BUILD: add detection of missing important CFLAGS
  BUG/MEDIUM: mworker: do not register an exit handler if exit is expected
  BUG/MINOR: mworker: do not export HAPROXY_MWORKER_REEXEC across programs
  BUG/MINOR: server: update last_change on maint->ready transitions too
  BUG/MAJOR: h2: enforce stricter syntax checks on the :method pseudo-header

---



[ANNOUNCE] haproxy-2.2.16

2021-08-17 Thread Willy Tarreau
IUM: spoe: Fix policy to close applets when SPOE connections are 
queued

Jonathon Lacher (1):
  DOC/MINOR: fix typo in management document

Remi Tricot-Le Breton (1):
  BUG/MINOR: connection: Add missing error labels to conn_err_code_str

William Lallemand (2):
  BUG/MINOR: systemd: must check the configuration using -Ws
  BUG/MINOR: buffer: fix buffer_dump() formatting

Willy Tarreau (14):
  BUILD: add detection of missing important CFLAGS
  BUG/MEDIUM: mworker: do not register an exit handler if exit is expected
  BUG/MINOR: mworker: do not export HAPROXY_MWORKER_REEXEC across programs
  BUG/MINOR: check: fix the condition to validate a port-less server
  BUG/MEDIUM: pollers: clear the sleeping bit after waking up, not before
  BUG/MINOR: select: fix excess number of dead/skip reported
  BUG/MINOR: poll: fix abnormally high skip_fd counter
  BUG/MINOR: pollers: always program an update for migrated FDs
  BUG/MINOR: server: update last_change on maint->ready transitions too
  MINOR: http: add a new function http_validate_scheme() to validate a 
scheme
  BUG/MAJOR: h2: verify early that non-http/https schemes match the valid 
syntax
  BUG/MAJOR: h2: verify that :path starts with a '/' before concatenating it
  BUG/MAJOR: h2: enforce stricter syntax checks on the :method pseudo-header
  BUG/MEDIUM: h2: give :authority precedence over Host

---



[ANNOUNCE] haproxy-2.3.13

2021-08-17 Thread Willy Tarreau
lua documentation
  DOC: config: Fix 'http-response send-spoe-group' documentation
  BUG/MEDIUM: spoe: Fix policy to close applets when SPOE connections are 
queued

Emeric Brun (1):
  BUG/MEDIUM: cfgcheck: verify existing log-forward listeners during config 
check

Jonathon Lacher (1):
  DOC/MINOR: fix typo in management document

Remi Tricot-Le Breton (1):
  BUG/MINOR: connection: Add missing error labels to conn_err_code_str

William Lallemand (2):
  BUG/MINOR: systemd: must check the configuration using -Ws
  BUG/MINOR: buffer: fix buffer_dump() formatting

Willy Tarreau (15):
  BUILD: add detection of missing important CFLAGS
  BUILD: lua: silence a build warning with TCC
  BUG/MEDIUM: mworker: do not register an exit handler if exit is expected
  BUG/MINOR: mworker: do not export HAPROXY_MWORKER_REEXEC across programs
  BUG/MINOR: check: fix the condition to validate a port-less server
  BUG/MEDIUM: pollers: clear the sleeping bit after waking up, not before
  BUG/MINOR: select: fix excess number of dead/skip reported
  BUG/MINOR: poll: fix abnormally high skip_fd counter
  BUG/MINOR: pollers: always program an update for migrated FDs
  BUG/MINOR: server: update last_change on maint->ready transitions too
  MINOR: http: add a new function http_validate_scheme() to validate a 
scheme
  BUG/MAJOR: h2: verify early that non-http/https schemes match the valid 
syntax
  BUG/MAJOR: h2: verify that :path starts with a '/' before concatenating it
  BUG/MAJOR: h2: enforce stricter syntax checks on the :method pseudo-header
  BUG/MEDIUM: h2: give :authority precedence over Host

---



[ANNOUNCE] haproxy-2.4.3

2021-08-17 Thread Willy Tarreau
OG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/

Willy
---
Complete changelog :
Amaury Denoyelle (6):
  BUILD: http_htx: fix ci compilation error with isdigit for Windows
  MINOR: mux_h2: define config to disable h2 websocket support
  BUG/MEDIUM: ssl_sample: fix segfault for srv samples on invalid request
  BUG/MINOR: server: remove srv from px list on CLI 'add server' error
  MINOR: server: unmark deprecated on enable health/agent cli
  REGTESTS: add a test to prevent h2 desync attacks

Christopher Faulet (12):
  BUG/MINOR: stats: Add missing agent stats on servers
  BUG/MINOR: resolvers: Use a null-terminated string to lookup in servers 
tree
  BUG/MINOR: mux-h1: Obey dontlognull option for empty requests
  BUG/MINOR: mux-h2: Obey dontlognull option during the preface
  BUG/MINOR: mux-h1: Be sure to swap H1C to splice mode when rcv_pipe() is 
called
  BUG/MEDIUM: mux-h2: Handle remaining read0 cases on partial frames
  MINOR: spoe: Add a pointer on the filter config in the spoe_agent 
structure
  BUG/MEDIUM: spoe: Create a SPOE applet if necessary when the last one is 
released
  BUG/MINOR: tcpcheck: Properly detect pending HTTP data in output buffer
  DOC: Improve the lua documentation
  DOC: config: Fix 'http-response send-spoe-group' documentation
  BUG/MEDIUM: spoe: Fix policy to close applets when SPOE connections are 
queued

David CARLIER (1):
  BUILD/MINOR: memprof fix macOs build.

Emeric Brun (1):
  BUG/MEDIUM: cfgcheck: verify existing log-forward listeners during config 
check

Ilya Shipitsin (1):
  CLEANUP: assorted typo fixes in the code and comments

Jonathon Lacher (1):
  DOC/MINOR: fix typo in management document

Miroslav Zagorac (1):
  BUILD: opentracing: fixed build when using pkg-config utility

Remi Tricot-Le Breton (2):
  BUG/MINOR: ssl: Default-server configuration ignored by server
  BUG/MINOR: connection: Add missing error labels to conn_err_code_str

William Lallemand (2):
  BUG/MINOR: systemd: must check the configuration using -Ws
  BUG/MINOR: buffer: fix buffer_dump() formatting

Willy Tarreau (19):
  BUILD: add detection of missing important CFLAGS
  BUILD: lua: silence a build warning with TCC
  BUG/MEDIUM: mworker: do not register an exit handler if exit is expected
  BUG/MINOR: mworker: do not export HAPROXY_MWORKER_REEXEC across programs
  BUG/MINOR: check: fix the condition to validate a port-less server
  BUG/MEDIUM: connection: close a rare race between idle conn close and 
takeover
  BUG/MEDIUM: pollers: clear the sleeping bit after waking up, not before
  BUG/MINOR: select: fix excess number of dead/skip reported
  BUG/MINOR: poll: fix abnormally high skip_fd counter
  BUG/MINOR: pollers: always program an update for migrated FDs
  BUG/MINOR: fd: protect fd state harder against a concurrent takeover
  DOC: internals: document the FD takeover process
  BUG/MINOR: server: update last_change on maint->ready transitions too
  ADMIN: dyncookie: implement a simple dynamic cookie calculator
  MINOR: http: add a new function http_validate_scheme() to validate a 
scheme
  BUG/MAJOR: h2: verify early that non-http/https schemes match the valid 
syntax
  BUG/MAJOR: h2: verify that :path starts with a '/' before concatenating it
  BUG/MAJOR: h2: enforce stricter syntax checks on the :method pseudo-header
  BUG/MEDIUM: h2: give :authority precedence over Host

---



[ANNOUNCE] haproxy-2.5-dev4

2021-08-17 Thread Willy Tarreau
ynamic server with checks test
  BUG/MINOR: check: test if server is not null in purge
  MINOR: global: define MODE_STOPPING
  BUG/MINOR: server: do not use refcount in free_server in stopping mode
  BUG/MINOR: check: do not reset check flags on purge
  BUG/MINOR: check: fix leak on add dynamic server with agent-check error
  BUG/MEDIUM: check: fix leak on agent-check purge
  BUG/MEDIUM: server: support both check/agent-check on a dynamic instance
  REGTESTS: add a test to prevent h2 desync attacks

Christopher Faulet (27):
  MINOR: spoe: Add a pointer on the filter config in the spoe_agent 
structure
  BUG/MEDIUM: spoe: Create a SPOE applet if necessary when the last one is 
released
  BUG/MEDIUM: spoe: Fix policy to close applets when SPOE connections are 
queued
  BUG/MINOR: tcpcheck: Properly detect pending HTTP data in output buffer
  BUG/MINOR: stream: Don't release a stream if FLT_END is still registered
  MINOR: lua: Add a flag on lua context to know the yield capability at run 
time
  BUG/MINOR: lua: Yield in channel functions only if lua context can yield
  BUG/MINOR: lua: Don't yield in channel.append() and channel.set()
  MINOR: filters/lua: Release filters before the lua context
  MINOR: lua: Add a function to get a reference on a table in the stack
  MEDIUM: lua: Process buffer data using an offset and a length
  MEDIUM: lua: Improve/revisit the lua api to manipulate channels
  DOC: Improve the lua documentation
  MEDIUM: filters/lua: Add support for dummy filters written in lua
  MINOR: lua: Add a function to get a filter attached to a channel class
  MINOR: lua: Add flags on the lua TXN to know the execution context
  MEDIUM: filters/lua: Be prepared to filter TCP payloads
  MEDIUM: filters/lua: Support declaration of some filter callback 
functions in lua
  MEDIUM: filters/lua: Add HTTPMessage class to help HTTP filtering
  MINOR: filters/lua: Add request and response HTTP messages in the lua TXN
  MINOR: filters/lua: Support the HTTP filtering from filters written in lua
  DOC: config: Fix 'http-response send-spoe-group' documentation
  BUG/MINOR: lua: Properly check negative offset in Channel/HttpMessage 
functions
  BUG/MINOR: lua: Properly catch alloc errors when parsing lua filter 
directives
  BUG/MINOR: filters: Always set FLT_END analyser when CF_FLT_ANALYZE flag 
is set
  BUG/MINOR: lua/filters: Return right code when txn:done() is called
  DOC: lua-api: Add documentation about lua filters

David Carlier (1):
  BUILD: tools: get the absolute path of the current binary on NetBSD.

Emeric Brun (1):
  BUG/MEDIUM: cfgcheck: verify existing log-forward listeners during config 
check

Ilya Shipitsin (3):
  CI: travis-ci: temporarily disable arm64 builds
  CLEANUP: assorted typo fixes in the code and comments
  CI: github actions: relax OpenSSL-3.0.0 version comparision

Jonathon Lacher (1):
  DOC/MINOR: fix typo in management document

Kunal Gangakhedkar (1):
  DOC: Minor typo fix - 'question mark' -> 'exclamation mark'

Tim Duesterhus (1):
  CI: Remove obsolete USE_SLZ=1 CI job

William Lallemand (8):
  MINOR: log: rename 'dontloglegacyconnerr' to 'log-error-via-logformat'
  MINOR: doc: rename conn_status in `option httsplog`
  MINOR: proxy: disabled takes a stopping and a disabled state
  MINOR: stats: shows proxy in a stopped state
  BUG/MINOR: buffer: fix buffer_dump() formatting
  MINOR: channel: remove an htx block from a channel
  MINOR: cli: delare the CLI frontend as an internal proxy
  MINOR: proxy: disable warnings for internal proxies

Willy Tarreau (15):
  CLEANUP: thread: fix fantaisist indentation of thread_harmless_till_end()
  MINOR: threads: make thread_release() not wait for other ones to complete
  MEDIUM: threads: add a stronger thread_isolate_full() call
  MEDIUM: servers: make the server deletion code run under full thread 
isolation
  MINOR: activity/fd: remove the dead_fd counter
  MAJOR: fd: get rid of the DWCAS when setting the running_mask
  CLEANUP: fd: remove the now unused fd_set_running()
  CLEANUP: fd: remove the now unneeded fd_mig_lock
  BUG/MINOR: server: update last_change on maint->ready transitions too
  ADMIN: dyncookie: implement a simple dynamic cookie calculator
  MINOR: http: add a new function http_validate_scheme() to validate a 
scheme
  BUG/MAJOR: h2: verify early that non-http/https schemes match the valid 
syntax
  BUG/MAJOR: h2: verify that :path starts with a '/' before concatenating it
  BUG/MAJOR: h2: enforce stricter syntax checks on the :method pseudo-header
  BUG/MEDIUM: h2: give :authority precedence over Host

---



[ANNOUNCE] HTTP/2 vulnerabilities from 2.0 to 2.5-dev

2021-08-17 Thread Willy Tarreau
Hi everyone,

HAProxy is affected by 4 vulnerabilities in its HTTP/2 implementation in
recent versions (starting with 2.0). Three of them are considered as having
a moderate impact as they only affect the interpretation of the authority
(Host header field) in H2->H2 communications in versions 2.2 and above.
One only affects a risk of misinterpretation from lenient HTTP/1 backend
servers, and affects version 2.0 and above, though at the time of writing
we're not aware of any such vulnerable server among the mainstream ones
that are commonly found behind HAProxy (Apache, NGINX, Varnish, etc).

Background: on Aug 05 a research article was published about flaws
affecting some HTTP/2 to HTTP/1 proxies or gateways:

 https://portswigger.net/research/http2

A first analysis of the article compared to some selected pieces of code
concluded that haproxy was safe. This was actually wrong as only older
versions are safe (2.0 in "legacy" mode and 1.8). Tim Düsterhus conducted
some additional tests and found some problems, which after digging for a
few days, revealed to be significantly more embarrassing. In practice,
version 2.0 in the default "HTX" mode and all later versions are affected
to some degrees.


1) Spaces in the ":method" field

The first problem is based on the ":method" field. By passing a space in
the method, it is possible to build an invalid HTTP/1 request on the
backend side, which some lenient servers might possibly interpret as valid,
resulting in a different request between the one seen by haproxy and by the
server. This might be abused to circumvent some switching rules for example,
and get a request to be routed to a wrong server. Example:

   H2 request
 :method: "GET /admin? HTTP/1.1"
 :path:   "/static/images"

HAProxy would route all "/static" requests to the static server farm,
but once the request is reassembled it would become this:

   GET /admin? HTTP/1.1 /static/images HTTP/1.1

This is not valid but if a server fails to properly validate this input,
it might be fooled into thinking this is a request for /admin.

Please note that HTTP/2 backend servers are not affect as the request is
sent as a new ":method" field there. Additionally, dangerous characters
like CR, LF or NUL are always blocked on input so is is not possible to
perform a request smuggling attack, and the risks are limited to HTTP/1
servers which fail to properly parse the request line (note that all
major server implementations are safe against this).

A workaround for this issue for those having to rely on possibly unsafe
servers is to reject invalid characters in the method by placing such a
filtering rule on the request path either in the frontend or the backend:

   http-request reject if { method -m reg [^A-Z0-9] }

A second workaround that may only be used on version 2.0 consists in
disabling the HTX internal representation in the affected backends and
the frontends that route to them:

   no option http-use-htx

This will have for effect to transform the HTTP/2 requests to HTTP/1 that
will then be submitted to the internal HTTP/1 parser which will reject
the poorly formatted request. This older representation called "legacy"
is not available any more in version 2.1 and above, and is not compatible
with HTTP/2 nor FastCGI backend servers.

This issue affects all versions from 2.0 and above, in HTX mode, with
HTTP/1 on the server side.


2) Domain parts in ":scheme" and ":path"

The ":scheme" HTTP/2 header field contains the scheme that prefixes the
request URI, typically "http" or "https". ":path" contains the part that
is local to the target host, and that usually starts with the "/". By
appending a part of a request to ":scheme" or by prepending a part of a
domain name to ":path", it is possible to make haproxy and a backend
server see a different authority or URL prefix. Please note that this
only affects HTTP/2 servers on versions 2.2 and above. These versions
are indeed capable of passing an absolute URI from end to end, while
earlier versions were limited to origin URIs. In addition, HTTP/2
requests are always forwarded in origin form to HTTP/1 backend servers
(i.e. they do not have a scheme nor authority parts). As such HTTP/1
servers are safe and only HTTP/2 servers are exposed.

What happens is that on the entry point, the :scheme, :authority and :path
fields are concatenated to rebuild a full URI that is then passed along the
chain, but the Host header is set from :authority before this concatenation
is performed. As such, the Host header field used internally may not always
match the authority part of the recomposed URI. Examples:

   H2 request
 :method:   "GET"
 :scheme:   "http://localhost/?orig=;
 :authority "example.org"
 :path: "/"

 or:

   H2 request
 :method:   "GET"
 :scheme:   "http"
 :authority "example.org"
 :path: ".local/"

An internal Host header will be build with "example.org" then the complete
URI will become 

Re: Clarification about http-reuse

2021-08-17 Thread Willy Tarreau
Hi Alex,

On Tue, Aug 17, 2021 at 02:19:38PM +0200, Aleksandar Lazic wrote:
> ```
> 3424 if ((curproxy->mode != PR_MODE_HTTP) && 
> (curproxy->options & PR_O_REUSE_MASK) != PR_O_REUSE_NEVR)
> 3425 curproxy->options &= ~PR_O_REUSE_MASK;
> ```
> 
> Does this mean that even when no "http-reuse ..." is set will the "http-reuse 
> safe" set on the proxy?

Yes, that's since 2.0. Reuse in "safe" mode is enabled by default.
You can forcefully disable it using "http-reuse never" if you want
(e.g. for debugging or if you suspect a bug in the server). But
"safe" is as safe as regular keep-alive.

Hoping this helps,
Willy



Re: [PR] DOC/MINOR: fix typo in management document

2021-08-17 Thread Willy Tarreau
On Wed, Aug 04, 2021 at 09:17:21AM +0200, PR Bot wrote:
> Dear list!
> 
> Author: Jonathon Lacher <6679714+jonathonlac...@users.noreply.github.com>
> Number of patches: 1
> 
> This is an automated relay of the Github pull request:
>DOC/MINOR: fix typo in management document
> 
> Patch title(s): 
>DOC/MINOR: fix typo in management document

Looks good, now applied. Sorry for the delay.

Willy



Re: [PATCH] DOC: Minor typo fix - 'question mark' -> 'exclamation mark'

2021-08-17 Thread Willy Tarreau
looks good, now applied, thank you!
Willy



Re: [External] Re: [PATCH] JA3 TLS Fingerprinting (take 2)

2021-08-17 Thread Willy Tarreau
Hi Marcin,

On Mon, Aug 16, 2021 at 01:55:02PM +0200, Marcin Deranek wrote:
> Hi,
> 
> Do you have any update on merging this?

Sorry, I think we've missed it :-(  Worse, I was wondering if you
managed to make any progress on it :-/

I'm currently working on preparing a set of stable branches, I'll have
a look after that.

Thanks for the ping!
Willy



Re: BUILD: tools: gets the absolute path on NetBSD

2021-08-17 Thread Willy Tarreau
Hi David,

On Tue, Aug 17, 2021 at 08:49:29AM +0100, David CARLIER wrote:
> Hi,
> 
> here a little patch proposal.

Applied, thank you!
Willy



Re: [PATCH] CI: relax OpenSSL version comparision

2021-08-16 Thread Willy Tarreau
On Sun, Aug 15, 2021 at 12:57:50PM +0500,  ??? wrote:
> we do not need strict comparison here, 3.0.0 is enough.
> 

Applied, thanks Ilya!
willy



Re: [PATCH] assorted spelling fixes

2021-08-16 Thread Willy Tarreau
On Sat, Aug 14, 2021 at 10:27:54AM +0500,  ??? wrote:
> Gentle ping

Sorry Ilya, I missed this one being busy dealing with bugs. Now
merged, thank you!
Willy



Re: [PATCH] CI: Remove obsolete USE_SLZ=1 CI job

2021-08-16 Thread Willy Tarreau
On Sat, Aug 14, 2021 at 02:36:55PM +0200, Tim Duesterhus wrote:
> Using SLZ is a default, thus this build is equivalent to the "no features"
> build.

Applied, thank you Tim!
Willy



Re: [WARNING] (1) : We generated two equal cookies for two different servers.

2021-08-11 Thread Willy Tarreau
On Wed, Aug 11, 2021 at 01:13:25PM +0200, Aleksandar Lazic wrote:
> > > But from my point of view and for server-template and dynamic-cookie-key 
> > > make
> > > this message no sense or am I wrong?
> > 
> > The problem is that when using dynamic cookies, the dynamic-cookie-key,
> > the server's IP, and its port are hashed together to generate a fixed
> > cookie value that will be stable across a cluster of haproxy LBs, but
> > hashes are never without collisions despite being 64-bit, and here you
> > apparently faced one. Given how unlikely it is, I suspect that the issue
> > in fact is that you might have multiple servers on the same address.
> > Maybe just during some DNS transitions. If that's the case, maybe we
> > should improve the collision check to only report it if it happens for
> > servers with different addresses.
> 
> Well not the same IP but quite similar.

"quite similar" isn't a valid reason for having a collision, the algo
uses XXH64() which features a pretty good distribution. I'm pretty sure
that during the life of your farm, maybe due to some DNS settings or
something like that, you end up with two servers having the same IP/port.
Well, at least they result in the right requests being routed to the right
place.

I've written a small tool to calculate the cookie values based on the
key, ip and port and committed it into admin/dyncookie/dyncookie. I'm
attaching the patch if you want to build it separately.

> Your explanation can be the reason for the warning.
> 
> ```
> dig cloud-service.namespace.svc.cluster.local
(...)

As you can see from your list, there's no collision with
this secret nor port:

  $ for i in 10.128.2.111 10.128.2.112 10.128.2.113 10.128.2.114 10.128.2.115 
10.129.9.83 10.129.9.84 10.129.9.85 10.129.9.86 10.129.9.87 10.131.4.233 
10.131.4.234 10.131.4.235 10.131.4.236 10.131.4.237; do 
./admin/dyncookie/dyncookie testphrase $i 29099; done | sort
  17419e9eba92b5af
  4945c2f00f65ba86
  5624d35859a00988
  6eec16ef4c233b5b
  843cc9b2cda23da5
  86aefdb6fe1b2e1a
  8b7cb2f615544695
  96282b2f2068ccc2
  b6de32af03e19900
  bd5a03458300e5df
  bf952365b0d16bf3
  c4daf5a0248612a6
  ca435943549734ef
  cbca5601c400f7e1
  e44d35f412164db8

I also think the warning message should be improved to report the two
offending IP/ports when found!

Willy



Re: [WARNING] (1) : We generated two equal cookies for two different servers.

2021-08-11 Thread Willy Tarreau
Hi Aleks,

On Mon, Aug 09, 2021 at 06:40:29PM +0200, Aleksandar Lazic wrote:
> Hi.
> 
> We use the HAProxy 2.4 image which have now HAProxy 2.4.2.
> https://hub.docker.com/layers/haproxy/library/haproxy/2.4/images/sha256-d5e2a5261d6367c31c8ce9b2e692fe67237bdc29f37f2e153d346e8b0dc7c13b?context=explore
> 
> I get this message for dynamic cookies.
> 
> ```
> [WARNING]  (1) : We generated two equal cookies for two different servers.
> Please change the secret key for 'my-haproxy'.
> ```
> 
> But from my point of view and for server-template and dynamic-cookie-key make
> this message no sense or am I wrong?

The problem is that when using dynamic cookies, the dynamic-cookie-key,
the server's IP, and its port are hashed together to generate a fixed
cookie value that will be stable across a cluster of haproxy LBs, but
hashes are never without collisions despite being 64-bit, and here you
apparently faced one. Given how unlikely it is, I suspect that the issue
in fact is that you might have multiple servers on the same address.
Maybe just during some DNS transitions. If that's the case, maybe we
should improve the collision check to only report it if it happens for
servers with different addresses.

Willy



Re: MaxMind config with HAProxy

2021-08-09 Thread Willy Tarreau
On Mon, Aug 09, 2021 at 08:19:40PM +0530, Amol Arote wrote:
> Dear Willy,
> 
> We are referring below reference links for configuration.where as per links
> we need to install hapee-2.3r1-lb-maxmind module.
> Need to know from where we can get this hapee-2.3r1-lb-maxmind module.
> 
> https://www.haproxy.com/documentation/hapee/latest/load-balancing/geolocation/maxmind/
> https://blog.maxmind.com/2020/03/02/using-maxmind-geoip2-databases-with-haproxy-enterprise/#more-588

Well, then you're referring to the Enterprise edition, which has nothing
to do with this mailing list which is about the community version. Please
contact your support then, as there's nothing the people on the list can
do for you in this case.

Regards,
Willy



Re: MaxMind config with HAProxy

2021-08-09 Thread Willy Tarreau
Hello,

On Mon, Aug 09, 2021 at 07:54:09PM +0530, Amol Arote wrote:
> Dear Team,
> 
> We want to configure MaxMind GeoIP2 Country DB with HAProxy.
> Please help with installation steps or help with which module of haproxy we
> need to configure for the same. PFB our server details.
> 
> HAProxy version 2.4.2-553dee3
> Apache-2.4.48
> CentOS Linux release 7.6

You provided no information regarding the steps you already tried and failed,
I doubt anybody can help you.

Regards,
Willy



Re: [PATCH] CI: travis-ci: disable arm64 builds

2021-08-09 Thread Willy Tarreau
Hi Martin,

On Mon, Aug 09, 2021 at 11:04:34AM +0300, Martin Grigorov wrote:
> TravisCI just announced some improvements related to 'arch: arm64' (using
> Equnix Metal machines) - https://blog.travis-ci.com/2021-08-06-oss-equinix.

Thanks for the info!

> But I also had some similar problems with them recently and replaced the
> config with 'arch: arm64-graviton2; group: edge; virt: vm;', i.e. AWS
> Graviton2 machines. In my experience they behave more stable!

Yeah, these machines are really fantastic. The real problem anyway is
likely that nowadays everyone is interested in testing on arm and very
few have one available, let alone even a cross-compiler, so I suspect
that these days a lot of people enable arm builds in such CI environments
because it's the only way they have to make sure their code builds there
at all.

Cheers,
Willy



Re: [PATCH] CI: travis-ci: disable arm64 builds

2021-08-06 Thread Willy Tarreau
Hi Ilya,

On Tue, Aug 03, 2021 at 02:58:40PM +0500,  ??? wrote:
> Hello,
> 
> it looks like "something on travis-ci side".
> 
> CC  src/raw_sock.o
> gcc: fatal error: Killed signal terminated program cc1
> compilation terminated.
> 
> let us disable arm64 for a while.

Yes I noticed a few of these lately, and sometimes it even looked like
impossible downloads. I suspect that github assigns a maximum runtime
to these VMs and that they're simply overloaded and victims of their
success. I could be wrong of course.

Now applied, thank you!
Willy



[ANNOUNCE] haproxy-2.5-dev3

2021-08-01 Thread Willy Tarreau
sts for the connection and SSL error fetches

William Lallemand (7):
  BUG/MINOR: systemd: must check the configuration using -Ws
  MINOR: proxy: rename PR_CAP_LUA to PR_CAP_INT
  MINOR: mworker: the mworker CLI proxy is internal
  MINOR: stats: don't output internal proxies (PR_CAP_INT)
  CLEANUP: mworker: use the proxy helper functions in 
mworker_cli_proxy_create()
  CLEANUP: mworker: PR_CAP already initialized with alloc_new_proxy()
  REGTESTS: ssl: ssl_errors.vtc does not work with old openssl version

Willy Tarreau (22):
  BUG/MINOR: arg: free all args on make_arg_list()'s error path
  BUG/MINOR: cfgcond: revisit the condition freeing mechanism to avoid a 
leak
  MEDIUM: proxy: remove long-broken 'option http_proxy'
  BUG/MEDIUM: cfgcond: limit recursion level in the condition expression 
parser
  BUG/MEDIUM: mworker: do not register an exit handler if exit is expected
  BUG/MINOR: mworker: do not export HAPROXY_MWORKER_REEXEC across programs
  BUG/MINOR: check: fix the condition to validate a port-less server
  BUILD: threads: fix pthread_mutex_unlock when !USE_THREAD
  BUG/MEDIUM: connection: close a rare race between idle conn close and 
takeover
  BUG/MEDIUM: pollers: clear the sleeping bit after waking up, not before
  BUG/MINOR: select: fix excess number of dead/skip reported
  BUG/MINOR: poll: fix abnormally high skip_fd counter
  BUG/MINOR: pollers: always program an update for migrated FDs
  BUG/MINOR: fd: protect fd state harder against a concurrent takeover
  DOC: internals: document the FD takeover process
  MINOR: fd: update flags only once in fd_update_events()
  MINOR: poll/epoll: move detection of RDHUP support earlier
  REORG: fd: uninline fd_update_events()
  MEDIUM: fd: rely more on fd_update_events() to detect changes
  BUG/MINOR: freq_ctr: use stricter barriers between updates and readings
  MEDIUM: atomic: simplify the atomic load/store/exchange operations
  MEDIUM: atomic: relax the load/store barriers on x86_64

jenny-cheung (1):
  MINOR: deinit: always deinit the init_mutex on failed initialization

---



Re: [PATCH] BUILD: opentracing: fixed build when using pkg-config utility

2021-08-01 Thread Willy Tarreau
Hi Miroslav,

On Thu, Jul 29, 2021 at 11:37:27AM +0200, Miroslav Zagorac wrote:
> Hello all,
> 
> I am sending a patch that fixes building of the HAProxy in case the
> system-installed opentracing c wrapper is used for the opentracing
> addon.
> 
> This resolves GitHub issue #1323.

Applied now, thank you!
Willy



Re: [PATCH] memprof fix OpenBSD build.

2021-07-26 Thread Willy Tarreau
Hi David,

On Sun, Jul 25, 2021 at 11:07:00AM +0100, David CARLIER wrote:
> +/* OpenBSD does not have anything close to malloc_usable_size, thus 
> profiling will be wrong regardless */
> +#if defined(USE_MEMORY_PROFILING) && defined(__OpenBSD__)
> +#undef USE_MEMORY_PROFILING
> +#endif

I disagree with this one. It means that someone who forces
USE_MEMORY_PROFILING on this OS will get no error and will not know
that it was silently disabled. We precisely use build-time options to
make sure that the user has control over what is enabled or disabled.
This would be an exception in the middle of all other options, let's
not do it.

If OpenBSD doesn't have malloc_usable_size(), there are two
solutions. Either the option must not be used there (which is the
default), or we decide that we know for sure that it's stored at
*((char *)ptr - sizeof(void *)) with a mask, and we dereference it
there, and then we can enable it. But while I started this way at
the beginning, I'd rather avoid doing this because that could
break for those linking with different allocators.

Regards,
Willy



Re: Test, please ignore

2021-07-23 Thread Willy Tarreau
On Fri, Jul 23, 2021 at 02:22:34PM +0200, Vincent Bernat wrote:
>  ? 23 July 2021 12:55 +02, Willy Tarreau:
> 
> > The list looks uncommonly quiet after having touched some
> > anti-spam rules, just testing.
> 
> It's the holidays Willy! :)

Yep, that's what I also suspected but given the coincidence with my
last additions to the spam filters, I preferred to verify, as I
know that I broke the service a few times over the years!

Thanks!
Willy



Test, please ignore

2021-07-23 Thread Willy Tarreau
The list looks uncommonly quiet after having touched some
anti-spam rules, just testing.

Willy



Re: [PATCH] BUILD/MINOR memprof macOs build fix

2021-07-21 Thread Willy Tarreau
On Tue, Jul 20, 2021 at 08:40:34PM +0100, David CARLIER wrote:
> Hi,
> 
> here a build fix proposal for macOS when USE_MEMORY_PROFILING option is set.

Merged, thanks David!
Willy



Re: no-stop keyword proposal

2021-07-20 Thread Willy Tarreau
Hi Joao,

On Tue, Jul 20, 2021 at 12:18:18PM -0300, Joao Morais wrote:
> 
> Hello list, the diff below is a proposal to add a bind keyword used to flag
> LI_O_NOSTOP option in the bind's listener.
> 
> Regarding the use case: I need the ability to reach a stopping, but still
> running haproxy instance to, at least: 1) fairly distribute shutdown sessions
> of long running connections (usually websockets) before hard-stop-after
> timeouts and kicks all the remaining connections at the same time[1]; 2)
> collect some relevant metrics from a stopping instance, e.g. current sessions
> and rps, which would be otherwise lost when these metrics are collected only
> from the current instance.

It's a bit confusing for me because it mentions two opposite needs, one
applies to the listeners (and will thus either prevent the new process
from binding, or randomly distribute connections between the old and the
new one), and the other one implies killing random and active connections,
something we don't do at all.
 
> Regarding the patch: it's just the changes I needed to make and confirm that
> it works like I was expecting, provided that the listening socket is changed
> before reloading haproxy into a new instance. Please let me know if such
> improvement can be made and also if I'm in the right path.

That's quite of a concern to me because this means that you'll accumulate
plenty of old processes, even if they do not have any connection at all
anymore.

I think that your various needs would have to be addressed differently
(the killing of active connections and keeping the old process active).
For example, if you connect to the old process' CLI it will not quit, as
this socket counts for one. So maybe as long as you can connect there it
is enough to keep it alive and monitorable ?

For the connection shutdown, maybe we could extend "shutdown sessions" to
take a percentage, and it could apply some randomness over all connections.
This way you could periodically emit some shutdowns to kill 1% of the
connections every few seconds until you reach 100%. It's possible that
for high numbers of long connections, this significantly improves the
reload. I don't know if we could even easily automate this, but I do
see some value in it. It could sometimes kill some stats connections
as well, but with a bit of cheating that could be avoided.

Willy



Re: [PR] Release the lock init_mutex before the program ends for issue#1326.

2021-07-20 Thread Willy Tarreau
Hello,

> Author: jenny-cheung 
> Number of patches: 2
> 
> This is an automated relay of the Github pull request:
>Release the lock init_mutex before the program ends for issue#1326.

So I've remerged these two patches into one, and detailed a bit more
what they aimed to do, and merged them. Thank you!

Willy



Re: HAProxy Network Namespace Support issues, and I also found a security flaw.

2021-07-20 Thread Willy Tarreau
On Tue, Jul 20, 2021 at 03:04:05AM -0500, Peter Jin wrote:
> Sorry, after analyzing the code again, it's not a security issue since the
> ancillary buffer can only hold one file descriptor.

No problem, it's better that way, and thanks for your detailed explanation!

Willy



Re: HAProxy Network Namespace Support issues, and I also found a security flaw.

2021-07-20 Thread Willy Tarreau
Hi Peter,

first, thanks for bringing this here.

On Tue, Jul 20, 2021 at 01:13:58AM -0500, Peter Jin wrote:
> 1. The network namespace support seems to be a bit broken. In the function
> "my_socketat" (lines 114-129 of src/namespace.c in the latest dev branch),
> you attempt to first change network namespace to the desired namespace, and
> then change back to the default namespace. While this is correct, this does
> not work in two cases, both involving user namespaces:
> 
> * First, HAProxy could be running in a non-initial user namespace with a
> full set of capabilities (including CAP_SYS_ADMIN), but the network
> namespace is still associated with the initial user namespace. Such an
> environment could be simulated with "unshare -r" (omitting -n), or by using
> a container runtime that supported user namespaces but the network namespace
> is still associated with the host (e.g. docker run --net=host, if it were
> supported in userns-remap mode). In this case, the first setns() would
> succeed, but the setns() back to the original namespace would fail because
> HAProxy would not have the CAP_SYS_ADMIN capability in the original network
> namespace (it is owned by the initial user namespace).

Interesting. However, this will return a socket error, so the problem
will be detected. In the case of a listener, if the user loses CAP_SYS_ADMIN
then this will fail during startup and it will be visible. I'm more concerned
about the risk of runtime failure when connecting to a particular namespace.

The real problem I'm seeing is that there is no boot-time check on this
to verify that it still works after the return to the temporary namespace.
At the very least we could try during boot, for each server-side namespace,
to enter and leave their namespace and verify that it doesn't fail, otherwise
we'd quit.

> To mitigate this,
> HAProxy would need to fork a new process, call setns() and create socket in
> the new process, and then transfer the socket back to the original process
> using SCM_RIGHTS (you can probably reuse the code in proto_sockpair.c or
> some other file mentioning SCM_RIGHTS to do that).

Unfortunately that's really not workable, it would cause terrible latencies
that are in no way compatible with HTTP usages. It *might* work for listeners
but not for outgoing connections, and whatever solution we'll find to those
will work on both sides.

> * Second, HAProxy could be running as a non-root user, and at least one
> "rootless" container with a separated network namespace exists for that
> user. It would be nice if HAProxy could create a socket in such a network
> namespace without root privileges. Judging by what I already see in the
> code, that does not seem to be possible as it currently stands.

While we strongly encourage against running as root, I can indeed imagine
that it can be an issue with setns().

> The solution
> to solve this case is identical to the first case; the only difference is
> that you also have to enter the associated user namespace first (hint: you
> can use the NS_GET_USERNS ioctl on the target network namespace to obtain a
> file descriptor to that user namespace, which you can pass to setns()) and
> set PR_SET_DUMPABLE to 0 before entering the user namespace for security.
> 
> These techniques have already been employed in software like "slirp4netns",
> which creates a TUN/TAP device in a given network namespace, and handles
> both of the above cases correctly. The only difference is that for HAProxy,
> we should be creating a socket instead, but the overall technique is still
> the same.

Yes but while that's probably totally affordable in terms of latency
for the creation of a tunnel, it definitely is not to create an outgoing
socket. We're speaking about tens to hundreds of microseconds in the very
best case, probably even more sometimes.

One intermediate possibility might be to drop everything but CAP_SYS_ADMIN
but even then it leaves a lot of capabilities to an attacker :-/

I really think that the deepest problem is that there's still no
userland-friendly way to do socketat() without going through all that
mess :-/ The permissions ought to be checked once and it should be
possible to just create a socket on the same namespace as another one
designated from an FD that already passed the permission checks. Maybe
it's about time to reload the old discussions around that API that
ended exactly 10 years ago :-/

> Another complaint about the network namespace support is that it only
> supports namespaces in /var/run/netns. My own tool (search for "ctrtool
> ns_open_file" on google), on the other hand, support network namespaces
> created in arbitrary locations (and even allows creating sockets in
> arbitrary namespaces that also account for the above two user namespace
> scenarios). It would be nice if HAProxy supported arbitrary network
> namespace locations too, to support the rootless container use case.

I have no opinion on this and didn't even remember about 

Re: HAProxy Network Namespace Support issues, and I also found a security flaw.

2021-07-20 Thread Willy Tarreau
Hi Lukas,

On Tue, Jul 20, 2021 at 08:48:28AM +0200, Lukas Tribus wrote:
> Hello,
> 
> 
> On Tue, 20 Jul 2021 at 08:13, Peter Jin  wrote:
> > 2. There is a stack buffer overflow found in one of the files. Not
> > disclosing it here because this email will end up on the public mailing
> > list. If there is a "security" email address I could disclose it to,
> > what is it?
> 
> It's secur...@haproxy.org, it's somehow well hidden in doc/intro.txt
> (that is the *starter* guide).

I agree it's too much hidden.

> I would definitely suggest putting it on the website haproxy.org, and

I'd rather not put it as-is on the site, or at least cut into small
pieces so that it's not too much spammed.

> in the repository move it to a different file, like MAINTAINERS.

Yes I agree that being placed into this file would definitely make a lot
of sense.

Willy



Re: HashiCorp

2021-07-20 Thread Willy Tarreau
Hello Joe,
 
On Tue, Jul 20, 2021 at 11:04:38AM +, Joe Siganto wrote:
> Hi Illya,
> 
> Please could you have our Emails removed from the subscription list? I will
> have all emails with your domains from our campaigns, and as checked our
> first email ever sent to you was on 14th of July sent by our external agency:
> mailto:donna.n...@acquireb2bleads.com
> 
> Kindly let me know if we agree on this. 

I don't think Ilya did anything on his own, but I rechecked and can
confirm you are not and were never subscribed to this list, so whatever
you receive is unrelated to this. Now please stop posting your messages
on the public list, it's a development list, not a marketing list.

Willy



Re: set mss on backend site on version 1.7.9

2021-07-18 Thread Willy Tarreau
On Thu, Jul 15, 2021 at 07:04:27PM +0200, Stefan Fuhrmann wrote:
> Hello Lukas,
> 
> 
> okay, thanks!!

By the way, I think we never implemented it because it didn't appear
useful. Out of curiosity, what is your use case ? If really useful, I
think it shouldn't be too hard to implement.

Willy



[ANNOUNCE] haproxy-2.5-dev2

2021-07-17 Thread Willy Tarreau
browsing : http://git.haproxy.org/?p=haproxy.git
   Changelog: http://www.haproxy.org/download/2.5/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/

Willy
---
Complete changelog :
Amaury Denoyelle (17):
  MINOR: http: implement http_get_scheme
  MEDIUM: http: implement scheme-based normalization
  MEDIUM: h1-htx: apply scheme-based normalization on h1 requests
  MEDIUM: h2: apply scheme-based normalization on h2 requests
  REGTESTS: add http scheme-based normalization test
  BUILD: http_htx: fix ci compilation error with isdigit for Windows
  MINOR: http: implement http uri parser
  MINOR: http: use http uri parser for scheme
  MINOR: http: use http uri parser for authority
  REORG: http_ana: split conditions for monitor-uri in wait for request
  MINOR: http: use http uri parser for path
  BUG/MEDIUM: http_ana: fix crash for http_proxy mode during uri rewrite
  MINOR: mux_h2: define config to disable h2 websocket support
  MINOR: srv: extract tracking server config function
  MINOR: srv: do not allow to track a dynamic server
  MEDIUM: server: support track keyword for dynamic servers
  REGTESTS: test track support for dynamic servers

Christopher Faulet (1):
  Revert "MINOR: tcp-act: Add set-src/set-src-port for "tcp-request 
content" rules"

Daniel Black (1):
  DOC: config: use CREATE USER for mysql-check

David Carlier (1):
  BUILD/MEDIUM: tcp: set-mark support for OpenBSD

Emeric Brun (10):
  BUG/MINOR: stick-table: fix several printf sign errors dumping tables
  BUG/MINOR: peers: fix data_type bit computation more than 32 data_types
  MINOR: stick-table: make skttable_data_cast to use only std types
  MEDIUM: stick-table: handle arrays of standard types into stick-tables
  MEDIUM: peers: handle arrays of std types in peers protocol
  DOC: stick-table: add missing documentation about gpt0 stored type
  MEDIUM: stick-table: add the new array of gpt data_type
  MEDIUM: stick-table: make the use of 'gpt' excluding the use of 'gpt0'
  MEDIUM: stick-table: add the new arrays of gpc and gpc_rate
  MEDIUM: stick-table: make the use of 'gpc' excluding the use of 'gpc0/1''

Marno Krahmer (1):
  MEDIUM: stats: include disabled proxies that hold active sessions to stats

Remi Tricot-Le Breton (1):
  BUG/MINOR: ssl: Default-server configuration ignored by server

Willy Tarreau (22):
  BUG/MEDIUM: sock: make sure to never miss early connection failures
  BUG/MINOR: cli: fix server name output in "show fd"
  BUILD: stick-table: shut up invalid "uninitialized" warning in gcc 8.3
  CLEANUP: applet: remove unused thread_mask
  BUILD: add detection of missing important CFLAGS
  BUILD: lua: silence a build warning with TCC
  MINOR: init: verify that there is a single word on "-cc"
  MINOR: init: make -cc support environment variables expansion
  MINOR: arg: add a free_args() function to free an args array
  CLEANUP: config: use free_args() to release args array in 
cfg_eval_condition()
  CLEANUP: hlua: use free_args() to release args arrays
  REORG: config: move the condition preprocessing code to its own file
  MINOR: cfgcond: start to split the condition parser to introduce terms
  MEDIUM: cfgcond: report invalid trailing chars after expressions
  MINOR: cfgcond: remerge all arguments into a single line
  MINOR: cfgcond: support negating conditional expressions
  MINOR: cfgcond: make the conditional term parser automatically allocate 
nodes
  MINOR: cfgcond: insert an expression between the condition and the term
  MINOR: cfgcond: support terms made of parenthesis around expressions
  REGTEST: make check_condition.vtc fail as soon as possible
  REGTESTS: add more complex check conditions to check_conditions.vtc
  BUG/MEDIUM: init: restore behavior of command-line "-m" for memory 
limitation

---



Re: [PATCH] JA3 TLS Fingerprinting

2021-07-12 Thread Willy Tarreau
Hi Marcin,

On Mon, Jul 12, 2021 at 04:59:32PM +0200, Marcin Deranek wrote:
> Hi,
> 
> Over a past few weeks I have been working on implementing JA3 compatible
> TLS Fingerprinting[1] in the HAProxy. You can find the outcome in
> attachments. Feel free to review/comment them.

Thanks for this. First, I had never heard about this JA3 thing. I'm
seeing at your link that there's even a preliminary list of well-known
signatures, I hope it will not start to be used too much as an access
restriction, or we risk to observe some ossifications of the clients
in field, and a real difficulty to introduce new ones. At least the
signature could be used to rate-limit unknown ones. Time will tell...

Just out of curiosity (feel free not to respond if you'd prefer not to),
how are you using this result ? Is it to try to figure outliers by
matching signatures against what the user-agent claims to be, or just
for monitoring/logging or maybe for rate limiting bots ? Did you detect
different SSL libraries for a same user-agent ?

> Here are some choices I made which you should be aware of:
> - I decided to go with a "modular" approach where you can build JA3
> compatible fingerprint with available fetchers/converters rather than a
> single JA3 fetcher. This makes approach more "reusable" in some other
> scenarios.

I agree with this approach and generally that's what we're doing, because
whatever you implement in this context may serve other cases later and
speed up development of new features.

> - Each Client Hello related fetcher has option to include/exclude GREASE
> (RFC8701) values from the output. This is mainly for backward compatibility
> and ability to get "pure" data. I suspect in most cases people do not want
> GREASE values to be present in the output (which is not the case right now
> for cipherlist fetchers).

Indeed, it makes sense to offer the option to exclude purposely added noise
from the computation. I don't know what JA3 specifies regarding this, but I
guess it excludes it.

> - exclude_grease function allocates trash on demand depending on GREASE
> (RFC8701) values position in the list. We can get away without creating
> trash buffer if GREASE values are present at the very beginning and/or the
> very end of the list. I decided to allocate trash buffer only when it's
> really needed, so that's why it's creation is "hidden" inside exlude_grease
> function.

>From what I'm seeing there, you could probably simplify the function and
consider that you always allocate and copy if you need to exclude grease.
A client hello is not huge anyway, and the time saved in the memcpy() of
a few hundred bytes is not much compared to the overall processing of an
SSL hello. By the way, it would be nice if you could use a different name
(e.g. "temp") for your local trash pointer, as it shadows the thread-local
"trash" and can be confusing.

> - Now ssl_capture (next to ciphersuite) contains data about extensions, ec
> ciphers etc. One of the reasons I decided to merge all those values in a
> single ssl_capture buffer is easier control of buffer size limit. I think
> it's beneficial to have a single buffer limit for all those values rather
> than separate values for each. Having said that probably
> tune.ssl.capture-cipherlist-size needs to change it's name to eg.
> tune.ssl.capture-buffer-limit to better reflect it's function.

All this would indeed make a lot of sense. However, just renaming a config
setting is not an option. What can be done is to create the new one and
continue to process the old one while emitting a deprecation warning asking
to use the other one instead. Maybe the size will differ and the doc will
need to explain how to transform the values.

I'm having an issue with your new definition of ssl_capture_location
and its use in ssl_capture:

 /* Location and size of the data in the buffer */
 struct ssl_capture_location {
unsigned char len;   // offset 0, size 1 byte, followed by a 7-byte 
hole
size_t offset;   // offset 8, size 8 bytes
 };

=> this structure takes 16 bytes of memory

 /* This memory pool is used for capturing clienthello parameters. */
 struct ssl_capture {
unsigned long long int xxh64;
unsigned int protocol_version;
struct ssl_capture_location ciphersuite;
struct ssl_capture_location extensions;
struct ssl_capture_location ec;
struct ssl_capture_location ec_formats;
char data[VAR_ARRAY];
 };

=> thus above just for the lengths we're using 64 bytes of memory, this
   starts to be quite a lot per capture. Given that no TLS record can be
   larger than 16kB (or is that 64?), you could use two unsigned shorts
   and divide this overhead by 4.

> - Instead of creating a new converter I decided to extend existing hex
> conveter to provide a similar functionality to bin2int. I thought this
> makes more sense as extended hex converter is fully backward compatible. It
> has to be noted that extended hex 

Re: Set information in ClientHello TLS Extension as header

2021-07-10 Thread Willy Tarreau
Hi Michael,

On Sat, Jul 10, 2021 at 09:03:40AM +0200, Michael Stiller wrote:
> Hi List,
> 
> we have the following issue to solve:
> 
> A client puts some data value into a TLS Extension section (reserved or
> arbitrary id) in the ClientHello packet. I want to read this value and set a
> request header with that value which should be available to the backend which
> is designated to handle this request. Haproxy terminates / handles the SSL
> connection. 
> 
> My current idea is to somehow send that packet to a SPOE program, parse the
> ClientHello and send the value back to haproxy which sets the header. 
> 
> I am unsure if this is even remotely possible or which other options exist to 
> solve this.
> 
> I especially don't know how to detect and send the ClientHello to the SPOE.

There are some callbacks for some pieces of the client hello, but these
are mostly used to extract SNI or client certificates. I don't know if
there's anything standard to extract arbitrary TLS extensions.

> So some pointers or ideas are greatly appreciated.

Please could you have a look at ssl_sock.c:ssl_sock_switchctx_cbk() ?
There are some calls to certain SSL functions to retrieve some such info,
I don't know if they can be used for what you have in mind.

Willy



Re: [ANNOUNCE] haproxy-2.3.12

2021-07-08 Thread Willy Tarreau
On Thu, Jul 08, 2021 at 07:11:27PM +0200, Vincent Bernat wrote:
>  ?  8 July 2021 17:47 +02, Willy Tarreau:
> 
> > I'm seeing that at least Vincent was fast enough to package 2.3.11 for
> > debian 10, I hope nobody deployed it yet. I'm really sorry for the mess.
> > For those who are wondering, 2.4 was not affected.
> 
> The new packages are available!

Excellent, many thanks Vincent for always being so reactive!

Willy



Long broken option http_proxy: should we kill it ?

2021-07-08 Thread Willy Tarreau
Hi all,

Amaury discovered that "option http_proxy" was broken. I quickly checked
when it started, and it got broken with the introduction of HTX in 1.9
three years ago. It still used to work in legacy mode in 1.9 and 2.0
but 2.0 uses HTX by default and legacy disappeared from 2.1. Thus to
summarize it, no single version emitted during the last 2.5 years saw it
working.

As such I was considering removing it from 2.5 without prior deprecation.
My opinion is that something that doesn't work for 2.5 years and that
triggers no single report is a sufficient indicator of non-use. We'll
still need to deploy reasonable efforts to see under what conditions it
can be fixed and the fix backported, of course. Does anyone object to
this ?

For a bit of background, this option was added 14 years ago to extract
an IP address an a port from an absolute URI, rewrite it to relative
and forward the request to the original IP:port, thus acting like a
non-resolving proxy. Nowadays one could probably achieve the same
by doing something such asthe following:

http-request set-dst url_ip
http-request set-dst-port url_port
http-request set-uri %[path]

And it could even involve the do_resolve() action to resolve names to
addresses. That's why I'm in favor of not even trying to keep this one
further.

Thanks,
Willy



[ANNOUNCE] haproxy-2.3.12

2021-07-08 Thread Willy Tarreau
Hi,

HAProxy 2.3.12 was released on 2021/07/08. It added 2 new commits
after version 2.3.11.

Please do not use 2.3.11! I failed the backport of the use-after-free bug
fix in the lockless pools between 2.4 and 2.3. The pools code in 2.4 was
significantly reworked to be cleaner and simpler, and I found two
occurrences in 2.3 and older that required the same fix and that were
missing it. The result can be a runtime deadlock depending on the build
options, the operating system and the load (the watchdog will catch it,
but nobody wants to deploy this, obviously).

After scrutinizing the code all the afternoon and torturing it under
different build options, I can now affirm that the code is properly fixed
in 2.3.12.

These patches were backported into 2.2 as well because the faulty patch was
already there. For 2.0 and below the patch was fixed to limit the risks of
incomplete backports (namely for those who continue to cherry-pick selected
fixes).

I'm seeing that at least Vincent was fast enough to package 2.3.11 for
debian 10, I hope nobody deployed it yet. I'm really sorry for the mess.
For those who are wondering, 2.4 was not affected.

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.3/src/
   Git repository   : http://git.haproxy.org/git/haproxy-2.3.git/
   Git Web browsing : http://git.haproxy.org/?p=haproxy-2.3.git
   Changelog: http://www.haproxy.org/download/2.3/src/CHANGELOG
   Cyril's HTML doc : http://cbonte.github.io/haproxy-dconv/

Willy
---
Complete changelog :
Willy Tarreau (2):
  BUG/MAJOR: pools: fix incomplete backport of lockless pool fix
  BUG/MAJOR: pools: second fix for incomplete backport of lockless pool fix

---



Re: Proposal about new default SSL log format

2021-07-08 Thread Willy Tarreau
On Thu, Jul 08, 2021 at 02:18:32PM +0200, William Lallemand wrote:
> I saw that you hesitated between "conn_status" and "conn_err_code", the
> "conn_" prefix could be confusing at some point once you try to have
> errors on the frontend and the backend side in the same log-format, I
> think something starting by "fc_conn_" would be more understandable.

Indeed, and more consistent with what we already have. fc_* is for the
front connection, bc_* is for the back connection. By the way if we're
focusing on SSL it should even be ssl_fc_* (we already have a ton of
them, nobody will find the new one if it doesn't use the same prefix).

> That seems good to me, we only need frontend info IMHO. People who need
> the SSL backend connection are not the most common case so they could
> make their own log-format with it.

I tend to think that if we're focusing on https vs http, it makes sense
to consider the frontend only as well for the standard logs.

Also some background on the log format, originally we used to place the
URI at the end of the line because most loggers used to truncate logs
at 1024 bytes and the tail of the URI was far less important than other
fields. This explains why we've started to insert certain fields at
certain places before this. 20 years later I think it is perfectly
reasonable to consider appending fields *after* the URI, which is also
a great way of applying minimal changes to existing log parsers, and
to allow http/https log lines to be easily compared when aligned.

Willy



Re: ratio spam/useful message

2021-07-07 Thread Willy Tarreau
Hi Julien,

On Tue, Jul 06, 2021 at 11:06:05AM +0200, Julien Pivotto wrote:
> Hello,
> 
> Lately, the ratio spam/useful message on the ML has been quite high.

Well, that's not what I'm seeing in the archives here:

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

I had to manually delete 9 messages this month and 29 last month from
my mbox, or 1 per day, I don't find it high at all, quite the opposite!
I've added the recurrent "NFP workshops" ones and the occasional
"sock-raw.org" to the block list, but I'm not seeing that much come in.
Are you getting more from the list ? Am I the only one getting so few ?
Even checking the local archives here didn't reveal anything exceptional.

Willy



Re: Proposal about new default SSL log format

2021-07-06 Thread Willy Tarreau
On Tue, Jul 06, 2021 at 12:19:56PM +0200, Tim Düsterhus wrote:
> Willy,
> 
> On 7/6/21 12:12 PM, Willy Tarreau wrote:
> > A few points first, that are needed to address various concerns. The
> > goal here is to defined an HTTPS log format because that's what the
> > vast majority of users are dealing with day-to-day. For specific usages,
> > everyone already redefines log formats. But for a very basic setup, right
> > now it's true that httplog is a bit limited, and all it shows to help guess
> > there was SSL involved is to append this '~' after the frontend's name.
> 
> It was not clear to me that this log format is meant to apply to HTTPS
> requests, because the example given by Remi does not include the HTTP verb
> and neither the request URI (unless I need glasses). I thought it was a
> format for TLS errors or something like this.

At least it was my understanding that it was mostly aimed at HTTPS given
that the discussion we had on the subject started around this :-)

> Is this a mistake in the examples? Or is HAProxy going to emit multiple log
> lines: One for the TLS connection and one for each HTTP request passing
> through this TLS connection?

There was a discussion around logging at various layers, I don't remember
if it was on the list or in a github issue, but I remember having been
involved in this some time ago. I think that it makes sense to ultimately
be able to emit error logs at various levels (i.e. when being certain that
no other event will happen) but it's particularly confusing to log successes
at various levels. Thus typically seeing TLS handshake failures is important
but their success should not be logged as all the info will instead be
available with the traffic logs. There are already enough users complaining
about noise caused by client-aborted connections :-)

Willy



Re: Proposal about new default SSL log format

2021-07-06 Thread Willy Tarreau
Hi Rémi,

[ I warned you that this was going to open a pandora box :-) ]

On Fri, Jul 02, 2021 at 04:26:48PM +0200, Remi Tricot-Le Breton wrote:
> Some work in ongoing to ease connection error and SSL handshake error
> logging.
> This will rely on some new sample fetches that could be added to a custom
> log-format string.
> In order to ease SSL logging and debugging, we will also add a new default
> log
> format for SSL connections. Now is then the good time to find the best
> format
> for everyone.
> The proposed format looks like the HTTP one to which the SSL specific
> information is added. But if anybody sees a missing information that could
> be
> beneficial for everybody, feel free to tell it, nothing is set in stone yet.

A few points first, that are needed to address various concerns. The
goal here is to defined an HTTPS log format because that's what the
vast majority of users are dealing with day-to-day. For specific usages,
everyone already redefines log formats. But for a very basic setup, right
now it's true that httplog is a bit limited, and all it shows to help guess
there was SSL involved is to append this '~' after the frontend's name.

However, it's also clear that most users will not violently migrate from
httplog to httpslog, and it's important to keep a smooth enough transition.
This also means not to change stuff that would be relevant to httplog as
well (e.g. delimitors, time format etc).

We can (and should) have discussions about what to change in future log
formats, but let's not use the https one as a bootstrap for passing
everyone's missing field. Instead, let's focus on the SSL-specific stuff
that users are always missing from HTTP logs, and try to establish a
reasonable list that will always be there and suit most users without
adding too much for others, and that will require limited adaptations
to parsers.

> The format would look like this :
>     >>> Jul  1 18:11:31 haproxy[143338]: 127.0.0.1:37740
> [01/Jul/2021:18:11:31.517] \
>   ssl_frontend~ ssl_frontend/s2 0/0/0/7/+7 \
>   0/0/0/0 2750  1/1/1/1/0 0/0 TLSv1.3 TLS_AES_256_GCM_SHA384

Like Aleks, I think that we could have a single version+cipher field,
as not all combinations are permitted anyway.

>   Field   Format    Extract from the example
> above
>   1   process_name '[' pid ']:' haproxy[143338]:
>   2   client_ip ':' client_port 127.0.0.1:37740
>   3   '[' request_date ']' [01/Jul/2021:18:11:31.517]
>   4   frontend_name ssl_frontend~
>   5   backend_name '/' server_name ssl_frontend/s2
>   6   TR '/' Tw '/' Tc '/' Tr '/' Ta*  
> 0/0/0/7/+7
>   7 *conn_status '/' SSL hsk error '/' SSL vfy '/' SSL CA vfy*0/0/0/0

Be careful above not to reproduce the errors from the past, namely
using a designation that could apply to both sides. "SSL hsk error"
etc could be valid for either frontend connection, backend connection
or both. We must absolutely be clear and explicit here about what
this is. The same will be true for the versions and ciphers.

I suspect that all of these will mostly be of interest for the front
side but I could be wrong (particularly for errors). But it's probable
that discussing the backend side already enters a new sub-topic in fact.

Willy



Re: [PATCH] MEDIUM: stats: include disabled proxies that hold active sessions to stats

2021-07-06 Thread Willy Tarreau
Hi Marno,

On Mon, Jun 28, 2021 at 07:32:19AM +, Marno Krahmer wrote:
> Hello,
> 
> I would like to add a path to HAProxy.
> This patch is supposed to change how stats are handled for disabled proxies.
(...)

It's been one week without comments, my concerns about the rare but
possible special corner cases are now addressed, I've merged it.

Thanks!
Willy



Re: Bad backend selected

2021-07-02 Thread Willy Tarreau
Hi Tim,

On Sat, Jun 26, 2021 at 08:20:20PM +0200, Tim Düsterhus wrote:
> Willy, Amaury,
> 
> On 6/18/21 5:55 PM, Willy Tarreau wrote:
> > Amaury started something in this direction but it was only in H2 and
> > I'd like that we explore the ability to do it the most generic way
> > possible so as not to have different behaviours depending on where
> > the requests enter and where they leave. I know it's tricky, which is
> > one more reason for exploring this possibility. At least regardless
> > of the final choice, we'll be certain not to have left unknown cases
> > incorrectly handled.
> > 
> > Thus no need to create a new issue for now, hopefully by next week
> > we'll all agree on a durable solution.
> 
> FYI: Another user in IRC (#haproxy on Libera.Chat) just stumbled upon this
> issue as well. So it appears it's a reasonably common occurence with Firefox
> and HAProxy 2.4.

Just a quick heads up on this since nobody responded, we had a long
discussion on the subject and Amaury proposed some interesting patches
that we've been discussing and that will experience a few changes to
preserve performance before being merged, but we're getting close to
having them merged and backported now.

Cheers,
Willy



[ANNOUNCE] haproxy-2.5-dev1

2021-06-30 Thread Willy Tarreau
  BUG/MINOR: server: Missing calloc return value check in srv_parse_source
  BUG/MINOR: peers: Missing calloc return value check in 
peers_register_table
  BUG/MINOR: ssl: Missing calloc return value check in 
ssl_init_single_engine
  BUG/MINOR: http: Missing calloc return value check in 
parse_http_req_capture
  BUG/MINOR: proxy: Missing calloc return value check in proxy_parse_declare
  BUG/MINOR: proxy: Missing calloc return value check in proxy_defproxy_cpy
  BUG/MINOR: http: Missing calloc return value check while parsing 
tcp-request/tcp-response
  BUG/MINOR: http: Missing calloc return value check while parsing 
tcp-request rule
  BUG/MINOR: compression: Missing calloc return value check in 
comp_append_type/algo
  BUG/MINOR: worker: Missing calloc return value check in 
mworker_env_to_proc_list
  BUG/MINOR: http: Missing calloc return value check while parsing redirect 
rule
  BUG/MINOR: http: Missing calloc return value check in make_arg_list
  BUG/MINOR: proxy: Missing calloc return value check in 
chash_init_server_tree
  BUG/MINOR: ssl: OCSP stapling does not work if expire too far in the 
future
  MINOR: ssl: Keep the actual key length in the certificate_ocsp structure
  MINOR: ssl: Add new "show ssl ocsp-response" CLI command
  MINOR: ssl: Add the OCSP entry key when displaying the details of a 
certificate
  MINOR: ssl: Add the "show ssl cert foo.pem.ocsp" CLI command
  REGTESTS: ssl: Add "show ssl ocsp-response" test
  BUILD: ssl: Fix compilation with BoringSSL
  MINOR: ssl: Use OpenSSL's ASN1_TIME convertor when available

Tim Duesterhus (13):
  MINOR: cfgparse: Fail when encountering extra arguments in macro
  CLEANUP: reg-tests: Remove obsolete no-htx parameter for reg-tests
  CLEANUP: cfgparse: Remove duplication of `MAX_LINE_ARGS + 1`
  CI: Make matrix.py executable and add shebang
  REGTESTS: Remove REQUIRE_VERSION=1.6 from all tests
  REGTESTS: Remove REQUIRE_VERSION=1.7 from all tests
  CI: Replace the requirement for 'sudo' with a call to 'ulimit -n'
  REGTESTS: Replace REQUIRE_VERSION=2.5 with 'haproxy -cc'
  REGTESTS: Replace REQUIRE_OPTIONS with 'haproxy -cc' for 2.5+ tests
  REGTESTS: Replace REQUIRE_BINARIES with 'command -v'
  REGTESTS: Remove support for REQUIRE_BINARIES
  BUG/MINOR: cache: Correctly handle existing-but-empty 'accept-encoding' 
header
  CLEANUP: Prevent channel-t.h from being detected as C++ by GitHub

Tim Düsterhus (1):
  DOC: Replace issue templates by issue forms

William Lallemand (4):
  BUILD: fix compilation for OpenSSL-3.0.0-alpha17
  CI: github actions: -Wno-deprecated-declarations with OpenSSL 3.0.0
  BUILD: make tune.ssl.keylog available again
  REGTESTS: ssl: show_ssl_ocspresponce.vtc is broken with BoringSSL

Willy Tarreau (94):
  CLEANUP: backend: fix incorrect comments on locking conditions for lb 
functions
  SCRIPTS: opentracing: enable parallel builds in build-ot.sh
  BUG/MINOR: pools: fix a possible memory leak in the lockless pool_flush()
  BUG/MINOR: pools: make DEBUG_UAF always write to the to-be-freed location
  MINOR: pools: do not maintain the lock during pool_flush()
  MINOR: pools: call malloc_trim() under thread isolation
  MEDIUM: pools: use a single pool_gc() function for locked and lockless
  BUG/MAJOR: pools: fix possible race with free() in the lockless variant
  CLEANUP: pools: remove now unused seq and pool_free_list
  MEDIUM: pools: remove the locked pools implementation
  BUG/MEDIUM: errors: include missing obj_type file
  MINOR: config: remove support for deprecated option "tune.chksize"
  MINOR: config: completely remove support for "no option http-use-htx"
  MINOR: log: remove the long-deprecated early log-format tags
  MINOR: http: remove the long deprecated "set-cookie()" sample fetch 
function
  MINOR: config: reject long-deprecated "option forceclose"
  MINOR: config: remove deprecated option "http-tunnel"
  MEDIUM: proxy: remove the deprecated "grace" keyword
  MAJOR: config: remove parsing of the global "nbproc" directive
  BUILD: init: remove initialization of multi-process thread mappings
  BUILD: log: remove unused fmt_directive()
  BUG/MEDIUM: shctx: use at least thread-based locking on USE_PRIVATE_CACHE
  BUG/MINOR: ssl: use atomic ops to update global shctx stats
  BUG/MINOR: mworker: fix typo in chroot error message
  CLEANUP: global: remove unused definition of stopping_task[]
  MEDIUM: init: remove the loop over processes during init
  MINOR: mworker: remove the initialization loop over processes
  CLEANUP: global: remove the nbproc field from the global structure
  CLEANUP: global: remove pid_bit and all_proc_mask
  MEDIUM: global: remove

Re: [PATCH] MEDIUM: stats: include disabled proxies that hold active sessions to stats

2021-06-30 Thread Willy Tarreau
Hi,

first, thanks Marno for sharing your work.

On Mon, Jun 28, 2021 at 07:32:19AM +, Marno Krahmer wrote:
> Hello,
> 
> I would like to add a path to HAProxy.
> This patch is supposed to change how stats are handled for disabled proxies.
> 
> Prior to this patch, when outputting stats information, disabled proxies were 
> ignored / skipped.
> This was an issue with old processes after a reload of HAProxy.
> 
> It caused "the old process", that was still holding active sessions, to not 
> report stats any more.
> This made it impossible for any monitoring solution to figure out, how many 
> currently active sessions exist.
> 
> While this issue might barely be noticeable when using HAProxy for
> HTTP-Traffic, for long-running TCP-Sessions, this can become an issue.
> 
> This patch will now not only check, if a proxy is disabled, but also if it
> still holds active sessions. And as long as it does, it will still output
> statistics.
> 
> Initially I opened the following Issue on GitHub: 
> https://github.com/haproxy/haproxy/issues/1307

For those not having followed the discussion there, this patch introduces
a trade-off which seems very reasonable to me. The goal is to have as
little behavior change as possible so as not to confuse those who are
used to monitor their stats sockets during reloads. I tend to consider
that the benefits of still seeing some proxies that still have active
connections substantially outweight the surprise of the change here
from the old situation where all old process stats instantly disappear.

I'd like to go further so that we could always keep the old process
stats available during reloads (i.e. even after the last connection
completes on a frontend, it remains visible). But Marno's tests
exhibited some unexpected behaviors making the master frontend
suddenly appear, so we thought we could postpone that to after we've
figured what causes this to happen.

I'd personally be in favor of merging this trivial patch into 2.5-dev
ASAP, and would not be opposed to backporting it to 2.4 after some time
if there is some demand for this. I'd rather do it after we've figured
why the alternate solution doesn't work though, as it might uncover
a hidden bug elsewhere.

If anyone thinks that listing stopping frontends/backends on a leaving
process could have annoying side effects that we've overlooked, please
detail them here so that a better solution could be designed. I consider
that the current situation is mostly an accident resulting from the same
state being used for a proxy being disabled in the config and one being
disabled during reload. But some might rely on this :-/ And similary if
you think you're having a better idea which shouldn't break deployed
stuff, feel free to suggest.

thanks,
Willy

> The patch:
> 
> From 0648fc0c148fe463ea9f0c77f34beeb484688eac Mon Sep 17 00:00:00 2001
> From: Marno Krahmer 
> Date: Thu, 24 Jun 2021 16:51:08 +0200
> Subject: [PATCH] MEDIUM: stats: include disabled proxies that hold active
> sessions to stats
> 
> After reloading HAProxy, the old process may still hold active sessions.
> Currently there is no way to gather information, how many sessions such
> a process still holds. This patch will not exclude disabled proxies from
> stats output when they hold at least one active session. This will allow
> sending `!@ show stat` through a master socket to the disabled
> process and have it returning its stats data.
> ---
> src/stats.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/src/stats.c b/src/stats.c
> index 3458924b7..a5620577a 100644
> --- a/src/stats.c
> +++ b/src/stats.c
> @@ -3575,8 +3575,11 @@ static int stats_dump_proxies(struct stream_interface 
> *si,
>   }
> 
>   px = appctx->ctx.stats.obj1;
> -  /* skip the disabled proxies, global frontend 
> and non-networked ones */
> -  if (!px->disabled && px->uuid > 0 && (px->cap 
> & (PR_CAP_FE | PR_CAP_BE))) {
> + /* skip the global frontend proxies and 
> non-networked ones
> + * also skip disabled proxies unless they are 
> still holding active sessions.
> + * This change allows retrieving stats from 
> "old" proxies after a reload.
> + */
> + if ((!px->disabled || px->served > 0) && 
> px->uuid > 0 && (px->cap & (PR_CAP_FE | PR_CAP_BE))) {
>   if 
> (stats_dump_proxy_to_buffer(si, htx, px, uri) == 0)
>   return 0;
>   }
> --
> 2.17.0
> 
> 
> Thanks a lot
> Marno
> 



Re: [PATCH] BUILD/MEDIUM: tcp-act: set-mark support fir FreeBSD

2021-06-27 Thread Willy Tarreau
On Sat, Jun 26, 2021 at 05:03:24PM +0100, David CARLIER wrote:
> Looks correct thanks !

thank you both, now merged!
Willy



Re: [PATCH] BUILD/MEDIUM: tcp-act: set-mark support fir FreeBSD

2021-06-26 Thread Willy Tarreau
Hi David,

On Sat, Jun 26, 2021 at 12:09:30PM +0100, David CARLIER wrote:
> Hi here a little patch to enable set-mark for the FreeBSD platform.

That's interesting, great finding! Could you please add a small note
about FreeBSD being supported in the doc for the related actions ? For
now it only mentions linux 2.6.32+. Maybe mentioning DTrace and filtering
as you said could give ideas to users.

Thanks,
Willy



Re: [PATCH spoa-server] BUG/MINOR: build: install binary inside bin/ directory

2021-06-25 Thread Willy Tarreau
On Sun, Jun 13, 2021 at 01:13:52AM +0100, Bertrand Jacquin wrote:
> Prior to the change, spoa is installed under DESTDIR with name `bin`

Thanks Bertrand. I've merged it now, after realizing that I still had
access to this now external repo :-)

Willy



Re: [EXTERNAL] Re: built in ACL, REQ_CONTENT

2021-06-25 Thread Willy Tarreau
On Tue, Jun 08, 2021 at 06:32:50PM +0200, Lukas Tribus wrote:
> Please try to ask the actual question directly next time, so we can
> help you right away (https://xyproblem.info/).

I didn't know this extremely common problem had a name, thanks Lukas
for the link, I'll share it more often!

Willy



Re: MINOR: fixes haiku linkage

2021-06-25 Thread Willy Tarreau
On Sat, Jun 19, 2021 at 02:47:52PM +0100, David CARLIER wrote:
> Hi here a little change proposal to fix haproxy at runtime in this platform.
> Cheers.

Thanks David, now merged.
Willy



Re: Fix small bug in srv_parse_agent_check

2021-06-25 Thread Willy Tarreau
Hi Dirkjan,

On Fri, Jun 18, 2021 at 10:03:17PM +0200, Dirkjan Bussink wrote:
> Hi all,
> 
> I was building HAProxy using scan-build to see if there were any issues and
> it called out an unused variable write. I think this was due to a bug that
> the err_code was not used in srv_parse_agent_check.

I agree with your analysis, I've now merged your patch.

Thank you!
Willy



  1   2   3   4   5   6   7   8   9   10   >