RFR: 8274308: Improve efficiency for HandshakeContext initialization. [v2]

[Daniel Jeliński]

On Fri, 5 Nov 2021 22:57:24 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

>> src/java.base/share/classes/sun/security/ssl/HandshakeContext.java line 74:
>> 
>>> 72:                     "sun.security.ssl.allowLegacyHelloMessages", true);
>>> 73: 
>>> 74:     static Cache<Double, HandshakeContextCacheItem> handshakeContextCache;
>> 
>> I think I understand the motivation, but a mutable cache in handshake context does not sound like the right direction.  A cache could impact the performance more and occupy additional memories , because we have to synchronizing the access to the cache.  Maybe, you could to benchmark the maximum operations allowed per seconds, and try to use more complex cases, and see what the result could be.
>> 
>> I did not yet try to find an improvement yet.  But at this moment, the idea to me is to thinking about the TLS user cases.  It is common that applications use one instance of SSLContext and default configurations.  From the SSLContext instance, with the default configuration, the connections get established.  Normally, we don't recommend to set protocols and cipher suites other than the default SSLContex configuration, because it is not algorithm aglity.  So, it may be a direction to have the default and final active protocols and cipher suites, parsed and cached in SSLContext.  Just for your reference if you want a further improvement.
>
>> Hi @XueleiFan,
>> 
>> Thank you for the feedback, though it’s not recommended to set the protocols and cipher suites, various frameworks often do set these values, [Some references below]. Would it still make sense to only optimize the default case?
> 
> I know, there are applications setting protocols and cipher suites for themselves.  But it should be common that the TLS connections created from the same SSLContext uses the same cipher suites and protocols.  There are still exceptions, but may be not that common.  Using an instance cache in SSLContext rather than static cache could be a further improvement.
> 
> I would not use static cache unless there is no other option.  The troublesome of static cache is more than the performance impact.  A series of problems like memory leaking and security issues could be followed even we design the cache very carefully.
> 
> About the benchmark, did you have a chance to check the result for full handshakes?  The code in the benchmark checks the initialization of the handshake, the handshake may not be full/completed.  A benchmark for full handshake may be able to provide more evidence about the overall impact of the improvement.

Can we extend the public API of `SSLContext` with methods for managing `activeProtocols`, `activeCipherSuites` and possibly `algorithmConstraints`? Without this API change we would need to check every time if the active protocols, ciphers and constraints match the cached ones.

Also, looking at the flame graphs provided, it appears that about 50% of the handshake time is spent in `SSLAlgorithmDecomposer#decompose`; given that there are only 3 instances of that class (and could be further reduced to 2), would it make sense to optimize the method by caching the results of algorithm decomposition? We could use a size-limited soft cache to avoid memory leaks, and I fail to see how the cached information could be exploitable.

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

PR: https://git.openjdk.java.net/jdk/pull/5793