Support GCM ciphers even if ENABLE_EXTERNAL_AUTH is defined.

webrtc/pc/srtpfilter.cc

Index: webrtc/pc/srtpfilter.cc
diff --git a/webrtc/pc/srtpfilter.cc b/webrtc/pc/srtpfilter.cc
index e7622f7bc58af78375dbd2d0c0cfd4ae098b05cf..816e00a52b93dce67806b204d1773e88b8d16459 100644
--- a/webrtc/pc/srtpfilter.cc
+++ b/webrtc/pc/srtpfilter.cc
@@ -204,6 +204,27 @@ bool SrtpFilter::UnprotectRtcp(void* p, int in_len, int* out_len) {
   }
 }
 
+bool SrtpFilter::AllowExternalAuth() {
+  if (!IsActive()) {
+    LOG(LS_WARNING) << "Failed to AllowExternalAuth: SRTP not active";
+    return false;

[Taylor Brandstetter, 2017/02/28 22:53:40]

Could DCHECK here.

[joachim, 2017/03/01 00:43:46]

I didn't DCHECK to stay consistent with the existing methods that only log a
warning.

[Taylor Brandstetter, 2017/03/01 01:45:32]

Acknowledged.

+  }
+
+  RTC_CHECK(send_session_);
+  return send_session_->AllowExternalAuth();
+}
+
+void SrtpFilter::DisableAllowExternalAuthForTests(
+    bool disable_allow_external_auth) {
+  force_disable_allow_external_auth_ = disable_allow_external_auth;
+  if (!IsActive()) {
+    return;
+  }
+
+  RTC_CHECK(send_session_);
+  send_session_->DisableAllowExternalAuthForTests(disable_allow_external_auth);
+}
+
 bool SrtpFilter::GetRtpAuthParams(uint8_t** key, int* key_len, int* tag_len) {
   if (!IsActive()) {
     LOG(LS_WARNING) << "Failed to GetRtpAuthParams: SRTP not active";
@@ -325,6 +346,9 @@ 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_->DisableAllowExternalAuthForTests(
+      force_disable_allow_external_auth_);
 }
 
 bool SrtpFilter::NegotiateParams(const std::vector<CryptoParams>& answer_params,
@@ -462,12 +486,7 @@ bool SrtpSession::inited_ = false;
 // This lock protects SrtpSession::inited_.
 rtc::GlobalLockPod SrtpSession::lock_;
 
-SrtpSession::SrtpSession()
-    : session_(nullptr),
-      rtp_auth_tag_len_(0),
-      rtcp_auth_tag_len_(0),
-      srtp_stat_(new SrtpStat()),
-      last_send_seq_num_(-1) {
+SrtpSession::SrtpSession() : srtp_stat_(new SrtpStat()) {
   SignalSrtpError.repeat(srtp_stat_->SignalSrtpError);
 }
 
@@ -590,9 +609,23 @@ bool SrtpSession::UnprotectRtcp(void* p, int in_len, int* out_len) {
   return true;
 }
 
+bool SrtpSession::AllowExternalAuth() {

[Taylor Brandstetter, 2017/02/28 22:53:40]

"AllowExternalAuth" implies some action, but this just returns a flag. Maybe
rename to "ExternalAuthAllowed"?

[joachim, 2017/03/01 00:43:46]

Renamed to "IsExternalAuthActive".

+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  return allow_external_auth_ && !force_disable_allow_external_auth_;
+}
+
+void SrtpSession::DisableAllowExternalAuthForTests(
+    bool disable_allow_external_auth) {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  force_disable_allow_external_auth_ = disable_allow_external_auth;
+}
+
 bool SrtpSession::GetRtpAuthParams(uint8_t** key, int* key_len, int* tag_len) {
-#if defined(ENABLE_EXTERNAL_AUTH)
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  if (!AllowExternalAuth()) {

[Taylor Brandstetter, 2017/02/28 22:53:40]

DCHECK?

[joachim, 2017/03/01 00:43:46]

Done.

+    return false;
+  }
+
   ExternalHmacContext* external_hmac = nullptr;
   // stream_template will be the reference context for other streams.
   // Let's use it for getting the keys.
@@ -611,9 +644,6 @@ 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 {
@@ -658,19 +688,20 @@ bool SrtpSession::SetKey(int type, int cs, const uint8_t* key, size_t len) {
   if (cs == rtc::SRTP_AES128_CM_SHA1_80) {
     srtp_crypto_policy_set_aes_cm_128_hmac_sha1_80(&policy.rtp);
     srtp_crypto_policy_set_aes_cm_128_hmac_sha1_80(&policy.rtcp);
+    allow_external_auth_ = true;
   } else if (cs == rtc::SRTP_AES128_CM_SHA1_32) {
     // RTP HMAC is shortened to 32 bits, but RTCP remains 80 bits.
     srtp_crypto_policy_set_aes_cm_128_hmac_sha1_32(&policy.rtp);
     srtp_crypto_policy_set_aes_cm_128_hmac_sha1_80(&policy.rtcp);
-#if !defined(ENABLE_EXTERNAL_AUTH)
-    // TODO(jbauch): Re-enable once https://crbug.com/628400 is resolved.
+    allow_external_auth_ = true;
   } else if (cs == rtc::SRTP_AEAD_AES_128_GCM) {
     srtp_crypto_policy_set_aes_gcm_128_16_auth(&policy.rtp);
     srtp_crypto_policy_set_aes_gcm_128_16_auth(&policy.rtcp);
+    allow_external_auth_ = false;
   } else if (cs == rtc::SRTP_AEAD_AES_256_GCM) {
     srtp_crypto_policy_set_aes_gcm_256_16_auth(&policy.rtp);
     srtp_crypto_policy_set_aes_gcm_256_16_auth(&policy.rtcp);
-#endif  // ENABLE_EXTERNAL_AUTH
+    allow_external_auth_ = false;
   } else {
     LOG(LS_WARNING) << "Failed to create SRTP session: unsupported"
                     << " cipher_suite " << cs;
@@ -703,12 +734,10 @@ 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.
-  if (type == ssrc_any_outbound) {
+  if (AllowExternalAuth() && type == ssrc_any_outbound) {
     policy.rtp.auth_type = EXTERNAL_HMAC_SHA1;
   }
-#endif
   policy.next = nullptr;
 
   int err = srtp_create(&session_, &policy);
@@ -740,13 +769,13 @@ 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;
   }