[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


Hello,

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
         #else
             lwsl_err("%s: openssl is too old\n", __func__);
         #endif

    (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 
lib/tls/openssl/openssl-session.c):
    +int
    +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,

Catalin




More information about the Libwebsockets mailing list