Re: RFR: 8319332: Security properties files inclusion [v12]
On Thu, 2 May 2024 21:07:03 GMT, Weijun Wang wrote: > The revised text is much clearer. For the last sentence, I would suggest > rephrasing "In order to prevent profile properties from being overridden" to > "In order to override properties defined in the main file". Isn't that your > real purpose? :-) Sounds good. Changed. - PR Comment: https://git.openjdk.org/jdk/pull/16483#issuecomment-2091681331
Re: RFR: 8319332: Security properties files inclusion [v13]
> 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 properties file can be loaded from a URL. ... Francisco Ferrari Bihurriet has updated the pull request incrementally with one additional commit since the last revision: Profiles documentation adjustments. Co-authored-by: Francisco Ferrari Co-authored-by: Martin Balao - Changes: - all: https://git.openjdk.org/jdk/pull/16483/files - new: https://git.openjdk.org/jdk/pull/16483/files/0b81f5c4..6fd9181a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16483=12 - incr: https://webrevs.openjdk.org/?repo=jdk=16483=11-12 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/16483.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16483/head:pull/16483 PR: https://git.openjdk.org/jdk/pull/16483
Re: RFR: 8319332: Security properties files inclusion [v12]
On Thu, 2 May 2024 20:34:22 GMT, Francisco Ferrari Bihurriet 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... > > Francisco Ferrari Bihurriet has updated the pull request incrementally with > one additional commit since the last revision: > > Profiles documentation adjustments. > > Co-authored-by: Francisco Ferrari > Co-authored-by: Martin Balao The revised text is much clearer. For the last sentence, I would suggest rephrasing "In order to prevent profile properties from being overridden" to "In order to override properties defined in the main file". Isn't that your real purpose? :-) - PR Comment: https://git.openjdk.org/jdk/pull/16483#issuecomment-2091599627
Re: Potential issues with javax.crypto under StructuredTaskScope/JDK22
Can you try recording the challenge and the answer in each round and then recalculate them in a plain single thread standalone program to verify if the answers are still the same? (Hopefully this calculation is deterministic). --Weijun > On May 2, 2024, at 4:18 PM, Chris Marshall wrote: > > Hi Alan, > > The code is completely unchanged between JDK21 and JDK22; it is using virtual > threads and StructuredTaskScope in both cases (and also, via a different > path, platform threads in both cases). > > I should perhaps have given a bit more background about why I think that the > crypto classes are at fault. The UserSRP auth class I linked to sends a > message to AWS (via an AWS API, ultimately made as an HTTP call) which > includes some information about the user. Amazon’s server responds (over > HTTP) with a “challenge” which includes some pieces of information. Using > those pieces of information plus knowledge of the password, the class I > linked to derives an answer to the challenge, which is then sent (via HTTP) > to AWS. AWS can verify that the response could only have been generated by > someone with knowledge of the password, and they will respond with a valid > bearer token, which can be used as an authorization header to HTTP calls (to > our internal services, which can validate the bearer token). In this way, our > services can communicate between themselves without any passwords ever being > transmitted anywhere. In the following diagram, the arrows represent HTTP > request/responses between us and AWS > > Us —-> (info) —> AWS > > Us <—- (challenge) <—- AWS > > Us —> (answer) —> AWS > > Us <— (bearer token | wrong user/pwd) <—- AWS > > “Wrong username or password” indicates that the response calculated by the > linked class is not correct. > > I don’t believe (though I’m not an expert, so could be wrong) that this has > anything to do with javax.security APIs > > Chris > >> On 2 May 2024, at 19:59, Alan Bateman wrote: >> >> >> >> On 02/05/2024 19:33, Chris Marshall wrote: >>> : >>> >>> Last week I upgraded the application to be compiled by JDK22, and run on >>> JDK22. Immediately, we started to see failures from within the User-SRP >>> auth code only when it was run on a virtual thread from within a >>> StructuredTaskScope. The failures are merely that the code appears to have >>> calculated the wrong authentication response (i.e. AWS Cognito returns a >>> message to the effect that we have the wrong username or password). It is >>> not possible that this could be the case, because the same application, >>> using the same username/password combo is able to successfully authenticate >>> to AWS Cognito using User-SRP auth from a platform thread. >>> >> Thanks for reporting a potential issue. >> >> You say that the code was running correctly on JDK 21. Was this in the >> context of virtual threads and using StructuredTaskScope? I'm trying to >> understand from your mail if you were using virtual threads with JDK 21 and >> whether you were using StructuredTaskScope in JDK 21 too. >> >> "wrong username or password" hints that maybe this is some kinda of >> inheritance issue, I'm specifically thinking of the inherit access control >> context. Would it be possible to search the code and libraries that are in >> use here to see if they are using the javax.security.auth.Subject API? It's >> just a wild guess at this point but I think might give some clues as to >> where inheritance might be coming from. >> >> -Alan >> >>
Re: RFR: 8319332: Security properties files inclusion [v11]
On Thu, 2 May 2024 19:07:50 GMT, Weijun Wang wrote: >> src/java.base/share/conf/security/java.security line 69: >> >>> 67: # statement will override any matching property defined in a profile. >>> In order >>> 68: # to avoid this behavior, the include statement may be placed at the >>> end of >>> 69: # this file. >> >> It's probably better to place the newly added "Security properties defined >> after an include statement..." in an individual paragraph since it's not >> limited to "the latter case" (or you mean "last"?). Then you might not want >> to use the "profile" word since it's not defined elsewhere. > > Instead of "to avoid this behavior", how about "if you want the properties > defined in the included file to override those in the main file"? We reorganized the documentation, split it into paragraphs, numbered examples and made some other changes. Changes include an explanation of what it means to "inline" a file in terms of overrides (for security properties defined both before and after the include statement). Please have a look. - PR Review Comment: https://git.openjdk.org/jdk/pull/16483#discussion_r1588320508
Re: RFR: 8319332: Security properties files inclusion [v12]
> 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 properties file can be loaded from a URL. ... Francisco Ferrari Bihurriet has updated the pull request incrementally with one additional commit since the last revision: Profiles documentation adjustments. Co-authored-by: Francisco Ferrari Co-authored-by: Martin Balao - Changes: - all: https://git.openjdk.org/jdk/pull/16483/files - new: https://git.openjdk.org/jdk/pull/16483/files/e906e3cf..0b81f5c4 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16483=11 - incr: https://webrevs.openjdk.org/?repo=jdk=16483=10-11 Stats: 40 lines in 1 file changed: 7 ins; 6 del; 27 mod Patch: https://git.openjdk.org/jdk/pull/16483.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16483/head:pull/16483 PR: https://git.openjdk.org/jdk/pull/16483
Re: Potential issues with javax.crypto under StructuredTaskScope/JDK22
Hi Alan, The code is completely unchanged between JDK21 and JDK22; it is using virtual threads and StructuredTaskScope in both cases (and also, via a different path, platform threads in both cases). I should perhaps have given a bit more background about why I think that the crypto classes are at fault. The UserSRP auth class I linked to sends a message to AWS (via an AWS API, ultimately made as an HTTP call) which includes some information about the user. Amazon’s server responds (over HTTP) with a “challenge” which includes some pieces of information. Using those pieces of information plus knowledge of the password, the class I linked to derives an answer to the challenge, which is then sent (via HTTP) to AWS. AWS can verify that the response could only have been generated by someone with knowledge of the password, and they will respond with a valid bearer token, which can be used as an authorization header to HTTP calls (to our internal services, which can validate the bearer token). In this way, our services can communicate between themselves without any passwords ever being transmitted anywhere. In the following diagram, the arrows represent HTTP request/responses between us and AWS Us —-> (info) —> AWS Us <—- (challenge) <—- AWS Us —> (answer) —> AWS Us <— (bearer token | wrong user/pwd) <—- AWS “Wrong username or password” indicates that the response calculated by the linked class is not correct. I don’t believe (though I’m not an expert, so could be wrong) that this has anything to do with javax.security APIs Chris On 2 May 2024, at 19:59, Alan Bateman wrote: On 02/05/2024 19:33, Chris Marshall wrote: : Last week I upgraded the application to be compiled by JDK22, and run on JDK22. Immediately, we started to see failures from within the User-SRP auth code only when it was run on a virtual thread from within a StructuredTaskScope. The failures are merely that the code appears to have calculated the wrong authentication response (i.e. AWS Cognito returns a message to the effect that we have the wrong username or password). It is not possible that this could be the case, because the same application, using the same username/password combo is able to successfully authenticate to AWS Cognito using User-SRP auth from a platform thread. Thanks for reporting a potential issue. You say that the code was running correctly on JDK 21. Was this in the context of virtual threads and using StructuredTaskScope? I'm trying to understand from your mail if you were using virtual threads with JDK 21 and whether you were using StructuredTaskScope in JDK 21 too. "wrong username or password" hints that maybe this is some kinda of inheritance issue, I'm specifically thinking of the inherit access control context. Would it be possible to search the code and libraries that are in use here to see if they are using the javax.security.auth.Subject API? It's just a wild guess at this point but I think might give some clues as to where inheritance might be coming from. -Alan
Re: RFR: 8319332: Security properties files inclusion [v11]
On Thu, 2 May 2024 19:09:07 GMT, Weijun Wang wrote: >> Francisco Ferrari Bihurriet has updated the pull request incrementally with >> one additional commit since the last revision: >> >> Profiles documentation adjustments. >> >> Co-authored-by: Francisco Ferrari >> Co-authored-by: Martin Balao > > src/java.base/share/conf/security/java.security line 35: > >> 33: # this file with a filesystem path value. The effect of each definition >> 34: # is to include a referred security properties file inline, adding all >> 35: # its properties and values. Included files, as well as files pointed by > > Not sure if "properties and values" are precise. A "property" already > contains its "key" and "value", right? Ok - PR Review Comment: https://git.openjdk.org/jdk/pull/16483#discussion_r1588228364
Re: RFR: 8319332: Security properties files inclusion [v11]
On Thu, 2 May 2024 19:06:00 GMT, Weijun Wang wrote: >> Francisco Ferrari Bihurriet has updated the pull request incrementally with >> one additional commit since the last revision: >> >> Profiles documentation adjustments. >> >> Co-authored-by: Francisco Ferrari >> Co-authored-by: Martin Balao > > src/java.base/share/conf/security/java.security line 69: > >> 67: # statement will override any matching property defined in a profile. In >> order >> 68: # to avoid this behavior, the include statement may be placed at the end >> of >> 69: # this file. > > It's probably better to place the newly added "Security properties defined > after an include statement..." in an individual paragraph since it's not > limited to "the latter case" (or you mean "last"?). Then you might not want > to use the "profile" word since it's not defined elsewhere. Instead of "to avoid this behavior", how about "if you want the properties defined in the included file to override those in the main file"? - PR Review Comment: https://git.openjdk.org/jdk/pull/16483#discussion_r1588204601
Re: RFR: 8319332: Security properties files inclusion [v11]
On Thu, 2 May 2024 16:45:09 GMT, Francisco Ferrari Bihurriet 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... > > Francisco Ferrari Bihurriet has updated the pull request incrementally with > one additional commit since the last revision: > > Profiles documentation adjustments. > > Co-authored-by: Francisco Ferrari > Co-authored-by: Martin Balao I still suggest you include all the new lines in `java.security` in the specification section of the the CSR in **raw code mode**. I know most of the content is already there but The CSR reviewers might care about the exact wording. src/java.base/share/conf/security/java.security line 35: > 33: # this file with a filesystem path value. The effect of each definition > 34: # is to include a referred security properties file inline, adding all > 35: # its properties and values. Included files, as well as files pointed by Not sure if "properties and values" are precise. A "property" already contains its "key" and "value", right? src/java.base/share/conf/security/java.security line 69: > 67: # statement will override any matching property defined in a profile. In > order > 68: # to avoid this behavior, the include statement may be placed at the end > of > 69: # this file. It's probably better to place the newly added "Security properties defined after an include statement..." in an individual paragraph since it's not limited to "the latter case" (or you mean "last"?). Then you might not want to use the "profile" word since it's not defined elsewhere. - PR Comment: https://git.openjdk.org/jdk/pull/16483#issuecomment-2091305451 PR Review Comment: https://git.openjdk.org/jdk/pull/16483#discussion_r1588209026 PR Review Comment: https://git.openjdk.org/jdk/pull/16483#discussion_r1588197310
Re: Potential issues with javax.crypto under StructuredTaskScope/JDK22
On 02/05/2024 19:33, Chris Marshall wrote: : Last week I upgraded the application to be compiled by JDK22, and run on JDK22. Immediately, we started to see failures from within the User-SRP auth code /only when it was run on a virtual thread from within a StructuredTaskScope./ The failures are merely that the code appears to have calculated the wrong authentication response (i.e. AWS Cognito returns a message to the effect that we have the wrong username or password). It is not possible that this could be the case, because the same application, using the same username/password combo is able to successfully authenticate to AWS Cognito using User-SRP auth from a platform thread. Thanks for reporting a potential issue. You say that the code was running correctly on JDK 21. Was this in the context of virtual threads and using StructuredTaskScope? I'm trying to understand from your mail if you were using virtual threads with JDK 21 and whether you were using StructuredTaskScope in JDK 21 too. "wrong username or password" hints that maybe this is some kinda of inheritance issue, I'm specifically thinking of the inherit access control context. Would it be possible to search the code and libraries that are in use here to see if they are using the javax.security.auth.Subject API? It's just a wild guess at this point but I think might give some clues as to where inheritance might be coming from. -Alan
Integrated: 8328864: NullPointerException in sun.security.jca.ProviderList.getService()
On Thu, 11 Apr 2024 16:29:00 GMT, Ben Perez wrote: > Updated `getService` to check whether `getProvider` returns null when > checking for preferred providers and `continue` the loop if that is the case. > Added `NullPreferredList` test. This pull request has now been integrated. Changeset: cd3a6075 Author:Ben Perez Committer: Weijun Wang URL: https://git.openjdk.org/jdk/commit/cd3a607576bede17f48c3d5ebde2bf05f3b615ba Stats: 48 lines in 3 files changed: 45 ins; 1 del; 2 mod 8328864: NullPointerException in sun.security.jca.ProviderList.getService() Reviewed-by: weijun - PR: https://git.openjdk.org/jdk/pull/18746
Potential issues with javax.crypto under StructuredTaskScope/JDK22
Hi,
I work for an organisation which runs a number of applications inside an EKS
cluster hosted in AWS. In order for the applications to communicate, they
obtained bearer tokens from the AWS Cognito library, and in order to obtain the
tokens, we have coded up User-SRP auth. This code has been running happily on
JDK21 since October 2023, and before that in JDK17.
The User-SRP auth implementation (which we took from
https://github.com/rundeck/repository/blob/master/repository-cli/src/main/groovy/com/rundeck/repository/auth/AuthenticationHelper.java)
runs within our applications sometimes from within a platform thread, and in
other cases on a virtual thread from inside a StructuredTaskScope. Again, this
code has been live for over 6 months.
Last week I upgraded the application to be compiled by JDK22, and run on JDK22.
Immediately, we started to see failures from within the User-SRP auth code only
when it was run on a virtual thread from within a StructuredTaskScope. The
failures are merely that the code appears to have calculated the wrong
authentication response (i.e. AWS Cognito returns a message to the effect that
we have the wrong username or password). It is not possible that this could be
the case, because the same application, using the same username/password combo
is able to successfully authenticate to AWS Cognito using User-SRP auth from a
platform thread.
My conclusion has to be that, on JDK22, and only from a virtual thread (within
a StructuredTaskScope?), the javax.crypto classes being used are not behaving
as intended.
I realise that this is not much to go on; I am not a security expert, and nor
do I know much about User-SRP, and don't really know how to go about
constructing a reproducer for this. We cannot replicate the issue running
locally on our machines from Windows; it only seems to happen from the machines
inside our AWS cluster. This is the relevant information:
OS: Linux amd64/5.10.184-175.731.amzn2.x86_64
Java: Oracle Corporation OpenJDK 64-bit Server VM/22+36-2370
Classes used within User-SRP auth layer:
MessageDigest.getInstance("SHA-256")
SecureRandom.getInstance("SHA1PRNG")
Mac.getInstance("HmacSHA256")
I hope that this is helpful.
Chris
Re: RFR: 8319332: Security properties files inclusion [v10]
On Thu, 2 May 2024 15:57:36 GMT, Weijun Wang wrote: > Another comment: now that the backward compatibility code is added, is it > still necessary to update the `DynStatic.java` test? @wangweij: it's no longer strictly necessary. However, we decided to leave it that way in order to lead by example with a well-formatted URI. Keeping `DynStatic.java` in this way, the wrongly formatted URIs passed to the `java.security.security` system property are all properly covered by _"Backward compatibility check"_ code comments. - PR Comment: https://git.openjdk.org/jdk/pull/16483#issuecomment-2091062323
Re: RFR: 8319332: Security properties files inclusion [v10]
On Thu, 2 May 2024 14:59:54 GMT, Weijun Wang wrote: > In the new `java.security` text, you have "the file profile.security must > exist". This is not necessary, right? I think what you meant to say is that > "if the system property is not defined, then it will be expanded to an empty > string and the file 'profie.security' must exist otherwise there will be an > error". Yes, you're right. We were thinking of a more common scenario but we have now made adjustments to document both options in the example. - PR Comment: https://git.openjdk.org/jdk/pull/16483#issuecomment-2091034333
Re: RFR: 8319332: Security properties files inclusion [v11]
> 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 properties file can be loaded from a URL. ... Francisco Ferrari Bihurriet has updated the pull request incrementally with one additional commit since the last revision: Profiles documentation adjustments. Co-authored-by: Francisco Ferrari Co-authored-by: Martin Balao - Changes: - all: https://git.openjdk.org/jdk/pull/16483/files - new: https://git.openjdk.org/jdk/pull/16483/files/a9b0ecbd..e906e3cf Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=16483=10 - incr: https://webrevs.openjdk.org/?repo=jdk=16483=09-10 Stats: 12 lines in 1 file changed: 7 ins; 0 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/16483.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16483/head:pull/16483 PR: https://git.openjdk.org/jdk/pull/16483
Re: RFR: 8319332: Security properties files inclusion [v10]
On Thu, 2 May 2024 14:07:13 GMT, Francisco Ferrari Bihurriet 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... > > Francisco Ferrari Bihurriet has updated the pull request with a new target > base due to a merge or a rebase. The pull request now contains 16 commits: > > - Update java.security documentation and fix test > >Adjust java.security documentation to match the latest CSR update, >explaining the definition of security profiles based on non-strict >system property expansion. > >Skip specialCharsIncludes test case in platforms whose filesystem >does not support paths containing Unicode characters. > >Co-authored-by: Francisco Ferrari >Co-authored-by: Martin Balao > - Merge 'openjdk/master' into JDK-8319332 > - Improve backward compatibility testing > >Co-authored-by: Francisco Ferrari >Co-authored-by: Martin Balao > - Extend backward compatibility support > >Co-authored-by: Francisco Ferrari >Co-authored-by: Martin Balao > - Merge 'openjdk/master' into JDK-8319332 > - Merge 'openjdk/master' into JDK-8319332 > - Merge 'openjdk/master' into JDK-8319332 > >Conflict in ConfigFileTest.java solved by keeping our file, which had >been previously adjusted. > >Commands: > git merge upstream/master > git restore --ours -- test/jdk/java/security/Security/ConfigFileTest.java > git add test/jdk/java/security/Security/ConfigFileTest.java > git merge --continue > - 8319332: Adjust code for JDK-8319673 changes > >JDK-8319673: Few security tests ignore VM flags > >Next, we will merge the openjdk/master branch and ignore the conflict in >this file. > >Co-authored-by: Martin Balao >Co-authored-by: Francisco Ferrari Bihurriet > - 8319332: Update copyright and ConfigFileTest.java. > >Bump copyright year to 2024 in all the modified files. > >Remove leaked host name from children JVMs debug command. > >Extract Executor::addSystemPropertiesAsJvmArgs from Executor::execute >and rename 'allJvmArgs' to 'command'. Also split class name and >RUNNER_ARG addition to 'command' as two separated command.add() calls. > >Co-authored-by: Martin Balao >Co-authored-by: Francisco Ferrari Bihurriet > - Merge 'openjdk/master' into JDK-8319332 > - ... and 6 more:
Re: RFR: 8319332: Security properties files inclusion [v10]
On Thu, 2 May 2024 14:07:13 GMT, Francisco Ferrari Bihurriet 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... > > Francisco Ferrari Bihurriet has updated the pull request with a new target > base due to a merge or a rebase. The pull request now contains 16 commits: > > - Update java.security documentation and fix test > >Adjust java.security documentation to match the latest CSR update, >explaining the definition of security profiles based on non-strict >system property expansion. > >Skip specialCharsIncludes test case in platforms whose filesystem >does not support paths containing Unicode characters. > >Co-authored-by: Francisco Ferrari >Co-authored-by: Martin Balao > - Merge 'openjdk/master' into JDK-8319332 > - Improve backward compatibility testing > >Co-authored-by: Francisco Ferrari >Co-authored-by: Martin Balao > - Extend backward compatibility support > >Co-authored-by: Francisco Ferrari >Co-authored-by: Martin Balao > - Merge 'openjdk/master' into JDK-8319332 > - Merge 'openjdk/master' into JDK-8319332 > - Merge 'openjdk/master' into JDK-8319332 > >Conflict in ConfigFileTest.java solved by keeping our file, which had >been previously adjusted. > >Commands: > git merge upstream/master > git restore --ours -- test/jdk/java/security/Security/ConfigFileTest.java > git add test/jdk/java/security/Security/ConfigFileTest.java > git merge --continue > - 8319332: Adjust code for JDK-8319673 changes > >JDK-8319673: Few security tests ignore VM flags > >Next, we will merge the openjdk/master branch and ignore the conflict in >this file. > >Co-authored-by: Martin Balao >Co-authored-by: Francisco Ferrari Bihurriet > - 8319332: Update copyright and ConfigFileTest.java. > >Bump copyright year to 2024 in all the modified files. > >Remove leaked host name from children JVMs debug command. > >Extract Executor::addSystemPropertiesAsJvmArgs from Executor::execute >and rename 'allJvmArgs' to 'command'. Also split class name and >RUNNER_ARG addition to 'command' as two separated command.add() calls. > >Co-authored-by: Martin Balao >Co-authored-by: Francisco Ferrari Bihurriet > - Merge 'openjdk/master' into JDK-8319332 > - ... and 6 more:
Re: RFR: 8319332: Security properties files inclusion [v10]
On Thu, 2 May 2024 14:07:13 GMT, Francisco Ferrari Bihurriet 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... > > Francisco Ferrari Bihurriet has updated the pull request with a new target > base due to a merge or a rebase. The pull request now contains 16 commits: > > - Update java.security documentation and fix test > >Adjust java.security documentation to match the latest CSR update, >explaining the definition of security profiles based on non-strict >system property expansion. > >Skip specialCharsIncludes test case in platforms whose filesystem >does not support paths containing Unicode characters. > >Co-authored-by: Francisco Ferrari >Co-authored-by: Martin Balao > - Merge 'openjdk/master' into JDK-8319332 > - Improve backward compatibility testing > >Co-authored-by: Francisco Ferrari >Co-authored-by: Martin Balao > - Extend backward compatibility support > >Co-authored-by: Francisco Ferrari >Co-authored-by: Martin Balao > - Merge 'openjdk/master' into JDK-8319332 > - Merge 'openjdk/master' into JDK-8319332 > - Merge 'openjdk/master' into JDK-8319332 > >Conflict in ConfigFileTest.java solved by keeping our file, which had >been previously adjusted. > >Commands: > git merge upstream/master > git restore --ours -- test/jdk/java/security/Security/ConfigFileTest.java > git add test/jdk/java/security/Security/ConfigFileTest.java > git merge --continue > - 8319332: Adjust code for JDK-8319673 changes > >JDK-8319673: Few security tests ignore VM flags > >Next, we will merge the openjdk/master branch and ignore the conflict in >this file. > >Co-authored-by: Martin Balao >Co-authored-by: Francisco Ferrari Bihurriet > - 8319332: Update copyright and ConfigFileTest.java. > >Bump copyright year to 2024 in all the modified files. > >Remove leaked host name from children JVMs debug command. > >Extract Executor::addSystemPropertiesAsJvmArgs from Executor::execute >and rename 'allJvmArgs' to 'command'. Also split class name and >RUNNER_ARG addition to 'command' as two separated command.add() calls. > >Co-authored-by: Martin Balao >Co-authored-by: Francisco Ferrari Bihurriet > - Merge 'openjdk/master' into JDK-8319332 > - ... and 6 more:
Re: RFR: 8319332: Security properties files inclusion [v10]
> 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 properties file can be loaded from a URL. ... Francisco Ferrari Bihurriet has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 16 commits: - Update java.security documentation and fix test Adjust java.security documentation to match the latest CSR update, explaining the definition of security profiles based on non-strict system property expansion. Skip specialCharsIncludes test case in platforms whose filesystem does not support paths containing Unicode characters. Co-authored-by: Francisco Ferrari Co-authored-by: Martin Balao - Merge 'openjdk/master' into JDK-8319332 - Improve backward compatibility testing Co-authored-by: Francisco Ferrari Co-authored-by: Martin Balao - Extend backward compatibility support Co-authored-by: Francisco Ferrari Co-authored-by: Martin Balao - Merge 'openjdk/master' into JDK-8319332 - Merge 'openjdk/master' into JDK-8319332 - Merge 'openjdk/master' into JDK-8319332 Conflict in ConfigFileTest.java solved by keeping our file, which had been previously adjusted. Commands: git merge upstream/master git restore --ours -- test/jdk/java/security/Security/ConfigFileTest.java git add test/jdk/java/security/Security/ConfigFileTest.java git merge --continue - 8319332: Adjust code for JDK-8319673 changes JDK-8319673: Few security tests ignore VM flags Next, we will merge the openjdk/master branch and ignore the conflict in this file. Co-authored-by: Martin Balao Co-authored-by: Francisco Ferrari Bihurriet - 8319332: Update copyright and ConfigFileTest.java. Bump copyright year to 2024 in all the modified files. Remove leaked host name from children JVMs debug command. Extract Executor::addSystemPropertiesAsJvmArgs from Executor::execute and rename 'allJvmArgs' to 'command'. Also split class name and RUNNER_ARG addition to 'command' as two separated command.add() calls. Co-authored-by: Martin Balao Co-authored-by: Francisco Ferrari Bihurriet - Merge 'openjdk/master' into JDK-8319332 - ... and 6 more: https://git.openjdk.org/jdk/compare/a024eed7...a9b0ecbd - Changes: https://git.openjdk.org/jdk/pull/16483/files Webrev: https://webrevs.openjdk.org/?repo=jdk=16483=09 Stats: 1282 lines in 6 files changed: 1052 ins;
