Revert "Merge pull request #188 from expressvpn/multi-thread-support"

This reverts commit 9e867a4, reversing
changes made to b98d3e9.

Author: xv-raihaan-m

Diff for: .github/workflows/ci.yml

@@ -13,7 +13,7 @@ jobs:
     runs-on: ubuntu-22.04
     strategy:
       matrix:
-        distro: [bookworm]
+        distro: [bullseye, bookworm]
     env:
       FORCE_COLOR: 1
     steps:
@@ -46,20 +46,6 @@ jobs:
           ./wolfcrypt/test/testwolfcrypt
       - name: Run build and test
         run: ceedling project:linux verbosity[4] test:all
-  linux-multithread:
-    runs-on: ubuntu-22.04
-    steps:
-      - uses: actions/checkout@v4
-      - name: Install Ceedling
-        run: sudo gem install ceedling -v 0.31.1 --no-user-install
-      - name: Build dependencies
-        run: ceedling project:linux_multithread verbosity[4] clobber dependencies:make
-      - name: Run wolfSSL Tests
-        run: |
-          cd third_party/wolfssl
-          ./wolfcrypt/test/testwolfcrypt
-      - name: Run build and test
-        run: ceedling project:linux_multithread verbosity[4] test:all
   linux-386:
     runs-on: ubuntu-22.04
     steps:
@@ -182,7 +168,6 @@ jobs:
         config:
           [
             { project: windows_64, arch: x64 },
-            { project: windows_64_multithread, arch: x64 },
             { project: windows_32, arch: x86 },
             { project: windows_arm64, arch: amd64_arm64 },
           ]
@@ -196,7 +181,7 @@ jobs:
         uses: TheMrMilchmann/setup-msvc-dev@v3
         with:
           arch: ${{ matrix.config.arch }}
-      - if: ${{ matrix.config.project != 'windows_arm64' }}
+      - if: ${{ matrix.config.project != 'windows_arm64' }} 
       # Skip making dependencies for ARM64 as we can only apply git patch once for WolfSSL
         name: Build dependencies
         run: ceedling project:${{ matrix.config.project }} verbosity[4] clobber dependencies:make

Diff for: 3rd_party_deps.yml

@@ -16,7 +16,7 @@
       --enable-dtls-frag-ch
       --enable-dtls-mtu
       --enable-secure-renegotiation
-      --disable-singlethreaded
+      --enable-singlethreaded
       --enable-sni
       --enable-sp=yes,4096
       --enable-static

Diff for: linux_multithread.yml

@@ -473,7 +473,7 @@ he_return_code_t he_internal_send_message(he_conn_t *conn, uint8_t *message, uin
         if(res == 0) {
           return HE_ERR_CONNECTION_WAS_CLOSED;
         } else {
-          he_conn_set_ssl_error(conn, error);
+          conn->wolf_error = error;
           return HE_ERR_SSL_ERROR;
         }
     }
@@ -536,7 +536,7 @@ bool he_internal_is_valid_state_for_server_config(he_conn_t *conn) {
   }
 
   // The server config message is only valid after the TLS link is established
-  switch((int)conn->state) {
+  switch(conn->state) {
     case HE_STATE_LINK_UP:
     case HE_STATE_AUTHENTICATING:
     case HE_STATE_CONFIGURING:
@@ -746,7 +746,7 @@ he_return_code_t he_internal_renegotiate_ssl(he_conn_t *conn) {
       case SECURE_RENEGOTIATION_E:
         return HE_ERR_SECURE_RENEGOTIATION_ERROR;
       default:
-        he_conn_set_ssl_error(conn, error);
+        conn->wolf_error = error;
         return HE_ERR_SSL_ERROR;
     }
   }
@@ -1213,7 +1213,7 @@ he_return_code_t he_conn_start_pmtu_discovery(he_conn_t *conn) {
   if(conn->pmtud_state_change_cb == NULL || conn->pmtud_time_cb == NULL) {
     return HE_ERR_PMTUD_CALLBACKS_NOT_SET;
   }
-  if(conn->pmtud.state != HE_PMTUD_STATE_DISABLED) {
+  if(conn->pmtud_state != HE_PMTUD_STATE_DISABLED) {
     // PMTUD is already started
     return HE_SUCCESS;
   }
@@ -1223,10 +1223,10 @@ he_return_code_t he_conn_start_pmtu_discovery(he_conn_t *conn) {
 }
 
 uint16_t he_conn_get_effective_pmtu(he_conn_t *conn) {
-  if(!conn || conn->pmtud.effective_pmtu == 0 || conn->pmtud.state != HE_PMTUD_STATE_SEARCH_COMPLETE) {
+  if(!conn || conn->effective_pmtu == 0) {
     return HE_MAX_MTU;
   }
-  return conn->pmtud.effective_pmtu;
+  return conn->effective_pmtu;
 }
 
 he_return_code_t he_conn_pmtud_probe_timeout(he_conn_t *conn) {
@@ -1236,25 +1236,6 @@ he_return_code_t he_conn_pmtud_probe_timeout(he_conn_t *conn) {
   return he_internal_pmtud_handle_probe_timeout(conn);
 }
 
-#ifdef HE_ENABLE_MULTITHREADED
-// wolf_error as thread local variable, so it can be used
-// from multiple threads without collision
-HE_THREAD_LOCAL int wolf_error = 0;
-#endif
-
-void he_conn_set_ssl_error(he_conn_t *conn, int error) {
-#ifdef HE_ENABLE_MULTITHREADED
-  (void) conn;
-  wolf_error = error;
-#else
-  conn->wolf_error = error;
-#endif
-}
-
 int he_conn_get_ssl_error(he_conn_t *conn) {
-#ifdef HE_ENABLE_MULTITHREADED
-  return wolf_error;
-#else
   return conn->wolf_error;
-#endif
 }

Diff for: src/he/conn.c

@@ -517,7 +517,4 @@ he_return_code_t he_conn_pmtud_probe_timeout(he_conn_t *conn);
  */
 int he_conn_get_ssl_error(he_conn_t* conn);
 
-
-void he_conn_set_ssl_error(he_conn_t *conn, int error);
-
 #endif  // CONN_H

Diff for: src/he/conn.h

@@ -18,13 +18,12 @@
  */
 
 #include "core.h"
-#include "conn.h"
 
 he_return_code_t he_internal_setup_stream_state(he_conn_t *conn, uint8_t *data, size_t length) {
   if(conn->incoming_data_left_to_read != 0) {
     // Somehow this function was called without reading all data from a previous buffer
     // This is bad
-    he_conn_set_ssl_error(conn, 0);
+    conn->wolf_error = 0;
     return HE_ERR_SSL_ERROR;
   }
   // Set up the location of the buffer and its length

Diff for: src/he/core.c

@@ -35,7 +35,8 @@
 #include <wolfssl/wolfcrypt/settings.h>
 
 bool he_internal_flow_should_fragment(he_conn_t *conn, uint16_t effective_pmtu, uint16_t length) {
-  return conn->connection_type == HE_CONNECTION_TYPE_DATAGRAM && length > effective_pmtu;
+  return conn->connection_type == HE_CONNECTION_TYPE_DATAGRAM &&
+         conn->pmtud_state == HE_PMTUD_STATE_SEARCH_COMPLETE && length > effective_pmtu;
 }
 
 he_return_code_t he_conn_inside_packet_received(he_conn_t *conn, uint8_t *packet, size_t length) {
@@ -69,9 +70,11 @@ he_return_code_t he_conn_inside_packet_received(he_conn_t *conn, uint8_t *packet
   // Clamp the MSS if PMTU has been fixed
   he_return_code_t ret = HE_SUCCESS;
   uint16_t effective_pmtu = he_conn_get_effective_pmtu(conn);
-  ret = he_internal_clamp_mss(packet, length, effective_pmtu - HE_MSS_OVERHEAD);
-  if(ret != HE_SUCCESS) {
-    return ret;
+  if(conn->pmtud_state == HE_PMTUD_STATE_SEARCH_COMPLETE) {
+    ret = he_internal_clamp_mss(packet, length, effective_pmtu - HE_MSS_OVERHEAD);
+    if(ret != HE_SUCCESS) {
+      return ret;
+    }
   }
 
   // Process the packet with plugins.
@@ -141,24 +144,26 @@ he_return_code_t he_conn_inside_packet_received(he_conn_t *conn, uint8_t *packet
   return ret;
 }
 
-he_return_code_t he_internal_flow_process_message(he_conn_t *conn, he_packet_buffer_t *read_packet) {
+he_return_code_t he_internal_flow_process_message(he_conn_t *conn) {
   // Return if conn is null
-  if(!conn || !read_packet) {
+  if(!conn) {
     return HE_ERR_NULL_POINTER;
   }
 
   // If the packet is too small then either the client is sending corrupted data or something is
   // very wrong with the SSL connection
-  if(read_packet->packet_size < sizeof(he_msg_hdr_t)) {
-    read_packet->has_packet = false;
-    he_conn_set_ssl_error(conn, 0);
+  if(conn->read_packet.packet_size < sizeof(he_msg_hdr_t)) {
+    conn->read_packet.has_packet = false;
+    conn->wolf_error = 0;
     return HE_ERR_SSL_ERROR;
   }
 
+  he_packet_buffer_t *pkt_buff = &conn->read_packet;
+
   // Cast the header
-  he_msg_hdr_t *msg_hdr = (he_msg_hdr_t *)&read_packet->packet;
-  uint8_t *buf = read_packet->packet;
-  int buf_len = read_packet->packet_size;
+  he_msg_hdr_t *msg_hdr = (he_msg_hdr_t *)&pkt_buff->packet;
+  uint8_t *buf = pkt_buff->packet;
+  int buf_len = pkt_buff->packet_size;
 
   switch(msg_hdr->msgid) {
     case HE_MSGID_NOOP:
@@ -213,19 +218,19 @@ he_return_code_t he_internal_flow_process_message(he_conn_t *conn, he_packet_buf
   return HE_SUCCESS;
 }
 
-he_return_code_t he_internal_flow_fetch_message(he_conn_t *conn, he_packet_buffer_t *read_packet) {
+he_return_code_t he_internal_flow_fetch_message(he_conn_t *conn) {
   // Return if conn is null
-  if(!conn || !read_packet) {
+  if(!conn) {
     return HE_ERR_NULL_POINTER;
   }
 
   // Try to read out a packet
   int res =
-      wolfSSL_read(conn->wolf_ssl, read_packet->packet, sizeof(read_packet->packet));
+      wolfSSL_read(conn->wolf_ssl, conn->read_packet.packet, sizeof(conn->read_packet.packet));
 
   if(res <= 0) {
-    read_packet->has_packet = false;
-    read_packet->packet_size = 0;
+    conn->read_packet.has_packet = false;
+    conn->read_packet.packet_size = 0;
 
     int error = wolfSSL_get_error(conn->wolf_ssl, res);
     switch(error) {
@@ -234,7 +239,7 @@ he_return_code_t he_internal_flow_fetch_message(he_conn_t *conn, he_packet_buffe
         return HE_SUCCESS;
 
       case APP_DATA_READY:
-        return he_internal_flow_fetch_message(conn, read_packet);
+        return he_internal_flow_fetch_message(conn);
 
       case SSL_ERROR_WANT_READ:
       case SSL_ERROR_WANT_WRITE:
@@ -246,7 +251,7 @@ he_return_code_t he_internal_flow_fetch_message(he_conn_t *conn, he_packet_buffe
         if(res == 0) {
           return HE_ERR_CONNECTION_WAS_CLOSED;
         } else {
-          he_conn_set_ssl_error(conn, error);
+          conn->wolf_error = error;
           // if this is TCP then any SSL error is fatal (stream corruption).
           // If this is D/TLS we can actually ignore corrupted packets.
           if(conn->connection_type == HE_CONNECTION_TYPE_STREAM) {
@@ -257,8 +262,8 @@ he_return_code_t he_internal_flow_fetch_message(he_conn_t *conn, he_packet_buffe
         }
     }
   } else {
-    read_packet->has_packet = true;
-    read_packet->packet_size = res;
+    conn->read_packet.has_packet = true;
+    conn->read_packet.packet_size = res;
   }
 
   return HE_SUCCESS;
@@ -335,12 +340,6 @@ he_return_code_t he_conn_outside_data_received(he_conn_t *conn, uint8_t *buffer,
   }
 }
 
-#ifdef HE_ENABLE_MULTITHREADED
-HE_THREAD_LOCAL uint8_t *cur_packet = NULL;
-HE_THREAD_LOCAL size_t cur_packet_length = 0;
-HE_THREAD_LOCAL bool packet_seen = false;
-#endif
-
 he_return_code_t he_internal_flow_outside_packet_received(he_conn_t *conn, uint8_t *packet,
                                                           size_t length) {
   // Return if packet or conn is null
@@ -381,10 +380,11 @@ he_return_code_t he_internal_flow_outside_packet_received(he_conn_t *conn, uint8
 
   // Update pointer and length in our connection state
   // We need to pull the wire header off first
-  he_internal_set_packet(conn, packet + sizeof(he_wire_hdr_t), length - sizeof(he_wire_hdr_t));
+  conn->incoming_data_length = length - sizeof(he_wire_hdr_t);
+  conn->incoming_data = packet + sizeof(he_wire_hdr_t);
 
   // Make sure that this packet is marked as unseen
-  he_internal_set_packet_seen(conn, false);
+  conn->packet_seen = false;
 
   return HE_DISPATCH(he_internal_flow_outside_data_verify_connection, conn);
 }
@@ -410,13 +410,8 @@ he_return_code_t he_internal_flow_outside_data_verify_connection(he_conn_t *conn
   }
 
   // Check to see if this is our first message and trigger an event change if it is
-#ifdef HE_ENABLE_MULTITHREADED
-  bool expected = false;
-  if (atomic_compare_exchange_strong(&conn->first_message_received, &expected, true)) {
-#else
-  if (!conn->first_message_received) {
+  if(!conn->first_message_received) {
     conn->first_message_received = true;
-#endif
     he_internal_generate_event(conn, HE_EVENT_FIRST_MESSAGE_RECEIVED);
   }
 
@@ -439,7 +434,7 @@ he_return_code_t he_internal_flow_outside_data_verify_connection(he_conn_t *conn
         }
 
         // We can't recover from any other errors
-        he_conn_set_ssl_error(conn, error);
+        conn->wolf_error = error;
         return HE_ERR_SSL_ERROR;
       }
 
@@ -461,8 +456,6 @@ he_return_code_t he_internal_flow_outside_data_verify_connection(he_conn_t *conn
 }
 
 he_return_code_t he_internal_flow_outside_data_handle_messages(he_conn_t *conn) {
-  he_packet_buffer_t read_packet = { 0 };
-
   // Return if conn is null
   if(!conn) {
     return HE_ERR_NULL_POINTER;
@@ -471,18 +464,18 @@ he_return_code_t he_internal_flow_outside_data_handle_messages(he_conn_t *conn)
   // Handle messages
   while(true) {
     // Do we have a message?
-    he_return_code_t ret = he_internal_flow_fetch_message(conn, &read_packet);
+    he_return_code_t ret = he_internal_flow_fetch_message(conn);
 
     if(ret != HE_SUCCESS) {
       return ret;
     }
 
-    if(!read_packet.has_packet) {
+    if(!conn->read_packet.has_packet) {
       break;
     }
 
     // Process the message
-    ret = he_internal_flow_process_message(conn, &read_packet);
+    ret = he_internal_flow_process_message(conn);
 
     if(ret != HE_SUCCESS) return ret;
   }
@@ -505,7 +498,7 @@ he_return_code_t he_internal_flow_outside_data_handle_messages(he_conn_t *conn)
       // D/TLS 1.3
       int wolf_rc = 0;
       if((wolf_rc = wolfSSL_key_update_response(conn->wolf_ssl, &resp_pending)) != 0) {
-        he_conn_set_ssl_error(conn, wolfSSL_get_error(conn->wolf_ssl, wolf_rc));
+        conn->wolf_error = wolfSSL_get_error(conn->wolf_ssl, wolf_rc);
         return HE_ERR_SSL_ERROR;
       }
     }
@@ -520,6 +513,10 @@ he_return_code_t he_internal_flow_outside_data_handle_messages(he_conn_t *conn)
     he_internal_update_timeout(conn);
   }
 
+  // Zero out the packet, ensures that if the connection is unused
+  // for extended periods the old outdated data is cleared from memory
+  memset(&conn->read_packet, 0, sizeof(conn->read_packet));
+
   // All went well
   return HE_SUCCESS;
 }