Report SRTP error codes to UMA

Bug: webrtc:8996
Change-Id: I75de77ed15c2829425c00f57ebd07109803425db
Reviewed-on: https://webrtc-review.googlesource.com/63122
Reviewed-by: Zhi Huang <zhihuang@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22521}

api/umametrics.h

@@ -42,6 +42,9 @@ enum PeerConnectionEnumCounterType {
   kEnumCounterSdpSemanticNegotiated,
   kEnumCounterKeyProtocolMediaType,
   kEnumCounterSdpFormatReceived,
+  // The next 2 counters log the value of srtp_err_status_t defined in libsrtp.
+  kEnumCounterSrtpUnprotectError,
+  kEnumCounterSrtcpUnprotectError,
   kPeerConnectionEnumCounterMax
 };
 

pc/BUILD.gn

@@ -312,6 +312,7 @@ if (rtc_include_tests) {
       ":rtc_pc",
       ":rtc_pc_base",
       "../api:array_view",
+      "../api:fakemetricsobserver",
       "../api:libjingle_peerconnection_api",
       "../logging:rtc_event_log_api",
       "../media:rtc_media_base",

pc/channel.cc

@@ -144,6 +144,12 @@ void BaseChannel::ConnectToRtpTransport() {
                                               &BaseChannel::OnWritableState);
   rtp_transport_->SignalSentPacket.connect(this,
                                            &BaseChannel::SignalSentPacket_n);
+  // TODO(bugs.webrtc.org/8587): Set the metrics observer through
+  // JsepTransportController once it takes responsibility for creating
+  // RtpTransports.
+  if (metrics_observer_) {
+    rtp_transport_->SetMetricsObserver(metrics_observer_);
+  }
 }
 
 void BaseChannel::DisconnectFromRtpTransport() {
@@ -153,6 +159,7 @@ void BaseChannel::DisconnectFromRtpTransport() {
   rtp_transport_->SignalNetworkRouteChanged.disconnect(this);
   rtp_transport_->SignalWritableState.disconnect(this);
   rtp_transport_->SignalSentPacket.disconnect(this);
+  rtp_transport_->SetMetricsObserver(nullptr);
 }
 
 void BaseChannel::Init_w(DtlsTransportInternal* rtp_dtls_transport,
@@ -354,6 +361,14 @@ void BaseChannel::SetTransport_n(
   }
 }
 
+void BaseChannel::SetMetricsObserver(
+    rtc::scoped_refptr<webrtc::MetricsObserverInterface> metrics_observer) {
+  metrics_observer_ = metrics_observer;
+  if (rtp_transport_) {
+    rtp_transport_->SetMetricsObserver(metrics_observer);
+  }
+}
+
 bool BaseChannel::Enable(bool enable) {
   worker_thread_->Invoke<void>(
       RTC_FROM_HERE,

pc/channel.h

@@ -204,6 +204,9 @@ class BaseChannel
     transport_name_ = transport_name;
   }
 
+  void SetMetricsObserver(
+      rtc::scoped_refptr<webrtc::MetricsObserverInterface> metrics_observer);
+
  protected:
   virtual MediaChannel* media_channel() const { return media_channel_.get(); }
 
@@ -395,6 +398,8 @@ class BaseChannel
 
   const bool rtcp_mux_required_;
 
+  rtc::scoped_refptr<webrtc::MetricsObserverInterface> metrics_observer_;
+
   // Separate DTLS/non-DTLS pointers to support using BaseChannel without DTLS.
   // Temporary measure until more refactoring is done.
   // If non-null, "X_dtls_transport_" will always equal "X_packet_transport_".

pc/peerconnection.cc

@@ -2866,6 +2866,13 @@ void PeerConnection::RegisterUMAObserver(UMAObserver* observer) {
     transport_controller()->SetMetricsObserver(uma_observer_);
   }
 
+  for (auto transceiver : transceivers_) {
+    auto* channel = transceiver->internal()->channel();
+    if (channel) {
+      channel->SetMetricsObserver(uma_observer_);
+    }
+  }
+
   // Send information about IPv4/IPv6 status.
   if (uma_observer_) {
     port_allocator_->SetMetricsObserver(uma_observer_);
@@ -5420,6 +5427,9 @@ cricket::VoiceChannel* PeerConnection::CreateVoiceChannel(
       this, &PeerConnection::OnDtlsSrtpSetupFailure);
   voice_channel->SignalSentPacket.connect(this,
                                           &PeerConnection::OnSentPacket_w);
+  if (uma_observer_) {
+    voice_channel->SetMetricsObserver(uma_observer_);
+  }
 
   return voice_channel;
 }
@@ -5458,6 +5468,9 @@ cricket::VideoChannel* PeerConnection::CreateVideoChannel(
       this, &PeerConnection::OnDtlsSrtpSetupFailure);
   video_channel->SignalSentPacket.connect(this,
                                           &PeerConnection::OnSentPacket_w);
+  if (uma_observer_) {
+    video_channel->SetMetricsObserver(uma_observer_);
+  }
 
   return video_channel;
 }
@@ -5511,6 +5524,9 @@ bool PeerConnection::CreateDataChannel(const std::string& mid,
         this, &PeerConnection::OnDtlsSrtpSetupFailure);
     rtp_data_channel_->SignalSentPacket.connect(
         this, &PeerConnection::OnSentPacket_w);
+    if (uma_observer_) {
+      rtp_data_channel_->SetMetricsObserver(uma_observer_);
+    }
   }
 
   return true;

pc/rtptransport.h

@@ -70,6 +70,9 @@ class RtpTransport : public RtpTransportInternal {
 
   void AddHandledPayloadType(int payload_type) override;
 
+  void SetMetricsObserver(
+      rtc::scoped_refptr<MetricsObserverInterface> metrics_observer) override {}
+
  protected:
   // TODO(zstein): Remove this when we remove RtpTransportAdapter.
   RtpTransportAdapter* GetInternal() override;

pc/rtptransportinternal.h

@@ -14,6 +14,7 @@
 #include <string>
 
 #include "api/ortc/rtptransportinterface.h"
+#include "api/umametrics.h"
 #include "p2p/base/icetransportinternal.h"
 #include "rtc_base/networkroute.h"
 #include "rtc_base/sigslot.h"
@@ -82,6 +83,9 @@ class RtpTransportInternal : public RtpTransportInterface,
   virtual bool HandlesPayloadType(int payload_type) const = 0;
 
   virtual void AddHandledPayloadType(int payload_type) = 0;
+
+  virtual void SetMetricsObserver(
+      rtc::scoped_refptr<MetricsObserverInterface> metrics_observer) = 0;
 };
 
 }  // namespace webrtc

pc/rtptransportinternaladapter.h

@@ -92,6 +92,11 @@ class RtpTransportInternalAdapter : public RtpTransportInternal {
     return transport_->GetParameters();
   }
 
+  void SetMetricsObserver(
+      rtc::scoped_refptr<MetricsObserverInterface> metrics_observer) override {
+    transport_->SetMetricsObserver(metrics_observer);
+  }
+
  protected:
   // Owned by the subclasses.
   RtpTransportInternal* transport_;

pc/srtpsession.cc

@@ -133,6 +133,10 @@ bool SrtpSession::UnprotectRtp(void* p, int in_len, int* out_len) {
   int err = srtp_unprotect(session_, p, out_len);
   if (err != srtp_err_status_ok) {
     RTC_LOG(LS_WARNING) << "Failed to unprotect SRTP packet, err=" << err;
+    if (metrics_observer_) {
+      metrics_observer_->IncrementSparseEnumCounter(
+          webrtc::kEnumCounterSrtpUnprotectError, err);
+    }
     return false;
   }
   return true;
@@ -149,6 +153,10 @@ bool SrtpSession::UnprotectRtcp(void* p, int in_len, int* out_len) {
   int err = srtp_unprotect_rtcp(session_, p, out_len);
   if (err != srtp_err_status_ok) {
     RTC_LOG(LS_WARNING) << "Failed to unprotect SRTCP packet, err=" << err;
+    if (metrics_observer_) {
+      metrics_observer_->IncrementSparseEnumCounter(
+          webrtc::kEnumCounterSrtcpUnprotectError, err);
+    }
     return false;
   }
   return true;
@@ -344,6 +352,11 @@ bool SrtpSession::UpdateKey(int type,
   return DoSetKey(type, cs, key, len, extension_ids);
 }
 
+void SrtpSession::SetMetricsObserver(
+    rtc::scoped_refptr<webrtc::MetricsObserverInterface> metrics_observer) {
+  metrics_observer_ = metrics_observer;
+}
+
 int g_libsrtp_usage_count = 0;
 rtc::GlobalLockPod g_libsrtp_lock;
 

pc/srtpsession.h

@@ -13,7 +13,9 @@
 
 #include <vector>
 
+#include "api/umametrics.h"
 #include "rtc_base/basictypes.h"
+#include "rtc_base/scoped_ref_ptr.h"
 #include "rtc_base/thread_checker.h"
 
 // Forward declaration to avoid pulling in libsrtp headers here
@@ -83,6 +85,9 @@ class SrtpSession {
   // been set.
   bool IsExternalAuthActive() const;
 
+  void SetMetricsObserver(
+      rtc::scoped_refptr<webrtc::MetricsObserverInterface> metrics_observer);
+
  private:
   bool DoSetKey(int type,
                 int cs,
@@ -122,6 +127,7 @@ class SrtpSession {
   int last_send_seq_num_ = -1;
   bool external_auth_active_ = false;
   bool external_auth_enabled_ = false;
+  rtc::scoped_refptr<webrtc::MetricsObserverInterface> metrics_observer_;
   RTC_DISALLOW_COPY_AND_ASSIGN(SrtpSession);
 };
 

pc/srtpsession_unittest.cc

@@ -12,13 +12,18 @@
 
 #include <string>
 
+#include "api/fakemetricsobserver.h"
 #include "media/base/fakertp.h"
 #include "pc/srtptestutil.h"
 #include "rtc_base/gunit.h"
+#include "rtc_base/ptr_util.h"
 #include "rtc_base/sslstreamadapter.h"  // For rtc::SRTP_*
+#include "third_party/libsrtp/include/srtp.h"
 
 namespace rtc {
 
+using webrtc::FakeMetricsObserver;
+
 std::vector<int> kEncryptedHeaderExtensionIds;
 
 class SrtpSessionTest : public testing::Test {
@@ -131,6 +136,9 @@ TEST_F(SrtpSessionTest, TestGetSendStreamPacketIndex) {
 
 // Test that we fail to unprotect if someone tampers with the RTP/RTCP paylaods.
 TEST_F(SrtpSessionTest, TestTamperReject) {
+  rtc::scoped_refptr<FakeMetricsObserver> metrics_observer(
+      new rtc::RefCountedObject<FakeMetricsObserver>());
+  s2_.SetMetricsObserver(metrics_observer);
   int out_len;
   EXPECT_TRUE(s1_.SetSend(SRTP_AES128_CM_SHA1_80, kTestKey1, kTestKeyLen,
                           kEncryptedHeaderExtensionIds));
@@ -141,18 +149,29 @@ TEST_F(SrtpSessionTest, TestTamperReject) {
   rtp_packet_[0] = 0x12;
   rtcp_packet_[1] = 0x34;
   EXPECT_FALSE(s2_.UnprotectRtp(rtp_packet_, rtp_len_, &out_len));
+  EXPECT_TRUE(metrics_observer->ExpectOnlySingleEnumCount(
+      webrtc::kEnumCounterSrtpUnprotectError, srtp_err_status_bad_param));
   EXPECT_FALSE(s2_.UnprotectRtcp(rtcp_packet_, rtcp_len_, &out_len));
+  EXPECT_TRUE(metrics_observer->ExpectOnlySingleEnumCount(
+      webrtc::kEnumCounterSrtcpUnprotectError, srtp_err_status_auth_fail));
 }
 
 // Test that we fail to unprotect if the payloads are not authenticated.
 TEST_F(SrtpSessionTest, TestUnencryptReject) {
+  rtc::scoped_refptr<FakeMetricsObserver> metrics_observer(
+      new rtc::RefCountedObject<FakeMetricsObserver>());
+  s2_.SetMetricsObserver(metrics_observer);
   int out_len;
   EXPECT_TRUE(s1_.SetSend(SRTP_AES128_CM_SHA1_80, kTestKey1, kTestKeyLen,
                           kEncryptedHeaderExtensionIds));
   EXPECT_TRUE(s2_.SetRecv(SRTP_AES128_CM_SHA1_80, kTestKey1, kTestKeyLen,
                           kEncryptedHeaderExtensionIds));
   EXPECT_FALSE(s2_.UnprotectRtp(rtp_packet_, rtp_len_, &out_len));
+  EXPECT_TRUE(metrics_observer->ExpectOnlySingleEnumCount(
+      webrtc::kEnumCounterSrtpUnprotectError, srtp_err_status_auth_fail));
   EXPECT_FALSE(s2_.UnprotectRtcp(rtcp_packet_, rtcp_len_, &out_len));
+  EXPECT_TRUE(metrics_observer->ExpectOnlySingleEnumCount(
+      webrtc::kEnumCounterSrtcpUnprotectError, srtp_err_status_cant_check));
 }
 
 // Test that we fail when using buffers that are too small.

pc/srtptransport.cc

@@ -259,6 +259,11 @@ bool SrtpTransport::SetRtcpParams(int send_cs,
     return false;
   }
 
+  if (metrics_observer_) {
+    send_rtcp_session_->SetMetricsObserver(metrics_observer_);
+    recv_rtcp_session_->SetMetricsObserver(metrics_observer_);
+  }
+
   RTC_LOG(LS_INFO) << "SRTCP activated with negotiated parameters:"
                       " send cipher_suite "
                    << send_cs << " recv cipher_suite " << recv_cs;
@@ -281,6 +286,10 @@ void SrtpTransport::ResetParams() {
 void SrtpTransport::CreateSrtpSessions() {
   send_session_.reset(new cricket::SrtpSession());
   recv_session_.reset(new cricket::SrtpSession());
+  if (metrics_observer_) {
+    send_session_->SetMetricsObserver(metrics_observer_);
+    recv_session_->SetMetricsObserver(metrics_observer_);
+  }
 
   if (external_auth_enabled_) {
     send_session_->EnableExternalAuth();
@@ -390,4 +399,22 @@ bool SrtpTransport::IsExternalAuthActive() const {
   return send_session_->IsExternalAuthActive();
 }
 
+void SrtpTransport::SetMetricsObserver(
+    rtc::scoped_refptr<MetricsObserverInterface> metrics_observer) {
+  metrics_observer_ = metrics_observer;
+  if (send_session_) {
+    send_session_->SetMetricsObserver(metrics_observer_);
+  }
+  if (recv_session_) {
+    recv_session_->SetMetricsObserver(metrics_observer_);
+  }
+  if (send_rtcp_session_) {
+    send_rtcp_session_->SetMetricsObserver(metrics_observer_);
+  }
+  if (recv_rtcp_session_) {
+    recv_rtcp_session_->SetMetricsObserver(metrics_observer_);
+  }
+  rtp_transport_->SetMetricsObserver(metrics_observer);
+}
+
 }  // namespace webrtc

pc/srtptransport.h

@@ -98,6 +98,9 @@ class SrtpTransport : public RtpTransportInternalAdapter {
     rtp_abs_sendtime_extn_id_ = rtp_abs_sendtime_extn_id;
   }
 
+  void SetMetricsObserver(
+      rtc::scoped_refptr<MetricsObserverInterface> metrics_observer) override;
+
  private:
   void ConnectToRtpTransport();
   void CreateSrtpSessions();
@@ -146,6 +149,8 @@ class SrtpTransport : public RtpTransportInternalAdapter {
   bool external_auth_enabled_ = false;
 
   int rtp_abs_sendtime_extn_id_ = -1;
+
+  rtc::scoped_refptr<MetricsObserverInterface> metrics_observer_;
 };
 
 }  // namespace webrtc