From ab8953b7478f72a2b80497cf54ce0b72ed719cfc Mon Sep 17 00:00:00 2001
From: Justin W Smith <103147162+justsmth@users.noreply.github.com>
Date: Mon, 18 Nov 2024 13:24:01 -0500
Subject: [PATCH] Minor improvement to DSA (ASN1) + DSA Tests (#1990)

### Issues:
Addresses: P169046023

### Description of changes:
* Improve error handling of some DSA (ASN1) functions.
* Improve handling of null returns in tests.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.
---
 crypto/evp_extra/evp_extra_test.cc | 20 +++++++++++++-------
 crypto/evp_extra/p_dsa_asn1.c      | 12 +++++++-----
 2 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/crypto/evp_extra/evp_extra_test.cc b/crypto/evp_extra/evp_extra_test.cc
index 0b948980bc..1ddf4698f9 100644
--- a/crypto/evp_extra/evp_extra_test.cc
+++ b/crypto/evp_extra/evp_extra_test.cc
@@ -3059,11 +3059,14 @@ TEST_P(PerParamgenCBTest, ParamgenCallbacks) {
 
 
   // Generating an DH params will trigger the callback.
-  EVP_PKEY *pkey = EVP_PKEY_new();
   ASSERT_EQ(EVP_PKEY_paramgen_init(ctx.get()), 1);
   ASSERT_TRUE(EVP_PKEY_CTX_ctrl_str(ctx.get(), GetParam().setup_command, GetParam().setup_arg));
-  ASSERT_TRUE(EVP_PKEY_paramgen(ctx.get(), &pkey));
-  ASSERT_TRUE(pkey);
+  {
+    EVP_PKEY *pkey = EVP_PKEY_new();
+    ASSERT_TRUE(pkey);
+    ASSERT_TRUE(EVP_PKEY_paramgen(ctx.get(), &pkey));
+    EVP_PKEY_free(pkey);
+  }
 
   // Verify that |ctx->keygen_info| has not been updated since a callback hasn't
   // been set.
@@ -3078,9 +3081,12 @@ TEST_P(PerParamgenCBTest, ParamgenCallbacks) {
   EXPECT_FALSE(app_data.state);
 
   // Call key generation again to trigger the callback.
-  ASSERT_TRUE(EVP_PKEY_paramgen(ctx.get(), &pkey));
-  ASSERT_TRUE(pkey);
-  bssl::UniquePtr<EVP_PKEY> ptr(pkey);
+  {
+    EVP_PKEY *pkey = EVP_PKEY_new();
+    ASSERT_TRUE(pkey);
+    ASSERT_TRUE(EVP_PKEY_paramgen(ctx.get(), &pkey));
+    EVP_PKEY_free(pkey);
+  }
 
   // The callback function should set the state to true. The contents of
   // |ctx->keygen_info| will only be populated once the callback has been set.
@@ -3158,7 +3164,7 @@ TEST(EVPExtraTest, DSAKeygen) {
     bssl::UniquePtr<EVP_PKEY> params = dsa_paramgen(512, EVP_sha1(), copy);
     ASSERT_TRUE(params);
     const DSA* params_dsa = EVP_PKEY_get0_DSA(params.get());
-
+    ASSERT_TRUE(params_dsa);
     bssl::UniquePtr<EVP_PKEY> pkey1 = dsa_keygen(params, copy);
     ASSERT_TRUE(pkey1);
 
diff --git a/crypto/evp_extra/p_dsa_asn1.c b/crypto/evp_extra/p_dsa_asn1.c
index 4369a484b6..470d3bd04e 100644
--- a/crypto/evp_extra/p_dsa_asn1.c
+++ b/crypto/evp_extra/p_dsa_asn1.c
@@ -95,8 +95,9 @@ static int dsa_pub_decode(EVP_PKEY *out, CBS *params, CBS *key) {
     goto err;
   }
 
-  EVP_PKEY_assign_DSA(out, dsa);
-  return 1;
+  if(1 == EVP_PKEY_assign_DSA(out, dsa)) {
+    return 1;
+  }
 
 err:
   DSA_free(dsa);
@@ -168,9 +169,10 @@ static int dsa_priv_decode(EVP_PKEY *out, CBS *params, CBS *key, CBS *pubkey) {
     goto err;
   }
 
-  BN_CTX_free(ctx);
-  EVP_PKEY_assign_DSA(out, dsa);
-  return 1;
+  if(1 == EVP_PKEY_assign_DSA(out, dsa)) {
+    BN_CTX_free(ctx);
+    return 1;
+  }
 
 err:
   BN_CTX_free(ctx);