]> git.ipfire.org Git - thirdparty/haproxy.git/log
thirdparty/haproxy.git
2 years agoMINOR: stats: add by HTTP version cumulated number of sessions and requests
Frédéric Lécaille [Wed, 18 Jan 2023 10:52:21 +0000 (11:52 +0100)] 
MINOR: stats: add by HTTP version cumulated number of sessions and requests

Add cum_sess_ver[] new array of counters to count the number of cumulated
HTTP sessions by version (h1, h2 or h3).
Implement proxy_inc_fe_cum_sess_ver_ctr() to increment these counter.
This function is called each a HTTP mux is correctly initialized. The QUIC
must before verify the application operations for the mux is for h3 before
calling proxy_inc_fe_cum_sess_ver_ctr().
ST_F_SESS_OTHER stat field for the cumulated of sessions others than
HTTP sessions is deduced from ->cum_sess_ver counter (for all the session,
not only HTTP sessions) from which the HTTP sessions counters are substracted.

Add cum_req[] new array of counters to count the number of cumulated HTTP
requests by version and others than HTTP requests. This new member replace ->cum_req.
Modify proxy_inc_fe_req_ctr() which increments these counters to pass an HTTP
version, 0 special values meaning "other than an HTTP request". This is the case
for instance for syslog.c from which proxy_inc_fe_req_ctr() is called with 0
as version parameter.
ST_F_REQ_TOT stat field compputing for the cumulated number of requests is modified
to count the sum of all the cum_req[] counters.

As this patch is useful for QUIC, it must be backported to 2.7.

2 years agoCLEANUP: quic: no need for atomics on packet refcnt
Willy Tarreau [Thu, 2 Feb 2023 14:37:24 +0000 (15:37 +0100)] 
CLEANUP: quic: no need for atomics on packet refcnt

This is a leftover from the implementation's history, but the
quic_rx_packet and quic_tx_packet ref counts were still atomically
updated. It was found in perf top that the cost of the atomic inc
in quic_tx_packet_refinc() alone was responsible for 1% of the CPU
usage at 135 Gbps. Given that packets are only processed on their
assigned thread we don't need that anymore and this can be replaced
with regular non-atomic operations.

Doing this alone has reduced the CPU usage of qc_do_build_pkt()
from 3.6 to 2.5% and increased the overall bit rate by about 1%.

2 years agoOPTIM: htx: inline the most common memcpy(8)
Willy Tarreau [Thu, 2 Feb 2023 14:32:20 +0000 (15:32 +0100)] 
OPTIM: htx: inline the most common memcpy(8)

On high traffic benchmarks, it's visible the the CPU is dominated by
calls to memcpy(), and many of those come from htx functions. It was
measured that 63% of those coming from htx are made on 8-byte blocks
which really are not worth a call to the function since a single
read-write cycle does it fine.

This commit adds an inline htx_memcpy() function that explicitly
checks for this length and just copies the data without that call.
It's even likely that it could be detected on const sizes, though
that was not done. This is already effective in reducing the number
of calls to memcpy().

2 years agoMINOR: quic: add config for retransmit limit
Amaury Denoyelle [Tue, 31 Jan 2023 10:44:50 +0000 (11:44 +0100)] 
MINOR: quic: add config for retransmit limit

Define a new configuration option "tune.quic.max-frame-loss". This is
used to specify the limit for which a single frame instance can be
detected as lost. If exceeded, the connection is closed.

This should be backported up to 2.7.

2 years agoMEDIUM: quic: implement a retransmit limit per frame
Amaury Denoyelle [Fri, 27 Jan 2023 16:54:15 +0000 (17:54 +0100)] 
MEDIUM: quic: implement a retransmit limit per frame

Add a <loss_count> new field in quic_frame structure. This field is set
to 0 and incremented each time a sent packet is declared lost. If
<loss_count> reached a hard-coded limit, the connection is deemed as
failing and is closed immediately with a CONNECTION_CLOSE using
INTERNAL_ERROR.

By default, limit is set to 10. This should ensure that overall memory
usage is limited if a peer behaves incorrectly.

This should be backported up to 2.7.

2 years agoMINOR: quic: refactor frame deallocation
Amaury Denoyelle [Thu, 2 Feb 2023 15:12:09 +0000 (16:12 +0100)] 
MINOR: quic: refactor frame deallocation

Define a new function qc_frm_free() to handle frame deallocation. New
BUG_ON() statements ensure that the deallocated frame is not referenced
by other frame. To support this, all LIST_DELETE() have been replaced by
LIST_DEL_INIT(). This should enforce that frame deallocation is robust.

As a complement, qc_frm_unref() has been moved into quic_frame module.
It is justified as this is a utility function related to frame
deallocation. It allows to use it in quic_pktns_tx_pkts_release() before
calling qc_frm_free().

This should be backported up to 2.7.

2 years agoMINOR: quic: define new functions for frame alloc
Amaury Denoyelle [Fri, 27 Jan 2023 16:47:49 +0000 (17:47 +0100)] 
MINOR: quic: define new functions for frame alloc

Define two utility functions for quic_frame allocation :
* qc_frm_alloc() is used to allocate a new frame
* qc_frm_dup() is used to allocate a new frame by duplicating an
  existing one

Theses functions are useful to centralize quic_frame initialization.
Note that pool_zalloc() is replaced by a proper pool_alloc() + explicit
initialization code.

This commit will simplify implementation of the per frame retransmission
limitation. Indeed, a new counter will be added in quic_frame structure
which must be initialized to 0.

This should be backported up to 2.7.

2 years agoMINOR: quic: ensure offset is properly set for STREAM frames
Amaury Denoyelle [Thu, 2 Feb 2023 15:45:07 +0000 (16:45 +0100)] 
MINOR: quic: ensure offset is properly set for STREAM frames

Care must be taken when reading/writing offset for STREAM frames. A
special OFF bit is set in the frame type to indicate that the field is
present. If not set, it is assumed that offset is 0.

To represent this, offset field of quic_stream structure must always be
initialized with a valid value in regards with its frame type OFF bit.

The previous code has no bug in part because pool_zalloc() is used to
allocate quic_frame instances. To be able to use pool_alloc(), offset is
always explicitely set to 0. If a non-null value is used, OFF bit is set
at the same occasion. A new BUG_ON() statement is added on frame builder
to ensure that the caller has set OFF bit if offset is non null.

This should be backported up to 2.7.

2 years agoMINOR: quic: remove fin from quic_stream frame type
Amaury Denoyelle [Thu, 2 Feb 2023 13:59:36 +0000 (14:59 +0100)] 
MINOR: quic: remove fin from quic_stream frame type

A dedicated <fin> field was used in quic_stream structure. However, this
info is already encoded in the frame type field as specified by QUIC
protocol.

In fact, only code for packet reception used the <fin> field. On the
sending side, we only checked for the FIN bit. To align both sides,
remove the <fin> field and only used the FIN bit.

This should be backported up to 2.7.

2 years agoBUILD: makefile: fix PCRE overriding specific lib path
Amaury Denoyelle [Fri, 3 Feb 2023 08:23:56 +0000 (09:23 +0100)] 
BUILD: makefile: fix PCRE overriding specific lib path

PCRE relies on pcre-config binary tool to provide includes/libs paths.
This may generate standard entries such as '/usr/lib' which will
override more specific ones if present before them on the linking step.

This situation was encountered when building with both QuicTLS and PCRE.
This generates a linking error as the default SSL libraries were used
for linking even with correct SSL flags pointing to QuicTLS dirs.

To fix this issue, USE_PCRE and its affiliated options have been moved
at the end of 'use_opts' variable. Indeed, related CFLAGS/LDFLAGS are
concatenated in their order of appearance through the macro
collect_opts_flags (see include/make/options.mk). PCRE in the last
position ensures it won't override specific entries declared before.

2 years agoBUG/MINOR: stats: use proper buffer size for http dump
Aurelien DARRAGON [Thu, 2 Feb 2023 14:03:12 +0000 (15:03 +0100)] 
BUG/MINOR: stats: use proper buffer size for http dump

In an attempt to fix GH #1873, ("BUG/MEDIUM: stats: Rely on a local trash
buffer to dump the stats") explicitly reduced output buffer size to leave
enough space for htx overhead under http context.

Github user debtsandbooze, who first reported the issue, came back to us
and said he was still able to make the http dump "hang" with the new fix.

After some tests, it became clear that htx_add_data_atonce() could fail from
time to time in stats_putchk(), even if htx was completely empty:

In http context, buffer size is maxed out at channel_htx_recv_limit().
Unfortunately, channel_htx_recv_limit() is not what we're looking for here
because limit() doesn't compute the proper htx overhead.

Using buf_room_for_htx_data() instead of channel_htx_recv_limit() to compute
max "usable" data space seems to be the last piece of work required for
the previous fix to work properly.

This should be backported everywhere the aforementioned commit is.

2 years agoBUG/MEDIUM: thread: consider secondary threads as idle+harmless during boot
Aurelien DARRAGON [Fri, 27 Jan 2023 14:13:28 +0000 (15:13 +0100)] 
BUG/MEDIUM: thread: consider secondary threads as idle+harmless during boot

idle and harmless bits in the tgroup_ctx structure were not explicitly
set during boot.

    | struct tgroup_ctx ha_tgroup_ctx[MAX_TGROUPS] = { };

As the structure is first statically initialized,
.threads_harmless and .threads_idle are automatically zero-
initialized by the compiler.

Unfortulately, this means that such threads are not considered idle
nor harmless by thread_isolate(_full)() functions until they enter
the polling loop (thread_harmless_now() and thread_idle_now() are
respectively called before entering the polling loop)

Because of this, any attempt to call thread_isolate() or thread_isolate_full()
during a startup phase with nbthreads >= 2 will cause thread_isolate to
loop until every secondary threads make it through their first polling loop.

If the startup phase is aborted during boot (ie: "-c" option to check the
configuration), secondary threads may be initialized but will never be started
(ie: they won't enter the polling loop), thus thread_isolate()
could would loop forever in such cases.

We can easily reveal the bug with this patch reproducer:

    |  diff --git a/src/haproxy.c b/src/haproxy.c
    |  index e91691658..0b733f6ee 100644
    |  --- a/src/haproxy.c
    |  +++ b/src/haproxy.c
    |  @@ -2317,6 +2317,10 @@ static void init(int argc, char **argv)
    |    if (pr || px) {
    |    /* At least one peer or one listener has been found */
    |    qfprintf(stdout, "Configuration file is valid\n");
    |  + printf("haproxy will loop...\n");
    |  + thread_isolate();
    |  + printf("we will never reach this\n");
    |  + thread_release();
    |    deinit_and_exit(0);
    |    }
    |    qfprintf(stdout, "Configuration file has no error but will not start (no listener) => exit(2).\n");

Now we start haproxy with a valid config:
$> haproxy -c -f valid.conf
Configuration file is valid
haproxy will loop...

^C

------------------------------------------------------------------------------

This did not cause any issue so far because no early deinit paths require
full thread isolation. But this may change when new features or requirements
are introduced, so we should fix this before it becomes a real issue.

To fix this, we explicitly assign .threads_harmless and .threads_idle
to .threads_enabled value in thread_map_to_groups() function during boot.
This is the proper place to do this since as long as .threads_enabled is not
explicitly set, its default value is also 0 (zero-initialized by the compiler)

code snippet from thread_isolate() function:
       ulong te = _HA_ATOMIC_LOAD(&ha_tgroup_info[tgrp].threads_enabled);
       ulong th = _HA_ATOMIC_LOAD(&ha_tgroup_ctx[tgrp].threads_harmless);

       if ((th & te) == te)
           break;

Thus thread_isolate(_full()) won't be looping forever in thread_isolate()
even if it were to be used before thread_map_to_groups() is executed.

No backport needed unless this is a requirement.

2 years agoBUG/MINOR: h3: fix crash due to h3 traces
Amaury Denoyelle [Tue, 31 Jan 2023 14:50:16 +0000 (15:50 +0100)] 
BUG/MINOR: h3: fix crash due to h3 traces

This commit is identical to the preceeding patch. However, these traces
are from another patch with a different backport scope :
    56a86ddfb97d740f965503f6b5991fafa347308c
    MINOR: h3: add missing traces on closure

This must be backported up to 2.7 where above patch is scheduled.

2 years agoBUG/MINOR: h3: fix crash due to h3 traces
Amaury Denoyelle [Tue, 31 Jan 2023 15:01:22 +0000 (16:01 +0100)] 
BUG/MINOR: h3: fix crash due to h3 traces

First H3 traces argument must be a connection instance or a NULL. Some
new traces were added recently with a qcc instance which caused a crash
when traces are activated.

This trace was added by the following patch :
    87f8766d3fbd10f9e8bf4902d37712612db64df5
    BUG/MEDIUM: h3: handle STOP_SENDING on control stream

This must be backported up to 2.6 along with the above patch.

2 years agoBUG/MEDIUM: ssl: wrong eviction from the session cache tree
William Lallemand [Tue, 31 Jan 2023 13:12:28 +0000 (14:12 +0100)] 
BUG/MEDIUM: ssl: wrong eviction from the session cache tree

When using WolfSSL, there are some cases were the SSL_CTX_sess_new_cb is
called with an existing session ID. These cases are not met with
OpenSSL.

When the ID is found in the session tree during the insertion, the
shared_block len is not set to 0 and is not used. However if later the
block is reused, since the len is not set to 0, the release callback
will be called an ebmb_delete will be tried on the block, even if it's
not in the tree, provoking a crash.

The code was buggy from the beginning, but the case never happen with
openssl which changes the ID.

Must be backported in every maintained branches.

2 years agoMINOR: h3: add missing traces on closure
Amaury Denoyelle [Mon, 30 Jan 2023 14:36:51 +0000 (15:36 +0100)] 
MINOR: h3: add missing traces on closure

Add traces for function h3_shutdown() / h3_send_goaway(). This should
help to debug problems related to connection closure.

This should be backported up to 2.7.

2 years agoBUG/MINOR: h3: reject RESET_STREAM received for control stream
Amaury Denoyelle [Mon, 30 Jan 2023 11:13:22 +0000 (12:13 +0100)] 
BUG/MINOR: h3: reject RESET_STREAM received for control stream

This commit is similar to the previous one. It reports an error if a
RESET_STREAM is received for the remote control stream. This will
generate a CONNECTION_CLOSE with H3_CLOSED_CRITICAL_STREAM error.

Note that contrary to the previous bug related to STOP_SENDING, this bug
was not encountered in real environment. As such, it is labelled as
MINOR. However, it could triggered the same crash as the previous patch.

This should be backported up to 2.6.

2 years agoBUG/MEDIUM: h3: handle STOP_SENDING on control stream
Amaury Denoyelle [Mon, 30 Jan 2023 11:12:43 +0000 (12:12 +0100)] 
BUG/MEDIUM: h3: handle STOP_SENDING on control stream

Before this patch, STOP_SENDING reception was considered valid even on
H3 control stream. This causes the emission in return of RESET_STREAM
and eventually the closure and freeing of the QCS instance. This then
causes a crash during connection closure as a GOAWAY frame is emitted on
the control stream which is now released.

To fix this crash, STOP_SENDING on the control stream is now properly
rejected as specified by RFC 9114. The new app_ops close callback is
used which in turn will generate a CONNECTION_CLOSE with error
H3_CLOSED_CRITICAL_STREAM.

This bug was detected in github issue #2006. Note that however it is
triggered by an incorrect client behavior. It may be useful to determine
which client behaves like this. If this case is too frequent,
STOP_SENDING should probably be silently ignored.

To reproduce this issue, quiche was patched to emit a STOP_SENDING on
its send() function in quiche/src/lib.rs:
     pub fn send(&mut self, out: &mut [u8]) -> Result<(usize, SendInfo)> {
-        self.send_on_path(out, None, None)
+        let ret = self.send_on_path(out, None, None);
+        self.streams.mark_stopped(3, true, 0);
+        ret
     }

This must be backported up to 2.6 along with the preceeding commit :
  MINOR: mux-quic/h3: define close callback

2 years agoMINOR: mux-quic/h3: define stream close callback
Amaury Denoyelle [Mon, 30 Jan 2023 11:12:11 +0000 (12:12 +0100)] 
MINOR: mux-quic/h3: define stream close callback

Define a new qcc_app_ops callback named close(). This will be used to
notify app-layer about the closure of a stream by the remote peer. Its
main usage is to ensure that the closure is allowed by the application
protocol specification.

For the moment, close is not implemented by H3 layer. However, this
function will be mandatory to properly reject a STOP_SENDING on the
control stream and preventing a later crash. As such, this commit must
be backported with the next one on 2.6.

This is related to github issue #2006.

2 years agoOPTIM: h3: skip buf realign if no trailer to encode
Amaury Denoyelle [Thu, 26 Jan 2023 16:49:21 +0000 (17:49 +0100)] 
OPTIM: h3: skip buf realign if no trailer to encode

h3_resp_trailers_send() may be called due to an HTX EOT block present
without preceeding HTX TRAILER block. In this case, no HEADERS frame
will be generated by H3 layer and MUX will emit an empty STREAM frame
with FIN set.

However, before skipping these, some operations are conducted on qcs
buffer to realign it and try to encode the QPACK field section line in a
buffer copy. These operation are thus unneeded if no trailer is
generated. Even worse, the function will fail if there is not enough
space in the buffer for the superfluous QPACK section line.

To improve this situation, this patch adds an early goto statement to
skip most operations in h3_resp_trailers_send() if no HTX trailer block
is found.

This patch is related to github issue #2006.

This should be backported up to 2.7.

2 years agoBUG/MEDIUM: h3: do not crash if no buf space for trailers
Amaury Denoyelle [Thu, 26 Jan 2023 16:41:58 +0000 (17:41 +0100)] 
BUG/MEDIUM: h3: do not crash if no buf space for trailers

Replace ABORT_NOW() by proper error management in
h3_resp_trailers_send() for QPACK encoding operation.

If a QPACK encoding operation fails, it means there is not enough space
in qcs buffer. In this case, flag qcs instance with QC_SF_BLK_MROOM and
return an error. MUX is responsible to remove this flag once buffer
space is available.

This should fix the crash reported by gabrieltz on github issue #2006.

This must be backported up to 2.7.

2 years agoBUG/MINOR: http_ext/7239: ipv6 dumping relies on out of scope variables
Aurelien DARRAGON [Mon, 30 Jan 2023 08:28:57 +0000 (09:28 +0100)] 
BUG/MINOR: http_ext/7239: ipv6 dumping relies on out of scope variables

In http_build_7239_header_nodename(), ip6 address dumping is performed
at a single place to prevent code duplication:
A goto statement combined with a local pointer variable (ip6_addr) were used
to perform ipv6 dump from different calling places inside the function.

However, when the goto was performed (ie: sample expression handling),
ip6_addr pointer was assigned to limited scope variable's address that is not
valid within the dumping code.
Because of this, we have an undefined behavior that could result in a bug or
a crash depending on the platform that is running haproxy.

This was found by Coverity (GH #2018)

To fix this, we add a simple ip6 printing helper that takes the ip6_addr
pointer as an argument.
This prevents any scope related bug as the function is executed under the
proper context.

if/else guards inside the function were reviewed to make sure that the goto
removal won't affect existing behavior.

----------

No backport needed, except if the commit ("MINOR: proxy/http_ext: introduce
proxy forwarded option") is backported.

Given that this commit needs to be backported with
"MINOR: proxy/http_ext: introduce proxy forwarded option", We're using it as a
reminder for another bug that was introduced with
"MINOR: proxy/http_ext: introduce proxy forwarded option" but has been silently
fixed since with
"MEDIUM: proxy/http_ext: implement dynamic http_ext".

If "MINOR: proxy/http_ext: introduce proxy forwarded option" needs to be
backported without
"MEDIUM: proxy/http_ext: implement dynamic http_ext", you should manually apply
the following patch on top of it:

    |  diff --git a/src/http_ext.c b/src/http_ext.c
    |  index fcb5a07bc..3921357a3 100644
    |  --- a/src/http_ext.c
    |  +++ b/src/http_ext.c
    |  @@ -609,7 +609,7 @@ static inline void http_build_7239_header_node(struct buffer *out,
    |    if (forby->np_mode)
    |    chunk_appendf(out, "\"");
    |    offset_save = out->data;
    |  - http_build_7239_header_node(out, s, curproxy, addr, &curproxy->http.fwd.p_by);
    |  + http_build_7239_header_nodename(out, s, curproxy, addr, forby);
    |    if (offset_save == out->data) {
    |    /* could not build nodename, either because some
    |     * data is not available or user is providing bad input
    |  @@ -619,7 +619,7 @@ static inline void http_build_7239_header_node(struct buffer *out,
    |    if (forby->np_mode) {
    |    chunk_appendf(out, ":");
    |    offset_save = out->data;
    |  - http_build_7239_header_nodeport(out, s, curproxy, addr, &curproxy->http.fwd.p_by);
    |  + http_build_7239_header_nodeport(out, s, curproxy, addr, forby);
    |    if (offset_save == out->data) {
    |    /* could not build nodeport, either because some data is
    |     * not available or user is providing bad input

(If you don't, forwarded option won't work properly and will crash
haproxy (stack overflow) when building 'for' or 'by' parameter)

2 years agoBUG/MINOR: mux-h2: Fix possible null pointer deref on h2c in _h2_trace_header()
Christopher Faulet [Mon, 30 Jan 2023 07:26:09 +0000 (08:26 +0100)] 
BUG/MINOR: mux-h2: Fix possible null pointer deref on h2c in _h2_trace_header()

As reported by Coverity, this function may be called with no h2c. Thus, the
pointer must always be checked before any access. One test was missing in
TRACE_PRINTF_LOC().

This patch should fix the issue #2015. No backport needed, except if the
commit 11e8a8c2a ("MEDIUM: mux-h2/trace: add tracing support for headers")
is backported.

2 years agoDOC: config: 'http-send-name-header' option may be used in default section
Aurelien DARRAGON [Thu, 12 Jan 2023 14:59:27 +0000 (15:59 +0100)] 
DOC: config: 'http-send-name-header' option may be used in default section

Both doc and code agree on the fact that 'http-send-name-header' option
could be used in default section, but the keyword compatibility matrix
in configuration.txt reported the opposite.

This could be backported to all stable versions.

2 years agoBUG/MINOR: fcgi-app: prevent 'use-fcgi-app' in default section
Aurelien DARRAGON [Thu, 12 Jan 2023 14:44:22 +0000 (15:44 +0100)] 
BUG/MINOR: fcgi-app: prevent 'use-fcgi-app' in default section

Despite the doc saying that 'use-fcgi-app' keyword may only be used in backend
or listen section, we forgot to prevent its usage in default section.

This is wrong because fcgi relies on a filter, and filters cannot be defined in
a default section.

Making sure such usage reports an error to the user and complies with the doc.

This could be backported up to 2.2.

2 years agoDOC: config: fix option spop-check proxy compatibility
Aurelien DARRAGON [Thu, 12 Jan 2023 14:06:11 +0000 (15:06 +0100)] 
DOC: config: fix option spop-check proxy compatibility

The doc mentioned that spop-check option may only be used for backends.
However, option may be used in default and listen sections as well
according to the code. Let's fix the doc so that doc and code are
consistent to each other.

This could be backported to all stable versions.

2 years agoMINOR: cfgparse/http_ext: move post-parsing http_ext steps to http_ext
Aurelien DARRAGON [Tue, 24 Jan 2023 16:55:09 +0000 (17:55 +0100)] 
MINOR: cfgparse/http_ext: move post-parsing http_ext steps to http_ext

This is a simple refactor to remove specific http_ext post-parsing treatment from
cfgparse.

Related work is now performed internally through check_http_ext_postconf()
function that is registered via REGISTER_POST_PROXY_CHECK() in http_ext.c.

2 years agoMEDIUM: proxy/http_ext: implement dynamic http_ext
Aurelien DARRAGON [Mon, 9 Jan 2023 10:09:03 +0000 (11:09 +0100)] 
MEDIUM: proxy/http_ext: implement dynamic http_ext

proxy http-only options implemented in http_ext were statically stored
within proxy struct.

We're making some changes so that http_ext are now stored in a dynamically
allocated structs.
http_ext related structs are only allocated when needed to save some space
whenever possible, and they are automatically freed upon proxy deletion.

Related PX_O_HTTP{7239,XFF,XOT) option flags were removed because we're now
considering an http_ext option as 'active' if it is allocated (ptr is not NULL)

A few checks (and BUG_ON) were added to make these changes safe because
it adds some (acceptable) complexity to the previous design.

Also, proxy.http was renamed to proxy.http_ext to make things more explicit.

2 years agoMINOR: http_ext/7239: warn the user when fetch is not available
Aurelien DARRAGON [Fri, 6 Jan 2023 14:06:49 +0000 (15:06 +0100)] 
MINOR: http_ext/7239: warn the user when fetch is not available

http option forwarded (rfc7239) supports sample expressions when
configuring 'host', 'for' and 'by' parameters.

However, since we are in a http-backend-only context, right after http
header is processed, we have a limited resolution scope for the sample
expression provided by the user.

To prevent any confusion, a warning is emitted when parsing the option
if the user relies on a sample expression (more precisely a fetch) which
would yield unexpected results at runtime when processing the option.

2 years agoOPTIM: http_ext/7239: introduce c_mode to save some space
Aurelien DARRAGON [Fri, 6 Jan 2023 11:16:28 +0000 (12:16 +0100)] 
OPTIM: http_ext/7239: introduce c_mode to save some space

forwarded header option (rfc7239) deals with sample expressions in two
steps: first a sample expression string is extracted from the config file
and later in startup sequence this string is converted into the resulting
sample_expr.

We need to perform these two steps because we cannot compile the expr
too early in the parsing sequence. (or we would miss some context)
Because of this, we have two dinstinct structure members (expr and expr_s)
for each 7239 field supporting sample expressions.
This is not cool, because we're bloating the http forwarded config structure,
and thus, bloating proxy config structure.

To address this, we now merge both expr and expr_s members inside a single
union to regain some space. This forces us to perform some additional logic
to make sure to use the proper structure member at different parsing steps.

Thanks to this, we're also able to free/release related config hints and
sample expression strings as soon as the sample expression
compilation is finished.

2 years agoREGTEST: add RFC7239 forwarded header tests
Aurelien DARRAGON [Fri, 30 Dec 2022 17:59:24 +0000 (18:59 +0100)] 
REGTEST: add RFC7239 forwarded header tests

Testing "option forwarded" and related RFC7239 converters.

Depends on:
  - "MINOR: http_ext: add 7239_n2np converter"
  - "MINOR: http_ext: add 7239_n2nn converter"
  - "MINOR: http_ext: add 7239_field converter"
  - "MINOR: http_ext: add 7239_is_valid converter"
  - "MINOR: proxy/http_ext: introduce proxy forwarded option"

2 years agoMINOR: http_ext: add rfc7239_n2np converter
Aurelien DARRAGON [Fri, 30 Dec 2022 15:56:08 +0000 (16:56 +0100)] 
MINOR: http_ext: add rfc7239_n2np converter

Adding new http converter: rfc7239_n2np.

Takes a string representing 7239 forwarded header node (extracted from
either 'for' or 'by' 7239 header fields) as input and translates it
to either unsigned integer or ('_' prefixed obfuscated identifier),
according to 7239RFC.

  Example:
    # extract 'by' field from forwarded header, extract node port from
    # resulting node identifier and store the result in req.fnp
    http-request set-var(req.fnp) req.hdr(forwarded),rfc7239_field(by),rfc7239_n2np
    #input: "by=\"127.0.0.1:9999\""
    #  output: 9999
    #input: "by=\"_name:_port\""
    #  output: "_port"

Depends on:
  - "MINOR: http_ext: introduce http ext converters"

2 years agoMINOR: http_ext: add rfc7239_n2nn converter
Aurelien DARRAGON [Fri, 30 Dec 2022 15:45:42 +0000 (16:45 +0100)] 
MINOR: http_ext: add rfc7239_n2nn converter

Adding new http converter: rfc7239_n2nn.

Takes a string representing 7239 forwarded header node (extracted from
either 'for' or 'by' 7239 header fields) as input and translates it
to either ipv4 address, ipv6 address or str ('_' prefixed if obfuscated
or "unknown" if unknown), according to 7239RFC.

  Example:
    # extract 'for' field from forwarded header, extract nodename from
    # resulting node identifier and store the result in req.fnn
    http-request set-var(req.fnn) req.hdr(forwarded),rfc7239_field(for),rfc7239_n2nn
    #input: "for=\"127.0.0.1:9999\""
    #  output: 127.0.0.1
    #input: "for=\"_name:_port\""
    #  output: "_name"

Depends on:
  - "MINOR: http_ext: introduce http ext converters"

2 years agoMINOR: http_ext: add rfc7239_field converter
Aurelien DARRAGON [Fri, 30 Dec 2022 15:37:03 +0000 (16:37 +0100)] 
MINOR: http_ext: add rfc7239_field converter

Adding new http converter: rfc7239_field.

Takes a string representing 7239 forwarded header single value as
input and extracts a single field/parameter from the header according
to user selection.

  Example:
    # extract host field from forwarded header and store it in req.fhost var
    http-request set-var(req.fhost) req.hdr(forwarded),rfc7239_field(host)
    #input: "proto=https;host=\"haproxy.org:80\""
    #  output: "haproxy.org:80"

    # extract for field from forwarded header and store it in req.ffor var
    http-request set-var(req.ffor) req.hdr(forwarded),rfc7239_field(for)
    #input: "proto=https;host=\"haproxy.org:80\";for=\"127.0.0.1:9999\""
    #  output: "127.0.0.1:9999"

Depends on:
  - "MINOR: http_ext: introduce http ext converters"

2 years agoMINOR: http_ext: add rfc7239_is_valid converter
Aurelien DARRAGON [Fri, 30 Dec 2022 15:23:08 +0000 (16:23 +0100)] 
MINOR: http_ext: add rfc7239_is_valid converter

Adding new http converter: rfc7239_is_valid.

Takes a string representing 7239 forwarded header single value as
input and returns bool:TRUE if header is RFC compliant and
bool:FALSE otherwise.

  Example:
    acl valid req.hdr(forwarded),rfc7239_is_valid
    #input: "for=127.0.0.1;proto=http"
    #  output: TRUE
    #input: "proto=custom"
    #  output: FALSE

Depends on:
  - "MINOR: http_ext: introduce http ext converters"

2 years agoMINOR: http_ext: introduce http ext converters
Aurelien DARRAGON [Thu, 29 Dec 2022 17:32:19 +0000 (18:32 +0100)] 
MINOR: http_ext: introduce http ext converters

This commit is really simple, it adds the required skeleton code to allow
new http_ext converter to be easily registered through STG_REGISTER facility.

2 years agoMINOR: proxy: move 'originalto' option to http_ext
Aurelien DARRAGON [Thu, 29 Dec 2022 14:45:41 +0000 (15:45 +0100)] 
MINOR: proxy: move 'originalto' option to http_ext

Just like forwarded (7239) header and forwardfor header, move parsing,
logic and management of 'originalto' option into http_ext dedicated class.

We're only doing this to standardize proxy http options management.
Existing behavior remains untouched.

2 years agoMINOR: proxy: move 'forwardfor' option to http_ext
Aurelien DARRAGON [Wed, 28 Dec 2022 17:53:05 +0000 (18:53 +0100)] 
MINOR: proxy: move 'forwardfor' option to http_ext

Just like forwarded (7239) header, move parsing, logic and management
of 'forwardfor' option into http_ext dedicated class.

We're only doing this to standardize proxy http options management.
Existing behavior remains untouched.

2 years agoREGTEST: add ifnone-forwardfor test
Aurelien DARRAGON [Mon, 9 Jan 2023 10:02:48 +0000 (11:02 +0100)] 
REGTEST: add ifnone-forwardfor test

Add a new test to prevent any regression for the if-none parameter in
the "forwardfor" proxy option.
This will ensure upcoming refactors don't break reference behavior.

2 years agoMINOR: proxy/http_ext: introduce proxy forwarded option
Aurelien DARRAGON [Wed, 28 Dec 2022 14:37:57 +0000 (15:37 +0100)] 
MINOR: proxy/http_ext: introduce proxy forwarded option

Introducing http_ext class for http extension related work that
doesn't fit into existing http classes.

HTTP extension "forwarded", introduced with 7239 RFC is now supported
by haproxy.

The option supports various modes from simple to complex usages involving
custom sample expressions.

  Examples :

    # Those servers want the ip address and protocol of the client request
    # Resulting header would look like this:
    #   forwarded: proto=http;for=127.0.0.1
    backend www_default
        mode http
        option forwarded
        #equivalent to: option forwarded proto for

    # Those servers want the requested host and hashed client ip address
    # as well as client source port (you should use seed for xxh32 if ensuring
    # ip privacy is a concern)
    # Resulting header would look like this:
    #   forwarded: host="haproxy.org";for="_000000007F2F367E:60138"
    backend www_host
        mode http
        option forwarded host for-expr src,xxh32,hex for_port

    # Those servers want custom data in host, for and by parameters
    # Resulting header would look like this:
    #   forwarded: host="host.com";by=_haproxy;for="[::1]:10"
    backend www_custom
        mode http
        option forwarded host-expr str(host.com) by-expr str(_haproxy) for for_port-expr int(10)

    # Those servers want random 'for' obfuscated identifiers for request
    # tracing purposes while protecting sensitive IP information
    # Resulting header would look like this:
    #   forwarded: for=_000000002B1F4D63
    backend www_for_hide
        mode http
        option forwarded for-expr rand,hex

By default (no argument provided), forwarded option will try to mimic
x-forward-for common setups (source client ip address + source protocol)

The option is not available for frontends.
no option forwarded is supported.

More info about 7239 RFC here: https://www.rfc-editor.org/rfc/rfc7239.html

More info about the feature in doc/configuration.txt

This should address feature request GH #575

Depends on:
  - "MINOR: http_htx: add http_append_header() to append value to header"
  - "MINOR: sample: add ARGC_OPT"
  - "MINOR: proxy: introduce http only options"

2 years agoMINOR: proxy: introduce http only options
Aurelien DARRAGON [Wed, 28 Dec 2022 13:57:55 +0000 (14:57 +0100)] 
MINOR: proxy: introduce http only options

This commit is innoffensive but will allow to do some code refactors in
existing proxy http options. Newly created http related proxy options
will also benefit from this.

2 years agoMINOR: sample: add ARGC_OPT
Aurelien DARRAGON [Wed, 28 Dec 2022 10:30:51 +0000 (11:30 +0100)] 
MINOR: sample: add ARGC_OPT

Add ARGC_OPT enum to provide more context for upcoming sample parse errors
involving proxy "option" config directives.

2 years agoMINOR: http_htx: add http_prepend_header() to prepend value to header
Aurelien DARRAGON [Thu, 5 Jan 2023 16:02:19 +0000 (17:02 +0100)] 
MINOR: http_htx: add http_prepend_header() to prepend value to header

Just like http_append_header(), but this time to insert new value before
an existing one.

If the header already contains one or multiple values, ',' is automatically
inserted after the new value.

2 years agoMINOR: http_htx: add http_append_header() to append value to header
Aurelien DARRAGON [Wed, 28 Dec 2022 10:21:31 +0000 (11:21 +0100)] 
MINOR: http_htx: add http_append_header() to append value to header

Calling this function as an alternative to http_replace_header_value()
to append a new value to existing header instead of replacing the whole
header content.

If the header already contains one or multiple values: a ',' is automatically
appended before the new value.

This function is not meant for prepending (providing empty ctx value),
in which case we should consider implementing dedicated prepend
alternative function.

2 years agoDEV: hpack: fix `trash` build regression
Aurelien DARRAGON [Wed, 25 Jan 2023 15:35:00 +0000 (16:35 +0100)] 
DEV: hpack: fix `trash` build regression

Since 7d84439 ("BUILD: hpack: include global.h for the trash that is needed
in debug mode"), hpack decode tool fails to compile on targets that enable
USE_THREAD. (ie: linux-glibc target as reported by Christian Ruppert)

When building hpack devtool, we are including src/hpack-dec.c as a dependency.
src/hpack-dec.c relies on the global trash whe debug mode is enabled.
But as we're building hpack tool with a limited scope of haproxy
sources, global trash (which is declared in src/chunk.c) is not available.
Thus, src/hpack-dec.c relies on a local 'trash' variable declared within
dev/hpack/decode.c

This used to work fine until 7d84439.
But now that global.h is explicitely included in src/hpack-dec.c,
trash variable definition from decode.c conflicts with the one from global.h:

  In file included from include/../src/hpack-dec.c:35,
                   from dev/hpack/decode.c:87:
  include/haproxy/global.h:52:35: error: thread-local declaration of 'trash' follows non-thread-local declaration
     52 | extern THREAD_LOCAL struct buffer trash;

Adding THREAD_LOCAL attribute to 'decode.c' local trash variable definition
makes the compiler happy again.

This should fix GH issue #2009 and should be backported to 2.7.

2 years agoCLEANUP: mux-h2/trace: shorten the name of the header enc/dec functions
Willy Tarreau [Thu, 26 Jan 2023 15:02:01 +0000 (16:02 +0100)] 
CLEANUP: mux-h2/trace: shorten the name of the header enc/dec functions

The functions in charge of processing headers have their names in the
traces and they're among the longest of the mux_h2.c file, while even
containing some redundancy. These names are not used outside, let's
shorten them:

- h2c_decode_headers        -> h2c_dec_hdrs
- h2s_bck_make_req_headers  -> h2s_snd_bhdrs
- h2s_frt_make_resp_headers -> h2s_snd_fhdrs

Now the traces are a bit more readable:

  [00|h2|5|mux_h2.c:4822] h2c_dec_hdrs(): h2c=0x1870510(F,FRP) dsi=1 rcvh :method: GET
  [00|h2|5|mux_h2.c:4822] h2c_dec_hdrs(): h2c=0x1870510(F,FRP) dsi=1 rcvh :path: /
  [00|h2|5|mux_h2.c:4822] h2c_dec_hdrs(): h2c=0x1870510(F,FRP) dsi=1 rcvh :scheme: http
  [00|h2|5|mux_h2.c:4822] h2c_dec_hdrs(): h2c=0x1870510(F,FRP) dsi=1 rcvh :authority: localhost:14446
  [00|h2|5|mux_h2.c:4822] h2c_dec_hdrs(): h2c=0x1870510(F,FRP) dsi=1 rcvh user-agent: curl/7.54.1
  [00|h2|5|mux_h2.c:4822] h2c_dec_hdrs(): h2c=0x1870510(F,FRP) dsi=1 rcvh accept: */*

2 years agoMEDIUM: mux-h2/trace: add tracing support for headers
Willy Tarreau [Tue, 24 Jan 2023 18:43:11 +0000 (19:43 +0100)] 
MEDIUM: mux-h2/trace: add tracing support for headers

Now we can make use of TRACE_PRINTF() to iterate over headers as they
are received or dumped. It's worth noting that the dumps may occasionally
be interrupted due to a buffer full or a realign, but in this case it
will be visible because the trace will restart from the first one. All
these headers (and trailers) may be interleaved with other connections'
so they're all preceeded by the pointer to the connection and optionally
the stream (or alternately the stream ID) to help discriminating them.

Since it's not easy to read the header directions, sent headers are
prefixed with "sndh" and received headers are prefixed with "rcvh", both
of which are rare enough in the traces to conveniently support a quick
grep.

In order to avoid code duplication, h2_encode_headers() was implemented
as a wrapper on top of hpack_encode_header(), which optionally emits the
header to the trace if the trace is active. In addition, for headers that
are encoded using a different method, h2_trace_header() was added as well.

Header names are truncated to 256 bytes and values to 1024 bytes. If
the lengths are larger, they will be truncated and suffixed with
"(... +xxx)" where "xxx" is the number of extra bytes.

Example of what an end-to-end H2 request gives:

  [00|h2|5|mux_h2.c:4818] h2c_decode_headers(): h2c=0x1c13120(F,FRP) dsi=1 rcvh :method: GET
  [00|h2|5|mux_h2.c:4818] h2c_decode_headers(): h2c=0x1c13120(F,FRP) dsi=1 rcvh :path: /
  [00|h2|5|mux_h2.c:4818] h2c_decode_headers(): h2c=0x1c13120(F,FRP) dsi=1 rcvh :scheme: http
  [00|h2|5|mux_h2.c:4818] h2c_decode_headers(): h2c=0x1c13120(F,FRP) dsi=1 rcvh :authority: localhost:14446
  [00|h2|5|mux_h2.c:4818] h2c_decode_headers(): h2c=0x1c13120(F,FRP) dsi=1 rcvh user-agent: curl/7.54.1
  [00|h2|5|mux_h2.c:4818] h2c_decode_headers(): h2c=0x1c13120(F,FRP) dsi=1 rcvh accept: */*
  [00|h2|5|mux_h2.c:4818] h2c_decode_headers(): h2c=0x1c13120(F,FRP) dsi=1 rcvh cookie: blah
  [00|h2|5|mux_h2.c:5491] h2s_bck_make_req_headers(): h2c=0x1c1cd90(B,FRH) h2s=0x1c1e3d0(1,IDL) sndh :method: GET
  [00|h2|5|mux_h2.c:5572] h2s_bck_make_req_headers(): h2c=0x1c1cd90(B,FRH) h2s=0x1c1e3d0(1,IDL) sndh :authority: localhost:14446
  [00|h2|5|mux_h2.c:5596] h2s_bck_make_req_headers(): h2c=0x1c1cd90(B,FRH) h2s=0x1c1e3d0(1,IDL) sndh :path: /
  [00|h2|5|mux_h2.c:5647] h2s_bck_make_req_headers(): h2c=0x1c1cd90(B,FRH) h2s=0x1c1e3d0(1,IDL) sndh user-agent: curl/7.54.1
  [00|h2|5|mux_h2.c:5647] h2s_bck_make_req_headers(): h2c=0x1c1cd90(B,FRH) h2s=0x1c1e3d0(1,IDL) sndh accept: */*
  [00|h2|5|mux_h2.c:5647] h2s_bck_make_req_headers(): h2c=0x1c1cd90(B,FRH) h2s=0x1c1e3d0(1,IDL) sndh cookie: blah
  [00|h2|5|mux_h2.c:4818] h2c_decode_headers(): h2c=0x1c1cd90(B,FRP) dsi=1 rcvh :status: 200
  [00|h2|5|mux_h2.c:4818] h2c_decode_headers(): h2c=0x1c1cd90(B,FRP) dsi=1 rcvh content-length: 0
  [00|h2|5|mux_h2.c:4818] h2c_decode_headers(): h2c=0x1c1cd90(B,FRP) dsi=1 rcvh x-req: size=102, time=0 ms
  [00|h2|5|mux_h2.c:4818] h2c_decode_headers(): h2c=0x1c1cd90(B,FRP) dsi=1 rcvh x-rsp: id=dummy, code=200, cache=1, size=0, time=0 ms (0 real)
  [00|h2|5|mux_h2.c:5210] h2s_frt_make_resp_headers(): h2c=0x1c13120(F,FRH) h2s=0x1c1c780(1,HCR) sndh :status: 200
  [00|h2|5|mux_h2.c:5231] h2s_frt_make_resp_headers(): h2c=0x1c13120(F,FRH) h2s=0x1c1c780(1,HCR) sndh content-length: 0
  [00|h2|5|mux_h2.c:5231] h2s_frt_make_resp_headers(): h2c=0x1c13120(F,FRH) h2s=0x1c1c780(1,HCR) sndh x-req: size=102, time=0 ms
  [00|h2|5|mux_h2.c:5231] h2s_frt_make_resp_headers(): h2c=0x1c13120(F,FRH) h2s=0x1c1c780(1,HCR) sndh x-rsp: id=dummy, code=200, cache=1, size=0, time=0 ms (0 real)

At some point the frontend/backend names would be useful but that's a more
general comment than just the H2 traces.

2 years agoMINOR: h2: add h2_phdr_to_ist() to make ISTs from pseudo headers
Willy Tarreau [Thu, 26 Jan 2023 08:38:53 +0000 (09:38 +0100)] 
MINOR: h2: add h2_phdr_to_ist() to make ISTs from pseudo headers

Till now pseudo headers were passed as const strings, but having them as
ISTs will be more convenient for traces. This doesn't change anything for
strings which are derived from them (and being constants they're still
zero-terminated).

2 years agoMINOR: trace: add the long awaited TRACE_PRINTF()
Willy Tarreau [Tue, 24 Jan 2023 17:23:59 +0000 (18:23 +0100)] 
MINOR: trace: add the long awaited TRACE_PRINTF()

TRACE_PRINTF() can be used to produce arbitrary trace contents at any
trace level. It uses the exact same arguments as other TRACE_* macros,
but here they are mandatory since they are followed by the format-string,
though they may be filled with zeroes. The reason for the arguments is to
match tracking or filtering and not pollute other non-inspected objects.

It will probably be used inside loops, in which case there are two points
to be careful about:
  - output atomicity is only per-message, so competing threads may see
    their messages interleaved. As such, it is recommended that the
    caller places a recognizable unique context at the beginning of the
    message such as a connection pointer.
  - iterating over arrays or lists for all requests could be very
    expensive. In order to avoid this it is best to condition the call
    via TRACE_ENABLED() with the same arguments, which will return the
    same decision.
  - messages longer than TRACE_MAX_MSG-1 (1023 by default) will be
    truncated.

For example, in order to dump the list of HTTP headers between hpack
and h2:

  if (outlen > 0 &&
      TRACE_ENABLED(TRACE_LEVEL_DEVELOPER,
      H2_EV_RX_FRAME|H2_EV_RX_HDR, h2c->conn, 0, 0, 0)) {
          int i;
          for (i = 0; list[i].n.len; i++)
              TRACE_PRINTF(TRACE_LEVEL_DEVELOPER, H2_EV_RX_FRAME|H2_EV_RX_HDR,
                           h2c->conn, 0, 0, 0, "h2c=%p hdr[%d]=%s:%s", h2c, i,
                           list[i].n.ptr, list[i].v.ptr);
  }

In addition, a lower-level TRACE_PRINTF_LOC() macro is provided, that takes
two extra arguments, the caller's location and the caller's function name.
This will allow to emit composite traces from central functions on the
behalf of another one.

2 years agoMINOR: trace: add a trace_no_cb() dummy callback for when to use no callback
Willy Tarreau [Tue, 24 Jan 2023 17:03:07 +0000 (18:03 +0100)] 
MINOR: trace: add a trace_no_cb() dummy callback for when to use no callback

By default, passing a NULL cb to the trace functions will result in the
source's default one to be used. For some cases we won't want to use any
callback at all, not event the default one. Let's define a trace_no_cb()
function for this, that does absolutely nothing.

2 years agoMINOR: trace: add a TRACE_ENABLED() macro to determine if a trace is active
Willy Tarreau [Tue, 24 Jan 2023 16:48:53 +0000 (17:48 +0100)] 
MINOR: trace: add a TRACE_ENABLED() macro to determine if a trace is active

Sometimes it would be necessary to prepare some messages, pre-process some
blocks or maybe duplicate some contents before they vanish for the purpose
of tracing them. However we don't want to do that for everything that is
submitted to the traces, it's important to do it only for what will really
be traced.

The __trace() function has all the knowledge for this, to the point of even
checking the lockon pointers. This commit splits the function in two, one
with the trace decision logic, and the other one for the trace production.
The first one is now usable through wrappers such as _trace_enabled() and
TRACE_ENABLED() which will indicate whether traces are going to be produced
for the current source, level, event mask, parameters and tracking.

2 years agoCLEANUP: trace: remove the QUIC-specific ifdefs
Willy Tarreau [Tue, 24 Jan 2023 15:02:27 +0000 (16:02 +0100)] 
CLEANUP: trace: remove the QUIC-specific ifdefs

There are ifdefs at several places to only define TRC_ARGS_QCON when
QUIC is defined, but nothing prevents this code from building without.
Let's just remove those ifdefs, the single "if" they avoid is not worth
the extra maintenance burden.

2 years agoBUG/MINOR: sink: free the forwarding task on exit
Willy Tarreau [Thu, 26 Jan 2023 14:46:08 +0000 (15:46 +0100)] 
BUG/MINOR: sink: free the forwarding task on exit

ASAN reported a small leak of the sink's forwarding task on exit.

This should be backported as far as 2.2.

2 years agoBUG/MINOR: ring: release the backing store name on exit
Willy Tarreau [Thu, 26 Jan 2023 14:34:31 +0000 (15:34 +0100)] 
BUG/MINOR: ring: release the backing store name on exit

ASAN found that a ring equipped with a backing store did not release
the store name on exit.

This should be backported to 2.7.

2 years agoBUG/MINOR: log: release global log servers on exit
Willy Tarreau [Thu, 26 Jan 2023 14:32:12 +0000 (15:32 +0100)] 
BUG/MINOR: log: release global log servers on exit

Since 2.6 we have a free_logsrv() function that is used to release log
servers. It must be called from deinit() instead of manually iterating
over the log servers, otherwise some parts of the structure are not
freed (namely the ring name), as reported by ASAN.

This should be backported to 2.6.

2 years agoBUG/MEDIUM: hpack: fix incorrect huffman decoding of some control chars
Willy Tarreau [Thu, 26 Jan 2023 10:12:11 +0000 (11:12 +0100)] 
BUG/MEDIUM: hpack: fix incorrect huffman decoding of some control chars

Commit 9f4f6b038 ("OPTIM: hpack-huff: reduce the cache footprint of the
huffman decoder") replaced the large tables with more space efficient
byte arrays, but one table, rht_bit15_11_11_4, has a 64 bytes hole in it
that wasn't materialized by filling it with zeroes to make the offsets
match, nor by adjusting the offset from the caller. This resulted in
some control chars not properly being decoded and being seen as byte 0,
and the associated messages to be rejected, as can be seen in issue #1971.

This commit fixes it by adjusting the offset used for the higher part of
the table so that we don't need to store 64 zeroes that will never be
accessed.

This needs to be backported to 2.7.

Thanks to Christopher for spotting the bug, and to Juanga Covas for
providing precious traces showing the problem.

2 years agoBUG/MEDIUM: mux-quic: fix crash on H3 SETTINGS emission
Amaury Denoyelle [Wed, 25 Jan 2023 16:44:36 +0000 (17:44 +0100)] 
BUG/MEDIUM: mux-quic: fix crash on H3 SETTINGS emission

A major regression was introduced by following patch
    commit 71fd03632fff43f11cebc6ff4974723c9dc81c67
    MINOR: mux-quic/h3: send SETTINGS as soon as transport is ready

H3 finalize operation is now called at an early stage in the middle of
qc_init(). However, some qcc members are not yet initialized. In
particular the stream tree which will cause a crash when H3 control
stream will be accessed.

To fix this, qcc_install_app_ops() has been delayed at the end of
qc_init(). This ensures that qcc is properly initialized when app_ops
operation are used.

This must be backported wherever above patch is. For the record, it has
been tagged up to 2.7.

2 years agoBUG/MINOR: h3: fix GOAWAY emission
Amaury Denoyelle [Wed, 25 Jan 2023 09:50:03 +0000 (10:50 +0100)] 
BUG/MINOR: h3: fix GOAWAY emission

Since the rework of QUIC streams send scheduling, each stream has to be
inserted in QUIC-mux send-list to be able to emit content. This was not
the case for GOAWAY which prevent it to be sent. This regression has
been introduced by the following patch :

  commit 20f2a425ffeda2e623aac4c702f4e44b1e122d1d
  MAJOR: mux-quic: rework stream sending priorization

This new patch fixes the issue by inserting H3 control stream in mux
send-list. The impact is deemed minor as for the moment GOAWAY is only
sent just before connection/mux cleanup with a CONNECTION_CLOSE.
However, it might cause some connections to hang up indefinitely.

This should be backported up to 2.7.

2 years agoMINOR: mux-quic/h3: send SETTINGS as soon as transport is ready
Amaury Denoyelle [Tue, 24 Jan 2023 16:35:37 +0000 (17:35 +0100)] 
MINOR: mux-quic/h3: send SETTINGS as soon as transport is ready

As specified by HTTP3 RFC, SETTINGS frame should be sent as soon as
possible. Before this patch, this was only done on the first qc_send()
invocation. This delay significantly SETTINGS emission until the first
H3 response is ready to be transferred.

This patch fixes this by ensuring SETTINGS is emitted when MUX-QUIC is
being setup.

As a side point, return value of finalize operation is checked. This
means that an error during SETTINGS emission will cause the connection
init to fail.

This should be backported up to 2.7.

2 years agoMINOR: connection: add a BUG_ON() to detect destroying connection in idle list
Olivier Houchard [Tue, 24 Jan 2023 22:59:32 +0000 (23:59 +0100)] 
MINOR: connection: add a BUG_ON() to detect destroying connection in idle list

Add a BUG_ON() in conn_free(), to check that when we're freeing a
connection, it is not still in the idle connections tree, otherwise the
next thread that will try to use it will probably crash.

2 years agoMINOR: ssl: Remove debug fprintf in 'update ssl ocsp-response' cli command
Remi Tricot-Le Breton [Mon, 23 Jan 2023 14:57:14 +0000 (15:57 +0100)] 
MINOR: ssl: Remove debug fprintf in 'update ssl ocsp-response' cli command

A debug fprintf was left behind in the new cli function.

2 years agoBUG/MINOR: ssl: Fix leaks in 'update ssl ocsp-response' CLI command
Remi Tricot-Le Breton [Mon, 23 Jan 2023 14:57:13 +0000 (15:57 +0100)] 
BUG/MINOR: ssl: Fix leaks in 'update ssl ocsp-response' CLI command

This patch fixes two leaks in the 'update ssl ocsp-response' cli
command. One rather significant one since a whole trash buffer was
allocated for every call of the command, and another more marginal one
in an error path.

This patch does not need to be backported.

2 years agoDEV: haring: add a new option "-r" to automatically repair broken files
Willy Tarreau [Tue, 24 Jan 2023 11:13:14 +0000 (12:13 +0100)] 
DEV: haring: add a new option "-r" to automatically repair broken files

In case a file-backed ring was not properly synced before being dumped,
the output can look bogus due to the head pointer not being perfectly
up to date. In this case, passing "-r" will make haring automatically
skip entries not starting with a zero, and resynchronize with the rest
of the messages.

This should be backported to 2.6.

2 years agoBUG/MINOR: sink: make sure to always properly unmap a file-backed ring
Willy Tarreau [Tue, 24 Jan 2023 11:11:41 +0000 (12:11 +0100)] 
BUG/MINOR: sink: make sure to always properly unmap a file-backed ring

The munmap() call performed on exit was incorrect since it used to apply
to the buffer instead of the area, so neither the pointer nor the size
were page-aligned. This patches corrects this and also adds a call to
msync() since munmap() alone doesn't guarantee that data will be dumped.

This should be backported to 2.6.

2 years ago[RELEASE] Released version 2.8-dev2 v2.8-dev2
Willy Tarreau [Sun, 22 Jan 2023 13:20:57 +0000 (14:20 +0100)] 
[RELEASE] Released version 2.8-dev2

Released version 2.8-dev2 with the following main changes :
    - CLEANUP: htx: fix a typo in an error message of http_str_to_htx
    - DOC: config: added optional rst-ttl argument to silent-drop in action lists
    - BUG/MINOR: ssl: Fix crash in 'update ssl ocsp-response' CLI command
    - BUG/MINOR: ssl: Crash during cleanup because of ocsp structure pointer UAF
    - MINOR: ssl: Create temp X509_STORE filled with cert chain when checking ocsp response
    - MINOR: ssl: Only set ocsp->issuer if issuer not in cert chain
    - MINOR: ssl: Release ssl_ocsp_task_ctx.cur_ocsp when destroying task
    - MINOR: ssl: Detect more OCSP update inconsistencies
    - BUG/MINOR: ssl: Fix OCSP_CERTID leak when same certificate is used multiple times
    - MINOR: ssl: Limit ocsp_uri buffer size to minimum
    - MINOR: ssl: Remove mention of ckch_store in error message of cli command
    - MINOR: channel: Don't test CF_READ_NULL while CF_SHUTR is enough
    - REORG: channel: Rename CF_READ_NULL to CF_READ_EVENT
    - REORG: channel: Rename CF_WRITE_NULL to CF_WRITE_EVENT
    - MEDIUM: channel: Use CF_READ_EVENT instead of CF_READ_PARTIAL
    - MEDIUM: channel: Use CF_WRITE_EVENT instead of CF_WRITE_PARTIAL
    - MINOR: channel: Remove CF_READ_ACTIVITY
    - MINOR: channel: Remove CF_WRITE_ACTIVITY
    - MINOR: channel: Remove CF_ANA_TIMEOUT and report CF_READ_EVENT instead
    - MEDIUM: channel: Remove CF_READ_ATTACHED and report CF_READ_EVENT instead
    - MINOR: channel: Stop to test CF_READ_ERROR flag if CF_SHUTR is enough
    - MINOR: channel/applets: Stop to test CF_WRITE_ERROR flag if CF_SHUTW is enough
    - DOC: management: add details on "Used" status
    - DOC: management: add details about @system-ca in "show ssl ca-file"
    - BUG/MINOR: mux-quic: fix transfer of empty HTTP response
    - MINOR: mux-quic: add traces for flow-control limit reach
    - MAJOR: mux-quic: rework stream sending priorization
    - MEDIUM: h3: send SETTINGS before STREAM frames
    - MINOR: mux-quic: use send-list for STOP_SENDING/RESET_STREAM emission
    - MINOR: mux-quic: use send-list for immediate sending retry
    - BUG/MINOR: h1-htx: Remove flags about protocol upgrade on non-101 responses
    - BUG/MINOR: hlua: Fix Channel.line and Channel.data behavior regarding the doc
    - BUG/MINOR: resolvers: Wait the resolution execution for a do_resolv action
    - BUG/MINOR: ssl: Remove unneeded pointer check in ocsp cli release function
    - BUG/MINOR: ssl: Missing ssl_conf pointer check when checking ocsp update inconsistencies
    - DEV: tcploop: add minimal support for unix sockets
    - BUG/MEDIUM: listener: duplicate inherited FDs if needed
    - BUG/MINOR: ssl: OCSP minimum update threshold not properly set
    - MINOR: ssl: Treat ocsp-update inconsistencies as fatal errors
    - MINOR: ssl: Do not wake ocsp update task if update tree empty
    - MINOR: ssl: Reinsert updated ocsp response later in tree in case of http error
    - REGTEST: ssl: Add test for 'update ssl ocsp-response' CLI command
    - OPTIM: global: move byte counts out of global and per-thread
    - BUG/MEDIUM: peers: make "show peers" more careful about partial initialization
    - BUG/MINOR: promex: Don't forget to consume the request on error
    - MINOR: http-ana: Add a function to set HTTP termination flags
    - MINOR: http-ana: Use http_set_term_flags() in most of HTTP analyzers
    - BUG/MINOR: http-ana: Report SF_FINST_R flag on error waiting the request body
    - MINOR: http-ana: Use http_set_term_flags() when waiting the request body
    - BUG/MINOR: http-fetch: Don't block HTTP sample fetch eval in HTTP_MSG_ERROR state
    - MAJOR: http-ana: Review error handling during HTTP payload forwarding
    - CLEANUP: http-ana: Remove HTTP_MSG_ERROR state
    - BUG/MEDIUM: mux-h2: Don't send CANCEL on shutw when response length is unkown
    - MINOR: htx: Add an HTX value for the extra field is payload length is unknown
    - BUG/MINOR: http-ana: make set-status also update txn->status
    - BUG/MINOR: listeners: fix suspend/resume of inherited FDs
    - DOC: config: fix wrong section number for "protocol prefixes"
    - DOC: config: fix aliases for protocol prefixes "udp4@" and "udp6@"
    - DOC: config: mention the missing "quic4@" and "quic6@" in protocol prefixes
    - MINOR: listener: also support "quic+" as an address prefix
    - CLEANUP: stconn: always use se_fl_set_error() to set the pending error
    - BUG/MEDIUM: stconn: also consider SE_FL_EOI to switch to SE_FL_ERROR
    - MINOR: quic: Useless test about datagram destination addresses
    - MINOR: quic: Disable the active connection migrations
    - MINOR: quic: Add "no-quic" global option
    - MINOR: sample: Add "quic_enabled" sample fetch
    - MINOR: quic: Replace v2 draft definitions by those of the final 2 version
    - BUG/MINOR: mux-fcgi: Correctly set pathinfo
    - DOC: config: fix "Address formats" chapter syntax
    - BUG/MEDIUM: jwt: Properly process ecdsa signatures (concatenated R and S params)
    - BUILD: ssl: add ECDSA_SIG_set0() for openssl < 1.1 or libressl < 2.7
    - Revert "BUILD: ssl: add ECDSA_SIG_set0() for openssl < 1.1 or libressl < 2.7"
    - BUG/MINOR: ssl: Fix compilation with OpenSSL 1.0.2 (missing ECDSA_SIG_set0)
    - BUG/MINOR: listener: close tiny race between resume_listener() and stopping
    - BUG/MINOR: h3: properly handle connection headers
    - MINOR: h3: extend function for QUIC varint encoding
    - MINOR: h3: implement TRAILERS encoding
    - BUG/MINOR: bwlim: Check scope for period expr for set-bandwitdh-limit actions
    - MEDIUM: bwlim: Support constants limit or period on set-bandwidth-limit actions
    - BUG/MINOR: bwlim: Fix parameters check for set-bandwidth-limit actions
    - MINOR: h3: implement TRAILERS decoding
    - BUG/MEDIUM: fd/threads: fix again incorrect thread selection in wakeup broadcast
    - BUG/MINOR: thread: always reload threads_enabled in loops
    - MINOR: threads: add a thread_harmless_end() version that doesn't wait
    - BUG/MEDIUM: debug/thread: make the debug handler not wait for !rdv_requests
    - BUG/MINOR: mux-h2: make sure to produce a log on invalid requests
    - BUG/MINOR: mux-h2: add missing traces on failed headers decoding
    - BUILD: hpack: include global.h for the trash that is needed in debug mode
    - BUG/MINOR: jwt: Wrong return value checked
    - BUG/MINOR: quic: Do not request h3 clients to close its unidirection streams
    - MEDIUM: quic-sock: fix udp source address for send on listener socket

2 years agoMEDIUM: quic-sock: fix udp source address for send on listener socket
Amaury Denoyelle [Thu, 19 Jan 2023 17:05:54 +0000 (18:05 +0100)] 
MEDIUM: quic-sock: fix udp source address for send on listener socket

When receiving a QUIC datagram, destination address is retrieved via
recvmsg() and stored in quic-conn as qc.local_addr. This address is then
reused when using the quic-conn owned socket.

When listener socket mode is preferred, send operation did not specify
the source address of the emitted datagram. If listener socket is bound
on a wildcard address, the kernel is free to choose any address assigned
to the local machine. This may be different from the address selected by
the client on its first datagram which will prevent the client to emit
next replies.

To address this, this patch fixes the UDP source address via sendmsg().
This process is similar to the reception and relies on ancillary
message, so the socket is left untouched after the operation. This is
heavily platform specific and may not be supported by some kernels.

This change has only an impact if listener socket only is used for QUIC
communications. This is the default behavior for 2.7 branch but not
anymore on 2.8. Use tune.quic.socket-owner set to listener to ensure set
it.

This should be backported up to 2.7.

2 years agoBUG/MINOR: quic: Do not request h3 clients to close its unidirection streams
Frédéric Lécaille [Fri, 20 Jan 2023 14:33:50 +0000 (15:33 +0100)] 
BUG/MINOR: quic: Do not request h3 clients to close its unidirection streams

It is forbidden to request h3 clients to close its Control and QPACK unidirection
streams. If not, the client closes the connection with H3_CLOSED_CRITICAL_STREAM(0x104).
Perhaps this could prevent some clients as Chrome to come back for a while.

But at quic_conn level there is no mean to identify the streams for which we cannot
send STOP_SENDING frame. Such a possibility is even not mentionned in RFC 9000.
At this time there is no choice than stopping sending STOP_SENDING frames for
all the h3 unidirectional streams inspecting the ->app_opps quic_conn value.

Must be backported to 2.7 and 2.6.

2 years agoBUG/MINOR: jwt: Wrong return value checked
Remi Tricot-Le Breton [Fri, 20 Jan 2023 08:37:26 +0000 (09:37 +0100)] 
BUG/MINOR: jwt: Wrong return value checked

The wrong return value was checked, resulting in dead code and
potential bugs.

It should fix GitHub issue #2005.
This patch should be backported up to 2.5.

2 years agoBUILD: hpack: include global.h for the trash that is needed in debug mode
Willy Tarreau [Thu, 19 Jan 2023 23:02:37 +0000 (00:02 +0100)] 
BUILD: hpack: include global.h for the trash that is needed in debug mode

When building with -DDEBUG_HPACK, the trash is needed, but it's declared
in global.h.

This may be backported to all supported versions.

2 years agoBUG/MINOR: mux-h2: add missing traces on failed headers decoding
Willy Tarreau [Thu, 19 Jan 2023 22:58:11 +0000 (23:58 +0100)] 
BUG/MINOR: mux-h2: add missing traces on failed headers decoding

In case HPACK cannot be decoded, logs are emitted but there's no info
in the H2 traces, so let's add them.

This may be backported to all supported versions.

2 years agoBUG/MINOR: mux-h2: make sure to produce a log on invalid requests
Willy Tarreau [Thu, 19 Jan 2023 22:22:03 +0000 (23:22 +0100)] 
BUG/MINOR: mux-h2: make sure to produce a log on invalid requests

As reported by Dominik Froehlich in github issue #1968, some H2 request
parsing errors do not result in a log being emitted. This is annoying
for debugging because while an RST_STREAM is correctly emitted to the
client, there's no way without enabling traces to find it on the
haproxy side.

After some testing with various abnormal requests, a few places were
found where logs were missing and could be added. In this case, we
simply use sess_log() so some sample fetch functions might not be
available since the stream is not created. But at least there will
be a BADREQ in the logs. A good eaxmple of this consists in sending
forbidden headers or header syntax (e.g. presence of LF in value).
Some quick tests can be done this way:

- protocol error (LF in value):
  curl -iv --http2-prior-knowledge -H "$(printf 'a:b\na')" http://0:8001/

- too large header block after decoding:
  curl -v --http2-prior-knowledge -H "a:$(perl -e "print('a'x10000)")"  -H "a:$(perl -e "print('a'x10000)")"  http://localhost:8001/

This should be backported where needed, most likely 2.7 and 2.6 at
least for a start, and progressively to other versions.

2 years agoBUG/MEDIUM: debug/thread: make the debug handler not wait for !rdv_requests
Willy Tarreau [Thu, 19 Jan 2023 17:52:31 +0000 (18:52 +0100)] 
BUG/MEDIUM: debug/thread: make the debug handler not wait for !rdv_requests

The debug handler may deadlock with some threads waiting for isolation.
This may happend during a "show threads" command or even during a panic.
The reason is the call to thread_harmless_end() which waits for rdv_requests
to turn to zero before releasing its position in thread_dump_state,
while that one may not progress if another thread was interrupted in
thread_isolate() and is waiting for that thread to drop thread_dump_state.

In order to address this, we now use thread_harmless_end_sig() introduced
by previous commit:

   MINOR: threads: add a thread_harmless_end() version that doesn't wait

However there's a catch: since commit f7afdd910 ("MINOR: debug: mark
oneself harmless while waiting for threads to finish"), there's a second
pair of thread_harmless_now()/thread_harmless_end() that surround the
loop around thread_dump_state. Marking a thread harmless before this
loop and dropping that without checking rdv_requests there could break
the harmless promise made to the other thread if it returns first and
proceeds with its isolated work. Hence we just drop this pair which was
only preventive for other signal handlers, while as indicated in that
patch's commit message, other signals are handled asynchronously and do
not require that extra protection.

This fix must be backported to 2.7.

The problem can be seen by running "show threads" in fast loops (100/s)
while reloading haproxy very quickly (10/s) and sending lots of traffic
to it (100krps, 15 Gbps). In this case the soft stop calls pool_gc()
which isolates a lot and manages to race with the dumps after a few
tens of seconds, leaving the process with all threads at 100%.

2 years agoMINOR: threads: add a thread_harmless_end() version that doesn't wait
Willy Tarreau [Thu, 19 Jan 2023 17:43:48 +0000 (18:43 +0100)] 
MINOR: threads: add a thread_harmless_end() version that doesn't wait

thread_harmless_end() needs to wait for rdv_requests to disappear so
that we're certain to respect a harmless promise that possibly allowed
another thread to proceed under isolation. But this doesn't work in a
signal handler because a thread could be interrupted by the debug
handler while already waiting for isolation and with rdv_request>0.
As such this function could cause a deadlock in such a signal handler.

Let's implement a specific variant for this, thread_harmless_end_sig(),
that just resets the thread's bit and doesn't wait. It must of course
not be used past a check point that would allow the isolation requester
to return and see the thread as temporarily harmless then turning back
on its promise.

This will be needed to fix a race in the debug handler.

2 years agoBUG/MINOR: thread: always reload threads_enabled in loops
Willy Tarreau [Thu, 19 Jan 2023 18:14:18 +0000 (19:14 +0100)] 
BUG/MINOR: thread: always reload threads_enabled in loops

A few loops waiting for threads to synchronize such as thread_isolate()
rightfully filter the thread masks via the threads_enabled field that
contains the list of enabled threads. However, it doesn't use an atomic
load on it. Before 2.7, the equivalent variables were marked as volatile
and were always reloaded. In 2.7 they're fields in ha_tgroup_ctx[], and
the risk that the compiler keeps them in a register inside a loop is not
null at all. In practice when ha_thread_relax() calls sched_yield() or
an x86 PAUSE instruction, it could be verified that the variable is
always reloaded. If these are avoided (e.g. architecture providing
neither solution), it's visible in asm code that the variables are not
reloaded. In this case, if a thread exists just between the moment the
two values are read, the loop could spin forever.

This patch adds the required _HA_ATOMIC_LOAD() on the relevant
threads_enabled fields. It must be backported to 2.7.

2 years agoBUG/MEDIUM: fd/threads: fix again incorrect thread selection in wakeup broadcast
Willy Tarreau [Thu, 19 Jan 2023 16:10:10 +0000 (17:10 +0100)] 
BUG/MEDIUM: fd/threads: fix again incorrect thread selection in wakeup broadcast

Commit c1640f79f ("BUG/MEDIUM: fd/threads: fix incorrect thread selection
in wakeup broadcast") fixed an incorrect range being used to pick a thread
when broadcasting a wakeup for a foreign thread, but the selection was still
wrong as the number of threads and their mask was taken from the current
thread instead of the target thread. In addition, the code dealing with the
wakeup of a thread from the same group was still relying on MAX_THREADS
instead of tg->count.

This could theoretically cause random crashes with more than one thread
group though this was never encountered.

This needs to be backported to 2.7.

2 years agoMINOR: h3: implement TRAILERS decoding
Amaury Denoyelle [Fri, 13 Jan 2023 15:40:31 +0000 (16:40 +0100)] 
MINOR: h3: implement TRAILERS decoding

Implement the conversion of H3 request trailers as HTX blocks. This is
done through a new function h3_trailers_to_htx(). If the request
contains forbidden trailers it is rejected with a stream error.

This should be backported up to 2.7.

2 years agoBUG/MINOR: bwlim: Fix parameters check for set-bandwidth-limit actions
Christopher Faulet [Fri, 13 Jan 2023 14:39:54 +0000 (15:39 +0100)] 
BUG/MINOR: bwlim: Fix parameters check for set-bandwidth-limit actions

First, the inspect-delay is now tested if the action is used on a
tcp-response content rule. Then, when an expressions scope is checked, we
now take care to detect the right scope depending on the ruleset used
(tcp-request, tcp-response, http-request or http-response).

This patch could be backported to 2.7.

2 years agoMEDIUM: bwlim: Support constants limit or period on set-bandwidth-limit actions
Christopher Faulet [Fri, 13 Jan 2023 14:33:32 +0000 (15:33 +0100)] 
MEDIUM: bwlim: Support constants limit or period on set-bandwidth-limit actions

It is now possible to set a constant for the limit or period parameters on a
set-bandwidth-limit actions. The limit must follow the HAProxy size format
and is expressed in bytes. The period must follow the HAProxy time format
and is expressed in milliseconds. Of course, it is still possible to use
sample expressions instead.

The documentation was updated accordingly.

It is not really a bug. Only exemples were written this way in the
documentation. But it could be good to backport this change in 2.7.

2 years agoBUG/MINOR: bwlim: Check scope for period expr for set-bandwitdh-limit actions
Christopher Faulet [Fri, 13 Jan 2023 14:21:53 +0000 (15:21 +0100)] 
BUG/MINOR: bwlim: Check scope for period expr for set-bandwitdh-limit actions

If a period expression is defined for a set-bandwitdh-limit action, its
scope must be tested.

This patch must be backported to 2.7.

2 years agoMINOR: h3: implement TRAILERS encoding
Amaury Denoyelle [Thu, 12 Jan 2023 13:53:43 +0000 (14:53 +0100)] 
MINOR: h3: implement TRAILERS encoding

This patch implement the conversion of an HTX response containing
trailer into a H3 HEADERS frame. This is done through a new function
named h3_resp_trailers_send().

This was tested with a nginx configuration using <add_trailer>
statement.

It may be possible that HTX buffer only contains a EOT block without
preceeding trailer. In this case, the conversion will produce nothing
but fin will be reported. This causes QUIC mux to generate an empty
STREAM frame with FIN bit set.

This should be backported up to 2.7.

2 years agoMINOR: h3: extend function for QUIC varint encoding
Amaury Denoyelle [Tue, 17 Jan 2023 14:21:16 +0000 (15:21 +0100)] 
MINOR: h3: extend function for QUIC varint encoding

Slighty adjust b_quic_enc_int(). This function is used to encode an
integer as a QUIC varint in a struct buffer.

A new parameter is added to the function API to specify the width of the
encoded integer. By default, 0 should be use to ensure that the minimum
space is used. Other valid values are 1, 2, 4 or 8. An error is reported
if the width is not large enough.

This new parameter will be useful when buffer space is reserved prior to
encode an unknown integer value. The maximum size of 8 bytes will be
reserved and some data can be put after. When finally encoding the
integer, the width can be requested to be 8 bytes.

With this new parameter, a small refactoring of the function has been
conducted to remove some useless internal variables.

This should be backported up to 2.7. It will be mostly useful to
implement H3 trailers encoding.

2 years agoBUG/MINOR: h3: properly handle connection headers
Amaury Denoyelle [Tue, 17 Jan 2023 16:47:06 +0000 (17:47 +0100)] 
BUG/MINOR: h3: properly handle connection headers

Connection headers are not used in HTTP/3. As specified by RFC 9114, a
received message containing one of those is considered as malformed and
rejected. When converting an HTX message to HTTP/3, these headers are
silently skipped.

This must be backported up to 2.6. Note that assignment to <h3s.err>
must be removed on 2.6 as stream level error has been introduced in 2.7
so this field does not exist in 2.6 A connection error will be used
instead automatically.

2 years agoBUG/MINOR: listener: close tiny race between resume_listener() and stopping
Willy Tarreau [Thu, 19 Jan 2023 10:34:21 +0000 (11:34 +0100)] 
BUG/MINOR: listener: close tiny race between resume_listener() and stopping

Pierre Cheynier reported a very rare race condition on soft-stop in the
listeners. What happens is that if a previously limited listener is
being resumed by another thread finishing an accept loop, and at the
same time a soft-stop is performed, the soft-stop will turn the
listener's state to LI_INIT, and once the listener's lock is released,
resume_listener() in the second thread will try to resume this listener
which has an fd==-1, yielding a crash in listener_set_state():

  FATAL: bug condition "l->rx.fd == -1" matched at src/listener.c:288

The reason is that resume_listener() only checks for LI_READY, but doesn't
consider being called with a non-initialized or a stopped listener. Let's
also make sure we don't try to ressuscitate such a listener there.

This will have to be backported to all versions.

2 years agoBUG/MINOR: ssl: Fix compilation with OpenSSL 1.0.2 (missing ECDSA_SIG_set0)
Remi Tricot-Le Breton [Wed, 18 Jan 2023 16:29:54 +0000 (17:29 +0100)] 
BUG/MINOR: ssl: Fix compilation with OpenSSL 1.0.2 (missing ECDSA_SIG_set0)

This function was introduced in OpenSSL 1.1.0. Prior to that, the
ECDSA_SIG structure was public.
This function was used in commit 5a8f02ae "BUG/MEDIUM: jwt: Properly
process ecdsa signatures (concatenated R and S params)".

This patch needs to be backported up to branch 2.5 alongside commit
5a8f02ae.

2 years agoRevert "BUILD: ssl: add ECDSA_SIG_set0() for openssl < 1.1 or libressl < 2.7"
William Lallemand [Thu, 19 Jan 2023 10:08:42 +0000 (11:08 +0100)] 
Revert "BUILD: ssl: add ECDSA_SIG_set0() for openssl < 1.1 or libressl < 2.7"

This reverts commit d65791e26c12b57723f2feb7eacdbbd99601371b.

Conflict with the patch which was originally written and lacks the
BN_clear_free() and the NULL check.

2 years agoBUILD: ssl: add ECDSA_SIG_set0() for openssl < 1.1 or libressl < 2.7
Willy Tarreau [Thu, 19 Jan 2023 09:50:13 +0000 (10:50 +0100)] 
BUILD: ssl: add ECDSA_SIG_set0() for openssl < 1.1 or libressl < 2.7

Commit 5a8f02ae6 ("BUG/MEDIUM: jwt: Properly process ecdsa signatures
(concatenated R and S params)") makes use of ECDSA_SIG_set0() which only
appeared in openssl-1.1.0 and libressl 2.7, and breaks the build before.
Let's just do what it minimally does (only assigns the two fields to the
destination).

This will need to be backported where the commit above is, likely 2.5.

2 years agoBUG/MEDIUM: jwt: Properly process ecdsa signatures (concatenated R and S params)
Remi Tricot-Le Breton [Wed, 18 Jan 2023 14:32:28 +0000 (15:32 +0100)] 
BUG/MEDIUM: jwt: Properly process ecdsa signatures (concatenated R and S params)

When the JWT token signature is using ECDSA algorithm (ES256 for
instance), the signature is a direct concatenation of the R and S
parameters instead of OpenSSL's DER format (see section
3.4 of RFC7518).
The code that verified the signatures wrongly assumed that they came in
OpenSSL's format and it did not actually work.
We now have the extra step of converting the signature into a complete
ECDSA_SIG that can be fed into OpenSSL's digest verification functions.

The ECDSA signatures in the regtest had to be recalculated and it was
made via the PyJWT python library so that we don't end up checking
signatures that we built ourselves anymore.

This patch should fix GitHub issue #2001.
It should be backported up to branch 2.5.

2 years agoDOC: config: fix "Address formats" chapter syntax
Daniel Corbett [Tue, 17 Jan 2023 23:32:31 +0000 (18:32 -0500)] 
DOC: config: fix "Address formats" chapter syntax

The section on "Address formats" doesn't provide the dot (.) after the
chapter numbers, which breaks parsing within the HTML converter.
This commit adds the dot (.) after each chapter within Section 11.

This should be backported to versions 2.4 and above.

2 years agoBUG/MINOR: mux-fcgi: Correctly set pathinfo
Paul Barnetta [Mon, 16 Jan 2023 22:44:11 +0000 (09:44 +1100)] 
BUG/MINOR: mux-fcgi: Correctly set pathinfo

Existing logic for checking whether a regex subexpression for pathinfo
is matched results in valid matches being ignored and non-matches having
a new zero length string stored in params->pathinfo. This patch reverses
the logic so params->pathinfo is set when the subexpression is matched.

Without this patch the example configuration in the documentation:

path-info ^(/.+\.php)(/.*)?$

does not result in PATH_INFO being sent to the FastCGI application, as
expected, when the second subexpression is matched (in which case both
pmatch[2].rm_so and pmatch[2].rm_eo will be non-negative integers).

This patch may be backported as far as 2.2, the first release that made
the capture of this second subexpression optional.

2 years agoMINOR: quic: Replace v2 draft definitions by those of the final 2 version
Frédéric Lécaille [Fri, 13 Jan 2023 15:37:02 +0000 (16:37 +0100)] 
MINOR: quic: Replace v2 draft definitions by those of the final 2 version

This should finalize the support for the QUIC version 2.

Must be backported to 2.7.

2 years agoMINOR: sample: Add "quic_enabled" sample fetch
Frédéric Lécaille [Thu, 12 Jan 2023 16:55:45 +0000 (17:55 +0100)] 
MINOR: sample: Add "quic_enabled" sample fetch

This sample fetch returns a boolean. True if the support for QUIC transport
protocol was built and if this protocol was not disabled by "no-quic"
global option.

Must be backported to 2.7.

2 years agoMINOR: quic: Add "no-quic" global option
Frédéric Lécaille [Thu, 12 Jan 2023 14:23:54 +0000 (15:23 +0100)] 
MINOR: quic: Add "no-quic" global option

Add "no-quic" to "global" section to disable the use of QUIC transport protocol
by all configured QUIC listeners. This is listeners with QUIC addresses on their
"bind" lines. Internally, the socket addresses binding is skipped by
protocol_bind_all() for receivers with <proto_quic4> or <proto_quic6> as
protocol (see protocol struct).
Add information about "no-quic" global option to the documentation.

Must be backported to 2.7.

2 years agoMINOR: quic: Disable the active connection migrations
Frédéric Lécaille [Thu, 12 Jan 2023 07:29:23 +0000 (08:29 +0100)] 
MINOR: quic: Disable the active connection migrations

Set "disable_active_migration" transport parameter to inform the peer
haproxy listeners does not the connection migration feature.
Also drop all received datagrams with a modified source address.

Must be backported to 2.7.

2 years agoMINOR: quic: Useless test about datagram destination addresses
Frédéric Lécaille [Thu, 12 Jan 2023 09:36:26 +0000 (10:36 +0100)] 
MINOR: quic: Useless test about datagram destination addresses

There is no reason to check if the peer has modified the destination
address of its connection.

May be backported to 2.7.

2 years agoBUG/MEDIUM: stconn: also consider SE_FL_EOI to switch to SE_FL_ERROR
Willy Tarreau [Tue, 17 Jan 2023 15:27:35 +0000 (16:27 +0100)] 
BUG/MEDIUM: stconn: also consider SE_FL_EOI to switch to SE_FL_ERROR

In se_fl_set_error() we used to switch to SE_FL_ERROR only when there
is already SE_FL_EOS indicating that the read side is closed. But that
is not sufficient, we need to consider all cases where no more reads
will be performed on the connection, and as such also include SE_FL_EOI.

Without this, some aborted connections during a transfer sometimes only
stop after the timeout, because the ERR_PENDING is never promoted to
ERROR.

This must be backported to 2.7 and requires previous patch "CLEANUP:
stconn: always use se_fl_set_error() to set the pending error".

2 years agoCLEANUP: stconn: always use se_fl_set_error() to set the pending error
Willy Tarreau [Tue, 17 Jan 2023 15:25:29 +0000 (16:25 +0100)] 
CLEANUP: stconn: always use se_fl_set_error() to set the pending error

In mux-h2 and mux-quic we still had two places manually setting
SE_FL_ERR_PENDING or SE_FL_ERROR depending on the EOS state, instead
of using se_fl_set_error() which takes care of the condition. Better
use the specialized function for this, it will allow to centralize
the conditions. Note that this will be needed to fix a bug.

2 years agoMINOR: listener: also support "quic+" as an address prefix
Willy Tarreau [Mon, 16 Jan 2023 12:55:27 +0000 (13:55 +0100)] 
MINOR: listener: also support "quic+" as an address prefix

While we do support quic4@ and quic6@ for listening addresses, it was
not possible to specify that we want to use an FD inherited from the
parent with QUIC. It's just a matter of making it possible to enable
a dgram-type socket and a stream-type transport, so let's add this.

Now it becomes possible to write "quic+fd@12", "quic+ipv4@addr" etc.

2 years agoDOC: config: mention the missing "quic4@" and "quic6@" in protocol prefixes
Willy Tarreau [Mon, 16 Jan 2023 11:14:11 +0000 (12:14 +0100)] 
DOC: config: mention the missing "quic4@" and "quic6@" in protocol prefixes

These two variants were missing from the section on protocol prefixes.

2 years agoDOC: config: fix aliases for protocol prefixes "udp4@" and "udp6@"
Willy Tarreau [Mon, 16 Jan 2023 11:11:38 +0000 (12:11 +0100)] 
DOC: config: fix aliases for protocol prefixes "udp4@" and "udp6@"

It was mentioned that they are equivalent to "stream+ipv*@" while it's
the equivalent of "dgram+ipv*@".

2 years agoDOC: config: fix wrong section number for "protocol prefixes"
Willy Tarreau [Mon, 16 Jan 2023 11:07:12 +0000 (12:07 +0100)] 
DOC: config: fix wrong section number for "protocol prefixes"

The socket type prefixes used to reference section "11.5.3" instead of
"11.3" for "protocol prefixes".