From 7bea1ffe772e837d96f8faa5c9dd06e531b95379 Mon Sep 17 00:00:00 2001 From: "pthatcher@webrtc.org" Date: Wed, 4 Mar 2015 01:38:30 +0000 Subject: [PATCH] Expose negotiated ciphers through stats API. Use the new internal API to expose the negotiated SRTP/SSL ciphers through the stats API. This is a follow-up to https://webrtc-codereview.appspot.com/37209004. BUG=3976 R=juberti@webrtc.org Review URL: https://webrtc-codereview.appspot.com/35169004 Cr-Commit-Position: refs/heads/master@{#8584} git-svn-id: http://webrtc.googlecode.com/svn/trunk@8584 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/app/webrtc/peerconnection_unittest.cc | 43 +++++++++++++++++++ talk/app/webrtc/statscollector.cc | 21 ++++++++- talk/app/webrtc/statscollector_unittest.cc | 26 +++++++++++ talk/app/webrtc/statstypes.cc | 12 ++++-- talk/app/webrtc/statstypes.h | 3 ++ .../webrtc/test/mockpeerconnectionobservers.h | 41 +++++++++++++++++- 6 files changed, 140 insertions(+), 6 deletions(-) diff --git a/talk/app/webrtc/peerconnection_unittest.cc b/talk/app/webrtc/peerconnection_unittest.cc index d6b9bf23b5..8e1c43f48e 100644 --- a/talk/app/webrtc/peerconnection_unittest.cc +++ b/talk/app/webrtc/peerconnection_unittest.cc @@ -102,6 +102,15 @@ static const char kVideoTrackLabelBase[] = "video_track"; static const char kAudioTrackLabelBase[] = "audio_track"; static const char kDataChannelLabel[] = "data_channel"; +// Disable for TSan v2, see +// https://code.google.com/p/webrtc/issues/detail?id=1205 for details. +// This declaration is also #ifdef'd as it causes unused-variable errors. +#if !defined(THREAD_SANITIZER) +// SRTP cipher name negotiated by the tests. This must be updated if the +// default changes. +static const char kDefaultSrtpCipher[] = "AES_CM_128_HMAC_SHA1_32"; +#endif + static void RemoveLinesFromSdp(const std::string& line_start, std::string* sdp) { const char kSdpLineEnd[] = "\r\n"; @@ -380,6 +389,24 @@ class PeerConnectionTestClientBase return bw; } + std::string GetDtlsCipherStats() { + rtc::scoped_refptr + observer(new rtc::RefCountedObject()); + EXPECT_TRUE(peer_connection_->GetStats( + observer, NULL, PeerConnectionInterface::kStatsOutputLevelStandard)); + EXPECT_TRUE_WAIT(observer->called(), kMaxWaitMs); + return observer->DtlsCipher(); + } + + std::string GetSrtpCipherStats() { + rtc::scoped_refptr + observer(new rtc::RefCountedObject()); + EXPECT_TRUE(peer_connection_->GetStats( + observer, NULL, PeerConnectionInterface::kStatsOutputLevelStandard)); + EXPECT_TRUE_WAIT(observer->called(), kMaxWaitMs); + return observer->SrtpCipher(); + } + int rendered_width() { EXPECT_FALSE(fake_video_renderers_.empty()); return fake_video_renderers_.empty() ? 1 : @@ -1280,6 +1307,22 @@ TEST_F(JsepPeerConnectionP2PTestClient, GetBytesSentStats) { kMaxWaitForStatsMs); } +// Test that we can get negotiated ciphers. +TEST_F(JsepPeerConnectionP2PTestClient, GetNegotiatedCiphersStats) { + ASSERT_TRUE(CreateTestClients()); + LocalP2PTest(); + + EXPECT_EQ_WAIT( + rtc::SSLStreamAdapter::GetDefaultSslCipher(), + initializing_client()->GetDtlsCipherStats(), + kMaxWaitForStatsMs); + + EXPECT_EQ_WAIT( + kDefaultSrtpCipher, + initializing_client()->GetSrtpCipherStats(), + kMaxWaitForStatsMs); +} + // This test sets up a call between two parties with audio, video and data. TEST_F(JsepPeerConnectionP2PTestClient, LocalP2PTestDataChannel) { FakeConstraints setup_constraints; diff --git a/talk/app/webrtc/statscollector.cc b/talk/app/webrtc/statscollector.cc index 968fa8f30a..d434aafe31 100644 --- a/talk/app/webrtc/statscollector.cc +++ b/talk/app/webrtc/statscollector.cc @@ -715,6 +715,18 @@ void StatsCollector::ExtractSessionInfo() { StatsReport::kStatsValueNameRemoteCertificateId, remote_cert_report_id); } + const std::string& srtp_cipher = channel_iter->srtp_cipher; + if (!srtp_cipher.empty()) { + channel_report->AddValue( + StatsReport::kStatsValueNameSrtpCipher, + srtp_cipher); + } + const std::string& ssl_cipher = channel_iter->ssl_cipher; + if (!ssl_cipher.empty()) { + channel_report->AddValue( + StatsReport::kStatsValueNameDtlsCipher, + ssl_cipher); + } for (size_t i = 0; i < channel_iter->connection_infos.size(); ++i) { @@ -742,8 +754,6 @@ void StatsCollector::ExtractSessionInfo() { info.writable); report->AddBoolean(StatsReport::kStatsValueNameReadable, info.readable); - report->AddBoolean(StatsReport::kStatsValueNameActiveConnection, - info.best_connection); report->AddValue(StatsReport::kStatsValueNameLocalCandidateId, AddCandidateReport(info.local_candidate, true)); report->AddValue( @@ -760,6 +770,13 @@ void StatsCollector::ExtractSessionInfo() { info.local_candidate.type()); report->AddValue(StatsReport::kStatsValueNameRemoteCandidateType, info.remote_candidate.type()); + report->AddBoolean(StatsReport::kStatsValueNameActiveConnection, + info.best_connection); + if (info.best_connection) { + channel_report->AddValue( + StatsReport::kStatsValueNameSelectedCandidatePairId, + report->id().ToString()); + } } } } diff --git a/talk/app/webrtc/statscollector_unittest.cc b/talk/app/webrtc/statscollector_unittest.cc index 8a1dc77ba5..8fd59e93a0 100644 --- a/talk/app/webrtc/statscollector_unittest.cc +++ b/talk/app/webrtc/statscollector_unittest.cc @@ -644,6 +644,8 @@ class StatsCollectorTest : public testing::Test { // Fake stats to process. cricket::TransportChannelStats channel_stats; channel_stats.component = 1; + channel_stats.srtp_cipher = "the-srtp-cipher"; + channel_stats.ssl_cipher = "the-ssl-cipher"; cricket::TransportStats transport_stats; transport_stats.content_name = "audio"; @@ -713,6 +715,18 @@ class StatsCollectorTest : public testing::Test { } else { EXPECT_EQ(kNotFound, remote_certificate_id); } + + // Check negotiated ciphers. + std::string dtls_cipher = ExtractStatsValue( + StatsReport::kStatsReportTypeComponent, + reports, + StatsReport::kStatsValueNameDtlsCipher); + EXPECT_EQ("the-ssl-cipher", dtls_cipher); + std::string srtp_cipher = ExtractStatsValue( + StatsReport::kStatsReportTypeComponent, + reports, + StatsReport::kStatsValueNameSrtpCipher); + EXPECT_EQ("the-srtp-cipher", srtp_cipher); } cricket::FakeMediaEngine* media_engine_; @@ -1318,6 +1332,18 @@ TEST_F(StatsCollectorTest, NoTransport) { reports, StatsReport::kStatsValueNameRemoteCertificateId); ASSERT_EQ(kNotFound, remote_certificate_id); + + // Check that the negotiated ciphers are absent. + std::string dtls_cipher = ExtractStatsValue( + StatsReport::kStatsReportTypeComponent, + reports, + StatsReport::kStatsValueNameDtlsCipher); + ASSERT_EQ(kNotFound, dtls_cipher); + std::string srtp_cipher = ExtractStatsValue( + StatsReport::kStatsReportTypeComponent, + reports, + StatsReport::kStatsValueNameSrtpCipher); + ASSERT_EQ(kNotFound, srtp_cipher); } // This test verifies that the stats are generated correctly when the transport diff --git a/talk/app/webrtc/statstypes.cc b/talk/app/webrtc/statstypes.cc index f41bf7a6a8..e88583f14a 100644 --- a/talk/app/webrtc/statstypes.cc +++ b/talk/app/webrtc/statstypes.cc @@ -54,7 +54,7 @@ const char* InternalTypeToString(StatsReport::StatsType type) { case StatsReport::kStatsReportTypeIceRemoteCandidate: return "remotecandidate"; case StatsReport::kStatsReportTypeTransport: - return "googTransport"; + return "transport"; case StatsReport::kStatsReportTypeComponent: return "googComponent"; case StatsReport::kStatsReportTypeCandidatePair: @@ -244,6 +244,8 @@ const char* StatsReport::Value::display_name() const { return "protocol"; case kStatsValueNameTransportId: return "transportId"; + case kStatsValueNameSelectedCandidatePairId: + return "selectedCandidatePairId"; case kStatsValueNameSsrc: return "ssrc"; case kStatsValueNameState: @@ -310,6 +312,8 @@ const char* StatsReport::Value::display_name() const { return "googDecodingPLCCNG"; case kStatsValueNameDer: return "googDerBase64"; + case kStatsValueNameDtlsCipher: + return "dtlsCipher"; case kStatsValueNameEchoCancellationQualityMin: return "googEchoCancellationQualityMin"; case kStatsValueNameEchoDelayMedian: @@ -383,7 +387,7 @@ const char* StatsReport::Value::display_name() const { case kStatsValueNameLocalCandidateType: return "googLocalCandidateType"; case kStatsValueNameLocalCertificateId: - return "googLocalCertificateId"; + return "localCertificateId"; case kStatsValueNameAdaptationChanges: return "googAdaptationChanges"; case kStatsValueNameNacksReceived: @@ -411,7 +415,7 @@ const char* StatsReport::Value::display_name() const { case kStatsValueNameRemoteCandidateType: return "googRemoteCandidateType"; case kStatsValueNameRemoteCertificateId: - return "googRemoteCertificateId"; + return "remoteCertificateId"; case kStatsValueNameRetransmitBitrate: return "googRetransmitBitrate"; case kStatsValueNameRtt: @@ -422,6 +426,8 @@ const char* StatsReport::Value::display_name() const { return "packetsDiscardedOnSend"; case kStatsValueNameSpeechExpandRate: return "googSpeechExpandRate"; + case kStatsValueNameSrtpCipher: + return "srtpCipher"; case kStatsValueNameTargetEncBitrate: return "googTargetEncBitrate"; case kStatsValueNameTransmitBitrate: diff --git a/talk/app/webrtc/statstypes.h b/talk/app/webrtc/statstypes.h index 65549df857..9fe9c11bfd 100644 --- a/talk/app/webrtc/statstypes.h +++ b/talk/app/webrtc/statstypes.h @@ -125,6 +125,7 @@ class StatsReport { kStatsValueNamePacketsSent, kStatsValueNameProtocol, kStatsValueNameReadable, + kStatsValueNameSelectedCandidatePairId, kStatsValueNameSsrc, kStatsValueNameState, kStatsValueNameTransportId, @@ -160,6 +161,7 @@ class StatsReport { kStatsValueNameDecodingPLC, kStatsValueNameDecodingPLCCNG, kStatsValueNameDer, + kStatsValueNameDtlsCipher, kStatsValueNameEchoCancellationQualityMin, kStatsValueNameEchoDelayMedian, kStatsValueNameEchoDelayStdDev, @@ -211,6 +213,7 @@ class StatsReport { kStatsValueNameSecondaryDecodedRate, kStatsValueNameSendPacketsDiscarded, kStatsValueNameSpeechExpandRate, + kStatsValueNameSrtpCipher, kStatsValueNameTargetDelayMs, kStatsValueNameTargetEncBitrate, kStatsValueNameTrackId, diff --git a/talk/app/webrtc/test/mockpeerconnectionobservers.h b/talk/app/webrtc/test/mockpeerconnectionobservers.h index 9b06c2fa5b..4ab6e8e1e9 100644 --- a/talk/app/webrtc/test/mockpeerconnectionobservers.h +++ b/talk/app/webrtc/test/mockpeerconnectionobservers.h @@ -123,7 +123,7 @@ class MockStatsObserver : public webrtc::StatsObserver { virtual void OnComplete(const StatsReports& reports) { ASSERT(!called_); called_ = true; - memset(&stats_, 0, sizeof(stats_)); + stats_.Clear(); stats_.number_of_reports = reports.size(); for (const auto* r : reports) { if (r->type() == StatsReport::kStatsReportTypeSsrc) { @@ -138,6 +138,11 @@ class MockStatsObserver : public webrtc::StatsObserver { } else if (r->type() == StatsReport::kStatsReportTypeBwe) { GetIntValue(r, StatsReport::kStatsValueNameAvailableReceiveBandwidth, &stats_.available_receive_bandwidth); + } else if (r->type() == StatsReport::kStatsReportTypeComponent) { + GetStringValue(r, StatsReport::kStatsValueNameDtlsCipher, + &stats_.dtls_cipher); + GetStringValue(r, StatsReport::kStatsValueNameSrtpCipher, + &stats_.srtp_cipher); } } } @@ -170,6 +175,16 @@ class MockStatsObserver : public webrtc::StatsObserver { return stats_.available_receive_bandwidth; } + std::string DtlsCipher() const { + ASSERT(called_); + return stats_.dtls_cipher; + } + + std::string SrtpCipher() const { + ASSERT(called_); + return stats_.srtp_cipher; + } + private: bool GetIntValue(const StatsReport* report, StatsReport::StatsValueName name, @@ -182,15 +197,39 @@ class MockStatsObserver : public webrtc::StatsObserver { } return false; } + bool GetStringValue(const StatsReport* report, + StatsReport::StatsValueName name, + std::string* value) { + for (const auto& v : report->values()) { + if (v->name == name) { + *value = v->value; + return true; + } + } + return false; + } bool called_; struct { + void Clear() { + number_of_reports = 0; + audio_output_level = 0; + audio_input_level = 0; + bytes_received = 0; + bytes_sent = 0; + available_receive_bandwidth = 0; + dtls_cipher.clear(); + srtp_cipher.clear(); + } + size_t number_of_reports; int audio_output_level; int audio_input_level; int bytes_received; int bytes_sent; int available_receive_bandwidth; + std::string dtls_cipher; + std::string srtp_cipher; } stats_; };