Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request #101 from hintjens/master
Problem: issue #1273, protocol downgrade attack
  • Loading branch information
hintjens committed Dec 5, 2014
2 parents f6968ec + 77ef79e commit b6e3e0f
Show file tree
Hide file tree
Showing 7 changed files with 197 additions and 57 deletions.
4 changes: 3 additions & 1 deletion NEWS
@@ -1,6 +1,8 @@
0MQ version 4.0.6 stable, released on 2015/xx/xx
0MQ version 4.0.6 stable, released on 2015/12/xx
================================================

* Fixed #1273 - V3 protocol handler vulnerable to downgrade attacks.


0MQ version 4.0.5 stable, released on 2014/10/14
================================================
Expand Down
8 changes: 8 additions & 0 deletions src/session_base.cpp
Expand Up @@ -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);
Expand Down
3 changes: 2 additions & 1 deletion src/session_base.hpp
Expand Up @@ -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.
Expand Down
15 changes: 15 additions & 0 deletions src/stream_engine.cpp
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand All @@ -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);

Expand Down
66 changes: 58 additions & 8 deletions tests/test_security_curve.cpp
Expand Up @@ -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
Expand All @@ -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"));
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
121 changes: 75 additions & 46 deletions 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.
Expand All @@ -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)
Expand All @@ -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);
Expand Down Expand Up @@ -57,7 +69,7 @@ zap_handler (void *handler)
free (identity);
free (mechanism);
}
zmq_close (handler);
close_zero_linger (handler);
}

int main (void)
Expand All @@ -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;
Expand Down
37 changes: 36 additions & 1 deletion 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.
Expand All @@ -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)
Expand Down Expand Up @@ -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);
Expand Down

3 comments on commit b6e3e0f

@jlec
Copy link

@jlec jlec commented on b6e3e0f Jun 4, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a series 4 only issue? Or also present in series 3?

@hintjens
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

V3.x does not have security mechanisms, so this issue is not present.

@jlec
Copy link

@jlec jlec commented on b6e3e0f Jun 5, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hintjens thanks for confirmation.

Please sign in to comment.