Revisiting encapsulation requirement

[Remi Forax]

> De: "David M. Lloyd" <david.lloyd at redhat.com>
> À: jpms-spec-experts at openjdk.java.net
> Envoyé: Mardi 15 Septembre 2015 23:06:35
> Objet: Re: Revisiting encapsulation requirement
> 
> 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!

yes,

> 
> 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.

Let say you have two modules, one that contains an interface
and another that contains a class that implements that interface in another package.

currently if you write a code like this:
  o.getClass().getMethods("foo").invoke(o)
by accessing to a public method of the implementation class that may be not declared in the interface,
you're breaking encapsulation.

if you write:
  Interface o = ...
  Interface.class.getMethods("foo").invoke(o)
there is no problem.
 
but maybe it's not a problem that worth to be fixed ?

Rémi

> 
> > 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
>