Browse Source

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)
master
Benjamin Kaduk 3 years ago
committed by Benjamin Kaduk
parent
commit
2e3ec2e157
3 changed files with 11 additions and 1 deletions
  1. +1
    -0
      crypto/err/openssl.txt
  2. +1
    -0
      include/openssl/sslerr.h
  3. +9
    -1
      ssl/statem/statem_srvr.c

+ 1
- 0
crypto/err/openssl.txt View File

@ -1310,6 +1310,7 @@ SSL_F_OSSL_STATEM_SERVER_CONSTRUCT_MESSAGE:431:*
SSL_F_OSSL_STATEM_SERVER_POST_PROCESS_MESSAGE:601:\
ossl_statem_server_post_process_message
SSL_F_OSSL_STATEM_SERVER_POST_WORK:602:ossl_statem_server_post_work
SSL_F_OSSL_STATEM_SERVER_PRE_WORK:640:
SSL_F_OSSL_STATEM_SERVER_PROCESS_MESSAGE:603:ossl_statem_server_process_message
SSL_F_OSSL_STATEM_SERVER_READ_TRANSITION:418:ossl_statem_server_read_transition
SSL_F_OSSL_STATEM_SERVER_WRITE_TRANSITION:604:\


+ 1
- 0
include/openssl/sslerr.h View File

@ -94,6 +94,7 @@ int ERR_load_SSL_strings(void);
# define SSL_F_OSSL_STATEM_SERVER_CONSTRUCT_MESSAGE 0
# define SSL_F_OSSL_STATEM_SERVER_POST_PROCESS_MESSAGE 0
# define SSL_F_OSSL_STATEM_SERVER_POST_WORK 0
# define SSL_F_OSSL_STATEM_SERVER_PRE_WORK 0
# define SSL_F_OSSL_STATEM_SERVER_PROCESS_MESSAGE 0
# define SSL_F_OSSL_STATEM_SERVER_READ_TRANSITION 0
# define SSL_F_OSSL_STATEM_SERVER_WRITE_TRANSITION 0


+ 9
- 1
ssl/statem/statem_srvr.c View File

@ -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;


Loading…
Cancel
Save