Add a ciphersuite config sanity check for servers

Ensure that there are ciphersuites enabled for the maximum supported
version we will accept in a ClientHello.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3334)

diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index f4860ea1fd282e133158ac8edc7c186b8978536a..d51772fa69c9a1e5cd20df5c705a778863e17f22 100644 (file)
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -2169,8 +2169,7 @@ __owur int ssl_check_version_downgrade(SSL *s);
 __owur int ssl_set_version_bound(int method_version, int version, int *bound);
 __owur int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello);
 __owur int ssl_choose_client_version(SSL *s, int version);
-int ssl_get_client_min_max_version(const SSL *s, int *min_version,
-                                   int *max_version);
+int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version);
 
 __owur long tls1_default_timeout(void);
 __owur int dtls1_do_write(SSL *s, int type);

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 8550dfe22e47dfb0cf03bceacabc262532dde923..cb420e8a0c8b3e5411d65c7f97a36557a4802c96 100644 (file)
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -639,7 +639,7 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
     }
 
     if ((context & EXT_CLIENT_HELLO) != 0) {
-        reason = ssl_get_client_min_max_version(s, &min_version, &max_version);
+        reason = ssl_get_min_max_version(s, &min_version, &max_version);
         if (reason != 0) {
             SSLerr(SSL_F_TLS_CONSTRUCT_EXTENSIONS, reason);
             goto err;

diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index c6cd0ce8a3231f6e501fd6afeb5309a9c52e7df8..5f33e2ceb85ce1ddc178ead5535edf1337d30709 100644 (file)
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -464,7 +464,7 @@ int tls_construct_ctos_supported_versions(SSL *s, WPACKET *pkt,
         return 0;
     }
 
-    reason = ssl_get_client_min_max_version(s, &min_version, &max_version);
+    reason = ssl_get_min_max_version(s, &min_version, &max_version);
     if (reason != 0) {
         SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_VERSIONS, reason);
         return 0;

diff --git a/ssl/statem/statem_lib.c b/ssl/statem/statem_lib.c
index 36c96e569737a0ce00d549c9a3277da00840ec20..ab72788224b990fabc32f66da253170a6fc2c3fa 100644 (file)
--- a/ssl/statem/statem_lib.c
+++ b/ssl/statem/statem_lib.c
@@ -78,6 +78,39 @@ int tls_setup_handshake(SSL *s)
         return 0;
 
     if (s->server) {
+        STACK_OF(SSL_CIPHER) *ciphers = SSL_get_ciphers(s);
+        int i, ver_min, ver_max, ok = 0;
+
+        /*
+         * Sanity check that the maximum version we accept has ciphers
+         * enabled. For clients we do this check during construction of the
+         * ClientHello.
+         */
+        if (ssl_get_min_max_version(s, &ver_min, &ver_max) != 0) {
+            SSLerr(SSL_F_TLS_SETUP_HANDSHAKE, ERR_R_INTERNAL_ERROR);
+            ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_INTERNAL_ERROR);
+            return 0;
+        }
+        for (i = 0; i < sk_SSL_CIPHER_num(ciphers); i++) {
+            const SSL_CIPHER *c = sk_SSL_CIPHER_value(ciphers, i);
+
+            if (SSL_IS_DTLS(s)) {
+                if (DTLS_VERSION_GE(ver_max, c->min_dtls) &&
+                        DTLS_VERSION_LE(ver_max, c->max_dtls))
+                    ok = 1;
+            } else if (ver_max >= c->min_tls && ver_max <= c->max_tls) {
+                ok = 1;
+            }
+            if (ok)
+                break;
+        }
+        if (!ok) {
+            SSLerr(SSL_F_TLS_SETUP_HANDSHAKE, SSL_R_NO_CIPHERS_AVAILABLE);
+            ERR_add_error_data(1, "No ciphers enabled for max supported "
+                                  "SSL/TLS version");
+            ssl3_send_alert(s, SSL3_AL_FATAL, SSL_AD_HANDSHAKE_FAILURE);
+            return 0;
+        }
         if (SSL_IS_FIRST_HANDSHAKE(s)) {
             s->ctx->stats.sess_accept++;
         } else if (!s->s3->send_connection_binding &&
@@ -1714,7 +1747,7 @@ int ssl_choose_client_version(SSL *s, int version)
 }
 
 /*
- * ssl_get_client_min_max_version - get minimum and maximum client version
+ * ssl_get_min_max_version - get minimum and maximum protocol version
  * @s: The SSL connection
  * @min_version: The minimum supported version
  * @max_version: The maximum supported version
@@ -1732,8 +1765,7 @@ int ssl_choose_client_version(SSL *s, int version)
  * Returns 0 on success or an SSL error reason number on failure.  On failure
  * min_version and max_version will also be set to 0.
  */
-int ssl_get_client_min_max_version(const SSL *s, int *min_version,
-                                   int *max_version)
+int ssl_get_min_max_version(const SSL *s, int *min_version, int *max_version)
 {
     int version;
     int hole;
@@ -1827,7 +1859,7 @@ int ssl_set_client_hello_version(SSL *s)
 {
     int ver_min, ver_max, ret;
 
-    ret = ssl_get_client_min_max_version(s, &ver_min, &ver_max);
+    ret = ssl_get_min_max_version(s, &ver_min, &ver_max);
 
     if (ret != 0)
         return ret;

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 83e493eb7cca6f1e4063aae65d5740e8cdce63fb..6ff3363300de7fce9f684bc47cb1f3fb783cb162 100644 (file)
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -1013,7 +1013,7 @@ void ssl_set_client_disabled(SSL *s)
     s->s3->tmp.mask_a = 0;
     s->s3->tmp.mask_k = 0;
     ssl_set_sig_mask(&s->s3->tmp.mask_a, s, SSL_SECOP_SIGALG_MASK);
-    ssl_get_client_min_max_version(s, &s->s3->tmp.min_ver, &s->s3->tmp.max_ver);
+    ssl_get_min_max_version(s, &s->s3->tmp.min_ver, &s->s3->tmp.max_ver);
 #ifndef OPENSSL_NO_PSK
     /* with PSK there must be client callback set */
     if (!s->psk_client_callback) {

diff --git a/test/recipes/70-test_sslmessages.t b/test/recipes/70-test_sslmessages.t
index 790b3aeda26c540ecef5567e73cd14c6f83f2f10..a6278dc6307835678e02720264aa4d467068b1fa 100644 (file)
--- a/test/recipes/70-test_sslmessages.t
+++ b/test/recipes/70-test_sslmessages.t
@@ -396,6 +396,7 @@ SKIP: {
     skip "No EC support in this OpenSSL build", 1 if disabled("ec");
     $proxy->clear();
     $proxy->clientflags("-no_tls1_3");
+    $proxy->serverflags("-no_tls1_3");
     $proxy->ciphers("ECDHE-RSA-AES128-SHA");
     $proxy->start();
     checkhandshake($proxy, checkhandshake::EC_HANDSHAKE,

diff --git a/test/ssl-tests/02-protocol-version.conf b/test/ssl-tests/02-protocol-version.conf
index d5e0779156e2b96202c5410c83626196ae44c068..41fa8ca17a5070100f8e70de3fac53a059443df5 100644 (file)
--- a/test/ssl-tests/02-protocol-version.conf
+++ b/test/ssl-tests/02-protocol-version.conf
@@ -700,7 +700,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-0]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -850,7 +850,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-6]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -1314,7 +1314,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-24]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -1339,7 +1339,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-25]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -4759,7 +4759,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-156]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -4915,7 +4915,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-162]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -5397,7 +5397,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-180]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -5423,7 +5423,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-181]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -17393,7 +17393,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-624]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -17549,7 +17549,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-630]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -18031,7 +18031,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-648]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -18057,7 +18057,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-649]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -18082,7 +18082,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-650]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -18232,7 +18232,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-656]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -18696,7 +18696,7 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-674]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 
 # ===========================================================
@@ -18721,6 +18721,6 @@ VerifyCAFile = ${ENV::TEST_CERTS_DIR}/rootcert.pem
 VerifyMode = Peer
 
 [test-675]
-ExpectedResult = InternalError
+ExpectedResult = ClientFail
 
 

diff --git a/test/ssl-tests/23-srp.conf b/test/ssl-tests/23-srp.conf
index 6ae49e6814d451c409633910bdce2e8338109827..610a0bb08ae3d9488183313f85930f68c42ef2c9 100644 (file)
--- a/test/ssl-tests/23-srp.conf
+++ b/test/ssl-tests/23-srp.conf
@@ -18,6 +18,7 @@ client = 0-srp-client
 [0-srp-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
 CipherString = SRP
+MaxProtocol = TLSv1.2
 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [0-srp-client]
@@ -52,6 +53,7 @@ client = 1-srp-bad-password-client
 [1-srp-bad-password-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
 CipherString = SRP
+MaxProtocol = TLSv1.2
 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [1-srp-bad-password-client]
@@ -86,6 +88,7 @@ client = 2-srp-auth-client
 [2-srp-auth-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
 CipherString = aSRP
+MaxProtocol = TLSv1.2
 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [2-srp-auth-client]
@@ -120,6 +123,7 @@ client = 3-srp-auth-bad-password-client
 [3-srp-auth-bad-password-server]
 Certificate = ${ENV::TEST_CERTS_DIR}/servercert.pem
 CipherString = aSRP
+MaxProtocol = TLSv1.2
 PrivateKey = ${ENV::TEST_CERTS_DIR}/serverkey.pem
 
 [3-srp-auth-bad-password-client]

diff --git a/test/ssl-tests/23-srp.conf.in b/test/ssl-tests/23-srp.conf.in
index b7601fc3e5b23bd0a5d68ccce5410fbc5bb7b781..dcbd9f4ff91ddd3e0e4fde0b851716141ab9f074 100644 (file)
--- a/test/ssl-tests/23-srp.conf.in
+++ b/test/ssl-tests/23-srp.conf.in
@@ -15,89 +15,93 @@ package ssltests;
 
 our @tests = (
     {
-       name => "srp",
-       server => {
-           "CipherString" => "SRP",
-           extra => {
-               "SRPUser" => "user",
-               "SRPPassword" => "password",
-           },
+        name => "srp",
+        server => {
+            "CipherString" => "SRP",
+            "MaxProtocol" => "TLSv1.2",
+            extra => {
+                "SRPUser" => "user",
+                "SRPPassword" => "password",
+            },
+        },
+        client => {
+            "CipherString" => "SRP",
+            "MaxProtocol" => "TLSv1.2",
+            extra => {
+                "SRPUser" => "user",
+                "SRPPassword" => "password",
+            },
+        },
+        test => {
+            "ExpectedResult" => "Success"
         },
-       client => {
-           "CipherString" => "SRP",
-           "MaxProtocol" => "TLSv1.2",
-           extra => {
-               "SRPUser" => "user",
-               "SRPPassword" => "password",
-           },
-       },
-       test => {
-           "ExpectedResult" => "Success"
-       },
     },
     {
-       name => "srp-bad-password",
-       server => {
-           "CipherString" => "SRP",
-           extra => {
-               "SRPUser" => "user",
-               "SRPPassword" => "password",
-           },
+        name => "srp-bad-password",
+        server => {
+            "CipherString" => "SRP",
+            "MaxProtocol" => "TLSv1.2",
+            extra => {
+                "SRPUser" => "user",
+                "SRPPassword" => "password",
+            },
+        },
+        client => {
+            "CipherString" => "SRP",
+            "MaxProtocol" => "TLSv1.2",
+            extra => {
+                "SRPUser" => "user",
+                "SRPPassword" => "passw0rd",
+            },
+        },
+        test => {
+            # Server fails first with bad client Finished.
+            "ExpectedResult" => "ServerFail"
         },
-       client => {
-           "CipherString" => "SRP",
-           "MaxProtocol" => "TLSv1.2",
-           extra => {
-               "SRPUser" => "user",
-               "SRPPassword" => "passw0rd",
-           },
-       },
-       test => {
-           # Server fails first with bad client Finished.
-           "ExpectedResult" => "ServerFail"
-       },
     },
     {
-       name => "srp-auth",
-       server => {
-           "CipherString" => "aSRP",
-           extra => {
-               "SRPUser" => "user",
-               "SRPPassword" => "password",
-           },
+        name => "srp-auth",
+        server => {
+            "CipherString" => "aSRP",
+            "MaxProtocol" => "TLSv1.2",
+            extra => {
+                "SRPUser" => "user",
+                "SRPPassword" => "password",
+            },
+        },
+        client => {
+            "CipherString" => "aSRP",
+            "MaxProtocol" => "TLSv1.2",
+            extra => {
+                "SRPUser" => "user",
+                "SRPPassword" => "password",
+            },
+        },
+        test => {
+            "ExpectedResult" => "Success"
         },
-       client => {
-           "CipherString" => "aSRP",
-           "MaxProtocol" => "TLSv1.2",
-           extra => {
-               "SRPUser" => "user",
-               "SRPPassword" => "password",
-           },
-       },
-       test => {
-           "ExpectedResult" => "Success"
-       },
     },
     {
-       name => "srp-auth-bad-password",
-       server => {
-           "CipherString" => "aSRP",
-           extra => {
-               "SRPUser" => "user",
-               "SRPPassword" => "password",
-           },
+        name => "srp-auth-bad-password",
+        server => {
+            "CipherString" => "aSRP",
+            "MaxProtocol" => "TLSv1.2",
+            extra => {
+                "SRPUser" => "user",
+                "SRPPassword" => "password",
+            },
+        },
+        client => {
+            "CipherString" => "aSRP",
+            "MaxProtocol" => "TLSv1.2",
+            extra => {
+                "SRPUser" => "user",
+                "SRPPassword" => "passw0rd",
+            },
+        },
+        test => {
+            # Server fails first with bad client Finished.
+            "ExpectedResult" => "ServerFail"
         },
-       client => {
-           "CipherString" => "aSRP",
-           "MaxProtocol" => "TLSv1.2",
-           extra => {
-               "SRPUser" => "user",
-               "SRPPassword" => "passw0rd",
-           },
-       },
-       test => {
-           # Server fails first with bad client Finished.
-           "ExpectedResult" => "ServerFail"
-       },
     },
-);
\ No newline at end of file
+);

diff --git a/test/ssl-tests/protocol_version.pm b/test/ssl-tests/protocol_version.pm
index 7c28bcf0f64aadc5895f998114fb81e11afcf9f9..ef922752577d6785c73175923798d9c3c2e716d9 100644 (file)
--- a/test/ssl-tests/protocol_version.pm
+++ b/test/ssl-tests/protocol_version.pm
@@ -242,7 +242,11 @@ sub expected_result {
     $c_max = min $c_max, $max_enabled;
     $s_max = min $s_max, $max_enabled;
 
-    if ($c_min > $c_max) {
+    if ($c_min > $c_max && $s_min > $s_max) {
+        # Client will fail to send a hello and server will fail to start. The
+        # client failed first so this is reported as ClientFail.
+        return ("ClientFail", undef);
+    } elsif ($c_min > $c_max) {
         # Client should fail to even send a hello.
         # This results in an internal error since the server will be
         # waiting for input that never arrives.