Skip to content

Commit

Permalink
Minor improvement to DSA (ASN1) + DSA Tests (#1990)
Browse files Browse the repository at this point in the history
### 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.
  • Loading branch information
justsmth authored Nov 18, 2024
1 parent 318413c commit ab8953b
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 12 deletions.
20 changes: 13 additions & 7 deletions crypto/evp_extra/evp_extra_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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);

Expand Down
12 changes: 7 additions & 5 deletions crypto/evp_extra/p_dsa_asn1.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit ab8953b

Please sign in to comment.