18397 Commits

Author SHA1 Message Date
Tim Duesterhus
7258191d11 CI: Replace the deprecated ::set-output command by writing to $GITHUB_OUTPUT in matrix.py
As announced in

https://github.blog/changelog/2022-10-11-github-actions-deprecating-save-state-and-set-output-commands/

the `::set-output` command is deprecated, because processes during the workflow
execution might output untrusted information that might include the
`::set-output` command, thus allowing these untrusted information to hijack the
build.

The replacement is writing to the file indicated by the `$GITHUB_OUTPUT`
environment variable.

(cherry picked from commit 8a03bf4052884a38ceb01274b8344676cc71a06f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:47:43 +02:00
Christopher Faulet
252ac81019 MINOR: httpclient/lua: Don't set req_payload callback if body is empty
The HTTPclient callback req_payload callback is set when a request payload
must be streamed. In the lua, this callback is set when a body is passed as
argument in one of httpclient functions (head/get/post/put/delete). However,
there is no reason to set it if body string is empty.

This patch is related to the issue #1898. It may be backported as far as
2.5.

(cherry picked from commit 380ae9c3ff4919c9ea36d3d713427575c44fe4b8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:47:11 +02:00
Christopher Faulet
bb170f4dff BUG/MEDIUM: httpclient: Don't set EOM flag on an empty HTX message
In the HTTP client, when the request body is streamed, at the end of the
payload, we must be sure to not set the EOM flag on an empty message.
Otherwise, because there is no data, the buffer is reset to be released and
the flag is lost. Thus, the HTTP client is never notified of the end of
payload for the request and the applet is blocked. If the HTTP client is
instanciated from a Lua script, it is even worse because we fall into a
wakeup loop between the lua script and the HTTP client applet. At the end,
HAProxy is killed because of the watchdog.

This patch should fix the issue #1898. It must be backported to 2.6.

(cherry picked from commit 48005de17c9af5dd2faf825fedfe118bdcc150ef)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:47:05 +02:00
Frédéric Lécaille
80bb8f8ab5 BUILD: ssl_sock: bind_conf uninitialized in ssl_sock_bind_verifycbk()
Even if this cannot happen, ensure <bind_conf> is initialized in this
function to please some compilers.

Takes the opportunity of this patch to replace an ABORT_NOW() by
a BUG_ON() because if the variable values they test are not initialized,
this is really because there is a bug.

Must be backported to 2.6.

(cherry picked from commit 48e46f98ccf97427995eb41c6f28cc38705bdd7e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:46:59 +02:00
Amaury Denoyelle
85c059a307 MINOR: quic: implement datagram cleanup for quic_receiver_buf
Each time data is read on QUIC receiver socket, we try to reuse the
first datagram of the currently used quic_receiver_buf instead of
allocating a new one. This algorithm is suboptimal if there is several
unused datagrams as only the first one is tested and its buffer removed
from quic_receiver_buf.

If QUIC traffic is quite substential, this can lead to an important
number of quic_dgram occurences allocated from pool_head_quic_dgram and
a lack of free space in allocated quic_receiver_buf buffers.

To improve this, each time we want to reuse a datagram, we pop elements
until a non-yet released datagram is found or the list is empty. All
intermediary elements are freed and the last found datagram can be
reused. This operation has been extracted in a dedicated function named
quic_rxbuf_purge_dgrams().

This should improve memory consumption incured by quic_dgram instances under heavy
QUIC traffic. Note that there is still room for improvement as if the
first datagram is still in use, it may block several unused datagram
after him. However this requires to support removal of datagrams out of
order which is currently not possible.

This should be backported up to 2.6.

(cherry picked from commit 91b2305ad79bb7086840797b6e98bd791992444f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:46:15 +02:00
Amaury Denoyelle
6f2acccd90 CLEANUP: quic: improve naming for rxbuf/datagrams handling
QUIC datagrams are read from a random thread. They are then redispatch
to the connection thread according to the first packet DCID. These
operations are implemented through a special buffer designed to avoid
locking.

Refactor this code with the following changes :
* <rxbuf> type is renamed <quic_receiver_buf>. Its list element is also
  renamed to highligh its attach point to a receiver.
* <quic_dgram> and <quic_receiver_buf> definition are moved to
  quic_sock-t.h. This helps to reduce the size of quic_conn-t.h.
* <quic_dgram> list elements are renamed to highlight their attach point
  into a <quic_receiver_buf> and a <quic_dghdlr>.

This should be backported up to 2.6.

(cherry picked from commit 1cba8d60f3f9ef6abf632386f490fd0b0dc2e6b5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:46:11 +02:00
Amaury Denoyelle
94cf0e576d CLEANUP: quic: remove unused rxbufs member in receiver
rxbuf is the structure used to store QUIC datagrams and redispatch them
to the connection thread.

Each receiver manages a list of rxbuf. This was stored both as an array
and a mt_list. Currently, only mt_list is needed so removed <rxbufs>
member from receiver structure.

This should be backported up to 2.6.

(cherry picked from commit 8c4d062d25f57ec983ea8e71a3ccc2f1f9ba7b00)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:46:08 +02:00
Frédéric Lécaille
118e94d0c0 MINOR: quic: Split the secrets key allocation in two parts
Implement quic_tls_secrets_keys_alloc()/quic_tls_secrets_keys_free() to allocate
the memory for only one direction (RX or TX).
Modify ha_quic_set_encryption_secrets() to call these functions for one of this
direction (or both). So, for now on we can rely on the value of the secret keys
to know if it was derived.
Remove QUIC_FL_TLS_SECRETS_SET flag which is no more useful.
Consequently, the secrets are dumped by the traces only if derived.

Must be backported to 2.6.

(cherry picked from commit e1a49cfd4dc58b1923e99931dd507d2945e5ec8e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:46:04 +02:00
Frédéric Lécaille
e0212bcb47 BUG/MINOR: quic: Stalled 0RTT connections with big ClientHello TLS message
This issue was reproduced with -Q picoquic client option to split a big ClientHello
message into two Initial packets and haproxy as server without any knowledged of
any previous ORTT session (restarted after a firt 0RTT session). The ORTT received
packets were removed from their queue when the second Initial packet was parsed,
and the QUIC handshake state never progressed and remained at Initial state.

To avoid such situations, after having treated some Initial packets we always
check if there are ORTT packets to parse and we never remove them from their
queue. This will be done after the hanshake is completed or upon idle timeout
expiration.

Also add more traces to be able to analize the handshake progression.

Tested with ngtcp2 and picoquic

Must be backported to 2.6.

(cherry picked from commit 4aa7d8197ac565976c2b2eec93a9b98e905829df)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:46:01 +02:00
Frédéric Lécaille
c094639d65 MINOR: quic: Use a non-contiguous buffer for RX CRYPTO data
Implement quic_get_ncbuf() to dynamically allocate a new ncbuf to be attached to
any quic_cstream struct which needs such a buffer. Note that there is no quic_cstream
for 0RTT encryption level. quic_free_ncbuf() is added to release the memory
allocated for a non-contiguous buffer.

Modify qc_handle_crypto_frm() to call this function and allocate an ncbuf for
crypto data which are not received in order. The crypto data which are received in
order are not buffered but provide to the TLS stack (calling qc_provide_cdata()).

Modify qc_treat_rx_crypto_frms() which is called after having provided the
in order received crypto data to the TLS stack to provide again the remaining
crypto data which has been buffered, if possible (if they are in order). Each time
buffered CRYPTO data were consumed, we try to release the memory allocated for
the non-contiguous buffer (ncbuf).
Also move rx.crypto.offset quic_enc_level struct member to rx.offset quic_cstream
struct member.

Must be backported to 2.6.

(cherry picked from commit 9f9263ed13ab28a492323484c4c4be89f218848f)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:45:58 +02:00
Frédéric Lécaille
f05bd4e5bd MINOR: quic: Extract CRYPTO frame parsing from qc_parse_pkt_frms()
Implement qc_handle_crypto_frm() to parse a CRYPTO frame.

Must be backported to 2.6.

(cherry picked from commit a20c93e6e2b3dfc2deaa96ff1d5890b1eda290b9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:45:54 +02:00
Frédéric Lécaille
6b84c17d99 MINOR: quic: New quic_cstream object implementation
Add new quic_cstream struct definition to implement the CRYPTO data stream.
This is a simplication of the qcs object (QUIC streams) for the CRYPTO data
without any information about the flow control. They are not attached to any
tree, but to a QUIC encryption level, one by encryption level except for
the early data encryption level (for 0RTT). A stream descriptor is also allocated
for each CRYPTO data stream.

Must be backported to 2.6

(cherry picked from commit 7e3f7c47e9acdf074c678cdee4202192fffb7de4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:45:49 +02:00
Willy Tarreau
2f824716d3 BUG/MINOR: server: make sure "show servers state" hides private bits
In the past we've seen "show servers state" dump some internal bits for
the check states, that were causing regtests to fail. The relevant bits
have been added to the doc to fix the public API and make sure they do
not change by accident, but the output doesn't take care of masking the
undesired ones, causing regtests (and possibly user programs) to fail
when new bits are added. Let's add the mask for the only documented ones
(0x0F for check and 0x1F for agent respectively).

This could be backported wherever the server state is present, though
there's a tiny risk that some undocumented bits might have already
leaked to some user scripts, so it might be wise to wait a bit before
doing that or even not to backport too far.

(cherry picked from commit 99521abd59a255538f2f9a64d3379c31aef5a630)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:45:28 +02:00
Willy Tarreau
603c54d9fa BUG/MAJOR: stick-tables: do not try to index a server name for applets
Since commit 03cdf55e6 ("MINOR: stream: Stickiness server lookup by name.")
in 2.0-dev6, server names may be used instead of their IDs, in order to
perform stickiness. However the commit above may end up trying to insert
an empty server name in the dictionary when the server is an applet
instead, resulting in an immediate segfault. This is typically what
happens when a "stick-store" rule is present in a backend featuring a
"stats" directive. As there doesn't seem to be an easy way around it,
it seems to imply that "stick-store" is not much used anymore.

The solution here is to only try to insert non-null keys into the
dictionary. The patch moves the check of the key type before the
first lock so that the test on the key can be performed under the lock
instead of locking twice (the patch is more readable with diff -b).

Note that before 2.4, there's no <key> variable there as it was
introduced by commit 92149f9a8 ("MEDIUM: stick-tables: Add srvkey
option to stick-table"), but the __objt_server(s->target)->id still
needs to be tested.

This needs to be backported as far as 2.0.

(cherry picked from commit bc7c207f745bf6406b38139b98cb5b8c794e13f0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:43:15 +02:00
Aurelien DARRAGON
6e7ac7cf7d DOC: configuration: missing 'if' in tcp-request content example
An example given for tcp-request content rule with lua
was missing 'if' keyword. Using it "as is" makes haproxy unhappy.

The example was introduced with 579d83b05.
So it may be backported as far as 1.6, but it is a really minor typo.

(cherry picked from commit d49b559a15a8781ba37877812d4d3ddf2cea9eb4)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:43:07 +02:00
Amaury Denoyelle
a28fac9d8e BUG/MINOR: quic: set IP_PKTINFO socket option for QUIC receivers only
Move code which activates IP_PKTINFO socket option (or affiliated
options) from sock_inet_bind_receiver() to quic_bind_listener()
function. This change is useful for two reasons :

* first, and the most important one : this activates IP_PKTINFO only for
  QUIC receivers. The previous version impacted all datagram receivers,
  used for example by log-forwarder. This should reduce memory usage for
  these datagram sockets which do not need this option.

* second, USE_QUIC preprocessor statements are removed from
  src/sock_inet.c which clean up the code.

IP_PKTINFO was introduced recently by the following patch :
  97ecc7a8ea5339a753507c3d4e4cd83028c6d038 (quic-dev/qns)
  MEDIUM: quic: retrieve frontend destination address

For the moment, this does not impact any stable release. However, as
previous patch is scheduled for 2.6 backporting, the current change must
also be backported to the same versions.

(cherry picked from commit 487d04f6d7410059b44b69d56c53be7802963bab)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:42:54 +02:00
Willy Tarreau
b088056f3d CLEANUP: quic/receiver: remove the now unused tx_qring list
The tx_qrings[] and tx_qring_list in the receiver are not used
anymore since commit f2476053f ("MINOR: quic: replace custom buf on Tx
by default struct buffer"), the only place where they're referenced
was in quic_alloc_tx_rings_listener(), which by the way implies that
these were not even freed on exit.

Let's just remove them. This should be backported to 2.6 since the
commit above also was.

(cherry picked from commit cab054bbf9908f3732648e5236d889650a6e33f7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:42:38 +02:00
Amaury Denoyelle
3701ab7823 MEDIUM: quic: retrieve frontend destination address
Retrieve the frontend destination address for a QUIC connection. This
address is retrieve from the first received datagram and then stored in
the associated quic-conn.

This feature relies on IP_PKTINFO or affiliated flags support on the
socket. This flag is set for each QUIC listeners in
sock_inet_bind_receiver(). To retrieve the destination address,
recvfrom() has been replaced by recvmsg() syscall. This operation and
parsing of msghdr structure has been extracted in a wrapper quic_recv().

This change is useful to finalize the implementation of 'dst' sample
fetch. As such, quic_sock_get_dst() has been edited to return local
address from the quic-conn. As a best effort, if local address is not
available due to kernel non-support of IP_PKTINFO, address of the
listener is returned instead.

This should be backported up to 2.6.

(cherry picked from commit 97ecc7a8ea5339a753507c3d4e4cd83028c6d038)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:42:27 +02:00
Willy Tarreau
d513bcc5d1 BUG/MEDIUM: config: count line arguments without dereferencing the output
Previous commit 8a6767d26 ("BUG/MINOR: config: don't count trailing spaces
as empty arg (v2)") was still not enough. As reported by ClusterFuzz in
issue 52049 (https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=52049),
there remains a case where for the sake of reporting the correct argument
count, the function may produce virtual args that span beyond the end of
the output buffer if that one is too short. That's what's happening with
a config file of one empty line followed by a large number of args.

This means that what args[] points to cannot be relied on and that a
different approach is needed. Since no output is produced for spaces and
comments, we know that args[arg] continues to point to out+outpos as long
as only comments or spaces are found, which is what we're interested in.

As such it's safe to check the last arg's pointer against the one before
the trailing zero was emitted, in order to decide to count one final arg.

No backport is needed, unless the commit above is backported.

(cherry picked from commit 94ab139266a2d2d39f7254644f69fb699559e8e2)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:41:58 +02:00
Erwan Le Goas
46fd5a187d BUG/MINOR: config: don't count trailing spaces as empty arg (v2)
In parse_line(), spaces increment the arg count and it is incremented
again on '#' or end of line, resulting in an extra empty arg at the
end of arg's list. The visible effect is that the reported arg count
is in excess of 1. It doesn't seem to affect regular function but
specialized ones like anonymisation depends on this count.

This is the second attempt for this problem, here the explanation :

When called for the first line, no <out> was allocated yet so it's NULL,
letting the caller realloc a larger line if needed. However the words are
parsed and their respective args[arg] are filled with out+position, which
means that while the first arg is NULL, the other ones are no and fail the
test that was meant to avoid dereferencing a NULL. Let's simply check <out>
instead of <args> since the latter is always derived from the former and
cannot be NULL without the former also being.

This may need to be backported to stable versions.

(cherry picked from commit 8a6767d266e0b885d1752a99cbe6b1e11c4e4256)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-25 11:41:54 +02:00
wrightlaw
3593db1a39 BUG/MINOR: smtpchk: SMTP Service check should gracefully close SMTP transaction
At present option smtpchk closes the TCP connection abruptly on completion of service checking,
even if successful. This can result in a very high volume of errors in backend SMTP server logs.
This patch ensures an SMTP QUIT is sent and a positive 2xx response is received from the SMTP
server prior to disconnection.

This patch depends on the following one:

 * MINOR: smtpchk: Update expect rule to fully match replies to EHLO commands

This patch should fix the issue #1812. It may be backported as far as 2.2
with the commit above On the 2.2, proxy_parse_smtpchk_opt() function is
located in src/check.c

[cf: I updated reg-tests script accordingly]

(cherry picked from commit 9a8d8a3fd0828a1cb64745318dcc5704a0b4b1a9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:45:06 +02:00
Christopher Faulet
9d21c53f63 MINOR: smtpchk: Update expect rule to fully match replies to EHLO commands
The response to EHLO command is a multiline reply. However the corresponding
expect rule only match on the first line. For now, it is not an issue. But
to be able to send the QUIT command and gracefully close the connection, we
must be sure to consume the full EHLO reply first.

To do so, the regex has been updated to match all 2xx lines at a time.

(cherry picked from commit 2ec1ffaed061cd3560934c7525b204dbca25ace3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:45:06 +02:00
Christopher Faulet
88316b6b99 BUG/MINOR: mux-h1: Account consumed output data on synchronous connection error
The commit 372b38f935 ("BUG/MEDIUM: mux-h1: Handle connection error after a
synchronous send") introduced a bug. In h1_snd_buf(), consumed data are not
properly accounted if a connection error is detected. Indeed, data are
consumed when the output buffer is filled. But, on connection error, we exit
from the loop without incremented total variable accordingly.

When this happens, this leaves the channel buffer in an inconsistent
state. The buffer may be empty with some output at the channel level.
Because an error is reported, it is harmless. But it is safer to fix this
bug now to avoid any regression in future.

This patch must be backported as far as 2.2.

(cherry picked from commit b0b8e9bbd2b720ed2aff469d0989ce00876007dc)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:45:06 +02:00
Amaury Denoyelle
3fb33cd62e CLEANUP: quic: fix indentation
Fix some indentation in qc_lstnr_pkt_rcv().

This should be backported up to 2.6.

(cherry picked from commit 90121b3321512a74df1a5349c0f99fa2ccd0c017)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
Amaury Denoyelle
17d919a749 MINOR: mux-quic: check quic-conn return code on Tx
Inspect return code of qc_send_mux(). If quic-conn layer reports an
error, this will interrupt the current emission process.

This should be backported up to 2.6.

(cherry picked from commit 036cc5d88061a05442183d243d07604f696537b5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
Amaury Denoyelle
68f165013c MINOR: quic: limit usage of ssl_sock_ctx in favor of quic_conn
Continue on the cleanup of QUIC stack and components.

quic_conn uses internally a ssl_sock_ctx to handle mandatory TLS QUIC
integration. However, this is merely as a convenience, and it is not
equivalent to stackable ssl xprt layer in the context of HTTP1 or 2.

To better emphasize this, ssl_sock_ctx usage in quic_conn has been
removed wherever it is not necessary : namely in functions not related
to TLS. quic_conn struct now contains its own wait_event for tasklet
quic_conn_io_cb().

This should be backported up to 2.6.

(cherry picked from commit 2ed840015f2d7ca37af77c0d9808cac3b0441d40)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
Aurelien DARRAGON
692efcc20a BUG/MINOR: hlua: hlua_channel_insert_data() behavior conflicts with documentation
Channel.insert(channel, string, [,offset]):

When no offset is provided, hlua_channel_insert_data() inserts
string at the end of incoming data.

This behavior conflicts with the documentation that explicitly says
that the default behavior is to insert the string in front of incoming data.

This patch fixes hlua_channel_insert_data() behavior so that it fully
complies with the documentation.

Thanks to Smackd0wn for noticing it.

This could be backported to 2.6 and 2.5

(cherry picked from commit afb7dafb44d240c9c306a41905812a40d700086c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
Willy Tarreau
fb36fd0f01 BUILD: http_fetch: silence an uninitiialized warning with gcc-4/5/6 at -Os
Gcc 4.x, 5.x and 6.x report this when compiling http_fetch.c:

  src/http_fetch.c: In function 'smp_fetch_meth':
  src/http_fetch.c:357:6: warning: 'htx' may be used uninitialized in this function [-Wmaybe-uninitialized]
     sl = http_get_stline(htx);

That's quite weird since there's no such code path, but presetting the
htx variable to NULL during declaration is enough to shut it up.

This may be backported to any version that has dbbdb25f1 ("BUG/MINOR:
http-fetch: Use integer value when possible in "method" sample fetch")
as it's the one that triggered this warning (hence at least 2.0).

(cherry picked from commit 2e2b79d157944d12922ae7ad487247259f3e6d70)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
Christopher Faulet
7e22d25354 BUG/MINOR: http-fetch: Update method after a prefetch in smp_fetch_meth()
In smp_fetch_meth(), smp_prefetch_htx() function may be called to parse the
HTX message and update the HTTP transaction accordingly. In this case, in
smp_fetch_metch() and on success, we must update "meth" variable. Otherwise,
the variable is still equal to HTTP_METH_OTHER and the string version is
always used instead of the enum for known methods.

This patch must be backported as far as 2.0.

(cherry picked from commit eefcd8a97de7d1b05b38d240b1061e2f2112f2f8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
Willy Tarreau
a3b16c2907 MINOR: init: do not try to shrink existing RLIMIT_NOFIlE
As seen in issue #1866, some environments will not allow to change the
current FD limit, and actually we don't need to do it, we only do it as
a byproduct of adjusting the limit to the one that fits. Here we're
replacing calls to setrlimit() with calls to raise_rlim_nofile(), which
will avoid making the setrlimit() syscall in case the desired value is
lower than the current process' one.

This depends on previous commit "MINOR: fd: add a new function to only
raise RLIMIT_NOFILE" and may need to be backported to 2.6, possibly
earlier, depending on users' experience in such environments.

(cherry picked from commit c06557c23b0e4e932405fe3f0739c303ac90926e)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
Willy Tarreau
d22ff05c3e MINOR: fd: add a new function to only raise RLIMIT_NOFILE
In issue #1866 an issue was reported under docker, by which a user cannot
lower the number of FD needed. It looks like a restriction imposed in this
environment, but it results in an error while it ought not have to in the
case of shrinking.

This patch adds a new function raise_rlim_nofile() that takes the desired
new setting, compares it to the current one, and only calls setrlimit() if
one of the values in the new setting is larger than the older one. As such
it will continue to emit warnings and errors in case of failure to raise
the limit but will never shrink it.

This patch is only preliminary to another one, but will have to be
backported where relevant (likely only 2.6).

(cherry picked from commit 922a907926d1cb02a8a10c7cdb9917755c934c84)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
2022-10-10 08:43:48 +02:00
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