ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store #680

ivmaykov · 2018-10-25T02:30:14Z

Allow reloading SSL trust stores and key stores from disk when the files on disk change.

Added support for reloading key/trust stores when the file on disk changes

new property sslQuorumReloadCertFiles which controls the behavior for reloading the key and trust store files for QuorumX509Util. Reloading of key and trust store for ClientX509Util is not in this PR but could be added easily
this allows a ZK server to keep running on a machine that uses short-lived certs that refresh frequently without having to restart the ZK process.

asfgit · 2018-10-25T02:44:36Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2502/

asfgit · 2018-10-26T01:42:17Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2512/

eolivelli

+1 for the last commit

asfgit · 2018-10-27T00:03:40Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2523/

asfgit · 2018-10-27T00:10:29Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2526/

zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java

ivmaykov · 2018-10-27T17:07:37Z

I should mention that this code has been internally reviewed at Facebook, has been landed on our internal fork, and has been running in production for weeks.

asfgit · 2018-10-27T20:05:57Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2530/

asfgit · 2018-10-30T00:59:58Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2534/

asfgit · 2018-10-30T01:19:26Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2538/

asfgit · 2018-10-31T18:10:14Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2542/

asfgit · 2018-10-31T18:43:50Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2546/

asfgit · 2018-11-01T19:25:23Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2557/

asfgit · 2018-11-01T20:49:39Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2561/

ivmaykov · 2018-11-02T16:31:18Z

@eolivelli switched to lambdas, kept the finalizer in for now but added a TODO to remove it.

asfgit · 2018-11-02T16:49:32Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2565/

anmolnar · 2018-12-10T17:28:27Z

@ivmaykov For the future, I think it's more convenient for reviewers if you submit your commits separately instead of squashing them. Especially if you provide some feedback on code review, it's hard to locate new changes if everything is in a single patch. Commit script will squash all them eventually, so it doesn't really matter in PRs.

ivmaykov · 2018-12-14T20:57:19Z

@anmolnar is there anything blocking this from being merged at this point?

anmolnar · 2018-12-15T06:36:15Z

Committed to master branch. Thanks @ivmaykov !
Would you please create separate PR for 3.5, I was not able to cherry-pick it.

ivmaykov · 2018-12-15T20:27:55Z

@anmolnar will do

Allow reloading SSL trust stores and key stores from disk when the files on disk change. ## Added support for reloading key/trust stores when the file on disk changes - new property sslQuorumReloadCertFiles which controls the behavior for reloading the key and trust store files for QuorumX509Util. Reloading of key and trust store for ClientX509Util is not in this PR but could be added easily - this allows a ZK server to keep running on a machine that uses short-lived certs that refresh frequently without having to restart the ZK process. This is the branch-3.5 version of #680 Author: Ilya Maykov <[email protected]> Reviewers: [email protected], [email protected] Closes #737 from ivmaykov/ZOOKEEPER-3174-branch3.5 and squashes the following commits: 6cc1d62 [Ilya Maykov] ZOOKEEPER-3219: Fix flaky FileChangeWatcherTest df72944 [Ilya Maykov] ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store

MathewManu · 2021-11-19T18:37:15Z

@ivmaykov Appreciate commiting this PR.
I have a question regarding ClientX509Util reload, Is this implemented already or any plans? I see your comment in the checkin-

Reloading of key and trust store for ClientX509Util is not in this PR but could be added easily.

I need the ClientX509Util reload functionality for an mTLS setup that I'm doing.
Looking at the source, I could see that, ClientX509Util extends X509Util, and the reload functionality is implemented in the base class X509Util.

Does that mean, I can just add a new client config property (say sslReloadCertFiles) and call enableCertFileReloading() if it's true ? In NettyServerCnxnFactory constructor?

something like this.. my initial testing seems to be working fine.

`--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/NettyServerCnxnFactory.java
-503,6 +503,16 @@ private ServerBootstrap configureBootstrapAllocator(ServerBootstrap bootstrap) {
NettyServerCnxnFactory() {
x509Util = new ClientX509Util();

    boolean useClientReload = Boolean.getBoolean("zookeeper.client.certReload");
    if (useClientReload == true)
    {
        try {
           x509Util.enableCertFileReloading();
        } catch (IOException e1) {
            LOG.error("unable to create filewatcher!!!");
            e1.printStackTrace();
        }
     }
    `

I would appreciate if you could take a look and direct me to have this implemented.

li4wang · 2022-03-08T22:28:49Z

@MathewManu did you get ClientX509Util reload functionality working?

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. Author: Manu Mathew <[email protected]> Author: mathewmanu <[email protected]> Author: Manu Mathew <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]> Closes #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 <[email protected]> Author: mathewmanu <[email protected]> Author: Manu Mathew <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]> 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 <[email protected]> Author: mathewmanu <[email protected]> Author: Manu Mathew <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]> Closes apache#1839 from mathew-manu/ZOOKEEPER-3806

Allow reloading SSL trust stores and key stores from disk when the files on disk change. ## Added support for reloading key/trust stores when the file on disk changes - new property `sslQuorumReloadCertFiles` which controls the behavior for reloading the key and trust store files for `QuorumX509Util`. Reloading of key and trust store for `ClientX509Util` is not in this PR but could be added easily - this allows a ZK server to keep running on a machine that uses short-lived certs that refresh frequently without having to restart the ZK process. Author: Ilya Maykov <[email protected]> Reviewers: [email protected] Closes apache#680 from ivmaykov/ZOOKEEPER-3174

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 <[email protected]> Author: mathewmanu <[email protected]> Author: Manu Mathew <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]> 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 <[email protected]> Author: mathewmanu <[email protected]> Author: Manu Mathew <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]> Closes apache#1839 from mathew-manu/ZOOKEEPER-3806 Co-authored-by: Manu Mathew <[email protected]>

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 <[email protected]> Author: mathewmanu <[email protected]> Author: Manu Mathew <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]> 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 <[email protected]> Author: mathewmanu <[email protected]> Author: Manu Mathew <[email protected]> Reviewers: Enrico Olivelli <[email protected]>, Mate Szalay-Beko <[email protected]> Closes apache#1839 from mathew-manu/ZOOKEEPER-3806 Co-authored-by: Manu Mathew <[email protected]>

ivmaykov mentioned this pull request Oct 25, 2018

ZOOKEEPER-3176: Quorum TLS - add SSL config options #681

Closed

ivmaykov mentioned this pull request Oct 25, 2018

ZOOKEEPER-236: SSL Support for Atomic Broadcast protocol [part 2] #627

Closed

ivmaykov force-pushed the ZOOKEEPER-3174 branch from 65edf69 to c37855a Compare October 25, 2018 22:27

eolivelli approved these changes Oct 26, 2018

View reviewed changes

ivmaykov force-pushed the ZOOKEEPER-3174 branch 2 times, most recently from 35045e8 to 2d3a6bb Compare October 26, 2018 19:30

ivmaykov commented Oct 27, 2018

View reviewed changes

zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java Outdated Show resolved Hide resolved

ivmaykov force-pushed the ZOOKEEPER-3174 branch from 2d3a6bb to 352fc3c Compare October 27, 2018 17:04

ivmaykov force-pushed the ZOOKEEPER-3174 branch 2 times, most recently from aea9fd4 to 213af71 Compare October 30, 2018 00:33

ivmaykov force-pushed the ZOOKEEPER-3174 branch 2 times, most recently from 464501f to bb868d7 Compare October 31, 2018 01:25

ivmaykov force-pushed the ZOOKEEPER-3174 branch 2 times, most recently from 1f02993 to 232232e Compare November 1, 2018 17:58

ivmaykov force-pushed the ZOOKEEPER-3174 branch 2 times, most recently from bb06dc8 to 76317e4 Compare November 2, 2018 16:19

ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store

cc72c08

ivmaykov force-pushed the ZOOKEEPER-3174 branch from 49194a6 to cc72c08 Compare December 10, 2018 22:57

ivmaykov closed this Dec 11, 2018

ivmaykov reopened this Dec 11, 2018

asfgit closed this in e043c32 Dec 15, 2018

ivmaykov mentioned this pull request Dec 17, 2018

ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store #737

Closed

mathew-manu mentioned this pull request Mar 17, 2022

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

Closed

li4wang mentioned this pull request May 23, 2022

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

Closed

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-3174: Quorum TLS - support reloading trust/key store #680

ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store #680

ivmaykov commented Oct 25, 2018 •

edited

Loading

asfgit commented Oct 25, 2018

asfgit commented Oct 26, 2018

eolivelli left a comment

asfgit commented Oct 27, 2018

asfgit commented Oct 27, 2018

ivmaykov commented Oct 27, 2018

asfgit commented Oct 27, 2018

asfgit commented Oct 30, 2018

asfgit commented Oct 30, 2018

asfgit commented Oct 31, 2018

asfgit commented Oct 31, 2018

asfgit commented Nov 1, 2018

asfgit commented Nov 1, 2018

ivmaykov commented Nov 2, 2018

asfgit commented Nov 2, 2018

anmolnar commented Dec 10, 2018

ivmaykov commented Dec 14, 2018

anmolnar commented Dec 15, 2018

ivmaykov commented Dec 15, 2018

MathewManu commented Nov 19, 2021 •

edited

Loading

li4wang commented Mar 8, 2022

ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store #680

ZOOKEEPER-3174: Quorum TLS - support reloading trust/key store #680

Conversation

ivmaykov commented Oct 25, 2018 • edited Loading

Added support for reloading key/trust stores when the file on disk changes

asfgit commented Oct 25, 2018

asfgit commented Oct 26, 2018

eolivelli left a comment

Choose a reason for hiding this comment

asfgit commented Oct 27, 2018

asfgit commented Oct 27, 2018

ivmaykov commented Oct 27, 2018

asfgit commented Oct 27, 2018

asfgit commented Oct 30, 2018

asfgit commented Oct 30, 2018

asfgit commented Oct 31, 2018

asfgit commented Oct 31, 2018

asfgit commented Nov 1, 2018

asfgit commented Nov 1, 2018

ivmaykov commented Nov 2, 2018

asfgit commented Nov 2, 2018

anmolnar commented Dec 10, 2018

ivmaykov commented Dec 14, 2018

anmolnar commented Dec 15, 2018

ivmaykov commented Dec 15, 2018

MathewManu commented Nov 19, 2021 • edited Loading

li4wang commented Mar 8, 2022

ivmaykov commented Oct 25, 2018 •

edited

Loading

MathewManu commented Nov 19, 2021 •

edited

Loading