Merge pull request #1661 from private-octopus/harden-retry-processing

Harden retry processing
private-octopus · Mar 26, 2024 · 4b49e9a · 4b49e9a
2 parents 25727d4 + d691212
commit 4b49e9a
Show file tree

Hide file tree

Showing 11 changed files with 547 additions and 244 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -8,7 +8,7 @@ else()
 endif()
 
 project(picoquic
-        VERSION 1.1.19.6
+        VERSION 1.1.19.7
         DESCRIPTION "picoquic library"
         LANGUAGES C CXX)
 

diff --git a/picoquic/packet.c b/picoquic/packet.c
diff --git a/picoquic/picoquic.h b/picoquic/picoquic.h
@@ -40,7 +40,7 @@
 extern "C" {
 #endif
 
-#define PICOQUIC_VERSION "1.1.19.6"
+#define PICOQUIC_VERSION "1.1.19.7"
 #define PICOQUIC_ERROR_CLASS 0x400
 #define PICOQUIC_ERROR_DUPLICATE (PICOQUIC_ERROR_CLASS + 1)
 #define PICOQUIC_ERROR_AEAD_CHECK (PICOQUIC_ERROR_CLASS + 3)
@@ -102,6 +102,8 @@ extern "C" {
 #define PICOQUIC_ERROR_PORT_BLOCKED (PICOQUIC_ERROR_CLASS + 58)
 #define PICOQUIC_ERROR_DATAGRAM_TOO_LONG (PICOQUIC_ERROR_CLASS + 59)
 #define PICOQUIC_ERROR_PATH_ID_INVALID (PICOQUIC_ERROR_CLASS + 60)
+#define PICOQUIC_ERROR_RETRY_NEEDED (PICOQUIC_ERROR_CLASS + 61)
+#define PICOQUIC_ERROR_SERVER_BUSY (PICOQUIC_ERROR_CLASS + 62)
 
 /*
  * Protocol errors defined in the QUIC spec

diff --git a/picoquic/picoquic_internal.h b/picoquic/picoquic_internal.h
@@ -1536,6 +1536,8 @@ int picoquic_register_cnx_id(picoquic_quic_t* quic, picoquic_cnx_t* cnx, picoqui
 /* Register or update default address and reset secret */
 int picoquic_register_net_secret(picoquic_cnx_t* cnx);
 
+void picoquic_create_local_cnx_id(picoquic_quic_t* quic, picoquic_connection_id_t* cnx_id, uint8_t id_length, picoquic_connection_id_t cnx_id_remote);
+
 /* Management of path */
 int picoquic_create_path(picoquic_cnx_t* cnx, uint64_t start_time,
     const struct sockaddr* local_addr, const struct sockaddr* peer_addr);
@@ -1657,6 +1659,12 @@ int picoquic_parse_packet_header(
     picoquic_cnx_t** pcnx,
     int receiving);
 
+size_t picoquic_create_long_header(picoquic_packet_type_enum packet_type, 
+    picoquic_connection_id_t* dest_cnx_id, picoquic_connection_id_t* srce_cnx_id,
+    int do_grease_quic_bit, uint32_t version, int version_index, uint64_t sequence_number,
+    size_t retry_token_length, uint8_t* retry_token,
+    uint8_t* bytes, size_t* pn_offset, size_t* pn_length);
+
 size_t picoquic_create_packet_header(
     picoquic_cnx_t* cnx,
     picoquic_packet_type_enum packet_type,
@@ -1677,6 +1685,8 @@ void picoquic_update_payload_length(
 
 size_t picoquic_get_checksum_length(picoquic_cnx_t* cnx, picoquic_epoch_enum is_cleartext_mode);
 
+void picoquic_protect_packet_header(uint8_t* send_buffer, size_t pn_offset, uint8_t first_mask, void* pn_enc);
+
 uint64_t picoquic_get_packet_number64(uint64_t highest, uint64_t mask, uint32_t pn);
 
 void picoquic_log_pn_dec_trial(picoquic_cnx_t* cnx); /* For debugging potential PN_ENC corruption */

diff --git a/picoquic/quicctx.c b/picoquic/quicctx.c
@@ -1467,6 +1467,16 @@ static void picoquic_create_random_cnx_id(picoquic_quic_t* quic, picoquic_connec
     cnx_id->id_len = id_length;
 }
 
+void picoquic_create_local_cnx_id(picoquic_quic_t* quic, picoquic_connection_id_t* cnx_id, uint8_t id_length, picoquic_connection_id_t cnx_id_remote)
+{
+    /* First call fills the CID with a random value */
+    picoquic_create_random_cnx_id(quic, cnx_id, quic->local_cnxid_length);
+    /* if required for application, call to function update that definition */
+    if (quic->cnx_id_callback_fn) {
+        quic->cnx_id_callback_fn(quic, *cnx_id, cnx_id_remote, quic->cnx_id_callback_ctx, cnx_id);
+    }
+}
+
 /* Path management -- returns the index of the path that was created. */
 
 int picoquic_create_path(picoquic_cnx_t* cnx, uint64_t start_time, const struct sockaddr* local_addr, const struct sockaddr* peer_addr)
@@ -3329,6 +3339,8 @@ picoquic_local_cnxid_list_t* picoquic_find_or_create_local_cnxid_list(picoquic_c
     return local_cnxid_list;
 }
 
+
+
 picoquic_local_cnxid_t* picoquic_create_local_cnxid(picoquic_cnx_t* cnx, 
     uint64_t unique_path_id, picoquic_connection_id_t* suggested_value, uint64_t current_time)
 {
@@ -3352,12 +3364,7 @@ picoquic_local_cnxid_t* picoquic_create_local_cnxid(picoquic_cnx_t* cnx,
                         l_cid->cnx_id = *suggested_value;
                     }
                     else {
-                        picoquic_create_random_cnx_id(cnx->quic, &l_cid->cnx_id, cnx->quic->local_cnxid_length);
-
-                        if (cnx->quic->cnx_id_callback_fn) {
-                            cnx->quic->cnx_id_callback_fn(cnx->quic, l_cid->cnx_id, cnx->initial_cnxid,
-                                cnx->quic->cnx_id_callback_ctx, &l_cid->cnx_id);
-                        }
+                        picoquic_create_local_cnx_id(cnx->quic, &l_cid->cnx_id, cnx->quic->local_cnxid_length, cnx->initial_cnxid);
                     }
 
                     if (picoquic_cnx_by_id(cnx->quic, l_cid->cnx_id, NULL) == NULL) {

diff --git a/picoquic/sender.c b/picoquic/sender.c
@@ -571,6 +571,76 @@ uint8_t picoquic_create_long_packet_type(picoquic_packet_type_enum pt, int versi
     return flags;
 }
 
+size_t picoquic_create_long_header(
+    picoquic_packet_type_enum packet_type,
+    picoquic_connection_id_t * dest_cnx_id,
+    picoquic_connection_id_t * srce_cnx_id,
+    int do_grease_quic_bit,
+    uint32_t version,
+    int version_index,
+    uint64_t sequence_number,
+    size_t retry_token_length,
+    uint8_t * retry_token,
+    uint8_t* bytes,
+    size_t* pn_offset,
+    size_t* pn_length)
+{
+    /* Create a long packet */
+    size_t length = 0;
+
+    /* The first byte is defined in RFC 9000 as:
+    *     Header Form (1) = 1,
+    *     Fixed Bit (1) = 1,
+    *     Long Packet Type (2),
+    *     Type-Specific Bits (4)
+    * The packet type is version dependent. In fact, the whole first byte is version
+    * dependent, the invariant draft only specifies the "header form" bit = 1 for long
+    * header. In version 1, the packet specific bytes are two reserved bytes +
+    * sequence number length, always set to 3 in picoquic (i.e., 4 bytes).
+    *
+    */
+    bytes[0] = picoquic_create_long_packet_type(packet_type, version_index);
+
+    if (do_grease_quic_bit) {
+        bytes[0] &= 0xBF;
+    }
+
+    length = 1;
+    picoformat_32(&bytes[length], version);
+    length += 4;
+
+    bytes[length++] = dest_cnx_id->id_len;
+    length += picoquic_format_connection_id(&bytes[length], PICOQUIC_MAX_PACKET_SIZE - length, *dest_cnx_id);
+    bytes[length++] = srce_cnx_id->id_len;
+    length += picoquic_format_connection_id(&bytes[length], PICOQUIC_MAX_PACKET_SIZE - length, *srce_cnx_id);
+
+    /* Special case of packet initial -- encode token as part of header */
+    if (packet_type == picoquic_packet_initial) {
+        length += picoquic_varint_encode(&bytes[length], PICOQUIC_MAX_PACKET_SIZE - length, retry_token_length);
+        if (retry_token_length > 0) {
+            memcpy(&bytes[length], retry_token, retry_token_length);
+            length += retry_token_length;
+        }
+    }
+
+    if (packet_type == picoquic_packet_retry) {
+        /* No payload length and no sequence number for Retry */
+        *pn_offset = 0;
+        *pn_length = 0;
+    }
+    else {
+        /* Reserve two bytes for payload length */
+        bytes[length++] = 0;
+        bytes[length++] = 0;
+        /* Encode the sequence number */
+        *pn_offset = length;
+        *pn_length = 4;
+        picoformat_32(&bytes[length], (uint32_t)sequence_number);
+        length += 4;
+    }
+    return length;
+}
+
 size_t picoquic_create_packet_header(
     picoquic_cnx_t* cnx,
     picoquic_packet_type_enum packet_type,
@@ -625,66 +695,28 @@ size_t picoquic_create_packet_header(
     }
     else {
         /* Create a long packet */
-        picoquic_connection_id_t dest_cnx_id =
+        picoquic_connection_id_t * dest_cnx_id =
             (cnx->client_mode && (packet_type == picoquic_packet_initial ||
                 packet_type == picoquic_packet_0rtt_protected)
                 && picoquic_is_connection_id_null(&path_x->p_remote_cnxid->cnx_id)) ?
-            cnx->initial_cnxid : path_x->p_remote_cnxid->cnx_id;
-
-        /* The first byte is defined in RFC 9000 as:
-         *     Header Form (1) = 1,
-         *     Fixed Bit (1) = 1,
-         *     Long Packet Type (2),
-         *     Type-Specific Bits (4)
-         * The packet type is version dependent. In fact, the whole first byte is version
-         * dependent, the invariant draft only specifies the "header form" bit = 1 for long
-         * header. In version 1, the packet specific bytes are two reserved bytes +
-         * sequence number length, always set to 3 in picoquic (i.e., 4 bytes).
-         * 
-         */
-        bytes[0] = picoquic_create_long_packet_type(packet_type, cnx->version_index);
-
-        if (cnx->do_grease_quic_bit) {
-            bytes[0] &= 0xBF;
-        }
-
-        length = 1;
-        if ((cnx->cnx_state == picoquic_state_client_init || cnx->cnx_state == picoquic_state_client_init_sent) && packet_type == picoquic_packet_initial) {
-            picoformat_32(&bytes[length], cnx->proposed_version);
-        }
-        else {
-            picoformat_32(&bytes[length], picoquic_supported_versions[cnx->version_index].version);
-        }
-        length += 4;
-
-        bytes[length++] = dest_cnx_id.id_len;
-        length += picoquic_format_connection_id(&bytes[length], PICOQUIC_MAX_PACKET_SIZE - length, dest_cnx_id);
-        bytes[length++] = path_x->p_local_cnxid->cnx_id.id_len;
-        length += picoquic_format_connection_id(&bytes[length], PICOQUIC_MAX_PACKET_SIZE - length, path_x->p_local_cnxid->cnx_id);
-
-        /* Special case of packet initial -- encode token as part of header */
-        if (packet_type == picoquic_packet_initial) {
-            length += picoquic_varint_encode(&bytes[length], PICOQUIC_MAX_PACKET_SIZE - length, cnx->retry_token_length);
-            if (cnx->retry_token_length > 0) {
-                memcpy(&bytes[length], cnx->retry_token, cnx->retry_token_length);
-                length += cnx->retry_token_length;
-            }
-        }
-
-        if (packet_type == picoquic_packet_retry) {
-            /* No payload length and no sequence number for Retry */
-            *pn_offset = 0;
-            *pn_length = 0;
-        } else {
-            /* Reserve two bytes for payload length */
-            bytes[length++] = 0;
-            bytes[length++] = 0;
-            /* Encode the sequence number */
-            *pn_offset = length;
-            *pn_length = 4;
-            picoformat_32(&bytes[length], (uint32_t) sequence_number);
-            length += 4;
-        }
+            &cnx->initial_cnxid : &path_x->p_remote_cnxid->cnx_id;
+        picoquic_connection_id_t* srce_cnx_id = &path_x->p_local_cnxid->cnx_id;
+        uint32_t version = ((cnx->cnx_state == picoquic_state_client_init || cnx->cnx_state == picoquic_state_client_init_sent) && packet_type == picoquic_packet_initial) ?
+            cnx->proposed_version : picoquic_supported_versions[cnx->version_index].version;
+
+        length = picoquic_create_long_header(
+            packet_type,
+            dest_cnx_id,
+            srce_cnx_id,
+            cnx->do_grease_quic_bit,
+            version,
+            cnx->version_index,
+            sequence_number,
+            cnx->retry_token_length,
+            cnx->retry_token,
+            bytes,
+            pn_offset,
+            pn_length);
     }
 
     return length;
@@ -772,6 +804,29 @@ size_t picoquic_get_checksum_length(picoquic_cnx_t* cnx, picoquic_epoch_enum epo
     return ret;
 }
 
+void picoquic_protect_packet_header(uint8_t * send_buffer, size_t pn_offset, uint8_t first_mask, void* pn_enc)
+{
+    /* The sample is located after the pn_offset */
+    size_t sample_offset = /* header_length */ pn_offset + 4;
+
+    if (pn_offset < sample_offset)
+    {
+        /* This is always true, as we use pn_length = 4 */
+        uint8_t mask_bytes[5] = { 0, 0, 0, 0, 0 };
+        uint8_t pn_l;
+
+        picoquic_pn_encrypt(pn_enc, send_buffer + sample_offset, mask_bytes, mask_bytes, 5);
+        /* Encode the first byte */
+        pn_l = (send_buffer[0] & 3) + 1;
+        send_buffer[0] ^= (mask_bytes[0] & first_mask);
+
+        /* Packet encoding is 1 to 4 bytes */
+        for (uint8_t i = 0; i < pn_l; i++) {
+            send_buffer[pn_offset+i] ^= mask_bytes[i+1];
+        }
+    }
+}
+
 static size_t picoquic_protect_packet(picoquic_cnx_t* cnx, 
     picoquic_packet_type_enum ptype,
     uint8_t * bytes, 
@@ -784,7 +839,6 @@ static size_t picoquic_protect_packet(picoquic_cnx_t* cnx,
     size_t send_length;
     size_t h_length;
     size_t pn_offset = 0;
-    size_t sample_offset = 0;
     size_t pn_length = 0;
     size_t aead_checksum_length = picoquic_aead_get_checksum_length(aead_context);
     uint8_t first_mask = 0x0F;
@@ -860,24 +914,7 @@ static size_t picoquic_protect_packet(picoquic_cnx_t* cnx,
         send_buffer, send_length, current_time);
 
     /* Next, encrypt the PN -- The sample is located after the pn_offset */
-    sample_offset = /* header_length */ pn_offset + 4;
-
-    if (pn_offset < sample_offset)
-    {
-        /* This is always true, as use pn_length = 4 */
-        uint8_t mask_bytes[5] = { 0, 0, 0, 0, 0 };
-        uint8_t pn_l;
-
-        picoquic_pn_encrypt(pn_enc, send_buffer + sample_offset, mask_bytes, mask_bytes, 5);
-        /* Encode the first byte */
-        pn_l = (send_buffer[0] & 3) + 1;
-        send_buffer[0] ^= (mask_bytes[0] & first_mask);
-
-        /* Packet encoding is 1 to 4 bytes */
-        for (uint8_t i = 0; i < pn_l; i++) {
-            send_buffer[pn_offset+i] ^= mask_bytes[i+1];
-        }
-    }
+    picoquic_protect_packet_header(send_buffer, pn_offset, first_mask, pn_enc);
 
     return send_length;
 }

diff --git a/picoquic/sockloop.c b/picoquic/sockloop.c
@@ -1156,37 +1156,7 @@ picoquic_network_thread_ctx_t* picoquic_start_custom_network_thread(picoquic_qui
 picoquic_network_thread_ctx_t* picoquic_start_network_thread(picoquic_quic_t* quic,
     picoquic_packet_loop_param_t* param, picoquic_packet_loop_cb_fn loop_callback, void* loop_callback_ctx, int* ret)
 {
-#if 1
     return picoquic_start_custom_network_thread(quic, param, NULL, NULL, NULL, NULL, loop_callback, loop_callback_ctx, ret);
-#else
-    picoquic_network_thread_ctx_t* thread_ctx = (picoquic_network_thread_ctx_t*)malloc(sizeof(picoquic_network_thread_ctx_t));
-    *ret = 0;
-
-    if (thread_ctx == NULL) {
-        /* Error, no memory */
-    }
-    else {
-        memset(thread_ctx, 0, sizeof(picoquic_network_thread_ctx_t));
-        /* Fill the arguments in the context */
-        thread_ctx->quic = quic;
-        thread_ctx->param = param;
-        thread_ctx->loop_callback = loop_callback;
-        thread_ctx->loop_callback_ctx = loop_callback_ctx;
-        /* Open the wake up pipe or event */
-        picoquic_open_network_wake_up(thread_ctx, ret);
-        /* Start thread at specified entry point */
-        if (thread_ctx->wake_up_defined){
-            thread_ctx->is_threaded = 1;
-            if ((*ret = picoquic_create_thread(&thread_ctx->thread_id, picoquic_packet_loop_v3, (void*)thread_ctx)) != 0) {
-                /* Free the context and return error condition if something went wrong */
-                thread_ctx->is_threaded = 0;
-                picoquic_delete_network_thread(thread_ctx);
-                thread_ctx = NULL;
-            }
-        }
-    }
-    return thread_ctx;
-#endif
 }
 
 int picoquic_wake_up_network_thread(picoquic_network_thread_ctx_t* thread_ctx)