diff --git a/talk/app/webrtc/fakemetricsobserver.cc b/talk/app/webrtc/fakemetricsobserver.cc index 4a100a079e..9c300ccd60 100644 --- a/talk/app/webrtc/fakemetricsobserver.cc +++ b/talk/app/webrtc/fakemetricsobserver.cc @@ -37,7 +37,10 @@ FakeMetricsObserver::FakeMetricsObserver() { void FakeMetricsObserver::Reset() { RTC_DCHECK(thread_checker_.CalledOnValidThread()); counters_.clear(); - memset(histogram_samples_, 0, sizeof(histogram_samples_)); + memset(int_histogram_samples_, 0, sizeof(int_histogram_samples_)); + for (std::string& type : string_histogram_samples_) { + type.clear(); + } } void FakeMetricsObserver::IncrementEnumCounter( @@ -49,31 +52,43 @@ void FakeMetricsObserver::IncrementEnumCounter( counters_.resize(type + 1); } auto& counters = counters_[type]; + if (counters.size() < static_cast(counter_max)) { + counters.resize(counter_max); + } ++counters[counter]; } void FakeMetricsObserver::AddHistogramSample(PeerConnectionMetricsName type, int value) { RTC_DCHECK(thread_checker_.CalledOnValidThread()); - RTC_DCHECK_EQ(histogram_samples_[type], 0); - histogram_samples_[type] = value; + RTC_DCHECK_EQ(int_histogram_samples_[type], 0); + int_histogram_samples_[type] = value; +} + +void FakeMetricsObserver::AddHistogramSample(PeerConnectionMetricsName type, + const std::string& value) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + string_histogram_samples_[type].assign(value); } int FakeMetricsObserver::GetEnumCounter(PeerConnectionEnumCounterType type, int counter) const { RTC_DCHECK(thread_checker_.CalledOnValidThread()); - RTC_CHECK(counters_.size() > static_cast(type)); - const auto& it = counters_[type].find(counter); - if (it == counters_[type].end()) { - return 0; - } - return it->second; + RTC_CHECK(counters_.size() > static_cast(type) && + counters_[type].size() > static_cast(counter)); + return counters_[type][counter]; } -int FakeMetricsObserver::GetHistogramSample( +int FakeMetricsObserver::GetIntHistogramSample( PeerConnectionMetricsName type) const { RTC_DCHECK(thread_checker_.CalledOnValidThread()); - return histogram_samples_[type]; + return int_histogram_samples_[type]; +} + +const std::string& FakeMetricsObserver::GetStringHistogramSample( + PeerConnectionMetricsName type) const { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + return string_histogram_samples_[type]; } } // namespace webrtc diff --git a/talk/app/webrtc/fakemetricsobserver.h b/talk/app/webrtc/fakemetricsobserver.h index e3a22841d8..39454bf4db 100644 --- a/talk/app/webrtc/fakemetricsobserver.h +++ b/talk/app/webrtc/fakemetricsobserver.h @@ -46,21 +46,25 @@ class FakeMetricsObserver : public MetricsObserverInterface { int counter_max) override; void AddHistogramSample(PeerConnectionMetricsName type, int value) override; + void AddHistogramSample(PeerConnectionMetricsName type, + const std::string& value) override; // Accessors to be used by the tests. int GetEnumCounter(PeerConnectionEnumCounterType type, int counter) const; - int GetHistogramSample(PeerConnectionMetricsName type) const; + int GetIntHistogramSample(PeerConnectionMetricsName type) const; + const std::string& GetStringHistogramSample( + PeerConnectionMetricsName type) const; protected: ~FakeMetricsObserver() {} private: rtc::ThreadChecker thread_checker_; - // The vector contains maps for each counter type. In the map, it's a mapping - // from individual counter to its count, such that it's memory efficient when - // comes to sparse enum types, like the SSL ciphers in the IANA registry. - std::vector> counters_; - int histogram_samples_[kPeerConnectionMetricsName_Max]; + // This is a 2 dimension array. The first index is the enum counter type. The + // 2nd index is the counter of that particular enum counter type. + std::vector> counters_; + int int_histogram_samples_[kPeerConnectionMetricsName_Max]; + std::string string_histogram_samples_[kPeerConnectionMetricsName_Max]; }; } // namespace webrtc diff --git a/talk/app/webrtc/peerconnection_unittest.cc b/talk/app/webrtc/peerconnection_unittest.cc index d593f888ff..c077fe003f 100644 --- a/talk/app/webrtc/peerconnection_unittest.cc +++ b/talk/app/webrtc/peerconnection_unittest.cc @@ -1342,22 +1342,21 @@ TEST_F(JsepPeerConnectionP2PTestClient, GetDtls12None) { initializing_client()->pc()->RegisterUMAObserver(init_observer); LocalP2PTest(); - EXPECT_EQ_WAIT(rtc::SSLStreamAdapter::GetSslCipherSuiteName( - rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( - rtc::SSL_PROTOCOL_DTLS_10, rtc::KT_DEFAULT)), - initializing_client()->GetDtlsCipherStats(), - kMaxWaitForStatsMs); - EXPECT_EQ(1, init_observer->GetEnumCounter( - webrtc::kEnumCounterAudioSslCipher, - rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( - rtc::SSL_PROTOCOL_DTLS_10, rtc::KT_DEFAULT))); + EXPECT_EQ_WAIT( + rtc::SSLStreamAdapter::GetDefaultSslCipher(rtc::SSL_PROTOCOL_DTLS_10), + initializing_client()->GetDtlsCipherStats(), + kMaxWaitForStatsMs); + EXPECT_EQ( + rtc::SSLStreamAdapter::GetDefaultSslCipher(rtc::SSL_PROTOCOL_DTLS_10), + init_observer->GetStringHistogramSample(webrtc::kAudioSslCipher)); - EXPECT_EQ_WAIT(kDefaultSrtpCipher, - initializing_client()->GetSrtpCipherStats(), - kMaxWaitForStatsMs); - EXPECT_EQ(1, init_observer->GetEnumCounter( - webrtc::kEnumCounterAudioSrtpCipher, - rtc::GetSrtpCryptoSuiteFromName(kDefaultSrtpCipher))); + EXPECT_EQ_WAIT( + kDefaultSrtpCipher, + initializing_client()->GetSrtpCipherStats(), + kMaxWaitForStatsMs); + EXPECT_EQ( + kDefaultSrtpCipher, + init_observer->GetStringHistogramSample(webrtc::kAudioSrtpCipher)); } // Test that DTLS 1.2 is used if both ends support it. @@ -1372,22 +1371,21 @@ TEST_F(JsepPeerConnectionP2PTestClient, GetDtls12Both) { initializing_client()->pc()->RegisterUMAObserver(init_observer); LocalP2PTest(); - EXPECT_EQ_WAIT(rtc::SSLStreamAdapter::GetSslCipherSuiteName( - rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( - rtc::SSL_PROTOCOL_DTLS_12, rtc::KT_DEFAULT)), - initializing_client()->GetDtlsCipherStats(), - kMaxWaitForStatsMs); - EXPECT_EQ(1, init_observer->GetEnumCounter( - webrtc::kEnumCounterAudioSslCipher, - rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( - rtc::SSL_PROTOCOL_DTLS_12, rtc::KT_DEFAULT))); + EXPECT_EQ_WAIT( + rtc::SSLStreamAdapter::GetDefaultSslCipher(rtc::SSL_PROTOCOL_DTLS_12), + initializing_client()->GetDtlsCipherStats(), + kMaxWaitForStatsMs); + EXPECT_EQ( + rtc::SSLStreamAdapter::GetDefaultSslCipher(rtc::SSL_PROTOCOL_DTLS_12), + init_observer->GetStringHistogramSample(webrtc::kAudioSslCipher)); - EXPECT_EQ_WAIT(kDefaultSrtpCipher, - initializing_client()->GetSrtpCipherStats(), - kMaxWaitForStatsMs); - EXPECT_EQ(1, init_observer->GetEnumCounter( - webrtc::kEnumCounterAudioSrtpCipher, - rtc::GetSrtpCryptoSuiteFromName(kDefaultSrtpCipher))); + EXPECT_EQ_WAIT( + kDefaultSrtpCipher, + initializing_client()->GetSrtpCipherStats(), + kMaxWaitForStatsMs); + EXPECT_EQ( + kDefaultSrtpCipher, + init_observer->GetStringHistogramSample(webrtc::kAudioSrtpCipher)); } // Test that DTLS 1.0 is used if the initator supports DTLS 1.2 and the @@ -1403,22 +1401,21 @@ TEST_F(JsepPeerConnectionP2PTestClient, GetDtls12Init) { initializing_client()->pc()->RegisterUMAObserver(init_observer); LocalP2PTest(); - EXPECT_EQ_WAIT(rtc::SSLStreamAdapter::GetSslCipherSuiteName( - rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( - rtc::SSL_PROTOCOL_DTLS_10, rtc::KT_DEFAULT)), - initializing_client()->GetDtlsCipherStats(), - kMaxWaitForStatsMs); - EXPECT_EQ(1, init_observer->GetEnumCounter( - webrtc::kEnumCounterAudioSslCipher, - rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( - rtc::SSL_PROTOCOL_DTLS_10, rtc::KT_DEFAULT))); + EXPECT_EQ_WAIT( + rtc::SSLStreamAdapter::GetDefaultSslCipher(rtc::SSL_PROTOCOL_DTLS_10), + initializing_client()->GetDtlsCipherStats(), + kMaxWaitForStatsMs); + EXPECT_EQ( + rtc::SSLStreamAdapter::GetDefaultSslCipher(rtc::SSL_PROTOCOL_DTLS_10), + init_observer->GetStringHistogramSample(webrtc::kAudioSslCipher)); - EXPECT_EQ_WAIT(kDefaultSrtpCipher, - initializing_client()->GetSrtpCipherStats(), - kMaxWaitForStatsMs); - EXPECT_EQ(1, init_observer->GetEnumCounter( - webrtc::kEnumCounterAudioSrtpCipher, - rtc::GetSrtpCryptoSuiteFromName(kDefaultSrtpCipher))); + EXPECT_EQ_WAIT( + kDefaultSrtpCipher, + initializing_client()->GetSrtpCipherStats(), + kMaxWaitForStatsMs); + EXPECT_EQ( + kDefaultSrtpCipher, + init_observer->GetStringHistogramSample(webrtc::kAudioSrtpCipher)); } // Test that DTLS 1.0 is used if the initator supports DTLS 1.0 and the @@ -1434,22 +1431,21 @@ TEST_F(JsepPeerConnectionP2PTestClient, GetDtls12Recv) { initializing_client()->pc()->RegisterUMAObserver(init_observer); LocalP2PTest(); - EXPECT_EQ_WAIT(rtc::SSLStreamAdapter::GetSslCipherSuiteName( - rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( - rtc::SSL_PROTOCOL_DTLS_10, rtc::KT_DEFAULT)), - initializing_client()->GetDtlsCipherStats(), - kMaxWaitForStatsMs); - EXPECT_EQ(1, init_observer->GetEnumCounter( - webrtc::kEnumCounterAudioSslCipher, - rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( - rtc::SSL_PROTOCOL_DTLS_10, rtc::KT_DEFAULT))); + EXPECT_EQ_WAIT( + rtc::SSLStreamAdapter::GetDefaultSslCipher(rtc::SSL_PROTOCOL_DTLS_10), + initializing_client()->GetDtlsCipherStats(), + kMaxWaitForStatsMs); + EXPECT_EQ( + rtc::SSLStreamAdapter::GetDefaultSslCipher(rtc::SSL_PROTOCOL_DTLS_10), + init_observer->GetStringHistogramSample(webrtc::kAudioSslCipher)); - EXPECT_EQ_WAIT(kDefaultSrtpCipher, - initializing_client()->GetSrtpCipherStats(), - kMaxWaitForStatsMs); - EXPECT_EQ(1, init_observer->GetEnumCounter( - webrtc::kEnumCounterAudioSrtpCipher, - rtc::GetSrtpCryptoSuiteFromName(kDefaultSrtpCipher))); + EXPECT_EQ_WAIT( + kDefaultSrtpCipher, + initializing_client()->GetSrtpCipherStats(), + kMaxWaitForStatsMs); + EXPECT_EQ( + kDefaultSrtpCipher, + init_observer->GetStringHistogramSample(webrtc::kAudioSrtpCipher)); } // This test sets up a call between two parties with audio, video and data. diff --git a/talk/app/webrtc/peerconnectioninterface.h b/talk/app/webrtc/peerconnectioninterface.h index d3d0d9e667..f301bafdff 100644 --- a/talk/app/webrtc/peerconnectioninterface.h +++ b/talk/app/webrtc/peerconnectioninterface.h @@ -137,16 +137,11 @@ class MetricsObserverInterface : public rtc::RefCountInterface { int counter, int counter_max) {} - // This is used to handle sparse counters like SSL cipher suites. - // TODO(guoweis): Remove the implementation once the dependency's interface - // definition is updated. - virtual void IncrementSparseEnumCounter(PeerConnectionEnumCounterType type, - int counter) { - IncrementEnumCounter(type, counter, 0 /* Ignored */); - } - virtual void AddHistogramSample(PeerConnectionMetricsName type, int value) = 0; + // TODO(jbauch): Make method abstract when it is implemented by Chromium. + virtual void AddHistogramSample(PeerConnectionMetricsName type, + const std::string& value) {} protected: virtual ~MetricsObserverInterface() {} diff --git a/talk/app/webrtc/statscollector.cc b/talk/app/webrtc/statscollector.cc index 6e2c950e22..021234709d 100644 --- a/talk/app/webrtc/statscollector.cc +++ b/talk/app/webrtc/statscollector.cc @@ -734,12 +734,10 @@ void StatsCollector::ExtractSessionInfo() { channel_report->AddString(StatsReport::kStatsValueNameSrtpCipher, srtp_cipher); } - uint16_t ssl_cipher = channel_iter.ssl_cipher; - if (ssl_cipher && - rtc::SSLStreamAdapter::GetSslCipherSuiteName(ssl_cipher).length()) { - channel_report->AddString( - StatsReport::kStatsValueNameDtlsCipher, - rtc::SSLStreamAdapter::GetSslCipherSuiteName(ssl_cipher)); + const std::string& ssl_cipher = channel_iter.ssl_cipher; + if (!ssl_cipher.empty()) { + channel_report->AddString(StatsReport::kStatsValueNameDtlsCipher, + ssl_cipher); } int connection_id = 0; diff --git a/talk/app/webrtc/statscollector_unittest.cc b/talk/app/webrtc/statscollector_unittest.cc index 5e658680be..7c055f6566 100644 --- a/talk/app/webrtc/statscollector_unittest.cc +++ b/talk/app/webrtc/statscollector_unittest.cc @@ -59,11 +59,6 @@ using webrtc::PeerConnectionInterface; using webrtc::StatsReport; using webrtc::StatsReports; -namespace { -// This value comes from openssl/tls1.h -const uint16_t TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA = 0xC014; -} // namespace - namespace cricket { class ChannelManager; @@ -652,7 +647,7 @@ class StatsCollectorTest : public testing::Test { cricket::TransportChannelStats channel_stats; channel_stats.component = 1; channel_stats.srtp_cipher = "the-srtp-cipher"; - channel_stats.ssl_cipher = TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA; + channel_stats.ssl_cipher = "the-ssl-cipher"; cricket::TransportStats transport_stats; transport_stats.transport_name = "audio"; @@ -721,9 +716,7 @@ class StatsCollectorTest : public testing::Test { StatsReport::kStatsReportTypeComponent, reports, StatsReport::kStatsValueNameDtlsCipher); - EXPECT_EQ(rtc::SSLStreamAdapter::GetSslCipherSuiteName( - TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA), - dtls_cipher); + EXPECT_EQ("the-ssl-cipher", dtls_cipher); std::string srtp_cipher = ExtractStatsValue( StatsReport::kStatsReportTypeComponent, reports, diff --git a/talk/app/webrtc/umametrics.h b/talk/app/webrtc/umametrics.h index 14fac962f4..8eaed6d2ff 100644 --- a/talk/app/webrtc/umametrics.h +++ b/talk/app/webrtc/umametrics.h @@ -42,13 +42,6 @@ enum PeerConnectionEnumCounterType { // to the TURN server in the case of TURN candidates. kEnumCounterIceCandidatePairTypeUdp, kEnumCounterIceCandidatePairTypeTcp, - - kEnumCounterAudioSrtpCipher, - kEnumCounterAudioSslCipher, - kEnumCounterVideoSrtpCipher, - kEnumCounterVideoSslCipher, - kEnumCounterDataSrtpCipher, - kEnumCounterDataSslCipher, kPeerConnectionEnumCounterMax }; @@ -85,6 +78,12 @@ enum PeerConnectionMetricsName { kTimeToConnect, // In milliseconds. kLocalCandidates_IPv4, // Number of IPv4 local candidates. kLocalCandidates_IPv6, // Number of IPv6 local candidates. + kAudioSrtpCipher, // Name of SRTP cipher used in audio channel. + kAudioSslCipher, // Name of SSL cipher used in audio channel. + kVideoSrtpCipher, // Name of SRTP cipher used in video channel. + kVideoSslCipher, // Name of SSL cipher used in video channel. + kDataSrtpCipher, // Name of SRTP cipher used in data channel. + kDataSslCipher, // Name of SSL cipher used in data channel. kPeerConnectionMetricsName_Max }; diff --git a/talk/app/webrtc/webrtcsession.cc b/talk/app/webrtc/webrtcsession.cc index 7241abf3cd..7f71961682 100644 --- a/talk/app/webrtc/webrtcsession.cc +++ b/talk/app/webrtc/webrtcsession.cc @@ -2149,33 +2149,32 @@ void WebRtcSession::ReportNegotiatedCiphers( } const std::string& srtp_cipher = stats.channel_stats[0].srtp_cipher; - uint16_t ssl_cipher = stats.channel_stats[0].ssl_cipher; - if (srtp_cipher.empty() && !ssl_cipher) { + const std::string& ssl_cipher = stats.channel_stats[0].ssl_cipher; + if (srtp_cipher.empty() && ssl_cipher.empty()) { return; } - PeerConnectionEnumCounterType srtp_counter_type; - PeerConnectionEnumCounterType ssl_counter_type; + PeerConnectionMetricsName srtp_name; + PeerConnectionMetricsName ssl_name; if (stats.transport_name == cricket::CN_AUDIO) { - srtp_counter_type = kEnumCounterAudioSrtpCipher; - ssl_counter_type = kEnumCounterAudioSslCipher; + srtp_name = kAudioSrtpCipher; + ssl_name = kAudioSslCipher; } else if (stats.transport_name == cricket::CN_VIDEO) { - srtp_counter_type = kEnumCounterVideoSrtpCipher; - ssl_counter_type = kEnumCounterVideoSslCipher; + srtp_name = kVideoSrtpCipher; + ssl_name = kVideoSslCipher; } else if (stats.transport_name == cricket::CN_DATA) { - srtp_counter_type = kEnumCounterDataSrtpCipher; - ssl_counter_type = kEnumCounterDataSslCipher; + srtp_name = kDataSrtpCipher; + ssl_name = kDataSslCipher; } else { RTC_NOTREACHED(); return; } if (!srtp_cipher.empty()) { - metrics_observer_->IncrementSparseEnumCounter( - srtp_counter_type, rtc::GetSrtpCryptoSuiteFromName(srtp_cipher)); + metrics_observer_->AddHistogramSample(srtp_name, srtp_cipher); } - if (ssl_cipher) { - metrics_observer_->IncrementSparseEnumCounter(ssl_counter_type, ssl_cipher); + if (!ssl_cipher.empty()) { + metrics_observer_->AddHistogramSample(ssl_name, ssl_cipher); } } diff --git a/talk/session/media/channel.cc b/talk/session/media/channel.cc index 56209800fa..5a6b7e198f 100644 --- a/talk/session/media/channel.cc +++ b/talk/session/media/channel.cc @@ -816,9 +816,9 @@ bool BaseChannel::SetDtlsSrtpCiphers(TransportChannel *tc, bool rtcp) { // We always use the default SRTP ciphers for RTCP, but we may use different // ciphers for RTP depending on the media type. if (!rtcp) { - GetSrtpCryptoSuiteNames(&ciphers); + GetSrtpCiphers(&ciphers); } else { - GetDefaultSrtpCryptoSuiteNames(&ciphers); + GetSupportedDefaultCryptoSuites(&ciphers); } return tc->SetSrtpCiphers(ciphers); } @@ -841,7 +841,7 @@ bool BaseChannel::SetupDtlsSrtp(bool rtcp_channel) { std::string selected_cipher; - if (!channel->GetSrtpCryptoSuite(&selected_cipher)) { + if (!channel->GetSrtpCipher(&selected_cipher)) { LOG(LS_ERROR) << "No DTLS-SRTP selected cipher"; return false; } @@ -1627,8 +1627,7 @@ void VoiceChannel::OnSrtpError(uint32 ssrc, SrtpFilter::Mode mode, } } -void VoiceChannel::GetSrtpCryptoSuiteNames( - std::vector* ciphers) const { +void VoiceChannel::GetSrtpCiphers(std::vector* ciphers) const { GetSupportedAudioCryptoSuites(ciphers); } @@ -2061,8 +2060,7 @@ void VideoChannel::OnSrtpError(uint32 ssrc, SrtpFilter::Mode mode, } } -void VideoChannel::GetSrtpCryptoSuiteNames( - std::vector* ciphers) const { +void VideoChannel::GetSrtpCiphers(std::vector* ciphers) const { GetSupportedVideoCryptoSuites(ciphers); } @@ -2397,8 +2395,7 @@ void DataChannel::OnSrtpError(uint32 ssrc, SrtpFilter::Mode mode, } } -void DataChannel::GetSrtpCryptoSuiteNames( - std::vector* ciphers) const { +void DataChannel::GetSrtpCiphers(std::vector* ciphers) const { GetSupportedDataCryptoSuites(ciphers); } diff --git a/talk/session/media/channel.h b/talk/session/media/channel.h index dec5c985ae..0a7ce9d1a9 100644 --- a/talk/session/media/channel.h +++ b/talk/session/media/channel.h @@ -284,8 +284,7 @@ class BaseChannel // Handled in derived classes // Get the SRTP ciphers to use for RTP media - virtual void GetSrtpCryptoSuiteNames( - std::vector* ciphers) const = 0; + virtual void GetSrtpCiphers(std::vector* ciphers) const = 0; virtual void OnConnectionMonitorUpdate(ConnectionMonitor* monitor, const std::vector& infos) = 0; @@ -414,7 +413,7 @@ class VoiceChannel : public BaseChannel { bool GetStats_w(VoiceMediaInfo* stats); virtual void OnMessage(rtc::Message* pmsg); - virtual void GetSrtpCryptoSuiteNames(std::vector* ciphers) const; + virtual void GetSrtpCiphers(std::vector* ciphers) const; virtual void OnConnectionMonitorUpdate( ConnectionMonitor* monitor, const std::vector& infos); virtual void OnMediaMonitorUpdate( @@ -511,7 +510,7 @@ class VideoChannel : public BaseChannel { bool GetStats_w(VideoMediaInfo* stats); virtual void OnMessage(rtc::Message* pmsg); - virtual void GetSrtpCryptoSuiteNames(std::vector* ciphers) const; + virtual void GetSrtpCiphers(std::vector* ciphers) const; virtual void OnConnectionMonitorUpdate( ConnectionMonitor* monitor, const std::vector& infos); virtual void OnMediaMonitorUpdate( @@ -636,7 +635,7 @@ class DataChannel : public BaseChannel { virtual bool WantsPacket(bool rtcp, rtc::Buffer* packet); virtual void OnMessage(rtc::Message* pmsg); - virtual void GetSrtpCryptoSuiteNames(std::vector* ciphers) const; + virtual void GetSrtpCiphers(std::vector* ciphers) const; virtual void OnConnectionMonitorUpdate( ConnectionMonitor* monitor, const std::vector& infos); virtual void OnMediaMonitorUpdate( diff --git a/talk/session/media/channel_unittest.cc b/talk/session/media/channel_unittest.cc index 8af5daebf5..9fef65ffd1 100644 --- a/talk/session/media/channel_unittest.cc +++ b/talk/session/media/channel_unittest.cc @@ -1817,8 +1817,8 @@ void ChannelTest::CreateContent( audio->set_rtcp_mux((flags & RTCP_MUX) != 0); if (flags & SECURE) { audio->AddCrypto(cricket::CryptoParams( - 1, rtc::CS_AES_CM_128_HMAC_SHA1_32, - "inline:" + rtc::CreateRandomString(40), std::string())); + 1, cricket::CS_AES_CM_128_HMAC_SHA1_32, + "inline:" + rtc::CreateRandomString(40), "")); } } @@ -1887,8 +1887,8 @@ void ChannelTest::CreateContent( video->set_rtcp_mux((flags & RTCP_MUX) != 0); if (flags & SECURE) { video->AddCrypto(cricket::CryptoParams( - 1, rtc::CS_AES_CM_128_HMAC_SHA1_80, - "inline:" + rtc::CreateRandomString(40), std::string())); + 1, cricket::CS_AES_CM_128_HMAC_SHA1_80, + "inline:" + rtc::CreateRandomString(40), "")); } } @@ -2580,7 +2580,7 @@ TEST_F(VideoChannelTest, TestApplyViewRequest) { // stream1: 0x0x0; stream2: 640x400x30 request.static_video_views.clear(); request.static_video_views.push_back(cricket::StaticVideoView( - cricket::StreamSelector(std::string(), stream2.id), 640, 400, 30)); + cricket::StreamSelector("", stream2.id), 640, 400, 30)); EXPECT_TRUE(channel1_->ApplyViewRequest(request)); EXPECT_TRUE(media_channel1_->GetSendStreamFormat(kSsrc1, &send_format)); EXPECT_EQ(0, send_format.width); @@ -2641,8 +2641,8 @@ void ChannelTest::CreateContent( data->set_rtcp_mux((flags & RTCP_MUX) != 0); if (flags & SECURE) { data->AddCrypto(cricket::CryptoParams( - 1, rtc::CS_AES_CM_128_HMAC_SHA1_32, - "inline:" + rtc::CreateRandomString(40), std::string())); + 1, cricket::CS_AES_CM_128_HMAC_SHA1_32, + "inline:" + rtc::CreateRandomString(40), "")); } } diff --git a/talk/session/media/mediasession.cc b/talk/session/media/mediasession.cc index 99e1ea3936..66d0caf9d7 100644 --- a/talk/session/media/mediasession.cc +++ b/talk/session/media/mediasession.cc @@ -155,24 +155,25 @@ bool FindMatchingCrypto(const CryptoParamsVec& cryptos, void GetSupportedAudioCryptoSuites( std::vector* crypto_suites) { #ifdef HAVE_SRTP - crypto_suites->push_back(rtc::CS_AES_CM_128_HMAC_SHA1_32); - crypto_suites->push_back(rtc::CS_AES_CM_128_HMAC_SHA1_80); + crypto_suites->push_back(CS_AES_CM_128_HMAC_SHA1_32); + crypto_suites->push_back(CS_AES_CM_128_HMAC_SHA1_80); #endif } void GetSupportedVideoCryptoSuites( std::vector* crypto_suites) { - GetDefaultSrtpCryptoSuiteNames(crypto_suites); + GetSupportedDefaultCryptoSuites(crypto_suites); } void GetSupportedDataCryptoSuites( std::vector* crypto_suites) { - GetDefaultSrtpCryptoSuiteNames(crypto_suites); + GetSupportedDefaultCryptoSuites(crypto_suites); } -void GetDefaultSrtpCryptoSuiteNames(std::vector* crypto_suites) { +void GetSupportedDefaultCryptoSuites( + std::vector* crypto_suites) { #ifdef HAVE_SRTP - crypto_suites->push_back(rtc::CS_AES_CM_128_HMAC_SHA1_80); + crypto_suites->push_back(CS_AES_CM_128_HMAC_SHA1_80); #endif } @@ -187,9 +188,8 @@ static bool SelectCrypto(const MediaContentDescription* offer, for (CryptoParamsVec::const_iterator i = cryptos.begin(); i != cryptos.end(); ++i) { - if (rtc::CS_AES_CM_128_HMAC_SHA1_80 == i->cipher_suite || - (rtc::CS_AES_CM_128_HMAC_SHA1_32 == i->cipher_suite && audio && - !bundle)) { + if (CS_AES_CM_128_HMAC_SHA1_80 == i->cipher_suite || + (CS_AES_CM_128_HMAC_SHA1_32 == i->cipher_suite && audio && !bundle)) { return CreateCryptoParams(i->tag, i->cipher_suite, crypto); } } diff --git a/talk/session/media/mediasession.h b/talk/session/media/mediasession.h index 98ae60a372..d329dcdf27 100644 --- a/talk/session/media/mediasession.h +++ b/talk/session/media/mediasession.h @@ -550,7 +550,7 @@ const DataContentDescription* GetFirstDataContentDescription( void GetSupportedAudioCryptoSuites(std::vector* crypto_suites); void GetSupportedVideoCryptoSuites(std::vector* crypto_suites); void GetSupportedDataCryptoSuites(std::vector* crypto_suites); -void GetDefaultSrtpCryptoSuiteNames(std::vector* crypto_suites); +void GetSupportedDefaultCryptoSuites(std::vector* crypto_suites); } // namespace cricket #endif // TALK_SESSION_MEDIA_MEDIASESSION_H_ diff --git a/talk/session/media/mediasession_unittest.cc b/talk/session/media/mediasession_unittest.cc index 3d69465dec..7787ab5621 100644 --- a/talk/session/media/mediasession_unittest.cc +++ b/talk/session/media/mediasession_unittest.cc @@ -84,8 +84,8 @@ using cricket::RtpHeaderExtension; using cricket::SEC_DISABLED; using cricket::SEC_ENABLED; using cricket::SEC_REQUIRED; -using rtc::CS_AES_CM_128_HMAC_SHA1_32; -using rtc::CS_AES_CM_128_HMAC_SHA1_80; +using cricket::CS_AES_CM_128_HMAC_SHA1_32; +using cricket::CS_AES_CM_128_HMAC_SHA1_80; static const AudioCodec kAudioCodecs1[] = { AudioCodec(103, "ISAC", 16000, -1, 1, 6), diff --git a/talk/session/media/srtpfilter.cc b/talk/session/media/srtpfilter.cc index f7ab0bed18..33e42c09d4 100644 --- a/talk/session/media/srtpfilter.cc +++ b/talk/session/media/srtpfilter.cc @@ -73,6 +73,8 @@ extern "C" debug_module_t mod_aes_hmac; namespace cricket { +const char CS_AES_CM_128_HMAC_SHA1_80[] = "AES_CM_128_HMAC_SHA1_80"; +const char CS_AES_CM_128_HMAC_SHA1_32[] = "AES_CM_128_HMAC_SHA1_32"; const int SRTP_MASTER_KEY_BASE64_LEN = SRTP_MASTER_KEY_LEN * 4 / 3; const int SRTP_MASTER_KEY_KEY_LEN = 16; const int SRTP_MASTER_KEY_SALT_LEN = 14; @@ -661,10 +663,10 @@ bool SrtpSession::SetKey(int type, const std::string& cs, srtp_policy_t policy; memset(&policy, 0, sizeof(policy)); - if (cs == rtc::CS_AES_CM_128_HMAC_SHA1_80) { + if (cs == CS_AES_CM_128_HMAC_SHA1_80) { crypto_policy_set_aes_cm_128_hmac_sha1_80(&policy.rtp); crypto_policy_set_aes_cm_128_hmac_sha1_80(&policy.rtcp); - } else if (cs == rtc::CS_AES_CM_128_HMAC_SHA1_32) { + } else if (cs == CS_AES_CM_128_HMAC_SHA1_32) { crypto_policy_set_aes_cm_128_hmac_sha1_32(&policy.rtp); // rtp is 32, crypto_policy_set_aes_cm_128_hmac_sha1_80(&policy.rtcp); // rtcp still 80 } else { diff --git a/talk/session/media/srtpfilter.h b/talk/session/media/srtpfilter.h index 1b52614151..a536327fb0 100644 --- a/talk/session/media/srtpfilter.h +++ b/talk/session/media/srtpfilter.h @@ -39,7 +39,6 @@ #include "webrtc/base/criticalsection.h" #include "webrtc/base/scoped_ptr.h" #include "webrtc/base/sigslotrepeater.h" -#include "webrtc/base/sslstreamadapter.h" // Forward declaration to avoid pulling in libsrtp headers here struct srtp_event_data_t; @@ -48,6 +47,13 @@ struct srtp_policy_t; namespace cricket { +// Cipher suite to use for SRTP. Typically a 80-bit HMAC will be used, except +// in applications (voice) where the additional bandwidth may be significant. +// A 80-bit HMAC is always used for SRTCP. +// 128-bit AES with 80-bit SHA-1 HMAC. +extern const char CS_AES_CM_128_HMAC_SHA1_80[]; +// 128-bit AES with 32-bit SHA-1 HMAC. +extern const char CS_AES_CM_128_HMAC_SHA1_32[]; // Key is 128 bits and salt is 112 bits == 30 bytes. B64 bloat => 40 bytes. extern const int SRTP_MASTER_KEY_BASE64_LEN; diff --git a/talk/session/media/srtpfilter_unittest.cc b/talk/session/media/srtpfilter_unittest.cc index 55408e6bd6..4cbcb4afd4 100644 --- a/talk/session/media/srtpfilter_unittest.cc +++ b/talk/session/media/srtpfilter_unittest.cc @@ -40,8 +40,8 @@ extern "C" { #endif } -using rtc::CS_AES_CM_128_HMAC_SHA1_80; -using rtc::CS_AES_CM_128_HMAC_SHA1_32; +using cricket::CS_AES_CM_128_HMAC_SHA1_80; +using cricket::CS_AES_CM_128_HMAC_SHA1_32; using cricket::CryptoParams; using cricket::CS_LOCAL; using cricket::CS_REMOTE; diff --git a/webrtc/base/opensslstreamadapter.cc b/webrtc/base/opensslstreamadapter.cc index 2b7eea869a..ed2505e8b7 100644 --- a/webrtc/base/opensslstreamadapter.cc +++ b/webrtc/base/opensslstreamadapter.cc @@ -51,13 +51,13 @@ struct SrtpCipherMapEntry { // This isn't elegant, but it's better than an external reference static SrtpCipherMapEntry SrtpCipherMap[] = { - {CS_AES_CM_128_HMAC_SHA1_80, "SRTP_AES128_CM_SHA1_80"}, - {CS_AES_CM_128_HMAC_SHA1_32, "SRTP_AES128_CM_SHA1_32"}, - {NULL, NULL}}; + {"AES_CM_128_HMAC_SHA1_80", "SRTP_AES128_CM_SHA1_80"}, + {"AES_CM_128_HMAC_SHA1_32", "SRTP_AES128_CM_SHA1_32"}, + {NULL, NULL} +}; #endif #ifndef OPENSSL_IS_BORINGSSL - // Cipher name table. Maps internal OpenSSL cipher ids to the RFC name. struct SslCipherMapEntry { uint32_t openssl_id; @@ -139,42 +139,32 @@ static const SslCipherMapEntry kSslCipherMap[] = { }; #endif // #ifndef OPENSSL_IS_BORINGSSL -#if defined(_MSC_VER) -#pragma warning(push) -#pragma warning(disable : 4309) -#pragma warning(disable : 4310) -#endif // defined(_MSC_VER) - // Default cipher used between OpenSSL/BoringSSL stream adapters. // This needs to be updated when the default of the SSL library changes. -// static_cast causes build warnings on windows platform. -static uint16_t kDefaultSslCipher10 = - static_cast(TLS1_CK_ECDHE_RSA_WITH_AES_256_CBC_SHA); -static uint16_t kDefaultSslEcCipher10 = - static_cast(TLS1_CK_ECDHE_ECDSA_WITH_AES_256_CBC_SHA); +static const char kDefaultSslCipher10[] = + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA"; +static const char kDefaultSslEcCipher10[] = + "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA"; + #ifdef OPENSSL_IS_BORINGSSL -static uint16_t kDefaultSslCipher12 = - static_cast(TLS1_CK_ECDHE_RSA_WITH_AES_128_GCM_SHA256); -static uint16_t kDefaultSslEcCipher12 = - static_cast(TLS1_CK_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256); +static const char kDefaultSslCipher12[] = + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"; +static const char kDefaultSslEcCipher12[] = + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"; // Fallback cipher for DTLS 1.2 if hardware-accelerated AES-GCM is unavailable. -static uint16_t kDefaultSslCipher12NoAesGcm = - static_cast(TLS1_CK_ECDHE_RSA_CHACHA20_POLY1305); -static uint16_t kDefaultSslEcCipher12NoAesGcm = - static_cast(TLS1_CK_ECDHE_ECDSA_CHACHA20_POLY1305); +static const char kDefaultSslCipher12NoAesGcm[] = + "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256"; +static const char kDefaultSslEcCipher12NoAesGcm[] = + "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256"; #else // !OPENSSL_IS_BORINGSSL // OpenSSL sorts differently than BoringSSL, so the default cipher doesn't // change between TLS 1.0 and TLS 1.2 with the current setup. -static uint16_t kDefaultSslCipher12 = - static_cast(TLS1_CK_ECDHE_RSA_WITH_AES_256_CBC_SHA); -static uint16_t kDefaultSslEcCipher12 = - static_cast(TLS1_CK_ECDHE_ECDSA_WITH_AES_256_CBC_SHA); +static const char kDefaultSslCipher12[] = + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA"; +static const char kDefaultSslEcCipher12[] = + "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA"; #endif -#if defined(_MSC_VER) -#pragma warning(pop) -#endif // defined(_MSC_VER) - ////////////////////////////////////////////////////////////////////// // StreamBIO ////////////////////////////////////////////////////////////////////// @@ -348,17 +338,9 @@ bool OpenSSLStreamAdapter::SetPeerCertificateDigest(const std::string return true; } -std::string OpenSSLStreamAdapter::GetSslCipherSuiteName(uint16_t cipher) { -#ifdef OPENSSL_IS_BORINGSSL - const SSL_CIPHER* ssl_cipher = SSL_get_cipher_by_value(cipher); - if (!ssl_cipher) { - return std::string(); - } - char* cipher_name = SSL_CIPHER_get_rfc_name(ssl_cipher); - std::string rfc_name = std::string(cipher_name); - OPENSSL_free(cipher_name); - return rfc_name; -#else +#ifndef OPENSSL_IS_BORINGSSL +const char* OpenSSLStreamAdapter::GetRfcSslCipherName( + const SSL_CIPHER* cipher) { ASSERT(cipher != NULL); for (const SslCipherMapEntry* entry = kSslCipherMap; entry->rfc_name; ++entry) { @@ -366,11 +348,11 @@ std::string OpenSSLStreamAdapter::GetSslCipherSuiteName(uint16_t cipher) { return entry->rfc_name; } } - return std::string(); -#endif + return NULL; } +#endif -bool OpenSSLStreamAdapter::GetSslCipherSuite(uint16_t* cipher) { +bool OpenSSLStreamAdapter::GetSslCipher(std::string* cipher) { if (state_ != SSL_CONNECTED) return false; @@ -379,7 +361,19 @@ bool OpenSSLStreamAdapter::GetSslCipherSuite(uint16_t* cipher) { return false; } - *cipher = static_cast(SSL_CIPHER_get_id(current_cipher)); +#ifdef OPENSSL_IS_BORINGSSL + char* cipher_name = SSL_CIPHER_get_rfc_name(current_cipher); +#else + const char* cipher_name = GetRfcSslCipherName(current_cipher); +#endif + if (cipher_name == NULL) { + return false; + } + + *cipher = cipher_name; +#ifdef OPENSSL_IS_BORINGSSL + OPENSSL_free(cipher_name); +#endif return true; } @@ -1131,7 +1125,7 @@ bool OpenSSLStreamAdapter::HaveExporter() { #endif } -uint16_t OpenSSLStreamAdapter::GetDefaultSslCipherForTest( +std::string OpenSSLStreamAdapter::GetDefaultSslCipher( SSLProtocolVersion version, KeyType key_type) { if (key_type == KT_RSA) { @@ -1169,8 +1163,7 @@ uint16_t OpenSSLStreamAdapter::GetDefaultSslCipherForTest( #endif } } else { - RTC_NOTREACHED(); - return kDefaultSslEcCipher12; + return std::string(); } } diff --git a/webrtc/base/opensslstreamadapter.h b/webrtc/base/opensslstreamadapter.h index dfcb43902e..cf332b155d 100644 --- a/webrtc/base/opensslstreamadapter.h +++ b/webrtc/base/opensslstreamadapter.h @@ -87,10 +87,12 @@ class OpenSSLStreamAdapter : public SSLStreamAdapter { void Close() override; StreamState GetState() const override; - // TODO(guoweis): Move this away from a static class method. - static std::string GetSslCipherSuiteName(uint16_t cipher); +#ifndef OPENSSL_IS_BORINGSSL + // Return the RFC (5246, 3268, etc.) cipher name for an OpenSSL cipher. + static const char* GetRfcSslCipherName(const SSL_CIPHER* cipher); +#endif - bool GetSslCipherSuite(uint16_t* cipher) override; + bool GetSslCipher(std::string* cipher) override; // Key Extractor interface bool ExportKeyingMaterial(const std::string& label, @@ -108,10 +110,8 @@ class OpenSSLStreamAdapter : public SSLStreamAdapter { static bool HaveDtls(); static bool HaveDtlsSrtp(); static bool HaveExporter(); - - // TODO(guoweis): Move this away from a static class method. - static uint16_t GetDefaultSslCipherForTest(SSLProtocolVersion version, - KeyType key_type); + static std::string GetDefaultSslCipher(SSLProtocolVersion version, + KeyType key_type); protected: void OnEvent(StreamInterface* stream, int events, int err) override; diff --git a/webrtc/base/sslstreamadapter.cc b/webrtc/base/sslstreamadapter.cc index 8930f2185d..42dea9c036 100644 --- a/webrtc/base/sslstreamadapter.cc +++ b/webrtc/base/sslstreamadapter.cc @@ -29,19 +29,6 @@ namespace rtc { -// TODO(guoweis): Move this to SDP layer and use int form internally. -// webrtc:5043. -const char CS_AES_CM_128_HMAC_SHA1_80[] = "AES_CM_128_HMAC_SHA1_80"; -const char CS_AES_CM_128_HMAC_SHA1_32[] = "AES_CM_128_HMAC_SHA1_32"; - -uint16_t GetSrtpCryptoSuiteFromName(const std::string& cipher) { - if (cipher == CS_AES_CM_128_HMAC_SHA1_32) - return SRTP_AES128_CM_SHA1_32; - if (cipher == CS_AES_CM_128_HMAC_SHA1_80) - return SRTP_AES128_CM_SHA1_80; - return 0; -} - SSLStreamAdapter* SSLStreamAdapter::Create(StreamInterface* stream) { #if SSL_USE_SCHANNEL return NULL; @@ -52,7 +39,7 @@ SSLStreamAdapter* SSLStreamAdapter::Create(StreamInterface* stream) { #endif } -bool SSLStreamAdapter::GetSslCipherSuite(uint16_t* cipher) { +bool SSLStreamAdapter::GetSslCipher(std::string* cipher) { return false; } @@ -79,10 +66,9 @@ bool SSLStreamAdapter::GetDtlsSrtpCipher(std::string* cipher) { bool SSLStreamAdapter::HaveDtls() { return false; } bool SSLStreamAdapter::HaveDtlsSrtp() { return false; } bool SSLStreamAdapter::HaveExporter() { return false; } -uint16_t SSLStreamAdapter::GetDefaultSslCipherForTest( - SSLProtocolVersion version, - KeyType key_type) { - return 0; +std::string SSLStreamAdapter::GetDefaultSslCipher(SSLProtocolVersion version, + KeyType key_type) { + return std::string(); } #elif SSL_USE_OPENSSL bool SSLStreamAdapter::HaveDtls() { @@ -94,14 +80,9 @@ bool SSLStreamAdapter::HaveDtlsSrtp() { bool SSLStreamAdapter::HaveExporter() { return OpenSSLStreamAdapter::HaveExporter(); } -uint16_t SSLStreamAdapter::GetDefaultSslCipherForTest( - SSLProtocolVersion version, - KeyType key_type) { - return OpenSSLStreamAdapter::GetDefaultSslCipherForTest(version, key_type); -} - -std::string SSLStreamAdapter::GetSslCipherSuiteName(uint16_t cipher) { - return OpenSSLStreamAdapter::GetSslCipherSuiteName(cipher); +std::string SSLStreamAdapter::GetDefaultSslCipher(SSLProtocolVersion version, + KeyType key_type) { + return OpenSSLStreamAdapter::GetDefaultSslCipher(version, key_type); } #endif // !SSL_USE_SCHANNEL && !SSL_USE_OPENSSL diff --git a/webrtc/base/sslstreamadapter.h b/webrtc/base/sslstreamadapter.h index 867f309a03..4fb238a290 100644 --- a/webrtc/base/sslstreamadapter.h +++ b/webrtc/base/sslstreamadapter.h @@ -19,23 +19,6 @@ namespace rtc { -// Constants for SRTP profiles. -const uint16_t SRTP_AES128_CM_SHA1_80 = 0x0001; -const uint16_t SRTP_AES128_CM_SHA1_32 = 0x0002; - -// Cipher suite to use for SRTP. Typically a 80-bit HMAC will be used, except -// in applications (voice) where the additional bandwidth may be significant. -// A 80-bit HMAC is always used for SRTCP. -// 128-bit AES with 80-bit SHA-1 HMAC. -extern const char CS_AES_CM_128_HMAC_SHA1_80[]; -// 128-bit AES with 32-bit SHA-1 HMAC. -extern const char CS_AES_CM_128_HMAC_SHA1_32[]; - -// Returns the DTLS-SRTP protection profile ID, as defined in -// https://tools.ietf.org/html/rfc5764#section-4.1.2, for the given SRTP -// Crypto-suite, as defined in https://tools.ietf.org/html/rfc4568#section-6.2 -uint16_t GetSrtpCryptoSuiteFromName(const std::string& cipher_rfc_name); - // SSLStreamAdapter : A StreamInterfaceAdapter that does SSL/TLS. // After SSL has been started, the stream will only open on successful // SSL verification of certificates, and the communication is @@ -150,9 +133,9 @@ class SSLStreamAdapter : public StreamAdapterInterface { // chain. The returned certificate is owned by the caller. virtual bool GetPeerCertificate(SSLCertificate** cert) 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"). - virtual bool GetSslCipherSuite(uint16_t* cipher); + // Retrieves the name of the cipher suite used for the connection + // (e.g. "TLS_RSA_WITH_AES_128_CBC_SHA"). + virtual bool GetSslCipher(std::string* cipher); // Key Exporter interface from RFC 5705 // Arguments are: @@ -184,14 +167,9 @@ class SSLStreamAdapter : public StreamAdapterInterface { // Returns the default Ssl cipher used between streams of this class // for the given protocol version. This is used by the unit tests. - // TODO(guoweis): Move this away from a static class method. - static uint16_t GetDefaultSslCipherForTest(SSLProtocolVersion version, - KeyType key_type); - - // TODO(guoweis): Move this away from a static class method. Currently this is - // introduced such that any caller could depend on sslstreamadapter.h without - // depending on specific SSL implementation. - static std::string GetSslCipherSuiteName(uint16_t cipher); + // TODO(torbjorng@webrtc.org): Fix callers to avoid default parameter. + static std::string GetDefaultSslCipher(SSLProtocolVersion version, + KeyType key_type = KT_DEFAULT); private: // If true, the server certificate need not match the configured diff --git a/webrtc/base/sslstreamadapter_unittest.cc b/webrtc/base/sslstreamadapter_unittest.cc index c8fe9a01e7..68e1b64dbe 100644 --- a/webrtc/base/sslstreamadapter_unittest.cc +++ b/webrtc/base/sslstreamadapter_unittest.cc @@ -410,11 +410,11 @@ class SSLStreamAdapterTestBase : public testing::Test, return server_ssl_->GetPeerCertificate(cert); } - bool GetSslCipherSuite(bool client, uint16_t* retval) { + bool GetSslCipher(bool client, std::string *retval) { if (client) - return client_ssl_->GetSslCipherSuite(retval); + return client_ssl_->GetSslCipher(retval); else - return server_ssl_->GetSslCipherSuite(retval); + return server_ssl_->GetSslCipher(retval); } bool ExportKeyingMaterial(const char *label, @@ -967,70 +967,70 @@ TEST_P(SSLStreamAdapterTestDTLSFromPEMStrings, TestDTLSGetPeerCertificate) { // Test getting the used DTLS ciphers. // DTLS 1.2 enabled for neither client nor server -> DTLS 1.0 will be used. -TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuite) { +TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipher) { MAYBE_SKIP_TEST(HaveDtls); SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_10); TestHandshake(); - uint16_t client_cipher; - ASSERT_TRUE(GetSslCipherSuite(true, &client_cipher)); - uint16_t server_cipher; - ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher)); + std::string client_cipher; + ASSERT_TRUE(GetSslCipher(true, &client_cipher)); + std::string server_cipher; + ASSERT_TRUE(GetSslCipher(false, &server_cipher)); ASSERT_EQ(client_cipher, server_cipher); - ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( + ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipher( rtc::SSL_PROTOCOL_DTLS_10, ::testing::get<1>(GetParam())), server_cipher); } // Test getting the used DTLS 1.2 ciphers. // DTLS 1.2 enabled for client and server -> DTLS 1.2 will be used. -TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Both) { +TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherDtls12Both) { MAYBE_SKIP_TEST(HaveDtls); SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_12, rtc::SSL_PROTOCOL_DTLS_12); TestHandshake(); - uint16_t client_cipher; - ASSERT_TRUE(GetSslCipherSuite(true, &client_cipher)); - uint16_t server_cipher; - ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher)); + std::string client_cipher; + ASSERT_TRUE(GetSslCipher(true, &client_cipher)); + std::string server_cipher; + ASSERT_TRUE(GetSslCipher(false, &server_cipher)); ASSERT_EQ(client_cipher, server_cipher); - ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( + ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipher( rtc::SSL_PROTOCOL_DTLS_12, ::testing::get<1>(GetParam())), server_cipher); } // DTLS 1.2 enabled for client only -> DTLS 1.0 will be used. -TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Client) { +TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherDtls12Client) { MAYBE_SKIP_TEST(HaveDtls); SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_10, rtc::SSL_PROTOCOL_DTLS_12); TestHandshake(); - uint16_t client_cipher; - ASSERT_TRUE(GetSslCipherSuite(true, &client_cipher)); - uint16_t server_cipher; - ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher)); + std::string client_cipher; + ASSERT_TRUE(GetSslCipher(true, &client_cipher)); + std::string server_cipher; + ASSERT_TRUE(GetSslCipher(false, &server_cipher)); ASSERT_EQ(client_cipher, server_cipher); - ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( + ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipher( rtc::SSL_PROTOCOL_DTLS_10, ::testing::get<1>(GetParam())), server_cipher); } // DTLS 1.2 enabled for server only -> DTLS 1.0 will be used. -TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherSuiteDtls12Server) { +TEST_P(SSLStreamAdapterTestDTLS, TestGetSslCipherDtls12Server) { MAYBE_SKIP_TEST(HaveDtls); SetupProtocolVersions(rtc::SSL_PROTOCOL_DTLS_12, rtc::SSL_PROTOCOL_DTLS_10); TestHandshake(); - uint16_t client_cipher; - ASSERT_TRUE(GetSslCipherSuite(true, &client_cipher)); - uint16_t server_cipher; - ASSERT_TRUE(GetSslCipherSuite(false, &server_cipher)); + std::string client_cipher; + ASSERT_TRUE(GetSslCipher(true, &client_cipher)); + std::string server_cipher; + ASSERT_TRUE(GetSslCipher(false, &server_cipher)); ASSERT_EQ(client_cipher, server_cipher); - ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( + ASSERT_EQ(rtc::SSLStreamAdapter::GetDefaultSslCipher( rtc::SSL_PROTOCOL_DTLS_10, ::testing::get<1>(GetParam())), server_cipher); } diff --git a/webrtc/p2p/base/dtlstransportchannel.cc b/webrtc/p2p/base/dtlstransportchannel.cc index ff42a4df13..b434d0860e 100644 --- a/webrtc/p2p/base/dtlstransportchannel.cc +++ b/webrtc/p2p/base/dtlstransportchannel.cc @@ -186,12 +186,12 @@ bool DtlsTransportChannelWrapper::GetSslRole(rtc::SSLRole* role) const { return true; } -bool DtlsTransportChannelWrapper::GetSslCipherSuite(uint16_t* cipher) { +bool DtlsTransportChannelWrapper::GetSslCipher(std::string* cipher) { if (dtls_state_ != STATE_OPEN) { return false; } - return dtls_->GetSslCipherSuite(cipher); + return dtls_->GetSslCipher(cipher); } bool DtlsTransportChannelWrapper::SetRemoteFingerprint( @@ -330,7 +330,7 @@ bool DtlsTransportChannelWrapper::SetSrtpCiphers( return true; } -bool DtlsTransportChannelWrapper::GetSrtpCryptoSuite(std::string* cipher) { +bool DtlsTransportChannelWrapper::GetSrtpCipher(std::string* cipher) { if (dtls_state_ != STATE_OPEN) { return false; } diff --git a/webrtc/p2p/base/dtlstransportchannel.h b/webrtc/p2p/base/dtlstransportchannel.h index f1a5231f9c..e52ca10d1d 100644 --- a/webrtc/p2p/base/dtlstransportchannel.h +++ b/webrtc/p2p/base/dtlstransportchannel.h @@ -135,13 +135,13 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl { bool SetSrtpCiphers(const std::vector& ciphers) override; // Find out which DTLS-SRTP cipher was negotiated - bool GetSrtpCryptoSuite(std::string* cipher) override; + bool GetSrtpCipher(std::string* cipher) override; bool GetSslRole(rtc::SSLRole* role) const override; bool SetSslRole(rtc::SSLRole role) override; // Find out which DTLS cipher was negotiated - bool GetSslCipherSuite(uint16_t* cipher) override; + bool GetSslCipher(std::string* cipher) override; // Once DTLS has been established, this method retrieves the certificate in // use by the remote peer, for use in external identity verification. diff --git a/webrtc/p2p/base/dtlstransportchannel_unittest.cc b/webrtc/p2p/base/dtlstransportchannel_unittest.cc index 814957785a..e62563e9b5 100644 --- a/webrtc/p2p/base/dtlstransportchannel_unittest.cc +++ b/webrtc/p2p/base/dtlstransportchannel_unittest.cc @@ -217,7 +217,7 @@ class DtlsTestClient : public sigslot::has_slots<> { channels_.begin(); it != channels_.end(); ++it) { std::string cipher; - bool rv = (*it)->GetSrtpCryptoSuite(&cipher); + bool rv = (*it)->GetSrtpCipher(&cipher); if (negotiated_dtls_ && !expected_cipher.empty()) { ASSERT_TRUE(rv); @@ -228,13 +228,13 @@ class DtlsTestClient : public sigslot::has_slots<> { } } - void CheckSsl(uint16_t expected_cipher) { + void CheckSsl(const std::string& expected_cipher) { for (std::vector::iterator it = channels_.begin(); it != channels_.end(); ++it) { - uint16_t cipher; + std::string cipher; - bool rv = (*it)->GetSslCipherSuite(&cipher); - if (negotiated_dtls_ && expected_cipher) { + bool rv = (*it)->GetSslCipher(&cipher); + if (negotiated_dtls_ && !expected_cipher.empty()) { ASSERT_TRUE(rv); ASSERT_EQ(cipher, expected_cipher); @@ -463,10 +463,10 @@ class DtlsTransportChannelTest : public testing::Test { client1_.CheckSrtp(""); client2_.CheckSrtp(""); } - client1_.CheckSsl(rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( - ssl_expected_version_, rtc::KT_DEFAULT)); - client2_.CheckSsl(rtc::SSLStreamAdapter::GetDefaultSslCipherForTest( - ssl_expected_version_, rtc::KT_DEFAULT)); + client1_.CheckSsl( + rtc::SSLStreamAdapter::GetDefaultSslCipher(ssl_expected_version_)); + client2_.CheckSsl( + rtc::SSLStreamAdapter::GetDefaultSslCipher(ssl_expected_version_)); return true; } diff --git a/webrtc/p2p/base/faketransportcontroller.h b/webrtc/p2p/base/faketransportcontroller.h index 961728d96d..826b1a6384 100644 --- a/webrtc/p2p/base/faketransportcontroller.h +++ b/webrtc/p2p/base/faketransportcontroller.h @@ -243,7 +243,7 @@ class FakeTransportChannel : public TransportChannelImpl, return true; } - bool GetSrtpCryptoSuite(std::string* cipher) override { + bool GetSrtpCipher(std::string* cipher) override { if (!chosen_srtp_cipher_.empty()) { *cipher = chosen_srtp_cipher_; return true; @@ -251,7 +251,7 @@ class FakeTransportChannel : public TransportChannelImpl, return false; } - bool GetSslCipherSuite(uint16_t* cipher) override { return false; } + bool GetSslCipher(std::string* cipher) override { return false; } rtc::scoped_refptr GetLocalCertificate() const { return local_cert_; diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h index 8859111bd1..b3e7bc1a84 100644 --- a/webrtc/p2p/base/p2ptransportchannel.h +++ b/webrtc/p2p/base/p2ptransportchannel.h @@ -111,10 +111,10 @@ class P2PTransportChannel : public TransportChannelImpl, } // Find out which DTLS-SRTP cipher was negotiated. - bool GetSrtpCryptoSuite(std::string* cipher) override { return false; } + bool GetSrtpCipher(std::string* cipher) override { return false; } // Find out which DTLS cipher was negotiated. - bool GetSslCipherSuite(uint16_t* cipher) override { return false; } + bool GetSslCipher(std::string* cipher) override { return false; } // Returns null because the channel is not encrypted by default. rtc::scoped_refptr GetLocalCertificate() const override { diff --git a/webrtc/p2p/base/transport.cc b/webrtc/p2p/base/transport.cc index 80ae900ae6..66ba63e8a2 100644 --- a/webrtc/p2p/base/transport.cc +++ b/webrtc/p2p/base/transport.cc @@ -307,8 +307,8 @@ bool Transport::GetStats(TransportStats* stats) { TransportChannelImpl* channel = kv.second; TransportChannelStats substats; substats.component = channel->component(); - channel->GetSrtpCryptoSuite(&substats.srtp_cipher); - channel->GetSslCipherSuite(&substats.ssl_cipher); + channel->GetSrtpCipher(&substats.srtp_cipher); + channel->GetSslCipher(&substats.ssl_cipher); if (!channel->GetStats(&substats.connection_infos)) { return false; } diff --git a/webrtc/p2p/base/transport.h b/webrtc/p2p/base/transport.h index df0a34ce01..a3daa39fbd 100644 --- a/webrtc/p2p/base/transport.h +++ b/webrtc/p2p/base/transport.h @@ -108,10 +108,10 @@ typedef std::vector ConnectionInfos; // Information about a specific channel struct TransportChannelStats { - int component = 0; + int component; ConnectionInfos connection_infos; std::string srtp_cipher; - uint16_t ssl_cipher = 0; + std::string ssl_cipher; }; // Information about all the channels of a transport. diff --git a/webrtc/p2p/base/transportchannel.h b/webrtc/p2p/base/transportchannel.h index dcfc14dfbf..5e7adfa9c7 100644 --- a/webrtc/p2p/base/transportchannel.h +++ b/webrtc/p2p/base/transportchannel.h @@ -106,10 +106,10 @@ class TransportChannel : public sigslot::has_slots<> { virtual bool SetSrtpCiphers(const std::vector& ciphers) = 0; // Finds out which DTLS-SRTP cipher was negotiated. - virtual bool GetSrtpCryptoSuite(std::string* cipher) = 0; + virtual bool GetSrtpCipher(std::string* cipher) = 0; // Finds out which DTLS cipher was negotiated. - virtual bool GetSslCipherSuite(uint16_t* cipher) = 0; + virtual bool GetSslCipher(std::string* cipher) = 0; // Gets the local RTCCertificate used for DTLS. virtual rtc::scoped_refptr