Release 2.29.6

- Documentation: describe lsquic internals ("guts").
- Two more fixes to compliance issues found by h3spec.
- Truncate, don't abort, SCIDs larger than 16 bytes (PR #244).
- Several small internal improvements and space optimizations.

CHANGELOG

@@ -1,3 +1,10 @@
+2021-03-31
+    - 2.29.6
+    - Documentation: describe lsquic internals ("guts").
+    - Two more fixes to compliance issues found by h3spec.
+    - Truncate, don't abort, SCIDs larger than 16 bytes (PR #244).
+    - Several small internal improvements and space optimizations.
+
 2021-03-17
     - 2.29.5
     - Fix a few issues detected by h3spec for better compliance with HTTP/3

bin/http_server.c

@@ -418,6 +418,11 @@ struct req
     enum {
         HAVE_XHDR   = 1 << 0,
     }            flags;
+    enum {
+        PH_AUTHORITY    = 1 << 0,
+        PH_METHOD       = 1 << 1,
+        PH_PATH         = 1 << 2,
+    }            pseudo_headers;
     char        *path;
     char        *method_str;
     char        *authority_str;
@@ -1829,7 +1834,16 @@ interop_server_hset_add_header (void *hset_p, struct lsxpack_header *xhdr)
     unsigned name_len, value_len;
 
     if (!xhdr)
-        return 0;
+    {
+        if (req->pseudo_headers == (PH_AUTHORITY|PH_METHOD|PH_PATH))
+            return 0;
+        else
+        {
+            LSQ_INFO("%s: missing some pseudo-headers: 0x%X", __func__,
+                req->pseudo_headers);
+            return 1;
+        }
+    }
 
     name = lsxpack_header_get_name(xhdr);
     value = lsxpack_header_get_value(xhdr);
@@ -1856,6 +1870,7 @@ interop_server_hset_add_header (void *hset_p, struct lsxpack_header *xhdr)
         req->path = strndup(value, value_len);
         if (!req->path)
             return -1;
+        req->pseudo_headers |= PH_PATH;
         return 0;
     }
 
@@ -1872,6 +1887,7 @@ interop_server_hset_add_header (void *hset_p, struct lsxpack_header *xhdr)
             req->method = POST;
         else
             req->method = UNSUPPORTED;
+        req->pseudo_headers |= PH_METHOD;
         return 0;
     }
 
@@ -1880,6 +1896,7 @@ interop_server_hset_add_header (void *hset_p, struct lsxpack_header *xhdr)
         req->authority_str = strndup(value, value_len);
         if (!req->authority_str)
             return -1;
+        req->pseudo_headers |= PH_AUTHORITY;
         return 0;
     }
 

docs/conf.py

@@ -26,7 +26,7 @@ author = u'LiteSpeed Technologies'
 # The short X.Y version
 version = u'2.29'
 # The full version, including alpha/beta/rc tags
-release = u'2.29.5'
+release = u'2.29.6'
 
 
 # -- General configuration ---------------------------------------------------

docs/index.rst

@@ -65,8 +65,8 @@ Contents
    gettingstarted
    tutorial
    apiref
-   internals
    devel
+   internals
    faq
 
 Indices and tables

include/lsquic.h

@@ -25,7 +25,7 @@ extern "C" {
 
 #define LSQUIC_MAJOR_VERSION 2
 #define LSQUIC_MINOR_VERSION 29
-#define LSQUIC_PATCH_VERSION 5
+#define LSQUIC_PATCH_VERSION 6
 
 /**
  * Engine flags:

src/liblsquic/lsquic_conn.c

@@ -242,7 +242,10 @@ void
 lsquic_generate_scid (void *ctx, struct lsquic_conn *lconn, lsquic_cid_t *scid,
                                                                 unsigned len)
 {
-    lsquic_generate_cid(scid, len);
+    if (len)
+        lsquic_generate_cid(scid, len);
+    else
+        scid->len = len;
 }
 
 

src/liblsquic/lsquic_engine.c

@@ -64,7 +64,6 @@
 #include "lsquic_sfcw.h"
 #include "lsquic_hash.h"
 #include "lsquic_conn.h"
-#include "lsquic_send_ctl.h"
 #include "lsquic_full_conn.h"
 #include "lsquic_util.h"
 #include "lsquic_qtags.h"
@@ -2908,6 +2907,8 @@ process_connections (lsquic_engine_t *engine, conn_iter_f next_conn,
         }
     }
 
+    cub_flush(&engine->new_scids);
+
     if ((engine->pub.enp_flags & ENPUB_CAN_SEND)
                         && lsquic_engine_has_unsent_packets(engine))
         send_packets_out(engine, &ticked_conns, &closed_conns);
@@ -2948,7 +2949,6 @@ process_connections (lsquic_engine_t *engine, conn_iter_f next_conn,
         }
     }
 
-    cub_flush(&engine->new_scids);
     cub_flush(&cub_live);
     cub_flush(&cub_old);
 }

src/liblsquic/lsquic_full_conn_ietf.c

@@ -158,6 +158,7 @@ enum more_flags
     MF_SEND_WRONG_COUNTS= 1 << 5,   /* Send wrong ECN counts to peer */
     MF_WANT_DATAGRAM_WRITE  = 1 << 6,
     MF_DOING_0RTT       = 1 << 7,
+    MF_HAVE_HCSI        = 1 << 8,   /* Have HTTP Control Stream Incoming */
 };
 
 
@@ -402,8 +403,6 @@ struct ietf_full_conn
     /* App PNS only, used to calculate was_missing: */
     lsquic_packno_t             ifc_max_ackable_packno_in;
     struct lsquic_send_ctl      ifc_send_ctl;
-    struct lsquic_stream       *ifc_stream_hcsi;    /* HTTP Control Stream Incoming */
-    struct lsquic_stream       *ifc_stream_hcso;    /* HTTP Control Stream Outgoing */
     struct lsquic_conn_public   ifc_pub;
     lsquic_alarmset_t           ifc_alset;
     struct lsquic_set64         ifc_closed_stream_ids[N_SITS];
@@ -424,9 +423,6 @@ struct ietf_full_conn
     struct conn_err             ifc_error;
     unsigned                    ifc_n_delayed_streams;
     unsigned                    ifc_n_cons_unretx;
-    const struct lsquic_stream_if
-                               *ifc_stream_if;
-    void                       *ifc_stream_ctx;
     const struct prio_iter_if  *ifc_pii;
     char                       *ifc_errmsg;
     struct lsquic_engine_public
@@ -452,7 +448,6 @@ struct ietf_full_conn
     unsigned                    ifc_max_retx_since_last_ack;
     lsquic_time_t               ifc_max_ack_delay;
     uint64_t                    ifc_ecn_counts_in[N_PNS][4];
-    uint64_t                    ifc_ecn_counts_out[N_PNS][4];
     lsquic_stream_id_t          ifc_max_req_id;
     struct hcso_writer          ifc_hcso;
     struct http_ctl_stream_in   ifc_hcsi;
@@ -1672,7 +1667,6 @@ lsquic_ietf_full_conn_server_new (struct lsquic_engine_public *enpub,
         for (i = 0; i < 4; ++i)
         {
             conn->ifc_ecn_counts_in[pns][i]  = imc->imc_ecn_counts_in[pns][i];
-            conn->ifc_ecn_counts_out[pns][i] = imc->imc_ecn_counts_out[pns][i];
         }
     conn->ifc_incoming_ecn = imc->imc_incoming_ecn;
     conn->ifc_pub.rtt_stats = imc->imc_rtt_stats;
@@ -2040,6 +2034,7 @@ can_issue_cids (const struct ietf_full_conn *conn)
 
     can = ((1 << conn->ifc_conn.cn_n_cces) - 1
                                             != conn->ifc_conn.cn_cces_mask)
+       && conn->ifc_enpub->enp_settings.es_scid_len
        && conn->ifc_active_cids_count < conn->ifc_active_cids_limit;
     LSQ_DEBUG("can issue CIDs: %d (n_cces %hhu; mask: 0x%hhX; "
                                         "active: %hhu; limit: %hhu)",
@@ -3775,7 +3770,7 @@ handshake_ok (struct lsquic_conn *lconn)
     if (conn->ifc_settings->es_dplpmtud)
         conn->ifc_mflags |= MF_CHECK_MTU_PROBE;
 
-    if (can_issue_cids(conn) && CN_SCID(&conn->ifc_conn)->len != 0)
+    if (can_issue_cids(conn))
         conn->ifc_send_flags |= SF_SEND_NEW_CID;
     maybe_create_delayed_streams(conn);
 
@@ -7379,8 +7374,7 @@ process_regular_packet (struct ietf_full_conn *conn,
                                                     packet_in->pi_received);
     switch (st) {
     case REC_ST_OK:
-        if (!(conn->ifc_flags & (IFC_SERVER|IFC_DCID_SET))
+        if (!(conn->ifc_flags & (IFC_SERVER|IFC_DCID_SET)))
-                                                && (packet_in->pi_scid_len))
             record_dcid(conn, packet_in);
         saved_path_id = conn->ifc_cur_path_id;
         parse_regular_packet(conn, packet_in);
@@ -7742,8 +7736,6 @@ ietf_full_conn_ci_packet_sent (struct lsquic_conn *lconn,
     s = lsquic_send_ctl_sent_packet(&conn->ifc_send_ctl, packet_out);
     if (s != 0)
         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) ];
     /* Set blocked keep-alive for a [1,8] seconds */
     if (packet_out->po_frame_types
                             & (QUIC_FTBIT_BLOCKED|QUIC_FTBIT_STREAM_BLOCKED))
@@ -9398,7 +9390,7 @@ hcsi_on_new (void *stream_if_ctx, struct lsquic_stream *stream)
     struct ietf_full_conn *const conn = (void *) stream_if_ctx;
     const struct hcsi_callbacks *callbacks;
 
-    conn->ifc_stream_hcsi = stream;
+    conn->ifc_mflags |= MF_HAVE_HCSI;
 
     switch ((!!(conn->ifc_flags & IFC_SERVER) << 8) | conn->ifc_conn.cn_version)
     {
@@ -9488,8 +9480,6 @@ hcsi_on_write (struct lsquic_stream *stream, lsquic_stream_ctx_t *ctx)
 static void
 hcsi_on_close (struct lsquic_stream *stream, lsquic_stream_ctx_t *ctx)
 {
-    struct ietf_full_conn *const conn = (void *) ctx;
-    conn->ifc_stream_hcsi = NULL;
 }
 
 
@@ -9509,7 +9499,7 @@ apply_uni_stream_class (struct ietf_full_conn *conn,
     switch (stream_type)
     {
     case HQUST_CONTROL:
-        if (!conn->ifc_stream_hcsi)
+        if (!(conn->ifc_mflags & MF_HAVE_HCSI))
         {
             LSQ_DEBUG("Incoming HTTP control stream ID: %"PRIu64,
                                                             stream->id);
@@ -9518,9 +9508,7 @@ apply_uni_stream_class (struct ietf_full_conn *conn,
         else
         {
             ABORT_QUIETLY(1, HEC_STREAM_CREATION_ERROR,
-                "Control stream %"PRIu64" already exists: cannot create "
+                "Attempt to create second control stream");
-                "second control stream %"PRIu64, conn->ifc_stream_hcsi->id,
-                stream->id);
             lsquic_stream_close(stream);
         }
         break;

src/liblsquic/lsquic_mini_conn.c

@@ -11,10 +11,6 @@
  * spooled, if necessary.  When mini connection is promoted to full
  * connection, the state, including spooled incoming packets, is transferred
  * to the full connection.
- *
- * Note that mini connections do not retransmit lost packets.  This is to
- * minimize the effect of magnification attacks.  Clients like Chrome and
- * Opera fall back to using TCP if QUIC handshake times out.
  */
 
 

src/liblsquic/lsquic_mini_conn_ietf.c

@@ -52,6 +52,9 @@ static unsigned highest_bit_set (unsigned long long);
 static int
 imico_can_send (const struct ietf_mini_conn *, size_t);
 
+static void
+ietf_mini_conn_ci_abort_error (struct lsquic_conn *lconn, int is_app,
+                                unsigned error_code, const char *fmt, ...);
 
 static const enum header_type el2hety[] =
 {
@@ -1432,11 +1435,19 @@ ietf_mini_conn_ci_packet_in (struct lsquic_conn *lconn,
 
     dec_packin = lconn->cn_esf_c->esf_decrypt_packet(lconn->cn_enc_session,
                                 conn->imc_enpub, &conn->imc_conn, packet_in);
-    if (dec_packin != DECPI_OK)
+    switch (dec_packin)
     {
+    case DECPI_OK:
+        break;
+    case DECPI_VIOLATION:
+        ietf_mini_conn_ci_abort_error(lconn, 0, TEC_PROTOCOL_VIOLATION,
+                    "protocol violation detected while decrypting packet");
+        return;
+    case DECPI_NOT_YET:
+        imico_maybe_delay_processing(conn, packet_in);
+        return;
+    default:
         LSQ_DEBUG("could not decrypt packet");
-        if (DECPI_NOT_YET == dec_packin)
-            imico_maybe_delay_processing(conn, packet_in);
         return;
     }
 
@@ -1511,8 +1522,6 @@ ietf_mini_conn_ci_packet_sent (struct lsquic_conn *lconn,
         mc->mc_flags &= ~MC_UNSENT_ACK;
     }
 #endif
-    ++conn->imc_ecn_counts_out[ lsquic_packet_out_pns(packet_out) ]
-                              [ lsquic_packet_out_ecn(packet_out) ];
     if (packet_out->po_header_type == HETY_HANDSHAKE)
         conn->imc_flags |= IMC_HSK_PACKET_SENT;
     LSQ_DEBUG("%s: packet %"PRIu64" sent", __func__, packet_out->po_packno);

src/liblsquic/lsquic_mini_conn_ietf.h

@@ -105,7 +105,6 @@ struct ietf_mini_conn
     uint8_t                         imc_ecn_packnos;
     uint8_t                         imc_ack_exp;
     uint8_t                         imc_ecn_counts_in[IMICO_N_PNS][4];
-    uint8_t                         imc_ecn_counts_out[IMICO_N_PNS][4];
     uint8_t                         imc_incoming_ecn;
     uint8_t                         imc_tls_alert;
 #define IMICO_MAX_DELAYED_PACKETS_UNVALIDATED 1u

src/liblsquic/lsquic_packet_in.h

@@ -23,13 +23,14 @@ struct data_frame
 
 typedef struct stream_frame
 {
-    /* Stream frames are stored in a list inside stream. */
+    /* Stream frames are stored in a list inside "di nocopy" (if "di nocopy"
+     * is used).
+     */
     TAILQ_ENTRY(stream_frame)       next_frame;
 
-    /* `data' points somewhere into the packet payload.  The packet object
+    /* `data_frame.df_data' points somewhere into the packet payload.  The
-     * is reference-counted.  When the frame is freed, the packet is released
+     * packet object is reference-counted.  When the frame is freed, the
-     * via lsquic_packet_put().  If data_length is zero, the frame does not
+     * packet is released via lsquic_packet_in_put().
-     * keep a reference to the incoming packet and this pointer is not set.
      */
     struct lsquic_packet_in        *packet_in;
 

src/liblsquic/lsquic_qdec_hdl.c

@@ -553,6 +553,25 @@ qdh_maybe_set_user_agent (struct qpack_dec_hdl *qdh,
 }
 
 
+/* Intercept header errors so that upper-layer errors do not get
+ * misinterpreted as QPACK errors.
+ */
+static int
+qdh_hsi_process_wrapper (struct qpack_dec_hdl *qdh, void *hset,
+                                                struct lsxpack_header *xhdr)
+{
+    int retval;
+
+    retval = qdh->qdh_enpub->enp_hsi_if->hsi_process_header(hset, xhdr);
+    if (0 != retval)
+        qdh->qdh_conn->cn_if->ci_abort_error(qdh->qdh_conn, 1,
+            1 == retval ? HEC_MESSAGE_ERROR : HEC_INTERNAL_ERROR,
+            "error processing headers");
+
+    return retval;
+}
+
+
 static int
 qdh_process_header (void *stream_p, struct lsxpack_header *xhdr)
 {
@@ -584,7 +603,7 @@ qdh_process_header (void *stream_p, struct lsxpack_header *xhdr)
     else if ((qdh->qdh_flags & QDH_SAVE_UA) && !qdh->qdh_ua)
         qdh_maybe_set_user_agent(qdh, xhdr, &qdh->qdh_ua);
 
-    return qdh->qdh_enpub->enp_hsi_if->hsi_process_header(u->ctx.hset, xhdr);
+    return qdh_hsi_process_wrapper(qdh, u->ctx.hset, xhdr);
 }
 
 
@@ -643,8 +662,7 @@ qdh_header_read_results (struct qpack_dec_hdl *qdh,
             uh->uh_exclusive = -1;
             if (qdh->qdh_enpub->enp_hsi_if == lsquic_http1x_if)
                 uh->uh_flags    |= UH_H1H;
-            if (0 != qdh->qdh_enpub->enp_hsi_if
+            if (0 != qdh_hsi_process_wrapper(qdh, hset, NULL))
-                                        ->hsi_process_header(hset, NULL))
             {
                 LSQ_DEBUG("finishing hset failed");
                 free(uh);

tools/gen-tags.pl

@@ -54,6 +54,7 @@ while (<TMPFILE>)
      or s/^iquic_esf_/esf_/
      or s/^gquic[0-9]?_esf_/esf_/
      or s/^iquic_esfi_/esfi_/
+     or s/^lsquic_[sh]pi_/pii_/
      or s/^(lsquic_cubic|lsquic_bbr)_/cci_/
     )
     {