I believe that I have addressed the concerns raised in this discussion. It seems reasonable to start a vote in about two days. Please raise any concerns you may have and I will be happy to attempt to address them.
/noa > On 10 Dec 2018, at 10:53, Noa Resare <n...@resare.com> wrote: > > Thank you for your comments, see replies inline. > >> On 9 Dec 2018, at 01:33, Harsha <ka...@harsha.io> wrote: >> >> Hi Noa, >> Based on KIP"s motivation section >> "If we had the ability to load a trust store from the classpath as well as >> from a file, the trust store could be shipped in a jar that could be >> declared as a dependency and piggyback on the distribution infrastructure >> already in place." >> >> It looks like you are making the assumption that distributing a jar is >> better than the file. I am not sure why one is better than the other. There >> are other use-cases where one can make a call local "daemon" over Unix >> socket to fetch a certificate as well. > > It was not my intention to convey that loading the trust store from the > classpath is inherently better in all cases. The proposed change simply > brings more choice. That said, I do believe that maven central and the > transitive dependency features of maven, gradle and ivy makes for a smoother > user experience in many cases. Validating broker certificates against a > organisation wide private CA cert has benefits in that it means that the > person setting up the kafka cluster(s) doesn’t need to bother with purchasing > or obtaining publicly trusted certs for every broker while still providing > strong cryptographic validation that a client is connecting to a legitimate > endpoint. If there was a way to distribute a private trust store that is as > easy as declaring an additional maven style dependency, I imagine that this > would be a more appealing proposition than it is today. I would assume that > many organisations opt to disable strict host checking in certificates to > sidestep the CA cert distribution problem. I think it would be valuable to > make it slightly more easy to do the right thing. > >> >> Just supporting a "classpath" option might work for a few users but it's not >> generic enough to support a wide variety of other infrastructures. My >> suggestion if the KIP motivation is to make the certificate/truststore >> available with different mechanisms, Lets make a interface that allow users >> to roll their own based on their infra and support File as the default >> mechanism so that we can support existing users. > > I agree that this is a fairly small change that doesn’t aim to support all > possible mechanisms that one might conceive of. I believe that KIP-383: > Pluggable interface for SSL Factory > <https://cwiki.apache.org/confluence/display/KAFKA/KIP-383:++Pluggable+interface+for+SSL+Factory> > might be a good vehicle to provide this level of flexibility, but even if > that proposal is accepted I still think that there is room for this KIP to > provide an easy to use solution to this problem. > > cheers > noa > >> -Harsha >> >> On Sat, Dec 8, 2018, at 7:03 AM, Noa Resare wrote: >>> >>> >>>> On 6 Dec 2018, at 20:16, Rajini Sivaram <rajinisiva...@gmail.com> wrote: >>>> >>>> Hi Noa, >>>> >>>> Thanks for the KIP. A few comments/questions: >>>> >>>> 1. If we support filenames starting with `classpath:` by requiring >>>> `file:`prefix, >>>> then we are presumably not supporting files starting `file:`. Not >>>> necessarily an issue, but we do need to document any restrictions. >>> >>> I think that it would be trivial to support ‘file:’ as a prefix in a >>> filesystem path >>> by just asking the user that really want that to add it twice: >>> >>> The config value "file:file:my_weird_file_name" would map to the >>> filesystem path "file:my_weird_file_name” >>> >>> >>>> 2. On the broker-side, trust stores are dynamically updatable. And we use >>>> file modification time to decide whether trust store needs to be reloaded. >>>> This is less of an issue once we implement >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-339%3A+Create+a+new+IncrementalAlterConfigs+API, >>>> but at the moment, we are relying on actual files on the file system for >>>> which we can compare modification times. >>> >>>> 3. On the client-side, trust stores are not currently updatable. And we >>>> don't have an API to make them updatable. By using class path, we preclude >>>> the use of file modification times in future to detect key or trust store >>>> updates for clients. It will be good to get feedback from the community on >>>> whether this is a reasonable longer-term restriction. >>> >>> Interesting. I think that it is a reasonable graceful degradation to >>> simply not pick up on changed truststores >>> read from the classpath as long as it is documented, but if we really >>> want we could save a checksum of >>> the truststore, re-read and compare to determine any changes. >>> >>>> 4. It will be good to get more feedback from the community on whether >>>> loading trust stores from CLASSPATH is a feature that is likely to be >>>> widely adopted. If not, perhaps >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-383%3A++Pluggable+interface+for+SSL+Factory >>>> will be sufficient to enable custom factories that do load trust store from >>>> the CLASSPATH. >>> >>> While this generic extension point would make it possible to do all >>> kinds of things, I think that the simplicity >>> of allowing for this fairly small modification would benefit the kafka >>> user that doesn’t feel comfortable >>> writing their own SSL Factory. That said, I would be thrilled to have >>> the opportunity to provide functionality >>> to get rid of the truststore file format and have future kafka users use >>> both the loading of CA certs and client >>> certs with PEM encoded cert and key files as the rest of the world has >>> for some time. >>> >>>> >>>> Regards, >>>> >>>> Rajini >>>> >>>> >>>> On Tue, Dec 4, 2018 at 7:17 PM Sönke Liebau >>>> <soenke.lie...@opencore.com.invalid> wrote: >>>> >>>>> Hi Neo, >>>>> >>>>> thanks for the KIP, the proposal sounds useful! >>>>> Also I agree on both assumptions that you made: >>>>> - users whose current truststore location starts with classpath: should be >>>>> very few and extremely far between (and arguably made questionable choices >>>>> when naming their files/directories), I personally think it is safe to >>>>> ignore these >>>>> - this could also be useful for loading keystores, not just truststores >>>>> >>>>> One additional idea maybe, looking at the Spring documentation they seem >>>>> to >>>>> support filesystem, classpath and URL resources. Would it make sense to >>>>> add >>>>> something to allow loading the truststore from a url as well when touching >>>>> this functionality? >>>>> >>>>> Best regards, >>>>> Sönke >>>>> >>>>> >>>>> On Fri, Nov 30, 2018 at 6:01 PM Noa Resare <n...@resare.com> wrote: >>>>> >>>>>> I wrote a KIP for my minimal suggested change to support reading a >>>>>> truststore from the classpath as well as from a file. >>>>>> >>>>>> The KIP is available here: >>>>>> >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-398%3A+Support+reading+trust+store+from+classpath >>>>>> < >>>>>> >>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-398:+Support+reading+trust+store+from+classpath >>>>>>> >>>>>> >>>>>> Any feedback or comments would be most welcome. >>>>>> >>>>>> Cheers >>>>>> Noa >>>>> >>>>> >>>>> >>>>> -- >>>>> Sönke Liebau >>>>> Partner >>>>> Tel. +49 179 7940878 >>>>> OpenCore GmbH & Co. KG - Thomas-Mann-Straße 8 - 22880 Wedel - Germany >>>>> >>> >
