18266 Commits

Author SHA1 Message Date
Willy Tarreau
c83e5448d3 BUILD: h1: silence an initiialized warning with gcc-4.7 and -Os
Building h1.c with gcc-4.7 -Os produces the following warning:

  src/h1.c: In function 'h1_headers_to_hdr_list':
  src/h1.c:1101:36: warning: 'ptr' may be used uninitialized in this function [-Wmaybe-uninitialized]

In fact ptr may be taken from sl.rq.u.ptr which is only initialized after
passing through the relevant states, but gcc doesn't know which states
are visited. Adding an ALREADY_CHECKED() statement there is sufficient to
shut it up and doesn't affect the emitted code.

This may be backported to stable versions to make sure that builds on older
distros and systems is clean.

(cherry picked from commit 55d2e8577e5ae0e7ac0cdbf5201b4836b5d7ad3c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
Olivier Houchard
2075777c35 BUG/MEDIUM: lua: handle stick table implicit arguments right.
In hlua_lua2arg_check(), we allow for the first argument to not be
provided, if it has a type we know, this is true for frontend, backend,
and stick table. However, the stick table code was changed. It used
to be deduced from the proxy, but it is now directly provided in struct
args. So setting the proxy there no longer work, and we have to
explicitely set the stick table.
Not doing so will lead the code do use the proxy pointer as a stick
table pointer, which will likely cause crashes.

This should be backported up to 2.0.

(cherry picked from commit 14f62688839dc7245ca87e040f14fbd2147698e6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
Olivier Houchard
bb0cb68fe6 BUG/MEDIUM: lua: Don't crash in hlua_lua2arg_check on failure
In hlua_lua2arg_check(), on failure, before calling free_argp(), make
sure to always mark the failed argument as ARGT_STOP. We only want to
free argument prior to that point, because we did not allocate the
strings after this one, and so we don't want to free them.

This should be backported up to 2.2.

(cherry picked from commit ca43161a8da278ec0948511f595827daf29a071e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
Amaury Denoyelle
114974dbc4 BUG/MINOR: mux-quic: ignore STOP_SENDING for locally closed stream
It is possible to receive a STOP_SENDING frame for a locally closed
stream. This was not properly managed as this would result in a BUG_ON()
crash from qcs_idle_open() call under qcc_recv_stop_sending().

Now, STOP_SENDING frames are ignored when received on streams already
locally closed. This has two consequences depending on the reason of
closure :

* if a RESET_STREAM was already emitted and closed the stream, this
  patch prevents to emit a new RESET_STREAM. This behavior is thus
  better.

* if stream was closed due to all data transmitted, no RESET_STREAM will
  be built. This is contrary to the RFC 9000 which advice to transmit
  it, even on "Data Sent" state. However, this is not mandatory so the
  new behavior is acceptable, even if it could be improved.

This crash has been detected on haproxy.org. This can be artifically
reproduced by adding the following snippet at the end of qc_send_mux()
when doing a request with a small payload response :
  qcc_recv_stop_sending(qc->qcc, 0, 0);

This must be backported up to 2.6.

(cherry picked from commit d7755375a51726e3d13c4f891ca8ab8cc1ba9a4d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
Amaury Denoyelle
4a6be622b2 CLEANUP: quic: create a dedicated quic_conn module
xprt_quic module was too large and did not reflect the true architecture
by contrast to the other protocols in haproxy.

Extract code related to XPRT layer and keep it under xprt_quic module.
This code should only contains a simple API to communicate between QUIC
lower layer and connection/MUX.

The vast majority of the code has been moved into a new module named
quic_conn. This module is responsible to the implementation of QUIC
lower layer. Conceptually, it overlaps with TCP kernel implementation
when comparing QUIC and HTTP1/2 stacks of haproxy.

This should be backported up to 2.6.

(cherry picked from commit 92fa63f73596bf7e567b7bbd600dd8621a1b49ad)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
Amaury Denoyelle
228883ca32 CLEANUP: quic: remove duplicated varint code from xprt_quic.h
There was some identical code between xprt_quic and quic_enc modules.
This concerns helper on QUIC varint type. Keep only the version in
quic_enc file : this should help to reduce dependency on xprt_quic
module.

Note that quic_max_int_by_size() has been removed and is replaced by the
identical quic_max_int().

This should be backported up to 2.6.

(cherry picked from commit a2639383ece04c2fee3bbdda54dab66a640f6aa1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
Amaury Denoyelle
6bff24ec61 CLEANUP: quic: remove unused function prototype
Removed hexdump unusued prototype from quic_tls.c.

This should be backported up to 2.6.

(cherry picked from commit ac9bf016bfcb5608b4d29d029f20a45709ac0a2d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
Amaury Denoyelle
69baa24752 CLEANUP: quic: fix headers
Clean up quic sources by adjusting headers list included depending
on the actual dependency of each source file.

On some occasion, xprt_quic.h was removed from included list. This is
useful to help reducing the dependency on this single file and cleaning
up QUIC haproxy architecture.

This should be backported up to 2.6.

(cherry picked from commit 5c25dc5bfd5d253925f954aab072a2bf1fd1d6e2)
[cf: Include <haproxy/global.h> from cfgparse-quic.c instead of only
     <haproxy/global-t.h">. On 2.7, it is shipped with "tools.h" (tools.h >
     cli.h > global.h). But not on the 2.6]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:41:20 +02:00
Amaury Denoyelle
90a008239e BUG/MINOR: quic: adjust quic_tls prototypes
Two prototypes in quic_tls module were not identical to the actual
function definition.

* quic_tls_decrypt2() : the second argument const attribute is not
  present, to be able to use it with EVP_CIPHER_CTX_ctlr(). As a
  consequence of this change, token field of quic_rx_packet is now
  declared as non-const.

* quic_tls_generate_retry_integrity_tag() : the second argument type
  differ between the two. Adjust this by fixing it to as unsigned char
  to match EVP_EncryptUpdate() SSL function.

This situation did not seem to have any visible effect. However, this is
clearly an undefined behavior and should be treated as a bug.

This should be backported up to 2.6.

(cherry picked from commit f3c40f83fbfc6fb60ba5608ccfbd00fb51e6f9b3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 07:43:42 +02:00
Amaury Denoyelle
adf910e519 CLEANUP: quic: remove global var definition in quic_tls header
Some variables related to QUIC TLS were defined in a header file : their
definitions are now moved properly in the implementation file, with only
declarations in the header.

This should be backported up to 2.6.

(cherry picked from commit a19bb6f0b2af1971775e4a88edfaed85d42162c6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 07:43:39 +02:00
Amaury Denoyelle
e53c5615da CLEANUP: mux-quic: remove usage of non-standard ull type
ull is a typedef to unsigned long long. It is only defined in
xprt_quic-t.h. Its usage should be limited over time to reduce xprt_quic
dependency over the whole code. It can be replaced by ullong typedef
from compat.h.

For the moment, ull references have been replaced in qmux_trace module.
They were only used for printf format and has been replaced by the true
variable type.

This change is useful to reduce dependencies on xprt_quic in other
files.

This should be backported up to 2.6.

(cherry picked from commit d6922d5471e0486958b8f979246d1ac3b37b222d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 07:43:35 +02:00
Christopher Faulet
e8da4211a9 DOC: config: Fix pgsql-check documentation to make user param mandatory
The username is required in the Start-up message. Thus, since the 2.2, when
this health-check was refactored, the user parameter is mandatory. On prior
versions, when no username is provided, no pgsql check is performed but only
a basic tcpcheck.

This patch should be backported as far as 2.2.

(cherry picked from commit 59307b3e4e1fbdbe07dcbe08781c81e6312f0c23)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 07:43:24 +02:00
Fatih Acar
3b26843205 BUG/MINOR: checks: update pgsql regex on auth packet
This patch adds support to the following authentication methods:

- AUTH_REQ_GSS (7)
- AUTH_REQ_SSPI (9)
- AUTH_REQ_SASL (10)

Note that since AUTH_REQ_SASL allows multiple authentication mechanisms
such as SCRAM-SHA-256 or SCRAM-SHA-256-PLUS, the auth payload length may
vary since the method is sent in plaintext. In order to allow this, the
regex now matches any payload length.

This partially fixes Github issue #1508 since user authentication is
still broken but should restore pre-2.2 behavior.

This should be backported up to 2.2.

Signed-off-by: Fatih Acar <facar@scaleway.com>
(cherry picked from commit 0d6fb7a3eb0a9754348ec15be14a017a1c84df0f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 07:43:19 +02:00
Willy Tarreau
22beb2ad21 BUG/MINOR: backend: only enforce turn-around state when not redispatching
In github issue #1878, Bart Butler reported observing turn-around states
(1 second pause) after connection retries going to different servers,
while this ought not happen.

In fact it does happen because back_handle_st_cer() enforces the TAR
state for any algo that's not round-robin. This means that even leastconn
has it, as well as hashes after the number of servers changed.

Prior to doing that, the call to stream_choose_redispatch() has already
had a chance to perform the correct choice and to check the algo and
the number of retries left. So instead we should just let that function
deal with the algo when needed (and focus on deterministic ones), and
let the former just obey. Bart confirmed that the fixed version works
as expected (no more delays during retries).

This may be backported to older releases, though it doesn't seem very
important. At least Bart would like to have it in 2.4 so let's go there
for now after it has cooked a few weeks in 2.6.

(cherry picked from commit 406efb96d135efe1d5a85bf58c589f7b6dbd8c70)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 07:43:13 +02:00
Willy Tarreau
c564abd107 BUG/MAJOR: conn-idle: fix hash indexing issues on idle conns
Idle connections do not work on 32-bit machines due to an alignment issue
causing the connection nodes to be indexed with their lower 32-bits set to
zero and the higher 32 ones containing the 32 lower bitss of the hash. The
cause is the use of ebmb_node with an aligned data, as on this platform
ebmb_node is only 32-bit aligned, leaving a hole before the following hash
which is a uint64_t:

  $ pahole -C conn_hash_node ./haproxy
  struct conn_hash_node {
        struct ebmb_node           node;                 /*     0    20 */

        /* XXX 4 bytes hole, try to pack */

        int64_t                    hash;                 /*    24     8 */
        struct connection *        conn;                 /*    32     4 */

        /* size: 40, cachelines: 1, members: 3 */
        /* sum members: 32, holes: 1, sum holes: 4 */
        /* padding: 4 */
        /* last cacheline: 40 bytes */
  };

Instead, eb64 nodes should be used when it comes to simply storing a
64-bit key, and that is what this patch does.

For backports, a variant consisting in simply marking the "hash" member
with a "packed" attribute on the struct also does the job (tested), and
might be preferable if the fix is difficult to adapt. Only 2.6 and 2.5
are affected by this.

(cherry picked from commit 852234848241f61a976f8856123a34a3c19275ba)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 07:40:32 +02:00
William Lallemand
7fe5c4d5d2 DOC: management: httpclient can resolve server names in URLs
The httpclient does support DNS resolution since 2.6.

Must be backported to 2.6.

(cherry picked from commit 9ae05bb1e082577858e9f51e04f8ef0c7cb25383)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 07:38:49 +02:00
Aurelien DARRAGON
430b18d027 BUG/MINOR: hlua: _hlua_http_msg_delete incorrect behavior when offset is used
Calling the function with an offset when "offset + len" was superior or equal
to the targeted blk length caused 'v' value to be improperly set.
And because 'v' is directly provided to htx_replace_blk_value(), blk consistency was compromised.
(It seems that blk was overrunning in htx_replace_blk_value() due to
this and header data was overwritten in this case).

This patch adds the missing checks to make the function behave as
expected when offset is set and offset+len is greater or equals to the targeted blk length.
Some comments were added to the function as well.

It may be backported to 2.6 and 2.5

(cherry picked from commit bcbcf98e0c0149f91365f1f2d207be018124f6f2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 07:38:40 +02:00
Aurelien DARRAGON
1745882224 BUG/MINOR: hlua: fixing hlua_http_msg_insert_data behavior
hlua_http_msg_insert_data() function is called upon
HTTPMessage.insert() method from lua script.

This function did not work properly for multiple reasons:

  - An incorrect argument check was performed and prevented the user
  from providing optional offset argument.

  - Input and output variables were inverted
  and offset was not handled properly. The same bug
  was also fixed in hlua_http_msg_del_data(), see:
  'BUG/MINOR: hlua: fixing hlua_http_msg_del_data behavior'

The function now behaves as described in the documentation.

This could be backported to 2.6 and 2.5.

(cherry picked from commit 7fdba0ae544cb1b14b7d3864b06f38c752505094)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 07:38:12 +02:00
Aurelien DARRAGON
4308695c6a BUG/MINOR: hlua: fixing hlua_http_msg_del_data behavior
GH issue #1885 reported that HTTPMessage.remove() did not
work as expected.

It turns out that underlying hlua_http_msg_del_data() function
was not working properly due to input / output inversion as well
as incorrect user offset handling.

This patch fixes it so that the behavior is the one described in
the documentation.

This could be backported to 2.6 and 2.5.

(cherry picked from commit d7c71b03d83144913a33a09080c3738b27395af8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 07:38:08 +02:00
Christopher Faulet
4d4abc49b3 BUG/MEDIUM: resolvers: Remove aborted resolutions from query_ids tree
To avoid any UAF when a resolution is released, a mechanism was added to
abort a resolution and delayed the released at the end of the current
execution path. This mechanism depends on an hard assumption: Any reference
on an aborted resolution must be removed. So, when a resolution is aborted,
it is removed from the resolver lists and inserted into a death row list.

However, a resolution may still be referenced in the query_ids tree. It is
the tree containing all resolutions with a pending request. Because aborted
resolutions are released outside the resolvers lock, it is possible to
release a resolution on a side while a query ansswer is received and
processed on another one. Thus, it is still possible to have a UAF because
of this bug.

To fix the issue, when a resolution is aborted, it is removed from any list,
but it is also removed from the query_ids tree.

This patch should solve the issue #1862 and may be related to #1875. It must
be backported as far as 2.2.

(cherry picked from commit eaabf060312f6aad1c0c195ad33e5ea612acc47a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-28 10:52:57 +02:00
Christopher Faulet
b93ce2eec3 BUG/MEDIUM: stconn: Reset SE descriptor when we fail to create a stream
If stream_new() fails after the frontend SC is attached, the underlying SE
descriptor is not properly reset. Among other things, SE_FL_ORPHAN flag is
not set again. Because of this error, a BUG_ON() is triggered when the mux
stream on the frontend side is destroyed.

Thus, now, when stream_new() fails, SE_FL_ORPHAN flag is set on the SE
descriptor and its stream-connector is set to NULL.

This patch should solve the issue #1880. It must be backported to 2.6.

(cherry picked from commit 3ab72c66a01ca81aa93cf1f0bd29430db8271792)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-28 10:52:50 +02:00
Christopher Faulet
b3dc43ddad BUG/MINOR: stream: Perform errors handling in right order in stream_new()
The frontend SC is attached before the backend one is allocated. Thus an
allocation error on backend SC must be handled before an error on the
frontend SC.

This patch must be backported to 2.6.

(cherry picked from commit 4cfc038cb19996f5d2fe60284fdb556503a5f9ef)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-28 10:52:43 +02:00
Thierry Fournier
e89dbc4ec6 BUG/MINOR: hlua: Remove \n in Lua error message built with memprintf
Because memprintf return an error to the caller and not on screen.
the function which perform display of message on the right output
is in charge of adding \n if it is necessary.

This patch may be backported.

(cherry picked from commit 70e38e91b41c3446cd6d7993dd71fe6100996fa3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-28 10:51:26 +02:00
Christopher Faulet
522bc42bdb REGTESTS: 4be_1srv_smtpchk_httpchk_layer47errors: Return valid SMTP replies
The s1 server is acting like a SMTP server. But it sends two CRLF at the end of
each line, while only one CRLF must be returned. It only works becaue both CRLF
are received at the same time.

(cherry picked from commit 330af2d7ed2d8d72800fc7761253ff6965681742)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-28 10:50:05 +02:00
Christopher Faulet
274d1a4dff [RELEASE] Released version 2.6.6
Released version 2.6.6 with the following main changes :
    - MEDIUM: peers: limit the number of updates sent at once
    - MINOR: Revert part of clarifying samples support per os commit
    - BUILD: makefile: enable crypt(3) for NetBSD
    - BUG/MINOR: quic: Retransmitted frames marked as acknowledged
    - BUG/MINOR: quic: Possible crash with "tls-ticket-keys" on QUIC bind lines
    - BUG/MINOR: h1: Support headers case adjustment for TCP proxies
    - BUG/MINOR: quic: Possible crash when verifying certificates
    - BUILD: quic: add some ifdef around the SSL_ERROR_* for libressl
    - BUILD: ssl: fix ssl_sock_switchtx_cbk when no client_hello_cb
    - BUILD: quic: temporarly ignore chacha20_poly1305 for libressl
    - BUILD: quic: enable early data only with >= openssl 1.1.1
    - BUILD: ssl: fix the ifdef mess in ssl_sock_initial_ctx
    - BUILD: quic: fix the #ifdef in ssl_quic_initial_ctx()
    - MINOR: quic: add QUIC support when no client_hello_cb
    - MINOR: quic: Add traces about sent or resent TX frames
    - MINOR: quic: No TRACE_LEAVE() in retrieve_qc_conn_from_cid()
    - BUG/MINOR: quic: Wrong connection ID to thread ID association
    - BUG/MINOR: task: always reset a new tasklet's call date
    - BUG/MINOR: task: make task_instant_wakeup() work on a task not a tasklet
    - MINOR: task: permanently enable latency measurement on tasklets
    - CLEANUP: task: rename ->call_date to ->wake_date
    - BUG/MINOR: task: Fix detection of tasks profiling in tasklet_wakeup_after()
    - BUG/MINOR: sched: properly account for the CPU time of dying tasks
    - MINOR: sched: store the current profile entry in the thread context
    - BUG/MINOR: stream/sched: take into account CPU profiling for the last call
    - BUG/MINOR: signals/poller: set the poller timeout to 0 when there are signals
    - BUG/MINOR: quic: Speed up the handshake completion only one time
    - BUG/MINOR: quic: Trace fix about packet number space information.
    - BUG/MINOR: h3: Crash when h3 trace verbosity is "minimal"
    - MINOR: h3: Add the quic_conn object to h3 traces
    - MINOR: h3: Missing connection argument for a TRACE_LEAVE() argument
    - MINOR: h3: Send the h3 settings with others streams (requests)
    - BUG/MINOR: signals/poller: ensure wakeup from signals
    - CI: cirrus-ci: bump FreeBSD image to 13-1
    - DEV: flags: fix usage message to reflect available options
    - DEV: flags: add missing CO_FL_FDLESS connection flag
    - BUG/MEDIUM: proxy: ensure pause_proxy() and resume_proxy() own PROXY_LOCK
    - MINOR: listener: small API change
    - MINOR: proxy/listener: support for additional PAUSED state
    - BUG/MINOR: stats: fixing stat shows disabled frontend status as 'OPEN'
    - CLEANUP: pollers: remove dead code in the polling loop
    - BUG/MINOR: mux-h1: Increment open_streams counter when H1 stream is created
    - REGTESTS: healthcheckmail: Relax matching on the healthcheck log message
    - CLEANUP: listener: function comment typo in stop_listener()
    - BUG/MINOR: listener: null pointer dereference suspected by coverity
    - REGTESTS: log: test the log-forward feature
    - BUG/MEDIUM: sink: bad init sequence on tcp sink from a ring.
    - REGTESTS: ssl/log: test the log-forward with SSL
    - DOC: fix TOC in starter guide for subsection 3.3.8. Statistics
    - MEDIUM: quic: separate path for rx and tx with set_encryption_secrets
    - BUG/MEDIUM: mux-quic: fix crash on early app-ops release
    - CLEANUP: mux-quic: remove stconn usage in h3/hq
    - BUG/MINOR: mux-quic: do not remotely close stream too early
    - BUG/MEDIUM: server: segv when adding server with hostname from CLI
    - CLEANUP: quic,ssl: fix tiny typos in C comments
    - BUG/MEDIUM: captures: free() an error capture out of the proxy lock
    - BUILD: fd: fix a build warning on the DWCAS
    - SCRIPTS: announce-release: update some URLs to https
    - BUG/MEDIUM: mux-quic: fix nb_hreq decrement
    - BUG/MINOR: mux-quic: do not keep detached qcs with empty Tx buffers
    - REORG: mux-quic: extract traces in a dedicated source file
    - REORG: mux-quic: export HTTP related function in a dedicated file
    - MINOR: mux-quic: refactor snd_buf
    - BUG/MEDIUM: mux-quic: properly trim HTX buffer on snd_buf reset
    - REGTESTS: ssl: adopt tests to OpenSSL-3.0.N
    - REGTESTS: ssl: adopt tests to OpenSSL-3.0.N
    - REGTESTS: ssl: fix grep invocation to use extended regex in ssl_generate_certificate.vtc
    - BUG/MINOR: log: improper behavior when escaping log data
2022-09-22 14:31:07 +02:00
Aurelien DARRAGON
9779c1b5d7 BUG/MINOR: log: improper behavior when escaping log data
Patrick Hemmer reported an improper log behavior when using
log-format to escape log data (+E option):
Some bytes were truncated from the output:

- escape_string() function now takes an extra parameter that
  allow the caller to specify input string stop pointer in
  case the input string is not guaranteed to be zero-terminated.
- Minors checks were added into lf_text_len() to make sure dst
  string will not overflow.
- lf_text_len() now makes proper use of escape_string() function.

This should be backported as far as 1.8.

(cherry picked from commit c5bff8e550cf49b0cb3a7abb998b2c915323eca9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-20 16:31:39 +02:00
Ilya Shipitsin
26ac4503fc REGTESTS: ssl: fix grep invocation to use extended regex in ssl_generate_certificate.vtc
in 2f2a2884b7464ccb56469cb94d8a1ae4015a8cb6 grep should have use regex flag -E, but flag
was lost by mistake

(cherry picked from commit b6189bc268255b799a77fdffcaaa7b221ca2d7a9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-20 16:08:50 +02:00
Ilya Shipitsin
a4ea870b46 REGTESTS: ssl: adopt tests to OpenSSL-3.0.N
on Ubuntu-22.04 openssl-3.0.5 is shipped which has changed ec curve
description to "Server Temp Key: ECDH, secp384r1, 384 bits"

(cherry picked from commit 2f2a2884b7464ccb56469cb94d8a1ae4015a8cb6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-20 16:08:50 +02:00
Ilya Shipitsin
b231c87e1a REGTESTS: ssl: adopt tests to OpenSSL-3.0.N
on Ubuntu-22.04 openssl-3.0.5 is shipped which has changed ec curve
description to "Server Temp Key: ECDH, prime256v1, 256 bits"

(cherry picked from commit 0865160b93b1ac8326ac0e5b57be24504070c2c1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-20 16:08:50 +02:00
Amaury Denoyelle
28e18246be BUG/MEDIUM: mux-quic: properly trim HTX buffer on snd_buf reset
MUX QUIC snd_buf operation whill return early if a qcs instance is
resetted. In this case, HTX is left untouched and the callback returns
the whole bufer size. This lead to an undefined behavior as the stream
layer is notified about a transfer but does not see its HTX buffer
emptied. In the end, the transfer may stall which will lead to a leak on
session.

To fix this, HTX buffer is now resetted when snd_buf is short-circuited.
This should fix the issue as now the stream layer can continue the
transfer until its completion.

This patch has already been tested by Tristan and is reported to solve
the github issue #1801.

This should be backported up to 2.6.

(cherry picked from commit 0ed617ac2ff377ce60bd9c8fd97fd9da32d43971)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-20 15:58:22 +02:00
Amaury Denoyelle
53e4116b6b MINOR: mux-quic: refactor snd_buf
Factorize common code between h3 and hq-interop snd_buf operation. This
is inserted in MUX QUIC snd_buf own callback.

The h3/hq-interop API has been adjusted to directly receive a HTX
message instead of a plain buf. This led to extracting part of MUX QUIC
snd_buf in qmux_http module.

This should be backported up to 2.6.

(cherry picked from commit 9534e59bb9057cfa5762f9c119579a67f705de37)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-20 15:58:18 +02:00
Amaury Denoyelle
0859fbf203 REORG: mux-quic: export HTTP related function in a dedicated file
Extract function dealing with HTX outside of MUX QUIC. For the moment,
only rcv_buf stream operation is concerned.

The main objective is to be able to support both TCP and HTTP proxy mode
with a common base and add specialized modules on top of it.

This should be backported up to 2.6.

(cherry picked from commit d80fbcaca266696a7d6de7342876d104c42e91e9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-20 15:58:14 +02:00
Amaury Denoyelle
f3cab28f96 REORG: mux-quic: extract traces in a dedicated source file
QUIC MUX implements several APIs to interface with stream, quic-conn and
app-ops layers. It is planified to better separate this roles, possibly
by using several files.

The first step is to extract QUIC MUX traces in a dedicated source
files. This will allow to reuse traces in multiple files.

The main objective is to be
able to support both TCP and HTTP proxy mode with a common base and add
specialized modules on top of it.

This should be backported up to 2.6.

(cherry picked from commit 36d50bff22563ba650918ccedaa695fcb6b8fa3e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-20 15:58:06 +02:00
Amaury Denoyelle
98afc4ec7d BUG/MINOR: mux-quic: do not keep detached qcs with empty Tx buffers
A qcs instance free may be postponed in stream detach operation if the
stream is not locally closed. This condition is there to achieve
transfering data still present in Tx buffer. Once all data have been
emitted to quic-conn layer, qcs instance can be released.

However, the stream is only closed locally if HTX EOM has been seen or
it has been resetted. In case the transfer finished without EOM, a
detached qcs won't be freed even if there is no more activity on it.

This bug was not reproduced but was found on code analysis. Its precise
impact is unknown but it should not cause any leak as all qcs instances
are freed with its parent qcc connection : this should eventually happen
on MUX timeout or QUIC idle timeout.

To adjust this, condition to mark a stream as locally closed has been
extended. On qcc_streams_sent_done() notification, if its Tx buffer has
been fully transmitted, it will be closed if either FIN STREAM was set
or the stream is detached.

This must be backported up to 2.6.

(cherry picked from commit 3dc4e5a5b947aa55b65a6bde17bbce331586894b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-20 15:52:40 +02:00
Amaury Denoyelle
4075ed06e6 BUG/MEDIUM: mux-quic: fix nb_hreq decrement
nb_hreq is a counter on qcc for active HTTP requests. It is incremented
for each qcs where a full HTTP request was received. It is decremented
when the stream is closed locally :
- on HTTP response fully transmitted
- on stream reset

A bug will occur if a stream is resetted without having processed a full
HTTP request. nb_hreq will be decremented whereas it was not
incremented. This will lead to a crash when building with
DEBUG_STRICT=2. If BUG_ON_HOT are not active, nb_hreq counter will wrap
which may break the timeout logic for the connection.

This bug was triggered on haproxy.org. It can be reproduced by
simulating the reception of a STOP_SENDING frame instead of a STREAM one
by patching qc_handle_strm_frm() :

+       if (quic_stream_is_bidi(strm_frm->id))
+               qcc_recv_stop_sending(qc->qcc, strm_frm->id, 0);
+       //ret = qcc_recv(qc->qcc, strm_frm->id, strm_frm->len,
+       //               strm_frm->offset.key, strm_frm->fin,
+       //               (char *)strm_frm->data);

To fix this bug, a qcs is now flagged with a new QC_SF_HREQ_RECV. This
is set when the full HTTP request is received. When the stream is closed
locally, nb_hreq will be decremented only if this flag was set.

This must be backported up to 2.6.

(cherry picked from commit afb7b9d8e5a70a741bbb890945fa9ff51dad027d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-20 15:52:18 +02:00
Willy Tarreau
59b23ebdf4 SCRIPTS: announce-release: update some URLs to https
Some components like Discourse were already redirecting to https. Other
ones like docs and git are covered by the certificate, and finally
switching the advertised scheme for www should increase the ratio of
H2 and H3 in the stats (resp 8.9 and 1.9%) and possibly help spot new
issues.

(cherry picked from commit 68b3e135e36ddb17a6b2643c7af938226705f713)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-19 11:44:04 +02:00
Willy Tarreau
3822ac3e5e BUILD: fd: fix a build warning on the DWCAS
Ilya reported in issue #1816 a build warning on armhf (promoted to error
here since -Werror):

  src/fd.c: In function fd_rm_from_fd_list:
  src/fd.c:209:87: error: passing argument 3 of __ha_cas_dw discards volatile qualifier from pointer target type [-Werror=discarded-array-qualifiers]
    209 |    unlikely(!_HA_ATOMIC_DWCAS(((long *)&fdtab[fd].update), (uint32_t *)&cur_list.u32, &next_list.u32))
        |                                                                                       ^~~~~~~~~~~~~~

This happens only on such an architecture because the DWCAS requires the
pointer not the value, and gcc seems to be needlessly picky about reading
a const from a volatile! This may safely be backported to older versions.

(cherry picked from commit 85af76070412d87433fbcaa0ac95833a8470159d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-19 11:43:28 +02:00
Willy Tarreau
61b4de015c BUG/MEDIUM: captures: free() an error capture out of the proxy lock
Ed Hein reported in github issue #1856 some occasional watchdog panics
in 2.4.18 showing extreme contention on the proxy's lock while the libc
was in malloc()/free(). One cause of this problem is that we call free()
under the proxy's lock in proxy_capture_error(), which makes no sense
since if we can free the object under the lock after it's been detached,
we can also free it after releasing the lock (since it's not referenced
anymore).

This should be backported to all relevant versions, likely all
supported ones.

(cherry picked from commit da9f25875958757fd1f16b74bd887977e78c8b09)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-19 11:42:32 +02:00
cui fliter
e8b0b14eff CLEANUP: quic,ssl: fix tiny typos in C comments
This fixes 4 tiny and harmless typos in mux_quic.c, quic_tls.c and
ssl_sock.c. Originally sent via GitHub PR #1843.

Signed-off-by: cui fliter <imcusg@gmail.com>
[Tim: Rephrased the commit message]
[wt: further complete the commit message]
(cherry picked from commit a94bedc0de218e784683e52ba669912b6cc71741)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-19 11:42:20 +02:00
Aurelien DARRAGON
d87047115f BUG/MEDIUM: server: segv when adding server with hostname from CLI
When calling 'add server' with a hostname from the cli (runtime),
str2sa_range() does not resolve hostname because it is purposely
called without PA_O_RESOLVE flag.

This leads to 'srv->addr_node.key' being NULL. According to Willy it
is fine behavior, as long as we handle it properly, and is already
handled like this in srv_set_addr_desc().

This patch fixes GH #1865 by adding an extra check before inserting
'srv->addr_node' into 'be->used_server_addr'. Insertion and removal
will be skipped if 'addr_node.key' is NULL.

It must be backported to 2.6 and 2.5 only.

(cherry picked from commit 8d0ff284064e7a47ae46897e0ce9b08abe539315)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-19 11:42:09 +02:00
Amaury Denoyelle
2ff9519605 BUG/MINOR: mux-quic: do not remotely close stream too early
A stream is considered as remotely closed once we have received all the
data with the FIN bit set.

The condition to close the stream was wrong. In particular, if we
receive an empty STREAM frame with FIN bit set, this would have close
the stream even if we do not have yet received all the data. The
condition is now adjusted to ensure that Rx buffer contains all the data
up to the stream final size.

In most cases, this bug is harmless. However, if compiled with
DEBUG_STRICT=2, a BUG_ON_HOT crash would have been triggered if close is
done too early. This was most notably the case sometimes on interop test
suite with quinn or kwik clients. This can also be artificially
reproduced by simulating reception of an empty STREAM frame with FIN bit
set in qc_handle_strm_frm() :

+       if (strm_frm->fin) {
+               qcc_recv(qc->qcc, strm_frm->id, 0,
+                        strm_frm->len, strm_frm->fin,
+                        (char *)strm_frm->data);
+       }
        ret = qcc_recv(qc->qcc, strm_frm->id, strm_frm->len,
                       strm_frm->offset.key, strm_frm->fin,
                       (char *)strm_frm->data);

This must be backported up to 2.6.

(cherry picked from commit d1310f8d327b7102558e8c549ce09e4925b1824b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-19 11:41:42 +02:00
Amaury Denoyelle
01a5be8c38 CLEANUP: mux-quic: remove stconn usage in h3/hq
Small cleanup on snd_buf for application protocol layer.
* do not export h3_snd_buf
* replace stconn by a qcs argument. This is better as h3/hq-interop only
  uses the qcs instance.

This should be backported up to 2.6.

(cherry picked from commit 8d4ac48d3def189190c29b6f1f5d697b180f7e30)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-19 11:41:38 +02:00
Amaury Denoyelle
57b3c47e70 BUG/MEDIUM: mux-quic: fix crash on early app-ops release
H3 SETTINGS emission has recently been delayed. The idea is to send it
with the first STREAM to reduce sendto syscall invocation. This was
implemented in the following patch :
  3dd79d378c86b3ebf60e029f518add5f1ed54815
  MINOR: h3: Send the h3 settings with others streams (requests)

This patch works fine under nominal conditions. However, it will cause a
crash if a HTTP/3 connection is released before having sent any data,
for example when receiving an invalid first request. In this case,
qc_release will first free qcc.app_ops HTTP/3 application protocol layer
via release callback. Then qc_send is called to emit any closing frames
built by app_ops release invocation. However, in qc_send, as no data has
been sent, it will try to complete application layer protocol
intialization, with a SETTINGS emission for HTTP/3. Thus, qcc.app_ops is
reused, which is invalid as it has been just freed. This will cause a
crash with h3_finalize in the call stack.

This bug can be reproduced artificially by generating incomplete HTTP/3
requests. This will in time trigger http-request timeout without any
data send. This is done by editing qc_handle_strm_frm function.

-       ret = qcc_recv(qc->qcc, strm_frm->id, strm_frm->len,
+       ret = qcc_recv(qc->qcc, strm_frm->id, strm_frm->len - 1,
                       strm_frm->offset.key, strm_frm->fin,
                       (char *)strm_frm->data);

To fix this, application layer closing API has been adjusted to be done
in two-steps. A new shutdown callback is implemented : it is used by the
HTTP/3 layer to generate GOAWAY frame in qc_release prologue.
Application layer context qcc.app_ops is then freed later in qc_release
via the release operation which is now only used to liberate app layer
ressources. This fixes the problem as the intermediary qc_send
invocation will be able to reuse app_ops before it is freed.

This patch fixes the crash, but it would be better to adjust H3 SETTINGS
emission in case of early connection closing : in this case, there is no
need to send it. This should be implemented in a future patch.

This should fix the crash recently experienced by Tristan in github
issue #1801.

This must be backported up to 2.6.

(cherry picked from commit f8aaf8bdfa40e21b1a2f600c3ed6455bf9b6a763)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-19 11:41:25 +02:00
William Lallemand
86120e4953 MEDIUM: quic: separate path for rx and tx with set_encryption_secrets
With quicTLS the set_encruption_secrets callback is always called with
the read_secret and the write_secret.

However this is not the case with libreSSL, which uses the
set_read_secret()/set_write_secret() mecanism. It still provides the
set_encryption_secrets() callback, which is called with a NULL
parameter for the write_secret during the read, and for the read_secret
during the write.

The exchange key was not designed in haproxy to be called separately for
read and write, so this patch allow calls with read or write key to
NULL.

(cherry picked from commit 95fc737fc6edfa2575ce982b739184e99475c215)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-19 11:41:19 +02:00
Mathias Weiersmueller
af88b39e2d DOC: fix TOC in starter guide for subsection 3.3.8. Statistics
This subsection has been moved from 3.4.9 to 3.3.8 somewhere along
2.4, but the TOC has not been updated - resulting in a invalid
anchor in the HTML version.

Needs to be backported to 2.4+

(cherry picked from commit 243c2d18221b36b087ad9c177293306119f3fafd)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-19 11:40:43 +02:00
William Lallemand
26dd079ed9 REGTESTS: ssl/log: test the log-forward with SSL
Test the log-forward section with an SSL server and an SSL bind.

Must be backported as far as 2.3.

(cherry picked from commit 23bc0b20bd82c983bccb289825c6024730aaf405)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-19 11:37:03 +02:00
Emeric Brun
2cc1ed89d4 BUG/MEDIUM: sink: bad init sequence on tcp sink from a ring.
The init of tcp sink, particularly for SSL, was done
too early in the code, during parsing, and this can cause
a crash specially if nbthread was not configured.

This was detected by William using ASAN on a new regtest
on log forward.

This patch adds the 'struct proxy' created for a sink
to a list and this list is now submitted to the same init
code than the main proxies list or the log_forward's proxies
list. Doing this, we are assured to use the right init sequence.
It also removes the ini code for ssl from post section parsing.

This patch should be backported as far as v2.2

Note: this fix uses 'goto' labels created by commit
'BUG/MAJOR: log-forward: Fix log-forward proxies not fully initialized'
but this code didn't exist before v2.3 so this patch needs to be
adapted for v2.2.

(cherry picked from commit d6e581de4be1d3564d771056303242c9ae930c40)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-19 11:36:59 +02:00
William Lallemand
4b10e9bea8 REGTESTS: log: test the log-forward feature
This reg-test test the log-forward feature by chaining a UDP and a TCP
log-forwarder.

It could be backported as far as 2.3.

(cherry picked from commit ebf600a8384040a023b5278c1005ee1a2c04d712)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-19 11:36:54 +02:00
Aurelien DARRAGON
e96752b16d BUG/MINOR: listener: null pointer dereference suspected by coverity
Please refer to GH #1859 for more info.
Coverity suspected improper proxy pointer handling.
Without the fix it is considered safe for the moment, but it might not
be the case in the future as we want to keep the ability to have
isolated listeners.

Making sure stop_listener(), pause_listener(), resume_listener()
and listener_release() functions make proper use
of px pointer in that context.

No need for backport except if multi-connection protocols (ie:FTP)
were to be backported as well.

(cherry picked from commit a57786e87d0746baec43ea888bf6cd30c490d2fb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-19 11:36:37 +02:00
Aurelien DARRAGON
c0f8d3477d CLEANUP: listener: function comment typo in stop_listener()
A minor typo related to stop_listener() function comment
was introduced in 0013288.

This makes stop_listener() function comment easier to read.

(cherry picked from commit 187396e34ed1ab28e73ebcd678fbe7acc32eaad4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-09-12 17:54:22 +02:00