8315487: Security Providers Filter #15539
Closed
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! |
Contributor
Author
|
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! |
Member
|
/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 |
Contributor
Author
|
/open |
|
@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. |
Contributor
|
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 |
Member
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, |
Member
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.
Contributor
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]>
Conflicts ========= src/java.base/share/classes/javax/crypto/Cipher.java Caused by JDK-8359388 (ec7c6be). Resolved by adding JDK-8359388 new checks on our codebase (we need the checks separated from splitting).
|
@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 |
openjdk
bot
added
the
merge-conflict
Member
|
/reviewer remove seanjmullan |
|
@seanjmullan Only the author (@martinuy) is allowed to issue the |
Member
|
Accidentally approved the wrong PR, so need to remove my approval. |
seanjmullan
suggested changes
Sep 4, 2025
Member
seanjmullan
left a comment
seanjmullan
left a comment
There was a problem hiding this comment.
Not requesting any specific changes right now, but this should remove my accidental approval.
|
@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 |
|
@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 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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
Warnings
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