[14] RFR(M) 8185139: [Graal] Tests which set too restrictive security manager fail with Graal

[Sean Mullan]

On 6/20/19 9:02 PM, Mandy Chung wrote:
> On 6/20/19 1:40 PM, Sean Mullan wrote:
>> Sorry, I'm just catching up and looking at this now.
>>
>> The one thing I don't like about these tests that use their own Policy 
>> implementation is that the permissions that are granted inside the 
>> test are granted to all code, and not just the test, which may lead to 
>> cases where permissions that should be granted to JDK modules in 
>> default.policy may be missed.
>>
> 
> When the test wants a fine-control on turning on security manager
> programmatically and test various security permission combinations,
> I prefer to use a custom Policy implementation.
> 
> Otherwise, I have to maintain multiple different test.policy files to
> test different permission combination which will be prone to editing
> error.
> 
> I think we should provide a test library to help building a custom Policy
> implementation that can take the default policy, preferrably the policy
> for the resolved modules such that the test can focus its logic of
> security configuration and does not need to code with the system
> modules in mind.

It seems a little related to 
https://bugs.openjdk.java.net/browse/JDK-8080294

--Sean

> 
> That's what I started with ModulePolicy and could potentially build up
> test library for configuring security permissions programmatically.
> 
> Mandy
> 
>> In tests that use policy files, we should grant permissions to only 
>> the test, for example:
>>
>>   grant codebase "file:${test.classes}/.../-" {
>>     permission ...;
>>   };
>>
>> However, in looking through our policy files, there are many that are 
>> not doing that. Something to fix, so I'll file a bug.
>>
>> This could also be fixed in these tests by inspecting the CodeSource 
>> of the ProtectionDomain. Or better yet, they should just use the jtreg 
>> java.security.policy option and a policy file and avoid the need to 
>> create a Policy implementation.
>>
>> Thanks,
>> Sean
>>
>>
>> On 6/20/19 11:04 AM, Mandy Chung wrote:
>>> The Policy API does not assume the presence of the default policy for
>>> the runtime to use.
>>>
>>> jdk.internal.vm.compiler already uses doPrivileged.   The module ends up
>>> with no permission as the test policy does not consult the default 
>>> policy.
>>>
>>> Mandy
>>>
>>> On 6/20/19 6:32 AM, Roger Riggs wrote:
>>>> Hi,
>>>>
>>>> If this were java.base, we would use doPrivilege to ignore the 
>>>> policy and place specific limits.
>>>> Encumbering the default policy with conditions needed by a trusted 
>>>> subsystem seems
>>>> like distributing what should be a local implementation issue.
>>>>
>>>> $.02, Roger
>>>>
>>>> On 6/20/19 2:23 AM, Mandy Chung wrote:
>>>>> Hi Vladimir,
>>>>>
>>>>> Indeed these are test issues that the tests will need to grant 
>>>>> permissions
>>>>> to jdk.internal.vm.compiler as the default policy grants.
>>>>>
>>>>> Thanks for going extra miles to fix the tests.
>>>>>
>>>>> My suggestion may be a bit general.  What I intend to say the custom
>>>>> security policy should extend the default policy unless it 
>>>>> intentionally
>>>>> excludes configuring permissions for specific modules.  I review the
>>>>> the patch but the test doesn't clearly tell what the test intends to
>>>>> do w.r.t. security configuration.
>>>>>
>>>>> We should avoid inadvertently granting permissions that the test 
>>>>> expects
>>>>> to disallow.  A better solution is to limit granting permissions 
>>>>> just for
>>>>> `jdk.internal.vm.compiler` module rather than all.
>>>>>
>>>>> Attached is ModulePolicy class that allows you to get the Policy for
>>>>> a specific module. It can be put in the test library that these tests
>>>>> can use them.
>>>>>
>>>>> So the test can call ModulePolicy.get("jdk.internal.vm.compiler") and
>>>>> implies method will call the returned ModulePolicy if present.
>>>>>
>>>>> test/lib/jdk/test/lib/security is one existing testlibrary for 
>>>>> security
>>>>> related stuff.  You can consider putting ModulePolicy.java there.
>>>>>
>>>>> This is one idea.
>>>>>
>>>>> Mandy
>>>>>
>>>>> On 6/19/19 6:03 PM, Vladimir Kozlov wrote:
>>>>>> http://cr.openjdk.java.net/~kvn/8185139/webrev.00/
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8185139
>>>>>>
>>>>>> For Graal to work we have to give Graal module all permissions 
>>>>>> which is specified in default policy [1].
>>>>>> Unfortunately this cause problem for Graal running tests which 
>>>>>> overwrite default policy.
>>>>>>
>>>>>> I discussed this with Mandy and she suggested that such tests 
>>>>>> should also check default policy. I implemented her suggestion. 
>>>>>> Note, this is not only Graal problem. There were already similar 
>>>>>> fixes before [2].
>>>>>>
>>>>>> I also updated Graal's problem list. Several tests were left on 
>>>>>> problem list (with different bug id) because they would not run 
>>>>>> with Java Graal (for example, they use --limit-modules flag which 
>>>>>> prevents loading Graal module). We will enable such tests again 
>>>>>> when libgraal is supported.
>>>>>>
>>>>>> I ran testing which execute these tests with Graal. It shows only 
>>>>>> known problems which are not related to these changes.
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir
>>>>>>
>>>>>> [1] 
>>>>>> http://hg.openjdk.java.net/jdk/jdk/file/49ed5e31fe1e/src/java.base/share/lib/security/default.policy#l156 
>>>>>>
>>>>>> [2] https://bugs.openjdk.java.net/browse/JDK-8189291
>>>>>
>>>>
>>>
>