BUG/MINOR: quic: Stalled 0RTT connections with big ClientHello TLS message
This issue was reproduced with -Q picoquic client option to split a big ClientHello
message into two Initial packets and haproxy as server without any knowledged of
any previous ORTT session (restarted after a firt 0RTT session). The ORTT received
packets were removed from their queue when the second Initial packet was parsed,
and the QUIC handshake state never progressed and remained at Initial state.
To avoid such situations, after having treated some Initial packets we always
check if there are ORTT packets to parse and we never remove them from their
queue. This will be done after the hanshake is completed or upon idle timeout
expiration.
Also add more traces to be able to analize the handshake progression.
Tested with ngtcp2 and picoquic
Must be backported to 2.6.
diff --git a/include/haproxy/quic_conn-t.h b/include/haproxy/quic_conn-t.h
index 863e452..87f6fb1 100644
--- a/include/haproxy/quic_conn-t.h
+++ b/include/haproxy/quic_conn-t.h
@@ -220,6 +220,7 @@
#define QUIC_EV_TRANSP_PARAMS (1ULL << 44)
#define QUIC_EV_CONN_IDLE_TIMER (1ULL << 45)
#define QUIC_EV_CONN_SUB (1ULL << 46)
+#define QUIC_EV_CONN_ELEVELSEL (1ULL << 47)
/* Similar to kernel min()/max() definitions. */
#define QUIC_MIN(a, b) ({ \
diff --git a/include/haproxy/quic_tls.h b/include/haproxy/quic_tls.h
index b91adb4..5c035a4 100644
--- a/include/haproxy/quic_tls.h
+++ b/include/haproxy/quic_tls.h
@@ -455,8 +455,12 @@
/* Set <*level> and <*next_level> depending on <state> QUIC handshake state. */
static inline int quic_get_tls_enc_levels(enum quic_tls_enc_level *level,
enum quic_tls_enc_level *next_level,
+ struct quic_conn *qc,
enum quic_handshake_state state, int zero_rtt)
{
+ int ret = 0;
+
+ TRACE_ENTER(QUIC_EV_CONN_ELEVELSEL, qc, &state, level, next_level);
switch (state) {
case QUIC_HS_ST_SERVER_INITIAL:
case QUIC_HS_ST_CLIENT_INITIAL:
@@ -477,10 +481,13 @@
*next_level = QUIC_TLS_ENC_LEVEL_NONE;
break;
default:
- return 0;
+ goto leave;
}
- return 1;
+ ret = 1;
+ leave:
+ TRACE_LEAVE(QUIC_EV_CONN_ELEVELSEL, qc, NULL, level, next_level);
+ return ret;
}
/* Flag the keys at <qel> encryption level as discarded.
diff --git a/src/quic_conn.c b/src/quic_conn.c
index ddf1c8f..aa11d2c 100644
--- a/src/quic_conn.c
+++ b/src/quic_conn.c
@@ -661,6 +661,20 @@
chunk_frm_appendf(&trace_buf, frm);
}
}
+
+ if (mask & QUIC_EV_CONN_ELEVELSEL) {
+ const enum quic_handshake_state *state = a2;
+ const enum quic_tls_enc_level *level = a3;
+ const enum quic_tls_enc_level *next_level = a4;
+
+ if (state)
+ chunk_appendf(&trace_buf, " state=%s", quic_hdshk_state_str(qc->state));
+ if (level)
+ chunk_appendf(&trace_buf, " level=%c", quic_enc_level_char(*level));
+ if (next_level)
+ chunk_appendf(&trace_buf, " next_level=%c", quic_enc_level_char(*next_level));
+
+ }
}
if (mask & QUIC_EV_CONN_LPKT) {
const struct quic_rx_packet *pkt = a2;
@@ -4300,10 +4314,13 @@
struct quic_conn *qc = context;
enum quic_tls_enc_level tel, next_tel;
struct quic_enc_level *qel, *next_qel;
+ /* Early-data encryption level */
+ struct quic_enc_level *eqel;
struct buffer *buf = NULL;
int st, force_ack, zero_rtt;
TRACE_ENTER(QUIC_EV_CONN_IO_CB, qc);
+ eqel = &qc->els[QUIC_TLS_ENC_LEVEL_EARLY_DATA];
st = qc->state;
TRACE_PROTO("connection state", QUIC_EV_CONN_IO_CB, qc, &st);
@@ -4330,8 +4347,8 @@
}
ssl_err = SSL_ERROR_NONE;
zero_rtt = st < QUIC_HS_ST_COMPLETE &&
- (!LIST_ISEMPTY(&qc->els[QUIC_TLS_ENC_LEVEL_EARLY_DATA].rx.pqpkts) ||
- qc_el_rx_pkts(&qc->els[QUIC_TLS_ENC_LEVEL_EARLY_DATA]));
+ (eqel->tls_ctx.flags & QUIC_FL_TLS_SECRETS_SET) &&
+ (!LIST_ISEMPTY(&eqel->rx.pqpkts) || qc_el_rx_pkts(eqel));
start:
if (st >= QUIC_HS_ST_COMPLETE &&
qc_el_rx_pkts(&qc->els[QUIC_TLS_ENC_LEVEL_HANDSHAKE])) {
@@ -4340,7 +4357,7 @@
tel = QUIC_TLS_ENC_LEVEL_HANDSHAKE;
next_tel = QUIC_TLS_ENC_LEVEL_APP;
}
- else if (!quic_get_tls_enc_levels(&tel, &next_tel, st, zero_rtt))
+ else if (!quic_get_tls_enc_levels(&tel, &next_tel, qc, st, zero_rtt))
goto out;
qel = &qc->els[tel];
@@ -4360,24 +4377,15 @@
!(qc->flags & QUIC_FL_CONN_IMMEDIATE_CLOSE))
goto out;
- if (next_qel && next_qel == &qc->els[QUIC_TLS_ENC_LEVEL_EARLY_DATA] &&
- !LIST_ISEMPTY(&next_qel->rx.pqpkts)) {
- if ((next_qel->tls_ctx.flags & QUIC_FL_TLS_SECRETS_SET)) {
- qel = next_qel;
- next_qel = NULL;
- goto next_level;
- }
- else {
- struct quic_rx_packet *pkt, *pkttmp;
- struct quic_enc_level *aqel = &qc->els[QUIC_TLS_ENC_LEVEL_EARLY_DATA];
-
- /* Drop these 0-RTT packets */
- TRACE_DEVEL("drop all 0-RTT packets", QUIC_EV_CONN_PHPKTS, qc);
- list_for_each_entry_safe(pkt, pkttmp, &aqel->rx.pqpkts, list) {
- LIST_DELETE(&pkt->list);
- quic_rx_packet_refdec(pkt);
- }
- }
+ zero_rtt = st < QUIC_HS_ST_COMPLETE &&
+ (eqel->tls_ctx.flags & QUIC_FL_TLS_SECRETS_SET) &&
+ (!LIST_ISEMPTY(&eqel->rx.pqpkts) || qc_el_rx_pkts(eqel));
+ if (next_qel && next_qel == eqel && zero_rtt) {
+ TRACE_DEVEL("select 0RTT as next encryption level",
+ QUIC_EV_CONN_PHPKTS, qc);
+ qel = next_qel;
+ next_qel = NULL;
+ goto next_level;
}
st = qc->state;
@@ -4406,7 +4414,7 @@
/* A listener does not send any O-RTT packet. O-RTT packet number space must not
* be considered.
*/
- if (!quic_get_tls_enc_levels(&tel, &next_tel, st, 0))
+ if (!quic_get_tls_enc_levels(&tel, &next_tel, qc, st, 0))
goto out;
if (!qc_need_sending(qc, qel) &&