Review request: 8040059 Change default policy for extensions to no permission (original) (raw)

Erik Joelsson erik.joelsson at oracle.com
Fri Apr 25 07:55:45 UTC 2014

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.

/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

More information about the core-libs-dev mailing list