From a7854573db165b31592c3760a6e4c244b25bc76b Mon Sep 17 00:00:00 2001
From: jsing <jsing@openbsd.org>
Date: Wed, 8 Mar 2023 05:45:31 +0000
Subject: [PATCH] Always clear EC groups and points on free.

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       |  6 ++++-
 lib/libcrypto/ec/ec2_smpl.c | 29 +++-----------------
 lib/libcrypto/ec/ec_asn1.c  | 10 +++----
 lib/libcrypto/ec/ec_lib.c   | 53 +++++++++----------------------------
 lib/libcrypto/ec/ec_local.h |  6 +----
 lib/libcrypto/ec/ec_mult.c  |  6 ++---
 lib/libcrypto/ec/ec_print.c |  4 +--
 lib/libcrypto/ec/ecp_mont.c | 11 +-------
 lib/libcrypto/ec/ecp_nist.c |  4 +--
 lib/libcrypto/ec/ecp_smpl.c | 20 +-------------
 10 files changed, 35 insertions(+), 114 deletions(-)

diff --git a/lib/libcrypto/ec/ec.h b/lib/libcrypto/ec/ec.h
index 8cafc5522ff..8ba62c87842 100644
--- a/lib/libcrypto/ec/ec.h
+++ b/lib/libcrypto/ec/ec.h
@@ -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
diff --git a/lib/libcrypto/ec/ec2_smpl.c b/lib/libcrypto/ec/ec2_smpl.c
index f995ff87181..84cba1b83ba 100644
--- a/lib/libcrypto/ec/ec2_smpl.c
+++ b/lib/libcrypto/ec/ec2_smpl.c
@@ -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 =
diff --git a/lib/libcrypto/ec/ec_asn1.c b/lib/libcrypto/ec/ec_asn1.c
index 2d7a1d4c3ba..fb6a8e84c19 100644
--- a/lib/libcrypto/ec/ec_asn1.c
+++ b/lib/libcrypto/ec/ec_asn1.c
@@ -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);
diff --git a/lib/libcrypto/ec/ec_lib.c b/lib/libcrypto/ec/ec_lib.c
index 0e863ddfef5..8eb0253a1fa 100644
--- a/lib/libcrypto/ec/ec_lib.c
+++ b/lib/libcrypto/ec/ec_lib.c
@@ -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)
 {
diff --git a/lib/libcrypto/ec/ec_local.h b/lib/libcrypto/ec/ec_local.h
index d4cb777c837..d26ec47a246 100644
--- a/lib/libcrypto/ec/ec_local.h
+++ b/lib/libcrypto/ec/ec_local.h
@@ -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 *,
diff --git a/lib/libcrypto/ec/ec_mult.c b/lib/libcrypto/ec/ec_mult.c
index 4b50184ff6c..c7927256612 100644
--- a/lib/libcrypto/ec/ec_mult.c
+++ b/lib/libcrypto/ec/ec_mult.c
@@ -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);
diff --git a/lib/libcrypto/ec/ec_print.c b/lib/libcrypto/ec/ec_print.c
index faa212f5021..2aa0aa66a93 100644
--- a/lib/libcrypto/ec/ec_print.c
+++ b/lib/libcrypto/ec/ec_print.c
@@ -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;
 	}
diff --git a/lib/libcrypto/ec/ecp_mont.c b/lib/libcrypto/ec/ecp_mont.c
index d0d497b0114..8b85bf32fa0 100644
--- a/lib/libcrypto/ec/ecp_mont.c
+++ b/lib/libcrypto/ec/ecp_mont.c
@@ -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 =
diff --git a/lib/libcrypto/ec/ecp_nist.c b/lib/libcrypto/ec/ecp_nist.c
index e3c13f7c65f..b8fb5dc90ff 100644
--- a/lib/libcrypto/ec/ecp_nist.c
+++ b/lib/libcrypto/ec/ecp_nist.c
@@ -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 =
diff --git a/lib/libcrypto/ec/ecp_smpl.c b/lib/libcrypto/ec/ecp_smpl.c
index c33347ad857..9af6034601a 100644
--- a/lib/libcrypto/ec/ecp_smpl.c
+++ b/lib/libcrypto/ec/ecp_smpl.c
@@ -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 =
-- 
2.20.1