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 <maulin.vasav...@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.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: > >>>>>> 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 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 > >>>>>> >> > >>>>>> >> > >>>>>> > >>>>> >
