From 2f0a7d44ac00b386f612a5f71c9a77cacf4eb9be Mon Sep 17 00:00:00 2001
From: huitema <huitema@huitema.net>
Date: Mon, 1 Oct 2018 22:07:18 -0700
Subject: [PATCH] Add tests of the retire CID and migration variants

---
 UnitTest1/unittest1.cpp      |  21 +++++
 picoquic/frames.c            |   8 +-
 picoquic/picoquic_internal.h |   2 +-
 picoquic/quicctx.c           |   2 +-
 picoquic/sender.c            |  19 ++--
 picoquic_t/picoquic_t.c      |   5 +-
 picoquictest/picoquictest.h  |   3 +
 picoquictest/tls_api_test.c  | 170 +++++++++++++++++++++++++++++++++--
 8 files changed, 213 insertions(+), 17 deletions(-)

diff --git a/UnitTest1/unittest1.cpp b/UnitTest1/unittest1.cpp
index b9e69e53..27c7d2bd 100644
--- a/UnitTest1/unittest1.cpp
+++ b/UnitTest1/unittest1.cpp
@@ -645,6 +645,20 @@ namespace UnitTest1
             Assert::AreEqual(ret, 0);
         }
 
+        TEST_METHOD(test_migration_long)
+        {
+            int ret = migration_test_long();
+
+            Assert::AreEqual(ret, 0);
+        }
+
+        TEST_METHOD(test_migration_loss)
+        {
+            int ret = migration_test_loss();
+
+            Assert::AreEqual(ret, 0);
+        }
+
         TEST_METHOD(test_cnxid_renewal)
         {
             int ret = cnxid_renewal_test();
@@ -652,6 +666,13 @@ namespace UnitTest1
             Assert::AreEqual(ret, 0);
         }
 
+        TEST_METHOD(test_retire_cnxid)
+        {
+            int ret = retire_cnxid_test();
+
+            Assert::AreEqual(ret, 0);
+        }
+
         TEST_METHOD(stress)
         {
             int ret = stress_test();
diff --git a/picoquic/frames.c b/picoquic/frames.c
index f09f152c..9558757a 100644
--- a/picoquic/frames.c
+++ b/picoquic/frames.c
@@ -482,7 +482,7 @@ uint8_t* picoquic_decode_new_connection_id_frame(picoquic_cnx_t* cnx, uint8_t* b
     }
 
     if (bytes == NULL || picoquic_is_connection_id_length_valid(cid_length) == 0) {
-        picoquic_connection_error(cnx, PICOQUIC_TRANSPORT_FRAME_FORMAT_ERROR,
+        picoquic_connection_error(cnx, (bytes == NULL) ? PICOQUIC_TRANSPORT_FRAME_FORMAT_ERROR : PICOQUIC_TRANSPORT_PROTOCOL_VIOLATION,
             picoquic_frame_type_new_connection_id);
     } else {
         int ret = picoquic_enqueue_cnxid_stash(cnx, sequence, cid_length, cnxid_bytes, secret_bytes, NULL);
@@ -533,6 +533,12 @@ int picoquic_queue_retire_connection_id_frame(picoquic_cnx_t * cnx, picoquic_con
 {
     size_t consumed = 0;
     uint8_t frame_buffer[258];
+
+    if (picoquic_supported_versions[cnx->version_index].version == PICOQUIC_SEVENTH_INTEROP_VERSION ||
+        picoquic_supported_versions[cnx->version_index].version == PICOQUIC_EIGHT_INTEROP_VERSION) {
+        return 0;
+    }
+
     int ret = picoquic_prepare_retire_connection_id_frame(cnx, cnxid, frame_buffer, sizeof(frame_buffer), &consumed);
 
     if (ret == 0 && consumed > 0) {
diff --git a/picoquic/picoquic_internal.h b/picoquic/picoquic_internal.h
index 4489c470..a6ec848e 100644
--- a/picoquic/picoquic_internal.h
+++ b/picoquic/picoquic_internal.h
@@ -41,7 +41,7 @@ extern "C" {
 #define PICOQUIC_PRACTICAL_MAX_MTU 1440
 #define PICOQUIC_RETRY_SECRET_SIZE 64
 #define PICOQUIC_DEFAULT_0RTT_WINDOW 4096
-#define PICOQUIC_NB_PATH_TARGET 4
+#define PICOQUIC_NB_PATH_TARGET 9
 
 #define PICOQUIC_NUMBER_OF_EPOCHS 4
 #define PICOQUIC_NUMBER_OF_EPOCH_OFFSETS (PICOQUIC_NUMBER_OF_EPOCHS+1)
diff --git a/picoquic/quicctx.c b/picoquic/quicctx.c
index a829bf7d..c425abdb 100644
--- a/picoquic/quicctx.c
+++ b/picoquic/quicctx.c
@@ -820,7 +820,7 @@ void picoquic_delete_abandoned_paths(picoquic_cnx_t* cnx, uint64_t current_time)
     while (path_index_current < cnx->nb_paths) {
         if (cnx->path[path_index_current]->challenge_failed ||
             (cnx->path[path_index_current]->path_is_demoted &&
-                current_time > cnx->path[path_index_current]->demotion_time)) {
+                current_time >= cnx->path[path_index_current]->demotion_time)) {
             /* Only increment the current index */
             path_index_current++;
         } else {
diff --git a/picoquic/sender.c b/picoquic/sender.c
index 34979516..b748dac8 100644
--- a/picoquic/sender.c
+++ b/picoquic/sender.c
@@ -1325,6 +1325,13 @@ void picoquic_cnx_set_next_wake_time(picoquic_cnx_t* cnx, uint64_t current_time)
         if (next_time > current_time) {
             for (int i = 0; i < cnx->nb_paths; i++) {
                 if (cnx->path[i]->path_is_demoted) {
+                    if (cnx->path[i]->demotion_time <= current_time) {
+                        next_time = current_time;
+                        break;
+                    }
+                    else if (cnx->path[i]->demotion_time < next_time) {
+                        next_time = cnx->path[i]->demotion_time;
+                    }
                     continue;
                 }
 
@@ -2665,10 +2672,9 @@ int picoquic_prepare_packet(picoquic_cnx_t* cnx,
 
     *send_length = 0;
 
-    if (cnx->client_mode) {
-        /* Remove delete paths */
-        picoquic_delete_abandoned_paths(cnx, current_time);
-    }
+    /* Remove delete paths */
+    picoquic_delete_abandoned_paths(cnx, current_time);
+
     /* Remove failed probes */
     picoquic_delete_failed_probes(cnx);
 
@@ -2689,8 +2695,9 @@ int picoquic_prepare_packet(picoquic_cnx_t* cnx,
                 break;
             }
             else if (path_x == NULL && cnx->path[i]->path_is_activated &&
-                (cnx->path[i]->challenge_required ||
-                    current_time >= (cnx->path[i]->challenge_time + cnx->path[i]->retransmit_timer))) {
+                (cnx->path[i]->challenge_required &&
+                    (cnx->path[i]->challenge_repeat_count == 0 ||
+                    current_time >= (cnx->path[i]->challenge_time + cnx->path[i]->retransmit_timer)))) {
                 /* will try this path, unless a validated path came in */
                 path_x = cnx->path[i];
             }
diff --git a/picoquic_t/picoquic_t.c b/picoquic_t/picoquic_t.c
index f4bd4b54..7bec0789 100644
--- a/picoquic_t/picoquic_t.c
+++ b/picoquic_t/picoquic_t.c
@@ -132,7 +132,10 @@ static const picoquic_test_def_t test_table[] = {
     { "transmit_cnxid", transmit_cnxid_test },
     { "probe_api", probe_api_test },
     { "migration" , migration_test },
-    { "cnxid_renewal",  cnxid_renewal_test},
+    { "migration_long", migration_test_long },
+    { "migration_with_loss", migration_test_loss },
+    { "cnxid_renewal",  cnxid_renewal_test },
+    { "retire_cnxid", retire_cnxid_test },
     { "stress", stress_test },
     { "fuzz", fuzz_test }
 };
diff --git a/picoquictest/picoquictest.h b/picoquictest/picoquictest.h
index 65ad51f3..76afdc62 100644
--- a/picoquictest/picoquictest.h
+++ b/picoquictest/picoquictest.h
@@ -129,7 +129,10 @@ int new_cnxid_test();
 int transmit_cnxid_test();
 int probe_api_test();
 int migration_test();
+int migration_test_long(); 
+int migration_test_loss();
 int cnxid_renewal_test();
+int retire_cnxid_test();
 
 #ifdef __cplusplus
 }
diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c
index f3dee3c2..c4a96e44 100644
--- a/picoquictest/tls_api_test.c
+++ b/picoquictest/tls_api_test.c
@@ -3463,6 +3463,18 @@ int client_error_test()
  * TODO: also test that no New Connection Id frames are sent if migration is disabled 
  */
 
+int test_cnxid_count_stash(picoquic_cnx_t * cnx) {
+    picoquic_cnxid_stash_t * stash = cnx->cnxid_stash_first;
+    int nb = 0;
+
+    while (stash != NULL) {
+        nb++;
+        stash = stash->next_in_stash;
+    }
+
+    return nb;
+}
+
 int transmit_cnxid_test_stash(picoquic_cnx_t * cnx1, picoquic_cnx_t * cnx2, char const * cnx_text)
 {
     int ret = 0;
@@ -3764,7 +3776,7 @@ int probe_api_test()
  * starts the migration by explicitly probing a new path.
  */
 
-int migration_test()
+int migration_test_scenario(test_api_stream_desc_t * scenario, size_t size_of_scenario, uint64_t loss_target)
 {
     uint64_t loss_mask_data = 0;
     uint64_t simulated_time = 0;
@@ -3835,20 +3847,23 @@ int migration_test()
 
     /* Prepare to send data */
     if (ret == 0) {
-        ret = test_api_init_send_recv_scenario(test_ctx, test_scenario_q_and_r, sizeof(test_scenario_q_and_r));
+        ret = test_api_init_send_recv_scenario(test_ctx, scenario, size_of_scenario);
     }
 
     /* Perform a data sending loop */
+    loss_mask = loss_target;
+
     if (ret == 0) {
         ret = tls_api_data_sending_loop(test_ctx, &loss_mask, &simulated_time, 0);
     }
 
     /* Add a time loop of 3 seconds to give some time for the probes to be repeated */
-    next_time = simulated_time + 3000000;
+    next_time = simulated_time + 4000000;
     loss_mask = 0;
     while (ret == 0 && simulated_time < next_time && test_ctx->cnx_client->cnx_state == picoquic_state_client_ready
         && test_ctx->cnx_server->cnx_state == picoquic_state_server_ready
-        && test_ctx->cnx_server->path[0]->challenge_verified != 1) {
+        && (test_ctx->cnx_server->path[0]->challenge_verified != 1 || test_ctx->cnx_client->path[0]->path_is_demoted == 1 ||
+            initial_challenge == test_ctx->cnx_server->path[0]->challenge)) {
         int was_active = 0;
 
         ret = tls_api_one_sim_round(test_ctx, &simulated_time, next_time, &was_active);
@@ -3858,11 +3873,11 @@ int migration_test()
     /* TODO: verify that exactly one challenge was sent */
     if (ret == 0) {
         if (initial_challenge == test_ctx->cnx_server->path[0]->challenge) {
-            DBG_PRINTF("%s", "Challenge was not renewed after NAT rebinding");
+            DBG_PRINTF("%s", "Challenge was not renewed after migration");
             ret = -1;
         }
         else if (test_ctx->cnx_server->path[0]->challenge_verified != 1) {
-            DBG_PRINTF("%s", "Challenge was not verified after NAT rebinding");
+            DBG_PRINTF("%s", "Challenge was not verified after migration");
             ret = -1;
         }
     }
@@ -3887,6 +3902,23 @@ int migration_test()
     return ret;
 }
 
+int migration_test()
+{
+    return migration_test_scenario(test_scenario_q_and_r, sizeof(test_scenario_q_and_r), 0);
+}
+
+int migration_test_long()
+{
+    return migration_test_scenario(test_scenario_very_long, sizeof(test_scenario_very_long), 0);
+}
+
+int migration_test_loss()
+{
+    uint64_t loss_mask = 0x09;
+
+    return migration_test_scenario(test_scenario_q_and_r, sizeof(test_scenario_q_and_r), loss_mask);
+}
+
 /* Connection ID renewal test.
  */
 
@@ -3985,4 +4017,128 @@ int cnxid_renewal_test()
     }
 
     return ret;
-}
\ No newline at end of file
+}
+
+
+/*
+ * Perform a test of the "retire connection id" function.
+ * The test will artificially retire connection ID on the client,
+ * and verify that the server will refill the stash of 
+ * connection ID.
+ */
+int retire_cnxid_test()
+{
+    uint64_t simulated_time = 0;
+    uint64_t loss_mask = 0;
+    picoquic_test_tls_api_ctx_t* test_ctx = NULL;
+    int ret = tls_api_init_ctx(&test_ctx, PICOQUIC_INTERNAL_TEST_VERSION_1,
+        PICOQUIC_TEST_SNI, PICOQUIC_TEST_ALPN, &simulated_time, NULL, 0, 0, 0);
+
+    if (ret == 0) {
+        ret = tls_api_connection_loop(test_ctx, &loss_mask, 0, &simulated_time);
+    }
+
+    /* run a receive loop until no outstanding data */
+    if (ret == 0) {
+        uint64_t time_out = simulated_time + 4000000;
+        int nb_rounds = 0;
+        int success = 0;
+
+        while (ret == 0 && simulated_time < time_out &&
+            nb_rounds < 2048 && test_ctx->cnx_client->cnx_state != picoquic_state_disconnected) {
+            int was_active = 0;
+
+            ret = tls_api_one_sim_round(test_ctx, &simulated_time, time_out, &was_active);
+            nb_rounds++;
+
+            if (test_ctx->cnx_client->nb_paths >= PICOQUIC_NB_PATH_TARGET &&
+                test_ctx->cnx_server->nb_paths >= PICOQUIC_NB_PATH_TARGET &&
+                picoquic_is_cnx_backlog_empty(test_ctx->cnx_client) &&
+                picoquic_is_cnx_backlog_empty(test_ctx->cnx_server)) {
+                success = 1;
+                break;
+            }
+        }
+
+        if (ret == 0 && success == 0) {
+            DBG_PRINTF("Exit synch loop after %d rounds, backlog or not enough paths (%d & %d).\n",
+                nb_rounds, test_ctx->cnx_client->nb_paths, test_ctx->cnx_server->nb_paths);
+        }
+    }
+
+    if (ret == 0) {
+        if (test_ctx->cnx_client->nb_paths < PICOQUIC_NB_PATH_TARGET) {
+            DBG_PRINTF("Only %d paths created on client.\n", test_ctx->cnx_client->nb_paths);
+            ret = -1;
+        }
+        else if (test_ctx->cnx_server->nb_paths < PICOQUIC_NB_PATH_TARGET) {
+            DBG_PRINTF("Only %d paths created on server.\n", test_ctx->cnx_server->nb_paths);
+        }
+    }
+
+    /* Delete several connection ID */
+    for (int i = 2; ret == 0 && i < PICOQUIC_NB_PATH_TARGET; i++) {
+        picoquic_cnxid_stash_t * stashed = picoquic_dequeue_cnxid_stash(test_ctx->cnx_client);
+
+        if (stashed == NULL) {
+            DBG_PRINTF("Could not retrieve cnx ID #%d.\n", i-1);
+        } else {
+            ret = picoquic_queue_retire_connection_id_frame(test_ctx->cnx_client, &stashed->cnx_id);
+            free(stashed);
+        }
+    }
+
+    /* run the loop again until no outstanding data */
+    if (ret == 0) {
+        uint64_t time_out = simulated_time + 8000000;
+        int nb_rounds = 0;
+        int success = 0;
+
+        while (ret == 0 && simulated_time < time_out &&
+            nb_rounds < 2048 && test_ctx->cnx_client->cnx_state != picoquic_state_disconnected) {
+            int was_active = 0; 
+
+            ret = tls_api_one_sim_round(test_ctx, &simulated_time, time_out, &was_active);
+            nb_rounds++;
+
+            if (test_ctx->cnx_client->nb_paths >= PICOQUIC_NB_PATH_TARGET &&
+                test_ctx->cnx_server->nb_paths == PICOQUIC_NB_PATH_TARGET &&
+                test_ctx->cnx_client->first_misc_frame == NULL &&
+                test_cnxid_count_stash(test_ctx->cnx_client) >= (PICOQUIC_NB_PATH_TARGET - 1) &&
+                picoquic_is_cnx_backlog_empty(test_ctx->cnx_client) &&
+                picoquic_is_cnx_backlog_empty(test_ctx->cnx_server)) {
+                success = 1;
+                break;
+            }
+        }
+
+        if (ret == 0 && success == 0) {
+            DBG_PRINTF("Exit synch loop after %d rounds, backlog or not enough paths (%d & %d).\n",
+                nb_rounds, test_ctx->cnx_client->nb_paths, test_ctx->cnx_server->nb_paths);
+        }
+    }
+
+    /* Check */
+
+    if (ret == 0) {
+        if (test_ctx->cnx_server->nb_paths!= PICOQUIC_NB_PATH_TARGET) {
+            DBG_PRINTF("Found %d paths active on server instead of %d.\n", test_ctx->cnx_server->nb_paths, PICOQUIC_NB_PATH_TARGET);
+            ret = -1;
+        }
+    }
+
+    if (ret == 0) {
+        ret = transmit_cnxid_test_stash(test_ctx->cnx_client, test_ctx->cnx_server, "client");
+    }
+
+    if (ret == 0) {
+        ret = transmit_cnxid_test_stash(test_ctx->cnx_server, test_ctx->cnx_client, "server");
+    }
+
+    if (test_ctx != NULL) {
+        tls_api_delete_ctx(test_ctx);
+        test_ctx = NULL;
+    }
+
+    return ret;
+}
-- 
GitLab