8315487: Security Providers Filter #15539
Open
+3,742
−398
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
👋 Welcome back mbalao! A progress list of the required criteria for merging this PR into |
|
@martinuy The following labels will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
|
@martinuy This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
I wish we could keep this PR open. This is the latest comment in the ongoing discussion: https://mail.openjdk.org/pipermail/security-dev/2023-October/037479.html |
|
@martinuy This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
Mailing list message from Sean Mullan on security-dev: Hi Martin and Francisco, Thank you for your work on proposing this feature. This is a significant change to the JCA/JCE provider mechanism and consists of (roughly) over a thousand lines of code changes. Based on that and after spending some time understanding and reviewing the proposal, we feel that a JEP is required for this feature [1, 2]. A JEP will give it more visibility, broader review, and provide more time to ensure that the design is sound and open issues have been considered and adequately resolved. This likely means that it won?t make JDK 22 but I think that is fine as I don?t see any critical urgency in getting this into the next release of the JDK. I can help with the JEP process should you have questions. Thanks, [1] https://openjdk.org/jeps/1 |
martinuy
force-pushed
the
JDK-8315487
branch
from
9b9350b to
ef6eafc
Compare
|
@martinuy Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information. |
|
@martinuy This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
@martinuy Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information. |
|
@martinuy this pull request can not be integrated into git checkout JDK-8315487
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push |
openjdk
bot
added
merge-conflict
|
@martinuy This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
❗ This change is not yet ready to be integrated. |
|
@martinuy This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
/label remove hotspot-runtime |
|
@dholmes-ora |
|
@martinuy This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
@martinuy This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the |
|
/open |
|
@martinuy I suggest you remove the core-libs component from this review (using |
|
/label remove core-libs |
|
@martinuy |
|
I have been starting to review the code, and am initially reviewing this with respect to how it complies with the current API specification. All of the JCA API
However, the providers filter can be configured to prevent that object from being returned. I think this is an important difference in behavior that it should be documented as an implementation note. My initial suggestion is something like the following: "The JDK Reference Implementation additionally uses the I think similar text will need to be added in the |
openjdk
bot
added
merge-conflict
@martinuy Any comments on the above? |
|
Hi @seanjmullan, We agree, including the need for documenting this in We will make these changes in the coming days, we were first addressing #22613 changes and adjusting this PR which depends on it. The two PRs are now in sync, so feel free to continue your review in the meantime. |
|
@martinuy This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
The specification update and implementation details is far from I can images from last read of the pull request. It reminds me the way security manager was used to be: permission code was inserted everywhere, and hard to configure to use it in practice. |
|
The complexity mainly comes from the legacy use of Provider.put() methods. I was just wondering if it is ok to places filter at Provider.putService() only, with specification update. Putservice() Specification compliant provider, including all JDK built-in providers, will work with the filter. Applications that would like to benefit from this feature could choose to use specification compliant providers. The Provider.put() methods could be "deprecated" somehow in the future. This is just for your reference, please just ignore it if you don't want to consider this direction. |
|
@martinuy This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
Co-authored-by: Francisco Ferrari Bihurriet <[email protected]> Co-authored-by: Martin Balao <[email protected]>
|
@martinuy Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information. |
|
New version pushed, independent from #22613. Regression run of
|
|
|
openjdk
bot
added
merge-conflict
openjdk
bot
added
the
merge-conflict
|
@martinuy This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a |
seanjmullan
reviewed
Jun 10, 2025
| * <li> The {@code jdk.security.providers.filter} | ||
| * {@link System#getProperty(String) System} and | ||
| * {@link Security#getProperty(String) Security} properties determine | ||
| * which services are enabled. A service that is not enabled by the |
There was a problem hiding this comment.
In these and other APIs, I think it would be useful to link to java.security.Provider.Service when mentioning "services" since this is the first mention of that term in this API.
| String matchKey = (String)e.nextElement(); | ||
| if (key.equalsIgnoreCase(matchKey)) { | ||
| prop = provider.getProperty(matchKey); | ||
| private static Provider.Service findService(String type, String algo, |
There was a problem hiding this comment.
You need to add a similar implementation note about the jdk.security.providers.filter property to the getProviders(String) method since it can affect what providers are returned.
Conflicts ========= src/java.base/share/classes/javax/crypto/Cipher.java Caused by JDK-8358159 (3ff83ec), which did something similar to what we had done for this proposal. Resolved by keeping JDK-8358159 changes. src/java.base/share/conf/security/java.security Caused by JDK-8298420 (bb2c80c), which added content at the end. Trivially resolved.
openjdk
bot
removed
the
merge-conflict
Due to our changes, AlgorithmDecomposer::getTransformationTokens returns an array which does not always have three elements, so the code from JDK-8358159 (3ff83ec) needs adjustment.
Add a java.security.Provider.Service link in the 'services' mention of the jdk.security.providers.filter @implNote. NOTE: instead of the first mention in each API, we preferred to add it everywhere in order to simplify the criteria. Co-authored-by: Francisco Ferrari Bihurriet <[email protected]> Co-authored-by: Martin Balao <[email protected]>
Create a jdk.security.providers.filter @implNote for: • java.security.Security::getProviders(String) • java.security.Security::getProviders(java.util.Map<String,String>) Co-authored-by: Francisco Ferrari Bihurriet <[email protected]> Co-authored-by: Martin Balao <[email protected]>
Create a jdk.security.providers.filter @implNote for java.security.Security::getAlgorithms, explaining that this method is NOT affected by the filter. NOTE: we could have modified the method to stop using the Provider Hashtable and start to be affected by the filter, but we think this is unnecessary, given the method doesn't look to be widely used. Co-authored-by: Francisco Ferrari Bihurriet <[email protected]> Co-authored-by: Martin Balao <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In addition to the goals, scope, motivation, specification and requirement notes in JDK-8315487, we would like to describe the most relevant decisions taken during the implementation of this enhancement. These notes are organized by feature, may encompass more than one file or code segment, and are aimed to provide a high-level view of this PR.
ProvidersFilter
Filter construction (parser)
The providers filter is constructed from a string value, taken from either a system or a security property with name "jdk.security.providers.filter". This process occurs at sun.security.jca.ProvidersFilter class —simply referred as ProvidersFilter onward— static initialization. Thus, changes to the filter's overridable property are not effective afterwards and no assumptions should be made regarding when this class gets initialized.
The filter's string value is processed with a custom parser of order 'n', being 'n' the number of characters. The parser, represented by the ProvidersFilter.Parser class, can be characterized as a Deterministic Finite Automaton (DFA). The ProvidersFilter.Parser::parse method is the starting point to get characters from the filter's string value and generate state transitions in the parser's internal state-machine. See ProvidersFilter.Parser::nextState for more details about the parser's states and both valid and invalid transitions. The ParsingState enum defines valid parser states and Transition the reasons to move between states. If a filter string cannot be parsed, a ProvidersFilter.ParserException exception is thrown, and turned into an unchecked IllegalArgumentException in the ProvidersFilter.Filter constructor.
While we analyzed —and even tried, at early stages of the development— the use of regular expressions for filter parsing, we discarded the approach in order to get maximum performance, support a more advanced syntax and have flexibility for further extensions in the future.
Filter (structure and behavior)
A filter is represented by the ProvidersFilter.Filter class. It consists of an ordered list of rules, returned by the parser, that represents filter patterns from left to right (see the filter syntax for reference). At the end of this list, a match-all and deny rule is added for default behavior. When a service is evaluated against the filter, each filter rule is checked in the ProvidersFilter.Filter::apply method. The rule makes an allow or deny decision if the service matches it. Otherwise, the filter moves to the next rule in the sequence.
Rules are made of 3 regular expressions, derived from a filter pattern: provider, service type and algorithm or alias. A service matches a rule when its provider, service type and algorithm or alias matches the corresponding regular expressions in the rule. When a rule is matched by a service, it casts a decision (represented by the ProvidersFilter::FilterDecision class) that has two values: an allow or deny result and a priority that depends on how early (or left, in filter string terms) the rule is positioned in relative terms. Priorities are used for services that have aliases, as a mechanism to disambiguate contradictory decision results depending on which alias or algorithm is evaluated.
When a service with aliases is passed through a filter, independent evaluations are made for the algorithm and each alias. The decision with highest priority (lowest in absolute numbers) is finally effective.
Filter decisions cache
To accomplish the goal of maximizing performance, most services are passed through the Providers filter at registration time, when added with the java.security.Provider::putService or java.security.Provider::put APIs. While uncommon, providers may override java.security.Provider::getService or java.security.Provider::getServices APIs and return services that were never registered. In these cases, the service is evaluated against the Providers filter the first time used.
Once a service is evaluated against the filter, the decision is stored in the private isAllowed Provider.Service class field. When authorizing further uses of the service, the value from this cache is read, instead of performing a new filter evaluation. If the service does not experience any change, such as gaining or losing an alias (only possible with the legacy API), the cached value remains valid. Otherwise, a new filter evaluation has to take place. For example, a service could have been not allowed but a new alias matches an authorization rule in the filter that flips the previous decision.
The method Provider.Service::computeIsAllowed (that internally invokes ProvidersFilter::computeIsAllowed) can be used to force the recomputation of an authorization cached decision. The method ProvidersFilter::isAllowed, when filtering capabilities are enabled, tries to get the service authorization from the Provider.Service isAllowed field, and triggers a computation if not initialized. For this mechanism to work, the Provider.Service::getIsAllowed private method is exposed through SharedSecrets and accessed from ProvidersFilter.
Filter checking idiom
At every point in the JDK where any of Provider::getService or Provider::getServices APIs are invoked, a Providers filter check must be applied by calling ProvidersFilter.isAllowed(serviceInstance). It's assumed that serviceInstance is not null. The returned value indicates if the serviceInstance service is allowed or not. When a service is not allowed, the caller must discard it. The reason why we need to apply this checking pattern is because Provider::getService or Provider::getServices APIs may be overwritten by a provider to return unregistered services that were not evaluated against the filter before. If these APIs were not overwritten, the implementation will only return allowed services.
Debugging the filter
There are 3 mechanisms to debug a filter:
1 - Set the "java.security.debug" System property to "jca" and find filter-related messages prefixed by "ProvidersFilter". This debug output includes information about the filter construction (parsing) as well as evaluations of services against the filter. Note: the application has to trigger the ProvidersFilter class static initialization for this output to be generated, for example by invoking java.security.Security::getProviders.
Example:
Filter construction messages:
Filter evaluation messages:
2 - Pass the -XshowSettings:security:providers JVM argument and check, for each statically installed security provider, which services are allowed and not allowed by the filter.
Example:
3 - When a filter cannot be constructed, the ProvidersFilter.ParserException exception includes the state of the filter at the time when the error occurred, and indicates which pattern could not be parsed.
Example:
Testing
As part of our testing, we observed no regressions in the following test categories:
Additionally, we introduced the following new regression tests:
Finally, we extended the following tests:
This contribution is co-authored by Francisco Ferrari and Martin Balao.
Progress
Issues
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/15539/head:pull/15539$ git checkout pull/15539Update a local copy of the PR:
$ git checkout pull/15539$ git pull https://git.openjdk.org/jdk.git pull/15539/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 15539View PR using the GUI difftool:
$ git pr show -t 15539Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/15539.diff
Using Webrev
Link to Webrev Comment