Hi Jun, No, password.encoder.secret cannot be updated dynamically at the moment. Dynamic configs are stored in ZooKeeper and since ZK is not secure, all password configs in ZK are encrypted using password.encoder.secret. We cannot make password.encoder.secret dynamic since it would need another secret to encrypt it for storing in ZK and that secret would need to be static and cannot be rotated.
On Mon, Jan 8, 2018 at 6:33 PM, Jun Rao <j...@confluent.io> wrote: > Hi, Rajini, > > Could password.encoder.secret be updated dynamically? If so, each broker > will still have access to the old secret when password.encoder.secret is > updated. Perhaps that's a simpler way to handle changing secret than > introducing an extra config. > > Thanks, > > Jun > > On Fri, Jan 5, 2018 at 3:09 AM, Rajini Sivaram <rajinisiva...@gmail.com> > wrote: > > > Hi Jun, > > > > We are using 2-way encryption. The password configs encoded are > > keystore/truststore passwords and JAAS configuration. We need to be able > to > > extract the actual values for these, so we cannot use 1-way hash. So if > we > > have the old secret, we can decrypt and get the original values. > > > > Thank you, > > > > Rajini > > > > On Fri, Jan 5, 2018 at 12:11 AM, Jun Rao <j...@confluent.io> wrote: > > > > > Hi, Rajin, > > > > > > Does providing the old-secret help? My understanding is that the > encoded > > > passwd is the result of a 1-way hash with the secret. So, one can't > > decode > > > the passwd with old-secret. If that's the case, one still needs to > > provide > > > the unencrypted paaswd to re-encode with the new secret? > > > > > > Thanks, > > > > > > Jun > > > > > > On Thu, Jan 4, 2018 at 1:28 AM, Rajini Sivaram < > rajinisiva...@gmail.com> > > > wrote: > > > > > > > Hi Jun/Jason, > > > > > > > > I was wondering whether it is worth adding a new property (static > > config > > > in > > > > server.properties) to pass in the previous encoder password as well > > when > > > > changing encoder password. So you would set: > > > > > > > > - password.encoder.secret=new-password > > > > - password.encoder.old.secret=old-password > > > > > > > > When the broker starts up and loads passwords from ZK, it would check > > if > > > > old-password is being used. If so, it would re-encode all passwords > in > > ZK > > > > using new-password and store them back in ZK. If the new-password is > > > > already in use in ZK, the old one will be ignored. This needs an > extra > > > > property, but makes it simpler for the user since all other passwords > > can > > > > be used from ZK. > > > > > > > > What do you think? > > > > > > > > > > > > > > > > On Wed, Jan 3, 2018 at 6:01 PM, Rajini Sivaram < > > rajinisiva...@gmail.com> > > > > wrote: > > > > > > > > > Hi Jason, > > > > > > > > > > Thank you for reviewing and voting. > > > > > > > > > > Thanks, I had missed the rename. Have updated the KIP. > > > > > > > > > > The configs can be defined in the static server.properties or in > > > > > ZooKeeper. If a ZK config cannot be decoded (or is not valid), we > log > > > an > > > > > error and revert to the static config or default. When updating the > > > > secret > > > > > used by the encode, we expect all password values to be specified > in > > > > > server.properties. And the decoding or sanity check of the password > > in > > > ZK > > > > > would fail with the new secret, so we would use the password values > > > from > > > > > server.properties. Once the broker starts up, the values can be > reset > > > in > > > > ZK > > > > > using AdminClient and they will be encoded using the new secret. > > > > > > > > > > > > > > > On Wed, Jan 3, 2018 at 5:34 PM, Jason Gustafson < > ja...@confluent.io> > > > > > wrote: > > > > > > > > > >> +1 Thanks for the KIP. One minor nit: I think we changed > > > > >> ConfigSource.TOPIC_CONFIG to ConfigSource.DYNAMIC_TOPIC_CONFIG in > > the > > > > PR. > > > > >> > > > > >> As far as updating secrets, I wasn't sure I understand how that > will > > > > work. > > > > >> Do the password configs accept multiple values? > > > > >> > > > > >> On Wed, Jan 3, 2018 at 2:58 AM, Rajini Sivaram < > > > rajinisiva...@gmail.com > > > > > > > > > >> wrote: > > > > >> > > > > >> > Hi Jun, > > > > >> > > > > > >> > Thank you for reviewing and voting. > > > > >> > > > > > >> > 50. I have updated the KIP to describe how the secret may be > > > changed. > > > > >> All > > > > >> > dynamically configurable passwords and per-broker configs. So > the > > > > secret > > > > >> > can be different across brokers and updated using rolling > restart. > > > In > > > > >> order > > > > >> > to update the secret, each broker needs to be restarted with an > > > > updated > > > > >> > server.properties which contains the new secret as well as the > > > current > > > > >> > values of all the password configs. Admin client can then be > used > > to > > > > >> update > > > > >> > the passwords in ZooKeeper that are encrypted using the new > > secret. > > > > >> > > > > > >> > 51. leader.replication.throttled.replicas and > > > > >> > follower.replication.throttled.replicas > > > > >> > are dynamically configurable at the topic level. But there are > no > > > > >> defaults > > > > >> > for these at the broker level since they refer to partitions of > > the > > > > >> topic. > > > > >> > The rates used for throttling were already configurable at the > > > broker > > > > >> > level. > > > > >> > > > > > >> > I made a couple of other changes to the KIP: > > > > >> > > > > > >> > 1. The config names used for encoding passwords are now prefixed > > > with > > > > >> > password.encoder. > > > > >> > Also added key length as a config since this is constrained by > the > > > > >> > algorithm which is also configurable. > > > > >> > 2. I moved the update of inter-broker security protocol and > > > > >> > inter-broker sasl mechanism to the follow-on KIP under Future > > Work. > > > As > > > > >> part > > > > >> > of the new KIP, we need to add protocol changes to validate that > > all > > > > >> > brokers in the cluster support the new > protocol/mechanism/version > > to > > > > >> avoid > > > > >> > accidental changes before all brokers are updated. > > > > >> > > > > > >> > > > > > >> > On Tue, Jan 2, 2018 at 10:58 PM, Jun Rao <j...@confluent.io> > > wrote: > > > > >> > > > > > >> > > Hi, Rajini, > > > > >> > > > > > > >> > > Thank for the KIP. +1. Just a couple of minor comments below. > > > > >> > > > > > > >> > > > > > > >> > > 50. config.secret.*: Could you document how the > > > > encryption/decryption > > > > >> of > > > > >> > > passwd work? In particular, how do we support changing > > > > config.secret? > > > > >> > > > > > > >> > > 51. At the topic level, we also have > > leader.replication.throttled. > > > > >> > replicas > > > > >> > > and follower.replication.throttled.replicas. Should they be > > > > >> dynamically > > > > >> > > configurable? > > > > >> > > > > > > >> > > Jun > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > > > > > >> > > On Tue, Dec 12, 2017 at 9:24 AM, Gwen Shapira < > > g...@confluent.io> > > > > >> wrote: > > > > >> > > > > > > >> > > > +1 (binding). Thank you for leading this, Rajini. > > > > >> > > > > > > > >> > > > On Tue, Dec 12, 2017 at 8:35 AM Tom Bentley < > > > > t.j.bent...@gmail.com> > > > > >> > > wrote: > > > > >> > > > > > > > >> > > > > +1 (nonbinding) > > > > >> > > > > > > > > >> > > > > On 12 December 2017 at 15:34, Ted Yu <yuzhih...@gmail.com > > > > > > wrote: > > > > >> > > > > > > > > >> > > > > > +1 > > > > >> > > > > > > > > > >> > > > > > On Tue, Dec 12, 2017 at 5:44 AM, Rajini Sivaram < > > > > >> > > > rajinisiva...@gmail.com > > > > >> > > > > > > > > > >> > > > > > wrote: > > > > >> > > > > > > > > > >> > > > > > > Since there are no more outstanding comments, I would > > like > > > > to > > > > >> > start > > > > >> > > > > vote > > > > >> > > > > > > for KIP-226: > > > > >> > > > > > > > > > > >> > > > > > > https://cwiki.apache.org/ > confluence/display/KAFKA/KIP- > > > > >> > > > > > > 226+-+Dynamic+Broker+Configuration > > > > >> > > > > > > > > > > >> > > > > > > > > > > >> > > > > > > The KIP enables dynamic update of commonly updated > > broker > > > > >> > > > configuration > > > > >> > > > > > > options to avoid expensive restarts. > > > > >> > > > > > > > > > > >> > > > > > > Thank you, > > > > >> > > > > > > > > > > >> > > > > > > Rajini > > > > >> > > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > >> > > > > > >> > > > > > > > > > > > > > > > > > > > >
