Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

Mystery solved. Didn't notice that. :P Valerie On 6/30/2016 7:00 PM, Wang Weijun wrote: On Jul 1, 2016, at 9:16 AM, Valerie Peng wrote: Weird, I still see -provider on the link below.

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Jul 1, 2016, at 9:16 AM, Valerie Peng wrote: > > Weird, I still see -provider on the link below. > http://cr.openjdk.java.net/~weijun/8130302/webrev.06/test/sun/security/tools/keytool/i18n.html.sdiff.html This is sdiff, you need to scroll to far right to see the

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

Weird, I still see -provider on the link below. http://cr.openjdk.java.net/~weijun/8130302/webrev.06/test/sun/security/tools/keytool/i18n.html.sdiff.html If you saw differently, I need to book an urgent doc appt for my eyes... Valerie On 6/30/2016 5:40 PM, Wang Weijun wrote: Confused.

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Jul 1, 2016, at 8:20 AM, Valerie Peng wrote: > > Max, > > Please find comments in line. > > On 6/29/2016 8:18 PM, Wang Weijun wrote: >>> On Jun 30, 2016, at 9:39 AM, Valerie Peng wrote: >>> >>> Hi Max, >>> >>> Changes look fine. Just

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

Max, Please find comments in line. On 6/29/2016 8:18 PM, Wang Weijun wrote: On Jun 30, 2016, at 9:39 AM, Valerie Peng wrote: Hi Max, Changes look fine. Just some comments/questions as below. - line 46, fix this {0} as well? I don't see {0} in

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

Hi Sean Thanks for the comment. This exception is thrown by a helper class, which is then caught by the application class and a new localized error message including the provider name is shown to the user. Is that enough? Thanks Max > On Jun 30, 2016, at 6:59 PM, Seán Coffey

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 30/06/2016 14:44, Wang Weijun wrote: Hi Sean Thanks for the comment. This exception is thrown by a helper class, which is then caught by the application class and a new localized error message including the provider name is shown to the user. Is that enough? Perfect. That should do!

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

minor comment from me Max. src/java.base/share/classes/sun/security/tools/KeyStoreUtil.java 293 throw new IllegalArgumentException("No provider found"); Can you print the provider name that was being searched for in your exception ? Regards, Sean. On 30/06/2016 04:18, Wang Weijun

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Jun 30, 2016, at 9:39 AM, Valerie Peng wrote: > > Hi Max, > > Changes look fine. Just some comments/questions as below. > > > - line 46, fix this {0} as well? I don't see {0} in keytool/Resources.java. There is one in jarsigner/Resources.java: 46

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

Hi Max, Changes look fine. Just some comments/questions as below. - line 46, fix this {0} as well? - line 151, maybe it should be calling the createUsingTestJDK? Otherwise the default is to use the compiler JDK - line 53, better to use -providerClass? Thanks, Valerie On 6/28/2016 6:09

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

I will take a look tomorrow. Thanks, Valerie On 6/28/2016 6:09 PM, Wang Weijun wrote: Ping again to security-dev. Anyone can approve it? The latest webrev is at http://cr.openjdk.java.net/~weijun/8130302/webrev.06 Change from webrev.05 [1] is tiny. Thanks Max [1]

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

Ping again to security-dev. Anyone can approve it? The latest webrev is at http://cr.openjdk.java.net/~weijun/8130302/webrev.06 Change from webrev.05 [1] is tiny. Thanks Max [1] http://cr.openjdk.java.net/~weijun/8130302/webrev.06/interdiff.patch.html > On Jun 16, 2016, at 9:33 AM, Wang

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Jun 16, 2016, at 7:50 AM, Valerie Peng wrote: > > No big difference to me. Good, I'll remove the cast. @security-dev, can someone approve the whole webrev.05? http://cr.openjdk.java.net/~weijun/8130302/webrev.05 Thanks Max > Valerie > > On 6/15/2016 8:40

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Jun 15, 2016, at 10:57 PM, Mandy Chung wrote: > >>> 241 throw (InvalidParameterException) >>> >>> This cast should not be needed? >>> >> >> } catch (UcryptoException ue) { >> throw (InvalidParameterException) >> new

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Jun 15, 2016, at 1:27 AM, Wang Weijun wrote: > > All suggestions accepted. Webrev updated at > > http://cr.openjdk.java.net/~weijun/8130302/webrev.05 > >> 241 throw (InvalidParameterException) >> >> This cast should not be needed? >> > > } catch

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

All suggestions accepted. Webrev updated at http://cr.openjdk.java.net/~weijun/8130302/webrev.05 > 241 throw (InvalidParameterException) > > This cast should not be needed? > } catch (UcryptoException ue) { throw (InvalidParameterException) new

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Jun 13, 2016, at 8:28 PM, Wang Weijun wrote: > > OK, please take a review at the new version at > > http://cr.openjdk.java.net/~weijun/8130302/webrev.04/ > The new -addProvider option is good. I mostly reviewed KeyStoreUtil.java and skimmed through other

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 14/06/2016 04:28, Wang Weijun wrote: OK, please take a review at the new version at http://cr.openjdk.java.net/~weijun/8130302/webrev.04/ Changes from webrev.03: 1. The new option name -addprovider is used, along with the changes in Resources.java. 2. In

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

OK, please take a review at the new version at http://cr.openjdk.java.net/~weijun/8130302/webrev.04/ Changes from webrev.03: 1. The new option name -addprovider is used, along with the changes in Resources.java. 2. In KeyStoreUtil::loadProviderByClass, special treatment for

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Jun 12, 2016, at 11:33 AM, Alan Bateman wrote: > > > > On 12/06/2016 13:44, Wang Weijun wrote: >> I was about to send out a new webrev (CCC just approved) but noticed a >> behavior change. >> >> Although "-addprovider SUN" is useless it still worked when I

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 12/06/2016 13:44, Wang Weijun wrote: I was about to send out a new webrev (CCC just approved) but noticed a behavior change. Although "-addprovider SUN" is useless it still worked when I posted webrev.03, but now it failed, because ServiceLoader.load(Provider.class) does not contain

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

I was about to send out a new webrev (CCC just approved) but noticed a behavior change. Although "-addprovider SUN" is useless it still worked when I posted webrev.03, but now it failed, because ServiceLoader.load(Provider.class) does not contain "SUN" anymore. Maybe it is inside java.base and

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 23/03/2016 08:12, Wang Weijun wrote: Now that jake/m3 is in jdk9/dev I assume I will just include the jake-related codes and directly push a single changeset to jdk9/dev. Right? Yes, changes like this this can be pushed via jdk9/dev. In the mean-time, we will continue to work in

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> >> The SunPKCS11 compatibility line will be add in a sub-patch for jake. >> > > You can send me the jake’s delta patch once you push the change to jdk9/dev. I've been busy recently on DRBG and will be back working on this one. Now that jake/m3 is in jdk9/dev I assume I will just include the

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 26, 2016, at 1:03 AM, Sean Mullan wrote: > > On 02/18/2016 03:10 AM, Weijun Wang wrote: >> There is another compatibility issue which is more important: >> >> Today, we tell users to load their own PKCS11 provider with >> >> -providerClass

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 02/18/2016 03:10 AM, Weijun Wang wrote: There is another compatibility issue which is more important: Today, we tell users to load their own PKCS11 provider with -providerClass sun.security.pkcs11.SunPKCS11 -providerArg some.cfg and seems the new options should be -provider

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 22, 2016, at 7:28 PM, Wang Weijun wrote: > > Webrev updated at http://cr.openjdk.java.net/~weijun/8130302/webrev.03/. > > -provider and loadProviderByName() are useless for jdk9/dev, and > loadProviderByClass() only uses reflection. > I reviewed the provider

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

Webrev updated at http://cr.openjdk.java.net/~weijun/8130302/webrev.03/. -provider and loadProviderByName() are useless for jdk9/dev, and loadProviderByClass() only uses reflection. The SunPKCS11 compatibility line will be add in a sub-patch for jake. --Max > On Feb 23, 2016, at 10:25 AM,

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

>> >> You mean not supporting all pre-loaded providers in modules, but only one or >> two popular ones? >> > > I meant not support -providerClass for arbitrary providers loaded via service > loader. The only exception is SunPKCS11. In other words, -providerClass can > only be used to load

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 22, 2016, at 5:55 PM, Wang Weijun wrote: > 303 // A provider in module can also be use class name 304 if (p.getClass().getName().equals(provClass)) { ProviderConfig::getProvider doesn’t compare the classname. I

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

>>> >>> 303 // A provider in module can also be use class name >>> 304 if (p.getClass().getName().equals(provClass)) { >>> >>> ProviderConfig::getProvider doesn’t compare the classname. I thought we >>> agree to discourage the use of -providerClass to load a provider and

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 21, 2016, at 1:45 AM, Wang Weijun wrote: > > >> On Feb 20, 2016, at 4:01 AM, Mandy Chung wrote: >> >> >>> On Feb 19, 2016, at 2:47 AM, Wang Weijun wrote: >>> >>> Updated at the same URL >>> >>>

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

[I thought I sent out this but cannot find it in my inbox. Send again] New webrev at http://cr.openjdk.java.net/~weijun/8130302/webrev.02/ This is for jdk9/dev. I'll send out a sub-patch for jake, which includes an extra addRead call and more test changes. security-dev, I haven't heard any

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 20, 2016, at 4:01 AM, Mandy Chung wrote: > > >> On Feb 19, 2016, at 2:47 AM, Wang Weijun wrote: >> >> Updated at the same URL >> >> http://cr.openjdk.java.net/~weijun/8130302/webrev.01 >> >> The help looks like this now: >> >>

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 19, 2016, at 2:47 AM, Wang Weijun wrote: > > Updated at the same URL > > http://cr.openjdk.java.net/~weijun/8130302/webrev.01 > > The help looks like this now: > > -provider add security provider by name (e.g. SunPKCS11) > [-providerArg ]

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 19/02/2016 11:17, Wang Weijun wrote: : When and which repo should this change go into? Certainly this is all about jake but there are a lot of other changes, esp, the option list. Good question. Ideally jdk9/dev as we are trying to minimize the changes in the jake forest. If you do that

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 19, 2016, at 6:58 PM, Alan Bateman wrote: > > One other thing is whether we should add a section to JEP 261 on these tool > updates. I had originally assumed that the updates to these tools would > follow the module system into JDK 9 but you've turned up

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 19/02/2016 10:47, Wang Weijun wrote: Updated at the same URL http://cr.openjdk.java.net/~weijun/8130302/webrev.01 The help looks like this now: -provider add security provider by name (e.g. SunPKCS11) [-providerArg ] configure argument for -provider

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

Updated at the same URL http://cr.openjdk.java.net/~weijun/8130302/webrev.01 The help looks like this now: -provider add security provider by name (e.g. SunPKCS11) [-providerArg ] configure argument for -provider -providerclass add security provider by fully-qualified

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 19, 2016, at 5:36 PM, Alan Bateman wrote: > > > On 19/02/2016 09:07, Wang Weijun wrote: >> : >> I don't want the line to be too long. Is the preferred terminal width still >> 80 now? I noticed the java help output still fits with 80 chars but javac is >>

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 19/02/2016 09:07, Wang Weijun wrote: : I don't want the line to be too long. Is the preferred terminal width still 80 now? I noticed the java help output still fits with 80 chars but javac is already not. It could fit on a second line, the java launcher usage output does this. I see

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 19, 2016, at 4:42 PM, Alan Bateman wrote: > > On 19/02/2016 08:22, Wang Weijun wrote: >> A new webrev at >> >>http://cr.openjdk.java.net/~weijun/8130302/webrev.01/ >> >> The options for keytool have >> >> -provider [-providerArg ]add a provider

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 19/02/2016 08:22, Wang Weijun wrote: A new webrev at http://cr.openjdk.java.net/~weijun/8130302/webrev.01/ The options for keytool have -provider [-providerArg ]add a provider by name -providerclass [-providerArg ] add a provider by classname (omit some words because

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

A new webrev at http://cr.openjdk.java.net/~weijun/8130302/webrev.01/ The options for keytool have -provider [-providerArg ]add a provider by name -providerclass [-providerArg ] add a provider by classname (omit some words because line is too long) for jarsigner [-provider

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 18, 2016, at 1:52 AM, Alan Bateman wrote: > > > On 18/02/2016 09:08, Weijun Wang wrote: >> OK, but with -providerClass I'd like to support a class name even if it is >> already defined in a module as a service and has its own name. This makes >> sure old

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 18/02/2016 09:08, Weijun Wang wrote: OK, but with -providerClass I'd like to support a class name even if it is already defined in a module as a service and has its own name. This makes sure old commands still work. I think it should work fine but I assume we would want to discourage this.

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

OK, but with -providerClass I'd like to support a class name even if it is already defined in a module as a service and has its own name. This makes sure old commands still work. The existing -providerClass takes a class name and works as before. The -provider takes the name of a security

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 18/02/2016 08:10, Weijun Wang wrote: : Today, we tell users to load their own PKCS11 provider with -providerClass sun.security.pkcs11.SunPKCS11 -providerArg some.cfg and seems the new options should be -provider SunPKCS11 -providerArg some.cfg Why not just support all these formats?

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

In keytool help, we will write -providerAdd a security provider with its name “Add a security provider by the provider’s name” -providerArgOptional argument for -provider above -providerClass Add a security provider with its class name “Add a security provider

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 17, 2016, at 8:04 PM, Wang Weijun wrote: > >> >> On Feb 18, 2016, at 9:21 AM, Mandy Chung wrote: >> >>> >>> On Feb 17, 2016, at 4:46 PM, Wang Weijun wrote: >>> >>> On Feb 18, 2016, at 5:15 AM, Mandy

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 18, 2016, at 9:21 AM, Mandy Chung wrote: > >> >> On Feb 17, 2016, at 4:46 PM, Wang Weijun wrote: >> >> >>> On Feb 18, 2016, at 5:15 AM, Mandy Chung wrote: >>> >>> Can I say -providerClass -providerArg is

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 17, 2016, at 4:46 PM, Wang Weijun wrote: > > >> On Feb 18, 2016, at 5:15 AM, Mandy Chung wrote: >> >> Can I say -providerClass -providerArg is equivalent to >> extending java.security to add “security.provider.N=NAME ARG”? > > Yes.

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 18, 2016, at 5:15 AM, Mandy Chung wrote: > > Can I say -providerClass -providerArg is equivalent to extending > java.security to add “security.provider.N=NAME ARG”? Yes. > > I suggest to keep -providerClass and -providerArg only for legacy security >

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

> On Feb 16, 2016, at 5:20 PM, Weijun Wang wrote: > > > > On 2/16/2016 22:54, Alan Bateman wrote: >> >> On 16/02/2016 14:44, Weijun Wang wrote: >>> Please review the code change at >>> >>> http://cr.openjdk.java.net/~weijun/8130302/webrev.00/ >>> >>> I didn't

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 2/17/2016 18:33, Alan Bateman wrote: On 17/02/2016 01:20, Weijun Wang wrote: : Technically they are independent. With -providerClass/-providerArg, the provider is added into system and getInstance() calls (of keyStore, KeyPairGenerator, etc) can use it. On the other hand, -providerName

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 17/02/2016 01:20, Weijun Wang wrote: : Technically they are independent. With -providerClass/-providerArg, the provider is added into system and getInstance() calls (of keyStore, KeyPairGenerator, etc) can use it. On the other hand, -providerName can be used to specifically tell

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 2/16/2016 22:54, Alan Bateman wrote: On 16/02/2016 14:44, Weijun Wang wrote: Please review the code change at http://cr.openjdk.java.net/~weijun/8130302/webrev.00/ I didn't abandon -providerClass and go all the way to -provideName because -providerClass has a sub-option -providerArg

Re: RFR 8130302: jarsigner and keytool -providerClass needs be re-examined for modules

On 16/02/2016 14:44, Weijun Wang wrote: Please review the code change at http://cr.openjdk.java.net/~weijun/8130302/webrev.00/ I didn't abandon -providerClass and go all the way to -provideName because -providerClass has a sub-option -providerArg that can be used to further configure the