diff options
author | Benjamin Kaduk <bkaduk@akamai.com> | 2020-01-24 13:44:27 -0800 |
---|---|---|
committer | Benjamin Kaduk <kaduk@mit.edu> | 2020-03-13 14:20:14 -0700 |
commit | 2e3ec2e1578977fca830a47fd7f521e290540e6d (patch) | |
tree | eefd4e2c6d1c13fb2615a8fe057bc9aabe60f953 /ssl | |
parent | d74014c4b8740f28a54b562f799ad1e754b517b9 (diff) |
Code to thread-safety in ChangeCipherState
The server-side ChangeCipherState processing stores the new cipher
in the SSL_SESSION object, so that the new state can be used if
this session gets resumed. However, writing to the session is only
thread-safe for initial handshakes, as at other times the session
object may be in a shared cache and in use by another thread at the
same time. Reflect this invariant in the code by only writing to
s->session->cipher when it is currently NULL (we do not cache sessions
with no cipher). The code prior to this change would never actually
change the (non-NULL) cipher value in a session object, since our
server enforces that (pre-TLS-1.3) resumptions use the exact same
cipher as the initial connection, and non-abbreviated renegotiations
have produced a new session object before we get to this point.
Regardless, include logic to detect such a condition and abort the
handshake if it occurs, to avoid any risk of inadvertently using
the wrong cipher on a connection.
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/10943)
Diffstat (limited to 'ssl')
-rw-r--r-- | ssl/statem/statem_srvr.c | 10 |
1 files changed, 9 insertions, 1 deletions
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 00905eb760..1cc106876c 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -744,7 +744,15 @@ WORK_STATE ossl_statem_server_pre_work(SSL *s, WORK_STATE wst) case TLS_ST_SW_CHANGE: if (SSL_IS_TLS13(s)) break; - s->session->cipher = s->s3.tmp.new_cipher; + /* Writes to s->session are only safe for initial handshakes */ + if (s->session->cipher == NULL) { + s->session->cipher = s->s3.tmp.new_cipher; + } else if (s->session->cipher != s->s3.tmp.new_cipher) { + SSLfatal(s, SSL_AD_INTERNAL_ERROR, + SSL_F_OSSL_STATEM_SERVER_PRE_WORK, + ERR_R_INTERNAL_ERROR); + return WORK_ERROR; + } if (!s->method->ssl3_enc->setup_key_block(s)) { /* SSLfatal() already called */ return WORK_ERROR; |