Hi,

Gentle reminder to kindly review the PR - Support TLS-only ZK server
<https://github.com/apache/zookeeper/pull/2117>.

Regards,
Abhilash Kishore



On Thu, 22 Feb 2024 at 13:28, Andor Molnar <an...@apache.org> wrote:

> Yes, I'll take a look also soon.
>
> Andor
>
>
>
> On Wed, 2024-02-21 at 13:03 -0800, Abhilash Kishore wrote:
> > Thanks for the review Enrico!
> >
> > Do we need another +1 or are we ready to merge?
> >
> > Regards,
> > Abhilash Kishore
> >
> >
> > On Tue, 20 Feb 2024 at 13:59, Enrico Olivelli <eolive...@gmail.com>
> > wrote:
> > > Il Mar 20 Feb 2024, 22:43 Abhilash Kishore <abhilash...@gmail.com>
> > > ha
> > > scritto:
> > >
> > > > Yes, that's the PR <
> > > https://github.com/apache/zookeeper/pull/2117>;.
> > > >
> > > > It's ready now. Do you mind taking a look?
> > > >
> > >
> > >
> > > Reviewed.
> > > Thanks
> > >
> > > Enrico
> > >
> > >
> > > > Regards,
> > > > Abhilash Kishore
> > > >
> > > >
> > > > On Tue, 13 Feb 2024 at 02:28, Andor Molnar <an...@apache.org>
> > > wrote:
> > > >
> > > > > Hi Abhilash,
> > > > >
> > > > > Is this the patch that you're working on?
> > > > >
> > > > > https://github.com/apache/zookeeper/pull/2117
> > > > >
> > > > > I see it's still draft, are u going to finish it soon?
> > > > >
> > > > > Andor
> > > > >
> > > > >
> > > > >
> > > > >
> > > > > On Mon, 2024-01-08 at 18:46 -0800, Abhilash Kishore wrote:
> > > > > > Thanks Andor, that makes sense. I agree with you, this is a
> > > simpler
> > > > > > and
> > > > > > cleaner solution.
> > > > > >
> > > > > > I'll work on the changes and will try to keep it backwards
> > > > > > compatible.
> > > > > >
> > > > > > Regards,
> > > > > > Abhilash Kishore
> > > > > >
> > > > > >
> > > > > > On Fri, 5 Jan 2024 at 09:00, Andor Molnar <an...@apache.org>
> > > wrote:
> > > > > >
> > > > > > > Hi Abhilash,
> > > > > > >
> > > > > > > Thanks for looking into this issue.
> > > > > > >
> > > > > > > I wouldn't complicate things by trying to get reconfig
> > > parameters
> > > > > > > aligned and mixed with clientPort/secureClientPort. Since
> > > the
> > > > > > > documentation says these options are already deprecated I
> > > suggest
> > > > > > > to
> > > > > > > upgrade Reconfig config line to support secure client port
> > > as well.
> > > > > > >
> > > > > > > So, the following reconfig line:
> > > > > > >
> > > > > > > "server.1=abhilash-
> > > ubuntu:3183:4183:participant;0.0.0.0:2181"
> > > > > > >
> > > > > > > will become:
> > > > > > >
> > > > > > > "server.1=abhilash-
> > > > > > > ubuntu:3183:4183:participant;0.0.0.0:2181;0.0.0.0:21
> > > > > > > 82".
> > > > > > >
> > > > > > > The 3 scenarios will become:
> > > > > > >
> > > > > > > 1. Non-TLS only:
> > > > > > >
> > > > > > > "server.1=abhilash-
> > > ubuntu:3183:4183:participant;0.0.0.0:2181;"
> > > > > > >
> > > > > > > 2. TLS-only:
> > > > > > >
> > > > > > > "server.1=abhilash-
> > > ubuntu:3183:4183:participant;;0.0.0.0:2182".
> > > > > > >
> > > > > > > 3. TLS/non-TLS mixed:
> > > > > > >
> > > > > > > "server.1=abhilash-
> > > > > > > ubuntu:3183:4183:participant;0.0.0.0:2181;0.0.0.0:21
> > > > > > > 82".
> > > > > > >
> > > > > > > In addition to that I would force the user to use either
> > > the
> > > > > > > deprecated
> > > > > > > settings (clientPort/secureClientPort) OR reconfig lines,
> > > but not
> > > > > > > both.
> > > > > > > Throw an exception and halt the server if both options are
> > > > > > > specified at
> > > > > > > the same time.
> > > > > > >
> > > > > > > Thoughts?
> > > > > > >
> > > > > > > Regards,
> > > > > > > Andor
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > On Tue, 2024-01-02 at 11:48 -0800, Abhilash Kishore wrote:
> > > > > > > > Many organizations, large and small, have strict security
> > > and
> > > > > > > > compliance
> > > > > > > > requirements to only accept encrypted/TLS connections and
> > > not
> > > > > > > > plain
> > > > > > > > text
> > > > > > > > connections.
> > > > > > > >
> > > > > > > > I'd like to discuss an issue which is preventing us from
> > > starting
> > > > > > > > our
> > > > > > > > ZK
> > > > > > > > clusters in TLS only mode (for client traffic).
> > > > > > > >
> > > > > > > > As per dynamic reconfig doc
> > > > > > > > <
> > > https://zookeeper.apache.org/doc/current/zookeeperReconfig.html>
> > > > > > > > ;;,
> > > > > > > >
> > > > > > > > > Starting with 3.5.0 the *clientPort* and
> > > *clientPortAddress*
> > > > > > > > > configuration
> > > > > > > > > parameters should no longer be used. Instead, this
> > > information
> > > > > > > > > is
> > > > > > > > > now part
> > > > > > > > > of the server keyword specification, which becomes as
> > > follows:
> > > > > > > > > server.<positive id> =
> > > > > > > > > <address1>:<port1>:<port2>[:role];[<client
> > > > > > > > > port
> > > > > > > > > address>:]<client port>
> > > > > > > >
> > > > > > > > Let's say the dynamic config entry of a server is
> > > > > > > > "server.1=abhilash-
> > > ubuntu:3183:4183:participant;0.0.0.0:2181".
> > > > > > > > The
> > > > > > > > server
> > > > > > > > starts up with a (plaintext) clientPort listener on 2181.
> > > > > > > >
> > > > > > > > Now, if we want to make this server TLS-only, what
> > > options do we
> > > > > > > > have? We
> > > > > > > > want to stop accepting plaintext traffic on 2181 and make
> > > the
> > > > > > > > same
> > > > > > > > port
> > > > > > > > accept TLS connections only (make clientPort as
> > > > > > > > secureClientPort).
> > > > > > > >
> > > > > > > > If we add "secureClientPort=2181" in zoo.cfg, then ZK
> > > server
> > > > > > > > first
> > > > > > > > starts a
> > > > > > > > plaintext listener on 2181 because of ";0.0.0.0:2181" in
> > > > > > > > "server.1"
> > > > > > > > dynamic
> > > > > > > > config entry and then attempts to start a TLS client
> > > listener on
> > > > > > > > the
> > > > > > > > same
> > > > > > > > port (2181) and fails. The reason for this behavior is
> > > already
> > > > > > > > described in
> > > > > > > > ZOOKEEPER-4276 <
> > > > > > > > https://issues.apache.org/jira/browse/ZOOKEEPER-4276'
> > > > > > > > > (highly
> > > > > > > > recommended pre-read).
> > > > > > > >
> > > > > > > > It is not possible to just remove the "<client port>"
> > > part from
> > > > > > > > the
> > > > > > > > "server.1" entry as well (I believe it is mandatory from
> > > v3.5). I
> > > > > > > > tried:
> > > > > > > >
> > > > > > > > [zk: localhost:2181(CONNECTED) 4] reconfig -remove 1
> > > > > > > > [zk: localhost:2181(CONNECTED) 5] reconfig -add
> > > > > > > > server.1=abhilash-ubuntu:3183:4183:participant
> > > > > > > > Arguments are not valid :
> > > > > > > >
> > > > > > > >
> > > > > > > > The reconfig command does not allow us to add a server
> > > entry
> > > > > > > > without
> > > > > > > > ";[<client
> > > > > > > > port address>:]<client port>".
> > > > > > > >
> > > > > > > > How do we support a "TLS-only" cluster in this case?
> > > > > > > >
> > > > > > > > My recommendation:
> > > > > > > >
> > > > > > > >    1. If both clientPort and secureClientPort are not set
> > > in
> > > > > > > > zoo.cfg,
> > > > > > > > then
> > > > > > > >    use the client port address from dynamic config.
> > > > > > > >    2. If only clientPort is set in zoo.cfg, then it has
> > > to match
> > > > > > > > the
> > > > > > > > port
> > > > > > > >    in dynamic config and ZK starts a plaintext listener
> > > on this
> > > > > > > > port.
> > > > > > > >    3. If only secureClientPort is set in zoo.cfg, then it
> > > has to
> > > > > > > > match the
> > > > > > > >    port in dynamic config and ZK starts a TLS listener on
> > > this
> > > > > > > > port.
> > > > > > > >    4. If both clientPort and secureClientPort are set in
> > > zoo.cfg,
> > > > > > > > then the
> > > > > > > >    client port in zoo.cfg should match the port in
> > > dynamic
> > > > > > > > config. ZK
> > > > > > > > starts a
> > > > > > > >    plaintext listener on clientPort and TLS listener on
> > > > > > > > secureClientPort (dual
> > > > > > > >    mode).
> > > > > > > >
> > > > > > > >
> > > > > > > > This would reintroduce the requirement to set
> > > "clientPort" in
> > > > > > > > zoo.cfg
> > > > > > > > if
> > > > > > > > someone wants to start the cluster in dual mode.
> > > > > > > >
> > > > > > > > For example,
> > > > > > > >
> > > > > > > > secureClientPort=2182
> > > > > > > > server.1=abhilash-
> > > ubuntu:3183:4183:participant;0.0.0.0:2181
> > > > > > > >
> > > > > > > > will no longer be a valid config because of rule 3 above.
> > > > > > > >
> > > > > > > > It has to be:
> > > > > > > >
> > > > > > > > clientPort=2181
> > > > > > > > secureClientPort=2182
> > > > > > > > server.1=abhilash-
> > > ubuntu:3183:4183:participant;0.0.0.0:2181
> > > > > > > >
> > > > > > > >
> > > > > > > > I can create a PR to make the above changes, but first
> > > I'd like
> > > > > > > > to
> > > > > > > > know
> > > > > > > > your thoughts on this and discuss further on whether
> > > there's a
> > > > > > > > better
> > > > > > > > way
> > > > > > > > to handle this.
> > > > > > > >
> > > > > > > > Regards,
> > > > > > > > Abhilash Kishore
> > > > >
> > > > >
> > > >
>
>

Reply via email to