Destroy a Connection if a CreatePermission request fails.

This means that if a TURN server denies permission for an
unreachable address, we'll no longer ping it fruitlessly.

BUG=webrtc:4917

Review URL: https://codereview.webrtc.org/1415313004

Cr-Commit-Position: refs/heads/master@{#10789}

webrtc/p2p/base/port.cc

@@ -1008,6 +1008,11 @@ void Connection::Destroy() {
   port_->thread()->Post(this, MSG_DELETE);
 }
 
+void Connection::FailAndDestroy() {
+  set_state(Connection::STATE_FAILED);
+  Destroy();
+}
+
 void Connection::PrintPingsSinceLastResponse(std::string* s, size_t max) {
   std::ostringstream oss;
   oss << std::boolalpha;
@@ -1242,8 +1247,7 @@ void Connection::OnConnectionRequestErrorResponse(ConnectionRequest* request,
     // This is not a valid connection.
     LOG_J(LS_ERROR, this) << "Received STUN error response, code="
                           << error_code << "; killing connection";
-    set_state(STATE_FAILED);
-    Destroy();
+    FailAndDestroy();
   }
 }
 
@@ -1390,10 +1394,10 @@ void Connection::MaybeAddPrflxCandidate(ConnectionRequest* request,
   SignalStateChange(this);
 }
 
-ProxyConnection::ProxyConnection(Port* port, size_t index,
-                                 const Candidate& candidate)
-  : Connection(port, index, candidate), error_(0) {
-}
+ProxyConnection::ProxyConnection(Port* port,
+                                 size_t index,
+                                 const Candidate& remote_candidate)
+    : Connection(port, index, remote_candidate) {}
 
 int ProxyConnection::Send(const void* data, size_t size,
                           const rtc::PacketOptions& options) {

webrtc/p2p/base/port.h

@@ -514,6 +514,9 @@ class Connection : public rtc::MessageHandler,
   // Makes the connection go away.
   void Destroy();
 
+  // Makes the connection go away, in a failed state.
+  void FailAndDestroy();
+
   // Checks that the state of this connection is up-to-date.  The argument is
   // the current time, which is compared against various timeouts.
   void UpdateState(uint32_t now);
@@ -634,17 +637,18 @@ class Connection : public rtc::MessageHandler,
   friend class ConnectionRequest;
 };
 
-// ProxyConnection defers all the interesting work to the port
+// ProxyConnection defers all the interesting work to the port.
 class ProxyConnection : public Connection {
  public:
-  ProxyConnection(Port* port, size_t index, const Candidate& candidate);
+  ProxyConnection(Port* port, size_t index, const Candidate& remote_candidate);
 
-  virtual int Send(const void* data, size_t size,
-                   const rtc::PacketOptions& options);
-  virtual int GetError() { return error_; }
+  int Send(const void* data,
+           size_t size,
+           const rtc::PacketOptions& options) override;
+  int GetError() override { return error_; }
 
  private:
-  int error_;
+  int error_ = 0;
 };
 
 }  // namespace cricket

webrtc/p2p/base/turnport.cc

@@ -1398,6 +1398,12 @@ void TurnEntry::OnCreatePermissionError(StunMessage* response, int code) {
   } else {
     // Send signal with error code.
     port_->SignalCreatePermissionResult(port_, ext_addr_, code);
+    Connection* c = port_->GetConnection(ext_addr_);
+    if (c) {
+      LOG_J(LS_ERROR, c) << "Received TURN CreatePermission error response, "
+                         << "code=" << code << "; killing connection.";
+      c->FailAndDestroy();
+    }
   }
 }
 

webrtc/p2p/base/turnport.h

@@ -133,7 +133,6 @@ class TurnPort : public Port {
                    const rtc::SocketAddress&,
                    const rtc::SocketAddress&> SignalResolvedServerAddress;
 
-  // This signal is only for testing purpose.
   sigslot::signal3<TurnPort*, const rtc::SocketAddress&, int>
       SignalCreatePermissionResult;
   // For testing only.

webrtc/p2p/base/turnport_unittest.cc

@@ -100,6 +100,24 @@ class TurnPortTestVirtualSocketServer : public rtc::VirtualSocketServer {
   using rtc::VirtualSocketServer::LookupBinding;
 };
 
+class TestConnectionWrapper : public sigslot::has_slots<> {
+ public:
+  TestConnectionWrapper(Connection* conn) : connection_(conn) {
+    conn->SignalDestroyed.connect(
+        this, &TestConnectionWrapper::OnConnectionDestroyed);
+  }
+
+  Connection* connection() { return connection_; }
+
+ private:
+  void OnConnectionDestroyed(Connection* conn) {
+    ASSERT_TRUE(conn == connection_);
+    connection_ = nullptr;
+  }
+
+  Connection* connection_;
+};
+
 class TurnPortTest : public testing::Test,
                      public sigslot::has_slots<>,
                      public rtc::MessageHandler {
@@ -256,11 +274,13 @@ class TurnPortTest : public testing::Test,
     turn_port_->SignalCreatePermissionResult.connect(this,
         &TurnPortTest::OnTurnCreatePermissionResult);
   }
-  void CreateUdpPort() {
+
+  void CreateUdpPort() { CreateUdpPort(kLocalAddr2); }
+
+  void CreateUdpPort(const SocketAddress& address) {
     udp_port_.reset(UDPPort::Create(main_, &socket_factory_, &network_,
-                                    kLocalAddr2.ipaddr(), 0, 0,
-                                    kIceUfrag2, kIcePwd2,
-                                    std::string(), false));
+                                    address.ipaddr(), 0, 0, kIceUfrag2,
+                                    kIcePwd2, std::string(), false));
     // UDP port will be controlled.
     udp_port_->SetIceRole(cricket::ICEROLE_CONTROLLED);
     udp_port_->SignalPortComplete.connect(
@@ -849,6 +869,29 @@ TEST_F(TurnPortTest, TestOriginHeader) {
   EXPECT_EQ(kTestOrigin, turn_server_.FindAllocation(local_address)->origin());
 }
 
+// Test that a CreatePermission failure will result in the connection being
+// destroyed.
+TEST_F(TurnPortTest, TestConnectionDestroyedOnCreatePermissionFailure) {
+  turn_server_.AddInternalSocket(kTurnTcpIntAddr, cricket::PROTO_TCP);
+  turn_server_.server()->set_reject_private_addresses(true);
+  CreateTurnPort(kTurnUsername, kTurnPassword, kTurnTcpProtoAddr);
+  turn_port_->PrepareAddress();
+  ASSERT_TRUE_WAIT(turn_ready_, kTimeout);
+
+  CreateUdpPort(SocketAddress("10.0.0.10", 0));
+  udp_port_->PrepareAddress();
+  ASSERT_TRUE_WAIT(udp_ready_, kTimeout);
+  // Create a connection.
+  TestConnectionWrapper conn(turn_port_->CreateConnection(
+      udp_port_->Candidates()[0], Port::ORIGIN_MESSAGE));
+  ASSERT_TRUE(conn.connection() != nullptr);
+
+  // Asynchronously, CreatePermission request should be sent and fail, closing
+  // the connection.
+  EXPECT_TRUE_WAIT(conn.connection() == nullptr, kTimeout);
+  EXPECT_FALSE(turn_create_permission_success_);
+}
+
 // Test that a TURN allocation is released when the port is closed.
 TEST_F(TurnPortTest, TestTurnReleaseAllocation) {
   CreateTurnPort(kTurnUsername, kTurnPassword, kTurnUdpProtoAddr);

webrtc/p2p/base/turnserver.cc

@@ -698,6 +698,12 @@ void TurnServerAllocation::HandleCreatePermissionRequest(
     return;
   }
 
+  if (server_->reject_private_addresses_ &&
+      rtc::IPIsPrivate(peer_attr->GetAddress().ipaddr())) {
+    SendErrorResponse(msg, STUN_ERROR_FORBIDDEN, STUN_ERROR_REASON_FORBIDDEN);
+    return;
+  }
+
   // Add this permission.
   AddPermission(peer_attr->GetAddress().ipaddr());
 

webrtc/p2p/base/turnserver.h

@@ -183,6 +183,11 @@ class TurnServer : public sigslot::has_slots<> {
 
   void set_enable_otu_nonce(bool enable) { enable_otu_nonce_ = enable; }
 
+  // If set to true, reject CreatePermission requests to RFC1918 addresses.
+  void set_reject_private_addresses(bool filter) {
+    reject_private_addresses_ = filter;
+  }
+
   // Starts listening for packets from internal clients.
   void AddInternalSocket(rtc::AsyncPacketSocket* socket,
                          ProtocolType proto);
@@ -255,6 +260,7 @@ class TurnServer : public sigslot::has_slots<> {
   // otu - one-time-use. Server will respond with 438 if it's
   // sees the same nonce in next transaction.
   bool enable_otu_nonce_;
+  bool reject_private_addresses_ = false;
 
   InternalSocketMap server_sockets_;
   ServerSocketMap server_listen_sockets_;