Fix: heed peer's max_packet_size transport parameter

include/lsquic.h

@@ -339,6 +339,9 @@ typedef struct ssl_ctx_st * (*lsquic_lookup_cert_f)(
 /* 1: Cubic; 2: BBR */
 #define LSQUIC_DF_CC_ALGO 1
 
+/** By default, incoming packet size is not limited. */
+#define LSQUIC_DF_MAX_PACKET_SIZE_RX 0
+
 struct lsquic_engine_settings {
     /**
      * This is a bit mask wherein each bit corresponds to a value in
@@ -737,6 +740,17 @@ struct lsquic_engine_settings {
      * Default value is @ref LSQUIC_DF_TIMESTAMPS
      */
     int             es_timestamps;
+
+    /**
+     * Maximum packet size we are willing to receive.  This is sent to
+     * peer in transport parameters: the library does not enforce this
+     * limit for incoming packets.
+     *
+     * If set to zero, limit is not set.
+     *
+     * Default value is @ref LSQUIC_DF_MAX_PACKET_SIZE_RX
+     */
+    unsigned short  es_max_packet_size_rx;
 };
 
 /* Initialize `settings' to default values */

src/liblsquic/lsquic_enc_sess_ietf.c

@@ -532,7 +532,6 @@ gen_trans_params (struct enc_sess_iquic *enc_sess, unsigned char *buf,
                             = TP_DEF_ACK_DELAY_EXP;
     params.tp_max_idle_timeout = settings->es_idle_timeout * 1000;
     params.tp_max_ack_delay = TP_DEF_MAX_ACK_DELAY;
-    params.tp_max_packet_size = 1370 /* XXX: based on socket */;
     params.tp_active_connection_id_limit = MAX_IETF_CONN_DCIDS;
     params.tp_set |= (1 << TPI_INIT_MAX_DATA)
                   |  (1 << TPI_INIT_MAX_STREAM_DATA_BIDI_LOCAL)
@@ -543,9 +542,13 @@ gen_trans_params (struct enc_sess_iquic *enc_sess, unsigned char *buf,
                   |  (1 << TPI_ACK_DELAY_EXPONENT)
                   |  (1 << TPI_MAX_IDLE_TIMEOUT)
                   |  (1 << TPI_MAX_ACK_DELAY)
-                  |  (1 << TPI_MAX_PACKET_SIZE)
                   |  (1 << TPI_ACTIVE_CONNECTION_ID_LIMIT)
                   ;
+    if (settings->es_max_packet_size_rx)
+    {
+        params.tp_max_packet_size = settings->es_max_packet_size_rx;
+        params.tp_set |= 1 << TPI_MAX_PACKET_SIZE;
+    }
     if (!settings->es_allow_migration)
         params.tp_set |= 1 << TPI_DISABLE_ACTIVE_MIGRATION;
     if (settings->es_ql_bits)

src/liblsquic/lsquic_mini_conn_ietf.c

@@ -162,6 +162,50 @@ read_from_msg_ctx (void *ctx, void *buf, size_t len)
 }
 
 
+static int
+imico_chlo_has_been_consumed (const struct ietf_mini_conn *conn)
+{
+    return conn->imc_streams[ENC_LEV_CLEAR].mcs_read_off > 3
+        && conn->imc_streams[ENC_LEV_CLEAR].mcs_read_off >= conn->imc_ch_len;
+}
+
+
+static int
+imico_maybe_process_params (struct ietf_mini_conn *conn)
+{
+    const struct transport_params *params;
+
+    if (imico_chlo_has_been_consumed(conn)
+        && (conn->imc_flags & (IMC_ENC_SESS_INITED|IMC_HAVE_TP))
+                                                    == IMC_ENC_SESS_INITED)
+    {
+        params = conn->imc_conn.cn_esf.i->esfi_get_peer_transport_params(
+                                                conn->imc_conn.cn_enc_session);
+        if (params)
+        {
+            conn->imc_flags |= IMC_HAVE_TP;
+            conn->imc_ack_exp = params->tp_ack_delay_exponent;
+            if (params->tp_set & (1 << TPI_MAX_PACKET_SIZE))
+            {
+                if (params->tp_numerics[TPI_MAX_PACKET_SIZE]
+                                                < conn->imc_path.np_pack_size)
+                    conn->imc_path.np_pack_size =
+                                    params->tp_numerics[TPI_MAX_PACKET_SIZE];
+            }
+            LSQ_DEBUG("read transport params, packet size is set to %hu bytes",
+                                                conn->imc_path.np_pack_size);
+        }
+        else
+        {
+            conn->imc_flags |= IMC_BAD_TRANS_PARAMS;
+            return -1;
+        }
+    }
+
+    return 0;
+}
+
+
 static ssize_t
 imico_stream_write (void *stream, const void *bufp, size_t bufsz)
 {
@@ -175,6 +219,9 @@ imico_stream_write (void *stream, const void *bufp, size_t bufsz)
     const unsigned char *p;
     int len;
 
+    if (0 != imico_maybe_process_params(conn))
+        return -1;
+
     if (PNS_INIT == lsquic_enclev2pns[ cryst->mcs_enc_level ]
                                         && (conn->imc_flags & IMC_IGNORE_INIT))
     {
@@ -281,14 +328,6 @@ imico_read_chlo_size (struct ietf_mini_conn *conn, const unsigned char *buf,
 }
 
 
-static int
-imico_chlo_has_been_consumed (const struct ietf_mini_conn *conn)
-{
-    return conn->imc_streams[ENC_LEV_CLEAR].mcs_read_off > 3
-        && conn->imc_streams[ENC_LEV_CLEAR].mcs_read_off >= conn->imc_ch_len;
-}
-
-
 static ssize_t
 imico_stream_readf (void *stream,
         size_t (*readf)(void *, const unsigned char *, size_t, int), void *ctx)
@@ -765,7 +804,6 @@ imico_process_crypto_frame (IMICO_PROC_FRAME_ARGS)
     int parsed_len;
     enum enc_level enc_level, i;
     struct stream_frame stream_frame;
-    const struct transport_params *params;
 
     parsed_len = conn->imc_conn.cn_pf->pf_parse_crypto_frame(p, len,
                                                                 &stream_frame);
@@ -836,25 +874,6 @@ imico_process_crypto_frame (IMICO_PROC_FRAME_ARGS)
     }
 
 
-    if (enc_level == ENC_LEV_CLEAR
-        && imico_chlo_has_been_consumed(conn)
-        && (conn->imc_flags & (IMC_ENC_SESS_INITED|IMC_HAVE_TP))
-                                                    == IMC_ENC_SESS_INITED)
-    {
-        params = conn->imc_conn.cn_esf.i->esfi_get_peer_transport_params(
-                                                conn->imc_conn.cn_enc_session);
-        if (params)
-        {
-            conn->imc_flags |= IMC_HAVE_TP;
-            conn->imc_ack_exp = params->tp_ack_delay_exponent;
-        }
-        else
-        {
-            conn->imc_flags |= IMC_BAD_TRANS_PARAMS;
-            return 0;
-        }
-    }
-
     return parsed_len;
 }
 

src/liblsquic/lsquic_trans_params.c

@@ -128,6 +128,9 @@ static const uint64_t def_vals[MAX_NUM_WITH_DEF_TPI + 1] =
 
 static const uint64_t max_vals[MAX_NUMERIC_TPI + 1] =
 {
+    /* We don't enforce the maximum practical UDP payload value of 65527, as
+     * it is not required by the spec and is not necessary.
+     */
     [TPI_MAX_PACKET_SIZE]                   =  VINT_MAX_VALUE,
     [TPI_ACK_DELAY_EXPONENT]                =  VINT_MAX_VALUE,
     [TPI_INIT_MAX_STREAMS_UNI]              =  VINT_MAX_VALUE,
@@ -146,6 +149,8 @@ static const uint64_t max_vals[MAX_NUMERIC_TPI + 1] =
 
 static const uint64_t min_vals[MAX_NUMERIC_TPI + 1] =
 {
+    /* On the other hand, we do enforce the lower bound. */
+    [TPI_MAX_PACKET_SIZE]                   =  1200,
     [TPI_MIN_ACK_DELAY]                     =  1,
 };
 

test/test_common.c

@@ -1911,6 +1911,11 @@ set_engine_option (struct lsquic_engine_settings *settings,
             settings->es_qpack_dec_max_size = atoi(val);
             return 0;
         }
+        if (0 == strncmp(name, "max_packet_size_rx", 18))
+        {
+            settings->es_max_packet_size_rx = atoi(val);
+            return 0;
+        }
         break;
     case 20:
         if (0 == strncmp(name, "max_header_list_size", 20))

test/unittests/test_trapa.c

@@ -63,15 +63,16 @@ static const struct trapa_test tests[] =
                     | (1 << TPI_ACTIVE_CONNECTION_ID_LIMIT),
             .tp_init_max_stream_data_bidi_local = 0x12348877,
             .tp_init_max_data = 0xAABB,
+            .tp_max_packet_size = 1213,
             .tp_max_idle_timeout = 10 * 1000,
             .tp_max_ack_delay = TP_DEF_MAX_ACK_DELAY,
             .tp_active_connection_id_limit = 7,
         },
         .is_server = 0,
-        .enc_len = 25,
+        .enc_len = 26,
         .encoded =
      /* Idle timeout */     "\x01\x02\x67\x10"
-     /* Packet size */      "\x03\x01\x00"
+     /* Packet size */      "\x03\x02\x44\xBD"
      /* Max data */         "\x04\x04\x80\x00\xAA\xBB"
      /* Bidi local */       "\x05\x04\x92\x34\x88\x77"
      /* Ack delay exp */    "\x0A\x01\x00"
@@ -103,13 +104,13 @@ static const struct trapa_test tests[] =
             TP_DEFAULT_VALUES,
             .tp_init_max_data = 0x123456,
             .tp_init_max_stream_data_bidi_local = 0xABCDEF88,
-            .tp_max_packet_size = 0x333,
+            .tp_max_packet_size = 0x555,
         },
         .is_server = 1,
         .addl_set = 1 << TPI_DISABLE_ACTIVE_MIGRATION,
         .enc_len = 22,
         .encoded =
-     /* Packet size */      "\x03\x02\x43\x33"
+     /* Packet size */      "\x03\x02\x45\x55"
      /* Max data */         "\x04\x04\x80\x12\x34\x56"
      /* Bidi local */       "\x05\x08\xC0\x00\x00\x00\xAB\xCD\xEF\x88"
      /* Migration */        "\x0C\x00"
@@ -124,7 +125,7 @@ static const struct trapa_test tests[] =
         .params = {
             TP_DEFAULT_VALUES,
             .tp_max_ack_delay = 25,
-            .tp_max_packet_size = 0x333,
+            .tp_max_packet_size = 0x555,
             .tp_preferred_address = {
                 .ipv4_addr = "\x01\x02\x03\x04",
                 .ipv4_port = 0x1234,
@@ -148,7 +149,7 @@ static const struct trapa_test tests[] =
                             "\x0B"  /* CID len */
                             "\x20\x21\x22\x23\x24\x25\x26\x27\x28\x29\x2A"
                             "\x30\x31\x32\x33\x34\x35\x36\x37\x38\x39\x3A\x3B\x3C\x3D\x3E\x3F"
-     /* Packet size */      "\x03\x02\x43\x33"
+     /* Packet size */      "\x03\x02\x45\x55"
     /* Trailer to make the end easily visible in gdb: */
     "\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF"
     },