Always clear EC groups and points on free.
authorjsing <jsing@openbsd.org>
Wed, 8 Mar 2023 05:45:31 +0000 (05:45 +0000)
committerjsing <jsing@openbsd.org>
Wed, 8 Mar 2023 05:45:31 +0000 (05:45 +0000)
Rather than sometimes clearing, turn the free functions into ones that
always clear (as we've done elsewhere). Turn the EC_GROUP_clear_free() and
EC_POINT_clear_free() functions into wrappers that call the *_free()
version. Do similar for the EC_METHOD implementations, removing the
group_clear_finish() and point_clear_finish() hooks in the process.

ok tb@

lib/libcrypto/ec/ec.h
lib/libcrypto/ec/ec2_smpl.c
lib/libcrypto/ec/ec_asn1.c
lib/libcrypto/ec/ec_lib.c
lib/libcrypto/ec/ec_local.h
lib/libcrypto/ec/ec_mult.c
lib/libcrypto/ec/ec_print.c
lib/libcrypto/ec/ecp_mont.c
lib/libcrypto/ec/ecp_nist.c
lib/libcrypto/ec/ecp_smpl.c

index 8cafc55..8ba62c8 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ec.h,v 1.30 2022/12/26 07:18:51 jmc Exp $ */
+/* $OpenBSD: ec.h,v 1.31 2023/03/08 05:45:31 jsing Exp $ */
 /*
  * Originally written by Bodo Moeller for the OpenSSL project.
  */
@@ -198,7 +198,9 @@ void EC_GROUP_free(EC_GROUP *group);
 /** Clears and frees a EC_GROUP object
  *  \param  group  EC_GROUP object to be cleared and freed.
  */
+#ifndef LIBRESSL_INTERNAL
 void EC_GROUP_clear_free(EC_GROUP *group);
+#endif
 
 /** Copies EC_GROUP objects. Note: both EC_GROUPs must use the same EC_METHOD.
  *  \param  dst  destination EC_GROUP object
@@ -425,7 +427,9 @@ void EC_POINT_free(EC_POINT *point);
 /** Clears and frees a EC_POINT object
  *  \param  point  EC_POINT object to be cleared and freed
  */
+#ifndef LIBRESSL_INTERNAL
 void EC_POINT_clear_free(EC_POINT *point);
+#endif
 
 /** Copies EC_POINT object
  *  \param  dst  destination EC_POINT object
index f995ff8..84cba1b 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ec2_smpl.c,v 1.32 2023/03/08 04:50:27 jsing Exp $ */
+/* $OpenBSD: ec2_smpl.c,v 1.33 2023/03/08 05:45:31 jsing Exp $ */
 /* ====================================================================
  * Copyright 2002 Sun Microsystems, Inc. ALL RIGHTS RESERVED.
  *
@@ -89,23 +89,11 @@ ec_GF2m_simple_group_init(EC_GROUP *group)
 }
 
 /*
- * Free a GF(2^m)-based EC_GROUP structure.
+ * Clear and free a GF(2^m)-based EC_GROUP structure.
  * Note that all other members are handled by EC_GROUP_free.
  */
 static void
 ec_GF2m_simple_group_finish(EC_GROUP *group)
-{
-       BN_free(&group->field);
-       BN_free(&group->a);
-       BN_free(&group->b);
-}
-
-/*
- * Clear and free a GF(2^m)-based EC_GROUP structure.
- * Note that all other members are handled by EC_GROUP_clear_free.
- */
-static void
-ec_GF2m_simple_group_clear_finish(EC_GROUP *group)
 {
        BN_free(&group->field);
        BN_free(&group->a);
@@ -272,18 +260,9 @@ ec_GF2m_simple_point_init(EC_POINT *point)
        return 1;
 }
 
-/* Frees an EC_POINT. */
-static void
-ec_GF2m_simple_point_finish(EC_POINT *point)
-{
-       BN_free(&point->X);
-       BN_free(&point->Y);
-       BN_free(&point->Z);
-}
-
 /* Clears and frees an EC_POINT. */
 static void
-ec_GF2m_simple_point_clear_finish(EC_POINT *point)
+ec_GF2m_simple_point_finish(EC_POINT *point)
 {
        BN_free(&point->X);
        BN_free(&point->Y);
@@ -727,7 +706,6 @@ static const EC_METHOD ec_GF2m_simple_method = {
        .field_type = NID_X9_62_characteristic_two_field,
        .group_init = ec_GF2m_simple_group_init,
        .group_finish = ec_GF2m_simple_group_finish,
-       .group_clear_finish = ec_GF2m_simple_group_clear_finish,
        .group_copy = ec_GF2m_simple_group_copy,
        .group_set_curve = ec_GF2m_simple_group_set_curve,
        .group_get_curve = ec_GF2m_simple_group_get_curve,
@@ -736,7 +714,6 @@ static const EC_METHOD ec_GF2m_simple_method = {
        .group_check_discriminant = ec_GF2m_simple_group_check_discriminant,
        .point_init = ec_GF2m_simple_point_init,
        .point_finish = ec_GF2m_simple_point_finish,
-       .point_clear_finish = ec_GF2m_simple_point_clear_finish,
        .point_copy = ec_GF2m_simple_point_copy,
        .point_set_to_infinity = ec_GF2m_simple_point_set_to_infinity,
        .point_set_affine_coordinates =
index 2d7a1d4..fb6a8e8 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ec_asn1.c,v 1.40 2022/11/26 16:08:52 tb Exp $ */
+/* $OpenBSD: ec_asn1.c,v 1.41 2023/03/08 05:45:31 jsing Exp $ */
 /*
  * Written by Nils Larsch for the OpenSSL project.
  */
@@ -1236,7 +1236,7 @@ ec_asn1_parameters2group(const ECPARAMETERS *params)
 
  err:
        if (!ok) {
-               EC_GROUP_clear_free(ret);
+               EC_GROUP_free(ret);
                ret = NULL;
        }
        BN_free(p);
@@ -1299,7 +1299,7 @@ d2i_ECPKParameters(EC_GROUP ** a, const unsigned char **in, long len)
        }
 
        if (a != NULL) {
-               EC_GROUP_clear_free(*a);
+               EC_GROUP_free(*a);
                *a = group;
        }
 
@@ -1347,7 +1347,7 @@ d2i_ECPrivateKey(EC_KEY ** a, const unsigned char **in, long len)
                ret = *a;
 
        if (priv_key->parameters) {
-               EC_GROUP_clear_free(ret->group);
+               EC_GROUP_free(ret->group);
                ret->group = ec_asn1_pkparameters2group(priv_key->parameters);
        }
        if (ret->group == NULL) {
@@ -1371,7 +1371,7 @@ d2i_ECPrivateKey(EC_KEY ** a, const unsigned char **in, long len)
        }
 
        if (ret->pub_key)
-               EC_POINT_clear_free(ret->pub_key);
+               EC_POINT_free(ret->pub_key);
        ret->pub_key = EC_POINT_new(ret->group);
        if (ret->pub_key == NULL) {
                ECerror(ERR_R_EC_LIB);
index 0e863dd..8eb0253 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ec_lib.c,v 1.49 2023/03/07 09:27:10 jsing Exp $ */
+/* $OpenBSD: ec_lib.c,v 1.50 2023/03/08 05:45:31 jsing Exp $ */
 /*
  * Originally written by Bodo Moeller for the OpenSSL project.
  */
@@ -117,46 +117,28 @@ EC_GROUP_new(const EC_METHOD *meth)
 void
 EC_GROUP_free(EC_GROUP *group)
 {
-       if (!group)
+       if (group != NULL)
                return;
 
-       if (group->meth->group_finish != 0)
+       if (group->meth->group_finish != NULL)
                group->meth->group_finish(group);
 
-       EC_EX_DATA_free_all_data(&group->extra_data);
+       EC_EX_DATA_clear_free_all_data(&group->extra_data);
 
        EC_POINT_free(group->generator);
        BN_free(&group->order);
        BN_free(&group->cofactor);
 
-       free(group->seed);
-
-       free(group);
+       freezero(group->seed, group->seed_len);
+       freezero(group, sizeof *group);
 }
 
-
 void
 EC_GROUP_clear_free(EC_GROUP *group)
 {
-       if (!group)
-               return;
-
-       if (group->meth->group_clear_finish != 0)
-               group->meth->group_clear_finish(group);
-       else if (group->meth->group_finish != 0)
-               group->meth->group_finish(group);
-
-       EC_EX_DATA_clear_free_all_data(&group->extra_data);
-
-       EC_POINT_clear_free(group->generator);
-       BN_free(&group->order);
-       BN_free(&group->cofactor);
-
-       freezero(group->seed, group->seed_len);
-       freezero(group, sizeof *group);
+       return EC_GROUP_free(group);
 }
 
-
 int
 EC_GROUP_copy(EC_GROUP *dest, const EC_GROUP *src)
 {
@@ -195,7 +177,7 @@ EC_GROUP_copy(EC_GROUP *dest, const EC_GROUP *src)
                        return 0;
        } else {
                /* src->generator == NULL */
-               EC_POINT_clear_free(dest->generator);
+               EC_POINT_free(dest->generator);
                dest->generator = NULL;
        }
 
@@ -851,33 +833,24 @@ EC_POINT_new(const EC_GROUP *group)
        return ret;
 }
 
-
 void
 EC_POINT_free(EC_POINT *point)
 {
-       if (!point)
+       if (point != NULL)
                return;
 
-       if (point->meth->point_finish != 0)
+       if (point->meth->point_finish != NULL)
                point->meth->point_finish(point);
-       free(point);
-}
 
+       freezero(point, sizeof *point);
+}
 
 void
 EC_POINT_clear_free(EC_POINT *point)
 {
-       if (!point)
-               return;
-
-       if (point->meth->point_clear_finish != 0)
-               point->meth->point_clear_finish(point);
-       else if (point->meth->point_finish != 0)
-               point->meth->point_finish(point);
-       freezero(point, sizeof *point);
+       return EC_POINT_free(point);
 }
 
-
 int
 EC_POINT_copy(EC_POINT *dest, const EC_POINT *src)
 {
index d4cb777..d26ec47 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ec_local.h,v 1.10 2023/03/08 04:50:27 jsing Exp $ */
+/* $OpenBSD: ec_local.h,v 1.11 2023/03/08 05:45:31 jsing Exp $ */
 /*
  * Originally written by Bodo Moeller for the OpenSSL project.
  */
@@ -91,7 +91,6 @@ struct ec_method_st {
 
        int (*group_init)(EC_GROUP *);
        void (*group_finish)(EC_GROUP *);
-       void (*group_clear_finish)(EC_GROUP *);
        int (*group_copy)(EC_GROUP *, const EC_GROUP *);
 
        int (*group_set_curve)(EC_GROUP *, const BIGNUM *p, const BIGNUM *a,
@@ -105,7 +104,6 @@ struct ec_method_st {
 
        int (*point_init)(EC_POINT *);
        void (*point_finish)(EC_POINT *);
-       void (*point_clear_finish)(EC_POINT *);
        int (*point_copy)(EC_POINT *, const EC_POINT *);
 
        int (*point_set_to_infinity)(const EC_GROUP *, EC_POINT *);
@@ -317,7 +315,6 @@ int ec_wNAF_have_precompute_mult(const EC_GROUP *group);
 /* method functions in ecp_smpl.c */
 int ec_GFp_simple_group_init(EC_GROUP *);
 void ec_GFp_simple_group_finish(EC_GROUP *);
-void ec_GFp_simple_group_clear_finish(EC_GROUP *);
 int ec_GFp_simple_group_copy(EC_GROUP *, const EC_GROUP *);
 int ec_GFp_simple_group_set_curve(EC_GROUP *, const BIGNUM *p, const BIGNUM *a, const BIGNUM *b, BN_CTX *);
 int ec_GFp_simple_group_get_curve(const EC_GROUP *, BIGNUM *p, BIGNUM *a, BIGNUM *b, BN_CTX *);
@@ -325,7 +322,6 @@ int ec_GFp_simple_group_get_degree(const EC_GROUP *);
 int ec_GFp_simple_group_check_discriminant(const EC_GROUP *, BN_CTX *);
 int ec_GFp_simple_point_init(EC_POINT *);
 void ec_GFp_simple_point_finish(EC_POINT *);
-void ec_GFp_simple_point_clear_finish(EC_POINT *);
 int ec_GFp_simple_point_copy(EC_POINT *, const EC_POINT *);
 int ec_GFp_simple_point_set_to_infinity(const EC_GROUP *, EC_POINT *);
 int ec_GFp_simple_set_Jprojective_coordinates(const EC_GROUP *, EC_POINT *,
index 4b50184..c792725 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ec_mult.c,v 1.27 2022/11/26 16:08:52 tb Exp $ */
+/* $OpenBSD: ec_mult.c,v 1.28 2023/03/08 05:45:31 jsing Exp $ */
 /*
  * Originally written by Bodo Moeller and Nils Larsch for the OpenSSL project.
  */
@@ -172,7 +172,7 @@ ec_pre_comp_clear_free(void *pre_)
                EC_POINT **p;
 
                for (p = pre->points; *p != NULL; p++) {
-                       EC_POINT_clear_free(*p);
+                       EC_POINT_free(*p);
                        explicit_bzero(p, sizeof *p);
                }
                free(pre->points);
@@ -694,7 +694,7 @@ ec_wNAF_mul(const EC_GROUP *group, EC_POINT *r, const BIGNUM *scalar,
        }
        if (val != NULL) {
                for (v = val; *v != NULL; v++)
-                       EC_POINT_clear_free(*v);
+                       EC_POINT_free(*v);
                free(val);
        }
        free(val_sub);
index faa212f..2aa0aa6 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ec_print.c,v 1.10 2023/03/07 09:27:10 jsing Exp $ */
+/* $OpenBSD: ec_print.c,v 1.11 2023/03/08 05:45:31 jsing Exp $ */
 /* ====================================================================
  * Copyright (c) 1998-2002 The OpenSSL Project.  All rights reserved.
  *
@@ -110,7 +110,7 @@ EC_POINT_bn2point(const EC_GROUP *group,
 
        if (!EC_POINT_oct2point(group, ret, buf, buf_len, ctx)) {
                if (point == NULL)
-                       EC_POINT_clear_free(ret);
+                       EC_POINT_free(ret);
                free(buf);
                return NULL;
        }
index d0d497b..8b85bf3 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ecp_mont.c,v 1.26 2023/03/08 04:50:27 jsing Exp $ */
+/* $OpenBSD: ecp_mont.c,v 1.27 2023/03/08 05:45:31 jsing Exp $ */
 /*
  * Originally written by Bodo Moeller for the OpenSSL project.
  */
@@ -93,13 +93,6 @@ ec_GFp_mont_group_finish(EC_GROUP *group)
        ec_GFp_simple_group_finish(group);
 }
 
-static void
-ec_GFp_mont_group_clear_finish(EC_GROUP *group)
-{
-       ec_GFp_mont_group_clear(group);
-       ec_GFp_simple_group_clear_finish(group);
-}
-
 static int
 ec_GFp_mont_group_copy(EC_GROUP *dest, const EC_GROUP *src)
 {
@@ -236,7 +229,6 @@ static const EC_METHOD ec_GFp_mont_method = {
        .field_type = NID_X9_62_prime_field,
        .group_init = ec_GFp_mont_group_init,
        .group_finish = ec_GFp_mont_group_finish,
-       .group_clear_finish = ec_GFp_mont_group_clear_finish,
        .group_copy = ec_GFp_mont_group_copy,
        .group_set_curve = ec_GFp_mont_group_set_curve,
        .group_get_curve = ec_GFp_simple_group_get_curve,
@@ -245,7 +237,6 @@ static const EC_METHOD ec_GFp_mont_method = {
        .group_check_discriminant = ec_GFp_simple_group_check_discriminant,
        .point_init = ec_GFp_simple_point_init,
        .point_finish = ec_GFp_simple_point_finish,
-       .point_clear_finish = ec_GFp_simple_point_clear_finish,
        .point_copy = ec_GFp_simple_point_copy,
        .point_set_to_infinity = ec_GFp_simple_point_set_to_infinity,
        .point_set_Jprojective_coordinates =
index e3c13f7..b8fb5dc 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ecp_nist.c,v 1.23 2023/03/08 04:50:27 jsing Exp $ */
+/* $OpenBSD: ecp_nist.c,v 1.24 2023/03/08 05:45:31 jsing Exp $ */
 /*
  * Written by Nils Larsch for the OpenSSL project.
  */
@@ -171,7 +171,6 @@ static const EC_METHOD ec_GFp_nist_method = {
        .field_type = NID_X9_62_prime_field,
        .group_init = ec_GFp_simple_group_init,
        .group_finish = ec_GFp_simple_group_finish,
-       .group_clear_finish = ec_GFp_simple_group_clear_finish,
        .group_copy = ec_GFp_nist_group_copy,
        .group_set_curve = ec_GFp_nist_group_set_curve,
        .group_get_curve = ec_GFp_simple_group_get_curve,
@@ -180,7 +179,6 @@ static const EC_METHOD ec_GFp_nist_method = {
        .group_check_discriminant = ec_GFp_simple_group_check_discriminant,
        .point_init = ec_GFp_simple_point_init,
        .point_finish = ec_GFp_simple_point_finish,
-       .point_clear_finish = ec_GFp_simple_point_clear_finish,
        .point_copy = ec_GFp_simple_point_copy,
        .point_set_to_infinity = ec_GFp_simple_point_set_to_infinity,
        .point_set_Jprojective_coordinates =
index c33347a..9af6034 100644 (file)
@@ -1,4 +1,4 @@
-/* $OpenBSD: ecp_smpl.c,v 1.41 2023/03/08 04:50:27 jsing Exp $ */
+/* $OpenBSD: ecp_smpl.c,v 1.42 2023/03/08 05:45:31 jsing Exp $ */
 /* Includes code written by Lenka Fibikova <fibikova@exp-math.uni-essen.de>
  * for the OpenSSL project.
  * Includes code written by Bodo Moeller for the OpenSSL project.
@@ -99,14 +99,6 @@ ec_GFp_simple_group_finish(EC_GROUP *group)
        BN_free(&group->b);
 }
 
-void
-ec_GFp_simple_group_clear_finish(EC_GROUP *group)
-{
-       BN_free(&group->field);
-       BN_free(&group->a);
-       BN_free(&group->b);
-}
-
 int
 ec_GFp_simple_group_copy(EC_GROUP *dest, const EC_GROUP *src)
 {
@@ -315,14 +307,6 @@ ec_GFp_simple_point_init(EC_POINT * point)
 
 void
 ec_GFp_simple_point_finish(EC_POINT *point)
-{
-       BN_free(&point->X);
-       BN_free(&point->Y);
-       BN_free(&point->Z);
-}
-
-void
-ec_GFp_simple_point_clear_finish(EC_POINT *point)
 {
        BN_free(&point->X);
        BN_free(&point->Y);
@@ -1657,7 +1641,6 @@ static const EC_METHOD ec_GFp_simple_method = {
        .field_type = NID_X9_62_prime_field,
        .group_init = ec_GFp_simple_group_init,
        .group_finish = ec_GFp_simple_group_finish,
-       .group_clear_finish = ec_GFp_simple_group_clear_finish,
        .group_copy = ec_GFp_simple_group_copy,
        .group_set_curve = ec_GFp_simple_group_set_curve,
        .group_get_curve = ec_GFp_simple_group_get_curve,
@@ -1666,7 +1649,6 @@ static const EC_METHOD ec_GFp_simple_method = {
        .group_check_discriminant = ec_GFp_simple_group_check_discriminant,
        .point_init = ec_GFp_simple_point_init,
        .point_finish = ec_GFp_simple_point_finish,
-       .point_clear_finish = ec_GFp_simple_point_clear_finish,
        .point_copy = ec_GFp_simple_point_copy,
        .point_set_to_infinity = ec_GFp_simple_point_set_to_infinity,
        .point_set_Jprojective_coordinates =