Release 2.14.5

- [BUGFIX] In coalesced datagram, ignore packets whose CID does not match.
- [BUGFIX] Frame reader: skip headers if target stream is not found.
- [BUGFIX] Log message in QPACK decoder handler.

CHANGELOG

@@ -1,3 +1,9 @@
+2020-04-29
+    - 2.14.5
+    - [BUGFIX] In coalesced datagram, ignore packets whose CID does not match.
+    - [BUGFIX] Frame reader: skip headers if target stream is not found.
+    - [BUGFIX] Log message in QPACK decoder handler.
+
 2020-04-24
     - 2.14.4
     - [BUGFIX] Heed es_rw_once for pushed HTTP/3 streams.

docs/conf.py

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

include/lsquic.h

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

src/liblsquic/lsquic_engine.c

@@ -2650,13 +2650,15 @@ lsquic_engine_packet_in (lsquic_engine_t *engine,
     const struct sockaddr *sa_local, const struct sockaddr *sa_peer,
     void *peer_ctx, int ecn)
 {
+    const unsigned char *const packet_begin = packet_in_data;
     const unsigned char *const packet_end = packet_in_data + packet_in_size;
     struct packin_parse_state ppstate;
     lsquic_packet_in_t *packet_in;
     int (*parse_packet_in_begin) (struct lsquic_packet_in *, size_t length,
                 int is_server, unsigned cid_len, struct packin_parse_state *);
     unsigned n_zeroes;
-    int s;
+    int s, is_ietf;
+    lsquic_cid_t cid;
 
     ENGINE_CALLS_INCR(engine);
 
@@ -2691,6 +2693,7 @@ lsquic_engine_packet_in (lsquic_engine_t *engine,
         parse_packet_in_begin = lsquic_parse_packet_in_begin;
 
     n_zeroes = 0;
+    is_ietf = 0;
     do
     {
         packet_in = lsquic_mm_get_packet_in(&engine->pub.enp_mm);
@@ -2711,7 +2714,24 @@ lsquic_engine_packet_in (lsquic_engine_t *engine,
             break;
         }
 
+        /* [draft-ietf-quic-transport-27] Section 12.2:
+         * " Receivers SHOULD ignore any subsequent packets with a different
+         * " Destination Connection ID than the first packet in the datagram.
+         */
+        if (is_ietf && packet_in_data > packet_begin)
+        {
+            if (!((packet_in->pi_flags & (PI_GQUIC|PI_CONN_ID)) == PI_CONN_ID
+                                && LSQUIC_CIDS_EQ(&packet_in->pi_dcid, &cid)))
+            {
                 packet_in_data += packet_in->pi_data_sz;
+                continue;
+            }
+        }
+
+        is_ietf = 0 == (packet_in->pi_flags & PI_GQUIC);
+        packet_in_data += packet_in->pi_data_sz;
+        if (is_ietf && packet_in_data < packet_end)
+            cid = packet_in->pi_dcid;
         packet_in->pi_received = lsquic_time_now();
         packet_in->pi_flags |= (3 & ecn) << PIBIT_ECN_SHIFT;
         eng_hist_inc(&engine->history, packet_in->pi_received, sl_packets_in);

src/liblsquic/lsquic_frame_reader.c

@@ -524,6 +524,44 @@ find_target_stream (const struct lsquic_frame_reader *fr)
 }
 
 
+static void
+skip_headers (struct lsquic_frame_reader *fr)
+{
+    const unsigned char *comp, *end;
+    void *buf;
+    int s;
+    struct lsxpack_header xhdr;
+    const size_t buf_len = 64 * 1024;
+
+    buf = malloc(buf_len);
+    if (!buf)
+    {
+        fr->fr_callbacks->frc_on_error(fr->fr_cb_ctx, fr_get_stream_id(fr),
+                                                        FR_ERR_OTHER_ERROR);
+        goto end;
+    }
+
+    comp = fr->fr_header_block;
+    end = comp + fr->fr_header_block_sz;
+    while (comp < end)
+    {
+        lsxpack_header_prepare_decode(&xhdr, buf, 0, buf_len);
+        s = lshpack_dec_decode(fr->fr_hdec, &comp, end, &xhdr);
+        if (s != 0)
+        {
+            fr->fr_callbacks->frc_on_error(fr->fr_cb_ctx, fr_get_stream_id(fr),
+                                                            FR_ERR_OTHER_ERROR);
+            break;
+        }
+    }
+
+  end:
+    if (buf)
+        free(buf);
+}
+
+
+/* TODO: this function always returns 0.  Make it void */
 static int
 decode_and_pass_payload (struct lsquic_frame_reader *fr)
 {
@@ -539,7 +577,20 @@ decode_and_pass_payload (struct lsquic_frame_reader *fr)
     lsquic_stream_t *target_stream = NULL;
 
     if (!(fr->fr_flags & FRF_SERVER))
+    {
         target_stream = find_target_stream(fr);
+        /* If the response is for a stream that cannot be found, one of two
+         * things is true: a) the stream has been closed or b) this is an
+         * error.  If (a), we discard this header block.  We choose to do the
+         * same for (b) instead of erroring out for the sake of simplicity.
+         * There is no way to exploit this behavior.
+         */
+        if (!target_stream)
+        {
+            skip_headers(fr);
+            return 0;
+        }
+    }
     hset = fr->fr_hsi_if->hsi_create_header_set(fr->fr_hsi_ctx, target_stream,
                             READER_PUSH_PROMISE == fr->fr_state.reader_type);
     if (!hset)

test/unittests/test_frame_reader.c

@@ -1100,6 +1100,13 @@ static const struct frame_reader_test tests[] = {
 };
 
 
+static struct lsquic_stream *
+my_get_stream_by_id (struct lsquic_conn *conn, lsquic_stream_id_t stream_id)
+{
+    return (void *) my_get_stream_by_id;
+}
+
+
 static void
 test_one_frt (const struct frame_reader_test *frt)
 {
@@ -1122,10 +1129,12 @@ test_one_frt (const struct frame_reader_test *frt)
     memset(&lconn, 0, sizeof(lconn));
     memset(&conn_pub, 0, sizeof(conn_pub));
     memset(&my_conn_if, 0, sizeof(my_conn_if));
+    my_conn_if.ci_get_stream_by_id = my_get_stream_by_id;
     lconn.cn_if = &my_conn_if;
     stream.conn_pub = &conn_pub;
     conn_pub.lconn = &lconn;
 
+  top:
     lsquic_mm_init(&mm);
     lshpack_dec_init(&hdec, LSHPACK_DEC_HTTP1X);
     memset(&input, 0, sizeof(input));
@@ -1154,11 +1163,14 @@ test_one_frt (const struct frame_reader_test *frt)
 
         assert(frt->frt_err || 0 == s);
 
+        if (my_conn_if.ci_get_stream_by_id)
+        {
             assert(g_cb_ctx.n_cb_vals == frt->frt_n_cb_vals);
 
             unsigned i;
             for (i = 0; i < g_cb_ctx.n_cb_vals; ++i)
                 compare_cb_vals(&g_cb_ctx.cb_vals[i], &frt->frt_cb_vals[i]);
+        }
 
         exp_off = frt->frt_in_off;
         if (!exp_off)
@@ -1169,6 +1181,14 @@ test_one_frt (const struct frame_reader_test *frt)
     }
     while (input.in_max_sz < input.in_max_req_sz);
     lshpack_dec_cleanup(&hdec);
+
+    if (!(frt->frt_fr_flags & FRF_SERVER) && my_conn_if.ci_get_stream_by_id)
+    {
+        /* Do it again, but this time test header block skip logic */
+        my_conn_if.ci_get_stream_by_id = NULL;
+        goto top;
+    }
+
     lsquic_mm_cleanup(&mm);
 }
 

test/unittests/test_frame_rw.c

@@ -58,7 +58,13 @@ lsquic_conn_id (const lsquic_conn_t *lconn)
     return &my_cid;
 }
 
-static const struct conn_iface s_if;
+static struct lsquic_stream *
+my_get_stream_by_id (struct lsquic_conn *conn, lsquic_stream_id_t stream_id)
+{
+    return (void *) my_get_stream_by_id;
+}
+
+static const struct conn_iface s_if = { .ci_get_stream_by_id = my_get_stream_by_id, };
 static struct lsquic_conn s_conn = { .cn_if = &s_if, };
 
 #if !defined(NDEBUG) && __GNUC__