Stop disabling TLSv1.3 if ec and dh are disabled

Even if EC and DH are disabled then we may still be able to use TLSv1.3 if we have groups that have been plugged in by an external provider. Fixes #13767 Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #13916)
openssl · Feb 5, 2021 · a763ca1 · a763ca1
1 parent 8b1db5d
commit a763ca1
Show file tree

Hide file tree

Showing 31 changed files with 241 additions and 119 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -23,6 +23,17 @@ OpenSSL 3.0
 
 ### Changes between 1.1.1 and 3.0 [xx XXX xxxx]
 
+ * Combining the Configure options no-ec and no-dh no longer disables TLSv1.3.
+   Typically if OpenSSL has no EC or DH algorithms then it cannot support
+   connections with TLSv1.3. However OpenSSL now supports "pluggable" groups
+   through providers. Therefore third party providers may supply group
+   implementations even where there are no built-in ones. Attempting to create
+   TLS connections in such a build without also disabling TLSv1.3 at run time or
+   using third party provider groups may result in handshake failures. TLSv1.3
+   can be disabled at compile time using the "no-tls1_3" Configure option.
+
+   *Matt Caswell*
+
  * The undocumented function X509_certificate_type() has been deprecated;
    applications can use X509_get0_pubkey() and X509_get0_signature() to
    get the same information.

diff --git a/Configure b/Configure
@@ -563,8 +563,6 @@ my @disable_cascades = (
     "zlib"              => [ "zlib-dynamic" ],
     "des"               => [ "mdc2" ],
     "ec"                => [ "ec2m", "ecdsa", "ecdh", "sm2", "gost" ],
-    sub { $disabled{"ec"} && $disabled{"dh"} }
-                        => [ "tls1_3" ],
     "dgram"             => [ "dtls", "sctp" ],
     "sock"              => [ "dgram" ],
     "dtls"              => [ @dtls ],

diff --git a/test/helpers/ssltestlib.c b/test/helpers/ssltestlib.c
@@ -685,18 +685,19 @@ static int always_retry_puts(BIO *bio, const char *str)
 }
 
 int create_ssl_ctx_pair(OSSL_LIB_CTX *libctx, const SSL_METHOD *sm,
-const SSL_METHOD *cm,
-                        int min_proto_version, int max_proto_version,
-                        SSL_CTX **sctx, SSL_CTX **cctx, char *certfile,
-                        char *privkeyfile)
+                        const SSL_METHOD *cm, int min_proto_version,
+                        int max_proto_version, SSL_CTX **sctx, SSL_CTX **cctx,
+                        char *certfile, char *privkeyfile)
 {
     SSL_CTX *serverctx = NULL;
     SSL_CTX *clientctx = NULL;
 
-    if (*sctx != NULL)
-        serverctx = *sctx;
-    else if (!TEST_ptr(serverctx = SSL_CTX_new_ex(libctx, NULL, sm)))
-        goto err;
+    if (sctx != NULL) {
+        if (*sctx != NULL)
+            serverctx = *sctx;
+        else if (!TEST_ptr(serverctx = SSL_CTX_new_ex(libctx, NULL, sm)))
+            goto err;
+    }
 
     if (cctx != NULL) {
         if (*cctx != NULL)
@@ -705,12 +706,25 @@ const SSL_METHOD *cm,
             goto err;
     }
 
-    if ((min_proto_version > 0
-         && !TEST_true(SSL_CTX_set_min_proto_version(serverctx,
-                                                     min_proto_version)))
-        || (max_proto_version > 0
-            && !TEST_true(SSL_CTX_set_max_proto_version(serverctx,
-                                                        max_proto_version))))
+#if !defined(OPENSSL_NO_TLS1_3) \
+    && defined(OPENSSL_NO_EC) \
+    && defined(OPENSSL_NO_DH)
+    /*
+     * There are no usable built-in TLSv1.3 groups if ec and dh are both
+     * disabled
+     */
+    if (max_proto_version == 0
+            && (sm == TLS_server_method() || cm == TLS_client_method()))
+        max_proto_version = TLS1_2_VERSION;
+#endif
+
+    if (serverctx != NULL
+            && ((min_proto_version > 0
+                 && !TEST_true(SSL_CTX_set_min_proto_version(serverctx,
+                                                            min_proto_version)))
+                || (max_proto_version > 0
+                    && !TEST_true(SSL_CTX_set_max_proto_version(serverctx,
+                                                                max_proto_version)))))
         goto err;
     if (clientctx != NULL
         && ((min_proto_version > 0
@@ -721,7 +735,7 @@ const SSL_METHOD *cm,
                                                             max_proto_version)))))
         goto err;
 
-    if (certfile != NULL && privkeyfile != NULL) {
+    if (serverctx != NULL && certfile != NULL && privkeyfile != NULL) {
         if (!TEST_int_eq(SSL_CTX_use_certificate_file(serverctx, certfile,
                                                       SSL_FILETYPE_PEM), 1)
                 || !TEST_int_eq(SSL_CTX_use_PrivateKey_file(serverctx,
@@ -731,13 +745,14 @@ const SSL_METHOD *cm,
             goto err;
     }
 
-    *sctx = serverctx;
+    if (sctx != NULL)
+        *sctx = serverctx;
     if (cctx != NULL)
         *cctx = clientctx;
     return 1;
 
  err:
-    if (*sctx == NULL)
+    if (sctx != NULL && *sctx == NULL)
         SSL_CTX_free(serverctx);
     if (cctx != NULL && *cctx == NULL)
         SSL_CTX_free(clientctx);

diff --git a/test/recipes/70-test_comp.t b/test/recipes/70-test_comp.t
@@ -65,7 +65,8 @@ SKIP: {
 }
 
 SKIP: {
-    skip "TLSv1.3 disabled", 2 if disabled("tls1_3");
+    skip "TLSv1.3 disabled", 2
+        if disabled("tls1_3") || (disabled("ec") && disabled("dh"));
     #Test 3: Check that sending multiple compression methods in a TLSv1.3
     #        ClientHello fails
     $proxy->clear();

diff --git a/test/recipes/70-test_key_share.t b/test/recipes/70-test_key_share.t
@@ -60,6 +60,9 @@ plan skip_all => "$test_name needs the sock feature enabled"
 plan skip_all => "$test_name needs TLS1.3 enabled"
     if disabled("tls1_3");
 
+plan skip_all => "$test_name needs EC or DH enabled"
+    if disabled("ec") && disabled("dh");
+
 $ENV{OPENSSL_ia32cap} = '~0x200000200000000';
 
 my $proxy = TLSProxy::Proxy->new(

diff --git a/test/recipes/70-test_sslcbcpadding.t b/test/recipes/70-test_sslcbcpadding.t
@@ -43,6 +43,7 @@ my @test_offsets = (0, 128, 254, 255);
 # Test that maximally-padded records are accepted.
 my $bad_padding_offset = -1;
 $proxy->serverflags("-tls1_2");
+$proxy->clientflags("-no_tls1_3");
 $proxy->serverconnects(1 + scalar(@test_offsets));
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
 plan tests => 1 + scalar(@test_offsets);
@@ -55,6 +56,7 @@ foreach my $offset (@test_offsets) {
     $bad_padding_offset = $offset;
     $fatal_alert = 0;
     $proxy->clearClient();
+    $proxy->clientflags("-no_tls1_3");
     $proxy->clientstart();
     ok($fatal_alert, "Invalid padding byte $bad_padding_offset");
 }

diff --git a/test/recipes/70-test_sslextension.t b/test/recipes/70-test_sslextension.t
@@ -197,6 +197,7 @@ ok($fatal_alert, "Duplicate ClientHello extension");
 $fatal_alert = 0;
 $proxy->clear();
 $proxy->filter(\&inject_duplicate_extension_serverhello);
+$proxy->clientflags("-no_tls1_3");
 $proxy->start();
 ok($fatal_alert, "Duplicate ServerHello extension");
 
@@ -207,6 +208,7 @@ SKIP: {
     $proxy->clear();
     $proxy->filter(\&extension_filter);
     $proxy->ciphers("AES128-SHA:\@SECLEVEL=0");
+    $proxy->clientflags("-no_tls1_3");
     $proxy->start();
     ok(TLSProxy::Message->success, "Zero extension length test");
 
@@ -244,7 +246,8 @@ SKIP: {
 }
 
 SKIP: {
-    skip "TLS 1.3 disabled", 1 if disabled("tls1_3");
+    skip "TLS 1.3 disabled", 1
+        if disabled("tls1_3") || (disabled("ec") && disabled("dh"));
     #Test 7: Inject an unsolicited extension (TLSv1.3)
     $fatal_alert = 0;
     $proxy->clear();
@@ -260,5 +263,6 @@ SKIP: {
 #        ignore it in a ClientHello
 $proxy->clear();
 $proxy->filter(\&inject_cryptopro_extension);
+$proxy->clientflags("-no_tls1_3");
 $proxy->start();
 ok(TLSProxy::Message->success(), "Cryptopro extension in ClientHello");
diff --git a/test/recipes/70-test_sslrecords.t b/test/recipes/70-test_sslrecords.t
@@ -43,6 +43,7 @@ my $fatal_alert = 0;    # set by filters at expected fatal alerts
 my $content_type = TLSProxy::Record::RT_APPLICATION_DATA;
 my $inject_recs_num = 1;
 $proxy->serverflags("-tls1_2");
+$proxy->clientflags("-no_tls1_3");
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
 plan tests => 20;
 ok($fatal_alert, "Out of context empty records test");
@@ -51,6 +52,7 @@ ok($fatal_alert, "Out of context empty records test");
 $proxy->clear();
 $content_type = TLSProxy::Record::RT_HANDSHAKE;
 $proxy->serverflags("-tls1_2");
+$proxy->clientflags("-no_tls1_3");
 $proxy->start();
 ok(TLSProxy::Message->success(), "In context empty records test");
 
@@ -60,6 +62,7 @@ $proxy->clear();
 #We allow 32 consecutive in context empty records
 $inject_recs_num = 33;
 $proxy->serverflags("-tls1_2");
+$proxy->clientflags("-no_tls1_3");
 $proxy->start();
 ok($fatal_alert, "Too many in context empty records test");
 
@@ -70,6 +73,7 @@ $fatal_alert = 0;
 $proxy->clear();
 $proxy->filter(\&add_frag_alert_filter);
 $proxy->serverflags("-tls1_2");
+$proxy->clientflags("-no_tls1_3");
 $proxy->start();
 ok($fatal_alert, "Fragmented alert records test");
 
@@ -92,6 +96,7 @@ my $sslv2testtype = TLSV1_2_IN_SSLV2;
 $proxy->clear();
 $proxy->filter(\&add_sslv2_filter);
 $proxy->serverflags("-tls1_2");
+$proxy->clientflags("-no_tls1_3");
 $proxy->ciphers("AES128-SHA:\@SECLEVEL=0");
 $proxy->start();
 ok(TLSProxy::Message->success(), "TLSv1.2 in SSLv2 ClientHello test");
@@ -102,6 +107,7 @@ ok(TLSProxy::Message->success(), "TLSv1.2 in SSLv2 ClientHello test");
 $sslv2testtype = SSLV2_IN_SSLV2;
 $proxy->clear();
 $proxy->serverflags("-tls1_2");
+$proxy->clientflags("-no_tls1_3");
 $proxy->ciphers("AES128-SHA:\@SECLEVEL=0");
 $proxy->start();
 ok(TLSProxy::Message->fail(), "SSLv2 in SSLv2 ClientHello test");
@@ -112,6 +118,7 @@ ok(TLSProxy::Message->fail(), "SSLv2 in SSLv2 ClientHello test");
 $sslv2testtype = FRAGMENTED_IN_TLSV1_2;
 $proxy->clear();
 $proxy->serverflags("-tls1_2");
+$proxy->clientflags("-no_tls1_3");
 $proxy->ciphers("AES128-SHA:\@SECLEVEL=0");
 $proxy->start();
 ok(TLSProxy::Message->success(), "Fragmented ClientHello in TLSv1.2 test");
@@ -121,6 +128,7 @@ ok(TLSProxy::Message->success(), "Fragmented ClientHello in TLSv1.2 test");
 $sslv2testtype = FRAGMENTED_IN_SSLV2;
 $proxy->clear();
 $proxy->serverflags("-tls1_2");
+$proxy->clientflags("-no_tls1_3");
 $proxy->ciphers("AES128-SHA:\@SECLEVEL=0");
 $proxy->start();
 ok(TLSProxy::Message->fail(), "Fragmented ClientHello in TLSv1.2/SSLv2 test");
@@ -130,6 +138,7 @@ ok(TLSProxy::Message->fail(), "Fragmented ClientHello in TLSv1.2/SSLv2 test");
 $sslv2testtype = ALERT_BEFORE_SSLV2;
 $proxy->clear();
 $proxy->serverflags("-tls1_2");
+$proxy->clientflags("-no_tls1_3");
 $proxy->ciphers("AES128-SHA:\@SECLEVEL=0");
 $proxy->start();
 ok(TLSProxy::Message->fail(), "Alert before SSLv2 ClientHello test");
@@ -140,6 +149,7 @@ ok(TLSProxy::Message->fail(), "Alert before SSLv2 ClientHello test");
 $fatal_alert = 0;
 $proxy->clear();
 $proxy->serverflags("-tls1_2");
+$proxy->clientflags("-no_tls1_3");
 $proxy->filter(\&add_unknown_record_type);
 $proxy->start();
 ok($fatal_alert, "Unrecognised record type in TLS1.2");
@@ -166,7 +176,8 @@ ok($fatal_alert, "Changed record version in TLS1.2");
 
 #TLS1.3 specific tests
 SKIP: {
-    skip "TLSv1.3 disabled", 8 if disabled("tls1_3");
+    skip "TLSv1.3 disabled", 8
+        if disabled("tls1_3") || (disabled("ec") && disabled("dh"));
 
     #Test 13: Sending a different record version in TLS1.3 should fail
     $proxy->clear();

diff --git a/test/recipes/70-test_sslsigalgs.t b/test/recipes/70-test_sslsigalgs.t
@@ -54,13 +54,15 @@ use constant {
 #      the sigalgs
 
 #Test 1: Default sig algs should succeed
+$proxy->clientflags("-no_tls1_3") if disabled("ec") && disabled("dh");
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
 plan tests => 26;
 ok(TLSProxy::Message->success, "Default sigalgs");
 my $testtype;
 
 SKIP: {
-    skip "TLSv1.3 disabled", 6 if disabled("tls1_3");
+    skip "TLSv1.3 disabled", 6
+        if disabled("tls1_3") || (disabled("ec") && disabled("dh"));
 
     $proxy->filter(\&sigalgs_filter);
 
@@ -237,7 +239,10 @@ SKIP: {
 
 my ($dsa_status, $sha1_status, $sha224_status);
 SKIP: {
-    skip "TLSv1.3 disabled", 2 if disabled("tls1_3") || disabled("dsa");
+    skip "TLSv1.3 disabled", 2
+        if disabled("tls1_3")
+           || disabled("dsa")
+           || (disabled("ec") && disabled("dh"));
     #Test 20: signature_algorithms with 1.3-only ClientHello
     $testtype = PURE_SIGALGS;
     $dsa_status = $sha1_status = $sha224_status = 0;
@@ -263,7 +268,8 @@ SKIP: {
 }
 
 SKIP: {
-    skip "TLSv1.3 disabled", 3 if disabled("tls1_3");
+    skip "TLSv1.3 disabled", 5
+        if disabled("tls1_3") || (disabled("ec") && disabled("dh"));
     #Test 22: Insert signature_algorithms_cert that match normal sigalgs
     $testtype = SIGALGS_CERT_ALL;
     $proxy->clear();
@@ -284,10 +290,7 @@ SKIP: {
     $proxy->filter(\&modify_sigalgs_cert_filter);
     $proxy->start();
     ok(TLSProxy::Message->fail, "No matching certificate for sigalgs_cert");
-}
 
-SKIP: {
-    skip "TLS 1.3 disabled", 2 if disabled("tls1_3");
     #Test 25: Send an unrecognized signature_algorithms_cert
     #        We should be able to skip over the unrecognized value and use a
     #        valid one that appears later in the list.

diff --git a/test/recipes/70-test_sslsignature.t b/test/recipes/70-test_sslsignature.t
@@ -45,12 +45,14 @@ $proxy->filter(\&signature_filter);
 
 #Test 1: No corruption should succeed
 my $testtype = NO_CORRUPTION;
+$proxy->clientflags("-no_tls1_3") if disabled("ec") && disabled("dh");
 $proxy->start() or plan skip_all => "Unable to start up Proxy for tests";
 plan tests => 4;
 ok(TLSProxy::Message->success, "No corruption");
 
 SKIP: {
-    skip "TLSv1.3 disabled", 1 if disabled("tls1_3");
+    skip "TLSv1.3 disabled", 1
+        if disabled("tls1_3") || (disabled("ec") && disabled("dh"));
 
     #Test 2: Corrupting a server CertVerify signature in TLSv1.3 should fail
     $proxy->clear();

diff --git a/test/recipes/70-test_sslversions.t b/test/recipes/70-test_sslversions.t
@@ -37,7 +37,10 @@ plan skip_all => "$test_name needs the sock feature enabled"
     if disabled("sock");
 
 plan skip_all => "$test_name needs TLS1.3, TLS1.2 and TLS1.1 enabled"
-    if disabled("tls1_3") || disabled("tls1_2") || disabled("tls1_1");
+    if disabled("tls1_3")
+       || (disabled("ec") && disabled("dh"))
+       || disabled("tls1_2")
+       || disabled("tls1_1");
 
 $ENV{OPENSSL_ia32cap} = '~0x200000200000000';
 

diff --git a/test/recipes/70-test_tls13alerts.t b/test/recipes/70-test_tls13alerts.t
@@ -24,7 +24,7 @@ plan skip_all => "$test_name needs the sock feature enabled"
     if disabled("sock");
 
 plan skip_all => "$test_name needs TLS1.3 enabled"
-    if disabled("tls1_3");
+    if disabled("tls1_3") || (disabled("ec") && disabled("dh"));
 
 $ENV{OPENSSL_ia32cap} = '~0x200000200000000';
 

diff --git a/test/recipes/70-test_tls13cookie.t b/test/recipes/70-test_tls13cookie.t
@@ -24,7 +24,7 @@ plan skip_all => "$test_name needs the sock feature enabled"
     if disabled("sock");
 
 plan skip_all => "$test_name needs TLS1.3 enabled"
-    if disabled("tls1_3");
+    if disabled("tls1_3") || (disabled("ec") && disabled("dh"));
 
 $ENV{OPENSSL_ia32cap} = '~0x200000200000000';
 

diff --git a/test/recipes/70-test_tls13downgrade.t b/test/recipes/70-test_tls13downgrade.t
@@ -24,7 +24,9 @@ plan skip_all => "$test_name needs the sock feature enabled"
     if disabled("sock");
 
 plan skip_all => "$test_name needs TLS1.3 and TLS1.2 enabled"
-    if disabled("tls1_3") || disabled("tls1_2");
+    if disabled("tls1_3")
+       || (disabled("ec") && disabled("dh"))
+       || disabled("tls1_2");
 
 $ENV{OPENSSL_ia32cap} = '~0x200000200000000';
 

diff --git a/test/recipes/70-test_tls13hrr.t b/test/recipes/70-test_tls13hrr.t
@@ -24,7 +24,7 @@ plan skip_all => "$test_name needs the sock feature enabled"
     if disabled("sock");
 
 plan skip_all => "$test_name needs TLS1.3 enabled"
-    if disabled("tls1_3");
+    if disabled("tls1_3") || (disabled("ec") && disabled("dh"));
 
 $ENV{OPENSSL_ia32cap} = '~0x200000200000000';
 

diff --git a/test/recipes/70-test_tls13kexmodes.t b/test/recipes/70-test_tls13kexmodes.t
@@ -26,7 +26,7 @@ plan skip_all => "$test_name needs the sock feature enabled"
     if disabled("sock");
 
 plan skip_all => "$test_name needs TLSv1.3 enabled"
-    if disabled("tls1_3");
+    if disabled("tls1_3") || (disabled("ec") && disabled("dh"));
 
 plan skip_all => "$test_name needs EC enabled"
     if disabled("ec");