On Thu, 22 Feb 2024 01:14:24 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
> 104.764 ? 3.237 ops/s
> SSLHandshak...
src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java line 194:
> 192: }
> 193: } catch (Exception e) {
> 194: // ignore
Should this exception be logged?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/17956#discussion_r1509051768
