Re: RFR 8043252: Debug of access control is obfuscated - NullPointerException in ProtectionDomain

[Wang Weijun]

> On Nov 16, 2016, at 12:40 PM, Jamil Nimeh  wrote:
> 
> Oops, good catch. I'll fix that.  I assume you don't need a third review for 
> that?

No. Just push your change.

--Max

> 
> 
> 
> --Jamil
> 
>  Original message 
> From: Wang Weijun 
> Date: 11/15/16 7:48 PM (GMT-08:00)
> To: Jamil Nimeh 
> Cc: security-dev@openjdk.java.net
> Subject: Re: RFR 8043252: Debug of access control is obfuscated - 
> NullPointerException in ProtectionDomain
> 
> Looks fine.
> 
> Small nit: new test has copyright 2003, 2016.
> 
> --Max
> 
> > On Nov 16, 2016, at 11:45 AM, Jamil Nimeh  wrote:
> > 
> > Good suggestions, thanks.
> > 
> > Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8043252/webrev.02
> > 
> > --Jamil
> > 
> > 
> > On 11/15/2016 06:18 PM, Wang Weijun wrote:
> >> 41 System.setSecurityManager(new SecurityManager());
> >> 
> >> This is strange. I suppose you only want to trigger a permission check?
> >> 
> >> If so, just change it to Policy.getPolicy(), and you can clean up the 
> >> policy file a little.
> >> 
> >> Thanks
> >> Max
> >> 
> >>> On Nov 16, 2016, at 8:36 AM, Jamil Nimeh  wrote:
> >>> 
> >>> Hello all,
> >>> 
> >>> This fixes an issue in ProtectionDomain, where Permission classes that 
> >>> take a loose interpretation of the getActions() method and return null 
> >>> cause an NPE to be thrown when a ProtectionDomain's toString method is 
> >>> called (during debugging for instance).
> >>> 
> >>> Bug: https://bugs.openjdk.java.net/browse/JDK-8043252
> >>> Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8043252/webrev.01/
> >>> 
> >>> Thanks,
> >>> --Jamil
> > 
> 

Re: RFR 8043252: Debug of access control is obfuscated - NullPointerException in ProtectionDomain

[Jamil Nimeh]

Oops, good catch. I'll fix that.  I assume you don't need a third review for 
that?


--Jamil
 Original message From: Wang Weijun  
Date: 11/15/16  7:48 PM  (GMT-08:00) To: Jamil Nimeh  
Cc: security-dev@openjdk.java.net Subject: Re: RFR 8043252: Debug of access 
control is obfuscated - NullPointerException in ProtectionDomain 
Looks fine.

Small nit: new test has copyright 2003, 2016.

--Max

> On Nov 16, 2016, at 11:45 AM, Jamil Nimeh  wrote:
> 
> Good suggestions, thanks.
> 
> Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8043252/webrev.02
> 
> --Jamil
> 
> 
> On 11/15/2016 06:18 PM, Wang Weijun wrote:
>> 41 System.setSecurityManager(new SecurityManager());
>> 
>> This is strange. I suppose you only want to trigger a permission check?
>> 
>> If so, just change it to Policy.getPolicy(), and you can clean up the policy 
>> file a little.
>> 
>> Thanks
>> Max
>> 
>>> On Nov 16, 2016, at 8:36 AM, Jamil Nimeh  wrote:
>>> 
>>> Hello all,
>>> 
>>> This fixes an issue in ProtectionDomain, where Permission classes that take 
>>> a loose interpretation of the getActions() method and return null cause an 
>>> NPE to be thrown when a ProtectionDomain's toString method is called 
>>> (during debugging for instance).
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8043252
>>> Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8043252/webrev.01/
>>> 
>>> Thanks,
>>> --Jamil
> 

Re: [9] RFR: 8168969: Merge SSLSocketSample and SSLSocketTemplate

[Xuelei Fan]

I don't like it, but as the 1st stage of the improvement, it looks fine 
to me.


Xuelei

On 11/16/2016 10:52 AM, Xuelei Fan wrote:

I don't like it, too.  It is not necessary to make the merge in a hurry.
 Maybe, we can wait for a while so that we have time for a real and
comfortable template.

Thanks,
Xuelei

On 11/16/2016 10:44 AM, Artem Smotrakov wrote:

I don't like it, but no lambdas, no nothing in SSLSocketTemplate.java

http://cr.openjdk.java.net/~asmotrak/8168969/webrev.01/

Xuelei,

Could you please take a look?

Artem


On 11/02/2016 09:14 AM, Artem Smotrakov wrote:

Hi Xuelei,

This totally makes sense. But in my opinion we should use new features
and APIs because:
- they are here to make code readable (okay, not to much lambdas)
- they help to avoid bugs (e.g., forgetting to close resources)
- we implicitly provide test coverage for new APIs
- we give examples how these features can be used
- finally, we encourage people to move to new Java versions

Not sure if I got correction what you mean by removing the part to
declare the store files in each sub-classes. Do you mean getting rid
of setting keystore/truststore in actual test files?

If so, I would prefer to do it in a separate enhancement. Most of
SSL/TLS tests use keystores from "etc" directory. The test use
"test.src" system property (set by jtreg) to build a path to "etc",
and it depends on actual directory where a test stays. Also if I
recall correctly, some tests use their own keystore. This update may
be quite big. Would you mind if we did it separately?

8168969 is about merging helper classes to remove possible confusions
Brad mentioned recently.

Artem

On 11/02/2016 03:57 AM, Xuelei Fan wrote:

Please use JDK 6 only features (no Lambda, try-with-resource,
multi-catch, java.util.Objects, etc.) so as to expedite porting to
previous releases.

It would be nice if removing the part to declare the store files in
each sub-classes.

Xuelei

On 11/2/2016 2:48 PM, John Jiang wrote:

Hi Artem,
Thanks for making the template to be used easily.

The tests in your patch extend class SSLSocketTemplate, but
SSLSocketTemplate looks like an utility class, but not a parent class.
For example,
test/sun/security/ssl/ServerHandshaker/AnonCipherWithWantClientAuth.java


  67 new SSLSocketTemplate()
AnonCipherWithWantClientAuth still creates a SSLSocketTemplate
instance,
but not itself.
In fact, if replacing "print()" with "SSLSocketTemplate.print()",
AnonCipherWithWantClientAuth can work fine without extending
SSLSocketTemplate.

The original SSLSocketSample.java defines the core methods, such as
doServerSide(), runServerApplication(), doClientSide() and
runClientApplication(), as non-static. In my eyes, this style may be
better. The child class can simply override the methods if necessary.

In addition, some tests have to configure SSLServerSocket. Why not
provide one more extension point in doServerSide()? Then, it
unnecessary
to re-write the whole doServerSide() (or, set a new server peer).
The code talks more clearly. Please take a look at my example:
http://cr.openjdk.java.net/~jjiang/8168969/example/

Best regards,
John Jiang

On 2016/11/2 8:54, Artem Smotrakov wrote:

Hello,

Please review the following patch which merges a couple of classes in
javax/net/ssl/templates.

SSLTest class contains re-usable parts of SSLSocketSample.
SSLSocketTemplate class is buggy (tests which follows it may fail
intermittently). I basically replaced SSLSocketTemplate with SSLTest,
and removed SSLSocketSample.

SSL/TLS tests should use SSLSocketTemplate class. I updated test
which
use SSLTest to use SSLSocketTemplate.

Bug: https://bugs.openjdk.java.net/browse/JDK-8168969
Webrev: http://cr.openjdk.java.net/~asmotrak/8168969/webrev.00/

Artem

Re: RFR 8043252: Debug of access control is obfuscated - NullPointerException in ProtectionDomain

[Wang Weijun]

Looks fine.

Small nit: new test has copyright 2003, 2016.

--Max

> On Nov 16, 2016, at 11:45 AM, Jamil Nimeh  wrote:
> 
> Good suggestions, thanks.
> 
> Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8043252/webrev.02
> 
> --Jamil
> 
> 
> On 11/15/2016 06:18 PM, Wang Weijun wrote:
>> 41 System.setSecurityManager(new SecurityManager());
>> 
>> This is strange. I suppose you only want to trigger a permission check?
>> 
>> If so, just change it to Policy.getPolicy(), and you can clean up the policy 
>> file a little.
>> 
>> Thanks
>> Max
>> 
>>> On Nov 16, 2016, at 8:36 AM, Jamil Nimeh  wrote:
>>> 
>>> Hello all,
>>> 
>>> This fixes an issue in ProtectionDomain, where Permission classes that take 
>>> a loose interpretation of the getActions() method and return null cause an 
>>> NPE to be thrown when a ProtectionDomain's toString method is called 
>>> (during debugging for instance).
>>> 
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8043252
>>> Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8043252/webrev.01/
>>> 
>>> Thanks,
>>> --Jamil
> 

Re: RFR 8043252: Debug of access control is obfuscated - NullPointerException in ProtectionDomain

[Jamil Nimeh]

Good suggestions, thanks.

Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8043252/webrev.02

--Jamil


On 11/15/2016 06:18 PM, Wang Weijun wrote:

41 System.setSecurityManager(new SecurityManager());

This is strange. I suppose you only want to trigger a permission check?

If so, just change it to Policy.getPolicy(), and you can clean up the policy 
file a little.

Thanks
Max


On Nov 16, 2016, at 8:36 AM, Jamil Nimeh  wrote:

Hello all,

This fixes an issue in ProtectionDomain, where Permission classes that take a 
loose interpretation of the getActions() method and return null cause an NPE to 
be thrown when a ProtectionDomain's toString method is called (during debugging 
for instance).

Bug: https://bugs.openjdk.java.net/browse/JDK-8043252
Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8043252/webrev.01/

Thanks,
--Jamil

Re: RFR 8169751: sun/security/krb5/auto/rcache_usemd5.sh fails on solaris

[Wang Weijun]

> On Nov 16, 2016, at 11:30 AM, Xuelei Fan  wrote:
> 
> Looks like a little bit hack, but fine to me.

Thanks.

> 
> BTW, is it more convenient to have 2 run tags in ReplayCacheTestProc.java?

I'd like to run ReplayCacheTestProc.java manually (still using jtreg) with 
various test.* properties. Having 2 run tags double the time and not what I 
wanted.

--Max

> 
> Xuelei
> 
> On 11/16/2016 11:23 AM, Wang Weijun wrote:
>> Sorry, the webrev here http://cr.openjdk.java.net/~weijun/8169751/webrev.00/
>> 
>> --Max
>> 
>>> On Nov 16, 2016, at 10:40 AM, Wang Weijun  wrote:
>>> 
>>> Please take a review at
>>> 
>>>  8169751: sun/security/krb5/auto/rcache_usemd5.sh fails on solaris
>>> 
>>> Different service names are used in the 2 tests (rcache_usemd5.sh and 
>>> ReplayCacheTestProc.java) now.
>>> 
>>> Thanks
>>> Max
>>> 
>> 

Re: RFR 8169751: sun/security/krb5/auto/rcache_usemd5.sh fails on solaris

[Xuelei Fan]

Looks like a little bit hack, but fine to me.

BTW, is it more convenient to have 2 run tags in ReplayCacheTestProc.java?

Xuelei

On 11/16/2016 11:23 AM, Wang Weijun wrote:

Sorry, the webrev here http://cr.openjdk.java.net/~weijun/8169751/webrev.00/

--Max


On Nov 16, 2016, at 10:40 AM, Wang Weijun  wrote:

Please take a review at

  8169751: sun/security/krb5/auto/rcache_usemd5.sh fails on solaris

Different service names are used in the 2 tests (rcache_usemd5.sh and 
ReplayCacheTestProc.java) now.

Thanks
Max

Re: RFR 8169751: sun/security/krb5/auto/rcache_usemd5.sh fails on solaris

[Wang Weijun]

Sorry, the webrev here http://cr.openjdk.java.net/~weijun/8169751/webrev.00/

--Max

> On Nov 16, 2016, at 10:40 AM, Wang Weijun  wrote:
> 
> Please take a review at
> 
>   8169751: sun/security/krb5/auto/rcache_usemd5.sh fails on solaris
> 
> Different service names are used in the 2 tests (rcache_usemd5.sh and 
> ReplayCacheTestProc.java) now.
> 
> Thanks
> Max
> 

Re: RFR 7004967: SecureRandom should be more explicit about threading

[Wang Weijun]

Please review the updated webrev at

   http://cr.openjdk.java.net/~weijun/7004967/webrev.02/

Only spec change [1].

This change also covers 8169312.

Thanks
Max

[1] 
http://cr.openjdk.java.net/~weijun/7004967/webrev.02/interdiff.patch.html


On 11/4/2016 10:54 PM, Sean Mullan wrote:

* SecureRandom

 131  * If this attribute is not set or is "false", this class will instead
 132  * synchronize access to each of the methods of the {@code
SecureRandomSpi}
 133  * implementation.

Not all of the methods are synchronized - engineGetParameters is not,
for example. I think to avoid ambiguity, you should list the names of
the methods that are synchronized.

810  * @throws IllegalArgumentException if {@code numBytes} is negative

Since this is a new @throws, you really need to file a new bug.

Please also file a docs bug with a description of the new attribute.

* SecureRandomSpi

lines 63-83, I think the wording could be improved/simplified, how about:

See {@link SecureRandom} for additional details on thread safety. By
default, a SecureRandomSpi implementation is considered to be not safe
for use by multiple concurrent threads and SecureRandom will synchronize
access to each of the applicable engine methods (see SecureRandom for
the list of methods). However, if a SecureRandomSpi implementation is
thread-safe, the service
provider attribute "ThreadSafe" should be set to "true" during its
registration, as follows:

  put("SecureRandom.AlgName ThreadSafe", "true");

  or

  putService(new Service(this, "SecureRandom", "AlgName", className,
 null, Map.of("ThreadSafe", "true")));

{@code SecureRandom} will then call the applicable engine methods
without any synchronization.

--Sean

On 11/2/16 3:27 AM, Wang Weijun wrote:

Ping again.

There is an updated version at
http://cr.openjdk.java.net/~weijun/7004967/webrev.01/ with doc-only
changes.

Thanks
Max


On Aug 25, 2016, at 10:00 AM, Weijun Wang 
wrote:

Please review the enhancement at

 http://cr.openjdk.java.net/~weijun/7004967/webrev.00/

Basically, we want SecureRandom to be more efficient by removing all
synchronized keywords from its public methods and let an
implementation to take care of thread-safety (We already did some in
JDK-8098581). On the other hand, we need to make sure that existing
implementations that have not synchronized correctly to behave just
as good as before.

Therefore a new Service Attribute "ThreadSafe" is introduced. If you
think your implementation is already thread-safe, set it to "true"
and SecureRandom will be happy. Otherwise, don't set it and
SecureRandom will continuously call your SecureRandomSpi engine
methods in synchronized blocks.

Thanks
Max

Re: [9] RFR: 8168969: Merge SSLSocketSample and SSLSocketTemplate

[Xuelei Fan]

I don't like it, too.  It is not necessary to make the merge in a hurry. 
 Maybe, we can wait for a while so that we have time for a real and 
comfortable template.


Thanks,
Xuelei

On 11/16/2016 10:44 AM, Artem Smotrakov wrote:

I don't like it, but no lambdas, no nothing in SSLSocketTemplate.java

http://cr.openjdk.java.net/~asmotrak/8168969/webrev.01/

Xuelei,

Could you please take a look?

Artem


On 11/02/2016 09:14 AM, Artem Smotrakov wrote:

Hi Xuelei,

This totally makes sense. But in my opinion we should use new features
and APIs because:
- they are here to make code readable (okay, not to much lambdas)
- they help to avoid bugs (e.g., forgetting to close resources)
- we implicitly provide test coverage for new APIs
- we give examples how these features can be used
- finally, we encourage people to move to new Java versions

Not sure if I got correction what you mean by removing the part to
declare the store files in each sub-classes. Do you mean getting rid
of setting keystore/truststore in actual test files?

If so, I would prefer to do it in a separate enhancement. Most of
SSL/TLS tests use keystores from "etc" directory. The test use
"test.src" system property (set by jtreg) to build a path to "etc",
and it depends on actual directory where a test stays. Also if I
recall correctly, some tests use their own keystore. This update may
be quite big. Would you mind if we did it separately?

8168969 is about merging helper classes to remove possible confusions
Brad mentioned recently.

Artem

On 11/02/2016 03:57 AM, Xuelei Fan wrote:

Please use JDK 6 only features (no Lambda, try-with-resource,
multi-catch, java.util.Objects, etc.) so as to expedite porting to
previous releases.

It would be nice if removing the part to declare the store files in
each sub-classes.

Xuelei

On 11/2/2016 2:48 PM, John Jiang wrote:

Hi Artem,
Thanks for making the template to be used easily.

The tests in your patch extend class SSLSocketTemplate, but
SSLSocketTemplate looks like an utility class, but not a parent class.
For example,
test/sun/security/ssl/ServerHandshaker/AnonCipherWithWantClientAuth.java

  67 new SSLSocketTemplate()
AnonCipherWithWantClientAuth still creates a SSLSocketTemplate
instance,
but not itself.
In fact, if replacing "print()" with "SSLSocketTemplate.print()",
AnonCipherWithWantClientAuth can work fine without extending
SSLSocketTemplate.

The original SSLSocketSample.java defines the core methods, such as
doServerSide(), runServerApplication(), doClientSide() and
runClientApplication(), as non-static. In my eyes, this style may be
better. The child class can simply override the methods if necessary.

In addition, some tests have to configure SSLServerSocket. Why not
provide one more extension point in doServerSide()? Then, it
unnecessary
to re-write the whole doServerSide() (or, set a new server peer).
The code talks more clearly. Please take a look at my example:
http://cr.openjdk.java.net/~jjiang/8168969/example/

Best regards,
John Jiang

On 2016/11/2 8:54, Artem Smotrakov wrote:

Hello,

Please review the following patch which merges a couple of classes in
javax/net/ssl/templates.

SSLTest class contains re-usable parts of SSLSocketSample.
SSLSocketTemplate class is buggy (tests which follows it may fail
intermittently). I basically replaced SSLSocketTemplate with SSLTest,
and removed SSLSocketSample.

SSL/TLS tests should use SSLSocketTemplate class. I updated test which
use SSLTest to use SSLSocketTemplate.

Bug: https://bugs.openjdk.java.net/browse/JDK-8168969
Webrev: http://cr.openjdk.java.net/~asmotrak/8168969/webrev.00/

Artem

Re: [9] RFR: 8168969: Merge SSLSocketSample and SSLSocketTemplate

[Artem Smotrakov]

I don't like it, but no lambdas, no nothing in SSLSocketTemplate.java

http://cr.openjdk.java.net/~asmotrak/8168969/webrev.01/

Xuelei,

Could you please take a look?

Artem


On 11/02/2016 09:14 AM, Artem Smotrakov wrote:

Hi Xuelei,

This totally makes sense. But in my opinion we should use new features 
and APIs because:

- they are here to make code readable (okay, not to much lambdas)
- they help to avoid bugs (e.g., forgetting to close resources)
- we implicitly provide test coverage for new APIs
- we give examples how these features can be used
- finally, we encourage people to move to new Java versions

Not sure if I got correction what you mean by removing the part to 
declare the store files in each sub-classes. Do you mean getting rid 
of setting keystore/truststore in actual test files?


If so, I would prefer to do it in a separate enhancement. Most of 
SSL/TLS tests use keystores from "etc" directory. The test use 
"test.src" system property (set by jtreg) to build a path to "etc", 
and it depends on actual directory where a test stays. Also if I 
recall correctly, some tests use their own keystore. This update may 
be quite big. Would you mind if we did it separately?


8168969 is about merging helper classes to remove possible confusions 
Brad mentioned recently.


Artem

On 11/02/2016 03:57 AM, Xuelei Fan wrote:
Please use JDK 6 only features (no Lambda, try-with-resource, 
multi-catch, java.util.Objects, etc.) so as to expedite porting to 
previous releases.


It would be nice if removing the part to declare the store files in 
each sub-classes.


Xuelei

On 11/2/2016 2:48 PM, John Jiang wrote:

Hi Artem,
Thanks for making the template to be used easily.

The tests in your patch extend class SSLSocketTemplate, but
SSLSocketTemplate looks like an utility class, but not a parent class.
For example,
test/sun/security/ssl/ServerHandshaker/AnonCipherWithWantClientAuth.java 


  67 new SSLSocketTemplate()
AnonCipherWithWantClientAuth still creates a SSLSocketTemplate 
instance,

but not itself.
In fact, if replacing "print()" with "SSLSocketTemplate.print()",
AnonCipherWithWantClientAuth can work fine without extending
SSLSocketTemplate.

The original SSLSocketSample.java defines the core methods, such as
doServerSide(), runServerApplication(), doClientSide() and
runClientApplication(), as non-static. In my eyes, this style may be
better. The child class can simply override the methods if necessary.

In addition, some tests have to configure SSLServerSocket. Why not
provide one more extension point in doServerSide()? Then, it 
unnecessary

to re-write the whole doServerSide() (or, set a new server peer).
The code talks more clearly. Please take a look at my example:
http://cr.openjdk.java.net/~jjiang/8168969/example/

Best regards,
John Jiang

On 2016/11/2 8:54, Artem Smotrakov wrote:

Hello,

Please review the following patch which merges a couple of classes in
javax/net/ssl/templates.

SSLTest class contains re-usable parts of SSLSocketSample.
SSLSocketTemplate class is buggy (tests which follows it may fail
intermittently). I basically replaced SSLSocketTemplate with SSLTest,
and removed SSLSocketSample.

SSL/TLS tests should use SSLSocketTemplate class. I updated test which
use SSLTest to use SSLSocketTemplate.

Bug: https://bugs.openjdk.java.net/browse/JDK-8168969
Webrev: http://cr.openjdk.java.net/~asmotrak/8168969/webrev.00/

Artem

RFR 8169751: sun/security/krb5/auto/rcache_usemd5.sh fails on solaris

[Wang Weijun]

Please take a review at

   8169751: sun/security/krb5/auto/rcache_usemd5.sh fails on solaris

Different service names are used in the 2 tests (rcache_usemd5.sh and 
ReplayCacheTestProc.java) now.

Thanks
Max

Re: RFR 8043252: Debug of access control is obfuscated - NullPointerException in ProtectionDomain

[Wang Weijun]

41 System.setSecurityManager(new SecurityManager());

This is strange. I suppose you only want to trigger a permission check?

If so, just change it to Policy.getPolicy(), and you can clean up the policy 
file a little.

Thanks
Max

> On Nov 16, 2016, at 8:36 AM, Jamil Nimeh  wrote:
> 
> Hello all,
> 
> This fixes an issue in ProtectionDomain, where Permission classes that take a 
> loose interpretation of the getActions() method and return null cause an NPE 
> to be thrown when a ProtectionDomain's toString method is called (during 
> debugging for instance).
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8043252
> Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8043252/webrev.01/
> 
> Thanks,
> --Jamil

Re: RFR: 8169335: Add a crypto.policy fallback in case Security Property 'crypto.policy' does not exist

[Bradford Wetmore]

Never noticed that before!  We have NOT been consistent in whether we use:

System.out.println()
or
debug.println()

I knew SeanC wants to rework the JCA/JCE/Security debugging output in 
another project, so I will remove the prefix for now.  Thanks for 
catching it.


I will also add a simple regression Test before I push.  In hindsight, 
it's not as trivial a change as I initially thought.  If you want to 
review it, I can wait until you are back tomorrow.


Brad


On 11/15/2016 4:12 PM, Wang Weijun wrote:

You create a debug field with a prefix string and then check both debug != null and 
Debug.isOn("policy") and then use System.out.println to print the message. 
Something must be useless.

--Max


On Nov 16, 2016, at 3:31 AM, Bradford Wetmore  
wrote:

Simple codereview:

   http://cr.openjdk.java.net/~wetmore/8169335/webrev.00

The "crypto.policy" Security property is normally defined/configured in the java.security file at build time. 
 (e.g. "limited" or "unlimited") Rather than currently failing catastrophically if this value 
doesn't exist, there should be a sensible default if it is undeclared for whatever reason.  We will use a sane fallback 
value of "limited".

If the distribution has also removed the "limited" policy directory then the VM 
will still fail to initialize, but we have at least made an effort to recover.

Thanks,

Brad

RFR 8043252: Debug of access control is obfuscated - NullPointerException in ProtectionDomain

[Jamil Nimeh]

Hello all,

This fixes an issue in ProtectionDomain, where Permission classes that 
take a loose interpretation of the getActions() method and return null 
cause an NPE to be thrown when a ProtectionDomain's toString method is 
called (during debugging for instance).


Bug: https://bugs.openjdk.java.net/browse/JDK-8043252
Webrev: http://cr.openjdk.java.net/~jnimeh/reviews/8043252/webrev.01/

Thanks,
--Jamil

Re: RFR: 8169335: Add a crypto.policy fallback in case Security Property 'crypto.policy' does not exist

[Wang Weijun]

You create a debug field with a prefix string and then check both debug != null 
and Debug.isOn("policy") and then use System.out.println to print the message. 
Something must be useless.

--Max

> On Nov 16, 2016, at 3:31 AM, Bradford Wetmore  
> wrote:
> 
> Simple codereview:
> 
>http://cr.openjdk.java.net/~wetmore/8169335/webrev.00
> 
> The "crypto.policy" Security property is normally defined/configured in the 
> java.security file at build time.  (e.g. "limited" or "unlimited") Rather 
> than currently failing catastrophically if this value doesn't exist, there 
> should be a sensible default if it is undeclared for whatever reason.  We 
> will use a sane fallback value of "limited".
> 
> If the distribution has also removed the "limited" policy directory then the 
> VM will still fail to initialize, but we have at least made an effort to 
> recover.
> 
> Thanks,
> 
> Brad
> 

Re: RFR 8164881: Add more tests for JDK-8139565.

[Artem Smotrakov]

Hi Mallikarjuna,

I have a couple of comments.

1. I see you extract DSA key size from "jdk.certpath.disabledAlgorithms" 
security property. I think I would be better not to rely on it, but 
expect that keys less than 1024 bits are not allowed by default. You can 
pass a boolean parameter to the test which defines what should be expected.


2. We are trying not to use lines more than 80 symbols. Could you please 
fix it?


3. Minor: DSAKeys.java line 342, you don't need to specify types for HashMap

4. It is up to you, but it would be good to update the test to use 
SSTest.java (see examples in jdk/tests) because we've been seeing 
intermittent failures of JSSE tests like this one you are updating


http://hg.openjdk.java.net/jdk9/dev/jdk/file/93fb16cbdf7f/test/javax/net/ssl/templates/SSLTest.java

Artem

On 11/13/2016 09:02 PM, Mallikarjuna Avaluri wrote:


Hi all,

Please review the fix for following issue.

JDK-8164881: Add more tests for JDK-8139565
https://bugs.openjdk.java.net/browse/JDK-8164881

*Summary:* Currently 
test/javax/net/ssl/TLSv12/DisabledShortDSAKeys.java checks only that 
DSA keys with size of 512 bits are disabled.
But we also need to check that DSA keys with sizes 1024 & 2048 are 
working fine.



*Fix: * Currently test/javax/net/ssl/TLSv12/DisabledShortDSAKeys.java 
checks only that DSA keys with size of 512 bits are disabled.
Added new tests with DSA keys with size of 960 bits disabled, 1024, 
2048, 3072 bits enabled.


*Webrev: * 
http://cr.openjdk.java.net/~bgopularam/mavaluri/JDK-8164881/webrev.00/ 




Thanks,
Mallikarjuna Avaluri 

RFR: 8169335: Add a crypto.policy fallback in case Security Property 'crypto.policy' does not exist

[Bradford Wetmore]

Simple codereview:

http://cr.openjdk.java.net/~wetmore/8169335/webrev.00

The "crypto.policy" Security property is normally defined/configured in 
the java.security file at build time.  (e.g. "limited" or "unlimited") 
Rather than currently failing catastrophically if this value doesn't 
exist, there should be a sensible default if it is undeclared for 
whatever reason.  We will use a sane fallback value of "limited".


If the distribution has also removed the "limited" policy directory then 
the VM will still fail to initialize, but we have at least made an 
effort to recover.


Thanks,

Brad