[PATCH] SSL: raised limit for upstream session size

[Maxim Dounin]

Hello!

On Tue, Dec 26, 2023 at 12:29:54AM +0400, Sergey Kandaurov wrote:

> > On 23 Dec 2023, at 01:46, Maxim Dounin <mdounin at mdounin.ru> wrote:
> > 
> > Hello!
> > 
> > On Fri, Dec 22, 2023 at 06:28:34PM +0400, Sergey Kandaurov wrote:
> > 
> >> # HG changeset patch
> >> # User Sergey Kandaurov <pluknet at nginx.com>
> >> # Date 1703255284 -14400
> >> #      Fri Dec 22 18:28:04 2023 +0400
> >> # Node ID a463fb67e143c051fd373d1df94e5813a37d5cea
> >> # Parent  44266e0651c44f530c4aa66e68c1b9464a9acee7
> >> SSL: raised limit for upstream session size.
> >> 
> >> Unlike shared session cache used to store multiple client SSL sessions and
> >> which may be per a single SSL connection, sessions saved from upstream are
> >> per upstream server peer, so there is no such multiplier effect, but they
> >> may be of noticeably larger size due to session tickets being used.
> >> 
> >> It was observed that session tickets sent from JVM backends may result in
> >> a decoded session size nearly the previous maximum session size limit of
> >> 4096 or slightly beyond.  Raising the limit allows to save such sessions.
> > 
> > Session tickets are not expected to be larger than sessions 
> > itself, except by several bytes used for key identification and 
> > encryption overhead.  I see no reasons why the limit should be 
> > different in different places.
> > 
> > And 4096 for an SSL session looks a lot.  The only justification I 
> > can assume here is an SSL session with the client certificate (or 
> > even certificate chain) being saved into the session.  It might 
> > worth looking into what actually happens here.
> > 
> 
> Indeed.  Both local and peer certificate chains are serialized and
> encrypted as part of constructing a session ticket.  Per the original
> change to support tickets, this is hardcoded and may not be adjusted:
> https://hg.openjdk.org/jdk/jdk/rev/c2398053ee90#l4.352
> https://hg.openjdk.org/jdk/jdk/rev/c2398053ee90#l10.261

>From my limited understanding of the JDK code, at least peerCerts 
seems to contain only certificates actually sent by the client, 
which is understandable (links to Github, since hg.openjdk.org 
used to be unresponsive when writing this, and returned 504 for 
almost all requests):

https://github.com/openjdk/jdk/blob/4fc6b0ffa4f771991a5ebd982b5133d2e364fdae/src/java.base/share/classes/sun/security/ssl/CertificateMessage.java#L416

But localCerts seems to be always set on the server side, with all 
the certificates being sent to the client:

https://github.com/openjdk/jdk/blob/4fc6b0ffa4f771991a5ebd982b5133d2e364fdae/src/java.base/share/classes/sun/security/ssl/CertificateMessage.java#L265 

This looks like an issue on the JDK side: there are no reasons why 
server certificates needs to be saved into the session on the 
server, as they are readily available on the server.  Further, 
relevant saving code seems to be commented as "Client identity", 
which suggests that these might be saved unintentionally with an 
assumption that the code is only used on the client (OTOH, I don't 
see why the client needs its own certificates in the session as 
well).

Do you have an affected JVM backend on hand to confirm it's indeed 
the case?

I tend to think this needs to be reported to JDK developers.  
Their current code results in sending server certificate chain to 
the client at least two times (once in the handshake itself, and 
at least once in the ticket; not to mention that there can be more 
than one ticket in TLSv1.3), and they might reconsider doing this.

(Funny enough, they seems to be using cache to deserialize 
certificates from such tickets, see 
https://bugs.openjdk.org/browse/JDK-8286433 for details.)

Meanwhile, we can consider implementing a workaround on our side 
(that is, raising the limit, though I don't think there should be 
separate limits; also, I'm somewhat concerned about using 8k 
buffers on stack, we currently don't use anything larger than 4k) 
or instead focus on providing some guidance to users of affected 
JVM backends (I guess switching off tickets and/or TLSv1.3 should 
be enough in most cases).

-- 
Maxim Dounin
http://mdounin.ru/