From e9bd9ad1db69b8329f95b755ad5118311332b0b7 Mon Sep 17 00:00:00 2001 From: Edward Seabrook Date: Tue, 6 May 2025 14:59:36 +0100 Subject: [PATCH 1/3] C Client Library: Relax requirement for client SSL certs --- .../zookeeper-client-c/src/zookeeper.c | 45 ++++++++++--------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/zookeeper-client/zookeeper-client-c/src/zookeeper.c b/zookeeper-client/zookeeper-client-c/src/zookeeper.c index 972215bf009..83c622efc0a 100644 --- a/zookeeper-client/zookeeper-client-c/src/zookeeper.c +++ b/zookeeper-client/zookeeper-client-c/src/zookeeper.c @@ -2769,27 +2769,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); From 150c99a36207ed361364534ddfd3afdbf8200ea1 Mon Sep 17 00:00:00 2001 From: Edward Seabrook Date: Thu, 29 May 2025 16:55:31 +0100 Subject: [PATCH 2/3] Add doc comment for zookeeper_init_ssl --- .../zookeeper-client-c/include/zookeeper.h | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/zookeeper-client/zookeeper-client-c/include/zookeeper.h b/zookeeper-client/zookeeper-client-c/include/zookeeper.h index 2fa6a7b5065..3aafddeda02 100644 --- a/zookeeper-client/zookeeper-client-c/include/zookeeper.h +++ b/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 From af14777589cde8c95cdc5197e539b9c9d043ea95 Mon Sep 17 00:00:00 2001 From: Kezhu Wang Date: Tue, 3 Jun 2025 07:41:27 +0800 Subject: [PATCH 3/3] ZOOKEEPER_4929: Add test case for server only tls --- .../zookeeper-client-c/tests/TestClient.cc | 33 ++++++++++++++++--- .../zookeeper-client-c/tests/zkServer.sh | 4 ++- .../zookeeper-client-c/tests/zoo.cfg | 1 + 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/zookeeper-client/zookeeper-client-c/tests/TestClient.cc b/zookeeper-client/zookeeper-client-c/tests/TestClient.cc index 8c79c01b038..83a7203d7b8 100644 --- a/zookeeper-client/zookeeper-client-c/tests/TestClient.cc +++ b/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 diff --git a/zookeeper-client/zookeeper-client-c/tests/zkServer.sh b/zookeeper-client/zookeeper-client-c/tests/zkServer.sh index 4e090a3cdf1..e1b850dd194 100755 --- a/zookeeper-client/zookeeper-client-c/tests/zkServer.sh +++ b/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 { diff --git a/zookeeper-client/zookeeper-client-c/tests/zoo.cfg b/zookeeper-client/zookeeper-client-c/tests/zoo.cfg index d7444238e73..5e3df626436 100644 --- a/zookeeper-client/zookeeper-client-c/tests/zoo.cfg +++ b/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