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