[PATCH 01 of 11] SSL: disabled saving tickets to session cache

[Sergey Kandaurov]

> On 17 Sep 2022, at 00:58, Maxim Dounin <mdounin at mdounin.ru> wrote:
> 
> Hello!
> 
> On Thu, Sep 15, 2022 at 09:36:31AM +0400, Sergey Kandaurov wrote:
> 
>>> On 26 Aug 2022, at 07:01, Maxim Dounin <mdounin at mdounin.ru> wrote:
>>> 
>>> # HG changeset patch
>>> # User Maxim Dounin <mdounin at mdounin.ru>
>>> # Date 1661481945 -10800
>>> #      Fri Aug 26 05:45:45 2022 +0300
>>> # Node ID 2cd8fbeb4edc5a99b725585edc02a16a8a0c503e
>>> # Parent  069a4813e8d6d7ec662d282a10f5f7062ebd817f
>>> SSL: disabled saving tickets to session cache.
>>> 
>>> OpenSSL for TLSv1.3 tries to save tickets into session cache "because some
>>> applications just want to know about the creation of a session".  To avoid
>>> trashing session cache with useless data, we do not save such sessions now.
>>> 
>> 
>> For the record, BoringSSL doesn't seem to call new_session_cb for TLSv1.3
>> at all, so there is no way to resume sessions with SSL_OP_NO_TICKET set.
>> In contrary, OpenSSL emits stateful tickets in this case, which contain
>> dummy session id used then as a session cache lookup key on server
>> (much like session ids in TLSv1.2)
>> 
>> OTOH, without SSL_OP_NO_TICKET set, OpenSSL emits self-containing tickets
>> with enough info to resume session, so nothing to lookup in session cache.
>> The latter makes impractical storing something in session cache, except
>> to use the callback for things like tracking "the creation of a session".
>> Namely, OpenSSL puts session (i.e. something that SSL_get_session returns)
>> and supplementary info to session ticket message as the ticket value.
> 
> It looks like you are trying to introduce "stateful tickets" and 
> "self-containing tickets" terms, which is somewhat confusing 
> unless carefully explained.  OpenSSL itself tries to use terms 
> "stateful tickets" and "stateless tickets", with the similar 
> drawbacks.

Indeed, OpenSSL SSL_OP_NO_TICKET documentation is what I refer to.
While OpenSSL terms may look odd, they are useful to describe the
difference (and tricks) in TLSv1.3 session resumption with and
without SSL_OP_NO_TICKET, as implemented to OpenSSL specifically.

> 
> A better explanation would be to follow generic term "session 
> ticket", as originally introduced in RFC 4507 for TLS session 
> resumption without server-side state.
> 
> In these terms (as always used before introduction of TLSv1.3 and 
> currently used in many places, including nginx own documentation 
> and the source code) there are two basic mechanisms to resume 
> sessions: server-side session cache and session tickets (used to 
> resume sessions without server-side state).
> 
> Without SSL_OP_NO_TICKET set, OpenSSL uses tickets as long as 
> supported by the client.
> 
> With SSL_OP_NO_TICKET set, OpenSSL does not use tickets, and uses 
> server-side session cache instead (if configured).
> 
> The only difference between TLSv1.3 and previous protocols is how 
> session ids are sent to the client if server-side session cache is 
> used.  In case of SSL and TLS up to and including TLSv1.2, session 
> ids are sent in the dedicated fields of the ServerHello and 
> ClientHello handshake messages.  In case of TLSv1.3, dedicated 
> fields were removed, so session ids are sent in the 
> NewSessionTicket messages ("a database lookup key" in terms of RFC 
> 8446).
> 
>> With these thoughts in mind, I think log could be clarified to emphasize:
>> - it's not tickets that are stored in cache
>> - with SSL_OP_NO_TICKET set TLSv1.3 session are still saved to lookup by id.
> 
> Hope it is clear enough now.

What I'd like to clarify is the difference between session and (session)
ticket applied to session cache.  Ticket is a container used to envelope
session state (essentially, a session) and send in the NewSessionTicket
message "to resume sessions and avoid keeping per-client session state",
as seen in RFC 8446, 4.6.1 (and somewhat similar in 4507/5077).

      struct {
          uint32 ticket_lifetime;
          uint32 ticket_age_add;
          opaque ticket_nonce<0..255>;
          opaque ticket<1..2^16-1>;
          Extension extensions<0..2^16-2>;
      } NewSessionTicket;

So it looks inappropriate to say that tickets are saved in cache.
OTOH, "ticket" can be seen a correct wording if speaking in terms
of NewSessionTicket ticket field, or in contrast to "session id"
synthesized by OpenSSL for the SSL_OP_NO_TICKET case in TLSv1.3.
In that sense, I'm fine with using "ticket" (not to say changing
"ticket" to "session" or "session state" brings a tautology).
However this looks not so important to spend more time on this,
I'm fine with either case.

> 
>> 
>>> diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
>>> --- a/src/event/ngx_event_openssl.c
>>> +++ b/src/event/ngx_event_openssl.c
>>> @@ -3815,6 +3815,22 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_
>>>    ngx_ssl_session_cache_t  *cache;
>>>    u_char                    buf[NGX_SSL_MAX_SESSION_SIZE];
>>> 
>>> +#ifdef TLS1_3_VERSION
>>> +
>>> +    /*
>>> +     * OpenSSL for TLSv1.3 tries to save tickets into session cache
>>> +     * "because some applications just want to know about the creation
>>> +     * of a session"; do not cache such sessions
>>> +     */
>>> +
>>> +    if (SSL_version(ssl_conn) == TLS1_3_VERSION
>>> +        && (SSL_get_options(ssl_conn) & SSL_OP_NO_TICKET) == 0)
>>> +    {
>>> +        return 0;
>>> +    }
>>> +
>>> +#endif
>>> +
>>>    len = i2d_SSL_SESSION(sess, NULL);
>>> 
>>>    /* do not cache too big session */
>>> 
>> 
>> Looks good.
>> [..]

-- 
Sergey Kandaurov