jdk.internal.reflect.ReflectionFactory and SecurityManager
claes.redestad at oracle.com
Mon Dec 26 20:55:19 UTC 2016
On 2016-12-26 21:39, Peter Levart wrote:
> Hi Claes,
> On 12/26/2016 09:11 PM, Claes Redestad wrote:
>> with strong encapsulation in place this seems perfectly fine to me.
>> The only downside is that we will lose the extra reminder that the
>> ReflectionFactory must not escape to untrusted code, but I think we can
>> all help ensure that doesn't happen, right? :-)
> With strong encapsulation in place, if the ReflectionFactory escaped, it
> could only be used by the following modules:
> exports jdk.internal.reflect to
> If SecurityManager check was removed, those modules could then also
> obtain ReflectionFactory (they currently don't use it except
> jdk.unsupported). Does this present any new danger?
Right, it shouldn't be possible to do anything with a reference to an
encapsulated object, but let's not challenge this assumption here and
> Changes needed to support removal of this SecurityManager check are as
> Changes mostly remove the need to store ReflectionFactory into a
> variable, therefore they encourage style where ReflectionFactory is less
> likely to escape.
That's a good point, and changes looks good to me. I think this could go
as both a standalone cleanup and rolled into a redo of 8062389 etc.
> Regards, Peter
>> On 2016-12-26 20:29, Peter Levart wrote:
>>> There are 2 ReflectionFactory classes in JDK 9. The old one is
>>> sun.reflect.ReflectionFactory which ended in jdk.unsupported module and
>>> to which access is restricted with SecurityManager. There is also new
>>> jdk.internal.reflect.ReflectionFactory which is used internally by JDK,
>>> is exported to internal modules only but still uses SecurityManager to
>>> restrict access to itself. I checked all usages and they all use
>>> AccessControler.doPrivileged() for obtaining the instance of
>>> jdk.internal.reflect.ReflectionFactory, which somehow defeats the
>>> purpose of SecurityManager access checks in this API.
>>> I think this could be simplified by removing the SecurityManager check
>>> from jdk.internal.reflect.ReflectionFactory#getReflectionFactory static
>>> method and change all usages to invoke this method directly without
>>> doPrivileged(). There are already two sensitive internal APIs exposed
>>> without SecurityManager checks: jdk.internal.misc.Unsafe#getUnsafe and
>>> various jdk.internal.misc.SharedSecrets#getXxxAccess methods. So why
>>> wouldn't internal ReflectionFactory be exposed the same way?
>>> This would make obtaining the ReflectionFactory more robust and not
>>> sensitive to bootstrap issues that surfaced recently after my push of a
>>> fix for issues 8062389, 8029459, 8061950.
>>> So, what do you think? Is this a worthwhile cleanup and simplification?
>>> Regards, Peter
More information about the core-libs-dev