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

[Bradford Wetmore]

I've updated to:

 * @run main/othervm CryptoPolicyFallback

I'll have a new review out shortly.

Brad


On 11/23/2016 2:29 AM, Wang Weijun wrote:

Hi Brad

I think I found a problem with the test. Before you set your local
java.security file, the system java.security file was already read (in
jtreg initialization) and limited was picked up. In fact, I modified the
java.security file of my own JDK to unlimited and the test fails.

This seems to show that we have to set the system property on the
command line. Either we provide a modified java.security with the test
like SeanM suggested, or we create it dynamically and manually launch a
new java process. I prefer the latter.

Thanks
Max

On 11/18/2016 1:46 AM, Bradford Wetmore wrote:

SeanM pointed out that we could do a:

@main -Djava.security.properties=xxx

but that would require storing a snapshot of java.security.  I think I
prefer it being dynamically generated.

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

[Seán Coffey]

On 17/11/2016 17:46, Bradford Wetmore wrote:



On 11/17/2016 7:19 AM, Seán Coffey wrote:

The /policy= jtreg tag was also another possible solution.


That's a policy file, not a java.security file.

Ah, of course. Wasn't thinking right.

regards,
Sean.


SeanM pointed out that we could do a:

@main -Djava.security.properties=xxx

but that would require storing a snapshot of java.security.  I think I 
prefer it being dynamically generated.


Brad



regards,
Sean.


On 17/11/2016 01:33, Bradford Wetmore wrote:



On 11/16/2016 4:14 PM, Wang Weijun wrote:



On Nov 17, 2016, at 6:10 AM, Bradford Wetmore
 wrote:


Current iteration:

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

Changes:

1.  Using Debug "jca" instead of "policy"

2.  Using debug.println (System.err), as the other jca calls are
also using it.


Still have a question:

Why must both (debug != null) and Debug.isOn("jca") be checked? If
Debug.getInstance("jca") is not null, shouldn't Debug.isOn("jca")
always be true?


Ok, I think I've finally figured it out.  For namespaces like JSSE and
x509, there is a "main" namespace like "ssl" or "x509", and then
subnames such as "plaintext" and "ava" which allow for more specific
output selection criteria (e.g. ssl/trustmanager).  For this case, I
don't need to get more specific, so I can skip the isOn.

(My first thought was that maybe older versions of the JDK would allow
for dynamic changes of the variable, but never saw that kind of code
anywhere.)



3.  Added regression test.  Strips out any crypto.policy entry to
create a new file, then uses it.


Looks fine, but I heard you can use some cool jdk8 classes like

   for (in = Files.lines(input); out = new PrintWriter(output)) {
  in.filter(x ->
!x.contains("crypto.policy")).forEach(out::println);
   }


Or:

try (PrintWriter out = new PrintWriter(FILENAME)) {
Files.lines(path)
.filter(x -> !x.trim().startsWith("crypto.policy"))
.forEach(out::println);
}


Latest at:

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

Brad




--Max




4.  Updated webrev with bugid/reviewers.

Brad




On 11/16/2016 6:21 AM, Seán Coffey wrote:

In the recent jdk8u-dev edits of this file for 8157561, we
introduced a
debug field based on this key :

Debug.getInstance("jca", "Cipher");

Can we continue to use 'jca' to be consistent for people upgrading ?

for the testcase, I guess you can edit
test/javax/crypto/CryptoPermission/TestUnlimited.java but you'll
have to
launch with a customized java.security file which doesn't have
crypto.policy set. (Security.setProperty doesn't allow null values)

Regards,
Sean.

On 16/11/16 00:40, Bradford Wetmore wrote:
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

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

[Bradford Wetmore]

On 11/17/2016 7:19 AM, Seán Coffey wrote:

The /policy= jtreg tag was also another possible solution.


That's a policy file, not a java.security file.

SeanM pointed out that we could do a:

@main -Djava.security.properties=xxx

but that would require storing a snapshot of java.security.  I think I 
prefer it being dynamically generated.


Brad



regards,
Sean.


On 17/11/2016 01:33, Bradford Wetmore wrote:



On 11/16/2016 4:14 PM, Wang Weijun wrote:



On Nov 17, 2016, at 6:10 AM, Bradford Wetmore
 wrote:


Current iteration:

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

Changes:

1.  Using Debug "jca" instead of "policy"

2.  Using debug.println (System.err), as the other jca calls are
also using it.


Still have a question:

Why must both (debug != null) and Debug.isOn("jca") be checked? If
Debug.getInstance("jca") is not null, shouldn't Debug.isOn("jca")
always be true?


Ok, I think I've finally figured it out.  For namespaces like JSSE and
x509, there is a "main" namespace like "ssl" or "x509", and then
subnames such as "plaintext" and "ava" which allow for more specific
output selection criteria (e.g. ssl/trustmanager).  For this case, I
don't need to get more specific, so I can skip the isOn.

(My first thought was that maybe older versions of the JDK would allow
for dynamic changes of the variable, but never saw that kind of code
anywhere.)



3.  Added regression test.  Strips out any crypto.policy entry to
create a new file, then uses it.


Looks fine, but I heard you can use some cool jdk8 classes like

   for (in = Files.lines(input); out = new PrintWriter(output)) {
  in.filter(x ->
!x.contains("crypto.policy")).forEach(out::println);
   }


Or:

try (PrintWriter out = new PrintWriter(FILENAME)) {
Files.lines(path)
.filter(x -> !x.trim().startsWith("crypto.policy"))
.forEach(out::println);
}


Latest at:

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

Brad




--Max




4.  Updated webrev with bugid/reviewers.

Brad




On 11/16/2016 6:21 AM, Seán Coffey wrote:

In the recent jdk8u-dev edits of this file for 8157561, we
introduced a
debug field based on this key :

Debug.getInstance("jca", "Cipher");

Can we continue to use 'jca' to be consistent for people upgrading ?

for the testcase, I guess you can edit
test/javax/crypto/CryptoPermission/TestUnlimited.java but you'll
have to
launch with a customized java.security file which doesn't have
crypto.policy set. (Security.setProperty doesn't allow null values)

Regards,
Sean.

On 16/11/16 00:40, Bradford Wetmore wrote:

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

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

[Seán Coffey]

Looks good to me. I didn't know you could pass a plain file 'name' to 
java.security.properties. The docs indicate that a URL is required but 
the jdk code suggests your approach will work.


The /policy= jtreg tag was also another possible solution.

regards,
Sean.


On 17/11/2016 01:33, Bradford Wetmore wrote:



On 11/16/2016 4:14 PM, Wang Weijun wrote:


On Nov 17, 2016, at 6:10 AM, Bradford Wetmore 
 wrote:



Current iteration:

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

Changes:

1.  Using Debug "jca" instead of "policy"

2.  Using debug.println (System.err), as the other jca calls are 
also using it.


Still have a question:

Why must both (debug != null) and Debug.isOn("jca") be checked? If 
Debug.getInstance("jca") is not null, shouldn't Debug.isOn("jca") 
always be true?


Ok, I think I've finally figured it out.  For namespaces like JSSE and 
x509, there is a "main" namespace like "ssl" or "x509", and then 
subnames such as "plaintext" and "ava" which allow for more specific 
output selection criteria (e.g. ssl/trustmanager).  For this case, I 
don't need to get more specific, so I can skip the isOn.


(My first thought was that maybe older versions of the JDK would allow 
for dynamic changes of the variable, but never saw that kind of code 
anywhere.)




3.  Added regression test.  Strips out any crypto.policy entry to 
create a new file, then uses it.


Looks fine, but I heard you can use some cool jdk8 classes like

   for (in = Files.lines(input); out = new PrintWriter(output)) {
  in.filter(x -> 
!x.contains("crypto.policy")).forEach(out::println);

   }


Or:

try (PrintWriter out = new PrintWriter(FILENAME)) {
Files.lines(path)
.filter(x -> !x.trim().startsWith("crypto.policy"))
.forEach(out::println);
}


Latest at:

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

Brad




--Max




4.  Updated webrev with bugid/reviewers.

Brad




On 11/16/2016 6:21 AM, Seán Coffey wrote:
In the recent jdk8u-dev edits of this file for 8157561, we 
introduced a

debug field based on this key :

Debug.getInstance("jca", "Cipher");

Can we continue to use 'jca' to be consistent for people upgrading ?

for the testcase, I guess you can edit
test/javax/crypto/CryptoPermission/TestUnlimited.java but you'll 
have to

launch with a customized java.security file which doesn't have
crypto.policy set. (Security.setProperty doesn't allow null values)

Regards,
Sean.

On 16/11/16 00:40, Bradford Wetmore wrote:

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

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

[Wang Weijun]

> On Nov 17, 2016, at 9:33 AM, Bradford Wetmore  
> wrote:
> 
>try (PrintWriter out = new PrintWriter(FILENAME)) {
>Files.lines(path)
>.filter(x -> !x.trim().startsWith("crypto.policy"))
>.forEach(out::println);
>}

Not very sure. Although IMHO a terminate method like forEach *should* close the 
stream, the spec has not said so:

* Streams have a {@link #close()} method and implement {@link AutoCloseable}.
* Operating on a stream after it has been closed will throw {@link 
IllegalStateException}.
* Most stream instances do not actually need to be closed after use, as they
* are backed by collections, arrays, or generating functions, which require no
* special resource management. Generally, only streams whose source is an IO 
channel,
* such as those returned by {@link Files#lines(Path)}, will require closing. If 
a
* stream does require closing, it must be opened as a resource within a 
try-with-resources
* statement or similar control structure to ensure that it is closed promptly 
after its
* operations have completed.

I know you use Windows, so if you haven't seen any test failure, maybe it's 
safe.

Thanks
Max

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

[Xuelei Fan]

> http://cr.openjdk.java.net/~wetmore/8169335/webrev.02
Looks fine to me.

BTW, I was just wondering, do you want to allow empty security property 
(cryptoPolicyProperty.length() is 0) as well?



Xuelei

On 11/17/2016 9:33 AM, Bradford Wetmore wrote:



On 11/16/2016 4:14 PM, Wang Weijun wrote:



On Nov 17, 2016, at 6:10 AM, Bradford Wetmore
 wrote:


Current iteration:

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

Changes:

1.  Using Debug "jca" instead of "policy"

2.  Using debug.println (System.err), as the other jca calls are also
using it.


Still have a question:

Why must both (debug != null) and Debug.isOn("jca") be checked? If
Debug.getInstance("jca") is not null, shouldn't Debug.isOn("jca")
always be true?


Ok, I think I've finally figured it out.  For namespaces like JSSE and
x509, there is a "main" namespace like "ssl" or "x509", and then
subnames such as "plaintext" and "ava" which allow for more specific
output selection criteria (e.g. ssl/trustmanager).  For this case, I
don't need to get more specific, so I can skip the isOn.

(My first thought was that maybe older versions of the JDK would allow
for dynamic changes of the variable, but never saw that kind of code
anywhere.)



3.  Added regression test.  Strips out any crypto.policy entry to
create a new file, then uses it.


Looks fine, but I heard you can use some cool jdk8 classes like

   for (in = Files.lines(input); out = new PrintWriter(output)) {
  in.filter(x -> !x.contains("crypto.policy")).forEach(out::println);
   }


Or:

try (PrintWriter out = new PrintWriter(FILENAME)) {
Files.lines(path)
.filter(x -> !x.trim().startsWith("crypto.policy"))
.forEach(out::println);
}


Latest at:

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

Brad




--Max




4.  Updated webrev with bugid/reviewers.

Brad




On 11/16/2016 6:21 AM, Seán Coffey wrote:

In the recent jdk8u-dev edits of this file for 8157561, we introduced a
debug field based on this key :

Debug.getInstance("jca", "Cipher");

Can we continue to use 'jca' to be consistent for people upgrading ?

for the testcase, I guess you can edit
test/javax/crypto/CryptoPermission/TestUnlimited.java but you'll
have to
launch with a customized java.security file which doesn't have
crypto.policy set. (Security.setProperty doesn't allow null values)

Regards,
Sean.

On 16/11/16 00:40, Bradford Wetmore wrote:

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

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

[Bradford Wetmore]

On 11/16/2016 4:14 PM, Wang Weijun wrote:



On Nov 17, 2016, at 6:10 AM, Bradford Wetmore  
wrote:


Current iteration:

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

Changes:

1.  Using Debug "jca" instead of "policy"

2.  Using debug.println (System.err), as the other jca calls are also using it.


Still have a question:

Why must both (debug != null) and Debug.isOn("jca") be checked? If 
Debug.getInstance("jca") is not null, shouldn't Debug.isOn("jca") always be true?


Ok, I think I've finally figured it out.  For namespaces like JSSE and 
x509, there is a "main" namespace like "ssl" or "x509", and then 
subnames such as "plaintext" and "ava" which allow for more specific 
output selection criteria (e.g. ssl/trustmanager).  For this case, I 
don't need to get more specific, so I can skip the isOn.


(My first thought was that maybe older versions of the JDK would allow 
for dynamic changes of the variable, but never saw that kind of code 
anywhere.)




3.  Added regression test.  Strips out any crypto.policy entry to create a new 
file, then uses it.


Looks fine, but I heard you can use some cool jdk8 classes like

   for (in = Files.lines(input); out = new PrintWriter(output)) {
  in.filter(x -> !x.contains("crypto.policy")).forEach(out::println);
   }


Or:

try (PrintWriter out = new PrintWriter(FILENAME)) {
Files.lines(path)
.filter(x -> !x.trim().startsWith("crypto.policy"))
.forEach(out::println);
}


Latest at:

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

Brad




--Max




4.  Updated webrev with bugid/reviewers.

Brad




On 11/16/2016 6:21 AM, Seán Coffey wrote:

In the recent jdk8u-dev edits of this file for 8157561, we introduced a
debug field based on this key :

Debug.getInstance("jca", "Cipher");

Can we continue to use 'jca' to be consistent for people upgrading ?

for the testcase, I guess you can edit
test/javax/crypto/CryptoPermission/TestUnlimited.java but you'll have to
launch with a customized java.security file which doesn't have
crypto.policy set. (Security.setProperty doesn't allow null values)

Regards,
Sean.

On 16/11/16 00:40, Bradford Wetmore wrote:

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

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

[Wang Weijun]

> On Nov 17, 2016, at 6:10 AM, Bradford Wetmore  
> wrote:
> 
> 
> Current iteration:
> 
>http://cr.openjdk.java.net/~wetmore/8169335/webrev.01
> 
> Changes:
> 
> 1.  Using Debug "jca" instead of "policy"
> 
> 2.  Using debug.println (System.err), as the other jca calls are also using 
> it.

Still have a question:

Why must both (debug != null) and Debug.isOn("jca") be checked? If 
Debug.getInstance("jca") is not null, shouldn't Debug.isOn("jca") always be 
true?

> 
> 3.  Added regression test.  Strips out any crypto.policy entry to create a 
> new file, then uses it.

Looks fine, but I heard you can use some cool jdk8 classes like

   for (in = Files.lines(input); out = new PrintWriter(output)) {
  in.filter(x -> !x.contains("crypto.policy")).forEach(out::println);
   }

--Max


> 
> 4.  Updated webrev with bugid/reviewers.
> 
> Brad
> 
> 
> 
> 
> On 11/16/2016 6:21 AM, Seán Coffey wrote:
>> In the recent jdk8u-dev edits of this file for 8157561, we introduced a
>> debug field based on this key :
>> 
>> Debug.getInstance("jca", "Cipher");
>> 
>> Can we continue to use 'jca' to be consistent for people upgrading ?
>> 
>> for the testcase, I guess you can edit
>> test/javax/crypto/CryptoPermission/TestUnlimited.java but you'll have to
>> launch with a customized java.security file which doesn't have
>> crypto.policy set. (Security.setProperty doesn't allow null values)
>> 
>> Regards,
>> Sean.
>> 
>> On 16/11/16 00:40, Bradford Wetmore wrote:
>>> 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
> 
 
>> 

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

[Bradford Wetmore]

Current iteration:

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

Changes:

1.  Using Debug "jca" instead of "policy"

2.  Using debug.println (System.err), as the other jca calls are also 
using it.


3.  Added regression test.  Strips out any crypto.policy entry to create 
a new file, then uses it.


4.  Updated webrev with bugid/reviewers.

Brad




On 11/16/2016 6:21 AM, Seán Coffey wrote:

In the recent jdk8u-dev edits of this file for 8157561, we introduced a
debug field based on this key :

Debug.getInstance("jca", "Cipher");

Can we continue to use 'jca' to be consistent for people upgrading ?

for the testcase, I guess you can edit
test/javax/crypto/CryptoPermission/TestUnlimited.java but you'll have to
launch with a customized java.security file which doesn't have
crypto.policy set. (Security.setProperty doesn't allow null values)

Regards,
Sean.

On 16/11/16 00:40, Bradford Wetmore wrote:

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

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

[Seán Coffey]

In the recent jdk8u-dev edits of this file for 8157561, we introduced a 
debug field based on this key :


Debug.getInstance("jca", "Cipher");

Can we continue to use 'jca' to be consistent for people upgrading ?

for the testcase, I guess you can edit 
test/javax/crypto/CryptoPermission/TestUnlimited.java but you'll have to 
launch with a customized java.security file which doesn't have 
crypto.policy set. (Security.setProperty doesn't allow null values)


Regards,
Sean.

On 16/11/16 00:40, Bradford Wetmore wrote:
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

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

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
> 

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