Rename tls1_get_curvelist.

Rename tls1_get_curvelist to tls1_get_grouplist, change to void as
it can never fail and remove unnecessary return value checks. Clean
up the code.
Reviewed-by: NRich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/=4412)

ssl/ssl_locl.h

@@ -2354,8 +2354,8 @@ __owur EVP_PKEY *ssl_generate_param_group(uint16_t id);
 #  endif                        /* OPENSSL_NO_EC */
 
 __owur int tls_curve_allowed(SSL *s, uint16_t curve, int op);
-__owur  int tls1_get_curvelist(SSL *s, int sess, const uint16_t **pcurves,
-                               size_t *num_curves);
+void tls1_get_grouplist(SSL *s, int sess, const uint16_t **pcurves,
+                        size_t *num_curves);
 
 __owur int tls1_set_server_sigalgs(SSL *s);
 

ssl/statem/extensions.c

@@ -1144,18 +1144,8 @@ static int final_key_share(SSL *s, unsigned int context, int sent, int *al)
             /* Check if a shared group exists */
 
             /* Get the clients list of supported groups. */
-            if (!tls1_get_curvelist(s, 1, &clntcurves, &clnt_num_curves)) {
-                *al = SSL_AD_INTERNAL_ERROR;
-                SSLerr(SSL_F_FINAL_KEY_SHARE, ERR_R_INTERNAL_ERROR);
-                return 0;
-            }
-
-            /* Get our list of available groups */
-            if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves)) {
-                *al = SSL_AD_INTERNAL_ERROR;
-                SSLerr(SSL_F_FINAL_KEY_SHARE, ERR_R_INTERNAL_ERROR);
-                return 0;
-            }
+            tls1_get_grouplist(s, 1, &clntcurves, &clnt_num_curves);
+            tls1_get_grouplist(s, 0, &pcurves, &num_curves);
 
             /* Find the first group we allow that is also in client's list */
             for (i = 0; i < num_curves; i++) {

ssl/statem/extensions_clnt.c

@@ -149,11 +149,7 @@ EXT_RETURN tls_construct_ctos_supported_groups(SSL *s, WPACKET *pkt,
      * Add TLS extension supported_groups to the ClientHello message
      */
     /* TODO(TLS1.3): Add support for DHE groups */
-    if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_SUPPORTED_GROUPS,
-               ERR_R_INTERNAL_ERROR);
-        return EXT_RETURN_FAIL;
-    }
+    tls1_get_grouplist(s, 0, &pcurves, &num_curves);
 
     if (!WPACKET_put_bytes_u16(pkt, TLSEXT_TYPE_supported_groups)
                /* Sub-packet for supported_groups extension */
@@ -608,10 +604,7 @@ EXT_RETURN tls_construct_ctos_key_share(SSL *s, WPACKET *pkt,
         return EXT_RETURN_FAIL;
     }
 
-    if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves)) {
-        SSLerr(SSL_F_TLS_CONSTRUCT_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR);
-        return EXT_RETURN_FAIL;
-    }
+    tls1_get_grouplist(s, 0, &pcurves, &num_curves);
 
     /*
      * TODO(TLS1.3): Make the number of key_shares sent configurable. For
@@ -1541,10 +1534,7 @@ int tls_parse_stoc_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
         }
 
         /* Validate the selected group is one we support */
-        if (!tls1_get_curvelist(s, 0, &pcurves, &num_curves)) {
-            SSLerr(SSL_F_TLS_PARSE_STOC_KEY_SHARE, ERR_R_INTERNAL_ERROR);
-            return 0;
-        }
+        tls1_get_grouplist(s, 0, &pcurves, &num_curves);
         for (i = 0; i < num_curves; i++) {
             if (group_id == pcurves[i])
                 break;

ssl/statem/extensions_srvr.c

@@ -520,18 +520,9 @@ int tls_parse_ctos_key_share(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
     }
 
     /* Get our list of supported curves */
-    if (!tls1_get_curvelist(s, 0, &srvrcurves, &srvr_num_curves)) {
-        *al = SSL_AD_INTERNAL_ERROR;
-        SSLerr(SSL_F_TLS_PARSE_CTOS_KEY_SHARE, ERR_R_INTERNAL_ERROR);
-        return 0;
-    }
-
+    tls1_get_grouplist(s, 0, &srvrcurves, &srvr_num_curves);
     /* 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;
-    }
+    tls1_get_grouplist(s, 1, &clntcurves, &clnt_num_curves);
     if (clnt_num_curves == 0) {
         /*
          * This can only happen if the supported_groups extension was not sent,
@@ -894,7 +885,8 @@ EXT_RETURN tls_construct_stoc_supported_groups(SSL *s, WPACKET *pkt,
         return EXT_RETURN_NOT_SENT;
 
     /* Get our list of supported groups */
-    if (!tls1_get_curvelist(s, 0, &groups, &numgroups) || numgroups == 0) {
+    tls1_get_grouplist(s, 0, &groups, &numgroups);
+    if (numgroups == 0) {
         SSLerr(SSL_F_TLS_CONSTRUCT_STOC_SUPPORTED_GROUPS, ERR_R_INTERNAL_ERROR);
         return EXT_RETURN_FAIL;
     }

ssl/t1_lib.c

@@ -213,43 +213,42 @@ static uint16_t tls1_nid2group_id(int nid)
  * The latter indicates an internal error: we should not be accepting such
  * lists in the first place.
  */
-int tls1_get_curvelist(SSL *s, int sess, const uint16_t **pcurves,
-                       size_t *num_curves)
+void tls1_get_grouplist(SSL *s, int sess, const uint16_t **pcurves,
+                        size_t *pcurveslen)
 {
-    size_t pcurveslen = 0;
 
     if (sess) {
         *pcurves = s->session->ext.supportedgroups;
-        pcurveslen = s->session->ext.supportedgroups_len;
-    } else {
-        /* For Suite B mode only include P-256, P-384 */
-        switch (tls1_suiteb(s)) {
-        case SSL_CERT_FLAG_SUITEB_128_LOS:
-            *pcurves = suiteb_curves;
-            pcurveslen = OSSL_NELEM(suiteb_curves);
-            break;
+        *pcurveslen = s->session->ext.supportedgroups_len;
+        return;
+    }
+    /* For Suite B mode only include P-256, P-384 */
+    switch (tls1_suiteb(s)) {
+    case SSL_CERT_FLAG_SUITEB_128_LOS:
+        *pcurves = suiteb_curves;
+        *pcurveslen = OSSL_NELEM(suiteb_curves);
+        break;
 
-        case SSL_CERT_FLAG_SUITEB_128_LOS_ONLY:
-            *pcurves = suiteb_curves;
-            pcurveslen = 1;
-            break;
+    case SSL_CERT_FLAG_SUITEB_128_LOS_ONLY:
+        *pcurves = suiteb_curves;
+        *pcurveslen = 1;
+        break;
 
-        case SSL_CERT_FLAG_SUITEB_192_LOS:
-            *pcurves = suiteb_curves + 1;
-            pcurveslen = 1;
-            break;
-        default:
-            *pcurves = s->ext.supportedgroups;
-            pcurveslen = s->ext.supportedgroups_len;
-        }
-        if (!*pcurves) {
+    case SSL_CERT_FLAG_SUITEB_192_LOS:
+        *pcurves = suiteb_curves + 1;
+        *pcurveslen = 1;
+        break;
+
+    default:
+        if (s->ext.supportedgroups == NULL) {
             *pcurves = eccurves_default;
-            pcurveslen = OSSL_NELEM(eccurves_default);
+            *pcurveslen = OSSL_NELEM(eccurves_default);
+        } else {
+            *pcurves = s->ext.supportedgroups;
+            *pcurveslen = s->ext.supportedgroups_len;
         }
+        break;
     }
-
-    *num_curves = pcurveslen;
-    return 1;
 }
 
 /* See if curve is allowed by security callback */
@@ -293,8 +292,7 @@ int tls1_check_curve(SSL *s, const unsigned char *p, size_t len)
         } else                  /* Should never happen */
             return 0;
     }
-    if (!tls1_get_curvelist(s, 0, &curves, &num_curves))
-        return 0;
+    tls1_get_grouplist(s, 0, &curves, &num_curves);
     for (i = 0; i < num_curves; i++) {
         if (curve_id == curves[i])
             return tls_curve_allowed(s, curve_id, SSL_SECOP_CURVE_CHECK);
@@ -337,17 +335,15 @@ uint16_t tls1_shared_group(SSL *s, int nmatch)
         nmatch = 0;
     }
     /*
-     * Avoid truncation. tls1_get_curvelist takes an int
+     * Avoid truncation. tls1_get_grouplist takes an int
      * but s->options is a long...
      */
-    if (!tls1_get_curvelist(s,
+    tls1_get_grouplist(s,
             (s->options & SSL_OP_CIPHER_SERVER_PREFERENCE) != 0,
-            &supp, &num_supp))
-        return 0;
-    if (!tls1_get_curvelist(s,
+            &supp, &num_supp);
+    tls1_get_grouplist(s,
             (s->options & SSL_OP_CIPHER_SERVER_PREFERENCE) == 0,
-            &pref, &num_pref))
-        return 0;
+            &pref, &num_pref);
 
     for (k = 0, i = 0; i < num_pref; i++) {
         uint16_t id = pref[i];
@@ -511,8 +507,7 @@ static int tls1_check_group_id(SSL *s, uint16_t group_id)
         return 0;
 
     /* Check group is one of our preferences */
-    if (!tls1_get_curvelist(s, 0, &groups, &groups_len))
-        return 0;
+    tls1_get_grouplist(s, 0, &groups, &groups_len);
     for (i = 0; i < groups_len; i++) {
         if (groups[i] == group_id)
             break;
@@ -525,8 +520,7 @@ static int tls1_check_group_id(SSL *s, uint16_t group_id)
         return 1;
 
     /* Check group is one of peers preferences */
-    if (!tls1_get_curvelist(s, 1, &groups, &groups_len))
-        return 0;
+    tls1_get_grouplist(s, 1, &groups, &groups_len);
 
     /*
      * RFC 4492 does not require the supported elliptic curves extension