On 2021. May 11., at 1:51, Thiago Henrique Hupner wrote:
>
> Hi all.
>
> I've been testing the JDK Dynalink recently and
> I think I've found a bug.
>
> The class jdk.dynalink.beans.CheckRestrictedPackage checks
> if a package is restricted.
>
> So, I have a class that has some static methods. But to Dynalink find the
> class,
> the class needs to be public. OK,
> I've changed my class to be public. However, it needs that
> the package of the class is exported in the module too.
>
> I think this is a little too restrictive. I don't want to expose
> to my users some internal implementation.
The module exports is exactly the feature used to regulate what are you
exposing from the module. In this case, you could have a class in a different
package exposing those static methods (even if only just delegating to the
original unexported class), and export those methods.
Dynalink’s BeansLinker deliberately implemented in such a manner that it only
allows access to entirely unrestricted classes. So what you’re experiencing is
not a bug, it is the expected behavior. It’s admittedly a compromise between a
simpler security model vs. losing some power.
>
> The main problem is that the method
> CheckRestrictedPackage::isRestrictedClass
> does the following check:
>
> // ...
> final String pkgName = name.substring(0, i);
> final Module module = clazz.getModule();
> if (module != null && !module.isExported(pkgName)) {
>return true;
> }
> // ...
>
> I have a feeling that this check should be changed to something like
>
> // ...
> final String pkgName = name.substring(0, i);
> final Module module = clazz.getModule();
> if (module != null && !module.isOpen(pkgName,
> CheckRestrictedPackage.class.getModule())) {
>return true;
> }
> // ...
>
> In this way, my code can only "opens" a single package to the jdk.dynalink
> module,
> not having to unconditionally export a package.
If we did just what you suggested, then every package you’d open to
jdk.dynalink would become wide open accessible to everyone through Dynalink.
That’d obviously be a problem as it’d allow circumventing module visibility
rules.
That would be fixable with some more work, though. It would be possible to
create an enhancement where Dynalink allows linking to methods of
non-globally-exported classes if it’d verify each such link attempt through
sun.invoke.util.VerifyAccess.isMemberAccessible, using the data from the
MethodHandle.Lookup object passed in LinkRequest. These lookup objects
typically originate from the bootstrap methods for invokedynamic call sites in
the target class.
Then you’d also indeed need to ensure that Dynalink can reflect on those
packages, so you’d still have to open the package towards Dynalink, but
Dynalink itself would enforce that it itself only allows linking call sites
according to JVM linkage rules.
This is not very simple to implement though, as the isRestrictedClass is used
for various purposes right now, and would need to be replaced with something
more granular, and then calls to VerifyAccess.isMemberAccessible would be
needed to be added into the logic for resolving property getters and setters
and method getters.
Dynalink already uses the Lookup objects for various purposes; it is used for
linking invocations of @CallerSensitive-marked methods (which Dynalink
supports) as well as for linkage-rules respecting creation of type conversions
(it’s somewhat surprising that the concern would pop up there, but it does…)
All just to say that Dynalink having to deal with JVM linkage rules isn’t
foreign to it, it’s already doing it in some contexts, but it indeed currently
doesn’t support linking to classes that aren’t world-exported.
Attila.
>
> From what I could understand, in Nashorn, the generated classes
> were defined in the unnamed module, that always exports all the packages,
> so the access always worked.
>
> The code can be found here:
> https://github.com/openjdk/jdk/blob/master/src/jdk.dynalink/share/classes/jdk/dynalink/beans/CheckRestrictedPackage.java#L94
>
> Best regards.
>
> Thiago
