haproxy-2.3.git
4 years agoBUG/MEDIUM: ssl/cli: abort ssl cert is freeing the old store
William Lallemand [Mon, 1 Feb 2021 14:31:00 +0000 (15:31 +0100)]
BUG/MEDIUM: ssl/cli: abort ssl cert is freeing the old store

The "abort ssl cert" command is buggy and removes the current ckch store,
and instances, leading to SNI removal. It must only removes the new one.

This patch also adds a check in set_ssl_cert.vtc and
set_ssl_server_cert.vtc.

Must be backported as far as 2.2.

(cherry picked from commit 8695ce0bae21238eba660438c819797a245be71e)
[wt: dropped reg-tests/ssl/set_ssl_server_cert.vtc]
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MINOR: stick-table: Always call smp_fetch_src() with a valid arg list
Christopher Faulet [Fri, 29 Jan 2021 09:27:47 +0000 (10:27 +0100)]
BUG/MINOR: stick-table: Always call smp_fetch_src() with a valid arg list

The sample fetch functions must always be called with a valid argument
list. When called by hand, if there is no argument to pass, empty_arg_list must
be used.

In the stick-table code, there are some calls to smp_fetch_src() with NULL as
argument list. It is changed to use empty_arg_list instead. It is not really a
bug because smp_fetch_src() does not use the argument list. But it is an API
bug.

This patch may be backported to all stable branches as a cleanup.

(cherry picked from commit bdbd5db2a50461fceb7fb89329e73d08f90e44fd)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoDOC: management: fix "show resolvers" alphabetical ordering
Willy Tarreau [Fri, 29 Jan 2021 11:01:46 +0000 (12:01 +0100)]
DOC: management: fix "show resolvers" alphabetical ordering

Not sure why it was located between "show ssl" and "show table"...
This should be backported.

(cherry picked from commit 87ef32397101c7bc4783982af952da12c1750e88)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: h1: Raise the chunk size limit up to (2^52 - 1)
Christopher Faulet [Wed, 27 Jan 2021 14:17:13 +0000 (15:17 +0100)]
MINOR: h1: Raise the chunk size limit up to (2^52 - 1)

The allowed chunk size was historically limited to 2GB to avoid risk of
overflow. This restriction is no longer necessary because the chunk size is
immediately stored into a 64bits integer after the parsing. Thus, it is now
possible to raise this limit. However to never fed possibly bogus values
from languages that use floats for their integers, we don't get more than 13
hexa-digit (2^52 - 1). 4 petabytes is probably enough !

This patch should fix the issue #1065. It may be backported as far as
2.1. For the 2.0, the legacy HTTP part must be reviewed. But there is
honestely no reason to do so.

(cherry picked from commit 405f05465252498429fb9dc38db40f0803f7cb69)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: mux-h1/show_fd: report as suspicious an entry with too many calls
Willy Tarreau [Thu, 21 Jan 2021 08:13:35 +0000 (09:13 +0100)]
MINOR: mux-h1/show_fd: report as suspicious an entry with too many calls

An FD entry that maps to an H1 connection whose stream was woken
up more than 1M times is now flagged as suspicious.

(cherry picked from commit 0c0c0a287834283eaaf5f25437a39a8f059e9878)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: mux-h2/show_fd: report as suspicious an entry with too many calls
Willy Tarreau [Thu, 21 Jan 2021 08:13:35 +0000 (09:13 +0100)]
MINOR: mux-h2/show_fd: report as suspicious an entry with too many calls

An FD entry that maps to an H2C connection whose last stream was woken
up more than 1M times is now flagged as suspicious.

(cherry picked from commit 06bf83e0ae101567796e05630ae227fb18d4e3e8)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: ssl/show_fd: report some FDs as suspicious when possible
Willy Tarreau [Thu, 21 Jan 2021 07:53:50 +0000 (08:53 +0100)]
MINOR: ssl/show_fd: report some FDs as suspicious when possible

If a subscriber's tasklet was called more than one million times, if
the ssl_ctx's connection doesn't match the current one, or if the
connection appears closed in one direction while the SSL stack is
still subscribed, the FD is reported as suspicious. The close cases
may occasionally trigger a false positive during very short and rare
windows. Similarly the 1M calls will trigger after 16GB are transferred
over a given connection. These are rare enough events to be reported as
suspicious.

(cherry picked from commit 4bd5d630ace94ee6b4cbb7375ec75f50a927a826)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: cli/show_fd: report some easily detectable suspicious states
Willy Tarreau [Thu, 21 Jan 2021 08:07:29 +0000 (09:07 +0100)]
MINOR: cli/show_fd: report some easily detectable suspicious states

A file descriptor which maps to a connection but has more than one
thread in its mask, or an FD handle that doesn't correspond to the FD,
or wiht no mux context, or an FD with no thread in its mask, or with
more than 1 million events is flagged as suspicious.

(cherry picked from commit dacfde4ba42e3a13dd5b13b7df7a37db06d702a6)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: cli: give the show_fd helpers the ability to report a suspicious entry
Willy Tarreau [Thu, 21 Jan 2021 07:26:06 +0000 (08:26 +0100)]
MINOR: cli: give the show_fd helpers the ability to report a suspicious entry

Now the show_fd helpers at the transport and mux levels return an integer
which indicates whether or not the inspected entry looks suspicious. When
an entry is reported as suspicious, "show fd" will suffix it with an
exclamation mark ('!') in the dump, that is supposed to help detecting
them.

For now, helpers were adjusted to adapt to the new API but none of them
reports any suspicious entry yet.

(cherry picked from commit 8050efeacb554f798a49db5ef213c8f673e5596e)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: mux-fcgi: make the "show fd" helper also decode the fstrm subscriber when...
Willy Tarreau [Wed, 20 Jan 2021 16:10:46 +0000 (17:10 +0100)]
MINOR: mux-fcgi: make the "show fd" helper also decode the fstrm subscriber when known

When dumping a live fcgi stream, also take the opportunity for reporting
the subscriber including the event, tasklet, handler and context.

(cherry picked from commit 1776ffb975017f91a12a9c6404bd3cb73789c7c7)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: mux-h1: make the "show fd" helper also decode the h1s subscriber when known
Willy Tarreau [Wed, 20 Jan 2021 16:05:58 +0000 (17:05 +0100)]
MINOR: mux-h1: make the "show fd" helper also decode the h1s subscriber when known

When dumping a live h1 stream, also take the opportunity for reporting
the subscriber including the event, tasklet, handler and context. Example:

   3030 : st=0x21(R:rA W:Ra) ev=0x04(heOpi) [Lc] tmask=0x4 umask=0x0 owner=0x7f97805c1f70 iocb=0x65b847(sock_conn_iocb) back=1 cflg=0x00002300 sv=s1/recv mux=H1 ctx=0x7f97805c21b0 h1c.flg=0x80000200 .sub=1 .ibuf=0@(nil)+0/0 .obuf=0@(nil)+0/0 h1s=0x7f97805c2380 h1s.flg=0x4010 .req.state=MSG_DATA .res.state=MSG_RPBEFORE .meth=POST status=0 .cs.flg=0x00000000 .cs.data=0x7f97805c1720 .subs=0x7f97805c1748(ev=1 tl=0x7f97805c1990 tl.calls=2 tl.ctx=0x7f97805c1720 tl.fct=si_cs_io_cb) xprt=RAW

(cherry picked from commit 150c4f8b729749a074ccdde75457fbcf8c8728f0)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: mux-h2: make the "show fd" helper also decode the h2s subscriber when known
Willy Tarreau [Wed, 20 Jan 2021 15:27:01 +0000 (16:27 +0100)]
MINOR: mux-h2: make the "show fd" helper also decode the h2s subscriber when known

When dumping a valid h2 stream, also dump the subscriber, its events,
tasklet context and handler. Example:

    128 : st=0x21(R:rA W:Ra) ev=0x01(heopI) [lc] tmask=0x1 umask=0x0 owner=0x7f40380d7370 iocb=0x65b71b(sock_conn_iocb) back=0 cflg=0x00001300 fe=recv mux=H2 ctx=0x1ad23e0 h2c.st0=FRP .err=0 .maxid=3 .lastid=-1 .flg=0x10000 .nbst=2 .nbcs=2 .fctl_cnt=0 .send_cnt=0 .tree_cnt=2 .orph_cnt=0 .sub=1 .dsi=3 .dbuf=16366@0x1ea9380+16441/16448 .msi=-1 .mbuf=[1..1|32],h=[0@(nil)+0/0],t=[0@(nil)+0/0] last_h2s=0x20a8340 .id=3 .st=OPN .flg=0x4100 .rxbuf=0@(nil)+0/0 .cs=0x20a8440(.flg=0x00100000 .data=0x20a8738) .subs=0x20a8760(ev=1 tl=0x20a89b0 tl.calls=22 tl.ctx=0x20a8738 tl.fct=si_cs_io_cb) xprt=SSL xprt_ctx=0x1aaf4c0 xctx.st=0 .xprt=RAW .wait.ev=1 .subs=0x1ad28e0(ev=1 tl=0x1ab3c70 tl.calls=176 tl.ctx=0x1ad23e0 tl.fct=h2_io_cb) .sent_early=0 .early_in=0

(cherry picked from commit 98e40b98180632ac2cd479b8e1900e6b698121f5)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: xprt/mux: export all *_io_cb functions so that "show fd" resolves them
Willy Tarreau [Wed, 20 Jan 2021 13:55:01 +0000 (14:55 +0100)]
MINOR: xprt/mux: export all *_io_cb functions so that "show fd" resolves them

In FD dumps it's often very important to figure what upper layer function
is going to be called. Let's export the few I/O callbacks that appear as
tasklet functions so that "show fd" can resolve them instead of printing
a pointer relative to main. For example:

   1028 : st=0x21(R:rA W:Ra) ev=0x01(heopI) [lc] tmask=0x2 umask=0x2 owner=0x7f00b889b200 iocb=0x65b638(sock_conn_iocb) back=0 cflg=0x00001300 fe=recv mux=H2 ctx=0x7f00c8824de0 h2c.st0=FRH .err=0 .maxid=795 .lastid=-1 .flg=0x0000 .nbst=0 .nbcs=0 .fctl_cnt=0 .send_cnt=0 .tree_cnt=0 .orph_cnt=0 .sub=1 .dsi=795 .dbuf=0@(nil)+0/0 .msi=-1 .mbuf=[1..1|32],h=[0@(nil)+0/0],t=[0@(nil)+0/0] xprt=SSL xprt_ctx=0x7f00c86d0750 xctx.st=0 .xprt=RAW .wait.ev=1 .subs=0x7f00c88252e0(ev=1 tl=0x7f00a07d1aa0 tl.calls=1047 tl.ctx=0x7f00c8824de0 tl.fct=h2_io_cb) .sent_early=0 .early_in=0

(cherry picked from commit 691d503896f2dc4944782cfe989fc8b44d66a6c0)
[wt: context adjustments; dropped quic]
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: ssl: provide a "show fd" helper to report important SSL information
Willy Tarreau [Wed, 20 Jan 2021 13:41:29 +0000 (14:41 +0100)]
MINOR: ssl: provide a "show fd" helper to report important SSL information

The SSL context contains a lot of important details that are currently
missing from debug outputs. Now that we detect ssl_sock, we can perform
some sanity checks, print the next xprt, the subscriber callback's context,
handler and number of calls. The process function is also resolved. This
now gives for example on an H2 connection:

   1029 : st=0x21(R:rA W:Ra) ev=0x01(heopI) [lc] tmask=0x2 umask=0x2 owner=0x7fc714881700 iocb=0x65b528(sock_conn_iocb) back=0 cflg=0x00001300 fe=recv mux=H2 ctx=0x7fc734545e50 h2c.st0=FRH .err=0 .maxid=217 .lastid=-1 .flg=0x0000 .nbst=0 .nbcs=0 .fctl_cnt=0 .send_cnt=0 .tree_cnt=0 .orph_cnt=0 .sub=1 .dsi=217 .dbuf=0@(nil)+0/0 .msi=-1 .mbuf=[1..1|32],h=[0@(nil)+0/0],t=[0@(nil)+0/0] xprt=SSL xprt_ctx=0x7fc73478f230 xctx.st=0 .xprt=RAW .wait.ev=1 .subs=0x7fc734546350(ev=1 tl=0x7fc7346702e0 tl.calls=278 tl.ctx=0x7fc734545e50 tl.fct=main-0x144efa) .sent_early=0 .early_in=0

(cherry picked from commit de5675a38c50a5c5d79c261f303c52ff9df241b3)
[wt: context adjustment]
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: xprt: add a new show_fd() helper to complete some "show fd" dumps.
Willy Tarreau [Wed, 20 Jan 2021 14:30:56 +0000 (15:30 +0100)]
MINOR: xprt: add a new show_fd() helper to complete some "show fd" dumps.

Just like we did for the muxes, now the transport layers will have the
ability to provide helpers to report more detailed information about their
internal context. When the helper is not known, the pointer continues to
be dumped as-is if it's not NULL. This way a transport with no context nor
dump function will not add a useless "xprt_ctx=(nil)" but the pointer will
be emitted if valid or if a helper is defined.

(cherry picked from commit 108a27104960da8196eb6ce3686a2b65be3c6067)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: cli: make "show fd" also report the xprt and xprt_ctx
Willy Tarreau [Wed, 20 Jan 2021 13:40:04 +0000 (14:40 +0100)]
MINOR: cli: make "show fd" also report the xprt and xprt_ctx

These ones are definitely missing from some dumps, let's report them! We
print the xprt's name instead of its useless pointer, as well as its ctx
when xprt is not NULL.

(cherry picked from commit 37be9534245152130fad75c74133c3111a2f382b)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoCLEANUP: cli: make "show fd" use a const connection to access other fields
Willy Tarreau [Wed, 20 Jan 2021 13:13:46 +0000 (14:13 +0100)]
CLEANUP: cli: make "show fd" use a const connection to access other fields

Over time the code has uglified, casting fdt.owner as a struct connection
for about everything. Let's have a const struct connection* there and take
this opportunity for passing all fields as const as well.

Additionally a misplaced closing parenthesis on the output was fixed.

(cherry picked from commit eb0595d039b5e5c56bf3f574ec7e364d926c406b)
[wt: s/sock_conn_iocb/conn_fd_handler]
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoCLEANUP: tools: make resolve_sym_name() take a const pointer
Willy Tarreau [Wed, 20 Jan 2021 13:37:59 +0000 (14:37 +0100)]
CLEANUP: tools: make resolve_sym_name() take a const pointer

When 0c439d895 ("BUILD: tools: make resolve_sym_name() return a const")
was written, the pointer argument ought to have been turned to const for
more flexibility. Let's do it now.

(cherry picked from commit 45fd1030d5eebb318fbcf70adadfcc1ff6b261b3)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: contrib: Make the wireshark peers dissector compile for more distribs.
Frédéric Lécaille [Tue, 19 Jan 2021 13:33:24 +0000 (14:33 +0100)]
MINOR: contrib: Make the wireshark peers dissector compile for more distribs.

With a 2.6.8 wireshark, this module could not compile because of ws_version.h
missing header. This patch offers the possibility to compile this plugin without
having to include this header. Furthermore with my wireshark version a
"plugin_release" object is required to make it be loaded by wireshark. This is
a string which seems to have to match a dotted string made of you wireshark
major and minor versions.

(cherry picked from commit aab6f7c3e642aa5e298a9d8765b5063abf518bfb)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MINOR: backend: check available list allocation for reuse
Amaury Denoyelle [Thu, 28 Jan 2021 16:33:26 +0000 (17:33 +0100)]
BUG/MINOR: backend: check available list allocation for reuse

Do not consider reuse connection if available list is not allocated for
the target server. This will prevent a crash when using a standalone
server for an external purpose like socket_tcp/socket_ssl on hlua code.
For the idle/safe lists, they are considered allocated if
srv.max_idle_conns is not null.

Note that the hlua code is currently safe thanks to the additional
checks on proxy http mode and stream reuse policy not never. However,
this might not be sufficient for future code.

This patch should be backported in every branches containing the
following patch :
  7f68d815af356fbe1b2e1080a88b9935581c91d2 (2.4 tree)
  REORG: backend: simplify conn_backend_get

(cherry picked from commit a81bb7197eb538628274bffe3b09e87e564aeb89)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MEDIUM: backend: never reuse a connection for tcp mode
Amaury Denoyelle [Tue, 26 Jan 2021 16:35:46 +0000 (17:35 +0100)]
BUG/MEDIUM: backend: never reuse a connection for tcp mode

The reuse of idle connections should only happen for a proxy with the
http mode. In case of a backend with the tcp mode, the reuse selection
and insertion in session list are skipped.

This behavior is present since commit :
MEDIUM: connection: Add private connections synchronously in session server list
It could also be further exagerated by :
MEDIUM: backend: add reused conn to sess if mux marked as HOL blocking

It can be backported up to 2.3.

(cherry picked from commit 08d87b3f49867440f66aee09173c84bf58cbc859)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoREORG: backend: simplify conn_backend_get
Amaury Denoyelle [Tue, 26 Jan 2021 13:35:26 +0000 (14:35 +0100)]
REORG: backend: simplify conn_backend_get

Reorganize the conditions for the reuse of idle/safe connections :
- reduce code by using variable to store reuse mode and idle/safe conns
  counts
- consider that idle/safe/avail lists are properly allocated if
  max_idle_conns not null. An allocation failure prevents haproxy
  startup.

(cherry picked from commit 7f68d815af356fbe1b2e1080a88b9935581c91d2)
[wt: harmless, backported since needed for the next one after careful
     review]
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MEDIUM: session: only retrieve ready idle conn from session
Amaury Denoyelle [Tue, 26 Jan 2021 13:14:37 +0000 (14:14 +0100)]
BUG/MEDIUM: session: only retrieve ready idle conn from session

A bug was introduced by the early insertion of idle connections at the
end of connect_server. It is possible to reuse a connection not yet
ready waiting for an handshake (for example with proxy protocol or ssl).
A wrong duplicate xprt_handshake_io_cb tasklet is thus registered as a
side-effect.

This triggers the BUG_ON statement of xprt_handshake_subscribe :
    BUG_ON(ctx->subs && ctx->subs != es);

To counter this, a check is now present in session_get_conn to only
return a connection without the flag CO_FL_WAIT_XPRT. This might cause
sometimes the creation of dedicated server connections when in theory
reuse could have been used, but probably only occurs rarely in real
condition.

This behavior is present since commit :
    MEDIUM: connection: Add private connections synchronously in session server list
It could also be further exagerated by :
    MEDIUM: backend: add reused conn to sess if mux marked as HOL blocking

It can be backported up to 2.3.

NOTE : This bug seems to be only reproducible with mode tcp, for an
unknown reason. However, reuse should never happen when not in http
mode. This improper behavior will be the subject of a dedicated patch.

This bug can easily be reproducible with the following config (a
webserver is required to accept proxy protocol on port 31080) :

    global

    defaults
      mode tcp
      timeout connect 1s
      timeout server 1s
      timeout client 1s

    listen li
      bind 0.0.0.0:4444
      server bla1 127.0.0.1:31080 check send-proxy-v2

with the inject client :
    $ inject -u 10000 -d 10 -G 127.0.0.1:4444

This should fix the github issue #1058.

(cherry picked from commit 3f07c20fab8bc9d98c4f9c0c6e356fb3023c5632)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MINOR: ssl: init tmp chunk correctly in ssl_sock_load_sctl_from_file()
William Lallemand [Wed, 27 Jan 2021 13:58:51 +0000 (14:58 +0100)]
BUG/MINOR: ssl: init tmp chunk correctly in ssl_sock_load_sctl_from_file()

Use chunk_inistr() for a chunk initialisation in
ssl_sock_load_sctl_from_file() instead of a manual initialisation which
was not initialising head.

Fix issue #1073.

Must be backported as far as 2.2

(cherry picked from commit 8d67394f6915c6d2db40bc1e9593fd392827da8d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MINOR: config: fix leak on proxy.conn_src.bind_hdr_name
Amaury Denoyelle [Tue, 26 Jan 2021 13:35:22 +0000 (14:35 +0100)]
BUG/MINOR: config: fix leak on proxy.conn_src.bind_hdr_name

Leak for parsing of option usesrc of the source keyword.

This can be backported to 1.8.

(cherry picked from commit 69c5c3ab330584f9c53e2cf7c86af371a84f104d)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MEDIUM: filters/htx: Fix data forwarding when payload length is unknown
Christopher Faulet [Mon, 25 Jan 2021 11:02:00 +0000 (12:02 +0100)]
BUG/MEDIUM: filters/htx: Fix data forwarding when payload length is unknown

It is only a problem on the response path because the request payload length
it always known. But when a filter is registered to analyze the response
payload, the filtering may hang if the server closes just after the headers.

The root cause of the bug comes from an attempt to allow the filters to not
immediately forward the headers if necessary. A filter may choose to hold
the headers by not forwarding any bytes of the payload. For a message with
no payload but a known payload length, there is always a EOM block to
forward. Thus holding the EOM block for bodyless messages is a good way to
also hold the headers. However, messages with an unknown payload length,
there is no EOM block finishing the message, but only a SHUTR flag on the
channel to mark the end of the stream. If there is no payload when it
happens, there is no payload at all to forward. In the filters API, it is
wrongly detected as a condition to not forward the headers.

Because it is not the most used feature and not the obvious one, this patch
introduces another way to hold the message headers at the begining of the
forwarding. A filter flag is added to explicitly says the headers should be
hold. A filter may choose to set the STRM_FLT_FL_HOLD_HTTP_HDRS flag and not
forwad anything to hold the headers. This flag is removed at each call, thus
it must always be explicitly set by filters. This flag is only evaluated if
no byte has ever been forwarded because the headers are forwarded with the
first byte of the payload.

reg-tests/filters/random-forwarding.vtc reg-test is updated to also test
responses with unknown payload length (with and without payload).

This patch must be backported as far as 2.0.

(cherry picked from commit 6071c2d12dd2ff1f5876a2ace313a81259f211cd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoDOC: Improve documentation of the various hdr() fetches
Tim Duesterhus [Sat, 23 Jan 2021 16:50:21 +0000 (17:50 +0100)]
DOC: Improve documentation of the various hdr() fetches

GitHub issue #796 notes that many administrators miss the fact that the `hdr()`
fetch (without the `f`) splits the header value at commas. This is only
mentioned at the end of a long paragraph.

This patch attempts to improve the documentation by:
- Explaning the "comma issue" as early as possible.
- Adding newlines to split the explanation into distinct sections.
- Reducing duplication by making the `res` siblings refer to their `req`
  counterparts.

This patch may be backported as long as it applies cleanly. During the
refactoring I needed to adjust several explanations for consistency and not all
of them might be available in older branches.

(cherry picked from commit 27c70ae23c1f5b39c9757387559072a096c8aa8c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUILD/MINOR: lua: define _GNU_SOURCE for LLONG_MAX
Bertrand Jacquin [Thu, 21 Jan 2021 21:14:07 +0000 (21:14 +0000)]
BUILD/MINOR: lua: define _GNU_SOURCE for LLONG_MAX

Lua requires LLONG_MAX defined with __USE_ISOC99 which is set by
_GNU_SOURCE, not necessarely defined by default on old compiler/glibc.

  $ make V=1 TARGET=linux-glibc-legacy USE_THREAD= USE_ACCEPT4= USE_PCRE=1 USE_OPENSSL=1 USE_ZLIB=1  USE_LUA=1
  ..
cc -Iinclude -O2 -g -Wall -Wextra -Wdeclaration-after-statement -fwrapv -Wno-strict-aliasing -Wno-unused-label -Wno-sign-compare -Wno-unused-parameter -Wno-missing-field-initializers -DUSE_EPOLL -DUSE_NETFILTER -DUSE_PCRE -DUSE_POLL -DUSE_TPROXY -DUSE_LINUX_TPROXY -DUSE_LINUX_SPLICE -DUSE_LIBCRYPT -DUSE_CRYPT_H -DUSE_GETADDRINFO -DUSE_OPENSSL -DUSE_LUA -DUSE_FUTEX -DUSE_ZLIB -DUSE_CPU_AFFINITY -DUSE_DL -DUSE_RT -DUSE_PRCTL -DUSE_THREAD_DUMP -I/usr/include/openssl101e/ -DUSE_PCRE -I/usr/include -DCONFIG_HAPROXY_VERSION=\"2.4-dev5-73246d-83\" -DCONFIG_HAPROXY_DATE=\"2021/01/21\" -c -o src/hlua.o src/hlua.c
  In file included from /usr/local/include/lua.h:15,
                   from /usr/local/include/lauxlib.h:15,
                   from src/hlua.c:16:
  /usr/local/include/luaconf.h:581:2: error: #error "Compiler does not support 'long long'. Use option '-DLUA_32BITS'   or '-DLUA_C89_NUMBERS' (see file 'luaconf.h' for details)"
  ..
cc -Iinclude -O2 -g -Wall -Wextra -Wdeclaration-after-statement -fwrapv -Wno-strict-aliasing -Wno-unused-label -Wno-sign-compare -Wno-unused-parameter -Wno-missing-field-initializers -DUSE_EPOLL -DUSE_NETFILTER -DUSE_PCRE -DUSE_POLL -DUSE_TPROXY -DUSE_LINUX_TPROXY -DUSE_LINUX_SPLICE -DUSE_LIBCRYPT -DUSE_CRYPT_H -DUSE_GETADDRINFO -DUSE_OPENSSL -DUSE_LUA -DUSE_FUTEX -DUSE_ZLIB -DUSE_CPU_AFFINITY -DUSE_DL -DUSE_RT -DUSE_PRCTL -DUSE_THREAD_DUMP -I/usr/include/openssl101e/ -DUSE_PCRE -I/usr/include -DCONFIG_HAPROXY_VERSION=\"2.4-dev5-73246d-83\" -DCONFIG_HAPROXY_DATE=\"2021/01/21\" -c -o src/hlua_fcn.o src/hlua_fcn.c
  In file included from /usr/local/include/lua.h:15,
                   from /usr/local/include/lauxlib.h:15,
                   from src/hlua_fcn.c:17:
  /usr/local/include/luaconf.h:581:2: error: #error "Compiler does not support 'long long'. Use option '-DLUA_32BITS'   or '-DLUA_C89_NUMBERS' (see file 'luaconf.h' for details)"
  ..

Cc: Thierry Fournier <tfournier@arpalert.org>
(cherry picked from commit f4c12d4da25cd9da14215b0c4ee740d2b2ef762e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: mux-h2: fix read0 handling on partial frames
Willy Tarreau [Wed, 20 Jan 2021 09:53:13 +0000 (10:53 +0100)]
BUG/MEDIUM: mux-h2: fix read0 handling on partial frames

Since commit aade4edc1 ("BUG/MEDIUM: mux-h2: Don't handle pending read0
too early on streams"), we've met a few cases where an early connection
close wouldn't be properly handled if some data were pending in a frame
header, because the test now considers the buffer's contents before
accepting to report the close, but given that frame headers or preface
are consumed at once, the buffer cannot make progress when it's stuck
at intermediary lengths.

In order to address this, this patch introduces two flags in the h2c
connection to store any reported shutdown and failed parsing. The idea
is that we cannot rely on conn_xprt_read0_pending() in the parser since
it wouldn't consider data pending in the buffer nor intermediary layers,
but we know for certain that after a read0 is reported by the transport
layer in presence of an RD_SH on the connection, no more progress will
be made there. This alone is not sufficient to decide to end processing,
we can only do this once these final data have been submitted to a parser.
Therefore, now when a parser fails on missing data, we check if a read0
has already been reported on this connection, and if so we set a new
END_REACHED flag on the connection to indicate a failure to process the
final data. The h2c_read0_pending() function now simply reports this
flag's status. This way we're certain that the input shutdown is only
considered after the demux attempted to parse the last frame.

Maybe over the long term the subscribe() API should be improved to
synchronously fail when trying to subscribe for an even that will not
happen. This may be an elegant solution that could possibly work across
multiple layers and even muxes, and be usable at a few specific places
where that's needed.

Given the patch above was backported as far as 2.0, this one should be
backported there as well. It is possible that the fcgi mux has the same
issue, but this was not analysed yet.

Thanks to Pierre Cheynier for providing detailed traces allowing to
quickly narrow the problem down, and to Olivier for his analysis.

(cherry picked from commit 3d4631fec626c5aa8f12582ee3713563b5e38519)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: tcpcheck: Don't destroy connection in the wake callback context
Christopher Faulet [Mon, 18 Jan 2021 14:47:03 +0000 (15:47 +0100)]
BUG/MEDIUM: tcpcheck: Don't destroy connection in the wake callback context

When a tcpcheck ruleset uses multiple connections, the existing one must be
closed and destroyed before openning the new one. This part is handled in
the tcpcheck_main() function, when called from the wake callback function
(wake_srv_chk). But it is indeed a problem, because this function may be
called from the mux layer. This means a mux may call the wake callback
function of the data layer, which may release the connection and the mux. It
is easy to see how it is hazardous. And actually, depending on the
scheduling, it leads to crashes.

Thus, we must avoid to release the connection in the wake callback context,
and move this part in the check's process function instead. To do so, we
rely on the CHK_ST_CLOSE_CONN flags. When a connection must be replaced by a
new one, this flag is set on the check, in tcpcheck_main() function, and the
check's task is woken up. Then, the connection is really closed in
process_chk_conn() function.

This patch must be backported as far as 2.2, with some adaptations however
because the code is not exactly the same.

(cherry picked from commit 8f100427c4bf56acff967e313cd9cb8d42da035d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: mworker: define _GNU_SOURCE for strsignal()
Bertrand Jacquin [Thu, 21 Jan 2021 01:31:46 +0000 (01:31 +0000)]
BUG/MINOR: mworker: define _GNU_SOURCE for strsignal()

glibc < 2.10 requires _GNU_SOURCE in order to make use of strsignal(),
otherwise leading to SEGV at runtime.

  $ make V=1 TARGET=linux-glibc-legacy USE_THREAD= USE_ACCEPT4=
  ..
  src/mworker.c: In function 'mworker_catch_sigchld':
  src/mworker.c:285: warning: implicit declaration of function 'strsignal'
  src/mworker.c:285: warning: pointer/integer type mismatch in conditional expression
  ..

  $ make V=1 reg-tests REGTESTS_TYPES=slow,default
  ..
  ###### Test case: reg-tests/mcli/mcli_start_progs.vtc ######
  ## test results in: "/tmp/haregtests-2021-01-19_15-18-07.n24989/vtc.29077.28f6153d"
  ---- h1    Bad exit status: 0x008b exit 0x0 signal 11 core 128
  ---- h1    Assert error in haproxy_wait(), src/vtc_haproxy.c line 792:  Condition(*(&h->fds[1]) >= 0) not true.  Errno=0 Success
  ..

  $ gdb ./haproxy /tmp/core.0.haproxy.30270
  ..
  Core was generated by `/root/haproxy/haproxy -d -W -S fd@8 -dM -f /tmp/haregtests-2021-01-19_15-18-07.'.
  Program terminated with signal 11, Segmentation fault.
  #0  0x00002aaaab387a10 in strlen () from /lib64/libc.so.6
  (gdb) bt
  #0  0x00002aaaab387a10 in strlen () from /lib64/libc.so.6
  #1  0x00002aaaab354b69 in vfprintf () from /lib64/libc.so.6
  #2  0x00002aaaab37788a in vsnprintf () from /lib64/libc.so.6
  #3  0x00000000004a76a3 in memvprintf (out=0x7fffedc680a0, format=0x5a5d58 "Current worker #%d (%d) exited with code %d (%s)\n", orig_args=0x7fffedc680d0)
      at src/tools.c:3868
  #4  0x00000000004bbd40 in print_message (label=0x58abed "ALERT", fmt=0x5a5d58 "Current worker #%d (%d) exited with code %d (%s)\n", argp=0x7fffedc680d0)
      at src/log.c:1066
  #5  0x00000000004bc07f in ha_alert (fmt=0x5a5d58 "Current worker #%d (%d) exited with code %d (%s)\n") at src/log.c:1109
  #6  0x0000000000534b7b in mworker_catch_sigchld (sh=<value optimized out>) at src/mworker.c:293
  #7  0x0000000000556af3 in __signal_process_queue () at src/signal.c:88
  #8  0x00000000004f6216 in signal_process_queue () at include/haproxy/signal.h:39
  #9  run_poll_loop () at src/haproxy.c:2859
  #10 0x00000000004f63b7 in run_thread_poll_loop (data=<value optimized out>) at src/haproxy.c:3028
  #11 0x00000000004faaac in main (argc=<value optimized out>, argv=0x7fffedc68498) at src/haproxy.c:904

See: https://man7.org/linux/man-pages/man3/strsignal.3.html

Must be backported as far as 2.0.

(cherry picked from commit 25439de1811aeda7bf472f70f40c3fac9725907e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: mux_h2: missing space between "st" and ".flg" in the "show fd" helper
Willy Tarreau [Wed, 20 Jan 2021 14:50:03 +0000 (15:50 +0100)]
BUG/MINOR: mux_h2: missing space between "st" and ".flg" in the "show fd" helper

That was causing confusing outputs like this one whenan H2S is known:

   1030 : ... last_h2s=0x2ed8390 .id=775 .st=HCR.flg=0x4001 .rxbuf=...
                                                ^^^^

This was introduced by commit ab2ec4540 in 2.1-dev2 so the fix can be
backported as far as 2.1.

(cherry picked from commit ed4464e6c698bfb410610b7f3850cd0fa3331eb1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: peers: Wrong "new_conn" value for "show peers" CLI command.
Frédéric Lécaille [Mon, 18 Jan 2021 14:14:39 +0000 (15:14 +0100)]
BUG/MINOR: peers: Wrong "new_conn" value for "show peers" CLI command.

This counter could be hugely incremented by the peer task responsible of managing
peer synchronizations and reconnections, for instance when a peer is not reachable
there is a period where the appctx is not created. If we receive  stick-table
updates before the peer session (appctx) is instantiated, we reach the code
responsible of incrementing the "new_conn" counter.
With this patch we increment this counter only when we really instantiate a new
peer session thanks to peer_session_create().

May be backported as far as 2.0.

(cherry picked from commit 2b0ba54ddb7d555bd0116eb8278b37ac409e7779)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: build: discard echoing in help target
Bertrand Jacquin [Sun, 17 Jan 2021 18:47:47 +0000 (18:47 +0000)]
MINOR: build: discard echoing in help target

When V=1 is used in conjuction with help, the output becomes pretty
difficult to read properly.

  $ make TARGET=linux-glibc V=1 help
  ..
    DEBUG_USE_ABORT: use abort() for program termination, see include/haproxy/bug.h for details
  echo; \
     if [ -n "" ]; then \
       if [ -n "" ]; then \
          echo "Current TARGET: "; \
       else \
          echo "Current TARGET:  (custom target)"; \
       fi; \
     else \
       echo "TARGET not set, you may pass 'TARGET=xxx' to set one among :";\
       echo "  linux-glibc, linux-glibc-legacy, solaris, freebsd, dragonfly, netbsd,"; \
       echo "  osx, openbsd, aix51, aix52, aix72-gcc, cygwin, haiku, generic,"; \
       echo "  custom"; \
     fi

  TARGET not set, you may pass 'TARGET=xxx' to set one among :
    linux-glibc, linux-glibc-legacy, solaris, freebsd, dragonfly, netbsd,
    osx, openbsd, aix51, aix52, aix72-gcc, cygwin, haiku, generic,
    custom
  echo;echo "Enabled features for TARGET '' (disable with 'USE_xxx=') :"

  Enabled features for TARGET '' (disable with 'USE_xxx=') :
  set --        POLL                                  ; echo "  $*" | (fmt || cat) 2>/dev/null
    POLL
  echo;echo "Disabled features for TARGET '' (enable with 'USE_xxx=1') :"

  Disabled features for TARGET '' (enable with 'USE_xxx=1') :
  set -- EPOLL KQUEUE NETFILTER PCRE PCRE_JIT PCRE2 PCRE2_JIT  PRIVATE_CACHE THREAD PTHREAD_PSHARED BACKTRACE STATIC_PCRE STATIC_PCRE2 TPROXY LINUX_TPROXY LINUX_SPLICE LIBCRYPT CRYPT_H GETADDRINFO OPENSSL LUA FUTEX ACCEPT4 CLOSEFROM ZLIB SLZ CPU_AFFINITY TFO NS DL RT DEVICEATLAS 51DEGREES WURFL SYSTEMD OBSOLETE_LINKER PRCTL THREAD_DUMP EVPORTS OT QUIC; echo "  $*" | (fmt || cat) 2>/dev/null
    EPOLL KQUEUE NETFILTER PCRE PCRE_JIT PCRE2 PCRE2_JIT PRIVATE_CACHE

This commit ensure the help target always discard line echoing
regardless of V variable as done for reg-tests-help target.

(cherry picked from commit b399a992f34f248466f4740432b506455d89236e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: peers: Possible appctx pointer dereference.
Frédéric Lécaille [Sun, 17 Jan 2021 12:08:39 +0000 (13:08 +0100)]
BUG/MINOR: peers: Possible appctx pointer dereference.

This bug may occur when enabling peers traces. It is possible that
peer->appctx is NULL when entering peer_session_release().

(cherry picked from commit 4b1a05fcf81b4343d29aeb86394d44f29d2116a4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: stats: add missing INF_BUILD_INFO definition
Adis Nezirovic [Fri, 15 Jan 2021 12:12:33 +0000 (13:12 +0100)]
BUG/MEDIUM: stats: add missing INF_BUILD_INFO definition

commit 5a982a71656ce885be4b1d4b90b8db31204788a1 ("MINOR:
contrib/prometheus-exporter: export build_info") is breaking lua
`core.get_info()`.

This patch makes sure build_info is correctly initialised in all cases.

Reviewed-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit b62b78be131de1848d71350d369deac07daf448a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUILD: peers: fix build warning about unused variable
Willy Tarreau [Fri, 15 Jan 2021 16:08:38 +0000 (17:08 +0100)]
BUILD: peers: fix build warning about unused variable

Previous commit da2b0844f ("MINOR: peers: Add traces for peer control
messages.") introduced a build warning on some compiler versions after
the removal of variable "peers" in peer_send_msgs() because variable
"s" was used only to assign this one, and variable "si" to assign "s".
Let's remove both to fix the warning. No backport is needed.

(cherry picked from commit 81d7092dbd25db04cc72d2dca77647d548342256)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: dns: SRV records ignores duplicated AR records (v2)
Baptiste Assmann [Fri, 15 Jan 2021 16:01:24 +0000 (17:01 +0100)]
BUG/MINOR: dns: SRV records ignores duplicated AR records (v2)

V2 of this fix which includes a missing pointer initialization which was
causing a segfault in v1 (949a7f64591458eb06c998acf409093ea991dc3a)

This bug happens when a service has multiple records on the same host
and the server provides the A/AAAA resolution in the response as AR
(Additional Records).

In such condition, the first occurence of the host will be taken from
the Additional section, while the second (and next ones) will be process
by an independent resolution task (like we used to do before 2.2).
This can lead to a situation where the "synchronisation" of the
resolution may diverge, like described in github issue #971.

Because of this behavior, HAProxy mixes various type of requests to
resolve the full list of servers: SRV+AR for all "first" occurences and
A/AAAA for all other occurences of an existing hostname.
IE: with the following type of response:

   ;; ANSWER SECTION:
   _http._tcp.be2.tld.     3600    IN      SRV     5 500 80 A2.tld.
   _http._tcp.be2.tld.     3600    IN      SRV     5 500 86 A3.tld.
   _http._tcp.be2.tld.     3600    IN      SRV     5 500 80 A1.tld.
   _http._tcp.be2.tld.     3600    IN      SRV     5 500 85 A3.tld.

   ;; ADDITIONAL SECTION:
   A2.tld.                 3600    IN      A       192.168.0.2
   A3.tld.                 3600    IN      A       192.168.0.3
   A1.tld.                 3600    IN      A       192.168.0.1
   A3.tld.                 3600    IN      A       192.168.0.3

the first A3 host is resolved using the Additional Section and the
second one through a dedicated A request.

When linking the SRV records to their respective Additional one, a
condition was missing (chek if said SRV record is already attached to an
Additional one), leading to stop processing SRV only when the target
SRV field matches the Additional record name. Hence only the first
occurence of a target was managed by an additional record.
This patch adds a condition in this loop to ensure the record being
parsed is not already linked to an Additional Record. If so, we can
carry on the parsing to find a possible next one with the same target
field value.

backport status: 2.2 and above

(cherry picked from commit 6554742b15880bc19e41f89761905baed33bc398)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: peers: Add traces for peer control messages.
Frédéric Lécaille [Fri, 15 Jan 2021 15:21:28 +0000 (16:21 +0100)]
MINOR: peers: Add traces for peer control messages.

Display traces when sending/receiving peer control messages (synchronisation, heartbeat).
Add remaining traces when parsing malformed messages (acks, stick-table definitions)
or ignoring them.
Also add traces when releasing session or when reaching the PEER_SESS_ST_ERRPROTO
peer protocol state.

(cherry picked from commit da2b0844fc4eafd9c63e3eb20a1a7020d9f92088)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: threads: Fixes the number of possible cpus report for Mac.
David CARLIER [Fri, 15 Jan 2021 08:09:56 +0000 (08:09 +0000)]
BUG/MINOR: threads: Fixes the number of possible cpus report for Mac.

There is no low level api to achieve same as Linux/FreeBSD, we rely
on CPUs available. Without this, the number of threads is just 1 for
Mac while having 8 cores in my M1.

Backporting to 2.1 should be enough if that's possible.

Signed-off-by: David CARLIER <devnexen@gmail.com>
(cherry picked from commit 6a9060189d66ca931984706d5e2a970ed913f457)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: server: Forbid server definitions in frontend sections
Christopher Faulet [Wed, 13 Jan 2021 12:14:13 +0000 (13:14 +0100)]
MINOR: server: Forbid server definitions in frontend sections

An fatal error is now reported if a server is defined in a frontend
section. til now, a warning was just emitted and the server was ignored. The
warning was added in the 1.3.4 when the frontend/backend keywords were
introduced to allow a smooth transition and to not break existing
configs. It is old enough now to emit an fatal error in this case.

This patch is related to the issue #1043. It may be backported at least as
far as 2.2, and possibly to older versions. It relies on the previous commit
("MINOR: config: Add failifnotcap() to emit an alert on proxy capabilities").

(cherry picked from commit e3bdc81f8aeb02b5c92b5d10fc9352daaea1cf20)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: config: Add failifnotcap() to emit an alert on proxy capabilities
Christopher Faulet [Wed, 13 Jan 2021 11:10:00 +0000 (12:10 +0100)]
MINOR: config: Add failifnotcap() to emit an alert on proxy capabilities

This function must be used to emit an alert if a proxy does not have at
least one of the requested capabilities. An additional message may be
appended to the alert.

(cherry picked from commit d4a83dd6b30a68605f13a447b398a15ab38f28d4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: init: Use a dynamic buffer to set HAPROXY_CFGFILES env variable
Christopher Faulet [Tue, 12 Jan 2021 17:57:38 +0000 (18:57 +0100)]
BUG/MINOR: init: Use a dynamic buffer to set HAPROXY_CFGFILES env variable

The HAPROXY_CFGFILES env variable is built using a static trash chunk, via a
call to get_trash_chunk() function. This chunk is reserved during the whole
configuration parsing. It is far too large to guarantee it will not be
reused during the configuration parsing. And in fact, it happens in the lua
code since the commit f67442efd ("BUG/MINOR: lua: warn when registering
action, conv, sf, cli or applet multiple times"), when a lua script is
loaded.

To fix the bug, we now use a dynamic buffer instead. And we call memprintf()
function to handle both the allocation and the formatting. Allocation errors
at this stage are fatal.

This patch should fix the issue #1041. It must be backported as far as 2.0.

(cherry picked from commit 4e36682d51f4e206c63d792a8cde3e669fb8a0d4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years ago[RELEASE] Released version 2.3.4 v2.3.4
Christopher Faulet [Wed, 13 Jan 2021 15:10:29 +0000 (16:10 +0100)]
[RELEASE] Released version 2.3.4

Released version 2.3.4 with the following main changes :
    - MINOR: reg-tests: add a way to add service dependency
    - BUG/MINOR: sample: check alloc_trash_chunk return value in concat()
    - BUG/MINOR: reg-tests: fix service dependency script
    - MINOR: reg-tests: add base prometheus test
    - Revert "BUG/MINOR: dns: SRV records ignores duplicated AR records"
    - BUG/MINOR: sample: Memory leak of sample_expr structure in case of error
    - BUG/MINOR: check: Don't perform any check on servers defined in a frontend
    - BUG/MINOR: init: enforce strict-limits when using master-worker
    - MINOR: contrib/prometheus-exporter: avoid connection close header
    - MINOR: contrib/prometheus-exporter: use fill_info for process dump

4 years agoMINOR: contrib/prometheus-exporter: use fill_info for process dump
William Dauchy [Mon, 11 Jan 2021 19:07:49 +0000 (20:07 +0100)]
MINOR: contrib/prometheus-exporter: use fill_info for process dump

use `stats_fill_info` when possible to avoid duplicating code.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit 5d9b8f3c9347a1a10b86f81d70b22c3cab0e6925)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: contrib/prometheus-exporter: avoid connection close header
William Dauchy [Mon, 11 Jan 2021 19:07:48 +0000 (20:07 +0100)]
MINOR: contrib/prometheus-exporter: avoid connection close header

it does not seem to have a reason to close connections after each
request; reflect that in tests by doing all requests within the same
client.

Signed-off-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit 1704efee89c9a0d430e63e7a9b54e97644d27a2a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: init: enforce strict-limits when using master-worker
Jerome Magnin [Tue, 12 Jan 2021 19:19:38 +0000 (20:19 +0100)]
BUG/MINOR: init: enforce strict-limits when using master-worker

The strict-limits global option was introduced with commit 0fec3ab7b
("MINOR: init: always fail when setrlimit fails"). When used in
conjuction with master-worker, haproxy will not fail when a setrlimit
fails. This happens because we only exit() if master-worker isn't used.

This patch removes all tests for master-worker mode for all cases covered
by strict-limits scope.

This should be backported from 2.1 onward.
This should fix issue #1042.

Reviewed by William Dauchy <wdauchy@gmail.com>

(cherry picked from commit 50f757c5fdd7e6eb404103d878f648453d00d73e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: check: Don't perform any check on servers defined in a frontend
Christopher Faulet [Tue, 12 Jan 2021 16:29:45 +0000 (17:29 +0100)]
BUG/MINOR: check: Don't perform any check on servers defined in a frontend

If a server is defined in a frontend, thus a proxy without the backend
capability, the 'check' and 'agent-check' keywords are ignored. This way, no
check is performed on an ignored server. This avoids a segfault because some
part of the tcpchecks are not fully initialized (or released for frontends
during the post-check).

In addition, an test on the server's proxy capabilities is performed when
checks or agent-checks are initialized and nothing is performed for servers
attached to a non-backend proxy.

This patch should fix the issue #1043. It must be backported as far as 2.2.

(cherry picked from commit 6ecd59326f7416dcba215f05a6674d4b9f970749)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: sample: Memory leak of sample_expr structure in case of error
Remi Tricot-Le Breton [Tue, 12 Jan 2021 13:55:12 +0000 (14:55 +0100)]
BUG/MINOR: sample: Memory leak of sample_expr structure in case of error

If an errors occurs during the sample expression parsing, the alloced
sample_expr is not freed despite having its main pointer reset.

This fixes GitHub issue #1046.
It could be backported as far as 1.8.

(cherry picked from commit 22e0d9b39cfb238e7bf866b3d78aa9b393004508)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoRevert "BUG/MINOR: dns: SRV records ignores duplicated AR records"
Christopher Faulet [Tue, 12 Jan 2021 09:27:26 +0000 (10:27 +0100)]
Revert "BUG/MINOR: dns: SRV records ignores duplicated AR records"

This reverts commit 949a7f64591458eb06c998acf409093ea991dc3a.

The first part of the patch introduces a bug. When a dns answer item is
allocated, its <ar_item> is only initialized at the end of the parsing, when
the item is added in the answer list. Thus, we must not try to release it
during the parsing.

The second part is also probably buggy. It fixes the issue #971 but reverts
a fix for the issue #841 (see commit fb0884c8297 "BUG/MEDIUM: dns: Don't
store additional records in a linked-list"). So it must be at least
revalidated.

This revert fixes a segfault reported in a comment of the issue #971. It
must be backported as far as 2.2.

(cherry picked from commit a1eea3bbb1973f3becc4603c3bbad4e96af3361f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: reg-tests: add base prometheus test
William Dauchy [Sun, 10 Jan 2021 20:13:06 +0000 (21:13 +0100)]
MINOR: reg-tests: add base prometheus test

Add a base test to start with something, even though this is not
necessarily complete.
Also make use of the recent REQUIRE_SERVICE option to exclude it from
test list of it was not build with prometheus included.

note: I thought it was possible to send multiple requests within the
same client, but I'm getting "HTTP header is incomplete" from the second
request

Signed-off-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit 76603f2552fafc23871a914600a18c3616959693)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: reg-tests: fix service dependency script
William Dauchy [Sun, 10 Jan 2021 20:13:05 +0000 (21:13 +0100)]
BUG/MINOR: reg-tests: fix service dependency script

I badly tested my previous patch forgetting to remove the "+" testing
present in options, and not in services; the list of services do not
have any "+" at the beginning of each service

this patch is fixing commit aabde7133242563109b4f36c42e732f083 ("MINOR:
reg-tests: add a way to add service dependency")

Signed-off-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit 4488434c97b4ae3ae8fde501b37f5ef9a588bd09)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: sample: check alloc_trash_chunk return value in concat()
William Dauchy [Mon, 11 Jan 2021 10:05:58 +0000 (11:05 +0100)]
BUG/MINOR: sample: check alloc_trash_chunk return value in concat()

like it is done in other places, check the return value of
`alloc_trash_chunk` before using it. This was detected by coverity.

this patch fixes commit 591fc3a330005c289b4705fe4cb37c4eec9f9eed
("BUG/MINOR: sample: fix concat() converter's corruption with non-string
variables"
As a consequence, this patch should be backported as far as 2.0

this should fix github issue #1039

Signed-off-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit e997010accdb17e70b628bc1e7c2810487d279c0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: reg-tests: add a way to add service dependency
William Dauchy [Sat, 9 Jan 2021 16:26:20 +0000 (17:26 +0100)]
MINOR: reg-tests: add a way to add service dependency

I was looking at writing a simple first test for prometheus but I
realised there is no proper way to exclude it if haproxy was not built
with prometheus plugin.

Today we have `REQUIRE_OPTIONS` in reg-tests which is based on `Feature
list` from `haproxy -vv`. Those options are coming from the Makefile
itself.

A plugin is build this way:
  EXTRA_OBJS="contrib/prometheus-exporter/service-prometheus.o"

It does register service actions through `service_keywords_register`.
Those are listed through `list_services` in `haproxy -vv`.
To facilitate parsing, I slightly changed the output to a single line
and integrate it in regtests shell script so that we can now specify a
dependency while writing a reg-test for prometheus, e.g:

  #REQUIRE_SERVICE=prometheus-exporter
  #REQUIRE_SERVICES=prometheus-exporter,foo

There might be other ways to handle this, but that's the cleanest I
found; I understand people might be concerned by this output change in
`haproxy -vv` which goes from:

  Available services :
          foo
          bar

to:

  Available services : foo bar

Signed-off-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit aabde7133242563109b4f36c42e732f083bddf4d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years ago[RELEASE] Released version 2.3.3 v2.3.3
Willy Tarreau [Fri, 8 Jan 2021 20:19:40 +0000 (21:19 +0100)]
[RELEASE] Released version 2.3.3

Released version 2.3.3 with the following main changes :
    - MINOR: plock: use an ARMv8 instruction barrier for the pause instruction
    - BUG/MEDIUM: lists: Lock the element while we check if it is in a list.
    - MINOR: task: remove __tasklet_remove_from_tasklet_list()
    - BUG/MEDIUM: task: close a possible data race condition on a tasklet's list link
    - BUG/MEDIUM: local log format regression.
    - BUG/MINOR: mux-h2/stats: make stream/connection proto errors more accurate
    - BUG/MINOR: mux-h2/stats: not all GOAWAY frames are errors
    - BUG/MINOR: lua: missing "\n" in error message
    - BUG/MINOR: lua: lua-load doesn't check its parameters
    - BUG/MINOR: lua: Post init register function are not executed beyond the first one
    - BUG/MINOR: lua: Some lua init operation are processed unsafe
    - MINOR: actions: Export actions lookup functions
    - MINOR: actions: add a function returning a service pointer from its name
    - MINOR: cli: add a function to look up a CLI service description
    - BUG/MINOR: lua: warn when registering action, conv, sf, cli or applet multiple times
    - BUG/MAJOR: ring: tcp forward on ring can break the reader counter.
    - BUILD/MINOR: haproxy DragonFlyBSD affinity build update.
    - DOC/MINOR: Fix formatting in Management Guide
    - BUG/MINOR: listener: use sockaddr_in6 for IPv6
    - BUG/MINOR: mux-h1: Handle keep-alive timeout for idle frontend connections
    - MINOR: protocol: add a ->set_port() helper to address families
    - MINOR: listener: automatically set the port when creating listeners
    - MINOR: listener: now use a generic add_listener() function
    - MEDIUM: ssl: fatal error with bundle + openssl < 1.1.1
    - BUG/MAJOR: spoa/python: Fixing return None
    - DOC: spoa/python: Fixing typo in IP related error messages
    - DOC: spoa/python: Rephrasing memory related error messages
    - DOC: spoa/python: Fixing typos in comments
    - BUG/MINOR: spoa/python: Cleanup references for failed Module Addobject operations
    - BUG/MINOR: spoa/python: Cleanup ipaddress objects if initialization fails
    - BUG/MEDIUM: spoa/python: Fixing PyObject_Call positional arguments
    - BUG/MEDIUM: spoa/python: Fixing references to None
    - DOC: email change of the DeviceAtlas maintainer
    - BUG/MINOR: http-check: Use right condition to consider HTX message as full
    - BUG/MINOR: tcpcheck: Don't rearm the check timeout on each read
    - MINOR: tcpcheck: Only wait for more payload data on HTTP expect rules
    - BUG/MINOR: tools: make parse_time_err() more strict on the timer validity
    - BUG/MINOR: tools: Reject size format not starting by a digit
    - BUG/MEDIUM: lb-leastconn: Reposition a server using the right eweight
    - BUG/MEDIUM: ssl/crt-list: bad behavior with "commit ssl cert"
    - REGTESTS: make use of HAPROXY_ARGS and pass -dM by default
    - BUILD: SSL: fine guard for SSL_CTX_add_server_custom_ext call
    - BUILD: Makefile: have "make clean" destroy .o/.a/.s in contrib subdirs as well
    - BUG/MINOR: mux-h1: Don't set CS_FL_EOI too early for protocol upgrade requests
    - BUG/MEDIUM: http-ana: Never for sending data in TUNNEL mode
    - BUG/MEDIUM: mux-h1: Handle h1_process() failures on a pipelined request
    - CONTRIB: halog: fix build issue caused by %L printf format
    - CONTRIB: halog: mark the has_zero* functions unused
    - CONTRIB: halog: fix signed/unsigned build warnings on counts and timestamps
    - CONTRIB: debug: address "poll" utility build on non-linux platforms
    - BUILD: plock: remove dead code that causes a warning in gcc 11
    - BUILD: ssl: fine guard for SSL_CTX_get0_privatekey call
    - BUG/MINOR: dns: SRV records ignores duplicated AR records
    - DOC: fix "smp_size" vs "sample_size" in "log" directive arguments
    - BUG/MEDIUM: mux_h2: Add missing braces in h2_snd_buf()around trace+wakeup
    - BUILD: hpack: hpack-tbl-t.h uses VAR_ARRAY but does not include compiler.h
    - MINOR: atomic: don't use ; to separate instruction on aarch64.
    - BUG/MINOR: sink: Return an allocation failure in __sink_new if strdup() fails
    - BUG/MINOR: cfgparse: Fail if the strdup() for `rule->be.name` for `use_backend` fails
    - BUG/MINOR: tcpcheck: Report a L7OK if the last evaluated rule is a send rule
    - DOC: Improve the message printed when running `make` w/o `TARGET`
    - BUG/MINOR: stats: Make stat_l variable used to dump a stat line thread local
    - SCRIPTS: improve announce-release to support different tag and versions
    - SCRIPTS: make announce release support preparing announces before tag exists
    - BUG/MINOR: srv: do not init address if backend is disabled
    - BUG/MINOR: srv: do not cleanup idle conns if pool max is null
    - MINOR: converter: adding support for url_enc
    - BUILD: Makefile: exclude broken tests by default
    - CLEANUP: cfgparse: replace "realloc" with "my_realloc2" to fix to memory leak on error
    - MINOR: contrib/prometheus-exporter: export build_info
    - DOC: fix some spelling issues over multiple files
    - SCRIPTS: announce-release: fix typo in help message
    - DOC: Add maintainers for the Prometheus exporter
    - BUG/MINOR: sample: fix concat() converter's corruption with non-string variables

4 years agoBUG/MINOR: sample: fix concat() converter's corruption with non-string variables
Willy Tarreau [Fri, 8 Jan 2021 15:08:43 +0000 (16:08 +0100)]
BUG/MINOR: sample: fix concat() converter's corruption with non-string variables

Patrick Hemmer reported that calling concat() with an integer variable
causes a %00 to appear at the beginning of the output. Looking at the
code, it's not surprising. The function uses get_trash_chunk() to get
one of the trashes, but can call casting functions which will also use
their trash in turn and will cycle back to ours, causing the trash to
be overwritten before being assigned to a sample.

By allocating the trash from a pool using alloc_trash_chunk(), we can
avoid this. However we must free it so the trash's contents must be
moved to a permanent trash buffer before returning. This is what's
achieved using smp_dup().

This should be backported as far as 2.0.

(cherry picked from commit 591fc3a330005c289b4705fe4cb37c4eec9f9eed)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoDOC: Add maintainers for the Prometheus exporter
Christopher Faulet [Fri, 8 Jan 2021 13:39:04 +0000 (14:39 +0100)]
DOC: Add maintainers for the Prometheus exporter

William Dauchy and Christopher Faulet are the official maintainers of the
Prometheus exporter. William better knows the Prometheus usages and is the
referent for the features while Christopher's role is more code-centric, he
takes care of bugs and the exporter integration into Haproxy.

(cherry picked from commit 84c88a58c5153b8fffa509934375c92435298a1b)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoSCRIPTS: announce-release: fix typo in help message
Thayne McCombs [Fri, 8 Jan 2021 04:36:27 +0000 (21:36 -0700)]
SCRIPTS: announce-release: fix typo in help message

s/relase/release in -p help message.

(cherry picked from commit 478e5ddc8d2ecab13391c96a4001081b1cf9b981)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoDOC: fix some spelling issues over multiple files
Thayne McCombs [Fri, 8 Jan 2021 04:24:41 +0000 (21:24 -0700)]
DOC: fix some spelling issues over multiple files

This is from the output of codespell and may be backported.

(cherry picked from commit cdbcca9995a4974c6998720e158d6f1d4d9a25c0)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: contrib/prometheus-exporter: export build_info
William Dauchy [Fri, 8 Jan 2021 12:18:06 +0000 (13:18 +0100)]
MINOR: contrib/prometheus-exporter: export build_info

commit c55a626217a7e676e1cc ("MINOR: contrib/prometheus-exporter: Add
missing global and per-server metrics") is renaming two metrics between
v2.2 and v2.3:
  server_idle_connections_current
  server_idle_connections_limit

It is breaking some tools which are making use of those metrics while
supporting several haproxy versions. This build_info will permit tools
which make use of metrics to be able to match the haproxy version and
change the list of expected metrics. This was possible using the haproxy
stats socket but not with prometheus export.

This patch follows prometheus best pratices to export specific software
informations. It is adding a new field `build_info` so we can extend it
to other parameters if needed in the future.

example output:
  # HELP haproxy_process_build_info HAProxy build info.
  # TYPE haproxy_process_build_info gauge
  haproxy_process_build_info{version="2.4-dev5-2e1a3f-5"} 1

Even though it is not a bugfix, this patch will make more sense when
backported up to >= 2.0

Signed-off-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit 5a982a71656ce885be4b1d4b90b8db31204788a1)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoCLEANUP: cfgparse: replace "realloc" with "my_realloc2" to fix to memory leak on...
Ilya Shipitsin [Thu, 7 Jan 2021 17:45:13 +0000 (22:45 +0500)]
CLEANUP: cfgparse: replace "realloc" with "my_realloc2" to fix to memory leak on error

my_realloc2 frees variable in case of allocation failure.

fixes #1030

realloc was introduced in 9e1758efbd68c8b1d27e17e2abe4444e110f3ebe

this might be backported to 2.2, 2.3

(cherry picked from commit 76837bc948578e91e113141bd4c73dad8093be3b)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUILD: Makefile: exclude broken tests by default
William Dauchy [Thu, 7 Jan 2021 16:10:51 +0000 (17:10 +0100)]
BUILD: Makefile: exclude broken tests by default

it could be sometimes a bit confusing to have tests which are known to
be broken executed in the default `make reg-tests` command, especially
for not frequent contributors which are not necessarily aware of all our
quirks.

without this patch, this test is failing on my side:
  #    top  TEST reg-tests/seamless-reload/abns_socket.vtc FAILED (2.228) exit=2
  1 tests failed, 0 tests skipped, 107 tests passed

Signed-off-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit 3bad3d5ee64caec53077018a848075bdc817580e)
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoMINOR: converter: adding support for url_enc
William Dauchy [Wed, 6 Jan 2021 22:39:50 +0000 (23:39 +0100)]
MINOR: converter: adding support for url_enc

add base support for url encode following RFC3986, supporting `query`
type only.

- add test checking url_enc/url_dec/url_enc
- update documentation
- leave the door open for future changes

this should resolve github issue #941

Signed-off-by: William Dauchy <wdauchy@gmail.com>
(cherry picked from commit 888b0ae8cf92cb23112bd8920b256b0bdf67caf0)
[wt: updated regtest to mention 2.3]
Signed-off-by: Willy Tarreau <w@1wt.eu>

4 years agoBUG/MINOR: srv: do not cleanup idle conns if pool max is null
Amaury Denoyelle [Wed, 6 Jan 2021 13:28:51 +0000 (14:28 +0100)]
BUG/MINOR: srv: do not cleanup idle conns if pool max is null

If a server is configured to not have any idle conns, returns immediatly
from srv_cleanup_connections. This avoids a segfault when a server is
configured with pool-max-conn to 0.

This should be backported up to 2.2.

(cherry picked from commit 10d5c3172b366e48b5851a20de9b68f0d5495b92)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: srv: do not init address if backend is disabled
Amaury Denoyelle [Wed, 6 Jan 2021 13:28:50 +0000 (14:28 +0100)]
BUG/MINOR: srv: do not init address if backend is disabled

Do not proceed on init_addr if the backend of the server is marked as
disabled. When marked as disabled, the server is not fully initialized
and some operation must be avoided to prevent segfault. It is correct
because there is no way to activate a disabled backend.

This fixes the github issue #1031.
This should be backported to 2.2.

(cherry picked from commit e3c419296211f08209300b9a06dc17467a6cb315)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoSCRIPTS: make announce release support preparing announces before tag exists
Willy Tarreau [Wed, 6 Jan 2021 14:46:33 +0000 (15:46 +0100)]
SCRIPTS: make announce release support preparing announces before tag exists

It takes so much time to write an announce message that sometimes it's
annoying not being able to start the work while a fix is being finished.
With the new "-p" argument, announce-release will allow to prepare the
announce message for the current HEAD and with no tag yet. It will
restart from the last tag and automatically increment the version using
the same algorithm as create-release so that everything is accurate. It
should then be easier at the last moment to just include the final entry
by hand when the last fix finally arrives. For convenience, this argument
also allows to create an announce from another branch than master.

(cherry picked from commit 96d5368202705de79756ebe7da78beb1af11fb2d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoSCRIPTS: improve announce-release to support different tag and versions
Willy Tarreau [Wed, 6 Jan 2021 14:16:46 +0000 (15:16 +0100)]
SCRIPTS: improve announce-release to support different tag and versions

By having three variables it will be easier to preset the version and
the tag separately. One contains the announced version, another one the
associated tag and the last one the final commit ID (used as the ending
point before the release). This initially allows to check for the HEAD
matching the tag only when the version was not forced, hence re-announce
already tagged versions after some extra commits were added for example.

(cherry picked from commit 0c612936b2f4319dcc55567af4aa425b1d6d52c7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: stats: Make stat_l variable used to dump a stat line thread local
Christopher Faulet [Wed, 6 Jan 2021 06:41:56 +0000 (07:41 +0100)]
BUG/MINOR: stats: Make stat_l variable used to dump a stat line thread local

Since ee63d4bd6 ("MEDIUM: stats: integrate static proxies stats in new
stats"), all dumped stats for a given domain, the default ones and the
modules ones, are merged in a signle array to dump them in a generic way.
For this purpose, the stat_l global variable is allocated at startup to
store a line of stats before the dump, i.e. all stats of an entity
(frontend, backend, listener, server or dns nameserver). But this variable
is not thread safe.  If stats are retrieved concurrently by several clients
on different threads, the same variable is used. This leads to corrupted
stats output.

To fix the bug, the stat_l variable is now thread local.

This patch should probably solve issues #972 and #992. It must be backported
to 2.3.

(cherry picked from commit de79cd28ec9e1facc5f2da02cd95dda82ac6fe58)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoDOC: Improve the message printed when running `make` w/o `TARGET`
Tim Duesterhus [Tue, 5 Jan 2021 17:10:41 +0000 (18:10 +0100)]
DOC: Improve the message printed when running `make` w/o `TARGET`

Rephrase the message to no longer talk about something that "is no longer
supported", but about what actually *is* supported.

Adjustments include:

- Removal of rare targets to make it easier to find the proper one.
- Reformatting to be easier to read (more newlines)
- Explanation of common non-default feature flags.

(cherry picked from commit 1ab76dd956042019aabbfc2a04dc6a3101ce1a02)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: tcpcheck: Report a L7OK if the last evaluated rule is a send rule
Christopher Faulet [Tue, 5 Jan 2021 15:56:07 +0000 (16:56 +0100)]
BUG/MINOR: tcpcheck: Report a L7OK if the last evaluated rule is a send rule

When all rules of a tcpcheck ruleset are successfully evaluated, the right
check status must always be reported. It is true if the last evaluated rule
is an expect or a connect rule. But not if it is a send rule. In this
situation, nothing more is done until the check timeout expiration and a
L7TOUT is reported instead of a L7OK.

Now, by default, when all rules were successfully evaluated, a L7OK is
reported. When the last evaluated rule is an expect or a connect, the
behavior remains unchanged.

This patch should fix the issue #1027. It must be backported as far as 2.2.

(cherry picked from commit 8d4977ae86bf70e6834f3e28bad5876769db5b07)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: cfgparse: Fail if the strdup() for `rule->be.name` for `use_backend` fails
Tim Duesterhus [Sun, 3 Jan 2021 21:54:43 +0000 (22:54 +0100)]
BUG/MINOR: cfgparse: Fail if the strdup() for `rule->be.name` for `use_backend` fails

This patch fixes GitHub issue #1024.

I could track the `strdup` back to commit
3a1f5fda109fc56ae127d03eaf34ce027c9542e1 which is 1.9-dev8. It's probably not
worth the effort to backport it across this refactoring.

This patch should be backported to 1.9+.

(cherry picked from commit 5ce5a1586d8c67affa604dedf0faa2c5a02ecab5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: sink: Return an allocation failure in __sink_new if strdup() fails
Tim Duesterhus [Sun, 3 Jan 2021 18:54:11 +0000 (19:54 +0100)]
BUG/MINOR: sink: Return an allocation failure in __sink_new if strdup() fails

This patch fixes GitHub issue #1023.

The function was introduced in commit 99c453d ("MEDIUM: ring: new
section ring to declare custom ring buffers."), which first appeared
in 2.2-dev9. The fix should be backported to 2.2+.

(cherry picked from commit a7ebffef660c2c83590798e0f1acceb9001ff92c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: atomic: don't use ; to separate instruction on aarch64.
Olivier Houchard [Wed, 23 Dec 2020 00:23:41 +0000 (01:23 +0100)]
MINOR: atomic: don't use ; to separate instruction on aarch64.

The assembler on MacOS aarch64 interprets ; as the beginning of comments,
so it is not suitable for separating instructions in inline asm. Use \n
instead.

This should be backported to 2.3, 2.2, 2.1, 2.0 and 1.9.

(cherry picked from commit 63ee28185430cbf5bdce21f82484f7de70fe4654)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUILD: hpack: hpack-tbl-t.h uses VAR_ARRAY but does not include compiler.h
Christian Ruppert [Mon, 9 Nov 2020 08:15:21 +0000 (09:15 +0100)]
BUILD: hpack: hpack-tbl-t.h uses VAR_ARRAY but does not include compiler.h

This fixes building hpack from contrib, which failed because of the
undeclared VAR_ARRAY:

make -C contrib/hpack
...
cc -O2 -Wall -g -I../../include -fwrapv -fno-strict-aliasing -c -o gen-enc.o gen-enc.c
In file included from gen-enc.c:18:
../../include/haproxy/hpack-tbl-t.h:105:23: error: 'VAR_ARRAY' undeclared here (not in a function)
  105 |  struct hpack_dte dte[VAR_ARRAY]; /* dynamic table entries */
...

As discussed in the thread below, let's redefine VAR_ARRAY in this file
so that it remains self-sustaining:

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

(cherry picked from commit b67e1558955c1c88dc2a3530322802ce3710bc88)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: mux_h2: Add missing braces in h2_snd_buf()around trace+wakeup
Tim Duesterhus [Mon, 21 Dec 2020 18:40:16 +0000 (19:40 +0100)]
BUG/MEDIUM: mux_h2: Add missing braces in h2_snd_buf()around trace+wakeup

This is a regression in 7838a79ba ("MEDIUM: mux-h2/trace: add lots of traces
all over the code"). The issue was found using -Wmisleading-indentation.
This patch fixes GitHub issue #1015.

The impact of this bug is that it could in theory cause occasional delays
on some long responses for connections having otherwise no traffic.

This patch should be backported to 2.1+, the commit was first tagged in
v2.1-dev2.

(cherry picked from commit 12a08d8849166ec0a779bbdee2ce9d768d5aa824)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoDOC: fix "smp_size" vs "sample_size" in "log" directive arguments
Jan Wagner [Thu, 17 Dec 2020 21:22:32 +0000 (22:22 +0100)]
DOC: fix "smp_size" vs "sample_size" in "log" directive arguments

The "log" directive syntax shows an argument named "smp_size" but the
description mentions "sample_size". Let's fix this.

(cherry picked from commit 3e678607e221bb38ff1032be239853b9243210fb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: dns: SRV records ignores duplicated AR records
Baptiste Assmann [Wed, 25 Nov 2020 07:17:59 +0000 (08:17 +0100)]
BUG/MINOR: dns: SRV records ignores duplicated AR records

This bug happens when a service has multiple records on the same host
and the server provides the A/AAAA resolution in the response as AR
(Additional Records).

In such condition, the first occurence of the host will be taken from
the Additional section, while the second (and next ones) will be process
by an independent resolution task (like we used to do before 2.2).
This can lead to a situation where the "synchronisation" of the
resolution may diverge, like described in github issue #971.

Because of this behavior, HAProxy mixes various type of requests to
resolve the full list of servers: SRV+AR for all "first" occurences and
A/AAAA for all other occurences of an existing hostname.
IE: with the following type of response:

   ;; ANSWER SECTION:
   _http._tcp.be2.tld.     3600    IN      SRV     5 500 80 A2.tld.
   _http._tcp.be2.tld.     3600    IN      SRV     5 500 86 A3.tld.
   _http._tcp.be2.tld.     3600    IN      SRV     5 500 80 A1.tld.
   _http._tcp.be2.tld.     3600    IN      SRV     5 500 85 A3.tld.

   ;; ADDITIONAL SECTION:
   A2.tld.                 3600    IN      A       192.168.0.2
   A3.tld.                 3600    IN      A       192.168.0.3
   A1.tld.                 3600    IN      A       192.168.0.1
   A3.tld.                 3600    IN      A       192.168.0.3

the first A3 host is resolved using the Additional Section and the
second one through a dedicated A request.

When linking the SRV records to their respective Additional one, a
condition was missing (chek if said SRV record is already attached to an
Additional one), leading to stop processing SRV only when the target
SRV field matches the Additional record name. Hence only the first
occurence of a target was managed by an additional record.
This patch adds a condition in this loop to ensure the record being
parsed is not already linked to an Additional Record. If so, we can
carry on the parsing to find a possible next one with the same target
field value.

backport status: 2.2 and above

(cherry picked from commit 949a7f64591458eb06c998acf409093ea991dc3a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUILD: ssl: fine guard for SSL_CTX_get0_privatekey call
Ilya Shipitsin [Fri, 18 Dec 2020 22:12:12 +0000 (03:12 +0500)]
BUILD: ssl: fine guard for SSL_CTX_get0_privatekey call

SSL_CTX_get0_privatekey is openssl/boringssl specific function present
since openssl-1.0.2, let us define readable guard for it, not depending
on HA_OPENSSL_VERSION

(cherry picked from commit af204881a3df36da1451af33f57b2c11ecb0972e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUILD: plock: remove dead code that causes a warning in gcc 11
Willy Tarreau [Mon, 21 Dec 2020 09:27:18 +0000 (10:27 +0100)]
BUILD: plock: remove dead code that causes a warning in gcc 11

As Ilya reported in issue #998, gcc 11 complains about misleading code
indentation which is in fact caused by dead assignments to zero after
a loop which stops on zero. Let's clean both of these.

(cherry picked from commit b1f54925fc17b00ddbe25e30c2207a8d68d47f34)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoCONTRIB: debug: address "poll" utility build on non-linux platforms
Willy Tarreau [Mon, 21 Dec 2020 07:43:50 +0000 (08:43 +0100)]
CONTRIB: debug: address "poll" utility build on non-linux platforms

MSG_NOSIGNAL and MSG_MORE are not defined everywhere, let's make them
zero when not defined. It will roughly result in the same behavior,
albeit a bit less optimal, which is no big deal when debugging. This
should fix issue #1014.

(cherry picked from commit 79d2e7bbbd56455f5f73c030df41e8e1ed621df4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoCONTRIB: halog: fix signed/unsigned build warnings on counts and timestamps
Willy Tarreau [Mon, 21 Dec 2020 07:40:04 +0000 (08:40 +0100)]
CONTRIB: halog: fix signed/unsigned build warnings on counts and timestamps

Some variables were signed while they were compared to unsigned ones,
causing warnings to be issued when -Wextra is enabled.

(cherry picked from commit 03ca6054d02350e83cb5227608ccec13efdd3ade)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoCONTRIB: halog: mark the has_zero* functions unused
Willy Tarreau [Mon, 21 Dec 2020 07:35:24 +0000 (08:35 +0100)]
CONTRIB: halog: mark the has_zero* functions unused

These ones will depend on the use of memchr() or not, let's mark them unused
to avoid the warning reported in issue #1013.

(cherry picked from commit f531dfff18becc87a3855ae6bc987afe9bffccc3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoCONTRIB: halog: fix build issue caused by %L printf format
Willy Tarreau [Mon, 21 Dec 2020 07:29:09 +0000 (08:29 +0100)]
CONTRIB: halog: fix build issue caused by %L printf format

%Ld isn't standard, %lld is more portable. In addition, the format
should be %llu since the printed values are unsigned. This should
address issue #1013.

(cherry picked from commit 2df860cb13e503fc8d80889bb93c20d5e8b24cd5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: mux-h1: Handle h1_process() failures on a pipelined request
Christopher Faulet [Fri, 18 Dec 2020 14:13:47 +0000 (15:13 +0100)]
BUG/MEDIUM: mux-h1: Handle h1_process() failures on a pipelined request

On frontend side, when a conn-stream is detached from a H1 connection, the
H1 stream is destroyed and if we already have some data to parse (a
pipelined request), we process these data immedialtely calling
h1_process(). Then we adjust the H1 connection timeout. But h1_process() may
fail and release the H1 connection. For instance, a parsing error may be
reported. Thus, when that happens, we must not use anymore the H1 connection
and exit.

This patch must be backported as far as the 2.2. This bug can impact the 2.3
and the 2.2, in theory, if h1 stream creation fails. But, concretly, it only
fails on the 2.4 because the requests are now parsed at this step.

(cherry picked from commit 0c366a87619206d1bf1c9f69698933d0e58b7be5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: http-ana: Never for sending data in TUNNEL mode
Christopher Faulet [Tue, 15 Dec 2020 12:32:55 +0000 (13:32 +0100)]
BUG/MEDIUM: http-ana: Never for sending data in TUNNEL mode

When a channel is set in TUNNEL mode, we now always set the CF_NEVER_WAIT flag,
to be sure to never wait for sending data. It is important because in TUNNEL
mode, we have no idea if more data are expected or not. Setting this flag
prevent the MSG_MORE flag to be set on the connection.

It is only a problem with the HTX, since the 2.2. On previous versions, the
MSG_MORE flag is only set on the mux initiative. In fact, the problem arises
because there is an ambiguity in tunnel mode about the HTX_FL_EOI flag. In this
mode, from the mux point of view, while the SHUTR is not received more data are
expected. But from the channel point of view, we want to send data asap.

At short term, this fix is good enough and is valid anyway. But for the long
term more reliable solution must be found. At least, the to_forward field must
regain its original meaning.

This patch must be backported as far as 2.2.

(cherry picked from commit 198ef8b1de3ddac116baeaf5b86f2f8335e69e62)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: mux-h1: Don't set CS_FL_EOI too early for protocol upgrade requests
Christopher Faulet [Mon, 7 Dec 2020 17:21:27 +0000 (18:21 +0100)]
BUG/MINOR: mux-h1: Don't set CS_FL_EOI too early for protocol upgrade requests

When a protocol upgrade request is received, once parsed, it is waiting for
the response in the DONE state. But we must not set the flag CS_FL_EOI
because we don't know if a protocol upgrade will be performed or not.

Now, it is set on the response path, if both sides reached the DONE
state. If a protocol upgrade is finally performed, both side are switched in
TUNNEL state. Thus the CS_FL_EOI flag is not set.

If backported, this patch must be adapted because for now it relies on last
2.4-dev changes. It may be backported as far as 2.0.

(cherry picked from commit 3e1748bbf30bc06407bda36517c00c0b97bfeb50)
[cf: context adjustment]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUILD: Makefile: have "make clean" destroy .o/.a/.s in contrib subdirs as well
Willy Tarreau [Wed, 16 Dec 2020 13:14:38 +0000 (14:14 +0100)]
BUILD: Makefile: have "make clean" destroy .o/.a/.s in contrib subdirs as well

Now that we sometimes link some contrib subparts directly into the
haproxy binary, it's becoming a real problem that they're not cleaned
on make clean.  Some of the tools there are useful as .so or pure
binaries and we don't want to remove them, but anything intermediary
susceptible to be linked into haproxy should be clenaed. This is what
this patch does for 3 levels of subdirs into contrib/, without touching
the rest. It should be sufficient for the vast majority of use cases.

(cherry picked from commit da867d8d68f19f9034a47d42d16db78a84a12299)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUILD: SSL: fine guard for SSL_CTX_add_server_custom_ext call
Ilya Shipitsin [Thu, 26 Nov 2020 21:39:48 +0000 (02:39 +0500)]
BUILD: SSL: fine guard for SSL_CTX_add_server_custom_ext call

SSL_CTX_add_server_custom_ext is openssl specific function present
since openssl-1.0.2, let us define readable guard for it, not depending
on HA_OPENSSL_VERSION

(cherry picked from commit ec609098718b9c1cd803ca57442b2b98c9ba4a16)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoREGTESTS: make use of HAPROXY_ARGS and pass -dM by default
Willy Tarreau [Wed, 16 Dec 2020 09:39:20 +0000 (10:39 +0100)]
REGTESTS: make use of HAPROXY_ARGS and pass -dM by default

Enabling memory poisonning is often pretty effective for detecting
uninitialized structure fields. Let's enable it by default and let
the user change the arguments at will (e.g. forcing some memory limits
or disabling a poller). This will work with the latest vtest version
to date (02a9bc1).

(cherry picked from commit a4009cd6103a92752db27c3a85051c6adcc832c1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: ssl/crt-list: bad behavior with "commit ssl cert"
William Lallemand [Tue, 15 Dec 2020 13:57:46 +0000 (14:57 +0100)]
BUG/MEDIUM: ssl/crt-list: bad behavior with "commit ssl cert"

In issue #1004, it was reported that it is not possible to remove
correctly a certificate after updating it when it came from a crt-list.

Indeed the "commit ssl cert" command on the CLI does not update the list
of ckch_inst in the crtlist_entry. Because of this, the "del ssl
crt-list" command does not remove neither the instances nor the SNIs
because they were never linked to the crtlist_entry.

This patch fixes the issue by inserting the ckch_inst in the
crtlist_entry once generated.

Must be backported as far as 2.2.

(cherry picked from commit a55685bfea0c95fd311b9bd0478950e534305786)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: lb-leastconn: Reposition a server using the right eweight
Christopher Faulet [Fri, 11 Dec 2020 14:36:01 +0000 (15:36 +0100)]
BUG/MEDIUM: lb-leastconn: Reposition a server using the right eweight

Depending on the context, the current eweight or the next one must be used
to reposition a server in the tree. When the server state is updated, for
instance its weight, the next eweight must be used because it is not yet
committed. However, when the server is used, on normal conditions, the
current eweight must be used.

In fact, it is only a bug on the 1.8. On newer versions, the changes on a
server are performed synchronously. But it is safer to rely on the right
eweight value to avoid any futur bugs.

On the 1.8, it is important to do so, because the server state is updated
and committed inside the rendez-vous point. Thus, the next server state may
be unsync with the current state for a short time, waiting all threads join
the rendez-vous point. It is especially a problem if the next eweight is set
to 0. Because otherwise, it must not be used to reposition the server in the
tree, leading to a divide by 0.

This patch must be backported as far as 1.8.

(cherry picked from commit cb33d3ac7f8dbc3f7323606a521d29dc100adbda)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: tools: Reject size format not starting by a digit
Christopher Faulet [Fri, 11 Dec 2020 08:30:45 +0000 (09:30 +0100)]
BUG/MINOR: tools: Reject size format not starting by a digit

parse_size_err() function is now more strict on the size format. The first
character must be a digit. Otherwise an error is returned. Thus "size k" is
now rejected.

This patch must be backported to all stable versions.

(cherry picked from commit 82635a0fc1d8196e5b199e2948d2bea4b1217ace)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: tools: make parse_time_err() more strict on the timer validity
Christopher Faulet [Fri, 11 Dec 2020 08:23:07 +0000 (09:23 +0100)]
BUG/MINOR: tools: make parse_time_err() more strict on the timer validity

First, an error is now reported if the first character is not a digit. Thus,
"timeout client s" triggers an error now. Then 'u' is also rejected
now. 'us' is valid and should be used set the timer in microseconds. However
'u' alone is not a valid unit. It was just ignored before (default to
milliseconds). Now, it is an error. Finally, a warning is reported if the
end of the text is not reached after the timer parsing. This warning will
probably be switched to an error in a futur version.

This patch must be backported to all stable versions.

(cherry picked from commit c20ad0d8dbd1bb5707bbfe23632415c3062e046c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoMINOR: tcpcheck: Only wait for more payload data on HTTP expect rules
Christopher Faulet [Wed, 9 Dec 2020 17:45:47 +0000 (18:45 +0100)]
MINOR: tcpcheck: Only wait for more payload data on HTTP expect rules

For HTTP expect rules, if the buffer is not empty, it is guarantee that all
responses headers are received, with the start-line. Thus, except for
payload matching, there is no reason to wait for more data from the moment
the htx message is not empty.

This patch may be backported as far as 2.2.

(cherry picked from commit cad5f5e1ede350e92a0e0b1626d636f1c70c646d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: tcpcheck: Don't rearm the check timeout on each read
Christopher Faulet [Wed, 9 Dec 2020 18:46:38 +0000 (19:46 +0100)]
BUG/MINOR: tcpcheck: Don't rearm the check timeout on each read

The check timeout is used to limit a health-check execution. By default
inter timeout is used. But when defined the check timeout is used. In this
case, the inter timeout (or connect timeout) is used for the connection
establishment only. And the check timeout for the health-check
execution. Thus, it must be set after a successfull connect. It means it is
rearm at the end of each connect rule.

This patch with the previous one (BUG/MINOR: http-check: Use right condition
to consider HTX message as full) should solve the issue #991. It must be
backported as far as 2.2. On the 2.3 and 2.2, there are 2 places were the
connection establishement is handled. The check timeout must be set on both.

(cherry picked from commit c878f56f7c921135acca5e3dccf7961422928285)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: http-check: Use right condition to consider HTX message as full
Christopher Faulet [Wed, 9 Dec 2020 18:45:07 +0000 (19:45 +0100)]
BUG/MINOR: http-check: Use right condition to consider HTX message as full

When an HTTP expect rule is evaluated, we must know if more data is expected
or not to wait if the matching fails. If the whole response is received or
if the HTX message is full, we must not wait. In this context,
htx_free_data_space() must be used instead of htx_free_space(). The fisrt
one count down the block size. Otherwise at the edge, when only the block
size remains free (8 bytes), we may think there is some place for more data
while the mux is unable to add more block.

This bug explains the loop described on the GH issue #991. It should be
backported as far as 2.2.

(cherry picked from commit 3f527197cd7d34fc1975862f346e2d778dc44dc2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoDOC: email change of the DeviceAtlas maintainer
David Carlier [Thu, 10 Dec 2020 09:56:09 +0000 (09:56 +0000)]
DOC: email change of the DeviceAtlas maintainer

(cherry picked from commit 1470b9d0459306c4784285cc7f16b150a5b4b17a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: spoa/python: Fixing references to None
Gilchrist Dadaglo [Tue, 8 Dec 2020 14:37:14 +0000 (14:37 +0000)]
BUG/MEDIUM: spoa/python: Fixing references to None

As per https://docs.python.org/3/c-api/none.html, None has to be treated
exactly like other objects for reference counting.
So, when we use it, we need to INCREF and when we are done, DECREF

This patch must be backported as far as 2.0.

(cherry picked from commit bdd034947c4a3a6f8c41af9bf6e62b7a74b4ebbc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MEDIUM: spoa/python: Fixing PyObject_Call positional arguments
Gilchrist Dadaglo [Tue, 8 Dec 2020 14:37:13 +0000 (14:37 +0000)]
BUG/MEDIUM: spoa/python: Fixing PyObject_Call positional arguments

As per https://docs.python.org/3/c-api/object.html#c.PyObject_Call,
positional arguments should be an empty tuple when not used.
Previously the code had a dictionary instead of tuple. This commit is to
fix it and use tuple to avoid unexpected consequences

This patch must be backported as far as 2.0.

(cherry picked from commit 042f697a0fce0255b569786d29e929a4a41cbc46)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

4 years agoBUG/MINOR: spoa/python: Cleanup ipaddress objects if initialization fails
Gilchrist Dadaglo [Tue, 8 Dec 2020 14:37:12 +0000 (14:37 +0000)]
BUG/MINOR: spoa/python: Cleanup ipaddress objects if initialization fails

This change is to ensure objects from the ipaddress module are cleaned
up when spoa module initialization fails.
In general the interpreter would just crash, but in a code where import
is conditional (try/except), then we would keep those objects around

This patch must be backported as far as 2.0.

(cherry picked from commit d1c0cf80bceb352b6bf33064b352f6dbe2b0b39c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>