You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by ivmaykov <gi...@git.apache.org> on 2018/10/25 02:34:33 UTC

[GitHub] zookeeper pull request #681: ZOOKEEPER-3176: Quorum TLS - add SSL config opt...

GitHub user ivmaykov opened a pull request:

    https://github.com/apache/zookeeper/pull/681

    ZOOKEEPER-3176: Quorum TLS - add SSL config options

    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.
    
    Note that this is stacked on top of #678, #679, and #680 and thus includes them. Please only consider the ZOOKEEPER-3176 commit when reviewing. Once the other PRs are merged upstream, I will rebase this so it only contains one commit.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ivmaykov/zookeeper ZOOKEEPER-3176

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zookeeper/pull/681.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #681
    
----
commit b8b687ae4dea912ef18ee2ee1ace406800f3fce7
Author: Ilya Maykov <il...@...>
Date:   2018-10-25T00:41:48Z

    ZOOKEEPER-3173: Quorum TLS - support PEM trust/key stores
    ZOOKEEPER-3175: Quorum TLS - test improvements
    
    Add support for loading key and trust stores from PEM files.
    Also added test utils for testing X509-related code, because it
    was very difficult to untangle them from the PEM support code.

commit f9fb9c69f15f4d23acc714de75efe4592c6578b9
Author: Ilya Maykov <il...@...>
Date:   2018-10-25T01:22:24Z

    ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades

commit 65edf69084bebfc50613daafefe7ebb3afbb6e36
Author: Ilya Maykov <il...@...>
Date:   2018-10-25T01:54:06Z

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

commit 38b636e3c933967e1613b4d19425bfb681f9d7b3
Author: Ilya Maykov <il...@...>
Date:   2018-10-25T02:12:04Z

    ZOOKEEPER-3176: Quorum TLS - add SSL config options

----


---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper pull request #681: ZOOKEEPER-3176: Quorum TLS - add SSL config opt...

Posted by ivmaykov <gi...@git.apache.org>.
Github user ivmaykov closed the pull request at:

    https://github.com/apache/zookeeper/pull/681


---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper pull request #681: ZOOKEEPER-3176: Quorum TLS - add SSL config opt...

Posted by ivmaykov <gi...@git.apache.org>.
Github user ivmaykov closed the pull request at:

    https://github.com/apache/zookeeper/pull/681


---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper pull request #681: ZOOKEEPER-3176: Quorum TLS - add SSL config opt...

Posted by ivmaykov <gi...@git.apache.org>.
GitHub user ivmaykov reopened a pull request:

    https://github.com/apache/zookeeper/pull/681

    ZOOKEEPER-3176: Quorum TLS - add SSL config options

    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.
    
    Note that this is stacked on top of #678, #679, and #680 and thus includes them. Please only consider the ZOOKEEPER-3176 commit when reviewing. Once the other PRs are merged upstream, I will rebase this so it only contains one commit.
    
    ## 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`

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ivmaykov/zookeeper ZOOKEEPER-3176

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zookeeper/pull/681.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #681
    
----
commit 367bef0980193e2761c7008844c5e9fe029d8a66
Author: Ilya Maykov <il...@...>
Date:   2018-10-25T01:22:24Z

    ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades

commit fd58fa45cdd76c6b4c1bb2f529ee8f6d7fff553d
Author: Ilya Maykov <il...@...>
Date:   2018-10-25T01:54:06Z

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

commit d232235519e2c6e252dcac700dcc05146cea5dbc
Author: Ilya Maykov <il...@...>
Date:   2018-10-25T02:12:04Z

    ZOOKEEPER-3176: Quorum TLS - add SSL config options

----


---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by ivmaykov <gi...@git.apache.org>.
Github user ivmaykov commented on the issue:

    https://github.com/apache/zookeeper/pull/681
  
    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. 


---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper pull request #681: ZOOKEEPER-3176: Quorum TLS - add SSL config opt...

Posted by ivmaykov <gi...@git.apache.org>.
GitHub user ivmaykov reopened a pull request:

    https://github.com/apache/zookeeper/pull/681

    ZOOKEEPER-3176: Quorum TLS - add SSL config options

    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.
    
    Note that this is stacked on top of #678, #679, and #680 and thus includes them. Please only consider the ZOOKEEPER-3176 commit when reviewing. Once the other PRs are merged upstream, I will rebase this so it only contains one commit.
    
    ## 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`

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ivmaykov/zookeeper ZOOKEEPER-3176

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/zookeeper/pull/681.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #681
    
----
commit b45b716a59709b933c0652f48d563b08e1091e92
Author: Ilya Maykov <il...@...>
Date:   2018-10-25T01:22:24Z

    ZOOKEEPER-3172: Quorum TLS - fix port unification to allow rolling upgrades

commit 28565e3466dd35a339a3d1d71de20298072abb1a
Author: Ilya Maykov <il...@...>
Date:   2018-10-25T01:54:06Z

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

commit 4bd95d4d0f69fa73342e2c6af57c38e41421e140
Author: Ilya Maykov <il...@...>
Date:   2018-10-25T02:12:04Z

    ZOOKEEPER-3176: Quorum TLS - add SSL config options

----


---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---

[GitHub] zookeeper issue #681: ZOOKEEPER-3176: Quorum TLS - add SSL config options

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:

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



---