From 307ca7fe5089bb86d38bb5408ffee6cc8fa7e08f Mon Sep 17 00:00:00 2001 From: Dmitri Tikhonov Date: Mon, 15 Jun 2020 16:34:30 -0400 Subject: [PATCH] Release 2.16.3 - [OPTIMIZATION] Stash up to two reordered packets in IETF mini conn instead of dropping them. - [BUGFIX] Crash: check decrypt context before using it. This regression was introduced in 2.16.2. --- CHANGELOG | 7 ++++++ docs/conf.py | 2 +- include/lsquic.h | 2 +- src/liblsquic/lsquic_enc_sess_ietf.c | 24 ++++++++++++------- src/liblsquic/lsquic_mini_conn_ietf.c | 33 +++++++++++++++++++++++---- src/liblsquic/lsquic_mini_conn_ietf.h | 3 +++ 6 files changed, 56 insertions(+), 15 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 9f44d7e..bd9e24a 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,10 @@ +2020-06-15 + - 2.16.3 + - [OPTIMIZATION] Stash up to two reordered packets in IETF mini conn + instead of dropping them. + - [BUGFIX] Crash: check decrypt context before using it. This regression + was introduced in 2.16.2. + 2020-06-12 - 2.16.2 - [BUGFIX] ID-28: do not use TLS middlebox compatibility mode in diff --git a/docs/conf.py b/docs/conf.py index 9a8741f..45aa4ff 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -26,7 +26,7 @@ author = u'LiteSpeed Technologies' # The short X.Y version version = u'2.16' # The full version, including alpha/beta/rc tags -release = u'2.16.2' +release = u'2.16.3' # -- General configuration --------------------------------------------------- diff --git a/include/lsquic.h b/include/lsquic.h index c1af68c..61c5f0d 100644 --- a/include/lsquic.h +++ b/include/lsquic.h @@ -25,7 +25,7 @@ extern "C" { #define LSQUIC_MAJOR_VERSION 2 #define LSQUIC_MINOR_VERSION 16 -#define LSQUIC_PATCH_VERSION 2 +#define LSQUIC_PATCH_VERSION 3 /** * Engine flags: diff --git a/src/liblsquic/lsquic_enc_sess_ietf.c b/src/liblsquic/lsquic_enc_sess_ietf.c index a201737..7155bb1 100644 --- a/src/liblsquic/lsquic_enc_sess_ietf.c +++ b/src/liblsquic/lsquic_enc_sess_ietf.c @@ -115,11 +115,15 @@ struct header_prot const EVP_CIPHER *hp_cipher; gen_hp_mask_f hp_gen_mask; enum enc_level hp_enc_level; + enum { + HP_CAN_READ = 1 << 0, + HP_CAN_WRITE = 1 << 1, + } hp_flags; unsigned hp_sz; unsigned char hp_buf[2][EVP_MAX_KEY_LENGTH]; }; -#define header_prot_inited(hp_) ((hp_)->hp_sz > 0) +#define header_prot_inited(hp_, rw_) ((hp_)->hp_flags & (1 << (rw_))) struct crypto_ctx @@ -177,12 +181,11 @@ derive_hp_secrets (struct header_prot *hp, const EVP_MD *md, const unsigned char *client_secret, const unsigned char *server_secret) { hp->hp_sz = EVP_AEAD_key_length(aead); - if (client_secret) - lsquic_qhkdf_expand(md, client_secret, secret_sz, PN_LABEL, PN_LABEL_SZ, - hp->hp_buf[0], hp->hp_sz); - if (server_secret) - lsquic_qhkdf_expand(md, server_secret, secret_sz, PN_LABEL, PN_LABEL_SZ, - hp->hp_buf[1], hp->hp_sz); + hp->hp_flags = HP_CAN_READ | HP_CAN_WRITE; + lsquic_qhkdf_expand(md, client_secret, secret_sz, PN_LABEL, PN_LABEL_SZ, + hp->hp_buf[0], hp->hp_sz); + lsquic_qhkdf_expand(md, server_secret, secret_sz, PN_LABEL, PN_LABEL_SZ, + hp->hp_buf[1], hp->hp_sz); } @@ -1987,7 +1990,7 @@ iquic_esf_decrypt_packet (enc_session_t *enc_session_p, else hp = NULL; - if (UNLIKELY(!(hp && header_prot_inited(hp)))) + if (UNLIKELY(!(hp && header_prot_inited(hp, 0)))) { LSQ_DEBUG("header protection for level %u not initialized yet", enc_level); @@ -2017,7 +2020,11 @@ iquic_esf_decrypt_packet (enc_session_t *enc_session_p, key_phase = (dst[0] & 0x04) > 0; pair = &enc_sess->esi_pairs[ key_phase ]; if (key_phase == enc_sess->esi_key_phase) + { crypto_ctx = &pair->ykp_ctx[ 0 ]; + /* Checked by header_prot_inited() above */ + assert(crypto_ctx->yk_flags & YK_INITED); + } else if (!is_valid_packno( enc_sess->esi_pairs[enc_sess->esi_key_phase].ykp_thresh) || packet_in->pi_packno @@ -2616,6 +2623,7 @@ set_secret (SSL *ssl, enum ssl_encryption_level_t level, } lsquic_qhkdf_expand(crypa.md, secret, secret_len, PN_LABEL, PN_LABEL_SZ, hp->hp_buf[rw], hp->hp_sz); + hp->hp_flags |= 1 << rw; if (enc_sess->esi_flags & ESI_LOG_SECRETS) { diff --git a/src/liblsquic/lsquic_mini_conn_ietf.c b/src/liblsquic/lsquic_mini_conn_ietf.c index 9f5c344..5e0a862 100644 --- a/src/liblsquic/lsquic_mini_conn_ietf.c +++ b/src/liblsquic/lsquic_mini_conn_ietf.c @@ -1185,6 +1185,31 @@ ignore_init (struct ietf_mini_conn *conn) } +static void +imico_maybe_delay_processing (struct ietf_mini_conn *conn, + struct lsquic_packet_in *packet_in) +{ + unsigned max_delayed; + + if (conn->imc_flags & IMC_ADDR_VALIDATED) + max_delayed = IMICO_MAX_DELAYED_PACKETS_VALIDATED; + else + max_delayed = IMICO_MAX_DELAYED_PACKETS_UNVALIDATED; + + if (conn->imc_delayed_packets_count < max_delayed) + { + ++conn->imc_delayed_packets_count; + lsquic_packet_in_upref(packet_in); + TAILQ_INSERT_TAIL(&conn->imc_app_packets, packet_in, pi_next); + LSQ_DEBUG("delay processing of packet (now delayed %hhu)", + conn->imc_delayed_packets_count); + } + else + LSQ_DEBUG("drop packet, already delayed %hhu packets", + conn->imc_delayed_packets_count); +} + + /* Only a single packet is supported */ static void ietf_mini_conn_ci_packet_in (struct lsquic_conn *lconn, @@ -1222,8 +1247,9 @@ ietf_mini_conn_ci_packet_in (struct lsquic_conn *lconn, conn->imc_enpub, &conn->imc_conn, packet_in); if (dec_packin != DECPI_OK) { - /* TODO: handle reordering perhaps? */ LSQ_DEBUG("could not decrypt packet"); + if (DECPI_NOT_YET == dec_packin) + imico_maybe_delay_processing(conn, packet_in); return; } @@ -1231,10 +1257,7 @@ ietf_mini_conn_ci_packet_in (struct lsquic_conn *lconn, if (pns == PNS_APP) { - lsquic_packet_in_upref(packet_in); - TAILQ_INSERT_TAIL(&conn->imc_app_packets, packet_in, pi_next); - LSQ_DEBUG("delay processing of packet %"PRIu64" in pns %u", - packet_in->pi_packno, pns); + imico_maybe_delay_processing(conn, packet_in); return; } else if (pns == PNS_HSK) diff --git a/src/liblsquic/lsquic_mini_conn_ietf.h b/src/liblsquic/lsquic_mini_conn_ietf.h index a6922ed..f2f4c71 100644 --- a/src/liblsquic/lsquic_mini_conn_ietf.h +++ b/src/liblsquic/lsquic_mini_conn_ietf.h @@ -94,6 +94,9 @@ struct ietf_mini_conn uint8_t imc_ecn_counts_out[N_PNS][4]; uint8_t imc_incoming_ecn; uint8_t imc_tls_alert; +#define IMICO_MAX_DELAYED_PACKETS_UNVALIDATED 1u +#define IMICO_MAX_DELAYED_PACKETS_VALIDATED 2u + unsigned char imc_delayed_packets_count; #define IMICO_MAX_STASHED_FRAMES 10u unsigned char imc_n_crypto_frames; struct network_path imc_path;