Make supported_groups code independent of EC and DH

The supported groups code was checking the OPENSSL_NO_EC and
OPENSSL_NO_DH guards in order to work, and the list of default groups was
based on those guards. However we now need it to work even in a no-ec
and no-dh build, because new groups might be added from providers.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13916)

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
index d64b356044e685998bfcead78925cf6182966d96..b8064ea678bff94ece2420c4b33babe6ee811161 100644 (file)
--- a/crypto/err/openssl.txt
+++ b/crypto/err/openssl.txt
@@ -1273,7 +1273,6 @@ SSL_R_CERT_CB_ERROR:377:cert cb error
 SSL_R_CERT_LENGTH_MISMATCH:135:cert length mismatch
 SSL_R_CIPHERSUITE_DIGEST_HAS_CHANGED:218:ciphersuite digest has changed
 SSL_R_CIPHER_CODE_WRONG_LENGTH:137:cipher code wrong length
-SSL_R_CIPHER_OR_HASH_UNAVAILABLE:138:cipher or hash unavailable
 SSL_R_CLIENTHELLO_TLSEXT:226:clienthello tlsext
 SSL_R_COMPRESSED_LENGTH_TOO_LONG:140:compressed length too long
 SSL_R_COMPRESSION_DISABLED:343:compression disabled
@@ -1399,6 +1398,7 @@ SSL_R_NO_SHARED_GROUPS:410:no shared groups
 SSL_R_NO_SHARED_SIGNATURE_ALGORITHMS:376:no shared signature algorithms
 SSL_R_NO_SRTP_PROFILES:359:no srtp profiles
 SSL_R_NO_SUITABLE_DIGEST_ALGORITHM:297:no suitable digest algorithm
+SSL_R_NO_SUITABLE_GROUPS:295:no suitable groups
 SSL_R_NO_SUITABLE_KEY_SHARE:101:no suitable key share
 SSL_R_NO_SUITABLE_SIGNATURE_ALGORITHM:118:no suitable signature algorithm
 SSL_R_NO_VALID_SCTS:216:no valid scts

diff --git a/include/openssl/sslerr.h b/include/openssl/sslerr.h
index 27664afd58d995dfc353b8a5f7cbe2cd46a50faf..d69055cf3d41b04d0ad28e306f7913941c292dca 100644 (file)
--- a/include/openssl/sslerr.h
+++ b/include/openssl/sslerr.h
@@ -195,6 +195,7 @@
 # define SSL_R_NO_SHARED_SIGNATURE_ALGORITHMS             376
 # define SSL_R_NO_SRTP_PROFILES                           359
 # define SSL_R_NO_SUITABLE_DIGEST_ALGORITHM               297
+# define SSL_R_NO_SUITABLE_GROUPS                         295
 # define SSL_R_NO_SUITABLE_KEY_SHARE                      101
 # define SSL_R_NO_SUITABLE_SIGNATURE_ALGORITHM            118
 # define SSL_R_NO_VALID_SCTS                              216

diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index 879b27652020ee720200b6febf5e18b8041c31b3..d6d0c671a259d74753c7adf3743d675d3c0662ef 100644 (file)
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -299,6 +299,7 @@ static const ERR_STRING_DATA SSL_str_reasons[] = {
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_SRTP_PROFILES), "no srtp profiles"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_SUITABLE_DIGEST_ALGORITHM),
     "no suitable digest algorithm"},
+    {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_SUITABLE_GROUPS), "no suitable groups"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_SUITABLE_KEY_SHARE),
     "no suitable key share"},
     {ERR_PACK(ERR_LIB_SSL, 0, SSL_R_NO_SUITABLE_SIGNATURE_ALGORITHM),

diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
index 4138f4eaa32f900464c900e61a72ede5750788e1..87a4f428f876eba557a506d7be17b6dfa2b770de 100644 (file)
--- a/ssl/ssl_local.h
+++ b/ssl/ssl_local.h
@@ -2661,7 +2661,7 @@ __owur int tls1_set_groups_list(SSL_CTX *ctx, uint16_t **pext, size_t *pextlen,
                                 const char *str);
 __owur EVP_PKEY *ssl_generate_pkey_group(SSL *s, uint16_t id);
 __owur int tls_valid_group(SSL *s, uint16_t group_id, int minversion,
-                           int maxversion);
+                           int maxversion, int isec, int *okfortls13);
 __owur EVP_PKEY *ssl_generate_param_group(SSL *s, uint16_t id);
 #  ifndef OPENSSL_NO_EC
 void tls1_get_formatlist(SSL *s, const unsigned char **pformats,

diff --git a/ssl/sslerr.h b/ssl/sslerr.h
index ed70efc264171b8087a382c402fb22925e727d22..3ad54e4dcc2a22dfa576193d7c7416e1b1a8a328 100644 (file)
--- a/ssl/sslerr.h
+++ b/ssl/sslerr.h
@@ -1,6 +1,6 @@
 /*
  * Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 2020 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 2020-2021 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the Apache License 2.0 (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy

diff --git a/ssl/statem/extensions_clnt.c b/ssl/statem/extensions_clnt.c
index 89e1422bbd4f21a5c2c313f1d090eada613e2d15..cc958aa1b0e44bedebfc623d0b53ede5dc3fbbf0 100644 (file)
--- a/ssl/statem/extensions_clnt.c
+++ b/ssl/statem/extensions_clnt.c
@@ -108,7 +108,6 @@ EXT_RETURN tls_construct_ctos_srp(SSL *s, WPACKET *pkt, unsigned int context,
 }
 #endif
 
-#ifndef OPENSSL_NO_EC
 static int use_ecc(SSL *s, int min_version, int max_version)
 {
     int i, end, ret = 0;
@@ -144,7 +143,7 @@ static int use_ecc(SSL *s, int min_version, int max_version)
     for (j = 0; j < num_groups; j++) {
         uint16_t ctmp = pgroups[j];
 
-        if (tls_valid_group(s, ctmp, min_version, max_version)
+        if (tls_valid_group(s, ctmp, min_version, max_version, 1, NULL)
                 && tls_group_allowed(s, ctmp, SSL_SECOP_CURVE_SUPPORTED))
             return 1;
     }
@@ -152,6 +151,7 @@ static int use_ecc(SSL *s, int min_version, int max_version)
     return 0;
 }
 
+#ifndef OPENSSL_NO_EC
 EXT_RETURN tls_construct_ctos_ec_pt_formats(SSL *s, WPACKET *pkt,
                                             unsigned int context, X509 *x,
                                             size_t chainidx)
@@ -189,7 +189,7 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt,
                                                size_t chainidx)
 {
     const uint16_t *pgroups = NULL;
-    size_t num_groups = 0, i;
+    size_t num_groups = 0, i, tls13added = 0, added = 0;
     int min_version, max_version, reason;
 
     reason = ssl_get_min_max_version(s, &min_version, &max_version, NULL);
@@ -198,13 +198,13 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt,
         return EXT_RETURN_FAIL;
     }
 
-#if defined(OPENSSL_NO_EC)
-    if (SSL_IS_DTLS(s) || max_version < TLS1_3_VERSION)
-        return EXT_RETURN_NOT_SENT;
-#else
-    if (!use_ecc(s, min_version, max_version) && max_version < TLS1_3_VERSION)
+    /*
+     * We only support EC groups in TLSv1.2 or below, and in DTLS. Therefore
+     * if we don't have EC support then we don't send this extension.
+     */
+    if (!use_ecc(s, min_version, max_version)
+            && (SSL_IS_DTLS(s) || max_version < TLS1_3_VERSION))
         return EXT_RETURN_NOT_SENT;
-#endif
 
     /*
      * Add TLS extension supported_groups to the ClientHello message
@@ -222,17 +222,25 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt,
     /* Copy group ID if supported */
     for (i = 0; i < num_groups; i++) {
         uint16_t ctmp = pgroups[i];
+        int okfortls13;
 
-        if (tls_valid_group(s, ctmp, min_version, max_version)
+        if (tls_valid_group(s, ctmp, min_version, max_version, 0, &okfortls13)
                 && tls_group_allowed(s, ctmp, SSL_SECOP_CURVE_SUPPORTED)) {
             if (!WPACKET_put_bytes_u16(pkt, ctmp)) {
                 SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
                 return EXT_RETURN_FAIL;
             }
+            if (okfortls13 && max_version == TLS1_3_VERSION)
+                tls13added++;
+            added++;
         }
     }
     if (!WPACKET_close(pkt) || !WPACKET_close(pkt)) {
-        SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+        if (added == 0 || (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");
+        else
+            SSLfatal(s, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
         return EXT_RETURN_FAIL;
     }
 

diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 99cd515386fdc8f1b00d27c5afde473badb6e9ff..42fd6ee7d3096bf4dda8ab4fde5a134b97d9d6f1 100644 (file)
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -1355,7 +1355,7 @@ EXT_RETURN tls_construct_stoc_supported_groups(SSL *s, WPACKET *pkt,
     for (i = 0; i < numgroups; i++) {
         uint16_t group = groups[i];
 
-        if (tls_valid_group(s, group, version, version)
+        if (tls_valid_group(s, group, version, version, 0, NULL)
                 && tls_group_allowed(s, group, SSL_SECOP_CURVE_SUPPORTED)) {
             if (first) {
                 /*

diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 24bfa96382f867363712bd91fa90acd4ff2cc726..6bc33215c1260c45b0206df1a7b71373c1d83235 100644 (file)
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -391,15 +391,17 @@ static int discover_provider_groups(OSSL_PROVIDER *provider, void *vctx)
 int ssl_load_groups(SSL_CTX *ctx)
 {
     size_t i, j, num_deflt_grps = 0;
-    uint16_t tmp_supp_groups[sizeof(supported_groups_default)];
+    uint16_t tmp_supp_groups[OSSL_NELEM(supported_groups_default)];
 
     if (!OSSL_PROVIDER_do_all(ctx->libctx, discover_provider_groups, ctx))
         return 0;
 
-    for (i = 0; i < sizeof(supported_groups_default); i++) {
+    for (i = 0; i < OSSL_NELEM(supported_groups_default); i++) {
         for (j = 0; j < ctx->group_list_len; j++) {
-            if (ctx->group_list[j].group_id == supported_groups_default[i])
+            if (ctx->group_list[j].group_id == supported_groups_default[i]) {
                 tmp_supp_groups[num_deflt_grps++] = ctx->group_list[j].group_id;
+                break;
+            }
         }
     }
 
@@ -414,7 +416,9 @@ int ssl_load_groups(SSL_CTX *ctx)
         return 0;
     }
 
-    memcpy(ctx->ext.supported_groups_default, tmp_supp_groups, num_deflt_grps);
+    memcpy(ctx->ext.supported_groups_default,
+           tmp_supp_groups,
+           num_deflt_grps * sizeof(tmp_supp_groups[0]));
     ctx->ext.supported_groups_default_len = num_deflt_grps;
 
     return 1;
@@ -534,11 +538,15 @@ void tls1_get_supported_groups(SSL *s, const uint16_t **pgroups,
     }
 }
 
-int tls_valid_group(SSL *s, uint16_t group_id, int minversion, int maxversion)
+int tls_valid_group(SSL *s, uint16_t group_id, int minversion, int maxversion,
+                    int isec, int *okfortls13)
 {
     const TLS_GROUP_INFO *ginfo = tls1_group_id_lookup(s->ctx, group_id);
     int ret;
 
+    if (okfortls13 != NULL)
+        okfortls13 = 0;
+
     if (ginfo == NULL)
         return 0;
 
@@ -560,7 +568,14 @@ int tls_valid_group(SSL *s, uint16_t group_id, int minversion, int maxversion)
             ret = (minversion <= ginfo->maxtls);
         if (ginfo->mintls > 0)
             ret &= (maxversion >= ginfo->mintls);
-    }
+        if (ret && okfortls13 != NULL && maxversion == TLS1_3_VERSION)
+            *okfortls13 = (ginfo->maxtls == 0)
+                          || (ginfo->maxtls >= TLS1_3_VERSION);
+    }
+    ret &= !isec
+           || strcmp(ginfo->algorithm, "EC") == 0
+           || strcmp(ginfo->algorithm, "X25519") == 0
+           || strcmp(ginfo->algorithm, "X448") == 0;
 
     return ret;
 }