RFR: 8229773: Resolve permissions for code source URLs lazily

Claes Redestad claes.redestad at oracle.com
Thu Aug 15 12:44:37 UTC 2019


Hi,

On 2019-08-15 12:56, Alan Bateman wrote:
> On 15/08/2019 11:03, Claes Redestad wrote:
>> Hi,
>>
>> by resolving permissions for code source URLs lazily, we can reduce
>> early class loading during bootstrap, which improves footprint, startup
>> and reduces the typical bootstrap dependency graph.
>>
>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8229773
>> Webrev: ...
>>
>> :
> I think the approach is okay as URL::openConnection doesn't actually 
> open the connection to the resource and the creation of the 
> URLStreamdHandler shouldn't depend on permissions granted to the caller. 
> If a handler needs permissions when creating the URLConnection then it 
> should do so in a privileged block. 

thanks!

I checked most of the handlers and the openConnection implementations
and couldn't find any path that isn't either free of permission checks
or doing permission sensitive steps in a privileged block already. Many
handlers are already potentially created lazily after a SM has already
been installed, so I don't think we need additional tests for this.

> I think it would be a bit cleaner if 
> the supporting class would lazily add the permissions for a CodeSource 
> to the collection. That is, create it with a permissions collection and 
> code source rather than a URL to match 
> SecureClassLoader::getPermissions. You could potentially use it in 
> URLClassLoader getPermission(CodeSource) method too.

That'd be cool. The logic in URLClassLoader is mostly a super-set of the
logic I'm hoisting from BuiltinClassLoader here, but the logic in
URLClassLoader also has a security check acting on the ACC of the
classloader which would be hard to move to the supporting class, and it
seems that check need to happen eagerly.

I'll readjust the API to wrap the CodeSource rather than the URL, but I
think we should leave URLClassLoader alone for now.

> 
> In System.setSecurityManager then you might need 
> DefaultFileSystemProvider.theFileSystem() to ensure that the default 
> file system is fully initialized before setting the SM.

Right, doesn't make much of a difference since all providers currently
set up their singleton file system on creation, but using theFileSystem
better captures intent.

> 
> A minor nit this adds a spurious import BuiltinClassLoader. Also it can 
> import the sun.security.uti class to be consistent with the existing code.

Cleaned up imports, too:

http://cr.openjdk.java.net/~redestad/8229773/webrev.01/

/Claes


More information about the core-libs-dev mailing list