mirrors/haproxy
1
0
Fork 0
mirror of https://git.haproxy.org/git/haproxy.git/ synced 2025-11-01 00:50:59 +01:00
Code Issues Projects Releases Wiki Activity

BUG/MAJOR: quic: use ncbmbuf for CRYPTO handling

Browse Source 
In QUIC, TLS handshake messages such as ClientHello are encapsulated in
CRYPTO frames. Each QUIC implementation can split the content in several
frames of random sizes. In fact, this feature is now used by several
clients, based on chrome so-called "Chaos protection" mechanism :

https://quiche.googlesource.com/quiche/+/cb6b51054274cb2c939264faf34a1776e0a5bab7

To support this, haproxy uses a ncbuf storage to store received CRYPTO
frames before passing it to the SSL library. However, this storage
suffers from a limitation as gaps between two filled blocks cannot be
smaller than 8 bytes. Thus, depending on the size of received CRYPTO
frames and their order, ncbuf may not be sufficient. Over time, several
mechanisms were implemented in haproxy QUIC frames parsing to overcome
the ncbuf limitation.

However, reports recently highlight that with some clients haproxy is
not able to deal with CRYPTO frames reception. In particular, this is
the case with the latest ngtcp2 release, which implements a similar
chaos protection mechanism via the following patch. It also seems that
this impacts haproxy interaction with firefox.

commit 89c29fd8611d5e6d2f6b1f475c5e3494c376028c
Author: Tatsuhiro Tsujikawa <tatsuhiro.t@gmail.com>
Date:   Mon Aug 4 22:48:06 2025 +0900

    Crumble Client Initial CRYPTO (aka chaos protection)

To fix haproxy CRYPTO frames buffering once and for all, an alternative
non-contiguous buffer named ncbmbuf has been recently implemented. This
type does not suffer from gaps size limitation, albeit at the cost of a
small reduction in the size available for data storage.

Thus, the purpose of this current patch is to replace ncbuf with the
newer ncbmbuf for QUIC CRYPTO frames parsing. Now, ncbmb_add() is used
to buffer received frames which is guaranteed to suceed. The only
remaining case of error is if a received frame offset and length exceed
the ncbmbuf data storage, which would result in a CRYPTO_BUFFER_EXCEEDED
error code.

A notable behavior change when switching to ncbmbuf implementation is
that NCB_ADD_COMPARE mode cannot be used anymore during add. Instead,
crypto frame content received at a similar offset will be overwritten.

A final note regarding STREAM frames parsing. For now, it is considered
unnecessary to switch from ncbuf in this case. Indeed, QUIC clients does
not perform aggressive fragmentation for them. Keeping ncbuf ensure that
the data storage size is bigger than the equivalent ncbmbuf area.

This should fix github issue #3141.

This patch must be backported up to 2.6. It is first necessary to pick
the relevant commits for ncbmbuf implementation prior to it.
This commit is contained in:
Amaury Denoyelle 2025-10-16 16:43:16 +02:00
parent 25e378fa65
commit 4c11206395
5 changed files with 39 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
include/haproxy/quic_conn.h
View File

@ -32,7 +32,7 @@
#include <haproxy/chunk.h>
#include <haproxy/dynbuf.h>
#include <haproxy/ncbuf.h>
#include <haproxy/ncbmbuf.h>
#include <haproxy/net_helper.h>
#include <haproxy/openssl-compat.h>
#include <haproxy/ticks.h>
@ -135,35 +135,35 @@ static inline void quic_conn_mv_cids_to_cc_conn(struct quic_conn_closed *cc_conn
*
* Returns the buffer instance or NULL on allocation failure.
*/
static inline struct ncbuf *quic_get_ncbuf(struct ncbuf *ncbuf)
static inline struct ncbmbuf *quic_get_ncbuf(struct ncbmbuf *ncbuf)
{
struct buffer buf = BUF_NULL;
if (!ncb_is_null(ncbuf))
if (!ncbmb_is_null(ncbuf))
return ncbuf;
if (!b_alloc(&buf, DB_MUX_RX))
return NULL;
*ncbuf = ncb_make(buf.area, buf.size, 0);
ncb_init(ncbuf, 0);
*ncbuf = ncbmb_make(buf.area, buf.size, 0);
ncbmb_init(ncbuf, 0);
return ncbuf;
}
/* Release the underlying memory use by <ncbuf> non-contiguous buffer */
static inline void quic_free_ncbuf(struct ncbuf *ncbuf)
static inline void quic_free_ncbuf(struct ncbmbuf *ncbuf)
{
struct buffer buf;
if (ncb_is_null(ncbuf))
if (ncbmb_is_null(ncbuf))
return;
buf = b_make(ncbuf->area, ncbuf->size, 0, 0);
b_free(&buf);
offer_buffers(NULL, 1);
*ncbuf = NCBUF_NULL;
*ncbuf = NCBMBUF_NULL;
}
/* Return the address of the QUIC counters attached to the proxy of

4
include/haproxy/quic_tls-t.h
View File

@ -26,7 +26,7 @@
#include <import/ebtree.h>
#include <haproxy/buf-t.h>
#include <haproxy/ncbuf-t.h>
#include <haproxy/ncbmbuf-t.h>
#include <haproxy/quic_ack-t.h>
/* Use EVP_CIPHER or EVP_AEAD API depending on the library */
@ -255,7 +255,7 @@ struct quic_crypto_buf {
struct quic_cstream {
struct {
uint64_t offset; /* absolute current base offset of ncbuf */
struct ncbuf ncbuf; /* receive buffer - can handle out-of-order offset frames */
struct ncbmbuf ncbuf; /* receive buffer - can handle out-of-order offset frames */
} rx;
struct {
uint64_t offset; /* last offset of data ready to be sent */

29
src/quic_rx.c
View File

@ -16,7 +16,7 @@
#include <haproxy/h3.h>
#include <haproxy/list.h>
#include <haproxy/ncbuf.h>
#include <haproxy/ncbmbuf.h>
#include <haproxy/proto_quic.h>
#include <haproxy/quic_ack.h>
#include <haproxy/quic_cc_drs.h>
@ -666,7 +666,7 @@ static enum quic_rx_ret_frm qc_handle_crypto_frm(struct quic_conn *qc,
.len = crypto_frm->len,
};
struct quic_cstream *cstream = qel->cstream;
struct ncbuf *ncbuf = &qel->cstream->rx.ncbuf;
struct ncbmbuf *ncbuf = &qel->cstream->rx.ncbuf;
uint64_t off_rel;
TRACE_ENTER(QUIC_EV_CONN_PRSHPKT, qc);
@ -691,7 +691,7 @@ static enum quic_rx_ret_frm qc_handle_crypto_frm(struct quic_conn *qc,
crypto_frm->offset_node.key = cstream->rx.offset;
}
if (!quic_get_ncbuf(ncbuf) || ncb_is_null(ncbuf)) {
if (!quic_get_ncbuf(ncbuf) || ncbmb_is_null(ncbuf)) {
TRACE_ERROR("CRYPTO ncbuf allocation failed", QUIC_EV_CONN_PRSHPKT, qc);
goto err;
}
@ -707,31 +707,18 @@ static enum quic_rx_ret_frm qc_handle_crypto_frm(struct quic_conn *qc,
* handshake. If an endpoint does not expand its buffer, it MUST close
* the connection with a CRYPTO_BUFFER_EXCEEDED error code.
*/
if (off_rel + crypto_frm->len > ncb_size(ncbuf)) {
if (off_rel + crypto_frm->len > ncbmb_size(ncbuf)) {
TRACE_ERROR("CRYPTO frame too large", QUIC_EV_CONN_PRSHPKT, qc);
quic_set_connection_close(qc, quic_err_transport(QC_ERR_CRYPTO_BUFFER_EXCEEDED));
goto err;
}
ncb_ret = ncb_add(ncbuf, off_rel, (const char *)crypto_frm->data,
crypto_frm->len, NCB_ADD_COMPARE);
if (ncb_ret != NCB_RET_OK) {
if (ncb_ret == NCB_RET_DATA_REJ) {
TRACE_ERROR("overlapping data rejected", QUIC_EV_CONN_PRSHPKT, qc);
quic_set_connection_close(qc, quic_err_transport(QC_ERR_PROTOCOL_VIOLATION));
qc_notify_err(qc);
goto err;
}
else if (ncb_ret == NCB_RET_GAP_SIZE) {
TRACE_DATA("cannot bufferize frame due to gap size limit",
QUIC_EV_CONN_PRSHPKT, qc);
ret = QUIC_RX_RET_FRM_AGAIN;
goto done;
}
}
ncb_ret = ncbmb_add(ncbuf, off_rel, (const char *)crypto_frm->data,
crypto_frm->len, NCB_ADD_OVERWRT);
BUG_ON(ncb_ret != NCB_RET_OK);
/* Reschedule with TASK_HEAVY if CRYPTO data ready for decoding. */
if (ncb_data(ncbuf, 0)) {
if (ncbmb_data(ncbuf, 0)) {
HA_ATOMIC_OR(&qc->wait_event.tasklet->state, TASK_HEAVY);
tasklet_wakeup(qc->wait_event.tasklet);
}

45
src/quic_ssl.c
View File

@ -1,5 +1,5 @@
#include <haproxy/errors.h>
#include <haproxy/ncbuf.h>
#include <haproxy/ncbmbuf.h>
#include <haproxy/proxy.h>
#include <haproxy/quic_conn.h>
#include <haproxy/quic_sock.h>
@ -439,7 +439,7 @@ static int ha_quic_ossl_crypto_recv_rcd(SSL *ssl,
{
struct quic_conn *qc = SSL_get_ex_data(ssl, ssl_qc_app_data_index);
struct quic_enc_level *qel;
struct ncbuf *ncbuf = NULL;
struct ncbmbuf *ncbuf = NULL;
struct quic_cstream *cstream = NULL;
ncb_sz_t data = 0;
@ -451,10 +451,10 @@ static int ha_quic_ossl_crypto_recv_rcd(SSL *ssl,
continue;
ncbuf = &cstream->rx.ncbuf;
if (ncb_is_null(ncbuf))
if (ncbmb_is_null(ncbuf))
continue;
data = ncb_data(ncbuf, 0);
data = ncbmb_data(ncbuf, 0);
if (data)
break;
}
@ -462,9 +462,9 @@ static int ha_quic_ossl_crypto_recv_rcd(SSL *ssl,
if (data) {
const unsigned char *cdata;
BUG_ON(ncb_is_null(ncbuf) || !cstream);
BUG_ON(ncbmb_is_null(ncbuf) || !cstream);
/* <ncbuf> must not be released at this time. */
cdata = (const unsigned char *)ncb_head(ncbuf);
cdata = (const unsigned char *)ncbmb_head(ncbuf);
cstream->rx.offset += data;
TRACE_DEVEL("buffered crypto data were provided to TLS stack",
QUIC_EV_CONN_PHPKTS, qc, qel);
@ -496,24 +496,24 @@ static int ha_quic_ossl_crypto_release_rcd(SSL *ssl,
list_for_each_entry(qel, &qc->qel_list, list) {
struct quic_cstream *cstream = qel->cstream;
struct ncbuf *ncbuf;
struct ncbmbuf *ncbuf;
ncb_sz_t data;
if (!cstream)
continue;
ncbuf = &cstream->rx.ncbuf;
if (ncb_is_null(ncbuf))
if (ncbmb_is_null(ncbuf))
continue;
data = ncb_data(ncbuf, 0);
data = ncbmb_data(ncbuf, 0);
if (!data)
continue;
data = data > bytes_read ? bytes_read : data;
ncb_advance(ncbuf, data);
ncbmb_advance(ncbuf, data);
bytes_read -= data;
if (ncb_is_empty(ncbuf)) {
if (ncbmb_is_empty(ncbuf)) {
TRACE_DEVEL("freeing crypto buf", QUIC_EV_CONN_PHPKTS, qc, qel);
quic_free_ncbuf(ncbuf);
}
@ -1069,7 +1069,7 @@ int qc_ssl_do_hanshake(struct quic_conn *qc, struct ssl_sock_ctx *ctx)
* Remaining parameter are there for debugging purposes.
* Return 1 if succeeded, 0 if not.
*/
static int qc_ssl_provide_quic_data(struct ncbuf *ncbuf,
static int qc_ssl_provide_quic_data(struct ncbmbuf *ncbuf,
enum ssl_encryption_level_t level,
struct ssl_sock_ctx *ctx,
const unsigned char *data, size_t len)
@ -1099,17 +1099,12 @@ static int qc_ssl_provide_quic_data(struct ncbuf *ncbuf,
/* The CRYPTO data are consumed even in case of an error to release
* the memory asap.
*/
if (!ncb_is_null(ncbuf)) {
if (!ncbmb_is_null(ncbuf)) {
#ifdef DEBUG_STRICT
ncb_ret = ncb_advance(ncbuf, len);
/* ncb_advance() must always succeed. This is guaranteed as
* this is only done inside a data block. If false, this will
* lead to handshake failure with quic_enc_level offset shifted
* from buffer data.
*/
ncb_ret = ncbmb_advance(ncbuf, len);
BUG_ON(ncb_ret != NCB_RET_OK);
#else
ncb_advance(ncbuf, len);
ncbmb_advance(ncbuf, len);
#endif
}
@ -1124,7 +1119,7 @@ int qc_ssl_provide_all_quic_data(struct quic_conn *qc, struct ssl_sock_ctx *ctx)
{
int ret = 0;
struct quic_enc_level *qel;
struct ncbuf *ncbuf;
struct ncbmbuf *ncbuf;
ncb_sz_t data;
TRACE_ENTER(QUIC_EV_CONN_PHPKTS, qc);
@ -1135,12 +1130,12 @@ int qc_ssl_provide_all_quic_data(struct quic_conn *qc, struct ssl_sock_ctx *ctx)
continue;
ncbuf = &cstream->rx.ncbuf;
if (ncb_is_null(ncbuf))
if (ncbmb_is_null(ncbuf))
continue;
/* TODO not working if buffer is wrapping */
while ((data = ncb_data(ncbuf, 0))) {
const unsigned char *cdata = (const unsigned char *)ncb_head(ncbuf);
while ((data = ncbmb_data(ncbuf, 0))) {
const unsigned char *cdata = (const unsigned char *)ncbmb_head(ncbuf);
if (!qc_ssl_provide_quic_data(&qel->cstream->rx.ncbuf, qel->level,
ctx, cdata, data))
@ -1151,7 +1146,7 @@ int qc_ssl_provide_all_quic_data(struct quic_conn *qc, struct ssl_sock_ctx *ctx)
QUIC_EV_CONN_PHPKTS, qc, qel);
}
if (!ncb_is_null(ncbuf) && ncb_is_empty(ncbuf)) {
if (!ncbmb_is_null(ncbuf) && ncbmb_is_empty(ncbuf)) {
TRACE_DEVEL("freeing crypto buf", QUIC_EV_CONN_PHPKTS, qc, qel);
quic_free_ncbuf(ncbuf);
}

2
src/quic_tls.c
View File

@ -143,7 +143,7 @@ struct quic_cstream *quic_cstream_new(struct quic_conn *qc)
}
cs->rx.offset = 0;
cs->rx.ncbuf = NCBUF_NULL;
cs->rx.ncbuf = NCBMBUF_NULL;
cs->rx.offset = 0;
cs->tx.offset = 0;