Bug 1194010 - Block webrtc socket creation on restricted ports r=drno
authorPaul Vitale <paul.m.vitale@gmail.com>
Thu, 29 Nov 2018 15:40:44 +0000
changeset 505179 f56dfb58f1c9a0e632df1448eb46f5bf30a2c70b
parent 505178 2b2777f72b09079f43018c04b1cbed49b959dff1
child 505180 15fb49ca9d9f1a37dfffa9af03535c40f9a4c7e0
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdrno
bugs1194010
milestone65.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1194010 - Block webrtc socket creation on restricted ports r=drno Port restriction is delegated to the io service. Port 0 is explicitly unrestricted. NS_CheckPortSafety emits a warning which pollutes the gtests a bit. It is only tested if the port is not 0. Differential Revision: https://phabricator.services.mozilla.com/D13187
media/mtransport/nr_socket_prsock.cpp
media/mtransport/nr_socket_prsock.h
media/mtransport/test/test_nr_socket_unittest.cpp
--- a/media/mtransport/nr_socket_prsock.cpp
+++ b/media/mtransport/nr_socket_prsock.cpp
@@ -111,16 +111,17 @@ nrappkit copyright:
 #include "nsTArray.h"
 #include "mozilla/dom/TCPSocketBinding.h"
 #include "mozilla/SystemGroup.h"
 #include "nsITCPSocketCallback.h"
 #include "nsIPrefService.h"
 #include "nsIPrefBranch.h"
 #include "nsISocketFilter.h"
 #include "nsDebug.h"
+#include "nsNetUtil.h"
 
 #ifdef XP_WIN
 #include "mozilla/WindowsVersion.h"
 #endif
 
 #if defined(MOZILLA_INTERNAL_API)
 // csi_platform.h deep in nrappkit defines LOG_INFO and LOG_WARNING
 #ifdef LOG_INFO
@@ -2155,16 +2156,20 @@ static nr_socket_vtbl nr_socket_local_vt
 /* static */
 int
 NrSocketBase::CreateSocket(nr_transport_addr *addr,
                            RefPtr<NrSocketBase> *sock,
                            const std::shared_ptr<NrSocketProxyConfig>& config)
 {
   int r, _status;
 
+  if (IsForbiddenAddress(addr)) {
+    ABORT(R_REJECTED);
+  }
+
   // create IPC bridge for content process
   if (XRE_IsParentProcess()) {
     *sock = new NrSocket();
   } else {
     switch (addr->protocol) {
       case IPPROTO_UDP:
         *sock = new NrUdpSocketIpc();
         break;
@@ -2191,16 +2196,36 @@ NrSocketBase::CreateSocket(nr_transport_
   _status = 0;
 abort:
   if (_status) {
     *sock = nullptr;
   }
   return _status;
 }
 
+// static
+bool NrSocketBase::IsForbiddenAddress(nr_transport_addr *addr) {
+  int r, port;
+
+  r = nr_transport_addr_get_port(addr, &port);
+  if (r) {
+    return true;
+  }
+
+  // allow auto assigned ports
+  if (port != 0) {
+    // Don't need to check an override scheme
+    nsresult rv = NS_CheckPortSafety(port, nullptr);
+    if(NS_FAILED(rv)) {
+      return true;
+    }
+  }
+
+  return false;
+}
 
 static int nr_socket_local_destroy(void **objp) {
   if(!objp || !*objp)
     return 0;
 
   NrSocketBase *sock = static_cast<NrSocketBase *>(*objp);
   *objp = nullptr;
 
--- a/media/mtransport/nr_socket_prsock.h
+++ b/media/mtransport/nr_socket_prsock.h
@@ -98,16 +98,17 @@ public:
   }
   virtual ~NrSocketBase() {}
 
   // Factory method; will create either an NrSocket, NrUdpSocketIpc, or
   // NrTcpSocketIpc as appropriate.
   static int CreateSocket(nr_transport_addr *addr,
                           RefPtr<NrSocketBase> *sock,
                           const std::shared_ptr<NrSocketProxyConfig>& config);
+  static bool IsForbiddenAddress(nr_transport_addr *addr);
 
   // the nr_socket APIs
   virtual int create(nr_transport_addr *addr) = 0;
   virtual int sendto(const void *msg, size_t len,
                      int flags, nr_transport_addr *to) = 0;
   virtual int recvfrom(void * buf, size_t maxlen,
                        size_t *len, int flags,
                        nr_transport_addr *from) = 0;
--- a/media/mtransport/test/test_nr_socket_unittest.cpp
+++ b/media/mtransport/test/test_nr_socket_unittest.cpp
@@ -457,16 +457,57 @@ class TestNrSocketTest : public Mtranspo
   std::vector<RefPtr<TestNrSocket>> private_addrs_;
   std::vector<RefPtr<TestNat>> nats_;
 };
 
 } // namespace mozilla
 
 using mozilla::TestNrSocketTest;
 using mozilla::TestNat;
+using mozilla::NrSocketBase;
+
+TEST_F(TestNrSocketTest, UnsafePortRejectedUDP) {
+  nr_transport_addr address;
+  ASSERT_FALSE(nr_str_port_to_transport_addr("127.0.0.1",
+                                             // ssh
+                                             22,
+                                             IPPROTO_UDP,
+                                             &address));
+  ASSERT_TRUE(NrSocketBase::IsForbiddenAddress(&address));
+}
+
+TEST_F(TestNrSocketTest, UnsafePortRejectedTCP) {
+  nr_transport_addr address;
+  ASSERT_FALSE(nr_str_port_to_transport_addr("127.0.0.1",
+                                             // ssh
+                                             22,
+                                             IPPROTO_TCP,
+                                             &address));
+  ASSERT_TRUE(NrSocketBase::IsForbiddenAddress(&address));
+}
+
+TEST_F(TestNrSocketTest, SafePortAcceptedUDP) {
+  nr_transport_addr address;
+  ASSERT_FALSE(nr_str_port_to_transport_addr("127.0.0.1",
+                                             // stuns
+                                             5349,
+                                             IPPROTO_UDP,
+                                             &address));
+  ASSERT_FALSE(NrSocketBase::IsForbiddenAddress(&address));
+}
+
+TEST_F(TestNrSocketTest, SafePortAcceptedTCP) {
+  nr_transport_addr address;
+  ASSERT_FALSE(nr_str_port_to_transport_addr("127.0.0.1",
+                                             // turns
+                                             5349,
+                                             IPPROTO_TCP,
+                                             &address));
+  ASSERT_FALSE(NrSocketBase::IsForbiddenAddress(&address));
+}
 
 TEST_F(TestNrSocketTest, PublicConnectivity) {
   CreatePublicAddrs(2);
 
   ASSERT_TRUE(CheckConnectivity(public_addrs_[0], public_addrs_[1]));
   ASSERT_TRUE(CheckConnectivity(public_addrs_[1], public_addrs_[0]));
   ASSERT_TRUE(CheckConnectivity(public_addrs_[0], public_addrs_[0]));
   ASSERT_TRUE(CheckConnectivity(public_addrs_[1], public_addrs_[1]));