Release 2.4.9

- [BUGFIX] IETF QUIC server: fix uninitialized variable use.
- [BUGFIX] make sure TLSv1.3 is not disabled in SSL object.
- [BUGFIX] Use issuer name and serial number to cache certs (SKID
  values are not unique).
- [BUGFIX] Always set the idle alarm in IETF connection so that it
  can time out.

CHANGELOG

@@ -1,3 +1,12 @@
+2019-10-24
+    - 2.4.9
+    - [BUGFIX] IETF QUIC server: fix uninitialized variable use.
+    - [BUGFIX] make sure TLSv1.3 is not disabled in SSL object.
+    - [BUGFIX] Use issuer name and serial number to cache certs (SKID
+      values are not unique).
+    - [BUGFIX] Always set the idle alarm in IETF connection so that it
+      can time out.
+
 2019-10-21
     - 2.4.8
     - [OPTIMIZATION, BUGFIX] Use ls-qpack v0.10.5.

CMakeLists.txt

@@ -22,6 +22,7 @@ MESSAGE(STATUS "Build type: ${CMAKE_BUILD_TYPE}")
 
 OPTION(LSQUIC_FIU "Use Fault Injection in Userspace (FIU)" OFF)
 
+SET(MY_CMAKE_FLAGS "-DLSQUIC_DEBUG_NEXT_ADV_TICK=1")
 
 IF (NOT MSVC)
 

include/lsquic.h

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

src/liblsquic/lsquic_enc_sess_ietf.c

@@ -1028,7 +1028,7 @@ iquic_lookup_cert (SSL *ssl, void *arg)
             SSL_clear_options(enc_sess->esi_ssl,
                                     SSL_get_options(enc_sess->esi_ssl));
             SSL_set_options(enc_sess->esi_ssl,
-                                    SSL_CTX_get_options(ssl_ctx));
+                            SSL_CTX_get_options(ssl_ctx) & ~SSL_OP_NO_TLSv1_3);
             return 1;
         }
         else
@@ -1113,6 +1113,7 @@ iquic_esfi_init_server (enc_session_t *enc_session_p)
         return -1;
     }
 
+    SSL_clear_options(enc_sess->esi_ssl, SSL_OP_NO_TLSv1_3);
     SSL_set_cert_cb(enc_sess->esi_ssl, iquic_lookup_cert, enc_sess);
     SSL_set_ex_data(enc_sess->esi_ssl, s_idx, enc_sess);
     SSL_set_accept_state(enc_sess->esi_ssl);

src/liblsquic/lsquic_engine.c

@@ -79,6 +79,10 @@
 #define LSQUIC_LOGGER_MODULE LSQLM_ENGINE
 #include "lsquic_logger.h"
 
+#ifndef LSQUIC_DEBUG_NEXT_ADV_TICK
+#define LSQUIC_DEBUG_NEXT_ADV_TICK 0
+#endif
+
 #define MIN(a, b) ((a) < (b) ? (a) : (b))
 
 /* The batch of outgoing packets grows and shrinks dynamically */
@@ -1010,7 +1014,6 @@ find_or_create_conn (lsquic_engine_t *engine, lsquic_packet_in_t *packet_in,
          const struct sockaddr *sa_peer, void *peer_ctx)
 {
     struct lsquic_hash_elem *el;
-    lsquic_cid_t odcid;
     struct purga_el *puel;
     lsquic_conn_t *conn;
 
@@ -1120,7 +1123,7 @@ find_or_create_conn (lsquic_engine_t *engine, lsquic_packet_in_t *packet_in,
     if ((1 << version) & LSQUIC_IETF_VERSIONS)
     {
         conn = lsquic_mini_conn_ietf_new(&engine->pub, packet_in, version,
-                    sa_peer->sa_family == AF_INET, odcid.len ? &odcid : NULL);
+                    sa_peer->sa_family == AF_INET, NULL);
     }
     else
     {
@@ -2592,40 +2595,48 @@ lsquic_engine_earliest_adv_tick (lsquic_engine_t *engine, int *diff)
 {
     const struct attq_elem *next_attq;
     lsquic_time_t now, next_time;
+#if LSQUIC_DEBUG_NEXT_ADV_TICK
     const struct lsquic_conn *conn;
     const lsquic_cid_t *cid;
     const enum lsq_log_level L = LSQ_LOG_DEBUG;  /* Easy toggle */
+#endif
 
     ENGINE_CALLS_INCR(engine);
 
     if ((engine->flags & ENG_PAST_DEADLINE)
                                     && lsquic_mh_count(&engine->conns_out))
     {
+#if LSQUIC_DEBUG_NEXT_ADV_TICK
         conn = lsquic_mh_peek(&engine->conns_out);
         cid = lsquic_conn_log_cid(conn);
         LSQ_LOGC(L, "next advisory tick is now: went past deadline last time "
             "and have %u outgoing connection%.*s (%"CID_FMT" first)",
             lsquic_mh_count(&engine->conns_out),
             lsquic_mh_count(&engine->conns_out) != 1, "s", CID_BITS(cid));
+#endif
         *diff = 0;
         return 1;
     }
 
     if (engine->pr_queue && prq_have_pending(engine->pr_queue))
     {
+#if LSQUIC_DEBUG_NEXT_ADV_TICK
         LSQ_LOG(L, "next advisory tick is now: have pending PRQ elements");
+#endif
         *diff = 0;
         return 1;
     }
 
     if (lsquic_mh_count(&engine->conns_tickable))
     {
+#if LSQUIC_DEBUG_NEXT_ADV_TICK
         conn = lsquic_mh_peek(&engine->conns_tickable);
         cid = lsquic_conn_log_cid(conn);
         LSQ_LOGC(L, "next advisory tick is now: have %u tickable "
             "connection%.*s (%"CID_FMT" first)",
             lsquic_mh_count(&engine->conns_tickable),
             lsquic_mh_count(&engine->conns_tickable) != 1, "s", CID_BITS(cid));
+#endif
         *diff = 0;
         return 1;
     }
@@ -2655,6 +2666,7 @@ lsquic_engine_earliest_adv_tick (lsquic_engine_t *engine, int *diff)
 
     now = lsquic_time_now();
     *diff = (int) ((int64_t) next_time - (int64_t) now);
+#if LSQUIC_DEBUG_NEXT_ADV_TICK
     if (next_attq)
     {
         cid = lsquic_conn_log_cid(next_attq->ae_conn);
@@ -2664,6 +2676,7 @@ lsquic_engine_earliest_adv_tick (lsquic_engine_t *engine, int *diff)
     }
     else
         LSQ_LOG(L, "next advisory tick is %d usec away: resume sending", *diff);
+#endif
     return 1;
 }
 

src/liblsquic/lsquic_full_conn_ietf.c

@@ -945,17 +945,19 @@ lsquic_ietf_full_conn_client_new (struct lsquic_engine_public *enpub,
     const struct enc_session_funcs_iquic *esfi;
     struct ietf_full_conn *conn;
     enum lsquic_version ver, zero_rtt_version;
+    lsquic_time_t now;
     unsigned versions;
 
     conn = calloc(1, sizeof(*conn));
     if (!conn)
         return NULL;
+    now = lsquic_time_now();
     /* Set the flags early so that correct CID is used for logging */
     conn->ifc_conn.cn_flags |= LSCONN_IETF;
     conn->ifc_conn.cn_cces = conn->ifc_cces;
     conn->ifc_conn.cn_n_cces = sizeof(conn->ifc_cces)
                                                 / sizeof(conn->ifc_cces[0]);
-    if (!ietf_full_conn_add_scid(conn, enpub, CCE_USED, lsquic_time_now()))
+    if (!ietf_full_conn_add_scid(conn, enpub, CCE_USED, now))
     {
         free(conn);
         return NULL;
@@ -1022,6 +1024,7 @@ lsquic_ietf_full_conn_client_new (struct lsquic_engine_public *enpub,
     if (conn->ifc_settings->es_handshake_to)
         lsquic_alarmset_set(&conn->ifc_alset, AL_HANDSHAKE,
                     lsquic_time_now() + conn->ifc_settings->es_handshake_to);
+    lsquic_alarmset_set(&conn->ifc_alset, AL_IDLE, now + conn->ifc_idle_to);
     if (enpub->enp_settings.es_support_push && CLIENT_PUSH_SUPPORT)
     {
         conn->ifc_u.cli.ifcli_flags |= IFCLI_PUSH_ENABLED;
@@ -1286,6 +1289,8 @@ lsquic_ietf_full_conn_server_new (struct lsquic_engine_public *enpub,
     /* TODO: check return valuee */ (void)
     handshake_ok(&conn->ifc_conn);
 
+    lsquic_alarmset_set(&conn->ifc_alset, AL_IDLE,
+                                        imc->imc_created + conn->ifc_idle_to);
     while ((packet_in = TAILQ_FIRST(&imc->imc_app_packets)))
     {
         TAILQ_REMOVE(&imc->imc_app_packets, packet_in, pi_next);

src/liblsquic/lsquic_handshake.c

@@ -20,6 +20,7 @@
 #include <openssl/x509.h>
 #include <openssl/rand.h>
 #include <openssl/nid.h>
+#include <openssl/bn.h>
 #include <zlib.h>
 
 #include "lsquic.h"
@@ -277,15 +278,14 @@ typedef struct cert_item_st
 {
     struct lsquic_str*      crt;
     struct lsquic_hash_elem hash_el;
-    size_t                  skid_len;
+    unsigned char           key[0];
-    unsigned char           skid_buf[0];
 } cert_item_t;
 
 /* server */
-static cert_item_t* s_find_cert(const unsigned char *skid_buf, size_t skid_len);
+static cert_item_t* s_find_cert(const unsigned char *, size_t);
 static void s_free_cert_hash_item(cert_item_t *item);
-static cert_item_t* s_insert_cert(const unsigned char *skid_buf,
+static cert_item_t* s_insert_cert(const unsigned char *key, size_t key_sz,
-                                size_t skid_len, const struct lsquic_str *crt);
+                                                const struct lsquic_str *crt);
 
 static compress_cert_hash_item_t* find_compress_certs(struct lsquic_str *domain);
 static compress_cert_hash_item_t *make_compress_cert_hash_item(struct lsquic_str *domain, struct lsquic_str *crts_compress_buf);
@@ -394,14 +394,14 @@ static int init_hs_hash_tables(int flags)
 
 /* server */
 static cert_item_t *
-s_find_cert (const unsigned char *skid_buf, size_t skid_len)
+s_find_cert (const unsigned char *key, size_t key_sz)
 {
     struct lsquic_hash_elem *el;
 
     if (!s_server_certs)
         return NULL;
 
-    el = lsquic_hash_find(s_server_certs, skid_buf, skid_len);
+    el = lsquic_hash_find(s_server_certs, key, key_sz);
     if (el == NULL)
         return NULL;
 
@@ -460,8 +460,7 @@ s_free_cert_hash_item (cert_item_t *item)
 
 /* server */
 static cert_item_t *
-s_insert_cert (const unsigned char *skid_buf, size_t skid_len,
+s_insert_cert (const unsigned char *key, size_t key_sz, const lsquic_str_t *crt)
-                                                    const lsquic_str_t *crt)
 {
     struct lsquic_hash_elem *el;
     lsquic_str_t *crt_copy;
@@ -471,17 +470,16 @@ s_insert_cert (const unsigned char *skid_buf, size_t skid_len,
     if (!crt_copy)
         return NULL;
 
-    item = calloc(1, sizeof(*item) + skid_len);
+    item = calloc(1, sizeof(*item) + key_sz);
     if (!item)
     {
         lsquic_str_delete(crt_copy);
         return NULL;
     }
 
-    memcpy(item->skid_buf, skid_buf, skid_len);
-    item->skid_len = skid_len;
     item->crt = crt_copy;
-    el = lsquic_hash_insert(s_server_certs, item->skid_buf, item->skid_len,
+    memcpy(item->key, key, key_sz);
+    el = lsquic_hash_insert(s_server_certs, item->key, key_sz,
                                                         item, &item->hash_el);
     if (el)
         return lsquic_hashelem_getdata(el);
@@ -1981,27 +1979,52 @@ gen_shlo_data (uint8_t *buf, size_t buf_len, struct lsquic_enc_session *enc_sess
 }
 
 
-/* We use ASN.1 string instead of the actual value, but it's only two
+/* Generate key based on issuer and serial number.  The key has the following
- * extra bytes of overhead.  We save a call to ASN1_get_object().
+ * structure:
+ *
+ *      size_t          length of issuer.  This field is required to prevent
+ *                        the chance (however remote) that concatenation of
+ *                        the next two fields is ambiguous.
+ *      uint8_t[]       DER-encoded issuer
+ *      uint8_t[]       Serial number represented as sequence of bytes output
+ *                        by BN_bn2bin
+ *
+ * Return size of the key or zero on error.
  */
-static int
+static size_t
-get_skid (X509 *cert, const unsigned char **skid_buf, size_t *skid_len)
+gen_iasn_key (X509 *cert, unsigned char *const out, size_t const sz)
 {
-    ASN1_OCTET_STRING *skid;
+    const unsigned char *name_bytes;
-    X509_EXTENSION *ext;
+    size_t name_sz;
-    int idx;
+    X509_NAME *name;
+    ASN1_INTEGER *sernum;
+    BIGNUM *bn;
+    unsigned bn_sz;
 
-    idx = X509_get_ext_by_NID(cert, NID_subject_key_identifier, -1);
+    name = X509_get_issuer_name(cert);
-    ext = X509_get_ext(cert, idx);
+    if (!name)
-    if (ext)
+        return 0;
+    if (!X509_NAME_get0_der(name, &name_bytes, &name_sz))
+        return 0;
+    sernum = X509_get_serialNumber(cert);
+    if (!sernum)
+        return 0;
+    bn = ASN1_INTEGER_to_BN(sernum, NULL);
+    if (!bn)
+        return 0;
+    bn_sz = BN_num_bytes(bn);
+    if (sizeof(size_t) + name_sz + bn_sz > sz)
     {
-        skid = X509_EXTENSION_get_data(ext);
+        BN_free(bn);
-        *skid_buf = skid->data;
-        *skid_len = (size_t) skid->length;
         return 0;
     }
-    else
+
-        return -1;
+    memcpy(out, &name_sz, sizeof(name_sz));
+    memcpy(out + sizeof(name_sz), name_bytes, name_sz);
+    BN_bn2bin(bn, out + sizeof(name_sz) +  name_sz);
+    BN_free(bn);
+
+    return sizeof(name_sz) + name_sz + bn_sz;
 }
 
 
@@ -2015,8 +2038,8 @@ get_sni_SSL_CTX(struct lsquic_enc_session *enc_session, lsquic_lookup_cert_f cb,
     lsquic_str_t crtstr;
     cert_item_t *item;
     struct ssl_ctx_st *ssl_ctx;
-    const unsigned char *skid_buf;
+    size_t key_sz;
-    size_t skid_len;
+    unsigned char key[0x200];
     
     if (!enc_session->ssl_ctx)
     {
@@ -2033,9 +2056,10 @@ get_sni_SSL_CTX(struct lsquic_enc_session *enc_session, lsquic_lookup_cert_f cb,
         crt = SSL_CTX_get0_certificate(enc_session->ssl_ctx);
         if (!crt)
             return GET_SNI_ERR;
-        if (0 == get_skid(crt, &skid_buf, &skid_len))
+        key_sz = gen_iasn_key(crt, key, sizeof(key));
+        if (key_sz)
         {
-            item = s_find_cert(skid_buf, skid_len);
+            item = s_find_cert(key, key_sz);
             if (item)
                 LSQ_DEBUG("found cert in cache");
             else
@@ -2045,7 +2069,7 @@ get_sni_SSL_CTX(struct lsquic_enc_session *enc_session, lsquic_lookup_cert_f cb,
                 if (len < 0)
                     return GET_SNI_ERR;
                 lsquic_str_set(&crtstr, (char *) out, len);
-                item = s_insert_cert(skid_buf, skid_len, &crtstr);
+                item = s_insert_cert(key, key_sz, &crtstr);
                 if (item)
                 {
                     OPENSSL_free(out);
@@ -2061,11 +2085,7 @@ get_sni_SSL_CTX(struct lsquic_enc_session *enc_session, lsquic_lookup_cert_f cb,
         }
         else
         {
-            LSQ_DEBUG("skid not available, make a copy");
+            LSQ_INFO("cannot generate cert cache key, make copy");
-            out = NULL;
-            len = i2d_X509(crt, &out);
-            if (len < 0)
-                return GET_SNI_ERR;
   copy:     enc_session->cert_ptr = lsquic_str_new((char *) out, len);
             OPENSSL_free(out);
             if (!enc_session->cert_ptr)