Simplify EC_GROUP_new_by_curve_name()

Pull the setting of the name a.k.a. nid into ec_group_new_from_data().
This way, we can return early on finding the nid in the curve_list[].
This also avoids a silly bug where a bogus ERR_R_UNKNOWN_BUG is pushed
onto the error stack when ec_group_new_from_data() failed.

While there rework the exit path of ec_group_new_from_data() a bit.
Instead of an ok variable we can use an additional pointer to keep
track of the return value and free the EC_GROUP unconditionally.

ok jsing

1 files changed, 14 insertions, 20 deletions

diff --git a/src/lib/libcrypto/ec/ec_curve.c b/src/lib/libcrypto/ec/ec_curve.c
index 9ab8c88f5e..e5c3d87644 100644
--- a/src/lib/libcrypto/ec/ec_curve.c
+++ b/src/lib/libcrypto/ec/ec_curve.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ec_curve.c,v 1.39 2023/05/01 17:53:01 tb Exp $ */
+/* $OpenBSD: ec_curve.c,v 1.40 2023/05/02 10:44:20 tb Exp $ */
 /*
  * Written by Nils Larsch for the OpenSSL project.
  */
@@ -3000,11 +3000,10 @@ static const struct ec_list_element {
 static EC_GROUP *
 ec_group_new_from_data(const struct ec_list_element *curve)
 {
-        EC_GROUP *group = NULL;
+        EC_GROUP *group = NULL, *ret = NULL;
         EC_POINT *P = NULL;
         BN_CTX *ctx = NULL;
         BIGNUM *p, *a, *b, *x, *y, *order, *cofactor;
-        int ok = 0;
 
         if ((ctx = BN_CTX_new()) == NULL) {
                 ECerror(ERR_R_MALLOC_FAILURE);
@@ -3057,6 +3056,7 @@ ec_group_new_from_data(const struct ec_list_element *curve)
                 ECerror(ERR_R_EC_LIB);
                 goto err;
         }
+        EC_GROUP_set_curve_name(group, curve->nid);
 
         if ((P = EC_POINT_new(group)) == NULL) {
                 ECerror(ERR_R_EC_LIB);
@@ -3086,47 +3086,41 @@ ec_group_new_from_data(const struct ec_list_element *curve)
                 ECerror(ERR_R_EC_LIB);
                 goto err;
         }
+
         if (curve->seed != NULL) {
                 if (!EC_GROUP_set_seed(group, curve->seed, curve->seed_len)) {
                         ECerror(ERR_R_EC_LIB);
                         goto err;
                 }
         }
-        ok = 1;
+
+        ret = group;
+        group = NULL;
+
  err:
-        if (!ok) {
-                EC_GROUP_free(group);
-                group = NULL;
-        }
+        EC_GROUP_free(group);
         EC_POINT_free(P);
         BN_CTX_end(ctx);
         BN_CTX_free(ctx);
 
-        return group;
+        return ret;
 }
 
 EC_GROUP *
 EC_GROUP_new_by_curve_name(int nid)
 {
         size_t i;
-        EC_GROUP *ret = NULL;
 
         if (nid <= 0)
                 return NULL;
 
         for (i = 0; i < CURVE_LIST_LENGTH; i++) {
-                if (curve_list[i].nid == nid) {
-                        ret = ec_group_new_from_data(&curve_list[i]);
-                        break;
-                }
-        }
-        if (ret == NULL) {
-                ECerror(EC_R_UNKNOWN_GROUP);
-                return NULL;
+                if (curve_list[i].nid == nid)
+                        return ec_group_new_from_data(&curve_list[i]);
         }
-        EC_GROUP_set_curve_name(ret, nid);
 
-        return ret;
+        ECerror(EC_R_UNKNOWN_GROUP);
+        return NULL;
 }
 
 size_t