431 lines
14 KiB
Diff
431 lines
14 KiB
Diff
|
From 77ef79e3b565f120172c6d1c30fabec6185553da Mon Sep 17 00:00:00 2001
|
||
|
From: Pieter Hintjens <ph@imatix.com>
|
||
|
Date: Fri, 5 Dec 2014 09:07:37 +0100
|
||
|
Subject: [PATCH] Problem: issue #1273, protocol downgrade attack
|
||
|
|
||
|
Solution: backport fix from libzmq master. Also backported test
|
||
|
cases.
|
||
|
---
|
||
|
NEWS | 4 +-
|
||
|
src/session_base.cpp | 8 +++
|
||
|
src/session_base.hpp | 3 +-
|
||
|
src/stream_engine.cpp | 15 ++++++
|
||
|
tests/test_security_curve.cpp | 66 ++++++++++++++++++++---
|
||
|
tests/test_security_null.cpp | 121 ++++++++++++++++++++++++++----------------
|
||
|
tests/test_security_plain.cpp | 37 ++++++++++++-
|
||
|
7 files changed, 197 insertions(+), 57 deletions(-)
|
||
|
|
||
|
diff --git a/src/session_base.cpp b/src/session_base.cpp
|
||
|
index 537dcb3..0b58436 100644
|
||
|
--- a/src/session_base.cpp
|
||
|
+++ b/src/session_base.cpp
|
||
|
@@ -323,6 +323,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);
|
||
|
diff --git a/src/session_base.hpp b/src/session_base.hpp
|
||
|
index 2ef7dc5..63e16bd 100644
|
||
|
--- a/src/session_base.hpp
|
||
|
+++ b/src/session_base.hpp
|
||
|
@@ -68,7 +68,8 @@ 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
|
||
|
// longer used.
|
||
|
diff --git a/src/stream_engine.cpp b/src/stream_engine.cpp
|
||
|
index 4d252d8..3d84d8f 100644
|
||
|
--- a/src/stream_engine.cpp
|
||
|
+++ b/src/stream_engine.cpp
|
||
|
@@ -464,6 +464,11 @@ 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 ();
|
||
|
+ return false;
|
||
|
+ }
|
||
|
encoder = new (std::nothrow) v1_encoder_t (out_batch_size);
|
||
|
alloc_assert (encoder);
|
||
|
|
||
|
@@ -505,6 +510,11 @@ 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 ();
|
||
|
+ return false;
|
||
|
+ }
|
||
|
encoder = new (std::nothrow) v1_encoder_t (
|
||
|
out_batch_size);
|
||
|
alloc_assert (encoder);
|
||
|
@@ -515,6 +525,11 @@ bool zmq::stream_engine_t::handshake ()
|
||
|
}
|
||
|
else
|
||
|
if (greeting_recv [revision_pos] == ZMTP_2_0) {
|
||
|
+ if (session->zap_enabled ()) {
|
||
|
+ // Reject ZMTP 1.0 connections if ZAP is enabled
|
||
|
+ error ();
|
||
|
+ return false;
|
||
|
+ }
|
||
|
encoder = new (std::nothrow) v2_encoder_t (out_batch_size);
|
||
|
alloc_assert (encoder);
|
||
|
|
||
|
diff --git a/tests/test_security_curve.cpp b/tests/test_security_curve.cpp
|
||
|
index a24466f..e99a4b3 100644
|
||
|
--- a/tests/test_security_curve.cpp
|
||
|
+++ b/tests/test_security_curve.cpp
|
||
|
@@ -18,12 +18,23 @@
|
||
|
*/
|
||
|
|
||
|
#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 [40];
|
||
|
-static char client_secret [40];
|
||
|
-static char server_public [40];
|
||
|
-static char server_secret [40];
|
||
|
+static char client_public [41];
|
||
|
+static char client_secret [41];
|
||
|
+static char server_public [41];
|
||
|
+static char server_secret [41];
|
||
|
|
||
|
// --------------------------------------------------------------------------
|
||
|
// This methods receives and validates ZAP requestes (allowing or denying
|
||
|
@@ -46,7 +57,7 @@ static void zap_handler (void *handler)
|
||
|
int size = zmq_recv (handler, client_key, 32, 0);
|
||
|
assert (size == 32);
|
||
|
|
||
|
- char client_key_text [40];
|
||
|
+ char client_key_text [41];
|
||
|
zmq_z85_encode (client_key_text, client_key, 32);
|
||
|
|
||
|
assert (streq (version, "1.0"));
|
||
|
@@ -181,8 +192,8 @@ int main (void)
|
||
|
|
||
|
// Check CURVE security with bogus client credentials
|
||
|
// This must be caught by the ZAP handler
|
||
|
- char bogus_public [40];
|
||
|
- char bogus_secret [40];
|
||
|
+ char bogus_public [41];
|
||
|
+ char bogus_secret [41];
|
||
|
zmq_curve_keypair (bogus_public, bogus_secret);
|
||
|
|
||
|
client = zmq_socket (ctx, ZMQ_DEALER);
|
||
|
@@ -217,7 +228,46 @@ int main (void)
|
||
|
assert (rc == 0);
|
||
|
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);
|
||
|
diff --git a/tests/test_security_null.cpp b/tests/test_security_null.cpp
|
||
|
index 8a55632..6b74e8c 100644
|
||
|
--- a/tests/test_security_null.cpp
|
||
|
+++ b/tests/test_security_null.cpp
|
||
|
@@ -1,5 +1,5 @@
|
||
|
/*
|
||
|
- Copyright (c) 2007-2013 Contributors as noted in the AUTHORS file
|
||
|
+ Copyright (c) 2007-2014 Contributors as noted in the AUTHORS file
|
||
|
|
||
|
This file is part of 0MQ.
|
||
|
|
||
|
@@ -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)
|
||
|
@@ -27,6 +38,7 @@ zap_handler (void *handler)
|
||
|
char *version = s_recv (handler);
|
||
|
if (!version)
|
||
|
break; // Terminating
|
||
|
+
|
||
|
char *sequence = s_recv (handler);
|
||
|
char *domain = s_recv (handler);
|
||
|
char *address = s_recv (handler);
|
||
|
@@ -57,7 +69,7 @@ zap_handler (void *handler)
|
||
|
free (identity);
|
||
|
free (mechanism);
|
||
|
}
|
||
|
- zmq_close (handler);
|
||
|
+ close_zero_linger (handler);
|
||
|
}
|
||
|
|
||
|
int main (void)
|
||
|
@@ -76,72 +88,89 @@ int main (void)
|
||
|
void *zap_thread = zmq_threadstart (&zap_handler, handler);
|
||
|
|
||
|
// We bounce between a binding server and a connecting client
|
||
|
+
|
||
|
+ // We first test client/server with no ZAP domain
|
||
|
+ // Libzmq does not call our ZAP handler, the connect must succeed
|
||
|
void *server = zmq_socket (ctx, ZMQ_DEALER);
|
||
|
assert (server);
|
||
|
void *client = zmq_socket (ctx, ZMQ_DEALER);
|
||
|
assert (client);
|
||
|
-
|
||
|
- // We first test client/server with no ZAP domain
|
||
|
- // Libzmq does not call our ZAP handler, the connect must succeed
|
||
|
rc = zmq_bind (server, "tcp://127.0.0.1:9000");
|
||
|
assert (rc == 0);
|
||
|
- rc = zmq_connect (client, "tcp://localhost:9000");
|
||
|
+ rc = zmq_connect (client, "tcp://127.0.0.1:9000");
|
||
|
assert (rc == 0);
|
||
|
bounce (server, client);
|
||
|
- zmq_unbind (server, "tcp://127.0.0.1:9000");
|
||
|
- zmq_disconnect (client, "tcp://localhost:9000");
|
||
|
-
|
||
|
+ close_zero_linger (client);
|
||
|
+ close_zero_linger (server);
|
||
|
+
|
||
|
// Now define a ZAP domain for the server; this enables
|
||
|
// authentication. We're using the wrong domain so this test
|
||
|
// must fail.
|
||
|
- // **************************************************************
|
||
|
- // PH: the following causes libzmq to get confused, so that the
|
||
|
- // next step fails. To reproduce, uncomment this block. Note that
|
||
|
- // even creating a new client/server socket pair, the behaviour
|
||
|
- // does not change.
|
||
|
- // **************************************************************
|
||
|
- // Destroying the old sockets and creating new ones isn't needed,
|
||
|
- // but it shows that the problem isn't related to specific sockets.
|
||
|
- //close_zero_linger (client);
|
||
|
- //close_zero_linger (server);
|
||
|
- //server = zmq_socket (ctx, ZMQ_DEALER);
|
||
|
- //assert (server);
|
||
|
- //client = zmq_socket (ctx, ZMQ_DEALER);
|
||
|
- //assert (client);
|
||
|
- //// The above code should not be required
|
||
|
- //rc = zmq_setsockopt (server, ZMQ_ZAP_DOMAIN, "WRONG", 5);
|
||
|
- //assert (rc == 0);
|
||
|
- //rc = zmq_bind (server, "tcp://127.0.0.1:9001");
|
||
|
- //assert (rc == 0);
|
||
|
- //rc = zmq_connect (client, "tcp://localhost:9001");
|
||
|
- //assert (rc == 0);
|
||
|
- //expect_bounce_fail (server, client);
|
||
|
- //zmq_unbind (server, "tcp://127.0.0.1:9001");
|
||
|
- //zmq_disconnect (client, "tcp://localhost:9001");
|
||
|
-
|
||
|
+ server = zmq_socket (ctx, ZMQ_DEALER);
|
||
|
+ assert (server);
|
||
|
+ client = zmq_socket (ctx, ZMQ_DEALER);
|
||
|
+ assert (client);
|
||
|
+ rc = zmq_setsockopt (server, ZMQ_ZAP_DOMAIN, "WRONG", 5);
|
||
|
+ assert (rc == 0);
|
||
|
+ rc = zmq_bind (server, "tcp://127.0.0.1:9001");
|
||
|
+ assert (rc == 0);
|
||
|
+ rc = zmq_connect (client, "tcp://127.0.0.1:9001");
|
||
|
+ assert (rc == 0);
|
||
|
+ expect_bounce_fail (server, client);
|
||
|
+ close_zero_linger (client);
|
||
|
+ close_zero_linger (server);
|
||
|
+
|
||
|
// Now use the right domain, the test must pass
|
||
|
+ server = zmq_socket (ctx, ZMQ_DEALER);
|
||
|
+ assert (server);
|
||
|
+ client = zmq_socket (ctx, ZMQ_DEALER);
|
||
|
+ assert (client);
|
||
|
rc = zmq_setsockopt (server, ZMQ_ZAP_DOMAIN, "TEST", 4);
|
||
|
assert (rc == 0);
|
||
|
rc = zmq_bind (server, "tcp://127.0.0.1:9002");
|
||
|
assert (rc == 0);
|
||
|
- rc = zmq_connect (client, "tcp://localhost:9002");
|
||
|
+ rc = zmq_connect (client, "tcp://127.0.0.1:9002");
|
||
|
assert (rc == 0);
|
||
|
- // **************************************************************
|
||
|
- // PH: it fails here; though the ZAP reply is 200 OK, and
|
||
|
- // null_mechanism.cpp correctly parses that, the connection
|
||
|
- // never succeeds and the test hangs.
|
||
|
- // **************************************************************
|
||
|
bounce (server, client);
|
||
|
- zmq_unbind (server, "tcp://127.0.0.1:9002");
|
||
|
- zmq_disconnect (client, "tcp://localhost:9002");
|
||
|
-
|
||
|
- // Shutdown
|
||
|
close_zero_linger (client);
|
||
|
close_zero_linger (server);
|
||
|
- rc = zmq_ctx_term (ctx);
|
||
|
+
|
||
|
+ // 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);
|
||
|
|
||
|
- // Wait until ZAP handler terminates.
|
||
|
+ 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);
|
||
|
+ // Wait until ZAP handler terminates
|
||
|
zmq_threadclose (zap_thread);
|
||
|
|
||
|
return 0;
|
||
|
diff --git a/tests/test_security_plain.cpp b/tests/test_security_plain.cpp
|
||
|
index 74973fd..c257840 100644
|
||
|
--- a/tests/test_security_plain.cpp
|
||
|
+++ b/tests/test_security_plain.cpp
|
||
|
@@ -1,5 +1,5 @@
|
||
|
/*
|
||
|
- Copyright (c) 2007-2013 Contributors as noted in the AUTHORS file
|
||
|
+ Copyright (c) 2007-2014 Contributors as noted in the AUTHORS file
|
||
|
|
||
|
This file is part of 0MQ.
|
||
|
|
||
|
@@ -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);
|