From 0e358455be3dcd6dd2275d731e48c13182450873 Mon Sep 17 00:00:00 2001
From: tb <tb@openbsd.org>
Date: Thu, 10 Aug 2023 15:11:16 +0000
Subject: [PATCH] Clean up {dh,dsa}_pub_encode()

This brings these two messy functions into more usual shape. There is a
lot more that can be done in here. It is a step in the right direction.

ok jsing
---
 lib/libcrypto/dh/dh_ameth.c   | 55 ++++++++++++++++++-----------------
 lib/libcrypto/dsa/dsa_ameth.c | 45 ++++++++++++++++------------
 2 files changed, 54 insertions(+), 46 deletions(-)

diff --git a/lib/libcrypto/dh/dh_ameth.c b/lib/libcrypto/dh/dh_ameth.c
index 12f2db7b8eb..3898db89f05 100644
--- a/lib/libcrypto/dh/dh_ameth.c
+++ b/lib/libcrypto/dh/dh_ameth.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: dh_ameth.c,v 1.31 2023/08/10 09:43:51 tb Exp $ */
+/* $OpenBSD: dh_ameth.c,v 1.32 2023/08/10 15:11:16 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2006.
  */
@@ -130,48 +130,49 @@ err:
 static int
 dh_pub_encode(X509_PUBKEY *pk, const EVP_PKEY *pkey)
 {
-	DH *dh;
-	int ptype;
-	unsigned char *penc = NULL;
-	int penclen;
-	ASN1_STRING *str;
+	const DH *dh = pkey->pkey.dh;
+	ASN1_STRING *str = NULL;
+	int ptype = V_ASN1_SEQUENCE;
 	ASN1_INTEGER *pub_key = NULL;
+	ASN1_OBJECT *aobj;
+	unsigned char *data = NULL, *penc = NULL;
+	int datalen = 0, penclen = 0;
 
-	dh=pkey->pkey.dh;
-
-	str = ASN1_STRING_new();
-	if (str == NULL) {
+	if ((datalen = i2d_DHparams(dh, &data)) <= 0) {
 		DHerror(ERR_R_MALLOC_FAILURE);
+		datalen = 0;
 		goto err;
 	}
-
-	str->length = i2d_DHparams(dh, &str->data);
-	if (str->length <= 0) {
+	if ((str = ASN1_STRING_new()) == NULL) {
 		DHerror(ERR_R_MALLOC_FAILURE);
 		goto err;
 	}
-	ptype = V_ASN1_SEQUENCE;
+	ASN1_STRING_set0(str, data, datalen);
+	data = NULL;
+	datalen = 0;
 
-	pub_key = BN_to_ASN1_INTEGER(dh->pub_key, NULL);
-	if (!pub_key)
+	if ((pub_key = BN_to_ASN1_INTEGER(dh->pub_key, NULL)) == NULL)
 		goto err;
-
-	penclen = i2d_ASN1_INTEGER(pub_key, &penc);
-
+	if ((penclen = i2d_ASN1_INTEGER(pub_key, &penc)) <= 0) {
+		DHerror(ERR_R_MALLOC_FAILURE);
+		penclen = 0;
+		goto err;
+	}
 	ASN1_INTEGER_free(pub_key);
+	pub_key = NULL;
 
-	if (penclen <= 0) {
-		DHerror(ERR_R_MALLOC_FAILURE);
+	if ((aobj = OBJ_nid2obj(EVP_PKEY_DH)) == NULL)
+		goto err;
+	if (!X509_PUBKEY_set0_param(pk, aobj, ptype, str, penc, penclen))
 		goto err;
-		}
 
-	if (X509_PUBKEY_set0_param(pk, OBJ_nid2obj(EVP_PKEY_DH), ptype,
-	    (void *)str, penc, penclen))
-		return 1;
+	return 1;
 
-err:
-	free(penc);
+ err:
 	ASN1_STRING_free(str);
+	ASN1_INTEGER_free(pub_key);
+	freezero(data, datalen);
+	freezero(penc, penclen);
 
 	return 0;
 }
diff --git a/lib/libcrypto/dsa/dsa_ameth.c b/lib/libcrypto/dsa/dsa_ameth.c
index ad5aa09cd0c..aab4588b5a0 100644
--- a/lib/libcrypto/dsa/dsa_ameth.c
+++ b/lib/libcrypto/dsa/dsa_ameth.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: dsa_ameth.c,v 1.44 2023/08/10 09:43:51 tb Exp $ */
+/* $OpenBSD: dsa_ameth.c,v 1.45 2023/08/10 15:11:16 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2006.
  */
@@ -138,47 +138,54 @@ err:
 static int
 dsa_pub_encode(X509_PUBKEY *pk, const EVP_PKEY *pkey)
 {
-	DSA *dsa;
-	ASN1_INTEGER *pubint = NULL;
+	const DSA *dsa = pkey->pkey.dsa;
 	ASN1_STRING *str = NULL;
 	int ptype = V_ASN1_UNDEF;
-	unsigned char *penc = NULL;
-	int penclen;
+	ASN1_INTEGER *pub_key = NULL;
+	ASN1_OBJECT *aobj;
+	unsigned char *data = NULL, *penc = NULL;
+	int datalen = 0, penclen = 0;
 
-	dsa = pkey->pkey.dsa;
 	if (pkey->save_parameters && dsa->p && dsa->q && dsa->g) {
-		if ((str = ASN1_STRING_new()) == NULL) {
+		if ((datalen = i2d_DSAparams(dsa, &data)) <= 0) {
 			DSAerror(ERR_R_MALLOC_FAILURE);
+			datalen = 0;
 			goto err;
 		}
-		str->length = i2d_DSAparams(dsa, &str->data);
-		if (str->length <= 0) {
+		if ((str = ASN1_STRING_new()) == NULL) {
 			DSAerror(ERR_R_MALLOC_FAILURE);
 			goto err;
 		}
+		ASN1_STRING_set0(str, data, datalen);
+		data = NULL;
+		datalen = 0;
 		ptype = V_ASN1_SEQUENCE;
 	}
 
-	if ((pubint = BN_to_ASN1_INTEGER(dsa->pub_key, NULL)) == NULL) {
+	if ((pub_key = BN_to_ASN1_INTEGER(dsa->pub_key, NULL)) == NULL) {
 		DSAerror(ERR_R_MALLOC_FAILURE);
 		goto err;
 	}
-
-	penclen = i2d_ASN1_INTEGER(pubint, &penc);
-	ASN1_INTEGER_free(pubint);
-
-	if (penclen <= 0) {
+	if ((penclen = i2d_ASN1_INTEGER(pub_key, &penc)) <= 0) {
 		DSAerror(ERR_R_MALLOC_FAILURE);
+		penclen = 0;
 		goto err;
 	}
+	ASN1_INTEGER_free(pub_key);
+	pub_key = NULL;
 
-	if (X509_PUBKEY_set0_param(pk, OBJ_nid2obj(EVP_PKEY_DSA), ptype, str,
-	    penc, penclen))
-		return 1;
+	if ((aobj = OBJ_nid2obj(EVP_PKEY_DSA)) == NULL)
+		goto err;
+	if (!X509_PUBKEY_set0_param(pk, aobj, ptype, str, penc, penclen))
+		goto err;
+
+	return 1;
 
  err:
-	free(penc);
 	ASN1_STRING_free(str);
+	ASN1_INTEGER_free(pub_key);
+	freezero(data, datalen);
+	freezero(penc, penclen);
 
 	return 0;
 }
-- 
2.20.1