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 <bradford.wetmore at oracle.com> 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
>>>>>> <bradford.wetmore at oracle.com> 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
>>>>>>
>>>>>
>>>
>