Fix the check for suitable groups and TLSv1.3

If we have TLSv1.3 enabled then we must have at least one TLSv1.3 capable
group available. This check was not always working

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

diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index b216e29f2666e258d9a276999bb0753c8c90c30a..cac713fff089ebb85240309630b89179f41472bc 100644 (file)
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -234,7 +234,7 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt,
         }
     }
     if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) {
-        if (added == 0 || (tls13added == 0 && max_version == TLS1_3_VERSION))
+        if (added == 0)
             SSLfatal_data(s, SSL_AD_INTERNAL_ERROR, SSL_R_NO_SUITABLE_GROUPS,
                           "No groups enabled for max supported SSL/TLS version");
         else
@@ -242,6 +242,12 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt,
         return EXT_RETURN_FAIL;
     }
 
+    if (tls13added == 0 && max_version == TLS1_3_VERSION) {
+        SSLfatal_data(s, SSL_AD_INTERNAL_ERROR, SSL_R_NO_SUITABLE_GROUPS,
+                      "No groups enabled for max supported SSL/TLS version");
+        return EXT_RETURN_FAIL;
+    }
+
     return EXT_RETURN_SENT;
 }
 

diff --git a/test/ssl-tests/20-cert-select.cnf b/test/ssl-tests/20-cert-select.cnf
index 267690ee353bcf5a852ad1ac593c0d3fd8abb265..79dcd4c8f4e2edf47eda085a5eaf51d337270190 100644 (file)
--- a/test/ssl-tests/20-cert-select.cnf
+++ b/test/ssl-tests/20-cert-select.cnf
@@ -1,6 +1,6 @@
 # Generated with generate_ssl_tests.pl
 
-num_tests = 56
+num_tests = 57
 
 test-0 = 0-ECDSA CipherString Selection
 test-1 = 1-ECDSA CipherString Selection
@@ -54,10 +54,11 @@ test-48 = 48-TLS 1.3 Ed25519 CipherString and Groups Selection
 test-49 = 49-TLS 1.3 Ed448 CipherString and Groups Selection
 test-50 = 50-TLS 1.3 Ed25519 Client Auth
 test-51 = 51-TLS 1.3 Ed448 Client Auth
-test-52 = 52-TLS 1.3 ECDSA with brainpool
-test-53 = 53-TLS 1.2 DSA Certificate Test
-test-54 = 54-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms
-test-55 = 55-TLS 1.3 DSA Certificate Test
+test-52 = 52-TLS 1.3 ECDSA with brainpool but no suitable groups
+test-53 = 53-TLS 1.3 ECDSA with brainpool
+test-54 = 54-TLS 1.2 DSA Certificate Test
+test-55 = 55-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms
+test-56 = 56-TLS 1.3 DSA Certificate Test
 # ===========================================================
 
 [0-ECDSA CipherString Selection]
@@ -783,6 +784,7 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/server-ecdsa-brainpoolP256r1-key.pem
 [22-ECDSA with brainpool-client]
 CipherString = aECDSA
 Groups = brainpoolP256r1
+MaxProtocol = TLSv1.2
 RequestCAFile = ${ENV::TEST_CERTS_DIR}/root-cert.pem
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
@@ -1705,19 +1707,45 @@ ExpectedResult = Success
 
 # ===========================================================
 
-[52-TLS 1.3 ECDSA with brainpool]
-ssl_conf = 52-TLS 1.3 ECDSA with brainpool-ssl
+[52-TLS 1.3 ECDSA with brainpool but no suitable groups]
+ssl_conf = 52-TLS 1.3 ECDSA with brainpool but no suitable groups-ssl
+
+[52-TLS 1.3 ECDSA with brainpool but no suitable groups-ssl]
+server = 52-TLS 1.3 ECDSA with brainpool but no suitable groups-server
+client = 52-TLS 1.3 ECDSA with brainpool but no suitable groups-client
+
+[52-TLS 1.3 ECDSA with brainpool but no suitable groups-server]
+Certificate = ${ENV::TEST_CERTS_DIR}/server-ecdsa-brainpoolP256r1-cert.pem
+CipherString = DEFAULT
+Groups = brainpoolP256r1
+PrivateKey = ${ENV::TEST_CERTS_DIR}/server-ecdsa-brainpoolP256r1-key.pem
+
+[52-TLS 1.3 ECDSA with brainpool but no suitable groups-client]
+CipherString = aECDSA
+Groups = brainpoolP256r1
+RequestCAFile = ${ENV::TEST_CERTS_DIR}/root-cert.pem
+VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
+VerifyMode = Peer
+
+[test-52]
+ExpectedResult = ClientFail
+
+
+# ===========================================================
+
+[53-TLS 1.3 ECDSA with brainpool]
+ssl_conf = 53-TLS 1.3 ECDSA with brainpool-ssl
 
-[52-TLS 1.3 ECDSA with brainpool-ssl]
-server = 52-TLS 1.3 ECDSA with brainpool-server
-client = 52-TLS 1.3 ECDSA with brainpool-client
+[53-TLS 1.3 ECDSA with brainpool-ssl]
+server = 53-TLS 1.3 ECDSA with brainpool-server
+client = 53-TLS 1.3 ECDSA with brainpool-client
 
-[52-TLS 1.3 ECDSA with brainpool-server]
+[53-TLS 1.3 ECDSA with brainpool-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/server-ecdsa-brainpoolP256r1-cert.pem
 CipherString = DEFAULT
 PrivateKey = ${ENV::TEST_CERTS_DIR}/server-ecdsa-brainpoolP256r1-key.pem
 
-[52-TLS 1.3 ECDSA with brainpool-client]
+[53-TLS 1.3 ECDSA with brainpool-client]
 CipherString = DEFAULT
 MaxProtocol = TLSv1.3
 MinProtocol = TLSv1.3
@@ -1725,20 +1753,20 @@ RequestCAFile = ${ENV::TEST_CERTS_DIR}/root-cert.pem
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
-[test-52]
+[test-53]
 ExpectedResult = ServerFail
 
 
 # ===========================================================
 
-[53-TLS 1.2 DSA Certificate Test]
-ssl_conf = 53-TLS 1.2 DSA Certificate Test-ssl
+[54-TLS 1.2 DSA Certificate Test]
+ssl_conf = 54-TLS 1.2 DSA Certificate Test-ssl
 
-[53-TLS 1.2 DSA Certificate Test-ssl]
-server = 53-TLS 1.2 DSA Certificate Test-server
-client = 53-TLS 1.2 DSA Certificate Test-client
+[54-TLS 1.2 DSA Certificate Test-ssl]
+server = 54-TLS 1.2 DSA Certificate Test-server
+client = 54-TLS 1.2 DSA Certificate Test-client
 
-[53-TLS 1.2 DSA Certificate Test-server]
+[54-TLS 1.2 DSA Certificate Test-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
 CipherString = ALL
 DHParameters = ${ENV::TEST_CERTS_DIR}/dhp2048.pem
@@ -1748,26 +1776,26 @@ MaxProtocol = TLSv1.2
 MinProtocol = TLSv1.2
 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
-[53-TLS 1.2 DSA Certificate Test-client]
+[54-TLS 1.2 DSA Certificate Test-client]
 CipherString = ALL
 SignatureAlgorithms = DSA+SHA256:DSA+SHA1
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
-[test-53]
+[test-54]
 ExpectedResult = Success
 
 
 # ===========================================================
 
-[54-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms]
-ssl_conf = 54-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms-ssl
+[55-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms]
+ssl_conf = 55-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms-ssl
 
-[54-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms-ssl]
-server = 54-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms-server
-client = 54-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms-client
+[55-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms-ssl]
+server = 55-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms-server
+client = 55-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms-client
 
-[54-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms-server]
+[55-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
 CipherString = DEFAULT
 ClientSignatureAlgorithms = ECDSA+SHA1:DSA+SHA256:RSA+SHA256
@@ -1775,25 +1803,25 @@ PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/root-cert.pem
 VerifyMode = Request
 
-[54-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms-client]
+[55-TLS 1.3 Client Auth No TLS 1.3 Signature Algorithms-client]
 CipherString = DEFAULT
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
-[test-54]
+[test-55]
 ExpectedResult = ServerFail
 
 
 # ===========================================================
 
-[55-TLS 1.3 DSA Certificate Test]
-ssl_conf = 55-TLS 1.3 DSA Certificate Test-ssl
+[56-TLS 1.3 DSA Certificate Test]
+ssl_conf = 56-TLS 1.3 DSA Certificate Test-ssl
 
-[55-TLS 1.3 DSA Certificate Test-ssl]
-server = 55-TLS 1.3 DSA Certificate Test-server
-client = 55-TLS 1.3 DSA Certificate Test-client
+[56-TLS 1.3 DSA Certificate Test-ssl]
+server = 56-TLS 1.3 DSA Certificate Test-server
+client = 56-TLS 1.3 DSA Certificate Test-client
 
-[55-TLS 1.3 DSA Certificate Test-server]
+[56-TLS 1.3 DSA Certificate Test-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
 CipherString = ALL
 DSA.Certificate = ${ENV::TEST_CERTS_DIR}/server-dsa-cert.pem
@@ -1802,13 +1830,13 @@ MaxProtocol = TLSv1.3
 MinProtocol = TLSv1.3
 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
-[55-TLS 1.3 DSA Certificate Test-client]
+[56-TLS 1.3 DSA Certificate Test-client]
 CipherString = ALL
 SignatureAlgorithms = DSA+SHA1:DSA+SHA256:ECDSA+SHA256
 VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
-[test-55]
+[test-56]
 ExpectedResult = ServerFail
 
 

diff --git a/test/ssl-tests/20-cert-select.cnf.in b/test/ssl-tests/20-cert-select.cnf.in
index 1aa3b0aeecb89c832abe8710c5ab8bcc762fa087..30cde592c6d0e99e38cb752b0d393b5a64094cde 100644 (file)
--- a/test/ssl-tests/20-cert-select.cnf.in
+++ b/test/ssl-tests/20-cert-select.cnf.in
@@ -433,8 +433,7 @@ my @tests_non_fips = (
             "Groups" => "brainpoolP256r1",
         },
         client => {
-            #We don't restrict this to TLSv1.2, although use of brainpool
-            #should force this anyway so that this should succeed
+            "MaxProtocol" => "TLSv1.2",
             "CipherString" => "aECDSA",
             "RequestCAFile" => test_pem("root-cert.pem"),
             "Groups" => "brainpoolP256r1",
@@ -894,6 +893,25 @@ my @tests_tls_1_3_non_fips = (
             "ExpectedResult" => "Success"
         },
     },
+    {
+        name => "TLS 1.3 ECDSA with brainpool but no suitable groups",
+        server =>  {
+            "Certificate" => test_pem("server-ecdsa-brainpoolP256r1-cert.pem"),
+            "PrivateKey" => test_pem("server-ecdsa-brainpoolP256r1-key.pem"),
+            "Groups" => "brainpoolP256r1",
+        },
+        client => {
+            "CipherString" => "aECDSA",
+            "RequestCAFile" => test_pem("root-cert.pem"),
+            "Groups" => "brainpoolP256r1",
+        },
+        test   => {
+            #We only configured brainpoolP256r1 on the client side, but TLSv1.3
+            #is enabled and this group is not allowed in TLSv1.3. Therefore this
+            #should fail
+            "ExpectedResult" => "ClientFail"
+        },
+    },
     {
         name => "TLS 1.3 ECDSA with brainpool",
         server =>  {