Merge pull request #3018 from bluca/null_auth_regression

Problem: backward incompatible change to NULL with ZAP

Author: c-rack

src/null_mechanism.cpp

@@ -69,24 +69,27 @@ int zmq::null_mechanism_t::next_handshake_command (msg_t *msg_)
             errno = EAGAIN;
             return -1;
         }
+        //  Given this is a backward-incompatible change, it's behind a socket
+        //  option disabled by default.
         int rc = session->zap_connect ();
-        if (rc == -1) {
+        if (rc == -1 && options.zap_enforce_domain) {
             session->get_socket ()->event_handshake_failed_no_detail (
               session->get_endpoint (), EFAULT);
             return -1;
+        } else if (rc == 0) {
+            send_zap_request ();
+            zap_request_sent = true;
+
+            //  TODO actually, it is quite unlikely that we can read the ZAP
+            //  reply already, but removing this has some strange side-effect
+            //  (probably because the pipe's in_active flag is true until a read
+            //  is attempted)
+            rc = receive_and_process_zap_reply ();
+            if (rc != 0)
+                return -1;
+
+            zap_reply_received = true;
         }
-        send_zap_request ();
-        zap_request_sent = true;
-
-        //  TODO actually, it is quite unlikely that we can read the ZAP
-        //  reply already, but removing this has some strange side-effect
-        //  (probably because the pipe's in_active flag is true until a read
-        //  is attempted)
-        rc = receive_and_process_zap_reply ();
-        if (rc != 0)
-            return -1;
-
-        zap_reply_received = true;
     }
 
     if (zap_reply_received && status_code != "200") {

tests/test_security_null.cpp

@@ -88,22 +88,65 @@ int main (void)
     void *ctx = zmq_ctx_new ();
     assert (ctx);
 
+    //  We first test client/server with a ZAP domain but with no handler
+    //  If there is no handler, libzmq should ignore the ZAP option unless
+    //  ZMQ_ZAP_ENFORCE_DOMAIN is set
+    void *server = zmq_socket (ctx, ZMQ_DEALER);
+    assert (server);
+    void *client = zmq_socket (ctx, ZMQ_DEALER);
+    assert (client);
+    int rc = zmq_setsockopt (server, ZMQ_ZAP_DOMAIN, "TEST", 5);
+    assert (rc == 0);
+    rc = zmq_bind (server, "tcp://127.0.0.1:*");
+    assert (rc == 0);
+    rc = zmq_getsockopt (server, ZMQ_LAST_ENDPOINT, my_endpoint, &len);
+    assert (rc == 0);
+    rc = zmq_connect (client, my_endpoint);
+    assert (rc == 0);
+    bounce (server, client);
+    close_zero_linger (client);
+    close_zero_linger (server);
+
+#ifdef ZMQ_ZAP_ENFORCE_DOMAIN
+    //  Now set ZMQ_ZAP_ENFORCE_DOMAIN which strictly enforces the ZAP
+    //  RFC but is backward-incompatible, now it should fail
+    server = zmq_socket (ctx, ZMQ_DEALER);
+    assert (server);
+    client = zmq_socket (ctx, ZMQ_DEALER);
+    assert (client);
+    int required = 1;
+    rc =
+      zmq_setsockopt (server, ZMQ_ZAP_ENFORCE_DOMAIN, &required, sizeof (int));
+    assert (rc == 0);
+    rc = zmq_setsockopt (server, ZMQ_ZAP_DOMAIN, "TEST", 5);
+    assert (rc == 0);
+    rc = zmq_bind (server, "tcp://127.0.0.1:*");
+    assert (rc == 0);
+    rc = zmq_getsockopt (server, ZMQ_LAST_ENDPOINT, my_endpoint, &len);
+    assert (rc == 0);
+    rc = zmq_connect (client, my_endpoint);
+    assert (rc == 0);
+    expect_bounce_fail (server, client);
+    close_zero_linger (client);
+    close_zero_linger (server);
+#endif
+
     //  Spawn ZAP handler
     //  We create and bind ZAP socket in main thread to avoid case
     //  where child thread does not start up fast enough.
     void *handler = zmq_socket (ctx, ZMQ_REP);
     assert (handler);
-    int rc = zmq_bind (handler, "inproc://zeromq.zap.01");
+    rc = zmq_bind (handler, "inproc://zeromq.zap.01");
     assert (rc == 0);
     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);
+    server = zmq_socket (ctx, ZMQ_DEALER);
     assert (server);
-    void *client = zmq_socket (ctx, ZMQ_DEALER);
+    client = zmq_socket (ctx, ZMQ_DEALER);
     assert (client);
     rc = zmq_bind (server, "tcp://127.0.0.1:*");
     assert (rc == 0);

tests/test_security_zap.cpp

@@ -326,10 +326,12 @@ void test_zap_errors (socket_config_fn server_socket_config_,
 
 #ifdef ZMQ_ZAP_ENFORCE_DOMAIN
     //  no ZAP handler
+    int enforce = 1;
     fprintf (stderr, "test_zap_unsuccessful no ZAP handler started\n");
-    setup_context_and_server_side (&ctx, &handler, &zap_thread, &server,
-                                   &server_mon, my_endpoint, NULL,
-                                   server_socket_config_);
+    setup_context_and_server_side (
+      &ctx, &handler, &zap_thread, &server, &server_mon, my_endpoint, NULL,
+      server_socket_config_,
+      server_socket_config_data_ ? server_socket_config_data_ : &enforce);
     test_zap_unsuccessful_no_handler (
       ctx, my_endpoint, server, server_mon,
 #ifdef ZMQ_BUILD_DRAFT_API

tests/testutil_security.hpp

@@ -47,11 +47,17 @@ void socket_config_null_client (void *server, void *server_secret)
 
 void socket_config_null_server (void *server, void *server_secret)
 {
-    LIBZMQ_UNUSED (server_secret);
-
     int rc = zmq_setsockopt (server, ZMQ_ZAP_DOMAIN, test_zap_domain,
                              strlen (test_zap_domain));
     assert (rc == 0);
+#ifdef ZMQ_ZAP_ENFORCE_DOMAIN
+    int required = server_secret ? *(int *) server_secret : 0;
+    rc =
+      zmq_setsockopt (server, ZMQ_ZAP_ENFORCE_DOMAIN, &required, sizeof (int));
+    assert (rc == 0);
+#else
+    LIBZMQ_UNUSED (server_secret);
+#endif
 }
 
 //  PLAIN specific functions