18214 Commits

Author SHA1 Message Date
Willy Tarreau
a0c0f446af CLEANUP: pollers: remove dead code in the polling loop
As reported by Ilya and Coverity in issue #1858, since recent commit
eea152ee6 ("BUG/MINOR: signals/poller: ensure wakeup from signals")
which removed the test for the global signal flag from the pollers'
loop, the remaining "wake" flag doesn't need to be tested since it
already participates to zeroing the wait_time and will be caught
on the previous line.

Let's just remove that test now.

(cherry picked from commit af985e0151c7d12d9dac4fc364b5c50d3db1e1db)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Aurelien DARRAGON
8cbc474b39 BUG/MINOR: stats: fixing stat shows disabled frontend status as 'OPEN'
This patch adresses the issue #1626.

Adding support for PR_FL_PAUSED flag in the function stats_fill_fe_stats().
The command 'show stat' now properly reports a disabled frontend
using "PAUSED" state label.

This patch depends on the following commits:
  - 7d00077fd5 "BUG/MEDIUM: proxy: ensure pause_proxy()
  and resume_proxy() own PROXY_LOCK".
  - 001328873c "MINOR: listener: small API change"
  - d46f437de6 "MINOR: proxy/listener: support for additional PAUSED state"

It should be backported to 2.6, 2.5 and 2.4

(cherry picked from commit cddec0aef526f2dc64bad5a83ad788d60c12639c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Aurelien DARRAGON
1f9d9f53f0 MINOR: proxy/listener: support for additional PAUSED state
This patch is a prerequisite for #1626.
Adding PAUSED state to the list of available proxy states.
The flag is set when the proxy is paused at runtime (pause_listener()).
It is cleared when the proxy is resumed (resume_listener()).

It should be backported to 2.6, 2.5 and 2.4

(cherry picked from commit d46f437de69d5d4d84a207531a3ba6f8d3d697dc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Aurelien DARRAGON
614f99ee0a MINOR: listener: small API change
A minor API change was performed in listener(.c/.h) to restore consistency
between stop_listener() and (resume/pause)_listener() functions.

LISTENER_LOCK was never locked prior to calling stop_listener():
lli variable hint is thus not useful anymore.

Added PROXY_LOCK locking in (resume/pause)_listener() functions
with related lpx variable hint (prerequisite for #1626).

It should be backported to 2.6, 2.5 and 2.4

(cherry picked from commit 001328873c352e5e4b1df0dcc8facaf2fc1408aa)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Aurelien DARRAGON
3c263c09fa BUG/MEDIUM: proxy: ensure pause_proxy() and resume_proxy() own PROXY_LOCK
There was a race involving hlua_proxy_* functions
and some proxy management functions.

pause_proxy() and resume_proxy() can be used directly from lua code,
but that could lead to some race as lua code didn't make sure PROXY_LOCK
was owned before calling the proxy functions.

This patch makes sure it won't happen again elsewhere in the code
by locking PROXY_LOCK directly in resume and pause proxy functions
so that it's not the caller's responsibility anymore.
(based on stop_proxy() behavior that was already safe prior to the patch)

This should be backported to stable series.
Note that the API will likely differ < 2.4

(cherry picked from commit 7d00077fd5bd21e13aa976e6f3221cd44ae05eea)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Willy Tarreau
23cd90d9d2 DEV: flags: add missing CO_FL_FDLESS connection flag
This was added in 2.6 by commit c78a9698e ("MINOR: connection: add a new
flag CO_FL_FDLESS on fd-less connections") but forgotten in flags.c.
This must be backported to 2.6.

(cherry picked from commit 20273ceec0a6ec2645ee26c44dcc20c57ed0c92e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Willy Tarreau
f67f8eaafa DEV: flags: fix usage message to reflect available options
The proposed decoding options were not updated after the changes in 2.6,
let's fix that by taking the names from the existing declaration. This
should be backported to 2.6.

(cherry picked from commit c7ac17412b3478eed1725d73e8f51b8cc1118d93)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Ilya Shipitsin
2a87e4d4c4 CI: cirrus-ci: bump FreeBSD image to 13-1
we use FreeBSD binary packages that we rebuilt for FreeBSD-13.1

Newer FreeBSD version for package zstd:
To ignore this error set IGNORE_OSVERSION=yes
- package: 1301000
- running kernel: 1300139

(cherry picked from commit b0ab121da142b68d2a53311eff5f9b66dc62531f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Matthias Wirth
4a1f703e9a BUG/MINOR: signals/poller: ensure wakeup from signals
Add self-wake in signal_handler() to fix a race condition with a signal
coming in between checking signal_queue_len and entering polling sleep.

The changes in commit 43c891dda ("BUG/MINOR: signals/poller: set the
poller timeout to 0 when there are signals") were insufficient.

Move the signal_queue_len check from the poll implementations to
run_poll_loop() to keep that logic in one place.

The poll loops are terminated either by the parameter wake being set or
wake up due to a write to their poller_wr_pipe by wake_thread() in
signal_handler().

This fixes issue #1841.

Must be backported in every stable version.

(cherry picked from commit eea152ee68e82eae49ae188cd1b1fbbf63dc6913)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Frédéric Lécaille
ba627a66ce MINOR: h3: Send the h3 settings with others streams (requests)
This is the ->finalize application callback which prepares the unidirectional STREAM
frames for h3 settings and wakeup the mux I/O handler to send them. As haproxy is
at the same time always waiting for the client request, this makes haproxy
call sendto() to send only about 20 bytes of stream data. Furthermore in case
of heavy loss, this give less chances to short h3 requests to succeed.

Drawback: as at this time the mux sends its streams by their IDs ascending order
the stream 0 is always embedded before the unidirectional stream 3 for h3 settings.
Nevertheless, as these settings may be lost and received after other h3 request
streams, this is permitted by the RFC.

Perhaps there is a better way to do. This will have to be checked with Amaury.

Must be backported to 2.6.

(cherry picked from commit 3dd79d378c86b3ebf60e029f518add5f1ed54815)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Frédéric Lécaille
5b723503b9 MINOR: h3: Missing connection argument for a TRACE_LEAVE() argument
This should help in debbuging issues to be able to associate this trace to a
QUIC connection.

Must be backported to 2.6.

(cherry picked from commit befcf7031d79298ab68c0d19ba77fa991aa9f024)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Frédéric Lécaille
5bf1c9b808 MINOR: h3: Add the quic_conn object to h3 traces
This is very useful to associate h3 traces to a QUIC connection when debugging.

Must be backported to 2.6.

(cherry picked from commit 2eb5faa2ad734c2c65186da2533732163ead5d43)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Frédéric Lécaille
7ab54248c2 BUG/MINOR: h3: Crash when h3 trace verbosity is "minimal"
This was due to a missing check in h3_trace() about the first argument
presence (connection) and h3_parse_settings_frm() which calls TRACE_LEAVE()
without any argument. Then this argument was dereferenced.

Must be backported to 2.6

(cherry picked from commit 1c725aa9cd0e3799d7751381aabc9862bed10aff)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Frédéric Lécaille
7652bec1cf BUG/MINOR: quic: Trace fix about packet number space information.
<qc> variable was confused with <qel>. The consequence was that it was
always the same packet number space which was displayed: the first one (or
the Initial packet number space).

Must be backported to 2.6.

(cherry picked from commit 3c1b81fdd7c0e9822bbb8a14c4b665b7df53ecf5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Frédéric Lécaille
68436e33d1 BUG/MINOR: quic: Speed up the handshake completion only one time
It is possible to speed up the handshake completion but only one time
by connection as mentionned in RFC 9002 "6.2.3. Speeding up Handshake Completion".
Add a flag to prevent this process to be run several times
(see https://www.rfc-editor.org/rfc/rfc9002#name-speeding-up-handshake-compl).

Must be backported to 2.6.

(cherry picked from commit bb995eafc7e8e7d0457e1c3af17a98ef94d8b40b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
William Lallemand
96aaeb5fc3 BUG/MINOR: signals/poller: set the poller timeout to 0 when there are signals
When receiving a signal before entering the poller, and without any
activity in the process, the poller will be entered with a timeout
calculated without checking the signals.

Since commit 4f59d3 ("MINOR: time: increase the minimum wakeup interval
to 60s") the issue is much more visible because it could be stuck for
60s.

When in mworker mode, if a worker quits and the SIGCHLD signal deliver
at the right time to the master, this one could be stuck for the time of
the timeout.

This should fix issue #1841

Must be backported in every stable version.

(cherry picked from commit 43c891dda0c7c1c9f12dab5b77ac20b158a68adc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Willy Tarreau
dc5f81140f BUG/MINOR: stream/sched: take into account CPU profiling for the last call
When task profiling is enabled, the reported CPU time for short requests
and responses (e.g. redirect) is always zero in the logs, because
process_stream() is only called once and the CPU time is measured after
it returns. This is particuarly annoying when dealing with denies and in
general anything that deals with parasitic traffic because it can be
difficult to figure where the CPU is spent.

The solution taken in this patch consists in having process_stream()
update the cpu time itself before logging and quitting. It's very simple.
It will not take into account the time taken to produce the log nor
freeing the stream, but that's marginal compared to always logging zero.
The task's wake_date is also reset so that the scheduler doesn't have to
perform these operations again. This is dependent on the following patch:

   MINOR: sched: store the current profile entry in the thread context

It should be backported to 2.6 as it does help for troubleshooting.

(cherry picked from commit beee600491c15861a923113ee322c9f57aba07e5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Willy Tarreau
397fcc008d MINOR: sched: store the current profile entry in the thread context
The profile entry that corresponds to the current task/tasklet being
profiled is now stored into the thread's context. This will allow it
to be accessed from the tasks themselves. This is needed for an upcoming
fix.

(cherry picked from commit 1efddfa6bfdcaf57198866db67e49b40442d278f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Willy Tarreau
1cb273c718 BUG/MINOR: sched: properly account for the CPU time of dying tasks
When task profiling is enabled, the scheduler can measure and report
the cumulated time spent in each task and their respective latencies. But
this was wrong for tasks with few wakeups as well as for self-waking ones,
because the call date needed to measure how long it takes to process the
task is retrieved in the task itself (->wake_date was turned to the call
date), and we could face two conditions:
  - a new wakeup while the task is executing would reset the ->wake_date
    field before returning and make abnormally low values being reported;
    that was likely the case for taskèrun_applet for self-waking applets;

  - when the task dies, NULL is returned and the call date couldn't be
    retrieved, so that CPU time was not being accounted for. This was
    particularly visible with process_stream() which is usually called
    only twice per request, and whose time was systematically halved.

The cleanest solution here is to keep in mind that the scheduler already
uses quite a bit of local context in th_ctx, and place the intermediary
values there so that they cannot vanish. The wake_date has to be reset
immediately once read, and only its copy is used along the function. Note
that this must be done both for tasks and tasklet, and that until recently
tasklets were also able to report wrong values due to their sole dependency
on TH_FL_TASK_PROFILING between tests.

One nice benefit for future improvements is that such information will now
be available from the task without having to be stored into the task itself
anymore.

Since the tasklet part was computed on wrapping 32-bit arithmetics and
the task one was on 64-bit, the values were now consistently moved to
32-bit as it's already largely sufficient (4s spent in a task is more
than twice what the watchdog would tolerate). Some further cleanups might
be necessary, but the patch aimed at staying minimal.

Task profiling output after 1 million HTTP request previously looked like
this:

  Tasks activity:
    function                      calls   cpu_tot   cpu_avg   lat_tot   lat_avg
    h1_io_cb                    2012338   4.850s    2.410us   12.91s    6.417us
    process_stream              2000136   9.594s    4.796us   34.26s    17.13us
    sc_conn_io_cb               2000135   1.973s    986.0ns   30.24s    15.12us
    h1_timeout_task                 137      -         -      2.649ms   19.34us
    accept_queue_process             49   152.3us   3.107us   321.7yr   6.564yr
    main+0x146430                     7   5.250us   750.0ns   25.92us   3.702us
    srv_cleanup_idle_conns            1   559.0ns   559.0ns   918.0ns   918.0ns
    task_run_applet                   1      -         -      2.162us   2.162us

  Now it looks like this:
  Tasks activity:
    function                      calls   cpu_tot   cpu_avg   lat_tot   lat_avg
    h1_io_cb                    2014194   4.794s    2.380us   13.75s    6.826us
    process_stream              2000151   20.01s    10.00us   36.04s    18.02us
    sc_conn_io_cb               2000148   2.167s    1.083us   32.27s    16.13us
    h1_timeout_task                 198   54.24us   273.0ns   3.487ms   17.61us
    accept_queue_process             52   158.3us   3.044us   409.9us   7.882us
    main+0x1466e0                    18   16.77us   931.0ns   63.98us   3.554us
    srv_cleanup_toremove_conns        8   282.1us   35.26us   546.8us   68.35us
    srv_cleanup_idle_conns            3   149.2us   49.73us   8.131us   2.710us
    task_run_applet                   3   268.1us   89.38us   11.61us   3.871us

Note the two-fold difference on process_stream().

This feature is essentially used for debugging so it has extremely limited
impact. However it's used quite a bit more in bug reports and it would be
desirable that at least 2.6 gets this fix backported. It depends on at least
these two previous patches which will then also have to be backported:

     MINOR: task: permanently enable latency measurement on tasklets
     CLEANUP: task: rename ->call_date to ->wake_date

(cherry picked from commit 62b5b96bcc91985cb6bf6a30264ef3c54315c7c7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Christopher Faulet
43bd98151e BUG/MINOR: task: Fix detection of tasks profiling in tasklet_wakeup_after()
The regression was introduced when ad548b54a7 ["MINOR: task: Add
tasklet_wakeup_after()"] was backported to 2.6 (21e0c31695).
TH_FL_TASK_PROFILING flag does not exist. To detect if tasks profiling is
enabled, "task_profiling_mask" variable must be used.

It is a 2.6-specific issue. Thus there is no upstream commit ID. This patch
must be backported if the commit above is also backported. For now, no
backport is needed.
2022-09-12 17:54:22 +02:00
Willy Tarreau
41f645a05a CLEANUP: task: rename ->call_date to ->wake_date
This field is misnamed because its real and important content is the
date the task was woken up, not the date it was called. It temporarily
holds the call date during execution but this remains confusing. In
fact before the latency measurements were possible it was indeed a call
date. Thus is will now be called wake_date.

This change is necessary because a subsequent fix will require the
introduction of the real call date in the thread ctx.

(cherry picked from commit 04e50b3d325fa35ce9557701513773a8a84e9230)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Willy Tarreau
c432738cb0 MINOR: task: permanently enable latency measurement on tasklets
When tasklet latency measurement was enabled in 2.4 with commit b2285de04
("MINOR: tasks: also compute the tasklet latency when DEBUG_TASK is set"),
the feature was conditionned on DEBUG_TASK because the field would add 8
bytes to the struct tasklet.

This approach was not a very good idea because the struct ends on an int
anyway thus it does finish with a 32-bit hole regardless of the presence
of this field. What is true however is that adding it turned a 64-byte
struct to 72-byte when caller debugging is enabled.

This patch revisits this with a minor change. Now only the lowest 32
bits of the call date are stored, so they always fit in the remaining
hole, and this allows to remove the dependency on DEBUG_TASK. With
debugging off, we're now seeing a 48-byte struct, and with debugging
on it's exactly 64 bytes, thus still exactly one cache line. 32 bits
allow a latency of 4 seconds on a tasklet, which already indicates a
completely dead process, so there's no point storing the upper bits at
all. And even in the event it would happen once in a while, the lost
upper bits do not really add any value to the debug reports. Also, now
one tasklet wakeup every 4 billion will not be sampled due to the test
on the value itself. Similarly we just don't care, it's statistics and
the measurements are not 9-digit accurate anyway.

(cherry picked from commit 768c2c5678d462a3622492a1230946978292571e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Willy Tarreau
87f8732e1b BUG/MINOR: task: make task_instant_wakeup() work on a task not a tasklet
There's a subtle (harmless) bug in task_instant_wakeup(). As it uses
some tasklet code instead of some task code, the debug part also acts
on the tasklet equivalent, and the call_date is only set when DEBUG_TASK
is set instead of inconditionally like with tasks. As such, without this
debugging macro, call dates are not updated for tasks woken this way.

There isn't any impact yet because this function was introduced in 2.6 to
solve certain classes of issues and is not used yet, and in the worst case
it would only affect the reported latency time.

This may be backported to 2.6 in case a future fix would depend on it but
currently will not fix existing code.

(cherry picked from commit 0fae3a0360314285a17153cac76413184143ee74)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Willy Tarreau
c94b84c6ba BUG/MINOR: task: always reset a new tasklet's call date
The tasklet's call date was not reset, so if profiling was enabled while
some tasklets were in the run queue, their initial random value could be
used to preload a bogus initial latency value into the task profiling bin.
Let's just zero the initial value.

This should be backported to 2.4 as it was brought with initial commit
b2285de04 ("MINOR: tasks: also compute the tasklet latency when DEBUG_TASK
is set"). The impact is very low though.

(cherry picked from commit f27acd961e9b4291f80bc54100e57969ec4372ec)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Frédéric Lécaille
d404122c2f BUG/MINOR: quic: Wrong connection ID to thread ID association
To work, quic_pin_cid_to_tid() must set cid[0] to a value with <target_id>
as <global.nbthread> modulo. For each integer n, (n - (n % m)) + d has always
d as modulo m (with d < m).

So, this statement seemed correct:

     cid[0] = cid[0] - (cid[0] % global.nbthread) + target_tid;

except when n wraps or when another modulo is applied to the addition result.
Here, for 8bit modulo arithmetic, if m does not divides 256, this cannot
works for values which wraps when we increment them by d.
For instance n=255 m=3 and d=1 the formula result is 0 (should be d).

To fix this, we first limit c[0] to 255 - <target_id> to prevent c[0] from wrapping.

Thank you to @esb for having reported this issue in GH #1855.

Must be backported to 2.6

(cherry picked from commit 3122c75fd1f9a73a13ec533a4f313be0af1c5348)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Frédéric Lécaille
089ed1081c MINOR: quic: No TRACE_LEAVE() in retrieve_qc_conn_from_cid()
This macro was confused with TRACE_ENTER().

Should be backported to 2.6.

(cherry picked from commit 614742b79c63626cf477b4a85779db41223adbf9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Frédéric Lécaille
c29b565e08 MINOR: quic: Add traces about sent or resent TX frames
Very useful to help in debugging issues, especially during retransmissions.

Should be backported to 2.6

(cherry picked from commit 449804e27dba70949b8495f46ee8de5664a5ddd1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
William Lallemand
1c2991ec14 MINOR: quic: add QUIC support when no client_hello_cb
Add QUIC support to the ssl_sock_switchctx_cbk() variant used only when
no client_hello_cb is available.

This could be used with libreSSL implementation of QUIC for example.
It also works with quictls when HAVE_SSL_CLIENT_HELLO_CB is removed from
openss-compat.h

(cherry picked from commit 70a6e637b47d8e0ccf49dff8e2f3f4bb1a9c0b29)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
William Lallemand
069ad6acc3 BUILD: quic: fix the #ifdef in ssl_quic_initial_ctx()
As done on with ssl_sock_initial_ctx(), cleanup the ifdef for the
client_hello_cb and the no anti replay.

(cherry picked from commit 373ce73695541b9bdb9826a63a6a092cb2dbe779)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
William Lallemand
825468a159 BUILD: ssl: fix the ifdef mess in ssl_sock_initial_ctx
ssl_sock_initial_ctx uses the wrong #ifdef to check the availability of
the client_hello_cb.

Cleanup the #ifdef, add comments and indentation.

(cherry picked from commit 4b7938d1604ce5cd782693add21b461b634a8005)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
William Lallemand
927e42b419 BUILD: quic: enable early data only with >= openssl 1.1.1
Disable the early data in the QUIC code when not built with openssl >=
1.1.1.

LibreSSL 3.6.0 is impacted.

(cherry picked from commit e6ec626ac5b21041b997de350f29e385c479155d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
William Lallemand
22181db9ba BUILD: quic: temporarly ignore chacha20_poly1305 for libressl
LibreSSL does not implement EVP_chacha20_poly1305() with EVP_CIPHER but
uses the EVP_AEAD API instead:

https://man.openbsd.org/EVP_AEAD_CTX_init

This patch disables this cipher for libreSSL for now.

(cherry picked from commit d2be9d4c48b71b2132938dbfac36142cc7b8f7c4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
William Lallemand
d452b03784 BUILD: ssl: fix ssl_sock_switchtx_cbk when no client_hello_cb
When building HAProxy with USE_QUIC and libressl 3.6.0, the
ssl_sock_switchtx_cbk symbol is not found because libressl does not
implement the client_hello_cb.

A ssl_sock_switchtx_cbk version for the servername callback is available
but wasn't exported correctly.

(cherry picked from commit 844009d77ac42182ab4d5cf3efaaf227318505a1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
William Lallemand
8aa94c3778 BUILD: quic: add some ifdef around the SSL_ERROR_* for libressl
SSL_ERROR_WANT_ASYNC, SSL_ERROR_WANT_ASYNC_JOB and
SSL_ERROR_WANT_CLIENT_HELLO_CB does not seems supported by libressl.

(cherry picked from commit 6d74e179ee012c2b4eb282c2b63f87e9a6235251)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Frédéric Lécaille
30e75aaec9 BUG/MINOR: quic: Possible crash when verifying certificates
This verification is done by ssl_sock_bind_verifycbk() which is set at different
locations in the ssl_sock.c code . About QUIC connections, there are a lot of chances
the connection object is not initialized when entering this function. What must
be accessed is the SSL object to retrieve the connection or quic_conn objects,
then the bind_conf object of the listener. If the connection object is not found,
we try to find the quic_conn object.

Modify ssl_sock_dump_errors() interface which takes a connection object as parameter
to also passed a quic_conn object as parameter. Again this function try first
to access the connection object if not NULL or the quic_conn object if not.

There is a remaining thing to do for QUIC: store the certificate verification error
code as it is currently stored in the connection object. This error code is at least
used by the "bc_err" and "fc_err" sample fetches.

There are chances this bug is in relation with GH #1851. Thank you to @tasavis
for the report.

Must be merged into 2.6.

(cherry picked from commit 2be0ac55e10aec6ba3eaef4ccd7d6fe3fe10633c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Christopher Faulet
eff57f2476 BUG/MINOR: h1: Support headers case adjustment for TCP proxies
On frontend side, "h1-case-adjust-bogus-client" option is now supported in
TCP mode. It is important to be able to adjust the case of response headers
when a connection is routed to an HTTP backend. In this case, the client
connection is upgraded to H1.

On backend side, "h1-case-adjust-bogus-server" option is now also supported
in TCP mode to be able to perform HTTP health-checks with a case adjustment
of the request headers.

This patch should be backported as far as 2.0.

(cherry picked from commit a9e934bbd189b7d85af454f03ec1ade692154fd5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00
Frédéric Lécaille
ad231786c9 BUG/MINOR: quic: Possible crash with "tls-ticket-keys" on QUIC bind lines
ssl_tlsext_ticket_key_cb() is called when "tls-ticket-keys" option is used on a
"bind" line. It needs to have an access to the TLS ticket keys which have been
stored into the listener bind_conf struct. The fix consists in nitializing the
<ref> variable (references to TLS secret keys) the correct way when this callback
is called for a QUIC connection. The bind_conf struct is store into the quic_conn
object (QUIC connection).

This issue may be in relation with GH #1851. Thank you for @tasavis for the report.

Must be backported to 2.6.

(cherry picked from commit 6aec1f380e095cc36b279c4c9e1a955d01d41f6c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:21 +02:00
Frédéric Lécaille
ff836c4c86 BUG/MINOR: quic: Retransmitted frames marked as acknowledged
Obviously, frames which are duplicated from others must not be retransmitted if
the original frame they were duplicated from was already acknowledged.
This should have been detected by qc_build_frms() which skips such frames,
except if the QUIC xprt does really bad things which are not supported by
the upper layer. This will have to be checked with Amaury.

To prevent the retransmision of these frames which leads to crashes as reported by
hpn0t0ad this gdb backtrace in GH #1809 where the frame builder tries to copy a huge
number of bytes to the packet buffer:

Thread 7 (Thread 0x7fddf373a700 (LWP 13)):
 #0  __memmove_sse2_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:520
No locals.
 #1  0x000055b17435705e in quic_build_stream_frame (buf=0x7fddf372ef78, end=<optimized out>, frm=0x7fdde08d3470, conn=<optimized out>) at src/quic_frame.c:515
        to_copy = 18446697703428890384
        stream = 0x7fdde08d3490
        wrap = <optimized out>

which matches this part of quic_frame.c code:

    wrap = (const unsigned char *)b_wrap(stream->buf);
    if (stream->data + stream->len > wrap) {
        size_t to_copy = wrap - stream->data;
        memcpy(*buf, stream->data, to_copy);
        *buf += to_copy;

we release as soon as possible the impacted frames as there is really no need
to retransmit such frames.

Thank you to @hpn0t0ad for having provided us with useful traces in github
issue #1809.

Must be backported in 2.6.

(cherry picked from commit 025945f12cde56dde22baec286393fd1f048c0fc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:21 +02:00
Brad Smith
ff6c55923a BUILD: makefile: enable crypt(3) for NetBSD
Allow NetBSD to support encrypted passwords in Userlists.

(cherry picked from commit 2f105b8a457625491f06c40ddea8d43b1ed5c561)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:21 +02:00
Brad Smith
b5772ee721 MINOR: Revert part of clarifying samples support per os commit
Commit 5c83e3a1563cd7face299bf08037e51f976eb5e3 made some adjustments
to clarify which TCP_INFO information is supported by each respective
OS.

There was a comment like so..

Note that fc_rtt and fc_rttvar are supported on any OS that has TCP_INFO,
not just linux/freebsd/netbsd, so we continue to expose them unconditionally.

But the diff didn't do so in a consistent manner.

(cherry picked from commit ef9d59483995c1fcdc827b88429c831fa5064aed)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:21 +02:00
Willy Tarreau
dd9b366919 MEDIUM: peers: limit the number of updates sent at once
As seen in GH issue #1770, peers synchronization do not cope well with
very large buffers because by default the only two reasons for stopping
the processing of updates is either that the end was reached or that
the buffer is full. This can cause high latencies, and even rightfully
trigger the watchdog when the operations are numerous and slowed down
by competition on the stick-table lock.

This patch introduces a limit to the number of messages one may send
at once, which now defaults to 200, regardless of the buffer size. This
means taking and releasing the lock up to 400 times in a row, which is
costly enough to let some other parts work.

After some observation this could be backported to 2.6. If so, however,
previous commits "BUG/MEDIUM: applet: fix incorrect check for abnormal
return condition from handler" and "BUG/MINOR: applet: make the call_rate
only count the no-progress calls" must be backported otherwise the call
rate might trigger the looping protection.

(cherry picked from commit 8bd146d8af78371f97b66e50cac718666eb93388)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:21 +02:00
Willy Tarreau
987a4e248b [RELEASE] Released version 2.6.5
Released version 2.6.5 with the following main changes :
    - BUG/MINOR: quic: Wrong list_for_each_entry() use when building packets from qc_do_build_pkt()
    - BUG/MINOR: quic: Safer QUIC frame builders
    - BUILD: tcp_sample: fix build of get_tcp_info() on OpenBSD
    - BUG/MINOR: resolvers: return the correct value in resolvers_finalize_config()
    - BUG/MINOR: mworker: does not create the "default" resolvers in wait mode
    - BUG/MINOR: tcpcheck: Disable QUICKACK only if data should be sent after connect
    - REGTESTS: Fix prometheus script to perform HTTP health-checks
    - MINOR: resolvers: shut the warning when "default" resolvers is implicit
    - BUG/MINOR: quic: Leak in qc_release_lost_pkts() for non in flight TX packets
    - BUG/MINOR: quic: Stalled connections (missing I/O handler wakeup)
    - CLEANUP: quic: No more use ->rx_list MT_LIST entry point (quic_rx_packet)
    - CLEANUP: quic: Remove a useless check in qc_lstnr_pkt_rcv()
    - DOC: configuration: do-resolve doesn't work with a port in the string
    - MINOR: sample: add the host_only and port_only converters
    - BUG/MINOR: httpclient: fix resolution with port
    - DOC: configuration.txt: do-resolve must use host_only to remove its port.
    - BUG/MINOR: quic: Frames added to packets even if not built.
    - BUG/MEDIUM: spoe: Properly update streams waiting for a ACK in async mode
    - BUG/MEDIUM: peers: Add connect and server timeut to peers proxy
    - BUG/MEDIUM: peers: Don't use resync timer when local resync is in progress
    - BUG/MEDIUM: peers: Don't start resync on reload if local peer is not up-to-date
    - BUG/MINOR: hlua: Rely on CF_EOI to detect end of message in HTTP applets
    - MINOR: quic: Replace MT_LISTs by LISTs for RX packets.
    - BUG/MEDIUM: applet: fix incorrect check for abnormal return condition from handler
    - BUG/MINOR: applet: make the call_rate only count the no-progress calls
    - MINOR: quic: Add a trace to distinguish the datagram from the packets inside
    - BUG/MINOR: tcpcheck: Disable QUICKACK for default tcp-check (with no rule)
    - BUG/MINOR: ssl: fix deinit of the ca-file tree
    - BUG/MINOR: ssl: leak of ckch_inst_link in ckch_inst_free()
    - BUG/MEDIUM: ssl: Fix a UAF when old ckch instances are released
    - MINOR: quic: Revert recent QUIC commits
    - BUG/MINOR: ssl: revert two wrong fixes with ckhi_link
    - BUG/MEDIUM: mux-h1: do not refrain from signaling errors after end of input
    - BUG/MINOR: dev/udp: properly preset the rx address size
    - MINOR: connection: support HTTP/3.0 for smp_*_http_major fetch
    - CLEANUP: Re-apply xalloc_size.cocci (2)
    - MINOR: mux-quic: simplify decode_qcs API
    - MINOR: mux-quic/h3: adjust demuxing function return values
    - BUG/MINOR: h3: fix return value on decode_qcs on error
    - BUG/MINOR: h3: fix incorrect BUG_ON assert on SETTINGS parsing
    - BUG/MEDIUM: h3: fix SETTINGS parsing
    - MINOR: mux-quic: complete BUG_ON on TX flow-control enforcing
    - CLEANUP: quic: use task_new_on() for single-threaded tasks
    - MINOR: qpack: reduce dependencies on other modules
    - MINOR: qpack: add ABORT_NOW on unimplemented decoding
    - MINOR: qpack: improve decoding function
    - MINOR: quic: Add several nonce and key definitions for Retry tag
    - MINOR: quic: Parse long packet version from qc_parse_hd_form()
    - CLEANUP: quid: QUIC draft-28 no more supported
    - MEDIUM: quic: Add QUIC v2 draft support
    - MINOR: quic: Released QUIC TLS extension for QUIC v2 draft
    - MEDIUM: quic: Compatible version negotiation implementation (draft-08)
    - CLEANUP: quic: Remove any reference to boringssl
    - BUILD: quic: Wrong HKDF label constant variable initializations
    - BUG/MINOR: qpack: abort on dynamic index field line decoding
    - MINOR: quic: Dump version_information transport parameter
    - CLEANUP: pool/quic: remove suffix "_pool" from certain pool names
    - BUG/MINOR: qpack: fix build with QPACK_DEBUG
    - BUG/MINOR: qpack: abort on dynamic index field line decoding
    - CLEANUP: mux-quic: adjust comment on qcs_consume()
    - CLEANUP: mux-quic: do not export qc_get_ncbuf
    - REORG: mux-quic: reorganize flow-control fields
    - MINOR: mux-quic: implement accessor for sedesc
    - MEDIUM: mux-quic: refactor streams opening
    - MINOR: mux-quic: rename qcs flag FIN_RECV to SIZE_KNOWN
    - MINOR: mux-quic: emit FINAL_SIZE_ERROR on invalid STREAM size
    - REORG: mux-quic: rename stream initialization function
    - MINOR: mux-quic: rename stream purge function
    - MINOR: mux-quic: add traces on frame parsing functions
    - MINOR: mux-quic: implement qcs_alert()
    - MINOR: mux-quic: filter send/receive-only streams on frame parsing
    - MINOR: mux-quic: do not ack STREAM frames on unrecoverable error
    - MINOR: mux-quic: support stream opening via MAX_STREAM_DATA
    - MINOR: mux-quic: define basic stream states
    - MINOR: mux-quic: use stream states to mark as detached
    - MEDIUM: mux-quic: implement RESET_STREAM emission
    - MEDIUM: mux-quic: implement STOP_SENDING handling
    - CLEANUP: quic: clean up include on quic_frame-t.h
    - MINOR: quic: define a generic QUIC error type
    - MINOR: mux-quic: support app graceful shutdown
    - MINOR: mux-quic/h3: prepare CONNECTION_CLOSE on release
    - MEDIUM: quic: send CONNECTION_CLOSE on released MUX
    - CLEANUP: mux-quic: move qc_release()
    - MINOR: mux-quic: send one last time before release
    - MINOR: h3: store control stream in h3c
    - MINOR: h3: implement graceful shutdown with GOAWAY
    - MINOR: mux-quic: save proxy instance into qcc
    - MINOR: mux-quic: use timeout server for backend conns
    - MEDIUM: mux-quic: adjust timeout refresh
    - MINOR: mux-quic: count in-progress requests
    - MEDIUM: mux-quic: implement http-keep-alive timeout
    - MINOR: h3: support HTTP request framing state
    - MINOR: mux-quic: refresh timeout on frame decoding
    - MINOR: mux-quic: refactor refresh timeout function
    - MEDIUM: mux-quic: implement http-request timeout
    - MINOR: quic: Add two new stats counters for sendto() errors
    - BUG/MINOR: quic: adjust errno handling on sendto
    - MINOR: quic: Replace pool_zalloc() by pool_malloc() for fake datagrams
    - MINOR: quic: replace custom buf on Tx by default struct buffer
    - MINOR: quic: release Tx buffer on each send
    - MINOR: quic: refactor datagram commit in Tx buffer
    - MINOR: quic: skip sending if no frame to send in io-cb
    - BUG/MINOR: mux-quic: open stream on STOP_SENDING
    - BUG/MINOR: quic: fix crash on handshake io-cb for null next enc level
    - MEDIUM: quic: xprt traces rework
    - MINOR: quic: Remove useless lock for RX packets
    - CLEANUP: quic: Remove trailing spaces
    - MINOR: mux-quic: adjust enter/leave traces
    - MINOR: mux-quic: define protocol error traces
    - CLEANUP: mux-quic: adjust traces level
    - MINOR: mux-quic: define new traces
    - BUG/MEDIUM: mux-quic: fix crash due to invalid trace arg
    - BUG/MEDIUM: quic: Possible use of uninitialized <odcid> variable in qc_lstnr_params_init()
    - BUG/MEDIUM: quic: Wrong use of <token_odcid> in qc_lsntr_pkt_rcv()
    - BUG/MINOR: mux-quic: fix crash with traces in qc_detach()
    - BUG/MINOR: quic: MIssing check when building TX packets
    - BUG/MINOR: quic: Wrong status returned by qc_pkt_decrypt()
    - MINOR: quic: adjust quic_frame flag manipulation
    - MINOR: h3: report error on control stream close
    - MINOR: qpack: report error on enc/dec stream close
    - BUG/MEDIUM: mux-quic: reject uni stream ID exceeding flow control
    - MINOR: mux-quic: adjust traces on stream init
    - MINOR: mux-quic: add missing args on some traces
    - MINOR: quic: refactor application send
    - BUG/MINOR: quic: do not notify MUX on frame retransmit
    - BUG/MINOR: quic: Missing initializations for ducplicated frames.
    - BUG/MEDIUM: quic: fix crash on MUX send notification
    - REORG: h2: extract cookies concat function in http_htx
    - REGTESTS: add test for HTTP/2 cookies concatenation
    - MEDIUM: h3: concatenate multiple cookie headers
    - BUG/MINOR: quic: Possible crashes when dereferencing ->pkt quic_frame struct member
    - MINOR: quic: Add frame addresses to QUIC_EV_CONN_PRSAFRM event traces
    - BUG/MINOR: quic: Wrong splitted duplicated frames handling
    - MINOR: quic: Add the QUIC connection to mux traces
    - MINOR: quic: Trace fix in qc_release_frm()
    - MINOR: quic: Add reusable cipher contexts for header protection
    - BUG/MINOR: mux-quic: Fix memleak on QUIC stream buffer for unacknowledged data
    - BUG/MINOR: quix: Memleak for non in flight TX packets
    - BUG/MINOR: quic: Wrong list_for_each_entry() use when building packets from qc_do_build_pkt()
    - BUG/MINOR: quic: Safer QUIC frame builders
    - MINOR: quic: Replace MT_LISTs by LISTs for RX packets.
    - Revert "BUG/MINOR: quix: Memleak for non in flight TX packets"
    - BUG/MINOR: quic: Leak in qc_release_lost_pkts() for non in flight TX packets
    - BUG/MINOR: quic: Stalled connections (missing I/O handler wakeup)
    - CLEANUP: quic: No more use ->rx_list MT_LIST entry point (quic_rx_packet)
    - CLEANUP: quic: Remove a useless check in qc_lstnr_pkt_rcv()
    - MINOR: quic: Remove useless traces about references to TX packets
    - Revert "MINOR: quic: Remove useless traces about references to TX packets"
    - BUG/MINOR: quic: Null packet dereferencing from qc_dup_pkt_frms() trace
    - BUG/MINOR: quic: Frames added to packets even if not built.
    - BUG/MINOR: quic: Missing header protection AES cipher context initialisations (draft-v2)
    - MINOR: quic: Add a trace to distinguish the datagram from the packets inside
    - MINOR: quic: Move traces about RX/TX bytes from QUIC_EV_CONN_PRSAFRM event
    - BUG/MINOR: quic: TX frames memleak
    - BUG/MINOR: ssl: leak of ckch_inst_link in ckch_inst_free() v2
    - BUILD: ring: forward-declare struct appctx to avoid a build warning
    - MINOR: ring: support creating a ring from a linear area
    - MINOR: ring: add support for a backing-file
    - BUILD: sink: replace S_IRUSR, S_IWUSR with their octal value
    - MINOR: ring: archive a previous file-backed ring on startup
    - MINOR: sink/ring: rotate non-empty file-backed contents only
    - DEV: haring: add a simple utility to read file-backed rings
    - DEV: haring: support remapping LF in contents with CR VT
    - CLEANUP: exclude haring with .gitignore
    - BUILD: debug: make sure debug macros are never empty
    - BUG/MINOR: regex: Properly handle PCRE2 lib compiled without JIT support
    - REGTESTS: http_request_buffer: Add a barrier to not mix up log messages
    - BUG/MEDIUM: mux-h1: always use RST to kill idle connections in pools
    - MINOR: backend: always satisfy the first req reuse rule with l7 retries
    - BUG/MINOR: quic: Do not ack when probing
    - MINOR: quic: Add TX frames addresses to traces to several trace events
    - MINOR: quic: Trace typo fix in qc_release_frm()
    - BUG/MINOR: quic: Frames leak during retransmissions
    - BUG/MINOR: h2: properly set the direction flag on HTX response
    - BUG/MEDIUM: httpclient: always detach the caller before self-killing
    - BUG/MINOR: httpclient: keep-alive was accidentely disabled
    - BUG/MINOR: mux-h2: fix the "show fd" dest buffer for the subscriber
    - BUG/MINOR: mux-h1: fix the "show fd" dest buffer for the subscriber
    - BUG/MINOR: mux-fcgi: fix the "show fd" dest buffer for the subscriber
    - DEBUG: stream: minor rearrangement of a few fields in struct stream.
    - MINOR: debug: report applet pointer and handler in crashes when known
    - BUG/MINOR: http-act: initialize http fmt head earlier
2022-09-03 04:40:51 +02:00
Willy Tarreau
63e385ed8e BUG/MINOR: http-act: initialize http fmt head earlier
In github issue #1850, Christian Ruppert reported a case of crash in
2.6 when failing to parse some http rules. This started to happen
with 2.6 commit dd7e6c6 ("BUG/MINOR: http-rules: completely free
incorrect TCP rules on error") but has some of its roots in 2.2
commit 2eb539687 ("MINOR: http-rules: Add release functions for
existing HTTP actions").

The cause is that when the release function is set for HTTP actions,
the rule->arg.http.fmt list head is not yet initialized, hence is
NULL, thus the release function crashes when it tries to iterate over
it. In fact this code was initially not written with the perspective
of releasing such elements upon error, so the arg list initialization
happened after error checking.

This patch just moves the list initialization just after setting the
release pointer and that's OK.

This patch must be backported to 2.6 since the problem is visible
there. It could be backported to 2.5 but the issue is not triggered
there without the first mentioned patch above that landed in 2.6, so
it will not bring any obvious benefit.

(cherry picked from commit 6a03a0d86d203342060f7072ae9271458ee0349c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2022-09-02 19:28:30 +02:00
Willy Tarreau
c79d0e6b74 MINOR: debug: report applet pointer and handler in crashes when known
When an appctx is found looping over itself, we report a number of info
but not the pointers to the definition nor the handler, which can be quite
handy in some cases. Let's add them and try to decode the symbol.

(cherry picked from commit 714900a3c967eae7ff8d528239e7e9d02fac5df9)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2022-09-02 17:28:46 +02:00
Willy Tarreau
ddcaae84cf DEBUG: stream: minor rearrangement of a few fields in struct stream.
Some recent traces started to show confusing stream pointers ending with
0xe. The reason was that the stream's obj_type was almost unused in the
past and was stuffed in a hole in the structure. But now it's present in
all "show sess all" outputs and having to mentally match this value against
another one that's 0x17e lower is painful. The solution here is to move the
obj_type at the top, like in almost every other structure, but without
breaking the efficient layout.

This patch moves a few fields around and manages to both plug some holes
(16 bytes saved, 976 to 960) and avoid channels needlessly crossing cache
boundaries (res was spread over 3 lines vs 2 now).

Nothing else was changed. It would be desirable to backport this to 2.6
since it's where dumps are currently being processed the most.

(cherry picked from commit 178dda6b41caa7baef02ac4754b1c97c6dd481fb)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2022-09-02 17:27:55 +02:00
Willy Tarreau
b716db161d BUG/MINOR: mux-fcgi: fix the "show fd" dest buffer for the subscriber
Commit 1776ffb97 ("MINOR: mux-fcgi: make the "show fd" helper also decode
the fstrm subscriber when known") improved the output of "show fd" for the
FCGI mux, but the output is sent to the trash buffer instead of the msg
argument. It turns out that this has no effect right now as the caller
passes the trash but this is risky.

This should be backported to 2.4.

(cherry picked from commit 410546145b58adc035c357fb89163ced4fb84829)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2022-09-02 17:27:47 +02:00
Willy Tarreau
d6c011021e BUG/MINOR: mux-h1: fix the "show fd" dest buffer for the subscriber
Commit 150c4f8b7 ("MINOR: mux-h1: make the "show fd" helper also decode
the h1s subscriber when known") improved the output of "show fd" for the
H1 mux, but the output is sent to the trash buffer instead of the msg
argument. It turns out that this has no effect right now as the caller
passes the trash but this is risky.

This should be backported to 2.4.

(cherry picked from commit 9b6a187e26cd0b25429ab79737b84c41e939bbcc)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2022-09-02 17:27:42 +02:00
Willy Tarreau
671bfab6fd BUG/MINOR: mux-h2: fix the "show fd" dest buffer for the subscriber
Commit 98e40b981 ("MINOR: mux-h2: make the "show fd" helper also decode
the h2s subscriber when known") improved the output of "show fd" for the
H2 mux, but the output is sent to the trash buffer instead of the msg
argument. It turns out that this has no effect right now as the caller
passes the trash but this is risky.

This should be backported to 2.4.

(cherry picked from commit ba7657ca0f2a15f9bb988bc05821d088f2f05947)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2022-09-02 17:27:24 +02:00
Willy Tarreau
ad32323591 BUG/MINOR: httpclient: keep-alive was accidentely disabled
The servers were not set with default settings, meaning that a few
settings including the pool_max_delay were not set, thus disabling
connection pools, which is the cause of the fact that keep-alive was
disabled as reported in issue #1831. There might possibly be other
issues pending since all these fields were left to zero.

Note that this patch alone will not fix keep-alive because the applet
does not enforce SE_FL_NOT_FIRST and relies on the default http-reuse
safe, thus if servers are not shared, all requests are considered
first ones and do not reuse existing connections.

In 2.7, commit ecb40b2c3 ("MINOR: backend: always satisfy the first
req reuse rule with l7 retries") addressed this in a more elegant way
by fixing http-reuse to take into account the fact that properly
configured l7 retries provide exactly the capability that reuse safe
was trying to cover, and this patch is suitable for backporting.

This patch should be backported to 2.6 only.

(cherry picked from commit f80713ba8eb4f5397134155330b9c6eb064eb7f7)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2022-09-02 17:27:12 +02:00
Willy Tarreau
19e72e569f BUG/MEDIUM: httpclient: always detach the caller before self-killing
If the caller dies before the server responds, the httpclient can crash
in hc_cli_res_end_cb() when unregistering because it dereferences
hc->caller which was already freed during the caller's unregistration.
The easiest way to reproduce it is by sending twice the following
request on the same CLI connection in expert mode, with httpterm
running on local port 8000:

   httpclient GET http://127.0.0.1:8000/?t=600

Note the 600ms delay that's larger than socat's default 500.

The code checks for a NULL everywhere hc->caller is used, but the NULL
was forgotten in this specific case. It must be placed in the second
half of httpclient_stop_and_destroy() which is responsible for signaling
the client that the caller leaves.

This must be backported to 2.6.

(cherry picked from commit b48292068bee8a54ed33a9e809c56ddcc566396c)
Signed-off-by: Willy Tarreau <w@1wt.eu>
2022-09-02 17:26:54 +02:00