From b4d01c4ded7ed1f7ff5ff75751dc52b7851a3918 Mon Sep 17 00:00:00 2001 From: kwiberg Date: Wed, 6 Apr 2016 05:15:06 -0700 Subject: [PATCH] A bunch of interfaces: Return scoped_ptr Instead of using a raw pointer output parameter. This affects SSLStreamAdapter::GetPeerCertificate Transport::GetRemoteSSLCertificate TransportChannel::GetRemoteSSLCertificate TransportController::GetRemoteSSLCertificate WebRtcSession::GetRemoteSSLCertificate This is a good idea in general, but will also be very convenient when scoped_ptr is gone, since unique_ptr doesn't have an .accept() method. BUG=webrtc:5520 Review URL: https://codereview.webrtc.org/1802013002 Cr-Commit-Position: refs/heads/master@{#12262} --- webrtc/api/statscollector.cc | 7 +- webrtc/api/statscollector_unittest.cc | 99 ++++++++++--------- webrtc/api/webrtcsession.cc | 6 +- webrtc/api/webrtcsession.h | 4 +- webrtc/base/opensslstreamadapter.cc | 11 +-- webrtc/base/opensslstreamadapter.h | 2 +- webrtc/base/sslstreamadapter.h | 4 +- webrtc/base/sslstreamadapter_unittest.cc | 25 +++-- webrtc/p2p/base/dtlstransportchannel.cc | 8 +- webrtc/p2p/base/dtlstransportchannel.h | 2 +- .../p2p/base/dtlstransportchannel_unittest.cc | 20 ++-- webrtc/p2p/base/faketransportcontroller.h | 11 +-- webrtc/p2p/base/p2ptransportchannel.h | 5 +- webrtc/p2p/base/transport.cc | 6 +- webrtc/p2p/base/transport.h | 2 +- webrtc/p2p/base/transportchannel.h | 5 +- webrtc/p2p/base/transportcontroller.cc | 21 ++-- webrtc/p2p/base/transportcontroller.h | 8 +- .../p2p/base/transportcontroller_unittest.cc | 9 +- 19 files changed, 129 insertions(+), 126 deletions(-) diff --git a/webrtc/api/statscollector.cc b/webrtc/api/statscollector.cc index 0182a37620..0901fc61b1 100644 --- a/webrtc/api/statscollector.cc +++ b/webrtc/api/statscollector.cc @@ -702,9 +702,10 @@ void StatsCollector::ExtractSessionInfo() { local_cert_report_id = r->id(); } - rtc::scoped_ptr cert; - if (pc_->session()->GetRemoteSSLCertificate( - transport_iter.second.transport_name, cert.accept())) { + rtc::scoped_ptr cert = + pc_->session()->GetRemoteSSLCertificate( + transport_iter.second.transport_name); + if (cert) { StatsReport* r = AddCertificateReports(cert.get()); if (r) remote_cert_report_id = r->id(); diff --git a/webrtc/api/statscollector_unittest.cc b/webrtc/api/statscollector_unittest.cc index 3b04383a85..5873e7382e 100644 --- a/webrtc/api/statscollector_unittest.cc +++ b/webrtc/api/statscollector_unittest.cc @@ -82,9 +82,15 @@ class MockWebRtcSession : public webrtc::WebRtcSession { MOCK_METHOD2(GetLocalCertificate, bool(const std::string& transport_name, rtc::scoped_refptr* certificate)); - MOCK_METHOD2(GetRemoteSSLCertificate, - bool(const std::string& transport_name, - rtc::SSLCertificate** cert)); + + // Workaround for gmock's inability to cope with move-only return values. + rtc::scoped_ptr GetRemoteSSLCertificate( + const std::string& transport_name) override { + return rtc::scoped_ptr( + GetRemoteSSLCertificate_ReturnsRawPointer(transport_name)); + } + MOCK_METHOD1(GetRemoteSSLCertificate_ReturnsRawPointer, + rtc::SSLCertificate*(const std::string& transport_name)); }; // The factory isn't really used; it just satisfies the base PeerConnection. @@ -662,10 +668,11 @@ class StatsCollectorTest : public testing::Test { VerifyVoiceReceiverInfoReport(track_report, *voice_receiver_info); } - void TestCertificateReports(const rtc::FakeSSLCertificate& local_cert, - const std::vector& local_ders, - const rtc::FakeSSLCertificate& remote_cert, - const std::vector& remote_ders) { + void TestCertificateReports( + const rtc::FakeSSLCertificate& local_cert, + const std::vector& local_ders, + rtc::scoped_ptr remote_cert, + const std::vector& remote_ders) { StatsCollectorForTest stats(&pc_); StatsReports reports; // returned values. @@ -694,10 +701,9 @@ class StatsCollectorTest : public testing::Test { EXPECT_CALL(session_, GetLocalCertificate(transport_stats.transport_name, _)) .WillOnce(DoAll(SetArgPointee<1>(local_certificate), Return(true))); - EXPECT_CALL(session_, - GetRemoteSSLCertificate(transport_stats.transport_name, _)) - .WillOnce( - DoAll(SetArgPointee<1>(remote_cert.GetReference()), Return(true))); + EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer( + transport_stats.transport_name)) + .WillOnce(Return(remote_cert.release())); EXPECT_CALL(session_, GetTransportStats(_)) .WillOnce(DoAll(SetArgPointee<0>(session_stats), Return(true))); @@ -807,8 +813,8 @@ TEST_F(StatsCollectorTest, BytesCounterHandles64Bits) { EXPECT_CALL(session_, GetLocalCertificate(_, _)) .WillRepeatedly(Return(false)); - EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _)) - .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_)) + .WillRepeatedly(Return(nullptr)); const char kVideoChannelName[] = "video"; @@ -853,8 +859,8 @@ TEST_F(StatsCollectorTest, BandwidthEstimationInfoIsReported) { EXPECT_CALL(session_, GetLocalCertificate(_, _)) .WillRepeatedly(Return(false)); - EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _)) - .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_)) + .WillRepeatedly(Return(nullptr)); const char kVideoChannelName[] = "video"; @@ -965,8 +971,8 @@ TEST_F(StatsCollectorTest, TrackAndSsrcObjectExistAfterUpdateSsrcStats) { EXPECT_CALL(session_, GetLocalCertificate(_, _)) .WillRepeatedly(Return(false)); - EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _)) - .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_)) + .WillRepeatedly(Return(nullptr)); const char kVideoChannelName[] = "video"; InitSessionStats(kVideoChannelName); @@ -1037,8 +1043,8 @@ TEST_F(StatsCollectorTest, TransportObjectLinkedFromSsrcObject) { EXPECT_CALL(session_, GetLocalCertificate(_, _)) .WillRepeatedly(Return(false)); - EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _)) - .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_)) + .WillRepeatedly(Return(nullptr)); MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); // The transport_name known by the video channel. @@ -1121,8 +1127,8 @@ TEST_F(StatsCollectorTest, RemoteSsrcInfoIsPresent) { EXPECT_CALL(session_, GetLocalCertificate(_, _)) .WillRepeatedly(Return(false)); - EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _)) - .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_)) + .WillRepeatedly(Return(nullptr)); MockVideoMediaChannel* media_channel = new MockVideoMediaChannel(); // The transport_name known by the video channel. @@ -1172,8 +1178,8 @@ TEST_F(StatsCollectorTest, ReportsFromRemoteTrack) { EXPECT_CALL(session_, GetLocalCertificate(_, _)) .WillRepeatedly(Return(false)); - EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _)) - .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_)) + .WillRepeatedly(Return(nullptr)); const char kVideoChannelName[] = "video"; InitSessionStats(kVideoChannelName); @@ -1333,9 +1339,11 @@ TEST_F(StatsCollectorTest, ChainedCertificateReportsCreated) { remote_ders[1] = "non-"; remote_ders[2] = "intersecting"; remote_ders[3] = "set"; - rtc::FakeSSLCertificate remote_cert(DersToPems(remote_ders)); + rtc::scoped_ptr remote_cert( + new rtc::FakeSSLCertificate(DersToPems(remote_ders))); - TestCertificateReports(local_cert, local_ders, remote_cert, remote_ders); + TestCertificateReports(local_cert, local_ders, std::move(remote_cert), + remote_ders); } // This test verifies that all certificates without chains are correctly @@ -1347,10 +1355,12 @@ TEST_F(StatsCollectorTest, ChainlessCertificateReportsCreated) { // Build remote certificate. std::string remote_der = "This is somebody else's der."; - rtc::FakeSSLCertificate remote_cert(DerToPem(remote_der)); + rtc::scoped_ptr remote_cert( + new rtc::FakeSSLCertificate(DerToPem(remote_der))); TestCertificateReports(local_cert, std::vector(1, local_der), - remote_cert, std::vector(1, remote_der)); + std::move(remote_cert), + std::vector(1, remote_der)); } // This test verifies that the stats are generated correctly when no @@ -1360,8 +1370,8 @@ TEST_F(StatsCollectorTest, NoTransport) { EXPECT_CALL(session_, GetLocalCertificate(_, _)) .WillRepeatedly(Return(false)); - EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _)) - .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_)) + .WillRepeatedly(Return(nullptr)); StatsReports reports; // returned values. @@ -1417,8 +1427,8 @@ TEST_F(StatsCollectorTest, NoCertificates) { EXPECT_CALL(session_, GetLocalCertificate(_, _)) .WillRepeatedly(Return(false)); - EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _)) - .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_)) + .WillRepeatedly(Return(nullptr)); StatsReports reports; // returned values. @@ -1469,11 +1479,12 @@ TEST_F(StatsCollectorTest, UnsupportedDigestIgnored) { // Build a remote certificate with an unsupported digest algorithm. std::string remote_der = "This is somebody else's der."; - rtc::FakeSSLCertificate remote_cert(DerToPem(remote_der)); - remote_cert.set_digest_algorithm("foobar"); + rtc::scoped_ptr remote_cert( + new rtc::FakeSSLCertificate(DerToPem(remote_der))); + remote_cert->set_digest_algorithm("foobar"); TestCertificateReports(local_cert, std::vector(1, local_der), - remote_cert, std::vector()); + std::move(remote_cert), std::vector()); } // This test verifies that a local stats object can get statistics via @@ -1483,8 +1494,8 @@ TEST_F(StatsCollectorTest, GetStatsFromLocalAudioTrack) { EXPECT_CALL(session_, GetLocalCertificate(_, _)) .WillRepeatedly(Return(false)); - EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _)) - .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_)) + .WillRepeatedly(Return(nullptr)); MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The transport_name known by the voice channel. @@ -1518,8 +1529,8 @@ TEST_F(StatsCollectorTest, GetStatsFromRemoteStream) { EXPECT_CALL(session_, GetLocalCertificate(_, _)) .WillRepeatedly(Return(false)); - EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _)) - .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_)) + .WillRepeatedly(Return(nullptr)); MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The transport_name known by the voice channel. @@ -1547,8 +1558,8 @@ TEST_F(StatsCollectorTest, GetStatsAfterRemoveAudioStream) { EXPECT_CALL(session_, GetLocalCertificate(_, _)) .WillRepeatedly(Return(false)); - EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _)) - .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_)) + .WillRepeatedly(Return(nullptr)); MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The transport_name known by the voice channel. @@ -1608,8 +1619,8 @@ TEST_F(StatsCollectorTest, LocalAndRemoteTracksWithSameSsrc) { EXPECT_CALL(session_, GetLocalCertificate(_, _)) .WillRepeatedly(Return(false)); - EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _)) - .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_)) + .WillRepeatedly(Return(nullptr)); MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The transport_name known by the voice channel. @@ -1695,8 +1706,8 @@ TEST_F(StatsCollectorTest, TwoLocalTracksWithSameSsrc) { EXPECT_CALL(session_, GetLocalCertificate(_, _)) .WillRepeatedly(Return(false)); - EXPECT_CALL(session_, GetRemoteSSLCertificate(_, _)) - .WillRepeatedly(Return(false)); + EXPECT_CALL(session_, GetRemoteSSLCertificate_ReturnsRawPointer(_)) + .WillRepeatedly(Return(nullptr)); MockVoiceMediaChannel* media_channel = new MockVoiceMediaChannel(); // The transport_name known by the voice channel. diff --git a/webrtc/api/webrtcsession.cc b/webrtc/api/webrtcsession.cc index 19f5aa4128..f546285574 100644 --- a/webrtc/api/webrtcsession.cc +++ b/webrtc/api/webrtcsession.cc @@ -1039,10 +1039,10 @@ bool WebRtcSession::GetLocalCertificate( certificate); } -bool WebRtcSession::GetRemoteSSLCertificate(const std::string& transport_name, - rtc::SSLCertificate** cert) { +rtc::scoped_ptr WebRtcSession::GetRemoteSSLCertificate( + const std::string& transport_name) { ASSERT(signaling_thread()->IsCurrent()); - return transport_controller_->GetRemoteSSLCertificate(transport_name, cert); + return transport_controller_->GetRemoteSSLCertificate(transport_name); } bool WebRtcSession::EnableBundle(const cricket::ContentGroup& bundle) { diff --git a/webrtc/api/webrtcsession.h b/webrtc/api/webrtcsession.h index 6164731982..01ec526501 100644 --- a/webrtc/api/webrtcsession.h +++ b/webrtc/api/webrtcsession.h @@ -292,8 +292,8 @@ class WebRtcSession : public AudioProviderInterface, rtc::scoped_refptr* certificate); // Caller owns returned certificate - virtual bool GetRemoteSSLCertificate(const std::string& transport_name, - rtc::SSLCertificate** cert); + virtual rtc::scoped_ptr GetRemoteSSLCertificate( + const std::string& transport_name); cricket::DataChannelType data_channel_type() const; diff --git a/webrtc/base/opensslstreamadapter.cc b/webrtc/base/opensslstreamadapter.cc index 8a3044d1a3..16dd9803b4 100644 --- a/webrtc/base/opensslstreamadapter.cc +++ b/webrtc/base/opensslstreamadapter.cc @@ -290,12 +290,11 @@ void OpenSSLStreamAdapter::SetServerRole(SSLRole role) { role_ = role; } -bool OpenSSLStreamAdapter::GetPeerCertificate(SSLCertificate** cert) const { - if (!peer_certificate_) - return false; - - *cert = peer_certificate_->GetReference(); - return true; +rtc::scoped_ptr OpenSSLStreamAdapter::GetPeerCertificate() + const { + return peer_certificate_ ? rtc::scoped_ptr( + peer_certificate_->GetReference()) + : nullptr; } bool OpenSSLStreamAdapter::SetPeerCertificateDigest(const std::string diff --git a/webrtc/base/opensslstreamadapter.h b/webrtc/base/opensslstreamadapter.h index 231d059ca8..463c8f2280 100644 --- a/webrtc/base/opensslstreamadapter.h +++ b/webrtc/base/opensslstreamadapter.h @@ -69,7 +69,7 @@ class OpenSSLStreamAdapter : public SSLStreamAdapter { const unsigned char* digest_val, size_t digest_len) override; - bool GetPeerCertificate(SSLCertificate** cert) const override; + rtc::scoped_ptr GetPeerCertificate() const override; int StartSSLWithServer(const char* server_name) override; int StartSSLWithPeer() override; diff --git a/webrtc/base/sslstreamadapter.h b/webrtc/base/sslstreamadapter.h index a50f674e32..f6f0befa05 100644 --- a/webrtc/base/sslstreamadapter.h +++ b/webrtc/base/sslstreamadapter.h @@ -154,8 +154,8 @@ class SSLStreamAdapter : public StreamAdapterInterface { // Retrieves the peer's X.509 certificate, if a connection has been // established. It returns the transmitted over SSL, including the entire - // chain. The returned certificate is owned by the caller. - virtual bool GetPeerCertificate(SSLCertificate** cert) const = 0; + // chain. + virtual rtc::scoped_ptr GetPeerCertificate() const = 0; // Retrieves the IANA registration id of the cipher suite used for the // connection (e.g. 0x2F for "TLS_RSA_WITH_AES_128_CBC_SHA"). diff --git a/webrtc/base/sslstreamadapter_unittest.cc b/webrtc/base/sslstreamadapter_unittest.cc index 1e18b8305c..8d5b275db8 100644 --- a/webrtc/base/sslstreamadapter_unittest.cc +++ b/webrtc/base/sslstreamadapter_unittest.cc @@ -474,11 +474,11 @@ class SSLStreamAdapterTestBase : public testing::Test, return server_ssl_->GetDtlsSrtpCryptoSuite(retval); } - bool GetPeerCertificate(bool client, rtc::SSLCertificate** cert) { + rtc::scoped_ptr GetPeerCertificate(bool client) { if (client) - return client_ssl_->GetPeerCertificate(cert); + return client_ssl_->GetPeerCertificate(); else - return server_ssl_->GetPeerCertificate(cert); + return server_ssl_->GetPeerCertificate(); } bool GetSslCipherSuite(bool client, int* retval) { @@ -1037,19 +1037,15 @@ TEST_F(SSLStreamAdapterTestDTLSFromPEMStrings, TestDTLSGetPeerCertificate) { MAYBE_SKIP_TEST(HaveDtls); // Peer certificates haven't been received yet. - rtc::scoped_ptr client_peer_cert; - ASSERT_FALSE(GetPeerCertificate(true, client_peer_cert.accept())); - ASSERT_FALSE(client_peer_cert != NULL); - - rtc::scoped_ptr server_peer_cert; - ASSERT_FALSE(GetPeerCertificate(false, server_peer_cert.accept())); - ASSERT_FALSE(server_peer_cert != NULL); + ASSERT_FALSE(GetPeerCertificate(true)); + ASSERT_FALSE(GetPeerCertificate(false)); TestHandshake(); // The client should have a peer certificate after the handshake. - ASSERT_TRUE(GetPeerCertificate(true, client_peer_cert.accept())); - ASSERT_TRUE(client_peer_cert != NULL); + rtc::scoped_ptr client_peer_cert = + GetPeerCertificate(true); + ASSERT_TRUE(client_peer_cert); // It's not kCERT_PEM. std::string client_peer_string = client_peer_cert->ToPEMString(); @@ -1059,8 +1055,9 @@ TEST_F(SSLStreamAdapterTestDTLSFromPEMStrings, TestDTLSGetPeerCertificate) { ASSERT_FALSE(client_peer_cert->GetChain()); // The server should have a peer certificate after the handshake. - ASSERT_TRUE(GetPeerCertificate(false, server_peer_cert.accept())); - ASSERT_TRUE(server_peer_cert != NULL); + rtc::scoped_ptr server_peer_cert = + GetPeerCertificate(false); + ASSERT_TRUE(server_peer_cert); // It's kCERT_PEM ASSERT_EQ(kCERT_PEM, server_peer_cert->ToPEMString()); diff --git a/webrtc/p2p/base/dtlstransportchannel.cc b/webrtc/p2p/base/dtlstransportchannel.cc index 667b2c82b2..88a11928cc 100644 --- a/webrtc/p2p/base/dtlstransportchannel.cc +++ b/webrtc/p2p/base/dtlstransportchannel.cc @@ -250,13 +250,13 @@ bool DtlsTransportChannelWrapper::SetRemoteFingerprint( return true; } -bool DtlsTransportChannelWrapper::GetRemoteSSLCertificate( - rtc::SSLCertificate** cert) const { +rtc::scoped_ptr +DtlsTransportChannelWrapper::GetRemoteSSLCertificate() const { if (!dtls_) { - return false; + return nullptr; } - return dtls_->GetPeerCertificate(cert); + return dtls_->GetPeerCertificate(); } bool DtlsTransportChannelWrapper::SetupDtls() { diff --git a/webrtc/p2p/base/dtlstransportchannel.h b/webrtc/p2p/base/dtlstransportchannel.h index 96ea29fa5d..b6c3cfd251 100644 --- a/webrtc/p2p/base/dtlstransportchannel.h +++ b/webrtc/p2p/base/dtlstransportchannel.h @@ -137,7 +137,7 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl { // Once DTLS has been established, this method retrieves the certificate in // use by the remote peer, for use in external identity verification. - bool GetRemoteSSLCertificate(rtc::SSLCertificate** cert) const override; + rtc::scoped_ptr GetRemoteSSLCertificate() const override; // Once DTLS has established (i.e., this channel is writable), this method // extracts the keys negotiated during the DTLS handshake, for use in external diff --git a/webrtc/p2p/base/dtlstransportchannel_unittest.cc b/webrtc/p2p/base/dtlstransportchannel_unittest.cc index 18b355ffd4..7643016b6a 100644 --- a/webrtc/p2p/base/dtlstransportchannel_unittest.cc +++ b/webrtc/p2p/base/dtlstransportchannel_unittest.cc @@ -855,12 +855,8 @@ TEST_F(DtlsTransportChannelTest, TestCertificatesBeforeConnect) { ASSERT_TRUE(client2_.transport()->GetLocalCertificate(&certificate2)); ASSERT_NE(certificate1->ssl_certificate().ToPEMString(), certificate2->ssl_certificate().ToPEMString()); - ASSERT_FALSE( - client1_.transport()->GetRemoteSSLCertificate(remote_cert1.accept())); - ASSERT_FALSE(remote_cert1 != NULL); - ASSERT_FALSE( - client2_.transport()->GetRemoteSSLCertificate(remote_cert2.accept())); - ASSERT_FALSE(remote_cert2 != NULL); + ASSERT_FALSE(client1_.transport()->GetRemoteSSLCertificate()); + ASSERT_FALSE(client2_.transport()->GetRemoteSSLCertificate()); } // Test Certificates state after connection. @@ -871,8 +867,6 @@ TEST_F(DtlsTransportChannelTest, TestCertificatesAfterConnect) { rtc::scoped_refptr certificate1; rtc::scoped_refptr certificate2; - rtc::scoped_ptr remote_cert1; - rtc::scoped_ptr remote_cert2; // After connection, each side has a distinct local certificate. ASSERT_TRUE(client1_.transport()->GetLocalCertificate(&certificate1)); @@ -881,12 +875,14 @@ TEST_F(DtlsTransportChannelTest, TestCertificatesAfterConnect) { certificate2->ssl_certificate().ToPEMString()); // Each side's remote certificate is the other side's local certificate. - ASSERT_TRUE( - client1_.transport()->GetRemoteSSLCertificate(remote_cert1.accept())); + rtc::scoped_ptr remote_cert1 = + client1_.transport()->GetRemoteSSLCertificate(); + ASSERT_TRUE(remote_cert1); ASSERT_EQ(remote_cert1->ToPEMString(), certificate2->ssl_certificate().ToPEMString()); - ASSERT_TRUE( - client2_.transport()->GetRemoteSSLCertificate(remote_cert2.accept())); + rtc::scoped_ptr remote_cert2 = + client2_.transport()->GetRemoteSSLCertificate(); + ASSERT_TRUE(remote_cert2); ASSERT_EQ(remote_cert2->ToPEMString(), certificate1->ssl_certificate().ToPEMString()); } diff --git a/webrtc/p2p/base/faketransportcontroller.h b/webrtc/p2p/base/faketransportcontroller.h index a267f20967..2e0c9a97fa 100644 --- a/webrtc/p2p/base/faketransportcontroller.h +++ b/webrtc/p2p/base/faketransportcontroller.h @@ -260,12 +260,11 @@ class FakeTransportChannel : public TransportChannelImpl, return local_cert_; } - bool GetRemoteSSLCertificate(rtc::SSLCertificate** cert) const override { - if (!remote_cert_) - return false; - - *cert = remote_cert_->GetReference(); - return true; + rtc::scoped_ptr GetRemoteSSLCertificate() + const override { + return remote_cert_ ? rtc::scoped_ptr( + remote_cert_->GetReference()) + : nullptr; } bool ExportKeyingMaterial(const std::string& label, diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index 5be9d042da..4a53d755af 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -144,8 +144,9 @@ class P2PTransportChannel : public TransportChannelImpl, return nullptr; } - bool GetRemoteSSLCertificate(rtc::SSLCertificate** cert) const override { - return false; + rtc::scoped_ptr GetRemoteSSLCertificate() + const override { + return nullptr; } // Allows key material to be extracted for external encryption. diff --git a/webrtc/p2p/base/transport.cc b/webrtc/p2p/base/transport.cc index b8d98100cf..a1988820ed 100644 --- a/webrtc/p2p/base/transport.cc +++ b/webrtc/p2p/base/transport.cc @@ -77,13 +77,13 @@ void Transport::SetIceRole(IceRole role) { } } -bool Transport::GetRemoteSSLCertificate(rtc::SSLCertificate** cert) { +rtc::scoped_ptr Transport::GetRemoteSSLCertificate() { if (channels_.empty()) { - return false; + return nullptr; } auto iter = channels_.begin(); - return iter->second->GetRemoteSSLCertificate(cert); + return iter->second->GetRemoteSSLCertificate(); } void Transport::SetIceConfig(const IceConfig& config) { diff --git a/webrtc/p2p/base/transport.h b/webrtc/p2p/base/transport.h index 66e300a154..8b30127b7f 100644 --- a/webrtc/p2p/base/transport.h +++ b/webrtc/p2p/base/transport.h @@ -214,7 +214,7 @@ class Transport : public sigslot::has_slots<> { } // Get a copy of the remote certificate in use by the specified channel. - bool GetRemoteSSLCertificate(rtc::SSLCertificate** cert); + rtc::scoped_ptr GetRemoteSSLCertificate(); // Create, destroy, and lookup the channels of this type by their components. TransportChannelImpl* CreateChannel(int component); diff --git a/webrtc/p2p/base/transportchannel.h b/webrtc/p2p/base/transportchannel.h index d21d60771b..24f90e3cf6 100644 --- a/webrtc/p2p/base/transportchannel.h +++ b/webrtc/p2p/base/transportchannel.h @@ -129,8 +129,9 @@ class TransportChannel : public sigslot::has_slots<> { virtual rtc::scoped_refptr GetLocalCertificate() const = 0; - // Gets a copy of the remote side's SSL certificate, owned by the caller. - virtual bool GetRemoteSSLCertificate(rtc::SSLCertificate** cert) const = 0; + // Gets a copy of the remote side's SSL certificate. + virtual rtc::scoped_ptr GetRemoteSSLCertificate() + const = 0; // Allows key material to be extracted for external encryption. virtual bool ExportKeyingMaterial(const std::string& label, diff --git a/webrtc/p2p/base/transportcontroller.cc b/webrtc/p2p/base/transportcontroller.cc index 128d2fc656..708c60f9df 100644 --- a/webrtc/p2p/base/transportcontroller.cc +++ b/webrtc/p2p/base/transportcontroller.cc @@ -86,12 +86,11 @@ bool TransportController::GetLocalCertificate( transport_name, certificate)); } -bool TransportController::GetRemoteSSLCertificate( - const std::string& transport_name, - rtc::SSLCertificate** cert) { - return worker_thread_->Invoke( - rtc::Bind(&TransportController::GetRemoteSSLCertificate_w, this, - transport_name, cert)); +rtc::scoped_ptr +TransportController::GetRemoteSSLCertificate( + const std::string& transport_name) { + return worker_thread_->Invoke>(rtc::Bind( + &TransportController::GetRemoteSSLCertificate_w, this, transport_name)); } bool TransportController::SetLocalTransportDescription( @@ -395,17 +394,17 @@ bool TransportController::GetLocalCertificate_w( return t->GetLocalCertificate(certificate); } -bool TransportController::GetRemoteSSLCertificate_w( - const std::string& transport_name, - rtc::SSLCertificate** cert) { +rtc::scoped_ptr +TransportController::GetRemoteSSLCertificate_w( + const std::string& transport_name) { RTC_DCHECK(worker_thread_->IsCurrent()); Transport* t = GetTransport_w(transport_name); if (!t) { - return false; + return nullptr; } - return t->GetRemoteSSLCertificate(cert); + return t->GetRemoteSSLCertificate(); } bool TransportController::SetLocalTransportDescription_w( diff --git a/webrtc/p2p/base/transportcontroller.h b/webrtc/p2p/base/transportcontroller.h index ed7216033b..9d06823920 100644 --- a/webrtc/p2p/base/transportcontroller.h +++ b/webrtc/p2p/base/transportcontroller.h @@ -59,8 +59,8 @@ class TransportController : public sigslot::has_slots<>, const std::string& transport_name, rtc::scoped_refptr* certificate); // Caller owns returned certificate - bool GetRemoteSSLCertificate(const std::string& transport_name, - rtc::SSLCertificate** cert); + rtc::scoped_ptr GetRemoteSSLCertificate( + const std::string& transport_name); bool SetLocalTransportDescription(const std::string& transport_name, const TransportDescription& tdesc, ContentAction action, @@ -166,8 +166,8 @@ class TransportController : public sigslot::has_slots<>, bool GetLocalCertificate_w( const std::string& transport_name, rtc::scoped_refptr* certificate); - bool GetRemoteSSLCertificate_w(const std::string& transport_name, - rtc::SSLCertificate** cert); + rtc::scoped_ptr GetRemoteSSLCertificate_w( + const std::string& transport_name); bool SetLocalTransportDescription_w(const std::string& transport_name, const TransportDescription& tdesc, ContentAction action, diff --git a/webrtc/p2p/base/transportcontroller_unittest.cc b/webrtc/p2p/base/transportcontroller_unittest.cc index 4016273708..c90fd700ce 100644 --- a/webrtc/p2p/base/transportcontroller_unittest.cc +++ b/webrtc/p2p/base/transportcontroller_unittest.cc @@ -303,20 +303,19 @@ TEST_F(TransportControllerTest, TestSetAndGetLocalCertificate) { TEST_F(TransportControllerTest, TestGetRemoteSSLCertificate) { rtc::FakeSSLCertificate fake_certificate("fake_data"); - rtc::scoped_ptr returned_certificate; FakeTransportChannel* channel = CreateChannel("audio", 1); ASSERT_NE(nullptr, channel); channel->SetRemoteSSLCertificate(&fake_certificate); - EXPECT_TRUE(transport_controller_->GetRemoteSSLCertificate( - "audio", returned_certificate.accept())); + rtc::scoped_ptr returned_certificate = + transport_controller_->GetRemoteSSLCertificate("audio"); + EXPECT_TRUE(returned_certificate); EXPECT_EQ(fake_certificate.ToPEMString(), returned_certificate->ToPEMString()); // Should fail if called for a nonexistant transport. - EXPECT_FALSE(transport_controller_->GetRemoteSSLCertificate( - "video", returned_certificate.accept())); + EXPECT_FALSE(transport_controller_->GetRemoteSSLCertificate("video")); } TEST_F(TransportControllerTest, TestSetLocalTransportDescription) {