From 33695d1da83f076061b35130edc4341c94b72041 Mon Sep 17 00:00:00 2001
From: Luca Boccassi <luca.boccassi@gmail.com>
Date: Tue, 13 Jun 2017 22:11:24 +0100
Subject: [PATCH] Problem: ZAP is allowed to be configured incorrectly or not
 to work

Solution: if inproc://zeromq.zap.01 exists, which means ZAP is
enabled, abort immediately if it cannot be used (eg: out of memory)
or it is configured incorrectly (eg: wrong socket type).
Otherwise authentication failures will simply be ignored and
unauthorised peers will be allowed to slip in.
---
 src/curve_server.cpp  |  3 +++
 src/gssapi_server.cpp |  2 ++
 src/plain_server.cpp  |  3 +++
 src/session_base.cpp  | 22 ++++++++++++----------
 4 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/src/curve_server.cpp b/src/curve_server.cpp
index 77a56c72..ec551534 100644
--- a/src/curve_server.cpp
+++ b/src/curve_server.cpp
@@ -490,6 +490,9 @@ 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;
diff --git a/src/gssapi_server.cpp b/src/gssapi_server.cpp
index 0568ffe8..33a3639c 100644
--- a/src/gssapi_server.cpp
+++ b/src/gssapi_server.cpp
@@ -120,6 +120,8 @@ 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.
         int rc = session->zap_connect ();
         if (rc != 0)
             return -1;
diff --git a/src/plain_server.cpp b/src/plain_server.cpp
index 4d946b56..917ef9e6 100644
--- a/src/plain_server.cpp
+++ b/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;
diff --git a/src/session_base.cpp b/src/session_base.cpp
index 97fd08a1..da771f74 100644
--- a/src/session_base.cpp
+++ b/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;