From 62987aa01fe22c3f95fbe5f7c517c62676853b66 Mon Sep 17 00:00:00 2001 From: winlin Date: Tue, 9 Mar 2021 22:12:23 +0800 Subject: [PATCH] DTLS: Refine retransmit between ClientHello and Certificate. --- trunk/src/app/srs_app_rtc_dtls.cpp | 35 +++-- trunk/src/utest/srs_utest_rtc.cpp | 239 ++++++----------------------- 2 files changed, 71 insertions(+), 203 deletions(-) diff --git a/trunk/src/app/srs_app_rtc_dtls.cpp b/trunk/src/app/srs_app_rtc_dtls.cpp index c4b5b4ffa..59b563f57 100644 --- a/trunk/src/app/srs_app_rtc_dtls.cpp +++ b/trunk/src/app/srs_app_rtc_dtls.cpp @@ -68,7 +68,7 @@ unsigned int dtls_timer_cb(SSL* dtls, unsigned int previous_us) // Never exceed the max timeout. timeout_us = srs_min(timeout_us, 30 * 1000 * 1000); // in us - srs_info("DTLS: ARQ timer cb timeout=%ums, previous=%ums", timeout_us, previous_us); + srs_info("DTLS: ARQ timer cb timeout=%ums, previous=%ums", timeout_us/1000, previous_us/1000); return timeout_us; } @@ -503,7 +503,7 @@ srs_error_t SrsDtlsImpl::do_on_dtls(char* data, int nb_data) // When already done, only for us, we still got message from client, // it might be our response is lost, or application data. if (handshake_done_for_us) { - srs_trace("DTLS: After done, got %d bytes", nb_data); + srs_info("DTLS: After done, got %d bytes", nb_data); } int r0 = 0; @@ -553,7 +553,7 @@ srs_error_t SrsDtlsImpl::do_on_dtls(char* data, int nb_data) int size = BIO_get_mem_data(bio_out, (char**)&data); // Logging when got SSL original data. - state_trace((uint8_t*)data, size, true, r0, r1, false); + state_trace((uint8_t*)data, size, false, r0, r1, false); if (size > 0 && (err = callback_->write_dtls_data(data, size)) != srs_success) { return srs_error_wrap(err, "dtls send size=%u, data=[%s]", size, @@ -578,6 +578,12 @@ srs_error_t SrsDtlsImpl::do_handshake() { srs_error_t err = srs_success; + // Done for use, ignore handshake packets. If need to ARQ the handshake packets, + // we should use SSL_read to handle it. + if (handshake_done_for_us) { + return err; + } + // Do handshake and get the result. int r0 = SSL_do_handshake(dtls); int r1 = SSL_get_error(dtls, r0); @@ -690,7 +696,7 @@ SrsDtlsClientImpl::SrsDtlsClientImpl(ISrsDtlsCallback* callback) : SrsDtlsImpl(c state_ = SrsDtlsStateInit; // the max dtls retry num is 12 in openssl. - arq_max_retry = 12 * 2; // ARQ for ClientHello and Certificate. + arq_max_retry = 12 * 2; // Max ARQ limit shared for ClientHello and Certificate. reset_timer_ = true; } @@ -745,15 +751,17 @@ srs_error_t SrsDtlsClientImpl::on_final_out_data(uint8_t* data, int size) // If we are sending client hello, change from init to new state. if (state_ == SrsDtlsStateInit && size > 14 && data[0] == 22 && data[13] == 1) { state_ = SrsDtlsStateClientHello; + return err; } - // If we are sending certificate, change from SrsDtlsStateServerHello to new state. - if (state_ == SrsDtlsStateServerHello && size > 14 && data[0] == 22 && data[13] == 11) { + // If we are sending certificate, change from SrsDtlsStateClientHello to new state. + if (state_ == SrsDtlsStateClientHello && size > 14 && data[0] == 22 && data[13] == 11) { state_ = SrsDtlsStateClientCertificate; // When we send out the certificate, we should reset the timer. reset_timer_ = true; srs_info("DTLS: Reset the timer for ServerHello"); + return err; } return err; @@ -850,11 +858,13 @@ srs_error_t SrsDtlsClientImpl::cycle() // There is timeout to wait, so we should wait, because there is no packet in openssl. if (timeout > 0) { - // Note that if we use very small timeout, say 10ms, the client might got two ClientHello, - // then it confused and send HelloVerifyRequest(3) to check it, this is not the efficiency - // way, so we limit the min timeout here to make it faster. - // TODO: FIXME: Config it. - srs_usleep(srs_max(50 * SRS_UTIME_MILLISECONDS, timeout)); + // Never wait too long, because we might need to retransmit other messages. + // For example, we have transmit 2 ClientHello as [50ms, 100ms] then we sleep(200ms), + // during this we reset the openssl timer to 50ms and need to retransmit Certificate, + // we still need to wait 200ms not 50ms. + timeout = srs_min(100 * SRS_UTIME_MILLISECONDS, timeout); + timeout = srs_max(50 * SRS_UTIME_MILLISECONDS, timeout); + srs_usleep(timeout); continue; } @@ -869,6 +879,9 @@ srs_error_t SrsDtlsClientImpl::cycle() // messages and returns 1. If too many timeouts had expired without progress or an error // occurs, it returns -1. r0 = DTLSv1_handle_timeout(dtls); r1 = SSL_get_error(dtls, r0); + if (r0 == 0) { + continue; // No timeout had expired. + } if (r0 != 1) { return srs_error_new(ERROR_RTC_DTLS, "ARQ r0=%d, r1=%d", r0, r1); } diff --git a/trunk/src/utest/srs_utest_rtc.cpp b/trunk/src/utest/srs_utest_rtc.cpp index bf839aca3..beabf73bf 100644 --- a/trunk/src/utest/srs_utest_rtc.cpp +++ b/trunk/src/utest/srs_utest_rtc.cpp @@ -871,9 +871,12 @@ srs_error_t MockDtlsCallback::cycle() } // Wait for mock io to done, try to switch to coroutine many times. -void mock_wait_dtls_io_done(int count = 100, int interval = 0) +void mock_wait_dtls_io_done(SrsDtlsImpl* client_impl, int count = 100, int interval = 0) { for (int i = 0; i < count; i++) { + if (client_impl) { + dynamic_cast(client_impl)->reset_timer_ = true; + } srs_usleep(interval * SRS_UTIME_MILLISECONDS); } } @@ -895,138 +898,6 @@ public: } }; -VOID TEST(KernelRTCTest, DTLSARQLimitTest) -{ - srs_error_t err = srs_success; - - // ClientHello lost, client retransmit the ClientHello. - if (true) { - MockDtlsCallback cio; SrsDtls client(&cio); - MockDtlsCallback sio; SrsDtls server(&sio); - MockBridgeDtlsIO b0(&cio, &server, NULL); MockBridgeDtlsIO b1(&sio, &client, NULL); - HELPER_EXPECT_SUCCESS(client.initialize("active", "dtls1.0")); - HELPER_EXPECT_SUCCESS(server.initialize("passive", "dtls1.0")); - - // Use very short interval for utest. - dynamic_cast(client.impl)->arq_interval = 1 * SRS_UTIME_MILLISECONDS; - HELPER_ARRAY_INIT(dynamic_cast(client.impl)->arq_to_ratios, 8, 1); - - // Lost 10 packets, total packets should be 9(max to 9). - // Note that only one server hello. - cio.nn_client_hello_lost = 10; - - HELPER_EXPECT_SUCCESS(client.start_active_handshake()); - mock_wait_dtls_io_done(10, 3); - - EXPECT_TRUE(sio.r0 == srs_success); - EXPECT_TRUE(cio.r0 == srs_success); - - EXPECT_FALSE(cio.done); - EXPECT_FALSE(sio.done); - - EXPECT_EQ(9, cio.nn_client_hello); - EXPECT_EQ(0, sio.nn_server_hello); - EXPECT_EQ(0, cio.nn_certificate); - EXPECT_EQ(0, sio.nn_new_session); - EXPECT_EQ(0, sio.nn_change_cipher); - } - - // Certificate lost, client retransmit the Certificate. - if (true) { - MockDtlsCallback cio; SrsDtls client(&cio); - MockDtlsCallback sio; SrsDtls server(&sio); - MockBridgeDtlsIO b0(&cio, &server, NULL); MockBridgeDtlsIO b1(&sio, &client, NULL); - HELPER_EXPECT_SUCCESS(client.initialize("active", "dtls1.0")); - HELPER_EXPECT_SUCCESS(server.initialize("passive", "dtls1.0")); - - // Use very short interval for utest. - dynamic_cast(client.impl)->arq_interval = 1 * SRS_UTIME_MILLISECONDS; - HELPER_ARRAY_INIT(dynamic_cast(client.impl)->arq_to_ratios, 8, 1); - - // Lost 10 packets, total packets should be 9(max to 9). - // Note that only one server NewSessionTicket. - cio.nn_certificate_lost = 10; - - HELPER_EXPECT_SUCCESS(client.start_active_handshake()); - mock_wait_dtls_io_done(10, 3); - - EXPECT_TRUE(sio.r0 == srs_success); - EXPECT_TRUE(cio.r0 == srs_success); - - EXPECT_FALSE(cio.done); - EXPECT_FALSE(sio.done); - - EXPECT_EQ(1, cio.nn_client_hello); - EXPECT_EQ(1, sio.nn_server_hello); - EXPECT_EQ(9, cio.nn_certificate); - EXPECT_EQ(0, sio.nn_new_session); - EXPECT_EQ(0, sio.nn_change_cipher); - } - - // ServerHello lost, client retransmit the ClientHello. - if (true) { - MockDtlsCallback cio; SrsDtls client(&cio); - MockDtlsCallback sio; SrsDtls server(&sio); - MockBridgeDtlsIO b0(&cio, &server, NULL); MockBridgeDtlsIO b1(&sio, &client, NULL); - HELPER_EXPECT_SUCCESS(client.initialize("active", "dtls1.0")); - HELPER_EXPECT_SUCCESS(server.initialize("passive", "dtls1.0")); - - // Use very short interval for utest. - dynamic_cast(client.impl)->arq_interval = 1 * SRS_UTIME_MILLISECONDS; - HELPER_ARRAY_INIT(dynamic_cast(client.impl)->arq_to_ratios, 8, 1); - - // Lost 10 packets, total packets should be 9(max to 9). - sio.nn_server_hello_lost = 10; - - HELPER_EXPECT_SUCCESS(client.start_active_handshake()); - mock_wait_dtls_io_done(10, 3); - - EXPECT_TRUE(sio.r0 == srs_success); - EXPECT_TRUE(cio.r0 == srs_success); - - EXPECT_FALSE(cio.done); - EXPECT_FALSE(sio.done); - - EXPECT_EQ(9, cio.nn_client_hello); - EXPECT_EQ(9, sio.nn_server_hello); - EXPECT_EQ(0, cio.nn_certificate); - EXPECT_EQ(0, sio.nn_new_session); - EXPECT_EQ(0, sio.nn_change_cipher); - } - - // NewSessionTicket lost, client retransmit the Certificate. - if (true) { - MockDtlsCallback cio; SrsDtls client(&cio); - MockDtlsCallback sio; SrsDtls server(&sio); - MockBridgeDtlsIO b0(&cio, &server, NULL); MockBridgeDtlsIO b1(&sio, &client, NULL); - HELPER_EXPECT_SUCCESS(client.initialize("active", "dtls1.0")); - HELPER_EXPECT_SUCCESS(server.initialize("passive", "dtls1.0")); - - // Use very short interval for utest. - dynamic_cast(client.impl)->arq_interval = 1 * SRS_UTIME_MILLISECONDS; - HELPER_ARRAY_INIT(dynamic_cast(client.impl)->arq_to_ratios, 8, 1); - - // Lost 10 packets, total packets should be 9(max to 9). - sio.nn_new_session_lost = 10; - - HELPER_EXPECT_SUCCESS(client.start_active_handshake()); - mock_wait_dtls_io_done(10, 3); - - EXPECT_TRUE(sio.r0 == srs_success); - EXPECT_TRUE(cio.r0 == srs_success); - - // Although the packet is lost, but it's done for server, and not done for client. - EXPECT_FALSE(cio.done); - EXPECT_TRUE(sio.done); - - EXPECT_EQ(1, cio.nn_client_hello); - EXPECT_EQ(1, sio.nn_server_hello); - EXPECT_EQ(9, cio.nn_certificate); - EXPECT_EQ(9, sio.nn_new_session); - EXPECT_EQ(0, sio.nn_change_cipher); - } -} - VOID TEST(KernelRTCTest, DTLSClientARQTest) { srs_error_t err = srs_success; @@ -1040,7 +911,7 @@ VOID TEST(KernelRTCTest, DTLSClientARQTest) HELPER_EXPECT_SUCCESS(server.initialize("passive", "dtls1.0")); HELPER_EXPECT_SUCCESS(client.start_active_handshake()); - mock_wait_dtls_io_done(30, 1); + mock_wait_dtls_io_done(client.impl, 15, 5); EXPECT_TRUE(sio.r0 == srs_success); EXPECT_TRUE(cio.r0 == srs_success); @@ -1050,8 +921,8 @@ VOID TEST(KernelRTCTest, DTLSClientARQTest) EXPECT_EQ(1, cio.nn_client_hello); EXPECT_EQ(1, sio.nn_server_hello); - EXPECT_TRUE(1 <= cio.nn_certificate); - EXPECT_TRUE(1 <= sio.nn_new_session); + EXPECT_EQ(1, cio.nn_certificate); + EXPECT_EQ(1, sio.nn_new_session); EXPECT_EQ(0, sio.nn_change_cipher); } @@ -1063,16 +934,12 @@ VOID TEST(KernelRTCTest, DTLSClientARQTest) HELPER_EXPECT_SUCCESS(client.initialize("active", "dtls1.0")); HELPER_EXPECT_SUCCESS(server.initialize("passive", "dtls1.0")); - // Use very short interval for utest. - dynamic_cast(client.impl)->arq_interval = 1 * SRS_UTIME_MILLISECONDS; - HELPER_ARRAY_INIT(dynamic_cast(client.impl)->arq_to_ratios, 8, 1); - // Lost 2 packets, total packets should be 3. // Note that only one server hello. - cio.nn_client_hello_lost = 2; + cio.nn_client_hello_lost = 1; HELPER_EXPECT_SUCCESS(client.start_active_handshake()); - mock_wait_dtls_io_done(10, 3); + mock_wait_dtls_io_done(client.impl, 15, 5); EXPECT_TRUE(sio.r0 == srs_success); EXPECT_TRUE(cio.r0 == srs_success); @@ -1080,10 +947,10 @@ VOID TEST(KernelRTCTest, DTLSClientARQTest) EXPECT_TRUE(cio.done); EXPECT_TRUE(sio.done); - EXPECT_TRUE(3 <= cio.nn_client_hello); - EXPECT_TRUE(1 <= sio.nn_server_hello); - EXPECT_TRUE(1 <= cio.nn_certificate); - EXPECT_TRUE(1 <= sio.nn_new_session); + EXPECT_EQ(2, cio.nn_client_hello); + EXPECT_EQ(1, sio.nn_server_hello); + EXPECT_EQ(1, cio.nn_certificate); + EXPECT_EQ(1, sio.nn_new_session); EXPECT_EQ(0, sio.nn_change_cipher); } @@ -1095,16 +962,12 @@ VOID TEST(KernelRTCTest, DTLSClientARQTest) HELPER_EXPECT_SUCCESS(client.initialize("active", "dtls1.0")); HELPER_EXPECT_SUCCESS(server.initialize("passive", "dtls1.0")); - // Use very short interval for utest. - dynamic_cast(client.impl)->arq_interval = 1 * SRS_UTIME_MILLISECONDS; - HELPER_ARRAY_INIT(dynamic_cast(client.impl)->arq_to_ratios, 8, 1); - // Lost 2 packets, total packets should be 3. // Note that only one server NewSessionTicket. - cio.nn_certificate_lost = 2; + cio.nn_certificate_lost = 1; HELPER_EXPECT_SUCCESS(client.start_active_handshake()); - mock_wait_dtls_io_done(10, 3); + mock_wait_dtls_io_done(client.impl, 15, 5); EXPECT_TRUE(sio.r0 == srs_success); EXPECT_TRUE(cio.r0 == srs_success); @@ -1113,9 +976,9 @@ VOID TEST(KernelRTCTest, DTLSClientARQTest) EXPECT_TRUE(sio.done); EXPECT_EQ(1, cio.nn_client_hello); - EXPECT_EQ(1, sio.nn_server_hello); - EXPECT_TRUE(3 <= cio.nn_certificate); - EXPECT_TRUE(1 <= sio.nn_new_session); + EXPECT_EQ(2, sio.nn_server_hello); + EXPECT_EQ(2, cio.nn_certificate); + EXPECT_EQ(0, sio.nn_new_session); EXPECT_EQ(0, sio.nn_change_cipher); } } @@ -1133,7 +996,7 @@ VOID TEST(KernelRTCTest, DTLSServerARQTest) HELPER_EXPECT_SUCCESS(server.initialize("passive", "dtls1.0")); HELPER_EXPECT_SUCCESS(client.start_active_handshake()); - mock_wait_dtls_io_done(30, 1); + mock_wait_dtls_io_done(client.impl, 15, 5); EXPECT_TRUE(sio.r0 == srs_success); EXPECT_TRUE(cio.r0 == srs_success); @@ -1143,8 +1006,8 @@ VOID TEST(KernelRTCTest, DTLSServerARQTest) EXPECT_EQ(1, cio.nn_client_hello); EXPECT_EQ(1, sio.nn_server_hello); - EXPECT_TRUE(1 <= cio.nn_certificate); - EXPECT_TRUE(1 <= sio.nn_new_session); + EXPECT_EQ(1, cio.nn_certificate); + EXPECT_EQ(1, sio.nn_new_session); EXPECT_EQ(0, sio.nn_change_cipher); } @@ -1156,15 +1019,11 @@ VOID TEST(KernelRTCTest, DTLSServerARQTest) HELPER_EXPECT_SUCCESS(client.initialize("active", "dtls1.0")); HELPER_EXPECT_SUCCESS(server.initialize("passive", "dtls1.0")); - // Use very short interval for utest. - dynamic_cast(client.impl)->arq_interval = 1 * SRS_UTIME_MILLISECONDS; - HELPER_ARRAY_INIT(dynamic_cast(client.impl)->arq_to_ratios, 8, 1); - // Lost 2 packets, total packets should be 3. - sio.nn_server_hello_lost = 2; + sio.nn_server_hello_lost = 1; HELPER_EXPECT_SUCCESS(client.start_active_handshake()); - mock_wait_dtls_io_done(10, 3); + mock_wait_dtls_io_done(client.impl, 15, 5); EXPECT_TRUE(sio.r0 == srs_success); EXPECT_TRUE(cio.r0 == srs_success); @@ -1172,10 +1031,10 @@ VOID TEST(KernelRTCTest, DTLSServerARQTest) EXPECT_TRUE(cio.done); EXPECT_TRUE(sio.done); - EXPECT_EQ(3, cio.nn_client_hello); - EXPECT_EQ(3, sio.nn_server_hello); - EXPECT_TRUE(1 <= cio.nn_certificate); - EXPECT_TRUE(1 <= sio.nn_new_session); + EXPECT_EQ(2, cio.nn_client_hello); + EXPECT_EQ(2, sio.nn_server_hello); + EXPECT_EQ(1, cio.nn_certificate); + EXPECT_EQ(1, sio.nn_new_session); EXPECT_EQ(0, sio.nn_change_cipher); } @@ -1187,15 +1046,11 @@ VOID TEST(KernelRTCTest, DTLSServerARQTest) HELPER_EXPECT_SUCCESS(client.initialize("active", "dtls1.0")); HELPER_EXPECT_SUCCESS(server.initialize("passive", "dtls1.0")); - // Use very short interval for utest. - dynamic_cast(client.impl)->arq_interval = 1 * SRS_UTIME_MILLISECONDS; - HELPER_ARRAY_INIT(dynamic_cast(client.impl)->arq_to_ratios, 8, 1); - // Lost 2 packets, total packets should be 3. - sio.nn_new_session_lost = 2; + sio.nn_new_session_lost = 1; HELPER_EXPECT_SUCCESS(client.start_active_handshake()); - mock_wait_dtls_io_done(10, 3); + mock_wait_dtls_io_done(client.impl, 15, 5); EXPECT_TRUE(sio.r0 == srs_success); EXPECT_TRUE(cio.r0 == srs_success); @@ -1205,8 +1060,8 @@ VOID TEST(KernelRTCTest, DTLSServerARQTest) EXPECT_EQ(1, cio.nn_client_hello); EXPECT_EQ(1, sio.nn_server_hello); - EXPECT_EQ(3, cio.nn_certificate); - EXPECT_EQ(3, sio.nn_new_session); + EXPECT_EQ(2, cio.nn_certificate); + EXPECT_EQ(2, sio.nn_new_session); EXPECT_EQ(0, sio.nn_change_cipher); } } @@ -1250,10 +1105,10 @@ VOID TEST(KernelRTCTest, DTLSClientFlowTest) {4, "auto", "dtls1.0", true, true, false, false}, // OK, Client: DTLS auto(v1.0 or v1.2), Server: DTLS v1.0 {5, "auto", "dtls1.2", true, true, false, false}, - // Fail, Client: DTLS v1.0, Server: DTLS v1.2 - {6, "dtls1.0", "dtls1.2", false, false, false, true}, - // Fail, Client: DTLS v1.2, Server: DTLS v1.0 - {7, "dtls1.2", "dtls1.0", false, false, true, false}, + // OK?, Client: DTLS v1.0, Server: DTLS v1.2 + {6, "dtls1.0", "dtls1.2", true, true, false, false}, + // OK?, Client: DTLS v1.2, Server: DTLS v1.0 + {7, "dtls1.2", "dtls1.0", true, true, false, false}, }; for (int i = 0; i < (int)(sizeof(cases) / sizeof(DTLSFlowCase)); i++) { @@ -1266,14 +1121,14 @@ VOID TEST(KernelRTCTest, DTLSClientFlowTest) HELPER_EXPECT_SUCCESS(server.initialize("passive", c.ServerVersion)) << c; HELPER_EXPECT_SUCCESS(client.start_active_handshake()) << c; - mock_wait_dtls_io_done(); + mock_wait_dtls_io_done(client.impl, 15, 5); // Note that the cio error is generated from server, vice versa. - EXPECT_EQ(c.ClientError, sio.r0 != srs_success) << c; - EXPECT_EQ(c.ServerError, cio.r0 != srs_success) << c; - EXPECT_EQ(c.ClientDone, cio.done) << c; EXPECT_EQ(c.ServerDone, sio.done) << c; + + EXPECT_EQ(c.ClientError, sio.r0 != srs_success) << c; + EXPECT_EQ(c.ServerError, cio.r0 != srs_success) << c; } } @@ -1294,10 +1149,10 @@ VOID TEST(KernelRTCTest, DTLSServerFlowTest) {4, "auto", "dtls1.0", true, true, false, false}, // OK, Client: DTLS auto(v1.0 or v1.2), Server: DTLS v1.0 {5, "auto", "dtls1.2", true, true, false, false}, - // Fail, Client: DTLS v1.0, Server: DTLS v1.2 - {6, "dtls1.0", "dtls1.2", false, false, false, true}, - // Fail, Client: DTLS v1.2, Server: DTLS v1.0 - {7, "dtls1.2", "dtls1.0", false, false, true, false}, + // OK?, Client: DTLS v1.0, Server: DTLS v1.2 + {6, "dtls1.0", "dtls1.2", true, true, false, false}, + // OK?, Client: DTLS v1.2, Server: DTLS v1.0 + {7, "dtls1.2", "dtls1.0", true, true, false, false}, }; for (int i = 0; i < (int)(sizeof(cases) / sizeof(DTLSFlowCase)); i++) { @@ -1310,14 +1165,14 @@ VOID TEST(KernelRTCTest, DTLSServerFlowTest) HELPER_EXPECT_SUCCESS(server.initialize("passive", c.ServerVersion)) << c; HELPER_EXPECT_SUCCESS(client.start_active_handshake()) << c; - mock_wait_dtls_io_done(); + mock_wait_dtls_io_done(NULL, 15, 5); // Note that the cio error is generated from server, vice versa. - EXPECT_EQ(c.ClientError, sio.r0 != srs_success) << c; - EXPECT_EQ(c.ServerError, cio.r0 != srs_success) << c; - EXPECT_EQ(c.ClientDone, cio.done) << c; EXPECT_EQ(c.ServerDone, sio.done) << c; + + EXPECT_EQ(c.ClientError, sio.r0 != srs_success) << c; + EXPECT_EQ(c.ServerError, cio.r0 != srs_success) << c; } }