Hi Rajini, updated the KIP to reflect the ordering of the providers passed and added information about the configured providers taking precedence over the login module providers in case of SASL. Also updated the config name. Please take a look and if everything looks good, I can start a vote.
On Fri, Jul 26, 2019 at 2:04 AM Rajini Sivaram <rajinisiva...@gmail.com> wrote: > Hi Harsha, > > Since one provider can implement multiple things, excluding SASL may mean > not adding providers which implement any SASL mechanism even though you are > actually interested only in one SSL algorithm that also happens to be in > the provider. Perhaps it would be simpler to allow SASL as well, but > guarantee that this config overrides the providers added by the login > module today? So the existing ones become the default providers for SASL > and the new config added by this KIP takes precedence. What do you think? > > Another minor point - I was looking at other configs that loads multiple > classes and found `interceptor.classes`. Perhaps this config should be ` > security.provider.classes` rather than `class.names` (we do use class.name > in some broker configs, but we generally seem to omit `name` since you can > also provide actual classes). > > Regards, > > Rajini > > > On Thu, Jul 25, 2019 at 6:37 PM Harsha <ka...@harsha.io> wrote: > > > Thanks Rajini . > > > > > 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: > > > > We can keep this to SSL only. Given for SASL users can configure a > > provider for LoginModule. > > Unless there is a usecase where we benefit from having a provider config > > for SASL. > > > > > > > > On Thu, Jul 25, 2019, at 5:25 AM, Rajini Sivaram wrote: > > > 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 > > > > > > > > > > > > > > > -- Thanks, M.Sai Sandeep
