From: tb <tb@openbsd.org>
Date: Thu, 28 Dec 2023 21:56:12 +0000 (+0000)
Subject: Clean up and fix pkey_cmac_keygen()
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=d6237f54e72b691997986b4b8b4e3cda78a61464;p=openbsd

Clean up and fix pkey_cmac_keygen()

A void pointer can be passed without any cast or assigning it to an
intermediate variable. That's one of hte puzzling things in old OpenSSL
code: there are plenty of unnecessary casts and assignments of void
pointers.

Make use of this fact and rework the function to be single exit, error
check consistently, including the EVP_PKEY_assign() call that can't
really fail and free the cmkey on exit.

Why coverity didn't flag this one is another mystery.

ok jsing
---

diff --git a/lib/libcrypto/cmac/cm_pmeth.c b/lib/libcrypto/cmac/cm_pmeth.c
index fa2d53e53de..03538e204eb 100644
--- a/lib/libcrypto/cmac/cm_pmeth.c
+++ b/lib/libcrypto/cmac/cm_pmeth.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: cm_pmeth.c,v 1.11 2023/11/29 21:35:57 tb Exp $ */
+/* $OpenBSD: cm_pmeth.c,v 1.12 2023/12/28 21:56:12 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2010.
  */
@@ -92,18 +92,23 @@ pkey_cmac_cleanup(EVP_PKEY_CTX *ctx)
 static int
 pkey_cmac_keygen(EVP_PKEY_CTX *ctx, EVP_PKEY *pkey)
 {
-	CMAC_CTX *cmkey = CMAC_CTX_new();
-	CMAC_CTX *cmctx = ctx->data;
+	CMAC_CTX *cmkey;
+	int ret = 0;
 
-	if (!cmkey)
-		return 0;
-	if (!CMAC_CTX_copy(cmkey, cmctx)) {
-		CMAC_CTX_free(cmkey);
-		return 0;
-	}
-	EVP_PKEY_assign(pkey, EVP_PKEY_CMAC, cmkey);
+	if ((cmkey = CMAC_CTX_new()) == NULL)
+		goto err;
+	if (!CMAC_CTX_copy(cmkey, ctx->data))
+		goto err;
+	if (!EVP_PKEY_assign(pkey, EVP_PKEY_CMAC, cmkey))
+		goto err;
+	cmkey = NULL;
 
-	return 1;
+	ret = 1;
+
+ err:
+	CMAC_CTX_free(cmkey);
+
+	return ret;
 }
 
 static int