From: Matt Caswell Date: Fri, 26 May 2017 16:59:34 +0000 (+0100) Subject: Allow the server to change the ciphersuite on resume X-Git-Tag: OpenSSL_1_1_1-pre1~1304 X-Git-Url: https://git.openssl.org/?p=openssl.git;a=commitdiff_plain;h=a055a8815587f402d700093dea0dec6bf34631a3 Allow the server to change the ciphersuite on resume Reviewed-by: Ben Kaduk (Merged from https://github.com/openssl/openssl/pull/3623) --- diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index e85c9a073e..37e31666be 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -2158,6 +2158,7 @@ SSL_R_CCS_RECEIVED_EARLY:133:ccs received early SSL_R_CERTIFICATE_VERIFY_FAILED:134:certificate verify failed SSL_R_CERT_CB_ERROR:377:cert cb error SSL_R_CERT_LENGTH_MISMATCH:135:cert length mismatch +SSL_R_CIPHERSUITE_DIGEST_HAS_CHANGED:218:ciphersuite digest has changed SSL_R_CIPHER_CODE_WRONG_LENGTH:137:cipher code wrong length SSL_R_CIPHER_OR_HASH_UNAVAILABLE:138:cipher or hash unavailable SSL_R_CLIENTHELLO_TLSEXT:226:clienthello tlsext diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h index fdb59f4a7a..8dfc974338 100644 --- a/include/openssl/sslerr.h +++ b/include/openssl/sslerr.h @@ -403,6 +403,7 @@ int ERR_load_SSL_strings(void); # define SSL_R_CERTIFICATE_VERIFY_FAILED 134 # define SSL_R_CERT_CB_ERROR 377 # define SSL_R_CERT_LENGTH_MISMATCH 135 +# define SSL_R_CIPHERSUITE_DIGEST_HAS_CHANGED 218 # define SSL_R_CIPHER_CODE_WRONG_LENGTH 137 # define SSL_R_CIPHER_OR_HASH_UNAVAILABLE 138 # define SSL_R_CLIENTHELLO_TLSEXT 226 diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index ffbe663d9a..e8bda66d61 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3728,11 +3728,24 @@ const SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt, (DTLS_VERSION_LT(s->version, c->min_dtls) || DTLS_VERSION_GT(s->version, c->max_dtls))) continue; - /* - * Since TLS 1.3 ciphersuites can be used with any auth or - * key exchange scheme skip tests. - */ - if (!SSL_IS_TLS13(s)) { + + if (SSL_IS_TLS13(s)) { + /* + * We must choose a ciphersuite that has a digest compatible with + * the session, unless we're going to do an HRR in which case we + * will just choose our most preferred ciphersuite regardless of + * whether it is compatible with the session or not. + */ + if (s->hit + && !s->hello_retry_request + && ssl_md(c->algorithm2) + != ssl_md(s->session->cipher->algorithm2)) + continue; + } else { + /* + * These tests do not apply to TLS 1.3 ciphersuites because they can + * be used with any auth or key exchange scheme. + */ mask_k = s->s3->tmp.mask_k; mask_a = s->s3->tmp.mask_a; #ifndef OPENSSL_NO_SRP diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c index 29e6648d91..eccc5af22b 100644 --- a/ssl/ssl_err.c +++ b/ssl/ssl_err.c @@ -622,6 +622,8 @@ static const ERR_STRING_DATA SSL_str_reasons[] = { {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_CERT_CB_ERROR), "cert cb error"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_CERT_LENGTH_MISMATCH), "cert length mismatch"}, + {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_CIPHERSUITE_DIGEST_HAS_CHANGED), + "ciphersuite digest has changed"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_CIPHER_CODE_WRONG_LENGTH), "cipher code wrong length"}, {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_CIPHER_OR_HASH_UNAVAILABLE), diff --git a/ssl/statem/statem_clnt.c b/ssl/statem/statem_clnt.c index 020589f1b1..7dd921eeff 100644 --- a/ssl/statem/statem_clnt.c +++ b/ssl/statem/statem_clnt.c @@ -1268,9 +1268,26 @@ static int set_client_ciphersuite(SSL *s, const unsigned char *cipherchars) if (s->session->cipher != NULL) s->session->cipher_id = s->session->cipher->id; if (s->hit && (s->session->cipher_id != c->id)) { - SSLerr(SSL_F_SET_CLIENT_CIPHERSUITE, - SSL_R_OLD_SESSION_CIPHER_NOT_RETURNED); - return 0; + if (SSL_IS_TLS13(s)) { + /* + * In TLSv1.3 it is valid for the server to select a different + * ciphersuite as long as the hash is the same. + */ + if (ssl_md(c->algorithm2) + != ssl_md(s->session->cipher->algorithm2)) { + SSLerr(SSL_F_SET_CLIENT_CIPHERSUITE, + SSL_R_CIPHERSUITE_DIGEST_HAS_CHANGED); + return 0; + } + } else { + /* + * Prior to TLSv1.3 resuming a session always meant using the same + * ciphersuite. + */ + SSLerr(SSL_F_SET_CLIENT_CIPHERSUITE, + SSL_R_OLD_SESSION_CIPHER_NOT_RETURNED); + return 0; + } } s->s3->tmp.new_cipher = c; diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index e2d618c863..8137a7da9e 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -1647,8 +1647,12 @@ static int tls_early_post_process_client_hello(SSL *s, int *pal) } } - /* If it is a hit, check that the cipher is in the list */ - if (s->hit) { + /* + * If it is a hit, check that the cipher is in the list. In TLSv1.3 we can + * resume with a differnt cipher as long as the hash is the same so this + * check does not apply. + */ + if (!SSL_IS_TLS13(s) && s->hit) { j = 0; id = s->session->cipher->id; @@ -1850,7 +1854,7 @@ static int tls_early_post_process_client_hello(SSL *s, int *pal) * Given s->session->ciphers and SSL_get_ciphers, we must pick a cipher */ - if (!s->hit || s->hello_retry_request) { + if (!s->hit || SSL_IS_TLS13(s)) { sk_SSL_CIPHER_free(s->session->ciphers); s->session->ciphers = ciphers; if (ciphers == NULL) { @@ -1956,7 +1960,7 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst) wst = WORK_MORE_B; } if (wst == WORK_MORE_B) { - if (!s->hit || s->hello_retry_request) { + if (!s->hit || SSL_IS_TLS13(s)) { /* Let cert callback update server certificates if required */ if (s->cert->cert_cb) { int rv = s->cert->cert_cb(s, s->cert->cert_cb_arg); @@ -1980,7 +1984,7 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst) SSL_R_NO_SHARED_CIPHER); goto f_err; } - if (SSL_IS_TLS13(s) && s->s3->tmp.new_cipher != NULL + if (s->hello_retry_request && s->s3->tmp.new_cipher != NULL && s->s3->tmp.new_cipher->id != cipher->id) { /* * A previous HRR picked a different ciphersuite to the one we