Allow the server to change the ciphersuite on resume

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
(Merged from https://github.com/openssl/openssl/pull/3623)

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index e85c9a073e2e6a8728a5366d9a90d448b15e9c31..37e31666bea2b8055c672690ccb1aa32f1d3b5c1 100644 (file)
--- 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 fdb59f4a7a00768e6cf9cd396fddde9043db2216..8dfc97433843b562f9a33ab9381ae2d9f35699e6 100644 (file)
--- 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 ffbe663d9a74a77c0eb0df7b2580b0b12f2b388d..e8bda66d61e7743d3239e571cfacc74aae372cc0 100644 (file)
--- 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 29e6648d9186d4763113c0006ade9b04b6fc0b4e..eccc5af22bb8a6a211be9e11f9661d455c2b2df4 100644 (file)
--- 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 020589f1b1b99004de13ec1655b97127c5234bf7..7dd921eeff0757c4b42fdca60ffe4cfd7171a2b1 100644 (file)
--- 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 e2d618c86365a57229366a1ee1e0e313caaf66d9..8137a7da9e95cd61988487bb3a0e01ac0c678a4f 100644 (file)
--- 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