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: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3316)

diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 8eb6ff52c9b5f980c009f9c13d61495eadc76057..15065c7d982f5915e633c25ca1d663db0ba8bd9e 100644 (file)
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -2194,8 +2194,7 @@ __owur int ssl_choose_server_version(SSL *s, CLIENTHELLO_MSG *hello,
                                      DOWNGRADE *dgrd);
 __owur int ssl_choose_client_version(SSL *s, int version, int checkdgrd,
                                      int *al);
                                      DOWNGRADE *dgrd);
 __owur int ssl_choose_client_version(SSL *s, int version, int checkdgrd,
                                      int *al);

-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);
 
 __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 7ec71281720938f29b5cbce6ab66a1f572f26499..c8ed722ab157144b187091c16ac972041b0be50e 100644 (file)
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -674,7 +674,7 @@ int tls_construct_extensions(SSL *s, WPACKET *pkt, unsigned int context,
     }
 
     if ((context & SSL_EXT_CLIENT_HELLO) != 0) {
     }
 
     if ((context & SSL_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;
         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 7d2a4b04a5f135c32492502836e9d5ccbefe19a5..898992d99b8f6f09dd8feaa370d060d4c62f53bf 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;
     }
 
         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;
     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 01804458fa11ddefc89b25a7095488659589ec44..36d55343b82aa9793a65c65cf52f1941c950e598 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) {
         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 &&
         if (SSL_IS_FIRST_HANDSHAKE(s)) {
             s->ctx->stats.sess_accept++;
         } else if (!s->s3->send_connection_binding &&


@@ -1781,7 +1814,7 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)
 }
 
 /*
 }
 
 /*

- * 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
  * @s: The SSL connection
  * @min_version: The minimum supported version
  * @max_version: The maximum supported version


@@ -1799,8 +1832,7 @@ int ssl_choose_client_version(SSL *s, int version, int checkdgrd, int *al)
  * 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.
  */
  * 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;
 {
     int version;
     int hole;


@@ -1894,7 +1926,7 @@ int ssl_set_client_hello_version(SSL *s)
 {
     int ver_min, ver_max, ret;
 
 {
     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;
 
     if (ret != 0)
         return ret;

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 5007f7ebb37a672eaeb6882b59c0f9fbb01399c8..0e1a97e175454856f97ac24a2977d9053222da1f 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);
     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) {
 #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");
     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,
     $proxy->ciphers("ECDHE-RSA-AES128-SHA");
     $proxy->start();
     checkhandshake($proxy, checkhandshake::EC_HANDSHAKE,

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
 [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]
 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
 [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]
 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
 [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]
 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
 [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]
 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 = (
     {
 
 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
+);