Fixing logic for using android_setsocknetwork() with bind().

webrtc/base/physicalsocketserver.cc

 /*
  *  Copyright 2004 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.
  */
 #include "webrtc/base/physicalsocketserver.h"
@@ skipping 171 matching lines @@
   if (result >= 0) {
     SocketAddressFromSockAddrStorage(addr_storage, &address);
   } else {
     LOG(LS_WARNING) << "GetRemoteAddress: unable to get remote addr, socket="
                     << s_;
   }
   return address;
 }
 
 int PhysicalSocket::Bind(const SocketAddress& bind_addr) {
+  SocketAddress copied_bind_addr = bind_addr;
+  // If a network binder is available, use it to bind a socket to an interface
+  // instead of bind(), since this is more reliable on an OS with a weak host
+  // model.
+  if (ss_->network_binder()) {
+    NetworkBindingResult result =
+        ss_->network_binder()->BindSocketToNetwork(s_, bind_addr.ipaddr());
+    if (result == NetworkBindingResult::SUCCESS) {
+      // Since the network binder handled binding the socket to the desired
+      // network interface, we don't need to (and shouldn't) include an IP in
+      // the bind() call; bind() just needs to assign a port.
+      copied_bind_addr.SetIP(GetAnyIP(copied_bind_addr.ipaddr().family()));
+    } else if (result == NetworkBindingResult::NOT_IMPLEMENTED) {
+      LOG(LS_INFO) << "Can't bind socket to network because "
+                      "network binding is not implemented for this OS.";
+    } else {
+      LOG(LS_WARNING) << "Binding socket to network address "
+                      << bind_addr.ipaddr().ToString()
+                      << " failed; result: " << static_cast<int>(result);
+      // If a network binding was attempted and failed, we should stop here and
+      // not try to use the socket. Otherwise, we may end up sending packets
+      // with an invalid source address.
+      // See: https://bugs.chromium.org/p/webrtc/issues/detail?id=7026
+      return -1;

[pthatcher1, 2017/02/10 18:12:19]

Should we return different errors based on the result?  In fact, should we
return a NetworkBindingResult from this method?

[Taylor Brandstetter, 2017/02/10 18:57:52]

Bind either returns -1 or 0, and I wasn't planning to change that in this CL.
There would be no point in returning a NetworkBindError now since nothing would
use it.

+    }
+  }
   sockaddr_storage addr_storage;
-  size_t len = bind_addr.ToSockAddrStorage(&addr_storage);
+  size_t len = copied_bind_addr.ToSockAddrStorage(&addr_storage);
   sockaddr* addr = reinterpret_cast<sockaddr*>(&addr_storage);
   int err = ::bind(s_, addr, static_cast<int>(len));
   UpdateLastError();
 #if !defined(NDEBUG)
   if (0 == err) {
     dbg_addr_ = "Bound @ ";
     dbg_addr_.append(GetLocalAddress().ToString());
   }
 #endif
-  if (ss_->network_binder()) {
-    int result =
-        ss_->network_binder()->BindSocketToNetwork(s_, bind_addr.ipaddr());
-    if (result < 0) {
-      LOG(LS_INFO) << "Binding socket to network address "
-                   << bind_addr.ipaddr().ToString() << " result " << result;
-    }
-  }
   return err;
 }
 
 int PhysicalSocket::Connect(const SocketAddress& addr) {
   // TODO(pthatcher): Implicit creation is required to reconnect...
   // ...but should we make it more explicit?
   if (state_ != CS_CLOSED) {
     SetError(EALREADY);
     return SOCKET_ERROR;
   }
@@ skipping 1376 matching lines @@
        break;
     }
   }
 
   // Done
   return true;
 }
 #endif  // WEBRTC_WIN
 
 }  // namespace rtc