Completed the functionalities of SrtpTransport.

webrtc/pc/srtptransport_unittest.cc

Index: webrtc/pc/srtptransport_unittest.cc
diff --git a/webrtc/pc/srtptransport_unittest.cc b/webrtc/pc/srtptransport_unittest.cc
index e54dac3ea29dab437e636365ce71f69246366578..ea57bedfd5b15ba44739986d6dd27ff53ab84dc0 100644
--- a/webrtc/pc/srtptransport_unittest.cc
+++ b/webrtc/pc/srtptransport_unittest.cc
@@ -10,67 +10,366 @@
 
 #include "webrtc/pc/srtptransport.h"
 
+#include "webrtc/media/base/fakertp.h"
+#include "webrtc/p2p/base/dtlstransportinternal.h"
+#include "webrtc/p2p/base/fakepackettransport.h"
 #include "webrtc/pc/rtptransport.h"
 #include "webrtc/pc/rtptransporttestutil.h"
+#include "webrtc/pc/srtptestutil.h"
 #include "webrtc/rtc_base/asyncpacketsocket.h"
 #include "webrtc/rtc_base/gunit.h"
 #include "webrtc/rtc_base/ptr_util.h"
-#include "webrtc/test/gmock.h"
+#include "webrtc/rtc_base/sslstreamadapter.h"
 
-namespace webrtc {
+using rtc::kTestKey1;
+using rtc::kTestKey2;
+using rtc::kTestKeyLen;
+using rtc::SRTP_AEAD_AES_128_GCM;
 
-using testing::_;
-using testing::Return;
+namespace webrtc {
+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.
 
-class MockRtpTransport : public RtpTransport {
+class FakeRtpTransport : public RtpTransport {
  public:
-  MockRtpTransport() : RtpTransport(true) {}
+  FakeRtpTransport() : RtpTransport(true) {
+    rtp_packet_transport_ =
+        rtc::MakeUnique<rtc::FakePacketTransport>("FakePacketTransport");
+    SetRtpPacketTransport(rtp_packet_transport_.get());
+  }
 
-  MOCK_METHOD4(SendPacket,
-               bool(bool rtcp,
-                    rtc::CopyOnWriteBuffer* packet,
-                    const rtc::PacketOptions& options,
-                    int flags));
+  void Connect(FakeRtpTransport* peer) { peer_ = peer; }
 
-  void PretendReceivedPacket() {
-    bool rtcp = false;
-    rtc::CopyOnWriteBuffer buffer;
-    rtc::PacketTime time;
-    SignalPacketReceived(rtcp, &buffer, time);
+  bool SendPacket(bool rtcp,
+                  rtc::CopyOnWriteBuffer* packet,
+                  const rtc::PacketOptions& options,
+                  int flags) override {
+    if (peer_) {
+      peer_->set_last_recv_encrypted_packet(packet);
+      peer_->SignalPacketReceived(rtcp, packet, rtc::PacketTime());
+    }
+    return true;
+  }
+
+  void set_last_recv_encrypted_packet(rtc::CopyOnWriteBuffer* packet) {
+    rtp_packet_transport_->set_last_received_packet(packet);
   }
+
+ private:
+  FakeRtpTransport* peer_ = nullptr;
+  std::unique_ptr<rtc::FakePacketTransport> rtp_packet_transport_;
 };
 
-TEST(SrtpTransportTest, SendPacket) {
-  auto rtp_transport = rtc::MakeUnique<MockRtpTransport>();
-  EXPECT_CALL(*rtp_transport, SendPacket(_, _, _, _)).WillOnce(Return(true));
+class SrtpTransportTest : public testing::Test, public sigslot::has_slots<> {
+ protected:
+  SrtpTransportTest() {
+    auto rtp_transport1 = rtc::MakeUnique<FakeRtpTransport>();
+    auto rtp_transport2 = rtc::MakeUnique<FakeRtpTransport>();
+    rtp_transport1->Connect(rtp_transport2.get());
+    rtp_transport2->Connect(rtp_transport1.get());
+
+    srtp_transport1_ =
+        rtc::MakeUnique<SrtpTransport>(std::move(rtp_transport1), "content");
+    srtp_transport2_ =
+        rtc::MakeUnique<SrtpTransport>(std::move(rtp_transport2), "content");
+
+    srtp_transport1_->SignalPacketReceived.connect(
+        this, &SrtpTransportTest::OnPacketReceived1);
+    srtp_transport2_->SignalPacketReceived.connect(
+        this, &SrtpTransportTest::OnPacketReceived2);
+  }
+
+  void OnPacketReceived1(bool rtcp,
+                         rtc::CopyOnWriteBuffer* packet,
+                         const rtc::PacketTime& packet_time) {
+    LOG(LS_INFO) << "SrtpTransport1 Received a packet.";
+    last_recv_packet1_ = *packet;
+  }
+
+  void OnPacketReceived2(bool rtcp,
+                         rtc::CopyOnWriteBuffer* packet,
+                         const rtc::PacketTime& packet_time) {
+    LOG(LS_INFO) << "SrtpTransport2 Received a packet.";
+    last_recv_packet2_ = *packet;
+  }
+
+  void TestSendRecvRtpPacket(const std::string& cipher_suite_name) {
+    bool rtcp = false;
+
+    int rtp_len = sizeof(kPcmuFrame);
+    int packet_size = rtp_len + rtc::rtp_auth_tag_len(cipher_suite_name);
+    char rtp_packet_data[packet_size];
+    memcpy(rtp_packet_data, 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_data) + 2,
+                 ++sequence_number_);
+    rtc::CopyOnWriteBuffer rtp_packet(rtp_packet_data, rtp_len, packet_size);
+
+    char original_rtp_data[rtp_len];
+    memcpy(original_rtp_data, rtp_packet_data, rtp_len);
 
-  SrtpTransport srtp_transport(std::move(rtp_transport), "a");
+    rtc::PacketOptions options;
+    int flags = cricket::PF_NORMAL;
+    // Send a packet from |srtp_transport1_| to |srtp_transport2_| and verify
+    // that the packet can be successfully received and decrypted.
+    ASSERT_TRUE(
+        srtp_transport1_->SendPacket(rtcp, &rtp_packet, options, flags));
+    ASSERT_TRUE(last_recv_packet2_.data());
+    EXPECT_TRUE(memcmp(last_recv_packet2_.data(), original_rtp_data, rtp_len) ==
+                0);
+    // Get the encrypted packet from underneath packet transport and verify the
+    // data is actually encrypted.
+    auto fake_rtp_packet_transport = static_cast<rtc::FakePacketTransport*>(
+        srtp_transport2_->rtp_packet_transport());
+    EXPECT_FALSE(
+        memcmp(fake_rtp_packet_transport->last_received_packet()->data(),
+               original_rtp_data, rtp_len) == 0);
 
-  const bool rtcp = false;
-  rtc::CopyOnWriteBuffer packet;
-  rtc::PacketOptions options;
-  int flags = 0;
-  EXPECT_TRUE(srtp_transport.SendPacket(rtcp, &packet, options, flags));
+    // Do the same thing in the opposite direction;
+    ASSERT_TRUE(
+        srtp_transport2_->SendPacket(rtcp, &rtp_packet, options, flags));
+    ASSERT_TRUE(last_recv_packet1_.data());
+    EXPECT_TRUE(memcmp(last_recv_packet1_.data(), original_rtp_data, rtp_len) ==
+                0);
+    fake_rtp_packet_transport = static_cast<rtc::FakePacketTransport*>(
+        srtp_transport1_->rtp_packet_transport());
+    EXPECT_FALSE(
+        memcmp(fake_rtp_packet_transport->last_received_packet()->data(),
+               original_rtp_data, rtp_len) == 0);
+  }
+
+  void TestSendRecvRtcpPacket(const std::string& cipher_suite_name) {
+    bool rtcp = true;
+
+    int rtcp_len = sizeof(kRtcpReport);
+    int packet_size = rtcp_len + 4 + rtc::rtcp_auth_tag_len(cipher_suite_name);
+    char rtcp_packet_data[packet_size];
+    memcpy(rtcp_packet_data, kRtcpReport, rtcp_len);
 
-  // TODO(zstein): Also verify that the packet received by RtpTransport has been
-  // protected once SrtpTransport handles that.
+    rtc::CopyOnWriteBuffer rtcp_packet(rtcp_packet_data, rtcp_len, packet_size);
+
+    rtc::PacketOptions options;
+    int flags = cricket::PF_NORMAL;
+    // Send a packet from |srtp_transport1_| to |srtp_transport2_| and verify
+    // that the packet can be successfully received and decrypted.
+    ASSERT_TRUE(
+        srtp_transport1_->SendPacket(rtcp, &rtcp_packet, options, flags));
+    ASSERT_TRUE(last_recv_packet2_.data());
+    EXPECT_TRUE(memcmp(last_recv_packet2_.data(), rtcp_packet_data, rtcp_len) ==
+                0);
+    // Get the encrypted packet from underneath packet transport and verify the
+    // data is actually encrypted.
+    auto fake_rtp_packet_transport = static_cast<rtc::FakePacketTransport*>(
+        srtp_transport2_->rtp_packet_transport());
+    EXPECT_FALSE(
+        memcmp(fake_rtp_packet_transport->last_received_packet()->data(),
+               rtcp_packet_data, rtcp_len) == 0);
+
+    // Do the same thing in the opposite direction;
+    ASSERT_TRUE(
+        srtp_transport2_->SendPacket(rtcp, &rtcp_packet, options, flags));
+    ASSERT_TRUE(last_recv_packet1_.data());
+    EXPECT_TRUE(memcmp(last_recv_packet1_.data(), rtcp_packet_data, rtcp_len) ==
+                0);
+    fake_rtp_packet_transport = static_cast<rtc::FakePacketTransport*>(
+        srtp_transport1_->rtp_packet_transport());
+    EXPECT_FALSE(
+        memcmp(fake_rtp_packet_transport->last_received_packet()->data(),
+               rtcp_packet_data, rtcp_len) == 0);
+  }
+
+  void TestSendRecvPacket(bool enable_external_auth,
+                          int cs,
+                          const uint8_t* key1,
+                          int key1_len,
+                          const uint8_t* key2,
+                          int key2_len,
+                          const std::string& cipher_suite_name) {
+    EXPECT_EQ(key1_len, key2_len);
+    EXPECT_EQ(cipher_suite_name, rtc::SrtpCryptoSuiteToName(cs));
+    if (enable_external_auth) {
+      srtp_transport1_->EnableExternalAuth();
+      srtp_transport2_->EnableExternalAuth();
+    }
+    EXPECT_TRUE(
+        srtp_transport1_->SetRtpParams(cs, key1, key1_len, cs, key2, key2_len));
+    EXPECT_TRUE(
+        srtp_transport2_->SetRtpParams(cs, key2, key2_len, cs, key1, key1_len));
+    EXPECT_TRUE(srtp_transport1_->SetRtcpParams(cs, key1, key1_len, cs, key2,
+                                                key2_len));
+    EXPECT_TRUE(srtp_transport2_->SetRtcpParams(cs, key2, key2_len, cs, key1,
+                                                key1_len));
+    EXPECT_TRUE(srtp_transport1_->IsActive());
+    EXPECT_TRUE(srtp_transport2_->IsActive());
+    if (rtc::IsGcmCryptoSuite(cs)) {
+      EXPECT_FALSE(srtp_transport1_->IsExternalAuthActive());
+      EXPECT_FALSE(srtp_transport2_->IsExternalAuthActive());
+    } else if (enable_external_auth) {
+      EXPECT_TRUE(srtp_transport1_->IsExternalAuthActive());
+      EXPECT_TRUE(srtp_transport2_->IsExternalAuthActive());
+    }
+    TestSendRecvRtpPacket(cipher_suite_name);
+    TestSendRecvRtcpPacket(cipher_suite_name);
+  }
+
+  void TestSendRecvPacketWithEncryptedHeaderExtension(
+      const std::string& cs,
+      const std::vector<int>& encrypted_header_ids) {
+    bool rtcp = false;
+
+    int rtp_len = sizeof(kPcmuFrameWithExtensions);
+    int packet_size = rtp_len + rtc::rtp_auth_tag_len(cs);
+    char rtp_packet_data[packet_size];
+    memcpy(rtp_packet_data, kPcmuFrameWithExtensions, 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_data) + 2,
+                 ++sequence_number_);
+    rtc::CopyOnWriteBuffer rtp_packet(rtp_packet_data, rtp_len, packet_size);
+
+    char original_rtp_data[rtp_len];
+    memcpy(original_rtp_data, rtp_packet_data, rtp_len);
+
+    rtc::PacketOptions options;
+    int flags = cricket::PF_NORMAL;
+    // Send a packet from |srtp_transport1_| to |srtp_transport2_| and verify
+    // that the packet can be successfully received and decrypted.
+    ASSERT_TRUE(
+        srtp_transport1_->SendPacket(rtcp, &rtp_packet, options, flags));
+    ASSERT_TRUE(last_recv_packet2_.data());
+    EXPECT_TRUE(memcmp(last_recv_packet2_.data(), original_rtp_data, rtp_len) ==
+                0);
+    // Get the encrypted packet from underneath packet transport and verify the
+    // data and header extension are actually encrypted.
+    auto fake_rtp_packet_transport = static_cast<rtc::FakePacketTransport*>(
+        srtp_transport2_->rtp_packet_transport());
+    EXPECT_FALSE(
+        memcmp(fake_rtp_packet_transport->last_received_packet()->data(),
+               original_rtp_data, rtp_len) == 0);
+    CompareHeaderExtensions(
+        reinterpret_cast<const char*>(
+            fake_rtp_packet_transport->last_received_packet()->data()),
+        fake_rtp_packet_transport->last_received_packet()->size(),
+        original_rtp_data, rtp_len, encrypted_header_ids, false);
+
+    // Do the same thing in the opposite direction;
+    ASSERT_TRUE(
+        srtp_transport2_->SendPacket(rtcp, &rtp_packet, options, flags));
+    ASSERT_TRUE(last_recv_packet1_.data());
+    EXPECT_TRUE(memcmp(last_recv_packet1_.data(), original_rtp_data, rtp_len) ==
+                0);
+    fake_rtp_packet_transport = static_cast<rtc::FakePacketTransport*>(
+        srtp_transport1_->rtp_packet_transport());
+    EXPECT_FALSE(
+        memcmp(fake_rtp_packet_transport->last_received_packet()->data(),
+               original_rtp_data, rtp_len) == 0);
+    CompareHeaderExtensions(
+        reinterpret_cast<const char*>(
+            fake_rtp_packet_transport->last_received_packet()->data()),
+        fake_rtp_packet_transport->last_received_packet()->size(),
+        original_rtp_data, rtp_len, encrypted_header_ids, false);
+  }
+
+  void TestSendRecvEncryptedHeaderExtension(int cs,
+                                            const uint8_t* key1,
+                                            int key1_len,
+                                            const uint8_t* key2,
+                                            int key2_len,
+                                            const std::string& cs_name) {
+    std::vector<int> encrypted_headers;
+    encrypted_headers.push_back(1);
+    // Don't encrypt header ids 2 and 3.
+    encrypted_headers.push_back(4);
+    EXPECT_EQ(key1_len, key2_len);
+    EXPECT_EQ(cs_name, rtc::SrtpCryptoSuiteToName(cs));
+    srtp_transport1_->SetEncryptedHeaderExtensionIds(cricket::CS_LOCAL,
+                                                     encrypted_headers);
+    srtp_transport1_->SetEncryptedHeaderExtensionIds(cricket::CS_REMOTE,
+                                                     encrypted_headers);
+    srtp_transport2_->SetEncryptedHeaderExtensionIds(cricket::CS_LOCAL,
+                                                     encrypted_headers);
+    srtp_transport2_->SetEncryptedHeaderExtensionIds(cricket::CS_REMOTE,
+                                                     encrypted_headers);
+    EXPECT_TRUE(
+        srtp_transport1_->SetRtpParams(cs, key1, key1_len, cs, key2, key2_len));
+    EXPECT_TRUE(
+        srtp_transport2_->SetRtpParams(cs, key2, key2_len, cs, key1, key1_len));
+    EXPECT_TRUE(srtp_transport1_->IsActive());
+    EXPECT_TRUE(srtp_transport2_->IsActive());
+    EXPECT_FALSE(srtp_transport1_->IsExternalAuthActive());
+    EXPECT_FALSE(srtp_transport2_->IsExternalAuthActive());
+    TestSendRecvPacketWithEncryptedHeaderExtension(cs_name, encrypted_headers);
+  }
+
+  std::unique_ptr<SrtpTransport> srtp_transport1_;
+  std::unique_ptr<SrtpTransport> srtp_transport2_;
+
+  rtc::CopyOnWriteBuffer last_recv_packet1_;
+  rtc::CopyOnWriteBuffer last_recv_packet2_;
+  int sequence_number_ = 0;
+};
+
+TEST_F(SrtpTransportTest, SendAndRecvPacket_AES_CM_128_HMAC_SHA1_80) {
+  bool enable_external_auth = false;
+  TestSendRecvPacket(enable_external_auth, rtc::SRTP_AES128_CM_SHA1_80,
+                     kTestKey1, kTestKeyLen, kTestKey2, kTestKeyLen,
+                     rtc::CS_AES_CM_128_HMAC_SHA1_80);
 }
 
-// Test that SrtpTransport fires SignalPacketReceived when the underlying
-// RtpTransport fires SignalPacketReceived.
-TEST(SrtpTransportTest, SignalPacketReceived) {
-  auto rtp_transport = rtc::MakeUnique<MockRtpTransport>();
-  MockRtpTransport* rtp_transport_raw = rtp_transport.get();
-  SrtpTransport srtp_transport(std::move(rtp_transport), "a");
+TEST_F(SrtpTransportTest,
+       SendAndRecvPacketWithHeaderExtension_AES_CM_128_HMAC_SHA1_80) {
+  TestSendRecvEncryptedHeaderExtension(rtc::SRTP_AES128_CM_SHA1_80, kTestKey1,
+                                       kTestKeyLen, kTestKey2, kTestKeyLen,
+                                       rtc::CS_AES_CM_128_HMAC_SHA1_80);
+}
 
-  SignalPacketReceivedCounter counter(&srtp_transport);
+TEST_F(SrtpTransportTest, SendAndRecvPacket_AES_CM_128_HMAC_SHA1_32) {
+  bool enable_external_auth = false;
+  TestSendRecvPacket(enable_external_auth, rtc::SRTP_AES128_CM_SHA1_32,
+                     kTestKey1, kTestKeyLen, kTestKey2, kTestKeyLen,
+                     rtc::CS_AES_CM_128_HMAC_SHA1_32);
+}
 
-  rtp_transport_raw->PretendReceivedPacket();
+TEST_F(SrtpTransportTest,
+       SendAndRecvPacketWithHeaderExtension_AES_CM_128_HMAC_SHA1_32) {
+  TestSendRecvEncryptedHeaderExtension(rtc::SRTP_AES128_CM_SHA1_32, kTestKey1,
+                                       kTestKeyLen, kTestKey2, kTestKeyLen,
+                                       rtc::CS_AES_CM_128_HMAC_SHA1_32);
+}
 
-  EXPECT_EQ(1, counter.rtp_count());
+TEST_F(SrtpTransportTest, SendAndRecvPacket_SRTP_AEAD_AES_128_GCM) {
+  bool enable_external_auth = false;
+  TestSendRecvPacket(enable_external_auth, rtc::SRTP_AEAD_AES_128_GCM,
+                     kTestKeyGcm128_1, kTestKeyGcm128Len, kTestKeyGcm128_2,
+                     kTestKeyGcm128Len, rtc::CS_AEAD_AES_128_GCM);
+}
+
+TEST_F(SrtpTransportTest,
+       SendAndRecvPacketWithHeaderExtension_SRTP_AEAD_AES_128_GCM) {
+  TestSendRecvEncryptedHeaderExtension(
+      rtc::SRTP_AEAD_AES_128_GCM, kTestKeyGcm128_1, kTestKeyGcm128Len,
+      kTestKeyGcm128_2, kTestKeyGcm128Len, rtc::CS_AEAD_AES_128_GCM);
+}
+
+TEST_F(SrtpTransportTest, SendAndRecvPacket_SRTP_AEAD_AES_256_GCM) {
+  bool enable_external_auth = false;

[Taylor Brandstetter, 2017/08/23 22:13:30]

These tests aren't using external auth, like the tests they're replacing are. To
make sure we don't lose test coverage, they should test it.

Some background about what "external auth" is: as you probably know, in
chromium, packets are actually delivered to the OS in a separate process, which
means the timestamps webrtc puts in them (in the "abs-send-time" extension) may
be inaccurate. More accurate send timestamps improve the performance of the
bandwidth estimator, so we want to fill in this timestamp right before the
packet is sent, in the browser process. But that also means the HMAC needs to be
recalculated, since it's computed from the full contents of the packet including
header extensions.

This rewriting happens in "cricket::ApplyPacketOptions", called here:
https://cs.chromium.org/chromium/src/content/browser/renderer_host/p2p/socket...

The packet options passed along everywhere include "PacketTimeUpdateParams",
which includes just enough information to do this rewriting of the relevant
bits. Though it doesn't work for GCM ciphers.

So, how does it relate to these tests? In these tests, there's nothing that
actually does this rewriting. So we'd need to add it; FakeRtpTransport could do
it I think. Or we could just do what the old tests do, and do
"TestRtpAuthParams" instead of actually trying to decode the packet. But testing
it for real would be preferable.

[Zhi Huang, 2017/08/24 23:38:07]

Thanks for the detailed explanation! Done. 

+  TestSendRecvPacket(enable_external_auth, rtc::SRTP_AEAD_AES_256_GCM,
+                     kTestKeyGcm256_1, kTestKeyGcm256Len, kTestKeyGcm256_2,
+                     kTestKeyGcm256Len, rtc::CS_AEAD_AES_256_GCM);
+}
 
-  // TODO(zstein): Also verify that the packet is unprotected once SrtpTransport
-  // handles that.
+TEST_F(SrtpTransportTest,
+       SendAndRecvPacketWithHeaderExtension_SRTP_AEAD_AES_256_GCM) {
+  TestSendRecvEncryptedHeaderExtension(
+      rtc::SRTP_AEAD_AES_256_GCM, kTestKeyGcm256_1, kTestKeyGcm256Len,
+      kTestKeyGcm256_2, kTestKeyGcm256Len, rtc::CS_AEAD_AES_256_GCM);
 }
 
 }  // namespace webrtc