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 > > > > > > > > > > >
