On Mon, 24 Jun 2024 16:03:43 GMT, Anthony Scarpino <ascarp...@openjdk.org> 
wrote:

>> Hi
>> 
>> This change is to improve TLS 1.3 session resumption by allowing a TLS 
>> server to send more than one resumption ticket per connection and clients to 
>> store more.  Resumption is a quick way to use an existing TLS session to 
>> establish another session by avoiding the long TLS full handshake process.  
>> In TLS 1.2 and below, clients can repeatedly resume a session by using the 
>> session ID from an established connection.  In TLS 1.3, a one-time 
>> "resumption ticket" is sent by the server after the TLS connection has been 
>> established.  The server may send multiple resumption tickets to help 
>> clients that rapidly resume connections.  If the client does not have 
>> another resumption ticket, it must go through the full TLS handshake again.  
>> The current implementation in JDK 23 and below, only sends and store one 
>> resumption ticket.
>> 
>> The number of resumption tickets a server can send should be configurable by 
>> the application developer or administrator. [RFC 
>> 8446](https://www.rfc-editor.org/rfc/rfc8446) does not specify a default 
>> value.  A system property called `jdk.tls.server.newSessionTicketCount` 
>> allows the user to change the number of resumption tickets sent by the 
>> server.  If this property is not set or given an invalid value, the default 
>> value of 3 is used. Further details are in the CSR.
>> 
>> A large portion of the changeset is on the client side by changing the 
>> caching system used by TLS.  It creates a new `CacheEntry<>` type called 
>> `QueueCacheEntry<>` that will store multiple values for a Map entry.
>
> Anthony Scarpino has updated the pull request incrementally with three 
> additional commits since the last revision:
> 
>  - remove frag issue
>  - Comments, remove thread, set NST default to 1, allow 0
>  - comment cleanup

Please add a test that starts multiple resumptions in parallel using the 
tickets received in the first connection. The test should verify that:
- each resumption uses a different ticket
- all resumptions succeed
- if all tickets are in use, starting another connection with 
setEnableSessionCreation(false) should fail

You can use multiple sslengines to run the handshakes in parallel, use 
setEnableSessionCreation to ensure that all sessions are resumptions, and 
disable stateless tickets on the server side to make sure that the sessions 
connections use distinct tickets.

src/java.base/share/classes/sun/security/ssl/NewSessionTicket.java line 491:

> 489:     }
> 490: 
> 491:         /**

the indent still looks wrong

src/java.base/share/classes/sun/security/ssl/SSLConfiguration.java line 214:

> 212:                 SSLLogger.isOn("ssl,handshake")) {
> 213:                 SSLLogger.fine(
> 214:                     "jdk.tls.server.newSessionTicketCount defaults to 3 
> as " +

Please update the default

src/java.base/share/classes/sun/security/ssl/SSLSessionContextImpl.java line 93:

> 91:         if (server) {
> 92:             sessionCache = Cache.newSoftMemoryCache(cacheLimit, timeout);
> 93:             sessionHostPortCache = Cache.newSoftMemoryCache(cacheLimit, 
> timeout);

preexisting, but I wonder if we could use nullCache here and in client's 
sessionCache

src/java.base/share/classes/sun/security/util/Cache.java line 716:

> 714:                 }
> 715:                 if (entry.isValid(time)) {
> 716:                     // SoftReference get() returns the same as 
> entry.getValue()

this doesn't look right

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

PR Review: https://git.openjdk.org/jdk/pull/19465#pullrequestreview-2137983069
PR Review Comment: https://git.openjdk.org/jdk/pull/19465#discussion_r1652325926
PR Review Comment: https://git.openjdk.org/jdk/pull/19465#discussion_r1652325340
PR Review Comment: https://git.openjdk.org/jdk/pull/19465#discussion_r1652327574
PR Review Comment: https://git.openjdk.org/jdk/pull/19465#discussion_r1652368738

Reply via email to