On Thu, 22 Dec 2022 15:00:18 GMT, Volker Simonis <simo...@openjdk.org> wrote:

>> Currently, TLS session tickets introduced by 
>> [JDK-8211018](https://bugs.openjdk.org/browse/JDK-8211018) in JDK 13 (i.e. 
>> `SessionTicketExtension$StatelessKey`) are generated in the class 
>> `SessionTicketExtension` and they use a single, global key ID 
>> (`currentKeyID`) for all `SSLContext`s.
>> 
>> This is problematic if more than one `SSLContext` is used, because every 
>> context which requests a session ticket will increment the global id 
>> `currentKeyID` when it creates a ticket. This means that in turn all the 
>> other contexts won't be able to find a ticket under the new id in their 
>> `SSLContextImpl` and create a new one (again incrementing `currentKeyID`). 
>> In fact, every time a ticket is requested from a different context, this 
>> will transitively trigger a new ticket creation in all the other contexts. 
>> We've observed millions of session ticket accumulating for some workloads.
>> 
>> Another issue with the curent implementation is that cleanup is racy because 
>> the underlying data structure (i.e. `keyHashMap` in `SSLContextImpl`) as 
>> well as the cleanup code itself are not threadsafe.
>> 
>> I therefor propose to move `currentKeyID` into the `SSLContextImpl` to solve 
>> these issues.
>> 
>> The following test program (contributed by Steven Collison 
>> (https://raycoll.com/)) can be used to demonstrate the current behaviour. It 
>> outputs the number of `StatelessKey` instances at the end of the program. 
>> Opening 1000 connections with a single `SSLContext` results in a single 
>> `StatelessKey` instance being created:
>> 
>> $ java -XX:+UseSerialGC -Xmx16m -cp ~/Java/ 
>> SSLSocketServerMultipleSSLContext 9999 1 1000
>> 605:             1             32  
>> sun.security.ssl.SessionTicketExtension$StatelessKey (java.base@20-internal)
>> 
>> The same example with the 1000 connections being opened alternatively on 
>> thwo different contexts will instead create 1000 `StatelessKey` instances:
>> 
>> $ java -XX:+UseSerialGC -Xmx16m -cp ~/Java/ 
>> SSLSocketServerMultipleSSLContext 9999 2 1000
>>   11:          1000          32000  
>> sun.security.ssl.SessionTicketExtension$StatelessKey (java.base@20-internal)
>> 
>> With my proposed patch, the numbers goes back to two instances again:
>> 
>> $ java -XX:+UseSerialGC -Xmx16m -cp ~/Java/ 
>> SSLSocketServerMultipleSSLContext 9999 2 1000
>> 611:             2             64  
>> sun.security.ssl.SessionTicketExtension$StatelessKey (java.base@20-internal)
>> 
>> 
>> I've attached the test program to the [JBS 
>> issue](https://bugs.openjdk.org/browse/JDK-8298381). If you think it makes 
>> sense, I can probably convert it into a JTreg test.
>
> Volker Simonis has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Moved stateless key logic from SSLContextImpl to SSLSessionContextImpl and 
> addressed comments by @XueleiFan and @ascarpino

src/java.base/share/classes/sun/security/ssl/SessionTicketExtension.java line 
122:

> 120:             try {
> 121:                 KeyGenerator kg = KeyGenerator.getInstance("AES");
> 122:                 kg.init(KEYLEN, hc.sslContext.getSecureRandom());

I think it's important to us random from hc. Configurations with specific 
providers would not want to use a different provider.

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

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

Reply via email to