Data race with SSL_SESSION reuse and tlsext_tick

[Devon H. O'Dell]

Hi there,

I'm working on an application that shares SSL_SESSION pointers between
SSL_CTXs in multiple threads. The logic for sharing the session is
roughly as follows:

lock(mtx);
sp = get_cached_session_pointer();
if (!SSL_set_session(ctx, sp)) {
  SSL_set_session(ctx, NULL);
}
unlock(mtx);

r = SSL_connect(ctx)

if (r  0  !SSL_session_reused(ctx)) {
  lock(mtx);
  set_cached_session_pointer(SSL_get1_session(ctx));
  unlock(mtx);
  SSL_SESSION_free(sp);
}

With OpenSSL 1.0.1e, I'm running into what looks like a race condition
in which SSL_connect will call ssl3_get_new_session_ticket when it is
reusing a shared session. In this case, there is a race when multiple
threads simultaneously enter this state and both observe
ctx-session-tlsext_tick to be non-NULL, both will call OPENSSL_free,
and this can crash, especially with custom allocators. This race
triggers quite rarely doing about 7 connections per second (that's
about all the information I can offer as to the environment), but
quite rarely is unfortunately still often enough to need to fix
this.

It seems to me that the only way to do this safely is for any time we
enter the state SSL3_ST_CR_SESSION_TICKET_[AB], we call
ssl_get_new_session prior to the ssl3_get_new_session_ticket call. If
we were to just serialize the ssl3_get_new_session_ticket alloc /
free, it seems that the race still exists -- two new tickets may be
negotiated, but only one will wind up stored in the session. I imagine
this would have consequences in later communications. Alternatively,
hanging the tlsext_tick, tlsext_ticklen, and tlsext_tick_lifetime_hint
off of the SSL_CTX instead of SSL_SESSION would also work.

Serializing SSL_connect also fixes it (of course) but adding
indeterminate latency to future connects to solve the problem doesn't
work for me.

I'd greatly appreciate feedback as to whether this analysis makes
sense (and if so which approach to fixing it might be preferable for
submitting a patch) given that I'm more of a concurrency and
performance guy than a crypto guy. I'm also always open to being told
that I'm doing it wrong; in this case, I would appreciate insight into
how I might better implement the session reuse code to avoid this
race.

--dho
__
OpenSSL Project http://www.openssl.org
User Support Mailing Listopenssl-users@openssl.org
Automated List Manager   majord...@openssl.org

Re: Data race with SSL_SESSION reuse and tlsext_tick

[Viktor Dukhovni]

On Wed, Mar 26, 2014 at 05:25:49PM -0400, Devon H. O'Dell wrote:

 Hi there,
 
 I'm working on an application that shares SSL_SESSION pointers between
 SSL_CTXs in multiple threads. The logic for sharing the session is
 roughly as follows:
 
 lock(mtx);
 sp = get_cached_session_pointer();

Don't share session *objects*, serialize the session to DER form,
and cache that.  Each SSL connectiont that wants to re-use a session
will de-serialize the session creating a new session object.

You'll need the new_session_cb callbacks, ... to detect when new
sessions are created despite your best attempt to reuse, and then
cache a replacement session.

-- 
Viktor.
__
OpenSSL Project http://www.openssl.org
User Support Mailing Listopenssl-users@openssl.org
Automated List Manager   majord...@openssl.org