zookeeper-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From anmolnar <...@git.apache.org>
Subject [GitHub] zookeeper pull request #655: ZOOKEEPER-236: SSL Support for Atomic Broadcast...
Date Fri, 05 Oct 2018 12:22:29 GMT
GitHub user anmolnar opened a pull request:

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

    ZOOKEEPER-236: SSL Support for Atomic Broadcast protocol (master)

    Cherry-picked from branch-3.5.

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

    $ git pull https://github.com/anmolnar/zookeeper ZOOKEEPER-236_master

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

    https://github.com/apache/zookeeper/pull/655.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 #655
    
----
commit 2e7451165174dcce34667f18f25ba0666b1c845e
Author: Abraham Fine <afine@...>
Date:   2018-10-05T10:44:02Z

    ZOOKEEPER-236: SSL Support for Atomic Broadcast protocol
    
    This is a work in progress, I wanted to get some feedback from the community while I worked
on this. Please do not merge yet. Tests, documentation, and some cleanup still coming.
    
    This is a first pass at ssl support for the zookeeper quorum. It supports encrypting both
leader election and normal operation.
    
    Rolling upgrades are supported via port unification (`portUnification=true`). This should
only be used while performing a rolling upgrade.
    
    Some open questions:
    - Anyone have any ideas for better names for the configuration options (`sslQuorum` and
`portUnification` currently).
    - I am using the same configuration that points to the truststore/keystore used for server
<-> client ssl. Do they need to be separate?
    - Is port unification the correct approach for rolling upgrades? Is the impact from the
use of `BufferedSocket`s during the upgrade acceptable? See: http://stackoverflow.com/questions/25637039/detecting-ssl-connection-and-converting-socket-to-sslsocket
http://stackoverflow.com/questions/6559859/is-it-possible-to-change-plain-socket-to-sslsocket
    - server <-> client ssl is implemented with netty. I did not feel that rewriting
our server <-> server logic with netty was necessary given how easy ssl was to implement
with standard java `SSLSocket`s. Any arguments to the contrary?
    
    Thanks,
    Abe
    
    Author: Andor Molnar <andor@cloudera.com>
    Author: Andor Molnar <andor@apache.org>
    
    Reviewers: hanm@apache.org, andor@apache.org
    
    Closes #184 from afine/ZOOKEEPER-236 and squashes the following commits:
    
    fdcc9151 [Andor Molnar] ZOOKEEPER-236. Replaced DefaultHostnameVerifier with custom impl
    c014a54c [Andor Molnar] ZOOKEEPER-236. Temporary disabled portUnification support
    e4144962 [Andor Molnar] ZOOKEEPER-236. Nit code review fixes
    209fbca7 [Andor Molnar] ZOOKEEPER-236. Added new JMX properties to expose SSL quorum related
settings
    1f8aab05 [Andor Molnar] ZOOKEEPER-236. Revert portUnification/sslQuorum logic
    a9fa6981 [Andor Molnar] ZOOKEEPER-236. Code review fixes:
    777f31ac [Andor Molnar] ZOOKEEPER-236. Added Java8/Java9 default cipher suites
    e8a17297 [Andor Molnar] ZOOKEEPER-236. Reverted to use single property for hostname verification
    d64eb26f [Andor Molnar] ZOOKEEPER-236. Code review related changes: - server & client
hostname verification can be set independently, - refactor defaultSSLContext to use AtomicReference,
- some minor nitpicks
    9ab476a7 [Andor Molnar] ZOOKEEPER-236. Trying to fix cipher suites test by changing the
default protocol to TLSv1.2 and filter suitable cipher suites
    ed10e88d [Andor Molnar] ZOOKEEPER-236. Added cipher suite to test to run on CentOS. Timeout
in constant value. Null checks
    c452d1b0 [Andor Molnar] ZOOKEEPER-236. Fixed unit test + added some extra debug logging
    88b61716 [Andor Molnar] ZOOKEEPER-236: SSL Support for Atomic Broadcast protocol

----


---

Mime
View raw message