Do not delete a connection in the turn port with permission error or refresh error.

webrtc/p2p/base/turnport.cc

 /*
  *  Copyright 2012 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.
  */
 
@@ skipping 431 matching lines @@
                                        CandidateOrigin origin) {
   // TURN-UDP can only connect to UDP candidates.
   if (!SupportsProtocol(address.protocol())) {
     return NULL;
   }
 
   if (!IsCompatibleAddress(address.address())) {
     return NULL;
   }
 
-  if (state_ == STATE_DISCONNECTED) {
+  if (state_ == STATE_DISCONNECTED || state_ == STATE_RECEIVEONLY) {
     return NULL;
   }
 
   // Create an entry, if needed, so we can get our permissions set up correctly.
   CreateOrRefreshEntry(address.address());
 
   // A TURN port will have two candiates, STUN and TURN. STUN may not
   // present in all cases. If present stun candidate will be added first
   // and TURN candidate later.
   for (size_t index = 0; index < Candidates().size(); ++index) {
     if (Candidates()[index].type() == RELAY_PORT_TYPE) {
       ProxyConnection* conn = new ProxyConnection(this, index, address);
       AddOrReplaceConnection(conn);
       return conn;
     }
   }
   return NULL;
 }
 
-bool TurnPort::DestroyConnection(const rtc::SocketAddress& address) {
+bool TurnPort::FailAndPruneConnection(const rtc::SocketAddress& address) {
   Connection* conn = GetConnection(address);
   if (conn != nullptr) {
-    conn->Destroy();
+    conn->FailAndPrune();
     return true;
   }
   return false;
 }
 
 int TurnPort::SetOption(rtc::Socket::Option opt, int value) {
   if (!socket_) {
     // If socket is not created yet, these options will be applied during socket
     // creation.
     socket_options_[opt] = value;
@@ skipping 68 matching lines @@
   }
 
   // The message must be at least the size of a channel header.
   if (size < TURN_CHANNEL_HEADER_SIZE) {
     LOG_J(LS_WARNING, this) << "Received TURN message that was too short";
     return false;
   }
 
   if (state_ == STATE_DISCONNECTED) {
     LOG_J(LS_WARNING, this)
-        << "Received TURN message while the Turn port is disconnected";
+        << "Received TURN message while the TURN port is disconnected";
     return false;
   }
 
   // Check the message type, to see if is a Channel Data message.
   // The message will either be channel data, a TURN data indication, or
   // a response to a previous request.
   uint16_t msg_type = rtc::GetBE16(data);
   if (IsTurnChannelData(msg_type)) {
     HandleChannelData(msg_type, data, size, packet_time);
     return true;
@@ skipping 157 matching lines @@
              server_priority_, true);
 }
 
 void TurnPort::OnAllocateError() {
   // We will send SignalPortError asynchronously as this can be sent during
   // port initialization. This way it will not be blocking other port
   // creation.
   thread()->Post(RTC_FROM_HERE, this, MSG_ALLOCATE_ERROR);
 }
 
-void TurnPort::OnTurnRefreshError() {
-  // Need to Close the port asynchronously because otherwise, the refresh
+void TurnPort::OnRefreshError() {
+  // Need to clear the requests asynchronously because otherwise, the refresh
   // request may be deleted twice: once at the end of the message processing
-  // and the other in Close().
+  // and the other in HandleRefreshError().
   thread()->Post(RTC_FROM_HERE, this, MSG_REFRESH_ERROR);
 }
 
+void TurnPort::HandleRefreshError() {
+  request_manager_.Clear();
+  state_ = STATE_RECEIVEONLY;
+  // Fail and prune all connections; stop sending data.
+  for (auto kv : connections()) {
+    kv.second->FailAndPrune();
+  }
+}
+
 void TurnPort::Close() {
   if (!ready()) {
     OnAllocateError();
   }
   request_manager_.Clear();
   // Stop the port from creating new connections.
   state_ = STATE_DISCONNECTED;
   // Delete all existing connections; stop sending data.
   for (auto kv : connections()) {
     kv.second->Destroy();
   }
 }
 
 void TurnPort::OnMessage(rtc::Message* message) {
   switch (message->message_id) {
     case MSG_ALLOCATE_ERROR:
       SignalPortError(this);
       break;
     case MSG_ALLOCATE_MISMATCH:
       OnAllocateMismatch();
       break;
     case MSG_REFRESH_ERROR:
-      Close();
+      HandleRefreshError();
       break;
     case MSG_TRY_ALTERNATE_SERVER:
       if (server_address().proto == PROTO_UDP) {
         // Send another allocate request to alternate server, with the received
         // realm and nonce values.
         SendRequest(new TurnAllocateRequest(this), 0);
       } else {
         // Since it's TCP, we have to delete the connected socket and reconnect
         // with the alternate server. PrepareAddress will send stun binding once
         // the new socket is connected.
@@ skipping 488 matching lines @@
   if (error_code->code() == STUN_ERROR_STALE_NONCE) {
     if (port_->UpdateNonce(response)) {
       // Send RefreshRequest immediately.
       port_->SendRequest(new TurnRefreshRequest(port_), 0);
     }
   } else {
     LOG_J(LS_WARNING, port_) << "Received TURN refresh error response"
                              << ", id=" << rtc::hex_encode(id())
                              << ", code=" << error_code->code()
                              << ", rtt=" << Elapsed();
-    port_->OnTurnRefreshError();
+    port_->OnRefreshError();
     port_->SignalTurnRefreshResult(port_, error_code->code());
   }
 }
 
 void TurnRefreshRequest::OnTimeout() {
   LOG_J(LS_WARNING, port_) << "TURN refresh timeout " << rtc::hex_encode(id());
-  port_->OnTurnRefreshError();
+  port_->OnRefreshError();
 }
 
 TurnCreatePermissionRequest::TurnCreatePermissionRequest(
     TurnPort* port, TurnEntry* entry,
     const rtc::SocketAddress& ext_addr)
     : StunRequest(new TurnMessage()),
       port_(port),
       entry_(entry),
       ext_addr_(ext_addr) {
   entry_->SignalDestroyed.connect(
@@ skipping 186 matching lines @@
                           << delay << "ms.";
   }
 }
 
 void TurnEntry::OnCreatePermissionError(StunMessage* response, int code) {
   if (code == STUN_ERROR_STALE_NONCE) {
     if (port_->UpdateNonce(response)) {
       SendCreatePermissionRequest(0);
     }
   } else {
-    port_->DestroyConnection(ext_addr_);
+    bool found = port_->FailAndPruneConnection(ext_addr_);

[juberti, 2016/06/23 21:07:29]

Shouldn't we destroy the connection if we can't create a permission for it? We
shouldn't be able to receive data if the permission fails.

[pthatcher1, 2016/06/23 21:29:23]

Not if the TURN server doesn't check for CreatePermission, right?

+    if (found) {
+      LOG(LS_ERROR) << "Received TURN CreatePermission error response, "
+                    << "code=" << code << "; pruned connection.";
+    }
     // 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();
-    }
   }
 }
 
 void TurnEntry::OnCreatePermissionTimeout() {
-  port_->DestroyConnection(ext_addr_);
+  port_->FailAndPruneConnection(ext_addr_);
 }
 
 void TurnEntry::OnChannelBindSuccess() {
   LOG_J(LS_INFO, port_) << "Channel bind for " << ext_addr_.ToSensitiveString()
                         << " succeeded";
   ASSERT(state_ == STATE_BINDING || state_ == STATE_BOUND);
   state_ = STATE_BOUND;
 }
 
 void TurnEntry::OnChannelBindError(StunMessage* response, int code) {
   // If the channel bind fails due to errors other than STATE_NONCE,
-  // we just destroy the connection and rely on ICE restart to re-establish
-  // the connection.
+  // we will fail and prune the connection and rely on ICE restart to
+  // re-establish a new connection if needed.
   if (code == STUN_ERROR_STALE_NONCE) {
     if (port_->UpdateNonce(response)) {
       // Send channel bind request with fresh nonce.
       SendChannelBindRequest(0);
     }
   } else {
     state_ = STATE_UNBOUND;
-    port_->DestroyConnection(ext_addr_);

[juberti, 2016/06/23 21:07:29]

This also seems like an error that should be fatal.

+    port_->FailAndPruneConnection(ext_addr_);
   }
 }
 void TurnEntry::OnChannelBindTimeout() {
   state_ = STATE_UNBOUND;
-  port_->DestroyConnection(ext_addr_);
+  port_->FailAndPruneConnection(ext_addr_);
 }
 }  // namespace cricket