[Libwebsockets] TLS session cache and reuse, issues in v4.2-stable

Catalin Raceanu cra at mega.nz
Tue Apr 27 12:08:04 CEST 2021


The recently added feature for caching and reusing TLS sessions has a 
few issues.
I was not sure which branch to create patches for (hopefully 
4.2-stable?), so I've added proposed solutions for each one here.

1. There is an incorrect call to increment ref count for a SSL_SESSION 
instance, in OpenSSL code, in
    lib/tls/openssl/openssl-session.c: lws_tls_session_dump_load()

    Current code does the following:
         sess = d2i_SSL_SESSION(...)    // docs: "the SSL_SESSION object 
is automatically allocated. The reference count is 1"

         // ...
         // later:
         #if defined(LWS_HAVE_SSL_SESSION_up_ref)
             SSL_SESSION_up_ref(sess);         // not needed
             lwsl_err("%s: openssl is too old\n", __func__);

    (There is no need to care about the call to SSL_set_session() 
either, as it does it ref count management by itself.)

    There is also an issue with some versions of BoringSSL, for which 
`#if defined(LWS_HAVE_SSL_SESSION_up_ref)` fails,
    and there are lots of "openssl is too old" error messages, although 
session caching API is present and functional
    (apart from the bugs reported here, that is).

    Proposed solution: simply remove the 5 lines of code above.

2. A SSL_SESSION loaded from persistent storage is lost and leaked, in 
OpenSSL code, in
    lib/tls/openssl/openssl-session.c: lws_tls_session_dump_load()

    Current code does correctly unserialize a SSL_SESSION instance, but 
it does not attach it to the container that is added to LWS cache.
    This is particularly bad, because `ts->session` will remain as NULL, 
it will fool the session-reuse mechanism (see the next bug),
    and it will crash the app when a dump from cache is attempted for 
its tag.

    Proposed solution: assign the newly unserialized SSL_SESSION 
instance to its designated pointer:
    +        ts->session = sess;
            lwsl_tlssess("%s: session loaded OK\n", __func__);

3. A SSL_SESSION instance is incorrectly evaluated for having been 
reused or not, in OpenSSL code, in
    lib/core-net/wsi.c: lws_tls_session_is_reused()

    Current code keeps an internal flag (tls_session_reused) that is set 
to 1 when a reuse for a session is attempted. It then
    later reports the session as being reused, based on this flag.
    This is incorrect, because a session intended to be reused may be 
rejected by the server and a new one would be created,
    which then would be incorrectly reported as reused.

    Proposed solution: have a dedicated implementation (for OpenSSL in 
    +lws_tls_session_is_reused(struct lws *wsi)
    +    int reused = SSL_session_reused(wsi->tls.ssl);
    +    return reused;

    Using tls_session_reused for mbedtls cannot be correct either. The 
ideal solution would be to remove it completely
    and have a similar check there too, but I don't know if it offers a 
similar API.
    However, if tls_session_reused will be kept for mbedtls, then when a 
new session is created, and it replaces an older one
    in the cache, this flag should also be reset.

Best regards,


More information about the Libwebsockets mailing list