From 3c5a2638fd6242766daa1e0b567e62b0fc050f26 Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 26 Nov 2024 11:32:29 -0800 Subject: [PATCH 01/12] Add test for ssl key log --- UnitTest1/unittest1.cpp | 6 + picoquic_t/picoquic_t.c | 1 + picoquictest/picoquictest.h | 1 + picoquictest/tls_api_test.c | 356 ++++++++---------------------------- 4 files changed, 87 insertions(+), 277 deletions(-) diff --git a/UnitTest1/unittest1.cpp b/UnitTest1/unittest1.cpp index 79d50ac7a..c18e3e1f3 100644 --- a/UnitTest1/unittest1.cpp +++ b/UnitTest1/unittest1.cpp @@ -1264,6 +1264,12 @@ namespace UnitTest1 Assert::AreEqual(ret, 0); } + + TEST_METHOD(keylog) { + int ret = keylog_test(); + + Assert::AreEqual(ret, 0); + } TEST_METHOD(draft17_vector) { diff --git a/picoquic_t/picoquic_t.c b/picoquic_t/picoquic_t.c index 097021b46..46edeb3e8 100644 --- a/picoquic_t/picoquic_t.c +++ b/picoquic_t/picoquic_t.c @@ -284,6 +284,7 @@ static const picoquic_test_def_t test_table[] = { { "nat_handshake", nat_handshake_test }, { "key_rotation_vector", key_rotation_vector_test }, { "key_rotation_stress", key_rotation_stress_test }, + { "keylog_test", keylog_test }, { "short_initial_cid", short_initial_cid_test }, { "stream_id_max", stream_id_max_test }, { "padding_test", padding_test }, diff --git a/picoquictest/picoquictest.h b/picoquictest/picoquictest.h index c0fdcdf5f..1bae308b0 100644 --- a/picoquictest/picoquictest.h +++ b/picoquictest/picoquictest.h @@ -224,6 +224,7 @@ int false_migration_test(); int nat_handshake_test(); int key_rotation_vector_test(); int key_rotation_stress_test(); +int keylog_test(); int short_initial_cid_test(); int stream_id_max_test(); int padding_test(); diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c index 440958ac8..89aa2b9b8 100644 --- a/picoquictest/tls_api_test.c +++ b/picoquictest/tls_api_test.c @@ -11549,284 +11549,7 @@ int random_padding_test() return ret; } -#if 0 -/* Tests of BDP option. - * = Verify that a download works faster with BDP option enabled - * = Verify that the BDP option is not validated if the min rtt changes - * - Verify that the BDP option is not validated if the IP address changes - * - Verify that the BDP option is not validated if the delay is too long - */ - -typedef enum { - bdp_test_option_none = 0, - bdp_test_option_basic, - bdp_test_option_rtt, - bdp_test_option_ip, - bdp_test_option_delay, - bdp_test_option_reno, - bdp_test_option_cubic, - bdp_test_option_short, - bdp_test_option_short_lo, - bdp_test_option_short_hi, -} bdp_test_option_enum; - -int bdp_option_test_one(bdp_test_option_enum bdp_test_option) -{ - uint64_t simulated_time = 0; - picoquic_test_tls_api_ctx_t* test_ctx = NULL; - char const* sni = PICOQUIC_TEST_SNI; - char const* alpn = PICOQUIC_TEST_ALPN; - uint32_t proposed_version = 0; - uint64_t max_completion_time = 6800000; - uint64_t latency = 300000ull; - uint64_t buffer_size = 2 * latency; - picoquic_connection_id_t initial_cid = { {0xbd, 0x80, 0, 0, 0, 0, 0, 0}, 8 }; - picoquic_congestion_algorithm_t* ccalgo = picoquic_bbr_algorithm; - picoquic_tp_t server_parameters; - picoquic_tp_t client_parameters; - - int ret = 0; - - /* Initialize an empty ticket store */ - ret = picoquic_save_tickets(NULL, simulated_time, ticket_file_name); - - for (int i = 0; ret == 0 && i < 2; i++) { - /* If testing delay, insert a delay before the second connection attempt */ - if (i == 1 && bdp_test_option == bdp_test_option_delay) { - simulated_time += 48ull * 3600ull * 1000000ull; - } - initial_cid.id[2] = i; - initial_cid.id[3] = (uint8_t)bdp_test_option; - /* Set up the context, while setting the ticket store parameter for the client */ - ret = tls_api_init_ctx_ex(&test_ctx, - (i == 0) ? 0 : proposed_version, sni, alpn, &simulated_time, ticket_file_name, NULL, 0, 1, 0, &initial_cid); - /* Set the various parameters */ - if (ret == 0) { - test_ctx->c_to_s_link->microsec_latency = latency; - test_ctx->s_to_c_link->microsec_latency = latency; - test_ctx->c_to_s_link->picosec_per_byte = (1000000ull * 8) / 20; - test_ctx->s_to_c_link->picosec_per_byte = (1000000ull * 8) / 20; - - if (bdp_test_option == bdp_test_option_short || - bdp_test_option == bdp_test_option_short_lo || - bdp_test_option == bdp_test_option_short_hi) { - /* Test that the BDP option also works well if delay < 250 ms */ - max_completion_time = 4500000; - test_ctx->c_to_s_link->microsec_latency = 100000ull; - test_ctx->s_to_c_link->microsec_latency = 100000ull; - buffer_size = 2 * test_ctx->c_to_s_link->microsec_latency; - if (i == 0) { - if (bdp_test_option == bdp_test_option_short_lo) { - test_ctx->c_to_s_link->picosec_per_byte *= 2; - test_ctx->s_to_c_link->picosec_per_byte *= 2; - } - else if (bdp_test_option == bdp_test_option_short_hi) { - test_ctx->c_to_s_link->picosec_per_byte /= 2; - test_ctx->s_to_c_link->picosec_per_byte /= 2; - } - } - else if (i == 1 && bdp_test_option == bdp_test_option_short_lo) { - max_completion_time = 4650000; - } - } - else if (i > 0) { - switch (bdp_test_option) { - case bdp_test_option_none: - break; - case bdp_test_option_basic: - max_completion_time = 5900000; - break; - case bdp_test_option_rtt: - max_completion_time = 4610000; - test_ctx->c_to_s_link->microsec_latency = 50000ull; - test_ctx->s_to_c_link->microsec_latency = 50000ull; - buffer_size = 2 * test_ctx->c_to_s_link->microsec_latency; - break; - case bdp_test_option_ip: - picoquic_set_test_address(&test_ctx->client_addr, 0x08080808, 2345); - max_completion_time = 9000000; - break; - case bdp_test_option_delay: - max_completion_time = 8000000; - break; - case bdp_test_option_reno: - max_completion_time = 6750000; - break; - default: - break; - } - } - if (bdp_test_option == bdp_test_option_reno) { - ccalgo = picoquic_newreno_algorithm; - } - else if (bdp_test_option == bdp_test_option_cubic) { - ccalgo = picoquic_cubic_algorithm; - max_completion_time = 10000000; - } - picoquic_set_default_congestion_algorithm(test_ctx->qserver, ccalgo); - picoquic_set_congestion_algorithm(test_ctx->cnx_client, ccalgo); - picoquic_set_default_bdp_frame_option(test_ctx->qclient, 1); - picoquic_set_default_bdp_frame_option(test_ctx->qserver, 1); - test_ctx->qserver->use_long_log = 1; - picoquic_set_binlog(test_ctx->qserver, "."); - /* Set parameters */ - picoquic_init_transport_parameters(&server_parameters, 0); - picoquic_init_transport_parameters(&client_parameters, 1); - server_parameters.enable_bdp_frame = 1; - client_parameters.enable_bdp_frame = 1; - client_parameters.initial_max_stream_data_bidi_remote = 1000000; - client_parameters.initial_max_data = 10000000; - picoquic_set_transport_parameters(test_ctx->cnx_client, &client_parameters); - ret = picoquic_set_default_tp(test_ctx->qserver, &server_parameters); - - if (ret == 0) { - ret = tls_api_one_scenario_body(test_ctx, &simulated_time, test_scenario_10mb, sizeof(test_scenario_10mb), 0, 0, 0, buffer_size, - (i==0)?0:max_completion_time); - } - - /* Verify that the BDP option was set and processed */ - if (ret == 0) { - if (i == 1 && test_ctx->cnx_client->nb_zero_rtt_acked == 0 && bdp_test_option != bdp_test_option_delay) { - DBG_PRINTF("BDP RTT test (bdp test: %d), cnx %d, no zero RTT data acked.\n", - bdp_test_option, i); - ret = -1; - } - if (!test_ctx->cnx_client->send_receive_bdp_frame) { - DBG_PRINTF("BDP RTT test (bdp test: %d), cnx %d, bdp option not negotiated on client.\n", - bdp_test_option, i); - ret = -1; - } - if (!test_ctx->cnx_server->send_receive_bdp_frame) { - DBG_PRINTF("BDP RTT test (bdp test: %d), cnx %d, bdp option not negotiated on server.\n", - bdp_test_option, i); - ret = -1; - } - if (ret == 0 && i == 1) { - if (test_ctx->cnx_server->nb_retransmission_total * 10 > - test_ctx->cnx_server->nb_packets_sent && - bdp_test_option != bdp_test_option_cubic && - bdp_test_option != bdp_test_option_delay && - bdp_test_option != bdp_test_option_ip) { - DBG_PRINTF("BDP RTT test (bdp test: %d), cnx %d, too many losses, %"PRIu64"/%"PRIu64".\n", - bdp_test_option, i, test_ctx->cnx_server->nb_retransmission_total, - test_ctx->cnx_server->nb_packets_sent); - ret = -1; - - } - /* Verify bdp test option was executed */ - if (!test_ctx->cnx_client->path[0]->is_bdp_sent) { - DBG_PRINTF("BDP RTT test (bdp test: %d), cnx %d, bdp frame not sent by client.\n", - bdp_test_option, i); - ret = -1; - } - else if (bdp_test_option == bdp_test_option_basic || - bdp_test_option == bdp_test_option_reno || - bdp_test_option == bdp_test_option_short || - bdp_test_option == bdp_test_option_short_hi || - bdp_test_option == bdp_test_option_short_lo || - bdp_test_option == bdp_test_option_cubic) { - if (!test_ctx->cnx_server->cwin_notified_from_seed) { - DBG_PRINTF("BDP RTT test (bdp test: %d), cnx %d, cwin not seed on server.\n", - bdp_test_option, i); - ret = -1; - } - } - else if (test_ctx->cnx_server->cwin_notified_from_seed) { - DBG_PRINTF("BDP RTT test (bdp test: %d), cnx %d, unexpected cwin seed on server.\n", - bdp_test_option, i); - ret = -1; - } - } - } - - /* Save the session tickets */ - if (ret == 0) { - if (test_ctx->qclient->p_first_ticket == NULL) { - DBG_PRINTF("BDP RTT test (bdp option: %d), cnx %d, no ticket received.\n", - bdp_test_option, i); - ret = -1; - } - else { - ret = picoquic_save_tickets(test_ctx->qclient->p_first_ticket, simulated_time, ticket_file_name); - if (ret != 0) { - DBG_PRINTF("Zero RTT test (bdp test option: %d), cnx %d, ticket save error (0x%x).\n", - bdp_test_option, i, ret); - } - } - } - - /* Free the resource, which will close the log file. */ - if (test_ctx != NULL) { - tls_api_delete_ctx(test_ctx); - test_ctx = NULL; - } - } - } - - return ret; -} - -int bdp_basic_test() -{ - return bdp_option_test_one(bdp_test_option_basic); -} - -int bdp_rtt_test() -{ - /* TODO: this test succeeds for the wrong reason. - * The goal of the test is to verify that the BDP is NOT set - * if the RTT on the second connection does not match the RTT - * on the first one. The test does that, but only because the - * second connection's RTT is lower than BBRLongRttThreshold, - * thus uses regular BBR startup, in which the BDP option is - * not implemented. - */ - return bdp_option_test_one(bdp_test_option_rtt); -} - -int bdp_ip_test() -{ - return bdp_option_test_one(bdp_test_option_ip); -} - -int bdp_delay_test() -{ - return bdp_option_test_one(bdp_test_option_delay); -} - -int bdp_reno_test() -{ - return bdp_option_test_one(bdp_test_option_reno); -} - -int bdp_short_test() -{ - return bdp_option_test_one(bdp_test_option_short); -} -int bdp_short_hi_test() -{ - return bdp_option_test_one(bdp_test_option_short_hi); -} - -int bdp_short_lo_test() -{ - return bdp_option_test_one(bdp_test_option_short_lo); -} - -#if defined(_WINDOWS) && !defined(_WINDOWS64) -int bdp_cubic_test() -{ - /* We do not run this test in Win32 builds. */ - return 0; -} -#else -int bdp_cubic_test() -{ - return bdp_option_test_one(bdp_test_option_cubic); -} -#endif -#endif /* Test closing a connection with a specific error message. */ @@ -12214,5 +11937,84 @@ int immediate_ack_test() test_ctx = NULL; } + return ret; +} + +/* Testing the key logging facility. + */ +#define TEST_KEYLOG_FILE_CLIENT "test_keylog_client.txt" +#define TEST_KEYLOG_FILE_SERVER "test_keylog_server.txt" + +static void keylog_reset_file(char const* file_name) +{ + FILE* F = picoquic_file_open(file_name, "w"); + if (F != NULL) { + (void)picoquic_file_close(F); + } +} + +static size_t keylog_file_size(char const* file_name) +{ + size_t sz = 0; + FILE* F = picoquic_file_open(file_name, "r"); + if (F != NULL) { + fseek(F, 0, SEEK_END); + sz = ftell(F); + (void)picoquic_file_close(F); + } + return(sz); +} + +int keylog_test() +{ + uint64_t simulated_time = 0; + uint64_t loss_mask = 0; + picoquic_test_tls_api_ctx_t* test_ctx = NULL; + picoquic_connection_id_t initial_cid = { {0x55, 0x17, 0xe9, 0x10, 0x90, 0x0, 0x0, 0x0}, 8 }; + int ret; + + /* Ensure that the log files are empty */ + keylog_reset_file(TEST_KEYLOG_FILE_SERVER); + keylog_reset_file(TEST_KEYLOG_FILE_CLIENT); + + /* Create the contexts */ + ret = tls_api_init_ctx_ex(&test_ctx, PICOQUIC_INTERNAL_TEST_VERSION_1, + PICOQUIC_TEST_SNI, PICOQUIC_TEST_ALPN, &simulated_time, NULL, NULL, 0, 1, 0, &initial_cid); + + if (ret == 0) { + /* program key logging. */ + picoquic_enable_sslkeylog(test_ctx->qserver, 1); + picoquic_enable_sslkeylog(test_ctx->qclient, 1); + picoquic_set_key_log_file(test_ctx->qserver, TEST_KEYLOG_FILE_SERVER); + picoquic_set_key_log_file(test_ctx->qclient, TEST_KEYLOG_FILE_CLIENT); + if (!picoquic_is_sslkeylog_enabled(test_ctx->qserver) || + !picoquic_is_sslkeylog_enabled(test_ctx->qclient)) { + ret = -1; + } + } + if (ret == 0) { + /* Execute a small scenario to force complete exchange of keys */ + ret = tls_api_one_scenario_body(test_ctx, &simulated_time, + test_scenario_q_and_r, sizeof(test_scenario_q_and_r), 1000000, 0, 0, 20000, + 1200000); + + if (ret != 0) + { + DBG_PRINTF("Scenario body returns error %d\n", ret); + } + } + + if (test_ctx != NULL) { + tls_api_delete_ctx(test_ctx); + test_ctx = NULL; + } + + if (ret == 0) { + /* Verify that data was properly written in log files. */ + if (keylog_file_size(TEST_KEYLOG_FILE_SERVER) < 128 || + keylog_file_size(TEST_KEYLOG_FILE_CLIENT) < 128) { + ret = -1; + } + } return ret; } \ No newline at end of file From d10cbf4c0f9d77f8cae08e0bbab4fb320260af5b Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 26 Nov 2024 11:41:47 -0800 Subject: [PATCH 02/12] Remove picoquic_does_tls_ticket_allow_early_data --- picoquic/tls_api.c | 73 ------------------------------------- picoquictest/tls_api_test.c | 5 ++- 2 files changed, 3 insertions(+), 75 deletions(-) diff --git a/picoquic/tls_api.c b/picoquic/tls_api.c index 0bf6a194f..72d0f517a 100644 --- a/picoquic/tls_api.c +++ b/picoquic/tls_api.c @@ -1973,79 +1973,6 @@ void picoquic_set_key_log_file(picoquic_quic_t *quic, char const * keylog_filena ctx->log_event = (ptls_log_event_t*)log_event; } -/* -Check whether the ticket that was received, or used, authorizes 0-RTT data. - -From TLS 1.3 spec: -struct { -uint32 ticket_lifetime; -uint32 ticket_age_add; -opaque ticket_nonce<0..255>; -opaque ticket<1..2^16-1>; -Extension extensions<0..2^16-2>; -} NewSessionTicket; - -struct { -ExtensionType extension_type; -opaque extension_data<0..2^16-1>; -} Extension; -*/ - -int picoquic_does_tls_ticket_allow_early_data(uint8_t* ticket, uint16_t ticket_length) -{ - uint8_t nonce_length = 0; - uint16_t ticket_val_length = 0; - uint16_t extension_length = 0; - uint8_t* extension_ptr = NULL; - uint16_t byte_index = 0; - uint16_t min_length = 4 + 4 + 1 + 2 + 2; - int ret = 0; - - if (ticket_length >= min_length) { - byte_index += 4; /* Skip lifetime */ - byte_index += 4; /* Skip age add */ - nonce_length = ticket[byte_index++]; - min_length += nonce_length; - if (ticket_length >= min_length) { - byte_index += nonce_length; - - ticket_val_length = PICOPARSE_16(ticket + byte_index); - byte_index += 2; - min_length += ticket_val_length; - if (ticket_length >= min_length) { - byte_index += ticket_val_length; - - extension_length = PICOPARSE_16(ticket + byte_index); - byte_index += 2; - min_length += extension_length; - if (ticket_length >= min_length) { - extension_ptr = &ticket[byte_index]; - } - } - } - } - - if (extension_ptr != NULL) { - uint16_t x_index = 0; - - while (x_index + 4 < extension_length) { - uint16_t x_type = PICOPARSE_16(extension_ptr + x_index); - uint16_t x_len = PICOPARSE_16(extension_ptr + x_index + 2); - x_index += 4 + x_len; - - if (x_type == 42 && x_len == 4) { - uint32_t ed_len = PICOPARSE_32(extension_ptr + x_index - 4); - if (ed_len == 0xFFFFFFFF) { - ret = 1; - } - break; - } - } - } - - return ret; -} - /* * Creation of a TLS context. * This includes setting the handshake properties that will later be diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c index 89aa2b9b8..9d7c0b63b 100644 --- a/picoquictest/tls_api_test.c +++ b/picoquictest/tls_api_test.c @@ -11968,7 +11968,6 @@ static size_t keylog_file_size(char const* file_name) int keylog_test() { uint64_t simulated_time = 0; - uint64_t loss_mask = 0; picoquic_test_tls_api_ctx_t* test_ctx = NULL; picoquic_connection_id_t initial_cid = { {0x55, 0x17, 0xe9, 0x10, 0x90, 0x0, 0x0, 0x0}, 8 }; int ret; @@ -12017,4 +12016,6 @@ int keylog_test() } } return ret; -} \ No newline at end of file +} + +/* Test the */ \ No newline at end of file From d54a505a3250eb086683b321702fd0f479e3f7a6 Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 26 Nov 2024 13:05:02 -0800 Subject: [PATCH 03/12] Add test of get hash APIs --- UnitTest1/unittest1.cpp | 6 ++++ picoquic/tls_api.c | 5 ---- picoquic/tls_api.h | 2 -- picoquic_t/picoquic_t.c | 1 + picoquictest/picoquictest.h | 1 + picoquictest/tls_api_test.c | 56 ++++++++++++++++++++++++++++++++++++- 6 files changed, 63 insertions(+), 8 deletions(-) diff --git a/UnitTest1/unittest1.cpp b/UnitTest1/unittest1.cpp index c18e3e1f3..bd974792f 100644 --- a/UnitTest1/unittest1.cpp +++ b/UnitTest1/unittest1.cpp @@ -2988,6 +2988,12 @@ namespace UnitTest1 Assert::AreEqual(ret, 0); } + TEST_METHOD(get_hash) { + int ret = get_hash_test(); + + Assert::AreEqual(ret, 0); + } + TEST_METHOD(getter) { int ret = getter_test(); diff --git a/picoquic/tls_api.c b/picoquic/tls_api.c index 72d0f517a..42bd67739 100644 --- a/picoquic/tls_api.c +++ b/picoquic/tls_api.c @@ -448,11 +448,6 @@ ptls_hash_algorithm_t* picoquic_get_sha256() return picoquic_get_hash_algorithm_by_name("sha256"); } -void* picoquic_get_sha256_v() -{ - return (void*)picoquic_get_sha256(); -} - /* Export hash functions so applications do not need to access picotls. * It is not clear that these functions are actually used by applications. */ diff --git a/picoquic/tls_api.h b/picoquic/tls_api.h index e7c507b99..e2a952810 100644 --- a/picoquic/tls_api.h +++ b/picoquic/tls_api.h @@ -164,9 +164,7 @@ void picoquic_cid_free_encrypt_global_ctx(void ** v_cid_enc); /* Define hash functions here so applications don't need to directly interface picotls */ #define PICOQUIC_HASH_SIZE_MAX 64 void * picoquic_hash_create(char const * algorithm_name); -#if 0 size_t picoquic_hash_get_length(char const* algorithm_name); -#endif void picoquic_hash_update(uint8_t* input, size_t input_length, void* hash_context); void picoquic_hash_finalize(uint8_t* output, void* hash_context); diff --git a/picoquic_t/picoquic_t.c b/picoquic_t/picoquic_t.c index 46edeb3e8..14dc4689a 100644 --- a/picoquic_t/picoquic_t.c +++ b/picoquic_t/picoquic_t.c @@ -486,6 +486,7 @@ static const picoquic_test_def_t test_table[] = { { "multipath_tunnel", multipath_tunnel_test }, { "monopath_0rtt", monopath_0rtt_test }, { "monopath_0rtt_loss", monopath_0rtt_loss_test }, + { "get_hash", get_hash_test }, { "getter", getter_test }, { "grease_quic_bit", grease_quic_bit_test }, { "grease_quic_bit_one_way", grease_quic_bit_one_way_test }, diff --git a/picoquictest/picoquictest.h b/picoquictest/picoquictest.h index 1bae308b0..440f6b92c 100644 --- a/picoquictest/picoquictest.h +++ b/picoquictest/picoquictest.h @@ -481,6 +481,7 @@ int multipath_discovery_test(); int multipath_qlog_test(); int multipath_tunnel_test(); int token_reuse_api_test(); +int get_hash_test(); int getter_test(); int grease_quic_bit_test(); int grease_quic_bit_one_way_test(); diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c index 9d7c0b63b..0cff4f8f5 100644 --- a/picoquictest/tls_api_test.c +++ b/picoquictest/tls_api_test.c @@ -12018,4 +12018,58 @@ int keylog_test() return ret; } -/* Test the */ \ No newline at end of file +/* Test the get hash API */ + +int get_hash_length_test(char const * alg_name) +{ + int ret = 0; + size_t sz = picoquic_hash_get_length(alg_name); + if (sz == 0 || sz > 1024) { + ret = -1; + } + return ret; +} + +int get_hash_algo_test(char const* alg_name) +{ + int ret = 0; + uint8_t test[] = { 1, 2, 3, 4 }; + uint8_t outbuf[1024]; + uint8_t ref[16]; + void * h = picoquic_hash_create(alg_name); + if (h == NULL) { + ret = -1; + } + else { + memset(outbuf, 0, sizeof(outbuf)); + memset(ref, 0, sizeof(ref)); + picoquic_hash_update(test, sizeof(test), h); + picoquic_hash_finalize(outbuf, h); + if (memcmp(outbuf, ref, sizeof(ref)) == 0) { + ret = -1; + } + } + return ret; +} + +int get_hash_test() +{ + int ret = 0; + char const* valid_hash = "sha256"; + char const* invalid_hash = "no_such_hash_nada_niente"; + + picoquic_tls_api_init(); + if (get_hash_length_test(valid_hash) != 0) { + ret = -1; + } + else if (get_hash_length_test(invalid_hash) == 0) { + ret = -1; + } + else if (get_hash_algo_test(valid_hash) != 0) { + ret = -1; + } + else if (get_hash_algo_test(invalid_hash) == 0) { + ret = -1; + } + return (ret); +} \ No newline at end of file From 6c7e73fc63234e6744eb3dfadbc4caa7a5de2235 Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 26 Nov 2024 13:14:40 -0800 Subject: [PATCH 04/12] Disable unused crypto under mask code --- picoquic/tls_api.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/picoquic/tls_api.c b/picoquic/tls_api.c index 42bd67739..4b5dafb16 100644 --- a/picoquic/tls_api.c +++ b/picoquic/tls_api.c @@ -2884,6 +2884,8 @@ int picoquic_verify_retry_token(picoquic_quic_t* quic, const struct sockaddr * a return ret; } +#if 0 +/* Disabling this code for now, as it is not used */ /* * Encryption functions for CID encryption */ @@ -2955,6 +2957,7 @@ void picoquic_cid_decrypt_under_mask(void *cid_enc, const picoquic_connection_id { picoquic_cid_encrypt_under_mask(cid_enc, cid_in, mask, cid_out); } +#endif /* Retry Packet Protection. * This is done by applying AES-GCM128 with a constant key and a NULL nonce, From 885d6bd217b1487125e7f153d6c8d1981677c271 Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 26 Nov 2024 13:49:56 -0800 Subject: [PATCH 05/12] Add tls API error test --- UnitTest1/unittest1.cpp | 5 +++++ picoquic/tls_api.c | 8 ++++++-- picoquic/tls_api.h | 4 +++- picoquic_t/picoquic_t.c | 1 + picoquictest/picoquictest.h | 1 + picoquictest/tls_api_test.c | 32 ++++++++++++++++++++++++++++++++ 6 files changed, 48 insertions(+), 3 deletions(-) diff --git a/UnitTest1/unittest1.cpp b/UnitTest1/unittest1.cpp index bd974792f..7eeb0f8ce 100644 --- a/UnitTest1/unittest1.cpp +++ b/UnitTest1/unittest1.cpp @@ -2994,6 +2994,11 @@ namespace UnitTest1 Assert::AreEqual(ret, 0); } + TEST_METHOD(get_tls_errors) { + int ret = get_tls_errors_test(); + + Assert::AreEqual(ret, 0); + } TEST_METHOD(getter) { int ret = getter_test(); diff --git a/picoquic/tls_api.c b/picoquic/tls_api.c index 4b5dafb16..abb0bf346 100644 --- a/picoquic/tls_api.c +++ b/picoquic/tls_api.c @@ -412,10 +412,11 @@ static ptls_cipher_algorithm_t* picoquic_get_ecb_cipher_by_id(const char* ecb_ci * then derive the ECB function from the selection of the AEAD function. * This will obviate the need of providing a specific API. */ -void* picoquic_aes128_ecb_create(int is_enc, const void* ecb_key) + +void* picoquic_ecb_create_by_name(int is_enc, const void* ecb_key, char const* alg_name) { void* created = NULL; - ptls_cipher_algorithm_t* ecb_cipher = picoquic_get_ecb_cipher_by_id("AES128-ECB"); + ptls_cipher_algorithm_t* ecb_cipher = picoquic_get_ecb_cipher_by_id(alg_name); if (ecb_cipher != NULL) { created = (void*)ptls_cipher_new(ecb_cipher, is_enc, ecb_key); @@ -423,6 +424,9 @@ void* picoquic_aes128_ecb_create(int is_enc, const void* ecb_key) return created; } +void* picoquic_aes128_ecb_create(int is_enc, const void* ecb_key) { + return picoquic_ecb_create_by_name(is_enc, ecb_key, "AES128-ECB"); +} /* Obtain a hash algorithm from the table of supported cipher suites.*/ ptls_hash_algorithm_t* picoquic_get_hash_algorithm_by_name(const char* hash_algorithm_name) diff --git a/picoquic/tls_api.h b/picoquic/tls_api.h index e2a952810..ef51564b4 100644 --- a/picoquic/tls_api.h +++ b/picoquic/tls_api.h @@ -154,12 +154,13 @@ int picoquic_verify_retry_token(picoquic_quic_t* quic, const struct sockaddr * a const picoquic_connection_id_t* rcid, uint32_t initial_pn, const uint8_t * token, size_t token_size, int check_reuse); +#if 0 void picoquic_cid_free_under_mask_ctx(void * v_pn_enc); int picoquic_cid_get_under_mask_ctx(void ** v_pn_enc, const void * secret, const char *prefix_label); void picoquic_cid_encrypt_under_mask(void * cid_enc, const picoquic_connection_id_t * cid_in, const picoquic_connection_id_t * mask, picoquic_connection_id_t * cid_out); void picoquic_cid_decrypt_under_mask(void * cid_enc, const picoquic_connection_id_t * cid_in, const picoquic_connection_id_t * mask, picoquic_connection_id_t * cid_out); - void picoquic_cid_free_encrypt_global_ctx(void ** v_cid_enc); +#endif /* Define hash functions here so applications don't need to directly interface picotls */ #define PICOQUIC_HASH_SIZE_MAX 64 @@ -191,6 +192,7 @@ void* picoquic_get_aes128gcm_sha256_v(int use_low_memory); void* picoquic_get_aes128gcm_v(int use_low_memory); /* AES ECB function used for CID encryption */ +void* picoquic_ecb_create_by_name(int is_enc, const void* ecb_key, char const * alg_name); void* picoquic_aes128_ecb_create(int is_enc, const void* ecb_key); void picoquic_aes128_ecb_free(void* v_aesecb); diff --git a/picoquic_t/picoquic_t.c b/picoquic_t/picoquic_t.c index 14dc4689a..8493b2eba 100644 --- a/picoquic_t/picoquic_t.c +++ b/picoquic_t/picoquic_t.c @@ -487,6 +487,7 @@ static const picoquic_test_def_t test_table[] = { { "monopath_0rtt", monopath_0rtt_test }, { "monopath_0rtt_loss", monopath_0rtt_loss_test }, { "get_hash", get_hash_test }, + { "get_tls_errors", get_tls_errors_test }, { "getter", getter_test }, { "grease_quic_bit", grease_quic_bit_test }, { "grease_quic_bit_one_way", grease_quic_bit_one_way_test }, diff --git a/picoquictest/picoquictest.h b/picoquictest/picoquictest.h index 440f6b92c..b9fabc77e 100644 --- a/picoquictest/picoquictest.h +++ b/picoquictest/picoquictest.h @@ -482,6 +482,7 @@ int multipath_qlog_test(); int multipath_tunnel_test(); int token_reuse_api_test(); int get_hash_test(); +int get_tls_errors_test(); int getter_test(); int grease_quic_bit_test(); int grease_quic_bit_one_way_test(); diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c index 0cff4f8f5..482ea24d6 100644 --- a/picoquictest/tls_api_test.c +++ b/picoquictest/tls_api_test.c @@ -12072,4 +12072,36 @@ int get_hash_test() ret = -1; } return (ret); +} + +int get_tls_errors_test() +{ + int ret = 0; + uint8_t data[128]; + char const* invalid_stuff = "no_such_stuff_nada_niente"; + picoquic_test_tls_api_ctx_t* test_ctx = NULL; + uint64_t simulated_time = 0; + picoquic_connection_id_t initial_cid = { {0x9e, 0x71, 0x5e, 0, 0, 0, 0, 0}, 8 }; + int invalid_id = 0xFFFE8808; + + ret = tls_api_init_ctx_ex2(&test_ctx, PICOQUIC_INTERNAL_TEST_VERSION_1, + PICOQUIC_TEST_SNI, PICOQUIC_TEST_ALPN, &simulated_time, NULL, NULL, 0, 0, 0, &initial_cid, 8, 0, 0, 0); + + memset(data, 0xaa, sizeof(data)); + if (ret == 0 && picoquic_ecb_create_by_name(0, data, invalid_stuff) != NULL) { + ret = -1; + } + if (ret == 0 && picoquic_get_cipher_suite_by_id_v(invalid_id, 0) != NULL) { + ret = -1; + } + if (ret == 0 && picoquic_set_key_exchange(test_ctx->qserver, invalid_id) == 0) { + ret = -1; + } + + if (test_ctx != NULL) { + tls_api_delete_ctx(test_ctx); + test_ctx = NULL; + } + + return ret; } \ No newline at end of file From 4b5d296256fa3f234c85b6f7667072cfef2284c4 Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 26 Nov 2024 14:02:56 -0800 Subject: [PATCH 06/12] More TLS API tests --- picoquictest/tls_api_test.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c index 482ea24d6..f6b7373b8 100644 --- a/picoquictest/tls_api_test.c +++ b/picoquictest/tls_api_test.c @@ -12094,9 +12094,22 @@ int get_tls_errors_test() if (ret == 0 && picoquic_get_cipher_suite_by_id_v(invalid_id, 0) != NULL) { ret = -1; } + if (ret == 0 && picoquic_set_cipher_suite(test_ctx->qserver, invalid_id) == 0) { + ret = -1; + } if (ret == 0 && picoquic_set_key_exchange(test_ctx->qserver, invalid_id) == 0) { ret = -1; } + if (ret == 0 && + picoquic_get_cipher_suite_by_id_v(PICOQUIC_AES_128_GCM_SHA256, 1) == NULL && + picoquic_get_cipher_suite_by_id_v(PICOQUIC_AES_128_GCM_SHA256, 0) == NULL) { + ret = -1; + } + if (ret == 0 && + picoquic_get_cipher_suite_by_id_v(invalid_id, 1) != NULL) { + ret = -1; + } + if (test_ctx != NULL) { tls_api_delete_ctx(test_ctx); From 3230b5c4127a9f750880c02db4365076ec367d41 Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 26 Nov 2024 17:33:11 -0800 Subject: [PATCH 07/12] Better picoquic_tlscontext_create --- picoquic/tls_api.c | 51 +++++++++++++++++-------------------- picoquictest/tls_api_test.c | 1 + 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/picoquic/tls_api.c b/picoquic/tls_api.c index abb0bf346..b442f63e2 100644 --- a/picoquic/tls_api.c +++ b/picoquic/tls_api.c @@ -366,7 +366,7 @@ int picoquic_set_cipher_suite(picoquic_quic_t* quic, int cipher_suite_id) } /* Obtain AES128GCM SHA256, AES256GCM_SHA384 or CHACHA20 suite according to current provider */ -ptls_cipher_suite_t* picoquic_get_selected_cipher_suite_by_id(int cipher_suite_id, int use_low_memory) +ptls_cipher_suite_t* picoquic_get_cipher_suite_by_id(int cipher_suite_id, int use_low_memory) { ptls_cipher_suite_t* selected_suites[4]; ptls_cipher_suite_t* cipher; @@ -377,13 +377,8 @@ ptls_cipher_suite_t* picoquic_get_selected_cipher_suite_by_id(int cipher_suite_i else { cipher = selected_suites[0]; } - - return cipher; -} -static ptls_cipher_suite_t* picoquic_get_cipher_suite_by_id(int cipher_suite_id, int use_low_memory) -{ - return picoquic_get_selected_cipher_suite_by_id(cipher_suite_id, use_low_memory); + return cipher; } static ptls_cipher_algorithm_t* picoquic_get_ecb_cipher_by_id(const char* ecb_cipher_name) @@ -1156,7 +1151,8 @@ uint64_t picoquic_get_simulated_time_cb(ptls_get_time_t* self) * Verify certificate */ -int picoquic_enable_custom_verify_certificate_callback(picoquic_quic_t* quic) { +int picoquic_enable_custom_verify_certificate_callback(picoquic_quic_t* quic) +{ ptls_context_t* ctx = (ptls_context_t*)quic->tls_master_ctx; ctx->verify_certificate = quic->verify_certificate_callback; @@ -1837,13 +1833,22 @@ uint64_t picoquic_get_tls_time(picoquic_quic_t* quic) int picoquic_tlscontext_create(picoquic_quic_t* quic, picoquic_cnx_t* cnx, uint64_t current_time) { int ret = 0; - /* allocate a context structure */ - picoquic_tls_ctx_t* ctx = (picoquic_tls_ctx_t*)malloc(sizeof(picoquic_tls_ctx_t)); + /* allocate a context structure, but only if checks are correct */ + picoquic_tls_ctx_t* ctx = NULL; + + if (!cnx->client_mode && ((ptls_context_t*)quic->tls_master_ctx)->encrypt_ticket == NULL) { + /* A server side connection, but no cert/key where given for the master context */ + ret = PICOQUIC_ERROR_TLS_SERVER_CON_WITHOUT_CERT; + } + else { + ctx = (picoquic_tls_ctx_t*)malloc(sizeof(picoquic_tls_ctx_t)); + if (ctx == NULL) { + ret = PICOQUIC_ERROR_MEMORY; + } + } /* Create the TLS context */ - if (ctx == NULL) { - ret = -1; - } else { + if (ctx != NULL) { memset(ctx, 0, sizeof(picoquic_tls_ctx_t)); ctx->ext_data_size = PICOQUIC_TRANSPORT_PARAMETERS_MAX_SIZE; if (!cnx->client_mode && quic->test_large_server_flight) { @@ -1864,22 +1869,14 @@ int picoquic_tlscontext_create(picoquic_quic_t* quic, picoquic_cnx_t* cnx, uint6 ctx->tls = ptls_new((ptls_context_t*)quic->tls_master_ctx, (ctx->client_mode) ? 0 : 1); - *ptls_get_data_ptr(ctx->tls) = cnx; - if (ctx->tls == NULL) { - free(ctx); + picoquic_tlscontext_free(ctx); ctx = NULL; - ret = -1; + ret = PICOQUIC_ERROR_MEMORY; } - else if (!ctx->client_mode) { - /* A server side connection, but no cert/key where given for the master context */ - if (((ptls_context_t*)quic->tls_master_ctx)->encrypt_ticket == NULL) { - ret = PICOQUIC_ERROR_TLS_SERVER_CON_WITHOUT_CERT; - picoquic_tlscontext_free(ctx); - ctx = NULL; - } - - if (ctx != NULL) { + else{ + *ptls_get_data_ptr(ctx->tls) = cnx; + if (!ctx->client_mode) { /* The server should never attempt a stateless retry */ ctx->handshake_properties.server.enforce_retry = 0; ctx->handshake_properties.server.retry_uses_cookie = 0; @@ -1890,7 +1887,7 @@ int picoquic_tlscontext_create(picoquic_quic_t* quic, picoquic_cnx_t* cnx, uint6 } } } - + if (cnx->tls_ctx != NULL) { picoquic_tlscontext_free(cnx->tls_ctx); } diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c index f6b7373b8..15146dff9 100644 --- a/picoquictest/tls_api_test.c +++ b/picoquictest/tls_api_test.c @@ -12111,6 +12111,7 @@ int get_tls_errors_test() } + if (test_ctx != NULL) { tls_api_delete_ctx(test_ctx); test_ctx = NULL; From 67fe568fb686c533ddde15cd87cead4566a88be0 Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 26 Nov 2024 17:44:06 -0800 Subject: [PATCH 08/12] Exercize the custom cert setup --- picoquictest/tls_api_test.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c index 15146dff9..b7dbf017c 100644 --- a/picoquictest/tls_api_test.c +++ b/picoquictest/tls_api_test.c @@ -12110,7 +12110,9 @@ int get_tls_errors_test() ret = -1; } - + if (ret == 0) { + (void)picoquic_enable_custom_verify_certificate(test_ctx->qserver); + } if (test_ctx != NULL) { tls_api_delete_ctx(test_ctx); From d94a22a3a770096f538c0819590b953dcecfbd08 Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 26 Nov 2024 17:46:47 -0800 Subject: [PATCH 09/12] Disbale unused custom cert verifier code --- picoquic/tls_api.c | 5 +++-- picoquictest/tls_api_test.c | 4 ---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/picoquic/tls_api.c b/picoquic/tls_api.c index b442f63e2..cf4146270 100644 --- a/picoquic/tls_api.c +++ b/picoquic/tls_api.c @@ -1150,7 +1150,8 @@ uint64_t picoquic_get_simulated_time_cb(ptls_get_time_t* self) /* * Verify certificate */ - +#if 0 +/* The custom cert call is not used and not tested, so disabled for now. */ int picoquic_enable_custom_verify_certificate_callback(picoquic_quic_t* quic) { ptls_context_t* ctx = (ptls_context_t*)quic->tls_master_ctx; @@ -1159,7 +1160,7 @@ int picoquic_enable_custom_verify_certificate_callback(picoquic_quic_t* quic) quic->is_cert_store_not_empty = 1; return 0; } - +#endif void picoquic_dispose_verify_certificate_callback(picoquic_quic_t* quic) { ptls_context_t* ctx = (ptls_context_t*)quic->tls_master_ctx; diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c index b7dbf017c..4f13975c3 100644 --- a/picoquictest/tls_api_test.c +++ b/picoquictest/tls_api_test.c @@ -12110,10 +12110,6 @@ int get_tls_errors_test() ret = -1; } - if (ret == 0) { - (void)picoquic_enable_custom_verify_certificate(test_ctx->qserver); - } - if (test_ctx != NULL) { tls_api_delete_ctx(test_ctx); test_ctx = NULL; From 46e6444eff05f475d258d8af3d0346032efa321f Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 26 Nov 2024 20:16:48 -0800 Subject: [PATCH 10/12] Add forced TLS errors --- picoquictest/tls_api_test.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c index 4f13975c3..2809598f5 100644 --- a/picoquictest/tls_api_test.c +++ b/picoquictest/tls_api_test.c @@ -12109,6 +12109,29 @@ int get_tls_errors_test() picoquic_get_cipher_suite_by_id_v(invalid_id, 1) != NULL) { ret = -1; } + if (ret == 0) { + + } + if (ret == 0) { + /* Unload the TLS API to force internal errors. */ + picoquic_cnx_t* cnx; + picoquic_tls_api_unload(); + + cnx = picoquic_create_cnx(test_ctx->qclient, picoquic_null_connection_id, picoquic_null_connection_id, + (struct sockaddr*)&test_ctx->server_addr, simulated_time, PICOQUIC_INTERNAL_TEST_VERSION_1, + PICOQUIC_TEST_SNI, PICOQUIC_TEST_ALPN, 0); + if (cnx != NULL) { + ret = -1; + picoquic_delete_cnx(cnx); + } + if (ret == 0) { + if (picoquic_set_private_key_from_file(test_ctx->qclient, "some bad file name.not") == 0) { + ret = -1; + } + } + /* Reinit the TLS API */ + picoquic_tls_api_init(); + } if (test_ctx != NULL) { tls_api_delete_ctx(test_ctx); From 06e60370b6ee02e4455be780f6d1405081e0190f Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 26 Nov 2024 20:32:36 -0800 Subject: [PATCH 11/12] Test get cert failure --- picoquictest/tls_api_test.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/picoquictest/tls_api_test.c b/picoquictest/tls_api_test.c index 2809598f5..aa7ad213d 100644 --- a/picoquictest/tls_api_test.c +++ b/picoquictest/tls_api_test.c @@ -12129,6 +12129,19 @@ int get_tls_errors_test() ret = -1; } } + if (ret == 0) { + size_t count = 0; + ptls_iovec_t* certs = picoquic_get_certs_from_file("some bad file name.not", &count); + if (certs != NULL) { + ret = -1; + for (size_t i = 0; i < count; i++) { + free(certs[i].base); + } + free(certs); + } + } + + ptls_iovec_t* picoquic_get_certs_from_file(char const* file_name, size_t * count); /* Reinit the TLS API */ picoquic_tls_api_init(); } From 4a13afc0585f2ad9940091a2fd1904da479fda8f Mon Sep 17 00:00:00 2001 From: huitema Date: Tue, 26 Nov 2024 20:39:31 -0800 Subject: [PATCH 12/12] Version 1.1.28.5 --- CMakeLists.txt | 2 +- picoquic/picoquic.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b301cfdc5..7cfb14f54 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,7 @@ else() endif() project(picoquic - VERSION 1.1.28.4 + VERSION 1.1.28.5 DESCRIPTION "picoquic library" LANGUAGES C CXX) diff --git a/picoquic/picoquic.h b/picoquic/picoquic.h index 744c15e37..89dfa0a16 100644 --- a/picoquic/picoquic.h +++ b/picoquic/picoquic.h @@ -40,7 +40,7 @@ extern "C" { #endif -#define PICOQUIC_VERSION "1.1.28.4" +#define PICOQUIC_VERSION "1.1.28.5" #define PICOQUIC_ERROR_CLASS 0x400 #define PICOQUIC_ERROR_DUPLICATE (PICOQUIC_ERROR_CLASS + 1) #define PICOQUIC_ERROR_AEAD_CHECK (PICOQUIC_ERROR_CLASS + 3)