ZOOKEEPER-5070: Support Single EKU certificates - #2429
Conversation
bad8dd7 to
d056eeb
Compare
anmolnar
left a comment
There was a problem hiding this comment.
Please add documentation to zookeeper-website/app/pages/_docs/docs/_mdx/admin-ops/administrators-guide/communication-using-the-netty-framework.mdx
There was a problem hiding this comment.
Please help me to understand the concept better.
You introduced only the following new properties:
ssl.(quorum).client.keyStore
ssl.(quorum).server.trustStore
Shouldn't we also have:
ssl.(quorum).client.trustStore
ssl.(quorum).server.keyStore
In my understand and EKU-supported zoo.cfg should look like this:
secureClientPort=2182
clientCnxnSocket=org.apache.zookeeper.ClientCnxnSocketNetty
ssl.server.keyStore.location=/Users/andor/work/ssl/server-keystore.jks
ssl.server.keyStore.password=password
ssl.server.trustStore.location=/Users/andor/work/ssl/server-truststore.jks
ssl.server.trustStore.password=password
ssl.client.keyStore.location=/Users/andor/work/ssl/client-keystore.jks
ssl.client.keyStore.password=password
ssl.client.trustStore.location=/Users/andor/work/ssl/client-truststore.jks
ssl.client.trustStore.password=password
...
sslQuorum=true
serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory
ssl.quorum.server.keyStore.location=/Users/andor/work/ssl/server-keystore.jks
ssl.quorum.server.keyStore.password=password
ssl.quorum.server.trustStore.location=/Users/andor/work/ssl/server-truststore.jks
ssl.quorum.server.trustStore.password=password
ssl.quorum.client.keyStore.location=/Users/andor/work/ssl/client-keystore.jks
ssl.quorum.client.keyStore.password=password
ssl.quorum.client.trustStore.location=/Users/andor/work/ssl/client-truststore.jks
ssl.quorum.client.trustStore.password=password
The original ssl.(quorum).keystore/truststore settings should be blank in the EKU case and vica versa.
Thanks, this is a very good point, I'll add it. 👍 |
Sorry, I did not mentioned this in the PR but my idea was to only introduce new TLS properties which were not yet present and use the existing TLS properties with the existing names. Here is an example of the relevant TLS properties from This approach makes less changes but might be a bit confusing as we don't have some client / server counterpart properties. What do you think? Is this a good approach or should we introduce those additional new properties? |
|
I've found (thanks to Codex/ChatGPT) the bug with client mTLS authentication. You missed to modify tm = x509Util.buildTrustManager(config);should be tm = x509Util.buildServerTrustManager(config);Otherwise it builds the trust relation chain with the wrong truststore. You might also want to add a unit test to p.s. I'm still not sure about the config parameters. Your approach makes sense to me too. |
|
I created a pull request with the code that ChatGPT generated: |
008b056 to
ea14b32
Compare
|
Hi @anmolnar, I added documentation about single EKU support and new config parameters to EDIT: |
Added client keystore, server truststore config
…pport Replace custom ClientServerX509KeyManager and ClientServerX509TrustManager wrappers with two separate SSLContext instances — one for client role (outgoing connections) and one for server role (incoming connections). The custom wrappers were passed to SSLContext.init(), which is rejected by JVM-level FIPS providers that only accept their own validated manager implementations. The new approach initializes each context with standard PKIX managers from their respective keystores, making single-EKU certificate support fully FIPS-compatible. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…client/server stores In the default deployment where ssl.client.keyStore.location and ssl.server.trustStore.location are not set, createSSLContextAndOptionsFromConfig loaded key/truststores twice
…ake cleanup Problem: finally block did not clear getSslKeystoreLocationProperty, leaving a deleted temp file path in system properties if serverSocket.close() throws. Fix: - Moved serverSocket.close() to the end of finally block - property cleanup now happens first, so an exception from close() can't skip it. - Added cleanup for the three shared keystore properties that were set in the test but never cleared.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
f096b2c to
8d39444
Compare
TestingTested the changes with single EKU certificates locally. With the help of Claude Code created a shell script to generate single EKU certs, keystores, truststores and config for a local quorum testing: Test ZooKeeper local quorum with single EKU certificates Executed the script, then started quorum, performed the tests manually: Test summary:
→ OK |
|
Merged to |
Goal
Support certificates with a single Extended Key Usage (EKU) - one cert with
serverAuthfor incoming connections, another withclientAuthfor outgoing connectionsApproach
Use separate
SSLContextinstances for client and server role instead of one. Each is initialized with standard PKIX managers from its own keystore / truststore.If these new properties are not set, ZooKeeper falls back to the existing keyStore.* and trustStore.* configs (shared for both roles), so the change is fully backward-compatible.
Configuration changes
Introduces the following new config properties:
Client keystore
Presented when initiating connections (client role). Allows using a certificate with only clientAuth EKU.
Server truststore
Used when accepting connections (server role). Validates the connecting client's certificate during mTLS.
Quorum client keystore
Quorum server truststore
Validates clients when accepting connections
TODO