Le 18/04/2025 à 1:38 PM, Miroslav Zagorac a écrit :
Hello all,
after processing the agent hello frame, the max-frame-size parameter for the
agent should be set; otherwise HAProxy can send frames longer than the one
that the agent will be able to process - which can lead to the crash of the
agent program.
This should be backported to version 3.1.
Thanks Miroslav ! Indeed there is an issue. However your fix is not the right
one. The agent configuration must not be changed this way. In fact, it must not
be changed at all. It represents the configuration settings. The max-frame-size
value that must be used is negotiated for a connection. Two connections to the
same agent may have different value. It is unusual but possible.
The right fix is to check the frame size before sending it to the agent.
DISCONNECT and NOTIFY frames are concerned. You can find the patch in attachement.
--
Christopher Faulet
From 6c24a1e729697c2fd57f4bd479ffa965d774e1f4 Mon Sep 17 00:00:00 2001
From: Christopher Faulet <cfau...@haproxy.com>
Date: Tue, 22 Apr 2025 15:27:12 +0200
Subject: [PATCH] BUG/MEDIUM: mux-spop: Respect the negociated max-frame-size
value to send frames
When a SPOP connection is opened, the maximum size for frames is negociated.
This negociated size is properly used when a frame is received and if a too
big frame is detected, an error is triggered. However, the same was not
performed on the sending path. No check was performed on frames sent to the
agent. So it was possible to send frames bigger than the maximum size
supported by the the SPOE agent.
Now, the size of NOTIFY and DISCONNECT frames is checked before sending them
to the agent.
Thanks to Miroslav to have reported the issue.
This patch must be backported to 3.1.
---
src/mux_spop.c | 28 +++++++++++++++++++---------
1 file changed, 19 insertions(+), 9 deletions(-)
diff --git a/src/mux_spop.c b/src/mux_spop.c
index 7cb49363c..6bd520362 100644
--- a/src/mux_spop.c
+++ b/src/mux_spop.c
@@ -1547,6 +1547,9 @@ static int spop_conn_send_disconnect(struct spop_conn *spop_conn)
outbuf.data += p - b_tail(&outbuf);
+ if (outbuf.data - 4 > spop_conn->max_frame_size)
+ goto too_big;
+
/* update the frame's size now */
TRACE_PROTO("SPOP HAPROXY DISCONNECT frame xferred", SPOP_EV_TX_FRAME|SPOP_EV_TX_DISCO, spop_conn->conn, 0, 0, (size_t[]){outbuf.data});
spop_set_frame_size(outbuf.area, outbuf.data - 4);
@@ -1596,10 +1599,8 @@ static int spop_conn_send_disconnect(struct spop_conn *spop_conn)
return ret;
full:
/* Too large to be encoded. For DISCONNECT frame, it is an error */
- if (!b_data(mbuf)) {
- TRACE_ERROR("SPOP HAPROXY DISCO frame too large", SPOP_EV_TX_FRAME|SPOP_EV_TX_DISCO|SPOP_EV_SPOP_CONN_ERR, spop_conn->conn);
- goto fail;
- }
+ if (!b_data(mbuf))
+ goto too_big;
if ((mbuf = br_tail_add(spop_conn->mbuf)) != NULL)
goto retry;
@@ -1608,6 +1609,10 @@ static int spop_conn_send_disconnect(struct spop_conn *spop_conn)
TRACE_STATE("mbuf ring full", SPOP_EV_TX_FRAME|SPOP_EV_SPOP_CONN_BLK, spop_conn->conn);
ret = 0;
goto end;
+
+ too_big:
+ TRACE_ERROR("SPOP HAPROXY DISCO frame too large", SPOP_EV_TX_FRAME|SPOP_EV_TX_DISCO|SPOP_EV_SPOP_CONN_ERR, spop_conn->conn);
+
fail:
spop_conn->state = SPOP_CS_CLOSED;
TRACE_STATE("switching to CLOSED", SPOP_EV_TX_FRAME|SPOP_EV_TX_DISCO|SPOP_EV_SPOP_CONN_END, spop_conn->conn);
@@ -3215,6 +3220,9 @@ static size_t spop_strm_send_notify(struct spop_strm *spop_strm, struct buffer *
outbuf.data += p - b_tail(&outbuf);
+ if (outbuf.data - 4 > spop_conn->max_frame_size)
+ goto too_big;
+
/* update the frame's size now */
TRACE_PROTO("SPOP HAPROXY NOTIFY frame xferred", SPOP_EV_TX_FRAME|SPOP_EV_TX_NOTIFY, spop_conn->conn, spop_strm, 0, (size_t[]){outbuf.data});
spop_set_frame_size(outbuf.area, outbuf.data - 4);
@@ -3228,11 +3236,8 @@ static size_t spop_strm_send_notify(struct spop_strm *spop_strm, struct buffer *
return ret;
full:
/* Too large to be encoded. For NOTIFY frame, it is an error */
- if (!b_data(mbuf)) {
- TRACE_ERROR("SPOP HAPROXY NOTIFY frame too large", SPOP_EV_TX_FRAME|SPOP_EV_TX_NOTIFY|SPOP_EV_SPOP_STRM_ERR, spop_conn->conn, spop_strm);
- spop_strm_error(spop_strm, SPOP_ERR_TOO_BIG);
- goto fail;
- }
+ if (!b_data(mbuf))
+ goto too_big;
if ((mbuf = br_tail_add(spop_conn->mbuf)) != NULL)
goto retry;
@@ -3241,6 +3246,11 @@ static size_t spop_strm_send_notify(struct spop_strm *spop_strm, struct buffer *
TRACE_STATE("mbuf ring full", SPOP_EV_TX_FRAME|SPOP_EV_TX_NOTIFY|SPOP_EV_SPOP_STRM_BLK, spop_conn->conn, spop_strm);
ret = 0;
goto end;
+
+ too_big:
+ TRACE_ERROR("SPOP HAPROXY NOTIFY frame too large", SPOP_EV_TX_FRAME|SPOP_EV_TX_NOTIFY|SPOP_EV_SPOP_STRM_ERR, spop_conn->conn, spop_strm);
+ spop_strm_error(spop_strm, SPOP_ERR_TOO_BIG);
+
fail:
TRACE_DEVEL("leaving on error", SPOP_EV_TX_FRAME|SPOP_EV_TX_NOTIFY|SPOP_EV_SPOP_STRM_ERR, spop_conn->conn, spop_strm);
return 0;
--
2.49.0
