Re: Randomly high CPU usage

2019-05-05 Thread Marco Corte

Il 2019-04-18 18:33 Willy Tarreau ha scritto:

Hello Marco,

On Thu, Apr 18, 2019 at 05:27:26PM +0200, Marco Corte wrote:

Hello!

From time to time, about twice daily, and without any apparent reason,
haproxy jumps from using about 15% CPU usage to 100% (relative to the 
single

core it can use).
The situation becomes normal again after about 15-20 minutes.

(...)

Please keep this one until you can confirm the issue is gone, I may
have to ask it to you if it's still present.
(...)
Thanks!
Willy


Hello to the list!

I can confirm that version 1.9.7 does not show the behaviour any more.
In about one week I did not notice any CPU usage peak, and, more 
interestingly, the CPU usage appears lower on the average.


Thank you for the fix!

.marcoc



Re: v1.9.6 socket unresponsive with high cpu usage

2019-05-05 Thread William Dauchy
Hi Willy,

On Sun, May 05, 2019 at 07:07:21AM +0200, Willy Tarreau wrote:
> Hi William,
> > we got a similar issue with last v1.9.7+HEAD
> At first I thought you were again on a deadlock that I couldn't spot, due
> to the fact that nearly all threads were waiting on the LB lock, and I
> couldn't find how this could happen. But I didn't notice this one which
> is the most important :
> 
> > Thread 15 (Thread 0x7fe9b6631700 (LWP 2808)):
> > #0  0x56153d96d7a0 in __eb_insert_dup (new=0x56157f52f424, 
> > sub=0x56157f5640a4) at ebtree/ebtree.h:478
> > #1  eb_insert_dup (sub=, new=0x56157f52f424) at 
> > ebtree/ebtree.c:31
> > #2  0x56153d96df10 in __eb32_insert (new=new@entry=0x56157f52f424, 
> > root=, root@entry=0x56157deb4140) at ebtree/eb32tree.h:337
> > #3  eb32_insert (root=root@entry=0x56157deb4140, 
> > new=new@entry=0x56157f52f424) at ebtree/eb32tree.c:27
> > #4  0x56153d957fcb in fwrr_queue_srv (s=s@entry=0x56157f52f080) at 
> > src/lb_fwrr.c:371
> > #5  0x56153d9585e8 in fwrr_update_server_weight (srv=0x56157f52f080) at 
> > src/lb_fwrr.c:242
> > #6  0x56153d8ae8ac in srv_update_status (s=0x56157f52f080) at 
> > src/server.c:4923
> > #7  0x56153d8adfc2 in server_recalc_eweight 
> > (sv=sv@entry=0x56157f52f080, must_update=must_update@entry=1) at 
> > src/server.c:1310
> > #8  0x56153d8b6edd in server_warmup (t=0x5615899be8a0, 
> > context=0x56157f52f080, state=) at src/checks.c:1492
> > #9  0x56153d94d97a in process_runnable_tasks () at src/task.c:390
> > #10 0x56153d8c5c4f in run_poll_loop () at src/haproxy.c:2661
> > #11 run_thread_poll_loop (data=) at src/haproxy.c:2726
> > #12 0x7fe9bd5e7dd5 in start_thread () from /lib64/libpthread.so.0
> > #13 0x7fe9bc320ead in clone () from /lib64/libc.so.6
> 
> Thus I conclude that it crashed, and that all other threads just met at
> the same lock while the core was being dumped in this one. I figured what
> was missing, the server_warmup() function was missing a lock since 1.8.
> I've just fixed this and backported it to 1.9. I would be grateful if
> you could test it again, as I failed to reproduce the issue (it requires
> a high concurrency and bad luck, as often in such cases).

thank you very much for the patch. we are pushing it today.
http://git.haproxy.org/?p=haproxy-1.9.git;a=commit;h=207ba5a6bc1c03f2ba15ac3cd49bfa756fb760bb
for reference
I however don't know when I will be able to confirm as the issue was not
showing that often.

> Or maybe the tree got corrupted and __eb_insert_dup() entered an endless
> loop. If that's the case (I mean if it froze and didn't crash), I may
> have something to make this safer soon. I more or less managed to create
> a watchdog timer to detect lockups and abort the whole process with a
> trace when this happens. This will avoid keeping a faulty process in
> prod and may even allow a quicker restart. I don't intend to backport
> it to 1.9 though but depending on how effective and helpful it is, I
> could change my mind. In all cases I don't want to use such solutions
> to hide the dust under the carpet but instead to take detailed traces
> without requiring human intervention when this happens.

I like the idea :)
It would be nice for us to gather those info later!
-- 
William



Re: MEDIUM: Adding upstream socks4 proxy support

2019-05-05 Thread Willy Tarreau
Hi Alec,

On Tue, Apr 30, 2019 at 07:59:16PM +0800, Alec Liu wrote:
> > I'm seeing that you copied a doc retrieved from somewhere else that is
> > found at various places on the net. Have you checked the license for
> > this doc to be sure we can copy and distribute it like this ? It might
> > be perfectly fine but we need to be sure. In any case we'll have to
> > mention the license if it differs from the other ones.
> 
> I have the "SOCKS: A protocol for TCP proxy across firewalls" doc
> found at openssh's website
> under OpenSSH Sprcifications along with those RFCs.
> "https://www.openssh.com/specs.html";,
> "https://www.openssh.com/txt/socks4.protocol";
> And OpenSSH is using GPL, so I believe it should be fine.

OK. Please at least mention it in the commit message so that if anybody
complains, there is no doubt about the origin and we can even take
appropriate actions such as removing and replacing it with a link or
anything else. Similarly if this doc ever were updated, someone could
propose us some updates.

> > >  struct check_status {
> > > diff --git a/include/types/connection.h b/include/types/connection.h
> > > index 308be7d7..2a1b23b4 100644
> > > --- a/include/types/connection.h
> > > +++ b/include/types/connection.h
> > > @@ -47,6 +47,15 @@ struct server;
> > >  struct session;
> > >  struct pipe;
> > >
> > > +/* socks4 upstream proxy definitions */
> > > +struct socks4_request {
> > > + uint8_t version;/* SOCKS version number, 1 byte, must be 
> > > 0x04 for this version */
> > > + uint8_t command;/* 0x01 = establish a TCP/IP stream 
> > > connection */
> > > + uint16_t port;  /* port number, 2 bytes (in network byte 
> > > order) */
> > > + uint32_t ip;/* IP address, 4 bytes (in network byte 
> > > order) */
> > > + char user_id[8];/* the user ID string, variable length, 
> > > terminated with a null (0x00); Using "HAProxy\0" */
> > > +};
> > > +
> > >  /* Note: subscribing to these events is only valid after the caller has 
> > > really
> > >   * attempted to perform the operation, and failed to proceed or complete.
> > >   */
> > > @@ -156,7 +165,7 @@ enum {
> > >
> > >   CO_FL_EARLY_SSL_HS  = 0x4000,  /* We have early data pending, 
> > > don't start SSL handshake yet */
> > >   CO_FL_EARLY_DATA= 0x8000,  /* At least some of the data are 
> > > early data */
> > > - /* unused : 0x0001 */
> > > + CO_FL_SOCKS_HS  = 0x0001,  /* handshaking with upstream 
> > > SOCKS proxy */
> > >   /* unused : 0x0002 */
> >
> > From what I'm seeing later in the code, it's not exactly a handshake,
> > you have to send a header and to wait for the response header to arrive.
> > I'd rather have one flag per direction which save from having to deal
> > with state management.
> 
> Actually, I have the same idea as you, using two flags only. But ends up with
> this design later on. The reason I am doing it in this way, that's because:
> 1) I might want to add the socks5 protocol support onto it later on, which
> will become a little more complex, not just one request and response.
> We will get something like authentication method handshake, login,
> connection request, etc.

Then this is exactly what muxes are made for. They live their own life
and encapsulate application data in a way that is transparent for the
application layer. Ultimately we should be able to stack multiple such
transformations much easier than what it is today. The SSL layer is
currently being modified for this for example. I'd suggest that you
stick to two individual headers for SOCKS4 and that we see later how
SOCKS5 has to be done (if ever done by the way).

> 2) Looks like do not have much unused bits leftover within CO_FL_*.

That's not *your* problem but the one for the person having something
else to implement after you. Bits are like desserts in restaurants :
first come, first served.

> > >  /* possible connection error codes */
> > >  enum {
> > > @@ -451,6 +469,13 @@ struct connection {
> > >   struct {
> > >   struct sockaddr_storage from;   /* client address, or 
> > > address to spoof when connecting to the server */
> > >   struct sockaddr_storage to; /* address reached by the 
> > > client, or address to connect to */
> > > + struct {
> > > + unsigned char use;  /* socks4 
> > > proxy enabled  */
> > > + struct sockaddr_storage addr;   /* the 
> > > address of the socks4 proxy */
> > > + struct socks4_request req_line; /* the info 
> > > send to socks4 proxy */
> > > + unsigned int status;/* 
> > > CO_SOCKS_* */
> > > + signed short req_remain_len;/* the 
> > > length remain to sent */
> > > + } socks_proxy;
> >
> > It is extremely huge for the struct connection unfortunately. We're
> > curren

Re: haproxy 2.0 docker images

2019-05-05 Thread Aleksandar Lazic

Hi.

Any answer to the questions below?

Regards
 Aleks

Sat Apr 27 12:47:17 GMT+02:00 2019 Aleksandar Lazic :

> Hi.
>
>
> I have now created some HAProxy 2.0 images ;-).
>
> The outputs below raises some questions to me.
>
> * Should in the OPTIONS output also be the EXTRA_OBJS ?
> * Should PCRE2 be used instead of PCRE ?
> * Should PRIVATE_CACHE be used in the default build?
> * Should SLZ be used in the default build?
> * Make NS sense in a container image?
> * Can DEVICEATLAS 51DEGREES WURFL be used together?
>  - From technically point of view
>  - From license point of view
>
> Images:
> https://hub.docker.com/r/me2digital/haproxy20-centos
> https://hub.docker.com/r/me2digital/haproxy20-boringssl
>
> Build logs:
> https://gitlab.com/aleks001/haproxy20-centos/-/jobs/203092688
> https://gitlab.com/aleks001/haproxy20-boringssl/-/jobs/203110753
>
> haproxy -vv outputs:
>
> ```
> HA-Proxy version 2.0-dev2-5e6a5b-228 2019/04/25 - https://haproxy.org/
> Build options :
>  TARGET = linux2628
>  CPU = generic
>  CC = gcc
>  CFLAGS = -O2 -g -fno-strict-aliasing -Wdeclaration-after-statement -fwrapv
> -Wno-unused-label -Wno-sign-compare -Wno-unused-parameter
> -Wno-old-style-declaration -Wno-ignored-qualifiers -Wno-clobbered
> -Wno-missing-field-initializers -Wtype-limits
>  OPTIONS = USE_PCRE=1 USE_PCRE_JIT=1 USE_THREAD=1 USE_PTHREAD_PSHARED=1
> USE_REGPARM=1 USE_LINUX_SPLICE=1 USE_GETADDRINFO=1 USE_OPENSSL=1 USE_LUA=1
> USE_ZLIB=1 USE_TFO=1
>
> Feature list : +EPOLL -KQUEUE -MY_EPOLL -MY_SPLICE +NETFILTER +PCRE +PCRE_JIT
> -PCRE2 -PCRE2_JIT +POLL -PRIVATE_CACHE +THREAD +PTHREAD_PSHARED +REGPARM
> -STATIC_PCRE -STATIC_PCRE2 +TPROXY +LINUX_TPROXY +LINUX_SPLICE +LIBCRYPT
> +CRYPT_H -VSYSCALL +GETADDRINFO +OPENSSL +LUA +FUTEX +ACCEPT4 -MY_ACCEPT4 
> +ZLIB
> -SLZ +CPU_AFFINITY +TFO -NS +DL +RT -DEVICEATLAS -51DEGREES -WURFL -SYSTEMD
> -OBSOLETE_LINKER +PRCTL
>
> Default settings :
>  bufsize = 16384, maxrewrite = 1024, maxpollevents = 200
>
> Built with multi-threading support (MAX_THREADS=64, default=1).
> Built with OpenSSL version : OpenSSL 1.1.1b 26 Feb 2019
> Running on OpenSSL version : OpenSSL 1.1.1b 26 Feb 2019
> OpenSSL library supports TLS extensions : yes
> OpenSSL library supports SNI : yes
> OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2 TLSv1.3
> Built with Lua version : Lua 5.3.5
> Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPARENT
> IP_FREEBIND
> Built with zlib version : 1.2.7
> Running on zlib version : 1.2.7
> Compression algorithms supported : identity("identity"), deflate("deflate"),
> raw-deflate("deflate"), gzip("gzip")
> Built with PCRE version : 8.32 2012-11-30
> Running on PCRE version : 8.32 2012-11-30
> PCRE library supports JIT : yes
> Encrypted password support via crypt(3): yes
> Built with the Prometheus exporter as a service
>
> Available polling systems :
>  epoll : pref=300, test result OK
>  poll : pref=200, test result OK
>  select : pref=150, test result OK
> Total: 3 (3 usable), will use epoll.
>
> Available multiplexer protocols :
> (protocols marked as   cannot be specified using 'proto' keyword)
>  h2 : mode=HTX side=FE|BE
>  h2 : mode=HTTP side=FE
>: mode=HTX side=FE|BE
>: mode=TCP|HTTP side=FE|BE
>
> Available services :
>  prometheus-exporter
>
> Available filters :
>  [SPOE] spoe
>  [COMP] compression
>  [CACHE] cache
>  [TRACE] trace
> ```
>
> ```
> $ docker run --rm --entrypoint /usr/local/sbin/haproxy
> [MASKED]/haproxy20-boringssl -vv
> HA-Proxy version 2.0-dev2-5e6a5b-228 2019/04/25 - https://haproxy.org/
> Build options :
>  TARGET = linux2628
>  CPU = generic
>  CC = gcc
>  CFLAGS = -O2 -g -fno-strict-aliasing -Wdeclaration-after-statement -fwrapv
> -Wno-unused-label -Wno-sign-compare -Wno-unused-parameter
> -Wno-old-style-declaration -Wno-ignored-qualifiers -Wno-clobbered
> -Wno-missing-field-initializers -Wtype-limits -Wshift-negative-value
> -Wshift-overflow=2 -Wduplicated-cond -Wnull-dereference
>  OPTIONS = USE_PCRE2=1 USE_PCRE2_JIT=1 USE_THREAD=1 USE_PTHREAD_PSHARED=1
> USE_REGPARM=1 USE_LINUX_SPLICE=1 USE_GETADDRINFO=1 USE_OPENSSL=1 USE_LUA=1
> USE_ZLIB=1 USE_TFO=1
>
> Feature list : +EPOLL -KQUEUE -MY_EPOLL -MY_SPLICE +NETFILTER -PCRE -PCRE_JIT
> +PCRE2 +PCRE2_JIT +POLL -PRIVATE_CACHE +THREAD +PTHREAD_PSHARED +REGPARM
> -STATIC_PCRE -STATIC_PCRE2 +TPROXY +LINUX_TPROXY +LINUX_SPLICE +LIBCRYPT
> +CRYPT_H -VSYSCALL +GETADDRINFO +OPENSSL +LUA +FUTEX +ACCEPT4 -MY_ACCEPT4 
> +ZLIB
> -SLZ +CPU_AFFINITY +TFO -NS +DL +RT -DEVICEATLAS -51DEGREES -WURFL -SYSTEMD
> -OBSOLETE_LINKER +PRCTL
>
> Default settings :
>  bufsize = 16384, maxrewrite = 1024, maxpollevents = 200
>
> Built with multi-threading support (MAX_THREADS=64, default=1).
> Built with OpenSSL version : BoringSSL
> OpenSSL library supports TLS extensions : yes
> OpenSSL library supports SNI : yes
> OpenSSL library supports : TLSv1.0 TLSv1.1 TLSv1.2 TLSv1.3
> Built with Lua version : Lua 5.3.5
> Built with transparent proxy support using: IP_TRANSPARENT IPV6_TRANSPA

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

2019-05-05 Thread Willy Tarreau
Hi Maciej,

On Mon, May 06, 2019 at 06:49:26AM +0200, Maciej Zdeb wrote:
> Hi,
> 
> I confirm Willy patch fixed the problem! Thanks!

Great, thanks for confirming!
Willy



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

2019-05-05 Thread Willy Tarreau
On Sun, May 05, 2019 at 10:11:38PM +0200, Frederic Lecaille wrote:
> 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.

Excellent, thank you Fred for testing and for the explanation! Patch
merged now, we'll see if things improve.

Willy



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

2019-05-05 Thread Willy Tarreau
Hi Ilya,

> I made another PR
> 
> https://github.com/haproxy/haproxy/pull/92

Thank you.

> (I really like automatic PR to mailing list routing)

Well, it was the only workable workaround we have when people send PRs.
Sadly we can't block them. Apparently only mirror repositories can block
PRs and github doesn't accept to create them anymore. But when there are
multiple patches in the PR it's not usable as-is anymore, it only serves
as a notification that someone sent something.

For me, PRs tremendously increase the amount of manipulations, which is
why I tend to postpone them until I think this will not divert me too
much from what I am doing. I just timed the operations on this small
batch and it went from ~15 seconds to review and merge your 4 patches
if they were in mails (just pressing Enter in mutt to review them,
possibly "e" to quickly edit if needed, and "A" to apply), then "g" to
ack the merge, to around 3 minutes to :
  - copy the link
  - open a browser
  - paste the link into the browser's URL bar
  - locate the links to the commits in the page
  - open each of them in a separate tab
  - visit each tab in turn, and for each of them, press Ctrl-L and
append ".patch" at the end of the URL, then press enter
  - after the patches are loaded, visit all of them again, to look at the
dates and figure in what order to apply them
  - then for each of them :
  - review the patch
  - Ctrl-S to save the patch
  - remember the file name (commit ID) and press Enter
  - Alt-tab to switch to the xterm in the development directory
  - git am ~/.patch
  - rm -f ~/.patch
  - Alt-tab to switch back to browser again
  - Ctrl-W to close the tab
  - close browser window
  - back to mutt to ack merging and rant about my hate of this workflow

=> This is way longer and more error-prone than the first variant. And
   this doesn't even include commenting if I disagree on something, which
   additionally requires copy-pasting the relevant parts of the patch
   into the response message. There is a reason why most projects
   developed on github are so low quality with so horrible commits
   and zero maintenance : it encourages laziness and dirtiness. It
   costs so much effort to fix minor issues and encourage people to
   improve, compared to lazily click on "merge" that you simply think
   "bah..." and you merge it as-is, making it impossible to review them
   later when trying to make a maintenance version. So you end up with
   continuous development made of a pile of junk patches :-/

Anyway now I've finally applied your patches. In the future, if you
want to help with a smoother process, please either attach your patches
or use git-send-email. The most efficient workflow is one patch per mail,
like git-send-email does, as it eases reviews (which can also be done by
multiple persons). If you attach them, make sure to use the file names
created by git-format-patch so that there's no doubt when saving files
for manual editing for example. Also it's important to indicate the
intent with a patch or a patch series, i.e. "please merge this", "please
don't merge this, it's just for discussing", "it should be mergeable once
the makefile is fixed", "I think it's OK for merging but I'd prefer an
extra review first", "how about we'd do this", etc. I'm all for being
accommodating with submissions and slightly edit them if needed because
I know that if the work has to be 100% on either side it can total more
time than 90/10 or even 95/5 (but I never edit signed patches beyond
fixing merging issues though). I'm just very careful about the time
spent on my side because I know I'm already a bottleneck, so each extra
minute added here delays everything else.

Thanks!
Willy



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 OPENSSL_IS

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