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

ivmaykov · 2018-10-25T02:34:29Z

Add SSL config options for enabled protocols and client auth mode.
Improve handling of SSL config options for protocols and cipher suites - previously these came from system properties, now they can come from ZKConfig which means they are easier to isolate in tests, and now we don't need to parse system properties every time we create a secure socket.

Added more options for ssl settings to X509Util and encapsulate them better

previously, some SSL settings came from a ZKConfig and others came from global System.getProperties(). This made it hard to isolate certain settings in tests.
now all SSL-related settings come from the ZKConfig object used to create the SSL context
new settings added:
zookeeper.ssl(.quorum).enabledProtocols - list of enabled protocols. If not set, defaults to a single-entry list with the value of zookeeper.ssl(.quorum).protocol.
zookeeper.ssl(.quorum).clientAuth - can be "NONE", "WANT", or "NEED". This controls whether the server doesn't want / allows / requires the client to present an X509 certificate.
zookeeper.ssl(.quorum).handshakeDetectionTimeoutMillis - timeout for the first read of 5 bytes to detect the transport mode (TLS or plaintext) of a client connection made to a UnifiedServerSocket

asfgit · 2018-10-25T03:03:24Z

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

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

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

asfgit · 2018-10-27T00:04:15Z

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

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

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

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

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:07:56Z

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

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

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

asfgit · 2018-10-30T01:31:35Z

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

asfgit · 2018-10-31T18:09:37Z

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

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

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

asfgit · 2018-11-01T19:28:46Z

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

asfgit · 2018-11-01T20:54:05Z

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

asfgit · 2018-11-02T17:09:08Z

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

asfgit · 2018-11-02T17:46:22Z

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

asfgit · 2018-11-02T22:24:08Z

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

ivmaykov · 2018-12-17T23:30:33Z

It looks like a test I added in my last PR is flaky. @anmolnar let's get #739 landed then I will rebase this.

anmolnar

+1 Awesome patch.
Just a small nitpick.

anmolnar · 2019-01-14T14:25:03Z

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

+     * since we can create different X509Util instances with different configurations in a single test process, and
+     * unit test interactions between them.
+     */
+    public class SSLContextAndOptions {


I think this class is big enough to live in a separate file.

ivmaykov · 2019-01-17T01:06:20Z

@anmolnar done

anmolnar

+1 Great work!

Add SSL config options for enabled protocols and client auth mode. Improve handling of SSL config options for protocols and cipher suites - previously these came from system properties, now they can come from ZKConfig which means they are easier to isolate in tests, and now we don't need to parse system properties every time we create a secure socket. ## Added more options for ssl settings to X509Util and encapsulate them better - previously, some SSL settings came from a `ZKConfig` and others came from global `System.getProperties()`. This made it hard to isolate certain settings in tests. - now all SSL-related settings come from the `ZKConfig` object used to create the SSL context - new settings added: - `zookeeper.ssl(.quorum).enabledProtocols` - list of enabled protocols. If not set, defaults to a single-entry list with the value of `zookeeper.ssl(.quorum).protocol`. - `zookeeper.ssl(.quorum).clientAuth` - can be "NONE", "WANT", or "NEED". This controls whether the server doesn't want / allows / requires the client to present an X509 certificate. - `zookeeper.ssl(.quorum).handshakeDetectionTimeoutMillis` - timeout for the first read of 5 bytes to detect the transport mode (TLS or plaintext) of a client connection made to a `UnifiedServerSocket` Author: Ilya Maykov <[email protected]> Reviewers: [email protected] Closes #681 from ivmaykov/ZOOKEEPER-3176 (cherry picked from commit 0f44fd9) Signed-off-by: Andor Molnar <[email protected]>

anmolnar · 2019-01-24T10:53:57Z

Merged to master and 3.5 branches.
Thanks @ivmaykov !

Add SSL config options for enabled protocols and client auth mode. Improve handling of SSL config options for protocols and cipher suites - previously these came from system properties, now they can come from ZKConfig which means they are easier to isolate in tests, and now we don't need to parse system properties every time we create a secure socket. ## Added more options for ssl settings to X509Util and encapsulate them better - previously, some SSL settings came from a `ZKConfig` and others came from global `System.getProperties()`. This made it hard to isolate certain settings in tests. - now all SSL-related settings come from the `ZKConfig` object used to create the SSL context - new settings added: - `zookeeper.ssl(.quorum).enabledProtocols` - list of enabled protocols. If not set, defaults to a single-entry list with the value of `zookeeper.ssl(.quorum).protocol`. - `zookeeper.ssl(.quorum).clientAuth` - can be "NONE", "WANT", or "NEED". This controls whether the server doesn't want / allows / requires the client to present an X509 certificate. - `zookeeper.ssl(.quorum).handshakeDetectionTimeoutMillis` - timeout for the first read of 5 bytes to detect the transport mode (TLS or plaintext) of a client connection made to a `UnifiedServerSocket` Author: Ilya Maykov <[email protected]> Reviewers: [email protected] Closes apache#681 from ivmaykov/ZOOKEEPER-3176

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-3176 branch from 38b636e to 550c9b7 Compare October 25, 2018 22:27

ivmaykov force-pushed the ZOOKEEPER-3176 branch 2 times, most recently from f1501e3 to 45ec833 Compare October 26, 2018 19:30

ivmaykov force-pushed the ZOOKEEPER-3176 branch from 45ec833 to d0c38d7 Compare October 27, 2018 17:04

ivmaykov force-pushed the ZOOKEEPER-3176 branch 2 times, most recently from 0e4c55a to 94ee042 Compare October 30, 2018 00:34

ivmaykov force-pushed the ZOOKEEPER-3176 branch 2 times, most recently from d0d2943 to 3ddba01 Compare October 31, 2018 01:25

ivmaykov force-pushed the ZOOKEEPER-3176 branch 2 times, most recently from 327ced4 to 4b9ee4e Compare November 1, 2018 17:59

ivmaykov force-pushed the ZOOKEEPER-3176 branch from 4b9ee4e to cf1a4e2 Compare November 2, 2018 16:19

ivmaykov force-pushed the ZOOKEEPER-3176 branch from cf1a4e2 to 515c059 Compare November 2, 2018 17:43

ivmaykov force-pushed the ZOOKEEPER-3176 branch 2 times, most recently from befadaf to 70051f9 Compare November 2, 2018 22:13

ivmaykov mentioned this pull request Nov 29, 2018

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

Closed

ivmaykov force-pushed the ZOOKEEPER-3176 branch 2 times, most recently from 758873f to 618732b Compare December 6, 2018 22:38

ivmaykov force-pushed the ZOOKEEPER-3176 branch 2 times, most recently from 90fd62e to 0eae6a0 Compare December 17, 2018 18:34

ivmaykov closed this Dec 17, 2018

ivmaykov reopened this Dec 17, 2018

ivmaykov closed this Dec 17, 2018

ivmaykov reopened this Dec 17, 2018

ivmaykov closed this Dec 17, 2018

ivmaykov reopened this Dec 17, 2018

ivmaykov force-pushed the ZOOKEEPER-3176 branch from 0eae6a0 to d87d6f3 Compare December 20, 2018 17:24

ivmaykov closed this Dec 20, 2018

ivmaykov reopened this Dec 20, 2018

ivmaykov force-pushed the ZOOKEEPER-3176 branch from d87d6f3 to 77aeafe Compare January 2, 2019 23:47

ivmaykov force-pushed the ZOOKEEPER-3176 branch from 77aeafe to 175559b Compare January 12, 2019 00:00

ivmaykov mentioned this pull request Jan 12, 2019

ZOOKEEPER-3229: [TLS] add AES-256 ciphers to default cipher list #744

Closed

anmolnar approved these changes Jan 14, 2019

View reviewed changes

ivmaykov force-pushed the ZOOKEEPER-3176 branch from 175559b to 03b257b Compare January 17, 2019 00:49

ZOOKEEPER-3176: Quorum TLS - add SSL config options

f09b970

ivmaykov force-pushed the ZOOKEEPER-3176 branch from 03b257b to f09b970 Compare January 17, 2019 01:06

anmolnar approved these changes Jan 17, 2019

View reviewed changes

asfgit closed this in 0f44fd9 Jan 24, 2019

ivmaykov mentioned this pull request Jan 24, 2019

ZOOKEEPER-3160: Custom User SSLContext #728

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

ivmaykov commented Oct 25, 2018 •

edited

Loading

asfgit commented Oct 25, 2018

asfgit commented Oct 26, 2018

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

asfgit commented Nov 2, 2018

asfgit commented Nov 2, 2018

asfgit commented Nov 2, 2018

ivmaykov commented Dec 17, 2018

anmolnar left a comment

anmolnar Jan 14, 2019

ivmaykov commented Jan 17, 2019

anmolnar left a comment

anmolnar commented Jan 24, 2019

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

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

Conversation

ivmaykov commented Oct 25, 2018 • edited Loading

Added more options for ssl settings to X509Util and encapsulate them better

asfgit commented Oct 25, 2018

asfgit commented Oct 26, 2018

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

asfgit commented Nov 2, 2018

asfgit commented Nov 2, 2018

asfgit commented Nov 2, 2018

ivmaykov commented Dec 17, 2018

anmolnar left a comment

Choose a reason for hiding this comment

anmolnar Jan 14, 2019

Choose a reason for hiding this comment

ivmaykov commented Jan 17, 2019

anmolnar left a comment

Choose a reason for hiding this comment

anmolnar commented Jan 24, 2019

ivmaykov commented Oct 25, 2018 •

edited

Loading