Improve testing of SRTP external auth code paths.

webrtc/pc/srtpfilter.cc

Index: webrtc/pc/srtpfilter.cc
diff --git a/webrtc/pc/srtpfilter.cc b/webrtc/pc/srtpfilter.cc
index 5094987a76568336780cf8214eb402c03a3a69c0..617856f4e44e08c277f06f4f92c361b0d1a1b835 100644
--- a/webrtc/pc/srtpfilter.cc
+++ b/webrtc/pc/srtpfilter.cc
@@ -51,9 +51,10 @@ void ShutdownSrtp() {
 #endif
 }
 
-SrtpFilter::SrtpFilter()
-    : state_(ST_INIT),
-      signal_silent_time_in_ms_(0) {
+SrtpFilter::SrtpFilter() {
+#if defined(ENABLE_EXTERNAL_AUTH)
+  external_auth_allowed_ = true;
+#endif

[Taylor Brandstetter, 2017/03/03 02:20:34]

Another option, which I slightly prefer, would be to have two methods called
"EnableExternalAuth" and "ExternalAuthEnabled", and put the #ifdef in
webrtcsession.cc where it constructs the SrtpFilter.

#ifdef ENABLE_EXTERNAL_AUTH
srtp_filter_.EnableExternalAuth();
#endif

That way no "for testing" method would be needed, and the SrtpFilter unit tests
would succeed even when ENABLE_EXTERNAL_AUTH is defined.

[joachim, 2017/03/03 20:42:57]

Done (gets enabled in channel.cc). I used "IsExternalAuthEnabled" to match other
methods that check for something.
Yes, that's better - the define is only used in channel.cc now.

[Taylor Brandstetter, 2017/03/03 23:03:57]

Yep, channel.cc is what I meant, sorry for the mixup.

 }
 
 SrtpFilter::~SrtpFilter() {
@@ -226,7 +227,18 @@ bool SrtpFilter::GetSrtpOverhead(int* srtp_overhead) const {
   return true;
 }
 
-#if defined(ENABLE_EXTERNAL_AUTH)
+void SrtpFilter::AllowExternalAuthForTest(bool allow) {
+  external_auth_allowed_ = allow;
+  if (IsActive()) {
+    RTC_CHECK(send_session_);
+    send_session_->AllowExternalAuthForTest(allow);
+  }
+}
+
+bool SrtpFilter::IsExternalAuthAllowed() const {
+  return external_auth_allowed_;
+}
+
 bool SrtpFilter::IsExternalAuthActive() const {
   if (!IsActive()) {
     LOG(LS_WARNING) << "Failed to check IsExternalAuthActive: SRTP not active";
@@ -236,7 +248,6 @@ bool SrtpFilter::IsExternalAuthActive() const {
   RTC_CHECK(send_session_);
   return send_session_->IsExternalAuthActive();
 }
-#endif
 
 void SrtpFilter::set_signal_silent_time(int signal_silent_time_in_ms) {
   signal_silent_time_in_ms_ = signal_silent_time_in_ms;
@@ -338,6 +349,7 @@ void SrtpFilter::CreateSrtpSessions() {
 
   send_session_->set_signal_silent_time(signal_silent_time_in_ms_);
   recv_session_->set_signal_silent_time(signal_silent_time_in_ms_);
+  send_session_->AllowExternalAuthForTest(external_auth_allowed_);
 }
 
 bool SrtpFilter::NegotiateParams(const std::vector<CryptoParams>& answer_params,
@@ -476,6 +488,9 @@ bool SrtpSession::inited_ = false;
 rtc::GlobalLockPod SrtpSession::lock_;
 
 SrtpSession::SrtpSession() : srtp_stat_(new SrtpStat()) {
+#if defined(ENABLE_EXTERNAL_AUTH)
+  external_auth_allowed_ = true;
+#endif
   SignalSrtpError.repeat(srtp_stat_->SignalSrtpError);
 }
 
@@ -599,7 +614,6 @@ bool SrtpSession::UnprotectRtcp(void* p, int in_len, int* out_len) {
 }
 
 bool SrtpSession::GetRtpAuthParams(uint8_t** key, int* key_len, int* tag_len) {
-#if defined(ENABLE_EXTERNAL_AUTH)
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   RTC_DCHECK(IsExternalAuthActive());
   if (!IsExternalAuthActive()) {
@@ -624,20 +638,23 @@ bool SrtpSession::GetRtpAuthParams(uint8_t** key, int* key_len, int* tag_len) {
   *key_len = external_hmac->key_length;
   *tag_len = rtp_auth_tag_len_;
   return true;
-#else
-  return false;
-#endif
 }
 
 int SrtpSession::GetSrtpOverhead() const {
   return rtp_auth_tag_len_;
 }
 
-#if defined(ENABLE_EXTERNAL_AUTH)
+void SrtpSession::AllowExternalAuthForTest(bool allow) {
+  external_auth_allowed_ = allow;
+}
+
+bool SrtpSession::IsExternalAuthAllowed() const {
+  return external_auth_allowed_;
+}
+
 bool SrtpSession::IsExternalAuthActive() const {
   return external_auth_active_;
 }
-#endif
 
 bool SrtpSession::GetSendStreamPacketIndex(void* p,
                                            int in_len,
@@ -719,13 +736,12 @@ bool SrtpSession::SetKey(int type, int cs, const uint8_t* key, size_t len) {
   // id EXTERNAL_HMAC_SHA1 in the policy structure.
   // We want to set this option only for rtp packets.
   // By default policy structure is initialized to HMAC_SHA1.
-#if defined(ENABLE_EXTERNAL_AUTH)
   // Enable external HMAC authentication only for outgoing streams and only
   // for cipher suites that support it (i.e. only non-GCM cipher suites).
-  if (type == ssrc_any_outbound && !rtc::IsGcmCryptoSuite(cs)) {
+  if (type == ssrc_any_outbound && IsExternalAuthAllowed() &&
+      !rtc::IsGcmCryptoSuite(cs)) {
     policy.rtp.auth_type = EXTERNAL_HMAC_SHA1;
   }
-#endif
   policy.next = nullptr;
 
   int err = srtp_create(&session_, &policy);
@@ -738,9 +754,7 @@ bool SrtpSession::SetKey(int type, int cs, const uint8_t* key, size_t len) {
   srtp_set_user_data(session_, this);
   rtp_auth_tag_len_ = policy.rtp.auth_tag_len;
   rtcp_auth_tag_len_ = policy.rtcp.auth_tag_len;
-#if defined(ENABLE_EXTERNAL_AUTH)
   external_auth_active_ = (policy.rtp.auth_type == EXTERNAL_HMAC_SHA1);
-#endif
   return true;
 }
 
@@ -760,13 +774,12 @@ bool SrtpSession::Init() {
       LOG(LS_ERROR) << "Failed to install SRTP event handler, err=" << err;
       return false;
     }
-#if defined(ENABLE_EXTERNAL_AUTH)
+
     err = external_crypto_init();
     if (err != srtp_err_status_ok) {
       LOG(LS_ERROR) << "Failed to initialize fake auth, err=" << err;
       return false;
     }
-#endif
     inited_ = true;
   }