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

[Sean Mullan]

On 6/21/19 7:15 AM, Daniel Fuchs wrote:
> Hi Sean,
> 
> On 20/06/2019 21:40, Sean Mullan wrote:
>   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.
> 
> In what logging tests are concerned that would be quite an undertaking.
> For those tests, part of the test need to have permission to change
> the configuration, and part of the test need to not have the
> permission to double check that either the permission is not
> required for those operations, or that the permission is checked - if
> it is required.
> 
> Hence the use of a custom Policy implementation that uses
> ThreadLocale<AtomicBoolean> to figure out whether to grant or
> deny permissions.

Ok, I see that is challenging, and I understand why you had to do that. 
You probably could have done something similar by breaking up the test 
into multiple classes in separate directories (or jars) with different 
ProtectionDomains, but that would require a bit of work, and maybe it is 
overkill. But I think then you could probably use the JDK Policy 
provider with a single policy file. (I'm not suggesting doing this right 
now, just was wondering if you considered this).

--Sean

> 
> best regards,
> 
> -- daniel
> 
>>
>> 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
>>>>>
>>>>
>>>
>