> Hi, this is a proposal to fix 
> [JDK-8352728](https://bugs.openjdk.org/browse/JDK-8352728 "InternalError 
> loading java.security due to Windows parent folder permissions").
> 
> Path resolution with `Path::toRealPath` fails under the following conditions:
> 
> * When there is a restricted 
> [_ACL_](https://learn.microsoft.com/en-us/windows/win32/secauthz/access-control-lists)
>  in a parent directory (_Windows_)
> * When dealing with an anonymous file only accesible through the _procfs_, 
> such as `/proc/<pid>/fd/<fd>` (_Linux_)
>     * Such a file can be created by a pipe, deleting an opened file, or with 
> the [`memfd_create` _Linux_ 
> API](https://man7.org/linux/man-pages/man2/memfd_create.2.html)
> 
> Original code from [JDK-8319332: Security properties files 
> inclusion](https://bugs.openjdk.org/browse/JDK-8319332) unconditionally 
> resolves with `Path::toRealPath` all the processed properties files. This PR 
> avoids resolving the paths unless strictly necessary: when determining the 
> base path to compute a relative `include` directive. Cyclic includes 
> detection is now performed with the unresolved paths and `Files::isSameFile`, 
> so `activePaths` no longer needs to be a `Set`.
> 
> #### Still unsupported use-case
> 
> In _Linux_, a relative `include` from an anonymous file referenced as 
> `/proc/<pid>/fd/<fd>` makes little sense.
> 
> However, in _Windows_, a relative `include` from a file where any of the 
> parent directories has a restricted _ACL_ will be expected to work. It's 
> important to note that such a restricted directory permissions scenario 
> occurs to every 
> [_UWP_](https://learn.microsoft.com/en-us/windows/uwp/get-started/universal-application-platform-guide)
>  app (see [JDK-8369741](https://bugs.openjdk.org/browse/JDK-8369741 "cannot 
> use java.security inside of UWP apps")).
> 
> When computing a relative `include`, we perform symlinks resolution of the 
> parent file under the rationale that the person writing the original 
> properties file is the one who decides where the relative includes should 
> resolve. In the absence of a recommended way or API to deal with resolution 
> under this restricted directory permissions scenario, this use-case is left 
> unsupported. This compromise-solution is better than the current status of 
> _UWP_ apps, where simply loading the `java.security.Security` class breaks 
> the JVM initialization by a failed attempt to resolve the `java.security` 
> path (not from an `include` statement, nor a relative path).
> 
> <details>
> <summary>Previous approach and <code>File::getCanonicalPath</code> vs ...

Francisco Ferrari Bihurriet has updated the pull request incrementally with 
four additional commits since the last revision:

 - 3/3) Adjust ConfigFileTest for unresolved paths
 - 2/3) Refactor ConfigFileTest ExtraMode enum
   
   Move ExtraMode as an ExtraPropsFile field, so we have this information
   as soon as the ExtraPropsFile is created. This will be useful for the
   next change.
 - 1/3) Revert ConfigFileTest adjustment
   
   This reverts commit 7c80874c25bc99783ad24fb22d2c080d33c5503a only for
   ConfigFileTest.
 - Do not resolve symlinks for relative includes
   
   As @wangweij pointed out:
   > The person writing the original properties file may have expected
   > includes to resolve relative to its own location, but whoever created
   > the symlink may have intended a different resolution path. If they
   > wanted the original location, they could have just used the real file
   > directly instead of introducing a symlink.
   
   Since path resolution is causing trouble under certain conditions, let's
   avoid doing it. Other programs with include directives support in their
   configuration files are doing the same.

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/24465/files
  - new: https://git.openjdk.org/jdk/pull/24465/files/51a3ce86..c33bf62c

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=24465&range=14
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24465&range=13-14

  Stats: 105 lines in 2 files changed: 28 ins; 31 del; 46 mod
  Patch: https://git.openjdk.org/jdk/pull/24465.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/24465/head:pull/24465

PR: https://git.openjdk.org/jdk/pull/24465

Reply via email to