diff --git a/webrtc/base/gunit.h b/webrtc/base/gunit.h index 5ba0bd916e..ad52a43c82 100644 --- a/webrtc/base/gunit.h +++ b/webrtc/base/gunit.h @@ -21,25 +21,25 @@ #endif // Wait until "ex" is true, or "timeout" expires. -#define WAIT(ex, timeout) \ - for (int64_t start = rtc::TimeMillis(); \ - !(ex) && rtc::TimeMillis() < start + timeout;) { \ - rtc::Thread::Current()->ProcessMessages(0); \ - rtc::Thread::Current()->SleepMs(1); \ +#define WAIT(ex, timeout) \ + for (int64_t start = rtc::SystemTimeMillis(); \ + !(ex) && rtc::SystemTimeMillis() < start + timeout;) { \ + rtc::Thread::Current()->ProcessMessages(0); \ + rtc::Thread::Current()->SleepMs(1); \ } // This returns the result of the test in res, so that we don't re-evaluate // the expression in the XXXX_WAIT macros below, since that causes problems // when the expression is only true the first time you check it. -#define WAIT_(ex, timeout, res) \ - do { \ - int64_t start = rtc::TimeMillis(); \ - res = (ex); \ - while (!res && rtc::TimeMillis() < start + timeout) { \ - rtc::Thread::Current()->ProcessMessages(0); \ - rtc::Thread::Current()->SleepMs(1); \ - res = (ex); \ - } \ +#define WAIT_(ex, timeout, res) \ + do { \ + int64_t start = rtc::SystemTimeMillis(); \ + res = (ex); \ + while (!res && rtc::SystemTimeMillis() < start + timeout) { \ + rtc::Thread::Current()->ProcessMessages(0); \ + rtc::Thread::Current()->SleepMs(1); \ + res = (ex); \ + } \ } while (0) // The typical EXPECT_XXXX and ASSERT_XXXXs, but done until true or a timeout. diff --git a/webrtc/base/logging.cc b/webrtc/base/logging.cc index 60603624a5..c951e156d8 100644 --- a/webrtc/base/logging.cc +++ b/webrtc/base/logging.cc @@ -124,7 +124,9 @@ LogMessage::LogMessage(const char* file, const char* module) : severity_(sev), tag_(kLibjingle) { if (timestamp_) { - int64_t time = TimeSince(LogStartTime()); + // Use SystemTimeMillis so that even if tests use fake clocks, the timestamp + // in log messages represents the real system time. + int64_t time = TimeDiff(SystemTimeMillis(), LogStartTime()); // Also ensure WallClockStartTime is initialized, so that it matches // LogStartTime. WallClockStartTime(); @@ -210,7 +212,7 @@ LogMessage::~LogMessage() { } int64_t LogMessage::LogStartTime() { - static const int64_t g_start = TimeMillis(); + static const int64_t g_start = SystemTimeMillis(); return g_start; } diff --git a/webrtc/base/opensslstreamadapter.cc b/webrtc/base/opensslstreamadapter.cc index 1d2d03aa02..fa558fad37 100644 --- a/webrtc/base/opensslstreamadapter.cc +++ b/webrtc/base/opensslstreamadapter.cc @@ -34,6 +34,7 @@ #include "webrtc/base/openssldigest.h" #include "webrtc/base/opensslidentity.h" #include "webrtc/base/stringutils.h" +#include "webrtc/base/timeutils.h" #include "webrtc/base/thread.h" namespace rtc { @@ -58,7 +59,13 @@ static SrtpCipherMapEntry SrtpCipherMap[] = { {nullptr, 0}}; #endif -#ifndef OPENSSL_IS_BORINGSSL +#ifdef OPENSSL_IS_BORINGSSL +static void TimeCallback(const SSL* ssl, struct timeval* out_clock) { + uint64_t time = TimeNanos(); + out_clock->tv_sec = time / kNumNanosecsPerSec; + out_clock->tv_usec = time / kNumNanosecsPerMicrosec; +} +#else // #ifdef OPENSSL_IS_BORINGSSL // Cipher name table. Maps internal OpenSSL cipher ids to the RFC name. struct SslCipherMapEntry { @@ -771,13 +778,19 @@ int OpenSSLStreamAdapter::BeginSSL() { SSL_set_app_data(ssl_, this); SSL_set_bio(ssl_, bio, bio); // the SSL object owns the bio now. -#ifndef OPENSSL_IS_BORINGSSL if (ssl_mode_ == SSL_MODE_DTLS) { +#ifdef OPENSSL_IS_BORINGSSL + // Change the initial retransmission timer from 1 second to 50ms. + // This will likely result in some spurious retransmissions, but + // it's useful for ensuring a timely handshake when there's packet + // loss. + DTLSv1_set_initial_timeout_duration(ssl_, 50); +#else // Enable read-ahead for DTLS so whole packets are read from internal BIO // before parsing. This is done internally by BoringSSL for DTLS. SSL_set_read_ahead(ssl_, 1); - } #endif + } SSL_set_mode(ssl_, SSL_MODE_ENABLE_PARTIAL_WRITE | SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER); @@ -985,6 +998,11 @@ SSL_CTX* OpenSSLStreamAdapter::SetupSSLContext() { DTLS1_2_VERSION : TLS1_2_VERSION); break; } + // Set a time callback for BoringSSL because: + // 1. Our time function is more accurate (doesn't just use gettimeofday). + // 2. This allows us to inject a fake clock for testing. + // SSL_CTX_set_current_time_cb(ctx, &TimeCallback); + ctx->current_time_cb = &TimeCallback; #endif if (identity_ && !identity_->ConfigureIdentity(ctx)) { @@ -1127,6 +1145,14 @@ bool OpenSSLStreamAdapter::HaveExporter() { #endif } +bool OpenSSLStreamAdapter::IsBoringSsl() { +#ifdef OPENSSL_IS_BORINGSSL + return true; +#else + return false; +#endif +} + #define CDEF(X) \ { static_cast(TLS1_CK_##X & 0xffff), "TLS_" #X } diff --git a/webrtc/base/opensslstreamadapter.h b/webrtc/base/opensslstreamadapter.h index 1e90bacc0e..05e8102169 100644 --- a/webrtc/base/opensslstreamadapter.h +++ b/webrtc/base/opensslstreamadapter.h @@ -111,6 +111,7 @@ class OpenSSLStreamAdapter : public SSLStreamAdapter { static bool HaveDtls(); static bool HaveDtlsSrtp(); static bool HaveExporter(); + static bool IsBoringSsl(); static bool IsAcceptableCipher(int cipher, KeyType key_type); static bool IsAcceptableCipher(const std::string& cipher, KeyType key_type); diff --git a/webrtc/base/sslstreamadapter.cc b/webrtc/base/sslstreamadapter.cc index e0fce3ad2f..44158d401f 100644 --- a/webrtc/base/sslstreamadapter.cc +++ b/webrtc/base/sslstreamadapter.cc @@ -82,6 +82,9 @@ bool SSLStreamAdapter::HaveDtlsSrtp() { bool SSLStreamAdapter::HaveExporter() { return OpenSSLStreamAdapter::HaveExporter(); } +bool SSLStreamAdapter::IsBoringSsl() { + return OpenSSLStreamAdapter::IsBoringSsl(); +} bool SSLStreamAdapter::IsAcceptableCipher(int cipher, KeyType key_type) { return OpenSSLStreamAdapter::IsAcceptableCipher(cipher, key_type); } diff --git a/webrtc/base/sslstreamadapter.h b/webrtc/base/sslstreamadapter.h index c5045f184f..ba60ce3da0 100644 --- a/webrtc/base/sslstreamadapter.h +++ b/webrtc/base/sslstreamadapter.h @@ -195,6 +195,7 @@ class SSLStreamAdapter : public StreamAdapterInterface { static bool HaveDtls(); static bool HaveDtlsSrtp(); static bool HaveExporter(); + static bool IsBoringSsl(); // Returns true iff the supplied cipher is deemed to be strong. // TODO(torbjorng): Consider removing the KeyType argument. diff --git a/webrtc/base/timeutils.cc b/webrtc/base/timeutils.cc index 3c89d808b3..1be368eb58 100644 --- a/webrtc/base/timeutils.cc +++ b/webrtc/base/timeutils.cc @@ -38,10 +38,7 @@ ClockInterface* SetClockForTesting(ClockInterface* clock) { return prev; } -uint64_t TimeNanos() { - if (g_clock) { - return g_clock->TimeNanos(); - } +uint64_t SystemTimeNanos() { int64_t ticks; #if defined(WEBRTC_MAC) static mach_timebase_info_data_t timebase; @@ -86,6 +83,17 @@ uint64_t TimeNanos() { return ticks; } +int64_t SystemTimeMillis() { + return static_cast(SystemTimeNanos() / kNumNanosecsPerMillisec); +} + +uint64_t TimeNanos() { + if (g_clock) { + return g_clock->TimeNanos(); + } + return SystemTimeNanos(); +} + uint32_t Time32() { return static_cast(TimeNanos() / kNumNanosecsPerMillisec); } diff --git a/webrtc/base/timeutils.h b/webrtc/base/timeutils.h index 113793ade3..25ef52105c 100644 --- a/webrtc/base/timeutils.h +++ b/webrtc/base/timeutils.h @@ -53,6 +53,11 @@ class ClockInterface { // that uses it, eliminating the need for a global variable and this function. ClockInterface* SetClockForTesting(ClockInterface* clock); +// Returns the actual system time, even if a clock is set for testing. +// Useful for timeouts while using a test clock, or for logging. +uint64_t SystemTimeNanos(); +int64_t SystemTimeMillis(); + // Returns the current time in milliseconds in 32 bits. uint32_t Time32(); diff --git a/webrtc/p2p/base/dtlstransportchannel_unittest.cc b/webrtc/p2p/base/dtlstransportchannel_unittest.cc index 486b51aec9..705df2d95f 100644 --- a/webrtc/p2p/base/dtlstransportchannel_unittest.cc +++ b/webrtc/p2p/base/dtlstransportchannel_unittest.cc @@ -22,10 +22,10 @@ #include "webrtc/base/sslstreamadapter.h" #include "webrtc/base/stringutils.h" -#define MAYBE_SKIP_TEST(feature) \ - if (!(rtc::SSLStreamAdapter::feature())) { \ - LOG(LS_INFO) << "Feature disabled... skipping"; \ - return; \ +#define MAYBE_SKIP_TEST(feature) \ + if (!(rtc::SSLStreamAdapter::feature())) { \ + LOG(LS_INFO) << #feature " feature disabled... skipping"; \ + return; \ } static const char kIceUfrag1[] = "TESTICEUFRAG0001"; @@ -413,7 +413,8 @@ class DtlsTestClient : public sigslot::has_slots<> { rtc::SentPacket sent_packet_; }; - +// Note that this test always uses a FakeClock, due to the |fake_clock_| member +// variable. class DtlsTransportChannelTest : public testing::Test { public: DtlsTransportChannelTest() @@ -561,6 +562,7 @@ class DtlsTransportChannelTest : public testing::Test { } protected: + rtc::ScopedFakeClock fake_clock_; DtlsTestClient client1_; DtlsTestClient client2_; int channel_ct_; @@ -974,3 +976,45 @@ TEST_F(DtlsTransportChannelTest, kTimeout); EXPECT_EQ(1, client1_.received_dtls_client_hellos()); } + +// Test that packets are retransmitted according to the expected schedule. +// Each time a timeout occurs, the retransmission timer should be doubled up to +// 60 seconds. The timer defaults to 1 second, but for WebRTC we should be +// initializing it to 50ms. +TEST_F(DtlsTransportChannelTest, TestRetransmissionSchedule) { + MAYBE_SKIP_TEST(HaveDtls); + // We can only change the retransmission schedule with a recently-added + // BoringSSL API. Skip the test if not built with BoringSSL. + MAYBE_SKIP_TEST(IsBoringSsl); + + PrepareDtls(true, true, rtc::KT_DEFAULT); + // Exchange transport descriptions. + Negotiate(cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTIVE); + + // Make client2_ writable, but not client1_. + // This means client1_ will send DTLS client hellos but get no response. + EXPECT_TRUE(client2_.Connect(&client1_, true)); + EXPECT_TRUE_WAIT(client2_.all_raw_channels_writable(), kTimeout); + + // Wait for the first client hello to be sent. + EXPECT_EQ_WAIT(1, client1_.received_dtls_client_hellos(), kTimeout); + EXPECT_FALSE(client1_.all_raw_channels_writable()); + + static int timeout_schedule_ms[] = {50, 100, 200, 400, 800, 1600, + 3200, 6400, 12800, 25600, 51200, 60000}; + + int expected_hellos = 1; + for (size_t i = 0; + i < (sizeof(timeout_schedule_ms) / sizeof(timeout_schedule_ms[0])); + ++i) { + // For each expected retransmission time, advance the fake clock a + // millisecond before the expected time and verify that no unexpected + // retransmissions were sent. Then advance it the final millisecond and + // verify that the expected retransmission was sent. + fake_clock_.AdvanceTime( + rtc::TimeDelta::FromMilliseconds(timeout_schedule_ms[i] - 1)); + EXPECT_EQ(expected_hellos, client1_.received_dtls_client_hellos()); + fake_clock_.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1)); + EXPECT_EQ(++expected_hellos, client1_.received_dtls_client_hellos()); + } +}