Merge pull request #10 from hintjens/master

Backported fix for #1273 (protocol downgrade attack)

.gitignore

@@ -22,84 +22,84 @@ autom4te.cache
 *~
 .*~
 tools/curve_keygen
-tests/test_resource
-tests/test_ipc_wildcard
-tests/test_stream_empty
-tests/test_stream_timeout
-tests/test_issue_566
-tests/test_ctx_destroy
-tests/test_term_endpoint
-tests/test_system
-tests/test_monitor
-tests/test_last_endpoint
-tests/test_pair_inproc
-tests/test_pair_ipc
-tests/test_pair_tcp
-tests/test_reqrep_inproc
-tests/test_reqrep_ipc
-tests/test_reqrep_tcp
-tests/test_shutdown_stress
-tests/test_hwm
-tests/test_timeo
-tests/test_reqrep_device
-tests/test_reqrep_drop
-tests/test_sub_forward
-tests/test_invalid_rep
-tests/test_msg_flags
-tests/test_ts_context
-tests/test_connect_resolve
-tests/test_immediate
-tests/test_term_endpoint
-tests/test_router_mandatory
-tests/test_disconnect_inproc
-tests/test_raw_sock
-tests/test_disconnect_inproc
-tests/test_ctx_options
-tests/test_iov
-tests/test_security
-tests/test_security_curve
-tests/test_probe_router
-tests/test_stream
-tests/test_spec_dealer
-tests/test_spec_pushpull
-tests/test_spec_rep
-tests/test_spec_req
-tests/test_spec_router
-tests/test_req_correlate
-tests/test_req_relaxed
-tests/test_fork
-tests/test_conflate
-tests/test_inproc_connect
-tests/test_linger
-tests/test_security_null
-tests/test_security_plain
-tests/test_proxy
-tests/test_abstract_ipc
-tests/test_filter_ipc
-tests/test_connect_delay_tipc
-tests/test_pair_tipc
-tests/test_reqrep_device_tipc
-tests/test_reqrep_tipc
-tests/test_router_handover
-tests/test_router_mandatory_tipc
-tests/test_shutdown_stress_tipc
-tests/test_sub_forward_tipc
-tests/test_term_endpoint_tipc
-tests/test_many_sockets
-tests/test_diffserv
-tests/test_connect_rid
-tests/test_srcfd
-tests/test_stream_disconnect
-tests/test_proxy_chain
-tests/test_bind_src_address
-tests/test_metadata
-tests/test_id2fd
-tests/test_capabilities
-tests/test_hwm_pubsub
-tests/test_router_mandatory_hwm
-tests/test_xpub_nodrop
-tests/test*.log
-tests/test*.trs
+test_resource
+test_ipc_wildcard
+test_stream_empty
+test_stream_timeout
+test_issue_566
+test_ctx_destroy
+test_term_endpoint
+test_system
+test_monitor
+test_last_endpoint
+test_pair_inproc
+test_pair_ipc
+test_pair_tcp
+test_reqrep_inproc
+test_reqrep_ipc
+test_reqrep_tcp
+test_shutdown_stress
+test_hwm
+test_timeo
+test_reqrep_device
+test_reqrep_drop
+test_sub_forward
+test_invalid_rep
+test_msg_flags
+test_ts_context
+test_connect_resolve
+test_immediate
+test_term_endpoint
+test_router_mandatory
+test_disconnect_inproc
+test_raw_sock
+test_disconnect_inproc
+test_ctx_options
+test_iov
+test_security
+test_security_curve
+test_probe_router
+test_stream
+test_spec_dealer
+test_spec_pushpull
+test_spec_rep
+test_spec_req
+test_spec_router
+test_req_correlate
+test_req_relaxed
+test_fork
+test_conflate
+test_inproc_connect
+test_linger
+test_security_null
+test_security_plain
+test_proxy
+test_abstract_ipc
+test_filter_ipc
+test_connect_delay_tipc
+test_pair_tipc
+test_reqrep_device_tipc
+test_reqrep_tipc
+test_router_handover
+test_router_mandatory_tipc
+test_shutdown_stress_tipc
+test_sub_forward_tipc
+test_term_endpoint_tipc
+test_many_sockets
+test_diffserv
+test_connect_rid
+test_srcfd
+test_stream_disconnect
+test_proxy_chain
+test_bind_src_address
+test_metadata
+test_id2fd
+test_capabilities
+test_hwm_pubsub
+test_router_mandatory_hwm
+test_xpub_nodrop
+test*.log
+test*.trs
 src/platform.hpp*
 src/stamp-h1
 perf/local_lat
@@ -133,3 +133,10 @@ zeromq-*.tar.gz
 zeromq-*.zip
 core
 build
+curve_keygen
+inproc_lat
+inproc_thr
+local_lat
+local_thr
+remote_lat
+remote_thr

NEWS

@@ -1,11 +1,13 @@
-0MQ version 4.1.1 rc2, released on 2014/10/xx
+0MQ version 4.1.1 rc2, released on 2014/12/xx
 =============================================
 
-* #1208 - fix recursion in automake packaging
+* Fixed #1208 - fix recursion in automake packaging
 
-* #1224 - crash when processing empty unsubscribe message.
+* Fixed #1224 - crash when processing empty unsubscribe message.
 
-* #1213 - properties files were missing from source packages.
+* Fixed #1213 - properties files were missing from source packages.
+
+* Fixed #1273 - V3 protocol handler vulnerable to downgrade attacks.
 
 
 0MQ version 4.1.0 rc1, released on 2014/10/14

configure.ac

@@ -394,7 +394,7 @@ have_sodium_library="no"
 AC_ARG_WITH([libsodium], [AS_HELP_STRING([--with-libsodium],
     [require libzmq build with libsodium. Requires pkg-config [default=no]])],
     [require_libsodium_ext=$withval],
-    [require_libsodium_ext=no])
+    [require_libsodium_ext=yes])
 
 # conditionally require libsodium package
 if test "x$require_libsodium_ext" != "xno"; then

src/session_base.cpp

@@ -331,6 +331,14 @@ int zmq::session_base_t::zap_connect ()
     return 0;
 }
 
+bool zmq::session_base_t::zap_enabled ()
+{
+    return (
+         options.mechanism != ZMQ_NULL ||
+        (options.mechanism == ZMQ_NULL && options.zap_domain.length() > 0)
+    );
+}
+
 void zmq::session_base_t::process_attach (i_engine *engine_)
 {
     zmq_assert (engine_ != NULL);

src/session_base.hpp

@@ -69,6 +69,7 @@ namespace zmq
         int push_msg (msg_t *msg_);
 
         int zap_connect ();
+        bool zap_enabled ();
 
         //  Fetches a message. Returns 0 if successful; -1 otherwise.
         //  The caller is responsible for freeing the message when no

src/stream_engine.cpp

@@ -534,6 +534,12 @@ bool zmq::stream_engine_t::handshake ()
     //  Is the peer using ZMTP/1.0 with no revision number?
     //  If so, we send and receive rest of identity message
     if (greeting_recv [0] != 0xff || !(greeting_recv [9] & 0x01)) {
+        if (session->zap_enabled ()) {
+           // reject ZMTP 1.0 connections if ZAP is enabled
+           error (protocol_error);
+           return false;
+        }
+
         encoder = new (std::nothrow) v1_encoder_t (out_batch_size);
         alloc_assert (encoder);
 
@@ -575,6 +581,12 @@ bool zmq::stream_engine_t::handshake ()
     }
     else
     if (greeting_recv [revision_pos] == ZMTP_1_0) {
+        if (session->zap_enabled ()) {
+           // reject ZMTP 1.0 connections if ZAP is enabled
+           error (protocol_error);
+           return false;
+        }
+
         encoder = new (std::nothrow) v1_encoder_t (
             out_batch_size);
         alloc_assert (encoder);
@@ -585,6 +597,12 @@ bool zmq::stream_engine_t::handshake ()
     }
     else
     if (greeting_recv [revision_pos] == ZMTP_2_0) {
+        if (session->zap_enabled ()) {
+           // reject ZMTP 2.0 connections if ZAP is enabled
+           error (protocol_error);
+           return false;
+        }
+
         encoder = new (std::nothrow) v2_encoder_t (out_batch_size);
         alloc_assert (encoder);
 

tests/test_security_curve.cpp

@@ -18,6 +18,17 @@
 */
 
 #include "testutil.hpp"
+#if defined (ZMQ_HAVE_WINDOWS)
+#   include <winsock2.h>
+#   include <ws2tcpip.h>
+#   include <stdexcept>
+#   define close closesocket
+#else
+#   include <sys/socket.h>
+#   include <netinet/in.h>
+#   include <arpa/inet.h>
+#   include <unistd.h>
+#endif
 
 //  We'll generate random test keys at startup
 static char client_public [41];
@@ -218,6 +229,45 @@ int main (void)
     expect_bounce_fail (server, client);
     close_zero_linger (client);
     
+    // Unauthenticated messages from a vanilla socket shouldn't be received
+    struct sockaddr_in ip4addr;
+    int s;
+
+    ip4addr.sin_family = AF_INET;
+    ip4addr.sin_port = htons (9998);
+    inet_pton (AF_INET, "127.0.0.1", &ip4addr.sin_addr);
+
+    s = socket (AF_INET, SOCK_STREAM, IPPROTO_TCP);
+    rc = connect (s, (struct sockaddr*) &ip4addr, sizeof (ip4addr));
+    assert (rc > -1);
+    // send anonymous ZMTP/1.0 greeting
+    send (s, "\x01\x00", 2, 0);
+    // send sneaky message that shouldn't be received
+    send (s, "\x08\x00sneaky\0", 9, 0);
+    int timeout = 150;
+    zmq_setsockopt (server, ZMQ_RCVTIMEO, &timeout, sizeof (timeout));
+    char *buf = s_recv (server);
+    if (buf != NULL) {
+        printf ("Received unauthenticated message: %s\n", buf);
+        assert (buf == NULL);
+    }
+    close (s);
+
+    //  Check return codes for invalid buffer sizes
+    client = zmq_socket (ctx, ZMQ_DEALER);
+    assert (client);
+    errno = 0;
+    rc = zmq_setsockopt (client, ZMQ_CURVE_SERVERKEY, server_public, 123);
+    assert (rc == -1 && errno == EINVAL);
+    errno = 0;
+    rc = zmq_setsockopt (client, ZMQ_CURVE_PUBLICKEY, client_public, 123);
+    assert (rc == -1 && errno == EINVAL);
+    errno = 0;
+    rc = zmq_setsockopt (client, ZMQ_CURVE_SECRETKEY, client_secret, 123);
+    assert (rc == -1 && errno == EINVAL);
+    rc = zmq_close (client);
+    assert (rc == 0);
+
     //  Shutdown
     rc = zmq_close (server);
     assert (rc == 0);

tests/test_security_null.cpp

@@ -18,6 +18,17 @@
 */
 
 #include "testutil.hpp"
+#if defined (ZMQ_HAVE_WINDOWS)
+#   include <winsock2.h>
+#   include <ws2tcpip.h>
+#   include <stdexcept>
+#   define close closesocket
+#else
+#   include <sys/socket.h>
+#   include <netinet/in.h>
+#   include <arpa/inet.h>
+#   include <unistd.h>
+#endif
 
 static void
 zap_handler (void *handler)
@@ -124,6 +135,38 @@ int main (void)
     close_zero_linger (client);
     close_zero_linger (server);
 
+    // Unauthenticated messages from a vanilla socket shouldn't be received
+    server = zmq_socket (ctx, ZMQ_DEALER);
+    assert (server);
+    rc = zmq_setsockopt (server, ZMQ_ZAP_DOMAIN, "WRONG", 5);
+    assert (rc == 0);
+    rc = zmq_bind (server, "tcp://127.0.0.1:9003");
+    assert (rc == 0);
+
+    struct sockaddr_in ip4addr;
+    int s;
+
+    ip4addr.sin_family = AF_INET;
+    ip4addr.sin_port = htons(9003);
+    inet_pton(AF_INET, "127.0.0.1", &ip4addr.sin_addr);
+
+    s = socket (AF_INET, SOCK_STREAM, IPPROTO_TCP);
+    rc = connect (s, (struct sockaddr*) &ip4addr, sizeof ip4addr);
+    assert (rc > -1);
+    // send anonymous ZMTP/1.0 greeting
+    send (s, "\x01\x00", 2, 0);
+    // send sneaky message that shouldn't be received
+    send (s, "\x08\x00sneaky\0", 9, 0);
+    int timeout = 150;
+    zmq_setsockopt (server, ZMQ_RCVTIMEO, &timeout, sizeof (timeout));
+    char *buf = s_recv (server);
+    if (buf != NULL) {
+        printf ("Received unauthenticated message: %s\n", buf);
+        assert (buf == NULL);
+    }
+    close (s);
+    close_zero_linger (server);
+
     //  Shutdown
     rc = zmq_ctx_term (ctx);
     assert (rc == 0);

tests/test_security_plain.cpp

@@ -18,6 +18,17 @@
 */
 
 #include "testutil.hpp"
+#if defined (ZMQ_HAVE_WINDOWS)
+#   include <winsock2.h>
+#   include <ws2tcpip.h>
+#   include <stdexcept>
+#   define close closesocket
+#else
+#   include <sys/socket.h>
+#   include <netinet/in.h>
+#   include <arpa/inet.h>
+#   include <unistd.h>
+#endif
 
 static void
 zap_handler (void *ctx)
@@ -137,6 +148,30 @@ int main (void)
     expect_bounce_fail (server, client);
     close_zero_linger (client);
 
+    // Unauthenticated messages from a vanilla socket shouldn't be received
+    struct sockaddr_in ip4addr;
+    int s;
+
+    ip4addr.sin_family = AF_INET;
+    ip4addr.sin_port = htons (9998);
+    inet_pton (AF_INET, "127.0.0.1", &ip4addr.sin_addr);
+
+    s = socket (AF_INET, SOCK_STREAM, IPPROTO_TCP);
+    rc = connect (s, (struct sockaddr*) &ip4addr, sizeof (ip4addr));
+    assert (rc > -1);
+    // send anonymous ZMTP/1.0 greeting
+    send (s, "\x01\x00", 2, 0);
+    // send sneaky message that shouldn't be received
+    send (s, "\x08\x00sneaky\0", 9, 0);
+    int timeout = 150;
+    zmq_setsockopt (server, ZMQ_RCVTIMEO, &timeout, sizeof (timeout));
+    char *buf = s_recv (server);
+    if (buf != NULL) {
+        printf ("Received unauthenticated message: %s\n", buf);
+        assert (buf == NULL);
+    }
+    close (s);
+
     //  Shutdown
     rc = zmq_close (server);
     assert (rc == 0);