[foreign-memaccess+abi] RFR: 8310659: The jar tool should support allowing access to restricted methods from executable jars [v4]

[Maurizio Cimadamore]

On Wed, 28 Jun 2023 08:57:39 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> There are 2 parts of the puzzle:
>> 1. we need to enable native access for a module. This is what `addEnableNativeAccessToAllUnnamed` does
>> 2. we need to track whether an enable native access flag has been set 'externally', through either the command line or the jar manifest. This dictates whether we should produce an error or a warning for modules that don't have native access.
>> 
>> Right now the latter is implemented by decoding a bunch of system properties that are initialized in arguments.cpp [1], [2]. This decoding than produces a set of module names that was specified on the command line. And this is used to initialize the `HAS_ENABLE_NATIVE_ACCESS_FLAG` variable [3].
>> 
>> I just hooked into the latter process by adding the `ENABLE_NATIVE_ACCESS_SET_IN_MANIFEST` flag. This flag is mutable since `ModuleBootstrap` is intialized before we read the jar manifest. i.e. the variable is mutable because it is lazily initialized.  This works out because we only ever call `Module::ensureNativeAccess` after reading the jar manifest.
>> 
>> `ModuleBoostrap::hasEnableNativeAccessFlag` is really `hasEnableNativeAccessFlagOnCommandLine`, and we need another `hasEnableNativeAccessFlagInJarManifest`, and then the call to `hasEnableNativeAccessFlag` in `Module::ensureNativeAccess` would need to be replaced by `hasEnableNativeAccessFlagOnTheCommandLine() || hasEnableNativeAccessFlagInJarManifest()`.
>> 
>> But, I think we would still have a chicken and egg problem, since we start using modules before reading the jar manifest, so before we really know whether we should be throwing an error or warning for native access. So, at least in theory, if someone called `ensureNativeAccess` before the jar manifest is read, then they could observe the incorrect behavior.
>> 
>> So, I'm not sure if we could avoid this issue, other than not letting the flag set in the manifest factor into the decision whether to throw an error or just issue a warning.
>> 
>> Another option which I tried earlier, is for `LauncherHelper` to set a system property, and then initialize a `static final` flag in a holder class based on that property the first time `ModuleBootsrap::hasEnableNativeAccessFlag` is called. But, that seemed like it was just hiding the mutable state in the system property. Though, maybe it's preferable? It would at least show that we only initialize the state once, and it is initialized somewhere before we call `hasEnableNativeAccessFlag`.
>> 
>> [1]: https://github.com/openjdk/panama-for...
>
> The launcher code is is java.base.  If `java -jar app.jar` is used then there shouldn't be application code running, and trying to do native access, before the application main class is loaded. In other words, there shouldn't be code in an unnamed module trying to do native access before any application or library code has been loaded. It's similar to Add-Exports and Add-Opens. They are used to reflectively export or open packages before the application code executes.
> 
> The same applies to an agent that may be packaged with the application and configured with Launcher-Agent-Class. We probably need to move the handling of Add-Exports, Add-Opens, and Enable-Native-Access to before Launcher-Agent-Class.

The problem here is that the current command line option for native access has a rather sophisticated behavior:
* if the option is not present, a warning is emitted
* if the option is present, names module M, and access occurs within M, nothing is emitted
* if the option is present, names module M, and access occurs outside M, an exception is thrown

This makes enable-native-access non-additive (unlike add-opens). That is, if `Enable-Native-Access:ALL-UNNAMED` is translated into `--enable-native-access=ALL-UNNAMED` well, that means that:

* access to restricted methods from unnamed module is granted
* access to restricted methods from a module M that is not unnamed is denied

Now, this patch tries to implement this approach, but I do have worries: effectively, once a manifest specifies the Enable-Native-Access attribute, can the command line still add other modules in the trusted bubble? If yes, that's ok. If no, then we have a problem.

-------------

PR Review Comment: https://git.openjdk.org/panama-foreign/pull/843#discussion_r1244942866