RFR: 8055206: Update SecurityManager::checkPackageAccess to restrict non-exported JDK packages by default

[Sean Mullan]

On 1/13/17 2:38 AM, Mandy Chung wrote:
>
>> On Jan 9, 2017, at 11:25 AM, Sean Mullan <sean.mullan at oracle.com>
>> wrote:
>>
>> Please review this JDK 9 change to make the
>> SecurityManager::checkPackageAccess and checkPackageDefinition
>> implementations restrict access to the same set of internal JDK
>> packages as the module system.
>>
>> This overall change will improve security by making these two
>> mechanisms consistent and reduce the amount of work needed to
>> maintain the package.access and package.definition security
>> properties going forward.
>>
>> JBS issue: https://bugs.openjdk.java.net/browse/JDK-8055206 JDK
>> webrev:
>> http://cr.openjdk.java.net/~mullan/webrevs/8055206/jdk/webrev.00/
>> JAXP webrev:
>> http://cr.openjdk.java.net/~mullan/webrevs/8055206/jaxp/webrev.00/
>>
>
> This looks quite good.  It’s good that the restricted package list
> for the system modules is no longer modifiable.  This change may
> impact existing applications that are running with security manager
> and uses a JDK internal API from the runtime that was not in the
> package.access list.  It may require  the application to grant
> additional “accessClassInPackage.<pkg>” permissions in addition to
> using `—-add-exports` to break into encapsulation.

Correct.

> Similar to the feedback I suggest for JDK-8168075.  We can move this
> initialization to the holder class and trigger the initialization in
> the SecurityManager constructor.

For now, I would prefer to leave it as is. In an earlier revision, I 
experimented with delaying the initialization of nonExportedPkgs until 
checkPackageAccess was called and ran into some circular permission 
checking issues. It could be moving it to the ctor is ok, but since I 
have every test passing right now, I'd rather not tinker with it :)

>> : Several tests also had to be modified to be granted additional
>> permission(s) to access the newly restricted packages under a
>> SecurityManager. JAXP also needed a change to grant additional
>> permissions to access internal packages that are exported to the
>> modules that are dynamically created for use with XSLT.
>
> We will have to see if any dynamic generated bytecode in the field
> will need access to internal API like XSLT case.

Yes. I have checked several other use cases in the JDK that create 
dynamic modules (nashorn, rmi, jmx) and not found any issues.

> It seems something
> not right for a module that has access to an internal package in
> another module via qualified exports is required to have
> “accessClassInPackage.<pkg>” runtime permission.  I believe the
> “accessClassInPackage.<pkg>” permission is checked when a
> deprivileged module loads an internal class in a module defined by
> the boot loader even if the package of that internal class is
> exported to it to access.  I do think we should follow up and look
> into the potential solutions to respect the qualified exports.

I agree.

 > In addition, we should file an issue to look into any possibility to
> respect `—-add-exports` if specified (perhaps automatically add an
> entry in the default policy?).  I’m okay to follow up these with
> separate JBS issues.

Will do.

> Otherwise looks good. Mandy

Thanks,
Sean