[9] RFR:8130360: Add tests to verify 3rd party security providers if they are in signed/unsigned modular JARs

Sibabrata Sahoo sibabrata.sahoo at oracle.com
Tue Dec 22 06:17:06 UTC 2015


Hi Max/Mandy,

Here is the updated webrev: http://cr.openjdk.java.net/~asmotrak/siba/8130360/webrev.05/

I tried to address all the comments bellow. The major change includes abstracting the common behavior to ModularTest.java and extend the rest by each specific test. i.e. SecurityProviderModularTest and JaasModularClientTest.

Thanks,
Siba

-----Original Message-----
From: Wang Weijun 
Sent: Monday, December 21, 2015 7:20 AM
To: Sibabrata Sahoo
Cc: Mandy Chung; Valerie Peng; jigsaw-dev at openjdk.java.net; security-dev at openjdk.java.net
Subject: Re: [9] RFR:8130360: Add tests to verify 3rd party security providers if they are in signed/unsigned modular JARs

Tests are good. Several comments:

1. Try run something like "hg mv -A" to let Mercurial knows that you are renaming files (SecurityUtils.java and those in login/modules/src) instead of removing some old and creating some new. The current webrev does not show this.

2. It's not a good practice extending a class just to use its static fields/methods. Instead, make the util class final.

   public class JaasModularClientTest extends SecurityUtils
   public class SecurityProviderModularTest extends SecurityUtils

In fact, if you do find similarities between SecurityProviderModularTest and JaasModularClientTest, try abstract them into a ModularTest class.

3. Please add enough comments to SecurityUtils since it will be reused. For example, move the descriptions of EXPLICIT, AUTO and UNNAMED from test @summary to definition of enum MODULE_TYPE.

Thanks
Max


> On Dec 20, 2015, at 2:10 PM, Sibabrata Sahoo <sibabrata.sahoo at oracle.com> wrote:
> 
> Hi Mandy/Max,
> 
> Here is the updated webrev with very minor change over previous: 
> http://cr.openjdk.java.net/~asmotrak/siba/8130360/webrev.04/
> 
> Thanks
> Siba
> 
> -----Original Message-----
> From: Sibabrata Sahoo
> Sent: Sunday, December 20, 2015 1:34 AM
> To: Mandy Chung; Weijun Wang
> Cc: Valerie Peng; jigsaw-dev at openjdk.java.net; 
> security-dev at openjdk.java.net
> Subject: RE: [9] RFR:8130360: Add tests to verify 3rd party security 
> providers if they are in signed/unsigned modular JARs
> 
> Hi Mandy/Max,
> 
> Please review the updated webrev: 
> http://cr.openjdk.java.net/~asmotrak/siba/8130360/webrev.03/
> 
> "3rd party security providers" Test change includes:
> - Test are converted to use TestNG framework.
> - java/security/modules/JigsawSecurityUtils.java, renamed to "SecurityUtils.java". Proper care required while pushing the change because "JigsawSecurityUtils.java" is an existing file in JAKE repository.
> - Common reusable operations moved to "SecurityUtils.java"
> - All the following comments associated.
> 
> 
> Based on the recent comments, I also did some changes to the "JAAS Modular test" because both test are very similar to each other. The change includes:
> - Flat folder structure. It places the Test as well as all its dependency in a single folder. i.e. "javax/security/auth/login/modules". That also means "javax/security/auth/login/modules/src/" and all its subfolders need to be removed.
> - Test converted to use TestNG framework.
> 
> Thanks,
> Siba
> 
> -----Original Message-----
> From: Mandy Chung
> Sent: Wednesday, December 09, 2015 11:55 PM
> To: Sibabrata Sahoo
> Cc: Valerie Peng; jigsaw-dev at openjdk.java.net; 
> security-dev at openjdk.java.net
> Subject: Re: [9] RFR:8130360: Add tests to verify 3rd party security 
> providers if they are in signed/unsigned modular JARs
> 
> Some high-level comment:
> 1. I suggest to rename JigsawSecurityUtils.java to SecurityUtils.java.  The Jigsaw prefix is not needed.
> 2. SecurityProviderModularTest does the setup and launch the test with a set of different settings. I suggest to convert it to TestNG and you can reference the existing test/jdk/jigsaw tests which are a driver test that does the compilation and setup with @BeforeTest and @Test for each run test together with @DataProvider.  [1] is a simple test for you to get started.
> 
> Some comments on JigsawSecurityUtils.java:
> 71         Arrays.asList(options).stream().forEach(o -> command.append(SPACE + o));
> 
> this can be replaced with 
>    Arrays.stream(options).collect(Collectors.joining(SPACE));
> 
> 111         System.out.println(String.format(
> 
> You can do:  System.out.format(….) instead;
> 
> Mandy
> [1] 
> http://hg.openjdk.java.net/jigsaw/jake/jdk/file/c1d583efa466/test/jdk/
> jigsaw/reflect/Proxy/ProxyTest.java
> 
>> On Dec 9, 2015, at 10:02 AM, Sibabrata Sahoo <sibabrata.sahoo at oracle.com> wrote:
>> 
>> Hi Valerie,
>> 
>> Here is the updated webrev: 
>> http://cr.openjdk.java.net/~ntv/siba/webrev.00/
>> 
>> Changes applied as per previous comments.
>> - File names are modified and do not include the term JCE.
>> - Source folder structure is flat now and all belongs to "java/security/Provider" folder.
>> - Sign jar functionality removed.
>> 
>> Thanks,
>> Siba
>> 
>> -----Original Message-----
>> From: Valerie Peng
>> Sent: Wednesday, December 09, 2015 5:37 AM
>> To: Sibabrata Sahoo
>> Cc: jigsaw-dev at openjdk.java.net; security-dev at openjdk.java.net; Alan 
>> Bateman
>> Subject: Re: [9] RFR:8130360: Add tests to verify 3rd party security 
>> providers if they are in signed/unsigned modular JARs
>> 
>> Hi Siba,
>> 
>> Here are some nits:
>> 
>> I think it's somewhat misleading to use the term JCE here as what you are testing here is just security provider loading. JCE is more about security providers supporting export-controlled services/algorithms.
>> Since your provider is just an empty one, I don't think u need to sign it (again, it's only for JCE providers).
>> You can remove a lot of code as a result.
>> Also, your directory seems a bit deep, e.g. 
>> modules/src/jceprovidermodule/provider/TestJCEProvider.java vs modules/src/jceclientmodule/provider/TestJCEProvider.java. Can all of these files be under the same directory instead of spreading over several level of subdirectories? The file names are different enough to tell which is which. Other regression tests for provider loading functionality are under test/java/security/Provider. Easier to find this way.
>> 
>> Can you please make the appropriate changes, e.g. don't use the term JCE, get rid of the signing, and simplify the directory structure if possible?
>> 
>> Thanks,
>> Valerie
>> 
>> On 12/8/2015 2:04 PM, Valerie Peng wrote:
>>> Right, that'd be my expectation as well. Sounds like everything works.
>>> I will change to look at your latest webrev.
>>> Valerie
>>> 
>>> On 12/8/2015 6:09 AM, Sibabrata Sahoo wrote:
>>>> Hi Valerie,
>>>> 
>>>> Here is the updated webrev: 
>>>> http://cr.openjdk.java.net/~ralexander/8130360/webrev.00/
>>>> 
>>>> Now the modular behavior for the test works as per expectation 
>>>> through JAKE build with the following condition.
>>>> If the provider jar is available under ModulePath then the 
>>>> "java.security" file should have the provider configuration entry 
>>>> as ProviderName while in case of ClassPath the entry should hold 
>>>> the full class name.
>>>> 
>>>> Thanks,
>>>> Siba
>>>> 
>>>> -----Original Message-----
>>>> From: Valerie Peng
>>>> Sent: Tuesday, December 08, 2015 4:46 AM
>>>> To: Sibabrata Sahoo
>>>> Cc: Alan Bateman; security-dev at openjdk.java.net; 
>>>> jigsaw-dev at openjdk.java.net
>>>> Subject: Re: [9] RFR:8130360: Add tests to verify 3rd party 
>>>> security providers if they are in signed/unsigned modular JARs
>>>> 
>>>> Siba,
>>>> 
>>>> I have just started to review this webrev and not done yet.
>>>> As for your question, the java.security file in OpenJDK9 still uses 
>>>> the provider class names instead of provider names. Are you talking 
>>>> about the java.security file in Jigsaw? Which build (OpenJDK or
>>>> Jigsaw) have you tested against. I am pretty sure that I tested the 
>>>> 3rd party provider on classpath setting when I worked on the 
>>>> 7191662 changes.
>>>> 
>>>> Supposedly, if the jar files are available on class path, then you 
>>>> should specify its full *class name* in the java.security file for 
>>>> it to be instantiated. Otherwise, how would it find the class? Only 
>>>> the classes discovered by ServiceLoader can be identified using the 
>>>> provider name (which is different from the class name referred 
>>>> above). So, in your scenario, that would be 
>>>> "provider.TestJCEProvider" instead of "TEST".
>>>> 
>>>> If you still run into problems, try enable the java security debug 
>>>> flag and u should get a good  idea why it isn't loaded. Let me know 
>>>> if you still have questions.
>>>> 
>>>> Thanks,
>>>> Valerie
>>>> 
>>>> On 11/30/2015 6:47 AM, Sibabrata Sahoo wrote:
>>>>> I would like to know more about this. As far, I can see the 
>>>>> "java.security" provider configuration available with JDK9, it 
>>>>> holds provider names instead of provider class names. In that case 
>>>>> how it resolve the fall back?
>>>>> 
>>>>> Thanks,
>>>>> Siba
>>>>> 
>>>>> -----Original Message-----
>>>>> From: Alan Bateman
>>>>> Sent: Monday, November 30, 2015 4:54 PM
>>>>> To: Sibabrata Sahoo; security-dev at openjdk.java.net; 
>>>>> jigsaw-dev at openjdk.java.net
>>>>> Subject: Re: [9] RFR:8130360: Add tests to verify 3rd party 
>>>>> security providers if they are in signed/unsigned modular JARs
>>>>> 
>>>>> On 30/11/2015 11:13, Sibabrata Sahoo wrote:
>>>>>> Here is the updated webrev:
>>>>>> http://cr.openjdk.java.net/~asmotrak/siba/8130360/webrev.02/
>>>>>> 
>>>>>> I have one question:
>>>>>> What should be the behavior when the older version of 3rd party 
>>>>>> JCE provider jar file(without service descriptor
>>>>>> "META-INF/services/*"&   working with<= JDK8) configured by 
>>>>>> "java.security" file, will be place in CLASS_PATH, running 
>>>>>> through
>>>>>> JDK9 and the client is using Security.getProvider() to look for 
>>>>>> the provider?
>>>>>> 
>>>>>> Currently the scenario fails to find the JCE provider. Is this 
>>>>>> right behavior? If it is, then jdk9 is not backward compatible to 
>>>>>> find the security provider provided through older jar files from 
>>>>>> CLASS_PATH.
>>>>>> 
>>>>> The JCE work in JDK 9 (via JDK-7191662) was meant to address this 
>>>>> point by falling back and attempting to load the class name
>>>>> specified via the security.provider.<N>   properties in the 
>>>>> java.security file. I'm sure Valerie can say more about this.
>>>>> 
>>>>> -Alan
> 



More information about the jigsaw-dev mailing list