Problem: no tests for ZAP handler that has an invalid protocol (#2648)

* Problem: no tests for ZAP protocol errors

Solution: added first test for a bogus version number in ZAP reply

* Problem: no tests for ZAP protocol errors

Solution: added more test cases

* Problem: cannot compile without ZMQ_BUILD_DRAFT_API

Solution: conditionally compile parts that depend on draft API

* Problem: test_security_curve times out in CI

Solution: Increase timeout for this test to 20 seconds

tests/CMakeLists.txt

@@ -169,6 +169,9 @@ foreach(test ${tests})
   set_tests_properties(${test} PROPERTIES TIMEOUT 10)
 endforeach()
 
+#override timeout for these tests
+set_tests_properties(test_security_curve PROPERTIES TIMEOUT 20)
+
 #Check whether all tests in the current folder are present
 file(READ "${CMAKE_CURRENT_LIST_FILE}" CURRENT_LIST_FILE_CONTENT)
 file(GLOB ALL_TEST_SOURCES "test_*.cpp")

tests/test_security_curve.cpp

@@ -134,7 +134,19 @@ int get_monitor_event_with_timeout (void *monitor,
 //  This methods receives and validates ZAP requests (allowing or denying
 //  each client connection).
 
-static void zap_handler (void *handler)
+enum zap_protocol_t
+{
+  zap_ok,
+  // ZAP-compliant non-standard cases
+  zap_status_internal_error,
+  // ZAP protocol errors
+  zap_wrong_version,
+  zap_wrong_request_id,
+  zap_status_invalid,
+  zap_too_many_parts
+};
+
+static void zap_handler_generic (void *handler, zap_protocol_t zap_protocol)
 {
     //  Process ZAP requests forever
     while (true) {
@@ -158,13 +170,24 @@ static void zap_handler (void *handler)
         assert (streq (mechanism, "CURVE"));
         assert (streq (identity, "IDENT"));
 
-        s_sendmore (handler, version);
-        s_sendmore (handler, sequence);
+        s_sendmore (handler, zap_protocol == zap_wrong_version
+                               ? "invalid_version"
+                               : version);
+        s_sendmore (handler, zap_protocol == zap_wrong_request_id
+                               ? "invalid_request_id"
+                               : sequence);
 
         if (streq (client_key_text, client_public)) {
-            s_sendmore (handler, "200");
+            s_sendmore (handler, zap_protocol == zap_status_internal_error
+                                   ? "500"
+                                   : (zap_protocol == zap_status_invalid
+                                        ? "invalid_status"
+                                        : "200"));
             s_sendmore (handler, "OK");
             s_sendmore (handler, "anonymous");
+            if (zap_protocol == zap_too_many_parts) {
+                s_sendmore (handler, "");
+            }
             s_send (handler, "");
         } else {
             s_sendmore (handler, "400");
@@ -182,6 +205,36 @@ static void zap_handler (void *handler)
     zmq_close (handler);
 }
 
+static void zap_handler (void *handler)
+{
+  zap_handler_generic (handler, zap_ok);
+}
+
+static void zap_handler_wrong_version (void *handler)
+{
+  zap_handler_generic (handler, zap_wrong_version);
+}
+
+static void zap_handler_wrong_request_id (void *handler)
+{
+  zap_handler_generic (handler, zap_wrong_request_id);
+}
+
+static void zap_handler_wrong_status_invalid (void *handler)
+{
+  zap_handler_generic (handler, zap_status_invalid);
+}
+
+static void zap_handler_wrong_status_internal_error (void *handler)
+{
+  zap_handler_generic (handler, zap_status_internal_error);
+}
+
+static void zap_handler_too_many_parts (void *handler)
+{
+  zap_handler_generic (handler, zap_too_many_parts);
+}
+
 void test_garbage_key (void *ctx,
                        void *server,
                        void *server_mon,
@@ -273,7 +326,8 @@ void setup_context_and_server_side (void **ctx,
                                     void **zap_thread,
                                     void **server,
                                     void **server_mon,
-                                    char *my_endpoint)
+                                    char *my_endpoint,
+                                    zmq_thread_fn zap_handler_ = &zap_handler)
 {
     *ctx = zmq_ctx_new ();
     assert (*ctx);
@@ -285,7 +339,7 @@ void setup_context_and_server_side (void **ctx,
     assert (*handler);
     int rc = zmq_bind (*handler, "inproc://zeromq.zap.01");
     assert (rc == 0);
-    *zap_thread = zmq_threadstart (&zap_handler, *handler);
+    *zap_thread = zmq_threadstart (zap_handler_, *handler);
 
     //  Server socket will accept connections
     *server = zmq_socket (*ctx, ZMQ_DEALER);
@@ -475,6 +529,62 @@ void test_curve_security_unauthenticated_message (char *my_endpoint,
     close (s);
 }
 
+void test_curve_security_zap_unsuccessful (void *ctx,
+                                           char *my_endpoint,
+                                           void *server,
+                                           void *server_mon,
+                                           int timeout,
+                                           int expected_event,
+                                           int expected_err)
+{
+    // TODO remove code duplication
+    void *client = zmq_socket (ctx, ZMQ_DEALER);
+    assert (client);
+    int rc = zmq_setsockopt (client, ZMQ_CURVE_SERVERKEY, server_public, 41);
+    assert (rc == 0);
+    rc = zmq_setsockopt (client, ZMQ_CURVE_PUBLICKEY, client_public, 41);
+    assert (rc == 0);
+    rc = zmq_setsockopt (client, ZMQ_CURVE_SECRETKEY, client_secret, 41);
+    assert (rc == 0);
+    rc = zmq_connect (client, my_endpoint);
+    assert (rc == 0);
+    expect_bounce_fail (server, client);
+    close_zero_linger (client);
+
+#ifdef ZMQ_BUILD_DRAFT_API
+    int count_of_expected_events = 0;
+
+    int event;
+    int err;
+    while (
+      (event = get_monitor_event_with_timeout (server_mon, &err, NULL, timeout))
+      != -1) {
+        if (event == expected_event) {
+            ++count_of_expected_events;
+            if (err != expected_err) {
+                fprintf (stderr, "Unexpected event: %x (err = %i)\n", event,
+                         err);
+                assert (false);
+            }
+        }
+    }
+    assert (count_of_expected_events > 0);
+#endif
+}
+
+void test_curve_security_zap_protocol_error(
+  void *ctx, char *my_endpoint, void *server, void *server_mon, int timeout)
+{
+    test_curve_security_zap_unsuccessful (
+      ctx, my_endpoint, server, server_mon, timeout,
+#ifdef ZMQ_BUILD_DRAFT_API
+      ZMQ_EVENT_HANDSHAKE_FAILED_ZAP, EPROTO
+#else
+      0, 0
+#endif
+    );
+}
+
 void test_curve_security_invalid_keysize (void *ctx)
 {
     //  Check return codes for invalid buffer sizes
@@ -574,6 +684,59 @@ int main (void)
     test_curve_security_unauthenticated_message (my_endpoint, server, timeout);
     shutdown_context_and_server_side (ctx, zap_thread, server, server_mon);
 
+    //  Invalid ZAP protocol tests
+
+    //  wrong version
+    setup_context_and_server_side (&ctx, &handler, &zap_thread, &server,
+                                   &server_mon, my_endpoint,
+                                   &zap_handler_wrong_version);
+    test_curve_security_zap_protocol_error (ctx, my_endpoint, server,
+                                            server_mon, timeout);
+    shutdown_context_and_server_side (ctx, zap_thread, server, server_mon);
+
+    //  wrong request id
+    setup_context_and_server_side (&ctx, &handler, &zap_thread, &server,
+                                   &server_mon, my_endpoint,
+                                   &zap_handler_wrong_request_id);
+    test_curve_security_zap_protocol_error (ctx, my_endpoint, server,
+                                            server_mon, timeout);
+    shutdown_context_and_server_side (ctx, zap_thread, server, server_mon);
+
+    //  status invalid (not a 3-digit number)
+    setup_context_and_server_side (&ctx, &handler, &zap_thread, &server,
+                                   &server_mon, my_endpoint,
+                                   &zap_handler_wrong_status_invalid);
+    test_curve_security_zap_protocol_error (ctx, my_endpoint, server,
+                                            server_mon, timeout);
+    shutdown_context_and_server_side (ctx, zap_thread, server, server_mon);
+
+    //  too many parts
+    setup_context_and_server_side (&ctx, &handler, &zap_thread, &server,
+                                   &server_mon, my_endpoint,
+                                   &zap_handler_too_many_parts);
+    test_curve_security_zap_protocol_error (ctx, my_endpoint, server,
+                                            server_mon, timeout);
+    shutdown_context_and_server_side (ctx, zap_thread, server, server_mon);
+
+    //  ZAP non-standard cases
+
+    //  status 500 internal error
+    setup_context_and_server_side (&ctx, &handler, &zap_thread, &server,
+                                   &server_mon, my_endpoint,
+                                   &zap_handler_wrong_status_internal_error);
+
+    //  TODO is this usable? EAGAIN does not appear to be an appropriate error 
+    //  code, and the status text is completely lost
+    test_curve_security_zap_unsuccessful (
+      ctx, my_endpoint, server, server_mon, timeout,
+#ifdef ZMQ_BUILD_DRAFT_API
+      ZMQ_EVENT_HANDSHAKE_FAILED_NO_DETAIL, EAGAIN
+#else
+      0, 0
+#endif
+    );
+    shutdown_context_and_server_side (ctx, zap_thread, server, server_mon);
+
     ctx = zmq_ctx_new ();
     test_curve_security_invalid_keysize (ctx);
     rc = zmq_ctx_term (ctx);