Merge pull request #2596 from bluca/zap_curve

Problems: ZAP can be set up incorrectly, CURVE and GSSAPI can be used only with ZAP

src/curve_server.cpp

@@ -490,22 +490,27 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_)
     zmq_assert (rc == 0);
 
     //  Use ZAP protocol (RFC 27) to authenticate the user.
+    //  Note that rc will be -1 only if ZAP is not set up (Stonehouse pattern -
+    //  encryption without authentication), but if it was requested and it does
+    //  not work properly the program will abort.
     rc = session->zap_connect ();
-    if (rc != 0)
-        return -1;
-    rc = send_zap_request (client_key);
-    if (rc != 0)
-        return -1;
-    rc = receive_and_process_zap_reply ();
-    if (rc == 0)
-        state = status_code == "200"
-            ? send_ready
-            : send_error;
+    if (rc == 0) {
+        rc = send_zap_request (client_key);
+        if (rc != 0)
+            return -1;
+        rc = receive_and_process_zap_reply ();
+        if (rc == 0)
+            state = status_code == "200"
+                ? send_ready
+                : send_error;
+        else
+        if (errno == EAGAIN)
+            state = expect_zap_reply;
+        else
+            return -1;
+    }
     else
-    if (errno == EAGAIN)
-        state = expect_zap_reply;
-    else
-        return -1;
+        state = send_ready;
 
     return parse_metadata (initiate_plaintext + crypto_box_ZEROBYTES + 128,
                            clen - crypto_box_ZEROBYTES - 128);

src/gssapi_server.cpp

@@ -120,21 +120,22 @@ int zmq::gssapi_server_t::process_handshake_command (msg_t *msg_)
 
     if (security_context_established) {
         //  Use ZAP protocol (RFC 27) to authenticate the user.
+        //  Note that rc will be -1 only if ZAP is not set up, but if it was
+        //  requested and it does not work properly the program will abort.
+        bool expecting_zap_reply = false;
         int rc = session->zap_connect ();
-        if (rc != 0)
-            return -1;
-        rc = send_zap_request ();
-        if (rc != 0)
-            return -1;
-        rc = receive_and_process_zap_reply ();
-        if (rc == 0)
-            state = send_ready;
-        else
-        if (errno == EAGAIN)
-            state = expect_zap_reply;
-        else
-            return -1;
-
+        if (rc == 0) {
+            rc = send_zap_request ();
+            if (rc != 0)
+                return -1;
+            rc = receive_and_process_zap_reply ();
+            if (rc != 0) {
+                if (errno != EAGAIN)
+                    return -1;
+                expecting_zap_reply = true;
+            }
+        }
+        state = expecting_zap_reply? expect_zap_reply: send_ready;
         return 0;
     }
 

src/plain_server.cpp

@@ -189,6 +189,9 @@ int zmq::plain_server_t::process_hello (msg_t *msg_)
     }
 
     //  Use ZAP protocol (RFC 27) to authenticate the user.
+    //  Note that there is no point to PLAIN if ZAP is not set up to handle the
+    //  username and password, so if ZAP is not configured it is considered a
+    //  failure.
     int rc = session->zap_connect ();
     if (rc != 0)
         return -1;

src/session_base.cpp

@@ -315,6 +315,12 @@ void zmq::session_base_t::process_plug ()
         start_connecting (false);
 }
 
+//  This functions can return 0 on success or -1 and errno=ECONNREFUSED if ZAP
+//  is not setup (IE: inproc://zeromq.zap.01 does not exist in the same context)
+//  or it aborts on any other error. In other words, either ZAP is not
+//  configured or if it is configured it MUST be configured correctly and it
+//  MUST work, otherwise authentication cannot be guaranteed and it would be a
+//  security flaw.
 int zmq::session_base_t::zap_connect ()
 {
     zmq_assert (zap_pipe == NULL);
@@ -324,12 +330,9 @@ int zmq::session_base_t::zap_connect ()
         errno = ECONNREFUSED;
         return -1;
     }
-    if (peer.options.type != ZMQ_REP
-    &&  peer.options.type != ZMQ_ROUTER
-    &&  peer.options.type != ZMQ_SERVER) {
-        errno = ECONNREFUSED;
-        return -1;
-    }
+    zmq_assert (peer.options.type == ZMQ_REP ||
+            peer.options.type == ZMQ_ROUTER ||
+            peer.options.type == ZMQ_SERVER);
 
     //  Create a bi-directional pipe that will connect
     //  session with zap socket.
@@ -353,10 +356,9 @@ int zmq::session_base_t::zap_connect ()
         rc = id.init ();
         errno_assert (rc == 0);
         id.set_flags (msg_t::identity);
-        if (zap_pipe->write (&id))
-            zap_pipe->flush ();
-        else
-            return -1;
+        bool ok = zap_pipe->write (&id);
+        zmq_assert (ok);
+        zap_pipe->flush ();
     }
 
     return 0;

tests/test_heartbeats.cpp

@@ -102,7 +102,32 @@ mock_handshake (raw_socket fd) {
 }
 
 static void
-prep_server_socket(void * ctx, int set_heartbeats, void ** server_out, void ** mon_out,
+setup_curve (void * socket, int is_server) {
+    const char *secret_key;
+    const char *public_key;
+    const char *server_key;
+
+    if (is_server) {
+        secret_key = "JTKVSB%%)wK0E.X)V>+}o?pNmC{O&4W4b!Ni{Lh6";
+        public_key = "rq:rM>}U?@Lns47E1%kR.o@n%FcmmsL/@{H8]yf7";
+        server_key = NULL;
+    }
+    else {
+        secret_key = "D:)Q[IlAW!ahhC2ac:9*A}h:p?([4%wOTJ%JR%cs";
+        public_key = "Yne@$w-vo<fVvi]a<NY6T1ed:M$fCG*[IaLV{hID";
+        server_key = "rq:rM>}U?@Lns47E1%kR.o@n%FcmmsL/@{H8]yf7";
+    }
+
+    zmq_setsockopt (socket, ZMQ_CURVE_SECRETKEY, secret_key, strlen(secret_key));
+    zmq_setsockopt (socket, ZMQ_CURVE_PUBLICKEY, public_key, strlen(public_key));
+    if (is_server)
+        zmq_setsockopt (socket, ZMQ_CURVE_SERVER, &is_server, sizeof(is_server));
+    else
+        zmq_setsockopt (socket, ZMQ_CURVE_SERVERKEY, server_key, strlen(server_key));
+}
+
+static void
+prep_server_socket(void * ctx, int set_heartbeats, int is_curve, void ** server_out, void ** mon_out,
         char *endpoint, size_t ep_length)
 {
     int rc;
@@ -120,6 +145,9 @@ prep_server_socket(void * ctx, int set_heartbeats, void ** server_out, void ** m
         assert (rc == 0);
     }
 
+    if (is_curve)
+        setup_curve(server, 1);
+
     rc = zmq_bind (server, "tcp://127.0.0.1:*");
     assert (rc == 0);
     rc = zmq_getsockopt (server, ZMQ_LAST_ENDPOINT, endpoint, &ep_length);
@@ -155,7 +183,7 @@ test_heartbeat_timeout (void)
     assert (ctx);
 
     void * server, * server_mon;
-    prep_server_socket (ctx, 1, &server, &server_mon, my_endpoint,
+    prep_server_socket (ctx, 1, 0, &server, &server_mon, my_endpoint,
             MAX_SOCKET_STRING);
 
     struct sockaddr_in ip4addr;
@@ -212,7 +240,7 @@ test_heartbeat_ttl (void)
     assert (ctx);
 
     void * server, * server_mon, *client;
-    prep_server_socket (ctx, 0, &server, &server_mon, my_endpoint,
+    prep_server_socket (ctx, 0, 0, &server, &server_mon, my_endpoint,
             MAX_SOCKET_STRING);
 
     client = zmq_socket (ctx, ZMQ_DEALER);
@@ -259,7 +287,7 @@ test_heartbeat_ttl (void)
 // exchanged normally. There should be an accepted event on the server,
 // and then no event afterwards.
 static void
-test_heartbeat_notimeout (void)
+test_heartbeat_notimeout (int is_curve)
 {
     int rc;
     char my_endpoint[MAX_SOCKET_STRING];
@@ -269,10 +297,12 @@ test_heartbeat_notimeout (void)
     assert (ctx);
 
     void * server, * server_mon;
-    prep_server_socket(ctx, 1, &server, &server_mon, my_endpoint,
+    prep_server_socket(ctx, 1, is_curve, &server, &server_mon, my_endpoint,
             MAX_SOCKET_STRING);
 
     void * client = zmq_socket (ctx, ZMQ_DEALER);
+    if (is_curve)
+        setup_curve(client, 0);
     rc = zmq_connect (client, my_endpoint);
 
     // Give it a sec to connect and handshake
@@ -304,5 +334,8 @@ int main (void)
     setup_test_environment ();
     test_heartbeat_timeout ();
     test_heartbeat_ttl ();
-    test_heartbeat_notimeout ();
+    // Run this test without curve
+    test_heartbeat_notimeout (0);
+    // Then rerun it with curve
+    test_heartbeat_notimeout (1);
 }