ZOOKEEPER-4929: Make c client side cert optional in connecting to tls server

Reviewers: kezhuw
Author: eseabrook1
Closes #2257 from eseabrook1/relax_client_certificate_requirements

Author: eseabrook1

zookeeper-client/zookeeper-client-c/include/zookeeper.h

@@ -537,6 +537,38 @@ ZOOAPI zhandle_t *zookeeper_init(const char *host, watcher_fn fn,
   int recv_timeout, const clientid_t *clientid, void *context, int flags);
 
 #ifdef HAVE_OPENSSL_H
+/**
+ * \brief create a handle to communicate with zookeeper using SSL.
+ *
+ * This method creates a new handle and a zookeeper session that corresponds
+ * to that handle. Session establishment is asynchronous, meaning that the
+ * session should not be considered established until (and unless) an
+ * event of state ZOO_CONNECTED_STATE is received.
+ * \param host comma separated host:port pairs, each corresponding to a zk
+ *   server. e.g. "127.0.0.1:3000,127.0.0.1:3001,127.0.0.1:3002"
+ * \param cert SSL certificate string. Two formats are supported: server CA
+ *   only e.g. "/path/to/server_ca.cer" and server CA with client certificate e.g.
+ *   "/path/to/server_ca.cer,/path/to/client_cert.pem,/path/to/client_cert.key,client_cert_password".
+ * \param fn the global watcher callback function. When notifications are
+ *   triggered this function will be invoked.
+ * \param recv_timeout the timeout for the zookeeper session.
+ * \param clientid the id of a previously established session that this
+ *   client will be reconnecting to. Pass 0 if not reconnecting to a previous
+ *   session. Clients can access the session id of an established, valid,
+ *   connection by calling \ref zoo_client_id. If the session corresponding to
+ *   the specified clientid has expired, or if the clientid is invalid for
+ *   any reason, the returned zhandle_t will be invalid -- the zhandle_t
+ *   state will indicate the reason for failure (typically
+ *   ZOO_EXPIRED_SESSION_STATE).
+ * \param context the handback object that will be associated with this instance
+ *   of zhandle_t. Application can access it (for example, in the watcher
+ *   callback) using \ref zoo_get_context. The object is not used by zookeeper
+ *   internally and can be null.
+ * \param flags reserved for future use. Should be set to zero.
+ * \return a pointer to the opaque zhandle structure. If it fails to create
+ * a new zhandle the function returns NULL and the errno variable
+ * indicates the reason.
+ */
 ZOOAPI zhandle_t *zookeeper_init_ssl(const char *host, const char *cert, watcher_fn fn,
   int recv_timeout, const clientid_t *clientid, void *context, int flags);
 #endif

zookeeper-client/zookeeper-client-c/src/zookeeper.c

@@ -2777,27 +2777,30 @@ static int init_ssl_for_socket(zsock_t *fd, zhandle_t *zh, int fail_on_error) {
             errno = EINVAL;
             return ZBADARGUMENTS;
         }
-        /*CLIENT CA FILE (With Certificate Chain)*/
-        if (SSL_CTX_use_certificate_chain_file(*ctx, fd->cert->cert) != 1) {
-            SSL_CTX_free(*ctx);
-            LOG_ERROR(LOGCALLBACK(zh), "Failed to load client certificate chain from %s", fd->cert->cert);
-            errno = EINVAL;
-            return ZBADARGUMENTS;
-        }
-        /*CLIENT PRIVATE KEY*/
-        SSL_CTX_set_default_passwd_cb_userdata(*ctx, fd->cert->passwd);
-        if (SSL_CTX_use_PrivateKey_file(*ctx, fd->cert->key, SSL_FILETYPE_PEM) != 1) {
-            SSL_CTX_free(*ctx);
-            LOG_ERROR(LOGCALLBACK(zh), "Failed to load client private key from %s", fd->cert->key);
-            errno = EINVAL;
-            return ZBADARGUMENTS;
-        }
-        /*CHECK*/
-        if (SSL_CTX_check_private_key(*ctx) != 1) {
-            SSL_CTX_free(*ctx);
-            LOG_ERROR(LOGCALLBACK(zh), "SSL_CTX_check_private_key failed");
-            errno = EINVAL;
-            return ZBADARGUMENTS;
+        if (fd->cert->cert != NULL && fd->cert->passwd != NULL && fd->cert->key != NULL)
+        {
+            /*CLIENT CA FILE (With Certificate Chain)*/
+            if (SSL_CTX_use_certificate_chain_file(*ctx, fd->cert->cert) != 1) {
+                SSL_CTX_free(*ctx);
+                LOG_ERROR(LOGCALLBACK(zh), "Failed to load client certificate chain from %s", fd->cert->cert);
+                errno = EINVAL;
+                return ZBADARGUMENTS;
+            }
+            /*CLIENT PRIVATE KEY*/
+            SSL_CTX_set_default_passwd_cb_userdata(*ctx, fd->cert->passwd);
+            if (SSL_CTX_use_PrivateKey_file(*ctx, fd->cert->key, SSL_FILETYPE_PEM) != 1) {
+                SSL_CTX_free(*ctx);
+                LOG_ERROR(LOGCALLBACK(zh), "Failed to load client private key from %s", fd->cert->key);
+                errno = EINVAL;
+                return ZBADARGUMENTS;
+            }
+            /*CHECK*/
+            if (SSL_CTX_check_private_key(*ctx) != 1) {
+                SSL_CTX_free(*ctx);
+                LOG_ERROR(LOGCALLBACK(zh), "SSL_CTX_check_private_key failed");
+                errno = EINVAL;
+                return ZBADARGUMENTS;
+            }
         }
         /*MULTIPLE HANDSHAKE*/
         SSL_CTX_set_mode(*ctx, SSL_MODE_AUTO_RETRY);

zookeeper-client/zookeeper-client-c/tests/TestClient.cc

@@ -212,6 +212,10 @@ class Zookeeper_simpleSystem : public CPPUNIT_NS::TestFixture
 #endif
 #ifdef HAVE_OPENSSL_H
     CPPUNIT_TEST(testSSL);
+    CPPUNIT_TEST(testSSLNoClientCert);
+
+    // Order after above two tests as it changes ssl.clientAuth
+    CPPUNIT_TEST(testSSLServerOnly);
 #endif
     CPPUNIT_TEST(testCreate);
     CPPUNIT_TEST(testCreateContainer);
@@ -314,6 +318,12 @@ class Zookeeper_simpleSystem : public CPPUNIT_NS::TestFixture
         CPPUNIT_ASSERT(system(cmd) == 0);
     }
 
+    void startServer(const char *envs) {
+        char cmd[1024];
+        sprintf(cmd, "%s %s start %s", envs, ZKSERVER_CMD, getHostPorts());
+        CPPUNIT_ASSERT(system(cmd) == 0);
+    }
+
     void stopServer() {
         char cmd[1024];
         sprintf(cmd, "%s stop %s", ZKSERVER_CMD, getHostPorts());
@@ -879,15 +889,28 @@ class Zookeeper_simpleSystem : public CPPUNIT_NS::TestFixture
     }
 
 #ifdef HAVE_OPENSSL_H
-    void testSSL() {
+    int checkSSL(const char *path, const char *certs) {
         watchctx_t ctx;
         zoo_set_debug_level(ZOO_LOG_LEVEL_DEBUG);
-        zhandle_t *zk = createSSLClient("127.0.0.1:22281", "/tmp/certs/server.crt,/tmp/certs/client.crt,/tmp/certs/clientkey.pem,password", &ctx);
+        zhandle_t *zk = createSSLClient("127.0.0.1:22281", certs, &ctx);
         CPPUNIT_ASSERT(zk);
         int rc = 0;
-        rc = zoo_create(zk, "/ssl", NULL, -1,
-                        &ZOO_OPEN_ACL_UNSAFE, 0, 0, 0);
-        CPPUNIT_ASSERT_EQUAL((int) ZOK, rc);
+        return zoo_create(zk, path, NULL, -1, &ZOO_OPEN_ACL_UNSAFE, 0, 0, 0);
+    }
+
+    void testSSL() {
+        CPPUNIT_ASSERT_EQUAL((int) ZOK, checkSSL("/ssl", "/tmp/certs/server.crt,/tmp/certs/client.crt,/tmp/certs/clientkey.pem,password"));
+    }
+
+    void testSSLNoClientCert() {
+        CPPUNIT_ASSERT(ZOK != checkSSL("/ssl-no-client-cert", "/tmp/certs/server.crt"));
+    }
+
+    void testSSLServerOnly() {
+        stopServer();
+        startServer("CLIENT_AUTH=none");
+
+        CPPUNIT_ASSERT_EQUAL((int) ZOK, checkSSL("/ssl-server-only", "/tmp/certs/server.crt"));
     }
 #endif
 

zookeeper-client/zookeeper-client-c/tests/zkServer.sh

@@ -24,6 +24,8 @@ ZOOPORT=${ZOOPORT:-"22181"}
 # Some tests are setting the maxClientConnections. When it is not set, we fallback to default 100
 ZKMAXCNXNS=${ZKMAXCNXNS:-"100"}
 
+CLIENT_AUTH=${CLIENT_AUTH:-"need"}
+
 EXTRA_JVM_ARGS=${EXTRA_JVM_ARGS:-""}
 
 if [[ -z $1 ]]; then
@@ -163,7 +165,7 @@ case $1 in
     )
 
     # ===== prepare the configs
-    sed "s#TMPDIR#$tmp_dir#g;s#CERTDIR#$certs_dir#g;s#MAXCLIENTCONNECTIONS#$ZKMAXCNXNS#g;s#CLIENTPORT#$ZOOPORT#g" "$tests_dir/zoo.cfg" >"$tmp_dir/zoo.cfg"
+    sed "s#TMPDIR#$tmp_dir#g;s#CERTDIR#$certs_dir#g;s#MAXCLIENTCONNECTIONS#$ZKMAXCNXNS#g;s#CLIENTPORT#$ZOOPORT#g;s#CLIENT_AUTH#$CLIENT_AUTH#g" "$tests_dir/zoo.cfg" >"$tmp_dir/zoo.cfg"
     if [[ -n $read_only ]]; then
       # we can put the new server to read-only mode by starting only a single instance of a three node server
       {

zookeeper-client/zookeeper-client-c/tests/zoo.cfg

@@ -8,6 +8,7 @@ localSessionsEnabled=true
 clientPort=CLIENTPORT
 secureClientPort=22281
 serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory
+ssl.clientAuth=CLIENT_AUTH
 ssl.keyStore.location=CERTDIR/server.jks
 ssl.keyStore.password=password
 ssl.trustStore.location=CERTDIR/servertrust.jks