From 27dc29b0df23eed5034f28d4d5f66ea0bb425d6c Mon Sep 17 00:00:00 2001 From: guoweis Date: Wed, 30 Sep 2015 19:23:09 -0700 Subject: [PATCH] Revert of Change WebRTC SslCipher to be exposed as number only. (patchset #20 id:750001 of https://codereview.webrtc.org/1337673002/ ) Reason for revert: This broke chromium.fyi bot. Original issue's description: > Change WebRTC SslCipher to be exposed as number only. > > This makes the SSL exposed as uint16_t which is the IANA value. GetRfcSslCipherName is introduced to handle the conversion to names from ID. IANA value will be used for UMA reporting. Names will still be used for WebRTC stats reporting. > > For SRTP, currently it's still string internally but is reported as IANA number. > > This is used by the ongoing CL https://codereview.chromium.org/1335023002. > > BUG=523033 > > Committed: https://crrev.com/4fe3c9a77386598db9abd1f0d6983aefee9cc943 > Cr-Commit-Position: refs/heads/master@{#10124} TBR=juberti@webrtc.org,rsleevi@chromium.org,pthatcher@webrtc.org,davidben@chromium.org,juberti@google.com,davidben@webrtc.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=523033 Review URL: https://codereview.webrtc.org/1380603005 Cr-Commit-Position: refs/heads/master@{#10125} --- talk/app/webrtc/fakemetricsobserver.cc | 37 ++++-- talk/app/webrtc/fakemetricsobserver.h | 16 ++- talk/app/webrtc/peerconnection_unittest.cc | 116 +++++++++--------- talk/app/webrtc/peerconnectioninterface.h | 11 +- talk/app/webrtc/statscollector.cc | 10 +- talk/app/webrtc/statscollector_unittest.cc | 11 +- talk/app/webrtc/umametrics.h | 13 +- talk/app/webrtc/webrtcsession.cc | 27 ++-- talk/session/media/channel.cc | 15 +-- talk/session/media/channel.h | 9 +- talk/session/media/channel_unittest.cc | 14 +-- talk/session/media/mediasession.cc | 18 +-- talk/session/media/mediasession.h | 2 +- talk/session/media/mediasession_unittest.cc | 4 +- talk/session/media/srtpfilter.cc | 6 +- talk/session/media/srtpfilter.h | 8 +- talk/session/media/srtpfilter_unittest.cc | 4 +- webrtc/base/opensslstreamadapter.cc | 91 +++++++------- webrtc/base/opensslstreamadapter.h | 14 +-- webrtc/base/sslstreamadapter.cc | 33 ++--- webrtc/base/sslstreamadapter.h | 34 +---- webrtc/base/sslstreamadapter_unittest.cc | 54 ++++---- webrtc/p2p/base/dtlstransportchannel.cc | 6 +- webrtc/p2p/base/dtlstransportchannel.h | 4 +- .../p2p/base/dtlstransportchannel_unittest.cc | 18 +-- webrtc/p2p/base/faketransportcontroller.h | 4 +- webrtc/p2p/base/p2ptransportchannel.h | 4 +- webrtc/p2p/base/transport.cc | 4 +- webrtc/p2p/base/transport.h | 4 +- webrtc/p2p/base/transportchannel.h | 4 +- 30 files changed, 275 insertions(+), 320 deletions(-) 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