Hi Sandeep/Harsha, I don't have any major concerns about this KIP since it solves a specific issue and is a relatively minor change. I am unconvinced about the SASL case, but it probably is better to add as a config that can be used with SASL as well in future anyway.
Just to complete the conversation above: 4) The main difference between SSL and SASL is that for SSL, you register a provider with your own algorithm name and you specify your algorithm name in a separate config. This algorithm name can be anything you choose. For SASL, we register providers for standard SASL mechanism names. If this KIP wants to address the SASL case, then basically you need to add a provider ahead of the built-in provider for that standard mechanism name. See below: 6) JVM starts off with an ordered list of providers, the `java.security` file you quote above shows the ordering. When you dynamically add providers, you have the choice of adding it anywhere in that list. Particularly with SASL where you may have multiple providers with the same name, this ordering is significant. Security.insertProviderAt(Provider provider, int position) allows you to choose the position. I think the KIP intends to add provider at the beginning using Security.addProvider(Provider provider), which is basically inserting at position 0. We should clarify the behaviour in the KIP. We should also clarify how this config will co-exist with existing dynamic updates of security providers in the SASL login modules in Kafka. Or clarify that you can't override those. What we don't want is non-deterministic behaviour which is the biggest issue with these static configs. On Wed, Jul 24, 2019 at 5:50 PM Harsha <ka...@harsha.io> wrote: > Thanks for the details. > Rajini, Can you please take a look and let us know if these addresses your > concerns. > > Thanks, > Harsha > > On Mon, Jul 22, 2019, at 9:36 AM, Sandeep Mopuri wrote: > > Hi Rajini, > > Thanks for raising the above questions. Please find the > > replies below > > > > On Wed, Jul 17, 2019 at 2:49 AM Rajini Sivaram <rajinisiva...@gmail.com> > > wrote: > > > > > Hi Sandeep, > > > > > > Thanks for the KIP. A few questions below: > > > > > > 1. Is the main use case for this KIP adding security providers for > SSL? > > > If so, wouldn't a more generic solution like KIP-383 work for this? > > > > > We’re trying to solve this for both SSL and SASL. KIP-383 allows > the > > creation of custom SSLFactory implementation, however adding the provides > > to new security algorithms doesn’t involve any new implementation of > > SSLFactory. Even after the KIP 383, people still are finding a need for > > loading custom keymanager and trustmanager implementations (KIP 486) > > > > 2. Presumably this config would also apply to clients. If so, have we > > > thought through the implications of changing static JVM-wide > security > > > providers in the client applications? > > > > > Yes, this config will be applied to clients as well and the > > responsibility to face the consequences of adding the security providers > > need to be taken by the clients. In cases of resource manager running > > streaming applications such as Yarn, Mesos etc.. each user needs to make > > sure they are passing these JVM arguments. > > > > 3. Since client applications can programmatically invoke the Java > > > Security API anyway, isn't the system property described in > `Rejected > > > Alternatives` a reasonable solution for brokers? > > > > > This is true in a kafka only environment, but with an eco-system of > > streaming applications like flink, spark etc which might produce to > kafka, > > it’s difficult to make changes to all the clients > > > > 4. We have SASL login modules in Kafka that automatically add security > > > providers for SASL mechanisms not supported by the JVM. We should > > > describe > > > the impact of this KIP on those and whether we would now require a > > > config > > > to enable these security providers > > > > In a single JVM, one can register multiple security.providers. By default > > JVM itself provides multiple providers and these will not stepped over > each > > other. The only way to activate a provider is through their registered > names > > Example: > > > > $ cat /usr/lib/jvm/jdk-8-oracle-x64/jre/lib/security/java.security > > ... > > security.provider.1=sun.security.provider.Sun > > security.provider.2=sun.security.rsa.SunRsaSign > > security.provider.3=sun.security.ec.SunEC > > security.provider.4=com.sun.net.ssl.internal.ssl.Provider > > security.provider.5=com.sun.crypto.provider.SunJCE > > security.provider.6=sun.security.jgss.SunProvider > > security.provider.7=com.sun.security.sasl.Provider > > security.provider.8=org.jcp.xml.dsig.internal.dom.XMLDSigRI > > security.provider.9=sun.security.smartcardio.SunPCSC > > ... > > > > A user of a provider will refer through their registered > provider > > > > > https://github.com/spiffe/spiffe-example/blob/master/java-spiffe/spiffe-security-provider/src/main/java/spiffe/api/provider/SpiffeProvider.java#L31 > > > > In the above example , we can register the SpiffeProvider and > > multiple other providers into the JVM. When a client or a broker wants to > > integrate with SpiffeProvider they need to add the config > > ssl.keymanager.alogirhtm = "Spiffe" . Another client can refer to a > > different provider or use a default one in the same JVM. > > > > > > > 5. We have been moving away from JVM-wide configs like the default > JAAS > > > config since they are hard to test reliably or update dynamically. > The > > > replacement config `sasl.jaas.config` doesn't insert a JVM-wide > > > configuration. Have we investigated similar options for the specific > > > scenario we are addressing here? > > > > > Yes, that is the case with jaas config, however in the case of > > security providers, along with adding the security providers to JVM > > properties, one also need to configure the provider algorithm. For > example, > > in the case of SSL configuration, besides adding the security provider to > > the JVM, we need to configure the “ssl.trustmanager.algorithm” and > > “ssl.keymanager.algorithm” inorder for the provider implementation to > > apply. Different components can opt for different key and trustmanager > > algorithms and can work independently simultaneously in the same JVM. > This > > case is different from the jaas config. > > > > > > > 6. Are we always going to insert new providers at the start of the > > > provider list? > > > > Can you please explain what exactly do you mean by this > > > > > > > > > > > Regards, > > > > > > Rajini > > > > > > > > > > > > On Wed, Jul 17, 2019 at 5:05 AM Harsha <ka...@harsha.io> wrote: > > > > > > > Thanks for the KIP Sandeep. LGTM. > > > > > > > > Mani & Rajini, can you please look at the KIP as well. > > > > > > > > Thanks, > > > > Harsha > > > > > > > > On Tue, Jul 16, 2019, at 2:54 PM, Sandeep Mopuri wrote: > > > > > Thanks for the suggestions, made changes accordingly. > > > > > > > > > > On Tue, Jul 16, 2019 at 9:27 AM Satish Duggana < > > > satish.dugg...@gmail.com > > > > > > > > > > wrote: > > > > > > > > > > > Hi Sandeep, > > > > > > Thanks for the KIP, I have few comments below. > > > > > > > > > > > > >>“To take advantage of these custom algorithms, we want to > support > > > > java > > > > > > security provider parameter in security config. This param can be > > > used > > > > by > > > > > > kafka brokers or kafka clients(when connecting to the kafka > brokers). > > > > The > > > > > > security providers can also be used for configuring security > > > > algorithms in > > > > > > SASL based communication.” > > > > > > > > > > > > You may want to mention use case like > > > > > > spiffe.provider.SpiffeProvider[1] in streaming applications like > > > > > > Flink, Spark or Storm etc. > > > > > > > > > > > > >>"We add new config parameter in KafkaConfig named > > > > > > “security.provider.class”. The value of “security.provider” is > > > > expected to > > > > > > be a string representing the provider’s full classname. This > provider > > > > class > > > > > > will be added to the JVM properties through Security.addProvider > api. > > > > > > Security class can be used to programmatically add the provider > > > > classes to > > > > > > the JVM." > > > > > > > > > > > > It is good to have this property as a list of providers instead > of a > > > > > > single property. This will allow configuring multiple providers > if it > > > > > > is needed in the future without introducing hacky solutions like > > > > > > security.provider.class.name.x, where x is a sequence number. > You > > > can > > > > > > change the property name to “security.provider.class.names” and > its > > > > > > value is a list of fully qualified provider class names > separated by > > > > > > ‘,'. > > > > > > For example: > > > > > > > > > > > > > > > > > > > > security.provider.class.names=spiffe.provider.SpiffeProvider,com.foo.MyProvider > > > > > > > > > > > > Typo in existing properties section: > > > > > > “ssl.provider” instead of “ssl.providers”. > > > > > > > > > > > > Thanks, > > > > > > Satish. > > > > > > > > > > > > 1. https://github.com/spiffe/java-spiffe > > > > > > > > > > > > > > > > > > On Mon, Jul 15, 2019 at 11:41 AM Sandeep Mopuri < > mpr...@gmail.com> > > > > wrote: > > > > > > > > > > > > > > Hello all, > > > > > > > > > > > > > > I'd like to start a discussion thread for KIP-492. > > > > > > > This KIP plans on introducing a new security config parameter > for a > > > > > > custom > > > > > > > security providers. Please take a look and let me know what do > you > > > > think. > > > > > > > > > > > > > > More information can be found here: > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-492%3A+Add+java+security+providers+in+Kafka+Security+config > > > > > > > -- > > > > > > > Thanks, > > > > > > > Sai Sandeep > > > > > > > > > > > > > > > > > > > > > -- > > > > > Thanks, > > > > > M.Sai Sandeep > > > > > > > > > > > > > > > > > > -- > > Thanks, > > M.Sai Sandeep > > > > > > -- > > Thanks, > > M.Sai Sandeep > > >
