Prevent DOS by asserts in TCP tuning (#2492)

* Prevent DOS by asserts in TCP tuning

-Propagates socket option errors from the
tuning functions to the callers.
-Asserts a subset of error conditions during tuning,
excluding external network causes.
-Checks tuning results in 3 call sites and treats
them like failures to connect, accept, etc.

* Fix variable name

* Remove lambda requiring C++11

src/socks_connecter.cpp

@@ -390,7 +390,7 @@ zmq::fd_t zmq::socks_connecter_t::check_proxy_connection ()
     socklen_t len = sizeof err;
 #endif
 
-    const int rc = getsockopt (s, SOL_SOCKET, SO_ERROR, (char*) &err, &len);
+    int rc = getsockopt (s, SOL_SOCKET, SO_ERROR, (char*) &err, &len);
 
     //  Assert if the error was caused by 0MQ bug.
     //  Networking problems are OK. No need to assert.
@@ -427,9 +427,11 @@ zmq::fd_t zmq::socks_connecter_t::check_proxy_connection ()
     }
 #endif
 
-    tune_tcp_socket (s);
-    tune_tcp_keepalives (s, options.tcp_keepalive, options.tcp_keepalive_cnt,
+    rc = tune_tcp_socket (s);
+    rc = rc | tune_tcp_keepalives (s, options.tcp_keepalive, options.tcp_keepalive_cnt,
         options.tcp_keepalive_idle, options.tcp_keepalive_intvl);
+    if (rc != 0)
+        return -1;
     
     return 0;
 }

src/tcp.cpp

@@ -45,7 +45,7 @@
 #include <ioctl.h>
 #endif
 
-void zmq::tune_tcp_socket (fd_t s_)
+int zmq::tune_tcp_socket (fd_t s_)
 {
     //  Disable Nagle's algorithm. We are doing data batching on 0MQ level,
     //  so using Nagle wouldn't improve throughput in anyway, but it would
@@ -53,44 +53,37 @@ void zmq::tune_tcp_socket (fd_t s_)
     int nodelay = 1;
     int rc = setsockopt (s_, IPPROTO_TCP, TCP_NODELAY, (char*) &nodelay,
         sizeof (int));
-#ifdef ZMQ_HAVE_WINDOWS
-    wsa_assert (rc != SOCKET_ERROR);
-#else
-    errno_assert (rc == 0);
-#endif
+    tcp_assert_tuning_error(s_, rc);
+    if (rc != 0)
+        return rc;
 
 #ifdef ZMQ_HAVE_OPENVMS
     //  Disable delayed acknowledgements as they hurt latency significantly.
     int nodelack = 1;
     rc = setsockopt (s_, IPPROTO_TCP, TCP_NODELACK, (char*) &nodelack,
         sizeof (int));
-    errno_assert (rc != SOCKET_ERROR);
+    tcp_assert_tuning_error(s_, rc);
 #endif
+    return rc;
 }
 
-void zmq::set_tcp_send_buffer (fd_t sockfd_, int bufsize_)
+int zmq::set_tcp_send_buffer (fd_t sockfd_, int bufsize_)
 {
     const int rc = setsockopt (sockfd_, SOL_SOCKET, SO_SNDBUF,
         (char*) &bufsize_, sizeof bufsize_);
-#ifdef ZMQ_HAVE_WINDOWS
-    wsa_assert (rc != SOCKET_ERROR);
-#else
-    errno_assert (rc == 0);
-#endif
+    tcp_assert_tuning_error(sockfd_, rc);
+    return rc;
 }
 
-void zmq::set_tcp_receive_buffer (fd_t sockfd_, int bufsize_)
+int zmq::set_tcp_receive_buffer (fd_t sockfd_, int bufsize_)
 {
     const int rc = setsockopt (sockfd_, SOL_SOCKET, SO_RCVBUF,
         (char *) &bufsize_, sizeof bufsize_);
-#ifdef ZMQ_HAVE_WINDOWS
-    wsa_assert (rc != SOCKET_ERROR);
-#else
-    errno_assert (rc == 0);
-#endif
+    tcp_assert_tuning_error(sockfd_, rc);
+    return rc;
 }
 
-void zmq::tune_tcp_keepalives (fd_t s_, int keepalive_, int keepalive_cnt_,
+int zmq::tune_tcp_keepalives (fd_t s_, int keepalive_, int keepalive_cnt_,
         int keepalive_idle_, int keepalive_intvl_)
 {
     // These options are used only under certain #ifdefs below.
@@ -115,20 +108,26 @@ void zmq::tune_tcp_keepalives (fd_t s_, int keepalive_, int keepalive_cnt_,
         DWORD num_bytes_returned;
         int rc = WSAIoctl(s_, SIO_KEEPALIVE_VALS, &keepalive_opts,
             sizeof(keepalive_opts), NULL, 0, &num_bytes_returned, NULL, NULL);
-        wsa_assert (rc != SOCKET_ERROR);
+        tcp_assert_tuning_error(s_, rc);
+        if (rc == SOCKET_ERROR)
+            return rc;
     }
 #else
 #ifdef ZMQ_HAVE_SO_KEEPALIVE
     if (keepalive_ != -1) {
         int rc = setsockopt (s_, SOL_SOCKET, SO_KEEPALIVE,
                 (char*) &keepalive_, sizeof (int));
-        errno_assert (rc == 0);
+        tcp_assert_tuning_error(s_, rc);
+        if (rc != 0)
+            return rc;
 
 #ifdef ZMQ_HAVE_TCP_KEEPCNT
         if (keepalive_cnt_ != -1) {
             int rc = setsockopt (s_, IPPROTO_TCP, TCP_KEEPCNT,
                     &keepalive_cnt_, sizeof (int));
-            errno_assert (rc == 0);
+            tcp_assert_tuning_error(s_, rc);
+            if (rc != 0)
+                return rc;
         }
 #endif // ZMQ_HAVE_TCP_KEEPCNT
 
@@ -136,14 +135,18 @@ void zmq::tune_tcp_keepalives (fd_t s_, int keepalive_, int keepalive_cnt_,
         if (keepalive_idle_ != -1) {
             int rc = setsockopt (s_, IPPROTO_TCP, TCP_KEEPIDLE,
                     &keepalive_idle_, sizeof (int));
-            errno_assert (rc == 0);
+            tcp_assert_tuning_error(s_, rc);
+            if (rc != 0)
+                return rc;
         }
 #else // ZMQ_HAVE_TCP_KEEPIDLE
 #ifdef ZMQ_HAVE_TCP_KEEPALIVE
         if (keepalive_idle_ != -1) {
             int rc = setsockopt (s_, IPPROTO_TCP, TCP_KEEPALIVE,
                     &keepalive_idle_, sizeof (int));
-            errno_assert (rc == 0);
+            tcp_assert_tuning_error(s_, rc);
+            if (rc != 0)
+                return rc;
         }
 #endif // ZMQ_HAVE_TCP_KEEPALIVE
 #endif // ZMQ_HAVE_TCP_KEEPIDLE
@@ -152,18 +155,22 @@ void zmq::tune_tcp_keepalives (fd_t s_, int keepalive_, int keepalive_cnt_,
         if (keepalive_intvl_ != -1) {
             int rc = setsockopt (s_, IPPROTO_TCP, TCP_KEEPINTVL,
                     &keepalive_intvl_, sizeof (int));
-            errno_assert (rc == 0);
+            tcp_assert_tuning_error(s_, rc);
+            if (rc != 0)
+                return rc;
         }
 #endif // ZMQ_HAVE_TCP_KEEPINTVL
     }
 #endif // ZMQ_HAVE_SO_KEEPALIVE
 #endif // ZMQ_HAVE_WINDOWS
+    
+    return 0;
 }
 
-void zmq::tune_tcp_maxrt (fd_t sockfd_, int timeout_)
+int zmq::tune_tcp_maxrt (fd_t sockfd_, int timeout_)
 {
     if (timeout_ <= 0)
-        return;
+        return 0;
 
     LIBZMQ_UNUSED (sockfd_);
 
@@ -172,13 +179,16 @@ void zmq::tune_tcp_maxrt (fd_t sockfd_, int timeout_)
     timeout_ /= 1000;    // in seconds
     int rc = setsockopt (sockfd_, IPPROTO_TCP, TCP_MAXRT, (char*) &timeout_,
         sizeof(timeout_));
-    wsa_assert (rc != SOCKET_ERROR);
+    tcp_assert_tuning_error(sockfd_, rc);
+    return rc;
 // FIXME: should be ZMQ_HAVE_TCP_USER_TIMEOUT
 #elif defined (TCP_USER_TIMEOUT)
     int rc = setsockopt (sockfd_, IPPROTO_TCP, TCP_USER_TIMEOUT, &timeout_,
         sizeof(timeout_));
-    errno_assert (rc == 0);
+    tcp_assert_tuning_error(sockfd_, rc);
+    return rc;
 #endif
+    return 0;
 }
 
  int zmq::tcp_write (fd_t s_, const void *data_, size_t size_)
@@ -289,3 +299,54 @@ int zmq::tcp_read (fd_t s_, void *data_, size_t size_)
 
 #endif
 }
+
+void zmq::tcp_assert_tuning_error(zmq::fd_t s_, int rc_)
+{
+    if (rc_ == 0)
+        return;
+
+    //  Check whether an error occurred
+    int err = 0;
+#ifdef ZMQ_HAVE_HPUX
+    int len = sizeof err;
+#else
+    socklen_t len = sizeof err;
+#endif
+    
+    int rc = getsockopt (s_, SOL_SOCKET, SO_ERROR, (char*) &err, &len);
+    
+    //  Assert if the error was caused by 0MQ bug.
+    //  Networking problems are OK. No need to assert.
+#ifdef ZMQ_HAVE_WINDOWS
+    zmq_assert (rc == 0);
+    if (err != 0) {
+        wsa_assert (err == WSAECONNREFUSED
+                 || err == WSAETIMEDOUT
+                 || err == WSAECONNABORTED
+                 || err == WSAEHOSTUNREACH
+                 || err == WSAENETUNREACH
+                 || err == WSAENETDOWN
+                 || err == WSAEACCES
+                 || err == WSAEINVAL
+                 || err == WSAEADDRINUSE);
+    }
+#else
+    //  Following code should handle both Berkeley-derived socket
+    //  implementations and Solaris.
+    if (rc == -1)
+        err = errno;
+    if (err != 0) {
+        errno = err;
+        errno_assert (
+            errno == ECONNREFUSED ||
+            errno == ECONNRESET ||
+            errno == ECONNABORTED ||
+            errno == EINTR ||
+            errno == ETIMEDOUT ||
+            errno == EHOSTUNREACH ||
+            errno == ENETUNREACH ||
+            errno == ENETDOWN ||
+            errno == EINVAL);
+    }
+#endif
+}

src/tcp.hpp

@@ -36,20 +36,20 @@ namespace zmq
 {
 
     //  Tunes the supplied TCP socket for the best latency.
-    void tune_tcp_socket (fd_t s_);
+    int tune_tcp_socket (fd_t s_);
 
     //  Sets the socket send buffer size.
-    void set_tcp_send_buffer (fd_t sockfd_, int bufsize_);
+    int set_tcp_send_buffer (fd_t sockfd_, int bufsize_);
 
     //  Sets the socket receive buffer size.
-    void set_tcp_receive_buffer (fd_t sockfd_, int bufsize_);
+    int set_tcp_receive_buffer (fd_t sockfd_, int bufsize_);
 
     //  Tunes TCP keep-alives
-    void tune_tcp_keepalives (fd_t s_, int keepalive_, int keepalive_cnt_,
+    int tune_tcp_keepalives (fd_t s_, int keepalive_, int keepalive_cnt_,
                               int keepalive_idle_, int keepalive_intvl_);
 
     //  Tunes TCP max retransmit timeout
-    void tune_tcp_maxrt (fd_t sockfd_, int timeout_);
+    int tune_tcp_maxrt (fd_t sockfd_, int timeout_);
 
     //  Writes data to the socket. Returns the number of bytes actually
     //  written (even zero is to be considered to be a success). In case
@@ -61,6 +61,9 @@ namespace zmq
     //  Zero indicates the peer has closed the connection.
     int tcp_read (fd_t s_, void *data_, size_t size_);
 
+    //  Asserts that an internal error did not occur.  Does not assert
+    //  on network errors such as reset or aborted connections.
+    void tcp_assert_tuning_error(fd_t s_, int rc_);
 }
 
 #endif

src/tcp_connecter.cpp

@@ -136,6 +136,7 @@ void zmq::tcp_connecter_t::out_event ()
     handle_valid = false;
 
     const fd_t fd = connect ();
+
     //  Handle the error condition by attempt to reconnect.
     if (fd == retired_fd) {
         close ();
@@ -143,10 +144,15 @@ void zmq::tcp_connecter_t::out_event ()
         return;
     }
 
-    tune_tcp_socket (fd);
-    tune_tcp_keepalives (fd, options.tcp_keepalive, options.tcp_keepalive_cnt,
+    int rc = tune_tcp_socket (fd);
+    rc = rc | tune_tcp_keepalives (fd, options.tcp_keepalive, options.tcp_keepalive_cnt,
         options.tcp_keepalive_idle, options.tcp_keepalive_intvl);
-    tune_tcp_maxrt (fd, options.tcp_maxrt);
+    rc = rc | tune_tcp_maxrt (fd, options.tcp_maxrt);
+    if (rc != 0) {
+        close ();
+        add_reconnect_timer ();
+        return;
+    }
 
     //  Create the engine object for this connection.
     stream_engine_t *engine = new (std::nothrow)

src/tcp_listener.cpp

@@ -97,10 +97,14 @@ void zmq::tcp_listener_t::in_event ()
         return;
     }
 
-    tune_tcp_socket (fd);
-    tune_tcp_keepalives (fd, options.tcp_keepalive, options.tcp_keepalive_cnt,
+    int rc = tune_tcp_socket (fd);
+    rc = rc | tune_tcp_keepalives (fd, options.tcp_keepalive, options.tcp_keepalive_cnt,
         options.tcp_keepalive_idle, options.tcp_keepalive_intvl);
-    tune_tcp_maxrt (fd, options.tcp_maxrt);
+    rc = rc | tune_tcp_maxrt (fd, options.tcp_maxrt);
+    if (rc != 0) {
+        socket->event_accept_failed (endpoint, zmq_errno());
+        return;
+    }
 
     //  Create the engine object for this connection.
     stream_engine_t *engine = new (std::nothrow)