Rename uses 'curve' to 'group' and rework tls1 group API.

This reworks various tls1_ curve APIs to indicate success via a boolean
return value and move the output to an out parameter. This makes the
caller code easier and more consistent.

Based on a suggestion by jsing

ok jsing

1 files changed, 158 insertions, 116 deletions

diff --git a/src/lib/libssl/t1_lib.c b/src/lib/libssl/t1_lib.c
index 9748901268..beaaae1eb0 100644
--- a/src/lib/libssl/t1_lib.c
+++ b/src/lib/libssl/t1_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t1_lib.c,v 1.190 2022/07/02 15:53:37 tb Exp $ */
+/* $OpenBSD: t1_lib.c,v 1.191 2022/07/02 16:00:12 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -150,12 +150,16 @@ tls1_clear(SSL *s)
         s->version = s->method->version;
 }
 
-struct curve {
+struct supported_group {
         int nid;
         int bits;
 };
 
-static const struct curve nid_list[] = {
+/*
+ * Supported groups (formerly known as named curves)
+ * https://www.iana.org/assignments/tls-parameters/#tls-parameters-8
+ */
+static const struct supported_group nid_list[] = {
         [1] = {
                 .nid = NID_sect163k1,
                 .bits = 80,
@@ -274,6 +278,8 @@ static const struct curve nid_list[] = {
         },
 };
 
+#define NID_LIST_LEN (sizeof(nid_list) / sizeof(nid_list[0]))
+
 #if 0
 static const uint8_t ecformats_list[] = {
         TLSEXT_ECPOINTFORMAT_uncompressed,
@@ -287,7 +293,7 @@ static const uint8_t ecformats_default[] = {
 };
 
 #if 0
-static const uint16_t eccurves_list[] = {
+static const uint16_t ecgroups_list[] = {
         29,                     /* X25519 (29) */
         14,                     /* sect571r1 (14) */
         13,                     /* sect571k1 (13) */
@@ -320,116 +326,155 @@ static const uint16_t eccurves_list[] = {
 };
 #endif
 
-static const uint16_t eccurves_client_default[] = {
+static const uint16_t ecgroups_client_default[] = {
         29,                     /* X25519 (29) */
         23,                     /* secp256r1 (23) */
         24,                     /* secp384r1 (24) */
         25,                     /* secp521r1 (25) */
 };
 
-static const uint16_t eccurves_server_default[] = {
+static const uint16_t ecgroups_server_default[] = {
         29,                     /* X25519 (29) */
         23,                     /* secp256r1 (23) */
         24,                     /* secp384r1 (24) */
 };
 
 int
-tls1_ec_curve_id2nid(const uint16_t curve_id)
+tls1_ec_group_id2nid(uint16_t group_id, int *out_nid)
 {
-        const struct curve *curve;
+        const struct supported_group *group;
 
-        /* ECC curves from draft-ietf-tls-ecc-12.txt (Oct. 17, 2005) */
-        if ((curve_id < 1) ||
-            ((unsigned int)curve_id >= sizeof(nid_list) / sizeof(nid_list[0])))
-                return NID_undef;
+        if (group_id < 1 || group_id >= NID_LIST_LEN)
+                return 0;
+
+        if ((group = &nid_list[group_id]) == NULL)
+                return 0;
 
-        if ((curve = &nid_list[curve_id]) == NULL)
-                return NID_undef;
+        *out_nid = group->nid;
 
-        return curve->nid;
+        return 1;
 }
 
 int
-tls1_ec_curve_id2bits(const uint16_t curve_id)
+tls1_ec_group_id2bits(uint16_t group_id, int *out_bits)
 {
-        const struct curve *curve;
+        const struct supported_group *group;
 
-        if ((curve_id < 1) ||
-            ((unsigned int)curve_id >= sizeof(nid_list) / sizeof(nid_list[0])))
+        if (group_id < 1 || group_id >= NID_LIST_LEN)
                 return 0;
 
-        if ((curve = &nid_list[curve_id]) == NULL)
+        if ((group = &nid_list[group_id]) == NULL)
                 return 0;
 
-        return curve->bits;
+        *out_bits = group->bits;
+
+        return 1;
 }
 
-uint16_t
-tls1_ec_nid2curve_id(const int nid)
+int
+tls1_ec_nid2group_id(const int nid, uint16_t *out_group_id)
 {
-        /* ECC curves from draft-ietf-tls-ecc-12.txt (Oct. 17, 2005) */
+        uint16_t group_id;
+
         switch (nid) {
-        case NID_sect163k1: /* sect163k1 (1) */
-                return 1;
-        case NID_sect163r1: /* sect163r1 (2) */
-                return 2;
-        case NID_sect163r2: /* sect163r2 (3) */
-                return 3;
-        case NID_sect193r1: /* sect193r1 (4) */
-                return 4;
-        case NID_sect193r2: /* sect193r2 (5) */
-                return 5;
-        case NID_sect233k1: /* sect233k1 (6) */
-                return 6;
-        case NID_sect233r1: /* sect233r1 (7) */
-                return 7;
-        case NID_sect239k1: /* sect239k1 (8) */
-                return 8;
-        case NID_sect283k1: /* sect283k1 (9) */
-                return 9;
-        case NID_sect283r1: /* sect283r1 (10) */
-                return 10;
-        case NID_sect409k1: /* sect409k1 (11) */
-                return 11;
-        case NID_sect409r1: /* sect409r1 (12) */
-                return 12;
-        case NID_sect571k1: /* sect571k1 (13) */
-                return 13;
-        case NID_sect571r1: /* sect571r1 (14) */
-                return 14;
-        case NID_secp160k1: /* secp160k1 (15) */
-                return 15;
-        case NID_secp160r1: /* secp160r1 (16) */
-                return 16;
-        case NID_secp160r2: /* secp160r2 (17) */
-                return 17;
-        case NID_secp192k1: /* secp192k1 (18) */
-                return 18;
-        case NID_X9_62_prime192v1: /* secp192r1 (19) */
-                return 19;
-        case NID_secp224k1: /* secp224k1 (20) */
-                return 20;
-        case NID_secp224r1: /* secp224r1 (21) */
-                return 21;
-        case NID_secp256k1: /* secp256k1 (22) */
-                return 22;
-        case NID_X9_62_prime256v1: /* secp256r1 (23) */
-                return 23;
-        case NID_secp384r1: /* secp384r1 (24) */
-                return 24;
-        case NID_secp521r1: /* secp521r1 (25) */
-                return 25;
-        case NID_brainpoolP256r1: /* brainpoolP256r1 (26) */
-                return 26;
-        case NID_brainpoolP384r1: /* brainpoolP384r1 (27) */
-                return 27;
-        case NID_brainpoolP512r1: /* brainpoolP512r1 (28) */
-                return 28;
-        case NID_X25519:                /* X25519 (29) */
-                return 29;
+        case NID_sect163k1:
+                group_id = 1;
+                break;
+        case NID_sect163r1:
+                group_id = 2;
+                break;
+        case NID_sect163r2:
+                group_id = 3;
+                break;
+        case NID_sect193r1:
+                group_id = 4;
+                break;
+        case NID_sect193r2:
+                group_id = 5;
+                break;
+        case NID_sect233k1:
+                group_id = 6;
+                break;
+        case NID_sect233r1:
+                group_id = 7;
+                break;
+        case NID_sect239k1:
+                group_id = 8;
+                break;
+        case NID_sect283k1:
+                group_id = 9;
+                break;
+        case NID_sect283r1:
+                group_id = 10;
+                break;
+        case NID_sect409k1:
+                group_id = 11;
+                break;
+        case NID_sect409r1:
+                group_id = 12;
+                break;
+        case NID_sect571k1:
+                group_id = 13;
+                break;
+        case NID_sect571r1:
+                group_id = 14;
+                break;
+        case NID_secp160k1:
+                group_id = 15;
+                break;
+        case NID_secp160r1:
+                group_id = 16;
+                break;
+        case NID_secp160r2:
+                group_id = 17;
+                break;
+        case NID_secp192k1:
+                group_id = 18;
+                break;
+        case NID_X9_62_prime192v1: /* aka secp192r1 */
+                group_id = 19;
+                break;
+        case NID_secp224k1:
+                group_id = 20;
+                break;
+        case NID_secp224r1:
+                group_id = 21;
+                break;
+        case NID_secp256k1:
+                group_id = 22;
+                break;
+        case NID_X9_62_prime256v1: /* aka secp256r1 */
+                group_id = 23;
+                break;
+        case NID_secp384r1:
+                group_id = 24;
+                break;
+        case NID_secp521r1:
+                group_id = 25;
+                break;
+        case NID_brainpoolP256r1:
+                group_id = 26;
+                break;
+        case NID_brainpoolP384r1:
+                group_id = 27;
+                break;
+        case NID_brainpoolP512r1:
+                group_id = 28;
+                break;
+        case NID_X25519:
+                group_id = 29;
+                break;
         default:
-                return 0;
+                group_id = 0;
+                break;
         }
+
+        if (group_id == 0)
+                return 0;
+
+        *out_group_id = group_id;
+
+        return 1;
 }
 
 /*
@@ -476,11 +521,11 @@ tls1_get_group_list(SSL *s, int client_groups, const uint16_t **pgroups,
                 return;
 
         if (!s->server) {
-                *pgroups = eccurves_client_default;
-                *pgroupslen = sizeof(eccurves_client_default) / 2;
+                *pgroups = ecgroups_client_default;
+                *pgroupslen = sizeof(ecgroups_client_default) / 2;
         } else {
-                *pgroups = eccurves_server_default;
-                *pgroupslen = sizeof(eccurves_server_default) / 2;
+                *pgroups = ecgroups_server_default;
+                *pgroupslen = sizeof(ecgroups_server_default) / 2;
         }
 }
 
@@ -491,13 +536,11 @@ tls1_set_groups(uint16_t **out_group_ids, size_t *out_group_ids_len,
         uint16_t *group_ids;
         size_t i;
 
-        group_ids = calloc(ngroups, sizeof(uint16_t));
-        if (group_ids == NULL)
+        if ((group_ids = calloc(ngroups, sizeof(uint16_t))) == NULL)
                 return 0;
 
         for (i = 0; i < ngroups; i++) {
-                group_ids[i] = tls1_ec_nid2curve_id(groups[i]);
-                if (group_ids[i] == 0) {
+                if (!tls1_ec_nid2group_id(groups[i], &group_ids[i])) {
                         free(group_ids);
                         return 0;
                 }
@@ -537,8 +580,7 @@ tls1_set_group_list(uint16_t **out_group_ids, size_t *out_group_ids_len,
                         goto err;
                 group_ids = new_group_ids;
 
-                group_ids[ngroups] = tls1_ec_nid2curve_id(nid);
-                if (group_ids[ngroups] == 0)
+                if (!tls1_ec_nid2group_id(nid, &group_ids[ngroups]))
                         goto err;
 
                 ngroups++;
@@ -558,9 +600,9 @@ tls1_set_group_list(uint16_t **out_group_ids, size_t *out_group_ids_len,
         return 0;
 }
 
-/* Check that a curve is one of our preferences. */
+/* Check that a group is one of our preferences. */
 int
-tls1_check_curve(SSL *s, const uint16_t curve_id)
+tls1_check_group(SSL *s, uint16_t group_id)
 {
         const uint16_t *groups;
         size_t groupslen, i;
@@ -570,14 +612,14 @@ tls1_check_curve(SSL *s, const uint16_t curve_id)
         for (i = 0; i < groupslen; i++) {
                 if (!ssl_security_supported_group(s, groups[i]))
                         continue;
-                if (groups[i] == curve_id)
-                        return (1);
+                if (groups[i] == group_id)
+                        return 1;
         }
-        return (0);
+        return 0;
 }
 
 int
-tls1_get_shared_curve(SSL *s)
+tls1_get_supported_group(SSL *s, int *out_nid)
 {
         size_t preflen, supplen, i, j;
         const uint16_t *pref, *supp;
@@ -585,9 +627,9 @@ tls1_get_shared_curve(SSL *s)
 
         /* Cannot do anything on the client side. */
         if (s->server == 0)
-                return (NID_undef);
+                return 0;
 
-        /* Return first preference shared curve. */
+        /* Return first preference supported group. */
         server_pref = (s->internal->options & SSL_OP_CIPHER_SERVER_PREFERENCE);
         tls1_get_group_list(s, (server_pref == 0), &pref, &preflen);
         tls1_get_group_list(s, (server_pref != 0), &supp, &supplen);
@@ -597,15 +639,15 @@ tls1_get_shared_curve(SSL *s)
                         continue;
                 for (j = 0; j < supplen; j++) {
                         if (pref[i] == supp[j])
-                                return (tls1_ec_curve_id2nid(pref[i]));
+                                return tls1_ec_group_id2nid(pref[i], out_nid);
                 }
         }
-        return (NID_undef);
+        return 0;
 }
 
 /* For an EC key set TLS ID and required compression based on parameters. */
 static int
-tls1_set_ec_id(uint16_t *curve_id, uint8_t *comp_id, EC_KEY *ec)
+tls1_set_ec_id(uint16_t *group_id, uint8_t *comp_id, EC_KEY *ec)
 {
         const EC_GROUP *grp;
         const EC_METHOD *meth;
@@ -615,18 +657,18 @@ tls1_set_ec_id(uint16_t *curve_id, uint8_t *comp_id, EC_KEY *ec)
         if (ec == NULL)
                 return (0);
 
-        /* Determine whether the curve is defined over a prime field. */
+        /* Determine whether the group is defined over a prime field. */
         if ((grp = EC_KEY_get0_group(ec)) == NULL)
                 return (0);
         if ((meth = EC_GROUP_method_of(grp)) == NULL)
                 return (0);
         prime_field = (EC_METHOD_get_field_type(meth) == NID_X9_62_prime_field);
 
-        /* Determine curve ID - NID_undef results in a curve ID of zero. */
+        /* Determine group ID. */
         nid = EC_GROUP_get_curve_name(grp);
-        /* If we have an ID set it, otherwise set arbitrary explicit curve. */
-        if ((*curve_id = tls1_ec_nid2curve_id(nid)) == 0)
-                *curve_id = prime_field ? 0xff01 : 0xff02;
+        /* If we have an ID set it, otherwise set arbitrary explicit group. */
+        if (!tls1_ec_nid2group_id(nid, group_id))
+                *group_id = prime_field ? 0xff01 : 0xff02;
 
         if (comp_id == NULL)
                 return (1);
@@ -646,7 +688,7 @@ tls1_set_ec_id(uint16_t *curve_id, uint8_t *comp_id, EC_KEY *ec)
 
 /* Check that an EC key is compatible with extensions. */
 static int
-tls1_check_ec_key(SSL *s, const uint16_t *curve_id, const uint8_t *comp_id)
+tls1_check_ec_key(SSL *s, const uint16_t *group_id, const uint8_t *comp_id)
 {
         size_t groupslen, formatslen, i;
         const uint16_t *groups;
@@ -667,12 +709,12 @@ tls1_check_ec_key(SSL *s, const uint16_t *curve_id, const uint8_t *comp_id)
         }
 
         /*
-         * Check curve list if present, otherwise everything is supported.
+         * Check group list if present, otherwise everything is supported.
          */
         tls1_get_group_list(s, 1, &groups, &groupslen);
-        if (curve_id != NULL && groups != NULL) {
+        if (group_id != NULL && groups != NULL) {
                 for (i = 0; i < groupslen; i++) {
-                        if (groups[i] == *curve_id)
+                        if (groups[i] == *group_id)
                                 break;
                 }
                 if (i == groupslen)
@@ -687,7 +729,7 @@ int
 tls1_check_ec_server_key(SSL *s)
 {
         SSL_CERT_PKEY *cpk = s->cert->pkeys + SSL_PKEY_ECC;
-        uint16_t curve_id;
+        uint16_t group_id;
         uint8_t comp_id;
         EC_KEY *eckey;
         EVP_PKEY *pkey;
@@ -698,10 +740,10 @@ tls1_check_ec_server_key(SSL *s)
                 return (0);
         if ((eckey = EVP_PKEY_get0_EC_KEY(pkey)) == NULL)
                 return (0);
-        if (!tls1_set_ec_id(&curve_id, &comp_id, eckey))
+        if (!tls1_set_ec_id(&group_id, &comp_id, eckey))
                 return (0);
 
-        return tls1_check_ec_key(s, &curve_id, &comp_id);
+        return tls1_check_ec_key(s, &group_id, &comp_id);
 }
 
 int