]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
5 years agoBUG/MEDIUM: ssl: fix several bad pointer aliases in a few sample fetch functions
Willy Tarreau [Tue, 25 Feb 2020 07:59:23 +0000 (08:59 +0100)] 
BUG/MEDIUM: ssl: fix several bad pointer aliases in a few sample fetch functions

Sample fetch functions ssl_x_sha1(), ssl_fc_npn(), ssl_fc_alpn(),
ssl_fc_session_id(), as well as the CLI's "show cert details" handler
used to dereference the output buffer's <data> field by casting it to
"unsigned *". But while doing this could work before 1.9, it broke
starting with commit 843b7cbe9d ("MEDIUM: chunks: make the chunk struct's
fields match the buffer struct") which merged chunks and buffers, causing
the <data> field to become a size_t. The impact is only on 64-bit platform
and depends on the endianness: on little endian, there should never be any
non-zero bits in the field as it is supposed to have been zeroed before the
call, so it shouldbe harmless; on big endian, the high part of the value
only is written instead of the lower one, often making the result appear
4 billion times larger, and making such values dropped everywhere due to
being larger than a buffer.

It seems that it would be wise to try to re-enable strict-aliasing to
catch such errors.

This must be backported till 1.9.

5 years agoBUG/MINOR: sample: fix the json converter's endian-sensitivity
Willy Tarreau [Tue, 25 Feb 2020 07:37:37 +0000 (08:37 +0100)] 
BUG/MINOR: sample: fix the json converter's endian-sensitivity

About every time there's a pointer cast in the code, there's a hidden
bug, and this one was no exception, as it passes the first octet of the
native representation of an integer as a single-character string, which
obviously only works on little endian machines. On big-endian machines,
something as simple as "str(foo),json" only returns zeroes.

This bug was introduced with the JSON converter in 1.6-dev1 by commit
317e1c4f1e ("MINOR: sample: add "json" converter"), the fix may be
backported to all stable branches.

5 years agoBUILD: general: always pass unsigned chars to is* functions
Willy Tarreau [Tue, 25 Feb 2020 07:16:33 +0000 (08:16 +0100)] 
BUILD: general: always pass unsigned chars to is* functions

The isalnum(), isalpha(), isdigit() etc functions from ctype.h are
supposed to take an int in argument which must either reflect an
unsigned char or EOF. In practice on some platforms they're implemented
as macros referencing an array, and when passed a char, they either cause
a warning "array subscript has type 'char'" when lucky, or cause random
segfaults when unlucky. It's quite unconvenient by the way since none of
them may return true for negative values. The recent introduction of
cygwin to the list of regularly tested build platforms revealed a lot
of breakage there due to the same issues again.

So this patch addresses the problem all over the code at once. It adds
unsigned char casts to every valid use case, and also drops the unneeded
double cast to int that was sometimes added on top of it.

It may be backported by dropping irrelevant changes if that helps better
support uncommon platforms. It's unlikely to fix bugs on platforms which
would already not emit any warning though.

5 years agoBUILD: ssl: only pass unsigned chars to isspace()
Willy Tarreau [Tue, 25 Feb 2020 06:51:59 +0000 (07:51 +0100)] 
BUILD: ssl: only pass unsigned chars to isspace()

A build failure on cygwin was reported on github actions here:

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

It's caused by a signed char being passed to isspace(), and this one
being implemented as a macro instead of a function as the man page
suggests. It's the same issue that regularly pops up on Solaris. This
comes from commit 98263291cc3 which was merged in 1.8-dev1. A backport
is possible though not incredibly useful.

5 years agoCLEANUP: cfgparse: Fix type of second calloc() parameter
Tim Duesterhus [Sat, 22 Feb 2020 15:39:05 +0000 (16:39 +0100)] 
CLEANUP: cfgparse: Fix type of second calloc() parameter

`curr_idle_thr` is of type `unsigned int`, not `int`. Fix this issue by
taking the size of the dereferenced `curr_idle_thr` array.

This issue was introduced when adding the `curr_idle_thr` struct member
in commit f131481a0af79037bc6616edf450ae81d80084d7. This commit is first
tagged in 2.0-dev1 and marked for backport to 1.9.

5 years agoBUILD: remove obsolete support for -mregparm / USE_REGPARM
Willy Tarreau [Tue, 25 Feb 2020 06:38:05 +0000 (07:38 +0100)] 
BUILD: remove obsolete support for -mregparm / USE_REGPARM

This used to be a minor optimization on ix86 where registers are scarce
and the calling convention not very efficient, but this platform is not
relevant enough anymore to warrant all this dirt in the code for the sake
of saving 1 or 2% of performance. Modern platforms don't use this at all
since their calling convention already defaults to using several registers
so better get rid of this once for all.

5 years agoCLEANUP: net_helper: Do not negate the result of unlikely
Tim Duesterhus [Fri, 21 Feb 2020 12:02:04 +0000 (13:02 +0100)] 
CLEANUP: net_helper: Do not negate the result of unlikely

This patch turns the double negation of 'not unlikely' into 'likely'
and then turns the negation of 'not smaller' into 'greater or equal'
in an attempt to improve readability of the condition.

[wt: this was not a bug but purposely written like this to improve code
 generation on older compilers but not needed anymore as described here:
 https://www.mail-archive.com/haproxy@formilux.org/msg36392.html ]

5 years agoCLEANUP: conn: Do not pass a pointer to likely
Tim Duesterhus [Fri, 21 Feb 2020 12:02:03 +0000 (13:02 +0100)] 
CLEANUP: conn: Do not pass a pointer to likely

Move the `!` inside the likely and negate it to unlikely.

The previous version should not have caused issues, because it is converted
to a boolean / integral value before being passed to __builtin_expect(), but
it's certainly unusual.

[wt: this was not a bug but purposely written like this to improve code
 generation on older compilers but not needed anymore as described here:
 https://www.mail-archive.com/haproxy@formilux.org/msg36392.html ]

5 years agoMINOR: compiler: drop special cases of likely/unlikely for older compilers
Willy Tarreau [Tue, 25 Feb 2020 06:14:43 +0000 (07:14 +0100)] 
MINOR: compiler: drop special cases of likely/unlikely for older compilers

We used to special-case the likely()/unlikely() macros for a series of
early gcc 4.x compilers which used to produce very bad code when using
__builtin_expect(x,1), which basically used to build an integer (0 or 1)
from a condition then compare it to integer 1. This was already fixed in
5.x, but even now, looking at the code produced by various flavors of 4.x
this bad behavior couldn't be witnessed anymore. So let's consider it as
fixed by now, which will allow to get rid of some ugly tricks at some
specific places. A test on 4.7.4 shows that the code shrinks by about 3kB
now, thanks to some tests being inlined closer to the call place and the
unlikely case being moved to real functions. See the link below for more
background on this.

Link: https://www.mail-archive.com/haproxy@formilux.org/msg36392.html
5 years agoBUG/MINOR: ssl: load .key in a directory only after PEM
William Lallemand [Mon, 24 Feb 2020 15:30:12 +0000 (16:30 +0100)] 
BUG/MINOR: ssl: load .key in a directory only after PEM

Don't try to load a .key in a directory without loading its associated
certificate file.

This patch ignores the .key files when iterating over the files in a
directory.

Introduced by 4c5adbf ("MINOR: ssl: load the key from a dedicated
file").

5 years agoMINOR: ssl: load the key from a dedicated file
William Lallemand [Mon, 24 Feb 2020 13:23:22 +0000 (14:23 +0100)] 
MINOR: ssl: load the key from a dedicated file

For a certificate on a bind line, if the private key was not found in
the PEM file, look for a .key and load it.

This default behavior can be changed by using the ssl-load-extra-files
directive in the global section

This feature was mentionned in the issue #221.

5 years agoBUILD: fix recent build failure on unaligned archs
Willy Tarreau [Fri, 21 Feb 2020 16:40:25 +0000 (17:40 +0100)] 
BUILD: fix recent build failure on unaligned archs

Last commit 2c315ee ("BUG/MEDIUM: ebtree: don't set attribute packed
without unaligned access support") accidently enclosed the semicolon
in the ifdef so it will fail when HA_UNALIGNED is not set.

5 years agoCLEANUP: http/h1: rely on HA_UNALIGNED_LE instead of checking for CPU families
Willy Tarreau [Fri, 21 Feb 2020 15:31:22 +0000 (16:31 +0100)] 
CLEANUP: http/h1: rely on HA_UNALIGNED_LE instead of checking for CPU families

Now that we have flags indicating the CPU's capabilities, better use
them instead of missing some updates for new CPU families (ARMv8 was
missing there).

5 years agoBUG/MEDIUM: ebtree: don't set attribute packed without unaligned access support
Willy Tarreau [Fri, 21 Feb 2020 14:47:36 +0000 (15:47 +0100)] 
BUG/MEDIUM: ebtree: don't set attribute packed without unaligned access support

An alignment issue on Sparc64 with ebtrees was reported in early 2017
here https://www.mail-archive.com/haproxy@formilux.org/msg25937.html and
a similar one was finally reported in issue #512.

The problem has its roots in the fact that 64-bit keys will end up being
unaligned on such archs which do not support unaligned accesses. But on
most platforms supporting unaligned accesses, dealing with smaller nodes
results in better performance.

One of the possible problems caused by attribute packed there is that it
promotes a structure both to be unaligned and unpadded, which may come
with fun if some fields of the struct itself are accessed and such a node
is placed at an unaligned location. It's not a problem for regular unaligned
accesses but may become one for atomic operations such as the CAS on leaf_p
that's used in struct task. In practice we know that this struct is properly
aligned and is a very edge case so this patch adds comments there to remind
to be careful about it.

This patch depends on previous patch "MINOR: compiler: move CPU capabilities
definition from config.h and complete them" and could be backported to all
stable branches. It fixes issues #512 and #9.

5 years agoMINOR: compiler: move CPU capabilities definition from config.h and complete them
Willy Tarreau [Fri, 21 Feb 2020 14:40:58 +0000 (15:40 +0100)] 
MINOR: compiler: move CPU capabilities definition from config.h and complete them

These ones are irrelevant to the config but rather to the platform, and
as such are better placed in compiler.h.

Here we take the opportunity for declaring a few extra capabilities:
 - HA_UNALIGNED         : CPU supports unaligned accesses
 - HA_UNALIGNED_LE      : CPU supports unaligned accesses in little endian
 - HA_UNALIGNED_FAST    : CPU supports fast unaligned accesses
 - HA_UNALIGNED_ATOMIC  : CPU supports unaligned accesses in atomics

This will help remove a number of #ifdefs with arch-specific statements.

5 years agoBUG/MEDIUM: shctx: make sure to keep all blocks aligned
Willy Tarreau [Fri, 21 Feb 2020 12:45:58 +0000 (13:45 +0100)] 
BUG/MEDIUM: shctx: make sure to keep all blocks aligned

The blocksize and the extra field are not necessarily aligned on a
machine word. This can result in crashing an align-sensitive machine
when initializing the shctx area. Let's round both sizes up to a pointer
size to make this safe everywhere.

This fixes issue #512. This should be backported as far as 1.8.

5 years agoCLEANUP: sample: use iststop instead of a for loop
Jerome Magnin [Fri, 21 Feb 2020 09:49:12 +0000 (10:49 +0100)] 
CLEANUP: sample: use iststop instead of a for loop

In sample_fetch_path we can use iststop() instead of a for loop
to find the '?' and return the correct length. This requires commit
"MINOR: ist: add an iststop() function".

5 years agoBUG/MINOR: http: http-request replace-path duplicates the query string
Jerome Magnin [Fri, 21 Feb 2020 09:37:48 +0000 (10:37 +0100)] 
BUG/MINOR: http: http-request replace-path duplicates the query string

In http_action_replace_uri() we call http_get_path() in the case of
a replace-path rule. http_get_path() will return an ist pointing to
the start of the path, but uri.ptr + uri.len points to the end of the
uri. As as result, we are matching against a string containing the
query, which we append to the "path" later, effectively duplicating
the query string.

This patch uses the iststop() function introduced in "MINOR: ist: add
an iststop() function" to find the '?' character and update the ist
length when needed.

This fixes issue #510.

The bug was introduced by commit 262c3f1a ("MINOR: http: add a new
"replace-path" action"), which was backported to 2.1 and 2.0.

5 years agoMINOR: ist: add an iststop() function
Jerome Magnin [Fri, 21 Feb 2020 09:33:12 +0000 (10:33 +0100)] 
MINOR: ist: add an iststop() function

Add a function that finds a character in an ist and returns an
updated ist with the length of the portion of the original string
that doesn't contain the char.

Might be backported to 2.1

5 years agoMINOR: mux-h1: pass CO_RFL_READ_ONCE to the lower layers when relevant
Willy Tarreau [Thu, 20 Feb 2020 10:11:50 +0000 (11:11 +0100)] 
MINOR: mux-h1: pass CO_RFL_READ_ONCE to the lower layers when relevant

When we're in H1_MSG_RQBEFORE or H1_MSG_RPBEFORE, we know that the
first message is highly likely the only one and that it's pointless
to try to perform a second recvfrom() to complete a first partial
read. This is similar to what used to be done in the older I/O methods
with the CF_READ_DONTWAIT flag on the channel. So let's pass
CO_RFL_READ_ONCE to the transport layer during rcv_buf() in this case.

By doing so, in a test involving keep-alive connections with a non-null
client think time, we remove 20% of the recvfrom() calls, all of which
used to systematically fail. More precisely, we observe a drop from 5.0
recvfrom() per request with 60% failure to 4.0 per request with 50%
failure.

5 years agoMINOR: connection: introduce a new receive flag: CO_RFL_READ_ONCE
Willy Tarreau [Thu, 20 Feb 2020 10:04:40 +0000 (11:04 +0100)] 
MINOR: connection: introduce a new receive flag: CO_RFL_READ_ONCE

This flag is currently supported by raw_sock to perform a single recv()
attempt and avoid subscribing. Typically on the request and response
paths with keep-alive, with short messages we know that it's very likely
that the first message is enough.

5 years agoCLEANUP: connection: remove the definitions of conn_xprt_{stop,want}_{send,recv}
Willy Tarreau [Fri, 21 Feb 2020 08:58:29 +0000 (09:58 +0100)] 
CLEANUP: connection: remove the definitions of conn_xprt_{stop,want}_{send,recv}

This marks the end of the transition from the connection polling states
introduced in 1.5-dev12 and the subscriptions in that arrived in 1.9.
The socket layer can now safely use its FD while all upper layers rely
exclusively on subscriptions. These old functions were removed. Some may
deserve some renaming to improved clarty though. The single call to
conn_xprt_stop_both() was dropped in favor of conn_cond_update_polling()
which already does the same.

5 years agoMINOR: connection: remove the last calls to conn_xprt_{want,stop}_*
Willy Tarreau [Fri, 21 Feb 2020 09:34:19 +0000 (10:34 +0100)] 
MINOR: connection: remove the last calls to conn_xprt_{want,stop}_*

The last few calls to conn_xprt_{want,stop}_{recv,send} in the central
connection code were replaced with their strictly exact equivalent fd_*,
adding the call to conn_ctrl_ready() when it was missing.

5 years agoMINOR: tcp/uxst/sockpair: use fd_want_send() instead of conn_xprt_want_send()
Willy Tarreau [Fri, 21 Feb 2020 09:24:51 +0000 (10:24 +0100)] 
MINOR: tcp/uxst/sockpair: use fd_want_send() instead of conn_xprt_want_send()

Just like previous commit, we don't need to pass through the connection
layer anymore to enable polling during a connect(), we know the FD, so
let's simply call fd_want_send().

5 years agoMINOR: raw_sock: directly call fd_stop_send() and not conn_xprt_stop_send()
Willy Tarreau [Fri, 21 Feb 2020 09:21:46 +0000 (10:21 +0100)] 
MINOR: raw_sock: directly call fd_stop_send() and not conn_xprt_stop_send()

Now that we know that the connection layer is transparent for polling
changes, we have no reason for hiding behind conn_xprt_stop_send() and
can safely call fd_stop_send() on the FD once the buffer is empty.

5 years agoMEDIUM: connection: remove the intermediary polling state from the connection
Willy Tarreau [Fri, 21 Feb 2020 07:46:19 +0000 (08:46 +0100)] 
MEDIUM: connection: remove the intermediary polling state from the connection

Historically we used to require that the connections held the desired
polling states for the data layer and the socket layer. Then with muxes
these were more or less merged into the transport layer, and now it
happens that with all transport layers having their own state, the
"transport layer state" as we have it in the connection (XPRT_RD_ENA,
XPRT_WR_ENA) is only an exact copy of the undelying file descriptor
state, but with a delay. All of this is causing some difficulties at
many places in the code because there are still some locations which
use the conn_want_* API to remain clean and only rely on connection,
and count on a later collection call to conn_cond_update_polling(),
while others need an immediate action and directly use the FD updates.

Since our updates are now much cheaper, most of them being only an
atomic test-and-set operation, and since our I/O callbacks are deferred,
there's no benefit anymore in trying to "cache" the transient state
change in the connection flags hoping to cancel them before they
become an FD event. Better make such calls transparent indirections
to the FD layer instead and get rid of the deferred operations which
needlessly complicate the logic inside.

This removes flags CO_FL_XPRT_{RD,WR}_ENA and CO_FL_WILL_UPDATE.
A number of functions related to polling updates were either greatly
simplified or removed.

Two places were using CO_FL_XPRT_WR_ENA as a hint to know if more data
were expected to be sent after a PROXY protocol or SOCKSv4 header. These
ones were simply replaced with a check on the subscription which is
where we ought to get the autoritative information from.

Now the __conn_xprt_want_* and their conn_xprt_want_* counterparts
are the same. conn_stop_polling() and conn_xprt_stop_both() are the
same as well. conn_cond_update_polling() only causes errors to stop
polling. It also becomes way more obvious that muxes should not at
all employ conn_xprt_{want|stop}_{recv,send}(), and that the call
to __conn_xprt_stop_recv() in case a mux failed to allocate a buffer
is inappropriate, it ought to unsubscribe from reads instead. All of
this definitely requires a serious cleanup.

5 years agoCLEANUP: epoll: place the struct epoll_event in the stack
Willy Tarreau [Thu, 20 Feb 2020 10:23:43 +0000 (11:23 +0100)] 
CLEANUP: epoll: place the struct epoll_event in the stack

Historically we used to have a global epoll_event for various
manipulations involving epoll_ctl() and when threads were added,
this was turned to a thread_local, which is needlessly expensive
since it's just a temporary variable. Let's move it to a local
variable wherever it's called instead.

5 years agoMINOR: checks: do not call conn_xprt_stop_send() anymore
Willy Tarreau [Fri, 21 Feb 2020 09:13:03 +0000 (10:13 +0100)] 
MINOR: checks: do not call conn_xprt_stop_send() anymore

While trying to address issue #253, Commit 5909380c ("BUG/MINOR: checks:
stop polling for write when we have nothing left to send") made sure that
we stop polling for writes when the buffer is empty. This was actually
more a workaround than a bug fix because by doing so we may be stopping
polling for an intermediary transport layer without acting on the check
itself in case there's SSL or send-proxy in the chain for example, thus
the approach is wrong. In practice due to the small size of check
requests, this will not have any impact. At best, we ought to unsubscribe
for sending, but that's already the case when we arrive in this function.
But given that the root cause of the issue was addressed later in commits
cc705a6bc5940392 and ccf3f6d1, we can now safely revert this change.

It was confirmed on the faulty config that this change doesn't have any
effect anymore on the test.

5 years agoBUG/MINOR: mux: do not call conn_xprt_stop_recv() on buffer shortage
Willy Tarreau [Fri, 21 Feb 2020 08:44:39 +0000 (09:44 +0100)] 
BUG/MINOR: mux: do not call conn_xprt_stop_recv() on buffer shortage

In H1/H2/FCGI, the *_get_buf() functions try to disable receipt of data
when there's no buffer available. But they do so at the lowest possible
level, which is unrelated to the upper transport layers which may still
be trying to feed data based on subscription. The correct approach here
would theorically be to only disable subscription, though when we get
there, the subscription will already have been dropped, so we can safely
just remove that call.

It's unlikely that this could have had any practical impact, as the upper
xprt layer would call this callback which would fail an not resubscribe.
Having the lowest layer disabled would just be temporary since when
re-enabling reading, a subscribe at the end of data would re-enable it.

Backport should not harm but seems useless at this point.

5 years agoBUG/MAJOR: http-ana: Always abort the request when a tarpit is triggered
Christopher Faulet [Fri, 21 Feb 2020 09:20:46 +0000 (10:20 +0100)] 
BUG/MAJOR: http-ana: Always abort the request when a tarpit is triggered

If an client error is reported on the request channel (CF_READ_ERROR) while a
session is tarpitted, no error is returned to the client. Concretly,
http_reply_and_close() function is not called. This function is reponsible to
forward the error to the client. But not only. It is also responsible to abort
the request. Because this function is not called when a read error is reported
on the request channel, and because the tarpit analyzer is the last one, there
is nothing preventing a connection attempt on a server while it is totally
unexpected.

So, a useless connexion on a backend server may be performed because of this
bug. If an HTTP load-balancing algorithm is used on the backend side, it leads
to a crash of HAProxy because the request was already erased.

If you have tarpit rules and if you use an HTTP load-balancing algorithm on your
backends, you must apply this patch. Otherwise a simple TCP reset on a tarpitted
connexion will most likely crash your HAProxy. A safe workaround is to use a
silent-drop rule or a deny rule instead of a tarpit.

This bug also affect the legacy code. It is in fact an very old hidden bug. But
the refactoring of process_stream() in the 1.9 makes it visible. And,
unfortunately, with the HTX, it is easier to hit it because many processing has
been moved in lower layers, in the muxes.

It must be backported as far as 1.9. For the 2.0 and the 1.9, the legacy HTTP
code must also be patched the same way. For older versions, it may be backported
but the bug seems to not impact them.

Thanks to Olivier D <webmaster@ajeux.com> to have reported the bug and provided
all the infos to analyze it.

5 years agoBUG/MINOR: ssl: Stop passing dynamic strings as format arguments
Tim Duesterhus [Wed, 19 Feb 2020 10:41:13 +0000 (11:41 +0100)] 
BUG/MINOR: ssl: Stop passing dynamic strings as format arguments

gcc complains rightfully:

src/ssl_sock.c: In function â€˜ssl_load_global_issuers_from_path’:
src/ssl_sock.c:9860:4: warning: format not a string literal and no format arguments [-Wformat-security]
    ha_warning(warn);
    ^

Introduced in 70df7bf19cebd5593c0abb01923e6c9f72961da6.

5 years agoMINOR: http-ana: Match on the path if the monitor-uri starts by a /
Christopher Faulet [Tue, 18 Feb 2020 14:34:58 +0000 (15:34 +0100)] 
MINOR: http-ana: Match on the path if the monitor-uri starts by a /

if the monitor-uri starts by a slash ('/'), the matching is performed against
the request's path instead of the request's uri. It is a workaround to let the
HTTP/2 requests match the monitor-uri. Indeed, in HTTP/2, clients are encouraged
to send absolute URIs only.

This patch is not tagged as a bug, because the previous behavior matched exactly
what the doc describes. But it may surprise that HTTP/2 requests don't match the
monitor-uri.

This patch may be backported to 2.1 because URIs of HTTP/2 are stored using the
absolute-form starting this version. For previous versions, this patch will only
helps explicitely absolute HTTP/1 requests (and only the HTX part because on the
legacy HTTP, all the URI is matched).

It should fix the issue #509.

5 years agoBUG/MINOR: http-ana: Matching on monitor-uri should be case-sensitive
Christopher Faulet [Tue, 18 Feb 2020 13:51:51 +0000 (14:51 +0100)] 
BUG/MINOR: http-ana: Matching on monitor-uri should be case-sensitive

The monitor-uri should be case-sensitive. In reality, the scheme and the host
part are case-insensitives and only the path is case-sensive. But concretely,
since the start, the matching on the monitor-uri is case-sensitive. And it is
probably the expected behavior of almost all users.

This patch must be backported as far as 1.9. For HAProxy 2.0 and 1.9, it must be
applied on src/proto_htx.c.

5 years agoREGTESTS: use "command -v" instead of "which"
Willy Tarreau [Tue, 18 Feb 2020 13:42:33 +0000 (14:42 +0100)] 
REGTESTS: use "command -v" instead of "which"

Ilya reported that the "which" utility is not that much portable and is
absent from Fedora. "type -p" is not portable either, and the correct
solution appears to be "command -v", so let's use this for now, we can
change it again in the future in case of problems.

Link: https://www.mail-archive.com/haproxy@formilux.org/msg36332.html
5 years agoMINOR: ssl: add "issuers-chain-path" directive.
Emmanuel Hocdet [Fri, 4 Jan 2019 10:08:20 +0000 (11:08 +0100)] 
MINOR: ssl: add "issuers-chain-path" directive.

Certificates loaded with "crt" and "crt-list" commonly share the same
intermediate certificate in PEM file. "issuers-chain-path" is a global
directive to share intermediate chain certificates in a directory. If
certificates chain is not included in certificate PEM file, haproxy
will complete chain if issuer match the first certificate of the chain
stored via "issuers-chain-path" directive. Such chains will be shared
in memory.

5 years agoBUG/MINOR: sample: exit regsub() in case of trash allocation error
Willy Tarreau [Tue, 18 Feb 2020 13:27:44 +0000 (14:27 +0100)] 
BUG/MINOR: sample: exit regsub() in case of trash allocation error

As reported in issue #507, since commiy 07e1e3c93e ("MINOR: sample:
regsub now supports backreferences") we must not proceed in regsub()
if we fali to allocate a trash (which in practice never happens). No
backport needed.

5 years agoBUG/MINOR: stream: Don't incr frontend cum_req counter when stream is closed
Christopher Faulet [Tue, 18 Feb 2020 10:51:11 +0000 (11:51 +0100)] 
BUG/MINOR: stream: Don't incr frontend cum_req counter when stream is closed

This counter is already incremented when a new request is received (or if an
error occurred waiting it). So it must not be incremented when the stream is
terminated, at the end of process_strem(). This bug was introduced by the commit
cff0f739e ("MINOR: counters: Review conditions to increment counters from
analysers").

No backport needed.

5 years agoBUG/MINOR: http-htx: Don't return error if authority is updated without changes
Christopher Faulet [Tue, 18 Feb 2020 10:02:21 +0000 (11:02 +0100)] 
BUG/MINOR: http-htx: Don't return error if authority is updated without changes

When an Host header is updated, the autority part, if any, is also updated to
keep the both syncrhonized. But, when the update is performed while there is no
change, a failure is reported while, in reality, no update is necessary. This
bug was introduced by the commit d7b7a1ce5 ("MEDIUM: http-htx: Keep the Host
header and the request start-line synchronized").

This commit was pushed in the 2.1. But on this version, the bug is hidden
because rewrite errors are silently ignored. And because it happens when there
is no change, if the rewrite fails, noone notices it. But since the 2.2, rewrite
errors are now fatals by default. So when the bug is hit, a 500 error is
returned to the client. Without this fix, a workaround is to disable the strict
rewriting mode (see the "strict-mode" HTTP rule).

The following HTTP rule is a good way to reproduce the bug if a request with an
authority is received. In HTT2, it is pretty common.

    acl host_header_exists req.hdr(host) -m found
    http-request set-header host %[req.hdr(host)] if host_header_exists

This patch must be backported to 2.1 and everywhere the commit d7b7a1ce5 is
backported. It should fix the issue #494.

5 years agoBUG/MINOR: filters: Count HTTP headers as filtered data but don't forward them
Christopher Faulet [Wed, 12 Feb 2020 14:31:20 +0000 (15:31 +0100)] 
BUG/MINOR: filters: Count HTTP headers as filtered data but don't forward them

In flt_analyze_http_headers() HTTP analyzer, we must not forward systematically
the headers. We must only count them as filtered data (ie. increment the offset
of the right size). It is the http_payload callback responsibility to decide to
forward headers or not by forwarding at least 1 byte of payload. And there is
always at least 1 byte of payload to forward, the EOM block.

This patch depends on following commits:

 * MINOR: filters: Forward data only if the last filter forwards something
 * MINOR: http-htx: Add a function to retrieve the headers size of an HTX message

This patch must be backported with commits above as far as 1.9. In HAProxy 2.0
and 1.9, the patch must be adapted because of the legacy HTTP code.

5 years agoMINOR: filters: Forward data only if the last filter forwards something
Christopher Faulet [Fri, 7 Feb 2020 15:40:33 +0000 (16:40 +0100)] 
MINOR: filters: Forward data only if the last filter forwards something

In flt_tcp_payload() and flt_http_payload(), if the last filter does not
forwarding anything, nothing is forwarded, not even the already filtered
data. For now, this patch is useless because the last filter is always sync with
the stream's offset. But it will be mandatory for a bugfix.

5 years agoMINOR: http-htx: Add a function to retrieve the headers size of an HTX message
Christopher Faulet [Fri, 7 Feb 2020 15:39:41 +0000 (16:39 +0100)] 
MINOR: http-htx: Add a function to retrieve the headers size of an HTX message

http_get_hdrs_size() function may now be used to get the bytes held by headers
in an HTX message. It only works if the headers were not already
forwarded. Metadata are not counted here.

5 years agoBUG/MINOR: tools: also accept '+' as a valid character in an identifier
Willy Tarreau [Mon, 17 Feb 2020 05:34:11 +0000 (06:34 +0100)] 
BUG/MINOR: tools: also accept '+' as a valid character in an identifier

The function is_idchar() was added by commit 36f586b ("MINOR: tools:
add is_idchar() to tell if a char may belong to an identifier") to
ease matching of sample fetch/converter names. But it lacked support
for the '+' character used in "base32+src" and "url32+src". A quick
way to figure the list of supported sample fetch+converter names is
to issue the following command:

   git grep '"[^"]*",.*SMP_T_.*SMP_USE_'|cut -f2 -d'"'|sort -u

No more entry is reported once searching for characters not covered
by is_idchar().

No backport is needed.

5 years agoMINOR: sample: regsub now supports backreferences
Jerome Magnin [Sun, 16 Feb 2020 18:20:19 +0000 (19:20 +0100)] 
MINOR: sample: regsub now supports backreferences

Now that the configuration parser is more flexible with samples,
converters and their arguments, we can leverage this to enable
support for backreferences in regsub.

5 years agoBUG/MINOR: arg: fix again incorrect argument length check
Willy Tarreau [Sun, 16 Feb 2020 09:46:37 +0000 (10:46 +0100)] 
BUG/MINOR: arg: fix again incorrect argument length check

Recent commit 807aef8a14 ("BUG/MINOR: arg: report an error if an argument
is larger than bufsize") aimed at fixing the argument length check but
relied on the fact that the end of string was not reached, except that
it forgot to consider the delimiters (comma and parenthesis) which are
valid conditions to break out of the loop. This used to break simple
expressions like "hdr(xff,1)". Thanks to Jérôme for reporting this.

No backport is needed.

5 years agoBUILD: enable ERR=1 in github cygwin builds
Ilya Shipitsin [Sat, 15 Feb 2020 13:58:22 +0000 (18:58 +0500)] 
BUILD: enable ERR=1 in github cygwin builds

5 years agoSCRIPTS: announce-release: use mutt -H instead of -i to include the draft
Willy Tarreau [Sat, 15 Feb 2020 14:24:28 +0000 (15:24 +0100)] 
SCRIPTS: announce-release: use mutt -H instead of -i to include the draft

Commit 0f5ce6014a ("SCRIPTS: announce-release: place the send command
in the mail's header") broke the announce-release script: by not having
to edit the message at all anymore, mutt does nothing when sending, but
it still does if the message is edited (which was the case before). With
some testing, it appears that mutt -H does work when there's no change,
so let's use this instead. This should be backported till 1.7.

5 years agoBUG/MINOR: arg: report an error if an argument is larger than bufsize
Willy Tarreau [Sat, 15 Feb 2020 13:54:28 +0000 (14:54 +0100)] 
BUG/MINOR: arg: report an error if an argument is larger than bufsize

Commit ef21facd99 ("MEDIUM: arg: make make_arg_list() support quotes
in arguments") removed the chunk_strncpy() to fill the trash buffer
as the input is being parsed, and accidently dropped the jump to the
error path in case the argument is too large, which is now fixed.

No backport is needed, this is for 2.2. This addresses issue #502.

5 years agoMEDIUM: log-format: make the LF parser aware of sample expressions' end
Willy Tarreau [Fri, 14 Feb 2020 16:33:06 +0000 (17:33 +0100)] 
MEDIUM: log-format: make the LF parser aware of sample expressions' end

For a very long time it used to be impossible to pass a closing square
bracket as a valid character in argument to a sample fetch function or
to a converter because the LF parser used to stop on the first such
character found and to pass what was between the first '[' and the first
']' to sample_parse_expr().

This patch addresses this by passing the whole string to sample_parse_expr()
which is the only one authoritative to indicate the first character that
does not belong to the expression. The LF parser then verifies it matches
a ']' or fails. As a result it is finally possible to write rules such as
the following, which is totally valid an unambigous :

    http-request redirect location %[url,regsub([.:/?-],!,g)]
                                                |-----| | |
                                                  arg1  | `---> arg3
                                                        `-----> arg2
                                         |-----------------|
                                              converter
                                     |---------------------|
                                        sample expression
                                   |------------------------|
                                         log-format tag

5 years agoMINOR: sample: make sample_parse_expr() able to return an end pointer
Willy Tarreau [Fri, 14 Feb 2020 15:50:14 +0000 (16:50 +0100)] 
MINOR: sample: make sample_parse_expr() able to return an end pointer

When an end pointer is passed, instead of complaining that a comma is
missing after a keyword, sample_parse_expr() will silently return the
pointer to the current location into this return pointer so that the
caller can continue its parsing. This will be used by more complex
expressions which embed sample expressions, and may even permit to
embed sample expressions into arguments of other expressions.

5 years agoMEDIUM: arg: make make_arg_list() support quotes in arguments
Willy Tarreau [Fri, 14 Feb 2020 12:37:20 +0000 (13:37 +0100)] 
MEDIUM: arg: make make_arg_list() support quotes in arguments

Now it becomes possible to reuse the quotes within arguments, allowing
the parser to distinguish a ',' or ')' that is part of the value from
one which delimits the argument. In addition, ',' and ')' may be escaped
using a backslash. However, it is also important to keep in mind that
just like in shell, quotes are first resolved by the word tokenizer, so
in order to pass quotes that are visible to the argument parser, a second
level is needed, either using backslash escaping, or by using an alternate
type.

For example, it's possible to write this to append a comma:

     http-request add-header paren-comma-paren "%[str('(--,--)')]"

or this:

     http-request add-header paren-comma-paren '%[str("(--,--)")]'

or this:

     http-request add-header paren-comma-paren %[str(\'(--,--)\')]

or this:

     http-request add-header paren-comma-paren %[str(\"(--,--)\")]

or this:

     http-request add-header paren-comma-paren %[str(\"(\"--\',\'--\")\")]

Note that due to the wide use of '\' in front of parenthesis in regex,
the backslash character will purposely *not* escape parenthesis, so that
'\)' placed in quotes is passed verbatim to a regex engine.

5 years agoMEDIUM: arg: copy parsed arguments into the trash instead of allocating them
Willy Tarreau [Fri, 14 Feb 2020 10:34:35 +0000 (11:34 +0100)] 
MEDIUM: arg: copy parsed arguments into the trash instead of allocating them

For each and every argument parsed by make_arg_list(), there was an
strndup() call, just so that we have a trailing zero for most functions,
and this temporary buffer is released afterwards except for strings where
it is kept.

Proceeding like this is not convenient because 1) it performs a huge
malloc/free dance, and 2) it forces to decide upfront where the argument
ends, which is what prevents commas and right parenthesis from being used.

This patch makes the function copy the temporary argument into the trash
instead, so that we avoid the malloc/free dance for most all non-string
args (e.g. integers, addresses, time, size etc), and that we can later
produce the contents on the fly while parsing the input. It adds a length
check to make sure that the argument is not longer than the buffer size,
which should obviously never be the case but who knows what people put in
their configuration.

5 years agoMEDIUM: arg: make make_arg_list() stop after its own arguments
Willy Tarreau [Fri, 14 Feb 2020 07:40:37 +0000 (08:40 +0100)] 
MEDIUM: arg: make make_arg_list() stop after its own arguments

The main problem we're having with argument parsing is that at the
moment the caller looks for the first character looking like an end
of arguments (')') and calls make_arg_list() on the sub-string inside
the parenthesis.

Let's first change the way it works so that make_arg_list() also
consumes the parenthesis and returns the pointer to the first char not
consumed. This will later permit to refine each argument parsing.

For now there is no functional change.

5 years agoMINOR: sample/acl: use is_idchar() to locate the fetch/conv name
Willy Tarreau [Fri, 14 Feb 2020 17:27:10 +0000 (18:27 +0100)] 
MINOR: sample/acl: use is_idchar() to locate the fetch/conv name

Instead of scanning a string looking for an end of line, ')' or ',',
let's only accept characters which are actually valid identifier
characters. This will let the parser know that in %[src], only "src"
is the sample fetch name, not "src]". This was done both for samples
and ACLs since they are the same here.

5 years agoMINOR: chunk: implement chunk_strncpy() to copy partial strings
Willy Tarreau [Fri, 14 Feb 2020 10:31:41 +0000 (11:31 +0100)] 
MINOR: chunk: implement chunk_strncpy() to copy partial strings

This does like chunk_strcpy() except that the maximum string length may
be limited by the caller. A trailing zero is always appended. This is
particularly handy to extract portions of strings to put into the trash
for use with libc functions requiring a nul-terminated string.

5 years agoMINOR: tools: add is_idchar() to tell if a char may belong to an identifier
Willy Tarreau [Fri, 14 Feb 2020 17:25:17 +0000 (18:25 +0100)] 
MINOR: tools: add is_idchar() to tell if a char may belong to an identifier

This function will simply be used to find the end of config identifiers
(proxies, servers, ACLs, sample fetches, converters, etc).

5 years agoMINOR: mux-fcgi: Make the capture of the path-info optional in pathinfo regex
Christopher Faulet [Fri, 14 Feb 2020 15:55:52 +0000 (16:55 +0100)] 
MINOR: mux-fcgi: Make the capture of the path-info optional in pathinfo regex

Now, only one capture is mandatory in the path-info regex, the one matching the
script-name. The path-info capture is optional. Of couse, it must be defined to
fill the PATH_INFO parameter. But it is not mandatory. This way, it is possible
to get the script-name part from the path, excluding the path-info.

This patch is small enough to be backported to 2.1.

5 years agoCLEANUP: ssl: remove unused functions in openssl-compat.h
Ilya Shipitsin [Thu, 13 Feb 2020 09:07:36 +0000 (14:07 +0500)] 
CLEANUP: ssl: remove unused functions in openssl-compat.h

functions SSL_SESSION_get0_id_context, SSL_CTX_get_default_passwd_cb,
SSL_CTX_get_default_passwd_cb_userdata are not used anymore

5 years agoBUG/MINOR: mux-fcgi: Forbid special characters when matching PATH_INFO param
Christopher Faulet [Fri, 14 Feb 2020 13:47:37 +0000 (14:47 +0100)] 
BUG/MINOR: mux-fcgi: Forbid special characters when matching PATH_INFO param

If a regex to match the PATH_INFO parameter is configured, it systematically
fails if a newline or a null character is present in the URL-decoded path. So,
from the moment there is at least a "%0a" or a "%00" in the request path, we
always fail to get the PATH_INFO parameter and all the decoded path is used for
the SCRIPT_NAME parameter.

It is probably not the expected behavior. Because, most of time, these
characters are not expected at all in a path, an error is now triggered when one
of these characters is found in the URL-decoded path before trying to execute
the path_info regex. However, this test is not performed if there is no regex
configured.

Note that in reality, the newline character is only a problem when HAProxy is
complied with pcre or pcre2 library and conversely, the null character is only a
problem for the libc's regex library. But both are always excluded to avoid any
inconsistency depending on compile options.

An alternative, not implemented yet, is to replace these characters by another
one. If someone complains about this behavior, it will be re-evaluated.

This patch must be backported to all versions supporting the FastCGI
applications, so to 2.1 for now.

5 years agoBUG/MEDIUM: muxes: Use the right argument when calling the destroy method.
Olivier Houchard [Fri, 14 Feb 2020 12:23:45 +0000 (13:23 +0100)] 
BUG/MEDIUM: muxes: Use the right argument when calling the destroy method.

When calling the mux "destroy" method, the argument should be the mux
context, not the connection. In a few instances in the mux code, the
connection was used (mainly when the session wouldn't handle the idle
connection, and the server pool was fool), and that could lead to random
segfaults.

This should be backported to 2.1, 2.0, and 1.9

5 years agoBUG/MINOR: namespace: avoid closing fd when socket failed in my_socketat
William Dauchy [Wed, 12 Feb 2020 20:23:20 +0000 (21:23 +0100)] 
BUG/MINOR: namespace: avoid closing fd when socket failed in my_socketat

we cannot return right after socket opening as we need to move back to
the default namespace first

this should fix github issue #500

this might be backported to all version >= 1.6

Fixes: b3e54fe387c7c1 ("MAJOR: namespace: add Linux network namespace
support")
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
5 years agoSCRIPTS: make announce-release executable again
Willy Tarreau [Wed, 12 Feb 2020 17:21:11 +0000 (18:21 +0100)] 
SCRIPTS: make announce-release executable again

I managed to mess up with the file's permission while using a temporary
one during last release, and to backport the non-exec version everywhere.
This can be backported as far as 1.7 now.

5 years agoBUG/MINOR: tcp: don't try to set defaultmss when value is negative
William Dauchy [Wed, 12 Feb 2020 14:53:04 +0000 (15:53 +0100)] 
BUG/MINOR: tcp: don't try to set defaultmss when value is negative

when `getsockopt` previously failed, we were trying to set defaultmss
with -2 value.

this is a followup of github issue #499

this should be backported to all versions >= v1.8

Fixes: 153659f1ae69a1 ("MINOR: tcp: When binding socket, attempt to
reuse one from the old proc.")
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
5 years agoBUILD: scripts/build-ssl.sh: use "uname" instead of ${TRAVIS_OS_NAME}
Ilya Shipitsin [Tue, 11 Feb 2020 09:36:23 +0000 (14:36 +0500)] 
BUILD: scripts/build-ssl.sh: use "uname" instead of ${TRAVIS_OS_NAME}

it is also useful for local builds, when ${TRAVIS_OS_NAME} is not set

5 years agoBUILD: travis-ci: harden builds, add ERR=1 (warning ought to be errors)
Ilya Shipitsin [Tue, 11 Feb 2020 09:31:13 +0000 (14:31 +0500)] 
BUILD: travis-ci: harden builds, add ERR=1 (warning ought to be errors)

5 years agoBUILD: travis-ci: no more allowed failures for openssl-1.0.2
Ilya Shipitsin [Tue, 11 Feb 2020 09:29:41 +0000 (14:29 +0500)] 
BUILD: travis-ci: no more allowed failures for openssl-1.0.2

since 23fb037 (which fixes #429) no need to allow failures
on openssl-1.0.2

5 years agoMINOR: build: add aix72-gcc build TARGET and power{8,9} CPUs
Christian Lachner [Mon, 10 Feb 2020 09:29:13 +0000 (10:29 +0100)] 
MINOR: build: add aix72-gcc build TARGET and power{8,9} CPUs

As haproxy wont build on AIX 7.2 using the old "aix52" TARGET a new
TARGET was introduced which adds two special CFLAGS to prevent the
loading of AIXs xmem.h and var.h. This is done by defining the
corresponding include-guards _H_XMEM and _H_VAR. Without excluding
those headers-files the build fails because of redefinition errors:

1)
  CC      src/mux_fcgi.o
In file included from /usr/include/sys/uio.h:90,
                 from /opt/freeware/lib/gcc/powerpc-ibm-aix7.1.0.0/8.3.0/include-fixed/sys/socket.h:104,
                 from include/common/compat.h:32,
                 from include/common/cfgparse.h:25,
                 from src/mux_fcgi.c:13:
src/mux_fcgi.c:204:13: error: expected ':', ',', ';', '}' or '__attribute__' before '.' token
  struct ist rem_addr;
             ^~~~~~~~

2)
  CC      src/cfgparse-listen.o
In file included from include/types/arg.h:31,
                 from include/types/acl.h:29,
                 from include/types/proxy.h:41,
                 from include/proto/log.h:34,
                 from include/common/cfgparse.h:30,
                 from src/mux_h2.c:13:
include/types/vars.h:30:8: error: redefinition of 'struct var'
 struct var {
        ^~~

Futhermore, to enable multithreading via USE_THREAD, the atomic
library was added to the LDFLAGS. Finally, two new CPUs were added
to simplify the usage of power8 and power9 optimizations.

This TARGET was only tested on GCC 8.3 and may or may not work on
IBM's native C-compiler (XLC).

Should be backported to 2.1.

5 years agoBUG/MINOR: tcp: avoid closing fd when socket failed in tcp_bind_listener
William Dauchy [Wed, 12 Feb 2020 09:09:14 +0000 (10:09 +0100)] 
BUG/MINOR: tcp: avoid closing fd when socket failed in tcp_bind_listener

we were trying to close file descriptor even when `socket` call was
failing.
this should fix github issue #499

this should be backported to all versions >= v1.8

Fixes: 153659f1ae69a1 ("MINOR: tcp: When binding socket, attempt to
reuse one from the old proc.")
Signed-off-by: William Dauchy <w.dauchy@criteo.com>
5 years agoBUG/MINOR: listener: enforce all_threads_mask on bind_thread on init
Willy Tarreau [Wed, 12 Feb 2020 09:15:34 +0000 (10:15 +0100)] 
BUG/MINOR: listener: enforce all_threads_mask on bind_thread on init

When intializing a listener, let's make sure the bind_thread mask is
always limited to all_threads_mask when inserting the FD. This will
avoid seeing listening FDs with bits corresponding to threads that are
not active (e.g. when using "bind ... process 1/even"). The side effect
is very limited, all that was identified is that atomic operations are
used in fd_update_events() when not necessary. It's more a matter of
long-term correctness in practice.

This fix might be backported as far as 1.8 (then proto_sockpair must
be dropped).

5 years agoBUG/MEDIUM: listener: only consider running threads when resuming listeners
Willy Tarreau [Wed, 12 Feb 2020 09:01:29 +0000 (10:01 +0100)] 
BUG/MEDIUM: listener: only consider running threads when resuming listeners

In bug #495 we found that it is possible to resume a listener on an
inexistent thread. This happens when a bind's thread_mask contains bits
out of the active threads mask, such as when using "1/odd" or "1/even".
The thread_mask was used as-is to pick a thread number to re-enable the
listener, and given that the highest number is used, 1/odd or 1/even can
produce quite high thread numbers and crash the process by queuing some
entries into non-existent lists.

This bug is an incomplete fix of commit 413e926ba ("BUG/MAJOR: listener:
fix thread safety in resume_listener()") though it will only trigger if
some bind lines are explicitly bound to thread numbers higher than the
thread count. The fix must be backported to all branches having the fix
above (as far as 1.8, though the code is different there, see the commit
message in 1.8 for changes).

There are a few other places where bind_thread is used without
enforcing all_thread_mask, namely when doing fd_insert() while creating
listeners. It seems harmless but would probably deserve another fix.

5 years agoBUILD: http_act: cast file sizes when reporting file size error
Willy Tarreau [Tue, 11 Feb 2020 09:58:56 +0000 (10:58 +0100)] 
BUILD: http_act: cast file sizes when reporting file size error

As seen in issue #496, st_size may be of varying types on different
systems. Let's simply cast it to long long and use long long for
all size outputs.

5 years agoCLEANUP: mini-clist: simplify nested do { while(1) {} } while (0)
Willy Tarreau [Tue, 11 Feb 2020 09:17:52 +0000 (10:17 +0100)] 
CLEANUP: mini-clist: simplify nested do { while(1) {} } while (0)

While looking for other occurrences of do { continue; } while (0) I
found these few leftovers in mini-clist where an outer loop was made
around "do { } while (0)" then another loop was placed inside just to
handle the continue. Let's clean this up by just removing the outer
one. Most of the patch is only the inner part of the loop that is
reindented. It was verified that the resulting code is the same.

5 years agoBUG/MINOR: connection: correctly retry I/O on signals
Willy Tarreau [Tue, 11 Feb 2020 09:08:05 +0000 (10:08 +0100)] 
BUG/MINOR: connection: correctly retry I/O on signals

Issue #490 reports that there are a few bogus constructs of the famous
"do { if (cond) continue; } while (0)" in the connection code, that are
used to retry on I/O failures caused by receipt of a signal. Let's turn
them into the more correct "while (1) { if (cond) continue; break }"
instead. This may or may not be backported, it shouldn't have any
visible effect.

5 years agoBUILD: cirrus-ci: add ERR=1 to freebsd builds
Ilya Shipitsin [Tue, 11 Feb 2020 08:20:32 +0000 (13:20 +0500)] 
BUILD: cirrus-ci: add ERR=1 to freebsd builds

5 years agoBUILD: cirrus-ci: workaround "pkg install" bug
Ilya Shipitsin [Tue, 11 Feb 2020 08:19:22 +0000 (13:19 +0500)] 
BUILD: cirrus-ci: workaround "pkg install" bug

there's a bug https://github.com/freebsd/pkg/issues/902
adding "pkg update -f && pkg upgrade -y" is workaround.

5 years agoBUILD: cirrus-ci: switch to "snap" images to unify openssl naming
Ilya Shipitsin [Tue, 11 Feb 2020 08:16:02 +0000 (13:16 +0500)] 
BUILD: cirrus-ci: switch to "snap" images to unify openssl naming

"snap" images are updated frequently, while regular images are updated quarterly.
so, mixing "snap" and regular images lead to package naming mismatch, which will occur every
quarter. we cannot use 11.3 release image, it is broken, so we have to use 11.3 "snap" image.
Thus let us use all "snap" images. 13-snap is first introduced with this commit.

5 years agoBUG/MINOR: unix: better catch situations where the unix socket path length is close...
Willy Tarreau [Tue, 11 Feb 2020 05:43:37 +0000 (06:43 +0100)] 
BUG/MINOR: unix: better catch situations where the unix socket path length is close to the limit

We do have some checks for the UNIX socket path length to validate the
full pathname of a unix socket but the pathname extension is only taken
into account when using a bind_prefix. The second check only matches
against MAXPATHLEN. So this means that path names between 98 and 108
might successfully parse but fail to bind. Let's adjust the check in
the address parser and refine the error checking at the bind() step.

This addresses bug #493.

5 years agoBUG/MAJOR: mux-h2: don't wake streams after connection was destroyed
Willy Tarreau [Tue, 11 Feb 2020 03:38:56 +0000 (04:38 +0100)] 
BUG/MAJOR: mux-h2: don't wake streams after connection was destroyed

In commit 477902b ("MEDIUM: connections: Get ride of the xprt_done
callback.") we added an inconditional call to h2_wake_some_streams()
in h2_wake(), though we must not do it if the connection is destroyed
or we end up with a use-after-free. In this case it's already done in
h2_process() before destroying the connection anyway.

Let's just add this test for now. A cleaner approach might consist in
doing it in the h2_process() function itself when a connection status
change is detected.

No backport is needed, this is purely 2.2.

5 years agoDOC: schematic of the SSL certificates architecture
William Lallemand [Mon, 10 Feb 2020 10:43:43 +0000 (11:43 +0100)] 
DOC: schematic of the SSL certificates architecture

This patch provides a schematic of the new architecture based on the
struct cert_key_and_chain which appeared with haproxy 2.1.

Could be backported in 2.1

5 years agoBUG/MEDIUM: tcp-rules: Fix track-sc* actions for L4/L5 TCP rules
Christopher Faulet [Mon, 10 Feb 2020 08:54:49 +0000 (09:54 +0100)] 
BUG/MEDIUM: tcp-rules: Fix track-sc* actions for L4/L5 TCP rules

A bug was introduced during TCP rules refactoring by the commit ac98d81f4
("MINOR: http-rule/tcp-rules: Make track-sc* custom actions"). There is no
stream when L4/L5 TCP rules are evaluated. For these rulesets, In track-sc*
actions, we must take care to rely on the session instead of the stream.

Because of this bug, any evaluation of L4/L5 TCP rules using a track-sc* action
leads to a crash of HAProxy.

No backport needed, except if the above commit is backported.

5 years agoBUG/MEDIUM: ssl/cli: 'commit ssl cert' wrong SSL_CTX init
William Lallemand [Fri, 7 Feb 2020 19:45:24 +0000 (20:45 +0100)] 
BUG/MEDIUM: ssl/cli: 'commit ssl cert' wrong SSL_CTX init

The code which is supposed to apply the bind_conf configuration on the
SSL_CTX was not called correctly. Indeed it was called with the previous
SSL_CTX so the new ones were left with default settings. For example the
ciphers were not changed.

This patch fixes #429.

Must be backported in 2.1.

5 years agoBUG/MINOR: http-act: Fix bugs on error path during parsing of return actions
Christopher Faulet [Fri, 7 Feb 2020 09:26:23 +0000 (10:26 +0100)] 
BUG/MINOR: http-act: Fix bugs on error path during parsing of return actions

This patch fixes memory leaks and a null pointer dereference found by coverity
on the error path when an HTTP return action is parsed. See issue #491.

No need to backport this patch except the HTT return action is backported too.

5 years agoBUG/MINOR: http-act: Set stream error flag before returning an error
Christopher Faulet [Fri, 7 Feb 2020 09:22:31 +0000 (10:22 +0100)] 
BUG/MINOR: http-act: Set stream error flag before returning an error

In action_http_set_status(), when a rewrite error occurred, the stream error
flag must be set before returning the error.

No need to backport this patch except if commit 333bf8c33 ("MINOR: http-rules:
Set SF_ERR_PRXCOND termination flag when a header rewrite fails") is
backported. This bug was reported in issue #491.

5 years agoSCRIPTS: backport: fix the master branch detection
Willy Tarreau [Fri, 7 Feb 2020 07:26:49 +0000 (08:26 +0100)] 
SCRIPTS: backport: fix the master branch detection

The condition was inverted. When the branch was the master, it was
harmless because it caused an extra "checkout master", but when it
was not the master, the commit could be applied to the wrong branch
and it could even possibly not match the name to stop on.

5 years agoSCRIPTS: announce-release: allow the user to force to overwrite old files
Willy Tarreau [Fri, 7 Feb 2020 07:11:45 +0000 (08:11 +0100)] 
SCRIPTS: announce-release: allow the user to force to overwrite old files

When starting the script multiple times, one had to remove the previous
files by hand. Now with -f it's not needed anymore, they get removed.

5 years agoSCRIPTS: announce-release: place the send command in the mail's header
Willy Tarreau [Fri, 7 Feb 2020 07:10:06 +0000 (08:10 +0100)] 
SCRIPTS: announce-release: place the send command in the mail's header

I'm fed up with having to scroll my terminals trying to look for the
mail send command printed 30 minutes before the release, let's have
it copied into the e-mail template itself, and replace the old headers
that used to be duplicated there and that are not needed anymore.

5 years ago[RELEASE] Released version 2.2-dev2 v2.2-dev2
Willy Tarreau [Fri, 7 Feb 2020 03:12:19 +0000 (04:12 +0100)] 
[RELEASE] Released version 2.2-dev2

Released version 2.2-dev2 with the following main changes :
    - BUILD: CI: temporarily mark openssl-1.0.2 as allowed failure
    - MEDIUM: cli: Allow multiple filter entries for "show table"
    - BUG/MEDIUM: netscaler: Don't forget to allocate storage for conn->src/dst.
    - BUG/MINOR: ssl: ssl_sock_load_pem_into_ckch is not consistent
    - BUILD: stick-table: fix build errors introduced by last stick-table change
    - BUG/MINOR: cli: Missing arg offset for filter data values.
    - MEDIUM: streams: Always create a conn_stream in connect_server().
    - MEDIUM: connections: Get ride of the xprt_done callback.
    - CLEANUP: changelog: remove the duplicate entry for 2.2-dev1
    - BUILD: CI: move cygwin builds to Github Actions
    - MINOR: cli: Report location of errors or any extra data for "show table"
    - BUG/MINOR: ssl/cli: free the previous ckch content once a PEM is loaded
    - CLEANUP: backend: remove useless test for inexistent connection
    - CLEANUP: backend: shut another false null-deref in back_handle_st_con()
    - CLEANUP: stats: shut up a wrong null-deref warning from gcc 9.2
    - BUG/MINOR: ssl: increment issuer refcount if in chain
    - BUG/MINOR: ssl: memory leak w/ the ocsp_issuer
    - BUG/MINOR: ssl: typo in previous patch
    - BUG/MEDIUM: connections: Set CO_FL_CONNECTED in conn_complete_session().
    - BUG/MINOR: ssl/cli: ocsp_issuer must be set w/ "set ssl cert"
    - MEDIUM: connection: remove CO_FL_CONNECTED and only rely on CO_FL_WAIT_*
    - BUG/MEDIUM: 0rtt: Only consider the SSL handshake.
    - MINOR: stream-int: always report received shutdowns
    - MINOR: connection: remove CO_FL_SSL_WAIT_HS from CO_FL_HANDSHAKE
    - MEDIUM: connection: use CO_FL_WAIT_XPRT more consistently than L4/L6/HANDSHAKE
    - MINOR: connection: remove checks for CO_FL_HANDSHAKE before I/O
    - MINOR: connection: do not check for CO_FL_SOCK_RD_SH too early
    - MINOR: connection: don't check for CO_FL_SOCK_WR_SH too early in handshakes
    - MINOR: raw-sock: always check for CO_FL_SOCK_WR_SH before sending
    - MINOR: connection: remove some unneeded checks for CO_FL_SOCK_WR_SH
    - BUG/MINOR: stktable: report the current proxy name in error messages
    - BUG/MEDIUM: mux-h2: make sure we don't emit TE headers with anything but "trailers"
    - MINOR: lua: Add hlua_prepend_path function
    - MINOR: lua: Add lua-prepend-path configuration option
    - MINOR: lua: Add HLUA_PREPEND_C?PATH build option
    - BUILD: cfgparse: silence a bogus gcc warning on 32-bit machines
    - BUG/MINOR: http-ana: Increment the backend counters on the backend
    - BUG/MINOR: stream: Be sure to have a listener to increment its counters
    - BUG/MEDIUM: streams: Move the conn_stream allocation outside #IF USE_OPENSSL.
    - REGTESTS: make the set_ssl_cert test require version 2.2
    - BUG/MINOR: ssl: Possible memleak when allowing the 0RTT data buffer.
    - MINOR: ssl: Remove dead code.
    - BUG/MEDIUM: ssl: Don't forget to free ctx->ssl on failure.
    - BUG/MEDIUM: stream: Don't install the mux in back_handle_st_con().
    - MEDIUM: streams: Don't close the connection in back_handle_st_con().
    - MEDIUM: streams: Don't close the connection in back_handle_st_rdy().
    - BUILD: CI: disable slow regtests on Travis
    - BUG/MINOR: tcpchecks: fix the connect() flags regarding delayed ack
    - BUG/MINOR: http-rules: Always init log-format expr for common HTTP actions
    - BUG/MINOR: connection: fix ip6 dst_port copy in make_proxy_line_v2
    - BUG/MINOR: dns: allow 63 char in hostname
    - MINOR: proxy: clarify number of connections log when stopping
    - DOC: word converter ignores delimiters at the start or end of input string
    - MEDIUM: raw-sock: remove obsolete calls to fd_{cant,cond,done}_{send,recv}
    - BUG/MINOR: ssl/cli: fix unused variable with openssl < 1.0.2
    - MEDIUM: pipe/thread: reduce the locking overhead
    - MEDIUM: pipe/thread: maintain a per-thread local cache of recently used pipes
    - BUG/MEDIUM: pipe/thread: fix atomicity of pipe counters
    - MINOR: tasks: move the list walking code to its own function
    - MEDIUM: tasks: implement 3 different tasklet classes with their own queues
    - MEDIUM: tasks: automatically requeue into the bulk queue an already running tasklet
    - OPTIM: task: refine task classes default CPU bandwidth ratios
    - BUG/MEDIUM: connections: Don't forget to unlock when killing a connection.
    - MINOR: task: permanently flag tasklets waking themselves up
    - MINOR: task: make sched->current also reflect tasklets
    - MINOR: task: detect self-wakeups on tl==sched->current instead of TASK_RUNNING
    - OPTIM: task: readjust CPU bandwidth distribution since last update
    - MINOR: task: don't set TASK_RUNNING on tasklets
    - BUG/MEDIUM: memory_pool: Update the seq number in pool_flush().
    - MINOR: memory: Only init the pool spinlock once.
    - BUG/MEDIUM: memory: Add a rwlock before freeing memory.
    - BUG/MAJOR: memory: Don't forget to unlock the rwlock if the pool is empty.
    - MINOR: ssl: ssl-load-extra-files configure loading of files
    - SCRIPTS: add a new "backport" script to simplify long series of backports
    - BUG/MINOR: ssl: we may only ignore the first 64 errors
    - SCRIPTS: use /usr/bin/env bash instead of /bin/bash for scripts
    - BUG/MINOR: ssl: clear the SSL errors on DH loading failure
    - CLEANUP: hpack: remove a redundant test in the decoder
    - CLEANUP: peers: Remove unused static function `free_dcache`
    - CLEANUP: peers: Remove unused static function `free_dcache_tx`
    - CONTRIB: debug: add missing flags SF_HTX and SF_MUX
    - CONTRIB: debug: add the possibility to decode the value as certain types only
    - CONTRIB: debug: support reporting multiple values at once
    - BUG/MINOR: http-act: Use the good message to test strict rewritting mode
    - MINOR: global: Set default tune.maxrewrite value during global structure init
    - MINOR: http-rules: Set SF_ERR_PRXCOND termination flag when a header rewrite fails
    - MINOR: http-htx: Emit a warning if an error file runs over the buffer's reserve
    - MINOR: htx: Add a function to append an HTX message to another one
    - MINOR: htx/channel: Add a function to copy an HTX message in a channel's buffer
    - BUG/MINOR: http-ana: Don't overwrite outgoing data when an error is reported
    - MINOR: dns: Dynamically allocate dns options to reduce the act_rule size
    - MINOR: dns: Add function to release memory allocated for a do-resolve rule
    - BUG/MINOR: http-ana: Reset HTX first index when HAPRoxy sends a response
    - BUG/MINOR: http-ana: Set HTX_FL_PROXY_RESP flag if a server perform a redirect
    - MINOR: http-rules: Add a flag on redirect rules to know the rule direction
    - MINOR: http-rules: Handle the rule direction when a redirect is evaluated
    - MINOR: http-ana: Rely on http_reply_and_close() to handle server error
    - MINOR: http-ana: Add a function for forward internal responses
    - MINOR: http-ana/http-rules: Use dedicated function to forward internal responses
    - MEDIUM: http: Add a ruleset evaluated on all responses just before forwarding
    - MEDIUM: http-rules: Add the return action to HTTP rules
    - MEDIUM: http-rules: Support extra headers for HTTP return actions
    - CLEANUP: lua: Remove consistency check for sample fetches and actions
    - BUG/MINOR: http-ana: Increment failed_resp counters on invalid response
    - MINOR: lua: Get the action return code on the stack when an action finishes
    - MINOR: lua: Create the global 'act' object to register all action return codes
    - MINOR: lua: Add act:wake_time() function to set a timeout when an action yields
    - MEDIUM: lua: Add ability for actions to intercept HTTP messages
    - REGTESTS: Add reg tests for the HTTP return action
    - REGTESTS: Add a reg test for http-after-response rulesets
    - BUILD: lua: silence a warning on systems where longjmp is not marked as noreturn
    - MINOR: acl: Warn when an ACL is named 'or'
    - CONTRIB: debug: also support reading values from stdin
    - SCRIPTS: backport: use short revs and resolve the initial commit
    - BUG/MINOR: acl: Fix type of log message when an acl is named 'or'

5 years agoBUG/MINOR: acl: Fix type of log message when an acl is named 'or'
Tim Duesterhus [Thu, 6 Feb 2020 21:04:03 +0000 (22:04 +0100)] 
BUG/MINOR: acl: Fix type of log message when an acl is named 'or'

The patch adding this check initially only issued a warning, instead of
being fatal. It was changed before committing. However when making this
change the type of the log message was not changed from `ha_warning` to
`ha-alert`. This patch makes this forgotten adjustment.

see 0cf811a5f941261176b67046dbc542d0479ff4a7
No backport needed. The initial patch was backported as a warning, thus
the log message type is correct.

5 years agoSCRIPTS: backport: use short revs and resolve the initial commit
Willy Tarreau [Thu, 6 Feb 2020 17:38:19 +0000 (18:38 +0100)] 
SCRIPTS: backport: use short revs and resolve the initial commit

I find myself often getting trapped into calling "backport 2.0 HEAD" which
doesn't work because "HEAD" is passed as the argument to cherry-pick in
other repos. Let's resolve it first. And also let's shorten the commit IDs
to make the error messages more readable and to ease copy-paste.

5 years agoCONTRIB: debug: also support reading values from stdin
Willy Tarreau [Thu, 6 Feb 2020 17:17:50 +0000 (18:17 +0100)] 
CONTRIB: debug: also support reading values from stdin

This is convenient when processing large dumps, it allows to copy-paste
values to inspect from one window to another, or to directly transfer
a "show fd"/"show stream" output through sed. In order to do this, simply
pass "-" alone instead of the value and they will all be read one line at
a time from stdin. For example, in order to quickly print the different
set of connection flags from "show fd", this is sufficient:

     sed -ne 's/^.* cflg=\([^ ]*\).*/\1/p' | contrib/debug/flags conn -

5 years agoMINOR: acl: Warn when an ACL is named 'or'
Tim Duesterhus [Wed, 5 Feb 2020 20:00:50 +0000 (21:00 +0100)] 
MINOR: acl: Warn when an ACL is named 'or'

Consider a configuration like this:

> acl t always_true
> acl or always_false
>
> http-response set-header Foo Bar if t or t

The 'or' within the condition will be treated as a logical disjunction
and the header will be set, despite the ACL 'or' being falsy.

This patch makes it an error to declare such an ACL that will never
work. This patch may be backported to stable releases, turning the
error into a warning only (the code was written in a way to make this
trivial). It should not break anything and might improve the users'
lifes.

5 years agoBUILD: lua: silence a warning on systems where longjmp is not marked as noreturn
Willy Tarreau [Thu, 6 Feb 2020 14:55:41 +0000 (15:55 +0100)] 
BUILD: lua: silence a warning on systems where longjmp is not marked as noreturn

If the longjmp() call is not flagged as "noreturn", for example, because the
operating system doesn't target a gcc-compatible compiler, we may get this
warning when building Lua :

  src/hlua.c: In function 'hlua_panic_ljmp':
  src/hlua.c:128:1: warning: no return statement in function returning non-void [-Wreturn-type]
   static int hlua_panic_ljmp(lua_State *L) { longjmp(safe_ljmp_env, 1); }
   ^~~~~~

The function's prototype cannot be changed because it must be compatible
with Lua's callbacks. Let's simply enclose the call inside WILL_LJMP()
which we created exactly to signal a call to longjmp(). It lets the compiler
know we won't get back into the function and that the return statement is
not needed.

5 years agoREGTESTS: Add a reg test for http-after-response rulesets
Christopher Faulet [Thu, 6 Feb 2020 13:27:09 +0000 (14:27 +0100)] 
REGTESTS: Add a reg test for http-after-response rulesets

A reg test has been added to ensure the evaluation of http-after-responses rules
is functionnal for all kind of responses (server, applet and internal
responses).

5 years agoREGTESTS: Add reg tests for the HTTP return action
Christopher Faulet [Wed, 5 Feb 2020 15:46:38 +0000 (16:46 +0100)] 
REGTESTS: Add reg tests for the HTTP return action

2 reg tests have been added to ensure the HTTP return action is functionnal. A
reg test is about returning error files. The other one is about returning
default responses and responses based on string or file payloads.

5 years agoMEDIUM: lua: Add ability for actions to intercept HTTP messages
Christopher Faulet [Fri, 31 Jan 2020 11:21:52 +0000 (12:21 +0100)] 
MEDIUM: lua: Add ability for actions to intercept HTTP messages

It is now possible to intercept HTTP messages from a lua action and reply to
clients. To do so, a reply object must be provided to the function
txn:done(). It may contain a status code with a reason, a header list and a
body. By default, if an empty reply object is used, an empty 200 response is
returned. If no reply is passed when txn:done() is called, the previous
behaviour is respected, the transaction is terminated and nothing is returned to
the client. The same is done for TCP streams. When txn:done() is called, the
action is terminated with the code ACT_RET_DONE on success and ACT_RET_ERR on
error, interrupting the message analysis.

The reply object may be created for the lua, by hand. Or txn:reply() may be
called. If so, this object provides some methods to fill it:

  * Reply:set_status(<status> [  <reason>]) : Set the status and optionally the
   reason. If no reason is provided, the default one corresponding to the status
   code is used.

  * Reply:add_header(<name>, <value>) : Add a header. For a given name, the
    values are stored in an ordered list.

  * Reply:del_header(<name>) : Removes all occurrences of a header name.

  * Reply:set_body(<body>) : Set the reply body.

Here are some examples, all doing the same:

    -- ex. 1
    txn:done{
        status  = 400,
        reason  = "Bad request",
        headers = {
            ["content-type"]  = { "text/html" },
            ["cache-control"] = { "no-cache", "no-store" },
        },
        body = "<html><body><h1>invalid request<h1></body></html>"
    }

    -- ex. 2
    local reply = txn:reply{
        status  = 400,
        reason  = "Bad request",
        headers = {
            ["content-type"]  = { "text/html" },
            ["cache-control"] = { "no-cache", "no-store" }
        },
        body = "<html><body><h1>invalid request<h1></body></html>"
    }
    txn:done(reply)

    -- ex. 3
    local reply = txn:reply()
    reply:set_status(400, "Bad request")
    reply:add_header("content-length", "text/html")
    reply:add_header("cache-control", "no-cache")
    reply:add_header("cache-control", "no-store")
    reply:set_body("<html><body><h1>invalid request<h1></body></html>")
    txn:done(reply)

5 years agoMINOR: lua: Add act:wake_time() function to set a timeout when an action yields
Christopher Faulet [Fri, 31 Jan 2020 18:07:52 +0000 (19:07 +0100)] 
MINOR: lua: Add act:wake_time() function to set a timeout when an action yields

This function may be used to defined a timeout when a lua action returns
act:YIELD. It is a way to force to reexecute the script after a short time
(defined in milliseconds).

Unlike core:sleep() or core:yield(), the script is fully reexecuted if it
returns act:YIELD. With core functions to yield, the script is interrupted and
restarts from the yield point. When a script returns act:YIELD, it is finished
but the message analysis is blocked on the action waiting its end.

5 years agoMINOR: lua: Create the global 'act' object to register all action return codes
Christopher Faulet [Fri, 31 Jan 2020 17:57:12 +0000 (18:57 +0100)] 
MINOR: lua: Create the global 'act' object to register all action return codes

ACT_RET_* code are now available from lua scripts. The gloabl object "act" is
used to register these codes as constant. Now, lua actions can return any of
following codes :

  * act.CONTINUE for ACT_RET_CONT
  * act.STOP for ACT_RET_STOP
  * act.YIELD for ACT_RET_YIELD
  * act.ERROR for ACT_RET_ERR
  * act.DONE for ACT_RET_DONE
  * act.DENY for ACT_RET_DENY
  * act.ABORT for ACT_RET_ABRT
  * act.INVALID for ACT_RET_INV

For instance, following script denied all requests :

  core.register_action("deny", { "http-req" }, function (txn)
      return act.DENY
  end)

Thus "http-request lua.deny" do exactly the same than "http-request deny".

5 years agoMINOR: lua: Get the action return code on the stack when an action finishes
Christopher Faulet [Wed, 29 Jan 2020 10:53:30 +0000 (11:53 +0100)] 
MINOR: lua: Get the action return code on the stack when an action finishes

When an action successfully finishes, the action return code (ACT_RET_*) is now
retrieve on the stack, ff the first element is an integer. In addition, in
hlua_txn_done(), the value ACT_RET_DONE is pushed on the stack before
exiting. Thus, when a script uses this function, the corresponding action still
finishes with the good code. Thanks to this change, the flag HLUA_STOP is now
useless. So it has been removed.

It is a mandatory step to allow a lua action to return any action return code.

5 years agoBUG/MINOR: http-ana: Increment failed_resp counters on invalid response
Christopher Faulet [Wed, 5 Feb 2020 09:16:41 +0000 (10:16 +0100)] 
BUG/MINOR: http-ana: Increment failed_resp counters on invalid response

In http_process_res_common() analyzer, when a invalid response is reported, the
failed_resp counters must be incremented.

No need to backport this patch, except if the commit b8a5371a ("MEDIUM:
http-ana: Properly handle internal processing errors") is backported too.

5 years agoCLEANUP: lua: Remove consistency check for sample fetches and actions
Christopher Faulet [Wed, 29 Jan 2020 10:51:39 +0000 (11:51 +0100)] 
CLEANUP: lua: Remove consistency check for sample fetches and actions

It is not possible anymore to alter the HTTP parser state from lua sample
fetches or lua actions. So there is no reason to still check for the parser
state consistency.

5 years agoMEDIUM: http-rules: Support extra headers for HTTP return actions
Christopher Faulet [Fri, 31 Jan 2020 16:36:01 +0000 (17:36 +0100)] 
MEDIUM: http-rules: Support extra headers for HTTP return actions

It is now possible to append extra headers to the generated responses by HTTP
return actions, while it is not based on an errorfile. For return actions based
on errorfiles, these extra headers are ignored. To define an extra header, a
"hdr" argument must be used with a name and a value. The value is a log-format
string. For instance:

  http-request status 200 hdr "x-src" "%[src]" hdr "x-dst" "%[dst]"