RFR: 8328608: Multiple NewSessionTicket support for TLS

[Anthony Scarpino]

On Tue, 4 Jun 2024 15:04:56 GMT, Jamil Nimeh <jnimeh at 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.
>
> src/java.base/share/classes/sun/security/ssl/SSLSessionImpl.java line 503:
> 
>> 501:                 this.preSharedKey = new SecretKeySpec(b, alg);
>> 502:                 // Get identity len
>> 503:                 i = buf.get();
> 
> Given that this is just a small change to a large function it may not be important to change, but a lot of these X-byte lengths followed by arrays might be simplified with Record.getBytes8 or the other similar routines that handle longer length fields.  But you wouldn't end up with a null byte[] when the length is zero, so that's a change, not necessarily a down-side.

I think that's something to consider as a new RFE as it would have a scope outside of this change

> src/java.base/share/classes/sun/security/util/Cache.java line 683:
> 
>> 681: 
>> 682:         // Limit the number of queue entries.
>> 683:         private static final int MAXQUEUESIZE = 10;
> 
> What do you think about making this tunable through the constructor, perhaps with a default value of 10?

I thought about this, but found no reason as this is an internal API.   `QueueCacheEntry<>` is only for TLS client NewSessionTicket situation and the API would have to set that at the time the whole cache is built.  Even with that, it would still be a hard coded number, just somewhere else.  Unless we have yet another system property.  I don’t see a need for individual tuning at this time and we can always up the hardcoded limit if more are needed. If it is necessary in the future we can revisit this.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19465#discussion_r1638775456
PR Review Comment: https://git.openjdk.org/jdk/pull/19465#discussion_r1628164520