Support GCM ciphers even if ENABLE_EXTERNAL_AUTH is defined.

webrtc/pc/srtpfilter_unittest.cc

 /*
  *  Copyright 2004 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/pc/srtpfilter.h"
 
 #include "third_party/libsrtp/include/srtp.h"
+#include "webrtc/base/buffer.h"
 #include "webrtc/base/byteorder.h"
 #include "webrtc/base/constructormagic.h"
 #include "webrtc/base/gunit.h"
 #include "webrtc/base/thread.h"
 #include "webrtc/media/base/cryptoparams.h"
 #include "webrtc/media/base/fakertp.h"
 #include "webrtc/p2p/base/sessiondescription.h"
 
 using rtc::CS_AES_CM_128_HMAC_SHA1_80;
 using rtc::CS_AES_CM_128_HMAC_SHA1_32;
 using rtc::CS_AEAD_AES_128_GCM;
 using rtc::CS_AEAD_AES_256_GCM;
 using cricket::CryptoParams;
 using cricket::CS_LOCAL;
 using cricket::CS_REMOTE;
 
 static const uint8_t kTestKey1[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ1234";
 static const uint8_t kTestKey2[] = "4321ZYXWVUTSRQPONMLKJIHGFEDCBA";
 static const int kTestKeyLen = 30;
+static const uint8_t kTestKeyGcm128_1[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ12";
+static const uint8_t kTestKeyGcm128_2[] = "21ZYXWVUTSRQPONMLKJIHGFEDCBA";
+static const int kTestKeyGcm128Len = 28;  // 128 bits key + 96 bits salt.
+static const uint8_t kTestKeyGcm256_1[] =
+    "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqr";
+static const uint8_t kTestKeyGcm256_2[] =
+    "rqponmlkjihgfedcbaZYXWVUTSRQPONMLKJIHGFEDCBA";
+static const int kTestKeyGcm256Len = 44;  // 256 bits key + 96 bits salt.
 static const std::string kTestKeyParams1 =
     "inline:WVNfX19zZW1jdGwgKCkgewkyMjA7fQp9CnVubGVz";
 static const std::string kTestKeyParams2 =
     "inline:PS1uQCVeeCFCanVmcjkpPywjNWhcYD0mXXtxaVBR";
 static const std::string kTestKeyParams3 =
     "inline:1234X19zZW1jdGwgKCkgewkyMjA7fQp9CnVubGVz";
 static const std::string kTestKeyParams4 =
     "inline:4567QCVeeCFCanVmcjkpPywjNWhcYD0mXXtxaVBR";
 static const std::string kTestKeyParamsGcm1 =
     "inline:e166KFlKzJsGW0d5apX+rrI05vxbrvMJEzFI14aTDCa63IRTlLK4iH66uOI=";
@@ skipping 10 matching lines @@
 static const cricket::CryptoParams kTestCryptoParamsGcm1(
     1, "AEAD_AES_256_GCM", kTestKeyParamsGcm1, "");
 static const cricket::CryptoParams kTestCryptoParamsGcm2(
     1, "AEAD_AES_256_GCM", kTestKeyParamsGcm2, "");
 static const cricket::CryptoParams kTestCryptoParamsGcm3(
     1, "AEAD_AES_128_GCM", kTestKeyParamsGcm3, "");
 static const cricket::CryptoParams kTestCryptoParamsGcm4(
     1, "AEAD_AES_128_GCM", kTestKeyParamsGcm4, "");
 
 static int rtp_auth_tag_len(const std::string& cs) {
-  return (cs == CS_AES_CM_128_HMAC_SHA1_32) ? 4 : 10;
+  if (cs == CS_AES_CM_128_HMAC_SHA1_32) {
+    return 4;
+  } else if (cs == CS_AEAD_AES_128_GCM || cs == CS_AEAD_AES_256_GCM) {
+    return 16;
+  } else {
+    return 10;
+  }
 }
 static int rtcp_auth_tag_len(const std::string& cs) {
-  return 10;
+  if (cs == CS_AEAD_AES_128_GCM || cs == CS_AEAD_AES_256_GCM) {
+    return 16;
+  } else {
+    return 10;
+  }
 }
 
 class SrtpFilterTest : public testing::Test {
  protected:
   SrtpFilterTest()
   // Need to initialize |sequence_number_|, the value does not matter.
       : sequence_number_(1) {
+    f1_.DisableAllowExternalAuthForTests(true);

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

Can you explain why this is needed in a comment?

+    f2_.DisableAllowExternalAuthForTests(true);
   }
   static std::vector<CryptoParams> MakeVector(const CryptoParams& params) {
     std::vector<CryptoParams> vec;
     vec.push_back(params);
     return vec;
   }
   void TestSetParams(const std::vector<CryptoParams>& params1,
                      const std::vector<CryptoParams>& params2) {
     EXPECT_TRUE(f1_.SetOffer(params1, CS_LOCAL));
     EXPECT_TRUE(f2_.SetOffer(params1, CS_REMOTE));
     EXPECT_FALSE(f1_.IsActive());
     EXPECT_FALSE(f2_.IsActive());
     EXPECT_TRUE(f2_.SetAnswer(params2, CS_LOCAL));
     EXPECT_TRUE(f1_.SetAnswer(params2, CS_REMOTE));
     EXPECT_TRUE(f1_.IsActive());
     EXPECT_TRUE(f2_.IsActive());
   }
   void TestProtectUnprotect(const std::string& cs1, const std::string& cs2) {
-    char rtp_packet[sizeof(kPcmuFrame) + 10];
+    rtc::Buffer rtp_buffer(sizeof(kPcmuFrame) + rtp_auth_tag_len(cs1));
+    char* rtp_packet = rtp_buffer.data<char>();
     char original_rtp_packet[sizeof(kPcmuFrame)];
-    char rtcp_packet[sizeof(kRtcpReport) + 4 + 10];
+    rtc::Buffer rtcp_buffer(sizeof(kRtcpReport) + 4 + rtcp_auth_tag_len(cs2));
+    char* rtcp_packet = rtcp_buffer.data<char>();
     int rtp_len = sizeof(kPcmuFrame), rtcp_len = sizeof(kRtcpReport), out_len;
     memcpy(rtp_packet, kPcmuFrame, rtp_len);
     // In order to be able to run this test function multiple times we can not
     // use the same sequence number twice. Increase the sequence number by one.
     rtc::SetBE16(reinterpret_cast<uint8_t*>(rtp_packet) + 2,
                  ++sequence_number_);
     memcpy(original_rtp_packet, rtp_packet, rtp_len);
     memcpy(rtcp_packet, kRtcpReport, rtcp_len);
 
     EXPECT_TRUE(f1_.ProtectRtp(rtp_packet, rtp_len,
-                               sizeof(rtp_packet), &out_len));
+                               rtp_buffer.size(), &out_len));
     EXPECT_EQ(out_len, rtp_len + rtp_auth_tag_len(cs1));
     EXPECT_NE(0, memcmp(rtp_packet, original_rtp_packet, rtp_len));
     EXPECT_TRUE(f2_.UnprotectRtp(rtp_packet, out_len, &out_len));
     EXPECT_EQ(rtp_len, out_len);
     EXPECT_EQ(0, memcmp(rtp_packet, original_rtp_packet, rtp_len));
 
     EXPECT_TRUE(f2_.ProtectRtp(rtp_packet, rtp_len,
-                               sizeof(rtp_packet), &out_len));
+                               rtp_buffer.size(), &out_len));
     EXPECT_EQ(out_len, rtp_len + rtp_auth_tag_len(cs2));
     EXPECT_NE(0, memcmp(rtp_packet, original_rtp_packet, rtp_len));
     EXPECT_TRUE(f1_.UnprotectRtp(rtp_packet, out_len, &out_len));
     EXPECT_EQ(rtp_len, out_len);
     EXPECT_EQ(0, memcmp(rtp_packet, original_rtp_packet, rtp_len));
 
     EXPECT_TRUE(f1_.ProtectRtcp(rtcp_packet, rtcp_len,
-                                sizeof(rtcp_packet), &out_len));
+                                rtcp_buffer.size(), &out_len));
     EXPECT_EQ(out_len, rtcp_len + 4 + rtcp_auth_tag_len(cs1));  // NOLINT
     EXPECT_NE(0, memcmp(rtcp_packet, kRtcpReport, rtcp_len));
     EXPECT_TRUE(f2_.UnprotectRtcp(rtcp_packet, out_len, &out_len));
     EXPECT_EQ(rtcp_len, out_len);
     EXPECT_EQ(0, memcmp(rtcp_packet, kRtcpReport, rtcp_len));
 
     EXPECT_TRUE(f2_.ProtectRtcp(rtcp_packet, rtcp_len,
-                                sizeof(rtcp_packet), &out_len));
+                                rtcp_buffer.size(), &out_len));
     EXPECT_EQ(out_len, rtcp_len + 4 + rtcp_auth_tag_len(cs2));  // NOLINT
     EXPECT_NE(0, memcmp(rtcp_packet, kRtcpReport, rtcp_len));
     EXPECT_TRUE(f1_.UnprotectRtcp(rtcp_packet, out_len, &out_len));
     EXPECT_EQ(rtcp_len, out_len);
     EXPECT_EQ(0, memcmp(rtcp_packet, kRtcpReport, rtcp_len));
   }
   cricket::SrtpFilter f1_;
   cricket::SrtpFilter f2_;
   int sequence_number_;
 };
@@ skipping 413 matching lines @@
                                 kTestKeyLen, rtc::SRTP_AES128_CM_SHA1_32,
                                 kTestKey2, kTestKeyLen));
   EXPECT_TRUE(f2_.SetRtcpParams(rtc::SRTP_AES128_CM_SHA1_32, kTestKey2,
                                 kTestKeyLen, rtc::SRTP_AES128_CM_SHA1_32,
                                 kTestKey1, kTestKeyLen));
   EXPECT_TRUE(f1_.IsActive());
   EXPECT_TRUE(f2_.IsActive());
   TestProtectUnprotect(CS_AES_CM_128_HMAC_SHA1_32, CS_AES_CM_128_HMAC_SHA1_32);
 }
 
+// Test directly setting the params with SRTP_AEAD_AES_128_GCM

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

nit: Period at end of comment.

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

Done.

+TEST_F(SrtpFilterTest, TestProtect_SetParamsDirect_SRTP_AEAD_AES_128_GCM) {
+  // GCM should not allow external auth anyway.
+  f1_.DisableAllowExternalAuthForTests(false);
+  f2_.DisableAllowExternalAuthForTests(false);
+  EXPECT_TRUE(f1_.SetRtpParams(rtc::SRTP_AEAD_AES_128_GCM, kTestKeyGcm128_1,
+                               kTestKeyGcm128Len, rtc::SRTP_AEAD_AES_128_GCM,
+                               kTestKeyGcm128_2, kTestKeyGcm128Len));
+  EXPECT_TRUE(f2_.SetRtpParams(rtc::SRTP_AEAD_AES_128_GCM, kTestKeyGcm128_2,
+                               kTestKeyGcm128Len, rtc::SRTP_AEAD_AES_128_GCM,
+                               kTestKeyGcm128_1, kTestKeyGcm128Len));
+  EXPECT_TRUE(f1_.SetRtcpParams(rtc::SRTP_AEAD_AES_128_GCM, kTestKeyGcm128_1,
+                                kTestKeyGcm128Len, rtc::SRTP_AEAD_AES_128_GCM,
+                                kTestKeyGcm128_2, kTestKeyGcm128Len));
+  EXPECT_TRUE(f2_.SetRtcpParams(rtc::SRTP_AEAD_AES_128_GCM, kTestKeyGcm128_2,
+                                kTestKeyGcm128Len, rtc::SRTP_AEAD_AES_128_GCM,
+                                kTestKeyGcm128_1, kTestKeyGcm128Len));
+  EXPECT_TRUE(f1_.IsActive());
+  EXPECT_TRUE(f2_.IsActive());
+  TestProtectUnprotect(CS_AEAD_AES_128_GCM, CS_AEAD_AES_128_GCM);
+}
+
+// Test directly setting the params with SRTP_AEAD_AES_256_GCM
+TEST_F(SrtpFilterTest, TestProtect_SetParamsDirect_SRTP_AEAD_AES_256_GCM) {
+  // GCM should not allow external auth anyway.
+  f1_.DisableAllowExternalAuthForTests(false);
+  f2_.DisableAllowExternalAuthForTests(false);
+  EXPECT_TRUE(f1_.SetRtpParams(rtc::SRTP_AEAD_AES_256_GCM, kTestKeyGcm256_1,
+                               kTestKeyGcm256Len, rtc::SRTP_AEAD_AES_256_GCM,
+                               kTestKeyGcm256_2, kTestKeyGcm256Len));
+  EXPECT_TRUE(f2_.SetRtpParams(rtc::SRTP_AEAD_AES_256_GCM, kTestKeyGcm256_2,
+                               kTestKeyGcm256Len, rtc::SRTP_AEAD_AES_256_GCM,
+                               kTestKeyGcm256_1, kTestKeyGcm256Len));
+  EXPECT_TRUE(f1_.SetRtcpParams(rtc::SRTP_AEAD_AES_256_GCM, kTestKeyGcm256_1,
+                                kTestKeyGcm256Len, rtc::SRTP_AEAD_AES_256_GCM,
+                                kTestKeyGcm256_2, kTestKeyGcm256Len));
+  EXPECT_TRUE(f2_.SetRtcpParams(rtc::SRTP_AEAD_AES_256_GCM, kTestKeyGcm256_2,
+                                kTestKeyGcm256Len, rtc::SRTP_AEAD_AES_256_GCM,
+                                kTestKeyGcm256_1, kTestKeyGcm256Len));
+  EXPECT_TRUE(f1_.IsActive());
+  EXPECT_TRUE(f2_.IsActive());
+  TestProtectUnprotect(CS_AEAD_AES_256_GCM, CS_AEAD_AES_256_GCM);
+}
+
 // Test directly setting the params with bogus keys
 TEST_F(SrtpFilterTest, TestSetParamsKeyTooShort) {
   EXPECT_FALSE(f1_.SetRtpParams(rtc::SRTP_AES128_CM_SHA1_80, kTestKey1,
                                 kTestKeyLen - 1, rtc::SRTP_AES128_CM_SHA1_80,
                                 kTestKey1, kTestKeyLen - 1));
   EXPECT_FALSE(f1_.SetRtcpParams(rtc::SRTP_AES128_CM_SHA1_80, kTestKey1,
                                  kTestKeyLen - 1, rtc::SRTP_AES128_CM_SHA1_80,
                                  kTestKey1, kTestKeyLen - 1));
 }
 
-#if defined(ENABLE_EXTERNAL_AUTH)
 TEST_F(SrtpFilterTest, TestGetSendAuthParams) {
+  // We're testing support for external auth, so don't disbale.
+  f1_.DisableAllowExternalAuthForTests(false);

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

Let's see, this is "don't disable allowing external auth"? Would be nice to
avoid a double negative.

+  f2_.DisableAllowExternalAuthForTests(false);
   EXPECT_TRUE(f1_.SetRtpParams(rtc::SRTP_AES128_CM_SHA1_32, kTestKey1,
                                kTestKeyLen, rtc::SRTP_AES128_CM_SHA1_32,
                                kTestKey2, kTestKeyLen));
   EXPECT_TRUE(f1_.SetRtcpParams(rtc::SRTP_AES128_CM_SHA1_32, kTestKey1,
                                 kTestKeyLen, rtc::SRTP_AES128_CM_SHA1_32,
                                 kTestKey2, kTestKeyLen));
+  // Non-GCM ciphers support external auth.
+  EXPECT_TRUE(f1_.AllowExternalAuth());
   uint8_t* auth_key = NULL;
   int auth_key_len = 0, auth_tag_len = 0;
   EXPECT_TRUE(f1_.GetRtpAuthParams(&auth_key, &auth_key_len, &auth_tag_len));
   EXPECT_TRUE(auth_key != NULL);
   EXPECT_EQ(20, auth_key_len);
   EXPECT_EQ(4, auth_tag_len);
 }
-#endif
 
 class SrtpSessionTest : public testing::Test {
  protected:
   virtual void SetUp() {
+    s1_.DisableAllowExternalAuthForTests(true);
+    s2_.DisableAllowExternalAuthForTests(true);
     rtp_len_ = sizeof(kPcmuFrame);
     rtcp_len_ = sizeof(kRtcpReport);
     memcpy(rtp_packet_, kPcmuFrame, rtp_len_);
     memcpy(rtcp_packet_, kRtcpReport, rtcp_len_);
   }
   void TestProtectRtp(const std::string& cs) {
     int out_len = 0;
     EXPECT_TRUE(s1_.ProtectRtp(rtp_packet_, rtp_len_,
                                sizeof(rtp_packet_), &out_len));
     EXPECT_EQ(out_len, rtp_len_ + rtp_auth_tag_len(cs));
@@ skipping 329 matching lines @@
   srtp_stat_.AddUnprotectRtcpResult(srtp_err_status_fail);
   EXPECT_EQ(-1, mode_);
   EXPECT_EQ(cricket::SrtpFilter::ERROR_NONE, error_);
   // Now the error will be triggered again.
   Reset();
   rtc::Thread::Current()->SleepMs(210);
   srtp_stat_.AddUnprotectRtcpResult(srtp_err_status_fail);
   EXPECT_EQ(cricket::SrtpFilter::UNPROTECT, mode_);
   EXPECT_EQ(cricket::SrtpFilter::ERROR_FAIL, error_);
 }