Re: [PATCH 1/2] CLEANUP: Remove appsession documentation

2019-05-05 Thread Willy Tarreau
On Mon, May 06, 2019 at 01:29:20AM +0200, Tim Düsterhus wrote:
> - What about 'resolution_pool_size'? The only thing it does is emitting
> a warning (not a fatal error). I believe it can also be removed from the
> documentation.

Apparently there is no single stable version with this directive, I
think it was added during development and removed later. Thus both
the doc entry and the parsing can be removed.

> - What about 'block' and 'redispatch' and the various *timeouts which
> are deprecated since more than 5 years (with HAProxy 1.5):
> https://github.com/haproxy/haproxy/commit/de9d2d7b86abb0d7110bc3190aca5b26fec6fd64,
> https://github.com/haproxy/haproxy/commit/a3c504c032614ee65af434e41c08ebe46855ebd8
> and
> https://github.com/haproxy/haproxy/commit/ed44649eb78a8ce5efc203f273c7df774defe2af
>   Currently they appear to still be functional. Perhaps they can be made
> non-functional (fatal error) and removed from the documentation.

While I totally agree with this, I want users of 1.9 to experience the
least possible friction when upgrading to 2.0 because 1.9 is not LTS.
However I'm totally OK with removing them from the doc right now and
removing them completely in 2.1. In addition, I wanted to remove all
the req* and rsp* directives for 1.9 but I recalled it too late in the
development cycle. Similarly I'd like to see all these directives emit
a warning now and be removed in 2.1. Their existence causes too much
trouble, they are not evaluated in the same order as the other ones,
and they are totally emulated nowadays (the header block is artificially
rebuilt, passed to them for regex processing, and the result is parsed
again and reinjected into the headers block). Thus I would welcome a
patch adding the warnings and recommendations for alternatives to all
of them. This along with the legacy HTTP code removal are the things
that could deserve opening the -next branch if someone is interested in
starting these cleanups.

BTW, I merged your two other patches.

Thanks,
Willy



Re: [1.9.7] One of haproxy processes using 100% CPU

2019-05-05 Thread Maciej Zdeb
Hi,

I confirm Willy patch fixed the problem! Thanks!

wt., 30 kwi 2019 o 13:49 Maciej Zdeb  napisał(a):

> Hi Olivier,
>
> Thank you very much. I'll test it and get back with feedback!
>
> Regards,
>
> wt., 30 kwi 2019 o 13:12 Olivier Houchard 
> napisał(a):
>
>> Hi Maciej,
>>
>> On Tue, Apr 30, 2019 at 08:43:07AM +0200, Maciej Zdeb wrote:
>> > Filtered results from show fd for that particular virtual server:
>> >
>> > 10 : st=0x22(R:pRa W:pRa) ev=0x01(heopI) [lc] cnext=-3 cprev=-2
>> tmask=0x1
>> > umask=0x0 owner=0x53a5690 iocb=0x59d689(conn_fd_handler) back=0
>> > cflg=0x80243300 fe=virtual-server_front mux=H2 ctx=0x6502860 h2c.st0=2
>> > .err=0 .maxid=17 .lastid=-1 .flg=0x1 .nbst=0 .nbcs=1 .fctl_cnt=0
>> > .send_cnt=0 .tree_cnt=1 .orph_cnt=0 .sub=0 .dsi=13 .dbuf=0@(nil)+0/0
>> > .msi=-1 .mbuf=0@(nil)+0/0 last_h2s=0x5907040 .id=13 .flg=0x4005
>> > .rxbuf=0@(nil)+0/0
>> > .cs=0x905b1b0 .cs.flg=0x00106a00 .cs.data=0x5d1d228
>> > 98 : st=0x22(R:pRa W:pRa) ev=0x01(heopI) [lc] cnext=-1809 cprev=-2
>> > tmask=0x1 umask=0x0 owner=0xa3bb7f0 iocb=0x59d689(conn_fd_handler)
>> back=0
>> > cflg=0x80201300 fe=virtual-server_front mux=H2 ctx=0xa71f310 h2c.st0=3
>> > .err=0 .maxid=0 .lastid=-1 .flg=0x0008 .nbst=0 .nbcs=0 .fctl_cnt=0
>> > .send_cnt=0 .tree_cnt=0 .orph_cnt=0 .sub=0 .dsi=3
>> > .dbuf=16384@0x5873f10+61/16384
>> > .msi=-1 .mbuf=0@(nil)+0/0
>>
>> I see that it seems to be HTTP/2. Not saying it's your problem, but a bug
>> that would cause haproxy to use 100% of the CPU has been fixed in the
>> HTTP/2
>> code just after the 1.9.7 release was done.
>> Any chance you can see if it still happens with that commit :
>> commit c980b511bfef566e9890eb9a06d607c193d63828
>> Author: Willy Tarreau 
>> Date:   Mon Apr 29 10:20:21 2019 +0200
>>
>> BUG/MEDIUM: mux-h2: properly deal with too large headers frames
>>
>> Regards,
>>
>> Olivier
>>
>


Re: [PATCH 1/2] CLEANUP: Remove appsession documentation

2019-05-05 Thread Tim Düsterhus
Willy,

partly related to these two patches I already sent:

- What about 'resolution_pool_size'? The only thing it does is emitting
a warning (not a fatal error). I believe it can also be removed from the
documentation.
- What about 'block' and 'redispatch' and the various *timeouts which
are deprecated since more than 5 years (with HAProxy 1.5):
https://github.com/haproxy/haproxy/commit/de9d2d7b86abb0d7110bc3190aca5b26fec6fd64,
https://github.com/haproxy/haproxy/commit/a3c504c032614ee65af434e41c08ebe46855ebd8
and
https://github.com/haproxy/haproxy/commit/ed44649eb78a8ce5efc203f273c7df774defe2af
  Currently they appear to still be functional. Perhaps they can be made
non-functional (fatal error) and removed from the documentation.

Best regards
Tim Düsterhus



[PATCH 2/2] DOC: Fix typo in keyword matrix

2019-05-05 Thread Tim Duesterhus
It should read 'deprecated' instead of 'deprectated'.
---
 doc/configuration.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 80c32863d..1fbe45272 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -2284,7 +2284,7 @@ option clitcpka  (*)  X  X
 X -
 option contstats (*)  X  X X -
 option dontlog-normal(*)  X  X X -
 option dontlognull   (*)  X  X X -
-option forceclose  (deprectated) (*)  X  X X X
+option forceclose   (deprecated) (*)  X  X X X
 -- keyword -- defaults - frontend - listen -- backend -
 option forwardfor X  X X X
 option http-buffer-request   (*)  X  X X X
-- 
2.21.0




[PATCH 1/2] CLEANUP: Remove appsession documentation

2019-05-05 Thread Tim Duesterhus
I was about to partly revert 294d0f08b3d100fcae0e71c26d4f9f93d26e3569,
because there were no 'X' for 'appsession' in the keyword matrix until
I checked the blame, realizing that the feature does not exist any more.

Clearly the documentation is confusing here, the removal note is only
listed *below* the old documentation and the supported sections still
show 'backend' and 'listen'.

It's been 3.5 years and 4 releases (1.6, 1.7, 1.8 and 1.9), I guess
this can be removed from the documentation of future versions.
---
 doc/configuration.txt | 54 ---
 src/cfgparse-listen.c |  2 +-
 2 files changed, 1 insertion(+), 55 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index ee6f81c1d..80c32863d 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -2221,7 +2221,6 @@ specified in a previous "defaults" section.
  keyword  defaults   frontend   listenbackend
 +--+--+-+-
 acl   -  X X X
-appsession-  - - -
 backlog   X  X X -
 balance   X  - X X
 bind  -  X X -
@@ -2434,59 +2433,6 @@ acl   [flags] [operator]  ...
   See section 7 about ACL usage.
 
 
-appsession  len  timeout 
-   [request-learn] [prefix] [mode ]
-  Define session stickiness on an existing application cookie.
-  May be used in sections :   defaults | frontend | listen | backend
- no|no|   yes  |   yes
-  Arguments :
-   this is the name of the cookie used by the application and which
-   HAProxy will have to learn for each new session.
-
-   this is the max number of characters that will be memorized and
-   checked in each cookie value.
-
- this is the time after which the cookie will be removed from
-   memory if unused. If no unit is specified, this time is in
-   milliseconds.
-
-request-learn
-   If this option is specified, then haproxy will be able to learn
-   the cookie found in the request in case the server does not
-   specify any in response. This is typically what happens with
-   PHPSESSID cookies, or when haproxy's session expires before
-   the application's session and the correct server is selected.
-   It is recommended to specify this option to improve reliability.
-
-prefix When this option is specified, haproxy will match on the cookie
-   prefix (or URL parameter prefix). The appsession value is the
-   data following this prefix.
-
-   Example :
-   appsession ASPSESSIONID len 64 timeout 3h prefix
-
-   This will match the cookie ASPSESSIONIDXXX=,
-   the appsession value will be XXX=.
-
-mode   This option allows to change the URL parser mode.
-   2 modes are currently supported :
-   - path-parameters :
- The parser looks for the appsession in the path parameters
- part (each parameter is separated by a semi-colon), which is
- convenient for JSESSIONID for example.
- This is the default mode if the option is not set.
-   - query-string :
- In this mode, the parser will look for the appsession in the
- query string.
-
-  As of version 1.6, appsessions was removed. It is more flexible and more
-  convenient to use stick-tables instead, and stick-tables support multi-master
-  replication and data conservation across reloads, which appsessions did not.
-
-  See also : "cookie", "capture cookie", "balance", "stick", "stick-table",
- "ignore-persist", "nbproc" and "bind-process".
-
-
 backlog 
   Give hints to the system about the approximate listen backlog desired size
   May be used in sections :   defaults | frontend | listen | backend
diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index 7c64be102..e7cd0663e 100644
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -1290,7 +1290,7 @@ int cfg_parse_listen(const char *file, int linenum, char 
**args, int kwm)
}
}
else if (!strcmp(args[0], "appsession")) {  /* cookie name */
-   ha_alert("parsing [%s:%d] : '%s' is not supported anymore, 
please check the documentation.\n", file, linenum, args[0]);
+   ha_alert("parsing [%s:%d] : '%s' is not supported anymore since 
HAProxy 1.6.\n", file, linenum, args[0]);
err_code |= ERR_ALERT | ERR_FATAL;
goto out;
}
-- 
2.21.0




Re: [ANNOUNCE] haproxy-1.8.20

2019-05-05 Thread Willy Tarreau
On Sun, May 05, 2019 at 10:59:29PM +0200, Vincent Bernat wrote:
>  ?  5 mai 2019 09:51 +02, Vincent Bernat :
> 
> >> So I'd suggest to insist on having the up to date version (even 1.8.21 if
> >> we have a reason to have this one released by then). In the worst case,
> >> if this is rejected for whatever reason, it's better to leave a well known
> >> version there and continue to encourage every user to switch to your up to
> >> date PPA than making them believe their version contains the essential
> >> fixes, which would be misleading.
> >
> > OK, I have just asked our release team if they would accept 1.8.20 as
> > is. Let's see!
> 
> They don't want to make an exception. So, Debian 10 will be with HAProxy
> 1.8.19.

OK, things have not changed much. Thanks for trying!

Willy



Re: [ANNOUNCE] haproxy-1.8.20

2019-05-05 Thread Vincent Bernat
 ❦  5 mai 2019 09:51 +02, Vincent Bernat :

>> So I'd suggest to insist on having the up to date version (even 1.8.21 if
>> we have a reason to have this one released by then). In the worst case,
>> if this is rejected for whatever reason, it's better to leave a well known
>> version there and continue to encourage every user to switch to your up to
>> date PPA than making them believe their version contains the essential
>> fixes, which would be misleading.
>
> OK, I have just asked our release team if they would accept 1.8.20 as
> is. Let's see!

They don't want to make an exception. So, Debian 10 will be with HAProxy
1.8.19.
-- 
Make it right before you make it faster.
- The Elements of Programming Style (Kernighan & Plauger)



Re: Zero RTT in backend server side

2019-05-05 Thread Olivier Houchard
Hi Igor,

On Mon, May 06, 2019 at 12:26:33AM +0800, Igor Pav wrote:
> Hi, Olivier, thanks for the effort. So can we force the server always
> to carry data to remote via 0RTT like below scenario(to protect
> http2http in unsecured env)?
> 
> listen http -- server default x.x ssl allow-0rtt (SSL) bind
> x.x ssl allow-0rtt -- http backend
> 

As it is currently, no. Haproxy will never attempt to use 0RTT on server
connections if the client didn't use 0RTT.
2.0, however, which should be released in a not to distant future, will let
you do that, with the new "retry-on" feature.

Regards,

Olivier




[PR] travis-ci: mark LibreSSL builds as allowed failures

2019-05-05 Thread PR Bot
Dear list!

Author: Ilya Shipitsin 
Number of patches: 4

This is an automated relay of the Github pull request:
   travis-ci: mark LibreSSL builds as allowed failures

Patch title(s): 
   BUILD: remove "build_libressl" duplicate declaration
   BUILD: travis-ci: get back to osx without openssl support
   BUILD: enable several LibreSSL hacks, including
   BUILD: temporarily mark LibreSSL builds as allowed to fail

Link:
   https://github.com/haproxy/haproxy/pull/92

Edit locally:
   wget https://github.com/haproxy/haproxy/pull/92.patch && vi 92.patch

Apply locally:
   curl https://github.com/haproxy/haproxy/pull/92.patch | git am -

Description:


Instructions:
   This github pull request will be closed automatically; patch should be
   reviewed on the haproxy mailing list (haproxy@formilux.org). Everyone is
   invited to comment, even the patch's author. Please keep the author and
   list CCed in replies. Please note that in absence of any response this
   pull request will be lost.



Re: reg-tests are broken when running osx + openssl

2019-05-05 Thread Frederic Lecaille

On 5/5/19 9:51 AM, Willy Tarreau wrote:

On Fri, May 03, 2019 at 07:27:48PM +0200, Frederic Lecaille wrote:

-  - env VTEST_PROGRAM=../vtest/vtest make reg-tests
+  - env TMPDIR=~/tmp VTEST_PROGRAM=../vtest/vtest make reg-tests


That may sound like a stupid question, but what makes you think that
~/tmp will actually result in a smaller path ? I mean ~ might very
well be "/home/automated-builds/travis/version-1.2.3.4/haproxy" on
the target system. Thus I think we're trying random locations until
it works :-/


I had no access to any travis environment when it has been told in 
previous mails that /tmp could not work, and /var/tmp could not either.

They were the first tested values.

Now that I have setup a travis account, I have just double checked, and 
contrary to what has been previously told, /tmp value for TMPDIR may 
make the reg tests work on OS X with travis.


So here is attached to this mail a better fix.


Fred.
>From dd2828071bc36375a485a7ab7d6d601d9ce729be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= 
Date: Fri, 3 May 2019 19:16:02 +0200
Subject: [PATCH] BUILD: travis: TMPDIR replacement.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

TMPDIR default value may be too long to create UNIX sockets for the stats
used during the reg tests. Indeed vtest builds its temporary working directory
${tmpdir} variable from TMPDIR variable, with /tmp as value if not already set.
This is the case on Linux contrary to OS X which sets TMPDIR with a too much long
value.

With this path we revert the part of 88c63a6 commit which tried to shorten this
TMPDIR value modifying script/run-regtests.sh. Unfortunately this was not
sufficient. Furthermore this patch force TMPDIR to /tmp value for all the OS'es.

Thank you to Tim Düsterhus and Ilya for having helped on this issue.
---
 .travis.yml | 8 +---
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/.travis.yml b/.travis.yml
index f689fe982..be3207113 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -27,16 +27,10 @@ install:
   # Special flags due to: https://github.com/vtest/VTest/issues/12
   - make -C ../vtest FLAGS="-O2 -s -Wall"
 
-before_script:
-  # This is a fix for the super long TMPDIR on Mac making
-  # the unix socket path names exceed the maximum allowed
-  # length.
-  - sed -i'.original' '/TESTDIR=.*haregtests/s/haregtests-.*XX/regtest.XXX/' scripts/run-regtests.sh
-
 script:
   - make CC=$CC V=1 TARGET=$TARGET $FLAGS
   - ./haproxy -vv
-  - env VTEST_PROGRAM=../vtest/vtest make reg-tests
+  - env TMPDIR=/tmp VTEST_PROGRAM=../vtest/vtest make reg-tests
 
 after_failure:
   - |
-- 
2.11.0



recent LibreSSL regressions

2019-05-05 Thread Илья Шипицин
hello,

when I first sent LibreSSL patches (it was 27th April 2019), reg-tests were
ok.
I suspect recent 0RTT patches could break LibreSSL things

can someone have a look ?

https://travis-ci.org/chipitsine/haproxy-1/builds/528535120

thanks!
Ilya Shipitcin


Re: [PR] BUILD: extend travis-ci matrix

2019-05-05 Thread Илья Шипицин
I made another PR

https://github.com/haproxy/haproxy/pull/92

(I really like automatic PR to mailing list routing)

LibreSSL builds were ok when I sent a patch for the first time. seem there
are some regression.
actually, patch is almost identical (I rebased it to current master)


вс, 5 мая 2019 г. в 23:29, Илья Шипицин :

> something has changed. build is passing now, reg-tests fail (they used to
> work earlier)
>
>
> I'd apply those patches (and we will fix reg-tests later)
>
> вс, 5 мая 2019 г. в 15:51, Willy Tarreau :
>
>> On Sun, May 05, 2019 at 02:26:11PM +0500,  ??? wrote:
>> > can we also apply patch from
>> > https://www.mail-archive.com/haproxy@formilux.org/msg33439.html ?
>> > it should repair libressl builds
>>
>> Ah indeed I remember having noticed this one and postponed it because it
>> needed to be edited to have a commit message and I didn't have the time
>> to study it further. Could you please provide a commit message indicating
>> what problem it fixes, how/when this problem manifests itself and how the
>> patch fixes it ?
>>
>> Thanks!
>> willy
>>
>


Re: clang address sanitizer findings

2019-05-05 Thread Willy Tarreau
On Sun, May 05, 2019 at 08:49:10PM +0200, Willy Tarreau wrote:
> Bingo! Alignment was forced to 2^5 when using the sanitizer, which
> causes it not only to detect issues, but may even cause some crashes
> upon startup when trying to dereference padding as function pointers.
> 
> You may want to try to apply the following change, though I'm not much
> convinced :
(...)

OK no need to waste your time on this one, I could reproduce the problem
and it's indeed when clang or gcc uses -fsanitize=address tht this
problem arises. And haproxy cannot even detect it since the size of
the symbols themselves doesn't include the padding. So in this case you
need to switch to USE_OBSOLETE_LINKER=1.

Willy



Re: clang address sanitizer findings

2019-05-05 Thread Willy Tarreau
On Sun, May 05, 2019 at 11:36:46PM +0500,  ??? wrote:
> with sanitizer:
>  27 init_STG_POOL 0800  00afb3a0  00afb3a0  006fa3a0
> 2**5
>   CONTENTS, ALLOC, LOAD, DATA
>  28 init_STG_LOCK 02c0  00afbba0  00afbba0  006faba0
> 2**5
>   CONTENTS, ALLOC, LOAD, DATA
>  29 init_STG_REGISTER 23c0  00afbe60  00afbe60
> 006fae60  2**5
>   CONTENTS, ALLOC, LOAD, DATA
>  30 init_STG_PREPARE 0100  00afe220  00afe220
> 006fd220  2**5
>   CONTENTS, ALLOC, LOAD, DATA
(...)

> without sanitizer:
>  25 init_STG_POOL 0100  005c3090  005c3090  001c2090
> 2**3
>   CONTENTS, ALLOC, LOAD, DATA
>  26 init_STG_LOCK 0058  005c3190  005c3190  001c2190
> 2**3
>   CONTENTS, ALLOC, LOAD, DATA
>  27 init_STG_REGISTER 0478  005c31e8  005c31e8
> 001c21e8  2**3
>   CONTENTS, ALLOC, LOAD, DATA
>  28 init_STG_PREPARE 0020  005c3660  005c3660
> 001c2660  2**3


Bingo! Alignment was forced to 2^5 when using the sanitizer, which
causes it not only to detect issues, but may even cause some crashes
upon startup when trying to dereference padding as function pointers.

You may want to try to apply the following change, though I'm not much
convinced :

diff --git a/include/common/initcall.h b/include/common/initcall.h
index 6da752c..f19d91c 100644
--- a/include/common/initcall.h
+++ b/include/common/initcall.h
@@ -104,7 +104,7 @@ struct initcall {
 __GLOBL(__start_init_##stg );  \
__GLOBL(__stop_init_##stg );   \
static const struct initcall *__initcb_##linenum   \
-   __attribute__((__used__,HA_SECTION(stg))) =\
+   __attribute__((__used__,aligned(8),HA_SECTION(stg))) = \
(stg < STG_SIZE) ? &(const struct initcall) {  \
.fct = (void (*)(void *,void *,void *))function,   \
.arg1 = (void *)(a1),  \

Otherwise you may simply prefer to build with USE_OBSOLETE_LINKER=1
when building with the address sanitizer as this one will make use of
larger linked lists but will be insensitive to alignment.

Cheers,
willy



Re: clang address sanitizer findings

2019-05-05 Thread Илья Шипицин
with sanitizer:

$ objdump -h haproxy

haproxy: file format elf64-x86-64

Sections:
Idx Name  Size  VMA   LMA   File off
Algn
  0 .interp   001c  004002e0  004002e0  02e0
2**0
  CONTENTS, ALLOC, LOAD, READONLY, DATA
  1 .note.ABI-tag 0020  004002fc  004002fc  02fc
2**2
  CONTENTS, ALLOC, LOAD, READONLY, DATA
  2 .note.gnu.build-id 0024  0040031c  0040031c
031c  2**2
  CONTENTS, ALLOC, LOAD, READONLY, DATA
  3 .gnu.hash 62b4  00400340  00400340  0340
2**3
  CONTENTS, ALLOC, LOAD, READONLY, DATA
  4 .dynsym   00015918  004065f8  004065f8  65f8
2**3
  CONTENTS, ALLOC, LOAD, READONLY, DATA
  5 .dynstr   00012388  0041bf10  0041bf10  0001bf10
2**0
  CONTENTS, ALLOC, LOAD, READONLY, DATA
  6 .gnu.version  1cc2  0042e298  0042e298  0002e298
2**1
  CONTENTS, ALLOC, LOAD, READONLY, DATA
  7 .gnu.version_r 01d0  0042ff60  0042ff60  0002ff60
2**3
  CONTENTS, ALLOC, LOAD, READONLY, DATA
  8 .rela.dyn 03f0  00430130  00430130  00030130
2**3
  CONTENTS, ALLOC, LOAD, READONLY, DATA
  9 .rela.plt 2940  00430520  00430520  00030520
2**3
  CONTENTS, ALLOC, LOAD, READONLY, DATA
 10 .init 001b  00433000  00433000  00033000
2**2
  CONTENTS, ALLOC, LOAD, READONLY, CODE
 11 .plt  1b90  00433020  00433020  00033020
2**4
  CONTENTS, ALLOC, LOAD, READONLY, CODE
 12 .text 00531882  00434bb0  00434bb0  00034bb0
2**4
  CONTENTS, ALLOC, LOAD, READONLY, CODE
 13 .fini 000d  00966434  00966434  00566434
2**2
  CONTENTS, ALLOC, LOAD, READONLY, CODE
 14 .rodata   000cd406  00967000  00967000  00567000
2**5
  CONTENTS, ALLOC, LOAD, READONLY, DATA
 15 .eh_frame_hdr a06c  00a34408  00a34408  00634408
2**2
  CONTENTS, ALLOC, LOAD, READONLY, DATA
 16 .eh_frame 00036ec8  00a3e478  00a3e478  0063e478
2**3
  CONTENTS, ALLOC, LOAD, READONLY, DATA
 17 .tdata0080  00a77110  00a77110  00676110
2**4
  CONTENTS, ALLOC, LOAD, DATA, THREAD_LOCAL
 18 .tbss a854  00a77190  00a77190  00676190
2**4
  ALLOC, THREAD_LOCAL
 19 .preinit_array 0008  00a77190  00a77190  00676190
2**3
  CONTENTS, ALLOC, LOAD, DATA
 20 .init_array   03a8  00a77198  00a77198  00676198
2**3
  CONTENTS, ALLOC, LOAD, DATA
 21 .fini_array   02f8  00a77540  00a77540  00676540
2**3
  CONTENTS, ALLOC, LOAD, DATA
 22 .data.rel.ro  03a8  00a77840  00a77840  00676840
2**5
  CONTENTS, ALLOC, LOAD, DATA
 23 .dynamic  02b0  00a77be8  00a77be8  00676be8
2**3
  CONTENTS, ALLOC, LOAD, DATA
 24 .got  0150  00a77e98  00a77e98  00676e98
2**3
  CONTENTS, ALLOC, LOAD, DATA
 25 .got.plt  0dd8  00a78000  00a78000  00677000
2**3
  CONTENTS, ALLOC, LOAD, DATA
 26 .data 000825c0  00a78de0  00a78de0  00677de0
2**5
  CONTENTS, ALLOC, LOAD, DATA
 27 init_STG_POOL 0800  00afb3a0  00afb3a0  006fa3a0
2**5
  CONTENTS, ALLOC, LOAD, DATA
 28 init_STG_LOCK 02c0  00afbba0  00afbba0  006faba0
2**5
  CONTENTS, ALLOC, LOAD, DATA
 29 init_STG_REGISTER 23c0  00afbe60  00afbe60
006fae60  2**5
  CONTENTS, ALLOC, LOAD, DATA
 30 init_STG_PREPARE 0100  00afe220  00afe220
006fd220  2**5
  CONTENTS, ALLOC, LOAD, DATA
 31 .bss  009c6990  00afe340  00afe340  006fd320
2**6
  ALLOC
 32 .comment  0087      006fd320
2**0
  CONTENTS, READONLY
 33 .gnu.build.attributes 00038af4  014c6cd0  014c6cd0
006fd3a8  2**2
  CONTENTS, READONLY
 34 .debug_info   002b4bf1      00735e9c
2**0
  CONTENTS, READONLY, DEBUGGING
 35 .debug_abbrev 000142b8      009eaa8d
2**0
  CONTENTS, READONLY, DEBUGGING
 36 .debug_line   001bcd73      009fed45
2**0
  CONTENTS, READONLY, DEBUGGING
 37 .debug_str0001e422      00bbbab8
2**0
   

Re: [PR] BUILD: extend travis-ci matrix

2019-05-05 Thread Илья Шипицин
something has changed. build is passing now, reg-tests fail (they used to
work earlier)


I'd apply those patches (and we will fix reg-tests later)

вс, 5 мая 2019 г. в 15:51, Willy Tarreau :

> On Sun, May 05, 2019 at 02:26:11PM +0500,  ??? wrote:
> > can we also apply patch from
> > https://www.mail-archive.com/haproxy@formilux.org/msg33439.html ?
> > it should repair libressl builds
>
> Ah indeed I remember having noticed this one and postponed it because it
> needed to be edited to have a commit message and I didn't have the time
> to study it further. Could you please provide a commit message indicating
> what problem it fixes, how/when this problem manifests itself and how the
> patch fixes it ?
>
> Thanks!
> willy
>
From f59ae0892b1ec660ec62e1284048b004d6bc995a Mon Sep 17 00:00:00 2001
From: Ilya Shipitsin 
Date: Sun, 5 May 2019 23:27:54 +0500
Subject: [PATCH 3/3] BUILD: enable several LibreSSL hacks, including

SSL_SESSION_get0_id_context is introduced in LibreSSL-2.7.0
async operations are not supported by LibreSSL
early data is not supported by LibreSSL
packet_length is removed from SSL struct in LibreSSL
---
 include/proto/openssl-compat.h |  4 ++--
 include/proto/ssl_sock.h   |  2 +-
 src/cli.c  |  2 +-
 src/ssl_sock.c | 44 +-
 4 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/include/proto/openssl-compat.h b/include/proto/openssl-compat.h
index ffee2e40..ed5c1ba3 100644
--- a/include/proto/openssl-compat.h
+++ b/include/proto/openssl-compat.h
@@ -89,9 +89,9 @@ static inline int SSL_SESSION_set1_id_context(SSL_SESSION *s, const unsigned cha
 }
 #endif
 
-#if (OPENSSL_VERSION_NUMBER < 0x101fL) || defined(LIBRESSL_VERSION_NUMBER)
+#if (OPENSSL_VERSION_NUMBER < 0x101fL) || (defined(LIBRESSL_VERSION_NUMBER) && (LIBRESSL_VERSION_NUMBER < 0x207fL))
 /*
- * Functions introduced in OpenSSL 1.1.0 and not yet present in LibreSSL
+ * Functions introduced in OpenSSL 1.1.0 and in LibreSSL 2.7.0
  */
 
 static inline const unsigned char *SSL_SESSION_get0_id_context(const SSL_SESSION *sess, unsigned int *sid_ctx_length)
diff --git a/include/proto/ssl_sock.h b/include/proto/ssl_sock.h
index ce52fb74..586ebb90 100644
--- a/include/proto/ssl_sock.h
+++ b/include/proto/ssl_sock.h
@@ -85,7 +85,7 @@ SSL_CTX *ssl_sock_get_generated_cert(unsigned int key, struct bind_conf *bind_co
 int ssl_sock_set_generated_cert(SSL_CTX *ctx, unsigned int key, struct bind_conf *bind_conf);
 unsigned int ssl_sock_generated_cert_key(const void *data, size_t len);
 
-#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC)
+#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC) && !defined(LIBRESSL_VERSION_NUMBER)
 void ssl_async_fd_handler(int fd);
 void ssl_async_fd_free(int fd);
 #endif
diff --git a/src/cli.c b/src/cli.c
index 88fbae33..e91e33b3 100644
--- a/src/cli.c
+++ b/src/cli.c
@@ -1002,7 +1002,7 @@ static int cli_io_handler_show_fd(struct appctx *appctx)
 			 (fdt.iocb == poller_pipe_io_handler) ? "poller_pipe_io_handler" :
 			 (fdt.iocb == mworker_accept_wrapper) ? "mworker_accept_wrapper" :
 #ifdef USE_OPENSSL
-#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC)
+#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC) && !defined(LIBRESSL_VERSION_NUMBER)
 			 (fdt.iocb == ssl_async_fd_free) ? "ssl_async_fd_free" :
 			 (fdt.iocb == ssl_async_fd_handler) ? "ssl_async_fd_handler" :
 #endif
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index f2d80e8c..e11ddb53 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -57,7 +57,7 @@
 #include 
 #endif
 
-#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC)
+#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC) && !defined(LIBRESSL_VERSION_NUMBER)
 #include 
 #endif
 
@@ -575,7 +575,7 @@ fail_get:
 }
 #endif
 
-#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC)
+#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC) && !defined(LIBRESSL_VERSION_NUMBER)
 /*
  * openssl async fd handler
  */
@@ -2297,7 +2297,7 @@ static void ssl_sock_switchctx_set(SSL *ssl, SSL_CTX *ctx)
 	SSL_set_SSL_CTX(ssl, ctx);
 }
 
-#if (OPENSSL_VERSION_NUMBER >= 0x10101000L) || defined(OPENSSL_IS_BORINGSSL)
+#if ((OPENSSL_VERSION_NUMBER >= 0x10101000L) || defined(OPENSSL_IS_BORINGSSL)) && !defined(LIBRESSL_VERSION_NUMBER)
 
 static int ssl_sock_switchctx_err_cbk(SSL *ssl, int *al, void *priv)
 {
@@ -4029,7 +4029,7 @@ ssl_sock_initial_ctx(struct bind_conf *bind_conf)
 
 	SSL_CTX_set_options(ctx, options);
 
-#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC)
+#if (OPENSSL_VERSION_NUMBER >= 0x101fL) && !defined(OPENSSL_NO_ASYNC) && !defined(LIBRESSL_VERSION_NUMBER)
 	if (global_ssl.async)
 		mode |= SSL_MODE_ASYNC;
 #endif
@@ -4041,7 +4041,7 @@ ssl_sock_initial_ctx(struct bind_conf *bind_conf)
 #ifdef 

Re: Zero RTT in backend server side

2019-05-05 Thread Igor Pav
Hi, Olivier, thanks for the effort. So can we force the server always
to carry data to remote via 0RTT like below scenario(to protect
http2http in unsecured env)?

listen http -- server default x.x ssl allow-0rtt (SSL) bind
x.x ssl allow-0rtt -- http backend

On Sat, May 4, 2019 at 3:06 AM Olivier Houchard  wrote:
>
> Hi Igor,
>
> On Fri, May 03, 2019 at 05:21:50PM +0800, Igor Pav wrote:
> > Just tested with openssl 1.1.1b and haproxy 1.9.7, it appears no
> > success, you are right :)
> >
>
> Indeed :)
> I just pushed commit 010941f87605e8219d25becdbc652350a687d6a2 to master, that
> let me do 0RTT both as server and as client. This should be backported to
> 1.8 and 1.9 soon.
> Please note, however, that we will only attempt to connect to a server
> using 0RTT if the client did so, as we have to be sure the client support it,
> in case it receives a 425.
> This may change in 2.0, if we add the ability to retry failed requests.
>
> Regards,
>
> Olivier



Re: [PATCH] MINOR: systemd: Make use of master socket in systemd unit

2019-05-05 Thread Tim Düsterhus
William,

Am 26.04.19 um 20:30 schrieb Tim Düsterhus:
> William,
> 
> Am 26.04.19 um 14:56 schrieb William Lallemand:
>> On Fri, Apr 26, 2019 at 12:15:37AM +0200, Tim Duesterhus wrote:
>>>  [Service]
>>> -Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid"
>>> +Environment="CONFIG=/etc/haproxy/haproxy.cfg" "PIDFILE=/run/haproxy.pid" 
>>> "MASTER_SOCKET=/run/haproxy-master.sock"
>>>  ExecStartPre=@SBINDIR@/haproxy -f $CONFIG -c -q
>>> -ExecStart=@SBINDIR@/haproxy -Ws -f $CONFIG -p $PIDFILE
>>> +ExecStart=@SBINDIR@/haproxy -Ws -S $MASTER_SOCKET -f $CONFIG -p $PIDFILE
>>>  ExecReload=@SBINDIR@/haproxy -f $CONFIG -c -q
>>
>> In my opinion that's not a good idea to do it this way, because you can't
>> disable the master CLI by unsetting the MASTER_SOCKET variable.
>>
>> It's probably better to have "-S /run/haproxy-master.sock" in an $OPTIONS
>> variable at the end of the ExecStart line.
>>
> 
> I'm not sure whether this is better. Yes you can disable the socket more
> easily then, but you have to remember to add it back when editing the
> 'OPTIONS' variable.
> 
> I believe it boils to to this question: Does the user usually want to
> run with the socket enabled or not? My guess is that most users want to
> have this socket (having is better than needing!), they might just want
> to move it to a different location rather than outright disabling it.
> During my tests it was only accessible to root, so there does not appear
> a security issue in the default configuration either.
> 
> On an unrelated note: Debian patches the unit file to add in such a
> variable, called 'EXTRAOPTS':
> https://salsa.debian.org/haproxy-team/haproxy/blob/master/debian/patches/haproxy.service-use-environment-variables.patch
> 
> So if we want to go the 'OPTIONS' variable path we might want to re-use
> that variable name. Any change will break their patch anyway so they
> definitely notice.
> 
> Best regards
> Tim Düsterhus
> 

I'd like to bump my reply to you once more. As outlined previously I
believe that the "default" expectation is having the socket, it can
still be disabled by overriding the ExecStart= declaration.

Best regards
Tim Düsterhus



Re: clang address sanitizer findings

2019-05-05 Thread Willy Tarreau
On Sun, May 05, 2019 at 03:04:22PM +0500,  ??? wrote:
> Hello,
> 
> I run fedora 30, it includes clang-8, I built haproxy using
> 
> make CC=clang V=1 TARGET=$TARGET $FLAGS DEBUG_CFLAGS="-fsanitize=address
> -ggdb" LDFLAGS="-fsanitize=address"
> 
> when running reg-tests, the following is caught
> 
> ==6340==ERROR: AddressSanitizer: global-buffer-overflow on address
> 0x00ab61c8 at pc 0x007360f5 bp 0x7ffc56ce9f90 sp 0x7ffc56ce9f88
> READ of size 8 at 0x00ab61c8 thread T0
> #0 0x7360f4  (/home/ilia/haproxy/haproxy+0x7360f4)
> #1 0x7f3952660f32  (/lib64/libc.so.6+0x23f32)
> #2 0x434b7d  (/home/ilia/haproxy/haproxy+0x434b7d)
> 
> 0x00ab61c8 is located 56 bytes to the left of global variable
> '__initcb_486' defined in 'src/task.c:486:1' (0xab6200) of size 8
> 0x00ab61c8 is located 0 bytes to the right of global variable
> '__initcb_1865' defined in 'src/log.c:1865:1' (0xab61c0) of size 8
> SUMMARY: AddressSanitizer: global-buffer-overflow
> (/home/ilia/haproxy/haproxy+0x7360f4)

This one is quite strange, it looks as if the linker had intentionally
left holes in the init_* sections by aligning each pointer on 64 bytes.
Maybe this is an artefact of using -fsanitize=address, though it seems
a bit unlikely.

Could you please run "objdump -h" on your haproxy executable with and
without this build option ? I suspect we'll see "2**6" at the end of
some init_* columns at least in one case. If so we may try to add
"aligned(8)" or even "packed" to the attributes when declaring the
initcalls.

Thanks,
Willy



Re: [PR] BUILD: extend travis-ci matrix

2019-05-05 Thread Willy Tarreau
On Sun, May 05, 2019 at 02:26:11PM +0500,  ??? wrote:
> can we also apply patch from
> https://www.mail-archive.com/haproxy@formilux.org/msg33439.html ?
> it should repair libressl builds

Ah indeed I remember having noticed this one and postponed it because it
needed to be edited to have a commit message and I didn't have the time
to study it further. Could you please provide a commit message indicating
what problem it fixes, how/when this problem manifests itself and how the
patch fixes it ?

Thanks!
willy



clang address sanitizer findings

2019-05-05 Thread Илья Шипицин
Hello,

I run fedora 30, it includes clang-8, I built haproxy using

make CC=clang V=1 TARGET=$TARGET $FLAGS DEBUG_CFLAGS="-fsanitize=address
-ggdb" LDFLAGS="-fsanitize=address"

when running reg-tests, the following is caught

==6340==ERROR: AddressSanitizer: global-buffer-overflow on address
0x00ab61c8 at pc 0x007360f5 bp 0x7ffc56ce9f90 sp 0x7ffc56ce9f88
READ of size 8 at 0x00ab61c8 thread T0
#0 0x7360f4  (/home/ilia/haproxy/haproxy+0x7360f4)
#1 0x7f3952660f32  (/lib64/libc.so.6+0x23f32)
#2 0x434b7d  (/home/ilia/haproxy/haproxy+0x434b7d)

0x00ab61c8 is located 56 bytes to the left of global variable
'__initcb_486' defined in 'src/task.c:486:1' (0xab6200) of size 8
0x00ab61c8 is located 0 bytes to the right of global variable
'__initcb_1865' defined in 'src/log.c:1865:1' (0xab61c0) of size 8
SUMMARY: AddressSanitizer: global-buffer-overflow
(/home/ilia/haproxy/haproxy+0x7360f4)


Re: [PR] BUILD: extend travis-ci matrix

2019-05-05 Thread Илья Шипицин
can we also apply patch from
https://www.mail-archive.com/haproxy@formilux.org/msg33439.html ?
it should repair libressl builds

вс, 5 мая 2019 г. в 13:25, Willy Tarreau :

> On Fri, May 03, 2019 at 10:23:05AM +, PR Bot wrote:
> > Description:
> >added openssl-1.0.2, 1.1.0, 1.1.1, libressl-2.7.5, 2.8.3, 2.9.1
> >added linux-ppc64le image
>
> Applied, thanks Ilya,
> Willy
>


Re: findings of gcc address sanitizer

2019-05-05 Thread Илья Шипицин
nice, I will add sanitizers to travis-ci matrix

вс, 5 мая 2019 г. в 13:18, Willy Tarreau :

> Hi Ilya,
>
> On Fri, May 03, 2019 at 06:58:19PM +0500,  ??? wrote:
> > ***  h10.1 debug|#0 0x6db986 in update_log_hdr src/log.c:1399
> (...)
> > ***  h10.1 debug|previously allocated by thread T0 here:
> > ***  h10.1 debug|#0 0x7f8ebd15c5de in realloc
> > (/lib64/libasan.so.5+0x10e5de)
> > ***  h10.1 debug|#1 0x6dbd31 in my_realloc2
> > include/common/standard.h:1432
> > ***  h10.1 debug|#2 0x6dbd31 in init_log_buffers src/log.c:1880
> > ***  h10.1 debug|
> > ***  h10.1 debug|SUMMARY: AddressSanitizer: heap-use-after-free
> > src/log.c:1399 in update_log_hdr
>
> Nice catch, even affects 1.8. Now fixed.
>
> > ***  h10.1 debug|==23684==ERROR: LeakSanitizer: detected memory leaks
> > ***  h10.1 debug|
> > ***  h10.1 debug|Direct leak of 24 byte(s) in 1 object(s) allocated
> > from:
> > ***  h10.1 debug|#0 0x7f9ac626f1a8 in __interceptor_malloc
> > (/lib64/libasan.so.5+0x10e1a8)
> > ***  h10.1 debug|#1 0x7f9ac6076b1b
> (/lib64/libssl.so.1.1+0x33b1b)
> > ***  h10.1 debug|
> > ***  h10.1 debug|SUMMARY: AddressSanitizer: 24 byte(s) leaked in 1
> > allocation(s).
>
> For this one I suspect that one object allocated by openssl was expected
> to be freed by us. I have no idea what this is though.
>
> Thanks!
> Willy
>


Re: haproxy 1.9.7 http mode error

2019-05-05 Thread Willy Tarreau
Hi Mikhail,

On Thu, May 02, 2019 at 03:41:32PM +0300, Mikhail Golub wrote:
> Hi.
> 
> After upgrade haproxy to version 1.9.7 from FreeBSD port i give error ... in 
> reverse proxy for MS Exchange.
> ActiveSync, OWA - works.
> MAPI stop working 
> (https://docs.microsoft.com/en-us/exchange/clients/mapi-over-http/mapi-over-http?view=exchserver-2019)
> Rollback to previous version of haproxy solve a problem.

Oh that's not good. Can you describe a little bit more "stop working" ?
Are your server still seen as "up" or do the checks fail ? What do you
see in the logs requests sent to these servers ?

Thanks,
Willy



Re: [ANNOUNCE] haproxy-1.8.20

2019-05-05 Thread Willy Tarreau
On Sun, May 05, 2019 at 09:51:11AM +0200, Vincent Bernat wrote:
>  ?  5 mai 2019 09:14 +02, Willy Tarreau :
> 
> > So I'd suggest to insist on having the up to date version (even 1.8.21 if
> > we have a reason to have this one released by then). In the worst case,
> > if this is rejected for whatever reason, it's better to leave a well known
> > version there and continue to encourage every user to switch to your up to
> > date PPA than making them believe their version contains the essential
> > fixes, which would be misleading.
> 
> OK, I have just asked our release team if they would accept 1.8.20 as
> is. Let's see!

Thank you,
Willy



Re: [PR] BUILD: extend travis-ci matrix

2019-05-05 Thread Willy Tarreau
On Fri, May 03, 2019 at 10:23:05AM +, PR Bot wrote:
> Description:
>added openssl-1.0.2, 1.1.0, 1.1.1, libressl-2.7.5, 2.8.3, 2.9.1
>added linux-ppc64le image

Applied, thanks Ilya,
Willy



Re: findings of gcc address sanitizer

2019-05-05 Thread Willy Tarreau
Hi Ilya,

On Fri, May 03, 2019 at 06:58:19PM +0500,  ??? wrote:
> ***  h10.1 debug|#0 0x6db986 in update_log_hdr src/log.c:1399
(...)
> ***  h10.1 debug|previously allocated by thread T0 here:
> ***  h10.1 debug|#0 0x7f8ebd15c5de in realloc
> (/lib64/libasan.so.5+0x10e5de)
> ***  h10.1 debug|#1 0x6dbd31 in my_realloc2
> include/common/standard.h:1432
> ***  h10.1 debug|#2 0x6dbd31 in init_log_buffers src/log.c:1880
> ***  h10.1 debug|
> ***  h10.1 debug|SUMMARY: AddressSanitizer: heap-use-after-free
> src/log.c:1399 in update_log_hdr

Nice catch, even affects 1.8. Now fixed.

> ***  h10.1 debug|==23684==ERROR: LeakSanitizer: detected memory leaks
> ***  h10.1 debug|
> ***  h10.1 debug|Direct leak of 24 byte(s) in 1 object(s) allocated
> from:
> ***  h10.1 debug|#0 0x7f9ac626f1a8 in __interceptor_malloc
> (/lib64/libasan.so.5+0x10e1a8)
> ***  h10.1 debug|#1 0x7f9ac6076b1b  (/lib64/libssl.so.1.1+0x33b1b)
> ***  h10.1 debug|
> ***  h10.1 debug|SUMMARY: AddressSanitizer: 24 byte(s) leaked in 1
> allocation(s).

For this one I suspect that one object allocated by openssl was expected
to be freed by us. I have no idea what this is though.

Thanks!
Willy



Re: reg-tests are broken when running osx + openssl

2019-05-05 Thread Willy Tarreau
On Fri, May 03, 2019 at 07:27:48PM +0200, Frederic Lecaille wrote:
> -  - env VTEST_PROGRAM=../vtest/vtest make reg-tests
> +  - env TMPDIR=~/tmp VTEST_PROGRAM=../vtest/vtest make reg-tests

That may sound like a stupid question, but what makes you think that
~/tmp will actually result in a smaller path ? I mean ~ might very
well be "/home/automated-builds/travis/version-1.2.3.4/haproxy" on
the target system. Thus I think we're trying random locations until
it works :-/

Theorically /tmp is always shorter. Now if /tmp doesn't work for
whatever reason we may end up having no other choice, but if so it
was not clear in this thread and further is not mentioned in the
commit message, which leaves me with a big doubt.

Willy



Re: [ANNOUNCE] haproxy-1.8.20

2019-05-05 Thread Vincent Bernat
 ❦  5 mai 2019 09:14 +02, Willy Tarreau :

> So I'd suggest to insist on having the up to date version (even 1.8.21 if
> we have a reason to have this one released by then). In the worst case,
> if this is rejected for whatever reason, it's better to leave a well known
> version there and continue to encourage every user to switch to your up to
> date PPA than making them believe their version contains the essential
> fixes, which would be misleading.

OK, I have just asked our release team if they would accept 1.8.20 as
is. Let's see!
-- 
Replace repetitive expressions by calls to a common function.
- The Elements of Programming Style (Kernighan & Plauger)



Re: HAProxy 1.9.6 unresponsive

2019-05-05 Thread Willy Tarreau
Hi Patrick,

On Fri, May 03, 2019 at 04:33:07PM -0400, Patrick Hemmer wrote:
> We are running HAProxy 1.9.6 and managed to get into a state where HAProxy
> was completely unresponsive. It was pegged at 100% like many of the other
> experiences here on the mailing list lately. But in addition it wouldn't
> respond to anything. The stats socket wasn't even responsive.
> 
> When I attached an strace, it sat there with no activity. When I attached
> GDB I got the following stack:
(...)
> Our config is big and complex, and not something I want to post here (I may
> be able to provide directly if required). However I think the important bit
> is that we we have a frontend and backend which are used for load balancing
> gRPC traffic (thus h2). The backend servers are h2c (no SSL).
(...)

Function h2s_htx_make_trailers() is called in loops here, and I see no way
this function can return without consuming the block, marking an error or
indicating that it's blocked. Thus I suspect this one could be a consequence
of the bug fixed by commit 9a0f559 ("BUG/MEDIUM: h2: Make sure we're not
already in the send_list in h2_subscribe().") which was backported into
1.9.7. Do not rush an upgrade though, I'm going to issue 1.9.8 soon with
a few more fixes.

With this said, after studying the code a little bit more, I'm seeing a
potential case where if we'd have a trailers entry in the HTX buffer but
no end of message, we could loop forever there not consuming this block.
I have no idea if this is possible in an HTX message, I'll ask Christopher
tomorrow. In any case we need to address this one way or another, possibly
reporting an error instead if required. Thus I'm postponing 1.9.8 for
tomorrow.

> The service has been restarted, so it cannot be probed any more. However I
> did capture a core file before doing so.

That might actually be useful to study the sequence of HTX messages there.
I may ask you to dig a little bit into it.

Thanks!
Willy



Re: [ANNOUNCE] haproxy-1.8.20

2019-05-05 Thread Willy Tarreau
Hi Vincent!

On Sat, May 04, 2019 at 01:57:06PM +0200, Vincent Bernat wrote:
>  ? 29 avril 2019 11:04 +02, Christopher Faulet :
> 
> > HAProxy 1.8.20 was released on 2019/04/25. It added 48 new commits
> > after version 1.8.19.
> 
> Hey!
> 
> Debian Buster will soon be released (nobody knows exactly when, but we
> are in full freeze since 2 months). It currently contains HAProxy
> 1.8.19. I don't think it would be possible to push 1.8.20 as is.
> 
> We can either keep 1.8.19 as is or select a few critical patches to
> apply on it. For example, we could take the MAJOR patches:
> 
> BUG/MAJOR: checks: segfault during tcpcheck_main
> BUG/MAJOR: http_fetch: Get the channel depending on the keyword used
> BUG/MAJOR: listener: Make sure the listener exist before using it.
> BUG/MAJOR: spoe: Fix initialization of thread-dependent fields
> BUG/MAJOR: stats: Fix how huge POST data are read from the channel
> 
> And maybe:
> 
> BUG/MEDIUM: listener: make sure the listener never accepts too many conns
> 
> The more changes, the less likely the release team will accept the
> change. Assuming we can only make one proposition (which is not true),
> what would you (as upstream) try? 1.8.19, one bug, all major bugs, even
> more bugs, or 1.8.20?

As you know, I'm strongly opposed to cherry-picking only a small selection
of bug fixes that are supposedly more important than other ones in certain
environments while some users will only be affected by the ones not picked,
because it means that you will deliberately ship known bugs to your users
and put them at risk, which is really sad. For the vast majority of users,
a deadlock in production caused by an unlikely bug is way more serious
than a segfault which "cleanly" gets rid of the faulty process.

In addition no single branch exists showing that only a random selection
of patches works fine without the other ones, which means you can actually
end up with debian-specific regressions caused exclusively by this bad
practice.

So I'd suggest to insist on having the up to date version (even 1.8.21 if
we have a reason to have this one released by then). In the worst case,
if this is rejected for whatever reason, it's better to leave a well known
version there and continue to encourage every user to switch to your up to
date PPA than making them believe their version contains the essential
fixes, which would be misleading.

Just my two cents,
Willy