Release 2.6.2

- [BUGFIX] SCID!=ODCID rule applies to Retry packets, not regular
  packets.
- [BUGFIX] Zero-RTT: BoringSSL no longer flips read/write secrets.
- [BUGFIX] Truncate ACK frame rather instead of aborting IETF
  connection.
- [BUGFIX] Client: don't send duplicate reset tokens.
- [BUGFIX] Remove invalid assertion in H3 framing code.
- Silence a warning in send ctl by restructuring switch() statement.

CHANGELOG

@@ -1,3 +1,14 @@
+2019-11-11
+    - 2.6.2
+    - [BUGFIX] SCID!=ODCID rule applies to Retry packets, not regular
+      packets.
+    - [BUGFIX] Zero-RTT: BoringSSL no longer flips read/write secrets.
+    - [BUGFIX] Truncate ACK frame rather instead of aborting IETF
+      connection.
+    - [BUGFIX] Client: don't send duplicate reset tokens.
+    - [BUGFIX] Remove invalid assertion in H3 framing code.
+    - Silence a warning in send ctl by restructuring switch() statement.
+
 2019-11-08
     - 2.6.1
     - [BUGFIX] set retry token on all resubmitted packets.

README.md

@@ -15,8 +15,8 @@ and OpenLiteSpeed.  We think it is free of major problems.  Nevertheless, do
 not hesitate to report bugs back to us.  Even better, send us fixes and
 improvements!
 
-Currently supported QUIC versions are Q039, Q043, Q046, and ID-23.  Support
-for newer versions will be added soon after they are released.
+Currently supported QUIC versions are Q039, Q043, Q046, ID-23, and ID-24.
+Support for newer versions will be added soon after they are released.
 
 Documentation
 -------------

include/lsquic.h

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

src/liblsquic/lsquic_enc_sess_ietf.c

@@ -2362,19 +2362,9 @@ cry_sm_set_encryption_secret (SSL *ssl, enum ssl_encryption_level_t level,
         return 0;
 
     if (enc_sess->esi_flags & ESI_SERVER)
-    {
-        if (enc_level != ENC_LEV_EARLY)
-            secrets[0] = read_secret, secrets[1] = write_secret;
-        else
-            secrets[1] = read_secret, secrets[0] = write_secret;
-    }
+        secrets[0] = read_secret, secrets[1] = write_secret;
     else
-    {
-        if (enc_level != ENC_LEV_EARLY)
-            secrets[0] = write_secret, secrets[1] = read_secret;
-        else
-            secrets[1] = write_secret, secrets[0] = read_secret;
-    }
+        secrets[0] = write_secret, secrets[1] = read_secret;
 
     if (enc_level < ENC_LEV_FORW)
     {

src/liblsquic/lsquic_engine.c

@@ -536,11 +536,11 @@ lsquic_engine_new (unsigned flags,
     if (hash_conns_by_addr(engine))
         engine->flags |= ENG_CONNS_BY_ADDR;
     engine->conns_hash = lsquic_hash_create();
+    engine->pub.enp_tokgen = lsquic_tg_new(&engine->pub);
+    if (!engine->pub.enp_tokgen)
+        return NULL;
     if (flags & ENG_SERVER)
     {
-        engine->pub.enp_tokgen = lsquic_tg_new(&engine->pub);
-        if (!engine->pub.enp_tokgen)
-            return NULL;
         engine->pr_queue = prq_create(
             10000 /* TODO: make configurable */, MAX_OUT_BATCH_SIZE,
             &engine->pub);

src/liblsquic/lsquic_full_conn_ietf.c

@@ -1512,11 +1512,8 @@ generate_new_cid_frame (struct ietf_full_conn *conn, lsquic_time_t now)
         return -1;
     }
 
-    if (conn->ifc_flags & IFC_SERVER)
-        lsquic_tg_generate_sreset(conn->ifc_enpub->enp_tokgen, &cce->cce_cid,
+    lsquic_tg_generate_sreset(conn->ifc_enpub->enp_tokgen, &cce->cce_cid,
                                                                     token_buf);
-    else
-        memset(token_buf, 0, sizeof(token_buf));
 
     if (0 != lsquic_engine_add_cid(conn->ifc_enpub, &conn->ifc_conn,
                                                         cce - conn->ifc_cces))
@@ -4880,6 +4877,7 @@ process_new_connection_id_frame (struct ietf_full_conn *conn,
     lsquic_cid_t cid;
     uint64_t seqno, retire_prior_to;
     int parsed_len, update_cur_dcid;
+    char tokstr[IQUIC_SRESET_TOKEN_SZ * 2 + 1];
 
     parsed_len = conn->ifc_conn.cn_pf->pf_parse_new_conn_id(p, len,
                                         &seqno, &retire_prior_to, &cid, &token);
@@ -4943,6 +4941,18 @@ process_new_connection_id_frame (struct ietf_full_conn *conn,
                     "numbers %u and %"PRIu64, (*el)->de_seqno, seqno);
                 return 0;
             }
+            else if (((*el)->de_flags & DE_SRST)
+                    && 0 == memcmp((*el)->de_srst, token,
+                                                    IQUIC_SRESET_TOKEN_SZ))
+            {
+                ABORT_QUIETLY(0, TEC_PROTOCOL_VIOLATION,
+                    "NEW_CONNECTION_ID: received second instance of reset "
+                    "token %s in seqno %"PRIu64", same as in seqno %u",
+                    (lsquic_hexstr(token, IQUIC_SRESET_TOKEN_SZ, tokstr,
+                                                    sizeof(tokstr)), tokstr),
+                    seqno, (*el)->de_seqno);
+                return 0;
+            }
         }
         else if (!dce)
             dce = el;
@@ -5458,6 +5468,21 @@ process_retry_packet (struct ietf_full_conn *conn,
         return 0;
     }
 
+    if (CUR_DCID(conn)->len == packet_in->pi_scid_len
+            && 0 == memcmp(CUR_DCID(conn)->idbuf,
+                    packet_in->pi_data + packet_in->pi_scid_off,
+                    packet_in->pi_scid_len))
+    {
+        /*
+         * [draft-ietf-quic-transport-24] Section 17.2.5:
+         " A client MUST discard a Retry packet that contains a Source
+         " Connection ID field that is identical to the Destination
+         " Connection ID field of its Initial packet.
+         */
+        LSQ_DEBUG("server provided same SCID as ODCID: discard packet");
+        return 0;
+    }
+
     if (!(CUR_DCID(conn)->len == packet_in->pi_odcid_len
             && 0 == memcmp(CUR_DCID(conn)->idbuf,
                             packet_in->pi_data + packet_in->pi_odcid,
@@ -5730,20 +5755,6 @@ process_regular_packet (struct ietf_full_conn *conn,
         if (!(conn->ifc_flags & (IFC_SERVER|IFC_DCID_SET))
                                                 && (packet_in->pi_scid_len))
         {
-            if (CUR_DCID(conn)->len == packet_in->pi_scid_len
-                    && 0 == memcmp(CUR_DCID(conn)->idbuf,
-                            packet_in->pi_data + packet_in->pi_scid_off,
-                            packet_in->pi_scid_len))
-            {
-                /*
-                 * [draft-ietf-quic-transport-24] Section 17.2.5:
-                 " A client MUST discard a Retry packet that contains a Source
-                 " Connection ID field that is identical to the Destination
-                 " Connection ID field of its Initial packet.
-                 */
-                LSQ_DEBUG("server provided same SCID as ODCID: discard packet");
-                return 0;
-            }
             conn->ifc_flags |= IFC_DCID_SET;
             lsquic_scid_from_packet_in(packet_in, CUR_DCID(conn));
             LSQ_DEBUGC("set DCID to %"CID_FMT,

src/liblsquic/lsquic_parse_ietf_v1.c

@@ -915,7 +915,7 @@ ietf_v1_gen_ack_frame (unsigned char *outbuf, size_t outbuf_sz,
     lsquic_packno_t packno_diff, gap, prev_low, maxno, rsize;
     size_t sz;
     const struct lsquic_packno_range *range;
-    unsigned a, b, c, addl_ack_blocks;
+    unsigned a, b, c, addl_ack_blocks, ecn_needs;
     unsigned bits[4];
     enum ecn ecn;
 
@@ -954,6 +954,15 @@ ietf_v1_gen_ack_frame (unsigned char *outbuf, size_t outbuf_sz,
 
     CHECKOUT(sz);
 
+    if (ecn_counts)
+    {
+        for (ecn = 1; ecn <= 3; ++ecn)
+            bits[ecn] = vint_val2bits(ecn_counts[ecn]);
+        ecn_needs = (1 << bits[1]) + (1 << bits[2]) + (1 << bits[3]);
+    }
+    else
+        ecn_needs = 0;
+
     *p = 0x02 + !!ecn_counts;
     ++p;
 
@@ -975,13 +984,14 @@ ietf_v1_gen_ack_frame (unsigned char *outbuf, size_t outbuf_sz,
         rsize = range->high - range->low;
         a = vint_val2bits(gap - 1);
         b = vint_val2bits(rsize);
+        if (ecn_needs + (1 << a) + (1 << b) > AVAIL())
+            break;
         if (addl_ack_blocks == VINT_MAX_ONE_BYTE)
         {
             memmove(block_count_p + 2, block_count_p + 1,
                                                 p - block_count_p - 1);
             ++p;
         }
-        CHECKOUT((1 << a) + (1 << b));
         vint_write(p, gap - 1, a, 1 << a);
         p += 1 << a;
         vint_write(p, rsize, b, 1 << b);
@@ -999,9 +1009,7 @@ ietf_v1_gen_ack_frame (unsigned char *outbuf, size_t outbuf_sz,
 
     if (ecn_counts)
     {
-        for (ecn = 1; ecn <= 3; ++ecn)
-            bits[ecn] = vint_val2bits(ecn_counts[ecn]);
-        CHECKOUT((1 << bits[1]) + (1 << bits[2]) + (1 << bits[3]));
+        assert(ecn_needs <= AVAIL());
         for (ecn = 1; ecn <= 3; ++ecn)
         {
             vint_write(p, ecn_counts[ecnmap[ecn]], bits[ecnmap[ecn]], 1 << bits[ecnmap[ecn]]);

src/liblsquic/lsquic_send_ctl.c

@@ -1373,17 +1373,14 @@ send_ctl_expire (struct lsquic_send_ctl *ctl, enum packnum_space pns,
                                                                         &next);
         }
         break;
-    case EXFI_LAST:
+    default:
+        assert(filter == EXFI_LAST);
         packet_out = send_ctl_last_unacked_retx_packet(ctl, pns);
         if (packet_out)
             n_resubmitted = send_ctl_handle_lost_packet(ctl, packet_out, NULL);
         else
             n_resubmitted = 0;
         break;
-#ifdef WIN32
-    default:
-        n_resubmitted = 0;
-#endif
     }
 
     LSQ_DEBUG("consider %s packets lost: %d resubmitted",

src/liblsquic/lsquic_stream.c

@@ -2943,7 +2943,6 @@ maybe_close_varsize_hq_frame (struct lsquic_stream *stream)
     }
     else if (!size)
     {
-        assert(!shf->shf_frame_ptr);
         LSQ_WARN("discard zero-sized HQ frame type 0x%X (off: %"PRIu64")",
                                         shf->shf_frame_type, shf->shf_off);
         stream_hq_frame_put(stream, shf);

test/unittests/CMakeLists.txt

@@ -24,6 +24,7 @@ SET(TESTS
     ack
     ackgen_gquic_be
     ackparse_gquic_be
+    ackparse_ietf
     alarmset
     alt_svc_ver
     arr