Release 1.17.0

- [API Change] Packet out Memory Interface (PMI) update: - Split PMI pool return into pmi_release and pmi_return - PMI callbacks take peer_ctx and is_ipv6 arguments - [BUGFIX] Fix use-after-free when certificate is updated - Silence gcc warning in optimized mode by performing useless initialization - cmake: use the standard variable CMAKE_BUILD_TYPE instead of DEVEL_MODE
litespeedtech · Oct 16, 2018 · 1e75f93 · 1e75f93
1 parent 66f9afc
commit 1e75f93
Show file tree

Hide file tree

Showing 15 changed files with 178 additions and 64 deletions.
diff --git a/BUILD-WINDOWS.md b/BUILD-WINDOWS.md
@@ -78,7 +78,7 @@ cmake must be run to prepare to build the software in the top level
 cloned directory.  The dot at the end is required.  Begin with the debug
 version as it includes all of the programs.
    ```
-   cmake -DCMAKE_GENERATOR_PLATFORM=x64 --config Debug -DBUILD_SHARED_LIBS=OFF -DDEVEL_MODE=1 .
+   cmake -DCMAKE_GENERATOR_PLATFORM=x64 --config Debug -DBUILD_SHARED_LIBS=OFF
    ```
 
 Visual Studio can now be brought up, and there will be projects in the
@@ -93,7 +93,7 @@ environment as they are compiled to different directories.
 To build the optimized version, repeat the process above with a slightly
 different cmake command:
    ```
-   cmake -DCMAKE_GENERATOR_PLATFORM=x64 --config Release -DBUILD_SHARED_LIBS=OFF -DDEVEL_MODE=0 .
+   cmake -DCMAKE_GENERATOR_PLATFORM=x64 --config Release -DBUILD_SHARED_LIBS=OFF -DCMAKE_BUILD_TYPE=Release .
    ```
 
 After cmake has finished, you can open the project, set the solution 

diff --git a/CHANGELOG b/CHANGELOG
@@ -1,3 +1,14 @@
+2018-10-16
+    - 1.17.0
+    - [API Change] Packet out Memory Interface (PMI) update:
+      - Split PMI pool return into pmi_release and pmi_return
+      - PMI callbacks take peer_ctx and is_ipv6 arguments
+    - [BUGFIX] Fix use-after-free when certificate is updated
+    - Silence gcc warning in optimized mode by performing useless
+      initialization
+    - cmake: use the standard variable CMAKE_BUILD_TYPE instead of
+      DEVEL_MODE
+
 2018-10-03
     - 1.16.0
     - [API Change] Add lsquic_conn_n_avail_streams()

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -25,16 +25,12 @@ IF(NOT GLIBC_VERSION EQUAL "" AND GLIBC_VERSION VERSION_LESS 2.17)
     SET(LIBS ${LIBS} rt)
 ENDIF()
 
-# By default, we compile in development mode.  To compile production code,
-# pass -DDEVEL_MODE=0 to cmake (before that, `make clean' and remove any
-# cmake cache files).
-#
-IF(NOT DEFINED DEVEL_MODE)
-    SET(DEVEL_MODE 1)
+IF("${CMAKE_BUILD_TYPE}" STREQUAL "")
+    SET(CMAKE_BUILD_TYPE Debug)
 ENDIF()
 
 
-MESSAGE(STATUS "DEVEL_MODE: ${DEVEL_MODE}")
+MESSAGE(STATUS "Build type: ${CMAKE_BUILD_TYPE}")
 
 
 IF (NOT MSVC)
@@ -45,7 +41,7 @@ SET(MY_CMAKE_FLAGS "${MY_CMAKE_FLAGS} -fno-omit-frame-pointer")
 IF(CMAKE_COMPILER_IS_GNUCC AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS 4.9.3)
     SET(MY_CMAKE_FLAGS "${MY_CMAKE_FLAGS} -Wno-missing-field-initializers")
 ENDIF()
-IF(DEVEL_MODE EQUAL 1)
+IF(CMAKE_BUILD_TYPE STREQUAL "Debug")
     SET(MY_CMAKE_FLAGS "${MY_CMAKE_FLAGS} -O0 -g3")
     IF(CMAKE_C_COMPILER MATCHES "clang" AND
                         NOT "$ENV{TRAVIS}" MATCHES "^true$")
@@ -78,7 +74,7 @@ ENDIF()
 #MSVC
 ELSE()
 SET(MY_CMAKE_FLAGS "${MY_CMAKE_FLAGS} -W4 -WX -Zi -DWIN32_LEAN_AND_MEAN -DNOMINMAX -D_CRT_SECURE_NO_WARNINGS -I${CMAKE_CURRENT_SOURCE_DIR}/wincompat")
-IF(DEVEL_MODE EQUAL 1)
+IF(CMAKE_BUILD_TYPE STREQUAL "Debug")
     SET(MY_CMAKE_FLAGS "${MY_CMAKE_FLAGS} -Od")
     #SET (MY_CMAKE_FLAGS "${MY_CMAKE_FLAGS} -DFIU_ENABLE=1")
     #SET(LIBS ${LIBS} fiu)
@@ -161,7 +157,7 @@ add_subdirectory(src)
 
 add_subdirectory(test)
 
-IF(DEVEL_MODE EQUAL 1)
+IF(NOT (CMAKE_C_FLAGS MATCHES "-DNDEBUG"))
     # Our test framework relies on assertions, only compile if assertions are
     # enabled.
     #

diff --git a/include/lsquic.h b/include/lsquic.h
@@ -24,7 +24,7 @@ extern "C" {
 #endif
 
 #define LSQUIC_MAJOR_VERSION 1
-#define LSQUIC_MINOR_VERSION 16
+#define LSQUIC_MINOR_VERSION 17
 #define LSQUIC_PATCH_VERSION 0
 
 /**
@@ -485,16 +485,29 @@ typedef int (*lsquic_packets_out_f)(
 /**
  * The packet out memory interface is used by LSQUIC to get buffers to
  * which outgoing packets will be written before they are passed to
- * ea_packets_out callback.  pmi_release() is called at some point,
- * usually after the packet is sent successfully, to return the buffer
- * to the pool.
+ * ea_packets_out callback.
  *
  * If not specified, malloc() and free() are used.
  */
 struct lsquic_packout_mem_if
 {
-    void *  (*pmi_allocate) (void *pmi_ctx, size_t sz);
-    void    (*pmi_release)  (void *pmi_ctx, void *obj);
+    /**
+     * Allocate buffer for sending.
+     */
+    void *  (*pmi_allocate) (void *pmi_ctx, void *conn_ctx, unsigned short sz,
+                                                                char is_ipv6);
+    /**
+     * This function is used to release the allocated buffer after it is
+     * sent via @ref ea_packets_out.
+     */
+    void    (*pmi_release)  (void *pmi_ctx, void *conn_ctx, void *buf,
+                                                                char is_ipv6);
+    /**
+     * If allocated buffer is not going to be sent, return it to the caller
+     * using this function.
+     */
+    void    (*pmi_return)  (void *pmi_ctx, void *conn_ctx, void *buf,
+                                                                char is_ipv6);
 };
 
 struct stack_st_X509;

diff --git a/src/liblsquic/lsquic_engine.c b/src/liblsquic/lsquic_engine.c
@@ -6,6 +6,7 @@
 #include <assert.h>
 #include <errno.h>
 #include <inttypes.h>
+#include <limits.h>
 #include <stdint.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -244,22 +245,22 @@ lsquic_engine_check_settings (const struct lsquic_engine_settings *settings,
 
 
 static void
-free_packet (void *ctx, unsigned char *packet_data)
+free_packet (void *ctx, void *conn_ctx, void *packet_data, char is_ipv6)
 {
     free(packet_data);
 }
 
 
 static void *
-malloc_buf (void *ctx, size_t size)
+malloc_buf (void *ctx, void *conn_ctx, unsigned short size, char is_ipv6)
 {
     return malloc(size);
 }
 
 
 static const struct lsquic_packout_mem_if stock_pmi =
 {
-    malloc_buf, (void(*)(void *, void *)) free_packet,
+    malloc_buf, free_packet, free_packet,
 };
 
 
@@ -821,6 +822,13 @@ really_encrypt_packet (const lsquic_conn_t *conn,
 }
 
 
+static int
+conn_peer_ipv6 (const struct lsquic_conn *conn)
+{
+    return AF_INET6 == ((struct sockaddr *) conn->cn_peer_addr)->sa_family;
+}
+
+
 static enum { ENCPA_OK, ENCPA_NOMEM, ENCPA_BADCRYPT, }
 encrypt_packet (lsquic_engine_t *engine, const lsquic_conn_t *conn,
                                             lsquic_packet_out_t *packet_out)
@@ -829,10 +837,15 @@ encrypt_packet (lsquic_engine_t *engine, const lsquic_conn_t *conn,
     size_t bufsz;
     unsigned sent_sz;
     unsigned char *buf;
+    int ipv6;
 
     bufsz = conn->cn_pf->pf_packout_header_size(conn, packet_out->po_flags) +
                                 packet_out->po_data_sz + QUIC_PACKET_HASH_SZ;
-    buf = engine->pub.enp_pmi->pmi_allocate(engine->pub.enp_pmi_ctx, bufsz);
+    if (bufsz > USHRT_MAX)
+        return ENCPA_BADCRYPT;  /* To cause connection to close */
+    ipv6 = conn_peer_ipv6(conn);
+    buf = engine->pub.enp_pmi->pmi_allocate(engine->pub.enp_pmi_ctx,
+                                            conn->cn_peer_ctx, bufsz, ipv6);
     if (!buf)
     {
         LSQ_DEBUG("could not allocate memory for outgoing packet of size %zd",
@@ -847,19 +860,51 @@ encrypt_packet (lsquic_engine_t *engine, const lsquic_conn_t *conn,
 
     if (enc_sz < 0)
     {
-        engine->pub.enp_pmi->pmi_release(engine->pub.enp_pmi_ctx, buf);
+        engine->pub.enp_pmi->pmi_return(engine->pub.enp_pmi_ctx,
+                                                conn->cn_peer_ctx, buf, ipv6);
         return ENCPA_BADCRYPT;
     }
 
     packet_out->po_enc_data    = buf;
     packet_out->po_enc_data_sz = enc_sz;
     packet_out->po_sent_sz     = sent_sz;
-    packet_out->po_flags |= PO_ENCRYPTED|PO_SENT_SZ;
+    packet_out->po_flags &= ~PO_IPv6;
+    packet_out->po_flags |= PO_ENCRYPTED|PO_SENT_SZ|(ipv6 << POIPv6_SHIFT);
 
     return ENCPA_OK;
 }
 
 
+static void
+release_or_return_enc_data (struct lsquic_engine *engine,
+                void (*pmi_rel_or_ret) (void *, void *, void *, char),
+                struct lsquic_conn *conn, struct lsquic_packet_out *packet_out)
+{
+    pmi_rel_or_ret(engine->pub.enp_pmi_ctx, conn->cn_peer_ctx,
+                packet_out->po_enc_data, lsquic_packet_out_ipv6(packet_out));
+    packet_out->po_flags &= ~PO_ENCRYPTED;
+    packet_out->po_enc_data = NULL;
+}
+
+
+static void
+release_enc_data (struct lsquic_engine *engine, struct lsquic_conn *conn,
+                                        struct lsquic_packet_out *packet_out)
+{
+    release_or_return_enc_data(engine, engine->pub.enp_pmi->pmi_release,
+                                conn, packet_out);
+}
+
+
+static void
+return_enc_data (struct lsquic_engine *engine, struct lsquic_conn *conn,
+                                        struct lsquic_packet_out *packet_out)
+{
+    release_or_return_enc_data(engine, engine->pub.enp_pmi->pmi_return,
+                                conn, packet_out);
+}
+
+
 STAILQ_HEAD(conns_stailq, lsquic_conn);
 TAILQ_HEAD(conns_tailq, lsquic_conn);
 
@@ -1006,12 +1051,7 @@ send_batch (lsquic_engine_t *engine, struct conns_out_iter *conns_iter,
          * or until it times out and regenerated.
          */
         if (batch->packets[i]->po_flags & PO_ENCRYPTED)
-        {
-            batch->packets[i]->po_flags &= ~PO_ENCRYPTED;
-            engine->pub.enp_pmi->pmi_release(engine->pub.enp_pmi_ctx,
-                                                batch->packets[i]->po_enc_data);
-            batch->packets[i]->po_enc_data = NULL;  /* JIC */
-        }
+            release_enc_data(engine, batch->conns[i], batch->packets[i]);
     }
     if (LSQ_LOG_ENABLED_EXT(LSQ_LOG_DEBUG, LSQLM_EVENT))
         for ( ; i < (int) n_to_send; ++i)
@@ -1074,6 +1114,14 @@ send_packets_out (struct lsquic_engine *engine,
             coi_deactivate(&conns_iter, conn);
             continue;
         }
+        if ((packet_out->po_flags & PO_ENCRYPTED)
+                && lsquic_packet_out_ipv6(packet_out) != conn_peer_ipv6(conn))
+        {
+            /* Peer address changed since the packet was encrypted.  Need to
+             * reallocate.
+             */
+            return_enc_data(engine, conn, packet_out);
+        }
         if (!(packet_out->po_flags & (PO_ENCRYPTED|PO_NOENCRYPT)))
         {
             switch (encrypt_packet(engine, conn, packet_out))

diff --git a/src/liblsquic/lsquic_handshake.c b/src/liblsquic/lsquic_handshake.c
@@ -173,6 +173,7 @@ static void free_info (lsquic_session_cache_info_t *);
 /* client */
 static cert_hash_item_t *make_cert_hash_item(struct lsquic_str *domain, struct lsquic_str **certs, int count);
 static int c_insert_certs(cert_hash_item_t *item);
+static void c_erase_certs(struct lsquic_hash_elem *el);
 static void c_free_cert_hash_item (cert_hash_item_t *item);
 
 static int get_tag_val_u32 (unsigned char *v, int len, uint32_t *val);
@@ -265,16 +266,23 @@ static int init_hs_hash_tables(int flags)
 
 
 /* client */
-cert_hash_item_t *
-c_find_certs (const lsquic_str_t *domain)
+struct lsquic_hash_elem *
+c_get_certs_elem (const lsquic_str_t *domain)
 {
-    struct lsquic_hash_elem *el;
-
     if (!s_cached_client_certs)
         return NULL;
 
-    el = lsquic_hash_find(s_cached_client_certs, lsquic_str_cstr(domain),
+    return lsquic_hash_find(s_cached_client_certs, lsquic_str_cstr(domain),
                                                     lsquic_str_len(domain));
+}
+
+
+/* client */
+cert_hash_item_t *
+c_find_certs (const lsquic_str_t *domain)
+{
+    struct lsquic_hash_elem *el = c_get_certs_elem(domain);
+
     if (el == NULL)
         return NULL;
 
@@ -335,6 +343,15 @@ c_insert_certs (cert_hash_item_t *item)
 }
 
 
+/* client */
+static void
+c_erase_certs (struct lsquic_hash_elem *el)
+{
+    if (s_cached_client_certs && el)
+        lsquic_hash_erase(s_cached_client_certs, el);
+}
+
+
 static int save_session_info_entry(lsquic_str_t *key, lsquic_session_cache_info_t *entry)
 {
     lsquic_str_setto(&entry->sni_key, lsquic_str_cstr(key), lsquic_str_len(key));
@@ -1364,7 +1381,11 @@ lsquic_enc_session_handle_chlo_reply (lsquic_enc_session_t *enc_session,
     int ret;
     lsquic_session_cache_info_t *info = enc_session->info;
     hs_ctx_t * hs_ctx = &enc_session->hs_ctx;
-    cert_hash_item_t *cached_certs_item = c_find_certs(&hs_ctx->sni);
+    cert_hash_item_t *cached_certs_item = NULL;
+    struct lsquic_hash_elem *el = c_get_certs_elem(&hs_ctx->sni);
+
+    if (el)
+        cached_certs_item = lsquic_hashelem_getdata(el);
 
     /* FIXME get the number first */
     lsquic_str_t **out_certs = NULL;
@@ -1427,6 +1448,8 @@ lsquic_enc_session_handle_chlo_reply (lsquic_enc_session_t *enc_session,
                             ;
                         else
                         {
+                            if (el)
+                                c_erase_certs(el);
                             if (cached_certs_item)
                                 c_free_cert_hash_item(cached_certs_item);
 

diff --git a/src/liblsquic/lsquic_packet_out.c b/src/liblsquic/lsquic_packet_out.c
@@ -279,7 +279,7 @@ lsquic_packet_out_new (struct lsquic_mm *mm, struct malo *malo, int use_cid,
 
 void
 lsquic_packet_out_destroy (lsquic_packet_out_t *packet_out,
-                           struct lsquic_engine_public *enpub)
+                           struct lsquic_engine_public *enpub, void *peer_ctx)
 {
     if (packet_out->po_flags & PO_SREC_ARR)
     {
@@ -292,8 +292,8 @@ lsquic_packet_out_destroy (lsquic_packet_out_t *packet_out,
         }
     }
     if (packet_out->po_flags & PO_ENCRYPTED)
-        enpub->enp_pmi->pmi_release(enpub->enp_pmi_ctx,
-                                                packet_out->po_enc_data);
+        enpub->enp_pmi->pmi_release(enpub->enp_pmi_ctx, peer_ctx,
+                packet_out->po_enc_data, lsquic_packet_out_ipv6(packet_out));
     if (packet_out->po_nonce)
         free(packet_out->po_nonce);
     lsquic_mm_put_packet_out(&enpub->enp_mm, packet_out);