On Tue, 26 Mar 2024 06:00:33 GMT, Hai-May Chao <hc...@openjdk.org> wrote:
>> For the PKIX KeyManager and PKCS12 Keystore, when the TLS server sends the
>> ServerHello message and ultimately calls the
>> X509KeyManagerImpl.chooseEngineServerAlias() method, it retrieves the
>> private key from the keystore, decrypts it, and caches both the key and its
>> certificate. This caching currently occurs only during a single handshake.
>> Since decryption can be time-consuming, a modification has been implemented
>> to cache the keystore entries at initialization time. This way, it won't be
>> necessary to retrieve and decrypt the keys for multiple handshakes, which
>> could lead to performance drawbacks.
>>
>> A change was made to also update/refresh the cached entry as the
>> certificates in the PKCS12 keystore may change, for scenarios like when the
>> certificate expires and a new one is added under a different alias, and the
>> certificate chain returned by the PKCS12 keystore is not the same as the one
>> in the cache. While attempting to handle when to refresh a cached entry to
>> accommodate keystore changes, we would like to know if you agree that this
>> improvement is worth the risk. We would also like to know if you have a
>> preference for other options:
>>
>> 1. Accept that PKIX+PKCS12 is slow.
>> 2. Add a configuration option (system property, maybe) to decide the level
>> of caching (1 - same as the existing one, 2 - same caching as in
>> SunX509KeyManagerImpl, 3 - the new caching introduced in this change).
>>
>> Additionally, the benchmark test SSLHandshake.java is modified to include a
>> @Param annotation, allowing it to pass different KeyManagerFactory values
>> (SunX509 and PKIX) to the benchmark method.
>>
>> Running modified SSLHandshake.java test prior to the change that caches the
>> PKCS12 keystore entries for PKIX:
>> Benchmark (keyMgr) (resume) (tlsVersion) Mode Cnt
>> Score Error Units
>> SSLHandshake.doHandshake SunX509 true TLSv1.2 thrpt 15
>> 9346.292 ? 379.023 ops/s
>> SSLHandshake.doHandshake SunX509 true TLS thrpt 15
>> 940.175 ? 21.215 ops/s
>> SSLHandshake.doHandshake SunX509 false TLSv1.2 thrpt 15
>> 594.418 ? 23.374 ops/s
>> SSLHandshake.doHandshake SunX509 false TLS thrpt 15
>> 534.030 ? 16.709 ops/s
>> SSLHandshake.doHandshake PKIX true TLSv1.2 thrpt 15
>> 9359.086 ? 246.257 ops/s
>> SSLHandshake.doHandshake PKIX true TLS thrpt 15
>> 933.835 ? 81.388 ops/s
>> SSLHandshake.doHandshake PKIX false TLSv1.2 thrpt 15 ...
>
> Hai-May Chao has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Updated with John's comments
The current changes are for the PKIX KeyManager, and the SunX509 KeyManager
will not execute the PKIX code path. Therefore, the performance of the SunX509
KeyManager becomes irrelevant. Here are the results of another set of runs:
Without changes:
Benchmark (keyMgr)(resume) (tlsVersion) Mode
Cnt Score Error Units
SSLHandshake.doHandshake SunX509 true TLSv1.2 thrpt 15
9071.959 ? 372.691 ops/s
SSLHandshake.doHandshake SunX509 true TLS thrpt 15
913.964 ? 41.041 ops/s
SSLHandshake.doHandshake SunX509 false TLSv1.2 thrpt 15
602.424 ? 16.202 ops/s
SSLHandshake.doHandshake SunX509 false TLS thrpt 15
518.974 ? 21.392 ops/s
SSLHandshake.doHandshake PKIX true TLSv1.2 thrpt 15
9490.990 ? 118.610 ops/s
SSLHandshake.doHandshake PKIX true TLS thrpt
15 937.166 ? 23.937 ops/s
SSLHandshake.doHandshake PKIX false TLSv1.2 thrpt 15
104.519 ? 2.657 ops/s
SSLHandshake.doHandshake PKIX false TLS thrpt
15 102.763 ? 1.536 ops/s
With changes:
Benchmark (keyMgr)(resume) (tlsVersion) Mode
Cnt Score Error Units
SSLHandshake.doHandshake SunX509 true TLSv1.2 thrpt 15
9386.371 ? 587.227 ops/s
SSLHandshake.doHandshake SunX509 true TLS thrpt 15
973.139 ? 12.022 ops/s
SSLHandshake.doHandshake SunX509 false TLSv1.2 thrpt 15
615.763 ? 9.911 ops/s
SSLHandshake.doHandshake SunX509 false TLS thrpt 15
539.656 ? 11.618 ops/s
SSLHandshake.doHandshake PKIX true TLSv1.2 thrpt 15
9713.760 ? 202.706 ops/s
SSLHandshake.doHandshake PKIX true TLS thrpt
15 910.733 ? 87.172 ops/s
SSLHandshake.doHandshake PKIX false TLSv1.2 thrpt 15
603.587 ? 26.367 ops/s
SSLHandshake.doHandshake PKIX false TLS thrpt
15 533.897 ? 11.513 ops/s
In the case where "resume=false" for TLSv1.2 and TLS, the performance is slow
without changes, which was also reported in the bug report. However, with the
current change, the performance has improved. While there may be slightly
different score numbers for each run, there is no significant performance
hurt/impact.
I created a JMH benchmark to measure the time needed to build a TLS context
using different SSL protocols ("TLSv1.2" and "TLS") and different KeyManager
types ("SunX509" and "PKIX”). Here is the benchmark result:
Benchmark (keyMgr) (tlsVersion) Mode Cnt
Score Error Units
SSLHandshake.doHandshake SunX509 TLSv1.2 thrpt 15 112.920 ?
10.518 ops/s
SSLHandshake.doHandshake SunX509 TLS thrpt 15 122.255 ?
3.128 ops/s
SSLHandshake.doHandshake PKIX TLSv1.2 thrpt 15 115.956
? 5.490 ops/s
SSLHandshake.doHandshake PKIX TLS thrpt 15
118.512 ? 3.574 ops/s
The performance between the SunX509 and PKIX KeyManagers for building SSL
contexts seems fairly comparable. The changes made to X509KeyManagerImpl.java
to cache keystore entries in the credentialsMap at initialization time for the
PKIX KeyManager do not impact performance compared to SunX509.
The SunX509 KeyManager also caches keystore entries in its credentialsMap at
initialization time. That’s why option 2 (PKIX does the same caching as SunX509
at initialization time) is listed as one of the preferences. However, the
SunX509 KeyManager does not attempt to refresh a keystore entry in its cached
map to accommodate keystore changes.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/17956#issuecomment-2021860848
