diff --git a/talk/app/webrtc/peerconnectioninterface.h b/talk/app/webrtc/peerconnectioninterface.h index b3a56006d4..ee81f7c916 100644 --- a/talk/app/webrtc/peerconnectioninterface.h +++ b/talk/app/webrtc/peerconnectioninterface.h @@ -81,6 +81,7 @@ #include "talk/app/webrtc/umametrics.h" #include "webrtc/base/fileutils.h" #include "webrtc/base/network.h" +#include "webrtc/base/rtccertificate.h" #include "webrtc/base/sslstreamadapter.h" #include "webrtc/base/socketaddress.h" @@ -230,6 +231,7 @@ class PeerConnectionInterface : public rtc::RefCountInterface { kTcpCandidatePolicyDisabled }; + // TODO(hbos): Change into class with private data and public getters. struct RTCConfiguration { // TODO(pthatcher): Rename this ice_transport_type, but update // Chromium at the same time. @@ -245,6 +247,7 @@ class PeerConnectionInterface : public rtc::RefCountInterface { TcpCandidatePolicy tcp_candidate_policy; int audio_jitter_buffer_max_packets; bool audio_jitter_buffer_fast_accelerate; + std::vector> certificates; RTCConfiguration() : type(kAll), diff --git a/talk/app/webrtc/test/fakedtlsidentitystore.h b/talk/app/webrtc/test/fakedtlsidentitystore.h index 5ef19004a0..5d7743de4f 100644 --- a/talk/app/webrtc/test/fakedtlsidentitystore.h +++ b/talk/app/webrtc/test/fakedtlsidentitystore.h @@ -32,6 +32,7 @@ #include "talk/app/webrtc/dtlsidentitystore.h" #include "talk/app/webrtc/peerconnectioninterface.h" +#include "webrtc/base/rtccertificate.h" static const char kRSA_PRIVATE_KEY_PEM[] = "-----BEGIN RSA PRIVATE KEY-----\n" @@ -88,6 +89,26 @@ class FakeDtlsIdentityStore : public webrtc::DtlsIdentityStoreInterface, this, should_fail_ ? MSG_FAILURE : MSG_SUCCESS, msg); } + static rtc::scoped_refptr GenerateCertificate() { + std::string cert; + std::string key; + rtc::SSLIdentity::PemToDer("CERTIFICATE", kCERT_PEM, &cert); + rtc::SSLIdentity::PemToDer("RSA PRIVATE KEY", kRSA_PRIVATE_KEY_PEM, &key); + + std::string pem_cert = rtc::SSLIdentity::DerToPem( + rtc::kPemTypeCertificate, + reinterpret_cast(cert.data()), + cert.length()); + std::string pem_key = rtc::SSLIdentity::DerToPem( + rtc::kPemTypeRsaPrivateKey, + reinterpret_cast(key.data()), + key.length()); + rtc::scoped_ptr identity( + rtc::SSLIdentity::FromPEMStrings(pem_key, pem_cert)); + + return rtc::RTCCertificate::Create(identity.Pass()); + } + private: enum { MSG_SUCCESS, diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index be567b88b4..3d192c22d1 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -576,6 +576,15 @@ bool WebRtcSession::Initialize( rtcp_mux_policy_ = rtc_configuration.rtcp_mux_policy; SetSslMaxProtocolVersion(options.ssl_max_version); + // Obtain a certificate from RTCConfiguration if any were provided (optional). + rtc::scoped_refptr certificate; + if (!rtc_configuration.certificates.empty()) { + // TODO(hbos,torbjorng): Decide on certificate-selection strategy instead of + // just picking the first one. The decision should be made based on the DTLS + // handshake. The DTLS negotiations need to know about all certificates. + certificate = rtc_configuration.certificates[0]; + } + // TODO(perkj): Take |constraints| into consideration. Return false if not all // mandatory constraints can be fulfilled. Note that |constraints| // can be null. @@ -584,13 +593,13 @@ bool WebRtcSession::Initialize( if (options.disable_encryption) { dtls_enabled_ = false; } else { - // Enable DTLS by default if we have a |dtls_identity_store|. - dtls_enabled_ = (dtls_identity_store != nullptr); + // Enable DTLS by default if we have an identity store or a certificate. + dtls_enabled_ = (dtls_identity_store || certificate); // |constraints| can override the default |dtls_enabled_| value. if (FindConstraint( constraints, MediaConstraintsInterface::kEnableDtlsSrtp, - &value, NULL)) { + &value, nullptr)) { dtls_enabled_ = value; } } @@ -707,15 +716,40 @@ bool WebRtcSession::Initialize( channel_manager_->SetDefaultVideoEncoderConfig( cricket::VideoEncoderConfig(default_codec)); - webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory( - signaling_thread(), - channel_manager_, - mediastream_signaling_, - dtls_identity_store.Pass(), - this, - id(), - data_channel_type_, - dtls_enabled_)); + if (!dtls_enabled_) { + // Construct with DTLS disabled. + webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory( + signaling_thread(), + channel_manager_, + mediastream_signaling_, + this, + id(), + data_channel_type_)); + } else { + // Construct with DTLS enabled. + if (!certificate) { + // Use the |dtls_identity_store| to generate a certificate. + DCHECK(dtls_identity_store); + webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory( + signaling_thread(), + channel_manager_, + mediastream_signaling_, + dtls_identity_store.Pass(), + this, + id(), + data_channel_type_)); + } else { + // Use the already generated certificate. + webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory( + signaling_thread(), + channel_manager_, + mediastream_signaling_, + certificate, + this, + id(), + data_channel_type_)); + } + } webrtc_session_desc_factory_->SignalIdentityReady.connect( this, &WebRtcSession::OnIdentityReady); @@ -1362,8 +1396,8 @@ void WebRtcSession::OnIdentityReady(rtc::SSLIdentity* identity) { SetIdentity(identity); } -bool WebRtcSession::waiting_for_identity() const { - return webrtc_session_desc_factory_->waiting_for_identity(); +bool WebRtcSession::waiting_for_identity_for_testing() const { + return webrtc_session_desc_factory_->waiting_for_certificate_for_testing(); } void WebRtcSession::SetIceConnectionState( diff --git a/talk/app/webrtc/webrtcsession.h b/talk/app/webrtc/webrtcsession.h index 47b49b661a..b580d3439b 100644 --- a/talk/app/webrtc/webrtcsession.h +++ b/talk/app/webrtc/webrtcsession.h @@ -252,7 +252,7 @@ class WebRtcSession : public cricket::BaseSession, void OnDtlsSetupFailure(cricket::BaseChannel*, bool rtcp); // For unit test. - bool waiting_for_identity() const; + bool waiting_for_identity_for_testing() const; void set_metrics_observer( webrtc::MetricsObserverInterface* metrics_observer) { diff --git a/talk/app/webrtc/webrtcsession_unittest.cc b/talk/app/webrtc/webrtcsession_unittest.cc index 1e1785fe57..a23130db50 100644 --- a/talk/app/webrtc/webrtcsession_unittest.cc +++ b/talk/app/webrtc/webrtcsession_unittest.cc @@ -156,13 +156,15 @@ static const char kSdpWithRtx[] = "a=rtpmap:96 rtx/90000\r\n" "a=fmtp:96 apt=0\r\n"; +enum RTCCertificateGenerationMethod { ALREADY_GENERATED, DTLS_IDENTITY_STORE }; + // Add some extra |newlines| to the |message| after |line|. static void InjectAfter(const std::string& line, const std::string& newlines, std::string* message) { const std::string tmp = line + newlines; rtc::replace_substrs(line.c_str(), line.length(), - tmp.c_str(), tmp.length(), message); + tmp.c_str(), tmp.length(), message); } class MockIceObserver : public webrtc::IceObserver { @@ -310,7 +312,8 @@ class FakeAudioRenderer : public cricket::AudioRenderer { cricket::AudioRenderer::Sink* sink_; }; -class WebRtcSessionTest : public testing::Test { +class WebRtcSessionTest + : public testing::TestWithParam { protected: // TODO Investigate why ChannelManager crashes, if it's created // after stun_server. @@ -352,6 +355,11 @@ class WebRtcSessionTest : public testing::Test { network_manager_.AddInterface(addr); } + // If |dtls_identity_store| != null or |rtc_configuration| contains + // |certificates| then DTLS will be enabled unless explicitly disabled by + // |rtc_configuration| options. When DTLS is enabled a certificate will be + // used if provided, otherwise one will be generated using the + // |dtls_identity_store|. void Init( rtc::scoped_ptr dtls_identity_store, const PeerConnectionInterface::RTCConfiguration& rtc_configuration) { @@ -399,10 +407,28 @@ class WebRtcSessionTest : public testing::Test { Init(nullptr, configuration); } - void InitWithDtls(bool identity_request_should_fail = false) { + // Successfully init with DTLS; with a certificate generated and supplied or + // with a store that generates it for us. + void InitWithDtls(RTCCertificateGenerationMethod cert_gen_method) { + rtc::scoped_ptr dtls_identity_store; + PeerConnectionInterface::RTCConfiguration configuration; + if (cert_gen_method == ALREADY_GENERATED) { + configuration.certificates.push_back( + FakeDtlsIdentityStore::GenerateCertificate()); + } else if (cert_gen_method == DTLS_IDENTITY_STORE) { + dtls_identity_store.reset(new FakeDtlsIdentityStore()); + dtls_identity_store->set_should_fail(false); + } else { + CHECK(false); + } + Init(dtls_identity_store.Pass(), configuration); + } + + // Init with DTLS with a store that will fail to generate a certificate. + void InitWithDtlsIdentityGenFail() { rtc::scoped_ptr dtls_identity_store( new FakeDtlsIdentityStore()); - dtls_identity_store->set_should_fail(identity_request_should_fail); + dtls_identity_store->set_should_fail(true); PeerConnectionInterface::RTCConfiguration configuration; Init(dtls_identity_store.Pass(), configuration); } @@ -1181,8 +1207,21 @@ class WebRtcSessionTest : public testing::Test { } void VerifyMultipleAsyncCreateDescription( + RTCCertificateGenerationMethod cert_gen_method, + CreateSessionDescriptionRequest::Type type) { + InitWithDtls(cert_gen_method); + VerifyMultipleAsyncCreateDescriptionAfterInit(true, type); + } + + void VerifyMultipleAsyncCreateDescriptionIdentityGenFailure( + CreateSessionDescriptionRequest::Type type) { + InitWithDtlsIdentityGenFail(); + VerifyMultipleAsyncCreateDescriptionAfterInit(false, type); + } + + void VerifyMultipleAsyncCreateDescriptionAfterInit( bool success, CreateSessionDescriptionRequest::Type type) { - InitWithDtls(!success); + CHECK(session_); SetFactoryDtlsSrtp(); if (type == CreateSessionDescriptionRequest::kAnswer) { cricket::MediaSessionOptions options; @@ -1256,8 +1295,8 @@ class WebRtcSessionTest : public testing::Test { rtc::scoped_refptr metrics_observer_; }; -TEST_F(WebRtcSessionTest, TestInitializeWithDtls) { - InitWithDtls(); +TEST_P(WebRtcSessionTest, TestInitializeWithDtls) { + InitWithDtls(GetParam()); // SDES is disabled when DTLS is on. EXPECT_EQ(cricket::SEC_DISABLED, session_->SdesPolicy()); } @@ -1564,10 +1603,10 @@ TEST_F(WebRtcSessionTest, TestSetRemoteNonSdesAnswerWhenSdesOn) { // Test that we accept an offer with a DTLS fingerprint when DTLS is on // and that we return an answer with a DTLS fingerprint. -TEST_F(WebRtcSessionTest, TestReceiveDtlsOfferCreateDtlsAnswer) { +TEST_P(WebRtcSessionTest, TestReceiveDtlsOfferCreateDtlsAnswer) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); mediastream_signaling_.SendAudioVideoStream1(); - InitWithDtls(); + InitWithDtls(GetParam()); SetFactoryDtlsSrtp(); cricket::MediaSessionOptions options; options.recv_video = true; @@ -1593,10 +1632,10 @@ TEST_F(WebRtcSessionTest, TestReceiveDtlsOfferCreateDtlsAnswer) { // Test that we set a local offer with a DTLS fingerprint when DTLS is on // and then we accept a remote answer with a DTLS fingerprint successfully. -TEST_F(WebRtcSessionTest, TestCreateDtlsOfferReceiveDtlsAnswer) { +TEST_P(WebRtcSessionTest, TestCreateDtlsOfferReceiveDtlsAnswer) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); mediastream_signaling_.SendAudioVideoStream1(); - InitWithDtls(); + InitWithDtls(GetParam()); SetFactoryDtlsSrtp(); // Verify that we get a crypto fingerprint in the answer. @@ -1623,9 +1662,9 @@ TEST_F(WebRtcSessionTest, TestCreateDtlsOfferReceiveDtlsAnswer) { // Test that if we support DTLS and the other side didn't offer a fingerprint, // we will fail to set the remote description. -TEST_F(WebRtcSessionTest, TestReceiveNonDtlsOfferWhenDtlsOn) { +TEST_P(WebRtcSessionTest, TestReceiveNonDtlsOfferWhenDtlsOn) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - InitWithDtls(); + InitWithDtls(GetParam()); cricket::MediaSessionOptions options; options.recv_video = true; options.bundle_enabled = true; @@ -1647,9 +1686,9 @@ TEST_F(WebRtcSessionTest, TestReceiveNonDtlsOfferWhenDtlsOn) { // Test that we return a failure when applying a local answer that doesn't have // a DTLS fingerprint when DTLS is required. -TEST_F(WebRtcSessionTest, TestSetLocalNonDtlsAnswerWhenDtlsOn) { +TEST_P(WebRtcSessionTest, TestSetLocalNonDtlsAnswerWhenDtlsOn) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - InitWithDtls(); + InitWithDtls(GetParam()); SessionDescriptionInterface* offer = NULL; SessionDescriptionInterface* answer = NULL; CreateDtlsOfferAndNonDtlsAnswer(&offer, &answer); @@ -1663,11 +1702,11 @@ TEST_F(WebRtcSessionTest, TestSetLocalNonDtlsAnswerWhenDtlsOn) { // Test that we return a failure when applying a remote answer that doesn't have // a DTLS fingerprint when DTLS is required. -TEST_F(WebRtcSessionTest, TestSetRemoteNonDtlsAnswerWhenDtlsOn) { +TEST_P(WebRtcSessionTest, TestSetRemoteNonDtlsAnswerWhenDtlsOn) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); // Enable both SDES and DTLS, so that offer won't be outright rejected as a // result of using the "UDP/TLS/RTP/SAVPF" profile. - InitWithDtls(); + InitWithDtls(GetParam()); session_->SetSdesPolicy(cricket::SEC_ENABLED); SessionDescriptionInterface* offer = CreateOffer(); cricket::MediaSessionOptions options; @@ -1684,10 +1723,10 @@ TEST_F(WebRtcSessionTest, TestSetRemoteNonDtlsAnswerWhenDtlsOn) { // Test that we create a local offer without SDES or DTLS and accept a remote // answer without SDES or DTLS when encryption is disabled. -TEST_F(WebRtcSessionTest, TestCreateOfferReceiveAnswerWithoutEncryption) { +TEST_P(WebRtcSessionTest, TestCreateOfferReceiveAnswerWithoutEncryption) { mediastream_signaling_.SendAudioVideoStream1(); options_.disable_encryption = true; - InitWithDtls(); + InitWithDtls(GetParam()); // Verify that we get a crypto fingerprint in the answer. SessionDescriptionInterface* offer = CreateOffer(); @@ -1713,9 +1752,9 @@ TEST_F(WebRtcSessionTest, TestCreateOfferReceiveAnswerWithoutEncryption) { // Test that we create a local answer without SDES or DTLS and accept a remote // offer without SDES or DTLS when encryption is disabled. -TEST_F(WebRtcSessionTest, TestCreateAnswerReceiveOfferWithoutEncryption) { +TEST_P(WebRtcSessionTest, TestCreateAnswerReceiveOfferWithoutEncryption) { options_.disable_encryption = true; - InitWithDtls(); + InitWithDtls(GetParam()); cricket::MediaSessionOptions options; options.recv_video = true; @@ -3411,7 +3450,7 @@ TEST_F(WebRtcSessionTest, TestRtpDataChannel) { EXPECT_EQ(cricket::DCT_RTP, data_engine_->last_channel_type()); } -TEST_F(WebRtcSessionTest, TestRtpDataChannelConstraintTakesPrecedence) { +TEST_P(WebRtcSessionTest, TestRtpDataChannelConstraintTakesPrecedence) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); constraints_.reset(new FakeConstraints()); @@ -3419,26 +3458,26 @@ TEST_F(WebRtcSessionTest, TestRtpDataChannelConstraintTakesPrecedence) { webrtc::MediaConstraintsInterface::kEnableRtpDataChannels, true); options_.disable_sctp_data_channels = false; - InitWithDtls(); + InitWithDtls(GetParam()); SetLocalDescriptionWithDataChannel(); EXPECT_EQ(cricket::DCT_RTP, data_engine_->last_channel_type()); } -TEST_F(WebRtcSessionTest, TestCreateOfferWithSctpEnabledWithoutStreams) { +TEST_P(WebRtcSessionTest, TestCreateOfferWithSctpEnabledWithoutStreams) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - InitWithDtls(); + InitWithDtls(GetParam()); rtc::scoped_ptr offer(CreateOffer()); EXPECT_TRUE(offer->description()->GetContentByName("data") == NULL); EXPECT_TRUE(offer->description()->GetTransportInfoByName("data") == NULL); } -TEST_F(WebRtcSessionTest, TestCreateAnswerWithSctpInOfferAndNoStreams) { +TEST_P(WebRtcSessionTest, TestCreateAnswerWithSctpInOfferAndNoStreams) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); SetFactoryDtlsSrtp(); - InitWithDtls(); + InitWithDtls(GetParam()); // Create remote offer with SCTP. cricket::MediaSessionOptions options; @@ -3454,40 +3493,40 @@ TEST_F(WebRtcSessionTest, TestCreateAnswerWithSctpInOfferAndNoStreams) { EXPECT_TRUE(answer->description()->GetTransportInfoByName("data") != NULL); } -TEST_F(WebRtcSessionTest, TestSctpDataChannelWithoutDtls) { +TEST_P(WebRtcSessionTest, TestSctpDataChannelWithoutDtls) { constraints_.reset(new FakeConstraints()); constraints_->AddOptional( webrtc::MediaConstraintsInterface::kEnableDtlsSrtp, false); - InitWithDtls(); + InitWithDtls(GetParam()); SetLocalDescriptionWithDataChannel(); EXPECT_EQ(cricket::DCT_NONE, data_engine_->last_channel_type()); } -TEST_F(WebRtcSessionTest, TestSctpDataChannelWithDtls) { +TEST_P(WebRtcSessionTest, TestSctpDataChannelWithDtls) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - InitWithDtls(); + InitWithDtls(GetParam()); SetLocalDescriptionWithDataChannel(); EXPECT_EQ(cricket::DCT_SCTP, data_engine_->last_channel_type()); } -TEST_F(WebRtcSessionTest, TestDisableSctpDataChannels) { +TEST_P(WebRtcSessionTest, TestDisableSctpDataChannels) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); options_.disable_sctp_data_channels = true; - InitWithDtls(); + InitWithDtls(GetParam()); SetLocalDescriptionWithDataChannel(); EXPECT_EQ(cricket::DCT_NONE, data_engine_->last_channel_type()); } -TEST_F(WebRtcSessionTest, TestSctpDataChannelSendPortParsing) { +TEST_P(WebRtcSessionTest, TestSctpDataChannelSendPortParsing) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); const int new_send_port = 9998; const int new_recv_port = 7775; - InitWithDtls(); + InitWithDtls(GetParam()); SetFactoryDtlsSrtp(); // By default, don't actually add the codecs to desc_factory_; they don't @@ -3536,13 +3575,29 @@ TEST_F(WebRtcSessionTest, TestSctpDataChannelSendPortParsing) { EXPECT_EQ(new_recv_port, portnum); } -// Verifies that CreateOffer succeeds when CreateOffer is called before async -// identity generation is finished. -TEST_F(WebRtcSessionTest, TestCreateOfferBeforeIdentityRequestReturnSuccess) { - MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - InitWithDtls(); +// TODO(hbos): Add the following test once RTCCertificate is passed around +// outside of WebRtcSessionDescriptionFactory code and there exists a +// WebRtcSession::certificate(). +//TEST_F(WebRtcSessionTest, TestUsesProvidedCertificate) { +// rtc::scoped_refptr certificate = +// FakeDtlsIdentityStore::GenerateCertificate(); +// +// PeerConnectionInterface::RTCConfiguration configuration; +// configuration.certificates.push_back(certificate); +// Init(nullptr, configuration); +// EXPECT_TRUE_WAIT(!session_->waiting_for_identity_for_testing(), 1000); +// +// EXPECT_EQ(session_->certificate(), certificate); +//} - EXPECT_TRUE(session_->waiting_for_identity()); +// Verifies that CreateOffer succeeds when CreateOffer is called before async +// identity generation is finished (even if a certificate is provided this is +// an async op). +TEST_P(WebRtcSessionTest, TestCreateOfferBeforeIdentityRequestReturnSuccess) { + MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); + InitWithDtls(GetParam()); + + EXPECT_TRUE(session_->waiting_for_identity_for_testing()); mediastream_signaling_.SendAudioVideoStream1(); rtc::scoped_ptr offer(CreateOffer()); @@ -3552,10 +3607,11 @@ TEST_F(WebRtcSessionTest, TestCreateOfferBeforeIdentityRequestReturnSuccess) { } // Verifies that CreateAnswer succeeds when CreateOffer is called before async -// identity generation is finished. -TEST_F(WebRtcSessionTest, TestCreateAnswerBeforeIdentityRequestReturnSuccess) { +// identity generation is finished (even if a certificate is provided this is +// an async op). +TEST_P(WebRtcSessionTest, TestCreateAnswerBeforeIdentityRequestReturnSuccess) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - InitWithDtls(); + InitWithDtls(GetParam()); SetFactoryDtlsSrtp(); cricket::MediaSessionOptions options; @@ -3572,12 +3628,13 @@ TEST_F(WebRtcSessionTest, TestCreateAnswerBeforeIdentityRequestReturnSuccess) { } // Verifies that CreateOffer succeeds when CreateOffer is called after async -// identity generation is finished. -TEST_F(WebRtcSessionTest, TestCreateOfferAfterIdentityRequestReturnSuccess) { +// identity generation is finished (even if a certificate is provided this is +// an async op). +TEST_P(WebRtcSessionTest, TestCreateOfferAfterIdentityRequestReturnSuccess) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - InitWithDtls(); + InitWithDtls(GetParam()); - EXPECT_TRUE_WAIT(!session_->waiting_for_identity(), 1000); + EXPECT_TRUE_WAIT(!session_->waiting_for_identity_for_testing(), 1000); rtc::scoped_ptr offer(CreateOffer()); EXPECT_TRUE(offer != NULL); @@ -3587,9 +3644,9 @@ TEST_F(WebRtcSessionTest, TestCreateOfferAfterIdentityRequestReturnSuccess) { // identity generation fails. TEST_F(WebRtcSessionTest, TestCreateOfferAfterIdentityRequestReturnFailure) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - InitWithDtls(true); + InitWithDtlsIdentityGenFail(); - EXPECT_TRUE_WAIT(!session_->waiting_for_identity(), 1000); + EXPECT_TRUE_WAIT(!session_->waiting_for_identity_for_testing(), 1000); rtc::scoped_ptr offer(CreateOffer()); EXPECT_TRUE(offer == NULL); @@ -3597,11 +3654,11 @@ TEST_F(WebRtcSessionTest, TestCreateOfferAfterIdentityRequestReturnFailure) { // Verifies that CreateOffer succeeds when Multiple CreateOffer calls are made // before async identity generation is finished. -TEST_F(WebRtcSessionTest, +TEST_P(WebRtcSessionTest, TestMultipleCreateOfferBeforeIdentityRequestReturnSuccess) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); VerifyMultipleAsyncCreateDescription( - true, CreateSessionDescriptionRequest::kOffer); + GetParam(), CreateSessionDescriptionRequest::kOffer); } // Verifies that CreateOffer fails when Multiple CreateOffer calls are made @@ -3609,17 +3666,17 @@ TEST_F(WebRtcSessionTest, TEST_F(WebRtcSessionTest, TestMultipleCreateOfferBeforeIdentityRequestReturnFailure) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - VerifyMultipleAsyncCreateDescription( - false, CreateSessionDescriptionRequest::kOffer); + VerifyMultipleAsyncCreateDescriptionIdentityGenFailure( + CreateSessionDescriptionRequest::kOffer); } // Verifies that CreateAnswer succeeds when Multiple CreateAnswer calls are made // before async identity generation is finished. -TEST_F(WebRtcSessionTest, +TEST_P(WebRtcSessionTest, TestMultipleCreateAnswerBeforeIdentityRequestReturnSuccess) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); VerifyMultipleAsyncCreateDescription( - true, CreateSessionDescriptionRequest::kAnswer); + GetParam(), CreateSessionDescriptionRequest::kAnswer); } // Verifies that CreateAnswer fails when Multiple CreateAnswer calls are made @@ -3627,8 +3684,8 @@ TEST_F(WebRtcSessionTest, TEST_F(WebRtcSessionTest, TestMultipleCreateAnswerBeforeIdentityRequestReturnFailure) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - VerifyMultipleAsyncCreateDescription( - false, CreateSessionDescriptionRequest::kAnswer); + VerifyMultipleAsyncCreateDescriptionIdentityGenFailure( + CreateSessionDescriptionRequest::kAnswer); } // Verifies that setRemoteDescription fails when DTLS is disabled and the remote @@ -3729,9 +3786,9 @@ TEST_F(WebRtcSessionTest, TestCombinedAudioVideoBweConstraint) { // Tests that we can renegotiate new media content with ICE candidates in the // new remote SDP. -TEST_F(WebRtcSessionTest, TestRenegotiateNewMediaWithCandidatesInSdp) { +TEST_P(WebRtcSessionTest, TestRenegotiateNewMediaWithCandidatesInSdp) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - InitWithDtls(); + InitWithDtls(GetParam()); SetFactoryDtlsSrtp(); mediastream_signaling_.UseOptionsAudioOnly(); @@ -3759,9 +3816,9 @@ TEST_F(WebRtcSessionTest, TestRenegotiateNewMediaWithCandidatesInSdp) { // Tests that we can renegotiate new media content with ICE candidates separated // from the remote SDP. -TEST_F(WebRtcSessionTest, TestRenegotiateNewMediaWithCandidatesSeparated) { +TEST_P(WebRtcSessionTest, TestRenegotiateNewMediaWithCandidatesSeparated) { MAYBE_SKIP_TEST(rtc::SSLStreamAdapter::HaveDtlsSrtp); - InitWithDtls(); + InitWithDtls(GetParam()); SetFactoryDtlsSrtp(); mediastream_signaling_.UseOptionsAudioOnly(); @@ -3875,11 +3932,6 @@ TEST_F(WebRtcSessionTest, CreateOffersAndShutdown) { session_.reset(); - // Make sure we process pending messages on the current (signaling) thread - // before checking we we got our callbacks. Quit() will do this and then - // immediately exit. We won't need the queue after this point anyway. - rtc::Thread::Current()->Quit(); - for (auto& o : observers) { // We expect to have received a notification now even if the session was // terminated. The offer creation may or may not have succeeded, but we @@ -3892,3 +3944,7 @@ TEST_F(WebRtcSessionTest, CreateOffersAndShutdown) { // TODO(bemasc): Add a TestIceStatesBundle with BUNDLE enabled. That test // currently fails because upon disconnection and reconnection OnIceComplete is // called more than once without returning to IceGatheringGathering. + +INSTANTIATE_TEST_CASE_P( + WebRtcSessionTests, WebRtcSessionTest, + testing::Values(ALREADY_GENERATED, DTLS_IDENTITY_STORE)); diff --git a/talk/app/webrtc/webrtcsessiondescriptionfactory.cc b/talk/app/webrtc/webrtcsessiondescriptionfactory.cc index 6c6981ce5a..4336372b57 100644 --- a/talk/app/webrtc/webrtcsessiondescriptionfactory.cc +++ b/talk/app/webrtc/webrtcsessiondescriptionfactory.cc @@ -68,7 +68,8 @@ static bool ValidStreams(const MediaSessionOptions::Streams& streams) { enum { MSG_CREATE_SESSIONDESCRIPTION_SUCCESS, - MSG_CREATE_SESSIONDESCRIPTION_FAILED + MSG_CREATE_SESSIONDESCRIPTION_FAILED, + MSG_USE_CONSTRUCTOR_CERTIFICATE }; struct CreateSessionDescriptionMsg : public rtc::MessageData { @@ -126,11 +127,14 @@ void WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription( } } +// Private constructor called by other constructors. WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( rtc::Thread* signaling_thread, cricket::ChannelManager* channel_manager, MediaStreamSignaling* mediastream_signaling, rtc::scoped_ptr dtls_identity_store, + const rtc::scoped_refptr& + identity_request_observer, WebRtcSession* session, const std::string& session_id, cricket::DataChannelType dct, @@ -144,31 +148,87 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( // |kInitSessionVersion|. session_version_(kInitSessionVersion), dtls_identity_store_(dtls_identity_store.Pass()), + identity_request_observer_(identity_request_observer), session_(session), session_id_(session_id), data_channel_type_(dct), - identity_request_state_(IDENTITY_NOT_NEEDED) { + certificate_request_state_(CERTIFICATE_NOT_NEEDED) { session_desc_factory_.set_add_legacy_streams(false); // SRTP-SDES is disabled if DTLS is on. SetSdesPolicy(dtls_enabled ? cricket::SEC_DISABLED : cricket::SEC_REQUIRED); +} - // If |dtls_enabled| we must have a |dtls_identity_store_|. - DCHECK(!dtls_enabled || dtls_identity_store_); +WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( + rtc::Thread* signaling_thread, + cricket::ChannelManager* channel_manager, + MediaStreamSignaling* mediastream_signaling, + WebRtcSession* session, + const std::string& session_id, + cricket::DataChannelType dct) + : WebRtcSessionDescriptionFactory( + signaling_thread, channel_manager, mediastream_signaling, nullptr, + nullptr, session, session_id, dct, false) { + LOG(LS_VERBOSE) << "DTLS-SRTP disabled."; +} - if (dtls_enabled && dtls_identity_store_) { - identity_request_observer_ = - new rtc::RefCountedObject(); +WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( + rtc::Thread* signaling_thread, + cricket::ChannelManager* channel_manager, + MediaStreamSignaling* mediastream_signaling, + rtc::scoped_ptr dtls_identity_store, + WebRtcSession* session, + const std::string& session_id, + cricket::DataChannelType dct) + : WebRtcSessionDescriptionFactory( + signaling_thread, + channel_manager, + mediastream_signaling, + dtls_identity_store.Pass(), + new rtc::RefCountedObject(), + session, + session_id, + dct, + true) { + DCHECK(dtls_identity_store_); - identity_request_observer_->SignalRequestFailed.connect( - this, &WebRtcSessionDescriptionFactory::OnIdentityRequestFailed); - identity_request_observer_->SignalIdentityReady.connect( - this, &WebRtcSessionDescriptionFactory::SetIdentity); + certificate_request_state_ = CERTIFICATE_WAITING; - LOG(LS_VERBOSE) << "DTLS-SRTP enabled; sending DTLS identity request."; - identity_request_state_ = IDENTITY_WAITING; - dtls_identity_store_->RequestIdentity(rtc::KT_DEFAULT, - identity_request_observer_); - } + identity_request_observer_->SignalRequestFailed.connect( + this, &WebRtcSessionDescriptionFactory::OnIdentityRequestFailed); + identity_request_observer_->SignalIdentityReady.connect( + this, &WebRtcSessionDescriptionFactory::SetIdentity); + + rtc::KeyType key_type = rtc::KT_DEFAULT; + LOG(LS_VERBOSE) << "DTLS-SRTP enabled; sending DTLS identity request (key " + << "type: " << key_type << ")."; + + // Request identity. This happens asynchronously, so the caller will have a + // chance to connect to SignalIdentityReady. + dtls_identity_store_->RequestIdentity(key_type, identity_request_observer_); +} + +WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( + rtc::Thread* signaling_thread, + cricket::ChannelManager* channel_manager, + MediaStreamSignaling* mediastream_signaling, + const rtc::scoped_refptr& certificate, + WebRtcSession* session, + const std::string& session_id, + cricket::DataChannelType dct) + : WebRtcSessionDescriptionFactory( + signaling_thread, channel_manager, mediastream_signaling, nullptr, + nullptr, session, session_id, dct, true) { + DCHECK(certificate); + + certificate_request_state_ = CERTIFICATE_WAITING; + + LOG(LS_VERBOSE) << "DTLS-SRTP enabled; has certificate parameter."; + // We already have a certificate but we wait to do SetIdentity; if we do + // it in the constructor then the caller has not had a chance to connect to + // SignalIdentityReady. + signaling_thread_->Post(this, MSG_USE_CONSTRUCTOR_CERTIFICATE, + new rtc::ScopedRefMessageData( + certificate)); } WebRtcSessionDescriptionFactory::~WebRtcSessionDescriptionFactory() { @@ -181,8 +241,19 @@ WebRtcSessionDescriptionFactory::~WebRtcSessionDescriptionFactory() { // this, requests will linger and not know they succeeded or failed. rtc::MessageList list; signaling_thread_->Clear(this, rtc::MQID_ANY, &list); - for (auto& msg : list) - OnMessage(&msg); + for (auto& msg : list) { + if (msg.message_id != MSG_USE_CONSTRUCTOR_CERTIFICATE) { + OnMessage(&msg); + } else { + // Skip MSG_USE_CONSTRUCTOR_CERTIFICATE because we don't want to trigger + // SetIdentity-related callbacks in the destructor. This can be a problem + // when WebRtcSession listens to the callback but it was the WebRtcSession + // destructor that caused WebRtcSessionDescriptionFactory's destruction. + // The callback is then ignored, leaking memory allocated by OnMessage for + // MSG_USE_CONSTRUCTOR_CERTIFICATE. + delete msg.pdata; + } + } transport_desc_factory_.set_identity(NULL); } @@ -193,7 +264,7 @@ void WebRtcSessionDescriptionFactory::CreateOffer( cricket::MediaSessionOptions session_options; std::string error = "CreateOffer"; - if (identity_request_state_ == IDENTITY_FAILED) { + if (certificate_request_state_ == CERTIFICATE_FAILED) { error += kFailedDueToIdentityFailed; LOG(LS_ERROR) << error; PostCreateSessionDescriptionFailed(observer, error); @@ -222,11 +293,11 @@ void WebRtcSessionDescriptionFactory::CreateOffer( CreateSessionDescriptionRequest request( CreateSessionDescriptionRequest::kOffer, observer, session_options); - if (identity_request_state_ == IDENTITY_WAITING) { + if (certificate_request_state_ == CERTIFICATE_WAITING) { create_session_description_requests_.push(request); } else { - ASSERT(identity_request_state_ == IDENTITY_SUCCEEDED || - identity_request_state_ == IDENTITY_NOT_NEEDED); + ASSERT(certificate_request_state_ == CERTIFICATE_SUCCEEDED || + certificate_request_state_ == CERTIFICATE_NOT_NEEDED); InternalCreateOffer(request); } } @@ -235,7 +306,7 @@ void WebRtcSessionDescriptionFactory::CreateAnswer( CreateSessionDescriptionObserver* observer, const MediaConstraintsInterface* constraints) { std::string error = "CreateAnswer"; - if (identity_request_state_ == IDENTITY_FAILED) { + if (certificate_request_state_ == CERTIFICATE_FAILED) { error += kFailedDueToIdentityFailed; LOG(LS_ERROR) << error; PostCreateSessionDescriptionFailed(observer, error); @@ -277,11 +348,11 @@ void WebRtcSessionDescriptionFactory::CreateAnswer( CreateSessionDescriptionRequest request( CreateSessionDescriptionRequest::kAnswer, observer, options); - if (identity_request_state_ == IDENTITY_WAITING) { + if (certificate_request_state_ == CERTIFICATE_WAITING) { create_session_description_requests_.push(request); } else { - ASSERT(identity_request_state_ == IDENTITY_SUCCEEDED || - identity_request_state_ == IDENTITY_NOT_NEEDED); + ASSERT(certificate_request_state_ == CERTIFICATE_SUCCEEDED || + certificate_request_state_ == CERTIFICATE_NOT_NEEDED); InternalCreateAnswer(request); } } @@ -311,6 +382,17 @@ void WebRtcSessionDescriptionFactory::OnMessage(rtc::Message* msg) { delete param; break; } + case MSG_USE_CONSTRUCTOR_CERTIFICATE: { + rtc::ScopedRefMessageData* param = + static_cast*>( + msg->pdata); + LOG(LS_INFO) << "Using certificate supplied to the constructor."; + // TODO(hbos): Pass around scoped_refptr instead of + // SSLIdentity* (then there will be no need to do GetReference here). + SetIdentity(param->data()->identity()->GetReference()); + delete param; + break; + } default: ASSERT(false); break; @@ -429,7 +511,7 @@ void WebRtcSessionDescriptionFactory::OnIdentityRequestFailed(int error) { ASSERT(signaling_thread_->IsCurrent()); LOG(LS_ERROR) << "Async identity request failed: error = " << error; - identity_request_state_ = IDENTITY_FAILED; + certificate_request_state_ = CERTIFICATE_FAILED; FailPendingRequests(kFailedDueToIdentityFailed); } @@ -438,7 +520,7 @@ void WebRtcSessionDescriptionFactory::SetIdentity( rtc::SSLIdentity* identity) { LOG(LS_VERBOSE) << "Setting new identity"; - identity_request_state_ = IDENTITY_SUCCEEDED; + certificate_request_state_ = CERTIFICATE_SUCCEEDED; SignalIdentityReady(identity); transport_desc_factory_.set_identity(identity); diff --git a/talk/app/webrtc/webrtcsessiondescriptionfactory.h b/talk/app/webrtc/webrtcsessiondescriptionfactory.h index 5d91a61dbb..8ba0ac2f4f 100644 --- a/talk/app/webrtc/webrtcsessiondescriptionfactory.h +++ b/talk/app/webrtc/webrtcsessiondescriptionfactory.h @@ -33,6 +33,7 @@ #include "talk/session/media/mediasession.h" #include "webrtc/p2p/base/transportdescriptionfactory.h" #include "webrtc/base/messagehandler.h" +#include "webrtc/base/rtccertificate.h" namespace cricket { class ChannelManager; @@ -87,16 +88,36 @@ struct CreateSessionDescriptionRequest { class WebRtcSessionDescriptionFactory : public rtc::MessageHandler, public sigslot::has_slots<> { public: + // Construct with DTLS disabled. + WebRtcSessionDescriptionFactory( + rtc::Thread* signaling_thread, + cricket::ChannelManager* channel_manager, + MediaStreamSignaling* mediastream_signaling, + WebRtcSession* session, + const std::string& session_id, + cricket::DataChannelType dct); + + // Construct with DTLS enabled using the specified |dtls_identity_store| to + // generate a certificate. WebRtcSessionDescriptionFactory( rtc::Thread* signaling_thread, cricket::ChannelManager* channel_manager, MediaStreamSignaling* mediastream_signaling, rtc::scoped_ptr dtls_identity_store, - // TODO(jiayl): remove the dependency on session once bug 2264 is fixed. WebRtcSession* session, const std::string& session_id, - cricket::DataChannelType dct, - bool dtls_enabled); + cricket::DataChannelType dct); + + // Construct with DTLS enabled using the specified (already generated) + // |certificate|. + WebRtcSessionDescriptionFactory( + rtc::Thread* signaling_thread, + cricket::ChannelManager* channel_manager, + MediaStreamSignaling* mediastream_signaling, + const rtc::scoped_refptr& certificate, + WebRtcSession* session, + const std::string& session_id, + cricket::DataChannelType dct); virtual ~WebRtcSessionDescriptionFactory(); static void CopyCandidatesFromSessionDescription( @@ -116,18 +137,30 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler, sigslot::signal1 SignalIdentityReady; // For testing. - bool waiting_for_identity() const { - return identity_request_state_ == IDENTITY_WAITING; + bool waiting_for_certificate_for_testing() const { + return certificate_request_state_ == CERTIFICATE_WAITING; } private: - enum IdentityRequestState { - IDENTITY_NOT_NEEDED, - IDENTITY_WAITING, - IDENTITY_SUCCEEDED, - IDENTITY_FAILED, + enum CertificateRequestState { + CERTIFICATE_NOT_NEEDED, + CERTIFICATE_WAITING, + CERTIFICATE_SUCCEEDED, + CERTIFICATE_FAILED, }; + WebRtcSessionDescriptionFactory( + rtc::Thread* signaling_thread, + cricket::ChannelManager* channel_manager, + MediaStreamSignaling* mediastream_signaling, + rtc::scoped_ptr dtls_identity_store, + const rtc::scoped_refptr& + identity_request_observer, + WebRtcSession* session, + const std::string& session_id, + cricket::DataChannelType dct, + bool dtls_enabled); + // MessageHandler implementation. virtual void OnMessage(rtc::Message* msg); @@ -152,12 +185,14 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler, cricket::TransportDescriptionFactory transport_desc_factory_; cricket::MediaSessionDescriptionFactory session_desc_factory_; uint64 session_version_; - rtc::scoped_ptr dtls_identity_store_; - rtc::scoped_refptr identity_request_observer_; + const rtc::scoped_ptr dtls_identity_store_; + const rtc::scoped_refptr + identity_request_observer_; + // TODO(jiayl): remove the dependency on session once bug 2264 is fixed. WebRtcSession* const session_; const std::string session_id_; const cricket::DataChannelType data_channel_type_; - IdentityRequestState identity_request_state_; + CertificateRequestState certificate_request_state_; DISALLOW_COPY_AND_ASSIGN(WebRtcSessionDescriptionFactory); }; diff --git a/webrtc/base/sslidentity.h b/webrtc/base/sslidentity.h index 15e23a357b..1112def098 100644 --- a/webrtc/base/sslidentity.h +++ b/webrtc/base/sslidentity.h @@ -145,6 +145,7 @@ class SSLIdentity { // Returns a new SSLIdentity object instance wrapping the same // identity information. // Caller is responsible for freeing the returned object. + // TODO(hbos,torbjorng): Rename to a less confusing name. virtual SSLIdentity* GetReference() const = 0; // Returns a temporary reference to the certificate.