BUG/MINOR: quic: Avoid crashing with unsupported cryptographic algos

This bug was detected when compiling haproxy against aws-lc TLS stack
during QUIC interop runner tests. Some algorithms could be negotiated by haproxy
through the TLS stack but not fully supported by haproxy QUIC implentation.
This leaded tls_aead() to return NULL (same thing for tls_md(), tls_hp()).
As these functions returned values were never checked, they could triggered
segfaults.

To fix this, one closes the connection as soon as possible with a
handshake_failure(40) TLS alert. Note that as the TLS stack successfully
negotiates an algorithm, it provides haproxy with CRYPTO data before entering
->set_encryption_secrets() callback. This is why this callback
(ha_set_encryption_secrets() on haproxy side) is modified to release all
the CRYPTO frames before triggering a CONNECTION_CLOSE with a TLS alert. This is
done calling qc_release_pktns_frms() for all the packet number spaces.
Modify some quic_tls_keys_hexdump to avoid crashes when the ->aead or ->hp EVP_CIPHER
are NULL.
Modify qc_release_pktns_frms() to do nothing if the packet number space passed
as parameter is not intialized.

This bug does not impact the QUIC TLS compatibily mode (USE_QUIC_OPENSSL_COMPAT).

Thank you to @ilia-shipitsin for having reported this issue in GH #2309.

Must be backported as far as 2.6.
This commit is contained in:
Frédéric Lécaille 2023-10-11 09:28:36 +02:00 committed by William Lallemand
parent cc743b698f
commit bd83b6effb
3 changed files with 36 additions and 11 deletions

View File

@ -552,9 +552,13 @@ static inline void qc_release_pktns_frms(struct quic_conn *qc,
TRACE_ENTER(QUIC_EV_CONN_PHPKTS, qc);
if (!pktns)
goto leave;
list_for_each_entry_safe(frm, frmbak, &pktns->tx.frms, list)
qc_frm_free(qc, &frm);
leave:
TRACE_LEAVE(QUIC_EV_CONN_PHPKTS, qc);
}

View File

@ -192,15 +192,17 @@ static int ha_quic_set_encryption_secrets(SSL *ssl, enum ssl_encryption_level_t
goto write;
rx = &tls_ctx->rx;
rx->aead = tls_aead(cipher);
rx->md = tls_md(cipher);
rx->hp = tls_hp(cipher);
if (!rx->aead || !rx->md || !rx->hp)
goto leave;
if (!quic_tls_secrets_keys_alloc(rx)) {
TRACE_ERROR("RX keys allocation failed", QUIC_EV_CONN_RWSEC, qc);
goto leave;
}
rx->aead = tls_aead(cipher);
rx->md = tls_md(cipher);
rx->hp = tls_hp(cipher);
if (!quic_tls_derive_keys(rx->aead, rx->hp, rx->md, ver, rx->key, rx->keylen,
rx->iv, rx->ivlen, rx->hp_key, sizeof rx->hp_key,
read_secret, secret_len)) {
@ -233,15 +235,17 @@ write:
goto keyupdate_init;
tx = &tls_ctx->tx;
tx->aead = tls_aead(cipher);
tx->md = tls_md(cipher);
tx->hp = tls_hp(cipher);
if (!tx->aead || !tx->md || !tx->hp)
goto leave;
if (!quic_tls_secrets_keys_alloc(tx)) {
TRACE_ERROR("TX keys allocation failed", QUIC_EV_CONN_RWSEC, qc);
goto leave;
}
tx->aead = tls_aead(cipher);
tx->md = tls_md(cipher);
tx->hp = tls_hp(cipher);
if (!quic_tls_derive_keys(tx->aead, tx->hp, tx->md, ver, tx->key, tx->keylen,
tx->iv, tx->ivlen, tx->hp_key, sizeof tx->hp_key,
write_secret, secret_len)) {
@ -298,6 +302,16 @@ write:
out:
ret = 1;
leave:
if (!ret) {
/* Release the CRYPTO frames which have been provided by the TLS stack
* to prevent the transmission of ack-eliciting packets.
*/
qc_release_pktns_frms(qc, qc->ipktns);
qc_release_pktns_frms(qc, qc->hpktns);
qc_release_pktns_frms(qc, qc->apktns);
quic_set_tls_alert(qc, SSL_AD_HANDSHAKE_FAILURE);
}
TRACE_LEAVE(QUIC_EV_CONN_RWSEC, qc, &level);
return ret;
}

View File

@ -47,9 +47,16 @@ void quic_tls_keys_hexdump(struct buffer *buf,
const struct quic_tls_secrets *secs)
{
int i;
size_t aead_keylen = (size_t)EVP_CIPHER_key_length(secs->aead);
size_t aead_ivlen = (size_t)EVP_CIPHER_iv_length(secs->aead);
size_t hp_len = (size_t)EVP_CIPHER_key_length(secs->hp);
size_t aead_keylen;
size_t aead_ivlen;
size_t hp_len;
if (!secs->aead || !secs->hp)
return;
aead_keylen = (size_t)EVP_CIPHER_key_length(secs->aead);
aead_ivlen = (size_t)EVP_CIPHER_iv_length(secs->aead);
hp_len = (size_t)EVP_CIPHER_key_length(secs->hp);
chunk_appendf(buf, "\n key=");
for (i = 0; i < aead_keylen; i++)