Hi Colin Thanks for your comments. I agree with most of them. This is not a pull-request ready code yet :)
If we want to make DefaultSslEngineBuilder final then what do you propose to address our requirements to be able to plugin custom way for loading keys/certs? (The original challenge we have documented with this KIP). What I gather from your comment on that is - we will have to standardize those requirements as public APIs like interfaces documented in the KIP-486 for KeyStoreLoader/TrustStoreLoader, correct? We have those pluggable APIs to provide custom source for keys/certs and have SslEngineBuilder Interface somehow take those? On Wed, Sep 4, 2019 at 10:58 AM Colin McCabe <cmcc...@apache.org> wrote: > On Tue, Sep 3, 2019, at 22:56, Maulin Vasavada wrote: > > Hi all > > > > Please check > > > https://github.com/maulin-vasavada/kafka/commit/44f86395b1ba3fe4bd87de89029d72da77995ff8 > > > > > > This is just the first cut obviously. There are few call outs I would > like > > to make, > > > > 1. So far I kept the old SslEngineBuilder hence I had to name the > interface > > with "I" (that can change later) > > Hi Maulin, > > Thanks for working on this. > > We don't use Hungarian notation in Kafka. The interface should probably > just be SslEngineBuilder. The default implementation can be > DefaultSslEngineBuilder. > > > > > 2. I did not yet add the creation of SslEngineBuilder via loading the > > configuration like 'ssl.engine.builder.class'. Hence you see direct > > creation of DefaultSslEngineBuilder class > > > > 3. Due to validation logic in the current SslFactory I had to add more > > methods in ISslEngineBuilder interface (like keystore(), truststore() > etc). > > Due to other classes like EchoServer depending upon needing SSLContext, I > > had to add getSSLContext() also in the interface. > > Hmm. I don't think we want to expose this stuff. EchoServer is just used > for testing, so it can cast the SslEngineBuilder to DefaultEngineBuilder > (the only one that it will use during JUnit tests) and get what it needs > that way. > > > > > 4. With these changes and with existing old SslEngineBuilder, the > > clients/core projects builds with tests successfully but I didn't add any > > additional tests yet > > > > 5. I wanted to have DefaultSslEngineBuilder in such a way that if > somebody > > wants to implement custom SslEngineBuilder they can extend and override > > only key required methods without repeating any logic. > > No, DefaultSslEngineBuilder should be final. We should not allow people > to extend the default engine builder, since then it becomes a public API. > If there are utility functions that we feel would be useful to everyone, we > can spell those out explicitly and standardize them as public APIs that > can't be changed. > > > > > 6. For reconfigurable interface I kept the way suggested by Rajini - > > meaning SslFactory really is reconfigurable BUT it relies on the > > ISslEngineBuilder to define the reconfigurable options. This means that > > ISslEngineBuilder dictates based on which reconfigurable params the > > SslFactory should try to reconfigure the SSLEngine. > > Each SslEngineBuilder should define its own set of reconfigurable > configurations. We don't know ahead of time what they will need. We want > to be flexible. People might want to fetch certificates from a central > location via HTTPS, for example. Or maybe they want to use a native > library of some kind. And so on. > > best, > Colin > > > > > With this - open to all the suggestions and further improvements. > > > > Thanks > > Maulin > > > > > > On Tue, Sep 3, 2019 at 10:07 AM Colin McCabe <cmcc...@apache.org> wrote: > > > > > On Mon, Sep 2, 2019, at 03:33, Rajini Sivaram wrote: > > > > I would expect SslEngineBuilder interface to look something like > this, > > > > perhaps with some tweaking: > > > > > > > > public interface SslEngineBuilder extends Configurable, Closeable { > > > > > > > > Set<String> reconfigurableConfigs(); > > > > > > > > boolean shouldBeRebuilt(Map<String, Object> nextConfigs); > > > > > > > > SSLEngine createSslEngine(Mode mode, String peerHost, int > > > > peerPort, String endpointIdentification); > > > > > > > > } > > > > > > > > The existing SslEngineBuilder class would be renamed and will > implement > > > > this interface. Loading of keystore/truststore will be in > > > SslEngineBuilder > > > > as it is now. The method `shouldBeRebuilt()` will validate configs > > > during > > > > reconfiguration and decide if reconfiguration is required because > > > keystore > > > > or truststore changed. SslFactory.reconfigurableConfigs() will return > > > > SslEngineBuilder.reconfigurableConfigs() as well including any custom > > > > configs of SslEngineBuilder, so no other changes will be required > when we > > > > eventually support custom SSL configs. > > > > > > > > We don't want to make SslFactory the pluggable class since that > contains > > > > validation logic for SSL engines. Everything that we want to > customise is > > > > contained in SslEngineBuilder. Basically custom SslEngineBuilder will > > > > validate custom configs during reconfiguration and create SSLEngine. > > > > SslFactory will continue to perform validation of SSLEngines and this > > > will > > > > not be customizable. SslEngineBuilder will not be reconfigurable, > instead > > > > we create a new builder as we do now to avoid having to deal with > > > > thread-safety and atomicity of updates. We could consider using a > public > > > > Reconfigurable interface as the pluggable interface for consistency, > but > > > I > > > > think we would still want to create a new Builder on reconfiguration > and > > > > retain non-pluggable SSL engine validation in SslFactory. > > > > > > +1 > > > > > > C. > > > > > > > > > > > > > > > On Fri, Aug 30, 2019 at 10:21 PM Maulin Vasavada < > > > maulin.vasav...@gmail.com> > > > > wrote: > > > > > > > > > Looking at SslFactory and SslEngineBuilder I feel the > responsibilities > > > are > > > > > not clear. Both has public method for createSSLEngine for example. > I > > > feel > > > > > the SslEngineBuilder was created to move out lot of code but it is > not > > > > > necessarily a public class (e.g. I don't think anybody calling > > > > > SslEngineBuilder separately without SslFactory in between). I am > > > currently > > > > > inclined toward what Celement is suggesting - having pluggable > > > SslFactory. > > > > > > > > > > Let me do this - let me refactor SslFactory and SslEngineBuilder > and > > > review > > > > > what I can come up with you guys. Let us see if we can address all > the > > > > > objections raised previously for KIP-383's iterations. I'll need > > > sometime > > > > > though. Let me try to do it by next of next week. > > > > > > > > > > Thanks > > > > > Maulin > > > > > > > > > > On Fri, Aug 30, 2019 at 12:25 PM Pellerin, Clement < > > > > > clement_pelle...@ibi.com> > > > > > wrote: > > > > > > > > > > > What is your solution to the objection that killed the second > > > iteration > > > > > of > > > > > > KIP-383? > > > > > > Mainly, how do you support validation of reconfiguration requests > > > that > > > > > > involve new custom properties implemented by the pluggable > factory? > > > > > > Custom properties do not exist yet, but that is very legitimate > > > thing to > > > > > > design for the future. > > > > > > > > > > > > That's why I favor a pluggable SslFactory instead of an > > > SslEngineBuilder > > > > > > factory. > > > > > > > > > > > > -----Original Message----- > > > > > > From: Maulin Vasavada [mailto:maulin.vasav...@gmail.com] > > > > > > Sent: Friday, August 30, 2019 3:07 PM > > > > > > To: dev@kafka.apache.org > > > > > > Subject: Re: [DISCUSS] KIP-486 Support for pluggable KeyStore and > > > > > > TrustStore > > > > > > > > > > > > +1 for making SslEngineBuilder configurable upon more thoughts. > > > > > > > > > > > > However, in the abstraction and default implementation we should > make > > > > > sure > > > > > > when we do have a requirement to plugin custom key/trust store > people > > > > > don't > > > > > > have to write lot more code which may not be related to it. > > > > > > > > > > > > Having said that, does this mean, we resurrect KIP-383 and > update it > > > with > > > > > > latest context and go from there? > > > > > > > > > > > > We are willing to take up that work for making it configurable. > > > > > > > > > > > > Thanks > > > > > > Maulin > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Fri, Aug 30, 2019 at 10:34 AM Maulin Vasavada < > > > > > > maulin.vasav...@gmail.com> > > > > > > wrote: > > > > > > > > > > > > > Why don't we make SSLEngineBuilder code delegate the whole > > > Key/Trust > > > > > > store > > > > > > > initialization to the interfaces we are proposing? Default > > > > > implementation > > > > > > > for those key/trust store loader interfaces will be "file > based" > > > > > loading > > > > > > vs > > > > > > > if somebody wants to customize any of it they can. > > > > > > > > > > > > > > Would that make sense? > > > > > > > > > > > > > > On Fri, Aug 30, 2019 at 10:03 AM Colin McCabe < > cmcc...@apache.org> > > > > > > wrote: > > > > > > > > > > > > > >> +1 for making SslEngineBuilder configurable. This would give > > > > > > >> implementers a lot more flexibility-- to use key distribution > > > methods > > > > > > that > > > > > > >> were not files, for example. > > > > > > >> > > > > > > >> best, > > > > > > >> Colin > > > > > > >> > > > > > > >> > > > > > > >> On Fri, Aug 30, 2019, at 02:03, Rajini Sivaram wrote: > > > > > > >> > Just to make sure we are on the same page - KIP-383 was > written > > > > > before > > > > > > >> > the > > > > > > >> > code was refactored. The refactoring addressed some of the > > > concerns > > > > > of > > > > > > >> > KIP-383. My suggestion was to make SslEngineBuilder > > > configurable. > > > > > The > > > > > > >> > default implementation of this pluggable class would be > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > > https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/security/ssl/SslEngineBuilder.java > > > > > > >> . > > > > > > >> > That should give an idea of the size of the configurable > part > > > that a > > > > > > >> > custom > > > > > > >> > class needs to implement. A large part of that is about > loading > > > > > > >> > keystore/truststore. I agree it has slightly more code than > > > KIP-486 > > > > > > >> > proposes, but since it lets you customize creation of > > > SSLEngine, it > > > > > > >> > would > > > > > > >> > address every possible scenario. > > > > > > >> > > > > > > > >> > Thoughts? > > > > > > >> > > > > > > > >> > > > > > > > >> > On Fri, Aug 30, 2019 at 2:02 AM Maulin Vasavada < > > > > > > >> maulin.vasav...@gmail.com> > > > > > > >> > wrote: > > > > > > >> > > > > > > > >> > > I thought about it more. I feel that no matter how we > > > refactor the > > > > > > >> code > > > > > > >> > > (with or without KIP-383 integrated), ultimately the need > of > > > > > > >> customizing > > > > > > >> > > loading for keys and certs will still remain. Whenever > that > > > need > > > > > > >> arises we > > > > > > >> > > might end up thinking about the solution suggested by our > > > KIP-486. > > > > > > >> Hence > > > > > > >> > > regardless of the other KIPs and configurations "if we do > > > need to > > > > > > >> customize > > > > > > >> > > loading of keys/certs, we will need the code changes > > > suggested by > > > > > > this > > > > > > >> > > KIP". > > > > > > >> > > > > > > > > >> > > Let me know what you guys think. > > > > > > >> > > > > > > > > >> > > Harsha, we are working on changing the interfaces for > > > key/trust > > > > > > store > > > > > > >> > > loaders with Certificate and PrivateKey objects. Will > > > probably be > > > > > > >> able to > > > > > > >> > > update it later today or tomorrow. > > > > > > >> > > > > > > > > >> > > Thanks > > > > > > >> > > Maulin > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > > > > > > > >> > > On Thu, Aug 29, 2019 at 2:30 PM Maulin Vasavada < > > > > > > >> maulin.vasav...@gmail.com > > > > > > >> > > > > > > > > > >> > > wrote: > > > > > > >> > > > > > > > > >> > > > On that, I actually looked at KIP-383 before briefly. > > > However, > > > > > > that > > > > > > >> > > > sounded like lot of changes suggested. > > > > > > >> > > > > > > > > > >> > > > One "key" thing we have to keep in mind is - IF we need > lot > > > of > > > > > > >> > > > customization Kafka already allows you to use your > > > SslProvider > > > > > via > > > > > > >> > > > ssl.providers or the changes done by KIP-492 and > > > > > > >> > > > SSLContext.getInstance(protocol, provider) call allows > us to > > > > > > return > > > > > > >> the > > > > > > >> > > > SSLContext with "ALL" the details we would like to > > > customize. > > > > > > Hence > > > > > > >> I am > > > > > > >> > > > not sure that customization suggested by KIP-383 would > be > > > worth > > > > > > the > > > > > > >> > > effort. > > > > > > >> > > > We also have similar SSLContext customization outside of > > > Kafka. > > > > > > >> > > > > > > > > > >> > > > Thanks > > > > > > >> > > > Maulin > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > > > > > > > >> > > > On Thu, Aug 29, 2019 at 12:47 PM Pellerin, Clement < > > > > > > >> > > > clement_pelle...@ibi.com> wrote: > > > > > > >> > > > > > > > > > >> > > >> KIP-383 in its present form was vetoed because it was > not > > > > > > possible > > > > > > >> to > > > > > > >> > > add > > > > > > >> > > >> validation of custom properties in a future KIP. The > > > solution > > > > > to > > > > > > >> that is > > > > > > >> > > >> the first proposal I wrote for KIP-383 which made the > whole > > > > > > >> SslFactory > > > > > > >> > > >> pluggable. That first solution was also vetoed hence > the > > > > > > deadlock. > > > > > > >> > > >> > > > > > > >> > > >> Replacing the whole factory was a much nicer solution. > It > > > was > > > > > > >> vetoed > > > > > > >> > > >> because doing this almost invariably meant the > replacement > > > lost > > > > > > >> all the > > > > > > >> > > >> complex validation code in the default SslFactory. > > > > > > >> > > >> > > > > > > >> > > >> My current idea is to extract the validation code into > > > another > > > > > > >> public > > > > > > >> > > API > > > > > > >> > > >> that SslFactory would call. I did not look at the newly > > > > > > refactored > > > > > > >> code > > > > > > >> > > and > > > > > > >> > > >> I did not study how to do this yet. KIP-383 was not > > > popular at > > > > > > the > > > > > > >> time > > > > > > >> > > and > > > > > > >> > > >> designing a new solution is a lot of work. > > > > > > >> > > >> > > > > > > >> > > >> Is there interest from 3 binding voters for something > like > > > > > this? > > > > > > >> > > >> > > > > > > >> > > >> -----Original Message----- > > > > > > >> > > >> From: Rajini Sivaram [mailto:rajinisiva...@gmail.com] > > > > > > >> > > >> Sent: Thursday, August 29, 2019 2:57 PM > > > > > > >> > > >> To: dev > > > > > > >> > > >> Subject: Re: [DISCUSS] KIP-486 Support for pluggable > > > KeyStore > > > > > and > > > > > > >> > > >> TrustStore > > > > > > >> > > >> > > > > > > >> > > >> Hi Maulin, > > > > > > >> > > >> > > > > > > >> > > >> In SSL scenarios, I imagine security providers > introduced > > > by > > > > > > >> KIP-492 are > > > > > > >> > > >> likely to be most useful when you want to use third > party > > > > > > >> providers. The > > > > > > >> > > >> biggest advantage of the config from that KIP is that > you > > > don't > > > > > > >> need to > > > > > > >> > > >> write much code to integrate existing security > providers > > > into > > > > > > Kafka > > > > > > >> > > >> brokers > > > > > > >> > > >> or clients. As I understand it, KIP-486 is a more > > > convenient > > > > > > >> option for > > > > > > >> > > >> the > > > > > > >> > > >> specific problem of loading keystores/truststores > > > differently. > > > > > It > > > > > > >> can be > > > > > > >> > > >> achieved in theory with KIP-492, but KIP-486 is a much > > > simpler > > > > > > >> option > > > > > > >> > > for > > > > > > >> > > >> this case. > > > > > > >> > > >> > > > > > > >> > > >> My concern about KIP-486 is that it introduces yet > another > > > > > > >> interface > > > > > > >> > > into > > > > > > >> > > >> our already complex security code, while only solving > one > > > > > > >> particular use > > > > > > >> > > >> case. Have you looked at > > > > > > >> > > >> > > > > > > >> > > >> > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-383%3A++Pluggable+interface+for+SSL+Factory > > > > > > >> > > >> ? > > > > > > >> > > >> The goal was to make > > > > > > >> > > >> org.apache.kafka.common.security.ssl.SslEngineBuilder > > > > > pluggable. > > > > > > >> > > >> The code has already been refactored by Colin after > that > > > KIP > > > > > was > > > > > > >> > > written, > > > > > > >> > > >> making it easier to implement KIP-383. This should > enable > > > you > > > > > to > > > > > > >> load > > > > > > >> > > your > > > > > > >> > > >> keystores and truststores differently. Using a > pluggable > > > > > > >> > > SslEngineBuilder > > > > > > >> > > >> will also solve several other use cases at the same > time. > > > > > KIP-383 > > > > > > >> hasn't > > > > > > >> > > >> been voted through yet, but perhaps you could take a > look > > > and > > > > > we > > > > > > >> could > > > > > > >> > > >> revive that instead if it solves your use case as well? > > > > > > >> > > >> > > > > > > >> > > >> Regards, > > > > > > >> > > >> > > > > > > >> > > >> Rajini > > > > > > >> > > >> > > > > > > >> > > >> > > > > > > >> > > >> On Thu, Aug 29, 2019 at 6:42 PM Maulin Vasavada < > > > > > > >> > > >> maulin.vasav...@gmail.com> > > > > > > >> > > >> wrote: > > > > > > >> > > >> > > > > > > >> > > >> > Hi Harsha > > > > > > >> > > >> > > > > > > > >> > > >> > Thank you. Appreciate your time and support on this. > Let > > > me > > > > > go > > > > > > >> back do > > > > > > >> > > >> some > > > > > > >> > > >> > more research and get back to you on the KeyStore > > > interface > > > > > > part. > > > > > > >> > > >> > Basically, if we return certs and keys in the > interface > > > then > > > > > > >> Kafka > > > > > > >> > > code > > > > > > >> > > >> > will have to build KeyStore object - which is also > > > > > reasonable. > > > > > > >> > > >> > > > > > > > >> > > >> > Thanks > > > > > > >> > > >> > Maulin > > > > > > >> > > >> > > > > > > > >> > > >> > On Thu, Aug 29, 2019 at 10:01 AM Harsha Chintalapani > < > > > > > > >> ka...@harsha.io > > > > > > >> > > > > > > > > > >> > > >> > wrote: > > > > > > >> > > >> > > > > > > > >> > > >> > > Hi Maulin, > > > > > > >> > > >> > > Use cases are clear now. I am > +1 > > > for > > > > > > moving > > > > > > >> > > >> forward > > > > > > >> > > >> > > with the discussions on having such configurable > > > option for > > > > > > >> users. > > > > > > >> > > But > > > > > > >> > > >> > the > > > > > > >> > > >> > > interfaces is proposed doesn't look right to me. > We are > > > > > still > > > > > > >> > > talking > > > > > > >> > > >> > about > > > > > > >> > > >> > > keystore interfaces. Given keystore's are used as > > > > > filebased > > > > > > >> way of > > > > > > >> > > >> > > transporting certificates I am not sure it will > help > > > the > > > > > rest > > > > > > >> of the > > > > > > >> > > >> > > user-base. > > > > > > >> > > >> > > In short, I am +1 on the KIP's > > > motivation > > > > > > >> and only > > > > > > >> > > >> have > > > > > > >> > > >> > > questions around returning keystores instead of > > > returning > > > > > > >> certs, > > > > > > >> > > >> private > > > > > > >> > > >> > > keys etc. . If others in the community are ok with > such > > > > > > >> interface we > > > > > > >> > > >> can > > > > > > >> > > >> > > move forward. > > > > > > >> > > >> > > > > > > > > >> > > >> > > Thanks, > > > > > > >> > > >> > > Harsha > > > > > > >> > > >> > > > > > > > > >> > > >> > > > > > > > > >> > > >> > > On Wed, Aug 28, 2019 at 1:51 PM, Maulin Vasavada < > > > > > > >> > > >> > > maulin.vasav...@gmail.com> > > > > > > >> > > >> > > wrote: > > > > > > >> > > >> > > > > > > > > >> > > >> > > > Hi Harsha > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > As we synced-up offline on this topic, we hope > you > > > don't > > > > > > >> have any > > > > > > >> > > >> more > > > > > > >> > > >> > > > clarifications that you are seeking. If that is > the > > > case, > > > > > > >> can you > > > > > > >> > > >> > please > > > > > > >> > > >> > > > help us move this forward and discuss what > changes > > > you > > > > > > would > > > > > > >> > > expect > > > > > > >> > > >> on > > > > > > >> > > >> > > the > > > > > > >> > > >> > > > KIP design in order to make it valuable > contribution? > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Just FYI - we verified our primary design change > > > with the > > > > > > >> author > > > > > > >> > > of > > > > > > >> > > >> > Sun's > > > > > > >> > > >> > > > X509 Trustmanager implementation and the outcome > is > > > that > > > > > > >> what we > > > > > > >> > > are > > > > > > >> > > >> > > > proposing makes sense at the heart of it - > "Instead > > > of > > > > > > >> writing > > > > > > >> > > >> > > TrustManager > > > > > > >> > > >> > > > just plugin the Trust store". We are open to > discuss > > > > > > >> additional > > > > > > >> > > >> changes > > > > > > >> > > >> > > > that you/anybody else would like to see on the > > > > > > functionality > > > > > > >> > > >> however. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks > > > > > > >> > > >> > > > Maulin > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Thu, Aug 22, 2019 at 9:12 PM Maulin Vasavada < > > > > > > >> > > >> > > maulin.vasav...@gmail.com> > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Hi Harsha > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Any response on my question? I feel this KIP is > worth > > > > > > >> > > accommodating. > > > > > > >> > > >> > Your > > > > > > >> > > >> > > > help is much appreciated. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks > > > > > > >> > > >> > > > Maulin > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Tue, Aug 20, 2019 at 11:52 PM Maulin Vasavada > < > > > > > > >> > > >> > maulin.vasavada@gmail. > > > > > > >> > > >> > > > com> wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Hi Harsha > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > I've examined the SPIFFE provider more and have > one > > > > > > question > > > > > > >> - > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > If SPIFFE didn't have a need to do > checkSpiffeId() > > > call > > > > > at > > > > > > >> the > > > > > > >> > > below > > > > > > >> > > >> > > > location, would you really still write the > Provider? > > > *OR* > > > > > > >> Would > > > > > > >> > > you > > > > > > >> > > >> > just > > > > > > >> > > >> > > > use TrustManagerFactory.init(KeyStore) signature > to > > > pass > > > > > > the > > > > > > >> > > >> KeyStore > > > > > > >> > > >> > > from > > > > > > >> > > >> > > > set of certs returned by spiffeIdManager. > > > > > > getTrustedCerts()? > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > >> > > > > > > > > >> > > > > > > > > > https://github.com/spiffe/java-spiffe/blob/master/src/main/java/spiffe/ > > > > > > >> > > >> > > > provider/CertificateUtils.java#L100 > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > /** > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > * Validates that the SPIFFE ID is present and > > > matches the > > > > > > >> SPIFFE > > > > > > >> > > ID > > > > > > >> > > >> > > > configured in > > > > > > >> > > >> > > > * the java.security property ssl.spiffe.accept > > > > > > >> > > >> > > > * > > > > > > >> > > >> > > > * If the authorized spiffe ids list is empty any > > > spiffe > > > > > id > > > > > > is > > > > > > >> > > >> > authorized > > > > > > >> > > >> > > > * > > > > > > >> > > >> > > > * @param chain an array of X509Certificate that > > > contains > > > > > > the > > > > > > >> > > Peer's > > > > > > >> > > >> > SVID > > > > > > >> > > >> > > > to be validated > > > > > > >> > > >> > > > * @throws CertificateException when either the > > > > > certificates > > > > > > >> > > doesn't > > > > > > >> > > >> > have > > > > > > >> > > >> > > a > > > > > > >> > > >> > > > SPIFFE ID or the SPIFFE ID is not authorized > > > > > > >> > > >> > > > */ > > > > > > >> > > >> > > > static void checkSpiffeId(X509Certificate[] > chain) > > > throws > > > > > > >> > > >> > > > CertificateException { > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks > > > > > > >> > > >> > > > Maulin > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Tue, Aug 20, 2019 at 4:49 PM Harsha > Chintalapani < > > > > > > >> > > >> ka...@harsha.io> > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Maulin, > > > > > > >> > > >> > > > The code parts you are pointing are specific for > > > Spiffe > > > > > and > > > > > > >> if > > > > > > >> > > >> > > > you are talking about validate method which uses > PKIX > > > > > check > > > > > > >> like > > > > > > >> > > any > > > > > > >> > > >> > > other > > > > > > >> > > >> > > > provider does. > > > > > > >> > > >> > > > If you want to default to SunJSSE everywhere you > can > > > do > > > > > so > > > > > > by > > > > > > >> > > >> > delegating > > > > > > >> > > >> > > > the calls in these methods to SunJSSE provider. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > TrustManagerFactory tmf = TrustManagerFactory > > > > > > >> > > >> > > > > > > > > .getInstance(TrustManagerFactory.getDefaultAlgorithm());and > > > > > > >> use > > > > > > >> > > >> > > > tmf.chekServerTrusted() > > > > > > >> > > >> > > > or use > > > > > > >> > > >> > > > > > > https://docs.oracle.com/javase/7/docs/api/javax/net/ssl/ > > > > > > >> > > >> > > > > > > TrustManagerFactory.html#getInstance(java.lang.String)if > > > > > > you > > > > > > >> want > > > > > > >> > > a > > > > > > >> > > >> > > > specific provider. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > -Harsha > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Tue, Aug 20, 2019 at 4:26 PM, Maulin Vasavada > < > > > > > > >> > > >> > maulin.vasavada@gmail. > > > > > > >> > > >> > > > com> > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Okay, so I take that you guys agree that I have > to > > > write > > > > > a > > > > > > >> > > 'custom' > > > > > > >> > > >> > > > algorithm and a provider to make it work , > correct? > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Now, for Harsha's comment "Here the 'Custom' > > > Algorithm is > > > > > > >> not an > > > > > > >> > > >> > > > implementation per say , ..." , I diagree. You > can > > > refer > > > > > to > > > > > > >> > > https:// > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > >> > > >> > > > > > > >> > > > > > > > > >> > > > > > > > > > > github.com/spiffe/java-spiffe/blob/master/src/main/java/spiffe/provider/ > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > SpiffeTrustManager.java#L91 < > > > > > > >> http://spiffetrustmanager.java/#L91> > > > > > > >> > > >> and > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > >> > > > > > > > > >> > > > > > > > > > https://github.com/spiffe/java-spiffe/blob/master/src/main/java/spiffe/ > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > provider/CertificateUtils.java#L100 > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > "that code" is the customization you have for the > > > custom > > > > > > way > > > > > > >> to > > > > > > >> > > >> check > > > > > > >> > > >> > > > something on top of regular checks. That method > is > > > NOT > > > > > > doing > > > > > > >> > > custom > > > > > > >> > > >> > > > truststore loading. It is validating/verifying > > > something > > > > > in > > > > > > >> the > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > "custom" > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > way with spiffeId. > > > > > > >> > > >> > > > I bet that without that you won't have a need of > the > > > > > custom > > > > > > >> > > >> algorithm > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > in > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > the first place. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Let me know if you agree to this. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks > > > > > > >> > > >> > > > Maulin > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Tue, Aug 20, 2019 at 2:08 PM Sandeep Mopuri < > > > > > > >> mpr...@gmail.com> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Hi Maulin, thanks for the discussion. As Harsha > > > pointed > > > > > > out, > > > > > > >> to > > > > > > >> > > use > > > > > > >> > > >> the > > > > > > >> > > >> > > > KIP492, you need to create a new provider, > register a > > > > > *new* > > > > > > >> custom > > > > > > >> > > >> > > > algorithm for your keymanager and trustmanager > > > factory > > > > > > >> > > >> implementations. > > > > > > >> > > >> > > > After this, the kafka server configuration can be > > > done as > > > > > > >> given > > > > > > >> > > >> below > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > # Register the provider class with custom > algorithm, > > > say > > > > > > >> CUSTOM > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > security. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > provider.classes=com.company.security.CustomProvider > > > > > > >> > > >> > > > <http://provider.classes > > > > > > >> =com.company.security.customprovider/> > > > > > > >> > > >> > > > <http://security.provider.classes > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > =com.company.security.customprovider/> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > # Register the keymanager and trustmanager > algorithms > > > > > > >> > > >> > > > # These algorithms indicate that the Keymanager > and > > > > > > >> Trustmanagers > > > > > > >> > > >> > > > registered under the algorithm "CUSTOM" needs to > be > > > used > > > > > > >> > > >> > > > ssl.trustmanager.algorithm=CUSTOM > > > > > > >> > > >> > > > ssl.keymanager.algorithm=CUSTOM > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > And the customprovider looks like this... > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > public class CustomProvider extends Provider { > > > > > > >> > > >> > > > public CustomProvider() { > > > > > > >> > > >> > > > super("NEW_CUSTOM_PROVIDER", 0.1, "Custom > KeyStore > > > and > > > > > > >> > > TrustStore"); > > > > > > >> > > >> > > > super.put("KeyManagerFactory.CUSTOM", > > > > > > >> "customKeyManagerFactory"); > > > > > > >> > > >> > > > super.put("TrustManagerFactory.CUSTOM", > > > > > > >> > > >> > > > "customTrustManagerFactory"); > > > > > > >> > > >> > > > } > > > > > > >> > > >> > > > } > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > The PR for this is in review and can be found > here: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > https://github.com/ > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > apache/kafka/pull/7090 > > > > > > >> > > >> > > > This PR includes the fixed insertProviderAt > function > > > > > call. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Tue, Aug 20, 2019 at 9:56 AM Harsha > Chintalapani < > > > > > > >> > > >> ka...@harsha.io> > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Answers are in-line > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Mon, Aug 19, 2019 at 10:45 PM, Maulin > Vasavada < > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > maulin.vasavada@gmail. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > com > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Hi Colin > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > When I refer to "standard" or "custom" > algorithms I > > > am > > > > > > >> following > > > > > > >> > > >> Java > > > > > > >> > > >> > > > security Provider Terminology. You can refer to > > > > > > >> > > >> > > > > > > > > > >> > https://docs.oracle.com/javase/7/docs/technotes/guides/security/ > > > > > > >> > > >> > > > StandardNames.html#TrustManagerFactory link I > > > provided > > > > > > >> earlier in > > > > > > >> > > >> the > > > > > > >> > > >> > > > emails. It says PKIX is the default Algorithm for > > > > > > >> > > >> TrustManagerFactory. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > 1. For SPIFFE, I am not sure why you are saying > 'it > > > does > > > > > > not > > > > > > >> > > >> implement > > > > > > >> > > >> > > > custom algorithms' because the following file > clearly > > > > > > >> indicates > > > > > > >> > > >> that it > > > > > > >> > > >> > > > does use custom algorithm- > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > >> > > > > > > > > >> > > > > > > > > > https://github.com/spiffe/java-spiffe/blob/master/src/main/java/spiffe/ > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > provider/SpiffeProvider.java#L17 > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Algorithm value: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > >> > > > > > > > > >> > > > > > > > > > https://github.com/spiffe/java-spiffe/blob/master/src/main/java/spiffe/ > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > provider/SpiffeProviderConstants.java#L6 > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > @Harsha do you want to chime in since you use > that > > > > > > provider? > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Here the "Custom" Algorithm is not an > implementation > > > per > > > > > > say > > > > > > >> , > > > > > > >> > > >> rather > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > used > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > to invoke the custom trust store factory and key > > > manager > > > > > > >> factory. > > > > > > >> > > >> You > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > are > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > not moving away from "standard" alogrithms that > are > > > > > > >> available. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > >> > > > > > > > > >> > > > > > > > > > https://github.com/spiffe/java-spiffe/blob/master/src/main/java/spiffe/ > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > provider/SpiffeTrustManager.java > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > As you can see it delegates all the calls of > > > verification > > > > > > of > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > certificate > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > the default implementation available. > > > > > > >> > > >> > > > So in our implementation we still use PKIX to > verify > > > the > > > > > > >> > > certificate > > > > > > >> > > >> > > > chain. So you are not losing anything here and > > > Spiffe is > > > > > > not > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > reimplementing > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > the verification process. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > 2. I already mentioned in my 3rd point, in my > > > previous > > > > > > post, > > > > > > >> why > > > > > > >> > > >> using > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > ssl.provider does NOT work. I updated KIP-486 in > > > > > "rejected > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > alternatives" > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > also why ssl.provider does not work. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > As mentioned before , provider is the right way > to > > > go. I > > > > > am > > > > > > >> still > > > > > > >> > > >> not > > > > > > >> > > >> > > > understanding the gap is. > > > > > > >> > > >> > > > If I understand correctly your argument is , > > > provider is > > > > > > >> going to > > > > > > >> > > >> ask > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > implement a custom algorithm. > > > > > > >> > > >> > > > My answer to that is , no you are not > > > re-implementing the > > > > > > >> > > algorithm. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Please > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > check the above link , TrustManager > implementation it > > > > > > >> delegates > > > > > > >> > > the > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > calls > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > back. There is no need to implement your own > here. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > 3. Security.insertProviderAt() comments were > based on > > > > > > >> assumption > > > > > > >> > > if > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > KIP-492 > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > changes are done and we use that mechanism to > > > configure > > > > > > >> providers > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > instead > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > of ssl.provider configuration. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > KIP-492 has patch available and going through > review. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Can you read my all the points, I mentioned in my > > > > > previous > > > > > > >> post, > > > > > > >> > > >> very > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > carefully? I am covering all the aspects in > > > explaining. I > > > > > > am > > > > > > >> open > > > > > > >> > > to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > still > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > discuss more to clarify any doubts. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > "3. If we just use existing ssl.provider kafka > > > > > > configuration > > > > > > >> then > > > > > > >> > > >> our > > > > > > >> > > >> > > > provider will be used in > > > SSLContext.getInstance(protocol, > > > > > > >> > > provider) > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > call > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > in > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > SslFactory.java <http://sslfactory.java/> < > > > > > > >> > > http://sslfactory.java/> > > > > > > >> > > >> < > > > > > > >> > > >> > > > http://sslfactory.java/> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > and > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > if our provider does not have > > > > > > >> > > >> > > > implementation for SSLContext.TLS/TLSv1.1/TLSv1.2 > > > etc it > > > > > > >> breaks > > > > > > >> > > (we > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > tested > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > it). Example: In MyProvider sample above you see > > > that I > > > > > > >> didn't add > > > > > > >> > > >> > > > SSLContext.TLSv1 as > > > > > > >> > > >> > > > "Service+Algorithm" and that didn't work for me. > In > > > > > SPIFFE > > > > > > >> > > provider > > > > > > >> > > >> you > > > > > > >> > > >> > > > don't have this challenge since you are planning > to > > > > > bypass > > > > > > >> > > >> ssl.provider > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > as > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > you mention in the KIP-492." > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Yes here you need to pass the protocol that your > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > KeyManager/TrustManager > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > registered with and in no way its deviating from > TLS > > > RFC > > > > > > >> spec. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > >> > > > > > > > > >> > > > > > > > > > https://github.com/srisatish/openjdk/blob/master/jdk/src/share/classes/ > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > javax/net/ssl/SSLContext.java#L134 > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > My suggestion here is for you to implement a > simple > > > > > > Security > > > > > > >> > > >> Provider > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > as > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > you did before and register a Algorithm. You can > use > > > the > > > > > > >> existing > > > > > > >> > > >> > > > implementation in SpiffeProvider and plug in > changes > > > > > where > > > > > > >> you > > > > > > >> > > need > > > > > > >> > > >> to > > > > > > >> > > >> > > > retrieve the certificates from by making RPC > call. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Run an end-to-end test with Kafka broker coming > up > > > with > > > > > > your > > > > > > >> > > >> provider > > > > > > >> > > >> > > > making calls to RPC call. You do need to pass the > > > "custom > > > > > > >> > > algorithm" > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > that > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > you registered in your key manager to invoke the > > > > > provider. > > > > > > I > > > > > > >> think > > > > > > >> > > >> your > > > > > > >> > > >> > > > concern is this approach is replacing the > existing > > > known > > > > > > >> > > >> ciphersuites > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > and > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > certificate validation provided by java. Which > its > > > not. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Now test the TLS connection you can do so via > openssl > > > > > > >> -s_client > > > > > > >> > > >> options > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > test if everything else is working. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > I am happy to share configs that we used if you > are > > > > > > >> interested. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks, > > > > > > >> > > >> > > > Harsha > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks > > > > > > >> > > >> > > > Maulin > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Mon, Aug 19, 2019 at 9:52 AM Colin McCabe < > > > > > > >> cmcc...@apache.org> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Hi Maulin, > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > A lot of JSSE providers don't implement custom > > > > > algorithms. > > > > > > >> Spire > > > > > > >> > > is > > > > > > >> > > >> a > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > good > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > example of a JSSE provider that doesn't, and yet > is > > > still > > > > > > >> useful > > > > > > >> > > to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > many > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > people. Your JSSE provider can work fine even if > it > > > > > doesn't > > > > > > >> > > >> implement a > > > > > > >> > > >> > > > custom algorithm. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Maybe I'm missing something, but I don't > understand > > > the > > > > > > >> discussion > > > > > > >> > > >> of > > > > > > >> > > >> > > > Security.insertProviderAt() that you included. > > > > > > >> SslEngineBuilder > > > > > > >> > > >> doesn't > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > use > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > that API to get the security provider. Instead, > it > > > calls > > > > > > >> > > >> > > > "SSLContext.getInstance(protocol, provider)", > where > > > > > > provider > > > > > > >> is > > > > > > >> > > the > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > name > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > of the provider. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > best, > > > > > > >> > > >> > > > Colin > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Sat, Aug 17, 2019, at 20:13, Maulin Vasavada > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On top of everything above I feel strongly to add > > > the 4th > > > > > > >> point > > > > > > >> > > >> which > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > is > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > based on Java APIs for > > > > > TrustManagerFactory.init(KeyStore) ( > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > https://docs.oracle.com/javase/7/docs/api/javax/net/ssl/ > > > > > > >> > > >> > > > > TrustManagerFactory.html#init(java.security.KeyStore > > > > > > >> > > >> > > > <http://java.security.keystore/> > > > > > > >> > > >> > > > <http://java.security.keystore/>) > > > > > > >> > > >> > > > ) > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > and KeyManagerFactory.init(KeyStore, char[]) ( > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > >> > > >> > > > > > > > >> > > >> > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > https://docs.oracle.com/javase/7/docs/api/javax/net/ssl/KeyManagerFactory > > > > > > >> > > >> > > > . > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > html#init(java.security.KeyStore < > > > > > > >> http://java.security.keystore/> > > > > > > >> > > >> > > <http:// > > > > > > >> > > >> > > > java.security.keystore/ > > > > > > >> > > >> > > > ,%20char[]) > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > ). > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > 4. The above APIs are intended to support > providing > > > > > > >> "trust/key > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > material" > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > from the user without having to write their own > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > TrustManager/KeyManagers. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > To quote from the TrustManagerFactory.init()'s > > > > > > documentation > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > "Initializes > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > this factory with a source of certificate > > > authorities and > > > > > > >> related > > > > > > >> > > >> trust > > > > > > >> > > >> > > > material." > > > > > > >> > > >> > > > To quote from the KeyManagerFactory.init()'s > > > > > documentation > > > > > > >> > > >> "Initializes > > > > > > >> > > >> > > > this factory with a source of key material." > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Based on this it is clear that there is a > flexibility > > > > > > >> provided by > > > > > > >> > > >> Java > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > to enable developers to provide the required > > > trust/key > > > > > > >> material > > > > > > >> > > >> loaded > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > from > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > "anywhere" without requiring them to write custom > > > > > provider > > > > > > OR > > > > > > >> > > >> trust/key > > > > > > >> > > >> > > > managers. This same flexibility is reflected in > Kafka > > > > > code > > > > > > >> also > > > > > > >> > > >> where > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > it > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > loads the trust/keys from a local file and > doesn't > > > > > require > > > > > > >> > > writing a > > > > > > >> > > >> > > > Provider necessarily. If we do NOT have a custom > > > > > algorithm, > > > > > > >> it > > > > > > >> > > makes > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > less > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > sense to write a Provider. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks > > > > > > >> > > >> > > > Maulin > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Thu, Aug 15, 2019 at 11:45 PM Maulin Vasavada > < > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > maulin.vasav...@gmail.com> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Hi Harsha/Colin > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > I did the sample with a custom Provider for > > > > > > >> TrustStoreManager and > > > > > > >> > > >> tried > > > > > > >> > > >> > > > using ssl.provider Kafka config AND the way > KIP-492 > > > is > > > > > > >> suggesting > > > > > > >> > > >> (by > > > > > > >> > > >> > > > adding Provider programmatically instead of > relying > > > on > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > ssl.provider+java. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > security. The below sample is followed by my > detailed > > > > > > >> findings. > > > > > > >> > > I'll > > > > > > >> > > >> > > > appreciate if you can go through it carefully > and see > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > if you > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > see my point. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > package providertest; > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > import java.security.Provider < > > > > > > >> http://java.security.provider/> > > > > > > >> > > >> > <http:// > > > > > > >> > > >> > > > java.security.provider/> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > <http:// > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > java.security.provider/>; > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > public class MyProvider extends Provider { > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > private static final String name = "MyProvider"; > > > private > > > > > > >> static > > > > > > >> > > >> double > > > > > > >> > > >> > > > version = 1.0d; > > > > > > >> > > >> > > > private static String info = "Maulin's SSL > Provider > > > > > > >> v"+version; > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > public MyProvider() { > > > > > > >> > > >> > > > super(name, version, info); > > > > > > >> > > >> > > > this.put("TrustManagerFactory.PKIX", > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > "providertest.MyTrustManagerFactory"); > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > } > > > > > > >> > > >> > > > } > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > *Details:* > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > KIP-492 documents that it will use > > > Security.addProvider() > > > > > > >> assuming > > > > > > >> > > >> it > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > will > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > add it as position '0' which is not a correct > > > assumption. > > > > > > The > > > > > > >> > > >> > > > addProvider()'s documentation says it will add > it to > > > the > > > > > > last > > > > > > >> > > >> available > > > > > > >> > > >> > > > position. You may want to correct that to say > > > > > > >> > > >> > > > Security.insertProviderAt(provider, 1). > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Now coming back to our specific discussion, > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > 1. SPIFFE example uses Custom Algorithm - spiffe. > > > Hence > > > > > > when > > > > > > >> you > > > > > > >> > > add > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > that > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > provider in the provider list via > > > Security.addProvider() > > > > > > the > > > > > > >> > > >> position > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > where > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > it gets added doesn't matter (even if you don't > end > > > up > > > > > > >> adding it > > > > > > >> > > as > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > first > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > entry) since that is the ONLY provider for SPIFFE > > > > > specific > > > > > > >> > > algorithm > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > you > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > might have. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > We do *not* have custom algorithm for Key/Trust > > > > > > StoreMangers. > > > > > > >> > > Which > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > means > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > we have to use X509, PKIX etc "Standard > Algorithms" > > > (( > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > https://docs.oracle.com/javase/7/docs/technotes/guides/security/ > > > > > > >> > > >> > > > StandardNames.html > > > > > > >> > > >> > > > )) > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > in our provider to override the TrustStoreManager > > > (see my > > > > > > >> sample > > > > > > >> > > >> code) > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > and > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > KeyStoreManger and KeyManager. This creates > another > > > > > > challenge > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > mentioned in > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > the below point. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > 2. In order to make our Provider for loading > custom > > > > > > >> TrustStore > > > > > > >> > > >> work, we > > > > > > >> > > >> > > > have to add the provider as 'first' in the list > since > > > > > there > > > > > > >> are > > > > > > >> > > >> others > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > with > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > the same algorithm. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > However, the programatic way of adding provider > > > > > > >> > > >> > > > (Security.insertProviderAt()) is *not* > deterministic > > > for > > > > > > >> ordering > > > > > > >> > > >> since > > > > > > >> > > >> > > > different code can call the method for a > different > > > > > provider > > > > > > >> and > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > depending > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > upon the order of the call our provider can be > first > > > or > > > > > > >> pushed > > > > > > >> > > down > > > > > > >> > > >> the > > > > > > >> > > >> > > > list. This can happen very well in any client > > > application > > > > > > >> using > > > > > > >> > > >> Kafka. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > This > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > is specially problematic for a case when you > want to > > > > > > >> guarantee > > > > > > >> > > order > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > for a > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Provider having "Standard Algorithms". > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > If we add our provider in java.security file that > > > > > > definitely > > > > > > >> > > >> guarantees > > > > > > >> > > >> > > > the order(unless somebody calls removeProvider() > > > which is > > > > > > >> less > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > likely). But > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > if we add our provider in java.security file it > will > > > > > defeat > > > > > > >> the > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > purpose of > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > the KIP-492. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > In the gist - Apache Kafka must not rely on > > > > > > >> "un-deterministic" > > > > > > >> > > >> method > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > rely on Provider ordering. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > 3. If we just use existing ssl.provider kafka > > > > > configuration > > > > > > >> then > > > > > > >> > > our > > > > > > >> > > >> > > > provider will be used in > > > SSLContext.getInstance(protocol, > > > > > > >> > > provider) > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > call in > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > SslFactory.java <http://sslfactory.java/> < > > > > > > >> > > http://sslfactory.java/> > > > > > > >> > > >> < > > > > > > >> > > >> > > > http://sslfactory.java/> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > and > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > if our provider does not have implementation for > > > > > > >> > > >> > > > SSLContext.TLS/TLSv1.1/TLSv1.2 etc it breaks > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > (we > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > tested it). Example: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > In > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > MyProvider sample above you see that I didn't add > > > > > > >> SSLContext.TLSv1 > > > > > > >> > > >> as > > > > > > >> > > >> > > > "Service+Algorithm" and that didn't work for me. > In > > > > > SPIFFE > > > > > > >> > > provider > > > > > > >> > > >> you > > > > > > >> > > >> > > > don't have this challenge since you are planning > to > > > > > bypass > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > ssl.provider as > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > you mention in the KIP-492. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > *Overall summary,* > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > 1. Any provider based mechanisms- a) existing > > > > > ssl.provider > > > > > > >> and > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > b)KIP-492, > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > for loading key/trust store using "Standard > > > Algorithms" > > > > > do > > > > > > >> not > > > > > > >> > > work > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > 2. Approach suggested in our KIP-486 works > without > > > any > > > > > > issue > > > > > > >> and > > > > > > >> > > it > > > > > > >> > > >> is > > > > > > >> > > >> > > > *not* our context specific solve > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > 3. Based on above we feel KIP-492 and KIP-486 are > > > > > > >> complimentary > > > > > > >> > > >> changes > > > > > > >> > > >> > > > and not contradicting or redundent. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > If you want we can do a joint session somehow to > walk > > > > > > >> through the > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > sample I > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > have and various experiments I did. I would > > > encourage you > > > > > > to > > > > > > >> do > > > > > > >> > > >> similar > > > > > > >> > > >> > > > exercise by writing a Provider for "Standard > > > Algorithm" > > > > > for > > > > > > >> > > >> > > > TrustStoreManager (like our needs) and see what > you > > > find > > > > > > >> since > > > > > > >> > > only > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > writing > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > samples can bring out the complexity/challenges > we > > > face. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks > > > > > > >> > > >> > > > Maulin > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Wed, Aug 14, 2019 at 11:15 PM Maulin Vasavada > < > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > maulin.vasavada@gmail. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > com> wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Just to update - still working on it. Get to work > > > only on > > > > > > >> and off > > > > > > >> > > on > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > it :( > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Thu, Aug 8, 2019 at 4:05 PM Maulin Vasavada < > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > maulin.vasav...@gmail.com> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Hi Harsha > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Let me try to write samples and will let you > know. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks > > > > > > >> > > >> > > > Maulin > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Thu, Aug 8, 2019 at 4:00 PM Harsha Ch < > > > > > > >> harsha...@gmail.com> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Hi Maulin, > > > > > > >> > > >> > > > With java security providers can be as custom you > > > would > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > like > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > it to > > > > > > >> > > >> > > > be. If you only want to to implement a custom > way of > > > > > > loading > > > > > > >> the > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > keystore > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > and truststore and not implement any > > > protocol/encryption > > > > > > >> handling > > > > > > >> > > >> you > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > can > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > leave them empty and no need to implement. Have > you > > > > > looked > > > > > > >> into > > > > > > >> > > the > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > links I > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > pasted before? > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > https://github.com/spiffe/spiffe-example/blob/master/java-spiffe/ > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > >> > > >> > > > > > > > >> > > >> > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > spiffe-security-provider/src/main/java/spiffe/api/provider/SpiffeKeyStore. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > java > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > https://github.com/spiffe/spiffe-example/blob/master/java-spiffe/ > > > > > > >> > > >> > > > > > > > > spiffe-security-provider/src/main/java/spiffe/api/provider/ > > > > > > >> > > >> > > > SpiffeTrustManager.java < > > > http://spiffetrustmanager.java/ > > > > > > > > > > > > >> > > <http:// > > > > > > >> > > >> > > > spiffetrustmanager.java/> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > https://github.com/spiffe/spiffe-example/blob/master/java-spiffe/ > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > >> > > >> > > > > > > > >> > > >> > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > spiffe-security-provider/src/main/java/spiffe/api/provider/SpiffeProvider. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > java > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Can you please tell me which methods are too > complex > > > in > > > > > > >> above to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > implement > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > or unnecessary? You are changing anything in > SSL/TLS > > > > > > >> > > implementations > > > > > > >> > > >> > > > provided by > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > All of the implementations delegating the checks > to > > > the > > > > > > >> default > > > > > > >> > > >> > > > implementation anyway. > > > > > > >> > > >> > > > Spire agent is an example, its nothing but a GRPC > > > server > > > > > > >> listening > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > on a > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > unix domain socket . Above code is making a RPC > call > > > to > > > > > the > > > > > > >> local > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > daemon > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > to > > > > > > >> > > >> > > > get the certificate and keys. The mechanics are > > > pretty > > > > > much > > > > > > >> same > > > > > > >> > > as > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > what > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > you are asking for. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks, > > > > > > >> > > >> > > > Harsha > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Thu, Aug 8, 2019 at 3:47 PM Maulin Vasavada < > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > maulin.vasav...@gmail.com> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Imagine a scenario like - We know we have a > custom > > > KMS > > > > > and > > > > > > >> as a > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Kafka > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > owner > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > we want to comply to using that KMS source to > load > > > > > > >> keys/certs. As > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > a > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Kafka > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > owner we know how to integrate with KMS but > doesn't > > > > > > >> necessarily > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > have > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > know anything about cipher suites, algorithms, > and > > > > > SSL/TLS > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > implementation. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Going the Provider way requires to know lot more > > > than we > > > > > > >> should, > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > isn't it? > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Not that we would have concern/shy-away knowing > those > > > > > > >> details - > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > but > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > if we > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > don't have to - why should we? > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Thu, Aug 8, 2019 at 3:23 PM Maulin Vasavada < > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > maulin.vasav...@gmail.com> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Hi Harsha > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > We don't have spire (or similar) agents and we > do not > > > > > have > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > keys/certs > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > locally on any brokers. To elaborate more on my > > > previous > > > > > > >> email, > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > I agree that Java security Providers are used in > much > > > > > > broader > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > sense > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > - to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > have a particular implementation of an > algorithm, use > > > > > > >> specific > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > cipher > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > suites for SSL , OR in our current team's case > have a > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > particular > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > way to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > leverage pre-generated SSL sessions. However, the > > > scope > > > > > of > > > > > > >> our > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > KIP > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > (486) > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > is > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > much restricted than that. We merely intend to > > > provide a > > > > > > >> custom > > > > > > >> > > >> > > > keystore/truststore for our SSL connections and > not > > > > > really > > > > > > >> worry > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > about > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > underlying specific SSL/TLS implementation. This > > > > > simplifies > > > > > > >> it > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > a > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > lot for > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > us to keep the concerns separate and clear. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > I feel our approach is more complimentary such > that > > > it > > > > > > allows > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > for > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > using > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > keystores of choice while retaining the > flexibility > > > to > > > > > use > > > > > > >> any > > > > > > >> > > >> > > > underlying/available Provider for actually making > > > the SSL > > > > > > >> call. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > We agree with KIP-492's approach based on > Providers > > > (and > > > > > > >> Java's > > > > > > >> > > >> > > > recommendation), but also strongly believe that > our > > > > > > approach > > > > > > >> can > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > compliment > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > it very effectively for reasons explained above. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks > > > > > > >> > > >> > > > Maulin > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Thu, Aug 8, 2019 at 3:05 PM Harsha > Chintalapani < > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > ka...@harsha.io > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Hi Maulin, > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Thu, Aug 08, 2019 at 2:04 PM, Maulin Vasavada > < > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > maulin.vasavada@gmail. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > com> > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Hi Harsha > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > The reason we rejected the SslProvider route is > that > > > - we > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > only > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > needed > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > a > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > custom way to load keys/certs. Not touch any > policy > > > that > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > existing > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Providers > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > govern like SunJSSE Provider. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > We have exactly the same requirements to load > certs > > > and > > > > > > keys > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > through > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > spire > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > agent. We used security.provider to do that > exactly. > > > I am > > > > > > not > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > sure > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > why > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > you > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > would be modifying any policies provided by > default > > > > > SunJSSE > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > provider. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Can > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > you give me an example of having custom provider > that > > > > > will > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > override an > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > existing policy in SunJSSE provider. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > As pointed out earlier, this kip > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > https://cwiki.apache.org/confluence/display/KAFKA/ > > > > > > >> > > >> > > > > > > > > > >> > KIP-492%3A+Add+java+security+providers+in+Kafka+Security+config > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > allows > > > > > > >> > > >> > > > you to load security.provider through config. > > > > > > >> > > >> > > > Take a look at the examples I gave before > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > https://github.com/spiffe/spiffe-example/blob/master/java-spiffe/ > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > >> > > >> > > > > > > > >> > > >> > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > spiffe-security-provider/src/main/java/spiffe/api/provider/SpiffeProvider. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > java > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > It registers KeyManagerFactory and > > > TrustManagerFactory > > > > > and > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Keystore > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > algorithm. > > > > > > >> > > >> > > > Implement your custom way of loading Keystore in > here > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > https://github.com/spiffe/spiffe-example/blob/master/java-spiffe/ > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > >> > > >> > > > > > > > >> > > >> > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > spiffe-security-provider/src/main/java/spiffe/api/provider/SpiffeKeyStore. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > java > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > and Trust manager like here > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > https://github.com/spiffe/spiffe-example/blob/master/java-spiffe/ > > > > > > >> > > >> > > > > > > > > spiffe-security-provider/src/main/java/spiffe/api/provider/ > > > > > > >> > > >> > > > SpiffeTrustManager.java < > > > http://spiffetrustmanager.java/ > > > > > > > > > > > > >> > > <http:// > > > > > > >> > > >> > > > spiffetrustmanager.java/> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > In your Kafka client you can set the > > > security.provider to > > > > > > >> your > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > custom > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > implementation and with this fix > > > > > > >> > > >> > > > https://issues.apache.org/jira/browse/KAFKA-8191 > > > you can > > > > > > set > > > > > > >> > > >> > > > keyManagerAlgorigthm and trustManagerAlgorithm > > > configs. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > All of this is in your clients and broker side > and > > > do not > > > > > > >> need > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > touch > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > any > > > > > > >> > > >> > > > policy changes at JVM level. You'll register the > > > > > providers > > > > > > in > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > the > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > priority > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > order and can still have SunJSSE provider and > have > > > your > > > > > > >> custom > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > provider > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > implement the key and trust managers. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > The ask here is different than KIP-492. We don't > > > have any > > > > > > >> need > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > modify/specify the algorithm parameter. Does that > > > make > > > > > > sense? > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > The ask in KIP is introducing new interfaces > where > > > the > > > > > > KIP's > > > > > > >> > > >> > > > goal/motivation can be achieved through the > > > > > > security.provider > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > and > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > we > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > worked > > > > > > >> > > >> > > > on similar goal without touching any Keystore or > > > > > Truststore > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > interfaces. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > My advise is against changing or introducing new > > > > > interfaces > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > when > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > it can > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > work through security.provider. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks, > > > > > > >> > > >> > > > Harsha > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Maulin > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Thu, Aug 8, 2019 at 7:48 AM Harsha > Chintalapani < > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > ka...@harsha.io> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > In your KIP you added security. provider as > rejected > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > alternative > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > and > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > specified "its not the correct way". Do you mind > > > > > explaining > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > why > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > its > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > not? I > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > didn't find any evidence in Java docs to say so. > > > Contrary > > > > > > to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > your > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > statement > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > it does say in the java docs > > > > > > >> > > >> > > > " However, please note that a provider can be > used to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > implement > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > any > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > security service in Java that uses a pluggable > > > > > architecture > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > with > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > a > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > choice > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > of implementations that fit underneath." > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Java Security Providers have been used by other > > > projects > > > > > to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > provide > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > such > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > integration . I am not sure if you looked into > Spiffe > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > project to > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > efficiently distribute certificates but here is > an > > > > > example > > > > > > of > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Java > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > provider > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > https://github.com/spiffe/spiffe-example/blob/master/java-spiffe/ > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > >> > > >> > > > > > > > >> > > >> > > > > > > >> > > > > > > > > >> > > > > > > > > > > > > > > > spiffe-security-provider/src/main/java/spiffe/api/provider/SpiffeProvider. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > java which > > > > > > >> > > >> > > > obtains certificates from local daemons. > > > > > > >> > > >> > > > These integrations are being used in Tomcat, > Jetty > > > etc.. > > > > > We > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > are > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > also > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > using > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Security provider to do the same in our Kafka > > > clusters. > > > > > So > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > unless I > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > see > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > more evidence why security.provider doesn't work > for > > > you > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > adding > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > new > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > interfaces while there exists more cleaner way of > > > > > achieving > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > the > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > goals > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > of > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > this KIP is unnecessary and breaks the well known > > > > > security > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > interfaces > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > provided by Java itself. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks, > > > > > > >> > > >> > > > Harsha > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Thu, Aug 08, 2019 at 6:54 AM, Harsha > Chintalapani > > > < > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > ka...@harsha.io > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Hi Maulin, > > > > > > >> > > >> > > > Not sure if you looked at my previous replies. > This > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > changes > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > are not required as there is already security > > > Provider to > > > > > > do > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > what you > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > are > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > proposing. This KIP > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > https://cwiki.apache.org/confluence/display/KAFKA/ > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > KIP-492%3A+Add+java+security+providers+in+Kafka+Security+config > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > also > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > addresses easy registration of such providers. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Thanks, > > > > > > >> > > >> > > > Harsha > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Wed, Aug 07, 2019 at 11:31 PM, Maulin Vasavada > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > <maulin.vasavada@gmail. > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > com> wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Bump! Can somebody please review this? > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > On Tue, Jul 16, 2019 at 1:51 PM Maulin Vasavada < > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > maulin.vasav...@gmail.com> > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > wrote: > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > Bump! Can somebody please review this? > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > -- > > > > > > >> > > >> > > > Thanks, > > > > > > >> > > >> > > > M.Sai Sandeep > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > > >> > > >> > > > > > > > > >> > > >> > > > > > > > >> > > >> > > > > > > >> > > > > > > > > > >> > > > > > > > > >> > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > >
