ZOOKEEPER-3806: TLS - dynamic loading for client trust/key store #1839

mathew-manu · 2022-03-17T23:05:59Z

ZooKeer currently has support for reloading the Quorum Truststore & Keystore automatically when the certificate files change in the filesystem without server restart (#680)

However, Reloading of key and trust store for ClientX509Util is not present; i.e., the server presented certs to the clients will not get reloaded automatically if the certificates in the filesystem change, short-lived certs requires the process restart.

Changes:

A new config property "zookeeper.client.certReload" is added, if it's true - ClientX509Util is reloaded automatically.
ZK uses an X509AuthenticationProvider which is backed by an X509TrustManager and an X509KeyManager to perform remote host certificate authentication. We need to update the X509AuthenticationProvider's TrustStore as part of the X509Util file-watcher.
Junit test case to verify the cert reload.

mathew-manu · 2022-03-21T03:45:30Z

2 workflows awaiting approval (First-time contributor)
Can someone approve the workflows?

mathew-manu · 2022-05-04T16:01:03Z

@symat Could you pls review this PR?
If certificates are updated on running clusters, server presented certificates to the clients are not refreshed automatically and requires process restarts. I think it's valuable to have this merged.

li4wang · 2022-05-04T18:50:48Z

Agreed. This is a great feature to have.

eolivelli

+1

Nice work

symat

thank you for the contribution!
this is a very nice feature, I like the test too!!

Can you please add documentation for this new configuration near to the other ssl configs on our admin page? https://github.com/apache/zookeeper/blob/master/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md

using "new in 3.5.9"

mathew-manu · 2022-05-06T04:07:19Z

thank you for the contribution! this is a very nice feature, I like the test too!!

Can you please add documentation for this new configuration near to the other ssl configs on our admin page? https://github.com/apache/zookeeper/blob/master/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md

using "new in 3.5.9"

@symat Updated the documentation with the new configuration.

symat · 2022-05-06T06:30:23Z

zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md

+
+* *client.certReload* :
+    (Java system property: **zookeeper.client.certReload**)
+    **New in 3.5.9:**


sorry @mathew-manu , I asked you to document this as "new in 3.5.9", it should be 3.9.0. Can you change it? I can merge after that.

Updated the version to 3.9.0

sonatype-lift · 2022-05-06T13:42:21Z

zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/ProviderRegistry.java

+            }
+        }
+    }
+
    public static ServerAuthenticationProvider getServerProvider(String scheme) {
        return WrappedAuthenticationProvider.wrap(getProvider(scheme));


THREAD_SAFETY_VIOLATION: Read/Write race. Non-private method ProviderRegistry.getServerProvider(...) indirectly reads without synchronization from auth.ProviderRegistry.initialized. Potentially races with write in method ProviderRegistry.reset().
Reporting because another access to the same memory occurs on a background thread, although this access may not.

(at-me in a reply with help or ignore)

Was this a good recommendation?
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

ProviderRegistry.reset() is only called from test.

symat

thanks!! merging it now to master

li4wang · 2022-05-19T23:57:48Z

@eolivelli @symat can we get this merged into 3.7 and 3.8 too? We are on 3.7 and it would be great if this can be supported in 3.7. Do we need to open a new JIRA? Please let me know if anything I can help. Thanks.

symat · 2022-05-20T06:46:09Z

Hello @li4wang !
Normally we merge new features (like this one) only to master and we cherry-pick only the security and bug fixes to older branches. However, I see this commit to be valuable for the community and it doesn't change the default behavior. Also, I don't know when 3.9.0 will happen... so I'm open to have it backported to branch-3.8 and branch-3.7. @eolivelli what do you think?

If we would do that, then I think no new Jira is necessary, but we would need two new github PRs targeting branch-3.7 and branch-3.8. Also we would need a small PR on master, fixing the documentation about the version numbers (since when this feature is available... like "New in 3.7.2, 3.8.1, 3.9.0")

Just for your info: it is perfectly normal and legal (many of us actually do this) to build your own ZooKeeper version, based on a branch you are familiar with / tested before, adding only a few commits from later branches. Then use this in production. Building ZooKeeper is quite simple (use jdk8 and a relatively recent maven, then simply mvn clean install -DskipTests)

eolivelli · 2022-05-20T12:25:38Z

I am +1 to port this to 3.8 and to 3.7.

Just for your info: it is perfectly normal and legal (many of us actually do this) to build your own ZooKeeper version, based on a branch you are familiar with / tested before, adding only a few commits from later branches. Then use this in production. Building ZooKeeper is quite simple (use jdk8 and a relatively recent maven, then simply mvn clean install -DskipTests)
This is "legal" but I must add that:

if you do this then you are not guaranteed that the project (PMC) verified the goodness of the release, so you should really know what are you doing (sometimes patches break the code and add side effects, that are fixed before releases while the community validates and blesses the release candidate).
if you go down this path, then ensure that all the tests are passing, especially using the JDK and OS that you are going to use in production

Every Apache product is meant to be consumed from the Source code, but ONLY using the Source Tarballs that have been approved with a formal and official VOTE.

li4wang · 2022-05-23T16:43:41Z

That's great! Thanks @symat and @eolivelli. I will open a JIRA and create PRs for 3.7 and 3.8.

li4wang · 2022-05-23T16:51:43Z

https://issues.apache.org/jira/browse/ZOOKEEPER-4545 (for 3.7)
https://issues.apache.org/jira/browse/ZOOKEEPER-4546 (for 3.8)

ZooKeer currently has support for reloading the Quorum Truststore & Keystore automatically when the certificate files change in the filesystem without server restart (apache#680) However, Reloading of key and trust store for **ClientX509Util** is not present; i.e., the server presented certs to the clients will not get reloaded automatically if the certificates in the filesystem change, short-lived certs requires the process restart. Changes: - A new config property "zookeeper.client.certReload" is added, if it's true - ClientX509Util is reloaded automatically. - ZK uses an _X509AuthenticationProvider_ which is backed by an X509TrustManager and an X509KeyManager to perform _remote host certificate authentication_. We need to update the X509AuthenticationProvider's TrustStore as part of the X509Util file-watcher. - Junit test case to verify the cert reload. Author: Manu Mathew <manu.mathew@netapp.com> Author: mathewmanu <manmathew@cs.stonybrook.edu> Author: Manu Mathew <101424654+mathew-manu@users.noreply.github.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org> Closes apache#1839 from mathew-manu/ZOOKEEPER-3806

Backporting ZOOKEEPER-4468 to branch-3.8 This is cherry-pick from apache#1839. This PR is the same as the apache#1839 on the master branch, only changing the documentation about the version numbers. ZooKeer currently has support for reloading the Quorum Truststore & Keystore automatically when the certificate files change in the filesystem without server restart (apache#680) However, Reloading of key and trust store for **ClientX509Util** is not present; i.e., the server presented certs to the clients will not get reloaded automatically if the certificates in the filesystem change, short-lived certs requires the process restart. Changes: - A new config property "zookeeper.client.certReload" is added, if it's true - ClientX509Util is reloaded automatically. - ZK uses an _X509AuthenticationProvider_ which is backed by an X509TrustManager and an X509KeyManager to perform _remote host certificate authentication_. We need to update the X509AuthenticationProvider's TrustStore as part of the X509Util file-watcher. - Junit test case to verify the cert reload.

ZooKeer currently has support for reloading the Quorum Truststore & Keystore automatically when the certificate files change in the filesystem without server restart (apache#680) However, Reloading of key and trust store for **ClientX509Util** is not present; i.e., the server presented certs to the clients will not get reloaded automatically if the certificates in the filesystem change, short-lived certs requires the process restart. Changes: - A new config property "zookeeper.client.certReload" is added, if it's true - ClientX509Util is reloaded automatically. - ZK uses an _X509AuthenticationProvider_ which is backed by an X509TrustManager and an X509KeyManager to perform _remote host certificate authentication_. We need to update the X509AuthenticationProvider's TrustStore as part of the X509Util file-watcher. - Junit test case to verify the cert reload. Author: Manu Mathew <manu.mathew@netapp.com> Author: mathewmanu <manmathew@cs.stonybrook.edu> Author: Manu Mathew <101424654+mathew-manu@users.noreply.github.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org> Closes apache#1839 from mathew-manu/ZOOKEEPER-3806

This is cherry-pick from apache#1839. This PR is the same as the apache#1839 on the master branch, only changing the documentation about the version numbers. Signed-off-by: Li Wang <li4wang@gmail.com>

ZooKeer currently has support for reloading the Quorum Truststore & Keystore automatically when the certificate files change in the filesystem without server restart (apache#680) However, Reloading of key and trust store for **ClientX509Util** is not present; i.e., the server presented certs to the clients will not get reloaded automatically if the certificates in the filesystem change, short-lived certs requires the process restart. Changes: - A new config property "zookeeper.client.certReload" is added, if it's true - ClientX509Util is reloaded automatically. - ZK uses an _X509AuthenticationProvider_ which is backed by an X509TrustManager and an X509KeyManager to perform _remote host certificate authentication_. We need to update the X509AuthenticationProvider's TrustStore as part of the X509Util file-watcher. - Junit test case to verify the cert reload. Author: Manu Mathew <manu.mathew@netapp.com> Author: mathewmanu <manmathew@cs.stonybrook.edu> Author: Manu Mathew <101424654+mathew-manu@users.noreply.github.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org> Closes apache#1839 from mathew-manu/ZOOKEEPER-3806

This is cherry-pick from apache#1839. This PR is the same as the apache#1839 on the master branch, only changing the documentation about the version numbers. Signed-off-by: Li Wang <li4wang@gmail.com>

This is cherry-pick from apache#1839. This PR is the same as the apache#1839 on the master branch, only changing the documentation about the version numbers. Signed-off-by: Li Wang <li4wang@gmail.com> ZOOKEEPER-4546 Backport auto reloading client key/trust store to 3.8 This is cherry-pick from apache#1839. This PR is the same as the apache#1839 on the master branch, only changing the documentation about the version numbers. Signed-off-by: Li Wang <li4wang@gmail.com>

This is cherry-pick from apache#1839. This PR is the same as the apache#1839 on the master branch, only changing the documentation about the version numbers. Signed-off-by: Li Wang <li4wang@gmail.com>

This is cherry-pick from apache#1839. This PR is the same as the apache#1839 on the master branch, only changing the documentation about the version numbers. Signed-off-by: Li Wang <li4wang@gmail.com> ZOOKEEPER-4546 Backport auto reloading client key/trust store to 3.8 This is cherry-pick from apache#1839. This PR is the same as the apache#1839 on the master branch, only changing the documentation about the version numbers. Signed-off-by: Li Wang <li4wang@gmail.com>

ZOOKEEPER-4545 Backport auto reloading client key/trust store to 3.7 This is cherry-pick from #1839. This PR is the same as the #1839 on the master branch, only changing the documentation about the version numbers. Signed-off-by: Li Wang <li4wanggmail.com> Author: Manu Mathew <manu.mathew@netapp.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org> Closes #1884 from li4wang/ZOOKEEPER-4545

ZooKeer currently has support for reloading the Quorum Truststore & Keystore automatically when the certificate files change in the filesystem without server restart (apache#680) However, Reloading of key and trust store for **ClientX509Util** is not present; i.e., the server presented certs to the clients will not get reloaded automatically if the certificates in the filesystem change, short-lived certs requires the process restart. Changes: - A new config property "zookeeper.client.certReload" is added, if it's true - ClientX509Util is reloaded automatically. - ZK uses an _X509AuthenticationProvider_ which is backed by an X509TrustManager and an X509KeyManager to perform _remote host certificate authentication_. We need to update the X509AuthenticationProvider's TrustStore as part of the X509Util file-watcher. - Junit test case to verify the cert reload. Author: Manu Mathew <manu.mathew@netapp.com> Author: mathewmanu <manmathew@cs.stonybrook.edu> Author: Manu Mathew <101424654+mathew-manu@users.noreply.github.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org> Closes apache#1839 from mathew-manu/ZOOKEEPER-3806

ZooKeer currently has support for reloading the Quorum Truststore & Keystore automatically when the certificate files change in the filesystem without server restart (apache#680) However, Reloading of key and trust store for **ClientX509Util** is not present; i.e., the server presented certs to the clients will not get reloaded automatically if the certificates in the filesystem change, short-lived certs requires the process restart. Changes: - A new config property "zookeeper.client.certReload" is added, if it's true - ClientX509Util is reloaded automatically. - ZK uses an _X509AuthenticationProvider_ which is backed by an X509TrustManager and an X509KeyManager to perform _remote host certificate authentication_. We need to update the X509AuthenticationProvider's TrustStore as part of the X509Util file-watcher. - Junit test case to verify the cert reload. Author: Manu Mathew <manu.mathew@netapp.com> Author: mathewmanu <manmathew@cs.stonybrook.edu> Author: Manu Mathew <101424654+mathew-manu@users.noreply.github.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org> Closes apache#1839 from mathew-manu/ZOOKEEPER-3806 Co-authored-by: Manu Mathew <manu.mathew@netapp.com>

ZooKeer currently has support for reloading the Quorum Truststore & Keystore automatically when the certificate files change in the filesystem without server restart (apache#680) However, Reloading of key and trust store for **ClientX509Util** is not present; i.e., the server presented certs to the clients will not get reloaded automatically if the certificates in the filesystem change, short-lived certs requires the process restart. Changes: - A new config property "zookeeper.client.certReload" is added, if it's true - ClientX509Util is reloaded automatically. - ZK uses an _X509AuthenticationProvider_ which is backed by an X509TrustManager and an X509KeyManager to perform _remote host certificate authentication_. We need to update the X509AuthenticationProvider's TrustStore as part of the X509Util file-watcher. - Junit test case to verify the cert reload. Author: Manu Mathew <manu.mathew@netapp.com> Author: mathewmanu <manmathew@cs.stonybrook.edu> Author: Manu Mathew <101424654+mathew-manu@users.noreply.github.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org> Closes apache#1839 from mathew-manu/ZOOKEEPER-3806

ZooKeer currently has support for reloading the Quorum Truststore & Keystore automatically when the certificate files change in the filesystem without server restart (apache#680) However, Reloading of key and trust store for **ClientX509Util** is not present; i.e., the server presented certs to the clients will not get reloaded automatically if the certificates in the filesystem change, short-lived certs requires the process restart. Changes: - A new config property "zookeeper.client.certReload" is added, if it's true - ClientX509Util is reloaded automatically. - ZK uses an _X509AuthenticationProvider_ which is backed by an X509TrustManager and an X509KeyManager to perform _remote host certificate authentication_. We need to update the X509AuthenticationProvider's TrustStore as part of the X509Util file-watcher. - Junit test case to verify the cert reload. Author: Manu Mathew <manu.mathew@netapp.com> Author: mathewmanu <manmathew@cs.stonybrook.edu> Author: Manu Mathew <101424654+mathew-manu@users.noreply.github.com> Reviewers: Enrico Olivelli <eolivelli@apache.org>, Mate Szalay-Beko <symat@apache.org> Closes apache#1839 from mathew-manu/ZOOKEEPER-3806 Co-authored-by: Manu Mathew <manu.mathew@netapp.com>

ZOOKEEPER-3806: dynamic reloading for client trust and key store

5b3a160

eolivelli approved these changes May 4, 2022

View reviewed changes

symat requested changes May 5, 2022

View reviewed changes

mathew-manu and others added 2 commits May 5, 2022 20:24

Merge branch 'apache:master' into ZOOKEEPER-3806

fa7ffd0

Added documentation for certificate reload config properties

7e7eb24

symat requested changes May 6, 2022

View reviewed changes

Update release zookeeerAdmin.md with release version

a23f613

sonatype-lift bot reviewed May 6, 2022

View reviewed changes

symat approved these changes May 6, 2022

View reviewed changes

asfgit closed this in 7248644 May 6, 2022

mathew-manu deleted the ZOOKEEPER-3806 branch May 6, 2022 21:32

li4wang mentioned this pull request May 23, 2022

ZOOKEEPER-4545 Backport auto reloading client key/trust store to 3.7 #1884

Closed

li4wang mentioned this pull request May 24, 2022

ZOOKEEPER-4546: Backport auto reloading client key/trust store to 3.8 #1885

Open

anuragmadnawat1 mentioned this pull request Nov 2, 2022

ZOOKEEPER-3806: TLS - dynamic loading for client trust/key store anuragmadnawat1/zookeeper#61

Merged

anurag-harness mentioned this pull request Jan 13, 2023

ZOOKEEPER-3806: TLS - dynamic loading for client trust/key store anurag-harness/zookeeper#28

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ZOOKEEPER-3806: TLS - dynamic loading for client trust/key store #1839

ZOOKEEPER-3806: TLS - dynamic loading for client trust/key store #1839

mathew-manu commented Mar 17, 2022

mathew-manu commented Mar 21, 2022

mathew-manu commented May 4, 2022

li4wang commented May 4, 2022

eolivelli left a comment

symat left a comment

mathew-manu commented May 6, 2022

symat May 6, 2022

mathew-manu May 6, 2022

sonatype-lift bot May 6, 2022

mathew-manu May 6, 2022

symat left a comment

li4wang commented May 19, 2022

symat commented May 20, 2022

eolivelli commented May 20, 2022

li4wang commented May 23, 2022

li4wang commented May 23, 2022

ZOOKEEPER-3806: TLS - dynamic loading for client trust/key store #1839

ZOOKEEPER-3806: TLS - dynamic loading for client trust/key store #1839

Conversation

mathew-manu commented Mar 17, 2022

mathew-manu commented Mar 21, 2022

mathew-manu commented May 4, 2022

li4wang commented May 4, 2022

eolivelli left a comment

Choose a reason for hiding this comment

symat left a comment

Choose a reason for hiding this comment

mathew-manu commented May 6, 2022

symat May 6, 2022

Choose a reason for hiding this comment

mathew-manu May 6, 2022

Choose a reason for hiding this comment

sonatype-lift bot May 6, 2022

Choose a reason for hiding this comment

mathew-manu May 6, 2022

Choose a reason for hiding this comment

symat left a comment

Choose a reason for hiding this comment

li4wang commented May 19, 2022

symat commented May 20, 2022

eolivelli commented May 20, 2022

li4wang commented May 23, 2022

li4wang commented May 23, 2022