From 46d2457debb617b5ff4dfdbe61e3c637b9fb787b Mon Sep 17 00:00:00 2001
From: jbauch <jbauch@webrtc.org>
Date: Fri, 10 Mar 2017 16:20:04 -0800
Subject: [PATCH] Fixed invalid filtering of SCTP datachannel packets on high
 ports.

Packets on source ports 32768-49151 got identified as RTP packets by
"IsRtpPacket" and were ignored by the SCTP transport.

This CL changes this to check the packet flags for "PF_SRTP_BYPASS".

BUG=webrtc:6959

Review-Url: https://codereview.webrtc.org/2743653005
Cr-Commit-Position: refs/heads/master@{#17179}
---
 webrtc/media/sctp/sctptransport.cc          |  6 ++----
 webrtc/media/sctp/sctptransport_unittest.cc | 22 +++++++++++++++++++--
 2 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/webrtc/media/sctp/sctptransport.cc b/webrtc/media/sctp/sctptransport.cc
index ae69628a98..45e8decb38 100644
--- a/webrtc/media/sctp/sctptransport.cc
+++ b/webrtc/media/sctp/sctptransport.cc
@@ -37,7 +37,6 @@ enum PreservedErrno {
 #include "webrtc/base/trace_event.h"
 #include "webrtc/media/base/codec.h"
 #include "webrtc/media/base/mediaconstants.h"
-#include "webrtc/media/base/rtputils.h"  // For IsRtpPacket
 #include "webrtc/media/base/streamparams.h"
 #include "webrtc/p2p/base/dtlstransportinternal.h"  // For PF_NORMAL
 
@@ -828,9 +827,8 @@ void SctpTransport::OnPacketRead(rtc::PacketTransportInternal* transport,
   RTC_DCHECK_EQ(transport_channel_, transport);
   TRACE_EVENT0("webrtc", "SctpTransport::OnPacketRead");
 
-  // TODO(pthatcher): Do this in a more robust way by checking for
-  // SCTP or DTLS.
-  if (IsRtpPacket(data, len)) {
+  if (flags & PF_SRTP_BYPASS) {
+    // We are only interested in SCTP packets.
     return;
   }
 
diff --git a/webrtc/media/sctp/sctptransport_unittest.cc b/webrtc/media/sctp/sctptransport_unittest.cc
index 7fcbd9ce8e..b8495d081a 100644
--- a/webrtc/media/sctp/sctptransport_unittest.cc
+++ b/webrtc/media/sctp/sctptransport_unittest.cc
@@ -130,6 +130,10 @@ class SctpTransportTest : public testing::Test, public sigslot::has_slots<> {
   static void SetUpTestCase() {}
 
   void SetupConnectedTransportsWithTwoStreams() {
+    SetupConnectedTransportsWithTwoStreams(kTransport1Port, kTransport2Port);
+  }
+
+  void SetupConnectedTransportsWithTwoStreams(int port1, int port2) {
     fake_dtls1_.reset(new FakeDtlsTransport("fake dtls 1", 0));
     fake_dtls2_.reset(new FakeDtlsTransport("fake dtls 2", 0));
     recv1_.reset(new SctpFakeDataReceiver());
@@ -153,8 +157,8 @@ class SctpTransportTest : public testing::Test, public sigslot::has_slots<> {
     LOG(LS_VERBOSE) << "Connect the transports -----------------------------";
     // Both transports need to have started (with matching ports) for an
     // association to be formed.
-    transport1_->Start(kTransport1Port, kTransport2Port);
-    transport2_->Start(kTransport2Port, kTransport1Port);
+    transport1_->Start(port1, port2);
+    transport2_->Start(port2, port1);
   }
 
   bool AddStream(int sid) {
@@ -451,6 +455,20 @@ TEST_F(SctpTransportTest, SendDataWithNonexistentStreamFails) {
   EXPECT_EQ(SDR_ERROR, result);
 }
 
+TEST_F(SctpTransportTest, SendDataHighPorts) {
+  SetupConnectedTransportsWithTwoStreams(32768, 32769);
+
+  SendDataResult result;
+  ASSERT_TRUE(SendData(transport1(), 1, "hello?", &result));
+  EXPECT_EQ(SDR_SUCCESS, result);
+  EXPECT_TRUE_WAIT(ReceivedData(receiver2(), 1, "hello?"), kDefaultTimeout);
+
+  ASSERT_TRUE(SendData(transport2(), 2, "hi transport1", &result));
+  EXPECT_EQ(SDR_SUCCESS, result);
+  EXPECT_TRUE_WAIT(ReceivedData(receiver1(), 2, "hi transport1"),
+                   kDefaultTimeout);
+}
+
 TEST_F(SctpTransportTest, ClosesRemoteStream) {
   SetupConnectedTransportsWithTwoStreams();
   SignalTransportClosedObserver transport1_sig_receiver,