[core-libs] RFR (L): 8010319: Implementation of JEP 181: Nest-Based Access Control

[Peter Levart]

Hi David,

On 05/15/2018 02:52 AM, David Holmes wrote:
> Master webrev of all changes:
>
> http://cr.openjdk.java.net/~dholmes/8010319-JEP181/webrev.full.v1/

I skimmed over reflection API changes.

In jl.Class:

3911         // returning a different class requires a security check
3912         SecurityManager sm = System.getSecurityManager();
3913         if (sm != null) {
3914             checkPackageAccess(sm,
3915 ClassLoader.getClassLoader(Reflection.getCallerClass()), true);
3916         }

...so here the "different" class is expected to be in the same package 
as "this" class. Is this invariant enforced in VM so it need not be 
checked here?


3871      * @apiNote The source language compiler is responsible for 
deciding which classes
3872      * and interfaces are nestmates, by generating the appropriate 
attributes
3873      * (JVMS 4.7.28 and 4.7.29) in the class file format (JVMS 4).
3874      * For example, the {@code javac} compiler
3875      * places a top-level class or interface into a nest with all 
of its direct,
3876      * and indirect, {@linkplain #getDeclaredClasses() nested 
classes and interfaces}
3877      * (JLS 8).
3878      * The top-level {@linkplain #getEnclosingClass() enclosing 
class or interface}
3879      * is designated as the nest host.

Should the text warn about not relying on this implementation detail to 
extract knowledge about nested/enclosing classes? Users should keep 
using getDeclaredClasses()/getEnclosingClass() for that purpose as 
nestmates may in future be used for other things too. OTOH, if users 
want to do an equivalent of private access check (on behalf of real 
caller and callee - for example in some kind of language runtime), it 
would be better they use the nestmate API...


in j.l.r.AccessibleMember:

   49  * <p> Java language access control prevents use of private 
members *outside*
   50  * *their top-level class;* package access members outside their 
package; protected members
   51  * outside their package or subclasses; and public members outside 
their
   52  * module unless they are declared in an {@link 
Module#isExported(String,Module)
   53  * exported} package and the user {@link Module#canRead reads} 
their module.

Could this be interpreted also as "private members can only be accessed 
from the top-level class"? I know that "nest" is not a Java language 
term, so it can not be used in the context of Java language access 
control. (If speaking of Java language, then even previous version of 
this text was wrong - if it was right, then it wouldn't have to be 
changed at all). So what about the following:

"Java language access control prevents use of private members outside 
the set of classes composed of top-level class and its transitive 
closure of nested classes".

Or would this be to lengthy and hard to understand?


in j.l.r.Reflection:

  140         // At this point we know that currentClass can access 
memberClass.
  141
  142         if (Modifier.isPublic(modifiers)) {
  143             return true;
  144         }
  145
  146         // Check for nestmate access if member is private
  147         if (Modifier.isPrivate(modifiers)) {
  148           // assert: isSubclassof(targetClass, memberClass)

although just in a function of explaining the following comment, I think 
the correct assert is

// assert targetClass == null || isSubclassof(targetClass, memberClass)

as for static members, targetClass is null.

  149           // Note: targetClass may be outside the nest, but that 
is okay
  150           //       as long as memberClass is in the nest.
  151           boolean nestmates = areNestMates(currentClass, memberClass);
  152           if (nestmates) {
  153             return true;
  154           }
  155         }

I'm wondering about the frequency of reflective accesses of various 
kinds of members. I imagine that most frequent are accesses of public 
members, others are less so (I'm not counting accesses made via 
.setAccessible(true) modified Field/Method/Constructor objects as they 
are bypassing this Reflection.verifyMemberAccess method). But among 
others I imagine reflective access to private members is the least 
frequent as there's no practical reason to use reflection inside and 
among classes that are most intimately logically coupled and "know" each 
other's internals at compile time. So it would make sense to check for 
private access at the end, after protected and package-private checks 
(that's how existing logic was structured).

So how about putting the nestmate access logic just before the last if 
(!successSoFar) { return false; }:

         // Check for nestmate access if member is private
         if (!successSoFar && Modifier.isPrivate(modifiers)) {
           // assert: targetClass == null || isSubclassof(targetClass, 
memberClass)
           // Note: targetClass may be outside the nest, but that is okay
           //       as long as memberClass is in the nest.
           successSoFar = areNestMates(currentClass, memberClass);
         }

         if (!successSoFar) {
             return false;
         }


This would not penalize access to package-private and protected members 
with areNestMates() JNI calls and maybe caching will not be needed at all.


Regards, Peter