In addition to the goals, scope, motivation, specification and requirement 
notes in [JDK-8315487](https://bugs.openjdk.org/browse/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:

java -Djava.security.debug=jca 
-Djdk.security.providers.filter="SunJCE.Cipher.AES" Main

Filter construction messages:

ProvidersFilter: Parsing: SunJCE.Cipher.AES
ProvidersFilter: --------------------
ProvidersFilter: Rule parsed: SunJCE.Cipher.AES
ProvidersFilter:  * Provider: SunJCE (regexp: \QSunJCE\E)
ProvidersFilter:  * Service type: Cipher (regexp: \QCipher\E)
ProvidersFilter:  * Algorithm: AES (regexp: \QAES\E)
ProvidersFilter:  * Decision: ALLOW - priority: 0
ProvidersFilter: Filter: SunJCE.Cipher.AES; !* (DEFAULT)
ProvidersFilter: --------------------

Filter evaluation messages:

ProvidersFilter: Service filter query (Provider: SunJCE, Service type: Cipher, 
Algorithm: AES)
ProvidersFilter:  * Decision: ALLOW - priority: 0
ProvidersFilter:  * Made by: SunJCE.Cipher.AES
ProvidersFilter: --------------------
ProvidersFilter: The queried service has aliases. Checking them for a final 
decision...
ProvidersFilter: --------------------
ProvidersFilter: Service filter query (Provider: SunJCE, Service type: Cipher, 
Algorithm: OID.2.16.840.1.101.3.4.1)
ProvidersFilter:  * Decision: DENY - priority: 1
ProvidersFilter:  * Made by: !* (DEFAULT)
ProvidersFilter: --------------------
ProvidersFilter: Service filter query (Provider: SunJCE, Service type: Cipher, 
Algorithm: 2.16.840.1.101.3.4.1)
ProvidersFilter:  * Decision: DENY - priority: 1
ProvidersFilter:  * Made by: !* (DEFAULT)
ProvidersFilter: --------------------
ProvidersFilter: Final decision based on AES algorithm: ALLOW - priority: 0


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:


java -XshowSettings:security:providers 
-Djdk.security.providers.filter="SunJCE.Cipher.AES" -version


Security provider static configuration: (in order of preference)
        ...
        ----------------------------------------
   Provider name: SunJCE
   ...
   Provider services allowed: (type : algorithm)
            Cipher.AES
              aliases: [2.16.840.1.101.3.4.1, OID.2.16.840.1.101.3.4.1]
   Provider services NOT allowed: (type : algorithm)
            AlgorithmParameterGenerator.DiffieHellman
              aliases: [1.2.840.113549.1.3.1, DH, OID.1.2.840.113549.1.3.1]
            ...
   ----------------------------------------
   ...


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:

java -XshowSettings:security:providers 
-Djdk.security.providers.filter="SunJCE.Cipher.AES; My Provider"


Caused by: sun.security.jca.ProvidersFilter$Filter$ParserException: Only 
whitespace characters are valid after a pattern. Whitespaces that are part of a 
provider name, service type or algorithm must be escaped.
 * State: POST_PATTERN
 * Filter string: SunJCE.Cipher.AES; My Provider
                                     ---^---


## ServicesMap

Existing Provider::getService and Provider.getServices APIs were changed to 
return services that are allowed by the Providers filter only. In addition, a 
new Provider::getServicesNotAllowed method (exposed through the SharedSecrets 
mechanism) was introduced to obtain services that are not allowed by the 
Providers filter, for informational purposes only (see 
-XshowSettings:security:providers). These changes motivated an analysis of how 
services are stored internally in a Provider instance, and lead to the 
refactoring that will be explained in this section.

### Existing bugs and inconsistencies

While analyzing the existing services map implementation, we categorized bugs 
found in 3 sets: 1) Concurrency bugs, 2) Serialization consistency bugs, and 3) 
Other bugs. While many of these bugs are related to corner cases that are 
unlikely to be triggered in real providers, others can potentially happen in 
highly concurrent scenarios and would be difficult to reproduce.

#### 1 - Concurrency bugs

1.1 - A concurrent Provider::getServices call may remove services that are 
temporarily considered invalid because they are in the midst of an update. I.e. 
a provider added an alias but not the class name still, and a reader removes 
the service in between:

Thread-1 (writer): Provider::put("Alg.Alias.MessageDigest.alias1", "alg1") ---> 
An invalid service is implicitly created
Thread-2 (reader): Provider::getServices() ---> The invalid service is removed
Thread-1 (writer): Provider::put("MessageDigest.alg1", "class1") ---> While the 
service is added again (valid, now), it won't have alias1.

While the situation sounds unlikely for a regular service registration, it is 
possible when deserializing Providers as the order in which Property map 
entries are visited is not guaranteed.

This scenario was not possible before JDK-8276660 because readers only removed 
invalid entries from legacyMap, but not from legacyStrings. As a result, 
invalid services could land in legacyMap without losing data after they become 
valid. See a reference to the JDK-8276660 removal here [1]. We have developed 
the ServicesConsistency::testInvalidGetServicesRemoval test to reflect this 
case.

Notice that the fact that legacyMap is a ConcurrentHashMap instance does not 
prevent this bug from happening, as the problem is between non-atomic and 
non-synchronized writes.

1.2 - This bug is similar to 1.1 but occurs in Provider::getService [2], and is 
reflected in the test ServicesConsistency::testInvalidGetServiceRemoval.

1.3 - When signaling legacyChanged = false after recomputing the services set 
here [3], a concurrent legacyMap update that right before set legacyChanged = 
true [4] [5] would be missed. This was introduced by JDK-8276660 because, 
previously, legacyChanged = false was done in ensureLegacyParsed and this 
method was called in a synchronized getService block. Notice here that making 
legacyChanged volatile did not prevent this issue, as the problem is that 
publishing the new computed set and resetting the legacyChanged variable is not 
an atomic operation and a new update could have happened in between. We think 
that this type of problem can be solved in a lock-free way with a pattern such 
as the one proposed in ServicesMap::getServicesSet, that includes a double 
compare and exchange with a placeholder while computing the set.

1.4 - In operations such as Provider::implReplaceAll, there is a window in 
which all services look gone for readers [6] and this may cause spurious 
NoSuchAlgorithmException exceptions. Before JDK-8276660 the operation was seen 
as atomic by readers. The replaceAll change in the Properties map was made on 
legacyStrings by writers but only visible after readers impacted changes under 
a synchronized block. We think that replaceAll and putAll should be atomic for 
readers. In particular, putAll would be the legacy API mechanism to ensure that 
readers see services only when they have all their attributes added, and there 
is no risk of obtaining a service with half of them. See a test that checks an 
atomic replaceAll behavior in ServicesConsistency::testReplaceAllIsAtomic and a 
test that checks a putAll atomic behavior in 
ServicesConsistency::testPutAllIsAtomic.

1.5 - When a reader obtains a service from the legacyMap, Service::newInstance 
or Service::supportsParameter may be invoked and cached values such as 
classCache, constructorCache, hasKeyAttributes, supportedFormats and 
supportedClasses set. If there are further changes to the service (i.e.: new 
attributes added), cached values are never invalidated and there is a single 
service instance. As a result, the service information becomes inconsistent and 
the new attributes are missed, even for new readers. This did not happen before 
JDK-8276660 because ensureLegacyParsed started with a clear legacyMap and new 
Service instances (with uninitialized cache fields) were added. This bug is 
reflected in ServicesConsistency::testInvalidCachedClass, 
ServicesConsistency::testInvalidCachedHasKeyAttributes and 
ServicesConsistency::testInvalidCachedSupportedKeyFormats tests.

#### 2 - Serialization consistency bugs

This type of bugs make a deserialized Provider to have different services 
(class names, aliases and attributes) than the original instance. What they 
have in common is one or more of the following traits: 1) lack of 
synchronization between the Properties map and the actual inner services map, 
2) an incorrect assumption that the Properties map is visited, while 
deserializing, in the same order in which entries were originally added, or 3) 
an inconsistent collision behavior between the Provider::put and 
Provider::putService APIs.

We will show some examples that, while unrealistic, serve for the purpose of 
illustrating the aforementioned inconsistencies:

2.1 - Case-sensitive entries

Ordered list of actions:

put("MessageDigest.alg", "class1")
put("MessageDigest.ALG", "class2")

The previous list of actions creates a single Service instance that has 
"class2" as its class name. In the Properties map, however, both entries are 
present.

List of actions in the order that they are executed while deserializing the 
provider:

put("MessageDigest.ALG", "class2")
put("MessageDigest.alg", "class1")

The created Service instance, after deserialization, has "class1" as its class 
name.

This case is reflected in the 
ServicesConsistency::testSerializationClassnameConsistency test.

2.2 - Entries by alias

Ordered list of actions:

put("MessageDigest.alg", "class1")
put("Alg.Alias.MessageDigest.alias", "alg")
put("Alg.Alias.MessageDigest.alias2", "alias")

The previous list of actions creates a single Service instance that has "alg" 
as its algorithm, and "alias" and "alias2" as its aliases.

List of actions in the order that they are executed while deserializing the 
provider:

put("Alg.Alias.MessageDigest.alias2", "alias")
put("Alg.Alias.MessageDigest.alias", "alg")
put("MessageDigest.alg", "class1")

After deserialization there will be two Service instances: one has "alias" as 
its algorithm and "alias2" as its alias, and the other has "alg" as its 
algorithm and "alias" as its alias. The former instance is invalid and 
reachable by "alias2" only, and the latter is valid and reachable by "alg" or 
"alias".

2.3 - Algorithm case-insensitive collision between Provider::putService and 
Provider::put

Ordered list of actions:

put("MessageDigest.ALG", "class1")
putService(new Service(provider, "MessageDigest", "alg", "class2", null, null))

The previous list of actions creates two Service instances, from which the one 
using "class2" as its class name is visible. However, the Properties map has 
entries for both services.

List of actions in the order that they are executed while deserializing the 
provider:

put("MessageDigest.alg", "class2")
put("MessageDigest.ALG", "class1")

After deserialization, only the Service instance that has "class1" as its class 
name is available.

This case is related to the ServicesConsistency::testCurrentAPIServicesOverride 
test.

2.4 - Alias collision between Provider::putService and Provider::put

putService(new Service(provider, "MessageDigest", "alg1", "class1", 
List.of("alias"), null))
put("MessageDigest.alg2", "class2")
put("Alg.Alias.MessageDigest.alias", "alg2")

The previous list of actions creates two service instances, from which the one 
using "class1" as its class name is visible by "alias". However, the Properties 
map has the entry "Alg.Alias.MessageDigest.alias" associated with the service 
instance using "class2" as its class name.

List of actions executed while deserializing the provider (in any order):

put("MessageDigest.alg1", "class1")
put("MessageDigest.alg2", "class2")
put("Alg.Alias.MessageDigest.alias", "alg2")

After deserialization, the Service instance using "class2" as its class name is 
the one identified by the alias "alias".

This same inconsistency may occur with the algorithm instead of the alias.

This case is related to the ServicesConsistency::legacyAPIServicesOverride test.

2.5 - Overwrites of algorithms with aliases

Ordered list of actions:

put("MessageDigest.alg1", "class1")
put("Alg.Alias.MessageDigest.alias1", "alg1")
put("MessageDigest.alg2", "class2")
put("Alg.Alias.MessageDigest.alg1", "alg2")

The previous list of actions creates two service instances, one that has "alg1" 
as its algorithm, "class1" as its class name and "alias1" as its alias, and the 
other that has "alg2" as its algorithm, "class2" as its class name and "alg1" 
as its alias.

List of actions in the order that they are executed while deserializing the 
provider:

put("MessageDigest.alg2", "class2")
put("Alg.Alias.MessageDigest.alg1", "alg2")
put("MessageDigest.alg1", "class1")
put("Alg.Alias.MessageDigest.alias1", "alg1")

After deserialization, a single Service instance is created. This instance has 
"alg2" as its algorithm, "class1" as its class name, and "alg1" and "alias1" as 
its aliases.

This case is related to the 
ServicesConsistency::testLegacyAPIAliasCannotBeAlgorithm test.

#### 3 - Other bugs

3.1 - Invalid services (without a class name) can be returned in 
Provider::getService, even when they are removed from the legacyMap [7]. This 
case is reflected in the ServicesConsistency::testInvalidServiceNotReturned 
test.

3.2 - Methods Provider::implMerge and Provider::implCompute pass a "null" value 
as the 2nd parameter to Provider::parseLegacy and a remove action [8]. In the 
case of an alias, Provider::parseLegacy will throw a NullPointerException here 
[9]. This issue has been introduced by JDK-8276660, and is reflected in the 
tests ServicesConsistency::testComputeDoesNotThrowNPE and 
ServicesConsistency::testMergeDoesNotThrowNPE.

Note: we might be overlooking more bugs, as we decided to go with a new 
implementation at this point.

### Proposal

We replaced the mechanism through which Service instances are organized inside 
a Provider, while maintaining the existing APIs to store and fetch services. 
These APIs are internally called Legacy and Current. The solution was designed 
to follow the principles of avoiding bottlenecks for service map readers 
(lock-free, thread-safe), ensuring consistency after Providers deserialization, 
ensuring consistency between the Provider's properties map and the actual 
services, enforcing consistency between the Legacy and Current APIs, and 
maintaining previous behavior to the maximum extent possible.

What follows is a description of the most relevant design choices:

1 - Properties map synchronization: what you see on the Properties map is what 
you get from the internal services map

Adding, modifying or removing a service has a direct or indirect impact on the 
Properties map to enforce consistency. For example, adding an uppercase entry 
may overwrite a previous lowercase one. While this behavior is different from a 
regular Properties structure, having both entries would break synchronization 
with the internal services map (which is case insensitive) and is problematic 
for serialization. Other cases, such as adding entries by aliases, are also 
handled and canonicalized.

2 - The Current API (Provider::putService) has preference over the Legacy API 
(Provider::put).

We kept the preference of Provider::getService and Provider::getServices 
methods for Current API services over Legacy API counterparts. However, the 
internal map is now unified and Legacy services may be overwritten —notice that 
the opposite is not possible—. This was redesigned considering that there is a 
single Properties map, and we aim to keep it aligned to the internal services 
map. For expected behavior between the Legacy and Current APIs, we suggest 
looking at the ServicesConsistency::testLegacyAPIServicesOverride and 
ServicesConsistency::testCurrentAPIServicesOverride tests.

3 - Copy-on-write strategy for Legacy API service changes

We implemented a copy-on-write strategy that reminds of the one before 
JDK-8276660 but has a key difference: the new implementation is lock-free and 
faster from a service readers point of view. This strategy makes it possible to 
have service consistency in multi-threaded scenarios, and fix many of the 
concurrency bugs previously described. In terms of time consistency, we do not 
enforce that constraint between different services obtained from 
Provider::getServices. In other words, the Set<Service> returned may have an 
old version of service A and a new version of service B, even when the change 
to service A was applied first. We consider that it is not worth paying a 
synchronization cost for this type of consistency. Notice that Current API 
services do not require this approach because they are immutable: once added, 
they cannot be changed.

4 - Lock-free Provider::getServices

The Provider::getServices method is optimized for service readers with a cached 
set and a lock-free implementation.

## SunNativeProvider

Changes were introduced to make the SunNativeProvider security provider use the 
Provider::putService API to register services, instead of the legacy 
Provider::putAll. This was the only security provider in OpenJDK using a 
non-Provider::putService API. While this change does not have any observable 
implications, it is in the interest of a better code —and sets a better 
example— to align it with the other OpenJDK security providers. In our 
assessment, these are the OpenJDK providers using the Providers::putService 
API: SunJCE, SunPKCS11, SunRsaSign, SunEC, SUN, SunJSSE, SunMSCAPI, XMLDSigRI, 
JdkLDAP, JdkSASL, SunJGSS, SunPCSC, Apple and SunJarVerification.

## Testing

As part of our testing, we observed no regressions in the following test 
categories:

  * jdk:tier1
  * jdk/java/security
  * jdk/sun/security

Additionally, we introduced the following new regression tests:

  * jdk/sun/security/provider/ProvidersFilterTest.java
  * jdk/java/security/Provider/ServicesConsistency.java

Finally, we extended the following tests:

  * jdk/tools/launcher/Settings.java


This contribution is co-authored by Francisco Ferrari and Martin Balao.

--
[1] - 
https://github.com/openjdk/jdk/blob/jdk-20-ga/src/java.base/share/classes/java/security/Provider.java#L1332
[2] - 
https://github.com/openjdk/jdk/blob/jdk-20-ga/src/java.base/share/classes/java/security/Provider.java#L1289
[3] - 
https://github.com/openjdk/jdk/blob/jdk-20-ga/src/java.base/share/classes/java/security/Provider.java#L1340
[4] - 
https://github.com/openjdk/jdk/blob/jdk-20-ga/src/java.base/share/classes/java/security/Provider.java#L1145
[5] - 
https://github.com/openjdk/jdk/blob/jdk-20-ga/src/java.base/share/classes/java/security/Provider.java#L1181
[6] - 
https://github.com/openjdk/jdk/blob/jdk-20-ga/src/java.base/share/classes/java/security/Provider.java#L976
[7] - 
https://github.com/openjdk/jdk/blob/jdk-20-ga/src/java.base/share/classes/java/security/Provider.java#L1285-L1301
[8] - 
https://github.com/openjdk/jdk/blob/jdk-20-ga/src/java.base/share/classes/java/security/Provider.java#L999-L1016
[9] - 
https://github.com/openjdk/jdk/blob/jdk-20-ga/src/java.base/share/classes/java/security/Provider.java#L1146

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

Commit messages:
 - 8315487: Security Providers Filter

Changes: https://git.openjdk.org/jdk/pull/15539/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15539&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8315487
  Stats: 4600 lines in 24 files changed: 4029 ins; 323 del; 248 mod
  Patch: https://git.openjdk.org/jdk/pull/15539.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/15539/head:pull/15539

PR: https://git.openjdk.org/jdk/pull/15539

Reply via email to