diff --git a/p2p/base/dtlstransport.cc b/p2p/base/dtlstransport.cc index 0520a8517b..179b032adc 100644 --- a/p2p/base/dtlstransport.cc +++ b/p2p/base/dtlstransport.cc @@ -240,6 +240,10 @@ bool DtlsTransport::SetRemoteFingerprint(const std::string& digest_alg, } // If the other side doesn't support DTLS, turn off |dtls_active_|. + // TODO(deadbeef): Remove this. It's dangerous, because it relies on higher + // level code to ensure DTLS is actually used, but there are tests that + // depend on it, for the case where an m= section is rejected. In that case + // SetRemoteFingerprint shouldn't even be called though. if (digest_alg.empty()) { RTC_DCHECK(!digest_len); LOG_J(LS_INFO, this) << "Other side didn't support DTLS."; diff --git a/p2p/base/dtlstransport.h b/p2p/base/dtlstransport.h index 4dd9bc257d..e7b0c5654a 100644 --- a/p2p/base/dtlstransport.h +++ b/p2p/base/dtlstransport.h @@ -98,14 +98,24 @@ class DtlsTransport : public DtlsTransportInternal { const std::string& transport_name() const override; int component() const override; - // Returns false if no local certificate was set, or if the peer doesn't - // support DTLS. + // DTLS is active if a local certificate was set. Otherwise this acts in a + // "passthrough" mode, sending packets directly through the underlying ICE + // transport. + // TODO(deadbeef): Remove this weirdness, and handle it in the upper layers. bool IsDtlsActive() const override; + // SetLocalCertificate is what makes DTLS active. It must be called before + // SetRemoteFinterprint. + // TODO(deadbeef): Once DtlsTransport no longer has the concept of being + // "active" or not (acting as a passthrough if not active), just require this + // certificate on construction or "Start". bool SetLocalCertificate( const rtc::scoped_refptr& certificate) override; rtc::scoped_refptr GetLocalCertificate() const override; + // SetRemoteFingerprint must be called after SetLocalCertificate, and any + // other methods like SetSslRole. It's what triggers the actual DTLS setup. + // TODO(deadbeef): Rename to "Start" like in ORTC? bool SetRemoteFingerprint(const std::string& digest_alg, const uint8_t* digest, size_t digest_len) override; diff --git a/p2p/base/dtlstransport_unittest.cc b/p2p/base/dtlstransport_unittest.cc index 27e12a4d2a..cac78647d6 100644 --- a/p2p/base/dtlstransport_unittest.cc +++ b/p2p/base/dtlstransport_unittest.cc @@ -30,8 +30,8 @@ return; \ } -static const char kIceUfrag1[] = "TESTICEUFRAG0001"; -static const char kIcePwd1[] = "TESTICEPWD00000000000001"; +namespace cricket { + static const size_t kPacketNumOffset = 8; static const size_t kPacketHeaderLen = 12; static const int kFakePacketId = 0x1234; @@ -41,33 +41,25 @@ static bool IsRtpLeadByte(uint8_t b) { return ((b & 0xC0) == 0x80); } -cricket::TransportDescription MakeTransportDescription( +// |modify_digest| is used to set modified fingerprints that are meant to fail +// validation. +void SetRemoteFingerprintFromCert( + DtlsTransport* transport, const rtc::scoped_refptr& cert, - cricket::ConnectionRole role) { - std::unique_ptr fingerprint; - if (cert) { - std::string digest_algorithm; - EXPECT_TRUE( - cert->ssl_certificate().GetSignatureDigestAlgorithm(&digest_algorithm)); - EXPECT_FALSE(digest_algorithm.empty()); - fingerprint.reset( - rtc::SSLFingerprint::Create(digest_algorithm, cert->identity())); - EXPECT_TRUE(fingerprint.get() != NULL); - EXPECT_EQ(rtc::DIGEST_SHA_256, digest_algorithm); + bool modify_digest = false) { + rtc::SSLFingerprint* fingerprint = + rtc::SSLFingerprint::CreateFromCertificate(cert); + if (modify_digest) { + ++fingerprint->digest[0]; } - return cricket::TransportDescription(std::vector(), kIceUfrag1, - kIcePwd1, cricket::ICEMODE_FULL, role, - fingerprint.get()); + // Even if digest is verified to be incorrect, should fail asynchrnously. + EXPECT_TRUE(transport->SetRemoteFingerprint( + fingerprint->algorithm, + reinterpret_cast(fingerprint->digest.data()), + fingerprint->digest.size())); + delete fingerprint; } -using cricket::ConnectionRole; -using webrtc::SdpType; - -enum Flags { NF_REOFFER = 0x1, NF_EXPECT_FAILURE = 0x2 }; - -// TODO(deadbeef): Remove the dependency on JsepTransport. This test should be -// testing DtlsTransportChannel by itself, calling methods to set the -// configuration directly instead of negotiating TransportDescriptions. class DtlsTestClient : public sigslot::has_slots<> { public: explicit DtlsTestClient(const std::string& name) : name_(name) {} @@ -82,148 +74,38 @@ class DtlsTestClient : public sigslot::has_slots<> { void SetupMaxProtocolVersion(rtc::SSLProtocolVersion version) { ssl_max_version_ = version; } - void SetupChannels(int count, cricket::IceRole role, int async_delay_ms = 0) { - transport_.reset( - new cricket::JsepTransport("dtls content name", certificate_)); - for (int i = 0; i < count; ++i) { - cricket::FakeIceTransport* fake_ice_channel = - new cricket::FakeIceTransport(transport_->mid(), i); - fake_ice_channel->SetAsync(true); - fake_ice_channel->SetAsyncDelay(async_delay_ms); - // Hook the raw packets so that we can verify they are encrypted. - fake_ice_channel->SignalReadPacket.connect( - this, &DtlsTestClient::OnFakeTransportChannelReadPacket); + // Set up fake ICE transport and real DTLS transport under test. + void SetupTransports(IceRole role, int async_delay_ms = 0) { + fake_ice_transport_.reset(new FakeIceTransport("fake", 0)); + fake_ice_transport_->SetAsync(true); + fake_ice_transport_->SetAsyncDelay(async_delay_ms); + fake_ice_transport_->SetIceRole(role); + fake_ice_transport_->SetIceTiebreaker((role == ICEROLE_CONTROLLING) ? 1 + : 2); + // Hook the raw packets so that we can verify they are encrypted. + fake_ice_transport_->SignalReadPacket.connect( + this, &DtlsTestClient::OnFakeIceTransportReadPacket); - cricket::DtlsTransport* dtls = - new cricket::DtlsTransport(fake_ice_channel, rtc::CryptoOptions()); - dtls->SetLocalCertificate(certificate_); - dtls->ice_transport()->SetIceRole(role); - dtls->ice_transport()->SetIceTiebreaker( - (role == cricket::ICEROLE_CONTROLLING) ? 1 : 2); - dtls->SetSslMaxProtocolVersion(ssl_max_version_); - dtls->SignalWritableState.connect( - this, &DtlsTestClient::OnTransportWritableState); - dtls->SignalReadPacket.connect(this, - &DtlsTestClient::OnTransportReadPacket); - dtls->SignalSentPacket.connect(this, - &DtlsTestClient::OnTransportSentPacket); - dtls_transports_.push_back(std::unique_ptr(dtls)); - fake_ice_transports_.push_back( - std::unique_ptr(fake_ice_channel)); - transport_->AddChannel(dtls, i); - } + dtls_transport_.reset( + new DtlsTransport(fake_ice_transport_.get(), rtc::CryptoOptions())); + dtls_transport_->SetSslMaxProtocolVersion(ssl_max_version_); + // Note: Certificate may be null here if testing passthrough. + dtls_transport_->SetLocalCertificate(certificate_); + dtls_transport_->SignalWritableState.connect( + this, &DtlsTestClient::OnTransportWritableState); + dtls_transport_->SignalReadPacket.connect( + this, &DtlsTestClient::OnTransportReadPacket); + dtls_transport_->SignalSentPacket.connect( + this, &DtlsTestClient::OnTransportSentPacket); } - cricket::JsepTransport* transport() { return transport_.get(); } + FakeIceTransport* fake_ice_transport() { return fake_ice_transport_.get(); } - cricket::FakeIceTransport* GetFakeIceTransort(int component) { - for (const auto& ch : fake_ice_transports_) { - if (ch->component() == component) { - return ch.get(); - } - } - return nullptr; - } - - cricket::DtlsTransport* GetDtlsTransport(int component) { - for (const auto& dtls : dtls_transports_) { - if (dtls->component() == component) { - return dtls.get(); - } - } - return nullptr; - } - - // Offer DTLS if we have an identity; pass in a remote fingerprint only if - // both sides support DTLS. - void Negotiate(DtlsTestClient* peer, - SdpType action, - ConnectionRole local_role, - ConnectionRole remote_role, - int flags) { - Negotiate(certificate_, certificate_ ? peer->certificate_ : nullptr, action, - local_role, remote_role, flags); - } - - void SetLocalTransportDescription( - const rtc::scoped_refptr& cert, - SdpType action, - ConnectionRole role, - int flags) { - // If |NF_EXPECT_FAILURE| is set, expect SRTD or SLTD to fail when - // content action is CA_ANSWER. - bool expect_success = - !((action == SdpType::kAnswer) && (flags & NF_EXPECT_FAILURE)); - EXPECT_EQ(expect_success, - transport_->SetLocalTransportDescription( - MakeTransportDescription(cert, role), action, nullptr)); - } - - void SetRemoteTransportDescription( - const rtc::scoped_refptr& cert, - SdpType action, - ConnectionRole role, - int flags) { - // If |NF_EXPECT_FAILURE| is set, expect SRTD or SLTD to fail when - // content action is CA_ANSWER. - bool expect_success = - !((action == SdpType::kAnswer) && (flags & NF_EXPECT_FAILURE)); - EXPECT_EQ(expect_success, - transport_->SetRemoteTransportDescription( - MakeTransportDescription(cert, role), action, nullptr)); - } - - // Allow any DTLS configuration to be specified (including invalid ones). - void Negotiate(const rtc::scoped_refptr& local_cert, - const rtc::scoped_refptr& remote_cert, - SdpType action, - ConnectionRole local_role, - ConnectionRole remote_role, - int flags) { - if (action == SdpType::kOffer) { - SetLocalTransportDescription(local_cert, SdpType::kOffer, local_role, - flags); - SetRemoteTransportDescription(remote_cert, SdpType::kAnswer, remote_role, - flags); - } else { - SetRemoteTransportDescription(remote_cert, SdpType::kOffer, remote_role, - flags); - // If remote if the offerer and has no DTLS support, answer will be - // without any fingerprint. - SetLocalTransportDescription(remote_cert ? local_cert : nullptr, - SdpType::kAnswer, local_role, flags); - } - } + DtlsTransport* dtls_transport() { return dtls_transport_.get(); } + // Simulate fake ICE transports connecting. bool Connect(DtlsTestClient* peer, bool asymmetric) { - for (auto& ice : fake_ice_transports_) { - ice->SetDestination(peer->GetFakeIceTransort(ice->component()), - asymmetric); - } - return true; - } - - bool all_dtls_transports_writable() const { - if (dtls_transports_.empty()) { - return false; - } - for (const auto& dtls : dtls_transports_) { - if (!dtls->writable()) { - return false; - } - } - return true; - } - - bool all_ice_transports_writable() const { - if (dtls_transports_.empty()) { - return false; - } - for (const auto& dtls : dtls_transports_) { - if (!dtls->ice_transport()->writable()) { - return false; - } - } + fake_ice_transport_->SetDestination(peer->fake_ice_transport(), asymmetric); return true; } @@ -235,13 +117,6 @@ class DtlsTestClient : public sigslot::has_slots<> { return received_dtls_server_hellos_; } - bool negotiated_dtls() const { - return transport_->local_description() && - transport_->local_description()->identity_fingerprint && - transport_->remote_description() && - transport_->remote_description()->identity_fingerprint; - } - void CheckRole(rtc::SSLRole role) { if (role == rtc::SSL_CLIENT) { ASSERT_EQ(0, received_dtls_client_hellos_); @@ -253,38 +128,29 @@ class DtlsTestClient : public sigslot::has_slots<> { } void CheckSrtp(int expected_crypto_suite) { - for (const auto& dtls : dtls_transports_) { - int crypto_suite; - - bool rv = dtls->GetSrtpCryptoSuite(&crypto_suite); - if (negotiated_dtls() && expected_crypto_suite) { - ASSERT_TRUE(rv); - - ASSERT_EQ(crypto_suite, expected_crypto_suite); - } else { - ASSERT_FALSE(rv); - } + int crypto_suite; + bool rv = dtls_transport_->GetSrtpCryptoSuite(&crypto_suite); + if (dtls_transport_->IsDtlsActive() && expected_crypto_suite) { + ASSERT_TRUE(rv); + ASSERT_EQ(crypto_suite, expected_crypto_suite); + } else { + ASSERT_FALSE(rv); } } void CheckSsl() { - for (const auto& dtls : dtls_transports_) { - int cipher; - - bool rv = dtls->GetSslCipherSuite(&cipher); - if (negotiated_dtls()) { - ASSERT_TRUE(rv); - - EXPECT_TRUE( - rtc::SSLStreamAdapter::IsAcceptableCipher(cipher, rtc::KT_DEFAULT)); - } else { - ASSERT_FALSE(rv); - } + int cipher; + bool rv = dtls_transport_->GetSslCipherSuite(&cipher); + if (dtls_transport_->IsDtlsActive()) { + ASSERT_TRUE(rv); + EXPECT_TRUE( + rtc::SSLStreamAdapter::IsAcceptableCipher(cipher, rtc::KT_DEFAULT)); + } else { + ASSERT_FALSE(rv); } } - void SendPackets(size_t transport, size_t size, size_t count, bool srtp) { - RTC_CHECK(transport < dtls_transports_.size()); + void SendPackets(size_t size, size_t count, bool srtp) { std::unique_ptr packet(new char[size]); size_t sent = 0; do { @@ -296,35 +162,35 @@ class DtlsTestClient : public sigslot::has_slots<> { static_cast(sent)); // Only set the bypass flag if we've activated DTLS. - int flags = (certificate_ && srtp) ? cricket::PF_SRTP_BYPASS : 0; + int flags = (certificate_ && srtp) ? PF_SRTP_BYPASS : 0; rtc::PacketOptions packet_options; packet_options.packet_id = kFakePacketId; - int rv = dtls_transports_[transport]->SendPacket(packet.get(), size, - packet_options, flags); + int rv = dtls_transport_->SendPacket(packet.get(), size, packet_options, + flags); ASSERT_GT(rv, 0); ASSERT_EQ(size, static_cast(rv)); ++sent; } while (sent < count); } - int SendInvalidSrtpPacket(size_t transport, size_t size) { - RTC_CHECK(transport < dtls_transports_.size()); + int SendInvalidSrtpPacket(size_t size) { std::unique_ptr packet(new char[size]); // Fill the packet with 0 to form an invalid SRTP packet. memset(packet.get(), 0, size); rtc::PacketOptions packet_options; - return dtls_transports_[transport]->SendPacket( - packet.get(), size, packet_options, cricket::PF_SRTP_BYPASS); + return dtls_transport_->SendPacket(packet.get(), size, packet_options, + PF_SRTP_BYPASS); } - void ExpectPackets(size_t transport, size_t size) { + void ExpectPackets(size_t size) { packet_size_ = size; received_.clear(); } size_t NumPacketsReceived() { return received_.size(); } + // Inverse of SendPackets. bool VerifyPacket(const char* data, size_t size, uint32_t* out_num) { if (size != packet_size_ || (data[0] != 0 && static_cast(data[0]) != 0x80)) { @@ -373,7 +239,7 @@ class DtlsTestClient : public sigslot::has_slots<> { received_.insert(packet_num); // Only DTLS-SRTP packets should have the bypass flag set. int expected_flags = - (certificate_ && IsRtpLeadByte(data[0])) ? cricket::PF_SRTP_BYPASS : 0; + (certificate_ && IsRtpLeadByte(data[0])) ? PF_SRTP_BYPASS : 0; ASSERT_EQ(expected_flags, flags); } @@ -385,11 +251,11 @@ class DtlsTestClient : public sigslot::has_slots<> { rtc::SentPacket sent_packet() const { return sent_packet_; } // Hook into the raw packet stream to make sure DTLS packets are encrypted. - void OnFakeTransportChannelReadPacket(rtc::PacketTransportInternal* transport, - const char* data, - size_t size, - const rtc::PacketTime& time, - int flags) { + void OnFakeIceTransportReadPacket(rtc::PacketTransportInternal* transport, + const char* data, + size_t size, + const rtc::PacketTime& time, + int flags) { // Flags shouldn't be set on the underlying TransportChannel packets. ASSERT_EQ(0, flags); @@ -401,7 +267,8 @@ class DtlsTestClient : public sigslot::has_slots<> { } else if (data[13] == 2) { ++received_dtls_server_hellos_; } - } else if (negotiated_dtls() && !(data[0] >= 20 && data[0] <= 22)) { + } else if (dtls_transport_->IsDtlsActive() && + !(data[0] >= 20 && data[0] <= 22)) { ASSERT_TRUE(data[0] == 23 || IsRtpLeadByte(data[0])); if (data[0] == 23) { ASSERT_TRUE(VerifyEncryptedPacket(data, size)); @@ -414,9 +281,8 @@ class DtlsTestClient : public sigslot::has_slots<> { private: std::string name_; rtc::scoped_refptr certificate_; - std::vector> fake_ice_transports_; - std::vector> dtls_transports_; - std::unique_ptr transport_; + std::unique_ptr fake_ice_transport_; + std::unique_ptr dtls_transport_; size_t packet_size_ = 0u; std::set received_; rtc::SSLProtocolVersion ssl_max_version_ = rtc::SSL_PROTOCOL_DTLS_12; @@ -433,88 +299,38 @@ class DtlsTestClient : public sigslot::has_slots<> { class DtlsTransportChannelTestBase { public: DtlsTransportChannelTestBase() - : client1_("P1"), - client2_("P2"), - channel_ct_(1), - use_dtls_(false), - ssl_expected_version_(rtc::SSL_PROTOCOL_DTLS_12) {} + : client1_("P1"), client2_("P2"), use_dtls_(false) {} - void SetChannelCount(size_t channel_ct) { - channel_ct_ = static_cast(channel_ct); - } void SetMaxProtocolVersions(rtc::SSLProtocolVersion c1, rtc::SSLProtocolVersion c2) { client1_.SetupMaxProtocolVersion(c1); client2_.SetupMaxProtocolVersion(c2); - ssl_expected_version_ = std::min(c1, c2); } - void PrepareDtls(bool c1, bool c2, rtc::KeyType key_type) { - if (c1) { - client1_.CreateCertificate(key_type); - } - if (c2) { - client2_.CreateCertificate(key_type); - } - if (c1 && c2) - use_dtls_ = true; + // If not called, DtlsTransport will be used in SRTP bypass mode. + void PrepareDtls(rtc::KeyType key_type) { + client1_.CreateCertificate(key_type); + client2_.CreateCertificate(key_type); + use_dtls_ = true; } - // Negotiate local/remote fingerprint before or after the underlying - // tranpsort is connected? - enum NegotiateOrdering { NEGOTIATE_BEFORE_CONNECT, CONNECT_BEFORE_NEGOTIATE }; - bool Connect(ConnectionRole client1_role, - ConnectionRole client2_role, - NegotiateOrdering ordering = NEGOTIATE_BEFORE_CONNECT) { - bool rv; - if (ordering == NEGOTIATE_BEFORE_CONNECT) { - Negotiate(client1_role, client2_role); - rv = client1_.Connect(&client2_, false); - } else { - client1_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLING); - client2_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLED); - // This is equivalent to an offer being processed on both sides, but an - // answer not yet being received on the initiating side. So the - // connection will be made before negotiation has finished on both sides. - client1_.SetLocalTransportDescription(client1_.certificate(), - SdpType::kOffer, client1_role, 0); - client2_.SetRemoteTransportDescription(client1_.certificate(), - SdpType::kOffer, client1_role, 0); - client2_.SetLocalTransportDescription(client2_.certificate(), - SdpType::kAnswer, client2_role, 0); - rv = client1_.Connect(&client2_, false); - client1_.SetRemoteTransportDescription(client2_.certificate(), - SdpType::kAnswer, client2_role, 0); - } + // This test negotiates DTLS parameters before the underlying transports are + // writable. DtlsEventOrderingTest is responsible for exercising differerent + // orderings. + bool Connect(bool client1_server = true) { + Negotiate(client1_server); + EXPECT_TRUE(client1_.Connect(&client2_, false)); - EXPECT_TRUE(rv); - if (!rv) - return false; - - EXPECT_TRUE_SIMULATED_WAIT(client1_.all_dtls_transports_writable() && - client2_.all_dtls_transports_writable(), + EXPECT_TRUE_SIMULATED_WAIT(client1_.dtls_transport()->writable() && + client2_.dtls_transport()->writable(), kTimeout, fake_clock_); - if (!client1_.all_dtls_transports_writable() || - !client2_.all_dtls_transports_writable()) + if (!client1_.dtls_transport()->writable() || + !client2_.dtls_transport()->writable()) return false; // Check that we used the right roles. if (use_dtls_) { - rtc::SSLRole client1_ssl_role = - (client1_role == cricket::CONNECTIONROLE_ACTIVE || - (client2_role == cricket::CONNECTIONROLE_PASSIVE && - client1_role == cricket::CONNECTIONROLE_ACTPASS)) - ? rtc::SSL_CLIENT - : rtc::SSL_SERVER; - - rtc::SSLRole client2_ssl_role = - (client2_role == cricket::CONNECTIONROLE_ACTIVE || - (client1_role == cricket::CONNECTIONROLE_PASSIVE && - client2_role == cricket::CONNECTIONROLE_ACTPASS)) - ? rtc::SSL_CLIENT - : rtc::SSL_SERVER; - - client1_.CheckRole(client1_ssl_role); - client2_.CheckRole(client2_ssl_role); + client1_.CheckRole(client1_server ? rtc::SSL_SERVER : rtc::SSL_CLIENT); + client2_.CheckRole(client1_server ? rtc::SSL_CLIENT : rtc::SSL_SERVER); } if (use_dtls_) { @@ -535,61 +351,27 @@ class DtlsTransportChannelTestBase { return true; } - bool Connect() { - // By default, Client1 will be Server and Client2 will be Client. - return Connect(cricket::CONNECTIONROLE_ACTPASS, - cricket::CONNECTIONROLE_ACTIVE); - } - - void Negotiate() { - Negotiate(cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTIVE); - } - - void Negotiate(ConnectionRole client1_role, ConnectionRole client2_role) { - client1_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLING); - client2_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLED); - // Expect success from SLTD and SRTD. - client1_.Negotiate(&client2_, SdpType::kOffer, client1_role, client2_role, - 0); - client2_.Negotiate(&client1_, SdpType::kAnswer, client2_role, client1_role, - 0); - } - - // Negotiate with legacy client |client2|. Legacy client doesn't use setup - // attributes, except NONE. - void NegotiateWithLegacy() { - client1_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLING); - client2_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLED); - // Expect success from SLTD and SRTD. - client1_.Negotiate(&client2_, SdpType::kOffer, - cricket::CONNECTIONROLE_ACTPASS, - cricket::CONNECTIONROLE_NONE, 0); - client2_.Negotiate(&client1_, SdpType::kAnswer, - cricket::CONNECTIONROLE_ACTIVE, - cricket::CONNECTIONROLE_NONE, 0); - } - - void Renegotiate(DtlsTestClient* reoffer_initiator, - ConnectionRole client1_role, - ConnectionRole client2_role, - int flags) { - if (reoffer_initiator == &client1_) { - client1_.Negotiate(&client2_, SdpType::kOffer, client1_role, client2_role, - flags); - client2_.Negotiate(&client1_, SdpType::kAnswer, client2_role, - client1_role, flags); - } else { - client2_.Negotiate(&client1_, SdpType::kOffer, client2_role, client1_role, - flags); - client1_.Negotiate(&client2_, SdpType::kAnswer, client1_role, - client2_role, flags); + void Negotiate(bool client1_server = true) { + client1_.SetupTransports(ICEROLE_CONTROLLING); + client2_.SetupTransports(ICEROLE_CONTROLLED); + client1_.dtls_transport()->SetSslRole(client1_server ? rtc::SSL_SERVER + : rtc::SSL_CLIENT); + client2_.dtls_transport()->SetSslRole(client1_server ? rtc::SSL_CLIENT + : rtc::SSL_SERVER); + if (client2_.certificate()) { + SetRemoteFingerprintFromCert(client1_.dtls_transport(), + client2_.certificate()); + } + if (client1_.certificate()) { + SetRemoteFingerprintFromCert(client2_.dtls_transport(), + client1_.certificate()); } } - void TestTransfer(size_t transport, size_t size, size_t count, bool srtp) { + void TestTransfer(size_t size, size_t count, bool srtp) { RTC_LOG(LS_INFO) << "Expect packets, size=" << size; - client2_.ExpectPackets(transport, size); - client1_.SendPackets(transport, size, count, srtp); + client2_.ExpectPackets(size); + client1_.SendPackets(size, count, srtp); EXPECT_EQ_SIMULATED_WAIT(count, client2_.NumPacketsReceived(), kTimeout, fake_clock_); } @@ -606,72 +388,35 @@ class DtlsTransportChannelTestBase { class DtlsTransportChannelTest : public DtlsTransportChannelTestBase, public ::testing::Test {}; -// Test that transport negotiation of ICE, no DTLS works properly. -TEST_F(DtlsTransportChannelTest, TestChannelSetupIce) { - Negotiate(); - cricket::FakeIceTransport* channel1 = client1_.GetFakeIceTransort(0); - cricket::FakeIceTransport* channel2 = client2_.GetFakeIceTransort(0); - ASSERT_TRUE(channel1 != NULL); - ASSERT_TRUE(channel2 != NULL); - EXPECT_EQ(cricket::ICEROLE_CONTROLLING, channel1->GetIceRole()); - EXPECT_EQ(1U, channel1->IceTiebreaker()); - EXPECT_EQ(kIceUfrag1, channel1->ice_ufrag()); - EXPECT_EQ(kIcePwd1, channel1->ice_pwd()); - EXPECT_EQ(cricket::ICEROLE_CONTROLLED, channel2->GetIceRole()); - EXPECT_EQ(2U, channel2->IceTiebreaker()); +// Connect without DTLS, and transfer RTP data. +TEST_F(DtlsTransportChannelTest, TestTransferRtp) { + ASSERT_TRUE(Connect()); + TestTransfer(1000, 100, /*srtp=*/false); } -// Connect without DTLS, and transfer some data. -TEST_F(DtlsTransportChannelTest, TestTransfer) { +// Test that the SignalSentPacket signal is wired up. +TEST_F(DtlsTransportChannelTest, TestSignalSentPacket) { ASSERT_TRUE(Connect()); - TestTransfer(0, 1000, 100, false); -} - -// Connect without DTLS, and transfer some data. -TEST_F(DtlsTransportChannelTest, TestOnSentPacket) { - ASSERT_TRUE(Connect()); - EXPECT_EQ(client1_.sent_packet().send_time_ms, -1); - TestTransfer(0, 1000, 100, false); + // Sanity check default value (-1). + ASSERT_EQ(client1_.sent_packet().send_time_ms, -1); + TestTransfer(1000, 100, false); + // Check that we get the expected fake packet ID, and a time of 0 from the + // fake clock. EXPECT_EQ(kFakePacketId, client1_.sent_packet().packet_id); EXPECT_GE(client1_.sent_packet().send_time_ms, 0); } -// Create two channels without DTLS, and transfer some data. -TEST_F(DtlsTransportChannelTest, TestTransferTwoChannels) { - SetChannelCount(2); - ASSERT_TRUE(Connect()); - TestTransfer(0, 1000, 100, false); - TestTransfer(1, 1000, 100, false); -} - // Connect without DTLS, and transfer SRTP data. TEST_F(DtlsTransportChannelTest, TestTransferSrtp) { ASSERT_TRUE(Connect()); - TestTransfer(0, 1000, 100, true); + TestTransfer(1000, 100, /*srtp=*/true); } -// Create two channels without DTLS, and transfer SRTP data. -TEST_F(DtlsTransportChannelTest, TestTransferSrtpTwoChannels) { - SetChannelCount(2); - ASSERT_TRUE(Connect()); - TestTransfer(0, 1000, 100, true); - TestTransfer(1, 1000, 100, true); -} - -// Connect with DTLS, and transfer some data. +// Connect with DTLS, and transfer data over DTLS. TEST_F(DtlsTransportChannelTest, TestTransferDtls) { - PrepareDtls(true, true, rtc::KT_DEFAULT); + PrepareDtls(rtc::KT_DEFAULT); ASSERT_TRUE(Connect()); - TestTransfer(0, 1000, 100, false); -} - -// Create two channels with DTLS, and transfer some data. -TEST_F(DtlsTransportChannelTest, TestTransferDtlsTwoChannels) { - SetChannelCount(2); - PrepareDtls(true, true, rtc::KT_DEFAULT); - ASSERT_TRUE(Connect()); - TestTransfer(0, 1000, 100, false); - TestTransfer(1, 1000, 100, false); + TestTransfer(1000, 100, /*srtp=*/false); } // Connect with DTLS, combine multiple DTLS records into one packet. @@ -679,246 +424,121 @@ TEST_F(DtlsTransportChannelTest, TestTransferDtlsTwoChannels) { // see https://tools.ietf.org/html/rfc6347#section-4.1.1. // This has caused interoperability problems with ORTCLib in the past. TEST_F(DtlsTransportChannelTest, TestTransferDtlsCombineRecords) { - PrepareDtls(true, true, rtc::KT_DEFAULT); + PrepareDtls(rtc::KT_DEFAULT); ASSERT_TRUE(Connect()); // Our DTLS implementation always sends one record per packet, so to simulate // an endpoint that sends multiple records per packet, we configure the fake // ICE transport to combine every two consecutive packets into a single // packet. - cricket::FakeIceTransport* transport = client1_.GetFakeIceTransort(0); + FakeIceTransport* transport = client1_.fake_ice_transport(); transport->combine_outgoing_packets(true); - TestTransfer(0, 500, 100, false); + TestTransfer(500, 100, /*srtp=*/false); } -// Connect with A doing DTLS and B not, and transfer some data. -TEST_F(DtlsTransportChannelTest, TestTransferDtlsRejected) { - PrepareDtls(true, false, rtc::KT_DEFAULT); - ASSERT_TRUE(Connect()); - TestTransfer(0, 1000, 100, false); -} +class DtlsTransportChannelVersionTest + : public DtlsTransportChannelTestBase, + public ::testing::TestWithParam< + ::testing::tuple> { +}; -// Connect with B doing DTLS and A not, and transfer some data. -TEST_F(DtlsTransportChannelTest, TestTransferDtlsNotOffered) { - PrepareDtls(false, true, rtc::KT_DEFAULT); - ASSERT_TRUE(Connect()); - TestTransfer(0, 1000, 100, false); -} - -// Create two channels with DTLS 1.0 and check ciphers. -TEST_F(DtlsTransportChannelTest, TestDtls12None) { - SetChannelCount(2); - PrepareDtls(true, true, rtc::KT_DEFAULT); - SetMaxProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_10); +// Test that an acceptable cipher suite is negotiated when different versions +// of DTLS are supported. Note that it's IsAcceptableCipher that does the actual +// work. +TEST_P(DtlsTransportChannelVersionTest, TestCipherSuiteNegotiation) { + PrepareDtls(rtc::KT_DEFAULT); + SetMaxProtocolVersions(::testing::get<0>(GetParam()), + ::testing::get<1>(GetParam())); ASSERT_TRUE(Connect()); } -// Create two channels with DTLS 1.2 and check ciphers. -TEST_F(DtlsTransportChannelTest, TestDtls12Both) { - SetChannelCount(2); - PrepareDtls(true, true, rtc::KT_DEFAULT); - SetMaxProtocolVersions(rtc::SSL_PROTOCOL_DTLS_12, rtc::SSL_PROTOCOL_DTLS_12); - ASSERT_TRUE(Connect()); -} - -// Create two channels with DTLS 1.0 / DTLS 1.2 and check ciphers. -TEST_F(DtlsTransportChannelTest, TestDtls12Client1) { - SetChannelCount(2); - PrepareDtls(true, true, rtc::KT_DEFAULT); - SetMaxProtocolVersions(rtc::SSL_PROTOCOL_DTLS_12, rtc::SSL_PROTOCOL_DTLS_10); - ASSERT_TRUE(Connect()); -} - -// Create two channels with DTLS 1.2 / DTLS 1.0 and check ciphers. -TEST_F(DtlsTransportChannelTest, TestDtls12Client2) { - SetChannelCount(2); - PrepareDtls(true, true, rtc::KT_DEFAULT); - SetMaxProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_12); - ASSERT_TRUE(Connect()); -} +// Will test every combination of 1.0/1.2 on the client and server. +INSTANTIATE_TEST_CASE_P( + TestCipherSuiteNegotiation, + DtlsTransportChannelVersionTest, + ::testing::Combine(::testing::Values(rtc::SSL_PROTOCOL_DTLS_10, + rtc::SSL_PROTOCOL_DTLS_12), + ::testing::Values(rtc::SSL_PROTOCOL_DTLS_10, + rtc::SSL_PROTOCOL_DTLS_12))); // Connect with DTLS, negotiating DTLS-SRTP, and transfer SRTP using bypass. TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtp) { - PrepareDtls(true, true, rtc::KT_DEFAULT); + PrepareDtls(rtc::KT_DEFAULT); ASSERT_TRUE(Connect()); - TestTransfer(0, 1000, 100, true); + TestTransfer(1000, 100, /*srtp=*/true); } // Connect with DTLS-SRTP, transfer an invalid SRTP packet, and expects -1 // returned. TEST_F(DtlsTransportChannelTest, TestTransferDtlsInvalidSrtpPacket) { - PrepareDtls(true, true, rtc::KT_DEFAULT); + PrepareDtls(rtc::KT_DEFAULT); ASSERT_TRUE(Connect()); - int result = client1_.SendInvalidSrtpPacket(0, 100); - ASSERT_EQ(-1, result); -} - -// Connect with DTLS. A does DTLS-SRTP but B does not. -TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpRejected) { - PrepareDtls(true, true, rtc::KT_DEFAULT); - ASSERT_TRUE(Connect()); -} - -// Connect with DTLS. B does DTLS-SRTP but A does not. -TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpNotOffered) { - PrepareDtls(true, true, rtc::KT_DEFAULT); - ASSERT_TRUE(Connect()); -} - -// Create two channels with DTLS, negotiate DTLS-SRTP, and transfer bypass SRTP. -TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpTwoChannels) { - SetChannelCount(2); - PrepareDtls(true, true, rtc::KT_DEFAULT); - ASSERT_TRUE(Connect()); - TestTransfer(0, 1000, 100, true); - TestTransfer(1, 1000, 100, true); + EXPECT_EQ(-1, client1_.SendInvalidSrtpPacket(100)); } // Create a single channel with DTLS, and send normal data and SRTP data on it. TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpDemux) { - PrepareDtls(true, true, rtc::KT_DEFAULT); + PrepareDtls(rtc::KT_DEFAULT); ASSERT_TRUE(Connect()); - TestTransfer(0, 1000, 100, false); - TestTransfer(0, 1000, 100, true); + TestTransfer(1000, 100, /*srtp=*/false); + TestTransfer(1000, 100, /*srtp=*/true); } -// Testing when the remote is passive. -TEST_F(DtlsTransportChannelTest, TestTransferDtlsAnswererIsPassive) { - SetChannelCount(2); - PrepareDtls(true, true, rtc::KT_DEFAULT); - ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, - cricket::CONNECTIONROLE_PASSIVE)); - TestTransfer(0, 1000, 100, true); - TestTransfer(1, 1000, 100, true); +// Test transferring when the "answerer" has the server role. +TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpAnswererIsPassive) { + PrepareDtls(rtc::KT_DEFAULT); + ASSERT_TRUE(Connect(/*client1_server=*/false)); + TestTransfer(1000, 100, /*srtp=*/true); } -// Testing with the legacy DTLS client which doesn't use setup attribute. -// In this case legacy is the answerer. -TEST_F(DtlsTransportChannelTest, TestDtlsSetupWithLegacyAsAnswerer) { - PrepareDtls(true, true, rtc::KT_DEFAULT); - NegotiateWithLegacy(); - EXPECT_EQ(rtc::SSL_SERVER, *client1_.transport()->GetSslRole()); - EXPECT_EQ(rtc::SSL_CLIENT, *client2_.transport()->GetSslRole()); -} - -// Testing re offer/answer after the session is estbalished. Roles will be -// kept same as of the previous negotiation. -TEST_F(DtlsTransportChannelTest, TestDtlsReOfferFromOfferer) { - SetChannelCount(2); - PrepareDtls(true, true, rtc::KT_DEFAULT); - // Initial role for client1 is ACTPASS and client2 is ACTIVE. - ASSERT_TRUE( - Connect(cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTIVE)); - TestTransfer(0, 1000, 100, true); - TestTransfer(1, 1000, 100, true); - // Using input roles for the re-offer. - Renegotiate(&client1_, cricket::CONNECTIONROLE_ACTPASS, - cricket::CONNECTIONROLE_ACTIVE, NF_REOFFER); - TestTransfer(0, 1000, 100, true); - TestTransfer(1, 1000, 100, true); -} - -TEST_F(DtlsTransportChannelTest, TestDtlsReOfferFromAnswerer) { - SetChannelCount(2); - PrepareDtls(true, true, rtc::KT_DEFAULT); - // Initial role for client1 is ACTPASS and client2 is ACTIVE. - ASSERT_TRUE( - Connect(cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTIVE)); - TestTransfer(0, 1000, 100, true); - TestTransfer(1, 1000, 100, true); - // Using input roles for the re-offer. - Renegotiate(&client2_, cricket::CONNECTIONROLE_PASSIVE, - cricket::CONNECTIONROLE_ACTPASS, NF_REOFFER); - TestTransfer(0, 1000, 100, true); - TestTransfer(1, 1000, 100, true); -} - -// Test that any change in role after the intial setup will result in failure. -TEST_F(DtlsTransportChannelTest, TestDtlsRoleReversal) { - SetChannelCount(2); - PrepareDtls(true, true, rtc::KT_DEFAULT); - ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, - cricket::CONNECTIONROLE_PASSIVE)); - - // Renegotiate from client2 with actpass and client1 as active. - Renegotiate(&client2_, cricket::CONNECTIONROLE_ACTPASS, - cricket::CONNECTIONROLE_ACTIVE, NF_REOFFER | NF_EXPECT_FAILURE); -} - -// Test that using different setup attributes which results in similar ssl -// role as the initial negotiation will result in success. -TEST_F(DtlsTransportChannelTest, TestDtlsReOfferWithDifferentSetupAttr) { - SetChannelCount(2); - PrepareDtls(true, true, rtc::KT_DEFAULT); - ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, - cricket::CONNECTIONROLE_PASSIVE)); - // Renegotiate from client2 with actpass and client1 as active. - Renegotiate(&client2_, cricket::CONNECTIONROLE_ACTIVE, - cricket::CONNECTIONROLE_ACTPASS, NF_REOFFER); - TestTransfer(0, 1000, 100, true); - TestTransfer(1, 1000, 100, true); -} - -// Test that re-negotiation can be started before the clients become connected -// in the first negotiation. +// Test that renegotiation (setting same role and fingerprint again) can be +// started before the clients become connected in the first negotiation. TEST_F(DtlsTransportChannelTest, TestRenegotiateBeforeConnect) { - SetChannelCount(2); - PrepareDtls(true, true, rtc::KT_DEFAULT); + PrepareDtls(rtc::KT_DEFAULT); + // Note: This is doing the same thing Connect normally does, minus some + // additional checks not relevant for this test. Negotiate(); - - Renegotiate(&client1_, cricket::CONNECTIONROLE_ACTPASS, - cricket::CONNECTIONROLE_ACTIVE, NF_REOFFER); - bool rv = client1_.Connect(&client2_, false); - EXPECT_TRUE(rv); - EXPECT_TRUE_SIMULATED_WAIT(client1_.all_dtls_transports_writable() && - client2_.all_dtls_transports_writable(), + Negotiate(); + EXPECT_TRUE(client1_.Connect(&client2_, false)); + EXPECT_TRUE_SIMULATED_WAIT(client1_.dtls_transport()->writable() && + client2_.dtls_transport()->writable(), kTimeout, fake_clock_); - - TestTransfer(0, 1000, 100, true); - TestTransfer(1, 1000, 100, true); + TestTransfer(1000, 100, true); } // Test Certificates state after negotiation but before connection. TEST_F(DtlsTransportChannelTest, TestCertificatesBeforeConnect) { - PrepareDtls(true, true, rtc::KT_DEFAULT); + PrepareDtls(rtc::KT_DEFAULT); Negotiate(); - rtc::scoped_refptr certificate1; - rtc::scoped_refptr certificate2; - std::unique_ptr remote_cert1; - std::unique_ptr remote_cert2; - // After negotiation, each side has a distinct local certificate, but still no // remote certificate, because connection has not yet occurred. - ASSERT_TRUE(client1_.transport()->GetLocalCertificate(&certificate1)); - ASSERT_TRUE(client2_.transport()->GetLocalCertificate(&certificate2)); + auto certificate1 = client1_.dtls_transport()->GetLocalCertificate(); + auto certificate2 = client2_.dtls_transport()->GetLocalCertificate(); ASSERT_NE(certificate1->ssl_certificate().ToPEMString(), certificate2->ssl_certificate().ToPEMString()); - ASSERT_FALSE(client1_.GetDtlsTransport(0)->GetRemoteSSLCertificate()); - ASSERT_FALSE(client2_.GetDtlsTransport(0)->GetRemoteSSLCertificate()); + ASSERT_FALSE(client1_.dtls_transport()->GetRemoteSSLCertificate()); + ASSERT_FALSE(client2_.dtls_transport()->GetRemoteSSLCertificate()); } // Test Certificates state after connection. TEST_F(DtlsTransportChannelTest, TestCertificatesAfterConnect) { - PrepareDtls(true, true, rtc::KT_DEFAULT); + PrepareDtls(rtc::KT_DEFAULT); ASSERT_TRUE(Connect()); - rtc::scoped_refptr certificate1; - rtc::scoped_refptr certificate2; - // After connection, each side has a distinct local certificate. - ASSERT_TRUE(client1_.transport()->GetLocalCertificate(&certificate1)); - ASSERT_TRUE(client2_.transport()->GetLocalCertificate(&certificate2)); + auto certificate1 = client1_.dtls_transport()->GetLocalCertificate(); + auto certificate2 = client2_.dtls_transport()->GetLocalCertificate(); ASSERT_NE(certificate1->ssl_certificate().ToPEMString(), certificate2->ssl_certificate().ToPEMString()); // Each side's remote certificate is the other side's local certificate. std::unique_ptr remote_cert1 = - client1_.GetDtlsTransport(0)->GetRemoteSSLCertificate(); + client1_.dtls_transport()->GetRemoteSSLCertificate(); ASSERT_TRUE(remote_cert1); ASSERT_EQ(remote_cert1->ToPEMString(), certificate2->ssl_certificate().ToPEMString()); std::unique_ptr remote_cert2 = - client2_.GetDtlsTransport(0)->GetRemoteSSLCertificate(); + client2_.dtls_transport()->GetRemoteSSLCertificate(); ASSERT_TRUE(remote_cert2); ASSERT_EQ(remote_cert2->ToPEMString(), certificate1->ssl_certificate().ToPEMString()); @@ -933,19 +553,19 @@ TEST_F(DtlsTransportChannelTest, TestRetransmissionSchedule) { // 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); + PrepareDtls(rtc::KT_DEFAULT); + // Exchange fingerprints and set SSL roles. + Negotiate(); // 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_SIMULATED_WAIT(client2_.all_ice_transports_writable(), kTimeout, - fake_clock_); + EXPECT_TRUE_SIMULATED_WAIT(client2_.fake_ice_transport()->writable(), + kTimeout, fake_clock_); // Wait for the first client hello to be sent. EXPECT_EQ_WAIT(1, client1_.received_dtls_client_hellos(), kTimeout); - EXPECT_FALSE(client1_.all_ice_transports_writable()); + EXPECT_FALSE(client1_.fake_ice_transport()->writable()); static int timeout_schedule_ms[] = {50, 100, 200, 400, 800, 1600, 3200, 6400, 12800, 25600, 51200, 60000}; @@ -966,16 +586,6 @@ TEST_F(DtlsTransportChannelTest, TestRetransmissionSchedule) { } } -// Test that a DTLS connection can be made even if the underlying transport -// is connected before DTLS fingerprints/roles have been negotiated. -TEST_F(DtlsTransportChannelTest, TestConnectBeforeNegotiate) { - PrepareDtls(true, true, rtc::KT_DEFAULT); - ASSERT_TRUE(Connect(cricket::CONNECTIONROLE_ACTPASS, - cricket::CONNECTIONROLE_ACTIVE, - CONNECT_BEFORE_NEGOTIATE)); - TestTransfer(0, 1000, 100, false); -} - // The following events can occur in many different orders: // 1. Caller receives remote fingerprint. // 2. Caller is writable. @@ -1010,46 +620,34 @@ class DtlsEventOrderingTest // Pre-setup: Set local certificate on both caller and callee, and // remote fingerprint on callee, but neither is writable and the caller // doesn't have the callee's fingerprint. - PrepareDtls(true, true, rtc::KT_DEFAULT); + PrepareDtls(rtc::KT_DEFAULT); // Simulate packets being sent and arriving asynchronously. // Otherwise the entire DTLS handshake would occur in one clock tick, and // we couldn't inject method calls in the middle of it. int simulated_delay_ms = 10; - client1_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLING, - simulated_delay_ms); - client2_.SetupChannels(channel_ct_, cricket::ICEROLE_CONTROLLED, - simulated_delay_ms); - client1_.SetLocalTransportDescription(client1_.certificate(), - SdpType::kOffer, - cricket::CONNECTIONROLE_ACTPASS, 0); - client2_.Negotiate(&client1_, SdpType::kAnswer, - cricket::CONNECTIONROLE_ACTIVE, - cricket::CONNECTIONROLE_ACTPASS, 0); + client1_.SetupTransports(ICEROLE_CONTROLLING, simulated_delay_ms); + client2_.SetupTransports(ICEROLE_CONTROLLED, simulated_delay_ms); + // Similar to how NegotiateOrdering works. + client1_.dtls_transport()->SetSslRole(rtc::SSL_SERVER); + client2_.dtls_transport()->SetSslRole(rtc::SSL_CLIENT); + SetRemoteFingerprintFromCert(client2_.dtls_transport(), + client1_.certificate()); for (DtlsTransportEvent e : events) { switch (e) { case CALLER_RECEIVES_FINGERPRINT: if (valid_fingerprint) { - client1_.SetRemoteTransportDescription( - client2_.certificate(), SdpType::kAnswer, - cricket::CONNECTIONROLE_ACTIVE, 0); + SetRemoteFingerprintFromCert(client1_.dtls_transport(), + client2_.certificate()); } else { - // Create a fingerprint with a correct algorithm but an invalid - // digest. - cricket::TransportDescription remote_desc = - MakeTransportDescription(client2_.certificate(), - cricket::CONNECTIONROLE_ACTIVE); - ++(remote_desc.identity_fingerprint->digest[0]); - // Even if certificate verification fails inside this method, - // it should return true as long as the fingerprint was formatted - // correctly. - EXPECT_TRUE(client1_.transport()->SetRemoteTransportDescription( - remote_desc, SdpType::kAnswer, nullptr)); + SetRemoteFingerprintFromCert(client1_.dtls_transport(), + client2_.certificate(), + true /*modify_digest*/); } break; case CALLER_WRITABLE: EXPECT_TRUE(client1_.Connect(&client2_, true)); - EXPECT_TRUE_SIMULATED_WAIT(client1_.all_ice_transports_writable(), + EXPECT_TRUE_SIMULATED_WAIT(client1_.fake_ice_transport()->writable(), kTimeout, fake_clock_); break; case CALLER_RECEIVES_CLIENTHELLO: @@ -1057,45 +655,44 @@ class DtlsEventOrderingTest EXPECT_EQ(0, client1_.received_dtls_client_hellos()); // Making client2_ writable will cause it to send the ClientHello. EXPECT_TRUE(client2_.Connect(&client1_, true)); - EXPECT_TRUE_SIMULATED_WAIT(client2_.all_ice_transports_writable(), + EXPECT_TRUE_SIMULATED_WAIT(client2_.fake_ice_transport()->writable(), kTimeout, fake_clock_); EXPECT_EQ_SIMULATED_WAIT(1, client1_.received_dtls_client_hellos(), kTimeout, fake_clock_); break; case HANDSHAKE_FINISHES: // Sanity check that the handshake hasn't already finished. - EXPECT_FALSE(client1_.GetDtlsTransport(0)->IsDtlsConnected() || - client1_.GetDtlsTransport(0)->dtls_state() == - cricket::DTLS_TRANSPORT_FAILED); + EXPECT_FALSE(client1_.dtls_transport()->IsDtlsConnected() || + client1_.dtls_transport()->dtls_state() == + DTLS_TRANSPORT_FAILED); EXPECT_TRUE_SIMULATED_WAIT( - client1_.GetDtlsTransport(0)->IsDtlsConnected() || - client1_.GetDtlsTransport(0)->dtls_state() == - cricket::DTLS_TRANSPORT_FAILED, + client1_.dtls_transport()->IsDtlsConnected() || + client1_.dtls_transport()->dtls_state() == + DTLS_TRANSPORT_FAILED, kTimeout, fake_clock_); break; } } - cricket::DtlsTransportState expected_final_state = - valid_fingerprint ? cricket::DTLS_TRANSPORT_CONNECTED - : cricket::DTLS_TRANSPORT_FAILED; + DtlsTransportState expected_final_state = + valid_fingerprint ? DTLS_TRANSPORT_CONNECTED : DTLS_TRANSPORT_FAILED; EXPECT_EQ_SIMULATED_WAIT(expected_final_state, - client1_.GetDtlsTransport(0)->dtls_state(), - kTimeout, fake_clock_); + client1_.dtls_transport()->dtls_state(), kTimeout, + fake_clock_); EXPECT_EQ_SIMULATED_WAIT(expected_final_state, - client2_.GetDtlsTransport(0)->dtls_state(), - kTimeout, fake_clock_); + client2_.dtls_transport()->dtls_state(), kTimeout, + fake_clock_); // Channel should be writable iff there was a valid fingerprint. - EXPECT_EQ(valid_fingerprint, client1_.GetDtlsTransport(0)->writable()); - EXPECT_EQ(valid_fingerprint, client2_.GetDtlsTransport(0)->writable()); + EXPECT_EQ(valid_fingerprint, client1_.dtls_transport()->writable()); + EXPECT_EQ(valid_fingerprint, client2_.dtls_transport()->writable()); // Check that no hello needed to be retransmitted. EXPECT_EQ(1, client1_.received_dtls_client_hellos()); EXPECT_EQ(1, client2_.received_dtls_server_hellos()); if (valid_fingerprint) { - TestTransfer(0, 1000, 100, false); + TestTransfer(1000, 100, false); } } }; @@ -1135,3 +732,5 @@ INSTANTIATE_TEST_CASE_P( CALLER_WRITABLE, HANDSHAKE_FINISHES, CALLER_RECEIVES_FINGERPRINT}), ::testing::Bool())); + +} // namespace cricket diff --git a/p2p/base/jseptransport_unittest.cc b/p2p/base/jseptransport_unittest.cc index ff8484d54b..bcd12c5595 100644 --- a/p2p/base/jseptransport_unittest.cc +++ b/p2p/base/jseptransport_unittest.cc @@ -16,11 +16,8 @@ #include "rtc_base/gunit.h" #include "rtc_base/network.h" -using cricket::JsepTransport; -using cricket::FakeDtlsTransport; -using cricket::FakeIceTransport; -using cricket::IceRole; -using cricket::TransportDescription; +namespace cricket { + using rtc::SocketAddress; using webrtc::SdpType; @@ -30,63 +27,188 @@ static const char kIcePwd1[] = "TESTICEPWD00000000000001"; static const char kIceUfrag2[] = "TESTICEUFRAG0002"; static const char kIcePwd2[] = "TESTICEPWD00000000000002"; +TransportDescription MakeTransportDescription( + const char* ufrag, + const char* pwd, + const rtc::scoped_refptr& cert, + ConnectionRole role = CONNECTIONROLE_NONE) { + std::unique_ptr fingerprint; + if (cert) { + fingerprint.reset(rtc::SSLFingerprint::CreateFromCertificate(cert)); + } + return TransportDescription(std::vector(), ufrag, pwd, + ICEMODE_FULL, role, fingerprint.get()); +} + class JsepTransportTest : public testing::Test, public sigslot::has_slots<> { public: JsepTransportTest() { RecreateTransport(); } - bool SetupChannel() { - fake_ice_transport_.reset(new FakeIceTransport(transport_->mid(), 1)); - fake_dtls_transport_.reset( - new FakeDtlsTransport(fake_ice_transport_.get())); - return transport_->AddChannel(fake_dtls_transport_.get(), 1); + bool SetupFakeTransports(int component) { + fake_ice_transports_.emplace_back( + new FakeIceTransport(transport_->mid(), component)); + fake_dtls_transports_.emplace_back( + new FakeDtlsTransport(fake_ice_transports_.back().get())); + return transport_->AddChannel(fake_dtls_transports_.back().get(), + component); } - void DestroyChannel() { transport_->RemoveChannel(1); } + void DestroyChannel(int component) { transport_->RemoveChannel(component); } void RecreateTransport() { transport_.reset(new JsepTransport("test content name", nullptr)); } protected: - std::unique_ptr fake_dtls_transport_; - std::unique_ptr fake_ice_transport_; + std::vector> fake_dtls_transports_; + std::vector> fake_ice_transports_; std::unique_ptr transport_; }; -// This test verifies channels are created with proper ICE -// ufrag/password after a transport description is applied. -TEST_F(JsepTransportTest, TestChannelIceParameters) { - cricket::TransportDescription local_desc(kIceUfrag1, kIcePwd1); +// This test verifies transports are created with proper ICE ufrag/password +// after a transport description is applied. +TEST_F(JsepTransportTest, TestIceTransportParameters) { + EXPECT_TRUE(SetupFakeTransports(1)); + TransportDescription local_desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription(local_desc, SdpType::kOffer, NULL)); - EXPECT_TRUE(SetupChannel()); - EXPECT_EQ(cricket::ICEMODE_FULL, fake_ice_transport_->remote_ice_mode()); - EXPECT_EQ(kIceUfrag1, fake_ice_transport_->ice_ufrag()); - EXPECT_EQ(kIcePwd1, fake_ice_transport_->ice_pwd()); + EXPECT_EQ(ICEMODE_FULL, fake_ice_transports_[0]->remote_ice_mode()); + EXPECT_EQ(kIceUfrag1, fake_ice_transports_[0]->ice_ufrag()); + EXPECT_EQ(kIcePwd1, fake_ice_transports_[0]->ice_pwd()); - cricket::TransportDescription remote_desc(kIceUfrag1, kIcePwd1); + TransportDescription remote_desc(kIceUfrag2, kIcePwd2); ASSERT_TRUE(transport_->SetRemoteTransportDescription( remote_desc, SdpType::kAnswer, NULL)); - EXPECT_EQ(cricket::ICEMODE_FULL, fake_ice_transport_->remote_ice_mode()); - EXPECT_EQ(kIceUfrag1, fake_ice_transport_->remote_ice_ufrag()); - EXPECT_EQ(kIcePwd1, fake_ice_transport_->remote_ice_pwd()); + EXPECT_EQ(ICEMODE_FULL, fake_ice_transports_[0]->remote_ice_mode()); + EXPECT_EQ(kIceUfrag2, fake_ice_transports_[0]->remote_ice_ufrag()); + EXPECT_EQ(kIcePwd2, fake_ice_transports_[0]->remote_ice_pwd()); +} + +// Similarly, test that DTLS parameters are applied after a transport +// description is applied. +TEST_F(JsepTransportTest, TestDtlsTransportParameters) { + EXPECT_TRUE(SetupFakeTransports(1)); + + // Create certificates. + rtc::scoped_refptr local_cert = + rtc::RTCCertificate::Create(std::unique_ptr( + rtc::SSLIdentity::Generate("local", rtc::KT_DEFAULT))); + rtc::scoped_refptr remote_cert = + rtc::RTCCertificate::Create(std::unique_ptr( + rtc::SSLIdentity::Generate("remote", rtc::KT_DEFAULT))); + transport_->SetLocalCertificate(local_cert); + + // Apply offer/answer. + TransportDescription local_desc = MakeTransportDescription( + kIceUfrag1, kIcePwd1, local_cert, CONNECTIONROLE_ACTPASS); + ASSERT_TRUE(transport_->SetLocalTransportDescription( + local_desc, SdpType::kOffer, nullptr)); + TransportDescription remote_desc = MakeTransportDescription( + kIceUfrag2, kIcePwd2, remote_cert, CONNECTIONROLE_ACTIVE); + ASSERT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, SdpType::kAnswer, nullptr)); + + // Verify that SSL role and remote fingerprint were set correctly based on + // transport descriptions. + rtc::SSLRole role; + EXPECT_TRUE(fake_dtls_transports_[0]->GetSslRole(&role)); + EXPECT_EQ(rtc::SSL_SERVER, role); // Because remote description was "active". + EXPECT_EQ(remote_desc.identity_fingerprint->ToString(), + fake_dtls_transports_[0]->dtls_fingerprint().ToString()); +} + +// Same as above test, but with remote transport description using +// CONNECTIONROLE_PASSIVE, expecting SSL_CLIENT role. +TEST_F(JsepTransportTest, TestDtlsTransportParametersWithPassiveAnswer) { + EXPECT_TRUE(SetupFakeTransports(1)); + + // Create certificates. + rtc::scoped_refptr local_cert = + rtc::RTCCertificate::Create(std::unique_ptr( + rtc::SSLIdentity::Generate("local", rtc::KT_DEFAULT))); + rtc::scoped_refptr remote_cert = + rtc::RTCCertificate::Create(std::unique_ptr( + rtc::SSLIdentity::Generate("remote", rtc::KT_DEFAULT))); + transport_->SetLocalCertificate(local_cert); + + // Apply offer/answer. + TransportDescription local_desc = MakeTransportDescription( + kIceUfrag1, kIcePwd1, local_cert, CONNECTIONROLE_ACTPASS); + ASSERT_TRUE(transport_->SetLocalTransportDescription( + local_desc, SdpType::kOffer, nullptr)); + TransportDescription remote_desc = MakeTransportDescription( + kIceUfrag2, kIcePwd2, remote_cert, CONNECTIONROLE_PASSIVE); + ASSERT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, SdpType::kAnswer, nullptr)); + + // Verify that SSL role and remote fingerprint were set correctly based on + // transport descriptions. + rtc::SSLRole role; + EXPECT_TRUE(fake_dtls_transports_[0]->GetSslRole(&role)); + EXPECT_EQ(rtc::SSL_CLIENT, + role); // Because remote description was "passive". + EXPECT_EQ(remote_desc.identity_fingerprint->ToString(), + fake_dtls_transports_[0]->dtls_fingerprint().ToString()); +} + +// Add two DtlsTransports/IceTransports and make sure parameters are applied to +// both of them. Applicable when RTP/RTCP are not multiplexed, so they share +// the same parameters but different connections. +TEST_F(JsepTransportTest, TestTransportParametersAppliedToTwoComponents) { + EXPECT_TRUE(SetupFakeTransports(1)); + EXPECT_TRUE(SetupFakeTransports(2)); + + // Create certificates. + rtc::scoped_refptr local_cert = + rtc::RTCCertificate::Create(std::unique_ptr( + rtc::SSLIdentity::Generate("local", rtc::KT_DEFAULT))); + rtc::scoped_refptr remote_cert = + rtc::RTCCertificate::Create(std::unique_ptr( + rtc::SSLIdentity::Generate("remote", rtc::KT_DEFAULT))); + transport_->SetLocalCertificate(local_cert); + + // Apply offer/answer. + TransportDescription local_desc = MakeTransportDescription( + kIceUfrag1, kIcePwd1, local_cert, CONNECTIONROLE_ACTPASS); + ASSERT_TRUE(transport_->SetLocalTransportDescription( + local_desc, SdpType::kOffer, nullptr)); + TransportDescription remote_desc = MakeTransportDescription( + kIceUfrag2, kIcePwd2, remote_cert, CONNECTIONROLE_ACTIVE); + ASSERT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, SdpType::kAnswer, nullptr)); + + for (int i = 0; i < 1; ++i) { + // Verify parameters of ICE transports. + EXPECT_EQ(ICEMODE_FULL, fake_ice_transports_[i]->remote_ice_mode()); + EXPECT_EQ(kIceUfrag1, fake_ice_transports_[i]->ice_ufrag()); + EXPECT_EQ(kIcePwd1, fake_ice_transports_[i]->ice_pwd()); + EXPECT_EQ(kIceUfrag2, fake_ice_transports_[i]->remote_ice_ufrag()); + EXPECT_EQ(kIcePwd2, fake_ice_transports_[i]->remote_ice_pwd()); + // Verify parameters of DTLS transports. + rtc::SSLRole role; + EXPECT_TRUE(fake_dtls_transports_[i]->GetSslRole(&role)); + EXPECT_EQ(rtc::SSL_SERVER, + role); // Because remote description was "active". + EXPECT_EQ(remote_desc.identity_fingerprint->ToString(), + fake_dtls_transports_[i]->dtls_fingerprint().ToString()); + } } // Verifies that IceCredentialsChanged returns true when either ufrag or pwd // changed, and false in other cases. TEST_F(JsepTransportTest, TestIceCredentialsChanged) { - EXPECT_TRUE(cricket::IceCredentialsChanged("u1", "p1", "u2", "p2")); - EXPECT_TRUE(cricket::IceCredentialsChanged("u1", "p1", "u2", "p1")); - EXPECT_TRUE(cricket::IceCredentialsChanged("u1", "p1", "u1", "p2")); - EXPECT_FALSE(cricket::IceCredentialsChanged("u1", "p1", "u1", "p1")); + EXPECT_TRUE(IceCredentialsChanged("u1", "p1", "u2", "p2")); + EXPECT_TRUE(IceCredentialsChanged("u1", "p1", "u2", "p1")); + EXPECT_TRUE(IceCredentialsChanged("u1", "p1", "u1", "p2")); + EXPECT_FALSE(IceCredentialsChanged("u1", "p1", "u1", "p1")); } // Tests SetNeedsIceRestartFlag and NeedsIceRestart, ensuring NeedsIceRestart // only starts returning "false" once an ICE restart has been initiated. TEST_F(JsepTransportTest, NeedsIceRestart) { // Do initial offer/answer so there's something to restart. - cricket::TransportDescription local_desc(kIceUfrag1, kIcePwd1); - cricket::TransportDescription remote_desc(kIceUfrag1, kIcePwd1); + TransportDescription local_desc(kIceUfrag1, kIcePwd1); + TransportDescription remote_desc(kIceUfrag1, kIcePwd1); ASSERT_TRUE(transport_->SetLocalTransportDescription( local_desc, SdpType::kOffer, nullptr)); ASSERT_TRUE(transport_->SetRemoteTransportDescription( @@ -107,8 +229,8 @@ TEST_F(JsepTransportTest, NeedsIceRestart) { EXPECT_TRUE(transport_->NeedsIceRestart()); // Doing an offer/answer that restarts ICE should clear the flag. - cricket::TransportDescription ice_restart_local_desc(kIceUfrag2, kIcePwd2); - cricket::TransportDescription ice_restart_remote_desc(kIceUfrag2, kIcePwd2); + TransportDescription ice_restart_local_desc(kIceUfrag2, kIcePwd2); + TransportDescription ice_restart_remote_desc(kIceUfrag2, kIcePwd2); ASSERT_TRUE(transport_->SetLocalTransportDescription( ice_restart_local_desc, SdpType::kOffer, nullptr)); ASSERT_TRUE(transport_->SetRemoteTransportDescription( @@ -117,18 +239,17 @@ TEST_F(JsepTransportTest, NeedsIceRestart) { } TEST_F(JsepTransportTest, TestGetStats) { - EXPECT_TRUE(SetupChannel()); - cricket::TransportStats stats; + EXPECT_TRUE(SetupFakeTransports(1)); + TransportStats stats; EXPECT_TRUE(transport_->GetStats(&stats)); // Note that this tests the behavior of a FakeIceTransport. ASSERT_EQ(1U, stats.channel_stats.size()); EXPECT_EQ(1, stats.channel_stats[0].component); // Set local transport description for FakeTransport before connecting. TransportDescription faketransport_desc( - std::vector(), - rtc::CreateRandomString(cricket::ICE_UFRAG_LENGTH), - rtc::CreateRandomString(cricket::ICE_PWD_LENGTH), cricket::ICEMODE_FULL, - cricket::CONNECTIONROLE_NONE, nullptr); + std::vector(), rtc::CreateRandomString(ICE_UFRAG_LENGTH), + rtc::CreateRandomString(ICE_PWD_LENGTH), ICEMODE_FULL, + CONNECTIONROLE_NONE, nullptr); transport_->SetLocalTransportDescription(faketransport_desc, SdpType::kOffer, nullptr); EXPECT_TRUE(transport_->GetStats(&stats)); @@ -174,24 +295,20 @@ TEST_F(JsepTransportTest, TestVerifyCertificateFingerprint) { // Tests the logic of DTLS role negotiation for an initial offer/answer. TEST_F(JsepTransportTest, DtlsRoleNegotiation) { + // Just use the same certificate for both sides; doesn't really matter in a + // non end-to-end test. rtc::scoped_refptr certificate = rtc::RTCCertificate::Create(std::unique_ptr( rtc::SSLIdentity::Generate("testing", rtc::KT_ECDSA))); - std::unique_ptr fingerprint( - rtc::SSLFingerprint::CreateFromCertificate(certificate)); - TransportDescription local_desc(kIceUfrag1, kIcePwd1); - TransportDescription remote_desc(kIceUfrag2, kIcePwd2); - // Just use the same fingerprint in both descriptions; the remote fingerprint - // doesn't matter in a non end-to-end test. - local_desc.identity_fingerprint.reset( - TransportDescription::CopyFingerprint(fingerprint.get())); - remote_desc.identity_fingerprint.reset( - TransportDescription::CopyFingerprint(fingerprint.get())); + TransportDescription local_desc = + MakeTransportDescription(kIceUfrag1, kIcePwd1, certificate); + TransportDescription remote_desc = + MakeTransportDescription(kIceUfrag2, kIcePwd2, certificate); struct NegotiateRoleParams { - cricket::ConnectionRole local_role; - cricket::ConnectionRole remote_role; + ConnectionRole local_role; + ConnectionRole remote_role; SdpType local_type; SdpType remote_type; }; @@ -200,14 +317,14 @@ TEST_F(JsepTransportTest, DtlsRoleNegotiation) { // Parameters which set the SSL role to SSL_CLIENT. NegotiateRoleParams valid_client_params[] = { - {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTPASS, - SdpType::kAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTPASS, - SdpType::kPrAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE, - SdpType::kOffer, SdpType::kAnswer}, - {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE, - SdpType::kOffer, SdpType::kPrAnswer}}; + {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_ACTPASS, SdpType::kAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_ACTPASS, SdpType::kPrAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_PASSIVE, SdpType::kOffer, + SdpType::kAnswer}, + {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_PASSIVE, SdpType::kOffer, + SdpType::kPrAnswer}}; for (auto& param : valid_client_params) { RecreateTransport(); @@ -233,14 +350,14 @@ TEST_F(JsepTransportTest, DtlsRoleNegotiation) { // Parameters which set the SSL role to SSL_SERVER. NegotiateRoleParams valid_server_params[] = { - {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTPASS, - SdpType::kAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTPASS, - SdpType::kPrAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTIVE, - SdpType::kOffer, SdpType::kAnswer}, - {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTIVE, - SdpType::kOffer, SdpType::kPrAnswer}}; + {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTPASS, SdpType::kAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTPASS, SdpType::kPrAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_ACTIVE, SdpType::kOffer, + SdpType::kAnswer}, + {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_ACTIVE, SdpType::kOffer, + SdpType::kPrAnswer}}; for (auto& param : valid_server_params) { RecreateTransport(); @@ -266,30 +383,30 @@ TEST_F(JsepTransportTest, DtlsRoleNegotiation) { // Invalid parameters due to both peers having a duplicate role. NegotiateRoleParams duplicate_params[] = { - {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTIVE, - SdpType::kAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTPASS, - SdpType::kAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_PASSIVE, - SdpType::kAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTIVE, - SdpType::kPrAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTPASS, - SdpType::kPrAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_PASSIVE, - SdpType::kPrAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTIVE, - SdpType::kOffer, SdpType::kAnswer}, - {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTPASS, - SdpType::kOffer, SdpType::kAnswer}, - {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_PASSIVE, - SdpType::kOffer, SdpType::kAnswer}, - {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_ACTIVE, - SdpType::kOffer, SdpType::kPrAnswer}, - {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTPASS, - SdpType::kOffer, SdpType::kPrAnswer}, - {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_PASSIVE, - SdpType::kOffer, SdpType::kPrAnswer}}; + {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_ACTIVE, SdpType::kAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_ACTPASS, SdpType::kAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_PASSIVE, SdpType::kAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_ACTIVE, SdpType::kPrAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_ACTPASS, SdpType::kPrAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_PASSIVE, SdpType::kPrAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_ACTIVE, SdpType::kOffer, + SdpType::kAnswer}, + {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_ACTPASS, SdpType::kOffer, + SdpType::kAnswer}, + {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_PASSIVE, SdpType::kOffer, + SdpType::kAnswer}, + {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_ACTIVE, SdpType::kOffer, + SdpType::kPrAnswer}, + {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_ACTPASS, SdpType::kOffer, + SdpType::kPrAnswer}, + {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_PASSIVE, SdpType::kOffer, + SdpType::kPrAnswer}}; for (auto& param : duplicate_params) { RecreateTransport(); @@ -314,30 +431,30 @@ TEST_F(JsepTransportTest, DtlsRoleNegotiation) { // Invalid parameters due to the offerer not using ACTPASS. NegotiateRoleParams offerer_without_actpass_params[] = { - {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_PASSIVE, - SdpType::kAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTIVE, - SdpType::kAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE, - SdpType::kAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_PASSIVE, - SdpType::kPrAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTIVE, - SdpType::kPrAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_PASSIVE, - SdpType::kPrAnswer, SdpType::kOffer}, - {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_PASSIVE, - SdpType::kOffer, SdpType::kAnswer}, - {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTIVE, - SdpType::kOffer, SdpType::kAnswer}, - {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTPASS, - SdpType::kOffer, SdpType::kAnswer}, - {cricket::CONNECTIONROLE_ACTIVE, cricket::CONNECTIONROLE_PASSIVE, - SdpType::kOffer, SdpType::kPrAnswer}, - {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTIVE, - SdpType::kOffer, SdpType::kPrAnswer}, - {cricket::CONNECTIONROLE_PASSIVE, cricket::CONNECTIONROLE_ACTPASS, - SdpType::kOffer, SdpType::kPrAnswer}}; + {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_PASSIVE, SdpType::kAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTIVE, SdpType::kAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_PASSIVE, SdpType::kAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_PASSIVE, SdpType::kPrAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTIVE, SdpType::kPrAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_ACTPASS, CONNECTIONROLE_PASSIVE, SdpType::kPrAnswer, + SdpType::kOffer}, + {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_PASSIVE, SdpType::kOffer, + SdpType::kAnswer}, + {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTIVE, SdpType::kOffer, + SdpType::kAnswer}, + {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTPASS, SdpType::kOffer, + SdpType::kAnswer}, + {CONNECTIONROLE_ACTIVE, CONNECTIONROLE_PASSIVE, SdpType::kOffer, + SdpType::kPrAnswer}, + {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTIVE, SdpType::kOffer, + SdpType::kPrAnswer}, + {CONNECTIONROLE_PASSIVE, CONNECTIONROLE_ACTPASS, SdpType::kOffer, + SdpType::kPrAnswer}}; for (auto& param : offerer_without_actpass_params) { RecreateTransport(); @@ -363,28 +480,88 @@ TEST_F(JsepTransportTest, DtlsRoleNegotiation) { } } +// Test that a reoffer in the opposite direction is successful as long as the +// role isn't changing. Doesn't test every possible combination like the test +// above. +TEST_F(JsepTransportTest, ValidDtlsReofferFromAnswerer) { + // Just use the same certificate for both sides; doesn't really matter in a + // non end-to-end test. + rtc::scoped_refptr certificate = + rtc::RTCCertificate::Create(std::unique_ptr( + rtc::SSLIdentity::Generate("testing", rtc::KT_ECDSA))); + transport_->SetLocalCertificate(certificate); + + TransportDescription local_offer = MakeTransportDescription( + kIceUfrag1, kIcePwd1, certificate, CONNECTIONROLE_ACTPASS); + TransportDescription remote_answer = MakeTransportDescription( + kIceUfrag2, kIcePwd2, certificate, CONNECTIONROLE_ACTIVE); + + EXPECT_TRUE(transport_->SetLocalTransportDescription( + local_offer, SdpType::kOffer, nullptr)); + EXPECT_TRUE(transport_->SetRemoteTransportDescription( + remote_answer, SdpType::kAnswer, nullptr)); + + // We were actpass->active previously, now in the other direction it's + // actpass->passive. + TransportDescription remote_offer = MakeTransportDescription( + kIceUfrag2, kIcePwd2, certificate, CONNECTIONROLE_ACTPASS); + TransportDescription local_answer = MakeTransportDescription( + kIceUfrag1, kIcePwd1, certificate, CONNECTIONROLE_PASSIVE); + + EXPECT_TRUE(transport_->SetRemoteTransportDescription( + remote_offer, SdpType::kOffer, nullptr)); + EXPECT_TRUE(transport_->SetLocalTransportDescription( + local_answer, SdpType::kAnswer, nullptr)); +} + +// Test that a reoffer in the opposite direction fails if the role changes. +// Inverse of test above. +TEST_F(JsepTransportTest, InvalidDtlsReofferFromAnswerer) { + // Just use the same certificate for both sides; doesn't really matter in a + // non end-to-end test. + rtc::scoped_refptr certificate = + rtc::RTCCertificate::Create(std::unique_ptr( + rtc::SSLIdentity::Generate("testing", rtc::KT_ECDSA))); + transport_->SetLocalCertificate(certificate); + + TransportDescription local_offer = MakeTransportDescription( + kIceUfrag1, kIcePwd1, certificate, CONNECTIONROLE_ACTPASS); + TransportDescription remote_answer = MakeTransportDescription( + kIceUfrag2, kIcePwd2, certificate, CONNECTIONROLE_ACTIVE); + + EXPECT_TRUE(transport_->SetLocalTransportDescription( + local_offer, SdpType::kOffer, nullptr)); + EXPECT_TRUE(transport_->SetRemoteTransportDescription( + remote_answer, SdpType::kAnswer, nullptr)); + + // Changing role to passive here isn't allowed. Though for some reason this + // only fails in SetLocalTransportDescription. + TransportDescription remote_offer = MakeTransportDescription( + kIceUfrag2, kIcePwd2, certificate, CONNECTIONROLE_PASSIVE); + TransportDescription local_answer = MakeTransportDescription( + kIceUfrag1, kIcePwd1, certificate, CONNECTIONROLE_ACTIVE); + + EXPECT_TRUE(transport_->SetRemoteTransportDescription( + remote_offer, SdpType::kOffer, nullptr)); + EXPECT_FALSE(transport_->SetLocalTransportDescription( + local_answer, SdpType::kAnswer, nullptr)); +} + // Test that a remote offer with the current negotiated role can be accepted. // This is allowed by dtls-sdp, though we'll never generate such an offer, // since JSEP requires generating "actpass". TEST_F(JsepTransportTest, RemoteOfferWithCurrentNegotiatedDtlsRole) { + // Just use the same certificate in both descriptions; the remote fingerprint + // doesn't matter in a non end-to-end test. rtc::scoped_refptr certificate = rtc::RTCCertificate::Create(std::unique_ptr( rtc::SSLIdentity::Generate("testing", rtc::KT_ECDSA))); - std::unique_ptr fingerprint( - rtc::SSLFingerprint::CreateFromCertificate(certificate)); transport_->SetLocalCertificate(certificate); - TransportDescription local_desc(kIceUfrag1, kIcePwd1); - TransportDescription remote_desc(kIceUfrag2, kIcePwd2); - // Just use the same fingerprint in both descriptions; the remote fingerprint - // doesn't matter in a non end-to-end test. - local_desc.identity_fingerprint.reset( - TransportDescription::CopyFingerprint(fingerprint.get())); - remote_desc.identity_fingerprint.reset( - TransportDescription::CopyFingerprint(fingerprint.get())); - - remote_desc.connection_role = cricket::CONNECTIONROLE_ACTPASS; - local_desc.connection_role = cricket::CONNECTIONROLE_ACTIVE; + TransportDescription remote_desc = MakeTransportDescription( + kIceUfrag1, kIcePwd1, certificate, CONNECTIONROLE_ACTPASS); + TransportDescription local_desc = MakeTransportDescription( + kIceUfrag2, kIcePwd2, certificate, CONNECTIONROLE_ACTIVE); // Normal initial offer/answer with "actpass" in the offer and "active" in // the answer. @@ -399,7 +576,7 @@ TEST_F(JsepTransportTest, RemoteOfferWithCurrentNegotiatedDtlsRole) { EXPECT_EQ(rtc::SSL_CLIENT, *role); // Subsequent offer with current negotiated role of "passive". - remote_desc.connection_role = cricket::CONNECTIONROLE_PASSIVE; + remote_desc.connection_role = CONNECTIONROLE_PASSIVE; EXPECT_TRUE(transport_->SetRemoteTransportDescription( remote_desc, SdpType::kOffer, nullptr)); EXPECT_TRUE(transport_->SetLocalTransportDescription( @@ -425,8 +602,8 @@ TEST_F(JsepTransportTest, RemoteOfferThatChangesNegotiatedDtlsRole) { remote_desc.identity_fingerprint.reset( TransportDescription::CopyFingerprint(fingerprint.get())); - remote_desc.connection_role = cricket::CONNECTIONROLE_ACTPASS; - local_desc.connection_role = cricket::CONNECTIONROLE_ACTIVE; + remote_desc.connection_role = CONNECTIONROLE_ACTPASS; + local_desc.connection_role = CONNECTIONROLE_ACTIVE; // Normal initial offer/answer with "actpass" in the offer and "active" in // the answer. @@ -444,9 +621,45 @@ TEST_F(JsepTransportTest, RemoteOfferThatChangesNegotiatedDtlsRole) { // endpoint's negotiated role. // TODO(deadbeef): Really this should fail as soon as the offer is // attempted to be applied, and not when the answer is applied. - remote_desc.connection_role = cricket::CONNECTIONROLE_ACTIVE; + remote_desc.connection_role = CONNECTIONROLE_ACTIVE; EXPECT_TRUE(transport_->SetRemoteTransportDescription( remote_desc, SdpType::kOffer, nullptr)); EXPECT_FALSE(transport_->SetLocalTransportDescription( local_desc, SdpType::kAnswer, nullptr)); } + +// Testing that a legacy client that doesn't use the setup attribute will be +// interpreted as having an active role. +TEST_F(JsepTransportTest, TestDtlsSetupWithLegacyAsAnswerer) { + rtc::scoped_refptr certificate = + rtc::RTCCertificate::Create(std::unique_ptr( + rtc::SSLIdentity::Generate("testing", rtc::KT_ECDSA))); + std::unique_ptr fingerprint( + rtc::SSLFingerprint::CreateFromCertificate(certificate)); + transport_->SetLocalCertificate(certificate); + + TransportDescription local_desc(kIceUfrag1, kIcePwd1); + TransportDescription remote_desc(kIceUfrag2, kIcePwd2); + // Just use the same fingerprint in both descriptions; the remote fingerprint + // doesn't matter in a non end-to-end test. + local_desc.identity_fingerprint.reset( + TransportDescription::CopyFingerprint(fingerprint.get())); + remote_desc.identity_fingerprint.reset( + TransportDescription::CopyFingerprint(fingerprint.get())); + + local_desc.connection_role = CONNECTIONROLE_ACTPASS; + ASSERT_TRUE(transport_->SetLocalTransportDescription( + local_desc, SdpType::kOffer, nullptr)); + // Use CONNECTIONROLE_NONE to simulate legacy endpoint. + remote_desc.connection_role = CONNECTIONROLE_NONE; + ASSERT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, SdpType::kAnswer, nullptr)); + + rtc::Optional role = transport_->GetSslRole(); + ASSERT_TRUE(role); + // Since legacy answer ommitted setup atribute, and we offered actpass, we + // should act as passive (server). + EXPECT_EQ(rtc::SSL_SERVER, *role); +} + +} // namespace cricket diff --git a/pc/peerconnectioninterface_unittest.cc b/pc/peerconnectioninterface_unittest.cc index c1168a0a4d..4f1ddb92ff 100644 --- a/pc/peerconnectioninterface_unittest.cc +++ b/pc/peerconnectioninterface_unittest.cc @@ -2128,6 +2128,7 @@ TEST_F(PeerConnectionInterfaceTest, ReceiveFireFoxOffer) { content = cricket::GetFirstDataContent(pc_->local_description()->description()); ASSERT_TRUE(content != NULL); + // Expected to fail since it's using an incompatible format. EXPECT_TRUE(content->rejected); #endif }