Revisiting encapsulation requirement

[David M. Lloyd]

On 09/15/2015 02:47 PM, Remi Forax wrote:
> Hi David,
>
> ----- Mail original -----
>> De: "David M. Lloyd" <david.lloyd at redhat.com>
>> À: jpms-spec-experts at openjdk.java.net
>> Envoyé: Mardi 15 Septembre 2015 18:12:56
>> Objet: Re: Revisiting encapsulation requirement
>>
>> Do you have an example of a real-world problem (security or otherwise)
>> that is solved by adding access checks based on the caller ?
>
> You take the problem by the wrong side :)

Maybe.

> Let's suppose you agree that both the notion of visibility (private, package private, etc) and the reflection API are something useful.

Sure, we've been calling that "accessibility" up until now.

> A simple model is to make the Java visibility rules and the reflection API behavior aligned.
> This implies that some reflexive calls have to be caller sensitive.
> Then as i said earlier, you need a way to escape the sandbox, a escape hatch, to program things like meta-protocols as most of the Java EE specs are, hence, the addition of setAccessible().

So if I understand correctly, you propose that, in addition to verifying 
that the target type is accessible to the caller, we should also verify 
that it is visible when we do the actual access check (i.e. "invoke() 
should throw...if...the class is not declared as a dependency") and 
forget doing a special check when we call setAccessible().  If I'm 
misunderstanding you, stop me here!

But assuming all that is true, my concern is that today, I can access 
any public member of any public class in the system that I can get a 
Class object for, even if it comes from a different, isolated class 
loader.  Under the new rules and even with your suggestion, a class has 
to be public *and* visible to me, else setAccessible() is the only 
choice - even if it's public, and was handed to me!  To me this makes no 
sense and does not add any security- or encapsulation-related benefits 
that I can come up with - quite the opposite actually, since now you 
need to be granted sweeping permissions to access it.  This is why I'm 
seeking a counter-example which supports this idea.

> You may think that the reflection is a bad API, and that something like invokedynamic + the method handle Lookup API is better API, i agree but currently it's not a replacement because there is no way to emit invokedynamic in Java.
> Yes, this decision is stupid, but this JSR is not about improving the way the introspection/reflection works in Java but how to introduce a simple module API (with the compiler and runtime behavior aligned) in the existing JDK world with the minimal breaking changes.
>
> regards,
> Rémi
>
>>
>> On 09/15/2015 10:52 AM, Remi Forax wrote:
>>> Hi David,
>>> I don't think we should modify the "encapsulation" requirement if the
>>> requirement
>>> only apply to regular Java code and not on reflection +
>>> setAccessible(true).
>>>
>>> Currently, when you use reflection, you have the same security checks as in
>>> plain Java,
>>> and you can bypass those security checks using setAccessible(true).
>>>
>>> I don't see why we should change that:
>>>     - Class.forName() should throw a ClassNotFoundException if the module of
>>>     the class is not declared as dependency.
>>>     - invoke() should throw a an IllegalAccess exception if the module of
>>>     the class is not declared as dependency.
>>>     - setAccessible() should allow to bypass the security check and works as
>>>     in pre-module world.
>>>
>>> I don't see the point to either add a new security check on setAccessible
>>> or to allow reflection between modules that are not listed in the
>>> dependencies.
>>>
>>> Rémi
>>>
>>> ----- Mail original -----
>>>> De: "David M. Lloyd" <david.lloyd at redhat.com>
>>>> À: jpms-spec-experts at openjdk.java.net
>>>> Envoyé: Mardi 15 Septembre 2015 14:01:07
>>>> Objet: Revisiting encapsulation requirement
>>>>
>>>> Experts, I think we should revisit the "Encapsulation" requirement with
>>>> a view towards removal.
>>>>
>>>> The requirement is:
>>>>
>>>>> Encapsulation — The access-control mechanisms of the Java language and
>>>>> virtual machine must prevent code from accessing classes and interfaces
>>>>> in
>>>>> packages that are not exported by their containing modules, or in
>>>>> packages
>>>>> whose containing modules are not required by the module containing the
>>>>> code. These mechanisms must be operative even when a security manager is
>>>>> not present.
>>>>
>>>> The implementation amounts to modifying the
>>>> AccessibleObject.setAccessible() implementation for classes to perform a
>>>> check of the caller's class loader before determining whether to make
>>>> the object accessible.
>>>>
>>>> As Rémi pointed out, this check is easily bypassed by using reflection,
>>>> by reflecting on AccessibleObject.class itself - since all modules
>>>> require "java.base", all modules implicitly can bypass this check.  But,
>>>> even assuming this could somehow be patched over, I think we should
>>>> consider dropping this requirement; I have two primary reasons for this
>>>> (though there may be others as well).
>>>>
>>>> The first reason is that without a security manager (or maybe even
>>>> *with* a security manager - but that's another discussion), it is
>>>> inevitable that any security measure used to protect this mechanism will
>>>> ultimately be bypassed, rendering its security value useless; adding
>>>> more complexity to the system to do so will only increase the overall
>>>> vulnerability of the platform.
>>>>
>>>> The second reason is that it is often useful to gain access to and
>>>> inspect public classes that are not necessarily visible from your
>>>> module.  Frameworks do this commonly for example - framework
>>>> implementations will almost never import modules that they introspect.
>>>>
>>>> I think in the end users will find it more convenient and intuitive (and
>>>> no less secure in any real sense) if public classes remain accessible.
>>>> In the absence of any dissent, I would move that we drop this requirement.
>>>>
>>>> --
>>>> - DML
>>>>
>>
>> --
>> - DML
>>

-- 
- DML