From d03c23b216b6f1ff1b9c9e4fae75151607813f40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Wed, 1 Jun 2016 11:44:18 +0200 Subject: [PATCH] Replacing DtlsIdentityStoreInterface with RTCCertificateGeneratorInterface. The store was used in WebRtcSessionDescriptionFactory to generate certificates, now a generator is used instead (new API). PeerConnection[Factory][Interface], and WebRtcSession are updated to pass generators all the way down to the WebRtcSessionDescriptionFactory instead of stores. The webrtc implementation of a generator, RTCCertificateGenerator, is used as the default generator (peerconnectionfactory.cc:189) instead of the webrtc implementation of a store, DtlsIdentityStoreImpl. The generator is fully parameterized and does not generate RSA-1024 unless you ask for it (which makes sense not to do beforehand since ECDSA is now default). The store was not fully parameterized (known filed bug). The "top" layer, PeerConnectionFactoryInterface::CreatePeerConneciton, is updated to take a generator instead of a store. Many unittests still use a store, to allow them to continue to do so the factory gets CreatePeerConnectionWithStore which uses the old function signature (and invokes the new signature by wrapping the store in an RTCCertificateGeneratorStoreWrapper). As soon as the FakeDtlsIdentityStore is turned into a certificate generator instead of a store, the unittests will be updated and we can remove CreatePeerConnectionWithStore. This is a reupload of https://codereview.webrtc.org/2013523002/ with minor changes. BUG=webrtc:5707, webrtc:5708 R=tommi@webrtc.org Review URL: https://codereview.webrtc.org/2017943002 . Cr-Commit-Position: refs/heads/master@{#12984} --- webrtc/api/peerconnection.cc | 4 +- webrtc/api/peerconnection.h | 3 +- webrtc/api/peerconnection_unittest.cc | 2 +- webrtc/api/peerconnectionfactory.cc | 50 ++++--- webrtc/api/peerconnectionfactory.h | 12 +- webrtc/api/peerconnectionfactory_unittest.cc | 47 +++--- webrtc/api/peerconnectionfactoryproxy.h | 12 +- webrtc/api/peerconnectioninterface.h | 39 ++++- .../api/peerconnectioninterface_unittest.cc | 2 +- webrtc/api/test/peerconnectiontestwrapper.cc | 2 +- webrtc/api/webrtcsession.cc | 15 +- webrtc/api/webrtcsession.h | 2 +- webrtc/api/webrtcsession_unittest.cc | 5 +- webrtc/api/webrtcsessiondescriptionfactory.cc | 140 +++++++----------- webrtc/api/webrtcsessiondescriptionfactory.h | 65 ++++---- 15 files changed, 205 insertions(+), 195 deletions(-) diff --git a/webrtc/api/peerconnection.cc b/webrtc/api/peerconnection.cc index d5cbd5ea87..426a8a3788 100644 --- a/webrtc/api/peerconnection.cc +++ b/webrtc/api/peerconnection.cc @@ -566,7 +566,7 @@ PeerConnection::~PeerConnection() { bool PeerConnection::Initialize( const PeerConnectionInterface::RTCConfiguration& configuration, std::unique_ptr allocator, - std::unique_ptr dtls_identity_store, + std::unique_ptr cert_generator, PeerConnectionObserver* observer) { TRACE_EVENT0("webrtc", "PeerConnection::Initialize"); RTC_DCHECK(observer != nullptr); @@ -594,7 +594,7 @@ bool PeerConnection::Initialize( stats_.reset(new StatsCollector(this)); // Initialize the WebRtcSession. It creates transport channels etc. - if (!session_->Initialize(factory_->options(), std::move(dtls_identity_store), + if (!session_->Initialize(factory_->options(), std::move(cert_generator), configuration)) { return false; } diff --git a/webrtc/api/peerconnection.h b/webrtc/api/peerconnection.h index cf9e3b9338..a683e8d685 100644 --- a/webrtc/api/peerconnection.h +++ b/webrtc/api/peerconnection.h @@ -16,7 +16,6 @@ #include #include -#include "webrtc/api/dtlsidentitystore.h" #include "webrtc/api/peerconnectionfactory.h" #include "webrtc/api/peerconnectioninterface.h" #include "webrtc/api/rtpreceiverinterface.h" @@ -70,7 +69,7 @@ class PeerConnection : public PeerConnectionInterface, bool Initialize( const PeerConnectionInterface::RTCConfiguration& configuration, std::unique_ptr allocator, - std::unique_ptr dtls_identity_store, + std::unique_ptr cert_generator, PeerConnectionObserver* observer); rtc::scoped_refptr local_streams() override; diff --git a/webrtc/api/peerconnection_unittest.cc b/webrtc/api/peerconnection_unittest.cc index 4efaa740bf..44d752db72 100644 --- a/webrtc/api/peerconnection_unittest.cc +++ b/webrtc/api/peerconnection_unittest.cc @@ -856,7 +856,7 @@ class PeerConnectionTestClient : public webrtc::PeerConnectionObserver, ice_server.uri = "stun:stun.l.google.com:19302"; config.servers.push_back(ice_server); - return peer_connection_factory_->CreatePeerConnection( + return peer_connection_factory_->CreatePeerConnectionWithStore( config, constraints, std::move(port_allocator), std::move(dtls_identity_store), this); } diff --git a/webrtc/api/peerconnectionfactory.cc b/webrtc/api/peerconnectionfactory.cc index a91589a98a..9a58452d2d 100644 --- a/webrtc/api/peerconnectionfactory.cc +++ b/webrtc/api/peerconnectionfactory.cc @@ -36,25 +36,27 @@ namespace webrtc { namespace { -// Passes down the calls to |store_|. See usage in CreatePeerConnection. -class DtlsIdentityStoreWrapper : public DtlsIdentityStoreInterface { +// Passes down the calls to |cert_generator_|. See usage in +// |CreatePeerConnection|. +class RTCCertificateGeneratorWrapper + : public rtc::RTCCertificateGeneratorInterface { public: - DtlsIdentityStoreWrapper( - const rtc::scoped_refptr& store) - : store_(store) { - RTC_DCHECK(store_); + RTCCertificateGeneratorWrapper( + const rtc::scoped_refptr& cert_gen) + : cert_generator_(cert_gen) { + RTC_DCHECK(cert_generator_); } - void RequestIdentity( + void GenerateCertificateAsync( const rtc::KeyParams& key_params, const rtc::Optional& expires_ms, - const rtc::scoped_refptr& - observer) override { - store_->RequestIdentity(key_params, expires_ms, observer); + const rtc::scoped_refptr& callback) + override { + cert_generator_->GenerateCertificateAsync(key_params, expires_ms, callback); } private: - rtc::scoped_refptr store_; + rtc::scoped_refptr cert_generator_; }; } // anonymous namespace @@ -141,9 +143,9 @@ PeerConnectionFactory::~PeerConnectionFactory() { channel_manager_.reset(nullptr); // Make sure |worker_thread_| and |signaling_thread_| outlive - // |dtls_identity_store_|, |default_socket_factory_| and + // |cert_generator_|, |default_socket_factory_| and // |default_network_manager_|. - dtls_identity_store_ = nullptr; + cert_generator_ = nullptr; default_socket_factory_ = nullptr; default_network_manager_ = nullptr; @@ -184,8 +186,8 @@ bool PeerConnectionFactory::Initialize() { return false; } - dtls_identity_store_ = - new RefCountedDtlsIdentityStore(signaling_thread_, network_thread_); + cert_generator_ = + new RefCountedRTCCertificateGenerator(signaling_thread_, network_thread_); return true; } @@ -255,7 +257,7 @@ PeerConnectionFactory::CreatePeerConnection( const PeerConnectionInterface::RTCConfiguration& configuration_in, const MediaConstraintsInterface* constraints, std::unique_ptr allocator, - std::unique_ptr dtls_identity_store, + std::unique_ptr cert_generator, PeerConnectionObserver* observer) { RTC_DCHECK(signaling_thread_->IsCurrent()); @@ -264,23 +266,23 @@ PeerConnectionFactory::CreatePeerConnection( CopyConstraintsIntoRtcConfiguration(constraints, &configuration); return CreatePeerConnection(configuration, std::move(allocator), - std::move(dtls_identity_store), observer); + std::move(cert_generator), observer); } rtc::scoped_refptr PeerConnectionFactory::CreatePeerConnection( const PeerConnectionInterface::RTCConfiguration& configuration, std::unique_ptr allocator, - std::unique_ptr dtls_identity_store, + std::unique_ptr cert_generator, PeerConnectionObserver* observer) { RTC_DCHECK(signaling_thread_->IsCurrent()); - if (!dtls_identity_store.get()) { - // Because |pc|->Initialize takes ownership of the store we need a new + if (!cert_generator.get()) { + // Because |pc|->Initialize takes ownership of the generator we need a new // wrapper object that can be deleted without deleting the underlying - // |dtls_identity_store_|, protecting it from being deleted multiple times. - dtls_identity_store.reset( - new DtlsIdentityStoreWrapper(dtls_identity_store_)); + // |cert_generator_|, protecting it from being deleted multiple times. + cert_generator.reset( + new RTCCertificateGeneratorWrapper(cert_generator_)); } if (!allocator) { @@ -295,7 +297,7 @@ PeerConnectionFactory::CreatePeerConnection( new rtc::RefCountedObject(this)); if (!pc->Initialize(configuration, std::move(allocator), - std::move(dtls_identity_store), observer)) { + std::move(cert_generator), observer)) { return nullptr; } return PeerConnectionProxy::Create(signaling_thread(), pc); diff --git a/webrtc/api/peerconnectionfactory.h b/webrtc/api/peerconnectionfactory.h index 21165cf3d2..66561ad9d3 100644 --- a/webrtc/api/peerconnectionfactory.h +++ b/webrtc/api/peerconnectionfactory.h @@ -14,12 +14,12 @@ #include #include -#include "webrtc/api/dtlsidentitystore.h" #include "webrtc/api/mediacontroller.h" #include "webrtc/api/mediastreaminterface.h" #include "webrtc/api/peerconnectioninterface.h" #include "webrtc/base/scoped_ref_ptr.h" #include "webrtc/base/thread.h" +#include "webrtc/base/rtccertificategenerator.h" #include "webrtc/pc/channelmanager.h" namespace rtc { @@ -29,8 +29,8 @@ class BasicPacketSocketFactory; namespace webrtc { -typedef rtc::RefCountedObject - RefCountedDtlsIdentityStore; +typedef rtc::RefCountedObject + RefCountedRTCCertificateGenerator; class PeerConnectionFactory : public PeerConnectionFactoryInterface { public: @@ -43,13 +43,13 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface { const PeerConnectionInterface::RTCConfiguration& configuration, const MediaConstraintsInterface* constraints, std::unique_ptr allocator, - std::unique_ptr dtls_identity_store, + std::unique_ptr cert_generator, PeerConnectionObserver* observer) override; virtual rtc::scoped_refptr CreatePeerConnection( const PeerConnectionInterface::RTCConfiguration& configuration, std::unique_ptr allocator, - std::unique_ptr dtls_identity_store, + std::unique_ptr cert_generator, PeerConnectionObserver* observer) override; bool Initialize(); @@ -129,7 +129,7 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface { std::unique_ptr default_network_manager_; std::unique_ptr default_socket_factory_; - rtc::scoped_refptr dtls_identity_store_; + rtc::scoped_refptr cert_generator_; }; } // namespace webrtc diff --git a/webrtc/api/peerconnectionfactory_unittest.cc b/webrtc/api/peerconnectionfactory_unittest.cc index 05bc6c8659..fde5260e42 100644 --- a/webrtc/api/peerconnectionfactory_unittest.cc +++ b/webrtc/api/peerconnectionfactory_unittest.cc @@ -144,8 +144,9 @@ TEST(PeerConnectionFactoryTestInternal, CreatePCUsingInternalModules) { std::unique_ptr dtls_identity_store( new FakeDtlsIdentityStore()); - rtc::scoped_refptr pc(factory->CreatePeerConnection( - config, nullptr, nullptr, std::move(dtls_identity_store), &observer)); + rtc::scoped_refptr pc( + factory->CreatePeerConnectionWithStore( + config, nullptr, nullptr, std::move(dtls_identity_store), &observer)); EXPECT_TRUE(pc.get() != nullptr); } @@ -165,9 +166,10 @@ TEST_F(PeerConnectionFactoryTest, CreatePCUsingIceServers) { config.servers.push_back(ice_server); std::unique_ptr dtls_identity_store( new FakeDtlsIdentityStore()); - rtc::scoped_refptr pc(factory_->CreatePeerConnection( - config, nullptr, std::move(port_allocator_), - std::move(dtls_identity_store), &observer_)); + rtc::scoped_refptr pc( + factory_->CreatePeerConnectionWithStore( + config, nullptr, std::move(port_allocator_), + std::move(dtls_identity_store), &observer_)); ASSERT_TRUE(pc.get() != NULL); cricket::ServerAddresses stun_servers; rtc::SocketAddress stun1("stun.l.google.com", 19302); @@ -195,9 +197,10 @@ TEST_F(PeerConnectionFactoryTest, CreatePCUsingIceServersUrls) { config.servers.push_back(ice_server); std::unique_ptr dtls_identity_store( new FakeDtlsIdentityStore()); - rtc::scoped_refptr pc(factory_->CreatePeerConnection( - config, nullptr, std::move(port_allocator_), - std::move(dtls_identity_store), &observer_)); + rtc::scoped_refptr pc( + factory_->CreatePeerConnectionWithStore( + config, nullptr, std::move(port_allocator_), + std::move(dtls_identity_store), &observer_)); ASSERT_TRUE(pc.get() != NULL); cricket::ServerAddresses stun_servers; rtc::SocketAddress stun1("stun.l.google.com", 19302); @@ -224,9 +227,10 @@ TEST_F(PeerConnectionFactoryTest, CreatePCUsingNoUsernameInUri) { config.servers.push_back(ice_server); std::unique_ptr dtls_identity_store( new FakeDtlsIdentityStore()); - rtc::scoped_refptr pc(factory_->CreatePeerConnection( - config, nullptr, std::move(port_allocator_), - std::move(dtls_identity_store), &observer_)); + rtc::scoped_refptr pc( + factory_->CreatePeerConnectionWithStore( + config, nullptr, std::move(port_allocator_), + std::move(dtls_identity_store), &observer_)); ASSERT_TRUE(pc.get() != NULL); std::vector turn_servers; cricket::RelayServerConfig turn("test.com", 1234, kTurnUsername, @@ -245,9 +249,10 @@ TEST_F(PeerConnectionFactoryTest, CreatePCUsingTurnUrlWithTransportParam) { config.servers.push_back(ice_server); std::unique_ptr dtls_identity_store( new FakeDtlsIdentityStore()); - rtc::scoped_refptr pc(factory_->CreatePeerConnection( - config, nullptr, std::move(port_allocator_), - std::move(dtls_identity_store), &observer_)); + rtc::scoped_refptr pc( + factory_->CreatePeerConnectionWithStore( + config, nullptr, std::move(port_allocator_), + std::move(dtls_identity_store), &observer_)); ASSERT_TRUE(pc.get() != NULL); std::vector turn_servers; cricket::RelayServerConfig turn("hello.com", kDefaultStunPort, "test", @@ -270,9 +275,10 @@ TEST_F(PeerConnectionFactoryTest, CreatePCUsingSecureTurnUrl) { config.servers.push_back(ice_server); std::unique_ptr dtls_identity_store( new FakeDtlsIdentityStore()); - rtc::scoped_refptr pc(factory_->CreatePeerConnection( - config, nullptr, std::move(port_allocator_), - std::move(dtls_identity_store), &observer_)); + rtc::scoped_refptr pc( + factory_->CreatePeerConnectionWithStore( + config, nullptr, std::move(port_allocator_), + std::move(dtls_identity_store), &observer_)); ASSERT_TRUE(pc.get() != NULL); std::vector turn_servers; cricket::RelayServerConfig turn1("hello.com", kDefaultStunTlsPort, "test", @@ -305,9 +311,10 @@ TEST_F(PeerConnectionFactoryTest, CreatePCUsingIPLiteralAddress) { config.servers.push_back(ice_server); std::unique_ptr dtls_identity_store( new FakeDtlsIdentityStore()); - rtc::scoped_refptr pc(factory_->CreatePeerConnection( - config, nullptr, std::move(port_allocator_), - std::move(dtls_identity_store), &observer_)); + rtc::scoped_refptr pc( + factory_->CreatePeerConnectionWithStore( + config, nullptr, std::move(port_allocator_), + std::move(dtls_identity_store), &observer_)); ASSERT_TRUE(pc.get() != NULL); cricket::ServerAddresses stun_servers; rtc::SocketAddress stun1("1.2.3.4", 1234); diff --git a/webrtc/api/peerconnectionfactoryproxy.h b/webrtc/api/peerconnectionfactoryproxy.h index c357de9b32..f0dea42258 100644 --- a/webrtc/api/peerconnectionfactoryproxy.h +++ b/webrtc/api/peerconnectionfactoryproxy.h @@ -29,7 +29,7 @@ BEGIN_SIGNALING_PROXY_MAP(PeerConnectionFactory) const PeerConnectionInterface::RTCConfiguration& a1, const MediaConstraintsInterface* a2, std::unique_ptr a3, - std::unique_ptr a4, + std::unique_ptr a4, PeerConnectionObserver* a5) override { return signaling_thread_ ->Invoke>( @@ -39,7 +39,7 @@ BEGIN_SIGNALING_PROXY_MAP(PeerConnectionFactory) rtc::scoped_refptr CreatePeerConnection( const PeerConnectionInterface::RTCConfiguration& a1, std::unique_ptr a3, - std::unique_ptr a4, + std::unique_ptr a4, PeerConnectionObserver* a5) override { return signaling_thread_ ->Invoke>( @@ -77,10 +77,10 @@ BEGIN_SIGNALING_PROXY_MAP(PeerConnectionFactory) const PeerConnectionInterface::RTCConfiguration& a1, const MediaConstraintsInterface* a2, cricket::PortAllocator* a3, - DtlsIdentityStoreInterface* a4, + rtc::RTCCertificateGeneratorInterface* a4, PeerConnectionObserver* a5) { std::unique_ptr ptr_a3(a3); - std::unique_ptr ptr_a4(a4); + std::unique_ptr ptr_a4(a4); return c_->CreatePeerConnection(a1, a2, std::move(ptr_a3), std::move(ptr_a4), a5); } @@ -88,10 +88,10 @@ BEGIN_SIGNALING_PROXY_MAP(PeerConnectionFactory) rtc::scoped_refptr CreatePeerConnection_ot( const PeerConnectionInterface::RTCConfiguration& a1, cricket::PortAllocator* a3, - DtlsIdentityStoreInterface* a4, + rtc::RTCCertificateGeneratorInterface* a4, PeerConnectionObserver* a5) { std::unique_ptr ptr_a3(a3); - std::unique_ptr ptr_a4(a4); + std::unique_ptr ptr_a4(a4); return c_->CreatePeerConnection(a1, std::move(ptr_a3), std::move(ptr_a4), a5); } diff --git a/webrtc/api/peerconnectioninterface.h b/webrtc/api/peerconnectioninterface.h index 166a1e5ef9..3a1b8dd988 100644 --- a/webrtc/api/peerconnectioninterface.h +++ b/webrtc/api/peerconnectioninterface.h @@ -68,6 +68,7 @@ #include "webrtc/base/fileutils.h" #include "webrtc/base/network.h" #include "webrtc/base/rtccertificate.h" +#include "webrtc/base/rtccertificategenerator.h" #include "webrtc/base/socketaddress.h" #include "webrtc/base/sslstreamadapter.h" #include "webrtc/media/base/mediachannel.h" @@ -600,14 +601,48 @@ class PeerConnectionFactoryInterface : public rtc::RefCountInterface { const PeerConnectionInterface::RTCConfiguration& configuration, const MediaConstraintsInterface* constraints, std::unique_ptr allocator, - std::unique_ptr dtls_identity_store, + std::unique_ptr cert_generator, PeerConnectionObserver* observer) = 0; + // TODO(hbos): To be removed in favor of the |cert_generator| version as soon + // as unittests stop using this version. See bugs.webrtc.org/5707, + // bugs.webrtc.org/5708. + rtc::scoped_refptr CreatePeerConnectionWithStore( + const PeerConnectionInterface::RTCConfiguration& configuration, + const MediaConstraintsInterface* constraints, + std::unique_ptr allocator, + std::unique_ptr dtls_identity_store, + PeerConnectionObserver* observer) { + return CreatePeerConnection( + configuration, + constraints, + std::move(allocator), + std::unique_ptr( + dtls_identity_store ? new RTCCertificateGeneratorStoreWrapper( + std::move(dtls_identity_store)) : nullptr), + observer); + } virtual rtc::scoped_refptr CreatePeerConnection( const PeerConnectionInterface::RTCConfiguration& configuration, std::unique_ptr allocator, - std::unique_ptr dtls_identity_store, + std::unique_ptr cert_generator, PeerConnectionObserver* observer) = 0; + // TODO(hbos): To be removed in favor of the |cert_generator| version as soon + // as unittests stop using this version. See bugs.webrtc.org/5707, + // bugs.webrtc.org/5708. + rtc::scoped_refptr CreatePeerConnectionWithStore( + const PeerConnectionInterface::RTCConfiguration& configuration, + std::unique_ptr allocator, + std::unique_ptr dtls_identity_store, + PeerConnectionObserver* observer) { + return CreatePeerConnection( + configuration, + std::move(allocator), + std::unique_ptr( + dtls_identity_store ? new RTCCertificateGeneratorStoreWrapper( + std::move(dtls_identity_store)) : nullptr), + observer); + } virtual rtc::scoped_refptr CreateLocalMediaStream(const std::string& label) = 0; diff --git a/webrtc/api/peerconnectioninterface_unittest.cc b/webrtc/api/peerconnectioninterface_unittest.cc index 6760a5464e..88a25b6f7b 100644 --- a/webrtc/api/peerconnectioninterface_unittest.cc +++ b/webrtc/api/peerconnectioninterface_unittest.cc @@ -603,7 +603,7 @@ class PeerConnectionInterfaceTest : public testing::Test { nullptr) && dtls) { dtls_identity_store.reset(new FakeDtlsIdentityStore()); } - pc_ = pc_factory_->CreatePeerConnection( + pc_ = pc_factory_->CreatePeerConnectionWithStore( config, constraints, std::move(port_allocator), std::move(dtls_identity_store), &observer_); ASSERT_TRUE(pc_.get() != NULL); diff --git a/webrtc/api/test/peerconnectiontestwrapper.cc b/webrtc/api/test/peerconnectiontestwrapper.cc index bc49be5459..f4027da6f3 100644 --- a/webrtc/api/test/peerconnectiontestwrapper.cc +++ b/webrtc/api/test/peerconnectiontestwrapper.cc @@ -82,7 +82,7 @@ bool PeerConnectionTestWrapper::CreatePc( std::unique_ptr dtls_identity_store( rtc::SSLStreamAdapter::HaveDtlsSrtp() ? new FakeDtlsIdentityStore() : nullptr); - peer_connection_ = peer_connection_factory_->CreatePeerConnection( + peer_connection_ = peer_connection_factory_->CreatePeerConnectionWithStore( config, constraints, std::move(port_allocator), std::move(dtls_identity_store), this); diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc index 24eb593638..0699308554 100644 --- a/webrtc/api/webrtcsession.cc +++ b/webrtc/api/webrtcsession.cc @@ -512,7 +512,7 @@ WebRtcSession::~WebRtcSession() { bool WebRtcSession::Initialize( const PeerConnectionFactoryInterface::Options& options, - std::unique_ptr dtls_identity_store, + std::unique_ptr cert_generator, const PeerConnectionInterface::RTCConfiguration& rtc_configuration) { bundle_policy_ = rtc_configuration.bundle_policy; rtcp_mux_policy_ = rtc_configuration.rtcp_mux_policy; @@ -533,7 +533,7 @@ bool WebRtcSession::Initialize( dtls_enabled_ = false; } else { // Enable DTLS by default if we have an identity store or a certificate. - dtls_enabled_ = (dtls_identity_store || certificate); + dtls_enabled_ = (cert_generator || certificate); // |rtc_configuration| can override the default |dtls_enabled_| value. if (rtc_configuration.enable_dtls_srtp) { dtls_enabled_ = *(rtc_configuration.enable_dtls_srtp); @@ -566,19 +566,18 @@ bool WebRtcSession::Initialize( if (!dtls_enabled_) { // Construct with DTLS disabled. webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory( - signaling_thread(), channel_manager_, this, id())); + signaling_thread(), channel_manager_, this, id(), + std::unique_ptr())); } else { // Construct with DTLS enabled. if (!certificate) { - // Use the |dtls_identity_store| to generate a certificate. - RTC_DCHECK(dtls_identity_store); webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory( - signaling_thread(), channel_manager_, std::move(dtls_identity_store), - this, id())); + signaling_thread(), channel_manager_, this, id(), + std::move(cert_generator))); } else { // Use the already generated certificate. webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory( - signaling_thread(), channel_manager_, certificate, this, id())); + signaling_thread(), channel_manager_, this, id(), certificate)); } } diff --git a/webrtc/api/webrtcsession.h b/webrtc/api/webrtcsession.h index 98217bff26..dd47229a61 100644 --- a/webrtc/api/webrtcsession.h +++ b/webrtc/api/webrtcsession.h @@ -153,7 +153,7 @@ class WebRtcSession : public AudioProviderInterface, bool Initialize( const PeerConnectionFactoryInterface::Options& options, - std::unique_ptr dtls_identity_store, + std::unique_ptr cert_generator, const PeerConnectionInterface::RTCConfiguration& rtc_configuration); // Deletes the voice, video and data channel and changes the session state // to STATE_CLOSED. diff --git a/webrtc/api/webrtcsession_unittest.cc b/webrtc/api/webrtcsession_unittest.cc index 4207c24234..571e1994f4 100644 --- a/webrtc/api/webrtcsession_unittest.cc +++ b/webrtc/api/webrtcsession_unittest.cc @@ -397,7 +397,10 @@ class WebRtcSessionTest EXPECT_EQ(PeerConnectionInterface::kIceGatheringNew, observer_.ice_gathering_state_); - EXPECT_TRUE(session_->Initialize(options_, std::move(dtls_identity_store), + std::unique_ptr cert_generator( + dtls_identity_store ? new webrtc::RTCCertificateGeneratorStoreWrapper( + std::move(dtls_identity_store)) : nullptr); + EXPECT_TRUE(session_->Initialize(options_, std::move(cert_generator), configuration_)); session_->set_metrics_observer(metrics_observer_); } diff --git a/webrtc/api/webrtcsessiondescriptionfactory.cc b/webrtc/api/webrtcsessiondescriptionfactory.cc index e88262fbdc..08392e5ff3 100644 --- a/webrtc/api/webrtcsessiondescriptionfactory.cc +++ b/webrtc/api/webrtcsessiondescriptionfactory.cc @@ -12,7 +12,6 @@ #include -#include "webrtc/api/dtlsidentitystore.h" #include "webrtc/api/jsep.h" #include "webrtc/api/jsepsessiondescription.h" #include "webrtc/api/mediaconstraintsinterface.h" @@ -68,28 +67,13 @@ struct CreateSessionDescriptionMsg : public rtc::MessageData { }; } // namespace -void WebRtcIdentityRequestObserver::OnFailure(int error) { - SignalRequestFailed(error); +void WebRtcCertificateGeneratorCallback::OnFailure() { + SignalRequestFailed(); } -void WebRtcIdentityRequestObserver::OnSuccess( - const std::string& der_cert, const std::string& der_private_key) { - std::string pem_cert = rtc::SSLIdentity::DerToPem( - rtc::kPemTypeCertificate, - reinterpret_cast(der_cert.data()), - der_cert.length()); - std::string pem_key = rtc::SSLIdentity::DerToPem( - rtc::kPemTypeRsaPrivateKey, - reinterpret_cast(der_private_key.data()), - der_private_key.length()); - std::unique_ptr identity( - rtc::SSLIdentity::FromPEMStrings(pem_key, pem_cert)); - SignalCertificateReady(rtc::RTCCertificate::Create(std::move(identity))); -} - -void WebRtcIdentityRequestObserver::OnSuccess( - std::unique_ptr identity) { - SignalCertificateReady(rtc::RTCCertificate::Create(std::move(identity))); +void WebRtcCertificateGeneratorCallback::OnSuccess( + const rtc::scoped_refptr& certificate) { + SignalCertificateReady(certificate); } // static @@ -127,12 +111,10 @@ void WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription( WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( rtc::Thread* signaling_thread, cricket::ChannelManager* channel_manager, - std::unique_ptr dtls_identity_store, - const rtc::scoped_refptr& - identity_request_observer, WebRtcSession* session, const std::string& session_id, - bool dtls_enabled) + std::unique_ptr cert_generator, + const rtc::scoped_refptr& certificate) : signaling_thread_(signaling_thread), session_desc_factory_(channel_manager, &transport_desc_factory_), // RFC 4566 suggested a Network Time Protocol (NTP) format timestamp @@ -140,89 +122,81 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( // to just use a random number as session id and start version from // |kInitSessionVersion|. session_version_(kInitSessionVersion), - dtls_identity_store_(std::move(dtls_identity_store)), - identity_request_observer_(identity_request_observer), + cert_generator_(std::move(cert_generator)), session_(session), session_id_(session_id), certificate_request_state_(CERTIFICATE_NOT_NEEDED) { + RTC_DCHECK(signaling_thread_); session_desc_factory_.set_add_legacy_streams(false); + bool dtls_enabled = cert_generator_ || certificate; // SRTP-SDES is disabled if DTLS is on. SetSdesPolicy(dtls_enabled ? cricket::SEC_DISABLED : cricket::SEC_REQUIRED); + if (!dtls_enabled) { + LOG(LS_VERBOSE) << "DTLS-SRTP disabled."; + return; + } + + if (certificate) { + // Use |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 + // |SignalCertificateReady|. + signaling_thread_->Post( + this, MSG_USE_CONSTRUCTOR_CERTIFICATE, + new rtc::ScopedRefMessageData(certificate)); + } else { + // Generate certificate. + certificate_request_state_ = CERTIFICATE_WAITING; + + rtc::scoped_refptr callback( + new rtc::RefCountedObject()); + callback->SignalRequestFailed.connect( + this, &WebRtcSessionDescriptionFactory::OnCertificateRequestFailed); + callback->SignalCertificateReady.connect( + this, &WebRtcSessionDescriptionFactory::SetCertificate); + + rtc::KeyParams key_params = rtc::KeyParams(); + LOG(LS_VERBOSE) << "DTLS-SRTP enabled; sending DTLS identity request (key " + << "type: " << key_params.type() << ")."; + + // Request certificate. This happens asynchronously, so that the caller gets + // a chance to connect to |SignalCertificateReady|. + cert_generator_->GenerateCertificateAsync( + key_params, rtc::Optional(), callback); + } } WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( rtc::Thread* signaling_thread, cricket::ChannelManager* channel_manager, WebRtcSession* session, - const std::string& session_id) - : WebRtcSessionDescriptionFactory(signaling_thread, - channel_manager, - nullptr, - nullptr, - session, - session_id, - false) { - LOG(LS_VERBOSE) << "DTLS-SRTP disabled."; -} - -WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( - rtc::Thread* signaling_thread, - cricket::ChannelManager* channel_manager, - std::unique_ptr dtls_identity_store, - WebRtcSession* session, - const std::string& session_id) + const std::string& session_id, + std::unique_ptr cert_generator) : WebRtcSessionDescriptionFactory( signaling_thread, channel_manager, - std::move(dtls_identity_store), - new rtc::RefCountedObject(), session, session_id, - true) { - RTC_DCHECK(dtls_identity_store_); - - certificate_request_state_ = CERTIFICATE_WAITING; - - identity_request_observer_->SignalRequestFailed.connect( - this, &WebRtcSessionDescriptionFactory::OnIdentityRequestFailed); - identity_request_observer_->SignalCertificateReady.connect( - this, &WebRtcSessionDescriptionFactory::SetCertificate); - - rtc::KeyParams key_params = rtc::KeyParams(); - LOG(LS_VERBOSE) << "DTLS-SRTP enabled; sending DTLS identity request (key " - << "type: " << key_params.type() << ")."; - - // Request identity. This happens asynchronously, so the caller will have a - // chance to connect to SignalIdentityReady. - dtls_identity_store_->RequestIdentity(key_params, - rtc::Optional(), - identity_request_observer_); + std::move(cert_generator), + nullptr) { } WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( rtc::Thread* signaling_thread, cricket::ChannelManager* channel_manager, - const rtc::scoped_refptr& certificate, WebRtcSession* session, - const std::string& session_id) + const std::string& session_id, + const rtc::scoped_refptr& certificate) : WebRtcSessionDescriptionFactory(signaling_thread, channel_manager, - nullptr, - nullptr, session, session_id, - true) { + nullptr, + certificate) { RTC_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() { @@ -488,10 +462,10 @@ void WebRtcSessionDescriptionFactory::PostCreateSessionDescriptionSucceeded( signaling_thread_->Post(this, MSG_CREATE_SESSIONDESCRIPTION_SUCCESS, msg); } -void WebRtcSessionDescriptionFactory::OnIdentityRequestFailed(int error) { +void WebRtcSessionDescriptionFactory::OnCertificateRequestFailed() { ASSERT(signaling_thread_->IsCurrent()); - LOG(LS_ERROR) << "Async identity request failed: error = " << error; + LOG(LS_ERROR) << "Asynchronous certificate generation request failed."; certificate_request_state_ = CERTIFICATE_FAILED; FailPendingRequests(kFailedDueToIdentityFailed); @@ -500,7 +474,7 @@ void WebRtcSessionDescriptionFactory::OnIdentityRequestFailed(int error) { void WebRtcSessionDescriptionFactory::SetCertificate( const rtc::scoped_refptr& certificate) { RTC_DCHECK(certificate); - LOG(LS_VERBOSE) << "Setting new certificate"; + LOG(LS_VERBOSE) << "Setting new certificate."; certificate_request_state_ = CERTIFICATE_SUCCEEDED; SignalCertificateReady(certificate); diff --git a/webrtc/api/webrtcsessiondescriptionfactory.h b/webrtc/api/webrtcsessiondescriptionfactory.h index 17e2ddd3b0..c0c45b6ee5 100644 --- a/webrtc/api/webrtcsessiondescriptionfactory.h +++ b/webrtc/api/webrtcsessiondescriptionfactory.h @@ -18,6 +18,7 @@ #include "webrtc/base/constructormagic.h" #include "webrtc/base/messagehandler.h" #include "webrtc/base/rtccertificate.h" +#include "webrtc/base/rtccertificategenerator.h" #include "webrtc/p2p/base/transportdescriptionfactory.h" #include "webrtc/pc/mediasession.h" @@ -32,17 +33,17 @@ class MediaConstraintsInterface; class SessionDescriptionInterface; class WebRtcSession; -// DTLS identity request callback class. -class WebRtcIdentityRequestObserver : public DtlsIdentityRequestObserver, - public sigslot::has_slots<> { +// DTLS certificate request callback class. +class WebRtcCertificateGeneratorCallback + : public rtc::RTCCertificateGeneratorCallback, + public sigslot::has_slots<> { public: - // DtlsIdentityRequestObserver overrides. - void OnFailure(int error) override; - void OnSuccess(const std::string& der_cert, - const std::string& der_private_key) override; - void OnSuccess(std::unique_ptr identity) override; + // |rtc::RTCCertificateGeneratorCallback| overrides. + void OnSuccess( + const rtc::scoped_refptr& certificate) override; + void OnFailure() override; - sigslot::signal1 SignalRequestFailed; + sigslot::signal0<> SignalRequestFailed; sigslot::signal1&> SignalCertificateReady; }; @@ -66,37 +67,29 @@ struct CreateSessionDescriptionRequest { cricket::MediaSessionOptions options; }; -// This class is used to create offer/answer session description with regards to -// the async DTLS identity generation for WebRtcSession. -// It queues the create offer/answer request until the DTLS identity -// request has completed, i.e. when OnIdentityRequestFailed or OnIdentityReady -// is called. +// This class is used to create offer/answer session description. Certificates +// for WebRtcSession/DTLS are either supplied at construction or generated +// asynchronously. It queues the create offer/answer request until the +// certificate generation has completed, i.e. when OnCertificateRequestFailed or +// OnCertificateReady is called. class WebRtcSessionDescriptionFactory : public rtc::MessageHandler, public sigslot::has_slots<> { public: - // Construct with DTLS disabled. - WebRtcSessionDescriptionFactory(rtc::Thread* signaling_thread, - cricket::ChannelManager* channel_manager, - WebRtcSession* session, - const std::string& session_id); - - // Construct with DTLS enabled using the specified |dtls_identity_store| to - // generate a certificate. + // If |certificate_generator| is not null, DTLS is enabled and a default + // certificate is generated asynchronously; otherwise DTLS is disabled. WebRtcSessionDescriptionFactory( rtc::Thread* signaling_thread, cricket::ChannelManager* channel_manager, - std::unique_ptr dtls_identity_store, WebRtcSession* session, - const std::string& session_id); - - // Construct with DTLS enabled using the specified (already generated) - // |certificate|. + const std::string& session_id, + std::unique_ptr cert_generator); + // Construct with DTLS enabled using the specified |certificate|. WebRtcSessionDescriptionFactory( rtc::Thread* signaling_thread, cricket::ChannelManager* channel_manager, - const rtc::scoped_refptr& certificate, WebRtcSession* session, - const std::string& session_id); + const std::string& session_id, + const rtc::scoped_refptr& certificate); virtual ~WebRtcSessionDescriptionFactory(); static void CopyCandidatesFromSessionDescription( @@ -130,15 +123,15 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler, CERTIFICATE_FAILED, }; + // If |certificate_generator| or |certificate| is not null DTLS is enabled, + // otherwise DTLS is disabled. WebRtcSessionDescriptionFactory( rtc::Thread* signaling_thread, cricket::ChannelManager* channel_manager, - std::unique_ptr dtls_identity_store, - const rtc::scoped_refptr& - identity_request_observer, WebRtcSession* session, const std::string& session_id, - bool dtls_enabled); + std::unique_ptr cert_generator, + const rtc::scoped_refptr& certificate); // MessageHandler implementation. virtual void OnMessage(rtc::Message* msg); @@ -154,7 +147,7 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler, CreateSessionDescriptionObserver* observer, SessionDescriptionInterface* description); - void OnIdentityRequestFailed(int error); + void OnCertificateRequestFailed(); void SetCertificate( const rtc::scoped_refptr& certificate); @@ -164,9 +157,7 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler, cricket::TransportDescriptionFactory transport_desc_factory_; cricket::MediaSessionDescriptionFactory session_desc_factory_; uint64_t session_version_; - const std::unique_ptr dtls_identity_store_; - const rtc::scoped_refptr - identity_request_observer_; + const std::unique_ptr cert_generator_; // TODO(jiayl): remove the dependency on session once bug 2264 is fixed. WebRtcSession* const session_; const std::string session_id_;