From c242832af31315315a0c8791af96aad5caa0ec38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20L=C3=A9caille?= Date: Mon, 29 Aug 2022 16:42:06 +0200 Subject: [PATCH] BUG/MINOR: quic: Missing header protection AES cipher context initialisations (draft-v2) This bug arrived with this commit: "MINOR: quic: Add reusable cipher contexts for header protection" haproxy could crash because of missing cipher contexts initializations for the header protection and draft-v2 Initial secrets. This was due to the fact that these initialization both for RX and TX secrets were done outside of qc_new_isecs(). The role of this function is definitively to initialize these cipher contexts in addition to the derived secrets. Indeed this function is called by qc_new_conn() which initializes the connection but also by qc_conn_finalize() which also calls qc_new_isecs() in case of a different QUIC version was negotiated by the peers from the one used by the client for its first Initial packet. This was reported by "v2" QUIC interop test with at least picoquic as client. Must be backported to 2.6. --- include/haproxy/quic_tls.h | 6 ++++++ src/xprt_quic.c | 4 ---- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/include/haproxy/quic_tls.h b/include/haproxy/quic_tls.h index 03ba40057..114ed0b3e 100644 --- a/include/haproxy/quic_tls.h +++ b/include/haproxy/quic_tls.h @@ -541,6 +541,9 @@ static inline int qc_new_isecs(struct quic_conn *qc, if (!quic_tls_rx_ctx_init(&rx_ctx->ctx, rx_ctx->aead, rx_ctx->key)) goto err; + if (!quic_tls_enc_aes_ctx_init(&rx_ctx->hp_ctx, rx_ctx->hp, rx_ctx->hp_key)) + goto err; + if (!quic_tls_derive_keys(ctx->tx.aead, ctx->tx.hp, ctx->tx.md, ver, tx_ctx->key, tx_ctx->keylen, tx_ctx->iv, tx_ctx->ivlen, @@ -551,6 +554,9 @@ static inline int qc_new_isecs(struct quic_conn *qc, if (!quic_tls_tx_ctx_init(&tx_ctx->ctx, tx_ctx->aead, tx_ctx->key)) goto err; + if (!quic_tls_enc_aes_ctx_init(&tx_ctx->hp_ctx, tx_ctx->hp, tx_ctx->hp_key)) + goto err; + ctx->flags |= QUIC_FL_TLS_SECRETS_SET; TRACE_LEAVE(QUIC_EV_CONN_ISEC, qc, rx_init_sec, tx_init_sec); diff --git a/src/xprt_quic.c b/src/xprt_quic.c index 99f07df4e..0172c4430 100644 --- a/src/xprt_quic.c +++ b/src/xprt_quic.c @@ -4802,10 +4802,6 @@ static struct quic_conn *qc_new_conn(const struct quic_version *qv, int ipv4, if (!qc_new_isecs(qc, ictx,qc->original_version, dcid->data, dcid->len, 1)) goto err; - if (!quic_tls_dec_aes_ctx_init(&ictx->rx.hp_ctx, ictx->rx.hp, ictx->rx.hp_key) || - !quic_tls_enc_aes_ctx_init(&ictx->tx.hp_ctx, ictx->tx.hp, ictx->tx.hp_key)) - goto err; - TRACE_LEAVE(QUIC_EV_CONN_INIT, qc); return qc;