From: Frédéric Lécaille <flecaille@haproxy.com>
Date: Mon, 29 Aug 2022 14:42:06 +0000 (+0200)
Subject: BUG/MINOR: quic: Missing header protection AES cipher context initialisations (draft-v2)
X-Git-Tag: v2.7-dev5~40
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c242832af31315315a0c8791af96aad5caa0ec38;p=thirdparty%2Fhaproxy.git

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.
---

diff --git a/include/haproxy/quic_tls.h b/include/haproxy/quic_tls.h
index 03ba400578..114ed0b3e3 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 99f07df4e5..0172c4430a 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;