RFE: support safely wrapping restricted FFM calls

[Rob Spoor]

Hi Maurizio,

I don't think you understand what my module is doing. For instance, it's 
not specifying the downcall method handles themselves, it's just making 
it easy to define them. Maybe a small example would show what I'm doing.

Consider the following partial interface from JNA:

     public interface Kernel32 extends StdCallLibrary, WinNT, Wincon {

         /** The instance. */
         Kernel32 INSTANCE = Native.load("kernel32", Kernel32.class, 
W32APIOptions.DEFAULT_OPTIONS);

         int GetLastError();
     }

What JNA is doing is creating an implementation based on this interface 
that delegates to the native kernel32 library. This is all done through 
the Native.load method.

What I'm building is not an FFM-backed version of Kernel32 interface, 
but something like Native.load. In other words, my module will call the 
restricted Linker.downcallHandle method. Somebody that then wants to 
build a Kernel32 interface would not directly call any restricted 
methods. As a result, their code would not need to have native access 
enabled because they would get it through my module. That's something I 
want to protect against, because it's definitely not safe to just grant 
any access. (In fact, some incorrectly written test code has already 
crashed my JVM.) While my module will definitely need to have native 
access enabled, I'd like to require that the module with such a Kernel32 
interface needs to have native access enabled as well.

The idea of using a MethodHandles.Lookup instance looks interesting, but 
it would only replace the use of the StackWalker. I still would need to 
print a message or throw an exception from my module instead of reusing 
the code from Module.ensureNativeAccess, which makes my module either 
stricter than the FFM API right now, or less strict in the future when 
the FFM API starts throwing exceptions.

Rob


On 15/11/2023 23:14, Maurizio Cimadamore wrote:
> Hi Rob,
> honestly I think this all looks ok (as in: I don't think you need to do 
> anything in particular). You are writing a library that will need to 
> create some downcall method handles and dereference some pointers. You 
> need unsafe access, so users of your library will need to pass your 
> module name to the --enable-native-access flag.
> 
> It is true that clients of your library will be able to, indirectly 
> (through your framework), perform unsafe operations, but _only if_ they 
> have explictly trusted your library module on the command line. At that 
> point they get full access to your library (and the pointers it generate).
> 
> This is not dissimilar, if you think about it, to what happens with 
> method handles. E.g. if a client A creates a method handle for a method 
> M that is _only_ accessible from A, it is then free to publish that 
> method handle to be used by other clients, including a client B who 
> might not otherwise have access to M. This is fine - method handles are 
> _capabilities_ that are granted at the moment they are obtained (via a 
> lookup object). Your library acts in a similar way - it creates a lot of 
> unsafe stuff which is then exposed (as safely as possible, I hope :-) ), 
> to a client that would not otherwise have access to such unsafe 
> capabilities.
> 
> P.S.
> 
> If you _really_ want to ban clients that don't have enable native access 
> supported, I propose that your API should take a MethodHandle.Lookup, 
> and then you could check the module of the lookup class, and throw an 
> exception if native access is not enabled:
> 
> https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/invoke/MethodHandles.Lookup.html#lookupClass()
> 
> https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/Module.html#isNativeAccessEnabled()
> 
> You can even throw if the lookup doesn't have enough access:
> 
> https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/lang/invoke/MethodHandles.Lookup.html#hasFullPrivilegeAccess()
> 
> Maurizio
> 
> 
> 
> On 15/11/2023 17:13, Rob Spoor wrote:
>> Hello all,
>>
>> I'm working on a module that makes working with FFM easier; think of 
>> something like JNA. For instance, it allows creating structures 
>> without having to manually manage var handles etc.
>>
>> My module uses restricted mehods like AddressLayout.withTargetLayout 
>> to support pointers. Those correctly give warnings if I don't use 
>> --enable-native-access. This is where I've identified a potential 
>> security risk. Native access would need to be enabled for *my* module, 
>> which would allow modules that use my module to call these restricted 
>> methods indirectly and without needing native access enabled 
>> themselves. This means that any malicious module could piggy-back on 
>> the native access that would be enabled for my module.
>>
>> I can implement my own access checks using the following:
>>
>>     StackWalker.getInstance(Set.of(Option.RETAIN_CLASS_REFERENCE))
>>             .getCallerClass()
>>             .getModule()
>>             .isNativeAccessEnabled()
>>
>> However, that would mean users of my module would need to provide 
>> access using two different mechanisms. I think that making some 
>> existing code public could help situations like mine:
>>
>> * Changing the visibility of java.lang.Module.ensureNativeAccess from 
>> package-private to public would allow me to check access using the 
>> JVM's own mechanism, in combination with the StackWalker class to get 
>> the caller (current) class and its module. Alternatively, new instance 
>> method Class.ensureNativeAccess(owner, methodName) could delegate to 
>> Reflection.ensureNativeAccess(this, owner, methodName) to make sure 
>> that a different module couldn't be used instead, or static method 
>> Class.ensureNativeAccess(currentClass, owner, methodName) could 
>> delegate to Reflection.ensureNativeAccess to support null classes.
>>
>> * Moving jdk.internal.javac.Restricted to java.lang.foreign would 
>> allow me to easily document that methods are restricted.
>>
>> There is an alternative in using --add-exports to access 
>> jdk.internal.reflect (for Reflection.ensureNativeAccess that 
>> indirectly calls Module.ensureNativeAccess) and jdk.internal.javac 
>> (for Restricted), but that adds another burden on callers. In this 
>> case, a burden that cannot be easily remedied using a manifest entry 
>> (Enable-Native-Access).
>>
>>
>> Kind regards,
>>
>> Rob