RFR: 8229773: Resolve permissions for code source URLs lazily

[Daniel Fuchs]

Hi Claes,

I wonder if initialize() should check the state of
the readOnly() flag - and if that's true, call
perms.setReadOnly() ?

see SecureClassLoader::getProtectionDomain(..)

best regards,

-- daniel

On 15/08/2019 13:44, Claes Redestad wrote:
> 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