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=<file> 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
>>>> <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
>>>>>>>>
>>>>>>>
>>>>>
>>>
>