Split configuration of TLSv1.3 ciphers from older ciphers

With the current mechanism, old cipher strings that used to work in 1.1.0,
may inadvertently disable all TLSv1.3 ciphersuites causing connections to
fail. This is confusing for users.

In reality TLSv1.3 are quite different to older ciphers. They are much
simpler and there are only a small number of them so, arguably, they don't
need the same level of control that the older ciphers have.

This change splits the configuration of TLSv1.3 ciphers from older ones.
By default the TLSv1.3 ciphers are on, so you cannot inadvertently disable
them through your existing config.

Fixes #5359

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/5392)

12 files changed, 76 insertions, 50 deletions

diff --git a/test/cipherlist_test.c b/test/cipherlist_test.c
index 50e3027028..b224983a34 100644
--- a/test/cipherlist_test.c
+++ b/test/cipherlist_test.c
@@ -63,6 +63,13 @@ static CIPHERLIST_TEST_FIXTURE *set_up(const char *const test_case_name)
  * are currently broken and should be considered mission impossible in libssl.
  */
 static const uint32_t default_ciphers_in_order[] = {
+#ifndef OPENSSL_NO_TLS1_3
+    TLS1_3_CK_AES_256_GCM_SHA384,
+# if !defined(OPENSSL_NO_CHACHA) && !defined(OPENSSL_NO_POLY1305)
+    TLS1_3_CK_CHACHA20_POLY1305_SHA256,
+# endif
+    TLS1_3_CK_AES_128_GCM_SHA256,
+#endif
 #ifndef OPENSSL_NO_TLS1_2
 # ifndef OPENSSL_NO_EC
     TLS1_CK_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384,
@@ -127,13 +134,6 @@ static const uint32_t default_ciphers_in_order[] = {
     TLS1_CK_RSA_WITH_AES_256_GCM_SHA384,
     TLS1_CK_RSA_WITH_AES_128_GCM_SHA256,
 #endif
-#ifndef OPENSSL_NO_TLS1_3
-    TLS1_3_CK_AES_256_GCM_SHA384,
-# if !defined(OPENSSL_NO_CHACHA) && !defined(OPENSSL_NO_POLY1305)
-    TLS1_3_CK_CHACHA20_POLY1305_SHA256,
-# endif
-    TLS1_3_CK_AES_128_GCM_SHA256,
-#endif
 #ifndef OPENSSL_NO_TLS1_2
     TLS1_CK_RSA_WITH_AES_256_SHA256,
     TLS1_CK_RSA_WITH_AES_128_SHA256,
diff --git a/test/clienthellotest.c b/test/clienthellotest.c
index 5cff519ead..45595d393a 100644
--- a/test/clienthellotest.c
+++ b/test/clienthellotest.c
@@ -97,8 +97,7 @@ static int test_client_hello(int currtest)
          * ClientHello is already going to be quite long. To avoid getting one
          * that is too long for this test we use a restricted ciphersuite list
          */
-        if (!TEST_true(SSL_CTX_set_cipher_list(ctx,
-                                               "TLS13-AES-128-GCM-SHA256")))
+        if (!TEST_true(SSL_CTX_set_cipher_list(ctx, "")))
             goto end;
          /* Fall through */
     case TEST_ADD_PADDING:
diff --git a/test/fatalerrtest.c b/test/fatalerrtest.c
index 23357351f5..85b11c84aa 100644
--- a/test/fatalerrtest.c
+++ b/test/fatalerrtest.c
@@ -38,6 +38,10 @@ static int test_fatalerr(void)
      */
     if (!TEST_true(SSL_CTX_set_cipher_list(sctx, "AES128-SHA"))
             || !TEST_true(SSL_CTX_set_cipher_list(cctx, "AES256-SHA"))
+            || !TEST_true(SSL_CTX_set_ciphersuites(sctx,
+                                                   "TLS_AES_128_GCM_SHA256"))
+            || !TEST_true(SSL_CTX_set_ciphersuites(cctx,
+                                                   "TLS_AES_256_GCM_SHA384"))
             || !TEST_true(create_ssl_objects(sctx, cctx, &sssl, &cssl, NULL,
                           NULL)))
         goto err;
diff --git a/test/recipes/70-test_sslsigalgs.t b/test/recipes/70-test_sslsigalgs.t
index c1d827208b..95af8a1913 100644
--- a/test/recipes/70-test_sslsigalgs.t
+++ b/test/recipes/70-test_sslsigalgs.t
@@ -116,7 +116,7 @@ SKIP: {
     #        should succeed
     $proxy->clear();
     $proxy->serverflags("-no_tls1_3");
-    $proxy->ciphers("ECDHE-RSA-AES128-SHA:TLS13-AES-128-GCM-SHA256");
+    $proxy->ciphers("ECDHE-RSA-AES128-SHA");
     $proxy->filter(undef);
     $proxy->start();
     ok(TLSProxy::Message->success, "TLSv1.3 client TLSv1.2 server");
@@ -131,7 +131,7 @@ SKIP: {
     $proxy->clear();
     $testtype = NO_SIG_ALGS_EXT;
     $proxy->clientflags("-no_tls1_3");
-    $proxy->ciphers("ECDHE-RSA-AES128-SHA:TLS13-AES-128-GCM-SHA256");
+    $proxy->ciphers("ECDHE-RSA-AES128-SHA");
     $proxy->start();
     ok(TLSProxy::Message->success, "No TLSv1.2 sigalgs");
 
@@ -139,7 +139,7 @@ SKIP: {
     $proxy->clear();
     $testtype = EMPTY_SIG_ALGS_EXT;
     $proxy->clientflags("-no_tls1_3");
-    $proxy->ciphers("ECDHE-RSA-AES128-SHA:TLS13-AES-128-GCM-SHA256");
+    $proxy->ciphers("ECDHE-RSA-AES128-SHA");
     $proxy->start();
     ok(TLSProxy::Message->fail, "Empty TLSv1.2 sigalgs");
 
@@ -147,7 +147,7 @@ SKIP: {
     $proxy->clear();
     $testtype = NO_KNOWN_SIG_ALGS;
     $proxy->clientflags("-no_tls1_3");
-    $proxy->ciphers("ECDHE-RSA-AES128-SHA:TLS13-AES-128-GCM-SHA256");
+    $proxy->ciphers("ECDHE-RSA-AES128-SHA");
     $proxy->start();
     ok(TLSProxy::Message->fail, "No known TLSv1.3 sigalgs");
 
@@ -156,7 +156,7 @@ SKIP: {
     $proxy->clear();
     $testtype = NO_PSS_SIG_ALGS;
     $proxy->clientflags("-no_tls1_3");
-    $proxy->ciphers("ECDHE-RSA-AES128-SHA:TLS13-AES-128-GCM-SHA256");
+    $proxy->ciphers("ECDHE-RSA-AES128-SHA");
     $proxy->start();
     ok(TLSProxy::Message->success, "No PSS TLSv1.2 sigalgs");
 
@@ -164,7 +164,7 @@ SKIP: {
     $proxy->clear();
     $testtype = PSS_ONLY_SIG_ALGS;
     $proxy->serverflags("-no_tls1_3");
-    $proxy->ciphers("ECDHE-RSA-AES128-SHA:TLS13-AES-128-GCM-SHA256");
+    $proxy->ciphers("ECDHE-RSA-AES128-SHA");
     $proxy->start();
     ok(TLSProxy::Message->success, "PSS only sigalgs in TLSv1.2");
 
@@ -175,7 +175,7 @@ SKIP: {
     $proxy->clear();
     $testtype = PSS_ONLY_SIG_ALGS;
     $proxy->clientflags("-no_tls1_3 -sigalgs RSA+SHA256");
-    $proxy->ciphers("ECDHE-RSA-AES128-SHA:TLS13-AES-128-GCM-SHA256");
+    $proxy->ciphers("ECDHE-RSA-AES128-SHA");
     $proxy->start();
     ok(TLSProxy::Message->fail, "Sigalg we did not send in TLSv1.2");
 
@@ -183,7 +183,7 @@ SKIP: {
     #         matches the certificate should fail in TLSv1.2
     $proxy->clear();
     $proxy->clientflags("-no_tls1_3 -sigalgs ECDSA+SHA256");
-    $proxy->ciphers("ECDHE-RSA-AES128-SHA:TLS13-AES-128-GCM-SHA256");
+    $proxy->ciphers("ECDHE-RSA-AES128-SHA");
     $proxy->filter(undef);
     $proxy->start();
     ok(TLSProxy::Message->fail, "No matching TLSv1.2 sigalgs");
@@ -197,7 +197,7 @@ SKIP: {
                                                "server-ecdsa-cert.pem") .
                         " -key " . srctop_file("test", "certs",
                                                "server-ecdsa-key.pem")),
-    $proxy->ciphers("ECDHE-ECDSA-AES128-SHA:TLS13-AES-128-GCM-SHA256");
+    $proxy->ciphers("ECDHE-ECDSA-AES128-SHA");
     $proxy->start();
     ok(TLSProxy::Message->success, "No TLSv1.2 sigalgs, ECDSA");
 }
diff --git a/test/recipes/70-test_tls13hrr.t b/test/recipes/70-test_tls13hrr.t
index 56b7faf63d..e0b47ed359 100644
--- a/test/recipes/70-test_tls13hrr.t
+++ b/test/recipes/70-test_tls13hrr.t
@@ -53,7 +53,7 @@ ok(TLSProxy::Message->fail(), "Server ciphersuite changes");
 #        we end up selecting a different ciphersuite between HRR and the SH
 $proxy->clear();
 $proxy->serverflags("-curves P-256");
-$proxy->ciphers("TLS13-AES-128-GCM-SHA256:TLS13-AES-256-GCM-SHA384");
+$proxy->ciphersuitess("TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384");
 $testtype = CHANGE_CH1_CIPHERSUITE;
 $proxy->start();
 ok(TLSProxy::Message->fail(), "Client ciphersuite changes");
@@ -85,8 +85,7 @@ sub hrr_filter
 
     my $ch1 = ${$proxy->message_list}[0];
 
-    # The server prefers TLS13-AES-256-GCM-SHA384 so it will pick that next
-    # time around
+    # The server will always pick TLS_AES_256_GCM_SHA384
     my @ciphersuites = (TLSProxy::Message::CIPHER_TLS13_AES_128_GCM_SHA256);
     $ch1->ciphersuite_len(2 * scalar @ciphersuites);
     $ch1->ciphersuites(\@ciphersuites);
diff --git a/test/recipes/70-test_tls13psk.t b/test/recipes/70-test_tls13psk.t
index bd2508dfa7..fa30dd10bb 100644
--- a/test/recipes/70-test_tls13psk.t
+++ b/test/recipes/70-test_tls13psk.t
@@ -82,10 +82,10 @@ $proxy->clear();
 $proxy->clientflags("-sess_in ".$session);
 $proxy->filter(\&modify_psk_filter);
 $proxy->serverflags("-curves P-256");
-$proxy->cipherc("TLS13-AES-128-GCM-SHA256:TLS13-AES-256-GCM-SHA384");
-$proxy->ciphers("TLS13-AES-256-GCM-SHA384");
+$proxy->ciphersuitesc("TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384");
+$proxy->ciphersuitess("TLS_AES_256_GCM_SHA384");
 #We force an early failure because TLS Proxy doesn't actually support
-#TLS13-AES-256-GCM-SHA384. That doesn't matter for this test though.
+#TLS_AES_256_GCM_SHA384. That doesn't matter for this test though.
 $testtype = ILLEGAL_EXT_SECOND_CH;
 $proxy->start();
 #Check if the PSK is present in the second ClientHello
diff --git a/test/recipes/80-test_ssl_old.t b/test/recipes/80-test_ssl_old.t
index f89779c0c3..377bf090ba 100644
--- a/test/recipes/80-test_ssl_old.t
+++ b/test/recipes/80-test_ssl_old.t
@@ -432,9 +432,12 @@ sub testssl {
             if $protocolciphersuitecount + scalar(keys %ciphersuites) == 0;
 
         # The count of protocols is because in addition to the ciphersuites
-        # we got above, we're running a weak DH test for each protocol
-        plan tests => scalar(@protocols) + $protocolciphersuitecount
-            + scalar(keys %ciphersuites);
+        # we got above, we're running a weak DH test for each protocol (except
+        # TLSv1.3)
+        my $testcount = scalar(@protocols) + $protocolciphersuitecount
+                        + scalar(keys %ciphersuites);
+        $testcount-- unless $no_tls1_3;
+        plan tests => $testcount;
 
         foreach my $protocol (@protocols) {
             ok($ciphersstatus{$protocol}, "Getting ciphers for $protocol");
@@ -445,21 +448,27 @@ sub testssl {
             # ssltest_old doesn't know -tls1_3, but that's fine, since that's
             # the default choice if TLSv1.3 enabled
             my $flag = $protocol eq "-tls1_3" ? "" : $protocol;
+            my $ciphersuites = "";
             foreach my $cipher (@{$ciphersuites{$protocol}}) {
                 if ($protocol eq "-ssl3" && $cipher =~ /ECDH/ ) {
                     note "*****SKIPPING $protocol $cipher";
                     ok(1);
                 } else {
+                    if ($protocol eq "-tls1_3") {
+                        $ciphersuites = $cipher;
+                        $cipher = "";
+                    }
                     ok(run(test([@ssltest, @exkeys, "-cipher", $cipher,
-                                 $flag || ()])),
-                    "Testing $cipher");
+                                 "-ciphersuites", $ciphersuites, $flag || ()])),
+                       "Testing $cipher");
                 }
             }
+            next if $protocol eq "-tls1_3";
             is(run(test([@ssltest,
                          "-s_cipher", "EDH",
                          "-c_cipher", 'EDH:@SECLEVEL=1',
                          "-dhe512",
-                         $protocol eq "SSLv3" ? ("-ssl3") : ()])), 0,
+                         $protocol])), 0,
                "testing connection with weak DH, expecting failure");
         }
     };
diff --git a/test/ssl-tests/02-protocol-version.conf b/test/ssl-tests/02-protocol-version.conf
index d0a64cdb06..4b7d7df218 100644
--- a/test/ssl-tests/02-protocol-version.conf
+++ b/test/ssl-tests/02-protocol-version.conf
@@ -18787,6 +18787,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [676-ciphersuite-sanity-check-client-client]
 CipherString = AES128-SHA
+Ciphersuites = 
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
@@ -18806,6 +18807,7 @@ client = 677-ciphersuite-sanity-check-server-client
 [677-ciphersuite-sanity-check-server-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
 CipherString = AES128-SHA
+Ciphersuites = 
 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [677-ciphersuite-sanity-check-server-client]
diff --git a/test/ssl-tests/protocol_version.pm b/test/ssl-tests/protocol_version.pm
index edc0dd2962..2cdc5db117 100644
--- a/test/ssl-tests/protocol_version.pm
+++ b/test/ssl-tests/protocol_version.pm
@@ -137,6 +137,7 @@ sub generate_version_tests {
         "client" => {
             #Offering only <=TLSv1.2 ciphersuites with TLSv1.3 should fail
             "CipherString" => "AES128-SHA",
+            "Ciphersuites" => "",
         },
         "server" => {
             "MaxProtocol" => "TLSv1.2"
@@ -154,6 +155,7 @@ sub generate_version_tests {
         "server" => {
             #Allowing only <=TLSv1.2 ciphersuites with TLSv1.3 should fail
             "CipherString" => "AES128-SHA",
+            "Ciphersuites" => "",
         },
         "test" => {
             "ExpectedResult" => "ServerFail",
diff --git a/test/sslapitest.c b/test/sslapitest.c
index 32f60e56e8..2c122be3c8 100644
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -2381,8 +2381,8 @@ static int test_ciphersuite_change(void)
     if (!TEST_true(create_ssl_ctx_pair(TLS_server_method(),
                                        TLS_client_method(), &sctx,
                                        &cctx, cert, privkey))
-            || !TEST_true(SSL_CTX_set_cipher_list(cctx,
-                                                  "TLS13-AES-128-GCM-SHA256"))
+            || !TEST_true(SSL_CTX_set_ciphersuites(cctx,
+                                                   "TLS_AES_128_GCM_SHA256"))
             || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl,
                                           &clientssl, NULL, NULL))
             || !TEST_true(create_ssl_connection(serverssl, clientssl,
@@ -2400,8 +2400,8 @@ static int test_ciphersuite_change(void)
 
 # if !defined(OPENSSL_NO_CHACHA) && !defined(OPENSSL_NO_POLY1305)
     /* Check we can resume a session with a different SHA-256 ciphersuite */
-    if (!TEST_true(SSL_CTX_set_cipher_list(cctx,
-                                           "TLS13-CHACHA20-POLY1305-SHA256"))
+    if (!TEST_true(SSL_CTX_set_ciphersuites(cctx,
+                                            "TLS_CHACHA20_POLY1305_SHA256"))
             || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
                                              NULL, NULL))
             || !TEST_true(SSL_set_session(clientssl, clntsess))
@@ -2423,7 +2423,7 @@ static int test_ciphersuite_change(void)
      * Check attempting to resume a SHA-256 session with no SHA-256 ciphersuites
      * succeeds but does not resume.
      */
-    if (!TEST_true(SSL_CTX_set_cipher_list(cctx, "TLS13-AES-256-GCM-SHA384"))
+    if (!TEST_true(SSL_CTX_set_ciphersuites(cctx, "TLS_AES_256_GCM_SHA384"))
             || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
                                              NULL, NULL))
             || !TEST_true(SSL_set_session(clientssl, clntsess))
@@ -2441,7 +2441,7 @@ static int test_ciphersuite_change(void)
     serverssl = clientssl = NULL;
 
     /* Create a session based on SHA384 */
-    if (!TEST_true(SSL_CTX_set_cipher_list(cctx, "TLS13-AES-256-GCM-SHA384"))
+    if (!TEST_true(SSL_CTX_set_ciphersuites(cctx, "TLS_AES_256_GCM_SHA384"))
             || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl,
                                           &clientssl, NULL, NULL))
             || !TEST_true(create_ssl_connection(serverssl, clientssl,
@@ -2455,10 +2455,10 @@ static int test_ciphersuite_change(void)
     SSL_free(clientssl);
     serverssl = clientssl = NULL;
 
-    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"))
+    if (!TEST_true(SSL_CTX_set_ciphersuites(cctx,
+                   "TLS_AES_128_GCM_SHA256:TLS_AES_256_GCM_SHA384"))
+            || !TEST_true(SSL_CTX_set_ciphersuites(sctx,
+                                                   "TLS_AES_256_GCM_SHA384"))
             || !TEST_true(create_ssl_objects(sctx, cctx, &serverssl, &clientssl,
                                              NULL, NULL))
             || !TEST_true(SSL_set_session(clientssl, clntsess))
@@ -2520,7 +2520,7 @@ static int test_tls13_psk(int idx)
      * We use a ciphersuite with SHA256 to ease testing old style PSK callbacks
      * which will always default to SHA256
      */
-    if (!TEST_true(SSL_CTX_set_cipher_list(cctx, "TLS13-AES-128-GCM-SHA256")))
+    if (!TEST_true(SSL_CTX_set_ciphersuites(cctx, "TLS_AES_128_GCM_SHA256")))
         goto end;
 
     /*
diff --git a/test/sslcorrupttest.c b/test/sslcorrupttest.c
index a0bca3821c..192310036a 100644
--- a/test/sslcorrupttest.c
+++ b/test/sslcorrupttest.c
@@ -202,19 +202,17 @@ static int test_ssl_corrupt(int testidx)
         goto end;
 
     if (!TEST_ptr(ciphers = SSL_CTX_get_ciphers(cctx))
+            || !TEST_true(SSL_CTX_set_ciphersuites(cctx, ""))
             || !TEST_int_eq(sk_SSL_CIPHER_num(ciphers), 1)
             || !TEST_ptr(currcipher = sk_SSL_CIPHER_value(ciphers, 0)))
         goto end;
 
     /*
-     * If we haven't got a TLSv1.3 cipher, then we mustn't attempt to use
-     * TLSv1.3. Version negotiation happens before cipher selection, so we will
-     * get a "no shared cipher" error.
+     * No ciphers we are using are TLSv1.3 compatible so we should not attempt
+     * to negotiate TLSv1.3
      */
-    if (strcmp(SSL_CIPHER_get_version(currcipher), "TLSv1.3") != 0) {
-        if (!TEST_true(SSL_CTX_set_max_proto_version(cctx, TLS1_2_VERSION)))
-            goto end;
-    }
+    if (!TEST_true(SSL_CTX_set_max_proto_version(cctx, TLS1_2_VERSION)))
+        goto end;
 
     if (!TEST_ptr(c_to_s_fbio = BIO_new(bio_f_tls_corrupt_filter())))
         goto end;
diff --git a/test/ssltest_old.c b/test/ssltest_old.c
index 349d30a325..7d31d368c7 100644
--- a/test/ssltest_old.c
+++ b/test/ssltest_old.c
@@ -612,6 +612,7 @@ static int custom_ext_3_srv_add_cb(SSL *s, unsigned int ext_type,
 }
 
 static char *cipher = NULL;
+static char *ciphersuites = NULL;
 static int verbose = 0;
 static int debug = 0;
 
@@ -671,7 +672,8 @@ static void sv_usage(void)
     fprintf(stderr, " -c_cert arg   - Client certificate file\n");
     fprintf(stderr,
             " -c_key arg    - Client key file (default: same as -c_cert)\n");
-    fprintf(stderr, " -cipher arg   - The cipher list\n");
+    fprintf(stderr, " -cipher arg   - The TLSv1.2 and below cipher list\n");
+    fprintf(stderr, " -ciphersuites arg   - The TLSv1.3 ciphersuites\n");
     fprintf(stderr, " -bio_pair     - Use BIO pairs\n");
     fprintf(stderr, " -ipv4         - Use IPv4 connection on localhost\n");
     fprintf(stderr, " -ipv6         - Use IPv6 connection on localhost\n");
@@ -918,7 +920,6 @@ int main(int argc, char *argv[])
 
     verbose = 0;
     debug = 0;
-    cipher = 0;
 
     bio_err = BIO_new_fp(stderr, BIO_NOCLOSE | BIO_FP_TEXT);
 
@@ -1046,6 +1047,10 @@ int main(int argc, char *argv[])
             if (--argc < 1)
                 goto bad;
             cipher = *(++argv);
+        } else if (strcmp(*argv, "-ciphersuites") == 0) {
+            if (--argc < 1)
+                goto bad;
+            ciphersuites = *(++argv);
         } else if (strcmp(*argv, "-CApath") == 0) {
             if (--argc < 1)
                 goto bad;
@@ -1377,6 +1382,14 @@ int main(int argc, char *argv[])
             goto end;
         }
     }
+    if (ciphersuites != NULL) {
+        if (!SSL_CTX_set_ciphersuites(c_ctx, ciphersuites)
+            || !SSL_CTX_set_ciphersuites(s_ctx, ciphersuites)
+            || !SSL_CTX_set_ciphersuites(s_ctx2, ciphersuites)) {
+            ERR_print_errors(bio_err);
+            goto end;
+        }
+    }
 
 #ifndef OPENSSL_NO_CT
     if (ct_validation &&