diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index e8bda66d61e7743d3239e571cfacc74aae372cc0..41c44ce62e3dcab009a2d12eb7b5380c04f883b3 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3680,7 +3680,7 @@ const SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt, const SSL_CIPHER *c, *ret = NULL; STACK_OF(SSL_CIPHER) *prio, *allow; int i, ii, ok; - unsigned long alg_k = 0, alg_a = 0, mask_k, mask_a; + unsigned long alg_k = 0, alg_a = 0, mask_k = 0, mask_a = 0; /* Let's see which ciphers we can support */ @@ -3714,8 +3714,10 @@ const SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt, allow = srvr; } - tls1_set_cert_validity(s); - ssl_set_masks(s); + if (!SSL_IS_TLS13(s)) { + tls1_set_cert_validity(s); + ssl_set_masks(s); + } for (i = 0; i < sk_SSL_CIPHER_num(prio); i++) { c = sk_SSL_CIPHER_value(prio, i); @@ -3729,23 +3731,11 @@ const SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt, DTLS_VERSION_GT(s->version, c->max_dtls))) continue; - 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. - */ + /* + * Since TLS 1.3 ciphersuites can be used with any auth or + * key exchange scheme skip tests. + */ + if (!SSL_IS_TLS13(s)) { mask_k = s->s3->tmp.mask_k; mask_a = s->s3->tmp.mask_a; #ifndef OPENSSL_NO_SRP diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c index fe181abad0aa3fff7df35b80145ab9663ec0c3c3..39e6c07fe0ba49d65cc0197ebdb48696be70af09 100644 --- a/ssl/statem/extensions_srvr.c +++ b/ssl/statem/extensions_srvr.c @@ -726,11 +726,8 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x, continue; md = ssl_md(sess->cipher->algorithm2); - if (md == NULL) { - /* - * Don't recognise this cipher so we can't use the session. - * Ignore it - */ + if (md != ssl_md(s->s3->tmp.new_cipher->algorithm2)) { + /* The ciphersuite is not compatible with this session. */ SSL_SESSION_free(sess); sess = NULL; continue; diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c index 8137a7da9e95cd61988487bb3a0e01ac0c678a4f..53b8ef943184d186cbb811f2de35b74d5619bcdb 100644 --- a/ssl/statem/statem_srvr.c +++ b/ssl/statem/statem_srvr.c @@ -1566,6 +1566,68 @@ static int tls_early_post_process_client_hello(SSL *s, int *pal) s->hit = 0; + if (!ssl_cache_cipherlist(s, &clienthello->ciphersuites, + clienthello->isv2, &al) || + !bytes_to_cipher_list(s, &clienthello->ciphersuites, &ciphers, &scsvs, + clienthello->isv2, &al)) { + goto err; + } + + s->s3->send_connection_binding = 0; + /* Check what signalling cipher-suite values were received. */ + if (scsvs != NULL) { + for(i = 0; i < sk_SSL_CIPHER_num(scsvs); i++) { + c = sk_SSL_CIPHER_value(scsvs, i); + if (SSL_CIPHER_get_id(c) == SSL3_CK_SCSV) { + if (s->renegotiate) { + /* SCSV is fatal if renegotiating */ + SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, + SSL_R_SCSV_RECEIVED_WHEN_RENEGOTIATING); + al = SSL_AD_HANDSHAKE_FAILURE; + goto err; + } + s->s3->send_connection_binding = 1; + } else if (SSL_CIPHER_get_id(c) == SSL3_CK_FALLBACK_SCSV && + !ssl_check_version_downgrade(s)) { + /* + * This SCSV indicates that the client previously tried + * a higher version. We should fail if the current version + * is an unexpected downgrade, as that indicates that the first + * connection may have been tampered with in order to trigger + * an insecure downgrade. + */ + SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, + SSL_R_INAPPROPRIATE_FALLBACK); + al = SSL_AD_INAPPROPRIATE_FALLBACK; + goto err; + } + } + } + + /* For TLSv1.3 we must select the ciphersuite *before* session resumption */ + if (SSL_IS_TLS13(s)) { + const SSL_CIPHER *cipher = + ssl3_choose_cipher(s, ciphers, SSL_get_ciphers(s)); + + if (cipher == NULL) { + SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, + SSL_R_NO_SHARED_CIPHER); + al = SSL_AD_HANDSHAKE_FAILURE; + goto err; + } + 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 + * just selected. Something must have changed. + */ + al = SSL_AD_ILLEGAL_PARAMETER; + SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, SSL_R_BAD_CIPHER); + goto err; + } + s->s3->tmp.new_cipher = cipher; + } + /* We need to do this before getting the session */ if (!tls_parse_extension(s, TLSEXT_IDX_extended_master_secret, SSL_EXT_CLIENT_HELLO, @@ -1609,48 +1671,9 @@ static int tls_early_post_process_client_hello(SSL *s, int *pal) } } - if (!ssl_cache_cipherlist(s, &clienthello->ciphersuites, - clienthello->isv2, &al) || - !bytes_to_cipher_list(s, &clienthello->ciphersuites, &ciphers, &scsvs, - clienthello->isv2, &al)) { - goto err; - } - - s->s3->send_connection_binding = 0; - /* Check what signalling cipher-suite values were received. */ - if (scsvs != NULL) { - for(i = 0; i < sk_SSL_CIPHER_num(scsvs); i++) { - c = sk_SSL_CIPHER_value(scsvs, i); - if (SSL_CIPHER_get_id(c) == SSL3_CK_SCSV) { - if (s->renegotiate) { - /* SCSV is fatal if renegotiating */ - SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, - SSL_R_SCSV_RECEIVED_WHEN_RENEGOTIATING); - al = SSL_AD_HANDSHAKE_FAILURE; - goto err; - } - s->s3->send_connection_binding = 1; - } else if (SSL_CIPHER_get_id(c) == SSL3_CK_FALLBACK_SCSV && - !ssl_check_version_downgrade(s)) { - /* - * This SCSV indicates that the client previously tried - * a higher version. We should fail if the current version - * is an unexpected downgrade, as that indicates that the first - * connection may have been tampered with in order to trigger - * an insecure downgrade. - */ - SSLerr(SSL_F_TLS_EARLY_POST_PROCESS_CLIENT_HELLO, - SSL_R_INAPPROPRIATE_FALLBACK); - al = SSL_AD_INAPPROPRIATE_FALLBACK; - goto err; - } - } - } - /* - * 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 it is a hit, check that the cipher is in the list. In TLSv1.3 we check + * ciphersuite compatibility with the session as part of resumption. */ if (!SSL_IS_TLS13(s) && s->hit) { j = 0; @@ -1720,7 +1743,11 @@ static int tls_early_post_process_client_hello(SSL *s, int *pal) } } - if (!s->hit && s->version >= TLS1_VERSION && s->ext.session_secret_cb) { + if (!s->hit + && s->version >= TLS1_VERSION + && !SSL_IS_TLS13(s) + && !SSL_IS_DTLS(s) + && s->ext.session_secret_cb) { const SSL_CIPHER *pref_cipher = NULL; /* * s->session->master_key_length is a size_t, but this is an int for @@ -1962,7 +1989,7 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst) if (wst == WORK_MORE_B) { if (!s->hit || SSL_IS_TLS13(s)) { /* Let cert callback update server certificates if required */ - if (s->cert->cert_cb) { + if (!s->hit && s->cert->cert_cb != NULL) { int rv = s->cert->cert_cb(s, s->cert->cert_cb_arg); if (rv == 0) { al = SSL_AD_INTERNAL_ERROR; @@ -1976,25 +2003,19 @@ WORK_STATE tls_post_process_client_hello(SSL *s, WORK_STATE wst) } s->rwstate = SSL_NOTHING; } - cipher = - ssl3_choose_cipher(s, s->session->ciphers, SSL_get_ciphers(s)); - if (cipher == NULL) { - SSLerr(SSL_F_TLS_POST_PROCESS_CLIENT_HELLO, - SSL_R_NO_SHARED_CIPHER); - goto f_err; - } - 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 - * just selected. Something must have changed. - */ - al = SSL_AD_ILLEGAL_PARAMETER; - SSLerr(SSL_F_TLS_POST_PROCESS_CLIENT_HELLO, SSL_R_BAD_CIPHER); - goto f_err; + /* In TLSv1.3 we selected the ciphersuite before resumption */ + if (!SSL_IS_TLS13(s)) { + cipher = + ssl3_choose_cipher(s, s->session->ciphers, SSL_get_ciphers(s)); + + if (cipher == NULL) { + SSLerr(SSL_F_TLS_POST_PROCESS_CLIENT_HELLO, + SSL_R_NO_SHARED_CIPHER); + goto f_err; + } + s->s3->tmp.new_cipher = cipher; } - s->s3->tmp.new_cipher = cipher; if (!s->hit) { if (!tls_choose_sigalg(s, &al)) goto f_err; diff --git a/test/sslapitest.c b/test/sslapitest.c index 6162625d062b9ad8474cbc435ab99ecd64e3afa0..13ba727c5daa74441a178c02372bb5404f966e0a 100644 --- a/test/sslapitest.c +++ b/test/sslapitest.c @@ -1850,16 +1850,15 @@ static int test_ciphersuite_change(void) /* * Check attempting to resume a SHA-256 session with no SHA-256 ciphersuites - * fails. + * succeeds but does not resume. */ if (!TEST_true(SSL_CTX_set_cipher_list(cctx, "TLS13-AES-256-GCM-SHA384")) || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, NULL, NULL)) || !TEST_true(SSL_set_session(clientssl, clntsess)) - || !TEST_false(create_ssl_connection(serverssl, clientssl, + || !TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_SSL)) - || !TEST_int_eq(ERR_GET_REASON(ERR_get_error()), - SSL_R_NO_SHARED_CIPHER)) + || !TEST_false(SSL_session_reused(clientssl))) goto end; SSL_SESSION_free(clntsess); @@ -1887,6 +1886,8 @@ static int test_ciphersuite_change(void) if (!TEST_true(SSL_CTX_set_cipher_list(cctx, "TLS13-AES-128-GCM-SHA256:TLS13-AES-256-GCM-SHA384")) + || !TEST_true(SSL_CTX_set_cipher_list(sctx, + "TLS13-AES-256-GCM-SHA384")) || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl, NULL, NULL)) || !TEST_true(SSL_set_session(clientssl, clntsess))