RFR 8223482: Unsupported ciphersuites may be offered by a TLS client

[Xuelei Fan]

Thank you, Marin! I can understand the benchmark more clearly.

The current update make an improvement by moving the checking to a 
singleton block.  That's good.  But I still has a concern about the 
loading performance impact of SSLContext.  That's, how long it will take 
to load the SSLContextImpl if checking the crypto operations there?

Thanks,
Xuelei

On 5/23/2019 12:52 PM, Martin Balao wrote:
> Hi Xuelei,
> 
> The JMH benchmark code is at
> http://cr.openjdk.java.net/~mbalao/webrevs/8223482/ciphersuites_benchmark_v0.tar.gz
> 
> Once the crypto providers and SSL engines are initialized, the following
> sequence is measured:
> 
>   1) Client and Server perform a handshake until status is
> HandshakeStatus.FINISHED on both ends
> 
>   2) Server renegotiates the session (SSLEngine.beginHandshake is called)
> so the sequence repeats.
> 
> See test_TLS12Communication method in
> ciphersuites/src/main/java/org/redhat/SupportedCiphersuites.java.
> 
> Webrev.00 was affecting performance because in each TLS handshake, the
> client was testing ciphersuites before moving forward. In Webrev.01 a
> performance impact is not seen here because ciphersuites checking occurs
> only at SSLContextImpl static class initialization time (not per
> instance but per static class initializer). Thus, to have a performance
> impact we would need the class loader to get rid of SSLContextImpl class
> -subclasses to be accurate- and load it again in every loop -which would
> make no sense-.
> 
> Here it's a call stack when "SSLContextImpl.getApplicableCipherSuites"
> gets executed:
> 
> Thread [main] (Suspended (breakpoint at line 374 in SSLContextImpl))	
> 	SSLContextImpl.getApplicableCipherSuites(Collection<CipherSuite>,
> List<ProtocolVersion>) line: 374	
> SSLContextImpl.getApplicableSupportedCipherSuites(List<ProtocolVersion>)
> line: 339	
> 	SSLContextImpl$AbstractTLSContext.<clinit>() line: 555	
> 	Class<T>.forName0(String, boolean, ClassLoader, Class<?>) line: not
> available [native method]	
> 	Class<T>.forName(String) line: 333	
> 	Provider$Service.getImplClass() line: 1842	
> 	Provider$Service.newInstance(Object) line: 1818	
> 	GetInstance.getInstance(Service, Class<?>) line: 236	
> 	GetInstance.getInstance(String, Class<?>, String, String) line: 206	
> 	SSLContext.getInstance(String, String) line: 229	
> 	SupportedCiphersuites.createSSLEngine(boolean) line: 273	
> 	SupportedCiphersuites.createSSLEngines() line: 256	
> 	SupportedCiphersuites.initialize() line: 239	
> 	SupportedCiphersuites.main(String[]) line: 71	
> 
> You see there how "SSLContextImpl$AbstractTLSContext.<clinit>()" static
> initializer is in the call stack.
> 
> I've not found any possible execution path to
> "SSLContextImpl.getApplicableCipherSuites" that does not come from a
> static class initializer.
> 
> Thanks,
> Martin.-
>