From da1fb430f6206246478f1aaee0b9b7fb480148ee Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Thu, 12 Nov 2020 11:18:00 +0900 Subject: [PATCH 1/5] send CONNECTION_CLOSE in all available epochs --- lib/quicly.c | 63 ++++++++++++++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 22 deletions(-) diff --git a/lib/quicly.c b/lib/quicly.c index 29c599d27..7b27a063e 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -3873,29 +3873,50 @@ size_t quicly_send_retry(quicly_context_t *ctx, ptls_aead_context_t *token_encry return ret == 0 ? buf.off : SIZE_MAX; } -static int send_handshake_flow(quicly_conn_t *conn, size_t epoch, quicly_send_context_t *s, int ack_only, int send_probe) +static struct st_quicly_pn_space_t *setup_send_epoch(quicly_conn_t *conn, size_t epoch, quicly_send_context_t *s) { struct st_quicly_pn_space_t *ack_space = NULL; - int ret = 0; switch (epoch) { case QUICLY_EPOCH_INITIAL: if (conn->initial == NULL || (s->current.cipher = &conn->initial->cipher.egress)->aead == NULL) - return 0; + return NULL; s->current.first_byte = QUICLY_PACKET_TYPE_INITIAL; ack_space = &conn->initial->super; break; case QUICLY_EPOCH_HANDSHAKE: if (conn->handshake == NULL || (s->current.cipher = &conn->handshake->cipher.egress)->aead == NULL) - return 0; + return NULL; s->current.first_byte = QUICLY_PACKET_TYPE_HANDSHAKE; ack_space = &conn->handshake->super; break; + case QUICLY_EPOCH_0RTT: + case QUICLY_EPOCH_1RTT: + if (conn->application == NULL || conn->application->cipher.egress.key.header_protection == NULL) + return NULL; + if ((epoch == QUICLY_EPOCH_0RTT) == conn->application->one_rtt_writable) + return NULL; + s->current.cipher = &conn->application->cipher.egress.key; + s->current.first_byte = epoch == QUICLY_EPOCH_0RTT ? QUICLY_PACKET_TYPE_0RTT : QUICLY_QUIC_BIT; + ack_space = &conn->application->super; + break; default: assert(!"logic flaw"); - return 0; + break; } + return ack_space; +} + +static int send_handshake_flow(quicly_conn_t *conn, size_t epoch, quicly_send_context_t *s, int ack_only, int send_probe) +{ + struct st_quicly_pn_space_t *ack_space; + int ret = 0; + + /* setup send epoch, or return if it's impossible to send in this epoch */ + if ((ack_space = setup_send_epoch(conn, epoch, s)) == NULL) + return 0; + /* send ACK */ if (ack_space != NULL && (ack_space->unacked_count != 0 || send_probe)) if ((ret = send_ack(conn, ack_space, s)) != 0) @@ -3927,12 +3948,16 @@ static int send_handshake_flow(quicly_conn_t *conn, size_t epoch, quicly_send_co return ret; } -static int send_connection_close(quicly_conn_t *conn, quicly_send_context_t *s) +static int send_connection_close(quicly_conn_t *conn, size_t epoch, quicly_send_context_t *s) { uint64_t error_code, offending_frame_type; const char *reason_phrase; int ret; + /* setup send epoch, or return if it's impossible to send in this epoch */ + if (setup_send_epoch(conn, epoch, s) == NULL) + return 0; + /* determine the payload, masking the application error when sending the frame using an unauthenticated epoch */ error_code = conn->egress.connection_close.error_code; offending_frame_type = conn->egress.connection_close.frame_type; @@ -4157,9 +4182,9 @@ static int do_send(quicly_conn_t *conn, quicly_send_context_t *s) if ((ret = send_handshake_flow(conn, QUICLY_EPOCH_HANDSHAKE, s, ack_only, min_packets_to_send != 0)) != 0) goto Exit; - /* send encrypted frames */ - if (conn->application != NULL && (s->current.cipher = &conn->application->cipher.egress.key)->header_protection != NULL) { - s->current.first_byte = conn->application->one_rtt_writable ? QUICLY_QUIC_BIT : QUICLY_PACKET_TYPE_0RTT; + /* setup 0-RTT or 1-RTT send context (as the availability of the two epochs are mutually exclusive, we can try 1-RTT first as an + * optimization), then send application data if that succeeds */ + if (setup_send_epoch(conn, QUICLY_EPOCH_1RTT, s) != NULL || setup_send_epoch(conn, QUICLY_EPOCH_0RTT, s) != NULL) { /* acks */ if (conn->application->one_rtt_writable && conn->egress.send_ack_at <= conn->stash.now && conn->application->super.unacked_count != 0) { @@ -4346,20 +4371,14 @@ int quicly_send(quicly_conn_t *conn, quicly_address_t *dest, quicly_address_t *s } } if (conn->super.state == QUICLY_STATE_CLOSING && conn->egress.send_ack_at <= conn->stash.now) { - destroy_all_streams(conn, 0, 0); /* delayed until the emission of CONNECTION_CLOSE frame to allow quicly_close to be - * called from a stream handler */ - if (conn->application != NULL && conn->application->one_rtt_writable) { - s.current.cipher = &conn->application->cipher.egress.key; - s.current.first_byte = QUICLY_QUIC_BIT; - } else if (conn->handshake != NULL && (s.current.cipher = &conn->handshake->cipher.egress)->aead != NULL) { - s.current.first_byte = QUICLY_PACKET_TYPE_HANDSHAKE; - } else { - s.current.cipher = &conn->initial->cipher.egress; - assert(s.current.cipher->aead != NULL); - s.current.first_byte = QUICLY_PACKET_TYPE_INITIAL; + /* destroy all streams; doing so is delayed until the emission of CONNECTION_CLOSE frame to allow quicly_close to be + * called from a stream handler */ + destroy_all_streams(conn, 0, 0); + /* send CONNECTION_CLOSE in all possible epochs */ + for (size_t epoch = 0; epoch < QUICLY_NUM_EPOCHS; ++epoch) { + if ((ret = send_connection_close(conn, epoch, &s)) != 0) + goto Exit; } - if ((ret = send_connection_close(conn, &s)) != 0) - goto Exit; if ((ret = commit_send_packet(conn, &s, QUICLY_COMMIT_SEND_PACKET_MODE_SMALL)) != 0) goto Exit; } From 891ae24a9a922064ce3e63f4dfc36050ec0a3bb5 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Thu, 12 Nov 2020 12:23:49 +0900 Subject: [PATCH 2/5] make sure not report bogus frame type --- lib/quicly.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/quicly.c b/lib/quicly.c index 7b27a063e..d3f223d73 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -5404,6 +5404,8 @@ static int handle_payload(quicly_conn_t *conn, size_t epoch, const uint8_t *_src /* slow path */ --state.src; if ((state.frame_type = quicly_decodev(&state.src, state.end)) == UINT64_MAX) { + state.frame_type = + QUICLY_FRAME_TYPE_PADDING; /* we cannot signal the offending frame type when failing to decode the frame type */ ret = QUICLY_TRANSPORT_ERROR_FRAME_ENCODING; break; } From b62ed8285fd61f23a7dd973133c7b4c6396d5c12 Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Thu, 12 Nov 2020 18:25:41 +0900 Subject: [PATCH 3/5] rename --- lib/quicly.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/quicly.c b/lib/quicly.c index d3f223d73..de1690e47 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -3873,22 +3873,22 @@ size_t quicly_send_retry(quicly_context_t *ctx, ptls_aead_context_t *token_encry return ret == 0 ? buf.off : SIZE_MAX; } -static struct st_quicly_pn_space_t *setup_send_epoch(quicly_conn_t *conn, size_t epoch, quicly_send_context_t *s) +static struct st_quicly_pn_space_t *setup_send_space(quicly_conn_t *conn, size_t epoch, quicly_send_context_t *s) { - struct st_quicly_pn_space_t *ack_space = NULL; + struct st_quicly_pn_space_t *pn_space = NULL; switch (epoch) { case QUICLY_EPOCH_INITIAL: if (conn->initial == NULL || (s->current.cipher = &conn->initial->cipher.egress)->aead == NULL) return NULL; s->current.first_byte = QUICLY_PACKET_TYPE_INITIAL; - ack_space = &conn->initial->super; + pn_space = &conn->initial->super; break; case QUICLY_EPOCH_HANDSHAKE: if (conn->handshake == NULL || (s->current.cipher = &conn->handshake->cipher.egress)->aead == NULL) return NULL; s->current.first_byte = QUICLY_PACKET_TYPE_HANDSHAKE; - ack_space = &conn->handshake->super; + pn_space = &conn->handshake->super; break; case QUICLY_EPOCH_0RTT: case QUICLY_EPOCH_1RTT: @@ -3898,14 +3898,14 @@ static struct st_quicly_pn_space_t *setup_send_epoch(quicly_conn_t *conn, size_t return NULL; s->current.cipher = &conn->application->cipher.egress.key; s->current.first_byte = epoch == QUICLY_EPOCH_0RTT ? QUICLY_PACKET_TYPE_0RTT : QUICLY_QUIC_BIT; - ack_space = &conn->application->super; + pn_space = &conn->application->super; break; default: assert(!"logic flaw"); break; } - return ack_space; + return pn_space; } static int send_handshake_flow(quicly_conn_t *conn, size_t epoch, quicly_send_context_t *s, int ack_only, int send_probe) @@ -3914,7 +3914,7 @@ static int send_handshake_flow(quicly_conn_t *conn, size_t epoch, quicly_send_co int ret = 0; /* setup send epoch, or return if it's impossible to send in this epoch */ - if ((ack_space = setup_send_epoch(conn, epoch, s)) == NULL) + if ((ack_space = setup_send_space(conn, epoch, s)) == NULL) return 0; /* send ACK */ @@ -3955,7 +3955,7 @@ static int send_connection_close(quicly_conn_t *conn, size_t epoch, quicly_send_ int ret; /* setup send epoch, or return if it's impossible to send in this epoch */ - if (setup_send_epoch(conn, epoch, s) == NULL) + if (setup_send_space(conn, epoch, s) == NULL) return 0; /* determine the payload, masking the application error when sending the frame using an unauthenticated epoch */ @@ -4184,7 +4184,7 @@ static int do_send(quicly_conn_t *conn, quicly_send_context_t *s) /* setup 0-RTT or 1-RTT send context (as the availability of the two epochs are mutually exclusive, we can try 1-RTT first as an * optimization), then send application data if that succeeds */ - if (setup_send_epoch(conn, QUICLY_EPOCH_1RTT, s) != NULL || setup_send_epoch(conn, QUICLY_EPOCH_0RTT, s) != NULL) { + if (setup_send_space(conn, QUICLY_EPOCH_1RTT, s) != NULL || setup_send_space(conn, QUICLY_EPOCH_0RTT, s) != NULL) { /* acks */ if (conn->application->one_rtt_writable && conn->egress.send_ack_at <= conn->stash.now && conn->application->super.unacked_count != 0) { From 82be7c64cdbdfffe7a7f78c41e07aaeae6afa75e Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Thu, 12 Nov 2020 18:28:55 +0900 Subject: [PATCH 4/5] we do not need to be verbose --- lib/quicly.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/quicly.c b/lib/quicly.c index de1690e47..8924272af 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -3875,20 +3875,20 @@ size_t quicly_send_retry(quicly_context_t *ctx, ptls_aead_context_t *token_encry static struct st_quicly_pn_space_t *setup_send_space(quicly_conn_t *conn, size_t epoch, quicly_send_context_t *s) { - struct st_quicly_pn_space_t *pn_space = NULL; + struct st_quicly_pn_space_t *space = NULL; switch (epoch) { case QUICLY_EPOCH_INITIAL: if (conn->initial == NULL || (s->current.cipher = &conn->initial->cipher.egress)->aead == NULL) return NULL; s->current.first_byte = QUICLY_PACKET_TYPE_INITIAL; - pn_space = &conn->initial->super; + space = &conn->initial->super; break; case QUICLY_EPOCH_HANDSHAKE: if (conn->handshake == NULL || (s->current.cipher = &conn->handshake->cipher.egress)->aead == NULL) return NULL; s->current.first_byte = QUICLY_PACKET_TYPE_HANDSHAKE; - pn_space = &conn->handshake->super; + space = &conn->handshake->super; break; case QUICLY_EPOCH_0RTT: case QUICLY_EPOCH_1RTT: @@ -3898,14 +3898,14 @@ static struct st_quicly_pn_space_t *setup_send_space(quicly_conn_t *conn, size_t return NULL; s->current.cipher = &conn->application->cipher.egress.key; s->current.first_byte = epoch == QUICLY_EPOCH_0RTT ? QUICLY_PACKET_TYPE_0RTT : QUICLY_QUIC_BIT; - pn_space = &conn->application->super; + space = &conn->application->super; break; default: assert(!"logic flaw"); break; } - return pn_space; + return space; } static int send_handshake_flow(quicly_conn_t *conn, size_t epoch, quicly_send_context_t *s, int ack_only, int send_probe) From 0cfef38c1889409d255008352ee9dd7316100d6f Mon Sep 17 00:00:00 2001 From: Kazuho Oku Date: Thu, 12 Nov 2020 19:43:20 +0900 Subject: [PATCH 5/5] rename --- lib/quicly.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/quicly.c b/lib/quicly.c index 8924272af..5eb64672e 100644 --- a/lib/quicly.c +++ b/lib/quicly.c @@ -3910,16 +3910,16 @@ static struct st_quicly_pn_space_t *setup_send_space(quicly_conn_t *conn, size_t static int send_handshake_flow(quicly_conn_t *conn, size_t epoch, quicly_send_context_t *s, int ack_only, int send_probe) { - struct st_quicly_pn_space_t *ack_space; + struct st_quicly_pn_space_t *space; int ret = 0; /* setup send epoch, or return if it's impossible to send in this epoch */ - if ((ack_space = setup_send_space(conn, epoch, s)) == NULL) + if ((space = setup_send_space(conn, epoch, s)) == NULL) return 0; /* send ACK */ - if (ack_space != NULL && (ack_space->unacked_count != 0 || send_probe)) - if ((ret = send_ack(conn, ack_space, s)) != 0) + if (space != NULL && (space->unacked_count != 0 || send_probe)) + if ((ret = send_ack(conn, space, s)) != 0) goto Exit; if (!ack_only) {