From 70b820fefe899f9a16401b98faf5f38d8728bea2 Mon Sep 17 00:00:00 2001 From: Zhi Huang Date: Sat, 27 Jan 2018 14:16:15 -0800 Subject: [PATCH] Implemented the GetRemoteAudioSSLCertificate method. This method returns the DTLS SSL certificate chain associated with the audio transport on the remote side. This will become populated once the DTLS connection with the peer has been completed. TBR=deadbeef@webrtc.org Bug: webrtc:8800 Change-Id: Ib90ccb3463415e798c17c187c5bdbfc4da26f11f Reviewed-on: https://webrtc-review.googlesource.com/44140 Commit-Queue: Zhi Huang Reviewed-by: Taylor Brandstetter Cr-Commit-Position: refs/heads/master@{#21785} --- p2p/base/dtlstransport.cc | 9 +++++ p2p/base/dtlstransport.h | 6 +++ p2p/base/dtlstransport_unittest.cc | 55 ++++++++++++++-------------- p2p/base/dtlstransportinternal.h | 3 ++ p2p/base/fakedtlstransport.h | 3 ++ pc/peerconnection.cc | 10 +++++ pc/peerconnection.h | 3 ++ pc/peerconnection_integrationtest.cc | 21 +++++++++++ pc/transportcontroller.cc | 15 ++++++++ pc/transportcontroller.h | 3 ++ 10 files changed, 100 insertions(+), 28 deletions(-) diff --git a/p2p/base/dtlstransport.cc b/p2p/base/dtlstransport.cc index 179b032adc..0e7bd60754 100644 --- a/p2p/base/dtlstransport.cc +++ b/p2p/base/dtlstransport.cc @@ -308,6 +308,15 @@ std::unique_ptr DtlsTransport::GetRemoteSSLCertificate() return dtls_->GetPeerCertificate(); } +std::unique_ptr DtlsTransport::GetRemoteSSLCertChain() + const { + if (!dtls_) { + return nullptr; + } + + return dtls_->GetPeerSSLCertChain(); +} + bool DtlsTransport::ExportKeyingMaterial(const std::string& label, const uint8_t* context, size_t context_len, diff --git a/p2p/base/dtlstransport.h b/p2p/base/dtlstransport.h index e7b0c5654a..a4f4bcd57b 100644 --- a/p2p/base/dtlstransport.h +++ b/p2p/base/dtlstransport.h @@ -141,8 +141,14 @@ class DtlsTransport : public DtlsTransportInternal { // Once DTLS has been established, this method retrieves the certificate in // use by the remote peer, for use in external identity verification. + // TODO(zhihuang): Remove all the SSLCertificate versions of these methods, + // and replace them with the SSLCertChain versions. Implement the + // PeerConnection::GetRemoteSSLCertificate using the SSLCertChain version. std::unique_ptr GetRemoteSSLCertificate() const override; + // Version of the above method that returns the full certificate chain. + std::unique_ptr GetRemoteSSLCertChain() const override; + // Once DTLS has established (i.e., this ice_transport is writable), this // method extracts the keys negotiated during the DTLS handshake, for use in // external encryption. DTLS-SRTP uses this to extract the needed SRTP keys. diff --git a/p2p/base/dtlstransport_unittest.cc b/p2p/base/dtlstransport_unittest.cc index cac78647d6..430b41dbc2 100644 --- a/p2p/base/dtlstransport_unittest.cc +++ b/p2p/base/dtlstransport_unittest.cc @@ -256,7 +256,7 @@ class DtlsTestClient : public sigslot::has_slots<> { size_t size, const rtc::PacketTime& time, int flags) { - // Flags shouldn't be set on the underlying TransportChannel packets. + // Flags shouldn't be set on the underlying Transport packets. ASSERT_EQ(0, flags); // Look at the handshake packets to see what role we played. @@ -291,15 +291,14 @@ class DtlsTestClient : public sigslot::has_slots<> { rtc::SentPacket sent_packet_; }; -// Base class for DtlsTransportChannelTest and DtlsEventOrderingTest, which +// Base class for DtlsTransportTest and DtlsEventOrderingTest, which // inherit from different variants of testing::Test. // // Note that this test always uses a FakeClock, due to the |fake_clock_| member // variable. -class DtlsTransportChannelTestBase { +class DtlsTransportTestBase { public: - DtlsTransportChannelTestBase() - : client1_("P1"), client2_("P2"), use_dtls_(false) {} + DtlsTransportTestBase() : client1_("P1"), client2_("P2"), use_dtls_(false) {} void SetMaxProtocolVersions(rtc::SSLProtocolVersion c1, rtc::SSLProtocolVersion c2) { @@ -380,22 +379,21 @@ class DtlsTransportChannelTestBase { rtc::ScopedFakeClock fake_clock_; DtlsTestClient client1_; DtlsTestClient client2_; - int channel_ct_; bool use_dtls_; rtc::SSLProtocolVersion ssl_expected_version_; }; -class DtlsTransportChannelTest : public DtlsTransportChannelTestBase, - public ::testing::Test {}; +class DtlsTransportTest : public DtlsTransportTestBase, + public ::testing::Test {}; // Connect without DTLS, and transfer RTP data. -TEST_F(DtlsTransportChannelTest, TestTransferRtp) { +TEST_F(DtlsTransportTest, TestTransferRtp) { ASSERT_TRUE(Connect()); TestTransfer(1000, 100, /*srtp=*/false); } // Test that the SignalSentPacket signal is wired up. -TEST_F(DtlsTransportChannelTest, TestSignalSentPacket) { +TEST_F(DtlsTransportTest, TestSignalSentPacket) { ASSERT_TRUE(Connect()); // Sanity check default value (-1). ASSERT_EQ(client1_.sent_packet().send_time_ms, -1); @@ -407,13 +405,13 @@ TEST_F(DtlsTransportChannelTest, TestSignalSentPacket) { } // Connect without DTLS, and transfer SRTP data. -TEST_F(DtlsTransportChannelTest, TestTransferSrtp) { +TEST_F(DtlsTransportTest, TestTransferSrtp) { ASSERT_TRUE(Connect()); TestTransfer(1000, 100, /*srtp=*/true); } // Connect with DTLS, and transfer data over DTLS. -TEST_F(DtlsTransportChannelTest, TestTransferDtls) { +TEST_F(DtlsTransportTest, TestTransferDtls) { PrepareDtls(rtc::KT_DEFAULT); ASSERT_TRUE(Connect()); TestTransfer(1000, 100, /*srtp=*/false); @@ -423,7 +421,7 @@ TEST_F(DtlsTransportChannelTest, TestTransferDtls) { // Our DTLS implementation doesn't do this, but other implementations may; // see https://tools.ietf.org/html/rfc6347#section-4.1.1. // This has caused interoperability problems with ORTCLib in the past. -TEST_F(DtlsTransportChannelTest, TestTransferDtlsCombineRecords) { +TEST_F(DtlsTransportTest, TestTransferDtlsCombineRecords) { PrepareDtls(rtc::KT_DEFAULT); ASSERT_TRUE(Connect()); // Our DTLS implementation always sends one record per packet, so to simulate @@ -435,8 +433,8 @@ TEST_F(DtlsTransportChannelTest, TestTransferDtlsCombineRecords) { TestTransfer(500, 100, /*srtp=*/false); } -class DtlsTransportChannelVersionTest - : public DtlsTransportChannelTestBase, +class DtlsTransportVersionTest + : public DtlsTransportTestBase, public ::testing::TestWithParam< ::testing::tuple> { }; @@ -444,7 +442,7 @@ class DtlsTransportChannelVersionTest // Test that an acceptable cipher suite is negotiated when different versions // of DTLS are supported. Note that it's IsAcceptableCipher that does the actual // work. -TEST_P(DtlsTransportChannelVersionTest, TestCipherSuiteNegotiation) { +TEST_P(DtlsTransportVersionTest, TestCipherSuiteNegotiation) { PrepareDtls(rtc::KT_DEFAULT); SetMaxProtocolVersions(::testing::get<0>(GetParam()), ::testing::get<1>(GetParam())); @@ -454,14 +452,14 @@ TEST_P(DtlsTransportChannelVersionTest, TestCipherSuiteNegotiation) { // Will test every combination of 1.0/1.2 on the client and server. INSTANTIATE_TEST_CASE_P( TestCipherSuiteNegotiation, - DtlsTransportChannelVersionTest, + DtlsTransportVersionTest, ::testing::Combine(::testing::Values(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_12), ::testing::Values(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_12))); // Connect with DTLS, negotiating DTLS-SRTP, and transfer SRTP using bypass. -TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtp) { +TEST_F(DtlsTransportTest, TestTransferDtlsSrtp) { PrepareDtls(rtc::KT_DEFAULT); ASSERT_TRUE(Connect()); TestTransfer(1000, 100, /*srtp=*/true); @@ -469,14 +467,15 @@ TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtp) { // Connect with DTLS-SRTP, transfer an invalid SRTP packet, and expects -1 // returned. -TEST_F(DtlsTransportChannelTest, TestTransferDtlsInvalidSrtpPacket) { +TEST_F(DtlsTransportTest, TestTransferDtlsInvalidSrtpPacket) { PrepareDtls(rtc::KT_DEFAULT); ASSERT_TRUE(Connect()); EXPECT_EQ(-1, client1_.SendInvalidSrtpPacket(100)); } -// Create a single channel with DTLS, and send normal data and SRTP data on it. -TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpDemux) { +// Create a single transport with DTLS, and send normal data and SRTP data on +// it. +TEST_F(DtlsTransportTest, TestTransferDtlsSrtpDemux) { PrepareDtls(rtc::KT_DEFAULT); ASSERT_TRUE(Connect()); TestTransfer(1000, 100, /*srtp=*/false); @@ -484,7 +483,7 @@ TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpDemux) { } // Test transferring when the "answerer" has the server role. -TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpAnswererIsPassive) { +TEST_F(DtlsTransportTest, TestTransferDtlsSrtpAnswererIsPassive) { PrepareDtls(rtc::KT_DEFAULT); ASSERT_TRUE(Connect(/*client1_server=*/false)); TestTransfer(1000, 100, /*srtp=*/true); @@ -492,7 +491,7 @@ TEST_F(DtlsTransportChannelTest, TestTransferDtlsSrtpAnswererIsPassive) { // Test that renegotiation (setting same role and fingerprint again) can be // started before the clients become connected in the first negotiation. -TEST_F(DtlsTransportChannelTest, TestRenegotiateBeforeConnect) { +TEST_F(DtlsTransportTest, TestRenegotiateBeforeConnect) { PrepareDtls(rtc::KT_DEFAULT); // Note: This is doing the same thing Connect normally does, minus some // additional checks not relevant for this test. @@ -506,7 +505,7 @@ TEST_F(DtlsTransportChannelTest, TestRenegotiateBeforeConnect) { } // Test Certificates state after negotiation but before connection. -TEST_F(DtlsTransportChannelTest, TestCertificatesBeforeConnect) { +TEST_F(DtlsTransportTest, TestCertificatesBeforeConnect) { PrepareDtls(rtc::KT_DEFAULT); Negotiate(); @@ -521,7 +520,7 @@ TEST_F(DtlsTransportChannelTest, TestCertificatesBeforeConnect) { } // Test Certificates state after connection. -TEST_F(DtlsTransportChannelTest, TestCertificatesAfterConnect) { +TEST_F(DtlsTransportTest, TestCertificatesAfterConnect) { PrepareDtls(rtc::KT_DEFAULT); ASSERT_TRUE(Connect()); @@ -548,7 +547,7 @@ TEST_F(DtlsTransportChannelTest, TestCertificatesAfterConnect) { // Each time a timeout occurs, the retransmission timer should be doubled up to // 60 seconds. The timer defaults to 1 second, but for WebRTC we should be // initializing it to 50ms. -TEST_F(DtlsTransportChannelTest, TestRetransmissionSchedule) { +TEST_F(DtlsTransportTest, TestRetransmissionSchedule) { // We can only change the retransmission schedule with a recently-added // BoringSSL API. Skip the test if not built with BoringSSL. MAYBE_SKIP_TEST(IsBoringSsl); @@ -609,7 +608,7 @@ enum DtlsTransportEvent { }; class DtlsEventOrderingTest - : public DtlsTransportChannelTestBase, + : public DtlsTransportTestBase, public ::testing::TestWithParam< ::testing::tuple, bool>> { protected: @@ -683,7 +682,7 @@ class DtlsEventOrderingTest client2_.dtls_transport()->dtls_state(), kTimeout, fake_clock_); - // Channel should be writable iff there was a valid fingerprint. + // Transports should be writable iff there was a valid fingerprint. EXPECT_EQ(valid_fingerprint, client1_.dtls_transport()->writable()); EXPECT_EQ(valid_fingerprint, client2_.dtls_transport()->writable()); diff --git a/p2p/base/dtlstransportinternal.h b/p2p/base/dtlstransportinternal.h index 2acbda64fa..1af32ac35c 100644 --- a/p2p/base/dtlstransportinternal.h +++ b/p2p/base/dtlstransportinternal.h @@ -82,6 +82,9 @@ class DtlsTransportInternal : public rtc::PacketTransportInternal { virtual std::unique_ptr GetRemoteSSLCertificate() const = 0; + // Gets a copy of the remote side's SSL certificate chain. + virtual std::unique_ptr GetRemoteSSLCertChain() const = 0; + // Allows key material to be extracted for external encryption. virtual bool ExportKeyingMaterial(const std::string& label, const uint8_t* context, diff --git a/p2p/base/fakedtlstransport.h b/p2p/base/fakedtlstransport.h index 54b70180d1..391486ceec 100644 --- a/p2p/base/fakedtlstransport.h +++ b/p2p/base/fakedtlstransport.h @@ -169,6 +169,9 @@ class FakeDtlsTransport : public DtlsTransportInternal { remote_cert_->GetReference()) : nullptr; } + std::unique_ptr GetRemoteSSLCertChain() const override { + return nullptr; + } bool ExportKeyingMaterial(const std::string& label, const uint8_t* context, size_t context_len, diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 3578095ce3..a6297616b9 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -2766,6 +2766,16 @@ PeerConnection::GetRemoteAudioSSLCertificate() { return GetRemoteSSLCertificate(voice_channel()->transport_name()); } +std::unique_ptr +PeerConnection::GetRemoteAudioSSLCertChain() { + if (!voice_channel()) { + return nullptr; + } + + return transport_controller_->GetRemoteSSLCertChain( + voice_channel()->transport_name()); +} + bool PeerConnection::StartRtcEventLog(rtc::PlatformFile file, int64_t max_size_bytes) { // TODO(eladalon): It would be better to not allow negative values into PC. diff --git a/pc/peerconnection.h b/pc/peerconnection.h index e88fab82a7..8dd40f4597 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -96,6 +96,9 @@ class PeerConnection : public PeerConnectionInternal, // See https://www.w3.org/TR/webrtc/#rtcdtlstransport-interface std::unique_ptr GetRemoteAudioSSLCertificate(); + // Version of the above method that returns the full certificate chain. + std::unique_ptr GetRemoteAudioSSLCertChain(); + rtc::scoped_refptr CreateDtmfSender( AudioTrackInterface* track) override; diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc index 2768358d82..788b432edd 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -1362,6 +1362,11 @@ TEST_F(PeerConnectionIntegrationTest, auto pc = reinterpret_cast(pci->internal()); return pc->GetRemoteAudioSSLCertificate(); }; + auto GetRemoteAudioSSLCertChain = [](PeerConnectionWrapper* wrapper) { + auto pci = reinterpret_cast(wrapper->pc()); + auto pc = reinterpret_cast(pci->internal()); + return pc->GetRemoteAudioSSLCertChain(); + }; auto caller_cert = rtc::RTCCertificate::FromPEM(kRsaPems[0]); auto callee_cert = rtc::RTCCertificate::FromPEM(kRsaPems[1]); @@ -1381,6 +1386,8 @@ TEST_F(PeerConnectionIntegrationTest, // calling this method should not crash). EXPECT_EQ(nullptr, GetRemoteAudioSSLCertificate(caller())); EXPECT_EQ(nullptr, GetRemoteAudioSSLCertificate(callee())); + EXPECT_EQ(nullptr, GetRemoteAudioSSLCertChain(caller())); + EXPECT_EQ(nullptr, GetRemoteAudioSSLCertChain(callee())); caller()->AddAudioTrack(); callee()->AddAudioTrack(); @@ -1400,6 +1407,20 @@ TEST_F(PeerConnectionIntegrationTest, ASSERT_TRUE(callee_remote_cert); EXPECT_EQ(caller_cert->ssl_certificate().ToPEMString(), callee_remote_cert->ToPEMString()); + + auto caller_remote_cert_chain = GetRemoteAudioSSLCertChain(caller()); + ASSERT_TRUE(caller_remote_cert_chain); + ASSERT_EQ(1U, caller_remote_cert_chain->GetSize()); + auto remote_cert = &caller_remote_cert_chain->Get(0); + EXPECT_EQ(callee_cert->ssl_certificate().ToPEMString(), + remote_cert->ToPEMString()); + + auto callee_remote_cert_chain = GetRemoteAudioSSLCertChain(callee()); + ASSERT_TRUE(callee_remote_cert_chain); + ASSERT_EQ(1U, callee_remote_cert_chain->GetSize()); + remote_cert = &callee_remote_cert_chain->Get(0); + EXPECT_EQ(caller_cert->ssl_certificate().ToPEMString(), + remote_cert->ToPEMString()); } // This test sets up a call between two parties (using DTLS) and tests that we diff --git a/pc/transportcontroller.cc b/pc/transportcontroller.cc index bd9ea7505a..38200f8539 100644 --- a/pc/transportcontroller.cc +++ b/pc/transportcontroller.cc @@ -200,6 +200,21 @@ TransportController::GetRemoteSSLCertificate( this, transport_name)); } +std::unique_ptr TransportController::GetRemoteSSLCertChain( + const std::string& transport_name) const { + if (!network_thread_->IsCurrent()) { + return network_thread_->Invoke>( + RTC_FROM_HERE, [&] { return GetRemoteSSLCertChain(transport_name); }); + } + + const RefCountedChannel* ch = + GetChannel_n(transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); + if (!ch) { + return nullptr; + } + return ch->dtls()->GetRemoteSSLCertChain(); +} + bool TransportController::SetLocalTransportDescription( const std::string& transport_name, const TransportDescription& tdesc, diff --git a/pc/transportcontroller.h b/pc/transportcontroller.h index 67fab734b9..705fc4c38f 100644 --- a/pc/transportcontroller.h +++ b/pc/transportcontroller.h @@ -93,6 +93,9 @@ class TransportController : public sigslot::has_slots<>, // reporting. std::unique_ptr GetRemoteSSLCertificate( const std::string& transport_name) const; + std::unique_ptr GetRemoteSSLCertChain( + const std::string& transport_name) const; + bool SetLocalTransportDescription(const std::string& transport_name, const TransportDescription& tdesc, webrtc::SdpType type,