Merge pull request #2773 from bluca/zap

Problems: strict ZAP protocol adherence is backward incompatible, minor static analysis warnings

doc/zmq_getsockopt.txt

@@ -838,6 +838,18 @@ Default value:: not set
 Applicable socket types:: all, when using TCP transport
 
 
+ZMQ_ZAP_ENFORCE_DOMAIN: Retrieve ZAP domain handling mode
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The 'ZMQ_ZAP_ENFORCE_DOMAIN' option shall retrieve the flag that determines
+whether a ZAP domain is strictly required or not.
+
+[horizontal]
+Option value type:: int
+Option value unit:: 0, 1
+Default value:: 0
+Applicable socket types:: all, when using ZAP
+
+
 ZMQ_VMCI_BUFFER_SIZE: Retrieve buffer size of the VMCI socket
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 The `ZMQ_VMCI_BUFFER_SIZE` option shall retrieve the size of the underlying

doc/zmq_setsockopt.txt

@@ -1081,7 +1081,9 @@ ZMQ_ZAP_DOMAIN: Set RFC 27 authentication domain
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Sets the domain for ZAP (ZMQ RFC 27) authentication. A ZAP domain must be 
 specified to enable authentication. When the ZAP domain is empty, which is 
-the default, ZAP authentication is disabled.
+the default, ZAP authentication is disabled. This is not compatible with
+previous versions of libzmq, so it can be controlled by ZMQ_ZAP_ENFORCE_DOMAIN
+which for now is disabled by default.
 See http://rfc.zeromq.org/spec:27 for more details.
 
 [horizontal]
@@ -1091,6 +1093,22 @@ Default value:: empty
 Applicable socket types:: all, when using TCP transport
 
 
+ZMQ_ZAP_ENFORCE_DOMAIN: Set ZAP domain handling to strictly adhere the RFC
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The ZAP (ZMQ RFC 27) authentication protocol specifies that a domain must
+always be set. Older versions of libzmq did not follow the spec and allowed
+an empty domain to be set.
+This option can be used to enabled or disable the stricter, backward
+incompatible behaviour. For now it is disabled by default, but in a future
+version it will be enabled by default.
+
+[horizontal]
+Option value type:: int
+Option value unit:: 0, 1
+Default value:: 0
+Applicable socket types:: all, when using ZAP
+
+
 ZMQ_TCP_ACCEPT_FILTER: Assign filters to allow new TCP connections
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 Assign an arbitrary number of filters that will be applied for each new TCP

include/zmq.h

@@ -570,6 +570,7 @@ ZMQ_EXPORT void zmq_threadclose (void* thread);
 #define ZMQ_GSSAPI_PRINCIPAL_NAMETYPE 90
 #define ZMQ_GSSAPI_SERVICE_PRINCIPAL_NAMETYPE 91
 #define ZMQ_BINDTODEVICE 92
+#define ZMQ_ZAP_ENFORCE_DOMAIN 93
 
 /*  DRAFT 0MQ socket events and monitoring                                    */
 /*  Unspecified system errors during handshake. Event value is an errno.      */

src/curve_server.cpp

@@ -390,7 +390,9 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_)
     rc = crypto_box_beforenm (cn_precom, cn_client, cn_secret);
     zmq_assert (rc == 0);
 
-    if (zap_required ()) {
+    //  Given this is a backward-incompatible change, it's behind a socket
+    //  option disabled by default.
+    if (zap_required () || !options.zap_enforce_domain) {
         //  Use ZAP protocol (RFC 27) to authenticate the user.
         rc = session->zap_connect ();
         if (rc == 0) {
@@ -404,6 +406,10 @@ int zmq::curve_server_t::process_initiate (msg_t *msg_)
             rc = receive_and_process_zap_reply ();
             if (rc == -1)
                 return -1;
+        } else if (!options.zap_enforce_domain) {
+            //  This supports the Stonehouse pattern (encryption without
+            //  authentication) in legacy mode (domain set but no handler).
+            state = sending_ready;
         } else {
             session->get_socket ()->event_handshake_failed_no_detail (
               session->get_endpoint (), EFAULT);

src/options.cpp

@@ -89,7 +89,8 @@ zmq::options_t::options_t () :
     heartbeat_ttl (0),
     heartbeat_interval (0),
     heartbeat_timeout (-1),
-    use_fd (-1)
+    use_fd (-1),
+    zap_enforce_domain (false)
 {
     memset (curve_public_key, 0, CURVE_KEYSIZE);
     memset (curve_secret_key, 0, CURVE_KEYSIZE);
@@ -628,6 +629,14 @@ int zmq::options_t::setsockopt (int option_, const void *optval_,
             }
             break;
 
+        case ZMQ_ZAP_ENFORCE_DOMAIN:
+            if (is_int) {
+                zap_enforce_domain = (value != 0);
+                return 0;
+            }
+            break;
+
+
         default:
 #if defined (ZMQ_ACT_MILITANT)
             //  There are valid scenarios for probing with unknown socket option
@@ -1052,6 +1061,13 @@ int zmq::options_t::getsockopt (int option_, void *optval_, size_t *optvallen_)
             }
             break;
 
+        case ZMQ_ZAP_ENFORCE_DOMAIN:
+            if (is_int) {
+                *value = zap_enforce_domain;
+                return 0;
+            }
+            break;
+
         default:
 #if defined (ZMQ_ACT_MILITANT)
             malformed = false;

src/options.hpp

@@ -166,7 +166,6 @@ namespace zmq
 
         // IPC accept() filters
 #       if defined ZMQ_HAVE_SO_PEERCRED || defined ZMQ_HAVE_LOCAL_PEERCRED
-        bool zap_ipc_creds;
         typedef std::set <uid_t> ipc_uid_accept_filters_t;
         ipc_uid_accept_filters_t ipc_uid_accept_filters;
         typedef std::set <gid_t> ipc_gid_accept_filters_t;
@@ -243,6 +242,9 @@ namespace zmq
 
         // Device to bind the underlying socket to, eg. VRF or interface
         std::string bound_device;
+
+        //  Enforce a non-empty ZAP domain requirement for PLAIN auth
+        bool zap_enforce_domain;
     };
 }
 

src/plain_server.cpp

@@ -47,7 +47,10 @@ zmq::plain_server_t::plain_server_t (session_base_t *session_,
     //  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.
-    zmq_assert (zap_required());
+    //  Given this is a backward-incompatible change, it's behind a socket
+    //  option disabled by default.
+    if (options.zap_enforce_domain)
+        zmq_assert (zap_required());
 }
 
 zmq::plain_server_t::~plain_server_t ()

src/socket_base.cpp

@@ -902,21 +902,21 @@ int zmq::socket_base_t::connect (const char *addr_)
     }
 #endif
 
-if (protocol  == "udp") {
-    if (options.type != ZMQ_RADIO) {
-        errno = ENOCOMPATPROTO;
-        LIBZMQ_DELETE(paddr);
-        return -1;
-    }
+    if (protocol  == "udp") {
+        if (options.type != ZMQ_RADIO) {
+            errno = ENOCOMPATPROTO;
+            LIBZMQ_DELETE(paddr);
+            return -1;
+        }
 
-    paddr->resolved.udp_addr = new (std::nothrow) udp_address_t ();
-    alloc_assert (paddr->resolved.udp_addr);
-    rc = paddr->resolved.udp_addr->resolve (address.c_str(), false);
-    if (rc != 0) {
-        LIBZMQ_DELETE(paddr);
-        return -1;
+        paddr->resolved.udp_addr = new (std::nothrow) udp_address_t ();
+        alloc_assert (paddr->resolved.udp_addr);
+        rc = paddr->resolved.udp_addr->resolve (address.c_str(), false);
+        if (rc != 0) {
+            LIBZMQ_DELETE(paddr);
+            return -1;
+        }
     }
-}
 
 // TBD - Should we check address for ZMQ_HAVE_NORM???
 
@@ -1143,7 +1143,7 @@ int zmq::socket_base_t::send (msg_t *msg_, int flags_)
 
     //  In case of non-blocking send we'll simply propagate
     //  the error - including EAGAIN - up the stack.
-    if (flags_ & ZMQ_DONTWAIT || options.sndtimeo == 0) {
+    if ((flags_ & ZMQ_DONTWAIT) || options.sndtimeo == 0) {
         return -1;
     }
 
@@ -1224,7 +1224,7 @@ int zmq::socket_base_t::recv (msg_t *msg_, int flags_)
     //  For non-blocking recv, commands are processed in case there's an
     //  activate_reader command already waiting in a command pipe.
     //  If it's not, return EAGAIN.
-    if (flags_ & ZMQ_DONTWAIT || options.rcvtimeo == 0) {
+    if ((flags_ & ZMQ_DONTWAIT) || options.rcvtimeo == 0) {
         if (unlikely (process_commands (0, false) != 0)) {
             return -1;
         }

src/zmq_draft.h

@@ -50,6 +50,7 @@
 #define ZMQ_GSSAPI_PRINCIPAL_NAMETYPE 90
 #define ZMQ_GSSAPI_SERVICE_PRINCIPAL_NAMETYPE 91
 #define ZMQ_BINDTODEVICE 92
+#define ZMQ_ZAP_ENFORCE_DOMAIN 93
 
 /*  DRAFT 0MQ socket events and monitoring                                    */
 /*  Unspecified system errors during handshake. Event value is an errno.      */

tests/test_security_zap.cpp

@@ -324,6 +324,7 @@ void test_zap_errors (socket_config_fn server_socket_config_,
     shutdown_context_and_server_side (ctx, zap_thread, server, server_mon,
                                       handler);
 
+#ifdef ZMQ_ZAP_ENFORCE_DOMAIN
     //  no ZAP handler
     fprintf (stderr, "test_zap_unsuccessful no ZAP handler started\n");
     setup_context_and_server_side (&ctx, &handler, &zap_thread, &server,
@@ -339,6 +340,7 @@ void test_zap_errors (socket_config_fn server_socket_config_,
       client_socket_config_, client_socket_config_data_);
     shutdown_context_and_server_side (ctx, zap_thread, server, server_mon,
                                       handler);
+#endif
 
     //  ZAP handler disconnecting on first message
     fprintf(stderr, "test_zap_unsuccessful ZAP handler disconnects\n");

tests/testutil_security.hpp

@@ -114,6 +114,13 @@ void socket_config_curve_server (void *server, void *server_secret)
     rc = zmq_setsockopt (server, ZMQ_ZAP_DOMAIN, test_zap_domain,
                          strlen (test_zap_domain));
     assert (rc == 0);
+
+#ifdef ZMQ_ZAP_ENFORCE_DOMAIN
+    int required = 1;
+    rc = zmq_setsockopt (server, ZMQ_ZAP_ENFORCE_DOMAIN, &required,
+            sizeof (int));
+    assert (rc == 0);
+#endif
 }
 
 struct curve_client_data_t