Skip to content

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

Merged
kezhuw merged 4 commits intoapache:masterfrom
eseabrook1:relax_client_certificate_requirements
Aug 18, 2025
Merged

ZOOKEEPER-4929: Make c client side cert optional in connecting to tls server#2257
kezhuw merged 4 commits intoapache:masterfrom
eseabrook1:relax_client_certificate_requirements

Conversation

@eseabrook1
Copy link
Copy Markdown
Contributor

@eseabrook1 eseabrook1 commented May 6, 2025

In the Zookeeper C library it is possible to initiate a connection using SSL by providing a "cert" string to zookeeper_init_ssl(). However in order to call this function, it is my understanding that callers must provide four things:

  1. The path to a Server CA file to validate the server's certificate
  2. The path to a Client CA file, with a complete certificate chain
  3. The path to a file containing the Client Private Key
  4. The password for the key file

This understanding is based on the implementation of init_ssl_for_socket

SSL_CTX_set_verify(*ctx, SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, 0);
/*SERVER CA FILE*/
if (SSL_CTX_load_verify_locations(*ctx, fd->cert->ca, 0) != 1) {
SSL_CTX_free(*ctx);
LOG_ERROR(LOGCALLBACK(zh), "Failed to load CA file %s", fd->cert->ca);
errno = EINVAL;
return ZBADARGUMENTS;
}
if (SSL_CTX_set_default_verify_paths(*ctx) != 1) {
SSL_CTX_free(*ctx);
LOG_ERROR(LOGCALLBACK(zh), "Call to SSL_CTX_set_default_verify_paths failed");
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;
}

For our use case, connecting to a server that does not support mTLS, it would be useful if we could specify only the CA for the server certificate, omitting the client parameters completely. This is something this is already possible with other Zookeeper client libraries, for example Kazoo: https://github.com/python-zk/kazoo/blob/c5ab98819b3a797e12a0315e97e51851525da70f/kazoo/handlers/utils.py#L253-L260

This Pull Request proposes a change to relax the requirements for the client SSL certificates and allow just a sever certificate to be provided.

@kezhuw kezhuw changed the title C Client Library: Relax requirement for client SSL certs ZOOKEEPER-4929: Make c client side cert optional in connecting to tls server May 12, 2025
Copy link
Copy Markdown
Member

@kezhuw kezhuw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Would you mind test this and document zookeeper_init_ssl somehow ?

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)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe need SSL_CTX_set_verify(ssl_ctx, SSL_VERIFY_NONE, NULL); in client when skip cert.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not particularly familiar with the SSL APIs, but my assumption was that we would be in client mode, and still want to validate the server certificate so the existing settings are still correct. We only want the TLS handshake to continue if ths server certificate is valid

https://docs.openssl.org/master/man3/SSL_CTX_set_verify/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like a server side option ssl.clientAuth.

I plan to construct a test for this pr.

Copy link
Copy Markdown
Member

@kezhuw kezhuw Jun 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @eseabrook1, I created eseabrook1#1 to add test case for this pr. Would you mind take a look ? This pr will be updated automatically once you merged that pr.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not particularly familiar with the SSL APIs, but my assumption was that we would be in client mode, and still want to validate the server certificate so the existing settings are still correct. We only want the TLS handshake to continue if ths server certificate is valid

https://docs.openssl.org/master/man3/SSL_CTX_set_verify/

This is the mode in which the client can ignore certificate verification.

@kezhuw kezhuw requested review from anmolnar, symat and ztzg June 4, 2025 17:36
@kezhuw kezhuw requested a review from cnauroth July 1, 2025 06:02
@kezhuw
Copy link
Copy Markdown
Member

kezhuw commented Jul 24, 2025

Hi, @ztzg @anmolnar @symat @cnauroth does anyone want to take another look ?

@kezhuw kezhuw merged commit d7f9717 into apache:master Aug 18, 2025
16 checks passed
@kezhuw
Copy link
Copy Markdown
Member

kezhuw commented Aug 18, 2025

Merged this as it has been exposed for a long time and I have constructed test for this.

@eseabrook1 Thank you for your contribution! Do you have a jira id so I have assign ZOOKEEPER-4929 to you ? If not, I think you could follow the link to register one. You probably will guide to sending account approval mail finally those days, please add link to this pr/jira so pmc know your contribution.

@eseabrook1
Copy link
Copy Markdown
Contributor Author

@eseabrook1 eseabrook1 deleted the relax_client_certificate_requirements branch September 22, 2025 13:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants