You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Rajini Sivaram <ra...@gmail.com> on 2020/02/05 13:50:55 UTC
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 <ma...@gmail.com>
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.vasavada@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.vasavada@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.vasavada@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.vasavada@gmail.com> wrote:
> >>>
> >>>> bump!
> >>>>
> >>>> On Sun, Oct 13, 2019 at 11:16 PM Maulin Vasavada <
> >>>> maulin.vasavada@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_Pellerin@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.vasavada@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.vasavada@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_Pellerin@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 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.vasavada@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
Posted by Maulin Vasavada <ma...@gmail.com>.
Hi all,
I resolved the challenge with having Class Type for the
ssl.engine.factory.class configuration. Raised pull request to the trunk -
https://github.com/apache/kafka/pull/8338. Updated on the JIRA -
https://issues.apache.org/jira/browse/KAFKA-8890
Please review & Vote!
Thanks
Maulin
On Mon, Mar 23, 2020 at 6:57 PM Maulin Vasavada <ma...@gmail.com>
wrote:
> 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 <ma...@gmail.com>
> wrote:
>
>> still working on the pull request. hopefully will be done soon.
>>
>> On Wed, Mar 11, 2020 at 11:48 AM Maulin Vasavada <
>> maulin.vasavada@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 <ra...@gmail.com>
>>> 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.vasavada@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.vasavada@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_Pellerin@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:rajinisivaram@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<String>
>>>> > >> reconfigurableConfigs()`
>>>> > >>
>>>> > >> On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram <
>>>> rajinisivaram@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
Posted by Maulin Vasavada <ma...@gmail.com>.
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 <ma...@gmail.com>
wrote:
> still working on the pull request. hopefully will be done soon.
>
> On Wed, Mar 11, 2020 at 11:48 AM Maulin Vasavada <
> maulin.vasavada@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 <ra...@gmail.com>
>> 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.vasavada@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.vasavada@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_Pellerin@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:rajinisivaram@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<String>
>>> > >> reconfigurableConfigs()`
>>> > >>
>>> > >> On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram <
>>> rajinisivaram@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
Posted by Maulin Vasavada <ma...@gmail.com>.
still working on the pull request. hopefully will be done soon.
On Wed, Mar 11, 2020 at 11:48 AM Maulin Vasavada <ma...@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 <ra...@gmail.com>
> 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.vasavada@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.vasavada@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_Pellerin@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:rajinisivaram@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<String>
>> > >> reconfigurableConfigs()`
>> > >>
>> > >> On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram <
>> rajinisivaram@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
Posted by Maulin Vasavada <ma...@gmail.com>.
Thanks Rajini. Sounds good. I'll make changes and update this thread.
On Wed, Mar 11, 2020 at 6:41 AM Rajini Sivaram <ra...@gmail.com>
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.vasavada@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.vasavada@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_Pellerin@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:rajinisivaram@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<String>
> > >> reconfigurableConfigs()`
> > >>
> > >> On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram <
> rajinisivaram@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
Posted by Rajini Sivaram <ra...@gmail.com>.
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 <ma...@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 <ma...@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_Pellerin@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:rajinisivaram@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<String>
> >> reconfigurableConfigs()`
> >>
> >> On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram <ra...@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
Posted by Maulin Vasavada <ma...@gmail.com>.
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 <ma...@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 <Cl...@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:rajinisivaram@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<String>
>> reconfigurableConfigs()`
>>
>> On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram <ra...@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
Posted by Maulin Vasavada <ma...@gmail.com>.
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 <Cl...@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:rajinisivaram@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<String>
> reconfigurableConfigs()`
>
> On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram <ra...@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
Posted by "Pellerin, Clement" <Cl...@ibi.com>.
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:rajinisivaram@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<String>
reconfigurableConfigs()`
On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram <ra...@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
Posted by Rajini Sivaram <ra...@gmail.com>.
One more point:
5) We should also add a method to SslEngineFactory that returns `Set<String>
reconfigurableConfigs()`
On Wed, Feb 5, 2020 at 1:50 PM Rajini Sivaram <ra...@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.
>
>
> On Mon, Jan 27, 2020 at 6:59 AM Maulin Vasavada <ma...@gmail.com>
> 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.vasavada@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.vasavada@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.vasavada@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.vasavada@gmail.com> wrote:
>> >>>
>> >>>> bump!
>> >>>>
>> >>>> On Sun, Oct 13, 2019 at 11:16 PM Maulin Vasavada <
>> >>>> maulin.vasavada@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_Pellerin@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.vasavada@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.vasavada@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_Pellerin@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 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.vasavada@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
>> >>>>>> >>
>> >>>>>> >>
>> >>>>>>
>> >>>>>
>>
>