Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
ime. I would suggest you do a deeper dive into the >>>> sample >>>> > Pull >>>> > > request, build the code to get better idea about it. I don't find it >>>> > > strange to have 'Mode' argument in this context of Kafka. Kafka is >>>> not >>>> > > doing anything out of norm here since ultimately it is using JSSE >>>> spec >>>> > for >>>> > > SSL Connection. >>>> > > >>>> > > I will try to respond to code comments in couple of weeks since I >>>> am out >>>> > > for few weeks. Will keep you guys posted. >>>> > > >>>> > > Thanks >>>> > > Maulin >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> > > On Wed, Feb 5, 2020 at 9:50 PM Pellerin, Clement < >>>> > clement_pelle...@ibi.com> >>>> > > wrote: >>>> > > >>>> > >> Many of these points came up before. >>>> > >> >>>> > >> I had great hope when Maulin suggested the custom factory could >>>> > >> return an SSLContext instead of SSLEngine. SSLContext factories >>>> are >>>> > >> common, >>>> > >> whereas I have never seen an SSLEngine factory being used before. >>>> > >> He must have hit the same problem I had with the Mode. >>>> > >> >>>> > >> If the Mode can be removed, can we find a way to return an >>>> SSLContext >>>> > now? >>>> > >> What is so special about Kafka that it needs to hardcode the Mode >>>> when >>>> > >> everyone >>>> > >> else works with the SSLContext and ignores the other mode they >>>> don't >>>> > use. >>>> > >> >>>> > >> -Original Message- >>>> > >> From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] >>>> > >> Sent: Wednesday, February 5, 2020 10:03 AM >>>> > >> To: dev >>>> > >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine >>>> configuration >>>> > >> extensible >>>> > >> >>>> > >> One more point: >>>> > >> 5) We should also add a method to SslEngineFactory that returns >>>> > >> `Set >>>> > >> reconfigurableConfigs()` >>>> > >> >>>> > >> On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram < >>>> rajinisiva...@gmail.com> >>>> > >> wrote: >>>> > >> >>>> > >> > Hi Maulin, >>>> > >> > >>>> > >> > Thanks for the updates. A few comments below: >>>> > >> > >>>> > >> > 1) SslEngineFactory is currently in the internal package >>>> > >> > org.apache.kafka.common.security.ssl. We should move it to the >>>> public >>>> > >> > package org.apache.kafka.common.security.auth. >>>> > >> > 2) SslEngineFactory should extend Configurable and Closeable. We >>>> > should >>>> > >> > expect the implementation class to have a default constructor and >>>> > >> invoke configure() >>>> > >> > to be consistent with otSslher pluggable classes. >>>> > >> > 3) SslEngineFactory.createSslEngine uses the internal enum >>>> `Mode`. It >>>> > >> > would be better to add two methods instead: >>>> > >> > >>>> > >> > >>>> > >> >- SSLEngine createClientSslEngine(String peerHost, int >>>> peerPort, >>>> > >> String endpointIdentification); >>>> > >> >- SSLEngine createServerSslEngine(String peerHost, int >>>> peerPort); >>>> > >> > >>>> > >> > 4) Don't think we need a method on SslEngineFactory to return >>>> configs. >>>> > >> It seems like an odd thing to do in a pubic Configurable API and is >>>> > >> inconsistent with other APIs. We can track configs in the internal >>>> > >> SslFactory class instead. >>>> > >> >>>> > > >>>> > >>>> >>>
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Rajini When I configure the Default value for ssl.engine.factory.class as Type Class, it is resulting into lot of test cases failures since in many places - tests and actual classes/scala code it is converting the configuration maps value to value.toString(). I verified that was the issues by fixing some of it but still evaluating how many places we need to fix if we really want to support Class Type key for the configuration. Password and List Types are also of non-String types but it seems their defaults are Null and they are optional vs in my case the ssl.engine.factory.class is *not* optional - it needs a value by default. Will keep you posted. I am thinking of reverting the config type to String and then load it as String and do Class loading in SslFactory. Thanks Maulin On Mon, Mar 23, 2020 at 1:38 AM Maulin Vasavada wrote: > still working on the pull request. hopefully will be done soon. > > On Wed, Mar 11, 2020 at 11:48 AM Maulin Vasavada < > maulin.vasav...@gmail.com> wrote: > >> Thanks Rajini. Sounds good. I'll make changes and update this thread. >> >> On Wed, Mar 11, 2020 at 6:41 AM Rajini Sivaram >> wrote: >> >>> Hi Maulin, >>> >>> I have reviewed the PR and left some comments, can you turn it into a PR >>> for https://github.com/apache/kafka? It looks good overall. >>> >>> We pass all configs to other plugins, so we can do the same here. That >>> would be safer than assuming that all custom SSL-related configs start >>> with >>> `ssl.`. You can use principal builder as an example for what we do today. >>> >>> Regards, >>> >>> Rajini >>> >>> On Thu, Mar 5, 2020 at 12:03 AM Maulin Vasavada < >>> maulin.vasav...@gmail.com> >>> wrote: >>> >>> > Hi Rajini >>> > >>> > I made changes suggested by you on >>> > https://github.com/maulin-vasavada/kafka/pull/4. Please check. >>> > >>> > In particular I had challenge in 'SslFactory#configure()' method the >>> first >>> > time to know which configs I have to add without having actual >>> > SslEngineFactory impl. I think it is best to just copy all configs with >>> > "ssl." prefix. Can you please review >>> > >>> > >>> https://github.com/maulin-vasavada/kafka/pull/4/files#diff-1e3432211fdbb7b2e2b44b5d8838a40bR90 >>> > particularly? >>> > >>> > Clement, I missed to see your point about Mode in previous post but >>> even >>> > after I realized what you are suggesting, my response would be the >>> same as >>> > before :) >>> > >>> > Thanks >>> > Maulin >>> > >>> > >>> > On Wed, Feb 5, 2020 at 8:40 PM Maulin Vasavada < >>> maulin.vasav...@gmail.com> >>> > wrote: >>> > >>> > > Hi Rajini >>> > > >>> > > Will accommodate your comments. >>> > > >>> > > Celement, while SSLContext factories are common, in this particular >>> case, >>> > > we need SSLEngine object because Kafka is using SSLEngine (as >>> mentioned >>> > > much before in this email thread, the SSLContext acts as factory for >>> > > getting SSLEngine, SSLSocketFactory or SSLServerSocketFactory and >>> Kafka >>> > > chooses SSLEngine to be used for SSL Connections). The 'Mode' >>> challenge >>> > > doesn't go away easily since Kafka is using the "same" classes for >>> Client >>> > > side and Server side. Other stacks where you don't see this >>> challenge is >>> > > because either libraries are client centric or server centric and not >>> > both >>> > > at the same time. I would suggest you do a deeper dive into the >>> sample >>> > Pull >>> > > request, build the code to get better idea about it. I don't find it >>> > > strange to have 'Mode' argument in this context of Kafka. Kafka is >>> not >>> > > doing anything out of norm here since ultimately it is using JSSE >>> spec >>> > for >>> > > SSL Connection. >>> > > >>> > > I will try to respond to code comments in couple of weeks since I am >>> out >>> > > for few weeks. Will keep you guys posted. >>> > > >>> > > Thanks >>> > > Maulin >>> > > >>> > > >>> >
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
still working on the pull request. hopefully will be done soon. On Wed, Mar 11, 2020 at 11:48 AM Maulin Vasavada wrote: > Thanks Rajini. Sounds good. I'll make changes and update this thread. > > On Wed, Mar 11, 2020 at 6:41 AM Rajini Sivaram > wrote: > >> Hi Maulin, >> >> I have reviewed the PR and left some comments, can you turn it into a PR >> for https://github.com/apache/kafka? It looks good overall. >> >> We pass all configs to other plugins, so we can do the same here. That >> would be safer than assuming that all custom SSL-related configs start >> with >> `ssl.`. You can use principal builder as an example for what we do today. >> >> Regards, >> >> Rajini >> >> On Thu, Mar 5, 2020 at 12:03 AM Maulin Vasavada < >> maulin.vasav...@gmail.com> >> wrote: >> >> > Hi Rajini >> > >> > I made changes suggested by you on >> > https://github.com/maulin-vasavada/kafka/pull/4. Please check. >> > >> > In particular I had challenge in 'SslFactory#configure()' method the >> first >> > time to know which configs I have to add without having actual >> > SslEngineFactory impl. I think it is best to just copy all configs with >> > "ssl." prefix. Can you please review >> > >> > >> https://github.com/maulin-vasavada/kafka/pull/4/files#diff-1e3432211fdbb7b2e2b44b5d8838a40bR90 >> > particularly? >> > >> > Clement, I missed to see your point about Mode in previous post but even >> > after I realized what you are suggesting, my response would be the same >> as >> > before :) >> > >> > Thanks >> > Maulin >> > >> > >> > On Wed, Feb 5, 2020 at 8:40 PM Maulin Vasavada < >> maulin.vasav...@gmail.com> >> > wrote: >> > >> > > Hi Rajini >> > > >> > > Will accommodate your comments. >> > > >> > > Celement, while SSLContext factories are common, in this particular >> case, >> > > we need SSLEngine object because Kafka is using SSLEngine (as >> mentioned >> > > much before in this email thread, the SSLContext acts as factory for >> > > getting SSLEngine, SSLSocketFactory or SSLServerSocketFactory and >> Kafka >> > > chooses SSLEngine to be used for SSL Connections). The 'Mode' >> challenge >> > > doesn't go away easily since Kafka is using the "same" classes for >> Client >> > > side and Server side. Other stacks where you don't see this challenge >> is >> > > because either libraries are client centric or server centric and not >> > both >> > > at the same time. I would suggest you do a deeper dive into the sample >> > Pull >> > > request, build the code to get better idea about it. I don't find it >> > > strange to have 'Mode' argument in this context of Kafka. Kafka is not >> > > doing anything out of norm here since ultimately it is using JSSE spec >> > for >> > > SSL Connection. >> > > >> > > I will try to respond to code comments in couple of weeks since I am >> out >> > > for few weeks. Will keep you guys posted. >> > > >> > > Thanks >> > > Maulin >> > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > On Wed, Feb 5, 2020 at 9:50 PM Pellerin, Clement < >> > clement_pelle...@ibi.com> >> > > wrote: >> > > >> > >> Many of these points came up before. >> > >> >> > >> I had great hope when Maulin suggested the custom factory could >> > >> return an SSLContext instead of SSLEngine. SSLContext factories are >> > >> common, >> > >> whereas I have never seen an SSLEngine factory being used before. >> > >> He must have hit the same problem I had with the Mode. >> > >> >> > >> If the Mode can be removed, can we find a way to return an SSLContext >> > now? >> > >> What is so special about Kafka that it needs to hardcode the Mode >> when >> > >> everyone >> > >> else works with the SSLContext and ignores the other mode they don't >> > use. >> > >> >> > >> -Original Message- >> > >> From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] >> > >> Sent: Wednesday, February 5, 2020 10:03 AM
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Thanks Rajini. Sounds good. I'll make changes and update this thread. On Wed, Mar 11, 2020 at 6:41 AM Rajini Sivaram wrote: > Hi Maulin, > > I have reviewed the PR and left some comments, can you turn it into a PR > for https://github.com/apache/kafka? It looks good overall. > > We pass all configs to other plugins, so we can do the same here. That > would be safer than assuming that all custom SSL-related configs start with > `ssl.`. You can use principal builder as an example for what we do today. > > Regards, > > Rajini > > On Thu, Mar 5, 2020 at 12:03 AM Maulin Vasavada > > wrote: > > > Hi Rajini > > > > I made changes suggested by you on > > https://github.com/maulin-vasavada/kafka/pull/4. Please check. > > > > In particular I had challenge in 'SslFactory#configure()' method the > first > > time to know which configs I have to add without having actual > > SslEngineFactory impl. I think it is best to just copy all configs with > > "ssl." prefix. Can you please review > > > > > https://github.com/maulin-vasavada/kafka/pull/4/files#diff-1e3432211fdbb7b2e2b44b5d8838a40bR90 > > particularly? > > > > Clement, I missed to see your point about Mode in previous post but even > > after I realized what you are suggesting, my response would be the same > as > > before :) > > > > Thanks > > Maulin > > > > > > On Wed, Feb 5, 2020 at 8:40 PM Maulin Vasavada < > maulin.vasav...@gmail.com> > > wrote: > > > > > Hi Rajini > > > > > > Will accommodate your comments. > > > > > > Celement, while SSLContext factories are common, in this particular > case, > > > we need SSLEngine object because Kafka is using SSLEngine (as mentioned > > > much before in this email thread, the SSLContext acts as factory for > > > getting SSLEngine, SSLSocketFactory or SSLServerSocketFactory and Kafka > > > chooses SSLEngine to be used for SSL Connections). The 'Mode' challenge > > > doesn't go away easily since Kafka is using the "same" classes for > Client > > > side and Server side. Other stacks where you don't see this challenge > is > > > because either libraries are client centric or server centric and not > > both > > > at the same time. I would suggest you do a deeper dive into the sample > > Pull > > > request, build the code to get better idea about it. I don't find it > > > strange to have 'Mode' argument in this context of Kafka. Kafka is not > > > doing anything out of norm here since ultimately it is using JSSE spec > > for > > > SSL Connection. > > > > > > I will try to respond to code comments in couple of weeks since I am > out > > > for few weeks. Will keep you guys posted. > > > > > > Thanks > > > Maulin > > > > > > > > > > > > > > > > > > > > > > > > > > > On Wed, Feb 5, 2020 at 9:50 PM Pellerin, Clement < > > clement_pelle...@ibi.com> > > > wrote: > > > > > >> Many of these points came up before. > > >> > > >> I had great hope when Maulin suggested the custom factory could > > >> return an SSLContext instead of SSLEngine. SSLContext factories are > > >> common, > > >> whereas I have never seen an SSLEngine factory being used before. > > >> He must have hit the same problem I had with the Mode. > > >> > > >> If the Mode can be removed, can we find a way to return an SSLContext > > now? > > >> What is so special about Kafka that it needs to hardcode the Mode when > > >> everyone > > >> else works with the SSLContext and ignores the other mode they don't > > use. > > >> > > >> -Original Message- > > >> From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] > > >> Sent: Wednesday, February 5, 2020 10:03 AM > > >> To: dev > > >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > > >> extensible > > >> > > >> One more point: > > >> 5) We should also add a method to SslEngineFactory that returns > > >> `Set > > >> reconfigurableConfigs()` > > >> > > >> On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram < > rajinisiva...@gmail.com> > > >> wrote: > > >> > > >> > Hi Maulin, > > >> > > > >> > Thanks for the updates.
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Maulin, I have reviewed the PR and left some comments, can you turn it into a PR for https://github.com/apache/kafka? It looks good overall. We pass all configs to other plugins, so we can do the same here. That would be safer than assuming that all custom SSL-related configs start with `ssl.`. You can use principal builder as an example for what we do today. Regards, Rajini On Thu, Mar 5, 2020 at 12:03 AM Maulin Vasavada wrote: > Hi Rajini > > I made changes suggested by you on > https://github.com/maulin-vasavada/kafka/pull/4. Please check. > > In particular I had challenge in 'SslFactory#configure()' method the first > time to know which configs I have to add without having actual > SslEngineFactory impl. I think it is best to just copy all configs with > "ssl." prefix. Can you please review > > https://github.com/maulin-vasavada/kafka/pull/4/files#diff-1e3432211fdbb7b2e2b44b5d8838a40bR90 > particularly? > > Clement, I missed to see your point about Mode in previous post but even > after I realized what you are suggesting, my response would be the same as > before :) > > Thanks > Maulin > > > On Wed, Feb 5, 2020 at 8:40 PM Maulin Vasavada > wrote: > > > Hi Rajini > > > > Will accommodate your comments. > > > > Celement, while SSLContext factories are common, in this particular case, > > we need SSLEngine object because Kafka is using SSLEngine (as mentioned > > much before in this email thread, the SSLContext acts as factory for > > getting SSLEngine, SSLSocketFactory or SSLServerSocketFactory and Kafka > > chooses SSLEngine to be used for SSL Connections). The 'Mode' challenge > > doesn't go away easily since Kafka is using the "same" classes for Client > > side and Server side. Other stacks where you don't see this challenge is > > because either libraries are client centric or server centric and not > both > > at the same time. I would suggest you do a deeper dive into the sample > Pull > > request, build the code to get better idea about it. I don't find it > > strange to have 'Mode' argument in this context of Kafka. Kafka is not > > doing anything out of norm here since ultimately it is using JSSE spec > for > > SSL Connection. > > > > I will try to respond to code comments in couple of weeks since I am out > > for few weeks. Will keep you guys posted. > > > > Thanks > > Maulin > > > > > > > > > > > > > > > > > > On Wed, Feb 5, 2020 at 9:50 PM Pellerin, Clement < > clement_pelle...@ibi.com> > > wrote: > > > >> Many of these points came up before. > >> > >> I had great hope when Maulin suggested the custom factory could > >> return an SSLContext instead of SSLEngine. SSLContext factories are > >> common, > >> whereas I have never seen an SSLEngine factory being used before. > >> He must have hit the same problem I had with the Mode. > >> > >> If the Mode can be removed, can we find a way to return an SSLContext > now? > >> What is so special about Kafka that it needs to hardcode the Mode when > >> everyone > >> else works with the SSLContext and ignores the other mode they don't > use. > >> > >> -Original Message- > >> From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] > >> Sent: Wednesday, February 5, 2020 10:03 AM > >> To: dev > >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > >> extensible > >> > >> One more point: > >> 5) We should also add a method to SslEngineFactory that returns > >> `Set > >> reconfigurableConfigs()` > >> > >> On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram > >> wrote: > >> > >> > Hi Maulin, > >> > > >> > Thanks for the updates. A few comments below: > >> > > >> > 1) SslEngineFactory is currently in the internal package > >> > org.apache.kafka.common.security.ssl. We should move it to the public > >> > package org.apache.kafka.common.security.auth. > >> > 2) SslEngineFactory should extend Configurable and Closeable. We > should > >> > expect the implementation class to have a default constructor and > >> invoke configure() > >> > to be consistent with otSslher pluggable classes. > >> > 3) SslEngineFactory.createSslEngine uses the internal enum `Mode`. It > >> > would be better to add two methods instead: > >> > > >> > > >> >- SSLEngine createClientSslEngine(String peerHost, int peerPort, > >> String endpointIdentification); > >> >- SSLEngine createServerSslEngine(String peerHost, int peerPort); > >> > > >> > 4) Don't think we need a method on SslEngineFactory to return configs. > >> It seems like an odd thing to do in a pubic Configurable API and is > >> inconsistent with other APIs. We can track configs in the internal > >> SslFactory class instead. > >> > > >
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Rajini I made changes suggested by you on https://github.com/maulin-vasavada/kafka/pull/4. Please check. In particular I had challenge in 'SslFactory#configure()' method the first time to know which configs I have to add without having actual SslEngineFactory impl. I think it is best to just copy all configs with "ssl." prefix. Can you please review https://github.com/maulin-vasavada/kafka/pull/4/files#diff-1e3432211fdbb7b2e2b44b5d8838a40bR90 particularly? Clement, I missed to see your point about Mode in previous post but even after I realized what you are suggesting, my response would be the same as before :) Thanks Maulin On Wed, Feb 5, 2020 at 8:40 PM Maulin Vasavada wrote: > Hi Rajini > > Will accommodate your comments. > > Celement, while SSLContext factories are common, in this particular case, > we need SSLEngine object because Kafka is using SSLEngine (as mentioned > much before in this email thread, the SSLContext acts as factory for > getting SSLEngine, SSLSocketFactory or SSLServerSocketFactory and Kafka > chooses SSLEngine to be used for SSL Connections). The 'Mode' challenge > doesn't go away easily since Kafka is using the "same" classes for Client > side and Server side. Other stacks where you don't see this challenge is > because either libraries are client centric or server centric and not both > at the same time. I would suggest you do a deeper dive into the sample Pull > request, build the code to get better idea about it. I don't find it > strange to have 'Mode' argument in this context of Kafka. Kafka is not > doing anything out of norm here since ultimately it is using JSSE spec for > SSL Connection. > > I will try to respond to code comments in couple of weeks since I am out > for few weeks. Will keep you guys posted. > > Thanks > Maulin > > > > > > > > > On Wed, Feb 5, 2020 at 9:50 PM Pellerin, Clement > wrote: > >> Many of these points came up before. >> >> I had great hope when Maulin suggested the custom factory could >> return an SSLContext instead of SSLEngine. SSLContext factories are >> common, >> whereas I have never seen an SSLEngine factory being used before. >> He must have hit the same problem I had with the Mode. >> >> If the Mode can be removed, can we find a way to return an SSLContext now? >> What is so special about Kafka that it needs to hardcode the Mode when >> everyone >> else works with the SSLContext and ignores the other mode they don't use. >> >> -Original Message- >> From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] >> Sent: Wednesday, February 5, 2020 10:03 AM >> To: dev >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >> extensible >> >> One more point: >> 5) We should also add a method to SslEngineFactory that returns >> `Set >> reconfigurableConfigs()` >> >> On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram >> wrote: >> >> > Hi Maulin, >> > >> > Thanks for the updates. A few comments below: >> > >> > 1) SslEngineFactory is currently in the internal package >> > org.apache.kafka.common.security.ssl. We should move it to the public >> > package org.apache.kafka.common.security.auth. >> > 2) SslEngineFactory should extend Configurable and Closeable. We should >> > expect the implementation class to have a default constructor and >> invoke configure() >> > to be consistent with otSslher pluggable classes. >> > 3) SslEngineFactory.createSslEngine uses the internal enum `Mode`. It >> > would be better to add two methods instead: >> > >> > >> >- SSLEngine createClientSslEngine(String peerHost, int peerPort, >> String endpointIdentification); >> >- SSLEngine createServerSslEngine(String peerHost, int peerPort); >> > >> > 4) Don't think we need a method on SslEngineFactory to return configs. >> It seems like an odd thing to do in a pubic Configurable API and is >> inconsistent with other APIs. We can track configs in the internal >> SslFactory class instead. >> >
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Rajini Will accommodate your comments. Celement, while SSLContext factories are common, in this particular case, we need SSLEngine object because Kafka is using SSLEngine (as mentioned much before in this email thread, the SSLContext acts as factory for getting SSLEngine, SSLSocketFactory or SSLServerSocketFactory and Kafka chooses SSLEngine to be used for SSL Connections). The 'Mode' challenge doesn't go away easily since Kafka is using the "same" classes for Client side and Server side. Other stacks where you don't see this challenge is because either libraries are client centric or server centric and not both at the same time. I would suggest you do a deeper dive into the sample Pull request, build the code to get better idea about it. I don't find it strange to have 'Mode' argument in this context of Kafka. Kafka is not doing anything out of norm here since ultimately it is using JSSE spec for SSL Connection. I will try to respond to code comments in couple of weeks since I am out for few weeks. Will keep you guys posted. Thanks Maulin On Wed, Feb 5, 2020 at 9:50 PM Pellerin, Clement wrote: > Many of these points came up before. > > I had great hope when Maulin suggested the custom factory could > return an SSLContext instead of SSLEngine. SSLContext factories are > common, > whereas I have never seen an SSLEngine factory being used before. > He must have hit the same problem I had with the Mode. > > If the Mode can be removed, can we find a way to return an SSLContext now? > What is so special about Kafka that it needs to hardcode the Mode when > everyone > else works with the SSLContext and ignores the other mode they don't use. > > -Original Message- > From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] > Sent: Wednesday, February 5, 2020 10:03 AM > To: dev > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > One more point: > 5) We should also add a method to SslEngineFactory that returns > `Set > reconfigurableConfigs()` > > On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram > wrote: > > > Hi Maulin, > > > > Thanks for the updates. A few comments below: > > > > 1) SslEngineFactory is currently in the internal package > > org.apache.kafka.common.security.ssl. We should move it to the public > > package org.apache.kafka.common.security.auth. > > 2) SslEngineFactory should extend Configurable and Closeable. We should > > expect the implementation class to have a default constructor and invoke > configure() > > to be consistent with otSslher pluggable classes. > > 3) SslEngineFactory.createSslEngine uses the internal enum `Mode`. It > > would be better to add two methods instead: > > > > > >- SSLEngine createClientSslEngine(String peerHost, int peerPort, > String endpointIdentification); > >- SSLEngine createServerSslEngine(String peerHost, int peerPort); > > > > 4) Don't think we need a method on SslEngineFactory to return configs. > It seems like an odd thing to do in a pubic Configurable API and is > inconsistent with other APIs. We can track configs in the internal > SslFactory class instead. >
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Many of these points came up before. I had great hope when Maulin suggested the custom factory could return an SSLContext instead of SSLEngine. SSLContext factories are common, whereas I have never seen an SSLEngine factory being used before. He must have hit the same problem I had with the Mode. If the Mode can be removed, can we find a way to return an SSLContext now? What is so special about Kafka that it needs to hardcode the Mode when everyone else works with the SSLContext and ignores the other mode they don't use. -Original Message- From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] Sent: Wednesday, February 5, 2020 10:03 AM To: dev Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible One more point: 5) We should also add a method to SslEngineFactory that returns `Set reconfigurableConfigs()` On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram wrote: > Hi Maulin, > > Thanks for the updates. A few comments below: > > 1) SslEngineFactory is currently in the internal package > org.apache.kafka.common.security.ssl. We should move it to the public > package org.apache.kafka.common.security.auth. > 2) SslEngineFactory should extend Configurable and Closeable. We should > expect the implementation class to have a default constructor and invoke > configure() > to be consistent with otSslher pluggable classes. > 3) SslEngineFactory.createSslEngine uses the internal enum `Mode`. It > would be better to add two methods instead: > > >- SSLEngine createClientSslEngine(String peerHost, int peerPort, String > endpointIdentification); >- SSLEngine createServerSslEngine(String peerHost, int peerPort); > > 4) Don't think we need a method on SslEngineFactory to return configs. It > seems like an odd thing to do in a pubic Configurable API and is inconsistent > with other APIs. We can track configs in the internal SslFactory class > instead.
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
One more point: 5) We should also add a method to SslEngineFactory that returns `Set reconfigurableConfigs()` On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram wrote: > Hi Maulin, > > Thanks for the updates. A few comments below: > > 1) SslEngineFactory is currently in the internal package > org.apache.kafka.common.security.ssl. We should move it to the public > package org.apache.kafka.common.security.auth. > 2) SslEngineFactory should extend Configurable and Closeable. We should > expect the implementation class to have a default constructor and invoke > configure() > to be consistent with otSslher pluggable classes. > 3) SslEngineFactory.createSslEngine uses the internal enum `Mode`. It > would be better to add two methods instead: > > >- SSLEngine createClientSslEngine(String peerHost, int peerPort, String > endpointIdentification); >- SSLEngine createServerSslEngine(String peerHost, int peerPort); > > 4) Don't think we need a method on SslEngineFactory to return configs. It > seems like an odd thing to do in a pubic Configurable API and is inconsistent > with other APIs. We can track configs in the internal SslFactory class > instead. > > > On Mon, Jan 27, 2020 at 6:59 AM Maulin Vasavada > wrote: > >> Hi all >> >> I will start the voting thread on this now. >> >> Thanks >> Maulin >> >> On Thu, Jan 23, 2020 at 12:51 AM Maulin Vasavada < >> maulin.vasav...@gmail.com> >> wrote: >> >> > Hi all, >> > >> > I have updated the KIP document with the current state of conclusions. >> > Please review it and see if we are ready to move to Voting! >> > >> > Thanks >> > Maulin >> > >> > On Wed, Jan 22, 2020 at 12:42 AM Maulin Vasavada < >> > maulin.vasav...@gmail.com> wrote: >> > >> >> Hi all, >> >> >> >> Finally I squeezed time and I've a suggested code changes shown at >> >> https://github.com/maulin-vasavada/kafka/pull/4/files for discussing >> >> this further. I'll update the KIP document soon. Meanwhile, can you >> please >> >> take a look and continue the discussion? >> >> >> >> One challenge is at: >> >> >> https://github.com/maulin-vasavada/kafka/pull/4/files#diff-1e3432211fdbb7b2e2b44b5d8838a40bR89 >> >> >> >> Thanks >> >> Maulin >> >> >> >> >> >> On Tue, Oct 22, 2019 at 11:13 PM Maulin Vasavada < >> >> maulin.vasav...@gmail.com> wrote: >> >> >> >>> bump! Clement/Rajini? Any responses based on the latest posts? >> >>> >> >>> On Wed, Oct 16, 2019 at 10:58 PM Maulin Vasavada < >> >>> maulin.vasav...@gmail.com> wrote: >> >>> >> >>>> bump! >> >>>> >> >>>> On Sun, Oct 13, 2019 at 11:16 PM Maulin Vasavada < >> >>>> maulin.vasav...@gmail.com> wrote: >> >>>> >> >>>>> Hi Clement >> >>>>> >> >>>>> 1) existing validation code will remain in SslFactory >> >>>>> 2) the createEngine() method in SslEngineBuilder will move to >> >>>>> SslFactory and the client/server mode setting will go there (I >> documented >> >>>>> this in the latest KIP update) >> >>>>> >> >>>>> In the current KIP I am proposing (as per the latest updates) to >> make >> >>>>> SSLContext loading/configuration/creation pluggable. I am not >> suggesting we >> >>>>> do/repeat anything that is already addressed by the existing >> Providers for >> >>>>> SSLContext implementation. The createEngine() method (which will >> move to >> >>>>> SslFactory) will call SslContextFactory.create() to get references >> to the >> >>>>> SSLContext and then call SSLContext#createEngine(peer, host) and set >> >>>>> client/server mode as it does today. I'll try to put that in a >> sequence >> >>>>> diagram and update the KIP to make it clearer. >> >>>>> >> >>>>> So to your question about SslFactory returning SSLContext - I am >> >>>>> saying register SslContextFactory interface to provide the >> SSLContext >> >>>>> object instead and keep SslFactory more-or-less as it is today with >> some >> >>>>> additional responsibility of createEngine() met
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Maulin, Thanks for the updates. A few comments below: 1) SslEngineFactory is currently in the internal package org.apache.kafka.common.security.ssl. We should move it to the public package org.apache.kafka.common.security.auth. 2) SslEngineFactory should extend Configurable and Closeable. We should expect the implementation class to have a default constructor and invoke configure() to be consistent with otSslher pluggable classes. 3) SslEngineFactory.createSslEngine uses the internal enum `Mode`. It would be better to add two methods instead: - SSLEngine createClientSslEngine(String peerHost, int peerPort, String endpointIdentification); - SSLEngine createServerSslEngine(String peerHost, int peerPort); 4) Don't think we need a method on SslEngineFactory to return configs. It seems like an odd thing to do in a pubic Configurable API and is inconsistent with other APIs. We can track configs in the internal SslFactory class instead. On Mon, Jan 27, 2020 at 6:59 AM Maulin Vasavada wrote: > Hi all > > I will start the voting thread on this now. > > Thanks > Maulin > > On Thu, Jan 23, 2020 at 12:51 AM Maulin Vasavada < > maulin.vasav...@gmail.com> > wrote: > > > Hi all, > > > > I have updated the KIP document with the current state of conclusions. > > Please review it and see if we are ready to move to Voting! > > > > Thanks > > Maulin > > > > On Wed, Jan 22, 2020 at 12:42 AM Maulin Vasavada < > > maulin.vasav...@gmail.com> wrote: > > > >> Hi all, > >> > >> Finally I squeezed time and I've a suggested code changes shown at > >> https://github.com/maulin-vasavada/kafka/pull/4/files for discussing > >> this further. I'll update the KIP document soon. Meanwhile, can you > please > >> take a look and continue the discussion? > >> > >> One challenge is at: > >> > https://github.com/maulin-vasavada/kafka/pull/4/files#diff-1e3432211fdbb7b2e2b44b5d8838a40bR89 > >> > >> Thanks > >> Maulin > >> > >> > >> On Tue, Oct 22, 2019 at 11:13 PM Maulin Vasavada < > >> maulin.vasav...@gmail.com> wrote: > >> > >>> bump! Clement/Rajini? Any responses based on the latest posts? > >>> > >>> On Wed, Oct 16, 2019 at 10:58 PM Maulin Vasavada < > >>> maulin.vasav...@gmail.com> wrote: > >>> > >>>> bump! > >>>> > >>>> On Sun, Oct 13, 2019 at 11:16 PM Maulin Vasavada < > >>>> maulin.vasav...@gmail.com> wrote: > >>>> > >>>>> Hi Clement > >>>>> > >>>>> 1) existing validation code will remain in SslFactory > >>>>> 2) the createEngine() method in SslEngineBuilder will move to > >>>>> SslFactory and the client/server mode setting will go there (I > documented > >>>>> this in the latest KIP update) > >>>>> > >>>>> In the current KIP I am proposing (as per the latest updates) to make > >>>>> SSLContext loading/configuration/creation pluggable. I am not > suggesting we > >>>>> do/repeat anything that is already addressed by the existing > Providers for > >>>>> SSLContext implementation. The createEngine() method (which will > move to > >>>>> SslFactory) will call SslContextFactory.create() to get references > to the > >>>>> SSLContext and then call SSLContext#createEngine(peer, host) and set > >>>>> client/server mode as it does today. I'll try to put that in a > sequence > >>>>> diagram and update the KIP to make it clearer. > >>>>> > >>>>> So to your question about SslFactory returning SSLContext - I am > >>>>> saying register SslContextFactory interface to provide the SSLContext > >>>>> object instead and keep SslFactory more-or-less as it is today with > some > >>>>> additional responsibility of createEngine() method. > >>>>> > >>>>> Thanks > >>>>> Maulin > >>>>> > >>>>> Thanks > >>>>> Maulin > >>>>> > >>>>> > >>>>> > >>>>> > >>>>> On Fri, Oct 11, 2019 at 6:17 AM Pellerin, Clement < > >>>>> clement_pelle...@ibi.com> wrote: > >>>>> > >>>>>> Can you clarify a few points for me? > >>>>>> > >>>>>> The two stumbling blocks we have are: >
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi all I will start the voting thread on this now. Thanks Maulin On Thu, Jan 23, 2020 at 12:51 AM Maulin Vasavada wrote: > Hi all, > > I have updated the KIP document with the current state of conclusions. > Please review it and see if we are ready to move to Voting! > > Thanks > Maulin > > On Wed, Jan 22, 2020 at 12:42 AM Maulin Vasavada < > maulin.vasav...@gmail.com> wrote: > >> Hi all, >> >> Finally I squeezed time and I've a suggested code changes shown at >> https://github.com/maulin-vasavada/kafka/pull/4/files for discussing >> this further. I'll update the KIP document soon. Meanwhile, can you please >> take a look and continue the discussion? >> >> One challenge is at: >> https://github.com/maulin-vasavada/kafka/pull/4/files#diff-1e3432211fdbb7b2e2b44b5d8838a40bR89 >> >> Thanks >> Maulin >> >> >> On Tue, Oct 22, 2019 at 11:13 PM Maulin Vasavada < >> maulin.vasav...@gmail.com> wrote: >> >>> bump! Clement/Rajini? Any responses based on the latest posts? >>> >>> On Wed, Oct 16, 2019 at 10:58 PM Maulin Vasavada < >>> maulin.vasav...@gmail.com> wrote: >>> >>>> bump! >>>> >>>> On Sun, Oct 13, 2019 at 11:16 PM Maulin Vasavada < >>>> maulin.vasav...@gmail.com> wrote: >>>> >>>>> Hi Clement >>>>> >>>>> 1) existing validation code will remain in SslFactory >>>>> 2) the createEngine() method in SslEngineBuilder will move to >>>>> SslFactory and the client/server mode setting will go there (I documented >>>>> this in the latest KIP update) >>>>> >>>>> In the current KIP I am proposing (as per the latest updates) to make >>>>> SSLContext loading/configuration/creation pluggable. I am not suggesting >>>>> we >>>>> do/repeat anything that is already addressed by the existing Providers for >>>>> SSLContext implementation. The createEngine() method (which will move to >>>>> SslFactory) will call SslContextFactory.create() to get references to the >>>>> SSLContext and then call SSLContext#createEngine(peer, host) and set >>>>> client/server mode as it does today. I'll try to put that in a sequence >>>>> diagram and update the KIP to make it clearer. >>>>> >>>>> So to your question about SslFactory returning SSLContext - I am >>>>> saying register SslContextFactory interface to provide the SSLContext >>>>> object instead and keep SslFactory more-or-less as it is today with some >>>>> additional responsibility of createEngine() method. >>>>> >>>>> Thanks >>>>> Maulin >>>>> >>>>> Thanks >>>>> Maulin >>>>> >>>>> >>>>> >>>>> >>>>> On Fri, Oct 11, 2019 at 6:17 AM Pellerin, Clement < >>>>> clement_pelle...@ibi.com> wrote: >>>>> >>>>>> Can you clarify a few points for me? >>>>>> >>>>>> The two stumbling blocks we have are: >>>>>> 1) reuse of the validation code in the existing SslFactory >>>>>> 2) the client/server mode on the SSLEngine >>>>>> >>>>>> How do you deal with those issues in your new proposal? >>>>>> >>>>>> My use case is to register a custom SslFactory that returns an >>>>>> SSLContext previously created elsewhere in the application. Can your new >>>>>> proposal handle this use case? >>>>>> >>>>>> -Original Message- >>>>>> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >>>>>> Sent: Friday, October 11, 2019 2:13 AM >>>>>> To: dev@kafka.apache.org >>>>>> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >>>>>> extensible >>>>>> >>>>>> Check this out- >>>>>> >>>>>> https://github.com/apache/httpcomponents-core/blob/master/httpcore5/src/main/java/org/apache/hc/core5/ssl/SSLContextBuilder.java#L349 >>>>>> >>>>>> This is exactly what I mean by using existing provider's SSLContext >>>>>> implementation and customizing it with our data points. The similar >>>>>> thing >>>>>> Kafka's SslEngineBuilder is doing right now. >&
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi all, I have updated the KIP document with the current state of conclusions. Please review it and see if we are ready to move to Voting! Thanks Maulin On Wed, Jan 22, 2020 at 12:42 AM Maulin Vasavada wrote: > Hi all, > > Finally I squeezed time and I've a suggested code changes shown at > https://github.com/maulin-vasavada/kafka/pull/4/files for discussing this > further. I'll update the KIP document soon. Meanwhile, can you please take > a look and continue the discussion? > > One challenge is at: > https://github.com/maulin-vasavada/kafka/pull/4/files#diff-1e3432211fdbb7b2e2b44b5d8838a40bR89 > > Thanks > Maulin > > > On Tue, Oct 22, 2019 at 11:13 PM Maulin Vasavada < > maulin.vasav...@gmail.com> wrote: > >> bump! Clement/Rajini? Any responses based on the latest posts? >> >> On Wed, Oct 16, 2019 at 10:58 PM Maulin Vasavada < >> maulin.vasav...@gmail.com> wrote: >> >>> bump! >>> >>> On Sun, Oct 13, 2019 at 11:16 PM Maulin Vasavada < >>> maulin.vasav...@gmail.com> wrote: >>> >>>> Hi Clement >>>> >>>> 1) existing validation code will remain in SslFactory >>>> 2) the createEngine() method in SslEngineBuilder will move to >>>> SslFactory and the client/server mode setting will go there (I documented >>>> this in the latest KIP update) >>>> >>>> In the current KIP I am proposing (as per the latest updates) to make >>>> SSLContext loading/configuration/creation pluggable. I am not suggesting we >>>> do/repeat anything that is already addressed by the existing Providers for >>>> SSLContext implementation. The createEngine() method (which will move to >>>> SslFactory) will call SslContextFactory.create() to get references to the >>>> SSLContext and then call SSLContext#createEngine(peer, host) and set >>>> client/server mode as it does today. I'll try to put that in a sequence >>>> diagram and update the KIP to make it clearer. >>>> >>>> So to your question about SslFactory returning SSLContext - I am saying >>>> register SslContextFactory interface to provide the SSLContext object >>>> instead and keep SslFactory more-or-less as it is today with some >>>> additional responsibility of createEngine() method. >>>> >>>> Thanks >>>> Maulin >>>> >>>> Thanks >>>> Maulin >>>> >>>> >>>> >>>> >>>> On Fri, Oct 11, 2019 at 6:17 AM Pellerin, Clement < >>>> clement_pelle...@ibi.com> wrote: >>>> >>>>> Can you clarify a few points for me? >>>>> >>>>> The two stumbling blocks we have are: >>>>> 1) reuse of the validation code in the existing SslFactory >>>>> 2) the client/server mode on the SSLEngine >>>>> >>>>> How do you deal with those issues in your new proposal? >>>>> >>>>> My use case is to register a custom SslFactory that returns an >>>>> SSLContext previously created elsewhere in the application. Can your new >>>>> proposal handle this use case? >>>>> >>>>> -Original Message- >>>>> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >>>>> Sent: Friday, October 11, 2019 2:13 AM >>>>> To: dev@kafka.apache.org >>>>> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >>>>> extensible >>>>> >>>>> Check this out- >>>>> >>>>> https://github.com/apache/httpcomponents-core/blob/master/httpcore5/src/main/java/org/apache/hc/core5/ssl/SSLContextBuilder.java#L349 >>>>> >>>>> This is exactly what I mean by using existing provider's SSLContext >>>>> implementation and customizing it with our data points. The similar >>>>> thing >>>>> Kafka's SslEngineBuilder is doing right now. >>>>> >>>>> On Thu, Oct 10, 2019 at 11:06 PM Maulin Vasavada < >>>>> maulin.vasav...@gmail.com> >>>>> wrote: >>>>> >>>>> > You meant JSSE not JCE right? We are not talking about cryptographic >>>>> > providers we are talking about ssl providers hence JSSE. >>>>> > >>>>> > I do understand how JSSE Providers work and also the impact of >>>>> multiple >>>>> > JSSE providers with same algorithms in same JVM a
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi all, Finally I squeezed time and I've a suggested code changes shown at https://github.com/maulin-vasavada/kafka/pull/4/files for discussing this further. I'll update the KIP document soon. Meanwhile, can you please take a look and continue the discussion? One challenge is at: https://github.com/maulin-vasavada/kafka/pull/4/files#diff-1e3432211fdbb7b2e2b44b5d8838a40bR89 Thanks Maulin On Tue, Oct 22, 2019 at 11:13 PM Maulin Vasavada wrote: > bump! Clement/Rajini? Any responses based on the latest posts? > > On Wed, Oct 16, 2019 at 10:58 PM Maulin Vasavada < > maulin.vasav...@gmail.com> wrote: > >> bump! >> >> On Sun, Oct 13, 2019 at 11:16 PM Maulin Vasavada < >> maulin.vasav...@gmail.com> wrote: >> >>> Hi Clement >>> >>> 1) existing validation code will remain in SslFactory >>> 2) the createEngine() method in SslEngineBuilder will move to SslFactory >>> and the client/server mode setting will go there (I documented this in the >>> latest KIP update) >>> >>> In the current KIP I am proposing (as per the latest updates) to make >>> SSLContext loading/configuration/creation pluggable. I am not suggesting we >>> do/repeat anything that is already addressed by the existing Providers for >>> SSLContext implementation. The createEngine() method (which will move to >>> SslFactory) will call SslContextFactory.create() to get references to the >>> SSLContext and then call SSLContext#createEngine(peer, host) and set >>> client/server mode as it does today. I'll try to put that in a sequence >>> diagram and update the KIP to make it clearer. >>> >>> So to your question about SslFactory returning SSLContext - I am saying >>> register SslContextFactory interface to provide the SSLContext object >>> instead and keep SslFactory more-or-less as it is today with some >>> additional responsibility of createEngine() method. >>> >>> Thanks >>> Maulin >>> >>> Thanks >>> Maulin >>> >>> >>> >>> >>> On Fri, Oct 11, 2019 at 6:17 AM Pellerin, Clement < >>> clement_pelle...@ibi.com> wrote: >>> >>>> Can you clarify a few points for me? >>>> >>>> The two stumbling blocks we have are: >>>> 1) reuse of the validation code in the existing SslFactory >>>> 2) the client/server mode on the SSLEngine >>>> >>>> How do you deal with those issues in your new proposal? >>>> >>>> My use case is to register a custom SslFactory that returns an >>>> SSLContext previously created elsewhere in the application. Can your new >>>> proposal handle this use case? >>>> >>>> -Original Message- >>>> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >>>> Sent: Friday, October 11, 2019 2:13 AM >>>> To: dev@kafka.apache.org >>>> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >>>> extensible >>>> >>>> Check this out- >>>> >>>> https://github.com/apache/httpcomponents-core/blob/master/httpcore5/src/main/java/org/apache/hc/core5/ssl/SSLContextBuilder.java#L349 >>>> >>>> This is exactly what I mean by using existing provider's SSLContext >>>> implementation and customizing it with our data points. The similar >>>> thing >>>> Kafka's SslEngineBuilder is doing right now. >>>> >>>> On Thu, Oct 10, 2019 at 11:06 PM Maulin Vasavada < >>>> maulin.vasav...@gmail.com> >>>> wrote: >>>> >>>> > You meant JSSE not JCE right? We are not talking about cryptographic >>>> > providers we are talking about ssl providers hence JSSE. >>>> > >>>> > I do understand how JSSE Providers work and also the impact of >>>> multiple >>>> > JSSE providers with same algorithms in same JVM along with sequencing >>>> > challenges for the same. >>>> > >>>> > Like you said- we need to allow customizing the configuration for >>>> > SSLContext, so how many ways we have? >>>> > >>>> > Option-1: Write a custom JSSE Provider with our SSLContext >>>> > >>>> > Option-2: Use whichever SSLContext impl that you get from existing >>>> JSSE >>>> > Provider for SSLContext AND customize data for key material, trust >>>> material >>>> > AND secure random. >>>> &
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
bump! Clement/Rajini? Any responses based on the latest posts? On Wed, Oct 16, 2019 at 10:58 PM Maulin Vasavada wrote: > bump! > > On Sun, Oct 13, 2019 at 11:16 PM Maulin Vasavada < > maulin.vasav...@gmail.com> wrote: > >> Hi Clement >> >> 1) existing validation code will remain in SslFactory >> 2) the createEngine() method in SslEngineBuilder will move to SslFactory >> and the client/server mode setting will go there (I documented this in the >> latest KIP update) >> >> In the current KIP I am proposing (as per the latest updates) to make >> SSLContext loading/configuration/creation pluggable. I am not suggesting we >> do/repeat anything that is already addressed by the existing Providers for >> SSLContext implementation. The createEngine() method (which will move to >> SslFactory) will call SslContextFactory.create() to get references to the >> SSLContext and then call SSLContext#createEngine(peer, host) and set >> client/server mode as it does today. I'll try to put that in a sequence >> diagram and update the KIP to make it clearer. >> >> So to your question about SslFactory returning SSLContext - I am saying >> register SslContextFactory interface to provide the SSLContext object >> instead and keep SslFactory more-or-less as it is today with some >> additional responsibility of createEngine() method. >> >> Thanks >> Maulin >> >> Thanks >> Maulin >> >> >> >> >> On Fri, Oct 11, 2019 at 6:17 AM Pellerin, Clement < >> clement_pelle...@ibi.com> wrote: >> >>> Can you clarify a few points for me? >>> >>> The two stumbling blocks we have are: >>> 1) reuse of the validation code in the existing SslFactory >>> 2) the client/server mode on the SSLEngine >>> >>> How do you deal with those issues in your new proposal? >>> >>> My use case is to register a custom SslFactory that returns an >>> SSLContext previously created elsewhere in the application. Can your new >>> proposal handle this use case? >>> >>> -Original Message- >>> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >>> Sent: Friday, October 11, 2019 2:13 AM >>> To: dev@kafka.apache.org >>> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >>> extensible >>> >>> Check this out- >>> >>> https://github.com/apache/httpcomponents-core/blob/master/httpcore5/src/main/java/org/apache/hc/core5/ssl/SSLContextBuilder.java#L349 >>> >>> This is exactly what I mean by using existing provider's SSLContext >>> implementation and customizing it with our data points. The similar thing >>> Kafka's SslEngineBuilder is doing right now. >>> >>> On Thu, Oct 10, 2019 at 11:06 PM Maulin Vasavada < >>> maulin.vasav...@gmail.com> >>> wrote: >>> >>> > You meant JSSE not JCE right? We are not talking about cryptographic >>> > providers we are talking about ssl providers hence JSSE. >>> > >>> > I do understand how JSSE Providers work and also the impact of multiple >>> > JSSE providers with same algorithms in same JVM along with sequencing >>> > challenges for the same. >>> > >>> > Like you said- we need to allow customizing the configuration for >>> > SSLContext, so how many ways we have? >>> > >>> > Option-1: Write a custom JSSE Provider with our SSLContext >>> > >>> > Option-2: Use whichever SSLContext impl that you get from existing JSSE >>> > Provider for SSLContext AND customize data for key material, trust >>> material >>> > AND secure random. >>> > >>> > Which one you prefer for this context? >>> > >>> > I feel we are making it complicated for no reason. It is very simple - >>> > When we need to have SSL we need data points like - 1) Keys, 2) Trust >>> certs >>> > and 3) Secure Random which is feed to SSLContext and we are done. So >>> we can >>> > keep existing Kafka implementation as is by just making those data >>> points >>> > pluggable. Now SecureRandom is already pluggable via >>> > 'ssl.secure.random.implementation' so that leaves us with keys and >>> trusted >>> > certs. For that purpose I raised KIP-486 BUT everybody feels we still >>> need >>> > higher level of pluggability hence this KIP. >>> > >>> > I"ve been taking advice from the dom
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
bump! On Sun, Oct 13, 2019 at 11:16 PM Maulin Vasavada wrote: > Hi Clement > > 1) existing validation code will remain in SslFactory > 2) the createEngine() method in SslEngineBuilder will move to SslFactory > and the client/server mode setting will go there (I documented this in the > latest KIP update) > > In the current KIP I am proposing (as per the latest updates) to make > SSLContext loading/configuration/creation pluggable. I am not suggesting we > do/repeat anything that is already addressed by the existing Providers for > SSLContext implementation. The createEngine() method (which will move to > SslFactory) will call SslContextFactory.create() to get references to the > SSLContext and then call SSLContext#createEngine(peer, host) and set > client/server mode as it does today. I'll try to put that in a sequence > diagram and update the KIP to make it clearer. > > So to your question about SslFactory returning SSLContext - I am saying > register SslContextFactory interface to provide the SSLContext object > instead and keep SslFactory more-or-less as it is today with some > additional responsibility of createEngine() method. > > Thanks > Maulin > > Thanks > Maulin > > > > > On Fri, Oct 11, 2019 at 6:17 AM Pellerin, Clement < > clement_pelle...@ibi.com> wrote: > >> Can you clarify a few points for me? >> >> The two stumbling blocks we have are: >> 1) reuse of the validation code in the existing SslFactory >> 2) the client/server mode on the SSLEngine >> >> How do you deal with those issues in your new proposal? >> >> My use case is to register a custom SslFactory that returns an SSLContext >> previously created elsewhere in the application. Can your new proposal >> handle this use case? >> >> -----Original Message- >> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >> Sent: Friday, October 11, 2019 2:13 AM >> To: dev@kafka.apache.org >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >> extensible >> >> Check this out- >> >> https://github.com/apache/httpcomponents-core/blob/master/httpcore5/src/main/java/org/apache/hc/core5/ssl/SSLContextBuilder.java#L349 >> >> This is exactly what I mean by using existing provider's SSLContext >> implementation and customizing it with our data points. The similar thing >> Kafka's SslEngineBuilder is doing right now. >> >> On Thu, Oct 10, 2019 at 11:06 PM Maulin Vasavada < >> maulin.vasav...@gmail.com> >> wrote: >> >> > You meant JSSE not JCE right? We are not talking about cryptographic >> > providers we are talking about ssl providers hence JSSE. >> > >> > I do understand how JSSE Providers work and also the impact of multiple >> > JSSE providers with same algorithms in same JVM along with sequencing >> > challenges for the same. >> > >> > Like you said- we need to allow customizing the configuration for >> > SSLContext, so how many ways we have? >> > >> > Option-1: Write a custom JSSE Provider with our SSLContext >> > >> > Option-2: Use whichever SSLContext impl that you get from existing JSSE >> > Provider for SSLContext AND customize data for key material, trust >> material >> > AND secure random. >> > >> > Which one you prefer for this context? >> > >> > I feel we are making it complicated for no reason. It is very simple - >> > When we need to have SSL we need data points like - 1) Keys, 2) Trust >> certs >> > and 3) Secure Random which is feed to SSLContext and we are done. So we >> can >> > keep existing Kafka implementation as is by just making those data >> points >> > pluggable. Now SecureRandom is already pluggable via >> > 'ssl.secure.random.implementation' so that leaves us with keys and >> trusted >> > certs. For that purpose I raised KIP-486 BUT everybody feels we still >> need >> > higher level of pluggability hence this KIP. >> > >> > I"ve been taking advice from the domain experts and Application security >> > teams and to them it is very straight-forward - Make SSLContext >> > configuration/building pluggable and that's it! >> > >> > Thanks >> > Maulin >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > >> > On Mon, Oct 7, 2019 at 5:47 AM Pellerin, Clement < >> clement_pelle...@ibi.com> >> > wrote: >> > >> >> If I understand correctly, you are pro
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Clement 1) existing validation code will remain in SslFactory 2) the createEngine() method in SslEngineBuilder will move to SslFactory and the client/server mode setting will go there (I documented this in the latest KIP update) In the current KIP I am proposing (as per the latest updates) to make SSLContext loading/configuration/creation pluggable. I am not suggesting we do/repeat anything that is already addressed by the existing Providers for SSLContext implementation. The createEngine() method (which will move to SslFactory) will call SslContextFactory.create() to get references to the SSLContext and then call SSLContext#createEngine(peer, host) and set client/server mode as it does today. I'll try to put that in a sequence diagram and update the KIP to make it clearer. So to your question about SslFactory returning SSLContext - I am saying register SslContextFactory interface to provide the SSLContext object instead and keep SslFactory more-or-less as it is today with some additional responsibility of createEngine() method. Thanks Maulin Thanks Maulin On Fri, Oct 11, 2019 at 6:17 AM Pellerin, Clement wrote: > Can you clarify a few points for me? > > The two stumbling blocks we have are: > 1) reuse of the validation code in the existing SslFactory > 2) the client/server mode on the SSLEngine > > How do you deal with those issues in your new proposal? > > My use case is to register a custom SslFactory that returns an SSLContext > previously created elsewhere in the application. Can your new proposal > handle this use case? > > -Original Message- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Friday, October 11, 2019 2:13 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Check this out- > > https://github.com/apache/httpcomponents-core/blob/master/httpcore5/src/main/java/org/apache/hc/core5/ssl/SSLContextBuilder.java#L349 > > This is exactly what I mean by using existing provider's SSLContext > implementation and customizing it with our data points. The similar thing > Kafka's SslEngineBuilder is doing right now. > > On Thu, Oct 10, 2019 at 11:06 PM Maulin Vasavada < > maulin.vasav...@gmail.com> > wrote: > > > You meant JSSE not JCE right? We are not talking about cryptographic > > providers we are talking about ssl providers hence JSSE. > > > > I do understand how JSSE Providers work and also the impact of multiple > > JSSE providers with same algorithms in same JVM along with sequencing > > challenges for the same. > > > > Like you said- we need to allow customizing the configuration for > > SSLContext, so how many ways we have? > > > > Option-1: Write a custom JSSE Provider with our SSLContext > > > > Option-2: Use whichever SSLContext impl that you get from existing JSSE > > Provider for SSLContext AND customize data for key material, trust > material > > AND secure random. > > > > Which one you prefer for this context? > > > > I feel we are making it complicated for no reason. It is very simple - > > When we need to have SSL we need data points like - 1) Keys, 2) Trust > certs > > and 3) Secure Random which is feed to SSLContext and we are done. So we > can > > keep existing Kafka implementation as is by just making those data points > > pluggable. Now SecureRandom is already pluggable via > > 'ssl.secure.random.implementation' so that leaves us with keys and > trusted > > certs. For that purpose I raised KIP-486 BUT everybody feels we still > need > > higher level of pluggability hence this KIP. > > > > I"ve been taking advice from the domain experts and Application security > > teams and to them it is very straight-forward - Make SSLContext > > configuration/building pluggable and that's it! > > > > Thanks > > Maulin > > > > > > > > > > > > > > > > > > > > > > On Mon, Oct 7, 2019 at 5:47 AM Pellerin, Clement < > clement_pelle...@ibi.com> > > wrote: > > > >> If I understand correctly, you are proposing to abandon the idea of a > >> pluggable extension point for SSL in Kafka because we could rely on the > JCE > >> provider mechanism. > >> > >> I will reiterate that nobody does it that way. That in itself should be > >> enough but let's discuss some of the reasons why. > >> > >> Changing the order of the JCE providers in the java.security file > affects > >> all java applications so you probably don't want to do it there. > Changing > >> the order of the JCE providers in the JVM instance affects
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Can you clarify a few points for me? The two stumbling blocks we have are: 1) reuse of the validation code in the existing SslFactory 2) the client/server mode on the SSLEngine How do you deal with those issues in your new proposal? My use case is to register a custom SslFactory that returns an SSLContext previously created elsewhere in the application. Can your new proposal handle this use case? -Original Message- From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] Sent: Friday, October 11, 2019 2:13 AM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Check this out- https://github.com/apache/httpcomponents-core/blob/master/httpcore5/src/main/java/org/apache/hc/core5/ssl/SSLContextBuilder.java#L349 This is exactly what I mean by using existing provider's SSLContext implementation and customizing it with our data points. The similar thing Kafka's SslEngineBuilder is doing right now. On Thu, Oct 10, 2019 at 11:06 PM Maulin Vasavada wrote: > You meant JSSE not JCE right? We are not talking about cryptographic > providers we are talking about ssl providers hence JSSE. > > I do understand how JSSE Providers work and also the impact of multiple > JSSE providers with same algorithms in same JVM along with sequencing > challenges for the same. > > Like you said- we need to allow customizing the configuration for > SSLContext, so how many ways we have? > > Option-1: Write a custom JSSE Provider with our SSLContext > > Option-2: Use whichever SSLContext impl that you get from existing JSSE > Provider for SSLContext AND customize data for key material, trust material > AND secure random. > > Which one you prefer for this context? > > I feel we are making it complicated for no reason. It is very simple - > When we need to have SSL we need data points like - 1) Keys, 2) Trust certs > and 3) Secure Random which is feed to SSLContext and we are done. So we can > keep existing Kafka implementation as is by just making those data points > pluggable. Now SecureRandom is already pluggable via > 'ssl.secure.random.implementation' so that leaves us with keys and trusted > certs. For that purpose I raised KIP-486 BUT everybody feels we still need > higher level of pluggability hence this KIP. > > I"ve been taking advice from the domain experts and Application security > teams and to them it is very straight-forward - Make SSLContext > configuration/building pluggable and that's it! > > Thanks > Maulin > > > > > > > > > > > On Mon, Oct 7, 2019 at 5:47 AM Pellerin, Clement > wrote: > >> If I understand correctly, you are proposing to abandon the idea of a >> pluggable extension point for SSL in Kafka because we could rely on the JCE >> provider mechanism. >> >> I will reiterate that nobody does it that way. That in itself should be >> enough but let's discuss some of the reasons why. >> >> Changing the order of the JCE providers in the java.security file affects >> all java applications so you probably don't want to do it there. Changing >> the order of the JCE providers in the JVM instance affects all code it >> runs. Your library is not alone in the JVM process and other code will want >> regular SSLContext instances. That leaves you with the only option of >> specifying the provider explicitly when you create the SSLContext instance >> in Kafka. That would work, as long as your users don't mess things up with >> the very common configuration approaches above. >> >> A JCE SSLContext provider is intended to be a mechanism to replace the >> SSLContext implementation. Our purpose is to customize the configuration, >> not to replace it. This becomes hard to do when your only chance is at >> creation time. Kafka then does its thing and you have no way to modify that >> behavior in Kafka. You no longer support many legitimate use cases. >> >> The final blow is the need to sign JCE providers using a certificate >> signed by Oracle's JCE Code Signing Certification Authority. >> >> https://www.oracle.com/technetwork/java/javase/tech/getcodesigningcertificate-361306.html >> JCE will refuse to load your provider if it is not signed. Getting the >> certificate is a pain and it takes time. You also have to worry about the >> certificate expiration date. There are JVMs that don't require signed JCE >> providers, but you cannot limit Kafka to just those JVMs. >> >> -Original Message- >> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >> Sent: Friday, October 4, 2019 5:31 PM >> To: dev@kafka.apache.org >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >> extensi
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Check this out- https://github.com/apache/httpcomponents-core/blob/master/httpcore5/src/main/java/org/apache/hc/core5/ssl/SSLContextBuilder.java#L349 This is exactly what I mean by using existing provider's SSLContext implementation and customizing it with our data points. The similar thing Kafka's SslEngineBuilder is doing right now. On Thu, Oct 10, 2019 at 11:06 PM Maulin Vasavada wrote: > You meant JSSE not JCE right? We are not talking about cryptographic > providers we are talking about ssl providers hence JSSE. > > I do understand how JSSE Providers work and also the impact of multiple > JSSE providers with same algorithms in same JVM along with sequencing > challenges for the same. > > Like you said- we need to allow customizing the configuration for > SSLContext, so how many ways we have? > > Option-1: Write a custom JSSE Provider with our SSLContext > > Option-2: Use whichever SSLContext impl that you get from existing JSSE > Provider for SSLContext AND customize data for key material, trust material > AND secure random. > > Which one you prefer for this context? > > I feel we are making it complicated for no reason. It is very simple - > When we need to have SSL we need data points like - 1) Keys, 2) Trust certs > and 3) Secure Random which is feed to SSLContext and we are done. So we can > keep existing Kafka implementation as is by just making those data points > pluggable. Now SecureRandom is already pluggable via > 'ssl.secure.random.implementation' so that leaves us with keys and trusted > certs. For that purpose I raised KIP-486 BUT everybody feels we still need > higher level of pluggability hence this KIP. > > I"ve been taking advice from the domain experts and Application security > teams and to them it is very straight-forward - Make SSLContext > configuration/building pluggable and that's it! > > Thanks > Maulin > > > > > > > > > > > On Mon, Oct 7, 2019 at 5:47 AM Pellerin, Clement > wrote: > >> If I understand correctly, you are proposing to abandon the idea of a >> pluggable extension point for SSL in Kafka because we could rely on the JCE >> provider mechanism. >> >> I will reiterate that nobody does it that way. That in itself should be >> enough but let's discuss some of the reasons why. >> >> Changing the order of the JCE providers in the java.security file affects >> all java applications so you probably don't want to do it there. Changing >> the order of the JCE providers in the JVM instance affects all code it >> runs. Your library is not alone in the JVM process and other code will want >> regular SSLContext instances. That leaves you with the only option of >> specifying the provider explicitly when you create the SSLContext instance >> in Kafka. That would work, as long as your users don't mess things up with >> the very common configuration approaches above. >> >> A JCE SSLContext provider is intended to be a mechanism to replace the >> SSLContext implementation. Our purpose is to customize the configuration, >> not to replace it. This becomes hard to do when your only chance is at >> creation time. Kafka then does its thing and you have no way to modify that >> behavior in Kafka. You no longer support many legitimate use cases. >> >> The final blow is the need to sign JCE providers using a certificate >> signed by Oracle's JCE Code Signing Certification Authority. >> >> https://www.oracle.com/technetwork/java/javase/tech/getcodesigningcertificate-361306.html >> JCE will refuse to load your provider if it is not signed. Getting the >> certificate is a pain and it takes time. You also have to worry about the >> certificate expiration date. There are JVMs that don't require signed JCE >> providers, but you cannot limit Kafka to just those JVMs. >> >> -Original Message- >> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >> Sent: Friday, October 4, 2019 5:31 PM >> To: dev@kafka.apache.org >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >> extensible >> >> In other words, Kafka doesn't necessarily need to derive another >> interface/mechanism to make SSLEngine pluggable. That interface/mechanism >> exists in Java with Security Provider's SSLContext Algorithms. >> Ref-1: >> >> https://docs.oracle.com/javase/9/docs/specs/security/standard-names.html#sslcontext-algorithms >> Ref-2 >> <https://docs.oracle.com/javase/9/docs/specs/security/standard-names.html#sslcontext-algorithmsRef-2> >> : >> >> https://github.com/bcgit/bc-java/blob/master/tls/src/main/java/org/bouncycastle/jsse/provider/BouncyCast
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
You meant JSSE not JCE right? We are not talking about cryptographic providers we are talking about ssl providers hence JSSE. I do understand how JSSE Providers work and also the impact of multiple JSSE providers with same algorithms in same JVM along with sequencing challenges for the same. Like you said- we need to allow customizing the configuration for SSLContext, so how many ways we have? Option-1: Write a custom JSSE Provider with our SSLContext Option-2: Use whichever SSLContext impl that you get from existing JSSE Provider for SSLContext AND customize data for key material, trust material AND secure random. Which one you prefer for this context? I feel we are making it complicated for no reason. It is very simple - When we need to have SSL we need data points like - 1) Keys, 2) Trust certs and 3) Secure Random which is feed to SSLContext and we are done. So we can keep existing Kafka implementation as is by just making those data points pluggable. Now SecureRandom is already pluggable via 'ssl.secure.random.implementation' so that leaves us with keys and trusted certs. For that purpose I raised KIP-486 BUT everybody feels we still need higher level of pluggability hence this KIP. I"ve been taking advice from the domain experts and Application security teams and to them it is very straight-forward - Make SSLContext configuration/building pluggable and that's it! Thanks Maulin On Mon, Oct 7, 2019 at 5:47 AM Pellerin, Clement wrote: > If I understand correctly, you are proposing to abandon the idea of a > pluggable extension point for SSL in Kafka because we could rely on the JCE > provider mechanism. > > I will reiterate that nobody does it that way. That in itself should be > enough but let's discuss some of the reasons why. > > Changing the order of the JCE providers in the java.security file affects > all java applications so you probably don't want to do it there. Changing > the order of the JCE providers in the JVM instance affects all code it > runs. Your library is not alone in the JVM process and other code will want > regular SSLContext instances. That leaves you with the only option of > specifying the provider explicitly when you create the SSLContext instance > in Kafka. That would work, as long as your users don't mess things up with > the very common configuration approaches above. > > A JCE SSLContext provider is intended to be a mechanism to replace the > SSLContext implementation. Our purpose is to customize the configuration, > not to replace it. This becomes hard to do when your only chance is at > creation time. Kafka then does its thing and you have no way to modify that > behavior in Kafka. You no longer support many legitimate use cases. > > The final blow is the need to sign JCE providers using a certificate > signed by Oracle's JCE Code Signing Certification Authority. > > https://www.oracle.com/technetwork/java/javase/tech/getcodesigningcertificate-361306.html > JCE will refuse to load your provider if it is not signed. Getting the > certificate is a pain and it takes time. You also have to worry about the > certificate expiration date. There are JVMs that don't require signed JCE > providers, but you cannot limit Kafka to just those JVMs. > > -Original Message- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Friday, October 4, 2019 5:31 PM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > In other words, Kafka doesn't necessarily need to derive another > interface/mechanism to make SSLEngine pluggable. That interface/mechanism > exists in Java with Security Provider's SSLContext Algorithms. > Ref-1: > > https://docs.oracle.com/javase/9/docs/specs/security/standard-names.html#sslcontext-algorithms > Ref-2 > <https://docs.oracle.com/javase/9/docs/specs/security/standard-names.html#sslcontext-algorithmsRef-2> > : > > https://github.com/bcgit/bc-java/blob/master/tls/src/main/java/org/bouncycastle/jsse/provider/BouncyCastleJsseProvider.java#L193 > > About the " whole world chooses to make the javax.net.ssl.SSLSocketFactory > pluggable" I found the official documentation reinforcing my point I made > above, > "The javax.net.ssl.SSLSocket class represents a network socket that > encapsulates SSL/TLS support on top of a normal stream socket ( > java.net.Socket). Some applications might want to use alternate data > transport abstractions (e.g., New-I/O); the javax.net.ssl.SSLEngine class > is available to produce and consume SSL/TLS packets." > Reference: > > https://docs.oracle.com/javase/7/docs/technotes/guides/security/overview/jsoverview.html > > I feel that we have to think about building SSLContext in a pluggable way > since that is the class that takes "key/trust" material and secure-random > config and help creates SSLEngine, SocketFactories via the TLS algorithm's > provider specified by Security Provider configuration. > > Thanks > Maulin > >
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
If I understand correctly, you are proposing to abandon the idea of a pluggable extension point for SSL in Kafka because we could rely on the JCE provider mechanism. I will reiterate that nobody does it that way. That in itself should be enough but let's discuss some of the reasons why. Changing the order of the JCE providers in the java.security file affects all java applications so you probably don't want to do it there. Changing the order of the JCE providers in the JVM instance affects all code it runs. Your library is not alone in the JVM process and other code will want regular SSLContext instances. That leaves you with the only option of specifying the provider explicitly when you create the SSLContext instance in Kafka. That would work, as long as your users don't mess things up with the very common configuration approaches above. A JCE SSLContext provider is intended to be a mechanism to replace the SSLContext implementation. Our purpose is to customize the configuration, not to replace it. This becomes hard to do when your only chance is at creation time. Kafka then does its thing and you have no way to modify that behavior in Kafka. You no longer support many legitimate use cases. The final blow is the need to sign JCE providers using a certificate signed by Oracle's JCE Code Signing Certification Authority. https://www.oracle.com/technetwork/java/javase/tech/getcodesigningcertificate-361306.html JCE will refuse to load your provider if it is not signed. Getting the certificate is a pain and it takes time. You also have to worry about the certificate expiration date. There are JVMs that don't require signed JCE providers, but you cannot limit Kafka to just those JVMs. -Original Message- From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] Sent: Friday, October 4, 2019 5:31 PM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible In other words, Kafka doesn't necessarily need to derive another interface/mechanism to make SSLEngine pluggable. That interface/mechanism exists in Java with Security Provider's SSLContext Algorithms. Ref-1: https://docs.oracle.com/javase/9/docs/specs/security/standard-names.html#sslcontext-algorithms Ref-2: https://github.com/bcgit/bc-java/blob/master/tls/src/main/java/org/bouncycastle/jsse/provider/BouncyCastleJsseProvider.java#L193 About the " whole world chooses to make the javax.net.ssl.SSLSocketFactory pluggable" I found the official documentation reinforcing my point I made above, "The javax.net.ssl.SSLSocket class represents a network socket that encapsulates SSL/TLS support on top of a normal stream socket ( java.net.Socket). Some applications might want to use alternate data transport abstractions (e.g., New-I/O); the javax.net.ssl.SSLEngine class is available to produce and consume SSL/TLS packets." Reference: https://docs.oracle.com/javase/7/docs/technotes/guides/security/overview/jsoverview.html I feel that we have to think about building SSLContext in a pluggable way since that is the class that takes "key/trust" material and secure-random config and help creates SSLEngine, SocketFactories via the TLS algorithm's provider specified by Security Provider configuration. Thanks Maulin
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
In other words, Kafka doesn't necessarily need to derive another interface/mechanism to make SSLEngine pluggable. That interface/mechanism exists in Java with Security Provider's SSLContext Algorithms. Ref-1: https://docs.oracle.com/javase/9/docs/specs/security/standard-names.html#sslcontext-algorithms Ref-2: https://github.com/bcgit/bc-java/blob/master/tls/src/main/java/org/bouncycastle/jsse/provider/BouncyCastleJsseProvider.java#L193 About the " whole world chooses to make the javax.net.ssl.SSLSocketFactory pluggable" I found the official documentation reinforcing my point I made above, "The javax.net.ssl.SSLSocket class represents a network socket that encapsulates SSL/TLS support on top of a normal stream socket ( java.net.Socket). Some applications might want to use alternate data transport abstractions (e.g., New-I/O); the javax.net.ssl.SSLEngine class is available to produce and consume SSL/TLS packets." Reference: https://docs.oracle.com/javase/7/docs/technotes/guides/security/overview/jsoverview.html I feel that we have to think about building SSLContext in a pluggable way since that is the class that takes "key/trust" material and secure-random config and help creates SSLEngine, SocketFactories via the TLS algorithm's provider specified by Security Provider configuration. Thanks Maulin On Fri, Oct 4, 2019 at 10:28 AM Maulin Vasavada wrote: > Hi Clement > > The explanation about using Java Security Providers if you want to use > customize SSLEngine is - similar to BouncyCastleJsseProvider we can have a > provider that registers SSLContext+(TLS/SSL) services and have > implementation for SSLContextSpi which provides various methods to > get/create different kind of objects offered by the specific TLS > implementation - example: SSLEngine, ServerSocketFactory, SocketFactory. > SSLContext works as kind of a "bridge" pattern that once we get SSLContext > for a specific protocol (TLSv1.1,TLSv1.2 etc) then ssl engine, socket > factory everything is for that protocol implementation. > > About the SSLContext vs SSLEngine, I debated the exact point you raise in > my mind and I came to the conclusion that - The implementation of Kafka or > HTTPs protocol (like ApacheHttpClient) ultimately relies on Java IO/NIO > packages and uses some kind of Channel (example: > https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/network/TransportLayer.java). > Now they have a choice to either use SSLEngine or SSLSocket. It is entirely > upto the protocol's implementation to decide that. Hence we can't say > everybody must use SSLSocketFacotry vs SSLEngine. I will dig into Apache > Http Client and find how they are using SocketFactory. > > Thanks > Maulin > > > > > > > > On Fri, Oct 4, 2019 at 8:37 AM Pellerin, Clement > wrote: > >> I am happy to see I am not the only one with reservations on the >> direction we were heading. >> >> >> I feel if you want to plug any JSSE compatible SSLEngine >> >> (example BouncyCastle, wildfly-OpenSSL etc) then we have to follow >> path of >> >> using Java security provider >> >> I like the conclusion but I did not understand your explanation. >> >> >> What should be pluggable - SSLContext or SSLEngine? >> I can only remark that the whole world chooses to make the >> javax.net.ssl.SSLSocketFactory pluggable. You can find projects making the >> SSLContext pluggable but I have never seen a project that makes the >> SSLEngine pluggable. >> >> The stumbling blocks in our case are the client/server mode customization >> and the reusability of the reconfiguration checks. We can always compromise >> on the reusability of the reconfiguration checks. On the other hand, we >> really have to learn more about the client/server mode customization >> because that's unavoidable at the moment. >> >> -Original Message- >> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >> Sent: Friday, October 4, 2019 4:13 AM >> To: dev@kafka.apache.org >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >> extensible >> >> Hi all >> >> I've been having more thoughts on SSLEngine vs SSLContext pluggability >> (reasons for hiatus from my side until now). Based on my further research >> and understanding, various TLS implementations >> https://en.wikipedia.org/wiki/Comparison_of_TLS_implementations , makes >> it >> clear that there are implementations that may not be originally complying >> to JSSE BUT they eventually might provide JSSE Provider, example - >> BouncyCastleJsseProvider - >> >> https://github.com/bcgit/bc-java/blob/mast
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Clement The explanation about using Java Security Providers if you want to use customize SSLEngine is - similar to BouncyCastleJsseProvider we can have a provider that registers SSLContext+(TLS/SSL) services and have implementation for SSLContextSpi which provides various methods to get/create different kind of objects offered by the specific TLS implementation - example: SSLEngine, ServerSocketFactory, SocketFactory. SSLContext works as kind of a "bridge" pattern that once we get SSLContext for a specific protocol (TLSv1.1,TLSv1.2 etc) then ssl engine, socket factory everything is for that protocol implementation. About the SSLContext vs SSLEngine, I debated the exact point you raise in my mind and I came to the conclusion that - The implementation of Kafka or HTTPs protocol (like ApacheHttpClient) ultimately relies on Java IO/NIO packages and uses some kind of Channel (example: https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/network/TransportLayer.java). Now they have a choice to either use SSLEngine or SSLSocket. It is entirely upto the protocol's implementation to decide that. Hence we can't say everybody must use SSLSocketFacotry vs SSLEngine. I will dig into Apache Http Client and find how they are using SocketFactory. Thanks Maulin On Fri, Oct 4, 2019 at 8:37 AM Pellerin, Clement wrote: > I am happy to see I am not the only one with reservations on the direction > we were heading. > > >> I feel if you want to plug any JSSE compatible SSLEngine > >> (example BouncyCastle, wildfly-OpenSSL etc) then we have to follow path > of > >> using Java security provider > > I like the conclusion but I did not understand your explanation. > > >> What should be pluggable - SSLContext or SSLEngine? > I can only remark that the whole world chooses to make the > javax.net.ssl.SSLSocketFactory pluggable. You can find projects making the > SSLContext pluggable but I have never seen a project that makes the > SSLEngine pluggable. > > The stumbling blocks in our case are the client/server mode customization > and the reusability of the reconfiguration checks. We can always compromise > on the reusability of the reconfiguration checks. On the other hand, we > really have to learn more about the client/server mode customization > because that's unavoidable at the moment. > > -Original Message- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Friday, October 4, 2019 4:13 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Hi all > > I've been having more thoughts on SSLEngine vs SSLContext pluggability > (reasons for hiatus from my side until now). Based on my further research > and understanding, various TLS implementations > https://en.wikipedia.org/wiki/Comparison_of_TLS_implementations , makes it > clear that there are implementations that may not be originally complying > to JSSE BUT they eventually might provide JSSE Provider, example - > BouncyCastleJsseProvider - > > https://github.com/bcgit/bc-java/blob/master/tls/src/main/java/org/bouncycastle/jsse/provider/BouncyCastleJsseProvider.java > . > In those cases, I feel if you want to plug any JSSE compatible SSLEngine > (example BouncyCastle, wildfly-OpenSSL etc) then we have to follow path of > using Java security provider (like using BouncyCastleJsseProvider class). > We can't easily just use the proposed SslEngineFactory interface. > > Also, the existing logic of createEngine() method actually just does > sslContext.createEngine() and then customize the object further based on > the Client/Server mode. It doesn't really do any customization of SSLEngine > wrap/unwrap methods which are the heart of it. That urges me to think more > that actually we need SSLContext to be pluggable. > > Either way, point of discussions about reconfigurability and questions > Clement asked remains similar BUT I think we might have to first really > resolve "What should be pluggable - SSLContext or SSLEngine?" question. > > Let me know your thoughts! > > Thanks > Maulin > > > > > > > > > On Wed, Sep 25, 2019 at 10:56 PM Maulin Vasavada < > maulin.vasav...@gmail.com> > wrote: > > > Ack. I should be able to get back to this on Friday. > > > > On Mon, Sep 23, 2019 at 10:35 AM Pellerin, Clement < > > clement_pelle...@ibi.com> wrote: > > > >> When I worked on KIP-383 I was told the way to pass extra arguments to > an > >> instance is to add extra arguments to configure. I would now suggest we > do > >> like the KeySerializer. If you look in KafkaProducer, it creates a > >> KeySerializer usi
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
I am happy to see I am not the only one with reservations on the direction we were heading. >> I feel if you want to plug any JSSE compatible SSLEngine >> (example BouncyCastle, wildfly-OpenSSL etc) then we have to follow path of >> using Java security provider I like the conclusion but I did not understand your explanation. >> What should be pluggable - SSLContext or SSLEngine? I can only remark that the whole world chooses to make the javax.net.ssl.SSLSocketFactory pluggable. You can find projects making the SSLContext pluggable but I have never seen a project that makes the SSLEngine pluggable. The stumbling blocks in our case are the client/server mode customization and the reusability of the reconfiguration checks. We can always compromise on the reusability of the reconfiguration checks. On the other hand, we really have to learn more about the client/server mode customization because that's unavoidable at the moment. -Original Message- From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] Sent: Friday, October 4, 2019 4:13 AM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Hi all I've been having more thoughts on SSLEngine vs SSLContext pluggability (reasons for hiatus from my side until now). Based on my further research and understanding, various TLS implementations https://en.wikipedia.org/wiki/Comparison_of_TLS_implementations , makes it clear that there are implementations that may not be originally complying to JSSE BUT they eventually might provide JSSE Provider, example - BouncyCastleJsseProvider - https://github.com/bcgit/bc-java/blob/master/tls/src/main/java/org/bouncycastle/jsse/provider/BouncyCastleJsseProvider.java. In those cases, I feel if you want to plug any JSSE compatible SSLEngine (example BouncyCastle, wildfly-OpenSSL etc) then we have to follow path of using Java security provider (like using BouncyCastleJsseProvider class). We can't easily just use the proposed SslEngineFactory interface. Also, the existing logic of createEngine() method actually just does sslContext.createEngine() and then customize the object further based on the Client/Server mode. It doesn't really do any customization of SSLEngine wrap/unwrap methods which are the heart of it. That urges me to think more that actually we need SSLContext to be pluggable. Either way, point of discussions about reconfigurability and questions Clement asked remains similar BUT I think we might have to first really resolve "What should be pluggable - SSLContext or SSLEngine?" question. Let me know your thoughts! Thanks Maulin On Wed, Sep 25, 2019 at 10:56 PM Maulin Vasavada wrote: > Ack. I should be able to get back to this on Friday. > > On Mon, Sep 23, 2019 at 10:35 AM Pellerin, Clement < > clement_pelle...@ibi.com> wrote: > >> When I worked on KIP-383 I was told the way to pass extra arguments to an >> instance is to add extra arguments to configure. I would now suggest we do >> like the KeySerializer. If you look in KafkaProducer, it creates a >> KeySerializer using AbstractConfig. getConfiguredInstance(). Since >> KeySerializer does not extend KeySerializer, AbstractConfig does not call >> configure(Map). KafkaProducer then calls configure() with two arguments. >> This removes the need for the init() method which would be called too late >> after configure(). By the way, the KeySerializer is not the only interface >> that does this. >> >> In summary, SslEngineFactory does not extend Configurable and it has a >> configure() method with more than 1 argument. >> >> The next item is to spec how config.originals() is passed to >> SslChannelBuilder: the KIP needs to explain we will push the choice of >> configs within the switch in ChannelBuilders.create() >> >> Finally, we need to spec which configs are passed to shouldRebuiltFor(). >> I assume these are now originals() instead of values(). We also need to >> specify if all configs are passed or just the reconfigurable ones. >> >>
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi all I've been having more thoughts on SSLEngine vs SSLContext pluggability (reasons for hiatus from my side until now). Based on my further research and understanding, various TLS implementations https://en.wikipedia.org/wiki/Comparison_of_TLS_implementations , makes it clear that there are implementations that may not be originally complying to JSSE BUT they eventually might provide JSSE Provider, example - BouncyCastleJsseProvider - https://github.com/bcgit/bc-java/blob/master/tls/src/main/java/org/bouncycastle/jsse/provider/BouncyCastleJsseProvider.java. In those cases, I feel if you want to plug any JSSE compatible SSLEngine (example BouncyCastle, wildfly-OpenSSL etc) then we have to follow path of using Java security provider (like using BouncyCastleJsseProvider class). We can't easily just use the proposed SslEngineFactory interface. Also, the existing logic of createEngine() method actually just does sslContext.createEngine() and then customize the object further based on the Client/Server mode. It doesn't really do any customization of SSLEngine wrap/unwrap methods which are the heart of it. That urges me to think more that actually we need SSLContext to be pluggable. Either way, point of discussions about reconfigurability and questions Clement asked remains similar BUT I think we might have to first really resolve "What should be pluggable - SSLContext or SSLEngine?" question. Let me know your thoughts! Thanks Maulin On Wed, Sep 25, 2019 at 10:56 PM Maulin Vasavada wrote: > Ack. I should be able to get back to this on Friday. > > On Mon, Sep 23, 2019 at 10:35 AM Pellerin, Clement < > clement_pelle...@ibi.com> wrote: > >> When I worked on KIP-383 I was told the way to pass extra arguments to an >> instance is to add extra arguments to configure. I would now suggest we do >> like the KeySerializer. If you look in KafkaProducer, it creates a >> KeySerializer using AbstractConfig. getConfiguredInstance(). Since >> KeySerializer does not extend KeySerializer, AbstractConfig does not call >> configure(Map). KafkaProducer then calls configure() with two arguments. >> This removes the need for the init() method which would be called too late >> after configure(). By the way, the KeySerializer is not the only interface >> that does this. >> >> In summary, SslEngineFactory does not extend Configurable and it has a >> configure() method with more than 1 argument. >> >> The next item is to spec how config.originals() is passed to >> SslChannelBuilder: the KIP needs to explain we will push the choice of >> configs within the switch in ChannelBuilders.create() >> >> Finally, we need to spec which configs are passed to shouldRebuiltFor(). >> I assume these are now originals() instead of values(). We also need to >> specify if all configs are passed or just the reconfigurable ones. >> >>
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Ack. I should be able to get back to this on Friday. On Mon, Sep 23, 2019 at 10:35 AM Pellerin, Clement wrote: > When I worked on KIP-383 I was told the way to pass extra arguments to an > instance is to add extra arguments to configure. I would now suggest we do > like the KeySerializer. If you look in KafkaProducer, it creates a > KeySerializer using AbstractConfig. getConfiguredInstance(). Since > KeySerializer does not extend KeySerializer, AbstractConfig does not call > configure(Map). KafkaProducer then calls configure() with two arguments. > This removes the need for the init() method which would be called too late > after configure(). By the way, the KeySerializer is not the only interface > that does this. > > In summary, SslEngineFactory does not extend Configurable and it has a > configure() method with more than 1 argument. > > The next item is to spec how config.originals() is passed to > SslChannelBuilder: the KIP needs to explain we will push the choice of > configs within the switch in ChannelBuilders.create() > > Finally, we need to spec which configs are passed to shouldRebuiltFor(). I > assume these are now originals() instead of values(). We also need to > specify if all configs are passed or just the reconfigurable ones. > >
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
When I worked on KIP-383 I was told the way to pass extra arguments to an instance is to add extra arguments to configure. I would now suggest we do like the KeySerializer. If you look in KafkaProducer, it creates a KeySerializer using AbstractConfig. getConfiguredInstance(). Since KeySerializer does not extend KeySerializer, AbstractConfig does not call configure(Map). KafkaProducer then calls configure() with two arguments. This removes the need for the init() method which would be called too late after configure(). By the way, the KeySerializer is not the only interface that does this. In summary, SslEngineFactory does not extend Configurable and it has a configure() method with more than 1 argument. The next item is to spec how config.originals() is passed to SslChannelBuilder: the KIP needs to explain we will push the choice of configs within the switch in ChannelBuilders.create() Finally, we need to spec which configs are passed to shouldRebuiltFor(). I assume these are now originals() instead of values(). We also need to specify if all configs are passed or just the reconfigurable ones.
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Yes the checks should be mandatory and as I mentioned before we should keep them outside of the pluggable implementation - the way currently it is in SslFactory. I think we can wait for comments from @Colin McCabe and @Rajini Sivaram as soon as they can get some time out of 2.4 release. May be later this week or early next week? Thanks Maulin On Sat, Sep 21, 2019 at 5:24 AM Pellerin, Clement wrote: > I may have found the technical reason. > > It would be logical if the customizable reconfigurable extension point > extends the Reconfigurable interface. That's what the first iteration of > KIP-383 proposed. It lost its vote because the voters wanted to preserve > the reconfiguration checks in SslFactory. The next thing to try is to push > those checks in the creator which is the channel builder. A Reconfigurable > instance is mutable by definition. It can either be in its current > configuration, or the reconfigured configuration but not in between. The > checks require to partially build the new configuration before accepting it > and therefore this approach does not work. > > We could move the checks in a helper validator class, then we would rely > on the Reconfigurable instance to call that code. This would reuse the code > but the checks would not be mandatory. > > The conclusion is the checks are mandatory or the interface extends the > Reconfigurable interface but not both. > > Is that reasoning what you were saying? > Do we make the checks mandatory or not? > Do we support all the use cases we want? > > -Original Message- > From: Pellerin, Clement > Sent: Friday, September 20, 2019 5:24 PM > To: dev@kafka.apache.org > Subject: RE: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > The KIP now says: We believe that making SSLEngine creation pluggable is > worth to allow SSL experts to write their own implementation having the SSL > domain knowledge and keep them free of knowing much about Kafka's > reconfigurability. > > The other custom reconfigurable extension points don't seem to have a > problem with that. You may have a point though, so I need to look at the > reconfiguration code you mention. > > Is your latest prototype available so I can study it? > > -Original Message- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Friday, September 20, 2019 4:40 PM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Thanks Clement for your thoughts. According to my current experience > rewriting the code twice I would say I did what you suggest in the last > point - " We must make an attempt, if only to explain why it fails in the > Rejected Alternatives section of the KIP." In the rejected alternatives I > already mention why not merge SslFactory and SslEngineFactory and make > SslFactory reconfigurable. > > @Colin can you please express your thoughts on this discussion so far? > Since you refactored the SslFactory's code to have SslEngineBuilder I feel > you would have more insights into future changes. > > Thanks > Maulin > > > On Fri, Sep 20, 2019 at 7:29 AM Pellerin, Clement < > clement_pelle...@ibi.com> > wrote: > > > There will be chaperon code in the base class of the channel builders. > > The arguments you gave me are emotional not technical. > > The SSL extension point is reconfigurable hence it should extend > > Reconfigurable. > > We will encounter issues when we try to prototype it that way. > > We will solve the issues or backtrack to another solution. > > We must make an attempt, if only to explain why it fails in the Rejected > > Alternatives section of the KIP. > > > > -Original Message- > > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > > Sent: Friday, September 20, 2019 2:40 AM > > To: dev@kafka.apache.org > > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > > extensible > > > > Overall my thinking is - When somebody wants to customize creation of > > SSLEngine, most likely they are more expert in dealing with SSL domain > > related stuff than "Kafka's reconfigurability" aspect. As a custom > > implementation it makes more sense to me to say - Hey I'll control how I > > initialize my SSL context/engine and btw if Kafka can give me a way to > just > > get re-created whenever some set of config keys changes, it is great! > This > > is similar to my thinking on KIP-486 which is- as a Kafka operator I am > > just trying to be compliant to my company's security policies to load > > keys/certs in certain way. For that, I should not be
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
I may have found the technical reason. It would be logical if the customizable reconfigurable extension point extends the Reconfigurable interface. That's what the first iteration of KIP-383 proposed. It lost its vote because the voters wanted to preserve the reconfiguration checks in SslFactory. The next thing to try is to push those checks in the creator which is the channel builder. A Reconfigurable instance is mutable by definition. It can either be in its current configuration, or the reconfigured configuration but not in between. The checks require to partially build the new configuration before accepting it and therefore this approach does not work. We could move the checks in a helper validator class, then we would rely on the Reconfigurable instance to call that code. This would reuse the code but the checks would not be mandatory. The conclusion is the checks are mandatory or the interface extends the Reconfigurable interface but not both. Is that reasoning what you were saying? Do we make the checks mandatory or not? Do we support all the use cases we want? -Original Message- From: Pellerin, Clement Sent: Friday, September 20, 2019 5:24 PM To: dev@kafka.apache.org Subject: RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible The KIP now says: We believe that making SSLEngine creation pluggable is worth to allow SSL experts to write their own implementation having the SSL domain knowledge and keep them free of knowing much about Kafka's reconfigurability. The other custom reconfigurable extension points don't seem to have a problem with that. You may have a point though, so I need to look at the reconfiguration code you mention. Is your latest prototype available so I can study it? -Original Message- From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] Sent: Friday, September 20, 2019 4:40 PM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Thanks Clement for your thoughts. According to my current experience rewriting the code twice I would say I did what you suggest in the last point - " We must make an attempt, if only to explain why it fails in the Rejected Alternatives section of the KIP." In the rejected alternatives I already mention why not merge SslFactory and SslEngineFactory and make SslFactory reconfigurable. @Colin can you please express your thoughts on this discussion so far? Since you refactored the SslFactory's code to have SslEngineBuilder I feel you would have more insights into future changes. Thanks Maulin On Fri, Sep 20, 2019 at 7:29 AM Pellerin, Clement wrote: > There will be chaperon code in the base class of the channel builders. > The arguments you gave me are emotional not technical. > The SSL extension point is reconfigurable hence it should extend > Reconfigurable. > We will encounter issues when we try to prototype it that way. > We will solve the issues or backtrack to another solution. > We must make an attempt, if only to explain why it fails in the Rejected > Alternatives section of the KIP. > > -Original Message- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Friday, September 20, 2019 2:40 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Overall my thinking is - When somebody wants to customize creation of > SSLEngine, most likely they are more expert in dealing with SSL domain > related stuff than "Kafka's reconfigurability" aspect. As a custom > implementation it makes more sense to me to say - Hey I'll control how I > initialize my SSL context/engine and btw if Kafka can give me a way to just > get re-created whenever some set of config keys changes, it is great! This > is similar to my thinking on KIP-486 which is- as a Kafka operator I am > just trying to be compliant to my company's security policies to load > keys/certs in certain way. For that, I should not be penalized by Kafka to > know all about Java security Providers and how to really create SSLContext > object etc given Java already provides a way to feed in KeyStore object > regardless of how I load it. > > On Thu, Sep 19, 2019 at 10:57 PM Maulin Vasavada < > maulin.vasav...@gmail.com> > wrote: > > > Hi Clement > > > > There will be good amount of state in the SslEngineFactory's default > > implementation. Hence I feel we might anyway have a chaperon class to > > provide reconfigurable functionality and will have one more class to host > > the state/behavior of actual SSLContext/SSLEngine creation. While doing > the > > internal rewrite (so far two times) both of the times I reached to the > same > > conclusion.I feel if we leave the reconfigurations to the implementation > - > > it w
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
The KIP now says: We believe that making SSLEngine creation pluggable is worth to allow SSL experts to write their own implementation having the SSL domain knowledge and keep them free of knowing much about Kafka's reconfigurability. The other custom reconfigurable extension points don't seem to have a problem with that. You may have a point though, so I need to look at the reconfiguration code you mention. Is your latest prototype available so I can study it? -Original Message- From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] Sent: Friday, September 20, 2019 4:40 PM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Thanks Clement for your thoughts. According to my current experience rewriting the code twice I would say I did what you suggest in the last point - " We must make an attempt, if only to explain why it fails in the Rejected Alternatives section of the KIP." In the rejected alternatives I already mention why not merge SslFactory and SslEngineFactory and make SslFactory reconfigurable. @Colin can you please express your thoughts on this discussion so far? Since you refactored the SslFactory's code to have SslEngineBuilder I feel you would have more insights into future changes. Thanks Maulin On Fri, Sep 20, 2019 at 7:29 AM Pellerin, Clement wrote: > There will be chaperon code in the base class of the channel builders. > The arguments you gave me are emotional not technical. > The SSL extension point is reconfigurable hence it should extend > Reconfigurable. > We will encounter issues when we try to prototype it that way. > We will solve the issues or backtrack to another solution. > We must make an attempt, if only to explain why it fails in the Rejected > Alternatives section of the KIP. > > -Original Message- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Friday, September 20, 2019 2:40 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Overall my thinking is - When somebody wants to customize creation of > SSLEngine, most likely they are more expert in dealing with SSL domain > related stuff than "Kafka's reconfigurability" aspect. As a custom > implementation it makes more sense to me to say - Hey I'll control how I > initialize my SSL context/engine and btw if Kafka can give me a way to just > get re-created whenever some set of config keys changes, it is great! This > is similar to my thinking on KIP-486 which is- as a Kafka operator I am > just trying to be compliant to my company's security policies to load > keys/certs in certain way. For that, I should not be penalized by Kafka to > know all about Java security Providers and how to really create SSLContext > object etc given Java already provides a way to feed in KeyStore object > regardless of how I load it. > > On Thu, Sep 19, 2019 at 10:57 PM Maulin Vasavada < > maulin.vasav...@gmail.com> > wrote: > > > Hi Clement > > > > There will be good amount of state in the SslEngineFactory's default > > implementation. Hence I feel we might anyway have a chaperon class to > > provide reconfigurable functionality and will have one more class to host > > the state/behavior of actual SSLContext/SSLEngine creation. While doing > the > > internal rewrite (so far two times) both of the times I reached to the > same > > conclusion.I feel if we leave the reconfigurations to the implementation > - > > it will repeat the same pattern of having two classes to manage it - > since > > most likely they will also have similar state information. Instead keep > > that reconfigurations in SslFactory as is today and just allow "plugin of > > creation of SSLEngine". > > > > One note I would like to make is: You are comparing this to > MetricReporter > > but we have to keep in mind that SSL configuration is inherently more > > complex than a MetricReporters functionality. There are no JSSE > equivalent > > documents needed to be written for MetricReporter for example. So what > > works best for simpler solutions may not work equally well for more > complex > > scenarios. > > > > > > Thanks > > Maulin > > > > > > On Thu, Sep 19, 2019 at 8:36 PM Pellerin, Clement < > > clement_pelle...@ibi.com> wrote: > > > >> We will get there eventually but I need to address another point first. > >> > >> My goal is to do exactly what the "other extension points with > >> reconfigurable custom configs" are doing unless there is a good reason > not > >> to. They provide a ready-made solution that will let us r
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Thanks Clement for your thoughts. According to my current experience rewriting the code twice I would say I did what you suggest in the last point - " We must make an attempt, if only to explain why it fails in the Rejected Alternatives section of the KIP." In the rejected alternatives I already mention why not merge SslFactory and SslEngineFactory and make SslFactory reconfigurable. @Colin can you please express your thoughts on this discussion so far? Since you refactored the SslFactory's code to have SslEngineBuilder I feel you would have more insights into future changes. Thanks Maulin On Fri, Sep 20, 2019 at 7:29 AM Pellerin, Clement wrote: > There will be chaperon code in the base class of the channel builders. > The arguments you gave me are emotional not technical. > The SSL extension point is reconfigurable hence it should extend > Reconfigurable. > We will encounter issues when we try to prototype it that way. > We will solve the issues or backtrack to another solution. > We must make an attempt, if only to explain why it fails in the Rejected > Alternatives section of the KIP. > > -Original Message- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Friday, September 20, 2019 2:40 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Overall my thinking is - When somebody wants to customize creation of > SSLEngine, most likely they are more expert in dealing with SSL domain > related stuff than "Kafka's reconfigurability" aspect. As a custom > implementation it makes more sense to me to say - Hey I'll control how I > initialize my SSL context/engine and btw if Kafka can give me a way to just > get re-created whenever some set of config keys changes, it is great! This > is similar to my thinking on KIP-486 which is- as a Kafka operator I am > just trying to be compliant to my company's security policies to load > keys/certs in certain way. For that, I should not be penalized by Kafka to > know all about Java security Providers and how to really create SSLContext > object etc given Java already provides a way to feed in KeyStore object > regardless of how I load it. > > On Thu, Sep 19, 2019 at 10:57 PM Maulin Vasavada < > maulin.vasav...@gmail.com> > wrote: > > > Hi Clement > > > > There will be good amount of state in the SslEngineFactory's default > > implementation. Hence I feel we might anyway have a chaperon class to > > provide reconfigurable functionality and will have one more class to host > > the state/behavior of actual SSLContext/SSLEngine creation. While doing > the > > internal rewrite (so far two times) both of the times I reached to the > same > > conclusion.I feel if we leave the reconfigurations to the implementation > - > > it will repeat the same pattern of having two classes to manage it - > since > > most likely they will also have similar state information. Instead keep > > that reconfigurations in SslFactory as is today and just allow "plugin of > > creation of SSLEngine". > > > > One note I would like to make is: You are comparing this to > MetricReporter > > but we have to keep in mind that SSL configuration is inherently more > > complex than a MetricReporters functionality. There are no JSSE > equivalent > > documents needed to be written for MetricReporter for example. So what > > works best for simpler solutions may not work equally well for more > complex > > scenarios. > > > > > > Thanks > > Maulin > > > > > > On Thu, Sep 19, 2019 at 8:36 PM Pellerin, Clement < > > clement_pelle...@ibi.com> wrote: > > > >> We will get there eventually but I need to address another point first. > >> > >> My goal is to do exactly what the "other extension points with > >> reconfigurable custom configs" are doing unless there is a good reason > not > >> to. They provide a ready-made solution that will let us reuse code, > avoid > >> pitfalls and show consistency. > >> > >> So far the roadblocks are > >> - the need to enforce mandatory compatibility checks for the keystores > >> and SSL handshake > >> - SslFactory is used in two channel builders. > >> > >> Both of these roadblocks can be overcome by moving the checks to a new > >> common base class of SslChannelBuilder and SaslChannelBuilder. This is > easy > >> since both classes extend Object directly. The new base class is not a > >> public API so any implementation will do. The chaperon class SslFactory > >> disappears and the int
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
There will be chaperon code in the base class of the channel builders. The arguments you gave me are emotional not technical. The SSL extension point is reconfigurable hence it should extend Reconfigurable. We will encounter issues when we try to prototype it that way. We will solve the issues or backtrack to another solution. We must make an attempt, if only to explain why it fails in the Rejected Alternatives section of the KIP. -Original Message- From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] Sent: Friday, September 20, 2019 2:40 AM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Overall my thinking is - When somebody wants to customize creation of SSLEngine, most likely they are more expert in dealing with SSL domain related stuff than "Kafka's reconfigurability" aspect. As a custom implementation it makes more sense to me to say - Hey I'll control how I initialize my SSL context/engine and btw if Kafka can give me a way to just get re-created whenever some set of config keys changes, it is great! This is similar to my thinking on KIP-486 which is- as a Kafka operator I am just trying to be compliant to my company's security policies to load keys/certs in certain way. For that, I should not be penalized by Kafka to know all about Java security Providers and how to really create SSLContext object etc given Java already provides a way to feed in KeyStore object regardless of how I load it. On Thu, Sep 19, 2019 at 10:57 PM Maulin Vasavada wrote: > Hi Clement > > There will be good amount of state in the SslEngineFactory's default > implementation. Hence I feel we might anyway have a chaperon class to > provide reconfigurable functionality and will have one more class to host > the state/behavior of actual SSLContext/SSLEngine creation. While doing the > internal rewrite (so far two times) both of the times I reached to the same > conclusion.I feel if we leave the reconfigurations to the implementation - > it will repeat the same pattern of having two classes to manage it - since > most likely they will also have similar state information. Instead keep > that reconfigurations in SslFactory as is today and just allow "plugin of > creation of SSLEngine". > > One note I would like to make is: You are comparing this to MetricReporter > but we have to keep in mind that SSL configuration is inherently more > complex than a MetricReporters functionality. There are no JSSE equivalent > documents needed to be written for MetricReporter for example. So what > works best for simpler solutions may not work equally well for more complex > scenarios. > > > Thanks > Maulin > > > On Thu, Sep 19, 2019 at 8:36 PM Pellerin, Clement < > clement_pelle...@ibi.com> wrote: > >> We will get there eventually but I need to address another point first. >> >> My goal is to do exactly what the "other extension points with >> reconfigurable custom configs" are doing unless there is a good reason not >> to. They provide a ready-made solution that will let us reuse code, avoid >> pitfalls and show consistency. >> >> So far the roadblocks are >> - the need to enforce mandatory compatibility checks for the keystores >> and SSL handshake >> - SslFactory is used in two channel builders. >> >> Both of these roadblocks can be overcome by moving the checks to a new >> common base class of SslChannelBuilder and SaslChannelBuilder. This is easy >> since both classes extend Object directly. The new base class is not a >> public API so any implementation will do. The chaperon class SslFactory >> disappears and the interface extends Reconfigurable. >> >> Does this proposal address all the reasons you had not to do exactly what >> other extension points are doing? >> >> -Original Message- >> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >> Sent: Thursday, September 19, 2019 10:21 PM >> To: dev@kafka.apache.org >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >> extensible >> >> Hi Clement >> >> So assuming there are two classes - SslFactory and SslEngineFactory like I >> suggested in my detailed post before this, we can use >> config.getConfiguredInstance() in SslFactory for SslEngineFactory class >> configuration and then followed by init() method. I don't see a challenge >> there. Can you please provide your input on my detailed post along with >> this recent point I am making? >> >> Thanks >> Maulin >> >> On Thu, Sep 19, 2019 at 5:04 PM Maulin Vasavada < >> maulin.vasav...@gmail.com> >> wrote: >> >> > Hi Clement,
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Overall my thinking is - When somebody wants to customize creation of SSLEngine, most likely they are more expert in dealing with SSL domain related stuff than "Kafka's reconfigurability" aspect. As a custom implementation it makes more sense to me to say - Hey I'll control how I initialize my SSL context/engine and btw if Kafka can give me a way to just get re-created whenever some set of config keys changes, it is great! This is similar to my thinking on KIP-486 which is- as a Kafka operator I am just trying to be compliant to my company's security policies to load keys/certs in certain way. For that, I should not be penalized by Kafka to know all about Java security Providers and how to really create SSLContext object etc given Java already provides a way to feed in KeyStore object regardless of how I load it. On Thu, Sep 19, 2019 at 10:57 PM Maulin Vasavada wrote: > Hi Clement > > There will be good amount of state in the SslEngineFactory's default > implementation. Hence I feel we might anyway have a chaperon class to > provide reconfigurable functionality and will have one more class to host > the state/behavior of actual SSLContext/SSLEngine creation. While doing the > internal rewrite (so far two times) both of the times I reached to the same > conclusion.I feel if we leave the reconfigurations to the implementation - > it will repeat the same pattern of having two classes to manage it - since > most likely they will also have similar state information. Instead keep > that reconfigurations in SslFactory as is today and just allow "plugin of > creation of SSLEngine". > > One note I would like to make is: You are comparing this to MetricReporter > but we have to keep in mind that SSL configuration is inherently more > complex than a MetricReporters functionality. There are no JSSE equivalent > documents needed to be written for MetricReporter for example. So what > works best for simpler solutions may not work equally well for more complex > scenarios. > > > Thanks > Maulin > > > On Thu, Sep 19, 2019 at 8:36 PM Pellerin, Clement < > clement_pelle...@ibi.com> wrote: > >> We will get there eventually but I need to address another point first. >> >> My goal is to do exactly what the "other extension points with >> reconfigurable custom configs" are doing unless there is a good reason not >> to. They provide a ready-made solution that will let us reuse code, avoid >> pitfalls and show consistency. >> >> So far the roadblocks are >> - the need to enforce mandatory compatibility checks for the keystores >> and SSL handshake >> - SslFactory is used in two channel builders. >> >> Both of these roadblocks can be overcome by moving the checks to a new >> common base class of SslChannelBuilder and SaslChannelBuilder. This is easy >> since both classes extend Object directly. The new base class is not a >> public API so any implementation will do. The chaperon class SslFactory >> disappears and the interface extends Reconfigurable. >> >> Does this proposal address all the reasons you had not to do exactly what >> other extension points are doing? >> >> -Original Message- >> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >> Sent: Thursday, September 19, 2019 10:21 PM >> To: dev@kafka.apache.org >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >> extensible >> >> Hi Clement >> >> So assuming there are two classes - SslFactory and SslEngineFactory like I >> suggested in my detailed post before this, we can use >> config.getConfiguredInstance() in SslFactory for SslEngineFactory class >> configuration and then followed by init() method. I don't see a challenge >> there. Can you please provide your input on my detailed post along with >> this recent point I am making? >> >> Thanks >> Maulin >> >> On Thu, Sep 19, 2019 at 5:04 PM Maulin Vasavada < >> maulin.vasav...@gmail.com> >> wrote: >> >> > Hi Clement, >> > >> > Thanks for pointing to AbstractConfig. Now I understand what you were >> > saying. I'll respond by tonight with more thoughts. >> > >> > Thanks >> > Maulin >> > >> > On Thu, Sep 19, 2019 at 5:46 AM Pellerin, Clement < >> > clement_pelle...@ibi.com> wrote: >> > >> >> I appreciate the effort you put into this. >> >> >> >> Lets do this in steps. You had a question on getConfiguredInstance(). >> >> >> >> The method getConfiguredInstance(key, Class) implemented in >> >> AbstractConfig is how the Metric
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Clement There will be good amount of state in the SslEngineFactory's default implementation. Hence I feel we might anyway have a chaperon class to provide reconfigurable functionality and will have one more class to host the state/behavior of actual SSLContext/SSLEngine creation. While doing the internal rewrite (so far two times) both of the times I reached to the same conclusion.I feel if we leave the reconfigurations to the implementation - it will repeat the same pattern of having two classes to manage it - since most likely they will also have similar state information. Instead keep that reconfigurations in SslFactory as is today and just allow "plugin of creation of SSLEngine". One note I would like to make is: You are comparing this to MetricReporter but we have to keep in mind that SSL configuration is inherently more complex than a MetricReporters functionality. There are no JSSE equivalent documents needed to be written for MetricReporter for example. So what works best for simpler solutions may not work equally well for more complex scenarios. Thanks Maulin On Thu, Sep 19, 2019 at 8:36 PM Pellerin, Clement wrote: > We will get there eventually but I need to address another point first. > > My goal is to do exactly what the "other extension points with > reconfigurable custom configs" are doing unless there is a good reason not > to. They provide a ready-made solution that will let us reuse code, avoid > pitfalls and show consistency. > > So far the roadblocks are > - the need to enforce mandatory compatibility checks for the keystores and > SSL handshake > - SslFactory is used in two channel builders. > > Both of these roadblocks can be overcome by moving the checks to a new > common base class of SslChannelBuilder and SaslChannelBuilder. This is easy > since both classes extend Object directly. The new base class is not a > public API so any implementation will do. The chaperon class SslFactory > disappears and the interface extends Reconfigurable. > > Does this proposal address all the reasons you had not to do exactly what > other extension points are doing? > > -Original Message- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Thursday, September 19, 2019 10:21 PM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Hi Clement > > So assuming there are two classes - SslFactory and SslEngineFactory like I > suggested in my detailed post before this, we can use > config.getConfiguredInstance() in SslFactory for SslEngineFactory class > configuration and then followed by init() method. I don't see a challenge > there. Can you please provide your input on my detailed post along with > this recent point I am making? > > Thanks > Maulin > > On Thu, Sep 19, 2019 at 5:04 PM Maulin Vasavada > > wrote: > > > Hi Clement, > > > > Thanks for pointing to AbstractConfig. Now I understand what you were > > saying. I'll respond by tonight with more thoughts. > > > > Thanks > > Maulin > > > > On Thu, Sep 19, 2019 at 5:46 AM Pellerin, Clement < > > clement_pelle...@ibi.com> wrote: > > > >> I appreciate the effort you put into this. > >> > >> Lets do this in steps. You had a question on getConfiguredInstance(). > >> > >> The method getConfiguredInstance(key, Class) implemented in > >> AbstractConfig is how the MetricsReporter and other extension points are > >> intantiated. Creating the extension point this way calls the default > >> constructor which is good. Since the (Re)Configurable interface dictates > >> the signature of the configure() method, that forces the addition of a > new > >> init(...) method to pass the other constructor arguments. > >> > >> Do we agree on that before we move on to other issues? > >> > >> -Original Message- > >> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > >> Sent: Wednesday, September 18, 2019 4:37 PM > >> To: dev@kafka.apache.org > >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > >> extensible > >> > >> Hi Clement > >> > >> Here are my thoughts based on my latest re-write attempt and learnings, > >> > >> 1. I think that it will be a great value to keep both classes separate - > >> SslFactory and SslEngineFactory and having method > reconfigurableConfigs() > >> in the SslEngineFactory. Here is the reasoning, > >> > >> a. It is kind of a Decorator pattern to me - even without named like > one > >> SslFactory is acting as a decor
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
We will get there eventually but I need to address another point first. My goal is to do exactly what the "other extension points with reconfigurable custom configs" are doing unless there is a good reason not to. They provide a ready-made solution that will let us reuse code, avoid pitfalls and show consistency. So far the roadblocks are - the need to enforce mandatory compatibility checks for the keystores and SSL handshake - SslFactory is used in two channel builders. Both of these roadblocks can be overcome by moving the checks to a new common base class of SslChannelBuilder and SaslChannelBuilder. This is easy since both classes extend Object directly. The new base class is not a public API so any implementation will do. The chaperon class SslFactory disappears and the interface extends Reconfigurable. Does this proposal address all the reasons you had not to do exactly what other extension points are doing? -Original Message- From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] Sent: Thursday, September 19, 2019 10:21 PM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Hi Clement So assuming there are two classes - SslFactory and SslEngineFactory like I suggested in my detailed post before this, we can use config.getConfiguredInstance() in SslFactory for SslEngineFactory class configuration and then followed by init() method. I don't see a challenge there. Can you please provide your input on my detailed post along with this recent point I am making? Thanks Maulin On Thu, Sep 19, 2019 at 5:04 PM Maulin Vasavada wrote: > Hi Clement, > > Thanks for pointing to AbstractConfig. Now I understand what you were > saying. I'll respond by tonight with more thoughts. > > Thanks > Maulin > > On Thu, Sep 19, 2019 at 5:46 AM Pellerin, Clement < > clement_pelle...@ibi.com> wrote: > >> I appreciate the effort you put into this. >> >> Lets do this in steps. You had a question on getConfiguredInstance(). >> >> The method getConfiguredInstance(key, Class) implemented in >> AbstractConfig is how the MetricsReporter and other extension points are >> intantiated. Creating the extension point this way calls the default >> constructor which is good. Since the (Re)Configurable interface dictates >> the signature of the configure() method, that forces the addition of a new >> init(...) method to pass the other constructor arguments. >> >> Do we agree on that before we move on to other issues? >> >> -Original Message- >> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >> Sent: Wednesday, September 18, 2019 4:37 PM >> To: dev@kafka.apache.org >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >> extensible >> >> Hi Clement >> >> Here are my thoughts based on my latest re-write attempt and learnings, >> >> 1. I think that it will be a great value to keep both classes separate - >> SslFactory and SslEngineFactory and having method reconfigurableConfigs() >> in the SslEngineFactory. Here is the reasoning, >> >> a. It is kind of a Decorator pattern to me - even without named like one >> SslFactory is acting as a decorator/surrogate to the SslEngineFactory and >> helping it get created and re-created as needed based on the >> terms/conditions specified by SslEngineFactory (via >> reconfigurableConfigs() >> method) >> >> b. SslEngineFactory will be pluggable class. By keeping the SslFactory >> reconfigurable with delegation of reconfigurableConfigs() to >> SslEngineFactory it allows the implementation of SslEngineFactory to be >> worry free of - How Kafka manages reconfigurations. The contract is - >> Kafka's SslFactory will ask the implementation to provide which >> configurations it is ready to be reconfigured for. Rest of the logic for >> triggering and reconfiguring and validation is in SslFactory. >> >> c. The current validation in SslFactory about inter-broker-ssl handshake >> AND verifying that certificate chain doesn't change via dynamic config >> changes is rightly owned by SslFactory. We should not give flexibility to >> SslEngineFactory to decide if they want that validation or not. >> >> d. If SslEngineFactory fails to be re-created with new dynamic config >> changes the constructor will throw some exception and the SslFactory will >> fail the validateReconfiguration() call resulting in no-change. Hence the >> validation if the new config is right is still controlled by the >> SslEngineFactory without necessarily having explicit validate method >> (assuming if you had a point about - we should keep validation of changed >
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Clement So assuming there are two classes - SslFactory and SslEngineFactory like I suggested in my detailed post before this, we can use config.getConfiguredInstance() in SslFactory for SslEngineFactory class configuration and then followed by init() method. I don't see a challenge there. Can you please provide your input on my detailed post along with this recent point I am making? Thanks Maulin On Thu, Sep 19, 2019 at 5:04 PM Maulin Vasavada wrote: > Hi Clement, > > Thanks for pointing to AbstractConfig. Now I understand what you were > saying. I'll respond by tonight with more thoughts. > > Thanks > Maulin > > On Thu, Sep 19, 2019 at 5:46 AM Pellerin, Clement < > clement_pelle...@ibi.com> wrote: > >> I appreciate the effort you put into this. >> >> Lets do this in steps. You had a question on getConfiguredInstance(). >> >> The method getConfiguredInstance(key, Class) implemented in >> AbstractConfig is how the MetricsReporter and other extension points are >> intantiated. Creating the extension point this way calls the default >> constructor which is good. Since the (Re)Configurable interface dictates >> the signature of the configure() method, that forces the addition of a new >> init(...) method to pass the other constructor arguments. >> >> Do we agree on that before we move on to other issues? >> >> -Original Message- >> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >> Sent: Wednesday, September 18, 2019 4:37 PM >> To: dev@kafka.apache.org >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >> extensible >> >> Hi Clement >> >> Here are my thoughts based on my latest re-write attempt and learnings, >> >> 1. I think that it will be a great value to keep both classes separate - >> SslFactory and SslEngineFactory and having method reconfigurableConfigs() >> in the SslEngineFactory. Here is the reasoning, >> >> a. It is kind of a Decorator pattern to me - even without named like one >> SslFactory is acting as a decorator/surrogate to the SslEngineFactory and >> helping it get created and re-created as needed based on the >> terms/conditions specified by SslEngineFactory (via >> reconfigurableConfigs() >> method) >> >> b. SslEngineFactory will be pluggable class. By keeping the SslFactory >> reconfigurable with delegation of reconfigurableConfigs() to >> SslEngineFactory it allows the implementation of SslEngineFactory to be >> worry free of - How Kafka manages reconfigurations. The contract is - >> Kafka's SslFactory will ask the implementation to provide which >> configurations it is ready to be reconfigured for. Rest of the logic for >> triggering and reconfiguring and validation is in SslFactory. >> >> c. The current validation in SslFactory about inter-broker-ssl handshake >> AND verifying that certificate chain doesn't change via dynamic config >> changes is rightly owned by SslFactory. We should not give flexibility to >> SslEngineFactory to decide if they want that validation or not. >> >> d. If SslEngineFactory fails to be re-created with new dynamic config >> changes the constructor will throw some exception and the SslFactory will >> fail the validateReconfiguration() call resulting in no-change. Hence the >> validation if the new config is right is still controlled by the >> SslEngineFactory without necessarily having explicit validate method >> (assuming if you had a point about - we should keep validation of changed >> configs in the pluggable class) >> >> >> 2. About the keystore validation in SslFactory - as I mentioned in above >> points, >> >> a. I feel it is Kafka's policy that it wants to mandate that validation >> regardless of the SslEngineFactory's implementation. I feel that >> regardless >> of customized implementation it is doing a 'logical' enforcement. I don't >> see many cases where you will end up changing certificate chain (I can't >> say the same about SANs entries though. see my below points). Hence that >> validation is reasonable to be generally enforced for dynamic config >> changes. If you change something violating that validation, you can avoid >> making such changes via dynamic configuration and do a rolling restarts of >> the boxes. >> >> b. If the implementation doesn't use keystore then automatically no >> validation will happen. Hence I don't see any issue with >> SslEngineFactory's >> implementations not having requirement to use keystores. >> >> c. There could be an argument however about - what it validates current
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Clement, Thanks for pointing to AbstractConfig. Now I understand what you were saying. I'll respond by tonight with more thoughts. Thanks Maulin On Thu, Sep 19, 2019 at 5:46 AM Pellerin, Clement wrote: > I appreciate the effort you put into this. > > Lets do this in steps. You had a question on getConfiguredInstance(). > > The method getConfiguredInstance(key, Class) implemented in AbstractConfig > is how the MetricsReporter and other extension points are intantiated. > Creating the extension point this way calls the default constructor which > is good. Since the (Re)Configurable interface dictates the signature of the > configure() method, that forces the addition of a new init(...) method to > pass the other constructor arguments. > > Do we agree on that before we move on to other issues? > > -Original Message- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Wednesday, September 18, 2019 4:37 PM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Hi Clement > > Here are my thoughts based on my latest re-write attempt and learnings, > > 1. I think that it will be a great value to keep both classes separate - > SslFactory and SslEngineFactory and having method reconfigurableConfigs() > in the SslEngineFactory. Here is the reasoning, > > a. It is kind of a Decorator pattern to me - even without named like one > SslFactory is acting as a decorator/surrogate to the SslEngineFactory and > helping it get created and re-created as needed based on the > terms/conditions specified by SslEngineFactory (via reconfigurableConfigs() > method) > > b. SslEngineFactory will be pluggable class. By keeping the SslFactory > reconfigurable with delegation of reconfigurableConfigs() to > SslEngineFactory it allows the implementation of SslEngineFactory to be > worry free of - How Kafka manages reconfigurations. The contract is - > Kafka's SslFactory will ask the implementation to provide which > configurations it is ready to be reconfigured for. Rest of the logic for > triggering and reconfiguring and validation is in SslFactory. > > c. The current validation in SslFactory about inter-broker-ssl handshake > AND verifying that certificate chain doesn't change via dynamic config > changes is rightly owned by SslFactory. We should not give flexibility to > SslEngineFactory to decide if they want that validation or not. > > d. If SslEngineFactory fails to be re-created with new dynamic config > changes the constructor will throw some exception and the SslFactory will > fail the validateReconfiguration() call resulting in no-change. Hence the > validation if the new config is right is still controlled by the > SslEngineFactory without necessarily having explicit validate method > (assuming if you had a point about - we should keep validation of changed > configs in the pluggable class) > > > 2. About the keystore validation in SslFactory - as I mentioned in above > points, > > a. I feel it is Kafka's policy that it wants to mandate that validation > regardless of the SslEngineFactory's implementation. I feel that regardless > of customized implementation it is doing a 'logical' enforcement. I don't > see many cases where you will end up changing certificate chain (I can't > say the same about SANs entries though. see my below points). Hence that > validation is reasonable to be generally enforced for dynamic config > changes. If you change something violating that validation, you can avoid > making such changes via dynamic configuration and do a rolling restarts of > the boxes. > > b. If the implementation doesn't use keystore then automatically no > validation will happen. Hence I don't see any issue with SslEngineFactory's > implementations not having requirement to use keystores. > > c. There could be an argument however about - what it validates currently > and is there a scope of change. Example: It validates SANs entries and that > to me is a challenge because I have had scenarios where I kept adding more > VIPs in my certs SANs entries without really changing any certificate > chain. The existing validation will fail that setup unnecessarily. Given > that - there could be change in SslFactory but that doesn't still make that > validation eligible to go to SslEngineFactory implementations. > > > 3. I am still in two minds about your point on - not using existing SSL > Reconfigurable configs to be used by SslFactory on top of > SslEngineFactory's reconfigurable configs. The reason for that is- > > a. I agree with you on that we should not worry about existing SSL > reconfigurable configs in new changed code for SslFactory. Why depend on > something you really do
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
I appreciate the effort you put into this. Lets do this in steps. You had a question on getConfiguredInstance(). The method getConfiguredInstance(key, Class) implemented in AbstractConfig is how the MetricsReporter and other extension points are intantiated. Creating the extension point this way calls the default constructor which is good. Since the (Re)Configurable interface dictates the signature of the configure() method, that forces the addition of a new init(...) method to pass the other constructor arguments. Do we agree on that before we move on to other issues? -Original Message- From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] Sent: Wednesday, September 18, 2019 4:37 PM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Hi Clement Here are my thoughts based on my latest re-write attempt and learnings, 1. I think that it will be a great value to keep both classes separate - SslFactory and SslEngineFactory and having method reconfigurableConfigs() in the SslEngineFactory. Here is the reasoning, a. It is kind of a Decorator pattern to me - even without named like one SslFactory is acting as a decorator/surrogate to the SslEngineFactory and helping it get created and re-created as needed based on the terms/conditions specified by SslEngineFactory (via reconfigurableConfigs() method) b. SslEngineFactory will be pluggable class. By keeping the SslFactory reconfigurable with delegation of reconfigurableConfigs() to SslEngineFactory it allows the implementation of SslEngineFactory to be worry free of - How Kafka manages reconfigurations. The contract is - Kafka's SslFactory will ask the implementation to provide which configurations it is ready to be reconfigured for. Rest of the logic for triggering and reconfiguring and validation is in SslFactory. c. The current validation in SslFactory about inter-broker-ssl handshake AND verifying that certificate chain doesn't change via dynamic config changes is rightly owned by SslFactory. We should not give flexibility to SslEngineFactory to decide if they want that validation or not. d. If SslEngineFactory fails to be re-created with new dynamic config changes the constructor will throw some exception and the SslFactory will fail the validateReconfiguration() call resulting in no-change. Hence the validation if the new config is right is still controlled by the SslEngineFactory without necessarily having explicit validate method (assuming if you had a point about - we should keep validation of changed configs in the pluggable class) 2. About the keystore validation in SslFactory - as I mentioned in above points, a. I feel it is Kafka's policy that it wants to mandate that validation regardless of the SslEngineFactory's implementation. I feel that regardless of customized implementation it is doing a 'logical' enforcement. I don't see many cases where you will end up changing certificate chain (I can't say the same about SANs entries though. see my below points). Hence that validation is reasonable to be generally enforced for dynamic config changes. If you change something violating that validation, you can avoid making such changes via dynamic configuration and do a rolling restarts of the boxes. b. If the implementation doesn't use keystore then automatically no validation will happen. Hence I don't see any issue with SslEngineFactory's implementations not having requirement to use keystores. c. There could be an argument however about - what it validates currently and is there a scope of change. Example: It validates SANs entries and that to me is a challenge because I have had scenarios where I kept adding more VIPs in my certs SANs entries without really changing any certificate chain. The existing validation will fail that setup unnecessarily. Given that - there could be change in SslFactory but that doesn't still make that validation eligible to go to SslEngineFactory implementations. 3. I am still in two minds about your point on - not using existing SSL Reconfigurable configs to be used by SslFactory on top of SslEngineFactory's reconfigurable configs. The reason for that is- a. I agree with you on that we should not worry about existing SSL reconfigurable configs in new changed code for SslFactory. Why depend on something you really don't need. However, Rajini's point is- if we decide to add more configs in the SSL reconfigurable configs which may be common across SslEngineFactory's implementations, it will make it easier. Again, just to make it easier we should not do it upfront. So now you see why I am double minded on it while more leaning toward your suggestion. 4. I think I totally miss what you refer by config.getConfiguredInstance(key, Class). Which Kafka existing class you are referring to when you do that? Do we have that in KafkaConfig? If you can clarify me on that I can think more about your input on it. 5. Now above all means- a. We
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
ctory to be involved at all." I think I am missing something. If we just have SslEngineFactory reconfigure itself it will generate new SSLContext and in-turn new SSLEgnine but how will it communicate that to the ChannelBuilders? Don't they have to refresh the reference to the SslEngineFactory via SslFactory's reconfigure() method in order to pick up that change? Thanks Maulin On Tue, Sep 17, 2019 at 4:49 AM Pellerin, Clement wrote: > Good point about the two callers of SslFactory. We can move the SslEngine > validation to a separate class and call it in both places. That SslEngine > validation class would not be part of the public API and therefore we don't > need to fuss about its API. > > -Original Message- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Tuesday, September 17, 2019 2:28 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Hi Clement/Rajini > > When I read your responses - I swing between both of your suggestions :) I > see both of your points. Let me ponder little bit more and give me take in > a day or so. > > I tend to agree with Clement in a sense that we need to define clear > responsibilities of classes. Right now I feel it's not clear. Also, I tend > to agree to both of you about keystore/truststore validation - the conflict > I've to propose a clean agreeable solution to. > > One clarification to Clement is - there are two classes using SslFactory > today - SslChannelBuilder and SaslChannelBuilder so we have to keep that in > mind. However, once we have clear responsibilities of classes, that should > automatically clear what goes where. > > Thanks > Maulin >
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Good point about the two callers of SslFactory. We can move the SslEngine validation to a separate class and call it in both places. That SslEngine validation class would not be part of the public API and therefore we don't need to fuss about its API. -Original Message- From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] Sent: Tuesday, September 17, 2019 2:28 AM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Hi Clement/Rajini When I read your responses - I swing between both of your suggestions :) I see both of your points. Let me ponder little bit more and give me take in a day or so. I tend to agree with Clement in a sense that we need to define clear responsibilities of classes. Right now I feel it's not clear. Also, I tend to agree to both of you about keystore/truststore validation - the conflict I've to propose a clean agreeable solution to. One clarification to Clement is - there are two classes using SslFactory today - SslChannelBuilder and SaslChannelBuilder so we have to keep that in mind. However, once we have clear responsibilities of classes, that should automatically clear what goes where. Thanks Maulin
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Clement/Rajini When I read your responses - I swing between both of your suggestions :) I see both of your points. Let me ponder little bit more and give me take in a day or so. I tend to agree with Clement in a sense that we need to define clear responsibilities of classes. Right now I feel it's not clear. Also, I tend to agree to both of you about keystore/truststore validation - the conflict I've to propose a clean agreeable solution to. One clarification to Clement is - there are two classes using SslFactory today - SslChannelBuilder and SaslChannelBuilder so we have to keep that in mind. However, once we have clear responsibilities of classes, that should automatically clear what goes where. Thanks Maulin On Mon, Sep 16, 2019 at 7:02 AM Pellerin, Clement wrote: > >> Pluggable instances have a default constructor and implement > Configurable. > >> Why wouldn't we just do that for SslEngineFactory? > Because the constructor that you are replacing has more than just a map. > You need to pass those extra arguments somewhere. > I once proposed to fake extra arguments as synthetic configs and I was > rightfully shut down. > I agree though that the map should be passed in configure() > > >> We can rename SslEngineFactory.reconfigurableConfigs() if it helps > You don't want to rename this method because it should contain only the > reconfigurable configs, not all configs. > > >> Perhaps we need to focus on the aspects of your use case that are not > >> handled with the proposed approach. > My use case is documented in KIP-383. In more detail, I want to pass an > object in a config and the custom implementation can call this object to > get the SslEngine from elsewhere in my application. There should be no > keystore/truststore validation what so ever and there should not be any SSL > Configs except the implementation class name and that object. > > The only known objection why SslFactory cannot be the extension point is > because it contains a lot of validation code that would likely be by-passed > by custom implementations. I don't agree with this objection because the > keystore validation must be moved to DefaultSslEngineFactory and the rest > can be moved to the SslChannelBuilder. This way SslFactory would fit the > existing extension pattern in Kafka. An extension point that extends > Reconfigurable is a lot cleaner. > > If you wonder why I changed my mind so strongly, it's because I realized > there is less and less validation left in SslFactory and I expect > SslChannelBuilder to be the only caller so it gives an anchor where to > attach the remaining validation code. > > Can we discuss the possibility of moving the validation code to > SslChannelBuilder? I did not look at the code and I don't know the > challenges we would face. > > -----Original Message----- > From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] > Sent: Monday, September 16, 2019 8:59 AM > To: dev > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > I am not sure what the confusion regarding Configurable interface is. > Pluggable instances have a default constructor and implement Configurable. > Why wouldn't we just do that for SslEngineFactory? > > We can rename SslEngineFactory.reconfigurableConfigs() if it helps, but > basically it is the factory that decides what configs it is interested in. > It doesn't really matter that we don't know what the reconfigurable configs > are until the SslFactory is configured. > > `sslContext()` method was restored to support some connectors that were > using it, but we don't need to add that to the interface since the plan is > to remove that anyway. We do need `keystore()` and `truststore()` methods > for validation. > > It is always the responsibility of plugins to validate the configs passed > to them. Custom plugins must validate configs and fail configure() if > configs are invalid. We would expect the same for SslEngineFactory. > > Perhaps we need to focus on the aspects of your use case that are not > handled with the proposed approach. > > On Mon, Sep 16, 2019 at 1:26 PM Pellerin, Clement < > clement_pelle...@ibi.com> > wrote: > > > There are pending issues we need to address. > > > > We want to be able to call config.getConfiguredInstance(key, Class) to > > instantiate the extension point. This requires a default constructor. The > > former constructor arguments must now be passed in a separate init() > > method. This has the advantage of moving the constructor signature from > the > > comment prose to the compiled language. I took inspiration from > > MetricsReporter for the init() method. > > > > I question the object or
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
>> Pluggable instances have a default constructor and implement Configurable. >> Why wouldn't we just do that for SslEngineFactory? Because the constructor that you are replacing has more than just a map. You need to pass those extra arguments somewhere. I once proposed to fake extra arguments as synthetic configs and I was rightfully shut down. I agree though that the map should be passed in configure() >> We can rename SslEngineFactory.reconfigurableConfigs() if it helps You don't want to rename this method because it should contain only the reconfigurable configs, not all configs. >> Perhaps we need to focus on the aspects of your use case that are not >> handled with the proposed approach. My use case is documented in KIP-383. In more detail, I want to pass an object in a config and the custom implementation can call this object to get the SslEngine from elsewhere in my application. There should be no keystore/truststore validation what so ever and there should not be any SSL Configs except the implementation class name and that object. The only known objection why SslFactory cannot be the extension point is because it contains a lot of validation code that would likely be by-passed by custom implementations. I don't agree with this objection because the keystore validation must be moved to DefaultSslEngineFactory and the rest can be moved to the SslChannelBuilder. This way SslFactory would fit the existing extension pattern in Kafka. An extension point that extends Reconfigurable is a lot cleaner. If you wonder why I changed my mind so strongly, it's because I realized there is less and less validation left in SslFactory and I expect SslChannelBuilder to be the only caller so it gives an anchor where to attach the remaining validation code. Can we discuss the possibility of moving the validation code to SslChannelBuilder? I did not look at the code and I don't know the challenges we would face. -Original Message- From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] Sent: Monday, September 16, 2019 8:59 AM To: dev Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible I am not sure what the confusion regarding Configurable interface is. Pluggable instances have a default constructor and implement Configurable. Why wouldn't we just do that for SslEngineFactory? We can rename SslEngineFactory.reconfigurableConfigs() if it helps, but basically it is the factory that decides what configs it is interested in. It doesn't really matter that we don't know what the reconfigurable configs are until the SslFactory is configured. `sslContext()` method was restored to support some connectors that were using it, but we don't need to add that to the interface since the plan is to remove that anyway. We do need `keystore()` and `truststore()` methods for validation. It is always the responsibility of plugins to validate the configs passed to them. Custom plugins must validate configs and fail configure() if configs are invalid. We would expect the same for SslEngineFactory. Perhaps we need to focus on the aspects of your use case that are not handled with the proposed approach. On Mon, Sep 16, 2019 at 1:26 PM Pellerin, Clement wrote: > There are pending issues we need to address. > > We want to be able to call config.getConfiguredInstance(key, Class) to > instantiate the extension point. This requires a default constructor. The > former constructor arguments must now be passed in a separate init() > method. This has the advantage of moving the constructor signature from the > comment prose to the compiled language. I took inspiration from > MetricsReporter for the init() method. > > I question the object oriented design that requires the > reconfigurableConfigs() method but declares the interface to be > non-reconfigurable with just the Configurable interface. > > My use case removes all built-in SSL configs (except the interface class > name of course). SslFactory should not hardcode any SSL configs in the > reconfigurableConfigs. It should delegate to the interface instance for all > reconfigurableConfigs. In particular, it cannot assume there are keystores > and truststores to validate. These checks should be moved to > DefaultSslEngineFactory. We can then consider moving the SslEngine > validation from SslFactory to SslChannelBuilder. What would be left in > SslFactory that forces us to keep it instead of making it the > Reconfigurable extension point itself? > > I believe we don't need the sslContext() method since it is only used by a > junit. > If we support my use case, there is a good chance we don't need the > keystore() and truststore() method. > > I am still not comfortable with the fact that reconfigurableConfigs() are > not known until the SslEngineFactory implementation is created and that > happens only af
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
I am not sure what the confusion regarding Configurable interface is.
Pluggable instances have a default constructor and implement Configurable.
Why wouldn't we just do that for SslEngineFactory?
We can rename SslEngineFactory.reconfigurableConfigs() if it helps, but
basically it is the factory that decides what configs it is interested in.
It doesn't really matter that we don't know what the reconfigurable configs
are until the SslFactory is configured.
`sslContext()` method was restored to support some connectors that were
using it, but we don't need to add that to the interface since the plan is
to remove that anyway. We do need `keystore()` and `truststore()` methods
for validation.
It is always the responsibility of plugins to validate the configs passed
to them. Custom plugins must validate configs and fail configure() if
configs are invalid. We would expect the same for SslEngineFactory.
Perhaps we need to focus on the aspects of your use case that are not
handled with the proposed approach.
On Mon, Sep 16, 2019 at 1:26 PM Pellerin, Clement
wrote:
> There are pending issues we need to address.
>
> We want to be able to call config.getConfiguredInstance(key, Class) to
> instantiate the extension point. This requires a default constructor. The
> former constructor arguments must now be passed in a separate init()
> method. This has the advantage of moving the constructor signature from the
> comment prose to the compiled language. I took inspiration from
> MetricsReporter for the init() method.
>
> I question the object oriented design that requires the
> reconfigurableConfigs() method but declares the interface to be
> non-reconfigurable with just the Configurable interface.
>
> My use case removes all built-in SSL configs (except the interface class
> name of course). SslFactory should not hardcode any SSL configs in the
> reconfigurableConfigs. It should delegate to the interface instance for all
> reconfigurableConfigs. In particular, it cannot assume there are keystores
> and truststores to validate. These checks should be moved to
> DefaultSslEngineFactory. We can then consider moving the SslEngine
> validation from SslFactory to SslChannelBuilder. What would be left in
> SslFactory that forces us to keep it instead of making it the
> Reconfigurable extension point itself?
>
> I believe we don't need the sslContext() method since it is only used by a
> junit.
> If we support my use case, there is a good chance we don't need the
> keystore() and truststore() method.
>
> I am still not comfortable with the fact that reconfigurableConfigs() are
> not known until the SslEngineFactory implementation is created and that
> happens only after configure() is called. Notice this goes away if
> SslFactory is the extension point, which would explain why this might not
> have been an issue with the other extension points exposing reconfigurable
> custom configs.
>
> We must document if the configs sent to the extension point implementation
> have been validated or not. I am pretty sure they are not since
> config.getConfiguredInstance() passes the originals() and there is no
> configurableConfigs() method (there is only the subset in
> reconfigurableConfigs()). Non-validated configs might be of the wrong type,
> be out of range, or missing since the default value is not applied. This is
> a burden to the extension point developer and Kafka should provide
> utilities for this.
>
> Can you confirm where the special case for the reconfiguration of
> keystore/truststore is implemented? I am still trying to determine if it is
> possible to trigger a reconfiguration when none of the known configs have
> changed.
>
>
> -Original Message-----
> From: Rajini Sivaram [mailto:rajinisiva...@gmail.com]
> Sent: Monday, September 16, 2019 5:29 AM
> To: dev
> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration
> extensible
>
> Hi Maulin,
>
> Thanks for writing that up. I think we are getting close.
>
> As an example interface (which can be adjusted as necessary, but just
> including something here to refer to), I think we would have an interface
> along these lines:
>
> public interface SslEngineFactory extends Configurable, Closeable {
>
> Set reconfigurableConfigs();
> boolean shouldBeRebuilt(Map nextConfigs);
>
> SSLEngine createSslEngine(Mode mode, String peerHost, int
> peerPort, String endpointIdentification);
> SSLContext sslContext();
> KeyStore keystore();
> KeyStore truststore();
> }
>
> A) *Creation:*
>
>- We typically tend to include configs in constructor for non-pluggable
>classes, but use Configurable for pluggable classes. So Option 2 works
>better
>
> B) *Returning reconfigurable configs: *
&g
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
There are pending issues we need to address.
We want to be able to call config.getConfiguredInstance(key, Class) to
instantiate the extension point. This requires a default constructor. The
former constructor arguments must now be passed in a separate init() method.
This has the advantage of moving the constructor signature from the comment
prose to the compiled language. I took inspiration from MetricsReporter for the
init() method.
I question the object oriented design that requires the reconfigurableConfigs()
method but declares the interface to be non-reconfigurable with just the
Configurable interface.
My use case removes all built-in SSL configs (except the interface class name
of course). SslFactory should not hardcode any SSL configs in the
reconfigurableConfigs. It should delegate to the interface instance for all
reconfigurableConfigs. In particular, it cannot assume there are keystores and
truststores to validate. These checks should be moved to
DefaultSslEngineFactory. We can then consider moving the SslEngine validation
from SslFactory to SslChannelBuilder. What would be left in SslFactory that
forces us to keep it instead of making it the Reconfigurable extension point
itself?
I believe we don't need the sslContext() method since it is only used by a
junit.
If we support my use case, there is a good chance we don't need the keystore()
and truststore() method.
I am still not comfortable with the fact that reconfigurableConfigs() are not
known until the SslEngineFactory implementation is created and that happens
only after configure() is called. Notice this goes away if SslFactory is the
extension point, which would explain why this might not have been an issue with
the other extension points exposing reconfigurable custom configs.
We must document if the configs sent to the extension point implementation have
been validated or not. I am pretty sure they are not since
config.getConfiguredInstance() passes the originals() and there is no
configurableConfigs() method (there is only the subset in
reconfigurableConfigs()). Non-validated configs might be of the wrong type, be
out of range, or missing since the default value is not applied. This is a
burden to the extension point developer and Kafka should provide utilities for
this.
Can you confirm where the special case for the reconfiguration of
keystore/truststore is implemented? I am still trying to determine if it is
possible to trigger a reconfiguration when none of the known configs have
changed.
-Original Message-
From: Rajini Sivaram [mailto:rajinisiva...@gmail.com]
Sent: Monday, September 16, 2019 5:29 AM
To: dev
Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Maulin,
Thanks for writing that up. I think we are getting close.
As an example interface (which can be adjusted as necessary, but just
including something here to refer to), I think we would have an interface
along these lines:
public interface SslEngineFactory extends Configurable, Closeable {
Set reconfigurableConfigs();
boolean shouldBeRebuilt(Map nextConfigs);
SSLEngine createSslEngine(Mode mode, String peerHost, int
peerPort, String endpointIdentification);
SSLContext sslContext();
KeyStore keystore();
KeyStore truststore();
}
A) *Creation:*
- We typically tend to include configs in constructor for non-pluggable
classes, but use Configurable for pluggable classes. So Option 2 works
better
B) *Returning reconfigurable configs: *
- Custom SslEngineFactory implementations will only return what they
care about in their implementation of reconfigurableConfigs().
- SslChannelBuilder will delegate to SslFactory as you mentioned.
- SslFactory.reconfigurableConfigs() will return
SslConfigs.RECONFIGURABLE_CONFIGS plus SslEngineFactory.
reconfigurableConfigs(). So one day if we make endpoint validation
reconfigurable, it would all just work. We can easily find a different way
of continuing to reconfigure SslFactory without config changes if we
needed to since it is not a pluggable class.
C) *Triggering reconfiguration:*
- We continue to use AdminClient for reconfiguration in brokers with
validation. That goes through SslEngineFactory.shouldBeRebuilt() and
creates a new SslEngineFactory instance if needed.
D) *Handling push notification kind of needs*
- For brokers, we want SslFactory to manage reconfiguration since
configs may change. Also, AdminClient requests in brokers give a clear
auditable path for config updates where update failures are returned to the
caller. C) enables this.
- For client-side, custom SslEngineFactory implementations could
reconfigure themselves, we don't really need SslFactory to be involved
at all.
On Sat, Sep 14, 2019 at 8:17 AM Maulin Vasavada
wrote:
> Hi Clement/Rajini
>
> Based on what I get from my own understanding and your latest inputs, I'll
> wri
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
so. As you would have noticed in
> KIP-486 motivation the deployment challenge on Broker and Client side both
> makes it challenging to manage key/cert rotations etc. Hence I feel we
> should consider option of method in the SslEngineFactory to take a
> reconfigurable and call it when it knows it needs to be reconfigured.
>
> Overall, I feel we should avoid any coupling with existing handling of SSL
> keystore and truststore via the SSL Configs. When we are allowing
> customization for the whole SSLEngine then we don't want it to rely on
> existing configs to assume/necessarily-reuse any mechanism for loading
> keystore and Truststore.
>
> For MetricReporters I feel it is using custom configs like this except that
> MetricReporter interface defines init() and extends Configurable. Please
> clarify if I am missing something.
>
> Thanks
> Maulin
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> On Fri, Sep 13, 2019 at 12:15 PM Maulin Vasavada <
> maulin.vasav...@gmail.com>
> wrote:
>
> > Hi Clement/Rajini
> >
> > I've gone through the code to understand how reconfigruation work
> > currently. I sent both of you a note separately also. Let us reconvene
> next
> > week and proceed.
> >
> > Thanks
> > Maulin
> >
> > On Thu, Sep 12, 2019 at 12:25 PM Pellerin, Clement <
> > clement_pelle...@ibi.com> wrote:
> >
> >> You are proposing a different design pattern for the SSL reconfigurable
> >> extension point than the one already implemented for MetricsReporter.
> You
> >> need a good reason to justify this decision. It is as if you consider
> >> SslFactory the extension point. This is indeed something I proposed
> >> multiple times and was always shut down.
> >>
> >> Going back to your latest proposal, notice you cannot call
> >> reconfigurableConfigs() until configure() is called because you need to
> >> instantiate SslEngineFactory() first. There is no way to enforce this in
> >> the API.
> >>
> >> You did not react to my observation that ConfigDef does a better job
> >> validating/casting configs based on the ConfigKey declarations compared
> to
> >> ad hoc code you force people to write in their extension point
> >> implementations.
> >>
> >> You suggest not to augment ConfigDef with custom configs, so that takes
> >> care of the recursive dependency.
> >> I just noticed reconfigurableConfigs() returns Set and that does
> >> not force the creation of a ConfigKey for custom configs.
> >>
> >> >> SslFactory.reconfigurableConfigs() returns all standard
> reconfigurable
> >> SSL configs as well as MySslEngineFactory.reconfigurableConfigs().
> >> That's no good because that implementation does not use the standard
> >> property for the keystore location. For that particular use case, it
> would
> >> probably be better to reuse the standard keystore location config and
> >> change its semantics to a URL. Regardless, my point is the custom
> >> implementation should decide all the reconfigurable properties. By the
> way,
> >> my original use case for KIP-383 was to replace all SSL configs with a
> >> single name.
> >>
> >> It is still not clear in your email if the keystore/truststore exception
> >> is handled locally in SslFactory or by the initiator of the whole
> >> AlterConfig. That determines whether "AlterConfig without config
> changes"
> >> always goes through or is usually blocked early by the initiator.
> >>
> >>
> >> -Original Message-
> >> From: Rajini Sivaram [mailto:rajinisiva...@gmail.com]
> >> Sent: Thursday, September 12, 2019 2:05 PM
> >> To: dev
> >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration
> >> extensible
> >>
> >> Sorry about the confusion, my notes were conflicting!
> >>
> >> Let me give an example to clarify. Let's say KIP-519 adds a new
> interface
> >> SslEngineFactory and I have a custom implementation MySslEngineFactory
> >> that
> >> gets keystore material from a web service using a custom config `
> >> my.ssl.keystore.url`. MySslEngineFactory.reconfigurableConfigs() returns
> >> {`
> >> my.ssl.keystore.url`}. We don't want to add this custom config to Kafka.
> >> My
> >> SslEngineFactory will be managed by the (non-pluggable) Reconfigurable
> >> SslFactory that invokes appropriate methods on MySslEngineFactory to
> >> c
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
custom configs.
>>
>> >> SslFactory.reconfigurableConfigs() returns all standard reconfigurable
>> SSL configs as well as MySslEngineFactory.reconfigurableConfigs().
>> That's no good because that implementation does not use the standard
>> property for the keystore location. For that particular use case, it would
>> probably be better to reuse the standard keystore location config and
>> change its semantics to a URL. Regardless, my point is the custom
>> implementation should decide all the reconfigurable properties. By the way,
>> my original use case for KIP-383 was to replace all SSL configs with a
>> single name.
>>
>> It is still not clear in your email if the keystore/truststore exception
>> is handled locally in SslFactory or by the initiator of the whole
>> AlterConfig. That determines whether "AlterConfig without config changes"
>> always goes through or is usually blocked early by the initiator.
>>
>>
>> -Original Message-
>> From: Rajini Sivaram [mailto:rajinisiva...@gmail.com]
>> Sent: Thursday, September 12, 2019 2:05 PM
>> To: dev
>> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration
>> extensible
>>
>> Sorry about the confusion, my notes were conflicting!
>>
>> Let me give an example to clarify. Let's say KIP-519 adds a new interface
>> SslEngineFactory and I have a custom implementation MySslEngineFactory
>> that
>> gets keystore material from a web service using a custom config `
>> my.ssl.keystore.url`. MySslEngineFactory.reconfigurableConfigs() returns
>> {`
>> my.ssl.keystore.url`}. We don't want to add this custom config to Kafka.
>> My
>> SslEngineFactory will be managed by the (non-pluggable) Reconfigurable
>> SslFactory that invokes appropriate methods on MySslEngineFactory to
>> create
>> a new SslEngine when required. SslFactory.reconfigurableConfigs() returns
>> all standard reconfigurable SSL configs as well as
>> MySslEngineFactory.reconfigurableConfigs().
>>
>> The existing code in the broker is sufficient for both validation and
>> reconfiguration. We can support custom configs as well as reconfiguration
>> without config change. The only non-SSL change we require is a fix for
>> KAFKA-7588.
>>
>> 1) *Initial validation*: SslFactory.configure() invokes
>> MySslEngineFactory#configure() which validates the custom config `
>> my.ssl.keystore.url`. Broker will fail to start if the URL is invalid even
>> though broker knows nothing about this config because the custom
>> implementation fails its `configure()`.
>> 2) *Validation during reconfiguration*: AdminClient sends an AlterConfig
>> request to update `my.ssl.keystore.url` Broker invokes.
>> SslFactory.validateReconfiguration() which will invoke
>> MySslEngineFactory.validateReconfiguration() and that can fail
>> reconfiguration if the provided URL is invalid. And AdminClient sees this
>> validation failure in its response. This validation is more flexible than
>> that provided by ConfigDef. During reconfiguration, we are not just
>> validating that the value is of the right type, we may want to verify that
>> you can connect to the remote web service for example. This validation
>> path
>> for custom configs is already supported.
>> 3) *Reconfiguration without config change*: This is supported specifically
>> for two configs `ssl.keystore.location` and `ssl.truststore.location`.
>> SslFactory is given the opportunity to reconfigure whenever there is an
>> AlterConfig request from AdminClient since it uses these configs. This is
>> regardless of whether there is a config change. SslFactory will invoke
>> MySslEngineFactory.shouldBeRebuilt(). This gives MySslEngineFactory the
>> opportunity to provide a new SslEngine whenever there is an AdminClient
>> request to alter configs, regardless of whether any config changed or not.
>>
>> Hope that helps,
>>
>> Rajini
>>
>> On Thu, Sep 12, 2019 at 5:26 PM Pellerin, Clement <
>> clement_pelle...@ibi.com>
>> wrote:
>>
>> > I'm confused. Can you launch a reconfiguration without a config change
>> or
>> > not?
>> >
>> > If I understand the test case correctly, the design pattern to
>> implement a
>> > reconfigurable extension point in Kafka is to implement the
>> Reconfigurable
>> > interface. This means SslEngineFactory would be created once and mutate
>> > with reconfigure. There is no ConfigKey created for the custom config
>> and
>> > therefore there is no validation by Confi
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Clement/Rajini
I've gone through the code to understand how reconfigruation work
currently. I sent both of you a note separately also. Let us reconvene next
week and proceed.
Thanks
Maulin
On Thu, Sep 12, 2019 at 12:25 PM Pellerin, Clement
wrote:
> You are proposing a different design pattern for the SSL reconfigurable
> extension point than the one already implemented for MetricsReporter. You
> need a good reason to justify this decision. It is as if you consider
> SslFactory the extension point. This is indeed something I proposed
> multiple times and was always shut down.
>
> Going back to your latest proposal, notice you cannot call
> reconfigurableConfigs() until configure() is called because you need to
> instantiate SslEngineFactory() first. There is no way to enforce this in
> the API.
>
> You did not react to my observation that ConfigDef does a better job
> validating/casting configs based on the ConfigKey declarations compared to
> ad hoc code you force people to write in their extension point
> implementations.
>
> You suggest not to augment ConfigDef with custom configs, so that takes
> care of the recursive dependency.
> I just noticed reconfigurableConfigs() returns Set and that does
> not force the creation of a ConfigKey for custom configs.
>
> >> SslFactory.reconfigurableConfigs() returns all standard reconfigurable
> SSL configs as well as MySslEngineFactory.reconfigurableConfigs().
> That's no good because that implementation does not use the standard
> property for the keystore location. For that particular use case, it would
> probably be better to reuse the standard keystore location config and
> change its semantics to a URL. Regardless, my point is the custom
> implementation should decide all the reconfigurable properties. By the way,
> my original use case for KIP-383 was to replace all SSL configs with a
> single name.
>
> It is still not clear in your email if the keystore/truststore exception
> is handled locally in SslFactory or by the initiator of the whole
> AlterConfig. That determines whether "AlterConfig without config changes"
> always goes through or is usually blocked early by the initiator.
>
>
> -Original Message-----
> From: Rajini Sivaram [mailto:rajinisiva...@gmail.com]
> Sent: Thursday, September 12, 2019 2:05 PM
> To: dev
> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration
> extensible
>
> Sorry about the confusion, my notes were conflicting!
>
> Let me give an example to clarify. Let's say KIP-519 adds a new interface
> SslEngineFactory and I have a custom implementation MySslEngineFactory that
> gets keystore material from a web service using a custom config `
> my.ssl.keystore.url`. MySslEngineFactory.reconfigurableConfigs() returns {`
> my.ssl.keystore.url`}. We don't want to add this custom config to Kafka. My
> SslEngineFactory will be managed by the (non-pluggable) Reconfigurable
> SslFactory that invokes appropriate methods on MySslEngineFactory to create
> a new SslEngine when required. SslFactory.reconfigurableConfigs() returns
> all standard reconfigurable SSL configs as well as
> MySslEngineFactory.reconfigurableConfigs().
>
> The existing code in the broker is sufficient for both validation and
> reconfiguration. We can support custom configs as well as reconfiguration
> without config change. The only non-SSL change we require is a fix for
> KAFKA-7588.
>
> 1) *Initial validation*: SslFactory.configure() invokes
> MySslEngineFactory#configure() which validates the custom config `
> my.ssl.keystore.url`. Broker will fail to start if the URL is invalid even
> though broker knows nothing about this config because the custom
> implementation fails its `configure()`.
> 2) *Validation during reconfiguration*: AdminClient sends an AlterConfig
> request to update `my.ssl.keystore.url` Broker invokes.
> SslFactory.validateReconfiguration() which will invoke
> MySslEngineFactory.validateReconfiguration() and that can fail
> reconfiguration if the provided URL is invalid. And AdminClient sees this
> validation failure in its response. This validation is more flexible than
> that provided by ConfigDef. During reconfiguration, we are not just
> validating that the value is of the right type, we may want to verify that
> you can connect to the remote web service for example. This validation path
> for custom configs is already supported.
> 3) *Reconfiguration without config change*: This is supported specifically
> for two configs `ssl.keystore.location` and `ssl.truststore.location`.
> SslFactory is given the opportunity to reconfigure whenever there is an
> AlterConfig request from AdminClient since it uses these configs. This is
> regardless of whether the
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
You are proposing a different design pattern for the SSL reconfigurable
extension point than the one already implemented for MetricsReporter. You need
a good reason to justify this decision. It is as if you consider SslFactory the
extension point. This is indeed something I proposed multiple times and was
always shut down.
Going back to your latest proposal, notice you cannot call
reconfigurableConfigs() until configure() is called because you need to
instantiate SslEngineFactory() first. There is no way to enforce this in the
API.
You did not react to my observation that ConfigDef does a better job
validating/casting configs based on the ConfigKey declarations compared to ad
hoc code you force people to write in their extension point implementations.
You suggest not to augment ConfigDef with custom configs, so that takes care of
the recursive dependency.
I just noticed reconfigurableConfigs() returns Set and that does not
force the creation of a ConfigKey for custom configs.
>> SslFactory.reconfigurableConfigs() returns all standard reconfigurable SSL
>> configs as well as MySslEngineFactory.reconfigurableConfigs().
That's no good because that implementation does not use the standard property
for the keystore location. For that particular use case, it would probably be
better to reuse the standard keystore location config and change its semantics
to a URL. Regardless, my point is the custom implementation should decide all
the reconfigurable properties. By the way, my original use case for KIP-383 was
to replace all SSL configs with a single name.
It is still not clear in your email if the keystore/truststore exception is
handled locally in SslFactory or by the initiator of the whole AlterConfig.
That determines whether "AlterConfig without config changes" always goes
through or is usually blocked early by the initiator.
-Original Message-
From: Rajini Sivaram [mailto:rajinisiva...@gmail.com]
Sent: Thursday, September 12, 2019 2:05 PM
To: dev
Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Sorry about the confusion, my notes were conflicting!
Let me give an example to clarify. Let's say KIP-519 adds a new interface
SslEngineFactory and I have a custom implementation MySslEngineFactory that
gets keystore material from a web service using a custom config `
my.ssl.keystore.url`. MySslEngineFactory.reconfigurableConfigs() returns {`
my.ssl.keystore.url`}. We don't want to add this custom config to Kafka. My
SslEngineFactory will be managed by the (non-pluggable) Reconfigurable
SslFactory that invokes appropriate methods on MySslEngineFactory to create
a new SslEngine when required. SslFactory.reconfigurableConfigs() returns
all standard reconfigurable SSL configs as well as
MySslEngineFactory.reconfigurableConfigs().
The existing code in the broker is sufficient for both validation and
reconfiguration. We can support custom configs as well as reconfiguration
without config change. The only non-SSL change we require is a fix for
KAFKA-7588.
1) *Initial validation*: SslFactory.configure() invokes
MySslEngineFactory#configure() which validates the custom config `
my.ssl.keystore.url`. Broker will fail to start if the URL is invalid even
though broker knows nothing about this config because the custom
implementation fails its `configure()`.
2) *Validation during reconfiguration*: AdminClient sends an AlterConfig
request to update `my.ssl.keystore.url` Broker invokes.
SslFactory.validateReconfiguration() which will invoke
MySslEngineFactory.validateReconfiguration() and that can fail
reconfiguration if the provided URL is invalid. And AdminClient sees this
validation failure in its response. This validation is more flexible than
that provided by ConfigDef. During reconfiguration, we are not just
validating that the value is of the right type, we may want to verify that
you can connect to the remote web service for example. This validation path
for custom configs is already supported.
3) *Reconfiguration without config change*: This is supported specifically
for two configs `ssl.keystore.location` and `ssl.truststore.location`.
SslFactory is given the opportunity to reconfigure whenever there is an
AlterConfig request from AdminClient since it uses these configs. This is
regardless of whether there is a config change. SslFactory will invoke
MySslEngineFactory.shouldBeRebuilt(). This gives MySslEngineFactory the
opportunity to provide a new SslEngine whenever there is an AdminClient
request to alter configs, regardless of whether any config changed or not.
Hope that helps,
Rajini
On Thu, Sep 12, 2019 at 5:26 PM Pellerin, Clement
wrote:
> I'm confused. Can you launch a reconfiguration without a config change or
> not?
>
> If I understand the test case correctly, the design pattern to implement a
> reconfigurable extension point in Kafka is to implement the Reconfigurable
> interface. This mean
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Sorry about the confusion, my notes were conflicting!
Let me give an example to clarify. Let's say KIP-519 adds a new interface
SslEngineFactory and I have a custom implementation MySslEngineFactory that
gets keystore material from a web service using a custom config `
my.ssl.keystore.url`. MySslEngineFactory.reconfigurableConfigs() returns {`
my.ssl.keystore.url`}. We don't want to add this custom config to Kafka. My
SslEngineFactory will be managed by the (non-pluggable) Reconfigurable
SslFactory that invokes appropriate methods on MySslEngineFactory to create
a new SslEngine when required. SslFactory.reconfigurableConfigs() returns
all standard reconfigurable SSL configs as well as
MySslEngineFactory.reconfigurableConfigs().
The existing code in the broker is sufficient for both validation and
reconfiguration. We can support custom configs as well as reconfiguration
without config change. The only non-SSL change we require is a fix for
KAFKA-7588.
1) *Initial validation*: SslFactory.configure() invokes
MySslEngineFactory#configure() which validates the custom config `
my.ssl.keystore.url`. Broker will fail to start if the URL is invalid even
though broker knows nothing about this config because the custom
implementation fails its `configure()`.
2) *Validation during reconfiguration*: AdminClient sends an AlterConfig
request to update `my.ssl.keystore.url` Broker invokes.
SslFactory.validateReconfiguration() which will invoke
MySslEngineFactory.validateReconfiguration() and that can fail
reconfiguration if the provided URL is invalid. And AdminClient sees this
validation failure in its response. This validation is more flexible than
that provided by ConfigDef. During reconfiguration, we are not just
validating that the value is of the right type, we may want to verify that
you can connect to the remote web service for example. This validation path
for custom configs is already supported.
3) *Reconfiguration without config change*: This is supported specifically
for two configs `ssl.keystore.location` and `ssl.truststore.location`.
SslFactory is given the opportunity to reconfigure whenever there is an
AlterConfig request from AdminClient since it uses these configs. This is
regardless of whether there is a config change. SslFactory will invoke
MySslEngineFactory.shouldBeRebuilt(). This gives MySslEngineFactory the
opportunity to provide a new SslEngine whenever there is an AdminClient
request to alter configs, regardless of whether any config changed or not.
Hope that helps,
Rajini
On Thu, Sep 12, 2019 at 5:26 PM Pellerin, Clement
wrote:
> I'm confused. Can you launch a reconfiguration without a config change or
> not?
>
> If I understand the test case correctly, the design pattern to implement a
> reconfigurable extension point in Kafka is to implement the Reconfigurable
> interface. This means SslEngineFactory would be created once and mutate
> with reconfigure. There is no ConfigKey created for the custom config and
> therefore there is no validation by ConfigDef.
>
> Optionally, to expose the built-in validation, it might be worthwhile to
> consider making ConfigKey a public API and move an individual config parse
> from ConfigDef to ConfigKey. It would be more object oriented anyway.
>
> One of the use case of custom configs in SslEngineFactory is to remove the
> need to specify the keystore and truststore locations. The special handling
> to detect changes in keystore/truststore should be pushed to
> DefaultSslEngineFactory and all calls to reconfigure should reach the
> SslEngineFactory instance. Am I missing something?
>
> -Original Message-
> From: Rajini Sivaram [mailto:rajinisiva...@gmail.com]
> Sent: Thursday, September 12, 2019 12:01 PM
> To: dev
> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration
> extensible
>
> Correction on my previous email. Custom implementations can use AlterConfig
> request without a config change by including `ssl.keystore.location` or
> `ssl.truststore.location` in their `reconfigurabkeConfigs()`. This will
> trigger the same codepath as we use for keystore updates when the file has
> changed.
>
> On Thu, Sep 12, 2019 at 4:43 PM Rajini Sivaram
> wrote:
>
> > Hi Clement,
> >
> > Kafka does special handling for keystore/truststore file changes when an
> > AlterConfig request is processed, but that is not easy to extend to
> custom
> > configs. I was thinking we could just add a custom config to trigger
> > reconfiguration. For example, a config like `my.custom.keystore.version`
> that
> > is incremented when the custom implementation detects a change in
> keystore.
> > And the custom implementation would invoke admin client to update
> `my.custom.keystore.version`.
> > Kafka would do the rest to reconfigure SslFactory. And the custom
>
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
I'm confused. Can you launch a reconfiguration without a config change or not? If I understand the test case correctly, the design pattern to implement a reconfigurable extension point in Kafka is to implement the Reconfigurable interface. This means SslEngineFactory would be created once and mutate with reconfigure. There is no ConfigKey created for the custom config and therefore there is no validation by ConfigDef. Optionally, to expose the built-in validation, it might be worthwhile to consider making ConfigKey a public API and move an individual config parse from ConfigDef to ConfigKey. It would be more object oriented anyway. One of the use case of custom configs in SslEngineFactory is to remove the need to specify the keystore and truststore locations. The special handling to detect changes in keystore/truststore should be pushed to DefaultSslEngineFactory and all calls to reconfigure should reach the SslEngineFactory instance. Am I missing something? -Original Message- From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] Sent: Thursday, September 12, 2019 12:01 PM To: dev Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Correction on my previous email. Custom implementations can use AlterConfig request without a config change by including `ssl.keystore.location` or `ssl.truststore.location` in their `reconfigurabkeConfigs()`. This will trigger the same codepath as we use for keystore updates when the file has changed. On Thu, Sep 12, 2019 at 4:43 PM Rajini Sivaram wrote: > Hi Clement, > > Kafka does special handling for keystore/truststore file changes when an > AlterConfig request is processed, but that is not easy to extend to custom > configs. I was thinking we could just add a custom config to trigger > reconfiguration. For example, a config like `my.custom.keystore.version` that > is incremented when the custom implementation detects a change in keystore. > And the custom implementation would invoke admin client to update > `my.custom.keystore.version`. > Kafka would do the rest to reconfigure SslFactory. And the custom > implementation can then create the new builder. > > For an example of custom config reconfiguration, this test may be useful: > https://github.com/apache/kafka/blob/d3559f628b2ccb23a9faf531796675376ac06abb/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala#L792 > > > > On Thu, Sep 12, 2019 at 3:24 PM Pellerin, Clement < > clement_pelle...@ibi.com> wrote: > >> For the push notification, Rajini prefers if the trigger to reconfigure >> comes from an admin client. He says the admin client can reconfigure even >> though none of the config values have changed. This allows your custom impl >> to discover other conditions that have changed, for example the contents of >> the keystore. >> >> @Rajini, can you point us to an existing example of a Kafka extension >> point that implements reconfigurable custom configs. This way we could >> study it and do the same thing. Consistency is good. It would be nice if >> there was a KIP that describes that design pattern. >> >> -Original Message- >> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >> Sent: Thursday, September 12, 2019 2:24 AM >> To: dev@kafka.apache.org >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >> extensible >> >> Thanks Clement and Rajini. Let me digest what both of you said. Clearly I >> need to understand more about the configurations - dynamic, custom etc. >> >> On the 'push notification' question Clement asked, >> The way I see is simple - Kafka defines the interface for >> SslEngineFactory. >> Implementation of that interface is owned by the Kafka operator who >> customized the implementation. Let us say, my SslEngineFactoryImpl ONLY >> customizes loading of keys/certs where it knows when they are updated. How >> is Kafka going to know that? You said 'next time it loads' but if there is >> NO additional configuration that was needed by my SslEngineFactoryImpl, >> there won't be any trigger coming from Kafka to reconfigure and SslFactory >> will never re-create my SslEngineFactoryImpl code which will help Kafka >> use >> new keys/certs in the next calls. Please let me know if this makes my >> 'push >> notification' needs clearer. >> >> Thanks >> Maulin >> >> >> >> >> >> >> On Wed, Sep 11, 2019 at 2:31 PM Pellerin, Clement < >> clement_pelle...@ibi.com> >> wrote: >> >> > Indeed, this is a general problem requiring a more general solution than >> > KIP-519. I'm glad there was work done on this already. >> >
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Correction on my previous email. Custom implementations can use AlterConfig request without a config change by including `ssl.keystore.location` or `ssl.truststore.location` in their `reconfigurabkeConfigs()`. This will trigger the same codepath as we use for keystore updates when the file has changed. On Thu, Sep 12, 2019 at 4:43 PM Rajini Sivaram wrote: > Hi Clement, > > Kafka does special handling for keystore/truststore file changes when an > AlterConfig request is processed, but that is not easy to extend to custom > configs. I was thinking we could just add a custom config to trigger > reconfiguration. For example, a config like `my.custom.keystore.version` that > is incremented when the custom implementation detects a change in keystore. > And the custom implementation would invoke admin client to update > `my.custom.keystore.version`. > Kafka would do the rest to reconfigure SslFactory. And the custom > implementation can then create the new builder. > > For an example of custom config reconfiguration, this test may be useful: > https://github.com/apache/kafka/blob/d3559f628b2ccb23a9faf531796675376ac06abb/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala#L792 > > > > On Thu, Sep 12, 2019 at 3:24 PM Pellerin, Clement < > clement_pelle...@ibi.com> wrote: > >> For the push notification, Rajini prefers if the trigger to reconfigure >> comes from an admin client. He says the admin client can reconfigure even >> though none of the config values have changed. This allows your custom impl >> to discover other conditions that have changed, for example the contents of >> the keystore. >> >> @Rajini, can you point us to an existing example of a Kafka extension >> point that implements reconfigurable custom configs. This way we could >> study it and do the same thing. Consistency is good. It would be nice if >> there was a KIP that describes that design pattern. >> >> -Original Message- >> From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] >> Sent: Thursday, September 12, 2019 2:24 AM >> To: dev@kafka.apache.org >> Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration >> extensible >> >> Thanks Clement and Rajini. Let me digest what both of you said. Clearly I >> need to understand more about the configurations - dynamic, custom etc. >> >> On the 'push notification' question Clement asked, >> The way I see is simple - Kafka defines the interface for >> SslEngineFactory. >> Implementation of that interface is owned by the Kafka operator who >> customized the implementation. Let us say, my SslEngineFactoryImpl ONLY >> customizes loading of keys/certs where it knows when they are updated. How >> is Kafka going to know that? You said 'next time it loads' but if there is >> NO additional configuration that was needed by my SslEngineFactoryImpl, >> there won't be any trigger coming from Kafka to reconfigure and SslFactory >> will never re-create my SslEngineFactoryImpl code which will help Kafka >> use >> new keys/certs in the next calls. Please let me know if this makes my >> 'push >> notification' needs clearer. >> >> Thanks >> Maulin >> >> >> >> >> >> >> On Wed, Sep 11, 2019 at 2:31 PM Pellerin, Clement < >> clement_pelle...@ibi.com> >> wrote: >> >> > Indeed, this is a general problem requiring a more general solution than >> > KIP-519. I'm glad there was work done on this already. >> > >> > So config.originals() still contains unknown configs but nothing has >> been >> > validated and cast to the proper type. >> > How does validation work for an extension point that receives >> > config.originals()? Is there a public validator helper to handle this? >> > Do we need to create ConfigKeys in the ConfigDef for custom configs only >> > known to a custom SslEngineFactory implementation? >> > Do we need to declare the standard SSL configs in ConfigDef if >> SslFactory >> > needs to revalidate them anyway because it receives config.originals()? >> > I assume there is such a thing as config.originals() also for a >> > reconfigure()? >> > >> > If we implement KIP-519 and later change from config.values() to >> > config.originals(), this will affect the contract for the constructor of >> > the SslEngineFactory. We might need to add custom configs support to >> > KIP-519 or delay KIP-519 until the change to config.originals(). >> > >> > >> > -Original Message- >> > From: Rajini Sivaram
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Clement, Kafka does special handling for keystore/truststore file changes when an AlterConfig request is processed, but that is not easy to extend to custom configs. I was thinking we could just add a custom config to trigger reconfiguration. For example, a config like `my.custom.keystore.version` that is incremented when the custom implementation detects a change in keystore. And the custom implementation would invoke admin client to update `my.custom.keystore.version`. Kafka would do the rest to reconfigure SslFactory. And the custom implementation can then create the new builder. For an example of custom config reconfiguration, this test may be useful: https://github.com/apache/kafka/blob/d3559f628b2ccb23a9faf531796675376ac06abb/core/src/test/scala/integration/kafka/server/DynamicBrokerReconfigurationTest.scala#L792 On Thu, Sep 12, 2019 at 3:24 PM Pellerin, Clement wrote: > For the push notification, Rajini prefers if the trigger to reconfigure > comes from an admin client. He says the admin client can reconfigure even > though none of the config values have changed. This allows your custom impl > to discover other conditions that have changed, for example the contents of > the keystore. > > @Rajini, can you point us to an existing example of a Kafka extension > point that implements reconfigurable custom configs. This way we could > study it and do the same thing. Consistency is good. It would be nice if > there was a KIP that describes that design pattern. > > -Original Message- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Thursday, September 12, 2019 2:24 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Thanks Clement and Rajini. Let me digest what both of you said. Clearly I > need to understand more about the configurations - dynamic, custom etc. > > On the 'push notification' question Clement asked, > The way I see is simple - Kafka defines the interface for SslEngineFactory. > Implementation of that interface is owned by the Kafka operator who > customized the implementation. Let us say, my SslEngineFactoryImpl ONLY > customizes loading of keys/certs where it knows when they are updated. How > is Kafka going to know that? You said 'next time it loads' but if there is > NO additional configuration that was needed by my SslEngineFactoryImpl, > there won't be any trigger coming from Kafka to reconfigure and SslFactory > will never re-create my SslEngineFactoryImpl code which will help Kafka use > new keys/certs in the next calls. Please let me know if this makes my 'push > notification' needs clearer. > > Thanks > Maulin > > > > > > > On Wed, Sep 11, 2019 at 2:31 PM Pellerin, Clement < > clement_pelle...@ibi.com> > wrote: > > > Indeed, this is a general problem requiring a more general solution than > > KIP-519. I'm glad there was work done on this already. > > > > So config.originals() still contains unknown configs but nothing has been > > validated and cast to the proper type. > > How does validation work for an extension point that receives > > config.originals()? Is there a public validator helper to handle this? > > Do we need to create ConfigKeys in the ConfigDef for custom configs only > > known to a custom SslEngineFactory implementation? > > Do we need to declare the standard SSL configs in ConfigDef if SslFactory > > needs to revalidate them anyway because it receives config.originals()? > > I assume there is such a thing as config.originals() also for a > > reconfigure()? > > > > If we implement KIP-519 and later change from config.values() to > > config.originals(), this will affect the contract for the constructor of > > the SslEngineFactory. We might need to add custom configs support to > > KIP-519 or delay KIP-519 until the change to config.originals(). > > > > > > -Original Message- > > From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] > > Sent: Wednesday, September 11, 2019 4:25 PM > > To: dev > > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > > extensible > > > > Kafka already has the notion of custom configs. And we support > > reconfigurable custom configs for some interfaces e.g. MetricsReporter. > We > > also recently added custom reconfigurable configs for Authorizer under > > KIP-504. > > > > The issue with custom configs for SSL is described in > > https://issues.apache.org/jira/browse/KAFKA-7588. We currently don't > pass > > in custom configs to ChannelBuilders. We need to fix this, not just for > SSL > > but for other security plugins as well. So it needs to b
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
For the push notification, Rajini prefers if the trigger to reconfigure comes from an admin client. He says the admin client can reconfigure even though none of the config values have changed. This allows your custom impl to discover other conditions that have changed, for example the contents of the keystore. @Rajini, can you point us to an existing example of a Kafka extension point that implements reconfigurable custom configs. This way we could study it and do the same thing. Consistency is good. It would be nice if there was a KIP that describes that design pattern. -Original Message- From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] Sent: Thursday, September 12, 2019 2:24 AM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Thanks Clement and Rajini. Let me digest what both of you said. Clearly I need to understand more about the configurations - dynamic, custom etc. On the 'push notification' question Clement asked, The way I see is simple - Kafka defines the interface for SslEngineFactory. Implementation of that interface is owned by the Kafka operator who customized the implementation. Let us say, my SslEngineFactoryImpl ONLY customizes loading of keys/certs where it knows when they are updated. How is Kafka going to know that? You said 'next time it loads' but if there is NO additional configuration that was needed by my SslEngineFactoryImpl, there won't be any trigger coming from Kafka to reconfigure and SslFactory will never re-create my SslEngineFactoryImpl code which will help Kafka use new keys/certs in the next calls. Please let me know if this makes my 'push notification' needs clearer. Thanks Maulin On Wed, Sep 11, 2019 at 2:31 PM Pellerin, Clement wrote: > Indeed, this is a general problem requiring a more general solution than > KIP-519. I'm glad there was work done on this already. > > So config.originals() still contains unknown configs but nothing has been > validated and cast to the proper type. > How does validation work for an extension point that receives > config.originals()? Is there a public validator helper to handle this? > Do we need to create ConfigKeys in the ConfigDef for custom configs only > known to a custom SslEngineFactory implementation? > Do we need to declare the standard SSL configs in ConfigDef if SslFactory > needs to revalidate them anyway because it receives config.originals()? > I assume there is such a thing as config.originals() also for a > reconfigure()? > > If we implement KIP-519 and later change from config.values() to > config.originals(), this will affect the contract for the constructor of > the SslEngineFactory. We might need to add custom configs support to > KIP-519 or delay KIP-519 until the change to config.originals(). > > > -Original Message- > From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] > Sent: Wednesday, September 11, 2019 4:25 PM > To: dev > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Kafka already has the notion of custom configs. And we support > reconfigurable custom configs for some interfaces e.g. MetricsReporter. We > also recently added custom reconfigurable configs for Authorizer under > KIP-504. > > The issue with custom configs for SSL is described in > https://issues.apache.org/jira/browse/KAFKA-7588. We currently don't pass > in custom configs to ChannelBuilders. We need to fix this, not just for SSL > but for other security plugins as well. So it needs to be a generic > solution, not specific to KIP-519. > > Once KAFKA-7588 is fixed, the existing dynamic reconfiguration mechanism in > brokers would simply work. Dynamic configs works exactly in the same way > for custom configs as it does for other configs. The list of reconfigurable > configs is returned by the implementation class and the class gets notified > when any of those configs changes. This includes validateReconfiguration() > as well the actual reconfigure(). > > For SSL alone, we have special handling of dynamic configs to enable > reloading of keystores/truststores when the file changes, even though none > of the config values have changed. Reconfiguration is triggered by an admin > client request to alter configs. In this case, none of the actual configs > may have changed, but we check if the file has changed. This is currently > done only for the standard keystore/truststore configs. With KIP-519, I > guess we want the custom SslEngineFactory to be able to decide whether > reconfiguration is required. The simplest way to achieve this would be to > have a custom config that is updated when reconfiguration is required. I am > not sure we need a separate mechanism to trigger reconfiguration that > doesn't rely on admin clients since admi
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Thanks Clement and Rajini. Let me digest what both of you said. Clearly I need to understand more about the configurations - dynamic, custom etc. On the 'push notification' question Clement asked, The way I see is simple - Kafka defines the interface for SslEngineFactory. Implementation of that interface is owned by the Kafka operator who customized the implementation. Let us say, my SslEngineFactoryImpl ONLY customizes loading of keys/certs where it knows when they are updated. How is Kafka going to know that? You said 'next time it loads' but if there is NO additional configuration that was needed by my SslEngineFactoryImpl, there won't be any trigger coming from Kafka to reconfigure and SslFactory will never re-create my SslEngineFactoryImpl code which will help Kafka use new keys/certs in the next calls. Please let me know if this makes my 'push notification' needs clearer. Thanks Maulin On Wed, Sep 11, 2019 at 2:31 PM Pellerin, Clement wrote: > Indeed, this is a general problem requiring a more general solution than > KIP-519. I'm glad there was work done on this already. > > So config.originals() still contains unknown configs but nothing has been > validated and cast to the proper type. > How does validation work for an extension point that receives > config.originals()? Is there a public validator helper to handle this? > Do we need to create ConfigKeys in the ConfigDef for custom configs only > known to a custom SslEngineFactory implementation? > Do we need to declare the standard SSL configs in ConfigDef if SslFactory > needs to revalidate them anyway because it receives config.originals()? > I assume there is such a thing as config.originals() also for a > reconfigure()? > > If we implement KIP-519 and later change from config.values() to > config.originals(), this will affect the contract for the constructor of > the SslEngineFactory. We might need to add custom configs support to > KIP-519 or delay KIP-519 until the change to config.originals(). > > > -Original Message- > From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] > Sent: Wednesday, September 11, 2019 4:25 PM > To: dev > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Kafka already has the notion of custom configs. And we support > reconfigurable custom configs for some interfaces e.g. MetricsReporter. We > also recently added custom reconfigurable configs for Authorizer under > KIP-504. > > The issue with custom configs for SSL is described in > https://issues.apache.org/jira/browse/KAFKA-7588. We currently don't pass > in custom configs to ChannelBuilders. We need to fix this, not just for SSL > but for other security plugins as well. So it needs to be a generic > solution, not specific to KIP-519. > > Once KAFKA-7588 is fixed, the existing dynamic reconfiguration mechanism in > brokers would simply work. Dynamic configs works exactly in the same way > for custom configs as it does for other configs. The list of reconfigurable > configs is returned by the implementation class and the class gets notified > when any of those configs changes. This includes validateReconfiguration() > as well the actual reconfigure(). > > For SSL alone, we have special handling of dynamic configs to enable > reloading of keystores/truststores when the file changes, even though none > of the config values have changed. Reconfiguration is triggered by an admin > client request to alter configs. In this case, none of the actual configs > may have changed, but we check if the file has changed. This is currently > done only for the standard keystore/truststore configs. With KIP-519, I > guess we want the custom SslEngineFactory to be able to decide whether > reconfiguration is required. The simplest way to achieve this would be to > have a custom config that is updated when reconfiguration is required. I am > not sure we need a separate mechanism to trigger reconfiguration that > doesn't rely on admin clients since admin clients provide useful logging > and auditability. > > Regards, > > Rajini > > On Wed, Sep 11, 2019 at 4:13 PM Pellerin, Clement < > clement_pelle...@ibi.com> > wrote: > > > I'm sorry if I divert the discussion, but without this issue, it would > > have been pretty trivial to update KIP-383 to go as far as you did. I am > > also happy to get a discussion going, the KIP-383 thread was a desolate > > place. > > > > Kafka needs to know about custom configs because it validates the configs > > before it passes them to (re)configure. Unknown configs are silently > > removed by ConfigDef. We could keep unknown configs as strings without > > validating them in ConfigDef, but I don't know if the Kafka community > would > >
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Indeed, this is a general problem requiring a more general solution than KIP-519. I'm glad there was work done on this already. So config.originals() still contains unknown configs but nothing has been validated and cast to the proper type. How does validation work for an extension point that receives config.originals()? Is there a public validator helper to handle this? Do we need to create ConfigKeys in the ConfigDef for custom configs only known to a custom SslEngineFactory implementation? Do we need to declare the standard SSL configs in ConfigDef if SslFactory needs to revalidate them anyway because it receives config.originals()? I assume there is such a thing as config.originals() also for a reconfigure()? If we implement KIP-519 and later change from config.values() to config.originals(), this will affect the contract for the constructor of the SslEngineFactory. We might need to add custom configs support to KIP-519 or delay KIP-519 until the change to config.originals(). -Original Message- From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] Sent: Wednesday, September 11, 2019 4:25 PM To: dev Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Kafka already has the notion of custom configs. And we support reconfigurable custom configs for some interfaces e.g. MetricsReporter. We also recently added custom reconfigurable configs for Authorizer under KIP-504. The issue with custom configs for SSL is described in https://issues.apache.org/jira/browse/KAFKA-7588. We currently don't pass in custom configs to ChannelBuilders. We need to fix this, not just for SSL but for other security plugins as well. So it needs to be a generic solution, not specific to KIP-519. Once KAFKA-7588 is fixed, the existing dynamic reconfiguration mechanism in brokers would simply work. Dynamic configs works exactly in the same way for custom configs as it does for other configs. The list of reconfigurable configs is returned by the implementation class and the class gets notified when any of those configs changes. This includes validateReconfiguration() as well the actual reconfigure(). For SSL alone, we have special handling of dynamic configs to enable reloading of keystores/truststores when the file changes, even though none of the config values have changed. Reconfiguration is triggered by an admin client request to alter configs. In this case, none of the actual configs may have changed, but we check if the file has changed. This is currently done only for the standard keystore/truststore configs. With KIP-519, I guess we want the custom SslEngineFactory to be able to decide whether reconfiguration is required. The simplest way to achieve this would be to have a custom config that is updated when reconfiguration is required. I am not sure we need a separate mechanism to trigger reconfiguration that doesn't rely on admin clients since admin clients provide useful logging and auditability. Regards, Rajini On Wed, Sep 11, 2019 at 4:13 PM Pellerin, Clement wrote: > I'm sorry if I divert the discussion, but without this issue, it would > have been pretty trivial to update KIP-383 to go as far as you did. I am > also happy to get a discussion going, the KIP-383 thread was a desolate > place. > > Kafka needs to know about custom configs because it validates the configs > before it passes them to (re)configure. Unknown configs are silently > removed by ConfigDef. We could keep unknown configs as strings without > validating them in ConfigDef, but I don't know if the Kafka community would > accept this weaker validation. > > It appears we are trying to invent the notion of a meta config. Anyway, I > think we have shown asking an instance of SslEngineFactory to contribute to > ConfigDef is way too late. > > For your push notification, would it be simpler to just let your > SslEngineFactory notice the change and make it effective the next time it > is called. SslFactory would not cache the SslEngine and always ask > SslEngineFactory for it. You don't even need an inner thread if > SslEngineFactory checks for a change when it is called. SslEngineFactory > would no longer be immutable, so maybe it is worth reconsidering how > reconfigure works for it. > > -Original Message- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Wednesday, September 11, 2019 3:29 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Hi all, > > Since the "custom config" seems the main topic of interest let us talk > about it. > > 1. I want to confirm that I interpret the definition of 'custom config of > SslEngineFactory' the same way Clement is suggesting - "a config that does > not exist in Kafka but is specified by a custom implementation of > SslEngineFactory". If
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Kafka already has the notion of custom configs. And we support reconfigurable custom configs for some interfaces e.g. MetricsReporter. We also recently added custom reconfigurable configs for Authorizer under KIP-504. The issue with custom configs for SSL is described in https://issues.apache.org/jira/browse/KAFKA-7588. We currently don't pass in custom configs to ChannelBuilders. We need to fix this, not just for SSL but for other security plugins as well. So it needs to be a generic solution, not specific to KIP-519. Once KAFKA-7588 is fixed, the existing dynamic reconfiguration mechanism in brokers would simply work. Dynamic configs works exactly in the same way for custom configs as it does for other configs. The list of reconfigurable configs is returned by the implementation class and the class gets notified when any of those configs changes. This includes validateReconfiguration() as well the actual reconfigure(). For SSL alone, we have special handling of dynamic configs to enable reloading of keystores/truststores when the file changes, even though none of the config values have changed. Reconfiguration is triggered by an admin client request to alter configs. In this case, none of the actual configs may have changed, but we check if the file has changed. This is currently done only for the standard keystore/truststore configs. With KIP-519, I guess we want the custom SslEngineFactory to be able to decide whether reconfiguration is required. The simplest way to achieve this would be to have a custom config that is updated when reconfiguration is required. I am not sure we need a separate mechanism to trigger reconfiguration that doesn't rely on admin clients since admin clients provide useful logging and auditability. Regards, Rajini On Wed, Sep 11, 2019 at 4:13 PM Pellerin, Clement wrote: > I'm sorry if I divert the discussion, but without this issue, it would > have been pretty trivial to update KIP-383 to go as far as you did. I am > also happy to get a discussion going, the KIP-383 thread was a desolate > place. > > Kafka needs to know about custom configs because it validates the configs > before it passes them to (re)configure. Unknown configs are silently > removed by ConfigDef. We could keep unknown configs as strings without > validating them in ConfigDef, but I don't know if the Kafka community would > accept this weaker validation. > > It appears we are trying to invent the notion of a meta config. Anyway, I > think we have shown asking an instance of SslEngineFactory to contribute to > ConfigDef is way too late. > > For your push notification, would it be simpler to just let your > SslEngineFactory notice the change and make it effective the next time it > is called. SslFactory would not cache the SslEngine and always ask > SslEngineFactory for it. You don't even need an inner thread if > SslEngineFactory checks for a change when it is called. SslEngineFactory > would no longer be immutable, so maybe it is worth reconsidering how > reconfigure works for it. > > -Original Message- > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > Sent: Wednesday, September 11, 2019 3:29 AM > To: dev@kafka.apache.org > Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration > extensible > > Hi all, > > Since the "custom config" seems the main topic of interest let us talk > about it. > > 1. I want to confirm that I interpret the definition of 'custom config of > SslEngineFactory' the same way Clement is suggesting - "a config that does > not exist in Kafka but is specified by a custom implementation of > SslEngineFactory". If there is a disagreement to that we have to bring it > up here sooner. > > 2. I've been thinking about it and I question why we are trying to make a > custom config a first class citizen in standard config? > The reasoning for that question is- > Kafka wants to delegate creation of SSLEngine to a class which is "not" > part of Kafka's distribution. Since the interface for SSLEngine creator > will be defined by the public method of createSSLEngine(), why would Kafka > care what does the implementation do other than fulfilling the contract of > creation of SSLEngine. The implementation can use any special configs i.e. > configs coming from input Map OR configs defined in a new file only known > to itself. Making the configs part of the interface contract in any way is > not necessary. This way we keep it simple and straightforward. > > 3. Now, 2nd point raises a question - if we follow that suggestion - how > can we ever re-create the SSLEngineFactory object and allow new object to > be created when something changes in the implementation. That is a valid > question. If you noticed in the KIP section titled "Other challenge" - we &g
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
I'm sorry if I divert the discussion, but without this issue, it would have been pretty trivial to update KIP-383 to go as far as you did. I am also happy to get a discussion going, the KIP-383 thread was a desolate place. Kafka needs to know about custom configs because it validates the configs before it passes them to (re)configure. Unknown configs are silently removed by ConfigDef. We could keep unknown configs as strings without validating them in ConfigDef, but I don't know if the Kafka community would accept this weaker validation. It appears we are trying to invent the notion of a meta config. Anyway, I think we have shown asking an instance of SslEngineFactory to contribute to ConfigDef is way too late. For your push notification, would it be simpler to just let your SslEngineFactory notice the change and make it effective the next time it is called. SslFactory would not cache the SslEngine and always ask SslEngineFactory for it. You don't even need an inner thread if SslEngineFactory checks for a change when it is called. SslEngineFactory would no longer be immutable, so maybe it is worth reconsidering how reconfigure works for it. -Original Message- From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] Sent: Wednesday, September 11, 2019 3:29 AM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Hi all, Since the "custom config" seems the main topic of interest let us talk about it. 1. I want to confirm that I interpret the definition of 'custom config of SslEngineFactory' the same way Clement is suggesting - "a config that does not exist in Kafka but is specified by a custom implementation of SslEngineFactory". If there is a disagreement to that we have to bring it up here sooner. 2. I've been thinking about it and I question why we are trying to make a custom config a first class citizen in standard config? The reasoning for that question is- Kafka wants to delegate creation of SSLEngine to a class which is "not" part of Kafka's distribution. Since the interface for SSLEngine creator will be defined by the public method of createSSLEngine(), why would Kafka care what does the implementation do other than fulfilling the contract of creation of SSLEngine. The implementation can use any special configs i.e. configs coming from input Map OR configs defined in a new file only known to itself. Making the configs part of the interface contract in any way is not necessary. This way we keep it simple and straightforward. 3. Now, 2nd point raises a question - if we follow that suggestion - how can we ever re-create the SSLEngineFactory object and allow new object to be created when something changes in the implementation. That is a valid question. If you noticed in the KIP section titled "Other challenge" - we do have scenario where the SslEngineFactory implementation ONLY knows that something changed - example: keystore got updated by a local daemon process only known to the specific implementation. This means we have a need of "push notification" from the SslEngineFactory's implementation to the SslFactory actually. I feel if we build the "push notification" via adding a method in the interface as "public void registerReconfigurableListener(Reconfigurable r)" and make SslFactory register itself with the SslEngineFactory's impl class, we can trigger the reconfiguration of SslEngineFactory implementation based on its own terms and conditions without getting into making custom configs complicated. I am just thinking out loud here and expressing my opinion not to avoid addressing custom configs BUT what I genuinely believe might be a better approach. Thanks Maulin On Tue, Sep 10, 2019 at 9:06 PM Pellerin, Clement wrote: > Regarding what I labeled the simplest solution below, SslConfigs could > instantiate the custom interface only if the yet to be validated configs > were passed in to the call to get the list of known SSL configs. > > -Original Message- > From: Pellerin, Clement > Sent: Tuesday, September 10, 2019 11:36 AM > To: dev@kafka.apache.org > Subject: [EXTERNAL]RE: [DISCUSS] KIP-519: Make SSL context/engine > configuration extensible > > Another solution could be a new standard ssl config that holds a list of > extra custom configs to accept. > Using a custom SslEngineFactory with custom configs would require setting > two configs, one for the class name and another for the list of custom > configs. > In SslConfigs, we see that declaring a single config takes 5 values, so > I'm not sure how it would work exactly. > > We could also declare a new interface to return the list of custom ssl > configs and the extra standard ssl config I'm proposing holds the name of > the implementation class instead. The reason a different interface
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi all, Since the "custom config" seems the main topic of interest let us talk about it. 1. I want to confirm that I interpret the definition of 'custom config of SslEngineFactory' the same way Clement is suggesting - "a config that does not exist in Kafka but is specified by a custom implementation of SslEngineFactory". If there is a disagreement to that we have to bring it up here sooner. 2. I've been thinking about it and I question why we are trying to make a custom config a first class citizen in standard config? The reasoning for that question is- Kafka wants to delegate creation of SSLEngine to a class which is "not" part of Kafka's distribution. Since the interface for SSLEngine creator will be defined by the public method of createSSLEngine(), why would Kafka care what does the implementation do other than fulfilling the contract of creation of SSLEngine. The implementation can use any special configs i.e. configs coming from input Map OR configs defined in a new file only known to itself. Making the configs part of the interface contract in any way is not necessary. This way we keep it simple and straightforward. 3. Now, 2nd point raises a question - if we follow that suggestion - how can we ever re-create the SSLEngineFactory object and allow new object to be created when something changes in the implementation. That is a valid question. If you noticed in the KIP section titled "Other challenge" - we do have scenario where the SslEngineFactory implementation ONLY knows that something changed - example: keystore got updated by a local daemon process only known to the specific implementation. This means we have a need of "push notification" from the SslEngineFactory's implementation to the SslFactory actually. I feel if we build the "push notification" via adding a method in the interface as "public void registerReconfigurableListener(Reconfigurable r)" and make SslFactory register itself with the SslEngineFactory's impl class, we can trigger the reconfiguration of SslEngineFactory implementation based on its own terms and conditions without getting into making custom configs complicated. I am just thinking out loud here and expressing my opinion not to avoid addressing custom configs BUT what I genuinely believe might be a better approach. Thanks Maulin On Tue, Sep 10, 2019 at 9:06 PM Pellerin, Clement wrote: > Regarding what I labeled the simplest solution below, SslConfigs could > instantiate the custom interface only if the yet to be validated configs > were passed in to the call to get the list of known SSL configs. > > -Original Message- > From: Pellerin, Clement > Sent: Tuesday, September 10, 2019 11:36 AM > To: dev@kafka.apache.org > Subject: [EXTERNAL]RE: [DISCUSS] KIP-519: Make SSL context/engine > configuration extensible > > Another solution could be a new standard ssl config that holds a list of > extra custom configs to accept. > Using a custom SslEngineFactory with custom configs would require setting > two configs, one for the class name and another for the list of custom > configs. > In SslConfigs, we see that declaring a single config takes 5 values, so > I'm not sure how it would work exactly. > > We could also declare a new interface to return the list of custom ssl > configs and the extra standard ssl config I'm proposing holds the name of > the implementation class instead. The reason a different interface is > needed is because it would be instantiated by SslConfigs, not SslFactory. > This seems the simplest solution. > > Anyway, the point of this exercise is to prove an acceptable solution for > custom configs is not affecting the public API in KIP-519. > > > -----Original Message----- > From: Pellerin, Clement > Sent: Tuesday, September 10, 2019 9:35 AM > To: dev@kafka.apache.org > Subject: [EXTERNAL]RE: [DISCUSS] KIP-519: Make SSL context/engine > configuration extensible > > Custom config is a term I invented to mean a config that does not exist in > Kafka but is specified by a custom implementation of SslEngineFactory. > The problem with custom configs (as I remember it) is that the list of > configs is static in SslConfigs and config names are checked before > SslFactory is created. > ==> You must solve this problem because that is what killed KIP-383 and > therefore is the sole reason why KIP-519 exists. > ==> Your KIP does not have to implement the solution since it can be done > in a future KIP, but your KIP must be compatible with the solution found. > ==> A method to return the list of configs would help. This cannot be a > static method in the interface since it cannot be overridden. > ==> You could require a static method in the implementation class by > convention, just like the constructor you require. > > This email did not originate from inside Information Builders. >
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Regarding what I labeled the simplest solution below, SslConfigs could instantiate the custom interface only if the yet to be validated configs were passed in to the call to get the list of known SSL configs. -Original Message- From: Pellerin, Clement Sent: Tuesday, September 10, 2019 11:36 AM To: dev@kafka.apache.org Subject: [EXTERNAL]RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Another solution could be a new standard ssl config that holds a list of extra custom configs to accept. Using a custom SslEngineFactory with custom configs would require setting two configs, one for the class name and another for the list of custom configs. In SslConfigs, we see that declaring a single config takes 5 values, so I'm not sure how it would work exactly. We could also declare a new interface to return the list of custom ssl configs and the extra standard ssl config I'm proposing holds the name of the implementation class instead. The reason a different interface is needed is because it would be instantiated by SslConfigs, not SslFactory. This seems the simplest solution. Anyway, the point of this exercise is to prove an acceptable solution for custom configs is not affecting the public API in KIP-519. -Original Message- From: Pellerin, Clement Sent: Tuesday, September 10, 2019 9:35 AM To: dev@kafka.apache.org Subject: [EXTERNAL]RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Custom config is a term I invented to mean a config that does not exist in Kafka but is specified by a custom implementation of SslEngineFactory. The problem with custom configs (as I remember it) is that the list of configs is static in SslConfigs and config names are checked before SslFactory is created. ==> You must solve this problem because that is what killed KIP-383 and therefore is the sole reason why KIP-519 exists. ==> Your KIP does not have to implement the solution since it can be done in a future KIP, but your KIP must be compatible with the solution found. ==> A method to return the list of configs would help. This cannot be a static method in the interface since it cannot be overridden. ==> You could require a static method in the implementation class by convention, just like the constructor you require. This email did not originate from inside Information Builders.
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Another solution could be a new standard ssl config that holds a list of extra custom configs to accept. Using a custom SslEngineFactory with custom configs would require setting two configs, one for the class name and another for the list of custom configs. In SslConfigs, we see that declaring a single config takes 5 values, so I'm not sure how it would work exactly. We could also declare a new interface to return the list of custom ssl configs and the extra standard ssl config I'm proposing holds the name of the implementation class instead. The reason a different interface is needed is because it would be instantiated by SslConfigs, not SslFactory. This seems the simplest solution. Anyway, the point of this exercise is to prove an acceptable solution for custom configs is not affecting the public API in KIP-519. -Original Message- From: Pellerin, Clement Sent: Tuesday, September 10, 2019 9:35 AM To: dev@kafka.apache.org Subject: [EXTERNAL]RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Custom config is a term I invented to mean a config that does not exist in Kafka but is specified by a custom implementation of SslEngineFactory. The problem with custom configs (as I remember it) is that the list of configs is static in SslConfigs and config names are checked before SslFactory is created. ==> You must solve this problem because that is what killed KIP-383 and therefore is the sole reason why KIP-519 exists. ==> Your KIP does not have to implement the solution since it can be done in a future KIP, but your KIP must be compatible with the solution found. ==> A method to return the list of configs would help. This cannot be a static method in the interface since it cannot be overridden. ==> You could require a static method in the implementation class by convention, just like the constructor you require.
RE: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Some of these items are copied from my email in the KIP-416 discussion. Custom config is a term I invented to mean a config that does not exist in Kafka but is specified by a custom implementation of SslEngineFactory. The problem with custom configs (as I remember it) is that the list of configs is static in SslConfigs and config names are checked before SslFactory is created. ==> You must solve this problem because that is what killed KIP-383 and therefore is the sole reason why KIP-519 exists. ==> Your KIP does not have to implement the solution since it can be done in a future KIP, but your KIP must be compatible with the solution found. ==> A method to return the list of configs would help. This cannot be a static method in the interface since it cannot be overridden. ==> You could require a static method in the implementation class by convention, just like the constructor you require. Please change it to say it takes the class name >> ssl.engine.factory.class - This configuration will take class of the below >> interface's type and will be used to create javax.net.ssl.SSLEngine object. I suggest to add an underscore to correspond to the dots and adjust the second constant name accordingly >> final static String SSL_ENGINEFACTORY_CLASS_CONFIG = >> "ssl.engine.factory.class"; >> final static String DEFAULT_SSL_ENGINEFACTORY_CLASS = >> "org.apache.kafka.common.security.ssl.DefaultSslEngineFactory"; Please specify in the KIP if the new config is reconfigurable or not. I agree your factory needs to create instances of SslEngine because of the client/server mode as used in Kafka. I don't understand why we need the SslContext in that factory. It is not even used by SslFactory. Last time I checked, it was only used by a junit which is not enough reason to pollute a public API. I was told SslFactory is a private API, so it is OK to remove sslContext() even in a minor release. The KIP must mention this change. Your time diagrams never call shouldBeRebuilt() I am not a fan of the method name shouldBeRebuilt()/shouldBeRebuiltFor() This was OK in a private implementation before but this will become a public API. Maybe the reason why this name does not fit the rest of Kafka's reconfiguration API is because your interface is replacing reconfigure with a call to the constructor but it is not a stated goal to keep the interface implementation immutable. This deserves some thought since I don't know what would be better yet. I would not document SslEngineFactoryInstantiator in the KIP since it is private to SslFactory. In fact, my preference would be to use a private method in SslFactory instead. In your Rejected Alternatives, you must justify why KIP-519 exists by explaining why KIP-383 should be rejected. I would prefer if your KIP was self-contained and did not refer to sample code when explaining something. Of course, you can link to your sample code once to say it exists. When I wrote KIP-383, I felt I needed a prototype before I could solidify the proposal. That's part of the reason why there was never a third iteration. -Original Message- From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] Sent: Tuesday, September 10, 2019 2:26 AM To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible Hi Clement/Rajini/Colin Please review our latest updates on the KIP and let me know your thoughts. Clement, please let me know if my understanding about the "custom configs" is correct based on what I wrote in the KIP. Thanks Maulin On Mon, Sep 9, 2019 at 3:28 PM Maulin Vasavada wrote: > Hi all > > Based on longer discussion on another KIP-486 we are opening this KIP-519 ( > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=128650952 > ). > > Please help us review this and provide your suggestions. > > Thanks > Maulin >
Re: [DISCUSS] KIP-519: Make SSL context/engine configuration extensible
Hi Clement/Rajini/Colin Please review our latest updates on the KIP and let me know your thoughts. Clement, please let me know if my understanding about the "custom configs" is correct based on what I wrote in the KIP. Thanks Maulin On Mon, Sep 9, 2019 at 3:28 PM Maulin Vasavada wrote: > Hi all > > Based on longer discussion on another KIP-486 we are opening this KIP-519 ( > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=128650952 > ). > > Please help us review this and provide your suggestions. > > Thanks > Maulin >
