Commit Graph

19463 Commits

Author SHA1 Message Date
Willy Tarreau
758c69d951 MINOR: listener: move the maxconn parameter to the bind_conf
The maxconn is set per bind line so let's move it there. This might
possibly even slightly reduce inter-thread contention since this one
is read-mostly and it was stored next to nbconn which changes for
each connection setup or teardown.
2023-02-03 18:00:20 +01:00
Willy Tarreau
1920f897d8 MINOR: listener: move the backlog setting from listener to bind_conf
The backlog setting is also defined by the bind_conf, so let's move
it there.
2023-02-03 18:00:20 +01:00
Willy Tarreau
882f2485a1 MINOR: listener: move maxaccept from listener to bind_conf
Like for previous values, maxaccept is really per-bind_conf, so let's
move it there. Some frontends (peers, log) set it to 1 so the assignment
was slightly moved.
2023-02-03 18:00:20 +01:00
Willy Tarreau
ee378165fb MINOR: listener: move maxseg and tcp_ut to bind_conf
These two arguments were only set and only used with tcpv4/tcpv6. Let's
just store them into the bind_conf instead of duplicating them for all
listeners since they're fixed per "bind" line.
2023-02-03 18:00:20 +01:00
Willy Tarreau
7866e8e50d MEDIUM: listener: move the analysers mask to the bind_conf
When bind_conf were created, some elements such as the analysers mask
ought to have moved there but that wasn't the case. Now that it's
getting clearer that bind_conf provides all binding parameters and
the listener is essentially a listener on an address, it's starting
to get really confusing to keep such parameters in the listener, so
let's move the mask to the bind_conf. We also take this opportunity
for pre-setting the mask to the frontend's upon initalization. Now
several loops have one less argument to take care of.
2023-02-03 18:00:20 +01:00
Frédéric Lécaille
0aa79953c9 BUG/MINOR: quic: Unchecked source connection ID
The SCID (source connection ID) used by a peer (client or server) is sent into the
long header of a QUIC packet in clear. But it is also sent into the transport
parameters (initial_source_connection_id). As these latter are encrypted into the
packet, one must check that these two pieces of information do not differ
due to a packet header corruption. Furthermore as such a connection is unusuable
it must be killed and must stop as soon as possible processing RX/TX packets.

Implement qc_kill_con() to flag a connection as unusable and to kille it asap
waking up the idle timer task to release the connection.

Add a check to quic_transport_params_store() to detect that the SCIDs do not
match and make it call qc_kill_con().

Add several tests about connection to be killed at several critial locations,
especially in the TLS stack callback to receive CRYPTO data from or derive secrets,
and before preparing packet after having received others.

Must be backported to 2.6 and 2.7.
2023-02-03 17:55:55 +01:00
Frédéric Lécaille
af25a69c8b MEDIUM: quic: Remove qc_conn_finalize() from the ClientHello TLS callbacks
This is a bad idea to make the TLS ClientHello callback call qc_conn_finalize().
If this latter fails, this would generate a TLS alert and make the connection
send packet whereas it is not functional. But qc_conn_finalize() job was to
install the transport parameters sent by the QUIC listener. This installation
cannot be done at any time. This must be done after having possibly negotiated
the QUIC version and before sending the first Handshake packets. It seems
the better moment to do that in when the Handshake TX secrets are derived. This
has been found inspecting the ngtcp2 code. Calling SSL_set_quic_transport_params()
too late would make the ServerHello to be sent without the transport parameters.

The code for the connection update which was done from qc_conn_finalize() has
been moved to quic_transport_params_store(). So, this update is done as soon as
possible.

Add QUIC_FL_CONN_TX_TP_RECEIVED to flag the connection as having received the
peer transport parameters. Indeed this is required when the ClientHello message
is splitted between packets.

Add QUIC_FL_CONN_FINALIZED to protect the connection from calling qc_conn_finalize()
more than one time. This latter is called only when the connection has received
the transport parameters and after returning from SSL_do_hanshake() which is the
function which trigger the TLS ClientHello callback call.

Remove the calls to qc_conn_finalize() from from the TLS ClientHello callbacks.

Must be backported to 2.6. and 2.7.
2023-02-03 17:55:55 +01:00
Frédéric Lécaille
8417beb7da BUG/MAJOR: quic: Possible crash when processing 1-RTT during 0-RTT session
This bug was revealed by some C1 interop tests (heavy hanshake packet
corruption) when receiving 1-RTT packets with a key phase update.
This lead the packet to be decrypted with the next key phase secrets.
But this latter is initialized only after the handshake is complete.

In fact, 1-RTT must never be processed before the handshake is complete.
Relying on the "qc->mux_state == QC_MUX_NULL" condition to check the
handshake is complete is wrong during 0-RTT sessions when the mux
is initialized before the handshake is complete.

Must be backported to 2.7 and 2.6.
2023-02-03 17:55:55 +01:00
Frédéric Lécaille
37ed4a3842 MINOR: quic: When probing Handshake packet number space, also probe the Initial one
This is not really a bug fix but an improvement. When the Handshake packet number
space has been detected as needed to be probed, we should also try to probe the
Initial packet number space if there are still packets in flight. Furthermore
we should also try to send up to two datagrams.

Must be backported to 2.6 and 2.7.
2023-02-03 17:55:55 +01:00
Frédéric Lécaille
055e82657e BUG/MINOR: quic: Do not ignore coalesced packets in qc_prep_fast_retrans()
This function is called only when probing only one packet number space
(Handshake) or two times the same one (Application). So, there is no risk
to prepare two times the same frame when uneeded because we wanted to
probe two packet number spaces. The condition "ignore the packets which
has been coalesced to another one" is not necessary. More importantly
the bug is when we want to prepare a Application packet which has
been coalesced to an Handshake packet. This is always the case when
the first Application packet is sent. It is always coalesced to
an Handshake packet with an ACK frame. So, when lost, this first
application packet was never resent. It contains the HANDSHAKE_DONE
frame to confirm the completion of the handshake to the client.

Must be backported to 2.6 and 2.7.
2023-02-03 17:55:55 +01:00
Frédéric Lécaille
6dead91b8a MINOR: quic: Add a trace about variable states in qc_prep_fast_retrans()
This has already been very useful to diagnose retransmission issues.

Must be backported to 2.6 and 2.7.
2023-02-03 17:55:55 +01:00
Frédéric Lécaille
b75eecc874 BUG/MINOR: quic: Too big PTO during handshakes
During the handshake and when the handshake has not been confirmed
the acknowledgement delays reported by the peer may be larger
than max_ack_delay. max_ack_delay SHOULD be ignored before the
handshake is completed when computing the PTO. But the current code considered
the wrong condition "before the hanshake is completed".

Replace the enum value QUIC_HS_ST_COMPLETED by QUIC_HS_ST_CONFIRMED to
fix this issue. In quic_loss.c, the parameter passed to quic_pto_pktns()
is renamed to avoid any possible confusion.

Must be backported to 2.7 and 2.6.
2023-02-03 17:55:55 +01:00
Frédéric Lécaille
dd419461ef BUG/MINOR: quic: Possible stream truncations under heavy loss
This may happen during retransmission of frames which can be splitted
(CRYPTO, or STREAM frames). One may have to split a frame to be
retransmitted due to the QUIC protocol properties (packet size limitation
and packet field encoding sizes). The remaining part of a frame which
cannot be retransmitted must be detached from the original frame it is
copied from. If not, when the really sent part will be acknowledged
the remaining part will be acknowledged too but not sent!

Must be backported to 2.7 and 2.6.
2023-02-03 17:55:55 +01:00
Frédéric Lécaille
9969adbcdc 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.
2023-02-03 17:55:49 +01:00
Willy Tarreau
1ea5f410ff 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%.
2023-02-03 13:39:20 +01:00
Willy Tarreau
23aa79d9a9 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().
2023-02-03 13:39:18 +01:00
Amaury Denoyelle
24d5b72ca9 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.
2023-02-03 11:56:46 +01:00
Amaury Denoyelle
e4abb1f2da 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.
2023-02-03 11:56:42 +01:00
Amaury Denoyelle
57b3eaa793 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.
2023-02-03 11:55:41 +01:00
Amaury Denoyelle
40c24f1a10 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.
2023-02-03 10:44:26 +01:00
Amaury Denoyelle
1dac018d9f 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.
2023-02-03 09:46:55 +01:00
Amaury Denoyelle
2216b0866e 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.
2023-02-03 09:46:55 +01:00
Amaury Denoyelle
565e3cc43a 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.
2023-02-03 09:42:49 +01:00
Aurelien DARRAGON
5e7ecbec99 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.
2023-02-02 17:10:11 +01:00
Aurelien DARRAGON
739281b3d6 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.
2023-02-02 08:21:15 +01:00
Amaury Denoyelle
78adb4b451 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 :
    56a86ddfb9
    MINOR: h3: add missing traces on closure

This must be backported up to 2.7 where above patch is scheduled.
2023-01-31 16:09:47 +01:00
Amaury Denoyelle
e31867b7fa 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 :
    87f8766d3f
    BUG/MEDIUM: h3: handle STOP_SENDING on control stream

This must be backported up to 2.6 along with the above patch.
2023-01-31 16:08:33 +01:00
William Lallemand
222e5a260b 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.
2023-01-31 14:34:40 +01:00
Amaury Denoyelle
56a86ddfb9 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.
2023-01-30 16:16:46 +01:00
Amaury Denoyelle
e269aeb46b 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.
2023-01-30 16:16:46 +01:00
Amaury Denoyelle
87f8766d3f 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
2023-01-30 16:12:23 +01:00
Amaury Denoyelle
1e340ba6bc 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.
2023-01-30 15:56:25 +01:00
Amaury Denoyelle
4be5435014 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.
2023-01-30 15:39:41 +01:00
Amaury Denoyelle
224ba5cffe 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.
2023-01-30 15:38:22 +01:00
Aurelien DARRAGON
8436c910f5 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)
2023-01-30 15:14:08 +01:00
Christopher Faulet
c254516c53 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.
2023-01-30 08:26:12 +01:00
Aurelien DARRAGON
df238c34c2 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.
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
d49a580fda 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.
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
f3a2ae7c63 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.
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
c07001bb56 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.
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
b2e2ec51b3 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.
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
d745a3f117 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.
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
9ded834adc 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.
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
b015b3eb14 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"
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
9a273b4069 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"
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
07d6753c89 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"
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
6fb58b8c9d 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"
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
5c6f86f465 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"
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
82faad1069 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.
2023-01-27 15:18:59 +01:00
Aurelien DARRAGON
f958341610 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.
2023-01-27 15:18:59 +01:00