Amaury Denoyelle [Fri, 15 Dec 2023 16:32:06 +0000 (17:32 +0100)]
BUG/MINOR: h3: properly handle alloc failure on finalize
If H3 control stream Tx buffer cannot be allocated, return a proper
errur through h3_finalize(). This will cause the emission of a
CONNECTION_CLOSE with error H3_INTERNAL_ERROR and closure of the whole
connection.
This should be backported up to 2.6. Note that 2.9 has some difference
which will cause conflict. The main one is that qcc_get_stream_txbuf()
does not exist in this version. Instead the check in h3_control_send()
should be made after mux_get_buf(). Finally, it may be useful to first
pick previous commit (MINOR: h3: add traces for connection init stage)
to improve context similarity.
(cherry picked from commit
7a3602a1f55dacda5a669865e04474f5d503bab7)
[cf: H3_EV_TX_FRAME removed from trace messages because it does not exist]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Fri, 15 Dec 2023 16:30:36 +0000 (17:30 +0100)]
MINOR: h3: add traces for connection init stage
Add traces H3_EV_H3C_NEW. These are used for h3_init() and h3_finalize()
functions.
(cherry picked from commit
a2dbd6d916ed06d7c7b3db5a8dc77bc01c13d2c0)
[cf: CTX adjt]
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Thu, 21 Dec 2023 16:42:43 +0000 (17:42 +0100)]
BUG/MINOR: h3: close connection on header list too big
When parsing a HTX response, if too many headers are present, stop
sending and close the connection with error code H3_INTERNAL_ERROR.
Previously, no error was reported despite the interruption of header
parsing. This cause an infinite loop. However, this is considered as
minor as it happens on the response path from backend side.
This should be backported up to 2.6.
It relies on previous commit
"MINOR: h3: check connection error during sending".
(cherry picked from commit
d077f7ccf48424fa40eea29c69283ec23732c96d)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Fri, 22 Dec 2023 10:45:54 +0000 (11:45 +0100)]
MINOR: h3: check connection error during sending
If an error occurs during HTX to H3 encoding, h3_snd_buf() should be
interrupted. This commit add this possibility by checking for <h3c.err>
member value. If non null, sending loop is stopped and an error is
reported using qcc_set_error().
This commit does not change any behavior for now, as <h3c.err> is never
set during sending. However, this will change in future commits, most
notably to reject too many headers or handle buffer allocation failure.
As such, this commit should be backported along the following fixes.
Note that in 2.6 qcc_set_error() does not exist and must be replaced by
qcc_emit_cc_app().
(cherry picked from commit
642016ce033aeb5ccc09b81b98db8046eefd2e2c)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Thu, 21 Dec 2023 15:11:35 +0000 (16:11 +0100)]
BUG/MINOR: quic: Missing call to TLS message callbacks
This bug impacts only the QUIC OpenSSL compatibility module (USE_QUIC_OPENSSL_COMPAT).
The TLS capture of information from client hello enabled by
tune.ssl.capture-buffer-size could not work with USE_QUIC_OPENSSL_COMPAT. This
is due to the fact the callback set for this feature was replaced by
quic_tls_compat_msg_callback(). In fact this called must be registered by
ssl_sock_register_msg_callback() as this done for the TLS client hello capture.
A call to this function appends the function passed as parameter to a list of
callbacks to be called when the TLS stack parse a TLS message.
quic_tls_compat_msg_callback() had to be modified to return if it is called
for a non-QUIC TLS session.
Must be backported to 2.8.
(cherry picked from commit
10e96fcd177e858284252d413aaf16af06474beb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Thu, 21 Dec 2023 13:14:22 +0000 (14:14 +0100)]
BUG/MINOR: quic: Wrong keylog callback setting.
This bug impacts only the QUIC OpenSSL compatibility module (USE_QUIC_OPENSSL_COMPAT).
To make this module works, quic_tls_compat_keylog_callback() function must be
set as keylog callback, or at least be called by another keylog callback.
This is what SSL_CTX_keylog() was supposed to do. In addition to export the TLS
secrets via sample fetches this latter also calls quic_tls_compat_keylog_callback()
when compiled with USE_QUIC_OPENSSL_COMPAT defined.
Before this patch, SSL_CTX_keylog() was replaced by quic_tls_compat_keylog_callback()
and the TLS secret were no more exported by sample fetches.
Must be backported to 2.8.
(cherry picked from commit
b26f6fb0cb358ef1df15e21151906e4c2e9f4364)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Thu, 21 Dec 2023 10:15:19 +0000 (11:15 +0100)]
BUG/MINOR: mux-quic: disable fast-fwd if connection on error
Add a check on nego_ff to ensure connection is not on error. If this is
the case, fast-forward is disable to prevent unnecessary sending. If
snd_buf is latter called, stconn will be notified of the error to
interrupt the stream.
This check is necessary to ensure snd_buf and nego_ff are consistent.
Note that previously, if fast-forward was conducted even on connection
error, no sending would occur as qcc_io_send() also check these flags.
However, there is a risk that stconn is never notified of the error
status, thus it is considered as a bug.
Its impact is minimal for now as fast-forward is disable by default on
QUIC. By fixing it, it should be possible to reactive it soon.
This should be backported up to 2.9.
(cherry picked from commit
19f4f4d890bc28bbe4b850e1ced9be399f59fafb)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Tue, 19 Dec 2023 10:22:28 +0000 (11:22 +0100)]
BUG/MINOR: mux-quic: always report error to SC on RESET_STREAM emission
On RESET_STREAM emission, the stream Tx channel is closed. This event
must be reported to stream-conn layer to interrupt future send
operations.
Previously, se_fl_set_error() was manually invocated before/after
qcc_reset_stream(). Change this by moving se_fl_set_error() invocation
into the latter. This ensures that notification won't be forget, most
notably in HTTP/3 layer.
In most cases, behavior should be identical as both functions were
called together unless not necessary. However, there is one exception
which could cause a RESET_STREAM emission without error notification :
this happens on H3 trailers parsing error. All other H3 errors happen
before the stream-layer creation and thus the error is notified on
stream creation. This regression has been caused by the following patch :
152beeec34baed98ad4c186454ddb25e4c496b50
MINOR: mux-quic: report error on stream-endpoint earlier
Thus it should be backported up to 2.7.
Note that the case described above did not cause any crash or protocol
error. This is because currently MUX QUIC snd_buf operation silently
reset buffer on transmission if QCS is already closed locally. This will
however be removed in a future commit so the current patch is necessary
to prevent an invalid behavior.
(cherry picked from commit
0a69750a98bbf4c8d9ab2c84e3890956116dbfa0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Thu, 21 Dec 2023 10:30:39 +0000 (11:30 +0100)]
DOC: fix typo for fastfwd QUIC option
Replace prefix 'tune.quit.' by 'tune.quic.'.
This should be backported up to 2.9.
(cherry picked from commit
c62487fd08c14efa9dcb89a34f470c2cee8b7a66)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Thu, 7 Dec 2023 16:08:08 +0000 (17:08 +0100)]
BUG/MINOR: server/event_hdl: propagate map port info through inetaddr event
server addr:svc_port updates during runtime might set or clear the
SRV_F_MAPPORTS flag. Unfortunately, the flag update is still directly
performed by srv_update_addr_port() function while the addr:svc_port
update is being scheduled for atomic update. Given that existing readers
don't take server's lock to read addr:svc_port, they also check the
SRV_F_MAPPORTS flag right after without the lock.
So we could cause the readers to incorrectly interpret the svc_port from
the server struct because the mapport information is not published
atomically, resulting in inconsistencies between svc_port / mapport flag.
(MAPPORTS flag causes svc_port to be used differently by the reader)
To fix this, we publish the mapport information within the INETADDR server
event and we let the task responsible for updating server's addr and port
position or clear the flag depending on the mapport hint.
This patch depends on:
- MINOR: server/event_hdl: add server_inetaddr struct to facilitate event data usage
- MINOR: server/event_hdl: update _srv_event_hdl_prepare_inetaddr prototype
This should be backported in 2.9 with
683b2ae01 ("MINOR: server/event_hdl:
add SERVER_INETADDR event")
(cherry picked from commit
545e72546c0d729fd3de0f753adc905dd0b13908)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Thu, 7 Dec 2023 15:39:32 +0000 (16:39 +0100)]
MINOR: server/event_hdl: update _srv_event_hdl_prepare_inetaddr prototype
Slightly change _srv_event_hdl_prepare_inetaddr() function prototype to
reduce the input arguments by learning some settings directly from the
server. Also taking this opportunity to make the function static inline
since it's relatively simple and not meant to be used directly.
(cherry picked from commit
4e50c31eab9a45bf82c5225fbaf456a8da983ce9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Thu, 7 Dec 2023 15:18:39 +0000 (16:18 +0100)]
MINOR: server/event_hdl: add server_inetaddr struct to facilitate event data usage
event_hdl_cb_data_server_inetaddr struct had some anonymous structs
defined in it, making it impossible to pass as a function argument and
harder to maintain since changes must be performed at multiple places
at once. So instead we define a storage struct named server_inetaddr
that helps to save addr:port server information in INET context.
(cherry picked from commit
14893a6a00f24e14dcca5baa335a7f9d78dafc80)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Tue, 5 Dec 2023 16:54:20 +0000 (17:54 +0100)]
BUG/MEDIUM: stats: unhandled switching rules with TCP frontend
Consider the following configuration:
backend back
mode http
frontend front
mode tcp
bind localhost:8080
stats enable
stats uri /stats
tcp-request content switch-mode http if FALSE
use_backend back
Firing a request to /stats on the haproxy process started with the above
configuration will cause a segfault in http_handle_stats().
The cause for the crash is that in this case, the upgrade doesn't simply
switches to HTTP mode, but also changes the stream backend (changing from
the frontend itself to the targeted HTTP backend).
However, there is an inconsitency in the stats logic between the check
for the stats URI and the actual handling of the stats page.
Indeed, http_stats_check_uri() checks uri parameters from the proxy
undergoing the http analyzers, whereas http_handle_stats() uses s->be
instead.
During stream analysis, from the frontend perspective: s->be defaults to
the frontend. But if the frontend is in TCP mode and the stream is
upgraded to HTTP via backend switching rules, then s->be will be assigned
to the actual HTTP-capable backend in stream_set_backend().
What this means is that when the http analyzer first checks if the current
URI matches the one from the "stats uri" directive, it will check against
the "stats uri" directive from the frontend, but later since the stats
handlers reads the uri from s->be it wil actually use the value from the
backend and the previous safety checks are thus garbage, resulting in
unexpected behavior. (In our test case since the backend didn't define
"stats uri" it is set to NULL, and http_handle_stats() dereferences it)
To fix this, we should ensure that prechecks and actual stats processing
always rely on the same proxy source for stats config directives.
This is what is done in this patch, thanks to the previous commit, since
we can make sure that the stat applet will use ->http_px as its parent
proxy. So here we simply propagate the current proxy being analyzed
through all the stats processing functions.
This patch depends on:
- MINOR: stats: store the parent proxy in stats ctx (http)
It should be backported up to 2.4.
For 2.4: the fix is less trivial since stats ctx was directly stored
within the applet struct at that time, so this alternative patch must be
used instead (without "MINOR: stats: store the parent proxy in stats ctx
(http)" dependency):
diff --git a/include/haproxy/applet-t.h b/include/haproxy/applet-t.h
index
014e01ed9..
1d9a63359 100644
--- a/include/haproxy/applet-t.h
+++ b/include/haproxy/applet-t.h
@@ -121,6 +121,7 @@ struct appctx {
* keep the grouped together and avoid adding new ones.
*/
struct {
+ struct proxy *http_px; /* parent proxy of the current applet (only relevant for HTTP applet) */
void *obj1; /* context pointer used in stats dump */
void *obj2; /* context pointer used in stats dump */
uint32_t domain; /* set the stats to used, for now only proxy stats are supported */
diff --git a/src/http_ana.c b/src/http_ana.c
index
b557da89d..
1025d7711 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -63,8 +63,8 @@ static enum rule_result http_req_restrict_header_names(struct stream *s, struct
static void http_manage_client_side_cookies(struct stream *s, struct channel *req);
static void http_manage_server_side_cookies(struct stream *s, struct channel *res);
-static int http_stats_check_uri(struct stream *s, struct http_txn *txn, struct proxy *backend);
-static int http_handle_stats(struct stream *s, struct channel *req);
+static int http_stats_check_uri(struct stream *s, struct http_txn *txn, struct proxy *px);
+static int http_handle_stats(struct stream *s, struct channel *req, struct proxy *px);
static int http_handle_expect_hdr(struct stream *s, struct htx *htx, struct http_msg *msg);
static int http_reply_100_continue(struct stream *s);
@@ -428,7 +428,7 @@ int http_process_req_common(struct stream *s, struct channel *req, int an_bit, s
}
/* parse the whole stats request and extract the relevant information */
- http_handle_stats(s, req);
+ http_handle_stats(s, req, px);
verdict = http_req_get_intercept_rule(px, &px->uri_auth->http_req_rules, s);
/* not all actions implemented: deny, allow, auth */
@@ -3959,16 +3959,16 @@ void http_check_response_for_cacheability(struct stream *s, struct channel *res)
/*
* In a GET, HEAD or POST request, check if the requested URI matches the stats uri
- * for the current backend.
+ * for the current proxy.
*
* It is assumed that the request is either a HEAD, GET, or POST and that the
* uri_auth field is valid.
*
* Returns 1 if stats should be provided, otherwise 0.
*/
-static int http_stats_check_uri(struct stream *s, struct http_txn *txn, struct proxy *backend)
+static int http_stats_check_uri(struct stream *s, struct http_txn *txn, struct proxy *px)
{
- struct uri_auth *uri_auth = backend->uri_auth;
+ struct uri_auth *uri_auth = px->uri_auth;
struct htx *htx;
struct htx_sl *sl;
struct ist uri;
@@ -4003,14 +4003,14 @@ static int http_stats_check_uri(struct stream *s, struct http_txn *txn, struct p
* s->target which is supposed to already point to the stats applet. The caller
* is expected to have already assigned an appctx to the stream.
*/
-static int http_handle_stats(struct stream *s, struct channel *req)
+static int http_handle_stats(struct stream *s, struct channel *req, struct proxy *px)
{
struct stats_admin_rule *stats_admin_rule;
struct stream_interface *si = &s->si[1];
struct session *sess = s->sess;
struct http_txn *txn = s->txn;
struct http_msg *msg = &txn->req;
- struct uri_auth *uri_auth = s->be->uri_auth;
+ struct uri_auth *uri_auth = px->uri_auth;
const char *h, *lookup, *end;
struct appctx *appctx;
struct htx *htx;
@@ -4020,6 +4020,7 @@ static int http_handle_stats(struct stream *s, struct channel *req)
memset(&appctx->ctx.stats, 0, sizeof(appctx->ctx.stats));
appctx->st1 = appctx->st2 = 0;
appctx->ctx.stats.st_code = STAT_STATUS_INIT;
+ appctx->ctx.stats.http_px = px;
appctx->ctx.stats.flags |= uri_auth->flags;
appctx->ctx.stats.flags |= STAT_FMT_HTML; /* assume HTML mode by default */
if ((msg->flags & HTTP_MSGF_VER_11) && (txn->meth != HTTP_METH_HEAD))
diff --git a/src/stats.c b/src/stats.c
index
d1f3daa98..
1f0b2bff7 100644
--- a/src/stats.c
+++ b/src/stats.c
@@ -2863,9 +2863,9 @@ static int stats_dump_be_stats(struct stream_interface *si, struct proxy *px)
return stats_dump_one_line(stats, stats_count, appctx);
}
-/* Dumps the HTML table header for proxy <px> to the trash for and uses the state from
- * stream interface <si> and per-uri parameters <uri>. The caller is responsible
- * for clearing the trash if needed.
+/* Dumps the HTML table header for proxy <px> to the trash and uses the state from
+ * stream interface <si>. The caller is responsible for clearing the trash if
+ * needed.
*/
static void stats_dump_html_px_hdr(struct stream_interface *si, struct proxy *px)
{
@@ -3015,17 +3015,19 @@ static void stats_dump_html_px_end(struct stream_interface *si, struct proxy *px
* input buffer. Returns 0 if it had to stop dumping data because of lack of
* buffer space, or non-zero if everything completed. This function is used
* both by the CLI and the HTTP entry points, and is able to dump the output
- * in HTML or CSV formats. If the later, <uri> must be NULL.
+ * in HTML or CSV formats.
*/
int stats_dump_proxy_to_buffer(struct stream_interface *si, struct htx *htx,
- struct proxy *px, struct uri_auth *uri)
+ struct proxy *px)
{
struct appctx *appctx = __objt_appctx(si->end);
- struct stream *s = si_strm(si);
struct channel *rep = si_ic(si);
struct server *sv, *svs; /* server and server-state, server-state=server or server->track */
struct listener *l;
+ struct uri_auth *uri = NULL;
+ if (appctx->ctx.stats.http_px)
+ uri = appctx->ctx.stats.http_px->uri_auth;
chunk_reset(&trash);
switch (appctx->ctx.stats.px_st) {
@@ -3045,7 +3047,7 @@ int stats_dump_proxy_to_buffer(struct stream_interface *si, struct htx *htx,
break;
/* match '.' which means 'self' proxy */
- if (strcmp(scope->px_id, ".") == 0 && px == s->be)
+ if (strcmp(scope->px_id, ".") == 0 && px == appctx->ctx.stats.http_px)
break;
scope = scope->next;
}
@@ -3227,10 +3229,16 @@ int stats_dump_proxy_to_buffer(struct stream_interface *si, struct htx *htx,
}
/* Dumps the HTTP stats head block to the trash for and uses the per-uri
- * parameters <uri>. The caller is responsible for clearing the trash if needed.
+ * parameters from the parent proxy. The caller is responsible for clearing
+ * the trash if needed.
*/
-static void stats_dump_html_head(struct appctx *appctx, struct uri_auth *uri)
+static void stats_dump_html_head(struct appctx *appctx)
{
+ struct uri_auth *uri;
+
+ BUG_ON(!appctx->ctx.stats.http_px);
+ uri = appctx->ctx.stats.http_px->uri_auth;
+
/* WARNING! This must fit in the first buffer !!! */
chunk_appendf(&trash,
"<!DOCTYPE HTML PUBLIC \"-//W3C//DTD HTML 4.01 Transitional//EN\"\n"
@@ -3345,17 +3353,21 @@ static void stats_dump_html_head(struct appctx *appctx, struct uri_auth *uri)
}
/* Dumps the HTML stats information block to the trash for and uses the state from
- * stream interface <si> and per-uri parameters <uri>. The caller is responsible
- * for clearing the trash if needed.
+ * stream interface <si> and per-uri parameters from the parent proxy. The caller
+ * is responsible for clearing the trash if needed.
*/
-static void stats_dump_html_info(struct stream_interface *si, struct uri_auth *uri)
+static void stats_dump_html_info(struct stream_interface *si)
{
struct appctx *appctx = __objt_appctx(si->end);
unsigned int up = (now.tv_sec - start_date.tv_sec);
char scope_txt[STAT_SCOPE_TXT_MAXLEN + sizeof STAT_SCOPE_PATTERN];
const char *scope_ptr = stats_scope_ptr(appctx, si);
+ struct uri_auth *uri;
unsigned long long bps = (unsigned long long)read_freq_ctr(&global.out_32bps) * 32;
+ BUG_ON(!appctx->ctx.stats.http_px);
+ uri = appctx->ctx.stats.http_px->uri_auth;
+
/* Turn the bytes per second to bits per second and take care of the
* usual ethernet overhead in order to help figure how far we are from
* interface saturation since it's the only case which usually matters.
@@ -3629,8 +3641,7 @@ static void stats_dump_json_end()
* a pointer to the current server/listener.
*/
static int stats_dump_proxies(struct stream_interface *si,
- struct htx *htx,
- struct uri_auth *uri)
+ struct htx *htx)
{
struct appctx *appctx = __objt_appctx(si->end);
struct channel *rep = si_ic(si);
@@ -3650,7 +3661,7 @@ static int stats_dump_proxies(struct stream_interface *si,
px = appctx->ctx.stats.obj1;
/* skip the disabled proxies, global frontend and non-networked ones */
if (!px->disabled && px->uuid > 0 && (px->cap & (PR_CAP_FE | PR_CAP_BE))) {
- if (stats_dump_proxy_to_buffer(si, htx, px, uri) == 0)
+ if (stats_dump_proxy_to_buffer(si, htx, px) == 0)
return 0;
}
@@ -3666,14 +3677,12 @@ static int stats_dump_proxies(struct stream_interface *si,
}
/* This function dumps statistics onto the stream interface's read buffer in
- * either CSV or HTML format. <uri> contains some HTML-specific parameters that
- * are ignored for CSV format (hence <uri> may be NULL there). It returns 0 if
- * it had to stop writing data and an I/O is needed, 1 if the dump is finished
- * and the stream must be closed, or -1 in case of any error. This function is
- * used by both the CLI and the HTTP handlers.
+ * either CSV or HTML format. It returns 0 if it had to stop writing data and
+ * an I/O is needed, 1 if the dump is finished and the stream must be closed,
+ * or -1 in case of any error. This function is used by both the CLI and the
+ * HTTP handlers.
*/
-static int stats_dump_stat_to_buffer(struct stream_interface *si, struct htx *htx,
- struct uri_auth *uri)
+static int stats_dump_stat_to_buffer(struct stream_interface *si, struct htx *htx)
{
struct appctx *appctx = __objt_appctx(si->end);
struct channel *rep = si_ic(si);
@@ -3688,7 +3697,7 @@ static int stats_dump_stat_to_buffer(struct stream_interface *si, struct htx *ht
case STAT_ST_HEAD:
if (appctx->ctx.stats.flags & STAT_FMT_HTML)
- stats_dump_html_head(appctx, uri);
+ stats_dump_html_head(appctx);
else if (appctx->ctx.stats.flags & STAT_JSON_SCHM)
stats_dump_json_schema(&trash);
else if (appctx->ctx.stats.flags & STAT_FMT_JSON)
@@ -3708,7 +3717,7 @@ static int stats_dump_stat_to_buffer(struct stream_interface *si, struct htx *ht
case STAT_ST_INFO:
if (appctx->ctx.stats.flags & STAT_FMT_HTML) {
- stats_dump_html_info(si, uri);
+ stats_dump_html_info(si);
if (!stats_putchk(rep, htx, &trash))
goto full;
}
@@ -3733,7 +3742,7 @@ static int stats_dump_stat_to_buffer(struct stream_interface *si, struct htx *ht
case STATS_DOMAIN_PROXY:
default:
/* dump proxies */
- if (!stats_dump_proxies(si, htx, uri))
+ if (!stats_dump_proxies(si, htx))
return 0;
break;
}
@@ -4112,11 +4121,14 @@ static int stats_process_http_post(struct stream_interface *si)
static int stats_send_http_headers(struct stream_interface *si, struct htx *htx)
{
struct stream *s = si_strm(si);
- struct uri_auth *uri = s->be->uri_auth;
+ struct uri_auth *uri;
struct appctx *appctx = __objt_appctx(si->end);
struct htx_sl *sl;
unsigned int flags;
+ BUG_ON(!appctx->ctx.stats.http_px);
+ uri = appctx->ctx.stats.http_px->uri_auth;
+
flags = (HTX_SL_F_IS_RESP|HTX_SL_F_VER_11|HTX_SL_F_XFER_ENC|HTX_SL_F_XFER_LEN|HTX_SL_F_CHNK);
sl = htx_add_stline(htx, HTX_BLK_RES_SL, flags, ist("HTTP/1.1"), ist("200"), ist("OK"));
if (!sl)
@@ -4166,11 +4178,14 @@ static int stats_send_http_redirect(struct stream_interface *si, struct htx *htx
{
char scope_txt[STAT_SCOPE_TXT_MAXLEN + sizeof STAT_SCOPE_PATTERN];
struct stream *s = si_strm(si);
- struct uri_auth *uri = s->be->uri_auth;
+ struct uri_auth *uri;
struct appctx *appctx = __objt_appctx(si->end);
struct htx_sl *sl;
unsigned int flags;
+ BUG_ON(!appctx->ctx.stats.http_px);
+ uri = appctx->ctx.stats.http_px->uri_auth;
+
/* scope_txt = search pattern + search query, appctx->ctx.stats.scope_len is always <= STAT_SCOPE_TXT_MAXLEN */
scope_txt[0] = 0;
if (appctx->ctx.stats.scope_len) {
@@ -4263,7 +4278,7 @@ static void http_stats_io_handler(struct appctx *appctx)
}
if (appctx->st0 == STAT_HTTP_DUMP) {
- if (stats_dump_stat_to_buffer(si, res_htx, s->be->uri_auth))
+ if (stats_dump_stat_to_buffer(si, res_htx))
appctx->st0 = STAT_HTTP_DONE;
}
@@ -4888,6 +4903,7 @@ static int cli_parse_show_stat(char **args, char *payload, struct appctx *appctx
appctx->ctx.stats.scope_str = 0;
appctx->ctx.stats.scope_len = 0;
+ appctx->ctx.stats.http_px = NULL; // not under http context
appctx->ctx.stats.flags = STAT_SHNODE | STAT_SHDESC;
if ((strm_li(si_strm(appctx->owner))->bind_conf->level & ACCESS_LVL_MASK) >= ACCESS_LVL_OPER)
@@ -4954,7 +4970,7 @@ static int cli_io_handler_dump_info(struct appctx *appctx)
*/
static int cli_io_handler_dump_stat(struct appctx *appctx)
{
- return stats_dump_stat_to_buffer(appctx->owner, NULL, NULL);
+ return stats_dump_stat_to_buffer(appctx->owner, NULL);
}
static int cli_io_handler_dump_json_schema(struct appctx *appctx)
(cherry picked from commit
64b7d8e173d169c31aff1cfc8495dc5e14fa1454)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Tue, 5 Dec 2023 18:49:13 +0000 (19:49 +0100)]
MINOR: stats: store the parent proxy in stats ctx (http)
Some HTTP related stats functions need to know the parent proxy, mainly
to get a pointer on the related uri_auth set by the proxy or to check
scope settings.
The current design (probably historical as only the http context existed
by then) took the other approach: it propagates the uri pointer from the
http context deep down the calling stack up to the relevant functions.
For non-http contexts (cli), the pointer is set to NULL.
Doing so is not very pretty and not easy to maintain. Moreover, there were
still some places in the code were the uri pointer was learned directly
from the stream proxy because the argument was not available as argument
from those functions. This is error-prone, because if one day we decide to
change the source proxy in the parent function, we might still have some
functions down the stack that ignore the top most argument and still do
on their own, and we'll probably end up with inconsistencies.
So in this patch, we take a safer approach: the caller responsible for
creating the stats applet should set the http_px pointer so that any stats
function running under the applet that needs to know if it's running in
http context or needs to access parent proxy info may do so thanks to
the dedicated ctx->http_px pointer.
(cherry picked from commit
ef9d692544f0bd961bfa5e6a4569447921bf1137)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Thu, 21 Dec 2023 09:27:53 +0000 (10:27 +0100)]
BUG/MAJOR: stconn: Disable zero-copy forwarding if consumer is shut or in error
A regression was introduced by commit
2421c6fa7d ("BUG/MEDIUM: stconn: Block
zero-copy forwarding if EOS/ERROR on consumer side"). When zero-copy
forwarding is inuse and the consumer side is shut or in error, we declare it
as blocked and it is woken up. The idea is to handle this state at the
stream-connector level. However this definitly blocks receives on the
producer side. So if the mux is unable to close by itself, but instead wait
the peer to shut, this can lead to a wake up loop. And indeed, with the
passthrough multiplexer this may happen.
To fix the issue and prevent any loop, instead of blocking the zero-copy
forwarding, we now disable it. This way, the stream-connector on producer
side will fallback on classical receives and will be able to handle peer
shutdown properly. In addition, the wakeup of the consumer side was
removed. This will be handled, if necessary, by sc_notify().
This patch should fix the issue #2395. It must be backported to 2.9.
(cherry picked from commit
123a9e7d83213bcc0478840bab2270113275fb0a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Wed, 20 Dec 2023 11:21:57 +0000 (12:21 +0100)]
BUG/MINOR: server: Use the configured address family for the initial resolution
A regression was introduced by the commit
c886fb58eb ("MINOR: server/ip:
centralize server ip updates"). The configured address family is lost when the
server address is initialized during the startup, for the resolution based on
the libc or based on the server state-file. Thus, "ipv4@" and "ipv6@" prefixed
are ignored.
To fix the bug, we take care to use the configured address family before calling
str2ip2() in srv_apply_lastaddr() and srv_apply_via_libc() functions.
This patch should fix the issue #2393. It must be backported to 2.9.
(cherry picked from commit
3811c1de25bd8b05fb4684614883a0f04f304978)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Christopher Faulet [Tue, 19 Dec 2023 07:51:26 +0000 (08:51 +0100)]
DOC: config: Update documentation about local haproxy response
Documentation about 'L' state in the termination state was outdated. Today,
not only the request may be intercepted, but also the response.
Documentation about 'L' must be more generic.
However, documentation about possible 2-letter termination states was also
extended to add 'LC' and 'LH' in the list. And 'LR' was adapted too.
This patch should fix the issue #2384. It may be backported to every stable
versions. Note that on 2.8 and lowers, we talk about session and not stream.
(cherry picked from commit
0a203c1d3f359396e66679991f5c84460fae80e2)
Signed-off-by: Willy Tarreau <w@1wt.eu>
William Lallemand [Mon, 18 Dec 2023 11:35:35 +0000 (12:35 +0100)]
BUG/MINOR: resolvers: default resolvers fails when network not configured
Bug #1740 was opened again, this time a user is complaining about the
"can't create socket for nameserver". This can happen if the resolv.conf
file contains a class of address which was not configured on the
machine, for example IPv6.
The fix does the same as
b10b1196b ("MINOR: resolvers: shut the warning
when "default" resolvers is implicit"), and uses the
"resolvers->conf.implicit" variable to emit the error.
Though it is not needed to convert the explicit behavior with a
ERR_WARN, because this is supposed to be an unrecoverable error, unlike
the connect().
Should fix issue #1740.
Must be backported were
b10b1196b was backported. (as far as 2.6)
(cherry picked from commit
0d2ebb53f752502137479c9e753a96046d3254ab)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 15 Dec 2023 13:35:36 +0000 (14:35 +0100)]
[RELEASE] Released version 2.9.1
Released version 2.9.1 with the following main changes :
- BUG/MINOR: ssl: Double free of OCSP Certificate ID
- MINOR: ssl/cli: Add ha_(warning|alert) msgs to CLI ckch callback
- BUG/MINOR: ssl: Wrong OCSP CID after modifying an SSL certficate
- BUG/MINOR: lua: Wrong OCSP CID after modifying an SSL certficate (LUA)
- DOC: configuration: typo req.ssl_hello_type
- BUG/MINOR: mworker/cli: fix set severity-output support
- BUG/MEDIUM: quic: Possible buffer overflow when building TLS records
- BUILD: ssl: update types in wolfssl cert selection callback
- BUG/MEDIUM: map/acl: pat_ref_{set,delete}_by_id regressions
- BUG/MINOR: ext-check: cannot use without preserve-env
- MINOR: version: mention that it's stable now
- BUG/MEDIUM: quic: QUIC CID removed from tree without locking
- BUG/MEDIUM: stconn: Block zero-copy forwarding if EOS/ERROR on consumer side
- BUG/MEDIUM: mux-h1: Cound data from input buf during zero-copy forwarding
- BUG/MEDIUM: mux-h1: Explicitly skip request's C-L header if not set originally
- CLEANUP: mux-h1: Fix a trace message about C-L header addition
- BUG/MEDIUM: mux-h2: Report too large HEADERS frame only when rxbuf is empty
- BUG/MEDIUM: mux-quic: report early error on stream
- DOC: config: add arguments to sample fetch methods in the table
- DOC: config: also add arguments to the converters in the table
Willy Tarreau [Fri, 15 Dec 2023 10:18:27 +0000 (11:18 +0100)]
DOC: config: also add arguments to the converters in the table
Now that dconv supports linking to keywords with arguments from tables,
let's mention the arguments in the summary table of converters.
(cherry picked from commit
c5bde03a0acac4263b7ff373f819b680d9a52102)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Fri, 15 Dec 2023 10:18:08 +0000 (11:18 +0100)]
DOC: config: add arguments to sample fetch methods in the table
Now that dconv supports linking to keywords with arguments from tables,
let's mention the arguments in the summary tables of sample fetch methods.
(cherry picked from commit
0d261dd13a9d475e3bf9d7bae07d156fa3f5c5a6)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Amaury Denoyelle [Wed, 13 Dec 2023 15:28:28 +0000 (16:28 +0100)]
BUG/MEDIUM: mux-quic: report early error on stream
On STOP_SENDING reception, an error is notified to the stream layer as
no more data can be responded. However, this is not done if the stream
instance is not allocated (already freed for example).
The issue occurs if STOP_SENDING is received and the stream instance is
instantiated after it. It happens if a STREAM frame is received after it
with H3 HEADERS, which is valid in QUIC protocol due to UDP packet
reordering. In this case, stream layer is never notified about the
underlying error. Instead, reponse buffers are silently purged by the
MUX in qmux_strm_snd_buf().
This is suboptimal as there is no point in exchanging data from the
server if it cannot be eventually transferred back to the client.
However, aside from this consideration, no other issue occured. However,
this is not the case with QUIC mux-to-mux implementation. Now, if
mux-to-mux is used, qmux_strm_snd_buf() is bypassed and response if
transferred via nego_ff/done_ff callbacks. However, these functions did
not checked if QCS is already locally closed. This causes a crash when
qcc_send_stream() is called via done_ff.
To fix this crash, there is several approach, one of them would be to
adjust nego_ff/done_ff QUIC callbacks. However, another method has been
chosen. Now stream layer is flagged on error just after its
instantiation if the stream is already locally closed. This ensures that
mux-to-mux won't try to emit data as se_nego_ff() check if the opposide
SD is not on error before continuing.
Note that an alternative solution could be to not instantiate at all
stream layer if QCS is already locally closed. This is the most optimal
solution as it reduce unnecessary allocations and task processing.
However, it's not easy to implement so the easier bug fix has been
chosen for the moment.
This patch is labelled as MEDIUM as it can change behavior of all QCS
instances, wheter mux-to-mux is used or not, and thus could reveal other
architecture issues.
This should fix latest crash occurence on github issue #2392.
It should be backported up to 2.6, until a necessary period of
observation.
(cherry picked from commit
af297f19f6ff954b739cde3fa76f99f665b556bd)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Wed, 13 Dec 2023 14:36:52 +0000 (15:36 +0100)]
BUG/MEDIUM: mux-h2: Report too large HEADERS frame only when rxbuf is empty
During HEADERS frames decoding, if a frame is too large to fit in a buffer,
an internal error is reported and a RST_STREAM is emitted. On the other
hand, we wait to have an empty rxbuf to decode the frame because we cannot
retry a failed HPACK decompression.
When we are decoding headers, it is valid to return an error if dbuf buffer
is full because no data can be blocked in the rxbuf (which hosts the HTX
message).
However, during the trailers decoding, it is possible to have some data not
sent yet for the current stream in the rxbug and data for another stream
fully filling the dbuf buffer. In this case, we don't decode the trailers
but we must not return an error. We must wait to empty the rxbuf first.
Now, a HEADERS frame is considered as too large if the dbuf buffer is full
and if the rxbuf is empty (the HTX message to be accurate).
This patch should fix the issue #2382. It must be backported to all stable
versions.
(cherry picked from commit
682f73b4fa6d76aa0b5b743fe92777822884772d)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Tue, 12 Dec 2023 13:04:35 +0000 (14:04 +0100)]
CLEANUP: mux-h1: Fix a trace message about C-L header addition
This fixes a cut-paste error on a trace message notifying a 'Content-Length'
header was added during the HTTP message formatting.
(cherry picked from commit
65ca4442401c527530f5b456a8ed47a35c224480)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Tue, 12 Dec 2023 12:56:05 +0000 (13:56 +0100)]
BUG/MEDIUM: mux-h1: Explicitly skip request's C-L header if not set originally
Commit
f89ba27caa ("BUG/MEDIUM: mux-h1; Ignore headers modifications about
payload representation") introduced a regression. The Content-Length is no
longer sent to the server for requests without payload but with a
'Content-Lnegth' header explicitly set to 0, like POST request with no
payload. It is of course unexpected. In some cases, depending on the server,
such requests are considered as invalid and a 411-Length-Required is returned.
The above commit is not directly responsible for the bug, it only reveals a too
lax condition to skip the 'Content-Length' header of bodyless requests. We must
only skip this header if none was originally found, during the parsing.
This patch should fix the issue #2386. It must be backported to 2.9.
(cherry picked from commit
966a18e2b485901bd986ebe65670dc4bfbfe65b5)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 11 Dec 2023 20:55:32 +0000 (21:55 +0100)]
BUG/MEDIUM: mux-h1: Cound data from input buf during zero-copy forwarding
During zero-copy forwarding, we first try to forward data found in the input
buffer before trying to receive more data. These data must be removed from
the amount of data to forward (the cound variable).
Otherwise, on an internal retry, in h1_fastfwd(), we can be lead to read
more data than expected. It is especially a problem on the end of a
chunk. An error is erroneously reported because more data than announced are
received.
This patch should fix the issue #2382. It must be backported to 2.9.
(cherry picked from commit
eed1e8733c279cda0b6a2db574ead4e0238177f6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Mon, 11 Dec 2023 12:56:15 +0000 (13:56 +0100)]
BUG/MEDIUM: stconn: Block zero-copy forwarding if EOS/ERROR on consumer side
When the producer side (h1 for now) negociates with the consumer side to
perform a zero-copy forwarding, we now consider the consumer side as blocked
if it is closed and this was reported to the SE via a end-of-stream or a
(pending) error.
It is performed before calling ->nego_ff callback function, in se_nego_ff().
This way, all consumer are concerned automatically. The aim of this patch is
to fix an issue with the QUIC mux. Indeed, it is unexpected to send a frame
on an closed stream. This triggers a BUG_ON(). Other muxes are not affected
but it remains useless to try to send data if the stream is closed.
This patch should fix the issue #2372. It must be backported to 2.9.
(cherry picked from commit
2421c6fa7db5b06109a88bffa411c8ba8f948384)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Frédéric Lécaille [Wed, 13 Dec 2023 10:45:43 +0000 (11:45 +0100)]
BUG/MEDIUM: quic: QUIC CID removed from tree without locking
This bug arrived with this commit:
BUG/MINOR: quic: Wrong RETIRE_CONNECTION_ID sequence number chec
Every connection ID manipulations against the by thread trees used to store the
connection IDs must be done under the trees locks. These trees are accessed by
the low level connection identification code.
When receiving a RETIRE_CONNECTION_ID frame, the concerned connection ID must
be deleted from the its underlying by thread tree but not without locking!
Add a WR lock around ebmb_delete() call to do so.
Must be backported as far as 2.7.
(cherry picked from commit
dd58dff1e64fa382d368958469506c296454fde6)
Signed-off-by: Willy Tarreau <w@1wt.eu>
Christopher Faulet [Fri, 8 Dec 2023 13:34:32 +0000 (14:34 +0100)]
MINOR: version: mention that it's stable now
This version will be maintained up to around Q1 2025. The INSTALL file
also mentions it.
Aurelien DARRAGON [Thu, 7 Dec 2023 08:58:27 +0000 (09:58 +0100)]
BUG/MINOR: ext-check: cannot use without preserve-env
Since 1de44da ("MINOR: ext-check: add an option to preserve environment
variables"), it is now possible to provide an extra argument to
"external-check" directive. This allows to support the "preserve-env"
option which differs from the default behavior.
However a mistake was made, because the config parser doesn't allow the
default configuration anymore: using external-check without argument will
trigger an error:
'external-check' only supports 'preserve-env' as an argument, found ''.
This is due to as small mistake in the code that make the check
systematically report an error if the first argument is not equal to
"preserve-env". The check was modified so that the error is only reported
if the argument is provided, so that the default behavior is restored.
This should fix GH #2380 and should be backported on 2.9 and potentially
further (anywhere 1de44da is, because a note about an optional backport
up to the 2.6 was left in the original commit message)
(cherry picked from commit
63282f3bfbaee50a0716823bdc8c4251a47e6e39)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Aurelien DARRAGON [Fri, 8 Dec 2023 10:46:15 +0000 (11:46 +0100)]
BUG/MEDIUM: map/acl: pat_ref_{set,delete}_by_id regressions
Some regressions were introduced by
5fea59754b ("MEDIUM: map/acl:
Accelerate several functions using pat_ref_elt struct ->head list")
pat_ref_delete_by_id() fails to properly unlink and free the removed
reference because it bypasses the pat_ref_delete_by_ptr() made for
that purpose. This function is normally used everywhere the target
reference is set for removal, such as the pat_ref_delete() function
that matches pattern against a string. The call was probably skipped
by accident during the rewrite of the function.
With the above commit also comes another undesirable change:
both pat_ref_delete_by_id() and pat_ref_set_by_id() directly use the
<refelt> argument as a valid pointer (they do dereference it).
This is wrong, because <refelt> is unsafe and should be handled as an
ID, not a pointer (hence the function name). Indeed, the calling function
may directly pass user input from the CLI as <refelt> argument, so we must
first ensure that it points to a valid element before using it, else it is
probably invalid and we shouldn't touch it.
What this patch essentially does, is that it reverts pat_ref_set_by_id()
and pat_ref_delete_by_id() to pre
5fea59754b behavior. This seems like
it was the only optimization from the patch that doesn't apply.
Hopefully, after reviewing the changes with Fred, it seems that the 2
functions are only being involved in commands for manipulating maps or
acls on the cli, so the "missed" opportunity to improve their performance
shouldn't matter much. Nonetheless, if we wanted to speed up the reference
lookup by ID, we could consider adding an eb64 tree for that specific
purpose that contains all pattern references IDs (ie: pointers) so that
eb lookup functions may be used instead of linear list search.
The issue was raised by Marko Juraga as he failed to perform an an acl
removal by reference on the CLI on 2.9 which was known to work properly
on other versions.
It should be backported on 2.9.
Co-Authored-by: Frédéric Lécaille <flecaille@haproxy.com>
(cherry picked from commit
d7964c52ceb18a690d33d35b0c802660241d9ba9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Fri, 8 Dec 2023 10:55:15 +0000 (11:55 +0100)]
BUILD: ssl: update types in wolfssl cert selection callback
The types have changed in the PR for the wolfSSL_get_sigalg_info()
function, let's update them.
Must be backported in 2.9.
(cherry picked from commit
dbe9cea35b7a43a4907fa726e7a8afc31a6fbee3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Thu, 7 Dec 2023 20:12:02 +0000 (21:12 +0100)]
BUG/MEDIUM: quic: Possible buffer overflow when building TLS records
This bug impacts only the OpenSSL QUIC compatibility module (USE_QUIC_OPENSSL_COMPAT).
This may happen only when the TLS stack has to be provided with more than 1024+1+5+16
bytes of CRYPTO data. In this case several TLS records have to be built in one
call to SSL_provide_quic_data(). A 5-bytes header is created at the head
of these records. This header is used as AAD to cipher the record. But
the length of this AAD was counted two times. One time here in
quic_tls_compat_create_record() (initialization):
adlen = quic_tls_compat_create_header(qc, rec, ad, 0);
and a second time here in the same function after quic_tls_tls_seal() return:
ret = aad_len + outlen;
This addition is useless. Note that this bug could be reproduced when haproxy has
to authenticate the client.
Thank you to @vifino for having reported this issue in GH #2381.
Must be backported to 2.8.
(cherry picked from commit
c075e4f2fc9e662459f7ab0ce8e13b70c059334a)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Wed, 6 Dec 2023 10:15:01 +0000 (11:15 +0100)]
BUG/MINOR: mworker/cli: fix set severity-output support
"set severity-output" is one of these command that changes the appctx
state so the next commands are affected.
Unfortunately the master CLI works with pipelining and server close
mode, which means the connection between the master and the worker is
closed after each response, so for the next command this is a new appctx
state.
To fix the problem, 2 new flags are added ACCESS_MCLI_SEVERITY_STR and
ACCESS_MCLI_SEVERITY_NB which are used to prefix each command sent to
the worker with the right "set severity-output" command.
This patch fixes issue #2350.
It could be backported as far as 2.6.
(cherry picked from commit
1c1bb8ef2abeab2dc8e4857ec2fef2b19b472be7)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
William Lallemand [Thu, 7 Dec 2023 14:00:58 +0000 (15:00 +0100)]
DOC: configuration: typo req.ssl_hello_type
rep_ssl_hello_type was renamed in res.ssl_hello_type a long time ago.
This patch fixes a typo where an example was renamed
"rep.ssl_hello_type" instead of "res.ssl_hello_type"
fixes issue #2377 and #2379.
Must be backported in all maintained versions.
(cherry picked from commit
906525b16bf53c156f8b559f8782a81f2b960c2b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Wed, 6 Dec 2023 10:42:42 +0000 (11:42 +0100)]
BUG/MINOR: lua: Wrong OCSP CID after modifying an SSL certficate (LUA)
This bugfix is the same as the following one:
"BUG/MINOR: ssl_ckch: Wrong OCSP CID after modifying an SSL certficate"
where the OCSP CID had to be reset when updating a certificate.
Must be backported to 2.8.
(cherry picked from commit
917f7c74d3d30a3f7ac77fd6ea69cfcd529e53a9)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Tue, 5 Dec 2023 14:38:29 +0000 (15:38 +0100)]
BUG/MINOR: ssl: Wrong OCSP CID after modifying an SSL certficate
This bug could be reproduced with the "set ssl cert" CLI command to update
a certificate. The OCSP CID is duplicated by ckchs_dup() which calls
ssl_sock_copy_cert_key_and_chain(). It should be computed again by
ssl_sock_load_ocsp(). This may be accomplished resetting the new ckch OCSP CID
returned by ckchs_dup().
This bug may be in relation with GH #2319.
Must be backported to 2.8.
(cherry picked from commit
75f5977ff4ccf205ee386e4f4b0c384fb6a110d5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Tue, 5 Dec 2023 14:11:33 +0000 (15:11 +0100)]
MINOR: ssl/cli: Add ha_(warning|alert) msgs to CLI ckch callback
This patch allows cli_io_handler_commit_cert() callback called upon
a "commit ssl cert ..." command to prefix the messages returned by the CLI
to the by the ones built by ha_warining(), ha_alert().
Should be interesting to backport this commit to 2.8.
(cherry picked from commit
456ba6e95f094c084c2513a594e59670b5eac2d5)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Frédéric Lécaille [Tue, 5 Dec 2023 13:50:40 +0000 (14:50 +0100)]
BUG/MINOR: ssl: Double free of OCSP Certificate ID
This bug could be reproduced loading several certificated from "bind" line:
with "server_ocsp.pem" as argument to "crt" setting and updating
the CDSA certificate with the RSA as follows:
echo -e "set ssl cert reg-tests/ssl/ocsp_update/multicert/server_ocsp.pem.ecdsa \
<<\n$(cat reg-tests/ssl/ocsp_update/multicert/server_ocsp.pem.rsa)\n" | socat - /tmp/stats
followed by an "commit ssl cert reg-tests/ssl/ocsp_update/multicert/server_ocsp.pem.ecdsa"
command. This could be detected by libasan as follows:
=================================================================
==507223==ERROR: AddressSanitizer: attempting double-free on 0x60200007afb0 in thread T3:
#0 0x7fabc6fb5527 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x54527)
#1 0x7fabc6ae8f8c in ossl_asn1_string_embed_free (/opt/quictls/lib/libcrypto.so.81.3+0xd4f8c)
#2 0x7fabc6af54e9 in ossl_asn1_primitive_free (/opt/quictls/lib/libcrypto.so.81.3+0xe14e9)
#3 0x7fabc6af5960 in ossl_asn1_template_free (/opt/quictls/lib/libcrypto.so.81.3+0xe1960)
#4 0x7fabc6af569f in ossl_asn1_item_embed_free (/opt/quictls/lib/libcrypto.so.81.3+0xe169f)
#5 0x7fabc6af58a4 in ASN1_item_free (/opt/quictls/lib/libcrypto.so.81.3+0xe18a4)
#6 0x46a159 in ssl_sock_free_cert_key_and_chain_contents src/ssl_ckch.c:723
#7 0x46aa92 in ckch_store_free src/ssl_ckch.c:869
#8 0x4704ad in cli_release_commit_cert src/ssl_ckch.c:1981
#9 0x962e83 in cli_io_handler src/cli.c:1140
#10 0xc1edff in task_run_applet src/applet.c:454
#11 0xaf8be9 in run_tasks_from_lists src/task.c:634
#12 0xafa2ed in process_runnable_tasks src/task.c:876
#13 0xa23c72 in run_poll_loop src/haproxy.c:3024
#14 0xa24aa3 in run_thread_poll_loop src/haproxy.c:3226
#15 0x7fabc69e7ea6 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x7ea6)
#16 0x7fabc6907a2e in __clone (/lib/x86_64-linux-gnu/libc.so.6+0xfba2e)
0x60200007afb0 is located 0 bytes inside of 3-byte region [0x60200007afb0,0x60200007afb3)
freed by thread T3 here:
#0 0x7fabc6fb5527 in __interceptor_free (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x54527)
#1 0x7fabc6ae8f8c in ossl_asn1_string_embed_free (/opt/quictls/lib/libcrypto.so.81.3+0xd4f8c)
previously allocated by thread T2 here:
#0 0x7fabc6fb573f in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x5473f)
#1 0x7fabc6ae8d77 in ASN1_STRING_set (/opt/quictls/lib/libcrypto.so.81.3+0xd4d77)
Thread T3 created by T0 here:
#0 0x7fabc6f84bba in pthread_create (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x23bba)
#1 0xc04f36 in setup_extra_threads src/thread.c:252
#2 0xa2761f in main src/haproxy.c:3917
#3 0x7fabc682fd09 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x23d09)
Thread T2 created by T0 here:
#0 0x7fabc6f84bba in pthread_create (/usr/lib/x86_64-linux-gnu/libasan.so.1+0x23bba)
#1 0xc04f36 in setup_extra_threads src/thread.c:252
#2 0xa2761f in main src/haproxy.c:3917
#3 0x7fabc682fd09 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x23d09)
SUMMARY: AddressSanitizer: double-free ??:0 __interceptor_free
==507223==ABORTING
Aborted
The OCSP CID stored in the impacted ckch data were freed but not reset to NULL,
leading to a subsequent double free.
Must be backported to 2.8.
(cherry picked from commit
7dab3e8266b8de8604dbae0a90eade85c5c0e3f1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
Willy Tarreau [Tue, 5 Dec 2023 15:15:30 +0000 (16:15 +0100)]
[RELEASE] Released version 2.9.0
Released version 2.9.0 with the following main changes :
- DOC: config: add missing colon to "bytes_out" sample fetch keyword (2)
- BUG/MINOR: cfgparse-listen: fix warning being reported as an alert
- DOC: config: add matrix entry for "max-session-srv-conns"
- DOC: config: fix monitor-fail typo
- DOC: config: add context hint for proxy keywords
- DEBUG: stream: Report lra/fsb values for front end back SC in stream dump
- REGTESTS: sample: Test the behavior of consecutive delimiters for the field converter
- BUG/MINOR: sample: Make the `word` converter compatible with `-m found`
- DOC: Clarify the differences between field() and word()
- BUG/MINOR: server/event_hdl: properly handle AF_UNSPEC for INETADDR event
- BUILD: http_htx: silence uninitialized warning on some gcc versions
- MINOR: acme.sh: don't use '*' in the filename for wildcard domain
- MINOR: global: Use a dedicated bitfield to customize zero-copy fast-forwarding
- MINOR: mux-pt: Add global option to enable/disable zero-copy forwarding
- MINOR: mux-h1: Add global option to enable/disable zero-copy forwarding
- MINOR: mux-h2: Add global option to enable/disable zero-copy forwarding
- MINOR: mux-quic: Add global option to enable/disable zero-copy forwarding
- MINOR: mux-quic: Disable zero-copy forwarding for send by default
- DOC: config: update the reminder on the HTTP model and add some terminology
- DOC: config: add a few more differences between HTTP/1 and 2+
- DOC: config: clarify session vs stream
- DOC: config: fix typo abandonned -> abandoned
- DOC: management: fix two latest typos (optionally, exception)
- BUG/MEDIUM: peers: fix partial message decoding
- DOC: management: update stream vs session
Willy Tarreau [Tue, 5 Dec 2023 08:30:44 +0000 (09:30 +0100)]
DOC: management: update stream vs session
Indicate for some commands such as "show sess" that we now dump streams
and not sessions.
Christopher Faulet [Tue, 5 Dec 2023 08:21:38 +0000 (09:21 +0100)]
BUG/MEDIUM: peers: fix partial message decoding
peer_recv_msg() may return because the message is incomplete without
checking if a shutdown is pending for the SC. The function relies on
co_getblk() to detect shutdowns. However, the message length decoding may be
interrupted if the multi-bytes integer is incomplete. In this case, the SC
is not check for shutdowns.
When this happens, this leads to an appctx spinning loop.
This patch should fix the issue #2373. It must be backported to 2.8.
Willy Tarreau [Tue, 5 Dec 2023 03:04:50 +0000 (04:04 +0100)]
DOC: management: fix two latest typos (optionally, exception)
No backport needed, these were introduced by latest commits
3dd55fa13
("MINOR: mworker/cli: implement hard-reload over the master CLI") and
cef29d370 ("MINOR: trace: define simple -dt argument").
Willy Tarreau [Tue, 5 Dec 2023 03:02:25 +0000 (04:02 +0100)]
DOC: config: fix typo abandonned -> abandoned
No need to backport, it was introduced by recent commit
fafa34e5f ("DOC:
config: update the reminder on the HTTP model and add some terminology").
Willy Tarreau [Mon, 4 Dec 2023 17:57:12 +0000 (18:57 +0100)]
DOC: config: clarify session vs stream
Rename "session" to "stream" where relevant (termination states, queue,
and so on).
Willy Tarreau [Mon, 4 Dec 2023 17:34:19 +0000 (18:34 +0100)]
DOC: config: add a few more differences between HTTP/1 and 2+
Mention the lack of reason phrase and the existence of pseudo-headers for
the request and the response.
Willy Tarreau [Mon, 4 Dec 2023 17:16:52 +0000 (18:16 +0100)]
DOC: config: update the reminder on the HTTP model and add some terminology
It was really necessary to try to clear the confusion between sessions
and streams, so let's first lift a little bit the HTTP model part to
better consider new protocols, and explain what a stream is and how this
differs from the earlier sessions.
Christopher Faulet [Mon, 4 Dec 2023 14:36:01 +0000 (15:36 +0100)]
MINOR: mux-quic: Disable zero-copy forwarding for send by default
There is at least an bug for now in this part and it is still unstable. Thus
it is better to disable it for now by default. It can be enable by setting
tune.quic.zero-copy-fwd-send to 'on'.
Christopher Faulet [Mon, 4 Dec 2023 14:26:23 +0000 (15:26 +0100)]
MINOR: mux-quic: Add global option to enable/disable zero-copy forwarding
tune.quic.zero-copy-fwd-send can now be used to enable or disable the
zero-copy fast-forwarding for the QUIC mux only, for sends. For now, there
is no option to disable it for receives because it is not supported yet.
It is enabled ('on') by default.
Christopher Faulet [Mon, 4 Dec 2023 14:18:49 +0000 (15:18 +0100)]
MINOR: mux-h2: Add global option to enable/disable zero-copy forwarding
tune.h2.zero-copy-fwd-send can now be used to enable or disable the
zero-copy fast-forwarding for the H2 mux only, for sends. For now, there is
no option to disable it for receives because it is not supported yet.
It is enabled ('on') by default.
Christopher Faulet [Mon, 4 Dec 2023 14:06:06 +0000 (15:06 +0100)]
MINOR: mux-h1: Add global option to enable/disable zero-copy forwarding
tune.h1.zero-copy-fwd-recv and tune.h1.zero-copy-fwd-send can now be used to
enable or disable the zero-copy fast-forwarding for the H1 mux only, for
receives or sends. Unlike the PT mux, there are 2 options here because
client and server sides can use difference muxes.
Both are enabled ('on') by default.
Christopher Faulet [Mon, 4 Dec 2023 13:48:52 +0000 (14:48 +0100)]
MINOR: mux-pt: Add global option to enable/disable zero-copy forwarding
tune.pt.zero-copy-forwarding parameter can now be used to enable or disable
the zero-copy fast-forwarding for the PT mux only. It is enabled ('on') by
default. It can be disabled by setting the parameter to 'off'. In this case,
this disables receive and send side.
Christopher Faulet [Mon, 4 Dec 2023 13:18:50 +0000 (14:18 +0100)]
MINOR: global: Use a dedicated bitfield to customize zero-copy fast-forwarding
Zero-copy fast-forwading feature is a quite new and is a bit sensitive.
There is an option to disable it globally. However, all protocols have not
the same maturity. For instance, for the PT multiplexer, there is nothing
really new. The zero-copy fast-forwading is only another name for the kernel
splicing. However, for the QUIC/H3, it is pretty new, not really optimized
and it will evolved. And soon, the support will be added for the cache
applet.
In this context, it is usefull to be able to enable/disable zero-copy
fast-forwading per-protocol and applet. And when it is applicable, on sends
or receives separately. So, instead of having one flag to disable it
globally, there is now a dedicated bitfield, global.tune.no_zero_copy_fwd.
William Lallemand [Mon, 4 Dec 2023 10:52:31 +0000 (11:52 +0100)]
MINOR: acme.sh: don't use '*' in the filename for wildcard domain
By default acme.sh uses the '*' character in the filename for wildcard.
That can be confusing within HAProxy since the * character in front of a
filename in the stat socket is used to specified an uncommitted
transaction.
This patch replace the '*' by a '_' in the filename. This is only done
when using the default filename, the name can still be forced with an
asterisk.
Willy Tarreau [Fri, 1 Dec 2023 14:29:44 +0000 (15:29 +0100)]
BUILD: http_htx: silence uninitialized warning on some gcc versions
Building on gcc 4.4 reports "start may be used uninitialized". This is
a classical case of dependency between two variables where the compiler
lost track of their initialization and doesn't know that if one is not
set, the other is. By just moving the second test in the else clause
of the assignment both fixes it and makes the code more efficient, and
this can be simplified as a ternary operator.
It's probably not needed to backport this, unless anyone reports build
warnings with more recent compilers (intermediary optimization levels
such as -O1 can sometimes trigger such warnings).
Aurelien DARRAGON [Fri, 1 Dec 2023 16:41:40 +0000 (17:41 +0100)]
BUG/MINOR: server/event_hdl: properly handle AF_UNSPEC for INETADDR event
It is possible that a server's addr family is temporarily set to AF_UNSPEC
even if we're certain to be in INET context (ipv4, ipv6).
Indeed, as soon as IP address resolving is involved, srv->addr family will
be set to AF_UNSPEC when the resolution fails (could happen at anytime).
However, _srv_event_hdl_prepare_inetaddr() wrongly assumed that it would
only be called with AF_INET or AF_INET6 families. Because of that, the
function will handle AF_UNSPEC address as an IPV6 address: not only
we could risk reading from an unititialized area, but we would then
propagate false information when publishing the event.
In this patch we make sure to properly handle the AF_UNSPEC family in
both the "prev" and the "next" part for SERVER_INETADDR event and that
every members are explicitly initialized.
This bug was introduced by
6fde37e046 ("MINOR: server/event_hdl: add
SERVER_INETADDR event"), no backport needed.
Tim Duesterhus [Thu, 30 Nov 2023 15:41:18 +0000 (16:41 +0100)]
DOC: Clarify the differences between field() and word()
word() mentions that delimiters at the start and end are ignored, but
it does not mention that consecutive delimiters are merged.
May be backported as far as the patch applies.
Tim Duesterhus [Thu, 30 Nov 2023 15:41:17 +0000 (16:41 +0100)]
BUG/MINOR: sample: Make the `word` converter compatible with `-m found`
Previously an expression like:
path,word(2,/) -m found
always returned `true`.
Bug exists since the `word` converter exists. That is:
c9a0f6d0232cf44d6b08d1964b9097a45a6c65f0
The same bug was previously fixed for the `field` converter in commit
4381d26edc03faa46401eb0fe82fd7be84be14fd.
The fix should be backported to 1.6+.
Tim Duesterhus [Thu, 30 Nov 2023 15:41:16 +0000 (16:41 +0100)]
REGTESTS: sample: Test the behavior of consecutive delimiters for the field converter
This is in preparation of a follow-up patch to fix the word converter.
Christopher Faulet [Fri, 1 Dec 2023 10:16:53 +0000 (11:16 +0100)]
DEBUG: stream: Report lra/fsb values for front end back SC in stream dump
REX and WEX date are already reported. But if the corresponding SC cannot
expire on read or write, "<NEVER>" is reported instead. The same is reported
if no expiration date is set. It is not really convenient because we cannot
distinguish the two cases.
So, now, for each SC, read and wirte timer (rto/wto) are also reported in
the dump, based on .lra/.fsb dates and the current I/O timeout. The SC I/O
timeout is also reported.
Aurelien DARRAGON [Mon, 20 Nov 2023 16:53:17 +0000 (17:53 +0100)]
DOC: config: add context hint for proxy keywords
Add a small list of contexts where each proxy keyword is expected to be
employed. (Similar to the defaults/frontend/backend/listen compatibility
grid).
Aurelien DARRAGON [Thu, 30 Nov 2023 10:11:43 +0000 (11:11 +0100)]
DOC: config: fix monitor-fail typo
monitor-fail doesn't exist, but it was mentionned in the documentation.
Fixing it with "monitor fail" instead.
Aurelien DARRAGON [Wed, 29 Nov 2023 09:13:18 +0000 (10:13 +0100)]
DOC: config: add matrix entry for "max-session-srv-conns"
Following 4039329 ("DOC: config: specify supported sections for
"max-session-srv-conns"), "max-session-srv-conns" was also missing
from the proxy keyword matrix.
Aurelien DARRAGON [Wed, 29 Nov 2023 14:03:25 +0000 (15:03 +0100)]
BUG/MINOR: cfgparse-listen: fix warning being reported as an alert
Since
b40542000d ("MEDIUM: proxy: Warn about ambiguous use of named
defaults sections") we introduced a new error to prevent user from
having an ambiguous named default section in the config which is both
inherited explicitly using "from" and implicitly by another proxy due to
the default section being the last one defined.
However, despite the error message being presented as a warning the
err_code, the commit message and the documentation, it is actually
reported as a fatal error because ha_alert() was used in place of
ha_warning().
In this patch we make the code comply with the documentation and the
intended behavior by using ha_warning() to report the error message.
This should be backported up to 2.6.
Tim Duesterhus [Thu, 30 Nov 2023 19:15:32 +0000 (20:15 +0100)]
DOC: config: add missing colon to "bytes_out" sample fetch keyword (2)
This reapplies
1eb049dc677f2de950158615ed3d8306ee5102d6, as the change was
accidentally reverted in
5ef48e063ecf992646c7af374153f106050fb8ec.
Willy Tarreau [Thu, 30 Nov 2023 17:07:06 +0000 (18:07 +0100)]
[RELEASE] Released version 2.9-dev12
Released version 2.9-dev12 with the following main changes :
- BUG/MINOR: global: Fix tune.disable-(fast-forward/zero-copy-forwarding) options
- DOC: config: removing "log-balance" references
- MINOR: server/event_hdl: add SERVER_INETADDR event
- MINOR: tools: use const for read only pointers in ip{cmp,cpy}
- MINOR: server/ip: centralize server ip updates
- MINOR: backend: remove invalid mode test for "hash-balance-factor"
- Revert "MINOR: cfgparse-listen: warn when use-server rules is used in wrong mode"
- MINOR: proxy: add free_logformat_list() helper function
- MINOR: proxy: add free_server_rules() helper function
- MINOR: log/backend: prevent "use-server" rules use with LOG mode
- MINOR: log/balance: set lbprm tot_weight on server on queue/dequeue
- DOC: config: specify supported sections for "max-session-srv-conns"
- DOC: config: fix timeout check inheritance restrictions
- REGTESTS: connection: disable http_reuse_be_transparent.vtc if !TPROXY
- DOC: lua: add sticktable class reference from Proxy.stktable
- DOC: lua: fix Proxy.get_mode() output
- DOC: lua: add "syslog" to Proxy.get_mode() output
- MEDIUM: ssl: implement rsa/ecdsa selection with WolfSSL
- MINOR: ssl: replace 'trash.area' by 'servername' in ssl_sock_switchctx_cbk()
- MINOR: ssl: move certificate selection in a dedicate function
- MEDIUM: ssl: use ssl_sock_chose_sni_ctx() in the clienthello callback
- MINOR: mworker/cli: implement hard-reload over the master CLI
- BUG/MEDIUM: mux-h1: Properly ignore trailers when a content-length is announced
- MINOR: task/profiling: do not record task_drop_running() as a caller
- OPTIM: pattern: save memory and time using ebst instead of ebis
- BUILD: map: fix build warning
- MINOR: trace: define simple -dt argument
- MINOR: trace: parse level in a function
- MINOR: trace: parse verbosity in a function
- MINOR: trace: support -dt optional format
- OPTIM: mux-h2/zero-copy: don't allocate more buffers per connections than streams
- BUG/MINOR: quic: fix CONNECTION_CLOSE_APP encoding
- BUG/MEDIUM: stconn: Don't perform zero-copy FF if opposite SC is blocked
- BUG/MEDIUM: mux-h2: Remove H2_SF_NOTIFIED flag for H2S blocked on fast-forward
- CLEANUP: quic: Remove dead definitions/declarations
- REORG: quic: Move some QUIC CLI code to its C file
- REORG: quic: Add a new module to handle QUIC connection IDs
- REORG: quic: QUIC connection types header cleaning
- BUILD: quic: Missing RX header inclusions
- REORG: quic: Move CRYPTO data buffer defintions to QUIC TLS module
- REORG: quic: Move QUIC CRYPTO stream definitions/declarations to QUIC TLS
- REORG: quic: Move several inlined functions from quic_conn.h
- REORG: quic: Move QUIC SSL BIO method related functions to quic_ssl.c
- REORG: quic: Move the QUIC DCID parser to quic_sock.c
- REORG: quic: Rename some functions used upon ACK receipt
- REORG: quic: Move QUIC path definitions/declarations to quic_cc module
- REORG: quic: Move qc_handle_conn_migration() to quic_conn.c
- REORG: quic: Move quic_build_post_handshake_frames() to quic_conn module
- REORG: quic: Move qc_may_probe_ipktns() to quic_tls.h
- REORG: quic: Move qc_pkt_long() to quic_rx.h
- REORG: quic: Rename some (quic|qc)_conn* objects to quic_conn_closed
- REORG: quic: Move NEW_CONNECTION_ID frame builder to quic_cid
- REORG: quic: Move ncbuf related function from quic_rx to quic_conn
- REORG: quic: Add a new module for QUIC retry
- BUILD: quic: Several compiler warns fixes after retry module creation
- REORG: quic: Move qc_notify_send() to quic_conn
- REORG: quic: Add a new module for retransmissions
- REORG: quic: Remove qc_pkt_insert() implementation
- REORG: quic: Move quic_increment_curr_handshake() to quic_sock
- BUG/MINOR: cache: Remove incomplete entries from the cache when stream is closed
- MEDIUM: cli: allow custom pattern for payload
- CLEANUP: mworker/cli: use a label to return errors
- MINOR: mworker/cli: implements the customized payload pattern for master CLI
- DOC: management: add documentation about customized payload pattern
- BUG/MEDIUM: server/event_hdl: memory overrun in _srv_event_hdl_prepare_inetaddr()
- MINOR: event_hdl: add global tunables
- BUG/MAJOR: server/addr: fix a race during server addr:svc_port updates
- MEDIUM: log/balance: support FQDN for UDP log servers
- BUG/MINOR: compression: possible NULL dereferences in comp_prepare_compress_request()
- BUG/MEDIUM: master/cli: Properly pin the master CLI on thread 1 / group 1
- BUG/MEDIUM: mux-quic: Stop zero-copy FF during nego if input is not empty
- CLEANUP: log: Fix %rc comment in sess_build_logline()
- BUG/MINOR: h3: fix TRAILERS encoding
- BUG/MINOR: h3: always reject PUSH_PROMISE
- MINOR: h3: use correct error code for missing SETTINGS
- MINOR: http-fetch: Add a sample to retrieve the server status code
- DOC: config: Improve 'status' sample documentation
- MINOR: http-fetch: Add a sample to get the transaction status code
- MEDIUM: http-ana: Set termination state before returning haproxy response
- MINOR: stream: Expose session terminate state via a new sample fetch
- MINOR: stream: add a sample fetch to get the number of connection retries
- MINOR: stream: Expose the stream's uniq_id via a new sample fetch
- MINOR: muxes: Rename mux_ctl_type values to use MUX_CTL_ prefix
- MINOR: muxes: Add a callback function to send commands to mux streams
- MINOR: muxes: Implement ->sctl() callback for muxes and return the stream id
- MINOR: Add sample fetches to get the frontend and backend stream ID
- BUG/MEDIUM: cli: Don't look for payload pattern on empty commands
- DOC: config: Add argument for tune.lua.maxmem
- DOC: config: fix mention of request slot in http-response capture
- DOC: config: fix remaining mention of @reverse for attach-srv action
- DOC: config: fix missing characters in set-spoe-group action
- DOC: config: reorganize actions into their own section
- BUG/MINOR: acme.sh: update the deploy script
- MINOR: rhttp: mark reverse HTTP as experimental
- CLEANUP: quic_cid: remove unused listener arg
- BUG/MINOR: quic_tp: fix preferred_address decoding
- MINOR: quic_tp: use in_addr/in6_addr for preferred_address
- MINOR: acme.sh: use the master CLI for hot update
- DOC: config: move the cache-use and cache-store actions to the proper section
- DOC: config: fix alphabetical ordering of converter keywords
- DOC: config: add missing colon to "bytes_out" sample fetch keyword
- DOC: config: add an index of converter keywords
- DOC: config: add an index of sample fetch keywords
- BUG/MINOR: config: Stopped parsing upon unmatched environment variables
- DEBUG: unstatify a few functions that are often present in backtraces
- BUILD: server: shut a bogus gcc warning on certain ubuntu
Willy Tarreau [Thu, 30 Nov 2023 16:48:03 +0000 (17:48 +0100)]
BUILD: server: shut a bogus gcc warning on certain ubuntu
On ubuntu 20.04 and 22.04 with gcc 9.4 and 11.4 respectively, we get
the following warning:
src/server.c: In function 'srv_update_addr_port':
src/server.c:4027:3: warning: 'new_port' may be used uninitialized in this function [-Wmaybe-uninitialized]
4027 | _srv_event_hdl_prepare_inetaddr(&cb_data.addr, &s->addr, s->svc_port,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4028 | ((ip_change) ? &sa : &s->addr),
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4029 | ((port_change) ? new_port : s->svc_port),
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
4030 | 1);
| ~~
It's clearly wrong, port_change only changes from 0 to anything else
*after* assigning new_port. Let's just preset new_port to zero instead
of trying to play smart with the compiler.
Willy Tarreau [Thu, 30 Nov 2023 16:04:16 +0000 (17:04 +0100)]
DEBUG: unstatify a few functions that are often present in backtraces
It's useful to be able to recognize certain functions that are often
present in backtraces as they call lower level functions, and for this
they must not be static. Let's remove "static" in front of these
functions:
sc_notify, sc_conn_recv, sc_conn_send, sc_conn_process,
sc_applet_process, back_establish, stream_update_both_sc,
httpclient_applet_io_handler, httpclient_applet_init,
httpclient_applet_release
Frédéric Lécaille [Thu, 30 Nov 2023 08:26:05 +0000 (09:26 +0100)]
BUG/MINOR: config: Stopped parsing upon unmatched environment variables
When an environment variable could not be matched by getenv(), the
current character to be parsed by parse_line() from <in> variable
is the trailing double quotes. If nothing is done in such a case,
this character is skipped by parse_line(), then the following spaces
are parsed as an empty argument.
To fix this, skip the double quotes character and the following spaces
to make <in> variable point to the next argument to be parsed.
Thank you to @sigint2 for having reported this issue in GH #2367.
Must be backported as far as 2.4.
Willy Tarreau [Thu, 30 Nov 2023 15:00:14 +0000 (16:00 +0100)]
DOC: config: add an index of sample fetch keywords
Now we're adding a table for each section, it allows to more easily
spot the list of available sample fetch functions and their types.
For now the arguments are not mentioned in the table because they'd
break indexing but they can be added back later.
Willy Tarreau [Thu, 30 Nov 2023 15:03:55 +0000 (16:03 +0100)]
DOC: config: add an index of converter keywords
The goal here is to have a centralized list of converters and
their in/out types.
Willy Tarreau [Thu, 30 Nov 2023 15:03:19 +0000 (16:03 +0100)]
DOC: config: add missing colon to "bytes_out" sample fetch keyword
The colon was missing between the keyword and the type, breaking
rendering and indexing.
Willy Tarreau [Thu, 30 Nov 2023 10:55:22 +0000 (11:55 +0100)]
DOC: config: fix alphabetical ordering of converter keywords
- rfc7239_* were misplaced and incorrectly ordered
- table_gpt was placed before some table_gpc*
- capture-req/res were misplaced
- htonl was misplaced
- upper/url_* were misplaced
- x509_v_err_str was misplaced
Let's fix these since poor ordering complicates their finding.
Willy Tarreau [Thu, 30 Nov 2023 15:25:58 +0000 (16:25 +0100)]
DOC: config: move the cache-use and cache-store actions to the proper section
Actions were grouped by previous commit
d54e8f810 ("DOC: config: reorganize
actions into their own section") but cache-use and cache-store were still
making reference to the cache section. This moves the text back to their
respective keywords in the actions section and leaves the example and an
explanation of how to use the keywords in the cache section.
William Lallemand [Thu, 30 Nov 2023 15:15:45 +0000 (16:15 +0100)]
MINOR: acme.sh: use the master CLI for hot update
DEPLOY_HAPROXY_MASTER_CLI allows to use the HAProxy master CLI
instead of a stats socket for DEPLOY_HAPROXY_HOT_UPDATE="yes"
The syntax of the master CLI is slightly different, a prefix with
the process number need to be added before any command.
This patch uses ${_cmdpfx} in front of every socat commands which is
filled when the master CLI is used.
Amaury Denoyelle [Thu, 30 Nov 2023 14:36:10 +0000 (15:36 +0100)]
MINOR: quic_tp: use in_addr/in6_addr for preferred_address
preferred_address is a transport parameter specify by the server. It
specified both an IPv4 and IPv6 address. These addresses were defined as
plain array in <struct tp_preferred_address>.
Convert these adressees to use the common types in_addr/in6_addr. With
this change, dumping of preferred_address is extended. It now displays
the addresses using inet_ntop() and CID value.
Amaury Denoyelle [Thu, 30 Nov 2023 13:01:51 +0000 (14:01 +0100)]
BUG/MINOR: quic_tp: fix preferred_address decoding
quic_transport_param_dec_pref_addr() is responsible to decode
preferred_address from received transport parameter. There was two
issues with this function :
* address and port location as defined in RFC were inverted for both
IPv4 and IPv6 during decoding
* an invalid check was done to ensure decoded CID length corresponds to
remaining buffer size. It did not take into account the final field
for stateless reset token.
These issues were never encountered as only server can emit
preferred_address transport parameter, so the impact of this bug is
invisible.
This should be backported up to 2.6.
Amaury Denoyelle [Thu, 30 Nov 2023 13:00:56 +0000 (14:00 +0100)]
CLEANUP: quic_cid: remove unused listener arg
retrieve_qc_conn_from_cid() requires listener as argument whereas it is
unused. This is an artifact from the old architecture where CID trees
where stored on listener instances instead of globally. Remove it to
better reflect this change.
Amaury Denoyelle [Thu, 30 Nov 2023 13:28:47 +0000 (14:28 +0100)]
MINOR: rhttp: mark reverse HTTP as experimental
Mark the reverse HTTP feature as experimental. This will allow to adjust
if needed the configuration mechanism with future developments without
maintaining retro-compatibility.
Concretely, each config directives linked to it now requires to specify
first global expose-experimental-directives before. This is the case for
the following directives :
- rhttp@ prefix uses in bind and server lines
- nbconn bind keyword
- attach-srv tcp rule
Each documentation section refering to these keywords are updated to
highlight this new requirement.
Note that this commit has duplicated on several places the code from the
global function check_kw_experimental(). This is because the latter only
work with cfg_keyword type. This is not adapted with bind_kw or
action_kw types. This should be improve in a future patch.
William Lallemand [Thu, 30 Nov 2023 13:15:06 +0000 (14:15 +0100)]
BUG/MINOR: acme.sh: update the deploy script
https://github.com/acmesh-official/acme.sh/pull/4581 was updated, this
patch update the haproxy repository with the update.
the following changes were done:
- sanitize the PEM to remove the '\n' (truncated certicate chain)
- shellcheck fixes
- socat format is directly used in the DEPLOY_HAPROXY_STATS_SOCKET variable
Willy Tarreau [Thu, 30 Nov 2023 08:28:39 +0000 (09:28 +0100)]
DOC: config: reorganize actions into their own section
The split of the rulesets from their respective actions has long been
overdue so it's time to do it because it has become extremely difficult
to add simple actions in the documentation, as well as it's hard to find
them.
This commit creates two new sections "4.3 Actions keywords matrix" and
"4.4 Alphabetically sorted actions reference" that enumerates all known
actions, with a check indicating for which rule sets they're valid. This
removes all the repetition, occurrences of "see http-request blah for
details" and significantly reduces the number of keywords listed in the
proxies section. This removes 2245 lines from the proxies section in
exchange of 1608 in these new sections.
Willy Tarreau [Thu, 30 Nov 2023 08:27:51 +0000 (09:27 +0100)]
DOC: config: fix missing characters in set-spoe-group action
It was written "Thaction" instead of "This action".
Willy Tarreau [Thu, 30 Nov 2023 08:26:38 +0000 (09:26 +0100)]
DOC: config: fix remaining mention of @reverse for attach-srv action
The new address is "rhttp@".
Willy Tarreau [Thu, 30 Nov 2023 08:24:21 +0000 (09:24 +0100)]
DOC: config: fix mention of request slot in http-response capture
It's response slot, not request slot.
Olivier Duclos [Wed, 29 Nov 2023 11:08:12 +0000 (12:08 +0100)]
DOC: config: Add argument for tune.lua.maxmem
Make it clear that tune.lua.maxmem expects a number.
Christopher Faulet [Wed, 29 Nov 2023 12:20:59 +0000 (13:20 +0100)]
BUG/MEDIUM: cli: Don't look for payload pattern on empty commands
A regression was introduced by commit
9431aa0bdf ("BUG/MEDIUM: cli: Don't
look for payload pattern on empty commands").
On empty commands (really empty or containing spaces and tabs), the number of
arguments set to 0. However we look for the payload pattern without checking
it. The result is an access at the index -1 in the argument array. It is of
course invalid.
To fix the issue, we just skip this part when there is no argument. Note that
the empty command is still sent to the worker.
This patch should solve the issue #2365. No backport needed.
Christopher Faulet [Tue, 28 Nov 2023 15:34:23 +0000 (16:34 +0100)]
MINOR: Add sample fetches to get the frontend and backend stream ID
"fc.id" and "bc.id" sample fetches can now be used to get, respectively, the
frontend or the backend stream ID. They rely on ->sctl() callback function
on the mux attached to the corresponding SC.
It means these sample fetches work only for connection, not applets, and
from the time a multiplexer is installed.
Christopher Faulet [Tue, 28 Nov 2023 14:12:51 +0000 (15:12 +0100)]
MINOR: muxes: Implement ->sctl() callback for muxes and return the stream id
All muxes now implements the ->sctl() callback function and are able to
return the stream ID. For the PT multiplexer, it is always 0. For the H1
multiplexer it is the request count for the current H1 connection (added for
this purpose). The FCGI, H2 and QUIC muxes, the stream ID is returned.
The stream ID is returned as a signed 64 bits integer.
Christopher Faulet [Tue, 28 Nov 2023 13:53:58 +0000 (14:53 +0100)]
MINOR: muxes: Add a callback function to send commands to mux streams
Just like the ->ctl() callback function, used to send commands to mux
connections, the ->sctl() callback function can now be used to send commands
to mux streams. The first command, MUX_SCTL_SID, is a way to request the mux
stream ID.
It will be implemented later for each mux.
Christopher Faulet [Tue, 28 Nov 2023 13:27:51 +0000 (14:27 +0100)]
MINOR: muxes: Rename mux_ctl_type values to use MUX_CTL_ prefix
Instead of the generic MUX_, we now use MUX_CTL_ prefix for all mux_ctl_type
value. This will avoid any ambiguities with other enums, especially with a
new one that will be added to get information on mux streams.
Christopher Faulet [Tue, 28 Nov 2023 20:01:18 +0000 (21:01 +0100)]
MINOR: stream: Expose the stream's uniq_id via a new sample fetch
"txn.id32" may now be used to get the stream's uniq_id. It is equivalent to
%rt in logs.
Christopher Faulet [Tue, 28 Nov 2023 12:59:59 +0000 (13:59 +0100)]
MINOR: stream: add a sample fetch to get the number of connection retries
"txn.conn_retries" can now be used to get the number of connection
retries. This value is only stable once the connection is fully
established. For HTTP sessions, L7-retries must also be passed.
Christopher Faulet [Tue, 28 Nov 2023 07:45:33 +0000 (08:45 +0100)]
MINOR: stream: Expose session terminate state via a new sample fetch
It is now possible to retrieve the session terminate state, using
"txn.sess_term_state". The sample fetch returns the 2-character session
termation state.
Of course, the result of this sample fetch is volatile. It is subject to
change. It is also most of time useless because no termation state is set
except at the end. It should only be useful in http-after-response rule
sets. It may also be used to customize the logs using a log-format
directive.
This patch should fix the issue #2221.
Christopher Faulet [Tue, 28 Nov 2023 07:38:45 +0000 (08:38 +0100)]
MEDIUM: http-ana: Set termination state before returning haproxy response
When, for any reason and at any step, HAProxy decides to interrupt an HTTP
transaction, it returns a dedicated responses to the client (possibly empty)
and it sets the stream flags used to produce the session termination state.
These both operation were performed in any order, depending on the code
path. Most of time, the HAPRoxy response is produced first.
With this patch, the stream flags for the termination state are now set
first. This way, these flags become visible from http-after-reponse rule
sets. Only errors when the HAProxy response is generated are reported later.
Christopher Faulet [Mon, 27 Nov 2023 17:49:47 +0000 (18:49 +0100)]
MINOR: http-fetch: Add a sample to get the transaction status code
It was possible get the status code in the HTTP response and the one
received from the server. Thanks to 'txn.status', it is now possible to get
the transaction status code. It is equivalent to '%ST' in log-format.
Most of time, it is the same than 'status', except if the status code of the
HTTP reply does not match the one used to interrupt the transaction. For
instance, an error file use mapped on 400 containing a 404.
Christopher Faulet [Mon, 27 Nov 2023 17:27:47 +0000 (18:27 +0100)]
DOC: config: Improve 'status' sample documentation
We clearly state the 'status' sample returns the status code the client will
receive, if no change happens on the HTTP response. This should avoid
ambiguities with the 'server-status' sample fetch.
Christopher Faulet [Mon, 27 Nov 2023 17:12:13 +0000 (18:12 +0100)]
MINOR: http-fetch: Add a sample to retrieve the server status code
The code returned by the "status" sample fetch is the one in the HTTP
response at the moment the sample is evaluated. It may be the status code in
the server response or the one of the HAProxy reply in case of error, deny,
redirect...
However, it could be handy to retrieve the status code returned by the
server, when a HTTP response was really received from it. It is the purpose
of the "server_status" sample fetch. The server status code itself is stored
in the HTTP txn.
Amaury Denoyelle [Tue, 28 Nov 2023 15:01:38 +0000 (16:01 +0100)]
MINOR: h3: use correct error code for missing SETTINGS
Each received HTTP/3 frame is checked to ensure it is valid given the
type of stream and its current status. This was implemented via
h3_is_frame_valid().
Previously, no distinction was made for error code, so every failure
triggered a CONNECTION_CLOSE_APP with code H3_FRAME_UNEXPECTED. However,
this function also ensures that the first frame received on control
frame is of type SETTINGS. If not, the error code to use is
H3_MISSING_SETTINGS.
To support this, adjust the function prototype. Instead of returning a
boolean, 0 is returned for success, or a HTTP/3 error code. The function
is renamed h3_check_frame_valid() to reflects the return type change.
This is not considered as a bug as previously the connection was
correctly closed on a missing SETTINGS, albeit with a non conform error
code. It's not deemed as sufficient to be backported.
Amaury Denoyelle [Tue, 28 Nov 2023 11:00:40 +0000 (12:00 +0100)]
BUG/MINOR: h3: always reject PUSH_PROMISE
The condition for checking PUSH_PROMISE was not correctly interpreted
from the RFC. Initially, it rejects such a frame for every stream
initiated from client side.
In fact, the RFC indicates that PUSH_PROMISE are never sent by a client.
Thus, it can be rejected in any case until HTTP/3 will be implemented on
the backend side.
This should be backported up to 2.6.
Amaury Denoyelle [Tue, 28 Nov 2023 14:59:38 +0000 (15:59 +0100)]
BUG/MINOR: h3: fix TRAILERS encoding
HTTP/3 trailers encoding was never working as intended. It's because
h3_trailers_to_htx() manipulate a newly allocated buffer instead of the
already existing channel one. Thus, HTX message handled by the stream
was incomplete as it lacked trailers and EOM.
Fix this by reusing the already allocated channel buffer in
h3_trailers_to_htx().
This bug was detected by simulating TRAILERS emission which generate
CL--- state due to missing request side termination signal. Its impact
is deemed as minimal as trailers are pretty infrequent for now in
HTTP/3.
This must be backported up to 2.7.