Send a missing_extension alert if key_share/supported groups not present

Only applies if we're not doing psk.

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

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 23dde118086d8134de9c1897496b1851293df623..4558b17c3c321f20626068a66ddf40cb39346efa 100644 (file)
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -2675,6 +2675,7 @@ int ERR_load_SSL_strings(void);
 # define SSL_R_MISSING_RSA_SIGNING_CERT                   170
 # define SSL_R_MISSING_SIGALGS_EXTENSION                  112
 # define SSL_R_MISSING_SRP_PARAM                          358
+# define SSL_R_MISSING_SUPPORTED_GROUPS_EXTENSION         209
 # define SSL_R_MISSING_TMP_DH_KEY                         171
 # define SSL_R_MISSING_TMP_ECDH_KEY                       311
 # define SSL_R_NOT_ON_RECORD_BOUNDARY                     182

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 42bd6aa67896e801ca6975ae42ead2131555643e..62d7d76835f1bb397397fdccc97b7655ae849a66 100644 (file)
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -641,6 +641,8 @@ static ERR_STRING_DATA SSL_str_reasons[] = {
     {ERR_REASON(SSL_R_MISSING_SIGALGS_EXTENSION),
      "missing sigalgs extension"},
     {ERR_REASON(SSL_R_MISSING_SRP_PARAM), "can't find SRP server param"},
+    {ERR_REASON(SSL_R_MISSING_SUPPORTED_GROUPS_EXTENSION),
+     "missing supported groups extension"},
     {ERR_REASON(SSL_R_MISSING_TMP_DH_KEY), "missing tmp dh key"},
     {ERR_REASON(SSL_R_MISSING_TMP_ECDH_KEY), "missing tmp ecdh key"},
     {ERR_REASON(SSL_R_NOT_ON_RECORD_BOUNDARY), "not on record boundary"},

diff --git a/ssl/statem/extensions.c b/ssl/statem/extensions.c
index 9b16014f7b86bbf89f771b685c477739aaeb1f7b..578ca13a742b06db0dbfd619b307e25176499185 100644 (file)
--- a/ssl/statem/extensions.c
+++ b/ssl/statem/extensions.c
@@ -1151,7 +1151,10 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al)
         if (!s->hit
                 || (s->ext.psk_kex_mode & TLSEXT_KEX_MODE_FLAG_KE) == 0) {
             /* Nothing left we can do - just fail */
-            *al = SSL_AD_HANDSHAKE_FAILURE;
+            if (!sent)
+                *al = SSL_AD_MISSING_EXTENSION;
+            else
+                *al = SSL_AD_HANDSHAKE_FAILURE;
             SSLerr(SSL_F_FINAL_KEY_SHARE, SSL_R_NO_SUITABLE_KEY_SHARE);
             return 0;
         }

diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 0daf7d4b010083cfbd388a31e3c2e82180d84887..f85477c8f4488ebb6e0cdc8314090fec2e39cad2 100644 (file)
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -524,16 +524,23 @@ int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
         return 0;
     }
 
-    /*
-     * Get the clients list of supported curves.
-     * TODO(TLS1.3): We should validate that we actually received
-     * supported_groups!
-     */
+    /* Get the clients list of supported curves. */
     if (!tls1_get_curvelist(s, 1, &clntcurves, &clnt_num_curves)) {
         *al = SSL_AD_INTERNAL_ERROR;
         SSLerr(SSL_F_TLS_PARSE_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR);
         return 0;
     }
+    if (clnt_num_curves == 0) {
+        /*
+         * This can only happen if the supported_groups extension was not sent,
+         * because we verify that the length is non-zero when we process that
+         * extension.
+         */
+        *al = SSL_AD_MISSING_EXTENSION;
+        SSLerr(SSL_F_TLS_PARSE_CTOS_KEY_SHARE,
+               SSL_R_MISSING_SUPPORTED_GROUPS_EXTENSION);
+        return 0;
+    }
 
     while (PACKET_remaining(&key_share_list) > 0) {
         if (!PACKET_get_net_2(&key_share_list, &group_id)