admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Added UMA counters for SDES vs DTLS key agreement

Browse Source 
This is required to figure out when we can deprecate and remove
SDES.

Bug: chromium:804275
Change-Id: Ie234e6b3c8f5de8e78dda8d755d955caa61b7aa7
Reviewed-on: https://webrtc-review.googlesource.com/43340
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21746}
This commit is contained in:
Harald Alvestrand 2018-01-24 16:04:13 +01:00 committed by Commit Bot
parent d2b5b1f5ba
commit 194939b8ca
3 changed files with 41 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
api/umametrics.h
View File

@ -37,6 +37,7 @@ enum PeerConnectionEnumCounterType {
kEnumCounterDtlsHandshakeError,
kEnumCounterIceRegathering,
kEnumCounterIceRestart,
kEnumCounterKeyProtocol,
kPeerConnectionEnumCounterMax
};
@ -111,6 +112,12 @@ enum IceCandidatePairType {
kIceCandidatePairMax
};
enum KeyExchangeProtocolType {
kEnumCounterKeyProtocolDtls,
kEnumCounterKeyProtocolSdes,
kEnumCounterKeyProtocolMax
};
class MetricsObserverInterface : public rtc::RefCountInterface {
public:
// |type| is the type of the enum counter to be incremented. |counter|

17
pc/peerconnection.cc
View File

@ -364,7 +364,9 @@ bool MediaSectionsHaveSameCount(const SessionDescription* desc1,
// needs a ufrag and pwd. Mismatches, such as replying with a DTLS fingerprint
// to SDES keys, will be caught in JsepTransport negotiation, and backstopped
// by Channel's |srtp_required| check.
RTCError VerifyCrypto(const SessionDescription* desc, bool dtls_enabled) {
RTCError VerifyCrypto(const SessionDescription* desc,
bool dtls_enabled,
rtc::scoped_refptr<webrtc::UMAObserver> uma_observer) {
const cricket::ContentGroup* bundle =
desc->GetGroupByName(cricket::GROUP_TYPE_BUNDLE);
for (const cricket::ContentInfo& content_info : desc->contents()) {
@ -396,12 +398,22 @@ RTCError VerifyCrypto(const SessionDescription* desc, bool dtls_enabled) {
return RTCError(RTCErrorType::INVALID_PARAMETER,
kSdpWithoutDtlsFingerprint);
}
if (uma_observer) {
uma_observer->IncrementEnumCounter(webrtc::kEnumCounterKeyProtocol,
webrtc::kEnumCounterKeyProtocolDtls,
webrtc::kEnumCounterKeyProtocolMax);
}
} else {
if (media->cryptos().empty()) {
RTC_LOG(LS_WARNING)
<< "Session description must have SDES when DTLS disabled.";
return RTCError(RTCErrorType::INVALID_PARAMETER, kSdpWithoutSdesCrypto);
}
if (uma_observer) {
uma_observer->IncrementEnumCounter(webrtc::kEnumCounterKeyProtocol,
webrtc::kEnumCounterKeyProtocolSdes,
webrtc::kEnumCounterKeyProtocolMax);
}
}
}
return RTCError::OK();
@ -5319,7 +5331,8 @@ RTCError PeerConnection::ValidateSessionDescription(
std::string crypto_error;
if (webrtc_session_desc_factory_->SdesPolicy() == cricket::SEC_REQUIRED ||
dtls_enabled_) {
RTCError crypto_error = VerifyCrypto(sdesc->description(), dtls_enabled_);
RTCError crypto_error =
VerifyCrypto(sdesc->description(), dtls_enabled_, uma_observer_);
if (!crypto_error.ok()) {
return crypto_error;
}

19
pc/peerconnection_integrationtest.cc
View File

@ -1284,6 +1284,10 @@ TEST_F(PeerConnectionIntegrationTest, DtmfSenderObserver) {
TEST_F(PeerConnectionIntegrationTest, EndToEndCallWithDtls) {
ASSERT_TRUE(CreatePeerConnectionWrappers());
ConnectFakeSignaling();
rtc::scoped_refptr<webrtc::FakeMetricsObserver> caller_observer =
new rtc::RefCountedObject<webrtc::FakeMetricsObserver>();
caller()->pc()->RegisterUMAObserver(caller_observer);
// Do normal offer/answer and wait for some frames to be received in each
// direction.
caller()->AddAudioVideoTracks();
@ -1294,6 +1298,12 @@ TEST_F(PeerConnectionIntegrationTest, EndToEndCallWithDtls) {
kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount,
kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount,
kMaxWaitForFramesMs);
EXPECT_LE(
1, caller_observer->GetEnumCounter(webrtc::kEnumCounterKeyProtocol,
webrtc::kEnumCounterKeyProtocolDtls));
EXPECT_EQ(
0, caller_observer->GetEnumCounter(webrtc::kEnumCounterKeyProtocol,
webrtc::kEnumCounterKeyProtocolSdes));
}
// Uses SDES instead of DTLS for key agreement.
@ -1302,6 +1312,9 @@ TEST_F(PeerConnectionIntegrationTest, EndToEndCallWithSdes) {
sdes_config.enable_dtls_srtp.emplace(false);
ASSERT_TRUE(CreatePeerConnectionWrappersWithConfig(sdes_config, sdes_config));
ConnectFakeSignaling();
rtc::scoped_refptr<webrtc::FakeMetricsObserver> caller_observer =
new rtc::RefCountedObject<webrtc::FakeMetricsObserver>();
caller()->pc()->RegisterUMAObserver(caller_observer);
// Do normal offer/answer and wait for some frames to be received in each
// direction.
@ -1313,6 +1326,12 @@ TEST_F(PeerConnectionIntegrationTest, EndToEndCallWithSdes) {
kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount,
kDefaultExpectedAudioFrameCount, kDefaultExpectedVideoFrameCount,
kMaxWaitForFramesMs);
EXPECT_LE(
1, caller_observer->GetEnumCounter(webrtc::kEnumCounterKeyProtocol,
webrtc::kEnumCounterKeyProtocolSdes));
EXPECT_EQ(
0, caller_observer->GetEnumCounter(webrtc::kEnumCounterKeyProtocol,
webrtc::kEnumCounterKeyProtocolDtls));
}
// Tests that the GetRemoteAudioSSLCertificate method returns the remote DTLS