Review request: 8040059 Change default policy for extensions to no permission

[Mandy Chung]

On 4/25/2014 12:55 AM, Erik Joelsson wrote:
> Hello Mandy,
>
> The logic looks fine. Just some style issues. I would like indentation 
> for the conditionals to be 2 spaces as is currently the standard in 
> the makefiles. I would also like to have POLICY_SRC_LIST to be 
> declared empty with := instead of just =. We only use = assignment 
> when explicitly needed.

Thanks for the review Erik.  I'll make the change before the push.

Mandy

>
> /Erik
>
> On 2014-04-25 01:07, Mandy Chung wrote:
>> Thanks Sean.
>>
>> I have updated the webrev:
>> http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8040059/webrev.01/
>>
>> Erik - I'm including build-dev to review the build change for 
>> java.policy file.
>>
>> Thanks
>> Mandy
>>
>> On 4/24/14 11:32 AM, Sean Mullan wrote:
>>> On 04/23/2014 05:29 PM, Mandy Chung wrote:
>>>>
>>>> On 4/23/2014 1:10 PM, Sean Mullan wrote:
>>>>> Just a few comments:
>>>>>
>>>>> 1. When you write a test that uses the jtreg /policy option, the
>>>>> policy file overrides the system policy file. If the test depends 
>>>>> on a
>>>>> standard extension, then you may get SecurityExceptions unless
>>>>> additional perms are granted. Thus, there are quite a few tests that
>>>>> define their own policy files and duplicate the grant statement for
>>>>> extensions from the system policy:
>>>>>
>>>>>     grant codeBase "file:${{java.ext.dirs}}/*" {
>>>>>         permission java.security.AllPermission;
>>>>>     }
>>>>>
>>>>> These tests should be modified to only grant the necessary
>>>>> permissions. However, ideally I think that a better solution would be
>>>>> to add a jtreg /policy option that doesn't override the system policy
>>>>> file, but rather appends to it, for example, using "==" :
>>>>>
>>>>
>>>> I suspect most of the tests only want to grant the permissions for the
>>>> test itself rather than overriding the system policy file. Having a 
>>>> new
>>>> jtreg/policy option not to override the system policy file may be a
>>>> better solution.    This would avoid updating the test's policy file
>>>> every time the system's policy is modified.   On the other hand, I 
>>>> think
>>>> the test policy may not need to grant permissions to the extensions
>>>> directory if it doesn't use the classes in extensions.  It's a good
>>>> opportunity to clean that up. This will require some closer look at 
>>>> the
>>>> tests.
>>>>
>>>> If you are okay, I'd like to separate the test's custom policy 
>>>> update as
>>>> a follow-on work.
>>>
>>> That's fine with me.
>>>
>>>>> @run main/othervm/policy==test.policy
>>>>>
>>>>> (this is the reverse behavior of the java.security.policy system
>>>>> property, so it might be a little confusing, so maybe it is better to
>>>>> add a new option)
>>>>>
>>>>
>>>> "==" is a confusing syntax. -Djava.security.policy==test.policy
>>>> overrides the system policy file ("=" prefix) whereas jtreg uses the
>>>> reverse syntax? I think it would be better to make jtreg/policy
>>>> consistent with -Djava.security.policy (i.e. default is to extend the
>>>> system java.security).
>>>
>>> Would be nice, but not sure if we can change it at this point. 
>>> Anyway, one of us should file a jtreg RFE.
>>>
>>>>> 3. jdk/nio/zipfs/ZipFileSystem.java
>>>>>
>>>>> If I understand the changes, the previous code would throw
>>>>> SecurityExceptions when run under a SecurityManager? It's not
>>>>> specifically related to this bug, is it?
>>>>>
>>>>
>>>> It's a bug in the zipfs provider and I can file a new JBS issue for 
>>>> the
>>>> zipfs change.  I prefer to push them in the same changeset that 
>>>> reduces
>>>> zipfs's privileges and added tests to run with security manager.
>>>
>>> Sure, it is fine with me to include them with this. I just wanted to 
>>> make sure I understood the changes.
>>>
>>> --Sean
>>
>