Hi Chris, Thanks a lot for the KIP. This will certainly be a useful feature. I would have preferred to use the topic approach as well but I also understand your point of view about the operational complexity for upgrades. If not with this KIP, I would certainly want to go that route at some point in the future.
As far as using the rebalance protocol goes, it would be great if you could elaborate on what exactly would be the rebalance impact when a key expires. I see that you have called out saying that there should be no significant impact but it will be great to explicitly state as what is to be expected. I would prefer to not have any sorts of rebalancing when this happens since the connector and task assignments should not change with this event. It will be useful to explain this a bit more. Thanks, Magesh On Wed, Aug 21, 2019 at 4:40 PM Chris Egerton <chr...@confluent.io> wrote: > Hi all, > > I've made some tweaks to the KIP that I believe are improvements. More > detail can be found on the KIP page itself, but as a brief summary, the > three changes are: > > - The removal of the internal.request.verification property in favor of > modifying the default value for the connect.protocol property from > "compatible" to "sessioned" > - The renaming of some configurations to use better terminology (mostly > just "request" instead of "key" where appropriate) > - The addition of two new configurations that dictate how session keys are > to be generated > > Thanks Ryanne for the feedback so far, hope to hear from some more of you > soon :) > > Cheers, > > Chris > > On Mon, Aug 19, 2019 at 12:02 PM Chris Egerton <chr...@confluent.io> > wrote: > > > Hi Ryanne, > > > > The reasoning for this is provided in the KIP: "There would be no clear > > way to achieve consensus amongst the workers in a cluster on whether to > > switch to this new behavior." To elaborate on this--it would be bad if a > > follower worker began writing task configurations to the topic while the > > leader continued to only listen on the internal REST endpoint. It's > > necessary to ensure that every worker in a cluster supports this new > > behavior before switching it on. > > > > To be fair, it is technically possible to achieve consensus without using > > the group coordination protocol by adding new configurations and using a > > multi-phase rolling upgrade, but the operational complexity of this > > approach would significantly complicate things for the default case of > just > > wanting to bump your Connect cluster to the next version without having > to > > alter your existing worker config files and with only a single restart > per > > worker. The proposed approach provides a much simpler user experience for > > the most common use case and doesn't impose much additional complexity > for > > other use cases. > > > > I've updated the KIP to expand on points from your last emails; let me > > know what you think. > > > > Cheers, > > > > Chris > > > > On Thu, Aug 15, 2019 at 4:53 PM Ryanne Dolan <ryannedo...@gmail.com> > > wrote: > > > >> Thanks Chris, that makes sense. > >> > >> I know you have already considered this, but I'm not convinced we should > >> rule out using Kafka topics for this purpose. That would enable the same > >> level of security without introducing any new authentication or > >> authorization mechanisms (your keys). And as you say, you'd need to lock > >> down Connect's topics and groups anyway. > >> > >> Can you explain what you mean when you say using Kafka topics would > >> require > >> "reworking the group coordination protocol"? I don't see how these are > >> related. Why would it matter if the workers sent Kafka messages vs POST > >> requests among themselves? > >> > >> Ryanne > >> > >> On Thu, Aug 15, 2019 at 3:57 PM Chris Egerton <chr...@confluent.io> > >> wrote: > >> > >> > Hi Ryanne, > >> > > >> > Yes, if the Connect group is left unsecured then that is a potential > >> > vulnerability. However, in a truly secure Connect cluster, the group > >> would > >> > need to be secured anyways to prevent attackers from joining the group > >> with > >> > the intent to either snoop on connector/task configurations or bring > the > >> > cluster to a halt by spamming the group with membership requests and > >> then > >> > not running the assigned connectors/tasks. Additionally, for a Connect > >> > cluster to be secure, access to internal topics (for configs, offsets, > >> and > >> > statuses) would also need to be restricted so that attackers could > not, > >> > e.g., write arbitrary connector/task configurations to the configs > >> topic. > >> > This is all currently possible in Kafka with the use of ACLs. > >> > > >> > I think the bottom line here is that there's a number of steps that > >> need to > >> > be taken to effectively lock down a Connect cluster; the point of this > >> KIP > >> > is to close a loophole that exists even after all of those steps are > >> taken, > >> > not to completely secure this one vulnerability even when no other > >> security > >> > measures are taken. > >> > > >> > Cheers, > >> > > >> > Chris > >> > > >> > On Wed, Aug 14, 2019 at 10:56 PM Ryanne Dolan <ryannedo...@gmail.com> > >> > wrote: > >> > > >> > > Chris, I don't understand how the rebalance protocol can be used to > >> give > >> > > out session tokens in a secure way. It seems that any attacker could > >> just > >> > > join the group and sign requests with the provided token. Am I > missing > >> > > something? > >> > > > >> > > Ryanne > >> > > > >> > > On Wed, Aug 14, 2019, 2:31 PM Chris Egerton <chr...@confluent.io> > >> wrote: > >> > > > >> > > > The KIP page can be found at > >> > > > > >> > > > > >> > > > >> > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-507%3A+Securing+Internal+Connect+REST+Endpoints > >> > > > , > >> > > > by the way. Apologies for neglecting to include it in my initial > >> email! > >> > > > > >> > > > On Wed, Aug 14, 2019 at 12:29 PM Chris Egerton < > chr...@confluent.io > >> > > >> > > > wrote: > >> > > > > >> > > > > Hi all, > >> > > > > > >> > > > > I'd like to start discussion on a KIP to secure the internal > "POST > >> > > > > /connectors/<name>/tasks" endpoint for the Connect framework. > The > >> > > > proposed > >> > > > > changes address a vulnerability in the framework in its current > >> state > >> > > > that > >> > > > > allows malicious users to write arbitrary task configurations > for > >> > > > > connectors; it is vital that this issue be addressed in order > for > >> any > >> > > > > Connect cluster to be secure. > >> > > > > > >> > > > > Looking forward to your thoughts, > >> > > > > > >> > > > > Chris > >> > > > > > >> > > > > >> > > > >> > > >> > > >
