haproxy 1.9.6 segfault in srv_update_status

2019-05-14 Thread Patrick Hemmer
We haven't had a chance to update to 1.9.8 yet, so we're still running 
1.9.6 (Linux) in production, and just had 2 segfaults happen a little 
over an hour apart. When I look at the core dumps from them, the stack 
trace is the same. I'm not sure if this is an issue already fixed, so 
providing just in case.


There was one oddity going on at the time these segfaults occurred. We 
had maxed out the Linux kernel's conntrack table. So haproxy would have 
been experiencing timeouts when attempting new connections, with health 
checks failing all over the place.



(gdb) bt full
#0  task_schedule (when=, task=0x0) at 
include/proto/task.h:439

No locals.
#1  srv_update_status (s=0x7f6ea12e2a80) at src/server.c:4872
    next_admin = 0
    check = 0x7f6ea12e2f20
    xferred = 
    px = 0x7f6ea12d8300
    prev_srv_count = 6
    srv_was_stopping = 0
    log_level = 
    tmptrash = 0x0
#2  0x7f6ea0a7bd22 in server_recalc_eweight 
(sv=sv@entry=0x7f6ea12e2a80, must_update=must_update@entry=1) at 
src/server.c:1310

    px = 
    w = 
#3  0x7f6ea0a81ce8 in srv_update_state (params=0x7ffc51186bf0, 
version=1, srv=0x7f6ea12e2a80) at src/server.c:3112

    p = 0x7ffc51186ccf ""
    srv_op_state = 
    bk_f_forced_id = 
    port = 8080
    srv_admin_state = 0
    srv_last_time_change = 6
    srv_check_state = 6
    srv_agent_state = 0
    srv_check_result = CHK_RES_PASSED
    fqdn_set_by_cli = 0
    srv_check_status = 15
    port_str = 0x7ffc51186cd2 "8080"
    srvrecord = 0x0
    msg = 0x7f6ea08d7fe0
    srv_uweight = 1
    srv_iweight = 1
    srv_check_health = 
    srv_f_forced_id = 
    fqdn = 0x0
#4  apply_server_state () at src/server.c:3514
    bk_f_forced_id = 
    check_id = 
    check_name = 
    cur = 
    end = 
    mybuf = 
"36\000backoffice\000\062\000iad1gbow02\000\061\060.3.66.169\000\061\000\060\000\061\000\061\000\066\000\061\065\000\063\000\062\000\066\000\060\000\060\000\060\000-\000\070\060\070\060\000-\000\000\000\000\061\000\060\000\062\000\060\000\060\000\060\000\060\000-\000\070\060\070\060\000-\000\000me_since_last_change 
srv_check_status srv_check_result srv_check_health srv_check_state 
srv_agent_st"...

    mybuflen = 
    params = {0x7ffc51186c90 "36", 0x7ffc51186c93 "backoffice", 
0x7ffc51186c9e "2", 0x7ffc51186ca0 "iad1gbow02", 0x7ffc51186cab 
"10.3.66.169",
  0x7ffc51186cb7 "1", 0x7ffc51186cb9 "0", 0x7ffc51186cbb "1", 
0x7ffc51186cbd "1", 0x7ffc51186cbf "6", 0x7ffc51186cc1 "15",
  0x7ffc51186cc4 "3", 0x7ffc51186cc6 "2", 0x7ffc51186cc8 "6", 
0x7ffc51186cca "0", 0x7ffc51186ccc "0", 0x7ffc51186cce "0",

  0x7ffc51186cd0 "-", 0x7ffc51186cd2 "8080", 0x7ffc51186cd7 "-"}
    srv_params = {0x7ffc51186cab "10.3.66.169", 0x7ffc51186cb7 "1", 
0x7ffc51186cb9 "0", 0x7ffc51186cbb "1", 0x7ffc51186cbd "1",
  0x7ffc51186cbf "6", 0x7ffc51186cc1 "15", 0x7ffc51186cc4 "3", 
0x7ffc51186cc6 "2", 0x7ffc51186cc8 "6", 0x7ffc51186cca "0",
  0x7ffc51186ccc "0", 0x7ffc51186cce "0", 0x7ffc51186cd0 "-", 
0x7ffc51186cd2 "8080", 0x7ffc51186cd7 "-", 0x0, 0x0, 0x0, 0x0}

    arg = 
    srv_arg = 
    version = 
    diff = 0
    f = 0x7f6ea16ad080
    filepath = 
    globalfilepath = "/var/lib/haproxy/state", '\000' times>...
    localfilepath = 
"d\000\000\000\000\000\000\000f\222Ҡn\177\000\000b\222Ҡn\177\000\000\000\177\030Q\374\177\000\000\020\000\000\000\000\000\000\000\265\221Ҡn\177\000\000\261\221Ҡn\177\000\000\003\000\000\000\000\000\000\000\250\221Ҡn\177\000\000\b\000\000\000\000\000\000\000\003\000\000\000\000\000\000\000\250\221Ҡn\177\000\000\001\222Ҡn\177\000\000\f\000\000\000\000\000\000\000\060\200\030Q\374\177\000\000\t\000\000\000\000\000\000\000\363\221Ҡn\177\000\000\344t\262\240n\177\000\000\265\221Ҡn\177\000\000\240\231m\241n\177\000\000\\\000\000\000\000\000\000\000x\331m\241n\177\000\000\016\222Ҡn\177\000\000\b\000\000\000\000\000\000\000\250\221Ҡn\177\000\000\302\000\000\000\000\000\000\000\060\200\030"...

    len = 
    fileopenerr = 
    globalfilepathlen = 
    localfilepathlen = 
    curproxy = 0x7f6ea12d8300
    bk = 0x7f6ea12d8300
    srv = 0x7f6ea12e2a80
#5  0x7f6ea0a8f48f in init (argc=, argc@entry=13, 
argv=, argv@entry=0x7ffc51189488) at src/haproxy.c:1843

    arg_mode = 
    tmp = 
    cfg_pidfile = 
    err_code = 9
    err_msg = 0x0
    wl = 
    progname = 0x7ffc5118acf6 "haproxy"
    change_dir = 
    px = 
    pcf = 
#6  0x7f6ea09e41a7 in main (argc=13, argv=0x7ffc51189488) at 
src/haproxy.c:2774

    err = 
    retry = 
    limit = {rlim_cur = 131072, rlim_max = 131072}
    errmsg = 

Re: [PATCH 0/6] Kill deprecated configuration options

2019-05-14 Thread Willy Tarreau
Hi Aleks,

On Wed, May 15, 2019 at 05:51:25AM +0200, Aleksandar Lazic wrote:
> For example such a generic replacement could be like this?
> 
> http://cbonte.github.io/haproxy-dconv/1.9/configuration.html#4.2-reqrep
> 
> # replace "/static/" with "/" at the beginning of any request path.
>  reqrep ^([^\ :]*)\ /static/(.*) \1\ /\2 # replace
> 
> http://cbonte.github.io/haproxy-dconv/1.9/configuration.html#4.2-http-request%20set-path
> 
> 
> http-request set-path %[path,regsub(/static/(.*),\1,g)

Exactly! While we can't expect the config parser to propose these, at
least it could point to replace-header, set-header, set-path etc so
that users take a look at the doc and update their 10-years old configs.

Willy



Re: [PATCH 0/6] Kill deprecated configuration options

2019-05-14 Thread Aleksandar Lazic


Hi.

Wed May 15 05:07:05 GMT+02:00 2019 Willy Tarreau :

> Hi Tim,
 >
 > On Tue, May 14, 2019 at 08:57:55PM +0200, Tim Duesterhus wrote:
 > > Okay, I did a sweep through the configuration parser and:
 > >
 > > 1. Made deprecated directives fatal and removed them from the docs. The
 > > error messages speak of "HAProxy 2.1", thus it should be merged into
 > > some kind of 'next' branch.
 > > 2. Made deprecated directives actually warn and remove them from the docs
 > > as well. No need to document deprecated options, users can simply peek
 > > into the old docs. Also the error messages are pretty clear on what
 > > needs to be done to fix it.
 >
 > OK, I think we can take all of these for a next branch indeed. Two of
 > them, the one adding a warning and the one removing the unused keyword
 > could be picked for 2.0 if you agree. Also I'll rename them "MEDIUM" as
 > they do change something in a way that is fixable by configuration,
 > these are not just functionally equivalent code cleanups.
 >
 > > What I did not update:
 > >
 > > 1. Usage of deprecated directives in the 'tests' directory. The fact that
 > > the tests still use deprecated directives indicates that no one even
 > > uses them any more. Maybe these tests should outright be removed?
 >
 > Many times I thought about removing them. But each time I look at them
 > I figure that some *might* still be usable. Now that we have the regtests
 > probably this is not true anymore. I also suspect a few of them are
 > referenced in the internal state machine tests, and might need to be
 > completely converted to regtests before being removed.
 >
 > > 2. 'req*' and 'rsp*'. I remember that they allow some modification that
 > > cannot easily be replicated otherwise (but I'll have to check that
 > > first).
 >
 > Sure but practically speaking such modifications do not make sense in
 > the real world (e.g. rename many header names at once). And the "excuse"
 > above has been the reason for continually postponing their removal. I'd
 > instead vote for warning on them in 2.0 and removing them very early in
 > 2.1. If someone has a compelling use case, we'll get some feedback thanks
 > to the warning and it will still be possible to figure how to implement
 > a replacement using http-request rules.


This would be very helpfully.

For example such a generic replacement could be like this?

http://cbonte.github.io/haproxy-dconv/1.9/configuration.html#4.2-reqrep

# replace "/static/" with "/" at the beginning of any request path.
 reqrep ^([^\ :]*)\ /static/(.*) \1\ /\2 # replace

http://cbonte.github.io/haproxy-dconv/1.9/configuration.html#4.2-http-request%20set-path


http-request set-path %[path,regsub(/static/(.*),\1,g)

> > What I wasn't sure about: The 'transparent' directive. It is deprecated in
 > > the docs, but it does not warn. When taking a look into the configuration
 > > parser it appears to set different flags compared to 'option transparent'.
 > > Can you please take a look at this and either add an appropriate warning or
 > > remove the deprecated note from the docs?
 >
 > This vaguely reminds me something indeed, I'll need to have a look at
 > this. We may even discover that one of them is bogus :-)
 >
 > Thanks,
 > Willy

Regards
 Aleks





Re: [PATCH 0/6] Kill deprecated configuration options

2019-05-14 Thread Willy Tarreau
Hi Tim,

On Tue, May 14, 2019 at 08:57:55PM +0200, Tim Duesterhus wrote:
> Okay, I did a sweep through the configuration parser and:
> 
> 1. Made deprecated directives fatal and removed them from the docs. The
>error messages speak of "HAProxy 2.1", thus it should be merged into
>some kind of 'next' branch.
> 2. Made deprecated directives actually warn and remove them from the docs
>as well. No need to document deprecated options, users can simply peek
>into the old docs. Also the error messages are pretty clear on what
>needs to be done to fix it.

OK, I think we can take all of these for a next branch indeed. Two of
them, the one adding a warning and the one removing the unused keyword
could be picked for 2.0 if you agree. Also I'll rename them "MEDIUM" as
they do change something in a way that is fixable by configuration,
these are not just functionally equivalent code cleanups.

> What I did not update:
> 
> 1. Usage of deprecated directives in the 'tests' directory. The fact that
>the tests still use deprecated directives indicates that no one even
>uses them any more. Maybe these tests should outright be removed?

Many times I thought about removing them. But each time I look at them
I figure that some *might* still be usable. Now that we have the regtests
probably this is not true anymore. I also suspect a few of them are
referenced in the internal state machine tests, and might need to be
completely converted to regtests before being removed.

> 2. 'req*' and 'rsp*'. I remember that they allow some modification that
>cannot easily be replicated otherwise (but I'll have to check that
>first).

Sure but practically speaking such modifications do not make sense in
the real world (e.g. rename many header names at once). And the "excuse"
above has been the reason for continually postponing their removal. I'd
instead vote for warning on them in 2.0 and removing them very early in
2.1. If someone has a compelling use case, we'll get some feedback thanks
to the warning and it will still be possible to figure how to implement
a replacement using http-request rules.

> What I wasn't sure about: The 'transparent' directive. It is deprecated in
> the docs, but it does not warn. When taking a look into the configuration
> parser it appears to set different flags compared to 'option transparent'.
> Can you please take a look at this and either add an appropriate warning or
> remove the deprecated note from the docs?

This vaguely reminds me something indeed, I'll need to have a look at
this. We may even discover that one of them is bogus :-)

Thanks,
Willy



Re: Loading multiple TLS certificates

2019-05-14 Thread Robin H. Johnson
On Mon, May 13, 2019 at 09:10:15PM +, Gibson, Brian (IMS) wrote:
> 
> For the first time, I have a client that refused to let me use a wildcard 
> certificate.
> So I submitted 6 separate CSRs and now have 6 separate certificates and 6 
> separate keys.
> The intermediate certificates all appear to be the same.
> So should I create 6 separate PEM files containing the certificate, the 
> intermediates, and the key,
> or should I create a single PEM file containing all 6 certificates, 6 keys, 
> and 1 intermediate file?
6 distinct files, and map them all via crt-list.

-- 
Robin Hugh Johnson
E-Mail : robb...@orbis-terrarum.net
Home Page  : http://www.orbis-terrarum.net/?l=people.robbat2
GnuPG FP   : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85


signature.asc
Description: PGP signature


[PATCH 3/6] CLEANUP: Make 'redispatch' directive fatal

2019-05-14 Thread Tim Duesterhus
It was deprecated with HAProxy 1.5. Time to remove it.
---
 doc/configuration.txt  | 26 --
 include/types/global.h |  2 +-
 src/cfgparse-listen.c  | 14 +++---
 3 files changed, 4 insertions(+), 38 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index e520c1548..edfcd02d5 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -2391,8 +2391,6 @@ external-check path   X  -
 X X
 persist rdp-cookieX  - X X
 rate-limit sessions   X  X X -
 redirect  -  X X X
-redisp  (deprecated)  X  - X X
-redispatch  (deprecated)  X  - X X
 reqadd-  X X X
 reqallow  -  X X X
 reqdel-  X X X
@@ -7680,30 +7678,6 @@ redirect scheme[code ]  [{if | 
unless} ]
   See section 7 about ACL usage.
 
 
-redisp (deprecated)
-redispatch (deprecated)
-  Enable or disable session redistribution in case of connection failure
-  May be used in sections:defaults | frontend | listen | backend
- yes   |no|   yes  |   yes
-  Arguments : none
-
-  In HTTP mode, if a server designated by a cookie is down, clients may
-  definitely stick to it because they cannot flush the cookie, so they will not
-  be able to access the service anymore.
-
-  Specifying "redispatch" will allow the proxy to break their persistence and
-  redistribute them to a working server.
-
-  It also allows to retry last connection to another server in case of multiple
-  connection failures. Of course, it requires having "retries" set to a nonzero
-  value.
-
-  This form is deprecated, do not use it in any new configuration, use the new
-  "option redispatch" instead.
-
-  See also : "option redispatch"
-
-
 reqadd   [{if | unless} ]
   Add a header at the end of the HTTP request
   May be used in sections :   defaults | frontend | listen | backend
diff --git a/include/types/global.h b/include/types/global.h
index ab1c3036e..d34dae9ff 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -247,7 +247,7 @@ __decl_hathreads(extern pthread_t *threads);
 /* bit values to go with "warned" above */
 /* unassigned : 0x0001 (previously: WARN_BLOCK_DEPRECATED) */
 /* unassigned : 0x0002 */
-#define WARN_REDISPATCH_DEPRECATED  0x0004
+/* unassigned : 0x0004 (previously: WARN_REDISPATCH_DEPRECATED) */
 #define WARN_CLITO_DEPRECATED   0x0008
 #define WARN_SRVTO_DEPRECATED   0x0010
 #define WARN_CONTO_DEPRECATED   0x0020
diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index 0a3aa9281..b29484478 100644
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -2779,18 +2779,10 @@ stats_error_parsing:
goto out;
}
else if (!strcmp(args[0], "redispatch") || !strcmp(args[0], "redisp")) {
-   if (warnifnotcap(curproxy, PR_CAP_BE, file, linenum, args[0], 
NULL))
-   err_code |= ERR_WARN;
-
-   if (!already_warned(WARN_REDISPATCH_DEPRECATED))
-   ha_warning("parsing [%s:%d]: keyword '%s' is deprecated 
in favor of 'option redispatch', and will not be supported by future 
versions.\n",
-  file, linenum, args[0]);
-   err_code |= ERR_WARN;
-   /* enable reconnections to dispatch */
-   curproxy->options |= PR_O_REDISP;
+   ha_alert("parsing [%s:%d] : keyword '%s' directive is not 
supported anymore since HAProxy 2.1. Use 'option redispatch'.\n", file, 
linenum, args[0]);
 
-   if (alertif_too_many_args_idx(1, 0, file, linenum, args, 
_code))
-   goto out;
+   err_code |= ERR_ALERT | ERR_FATAL;
+   goto out;
}
else if (!strcmp(args[0], "http-reuse")) {
if (warnifnotcap(curproxy, PR_CAP_BE, file, linenum, args[0], 
NULL))
-- 
2.21.0




[PATCH 2/6] CLEANUP: Make 'block' directive fatal

2019-05-14 Thread Tim Duesterhus
It was deprecated with HAProxy 1.5. Time to remove it.
---
 doc/configuration.txt  | 30 --
 include/types/global.h |  2 +-
 src/cfgparse-listen.c  | 30 --
 3 files changed, 5 insertions(+), 57 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index c8ccaad1b..e520c1548 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -2283,7 +2283,6 @@ backlog   X  X
 X -
 balance   X  - X X
 bind  -  X X -
 bind-process  X  X X X
-block   (deprecated)  -  X X X
 capture cookie-  X X -
 capture request header-  X X -
 capture response header   -  X X -
@@ -2935,35 +2934,6 @@ bind-process [ all | odd | even | 
[-[]] ] ...
   See also : "nbproc" in global section, and "process" in section 5.1.
 
 
-block { if | unless }  (deprecated)
-  Block a layer 7 request if/unless a condition is matched
-  May be used in sections :   defaults | frontend | listen | backend
- no|yes   |   yes  |   yes
-
-  The HTTP request will be blocked very early in the layer 7 processing
-  if/unless  is matched. A 403 error will be returned if the request
-  is blocked. The condition has to reference ACLs (see section 7). This is
-  typically used to deny access to certain sensitive resources if some
-  conditions are met or not met. There is no fixed limit to the number of
-  "block" statements per instance. To block connections at layer 4 (without
-  sending a 403 error) see "tcp-request connection reject" and
-  "tcp-request content reject" rules.
-
-  This form is deprecated, do not use it in any new configuration, use the new
-  "http-request deny" instead.
-
-  Example:
-acl invalid_src  src  0.0.0.0/7 224.0.0.0/3
-acl invalid_src  src_port 0:1023
-acl local_dsthdr(host) -i localhost
-# block is deprecated. Use http-request deny instead:
-#block if invalid_src || local_dst
-http-request deny if invalid_src || local_dst
-
-  See also : section 7 about ACL usage, "http-request deny",
-"http-response deny", "tcp-request connection reject" and
-"tcp-request content reject".
-
 capture cookie  len 
   Capture and log a cookie in the request and in the response.
   May be used in sections :   defaults | frontend | listen | backend
diff --git a/include/types/global.h b/include/types/global.h
index 2df0da9b4..ab1c3036e 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -245,7 +245,7 @@ extern int atexit_flag;
 __decl_hathreads(extern pthread_t *threads);
 
 /* bit values to go with "warned" above */
-#define WARN_BLOCK_DEPRECATED   0x0001
+/* unassigned : 0x0001 (previously: WARN_BLOCK_DEPRECATED) */
 /* unassigned : 0x0002 */
 #define WARN_REDISPATCH_DEPRECATED  0x0004
 #define WARN_CLITO_DEPRECATED   0x0008
diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index 7760d9c3c..0a3aa9281 100644
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -1521,33 +1521,11 @@ int cfg_parse_listen(const char *file, int linenum, 
char **args, int kwm)
curproxy->server_id_hdr_name = strdup(args[1]);
curproxy->server_id_hdr_len  = 
strlen(curproxy->server_id_hdr_name);
}
-   else if (!strcmp(args[0], "block")) {  /* early blocking based on ACLs 
*/
-   struct act_rule *rule;
-
-   if (curproxy == ) {
-   ha_alert("parsing [%s:%d] : '%s' not allowed in 
'defaults' section.\n", file, linenum, args[0]);
-   err_code |= ERR_ALERT | ERR_FATAL;
-   goto out;
-   }
-
-   /* emulate "block" using "http-request block". Since these 
rules are supposed to
-* be processed before all http-request rules, we put them into 
their own list
-* and will insert them at the end.
-*/
-   rule = parse_http_req_cond((const char **)args, file, linenum, 
curproxy);
-   if (!rule) {
-   err_code |= ERR_ALERT | ERR_ABORT;
-   goto out;
-   }
-   err_code |= warnif_misplaced_block(curproxy, file, linenum, 
args[0]);
-   err_code |= warnif_cond_conflicts(rule->cond,
- (curproxy->cap & PR_CAP_FE) ? 
SMP_VAL_FE_HRQ_HDR : SMP_VAL_BE_HRQ_HDR,
- file, linenum);
-   LIST_ADDQ(>block_rules, >list);
-
-   if 

[PATCH 0/6] Kill deprecated configuration options

2019-05-14 Thread Tim Duesterhus
Willy,

Am 06.05.19 um 07:23 schrieb 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.

I opted to make it fatal (because there is no harm in doing so).

>> - 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.

Okay, I did a sweep through the configuration parser and:

1. Made deprecated directives fatal and removed them from the docs. The
   error messages speak of "HAProxy 2.1", thus it should be merged into
   some kind of 'next' branch.
2. Made deprecated directives actually warn and remove them from the docs
   as well. No need to document deprecated options, users can simply peek
   into the old docs. Also the error messages are pretty clear on what
   needs to be done to fix it.

What I did not update:

1. Usage of deprecated directives in the 'tests' directory. The fact that
   the tests still use deprecated directives indicates that no one even
   uses them any more. Maybe these tests should outright be removed?
2. 'req*' and 'rsp*'. I remember that they allow some modification that
   cannot easily be replicated otherwise (but I'll have to check that
   first).

What I wasn't sure about: The 'transparent' directive. It is deprecated in
the docs, but it does not warn. When taking a look into the configuration
parser it appears to set different flags compared to 'option transparent'.
Can you please take a look at this and either add an appropriate warning or
remove the deprecated note from the docs?

Best regards

Tim Duesterhus (6):
  CLEANUP: Make 'resolution_pool_size' directive fatal
  CLEANUP: Make 'block' directive fatal
  CLEANUP: Make 'redispatch' directive fatal
  CLEANUP: Make '(cli|con|srv)timeout' directive fatal
  CLEANUP: Make 'option forceclose' actually warn
  CLEANUP: Remove 'option independant-streams'

 doc/configuration.txt  | 212 +
 examples/haproxy.vim   |   6 +-
 include/types/global.h |  11 ++-
 src/cfgparse-listen.c  |  50 +++---
 src/cfgparse.c |   4 +-
 src/proxy.c|  30 +++---
 6 files changed, 44 insertions(+), 269 deletions(-)

-- 
2.21.0




[PATCH 5/6] CLEANUP: Make 'option forceclose' actually warn

2019-05-14 Thread Tim Duesterhus
It is deprecated since 315b39c3914f4c2301ce19a93564566caa2ede50 (1.9-dev),
but only was deprecated in the docs.

Make it warn when being used and remove it from the docs.
---
 doc/configuration.txt  | 8 
 examples/haproxy.vim   | 2 +-
 include/types/global.h | 1 +
 src/cfgparse-listen.c  | 6 ++
 4 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 6920f129c..85309bcc9 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -2339,7 +2339,6 @@ option clitcpka  (*)  X  X
 X -
 option contstats (*)  X  X X -
 option dontlog-normal(*)  X  X X -
 option dontlognull   (*)  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
@@ -5943,13 +5942,6 @@ no option dontlognull
  section 8 about logging.
 
 
-option forceclose (deprecated)
-no option forceclose (deprecated)
-  This is an alias for "option httpclose". Thus this option is deprecated.
-
-  See also : "option httpclose" and "option http-pretend-keepalive"
-
-
 option forwardfor [ except  ] [ header  ] [ if-none ]
   Enable insertion of the X-Forwarded-For header to requests sent to servers
   May be used in sections :   defaults | frontend | listen | backend
diff --git a/examples/haproxy.vim b/examples/haproxy.vim
index 502bbfee3..48fd78c63 100644
--- a/examples/haproxy.vim
+++ b/examples/haproxy.vim
@@ -82,7 +82,7 @@ syn keyword hapBalance   contained roundrobin source
 syn keyword hapLen   contained len
 syn keyword hapGLog  contained global
 syn keyword hapMode  contained http tcp health
-syn keyword hapOptioncontained abortonclose allbackups checkcache clitcpka 
dontlognull forceclose forwardfor
+syn keyword hapOptioncontained abortonclose allbackups checkcache clitcpka 
dontlognull forwardfor
 syn keyword hapOptioncontained httpchk httpclose httplog keepalive logasap 
persist srvtcpka ssl-hello-chk
 syn keyword hapOptioncontained tcplog tcpka tcpsplice
 syn keyword hapOptioncontained except skipwhite nextgroup=hapIPv4Mask
diff --git a/include/types/global.h b/include/types/global.h
index de2516814..cc691a408 100644
--- a/include/types/global.h
+++ b/include/types/global.h
@@ -251,6 +251,7 @@ __decl_hathreads(extern pthread_t *threads);
 /* unassigned : 0x0008 (previously: WARN_CLITO_DEPRECATED) */
 /* unassigned : 0x0010 (previously: WARN_SRVTO_DEPRECATED) */
 /* unassigned : 0x0020 (previously: WARN_CONTO_DEPRECATED) */
+#define WARN_FORCECLOSE_DEPRECATED 0x0040
 
 /* to be used with warned and WARN_* */
 static inline int already_warned(unsigned int warning)
diff --git a/src/cfgparse-listen.c b/src/cfgparse-listen.c
index b29484478..cae345de9 100644
--- a/src/cfgparse-listen.c
+++ b/src/cfgparse-listen.c
@@ -2138,6 +2138,12 @@ stats_error_parsing:
 * sections).
 */
if (strcmp(args[1], "httpclose") == 0 || strcmp(args[1], 
"forceclose") == 0) {
+   if (strcmp(args[1], "forceclose") == 0) {
+   if (!already_warned(WARN_FORCECLOSE_DEPRECATED))
+   ha_warning("parsing [%s:%d]: keyword 
'%s' is deprecated in favor of 'httpclose', and will not be supported by future 
versions.\n",
+ file, linenum, args[1]);
+   err_code |= ERR_WARN;
+   }
if (alertif_too_many_args_idx(0, 1, file, linenum, 
args, _code))
goto out;
if (kwm == KWM_STD) {
-- 
2.21.0




[PATCH 1/6] CLEANUP: Make 'resolution_pool_size' directive fatal

2019-05-14 Thread Tim Duesterhus
This directive never appeared in a stable release and instead was
introduced and deprecated within 1.8-dev. While it technically could
be outright removed we detect it and error out for good measure.
---
 doc/configuration.txt | 5 -
 src/cfgparse.c| 4 ++--
 2 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 754ea8bec..c8ccaad1b 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -12767,11 +12767,6 @@ hold  
 
   Default value is 10s for "valid", 0s for "obsolete" and 30s for others.
 
-resolution_pool_size   (deprecated)
-  Defines the number of resolutions available in the pool for this resolvers.
-  If not defines, it defaults to 64. If your configuration requires more than
-  , then HAProxy will return an error when parsing the configuration.
-
 resolve_retries 
   Defines the number  of queries to send to resolve a server name before
   giving up.
diff --git a/src/cfgparse.c b/src/cfgparse.c
index ab5a2eb75..a16628a5c 100644
--- a/src/cfgparse.c
+++ b/src/cfgparse.c
@@ -1250,9 +1250,9 @@ resolv_out:
curr_resolvers->accepted_payload_size = i;
}
else if (strcmp(args[0], "resolution_pool_size") == 0) {
-   ha_warning("parsing [%s:%d] : '%s' directive is now deprecated 
and ignored.\n",
+   ha_alert("parsing [%s:%d] : '%s' directive is not supported 
anymore (it never appeared in a stable release).\n",
   file, linenum, args[0]);
-   err_code |= ERR_WARN;
+   err_code |= ERR_ALERT | ERR_FATAL;
goto out;
}
else if (strcmp(args[0], "resolve_retries") == 0) {
-- 
2.21.0




[PATCH 4/6] CLEANUP: Make '(cli|con|srv)timeout' directive fatal

2019-05-14 Thread Tim Duesterhus
They were deprecated with HAProxy 1.5. Time to remove them.
---
 doc/configuration.txt  | 139 ++---
 examples/haproxy.vim   |   4 +-
 include/types/global.h |   6 +-
 src/proxy.c|  29 -
 4 files changed, 25 insertions(+), 153 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index edfcd02d5..6920f129c 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -2286,9 +2286,7 @@ bind-process  X  X
 X X
 capture cookie-  X X -
 capture request header-  X X -
 capture response header   -  X X -
-clitimeout  (deprecated)  X  X X -
 compression   X  X X X
-contimeout  (deprecated)  X  - X X
 cookieX  - X X
 declare capture   -  X X -
 default-serverX  - X X
@@ -2418,7 +2416,6 @@ server-  -
 X X
 server-state-file-nameX  - X X
 server-template   -  - X X
 sourceX  - X X
-srvtimeout  (deprecated)  X  - X X
 stats admin   -  X X X
 stats authX  X X X
 stats enable  X  X X X
@@ -2450,15 +2447,12 @@ tcp-response inspect-delay-  -  
   X X
 timeout check X  - X X
 timeout clientX  X X -
 timeout client-finX  X X -
-timeout clitimeout  (deprecated)  X  X X -
 timeout connect   X  - X X
-timeout contimeout  (deprecated)  X  - X X
 timeout http-keep-alive   X  X X X
 timeout http-request  X  X X X
 timeout queue X  - X X
 timeout serverX  - X X
 timeout server-finX  - X X
-timeout srvtimeout  (deprecated)  X  - X X
 timeout tarpitX  X X X
 timeout tunnelX  - X X
 transparent (deprecated)  X  - X X
@@ -3054,40 +3048,6 @@ capture response header  len 
  about logging.
 
 
-clitimeout  (deprecated)
-  Set the maximum inactivity time on the client side.
-  May be used in sections :   defaults | frontend | listen | backend
- yes   |yes   |   yes  |   no
-  Arguments :
- is the timeout value is specified in milliseconds by default, but
-  can be in any other unit if the number is suffixed by the unit,
-  as explained at the top of this document.
-
-  The inactivity timeout applies when the client is expected to acknowledge or
-  send data. In HTTP mode, this timeout is particularly important to consider
-  during the first phase, when the client sends the request, and during the
-  response while it is reading data sent by the server. The value is specified
-  in milliseconds by default, but can be in any other unit if the number is
-  suffixed by the unit, as specified at the top of this document. In TCP mode
-  (and to a lesser extent, in HTTP mode), it is highly recommended that the
-  client timeout remains equal to the server timeout in order to avoid complex
-  situations to debug. It is a good practice to cover one or several TCP packet
-  losses by specifying timeouts that are slightly above multiples of 3 seconds
-  (e.g. 4 or 5 seconds).
-
-  This parameter is specific to frontends, but can be specified once for all in
-  "defaults" sections. This is in fact one of the easiest solutions not to
-  forget about it. An unspecified timeout results in an infinite timeout, which
-  is not recommended. Such a usage is accepted and works but reports a warning
-  during startup because it may results in accumulation of expired sessions in
-  the system if the system's timeouts are not configured either.
-
-  This parameter is provided for compatibility but is currently deprecated.
-  Please 

[PATCH 6/6] CLEANUP: Remove 'option independant-streams'

2019-05-14 Thread Tim Duesterhus
It is deprecated with HAProxy 1.5. Time to remove it.
---
 doc/configuration.txt | 4 
 src/proxy.c   | 1 -
 2 files changed, 5 deletions(-)

diff --git a/doc/configuration.txt b/doc/configuration.txt
index 85309bcc9..86251768c 100644
--- a/doc/configuration.txt
+++ b/doc/configuration.txt
@@ -6514,10 +6514,6 @@ no option independent-streams
   data sent to the server. Doing so will typically break large HTTP posts from
   slow lines, so use it with caution.
 
-  Note: older versions used to call this setting "option independant-streams"
-with a spelling mistake. This spelling is still supported but
-deprecated.
-
   See also : "timeout client", "timeout server" and "timeout tunnel"
 
 
diff --git a/src/proxy.c b/src/proxy.c
index 9cbb7e7a2..9009293fb 100644
--- a/src/proxy.c
+++ b/src/proxy.c
@@ -106,7 +106,6 @@ const struct cfg_opt cfg_opts2[] =
{ "socket-stats", PR_O2_SOCKSTAT,  PR_CAP_FE, 0, 0 },
{ "tcp-smart-accept", PR_O2_SMARTACC,  PR_CAP_FE, 0, 0 },
{ "tcp-smart-connect",PR_O2_SMARTCON,  PR_CAP_BE, 0, 0 },
-   { "independant-streams",  PR_O2_INDEPSTR,  PR_CAP_FE|PR_CAP_BE, 
0, 0 },
{ "independent-streams",  PR_O2_INDEPSTR,  PR_CAP_FE|PR_CAP_BE, 
0, 0 },
{ "http-use-proxy-header",PR_O2_USE_PXHDR, PR_CAP_FE, 0, 
PR_MODE_HTTP },
{ "http-pretend-keepalive",   PR_O2_FAKE_KA,   PR_CAP_BE, 0, 
PR_MODE_HTTP },
-- 
2.21.0




Re: [RFC PATCH v2] BUG/MEDIUM: compression: Rewrite strong ETags

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

Am 29.01.19 um 19:42 schrieb Willy Tarreau:
>> Note: I added an `assert` in there to make sure that ht*_select_comp_reshdr
>> actually verified the ETag header before I am touching it. There *is* 
>> precedence
>> for `assert` in `checks.c`. Please remove the `assert` if you are not happy
>> with it.
> 
> I will do :-) I absolute hate assert, almost every time they are used,
> they are wrong, or a lazy error handling case serving as a todo for
> later, and generally when they are used it's because the person who
> places them is not yet very certain to have caught all cases, thus it
> means that practically speaking they WILL cause crashes in field under
> unplanned situations. Anyone who has ever used Gimp in foreground knows
> how scary it can be to be all these asserts continuously scrolling and
> making you figure that you avoided a crash every 200 milliseconds
> because they're running in warning mode instead of crash mode.

I notice the new BUG_ON macro: Should the assert from this patch be
re-considered?

Best regards
Tim Düsterhus



Info required regarding health check in http mode.

2019-05-14 Thread Badari Prasad
Hi ,
I am using haproxy as L7 load balancer and in my configuration have
enabled L4 level health checks to back end nodes. During testing for long
duration saw intermittent healthcheck errors.
Have few queries:
1) is it advisable to use L4 health checks for L7 load balancer ?
2) if backend nodes have some internal issue and respond to request with
500 internal server error and close the TCP sessions for duration  say 2
mins, would this impact the L4 health checks ? Server tough is capable of
receiving new requests.

For this test I am using default values for tcp health check timeouts.

Thanks
 Badari


[PATCH] wurfl device detection fixes

2019-05-14 Thread Massimiliano Bellomi
Hi All.

Here attached you may find a set of patches related to WURFL module.

Patches from 0001 to 0004 should implements Christopher's last
suggestions/issues.

   - segfault when I try to retrieve an unknown data (I mean not listed in
   wurfl-information-list).
   - the channel validity must be checked calling the macro
   CHECK_HTTP_MESSAGE_FIRST().
   - the function ha_wurfl_retrieve_header() is not HTX aware
   - It could be cool to call ha_wurfl_retrieve_header() from the dummy
   library, in wurfl_lookup().

Patches from 0005 to 0008 corrects some small issues found during this
activity.

Thank you in advance for your feedback. ( ...and thank you Willy for your
clarifications on send_log() )

Regards
-Max

-- 
Massimiliano Bellomi
Senior Software Engineer
Scientiamobile Italy -  massimili...@scientiamobile.com +39 338 6990288
Milano Office : +39 02 620227260
skype: massimiliano.bellomi
From fab346cee058d443c32d010a3f6ad0a864ebf82d Mon Sep 17 00:00:00 2001
From: mbellomi 
Date: Thu, 9 May 2019 16:36:12 +0200
Subject: [PATCH 1/8] BUG/MEDIUM WURFL fixed segfault in reference to information
 not present in wurfl-information-list

---
 src/wurfl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/wurfl.c b/src/wurfl.c
index 325cba64..a53209c8 100644
--- a/src/wurfl.c
+++ b/src/wurfl.c
@@ -514,9 +514,9 @@ static int ha_wurfl_get(const struct arg *args, struct sample *smp, const char *
 	while (args[i].data.str.area) {
 		chunk_appendf(temp, "%c", global_wurfl.information_list_separator);
 		node = ebst_lookup(_wurfl.btree, args[i].data.str.area);
-		wn = container_of(node, wurfl_data_t, nd);
 
-		if (wn) {
+		if (node) {
+			wn = container_of(node, wurfl_data_t, nd);
 
 			switch(wn->type) {
 			case HA_WURFL_DATA_TYPE_UNKNOWN :
-- 
2.17.1

From 028b5e4bf5ae4f179e87be5734f319abf6715181 Mon Sep 17 00:00:00 2001
From: mbellomi 
Date: Mon, 13 May 2019 15:33:44 +0200
Subject: [PATCH 2/8] MINOR WURFL checks channel validity in fetch functions

---
 src/wurfl.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/src/wurfl.c b/src/wurfl.c
index a53209c8..40ea1b53 100644
--- a/src/wurfl.c
+++ b/src/wurfl.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -433,6 +434,9 @@ static int ha_wurfl_get_all(const struct arg *args, struct sample *smp, const ch
 	wurfl_information_t *wi;
 	ha_wurfl_header_t wh;
 
+	struct channel *chn = (smp->strm ? >strm->req : NULL);
+	CHECK_HTTP_MESSAGE_FIRST(chn);
+
 	ha_wurfl_log("WURFL: starting ha_wurfl_get_all\n");
 	wh.wsmp = smp;
 	dHandle = wurfl_lookup(global_wurfl.handle, _wurfl_retrieve_header, );
@@ -487,6 +491,7 @@ static int ha_wurfl_get_all(const struct arg *args, struct sample *smp, const ch
 	wurfl_device_destroy(dHandle);
 	smp->data.u.str.area = temp->area;
 	smp->data.u.str.data = temp->data;
+	smp->data.type = SMP_T_STR;
 	return 1;
 }
 
@@ -499,6 +504,9 @@ static int ha_wurfl_get(const struct arg *args, struct sample *smp, const char *
 	ha_wurfl_header_t wh;
 	int i = 0;
 
+	struct channel *chn = (smp->strm ? >strm->req : NULL);
+	CHECK_HTTP_MESSAGE_FIRST(chn);
+
 	ha_wurfl_log("WURFL: starting ha_wurfl_get\n");
 	wh.wsmp = smp;
 	dHandle = wurfl_lookup(global_wurfl.handle, _wurfl_retrieve_header, );
@@ -563,6 +571,7 @@ static int ha_wurfl_get(const struct arg *args, struct sample *smp, const char *
 	wurfl_device_destroy(dHandle);
 	smp->data.u.str.area = temp->area;
 	smp->data.u.str.data = temp->data;
+	smp->data.type = SMP_T_STR;
 	return 1;
 }
 
-- 
2.17.1

From 08295dec46f256332bb6f49d5ab28f4e9f77e41f Mon Sep 17 00:00:00 2001
From: mbellomi 
Date: Mon, 13 May 2019 15:40:53 +0200
Subject: [PATCH 3/8] MINOR WURFL makes ha_wurfl_retrieve_header() HTX aware

---
 src/wurfl.c | 67 +++--
 1 file changed, 55 insertions(+), 12 deletions(-)

diff --git a/src/wurfl.c b/src/wurfl.c
index 40ea1b53..c6fb26eb 100644
--- a/src/wurfl.c
+++ b/src/wurfl.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -688,25 +689,67 @@ static const char *(*ha_wurfl_get_property_callback(char *name)) (wurfl_handle w
 static const char *ha_wurfl_retrieve_header(const char *header_name, const void *wh)
 {
 	struct sample *smp;
-	struct hdr_idx *idx;
-	struct hdr_ctx ctx;
-	const struct http_msg *msg;
+	struct channel *chn;
 	int header_len = HA_WURFL_MAX_HEADER_LENGTH;
 
-	ha_wurfl_log("WURFL: retrieve header request [%s]\n", header_name);
 	smp =  ((ha_wurfl_header_t *)wh)->wsmp;
-	idx = >strm->txn->hdr_idx;
-	msg = >strm->txn->req;
-	ctx.idx = 0;
+	chn = (smp->strm ? >strm->req : NULL);
 
-	if (http_find_full_header2(header_name, strlen(header_name), ci_head(msg->chn), idx, ) == 0)
-		return 0;
+	if (smp->px->options2 & PR_O2_USE_HTX) {
+		/* HTX version */
+		struct htx *htx;
+		struct http_hdr_ctx ctx;
+		struct ist name;
 
-	if (header_len > ctx.vlen)
-		header_len = ctx.vlen;
+		ha_wurfl_log("WURFL: 

Re: [PATCH 1/2] BUG/MINOR: mworker: Prevent potential use-after-free in mworker_env_to_proc_list

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

Am 14.05.19 um 11:40 schrieb William Lallemand:
> Sorry, I'm only reading this mail and I already fixed this one in the master!
> 

All good, at least it's fixed. Take a look at the other patch (the
memory leak), though!

Best regards
Tim Düsterhus



Re: [PATCH 1/2] BUG/MINOR: mworker: Prevent potential use-after-free in mworker_env_to_proc_list

2019-05-14 Thread William Lallemand
Hi Tim,

On Mon, May 13, 2019 at 02:37:24PM +0200, Tim Duesterhus wrote:
> This was found by reading the code while investigating issue #96 and not
> verified with any tools:
> 
> If `child->pid` is falsy `child` will be freed instead of being added to
> `proc_list`. The setting of `PROC_O_LEAVING` happens unconditionally after
> this check.
> 
> Fix the issue by mising the setting of the LEAVING option right behind the
> allocation of `child`.
> 
> This bug was introduced in 4528611ed66d8bfa344782f6c7f1e7151cf48bf5, which
> is specific to the 2.0-dev branch. No backport required.
 
Sorry, I'm only reading this mail and I already fixed this one in the master!

-- 
William Lallemand



Did you know your society can be made more secure and convenient without any additional infrastructure?

2019-05-14 Thread Rohit Jindal