From 936463fe29e163de72a7fa5bead925a52723684f Mon Sep 17 00:00:00 2001 From: Dmitri Tikhonov Date: Thu, 5 Dec 2019 08:44:25 -0500 Subject: [PATCH] Release 2.7.1 - [BUGFIX] client: don't call ignore_init() in middle of batch send. ignore_init() makes an assumption that the send controller has access to all outgoing packets. This change wraps a few IETF full connection methods to delay calling ignore_init() until the engine returns all outgoing packets that were batched. - [BUGFIX] set errno to EAGAIN if sendmmsg() can't send all of them. This needs to be done because the value of errno may be lost on some platforms. - [BUGFIX] Typo that set all bits in sm_qflags lead to crashes. - [BUGFIX] Do not cancel header block processing after failure, as QPACK releases the reference in that case. - [CLEANUP] IETF encrypt: replace assert(0) with a warning. - Several small improvements to the test server. --- CHANGELOG | 16 +++ include/lsquic.h | 2 +- src/liblsquic/lsquic_enc_sess_ietf.c | 3 +- src/liblsquic/lsquic_full_conn_ietf.c | 157 ++++++++++++++++++-------- src/liblsquic/lsquic_stream.c | 3 +- test/http_server.c | 2 +- test/prog.c | 12 ++ test/test_common.c | 4 + 8 files changed, 150 insertions(+), 49 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index e4f8463..d927e6d 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,19 @@ +2019-12-05 + - 2.7.1 + - [BUGFIX] client: don't call ignore_init() in middle of batch send. + ignore_init() makes an assumption that the send controller has access + to all outgoing packets. This change wraps a few IETF full connection + methods to delay calling ignore_init() until the engine returns all + outgoing packets that were batched. + - [BUGFIX] set errno to EAGAIN if sendmmsg() can't send all of them. + This needs to be done because the value of errno may be lost on + some platforms. + - [BUGFIX] Typo that set all bits in sm_qflags lead to crashes. + - [BUGFIX] Do not cancel header block processing after failure, as + QPACK releases the reference in that case. + - [CLEANUP] IETF encrypt: replace assert(0) with a warning. + - Several small improvements to the test server. + 2019-11-27 - 2.7.0 - [API, FEATURE] Close connection immediately when ea_packets_out() diff --git a/include/lsquic.h b/include/lsquic.h index d7d9052..0cbdccd 100644 --- a/include/lsquic.h +++ b/include/lsquic.h @@ -25,7 +25,7 @@ extern "C" { #define LSQUIC_MAJOR_VERSION 2 #define LSQUIC_MINOR_VERSION 7 -#define LSQUIC_PATCH_VERSION 0 +#define LSQUIC_PATCH_VERSION 1 /** * Engine flags: diff --git a/src/liblsquic/lsquic_enc_sess_ietf.c b/src/liblsquic/lsquic_enc_sess_ietf.c index 6dfaa83..182c136 100644 --- a/src/liblsquic/lsquic_enc_sess_ietf.c +++ b/src/liblsquic/lsquic_enc_sess_ietf.c @@ -1707,7 +1707,8 @@ iquic_esf_encrypt_packet (enc_session_t *enc_session_p, } else { - assert(0); + LSQ_WARN("no keys for encryption level %s", + lsquic_enclev2str[enc_level]); return ENCPA_BADCRYPT; } diff --git a/src/liblsquic/lsquic_full_conn_ietf.c b/src/liblsquic/lsquic_full_conn_ietf.c index 8844eb4..b261850 100644 --- a/src/liblsquic/lsquic_full_conn_ietf.c +++ b/src/liblsquic/lsquic_full_conn_ietf.c @@ -369,7 +369,9 @@ struct ietf_full_conn uint64_t ifcli_max_push_id; enum { IFCLI_PUSH_ENABLED = 1 << 0, + IFCLI_HSK_SENT_OR_DEL = 1 << 1, } ifcli_flags; + unsigned ifcli_packets_out; } cli; struct { uint64_t ifser_max_push_id; @@ -397,6 +399,7 @@ struct ietf_full_conn static const struct ver_neg server_ver_neg; static const struct conn_iface *ietf_full_conn_iface_ptr; +static const struct conn_iface *ietf_full_conn_prehsk_iface_ptr; static int process_incoming_packet_verneg (struct ietf_full_conn *, @@ -880,7 +883,10 @@ static int ietf_full_conn_init (struct ietf_full_conn *conn, struct lsquic_engine_public *enpub, unsigned flags, int ecn) { - conn->ifc_conn.cn_if = ietf_full_conn_iface_ptr; + if (flags & IFC_SERVER) + conn->ifc_conn.cn_if = ietf_full_conn_iface_ptr; + else + conn->ifc_conn.cn_if = ietf_full_conn_prehsk_iface_ptr; if (enpub->enp_settings.es_scid_len) assert(CN_SCID(&conn->ifc_conn)->len); conn->ifc_enpub = enpub; @@ -3699,6 +3705,20 @@ ietf_full_conn_ci_next_packet_to_send (struct lsquic_conn *lconn, size_t size) } +static struct lsquic_packet_out * +ietf_full_conn_ci_next_packet_to_send_pre_hsk (struct lsquic_conn *lconn, + size_t size) +{ + struct ietf_full_conn *conn = (struct ietf_full_conn *) lconn; + struct lsquic_packet_out *packet_out; + + packet_out = ietf_full_conn_ci_next_packet_to_send(lconn, size); + if (packet_out) + ++conn->ifc_u.cli.ifcli_packets_out; + return packet_out; +} + + static lsquic_time_t ietf_full_conn_ci_next_tick_time (struct lsquic_conn *lconn, unsigned *why) { @@ -5601,11 +5621,14 @@ ignore_init (struct ietf_full_conn *conn) lsquic_send_ctl_empty_pns(&conn->ifc_send_ctl, PNS_INIT); lsquic_rechist_cleanup(&conn->ifc_rechist[PNS_INIT]); if (!(conn->ifc_flags & IFC_SERVER)) + { if (conn->ifc_u.cli.crypto_streams[ENC_LEV_CLEAR]) { lsquic_stream_destroy(conn->ifc_u.cli.crypto_streams[ENC_LEV_CLEAR]); conn->ifc_u.cli.crypto_streams[ENC_LEV_CLEAR] = NULL; } + conn->ifc_conn.cn_if = ietf_full_conn_iface_ptr; + } } @@ -5965,6 +5988,34 @@ ietf_full_conn_ci_packet_not_sent (struct lsquic_conn *lconn, } +/* Calling of ignore_init() must be delayed until all batched packets have + * been returned by the engine. + */ +static void +pre_hsk_packet_sent_or_delayed (struct ietf_full_conn *conn, + const struct lsquic_packet_out *packet_out) +{ + /* Once IFC_IGNORE_INIT is set, the pre-hsk wrapper is removed: */ + assert(!(conn->ifc_flags & IFC_IGNORE_INIT)); + --conn->ifc_u.cli.ifcli_packets_out; + if (PNS_HSK == lsquic_packet_out_pns(packet_out)) + conn->ifc_u.cli.ifcli_flags |= IFCLI_HSK_SENT_OR_DEL; + if (0 == conn->ifc_u.cli.ifcli_packets_out + && (conn->ifc_u.cli.ifcli_flags & IFCLI_HSK_SENT_OR_DEL)) + ignore_init(conn); +} + + +static void +ietf_full_conn_ci_packet_not_sent_pre_hsk (struct lsquic_conn *lconn, + struct lsquic_packet_out *packet_out) +{ + struct ietf_full_conn *conn = (struct ietf_full_conn *) lconn; + ietf_full_conn_ci_packet_not_sent(lconn, packet_out); + pre_hsk_packet_sent_or_delayed(conn, packet_out); +} + + static void ietf_full_conn_ci_packet_sent (struct lsquic_conn *lconn, struct lsquic_packet_out *packet_out) @@ -5981,11 +6032,16 @@ ietf_full_conn_ci_packet_sent (struct lsquic_conn *lconn, ABORT_ERROR("sent packet failed: %s", strerror(errno)); ++conn->ifc_ecn_counts_out[ lsquic_packet_out_pns(packet_out) ] [ lsquic_packet_out_ecn(packet_out) ]; - if (0 == (conn->ifc_flags & (IFC_SERVER|IFC_IGNORE_INIT))) - { - if (PNS_HSK == lsquic_packet_out_pns(packet_out)) - ignore_init(conn); - } +} + + +static void +ietf_full_conn_ci_packet_sent_pre_hsk (struct lsquic_conn *lconn, + struct lsquic_packet_out *packet_out) +{ + struct ietf_full_conn *const conn = (struct ietf_full_conn *) lconn; + ietf_full_conn_ci_packet_sent(lconn, packet_out); + pre_hsk_packet_sent_or_delayed(conn, packet_out); } @@ -6586,49 +6642,60 @@ ietf_full_conn_ci_drop_crypto_streams (struct lsquic_conn *lconn) } -static const struct conn_iface ietf_full_conn_iface = { - .ci_abort = ietf_full_conn_ci_abort, - .ci_abort_error = ietf_full_conn_ci_abort_error, - .ci_retire_cid = ietf_full_conn_ci_retire_cid, - .ci_can_write_ack = ietf_full_conn_ci_can_write_ack, - .ci_cancel_pending_streams = ietf_full_conn_ci_cancel_pending_streams, - .ci_client_call_on_new = ietf_full_conn_ci_client_call_on_new, - .ci_close = ietf_full_conn_ci_close, - .ci_destroy = ietf_full_conn_ci_destroy, - .ci_drain_time = ietf_full_conn_ci_drain_time, - .ci_drop_crypto_streams = ietf_full_conn_ci_drop_crypto_streams, - .ci_get_ctx = ietf_full_conn_ci_get_ctx, - .ci_get_engine = ietf_full_conn_ci_get_engine, - .ci_get_log_cid = ietf_full_conn_ci_get_log_cid, - .ci_get_path = ietf_full_conn_ci_get_path, - .ci_get_stream_by_id = ieft_full_conn_ci_get_stream_by_id, - .ci_going_away = ietf_full_conn_ci_going_away, - .ci_hsk_done = ietf_full_conn_ci_hsk_done, - .ci_internal_error = ietf_full_conn_ci_internal_error, - .ci_is_push_enabled = ietf_full_conn_ci_is_push_enabled, - .ci_is_tickable = ietf_full_conn_ci_is_tickable, - .ci_make_stream = ietf_full_conn_ci_make_stream, - .ci_n_avail_streams = ietf_full_conn_ci_n_avail_streams, - .ci_n_pending_streams = ietf_full_conn_ci_n_pending_streams, - .ci_next_packet_to_send = ietf_full_conn_ci_next_packet_to_send, - .ci_next_tick_time = ietf_full_conn_ci_next_tick_time, - .ci_packet_in = ietf_full_conn_ci_packet_in, - .ci_packet_not_sent = ietf_full_conn_ci_packet_not_sent, - .ci_packet_sent = ietf_full_conn_ci_packet_sent, - .ci_push_stream = ietf_full_conn_ci_push_stream, - .ci_record_addrs = ietf_full_conn_ci_record_addrs, - .ci_report_live = ietf_full_conn_ci_report_live, - .ci_set_ctx = ietf_full_conn_ci_set_ctx, - .ci_status = ietf_full_conn_ci_status, - .ci_stateless_reset = ietf_full_conn_ci_stateless_reset, - .ci_tick = ietf_full_conn_ci_tick, - .ci_tls_alert = ietf_full_conn_ci_tls_alert, - .ci_write_ack = ietf_full_conn_ci_write_ack, -}; +#define IETF_FULL_CONN_FUNCS \ + .ci_abort = ietf_full_conn_ci_abort, \ + .ci_abort_error = ietf_full_conn_ci_abort_error, \ + .ci_retire_cid = ietf_full_conn_ci_retire_cid, \ + .ci_can_write_ack = ietf_full_conn_ci_can_write_ack, \ + .ci_cancel_pending_streams = ietf_full_conn_ci_cancel_pending_streams, \ + .ci_client_call_on_new = ietf_full_conn_ci_client_call_on_new, \ + .ci_close = ietf_full_conn_ci_close, \ + .ci_destroy = ietf_full_conn_ci_destroy, \ + .ci_drain_time = ietf_full_conn_ci_drain_time, \ + .ci_drop_crypto_streams = ietf_full_conn_ci_drop_crypto_streams, \ + .ci_get_ctx = ietf_full_conn_ci_get_ctx, \ + .ci_get_engine = ietf_full_conn_ci_get_engine, \ + .ci_get_log_cid = ietf_full_conn_ci_get_log_cid, \ + .ci_get_path = ietf_full_conn_ci_get_path, \ + .ci_get_stream_by_id = ieft_full_conn_ci_get_stream_by_id, \ + .ci_going_away = ietf_full_conn_ci_going_away, \ + .ci_hsk_done = ietf_full_conn_ci_hsk_done, \ + .ci_internal_error = ietf_full_conn_ci_internal_error, \ + .ci_is_push_enabled = ietf_full_conn_ci_is_push_enabled, \ + .ci_is_tickable = ietf_full_conn_ci_is_tickable, \ + .ci_make_stream = ietf_full_conn_ci_make_stream, \ + .ci_n_avail_streams = ietf_full_conn_ci_n_avail_streams, \ + .ci_n_pending_streams = ietf_full_conn_ci_n_pending_streams, \ + .ci_next_tick_time = ietf_full_conn_ci_next_tick_time, \ + .ci_packet_in = ietf_full_conn_ci_packet_in, \ + .ci_push_stream = ietf_full_conn_ci_push_stream, \ + .ci_record_addrs = ietf_full_conn_ci_record_addrs, \ + .ci_report_live = ietf_full_conn_ci_report_live, \ + .ci_set_ctx = ietf_full_conn_ci_set_ctx, \ + .ci_status = ietf_full_conn_ci_status, \ + .ci_stateless_reset = ietf_full_conn_ci_stateless_reset, \ + .ci_tick = ietf_full_conn_ci_tick, \ + .ci_tls_alert = ietf_full_conn_ci_tls_alert, \ + .ci_write_ack = ietf_full_conn_ci_write_ack +static const struct conn_iface ietf_full_conn_iface = { + IETF_FULL_CONN_FUNCS, + .ci_next_packet_to_send = ietf_full_conn_ci_next_packet_to_send, + .ci_packet_not_sent = ietf_full_conn_ci_packet_not_sent, + .ci_packet_sent = ietf_full_conn_ci_packet_sent, +}; static const struct conn_iface *ietf_full_conn_iface_ptr = &ietf_full_conn_iface; +static const struct conn_iface ietf_full_conn_prehsk_iface = { + IETF_FULL_CONN_FUNCS, + .ci_next_packet_to_send = ietf_full_conn_ci_next_packet_to_send_pre_hsk, + .ci_packet_not_sent = ietf_full_conn_ci_packet_not_sent_pre_hsk, + .ci_packet_sent = ietf_full_conn_ci_packet_sent_pre_hsk, +}; +static const struct conn_iface *ietf_full_conn_prehsk_iface_ptr = + &ietf_full_conn_prehsk_iface; + static void on_cancel_push (void *ctx, uint64_t push_id) diff --git a/src/liblsquic/lsquic_stream.c b/src/liblsquic/lsquic_stream.c index a507993..f18a585 100644 --- a/src/liblsquic/lsquic_stream.c +++ b/src/liblsquic/lsquic_stream.c @@ -3610,7 +3610,7 @@ lsquic_stream_reset_ext (lsquic_stream_t *stream, uint64_t error_code, if (stream->sm_qflags & SMQF_QPACK_DEC) { lsquic_qdh_cancel_stream(stream->conn_pub->u.ietf.qdh, stream); - stream->sm_qflags |= ~SMQF_QPACK_DEC; + stream->sm_qflags &= ~SMQF_QPACK_DEC; } drop_buffered_data(stream); @@ -4246,6 +4246,7 @@ hq_read (void *ctx, const unsigned char *buf, size_t sz, int fin) goto end; default: assert(LQRHS_ERROR == rhs); + stream->sm_qflags &= ~SMQF_QPACK_DEC; filter->hqfi_flags |= HQFI_FLAG_ERROR; LSQ_INFO("error processing header block"); abort_connection(stream); /* XXX Overkill? */ diff --git a/test/http_server.c b/test/http_server.c index 871e6d8..02fcb12 100644 --- a/test/http_server.c +++ b/test/http_server.c @@ -983,7 +983,7 @@ send_headers2 (struct lsquic_stream *stream, struct lsquic_stream_ctx *st_h, }, { .name = { .iov_base = "content-type", .iov_len = 12, }, - .value = { .iov_base = "application/html", .iov_len = 16, }, + .value = { .iov_base = "text/html", .iov_len = 9, }, }, { .name = { .iov_base = "content-length", .iov_len = 14, }, diff --git a/test/prog.c b/test/prog.c index 69f3f53..e6990a4 100644 --- a/test/prog.c +++ b/test/prog.c @@ -613,6 +613,12 @@ static const struct lsquic_keylog_if keylog_if = }; +static struct ssl_ctx_st * +no_cert (void *cert_lu_ctx, const struct sockaddr *sa_UNUSED, const char *sni) +{ + return NULL; +} + int prog_prep (struct prog *prog) @@ -656,6 +662,12 @@ prog_prep (struct prog *prog) prog->prog_api.ea_lookup_cert = lookup_cert; prog->prog_api.ea_cert_lu_ctx = prog->prog_certs; } + else + { + if (prog->prog_engine_flags & LSENG_SERVER) + LSQ_WARN("Not a single service specified. Use -c option."); + prog->prog_api.ea_lookup_cert = no_cert; + } prog->prog_eb = event_base_new(); prog->prog_engine = lsquic_engine_new(prog->prog_engine_flags, diff --git a/test/test_common.c b/test/test_common.c index 3ce1e0d..988b66f 100644 --- a/test/test_common.c +++ b/test/test_common.c @@ -1441,6 +1441,10 @@ send_packets_using_sendmmsg (const struct lsquic_out_spec *specs, LSQ_WARN("sendmmsg failed: %s", strerror(saved_errno)); errno = saved_errno; } + else if (s > 0) + errno = EAGAIN; + else + errno = saved_errno; } return s;