RFR 7191662: JCE providers should be located via ServiceLoader,

[Mandy Chung]

> On May 20, 2015, at 9:21 PM, Valerie Peng <valerie.peng at oracle.com> wrote:
> 
> Sean,
> 
> Could you please review this change? The changes are mostly the same as the prototype in Jake, but I have to make some modification due to the difference in ServiceLoader lookup in OpenJDK (corresponding META-INF/services/java.security.Provider files in each module) and the related makefile change (merge their content into one for the final image build). Then, I adjusted the Provider.configure() method to take a single String argument to be consistent with the "providerarg" option that keytool defined.
> 
> In addition, I also made some misc changes, such as configuring the providers inside ProviderConfig instead of ProviderLoader, add back the doPrivileged block to all the provider constructors. I also have second thought on making the switch to privider name (instead of provider class name) in java.security file, so I reverted the changes on that - that SunPKCS11 provider has its name specified in its configuration file, so when ServiceLoader loads the PKCS11 provider, the configuration file has not been passed to it, so the name is not known at that time. Thus, using the class name for the provider list entry seems to fit the flow better. I also updated the default policy for SunPKCS11 provider given its recent change of using sun.misc.
> 
> Webrev: http://cr.openjdk.java.net/~valeriep/7191662/webrev.00/
> CCC: http://ccc.us.oracle.com/7191662 <http://ccc.us.oracle.com/7191662>


A quick comment on the META-INF/services config files and the makefile. Merging the service config files is temporary until the module system is moving further along.

src/jdk.crypto.ucrypto/solaris/classes/META-INF/services/java.security.Provider.html
and other os-specific service configuration:
   1 #[solaris]com.oracle.security.ucrypto.UcryptoProvider

- why is this commented out?  Does the makefile uncomment it?  It should be simple
concatenation with

The makefile doesn’t seem right though.

make/gensrc/Gensrc-java.naming.gmk.html

  96 jdk.crypto.ec: $(GENSRC_PROVIDER)
  98 all: jdk.crypto.ec

java.naming doesn’t seem an appropriate module to be the main module for containing all service provider config files.  I initially propose to use jdk.crypto.ec as the gensrc module as indicated in line 96,98.

You can rename the file to Gensrc-jdk.crypto.ec and update the content..

GENSRC_PROVIDER := $(SUPPORT_OUTPUTDIR)/gensrc/java.naming/META-INF/services/java.security.Provider

GENSRC_PROVIDER is the output file.  line 79-89 is building the target list.   I think you need another variable to build up the target list but not GENSRC_PROVIDER.

You can reference how Gensrc-jdk.jdi.gmk concatenates the service config for jdk.jdi and dk.hotspot.agent module.

# Filter com.sun.jdi.connect.Connector
$(SUPPORT_OUTPUTDIR)/gensrc/jdk.jdi/META-INF/services/com.sun.jdi.connect.Connector: \
    $(JDK_TOPDIR)/src/jdk.jdi/share/classes/META-INF/services/com.sun.jdi.connect.Connector \
    $(SUPPORT_OUTPUTDIR)/gensrc/jdk.hotspot.agent/_the.sa.services
        $(process-provider)

# Copy the same service file into jdk.hotspot.agent so that they are kept the same.
$(JDK_OUTPUTDIR)/modules/jdk.hotspot.agent/META-INF/services/com.sun.jdi.connect.Connector: \
    $(SUPPORT_OUTPUTDIR)/gensrc/jdk.jdi/META-INF/services/com.sun.jdi.connect.Connector
        $(install-file)

Mandy
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/security-dev/attachments/20150520/8044d0d5/attachment.html>