From d8cfa1af389d815f6dd1681fcf7f168fc5e61818 Mon Sep 17 00:00:00 2001 From: deadbeef Date: Mon, 27 Mar 2017 10:33:26 -0700 Subject: [PATCH] Accept remote offers with current DTLS role, rather than "actpass". JSEP implementations are required to always generate offers with "actpass", but remote endpoints are not. So we should accept remote offers with the current negotiated DTLS role. This was recently clarified in dtls-sdp; it was somewhat ambiguous before. Also doing a bit of refactoring of JsepTransport (making a method private that should have been private, fixing unit tests that were directly calling said method). BUG=webrtc:7072 Review-Url: https://codereview.webrtc.org/2770903003 Cr-Commit-Position: refs/heads/master@{#17396} --- .../p2p/base/dtlstransportchannel_unittest.cc | 8 +- webrtc/p2p/base/jseptransport.cc | 50 +++-- webrtc/p2p/base/jseptransport.h | 21 +- webrtc/p2p/base/jseptransport_unittest.cc | 204 +++++++++++++++--- webrtc/p2p/base/transportcontroller.cc | 6 +- .../p2p/base/transportcontroller_unittest.cc | 39 +++- 6 files changed, 254 insertions(+), 74 deletions(-) diff --git a/webrtc/p2p/base/dtlstransportchannel_unittest.cc b/webrtc/p2p/base/dtlstransportchannel_unittest.cc index 696135c5b0..17e7a0bcad 100644 --- a/webrtc/p2p/base/dtlstransportchannel_unittest.cc +++ b/webrtc/p2p/base/dtlstransportchannel_unittest.cc @@ -818,12 +818,8 @@ TEST_F(DtlsTransportChannelTest, TestTransferDtlsAnswererIsPassive) { TEST_F(DtlsTransportChannelTest, TestDtlsSetupWithLegacyAsAnswerer) { PrepareDtls(true, true, rtc::KT_DEFAULT); NegotiateWithLegacy(); - rtc::SSLRole channel1_role; - rtc::SSLRole channel2_role; - client1_.transport()->GetSslRole(&channel1_role); - client2_.transport()->GetSslRole(&channel2_role); - EXPECT_EQ(rtc::SSL_SERVER, channel1_role); - EXPECT_EQ(rtc::SSL_CLIENT, channel2_role); + 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 diff --git a/webrtc/p2p/base/jseptransport.cc b/webrtc/p2p/base/jseptransport.cc index 4bc24fd07b..301ae24003 100644 --- a/webrtc/p2p/base/jseptransport.cc +++ b/webrtc/p2p/base/jseptransport.cc @@ -278,9 +278,8 @@ bool JsepTransport::NeedsIceRestart() const { return needs_ice_restart_; } -void JsepTransport::GetSslRole(rtc::SSLRole* ssl_role) const { - RTC_DCHECK(ssl_role); - *ssl_role = secure_role_; +rtc::Optional JsepTransport::GetSslRole() const { + return ssl_role_; } bool JsepTransport::GetStats(TransportStats* stats) { @@ -342,11 +341,6 @@ bool JsepTransport::ApplyLocalTransportDescription( bool JsepTransport::ApplyRemoteTransportDescription( DtlsTransportInternal* dtls_transport, std::string* error_desc) { - // Currently, all ICE-related calls still go through this DTLS channel. But - // that will change once we get rid of TransportChannelImpl, and the DTLS - // channel interface no longer includes ICE-specific methods. Then this class - // will need to call dtls->ice()->SetIceRole(), for example, assuming the Dtls - // interface will expose its inner ICE channel. dtls_transport->ice_transport()->SetRemoteIceParameters( remote_description_->GetIceParameters()); dtls_transport->ice_transport()->SetRemoteIceMode( @@ -359,7 +353,7 @@ bool JsepTransport::ApplyNegotiatedTransportDescription( std::string* error_desc) { // Set SSL role. Role must be set before fingerprint is applied, which // initiates DTLS setup. - if (!dtls_transport->SetSslRole(secure_role_)) { + if (ssl_role_ && !dtls_transport->SetSslRole(*ssl_role_)) { return BadTransportDescription("Failed to set SSL role for the channel.", error_desc); } @@ -374,8 +368,9 @@ bool JsepTransport::ApplyNegotiatedTransportDescription( return true; } -bool JsepTransport::NegotiateTransportDescription(ContentAction local_role, - std::string* error_desc) { +bool JsepTransport::NegotiateTransportDescription( + ContentAction local_description_type, + std::string* error_desc) { if (!local_description_ || !remote_description_) { const std::string msg = "Applying an answer transport description " @@ -388,10 +383,10 @@ bool JsepTransport::NegotiateTransportDescription(ContentAction local_role, remote_description_->identity_fingerprint.get(); if (remote_fp && local_fp) { remote_fingerprint_.reset(new rtc::SSLFingerprint(*remote_fp)); - if (!NegotiateRole(local_role, &secure_role_, error_desc)) { + if (!NegotiateRole(local_description_type, error_desc)) { return false; } - } else if (local_fp && (local_role == CA_ANSWER)) { + } else if (local_fp && (local_description_type == CA_ANSWER)) { return BadTransportDescription( "Local fingerprint supplied when caller didn't offer DTLS.", error_desc); @@ -412,10 +407,8 @@ bool JsepTransport::NegotiateTransportDescription(ContentAction local_role, return true; } -bool JsepTransport::NegotiateRole(ContentAction local_role, - rtc::SSLRole* ssl_role, - std::string* error_desc) const { - RTC_DCHECK(ssl_role); +bool JsepTransport::NegotiateRole(ContentAction local_description_type, + std::string* error_desc) { if (!local_description_ || !remote_description_) { const std::string msg = "Local and Remote description must be set before " @@ -450,7 +443,7 @@ bool JsepTransport::NegotiateRole(ContentAction local_role, ConnectionRole remote_connection_role = remote_description_->connection_role; bool is_remote_server = false; - if (local_role == CA_OFFER) { + if (local_description_type == CA_OFFER) { if (local_connection_role != CONNECTIONROLE_ACTPASS) { return BadTransportDescription( "Offerer must use actpass value for setup attribute.", error_desc); @@ -470,8 +463,23 @@ bool JsepTransport::NegotiateRole(ContentAction local_role, } else { if (remote_connection_role != CONNECTIONROLE_ACTPASS && remote_connection_role != CONNECTIONROLE_NONE) { - return BadTransportDescription( - "Offerer must use actpass value for setup attribute.", error_desc); + // Accept a remote role attribute that's not "actpass", but matches the + // current negotiated role. This is allowed by dtls-sdp, though our + // implementation will never generate such an offer as it's not + // recommended. + // + // See https://datatracker.ietf.org/doc/html/draft-ietf-mmusic-dtls-sdp, + // section 5.5. + if (!ssl_role_ || + (*ssl_role_ == rtc::SSL_CLIENT && + remote_connection_role == CONNECTIONROLE_ACTIVE) || + (*ssl_role_ == rtc::SSL_SERVER && + remote_connection_role == CONNECTIONROLE_PASSIVE)) { + return BadTransportDescription( + "Offerer must use actpass value or current negotiated role for " + "setup attribute.", + error_desc); + } } if (local_connection_role == CONNECTIONROLE_ACTIVE || @@ -487,7 +495,7 @@ bool JsepTransport::NegotiateRole(ContentAction local_role, // If local is passive, local will act as server. } - *ssl_role = is_remote_server ? rtc::SSL_CLIENT : rtc::SSL_SERVER; + ssl_role_.emplace(is_remote_server ? rtc::SSL_CLIENT : rtc::SSL_SERVER); return true; } diff --git a/webrtc/p2p/base/jseptransport.h b/webrtc/p2p/base/jseptransport.h index e29dd0d02a..7b81be06f6 100644 --- a/webrtc/p2p/base/jseptransport.h +++ b/webrtc/p2p/base/jseptransport.h @@ -298,7 +298,9 @@ class JsepTransport : public sigslot::has_slots<> { // changed ufrag/password). bool NeedsIceRestart() const; - void GetSslRole(rtc::SSLRole* ssl_role) const; + // Returns role if negotiated, or empty Optional if it hasn't been negotiated + // yet. + rtc::Optional GetSslRole() const; // TODO(deadbeef): Make this const. See comment in transportcontroller.h. bool GetStats(TransportStats* stats); @@ -325,22 +327,21 @@ class JsepTransport : public sigslot::has_slots<> { const rtc::SSLFingerprint* fingerprint, std::string* error_desc) const; - // Negotiates the SSL role based off the offer and answer as specified by - // RFC 4145, section-4.1. Returns false if the SSL role cannot be determined - // from the local description and remote description. - bool NegotiateRole(ContentAction local_role, - rtc::SSLRole* ssl_role, - std::string* error_desc) const; - private: // Negotiates the transport parameters based on the current local and remote // transport description, such as the ICE role to use, and whether DTLS // should be activated. // // Called when an answer TransportDescription is applied. - bool NegotiateTransportDescription(ContentAction local_role, + bool NegotiateTransportDescription(ContentAction local_description_type, std::string* error_desc); + // Negotiates the SSL role based off the offer and answer as specified by + // RFC 4145, section-4.1. Returns false if the SSL role cannot be determined + // from the local description and remote description. + bool NegotiateRole(ContentAction local_description_type, + std::string* error_desc); + // Pushes down the transport parameters from the local description, such // as the ICE ufrag and pwd. bool ApplyLocalTransportDescription(DtlsTransportInternal* dtls_transport, @@ -360,7 +361,7 @@ class JsepTransport : public sigslot::has_slots<> { // needs-ice-restart bit as described in JSEP. bool needs_ice_restart_ = false; rtc::scoped_refptr certificate_; - rtc::SSLRole secure_role_ = rtc::SSL_CLIENT; + rtc::Optional ssl_role_; std::unique_ptr remote_fingerprint_; std::unique_ptr local_description_; std::unique_ptr remote_description_; diff --git a/webrtc/p2p/base/jseptransport_unittest.cc b/webrtc/p2p/base/jseptransport_unittest.cc index ccb7016e05..23ed4d857d 100644 --- a/webrtc/p2p/base/jseptransport_unittest.cc +++ b/webrtc/p2p/base/jseptransport_unittest.cc @@ -32,16 +32,21 @@ static const char kIcePwd2[] = "TESTICEPWD00000000000002"; class JsepTransportTest : public testing::Test, public sigslot::has_slots<> { public: - JsepTransportTest() - : transport_(new JsepTransport("test content name", nullptr)) {} + 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); } + void DestroyChannel() { transport_->RemoveChannel(1); } + void RecreateTransport() { + transport_.reset(new JsepTransport("test content name", nullptr)); + } + protected: std::unique_ptr fake_dtls_transport_; std::unique_ptr fake_ice_transport_; @@ -167,10 +172,22 @@ TEST_F(JsepTransportTest, TestVerifyCertificateFingerprint) { } } -// Tests that NegotiateRole sets the SSL role correctly. -TEST_F(JsepTransportTest, TestNegotiateRole) { +// Tests the logic of DTLS role negotiation for an initial offer/answer. +TEST_F(JsepTransportTest, DtlsRoleNegotiation) { + 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())); struct NegotiateRoleParams { cricket::ConnectionRole local_role; @@ -179,7 +196,6 @@ TEST_F(JsepTransportTest, TestNegotiateRole) { cricket::ContentAction remote_action; }; - rtc::SSLRole ssl_role; std::string error_desc; // Parameters which set the SSL role to SSL_CLIENT. @@ -194,16 +210,25 @@ TEST_F(JsepTransportTest, TestNegotiateRole) { cricket::CA_OFFER, cricket::CA_PRANSWER}}; for (auto& param : valid_client_params) { + RecreateTransport(); + transport_->SetLocalCertificate(certificate); + local_desc.connection_role = param.local_role; remote_desc.connection_role = param.remote_role; - ASSERT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, param.remote_action, nullptr)); - ASSERT_TRUE(transport_->SetLocalTransportDescription( - local_desc, param.local_action, nullptr)); - EXPECT_TRUE( - transport_->NegotiateRole(param.local_action, &ssl_role, &error_desc)); - EXPECT_EQ(rtc::SSL_CLIENT, ssl_role); + // Set the offer first. + if (param.local_action == cricket::CA_OFFER) { + EXPECT_TRUE(transport_->SetLocalTransportDescription( + local_desc, param.local_action, nullptr)); + EXPECT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, param.remote_action, nullptr)); + } else { + EXPECT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, param.remote_action, nullptr)); + EXPECT_TRUE(transport_->SetLocalTransportDescription( + local_desc, param.local_action, nullptr)); + } + EXPECT_EQ(rtc::SSL_CLIENT, *transport_->GetSslRole()); } // Parameters which set the SSL role to SSL_SERVER. @@ -218,16 +243,25 @@ TEST_F(JsepTransportTest, TestNegotiateRole) { cricket::CA_OFFER, cricket::CA_PRANSWER}}; for (auto& param : valid_server_params) { + RecreateTransport(); + transport_->SetLocalCertificate(certificate); + local_desc.connection_role = param.local_role; remote_desc.connection_role = param.remote_role; - ASSERT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, param.remote_action, nullptr)); - ASSERT_TRUE(transport_->SetLocalTransportDescription( - local_desc, param.local_action, nullptr)); - EXPECT_TRUE( - transport_->NegotiateRole(param.local_action, &ssl_role, &error_desc)); - EXPECT_EQ(rtc::SSL_SERVER, ssl_role); + // Set the offer first. + if (param.local_action == cricket::CA_OFFER) { + EXPECT_TRUE(transport_->SetLocalTransportDescription( + local_desc, param.local_action, nullptr)); + EXPECT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, param.remote_action, nullptr)); + } else { + EXPECT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, param.remote_action, nullptr)); + EXPECT_TRUE(transport_->SetLocalTransportDescription( + local_desc, param.local_action, nullptr)); + } + EXPECT_EQ(rtc::SSL_SERVER, *transport_->GetSslRole()); } // Invalid parameters due to both peers having a duplicate role. @@ -258,15 +292,24 @@ TEST_F(JsepTransportTest, TestNegotiateRole) { cricket::CA_OFFER, cricket::CA_PRANSWER}}; for (auto& param : duplicate_params) { + RecreateTransport(); + transport_->SetLocalCertificate(certificate); + local_desc.connection_role = param.local_role; remote_desc.connection_role = param.remote_role; - ASSERT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, param.remote_action, nullptr)); - ASSERT_TRUE(transport_->SetLocalTransportDescription( - local_desc, param.local_action, nullptr)); - EXPECT_FALSE( - transport_->NegotiateRole(param.local_action, &ssl_role, &error_desc)); + // Set the offer first. + if (param.local_action == cricket::CA_OFFER) { + EXPECT_TRUE(transport_->SetLocalTransportDescription( + local_desc, param.local_action, nullptr)); + EXPECT_FALSE(transport_->SetRemoteTransportDescription( + remote_desc, param.remote_action, nullptr)); + } else { + EXPECT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, param.remote_action, nullptr)); + EXPECT_FALSE(transport_->SetLocalTransportDescription( + local_desc, param.local_action, nullptr)); + } } // Invalid parameters due to the offerer not using ACTPASS. @@ -297,14 +340,113 @@ TEST_F(JsepTransportTest, TestNegotiateRole) { cricket::CA_OFFER, cricket::CA_PRANSWER}}; for (auto& param : offerer_without_actpass_params) { + RecreateTransport(); + transport_->SetLocalCertificate(certificate); + local_desc.connection_role = param.local_role; remote_desc.connection_role = param.remote_role; - ASSERT_TRUE(transport_->SetRemoteTransportDescription( - remote_desc, param.remote_action, nullptr)); - ASSERT_TRUE(transport_->SetLocalTransportDescription( - local_desc, param.local_action, nullptr)); - EXPECT_FALSE( - transport_->NegotiateRole(param.local_action, &ssl_role, &error_desc)); + // Set the offer first. + // TODO(deadbeef): Really this should fail as soon as the offer is + // attempted to be applied, and not when the answer is applied. + if (param.local_action == cricket::CA_OFFER) { + EXPECT_TRUE(transport_->SetLocalTransportDescription( + local_desc, param.local_action, nullptr)); + EXPECT_FALSE(transport_->SetRemoteTransportDescription( + remote_desc, param.remote_action, nullptr)); + } else { + EXPECT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, param.remote_action, nullptr)); + EXPECT_FALSE(transport_->SetLocalTransportDescription( + local_desc, param.local_action, 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) { + 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; + + // Normal initial offer/answer with "actpass" in the offer and "active" in + // the answer. + ASSERT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, cricket::CA_OFFER, nullptr)); + ASSERT_TRUE(transport_->SetLocalTransportDescription( + local_desc, cricket::CA_ANSWER, nullptr)); + + // Sanity check that role was actually negotiated. + rtc::Optional role = transport_->GetSslRole(); + ASSERT_TRUE(role); + EXPECT_EQ(rtc::SSL_CLIENT, *role); + + // Subsequent offer with current negotiated role of "passive". + remote_desc.connection_role = cricket::CONNECTIONROLE_PASSIVE; + EXPECT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, cricket::CA_OFFER, nullptr)); + EXPECT_TRUE(transport_->SetLocalTransportDescription( + local_desc, cricket::CA_ANSWER, nullptr)); +} + +// Test that a remote offer with the inverse of the current negotiated DTLS +// role is rejected. +TEST_F(JsepTransportTest, RemoteOfferThatChangesNegotiatedDtlsRole) { + 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; + + // Normal initial offer/answer with "actpass" in the offer and "active" in + // the answer. + ASSERT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, cricket::CA_OFFER, nullptr)); + ASSERT_TRUE(transport_->SetLocalTransportDescription( + local_desc, cricket::CA_ANSWER, nullptr)); + + // Sanity check that role was actually negotiated. + rtc::Optional role = transport_->GetSslRole(); + ASSERT_TRUE(role); + EXPECT_EQ(rtc::SSL_CLIENT, *role); + + // Subsequent offer with "active", which is the opposite of the remote + // 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; + EXPECT_TRUE(transport_->SetRemoteTransportDescription( + remote_desc, cricket::CA_OFFER, nullptr)); + EXPECT_FALSE(transport_->SetLocalTransportDescription( + local_desc, cricket::CA_ANSWER, nullptr)); +} diff --git a/webrtc/p2p/base/transportcontroller.cc b/webrtc/p2p/base/transportcontroller.cc index 3277ee9702..f06a180088 100644 --- a/webrtc/p2p/base/transportcontroller.cc +++ b/webrtc/p2p/base/transportcontroller.cc @@ -521,7 +521,11 @@ bool TransportController::GetSslRole_n(const std::string& transport_name, if (!t) { return false; } - t->GetSslRole(role); + rtc::Optional current_role = t->GetSslRole(); + if (!current_role) { + return false; + } + *role = *current_role; return true; } diff --git a/webrtc/p2p/base/transportcontroller_unittest.cc b/webrtc/p2p/base/transportcontroller_unittest.cc index 6ea83a4809..44ad54dcbe 100644 --- a/webrtc/p2p/base/transportcontroller_unittest.cc +++ b/webrtc/p2p/base/transportcontroller_unittest.cc @@ -272,13 +272,42 @@ TEST_F(TransportControllerTest, TestIceRoleConflict) { } TEST_F(TransportControllerTest, TestGetSslRole) { - FakeDtlsTransport* transport = CreateFakeDtlsTransport("audio", 1); - ASSERT_NE(nullptr, transport); - ASSERT_TRUE(transport->SetSslRole(rtc::SSL_CLIENT)); rtc::SSLRole role; - EXPECT_FALSE(transport_controller_->GetSslRole("video", &role)); + CreateFakeDtlsTransport("audio", 1); + + // Should return false before role has been negotiated. + EXPECT_FALSE(transport_controller_->GetSslRole("audio", &role)); + + // To negotiate an SSL role, need to set a local certificate, and + // local/remote transport descriptions with DTLS info. + 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_controller_->SetLocalCertificate(certificate); + + // Set the same fingerprint on both sides since the remote fingerprint + // doesn't really matter for this test. + TransportDescription local_desc(std::vector(), kIceUfrag1, + kIcePwd1, ICEMODE_FULL, + CONNECTIONROLE_ACTPASS, fingerprint.get()); + TransportDescription remote_desc(std::vector(), kIceUfrag2, + kIcePwd2, ICEMODE_FULL, + CONNECTIONROLE_ACTIVE, fingerprint.get()); + std::string err; + EXPECT_TRUE(transport_controller_->SetLocalTransportDescription( + "audio", local_desc, cricket::CA_OFFER, &err)); + EXPECT_TRUE(transport_controller_->SetRemoteTransportDescription( + "audio", remote_desc, cricket::CA_ANSWER, &err)); + + // Finally we can get the role. Should be "server" since the remote + // endpoint's role was "active". EXPECT_TRUE(transport_controller_->GetSslRole("audio", &role)); - EXPECT_EQ(rtc::SSL_CLIENT, role); + EXPECT_EQ(rtc::SSL_SERVER, role); + + // Lastly, test that GetSslRole returns false for a nonexistent transport. + EXPECT_FALSE(transport_controller_->GetSslRole("video", &role)); } TEST_F(TransportControllerTest, TestSetAndGetLocalCertificate) {