RFR: 8319332: Security properties files inclusion

[Martin Balao]

On Thu, 2 Nov 2023 20:07:28 GMT, Alan Bateman <alanb at openjdk.org> wrote:

>> The implementation of this proposal is based on the requirements, specification and design choices described in the [JDK-8319332] ticket and its respective CSR [JDK-8319333]. What follows are implementation notes organized per functional component, with the purpose of assisting to navigate the code changes in this pull-request.
>> 
>> ## Security properties loading (overview)
>> 
>> A new static class named `SecPropLoader` (nested within `java.security.Security`) is introduced to handle the loading of all security properties. Its method `loadAll` is the first one to be called, at `java.security.Security` static class initialization. The master security properties file is then loaded by `loadMaster`. When additional security properties files are allowed (the security property `security.overridePropertiesFile` is set to `true`) and the `java.security.properties` system property is passed, the method `loadExtra` handles the extra load.
>> 
>> The master properties file is loaded in `OVERRIDE` mode, meaning that the map of properties is originally empty. Any failure occurred while loading these properties is considered fatal. The extra properties file (`java.security.properties`) may be loaded in `OVERRIDE` or `APPEND` mode. Any failure in this case is ignored. This behavior maintains compatibility with the previous implementation.
>> 
>> While the `java.security.properties` system property is documented to accept an URL type of value, filesystem path values are supported in the same way that they were prior to this enhancement. Values are then interpreted as paths and, only if that fails, are considered URLs. In the latter case, there is one more attempt after opening the stream to check if there is a local file path underneath (e.g. the URL has the form of `file:///path/to/a/local/file`). The reason for preferring paths over URLs is to support relative path file inclusion in properties files.
>> 
>> ## Loading security properties from paths (`loadFromPath` method)
>> 
>> When loading a properties file from a path, the normalized file location is stored in the static field `currentPath`. This value is the current base to resolve any relative path encountered while handling an _include_ definition. Normalized paths are also saved in the `activePaths` set to detect recursive cycles. As we move down or up in the _includes_ stack, `currentPath` and `activePaths` values are updated.
>> 
>> ## Loading security properties from URLs (`loadFromUrl` method)
>> 
>> The extra properti...
>
> src/java.base/share/classes/java/security/Security.java line 243:
> 
>> 241:             if (connection instanceof FileURLConnection fileConnection) {
>> 242:                 // A local file URL can be interpreted as a Path
>> 243:                 loadFromPath(fileConnection.getFile().toPath(), mode);
> 
> Ugh, shouldn't be direct using FileURLConnection here. Instead I think you should check if the url scheme is "file" (equalsIgnoreCase). If it is then use `Path.of(url.toURI())`.

Checking for _file_ in the URL scheme is not conclusive evidence that there is a local file path behind. I'll give a couple of examples. In Unix/Linux platforms, an URL of the form `file://example.com/path/to/some/file.txt` is processed with a remote FTP request (see Unix `sun.net.www.protocol.file.Handler`). In Windows, file URLs may be interpreted as UNCs but, if not possible, there is an FTP fallback (see Windows `sun.net.www.protocol.file.Handler`). While checking the host name in the URL is possible, there are three types of drawbacks: 1) a DNS query during the Security class initialization process should be avoided, 2) looking for hardcoded host names such as _localhost_ might lead to false negatives (i.e. a host is considered remote when it is not), and 3) there will be platform-specific and duplicated logic to deal with UNC file URLs. In addition, OpenJDK supports ill-formed relative path file URLs such as `file:some/relative/path`. In these cases, there is not a host name bu
 t there is a local file path underneath (relative to the current working directory). We did not find normative elements in [RFC 8089](https://www.rfc-editor.org/rfc/rfc8089) for all previously described behaviors, that would have been helpful for a URL-based check. Misinterpreting a file URL as remote will unnecessarily block the possibility of relative path includes.

We think that `FileURLConnection` is the most accurate indicator of a local file path because it includes the decision logic that is specific to OpenJDK and varies depending on the platform.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16483#discussion_r1380842630