Remove net::ServerSocket::setAllowReuseAddress and fix server socket on Windows.

SO_REUSEADDR is useful for server sockets to bind to a recently unbound port. When a socket is closed, the end point changes its state to TIME_WAIT and wait for 2 MSL (maximum segment lifetime) to ensure the remote peer acknowledges its closure. For server sockets, it is usually safe to bind to a TIME_WAIT end point immediately, which is a widely adopted behavior.

On Linux SO_REUSEADDR does not enable the TCP socket to bind to an end point that is already bound by another socket. To do that one must set SO_REUSEPORT instead.

However, on Windows, SO_REUSEADDR works as if both SO_REUSEDPORT and SO_REUSEADDR are set. Furthermore, A bound end point can be hijacked by another process by setting SO_REUSEADDR. Therefore a Windows-only option SO_EXCLUSIVEADDRUSE was introduced in Windows NT 4.0 SP4. If the socket that is bound to the end point has SO_EXCLUSIVEADDRUSE enabled, it is not possible for another socket to forcibly bind to the end point until the end point is unbound. Also, unlike on *nix, on Windows a TCP server socket can always bind to an end point in TIME_WAIT state without setting SO_REUSEADDR, therefore we cannot emulate a similar behavior of disabling SO_REUSEADDR here.


This is considered a security issue and MSDN (http://goo.gl/M6fjQ) recommends all server applications to use SO_EXCLUSIVEADDRUSE.


BUG=173533
TBR=brettw@chromium.org

Review URL: https://chromiumcodereview.appspot.com/15179003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@210147 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 105 insertions, 82 deletions

diff --git a/chrome/browser/extensions/api/socket/tcp_socket.cc b/chrome/browser/extensions/api/socket/tcp_socket.cc
index 1688fb1..d685ab2 100644
--- a/chrome/browser/extensions/api/socket/tcp_socket.cc
+++ b/chrome/browser/extensions/api/socket/tcp_socket.cc
@@ -184,7 +184,6 @@ int TCPSocket::Listen(const std::string& address, int port, int backlog,
   if (!server_socket_.get()) {
     server_socket_.reset(new net::TCPServerSocket(NULL,
                                                   net::NetLog::Source()));
-    server_socket_->AllowAddressReuse();
   }
   int result = server_socket_->Listen(*bind_address, backlog);
   if (result)
diff --git a/content/browser/devtools/tethering_handler.cc b/content/browser/devtools/tethering_handler.cc
index 5c33231..fc25e63 100644
--- a/content/browser/devtools/tethering_handler.cc
+++ b/content/browser/devtools/tethering_handler.cc
@@ -191,7 +191,6 @@ class TetheringHandler::BoundSocket {
       return false;
 
     net::IPEndPoint end_point(ip_number, port);
-    socket_->AllowAddressReuse();
     int result = socket_->Listen(end_point, kListenBacklog);
     if (result < 0)
       return false;
@@ -254,8 +253,8 @@ TetheringHandler::TetheringHandler(DevToolsHttpHandlerDelegate* delegate)
 }
 
 TetheringHandler::~TetheringHandler() {
-   STLDeleteContainerPairSecondPointers(bound_sockets_.begin(),
-                                        bound_sockets_.end());
+  STLDeleteContainerPairSecondPointers(bound_sockets_.begin(),
+                                       bound_sockets_.end());
 }
 
 void TetheringHandler::Accepted(int port, const std::string& name) {
diff --git a/content/browser/renderer_host/p2p/socket_host_tcp_server_unittest.cc b/content/browser/renderer_host/p2p/socket_host_tcp_server_unittest.cc
index 6bca738..9eba741 100644
--- a/content/browser/renderer_host/p2p/socket_host_tcp_server_unittest.cc
+++ b/content/browser/renderer_host/p2p/socket_host_tcp_server_unittest.cc
@@ -45,10 +45,6 @@ class FakeServerSocket : public net::ServerSocket {
     }
   }
 
-  // net::ServerSocket implementation.
-  virtual void AllowAddressReuse() OVERRIDE {
-  }
-
   virtual int Listen(const net::IPEndPoint& address, int backlog) OVERRIDE {
     local_address_ = address;
     listening_ = true;
@@ -113,7 +109,7 @@ class P2PSocketHostTcpServerTest : public testing::Test {
   }
 
   MockIPCSender sender_;
-  FakeServerSocket* socket_; // Owned by |socket_host_|.
+  FakeServerSocket* socket_;  // Owned by |socket_host_|.
   scoped_ptr<P2PSocketHostTcpServer> socket_host_;
 };
 
diff --git a/net/socket/server_socket.h b/net/socket/server_socket.h
index a48ebec..11151ee 100644
--- a/net/socket/server_socket.h
+++ b/net/socket/server_socket.h
@@ -19,10 +19,6 @@ class NET_EXPORT ServerSocket {
   ServerSocket() { }
   virtual ~ServerSocket() { }
 
-  // Allows the socket to share the local address to which the socket will be
-  // bound with other processes. Should be called before Listen().
-  virtual void AllowAddressReuse() = 0;
-
   // Bind the socket and start listening. Destroy the socket to stop
   // listening.
   virtual int Listen(const net::IPEndPoint& address, int backlog) = 0;
diff --git a/net/socket/tcp_server_socket_libevent.cc b/net/socket/tcp_server_socket_libevent.cc
index c46463c..38dda96 100644
--- a/net/socket/tcp_server_socket_libevent.cc
+++ b/net/socket/tcp_server_socket_libevent.cc
@@ -35,7 +35,6 @@ TCPServerSocketLibevent::TCPServerSocketLibevent(
     const net::NetLog::Source& source)
     : socket_(kInvalidSocket),
       accept_socket_(NULL),
-      reuse_address_(false),
       net_log_(BoundNetLog::Make(net_log, NetLog::SOURCE_SOCKET)) {
   net_log_.BeginEvent(NetLog::TYPE_SOCKET_ALIVE,
                       source.ToEventParametersCallback());
@@ -47,13 +46,6 @@ TCPServerSocketLibevent::~TCPServerSocketLibevent() {
   net_log_.EndEvent(NetLog::TYPE_SOCKET_ALIVE);
 }
 
-void TCPServerSocketLibevent::AllowAddressReuse() {
-  DCHECK(CalledOnValidThread());
-  DCHECK_EQ(socket_, kInvalidSocket);
-
-  reuse_address_ = true;
-}
-
 int TCPServerSocketLibevent::Listen(const IPEndPoint& address, int backlog) {
   DCHECK(CalledOnValidThread());
   DCHECK_GT(backlog, 0);
@@ -72,12 +64,16 @@ int TCPServerSocketLibevent::Listen(const IPEndPoint& address, int backlog) {
   }
 
   int result = SetSocketOptions();
-  if (result != OK)
+  if (result != OK) {
+    Close();
     return result;
+  }
 
   SockaddrStorage storage;
-  if (!address.ToSockAddr(storage.addr, &storage.addr_len))
-    return ERR_INVALID_ARGUMENT;
+  if (!address.ToSockAddr(storage.addr, &storage.addr_len)) {
+    Close();
+    return ERR_ADDRESS_INVALID;
+  }
 
   result = bind(socket_, storage.addr, storage.addr_len);
   if (result < 0) {
@@ -138,13 +134,24 @@ int TCPServerSocketLibevent::Accept(
 }
 
 int TCPServerSocketLibevent::SetSocketOptions() {
+  // SO_REUSEADDR is useful for server sockets to bind to a recently unbound
+  // port. When a socket is closed, the end point changes its state to TIME_WAIT
+  // and wait for 2 MSL (maximum segment lifetime) to ensure the remote peer
+  // acknowledges its closure. For server sockets, it is usually safe to
+  // bind to a TIME_WAIT end point immediately, which is a widely adopted
+  // behavior.
+  //
+  // Note that on *nix, SO_REUSEADDR does not enable the TCP socket to bind to
+  // an end point that is already bound by another socket. To do that one must
+  // set SO_REUSEPORT instead. This option is not provided on Linux prior
+  // to 3.9.
+  //
+  // SO_REUSEPORT is provided in MacOS X and iOS.
   int true_value = 1;
-  if (reuse_address_) {
-    int rv = setsockopt(socket_, SOL_SOCKET, SO_REUSEADDR, &true_value,
-                        sizeof(true_value));
-    if (rv < 0)
-      return MapSystemError(errno);
-  }
+  int rv = setsockopt(socket_, SOL_SOCKET, SO_REUSEADDR, &true_value,
+                      sizeof(true_value));
+  if (rv < 0)
+    return MapSystemError(errno);
   return OK;
 }
 
diff --git a/net/socket/tcp_server_socket_libevent.h b/net/socket/tcp_server_socket_libevent.h
index 7fb2447..40ba659 100644
--- a/net/socket/tcp_server_socket_libevent.h
+++ b/net/socket/tcp_server_socket_libevent.h
@@ -26,7 +26,6 @@ class NET_EXPORT_PRIVATE TCPServerSocketLibevent :
   virtual ~TCPServerSocketLibevent();
 
   // net::ServerSocket implementation.
-  virtual void AllowAddressReuse() OVERRIDE;
   virtual int Listen(const net::IPEndPoint& address, int backlog) OVERRIDE;
   virtual int GetLocalAddress(IPEndPoint* address) const OVERRIDE;
   virtual int Accept(scoped_ptr<StreamSocket>* socket,
@@ -48,8 +47,6 @@ class NET_EXPORT_PRIVATE TCPServerSocketLibevent :
   scoped_ptr<StreamSocket>* accept_socket_;
   CompletionCallback accept_callback_;
 
-  bool reuse_address_;
-
   BoundNetLog net_log_;
 };
 
diff --git a/net/socket/tcp_server_socket_win.cc b/net/socket/tcp_server_socket_win.cc
index c834506..7ae7a85 100644
--- a/net/socket/tcp_server_socket_win.cc
+++ b/net/socket/tcp_server_socket_win.cc
@@ -21,7 +21,6 @@ TCPServerSocketWin::TCPServerSocketWin(net::NetLog* net_log,
     : socket_(INVALID_SOCKET),
       socket_event_(WSA_INVALID_EVENT),
       accept_socket_(NULL),
-      reuse_address_(false),
       net_log_(BoundNetLog::Make(net_log, NetLog::SOURCE_SOCKET)) {
   net_log_.BeginEvent(NetLog::TYPE_SOCKET_ALIVE,
                       source.ToEventParametersCallback());
@@ -33,14 +32,6 @@ TCPServerSocketWin::~TCPServerSocketWin() {
   net_log_.EndEvent(NetLog::TYPE_SOCKET_ALIVE);
 }
 
-void TCPServerSocketWin::AllowAddressReuse() {
-  DCHECK(CalledOnValidThread());
-  DCHECK_EQ(socket_, INVALID_SOCKET);
-  DCHECK_EQ(socket_event_, WSA_INVALID_EVENT);
-
-  reuse_address_ = true;
-}
-
 int TCPServerSocketWin::Listen(const IPEndPoint& address, int backlog) {
   DCHECK(CalledOnValidThread());
   DCHECK_GT(backlog, 0);
@@ -66,12 +57,16 @@ int TCPServerSocketWin::Listen(const IPEndPoint& address, int backlog) {
   }
 
   int result = SetSocketOptions();
-  if (result != OK)
+  if (result != OK) {
+    Close();
     return result;
+  }
 
   SockaddrStorage storage;
-  if (!address.ToSockAddr(storage.addr, &storage.addr_len))
-    return ERR_INVALID_ARGUMENT;
+  if (!address.ToSockAddr(storage.addr, &storage.addr_len)) {
+    Close();
+    return ERR_ADDRESS_INVALID;
+  }
 
   result = bind(socket_, storage.addr, storage.addr_len);
   if (result < 0) {
@@ -129,14 +124,28 @@ int TCPServerSocketWin::Accept(
 }
 
 int TCPServerSocketWin::SetSocketOptions() {
+  // On Windows, a bound end point can be hijacked by another process by
+  // setting SO_REUSEADDR. Therefore a Windows-only option SO_EXCLUSIVEADDRUSE
+  // was introduced in Windows NT 4.0 SP4. If the socket that is bound to the
+  // end point has SO_EXCLUSIVEADDRUSE enabled, it is not possible for another
+  // socket to forcibly bind to the end point until the end point is unbound.
+  // It is recommend that all server applications must use SO_EXCLUSIVEADDRUSE.
+  // MSDN: http://goo.gl/M6fjQ.
+  //
+  // Unlike on *nix, on Windows a TCP server socket can always bind to an end
+  // point in TIME_WAIT state without setting SO_REUSEADDR, therefore it is not
+  // needed here.
+  //
+  // SO_EXCLUSIVEADDRUSE will prevent a TCP client socket from binding to an end
+  // point in TIME_WAIT status. It does not have this effect for a TCP server
+  // socket.
+
   BOOL true_value = 1;
-  if (reuse_address_) {
-    int rv = setsockopt(socket_, SOL_SOCKET, SO_REUSEADDR,
-                        reinterpret_cast<const char*>(&true_value),
-                        sizeof(true_value));
-    if (rv < 0)
-      return MapSystemError(errno);
-  }
+  int rv = setsockopt(socket_, SOL_SOCKET, SO_EXCLUSIVEADDRUSE,
+                      reinterpret_cast<const char*>(&true_value),
+                      sizeof(true_value));
+  if (rv < 0)
+    return MapSystemError(errno);
   return OK;
 }
 
diff --git a/net/socket/tcp_server_socket_win.h b/net/socket/tcp_server_socket_win.h
index fab8bf0..cbd4174 100644
--- a/net/socket/tcp_server_socket_win.h
+++ b/net/socket/tcp_server_socket_win.h
@@ -29,7 +29,6 @@ class NET_EXPORT_PRIVATE TCPServerSocketWin
   ~TCPServerSocketWin();
 
   // net::ServerSocket implementation.
-  virtual void AllowAddressReuse() OVERRIDE;
   virtual int Listen(const net::IPEndPoint& address, int backlog) OVERRIDE;
   virtual int GetLocalAddress(IPEndPoint* address) const OVERRIDE;
   virtual int Accept(scoped_ptr<StreamSocket>* socket,
@@ -51,8 +50,6 @@ class NET_EXPORT_PRIVATE TCPServerSocketWin
   scoped_ptr<StreamSocket>* accept_socket_;
   CompletionCallback accept_callback_;
 
-  bool reuse_address_;
-
   BoundNetLog net_log_;
 };
 
diff --git a/net/udp/udp_socket_libevent.cc b/net/udp/udp_socket_libevent.cc
index d2446af..073e61c 100644
--- a/net/udp/udp_socket_libevent.cc
+++ b/net/udp/udp_socket_libevent.cc
@@ -8,6 +8,7 @@
 #include <fcntl.h>
 #include <netdb.h>
 #include <sys/socket.h>
+#include <netinet/in.h>
 
 #include "base/callback.h"
 #include "base/logging.h"
@@ -21,17 +22,14 @@
 #include "net/base/net_log.h"
 #include "net/base/net_util.h"
 #include "net/udp/udp_net_log_parameters.h"
-#if defined(OS_POSIX)
-#include <netinet/in.h>
-#endif
 
 namespace {
 
-static const int kBindRetries = 10;
-static const int kPortStart = 1024;
-static const int kPortEnd = 65535;
+const int kBindRetries = 10;
+const int kPortStart = 1024;
+const int kPortEnd = 65535;
 
-}  // namespace net
+}  // namespace
 
 namespace net {
 
@@ -88,6 +86,7 @@ void UDPSocketLibevent::Close() {
     PLOG(ERROR) << "close";
 
   socket_ = kInvalidSocket;
+  addr_family_ = 0;
 }
 
 int UDPSocketLibevent::GetPeerAddress(IPEndPoint* address) const {
@@ -235,16 +234,24 @@ int UDPSocketLibevent::InternalConnect(const IPEndPoint& address) {
     rv = RandomBind(address);
   // else connect() does the DatagramSocket::DEFAULT_BIND
 
-  if (rv < 0)
+  if (rv < 0) {
+    Close();
     return rv;
+  }
 
   SockaddrStorage storage;
-  if (!address.ToSockAddr(storage.addr, &storage.addr_len))
-    return ERR_FAILED;
+  if (!address.ToSockAddr(storage.addr, &storage.addr_len)) {
+    Close();
+    return ERR_ADDRESS_INVALID;
+  }
 
   rv = HANDLE_EINTR(connect(socket_, storage.addr, storage.addr_len));
-  if (rv < 0)
-    return MapSystemError(errno);
+  if (rv < 0) {
+    // Close() may change the current errno. Map errno beforehand.
+    int result = MapSystemError(errno);
+    Close();
+    return result;
+  }
 
   remote_address_.reset(new IPEndPoint(address));
   return rv;
@@ -256,12 +263,17 @@ int UDPSocketLibevent::Bind(const IPEndPoint& address) {
   int rv = CreateSocket(address);
   if (rv < 0)
     return rv;
+
   rv = SetSocketOptions();
-  if (rv < 0)
+  if (rv < 0) {
+    Close();
     return rv;
+  }
   rv = DoBind(address);
-  if (rv < 0)
+  if (rv < 0) {
+    Close();
     return rv;
+  }
   local_address_.reset();
   return rv;
 }
diff --git a/net/udp/udp_socket_win.cc b/net/udp/udp_socket_win.cc
index fe97d0b..500abfd 100644
--- a/net/udp/udp_socket_win.cc
+++ b/net/udp/udp_socket_win.cc
@@ -24,11 +24,11 @@
 
 namespace {
 
-static const int kBindRetries = 10;
-static const int kPortStart = 1024;
-static const int kPortEnd = 65535;
+const int kBindRetries = 10;
+const int kPortStart = 1024;
+const int kPortEnd = 65535;
 
-}  // namespace net
+}  // namespace
 
 namespace net {
 
@@ -198,6 +198,7 @@ void UDPSocketWin::Close() {
   UMA_HISTOGRAM_TIMES("Net.UDPSocketWinClose",
                       base::TimeTicks::Now() - start_time);
   socket_ = INVALID_SOCKET;
+  addr_family_ = 0;
 
   core_->Detach();
   core_ = NULL;
@@ -216,7 +217,7 @@ int UDPSocketWin::GetPeerAddress(IPEndPoint* address) const {
       return MapSystemError(WSAGetLastError());
     scoped_ptr<IPEndPoint> address(new IPEndPoint());
     if (!address->FromSockAddr(storage.addr, storage.addr_len))
-      return ERR_FAILED;
+      return ERR_ADDRESS_INVALID;
     remote_address_.reset(address.release());
   }
 
@@ -237,7 +238,7 @@ int UDPSocketWin::GetLocalAddress(IPEndPoint* address) const {
       return MapSystemError(WSAGetLastError());
     scoped_ptr<IPEndPoint> address(new IPEndPoint());
     if (!address->FromSockAddr(storage.addr, storage.addr_len))
-      return ERR_FAILED;
+      return ERR_ADDRESS_INVALID;
     local_address_.reset(address.release());
   }
 
@@ -326,16 +327,22 @@ int UDPSocketWin::InternalConnect(const IPEndPoint& address) {
     rv = RandomBind(address);
   // else connect() does the DatagramSocket::DEFAULT_BIND
 
-  if (rv < 0)
+  if (rv < 0) {
+    Close();
     return rv;
+  }
 
   SockaddrStorage storage;
   if (!address.ToSockAddr(storage.addr, &storage.addr_len))
-    return ERR_FAILED;
+    return ERR_ADDRESS_INVALID;
 
   rv = connect(socket_, storage.addr, storage.addr_len);
-  if (rv < 0)
-    return MapSystemError(WSAGetLastError());
+  if (rv < 0) {
+    // Close() may change the last error. Map it beforehand.
+    int result = MapSystemError(WSAGetLastError());
+    Close();
+    return result;
+  }
 
   remote_address_.reset(new IPEndPoint(address));
   return rv;
@@ -347,11 +354,15 @@ int UDPSocketWin::Bind(const IPEndPoint& address) {
   if (rv < 0)
     return rv;
   rv = SetSocketOptions();
-  if (rv < 0)
+  if (rv < 0) {
+    Close();
     return rv;
+  }
   rv = DoBind(address);
-  if (rv < 0)
+  if (rv < 0) {
+    Close();
     return rv;
+  }
   local_address_.reset();
   return rv;
 }
@@ -424,7 +435,7 @@ void UDPSocketWin::DidCompleteRead() {
   // Convert address.
   if (recv_from_address_ && result >= 0) {
     if (!ReceiveAddressToIPEndpoint(recv_from_address_))
-      result = ERR_FAILED;
+      result = ERR_ADDRESS_INVALID;
   }
   LogRead(result, core_->read_iobuffer_->data());
   core_->read_iobuffer_ = NULL;