> 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 any paths. Cyclic includes detection is now performed with > the unresolved paths and `Files::isSameFile`, so `activePaths` no longer > needs to be a `Set`. > > <details> > <summary>Previous approach and rationale for resolving <strong>file</strong> > symlinks (kept here for historical reasons).</summary><br> > > 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 were performing 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. This reasoning has been nuanced and re-evaluated in a [subsequent > discussion](#discussion_r2585623241). > > The original idea was to replace > [`java.nio.file.Path::toRealPath`](https://docs.oracle.com/en/java/javase/24/docs/api/java.base/java/nio/file/Path.html#toRealPath(java.nio.file.LinkOption...)) > by > [`java.io.File::getCanonicalPath`](https://docs.oracle.com/en/java/javase/24/docs/api/java.base/java/io/File.html#getCanonicalPath()) > for path canonicalization purposes. The decision wa...
Francisco Ferrari Bihurriet has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 29 additional commits since the last revision: - Merge openjdk:master into franferrax:JDK-8352728 - Remove superseded SecurityPropFile - Move tests into SecurityPropFile and rename them - Merge openjdk:master into franferrax:JDK-8352728 - 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. - Convert ConfigFileTestAnonymousPipes to Java - Review suggestion: go back tolerating IOException We agreed an IOException in this case is recoverable, and decided to tolerate it, while adding a debug log message with the exception. - ... and 19 more: https://git.openjdk.org/jdk/compare/d52e2861...6f1ffb66 ------------- Changes: - all: https://git.openjdk.org/jdk/pull/24465/files - new: https://git.openjdk.org/jdk/pull/24465/files/df9dc5c7..6f1ffb66 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=24465&range=16 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=24465&range=15-16 Stats: 28 lines in 5 files changed: 8 ins; 2 del; 18 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
