Skip to content

Fix NULL reinit handling for signatures #154

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
padelsbach merged 2 commits into from
Jun 9, 2025
Merged

Fix NULL reinit handling for signatures #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8060a2f
Fix NULL reinit handling for signatures
ColtonWilley Jun 5, 2025
c7d8fbc
Cleanup to address review comments
ColtonWilley Jun 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/wp_ecdsa_sig.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,14 +185,20 @@ static int wp_ecdsa_signverify_init(wp_EcdsaSigCtx *ctx, wp_Ecc* ecc,
{
int ok = 1;

if (ctx->ecc != ecc) {
wp_ecc_free(ctx->ecc);
if (ctx == NULL || (ecc == NULL && ctx->ecc == NULL)) {
ok = 0;
}
else if (ecc != NULL) {
if (!wp_ecc_up_ref(ecc)) {
ok = 0;
}
if (ok) {
wp_ecc_free(ctx->ecc);
ctx->ecc = ecc;
}
}

if (ok) {
ctx->ecc = ecc;
ctx->op = op;

if (!wp_ecdsa_set_ctx_params(ctx, params)) {
Expand Down
11 changes: 8 additions & 3 deletions src/wp_ecx_sig.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,14 +188,19 @@ static int wp_ecx_digest_signverify_init(wp_EcxSigCtx *ctx,
ok = 0;
}

if (ok && (ctx->ecx != ecx)) {
wp_ecx_free(ctx->ecx);
if (ok && (ctx == NULL || (ctx->ecx == NULL && ecx == NULL))) {
ok = 0;
}
else if (ok && ecx != NULL) {
if (!wp_ecx_up_ref(ecx)) {
ok = 0;
}
if (ok) {
wp_ecx_free(ctx->ecx);
ctx->ecx = ecx;
}
}
if (ok) {
ctx->ecx = ecx;
ctx->op = op;
}

Expand Down
9 changes: 4 additions & 5 deletions src/wp_rsa_sig.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -455,16 +455,15 @@ static int wp_rsa_signverify_init(wp_RsaSigCtx* ctx, wp_Rsa* rsa,
{
int ok = 1;

if ((ctx == NULL) || (rsa == NULL)) {
if ((ctx == NULL) || (ctx->rsa == NULL && rsa == NULL)) {
ok = 0;
}
if (ok && (ctx->rsa != rsa)) {
wp_rsa_free(ctx->rsa);
ctx->rsa = NULL;
else if (rsa != NULL) {
if (!wp_rsa_up_ref(rsa)) {
ok = 0;
}
else {
if (ok) {
wp_rsa_free(ctx->rsa);
ctx->rsa = rsa;
}
}
Expand Down
49 changes: 49 additions & 0 deletions test/test_ecc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2005,5 +2005,54 @@ int test_ec_import(void* data)
return err;
}

static int test_ec_null_sign_init_ex(OSSL_LIB_CTX *libCtx)
{
#ifndef WP_HAVE_EC_P256
(void)libCtx;
PRINT_MSG("Skipping test - WP_HAVE_EC_P256 not defined");
return 0;
#else
int err = 0;
EVP_MD_CTX *ctx = NULL;
EVP_MD *md = NULL;
EVP_PKEY *pkey = NULL;
const unsigned char *p = ecc_key_der_256;

err = (ctx = EVP_MD_CTX_new()) == NULL;
if (err == 0) {
md = EVP_MD_fetch(libCtx, "SHA256", NULL);
err = md == NULL;
}
if (err == 0) {
pkey = d2i_PrivateKey(EVP_PKEY_EC, NULL, &p, sizeof(ecc_key_der_256));
Comment thread
padelsbach marked this conversation as resolved.
err = pkey == NULL;
}
if (err == 0) {
err = EVP_DigestSignInit_ex(ctx, NULL, "SHA256", libCtx, NULL, pkey, NULL) != 1;
}
if (err == 0) {
err = EVP_DigestSignInit_ex(ctx, NULL, "SHA256", libCtx, NULL, NULL, NULL) != 1;
}

EVP_PKEY_free(pkey);
EVP_MD_free(md);
EVP_MD_CTX_free(ctx);

return err;
#endif
}

int test_ec_null_init(void* data)
{
int err = 0;
(void)data;

err = test_ec_null_sign_init_ex(osslLibCtx);
if (err == 0) {
err = test_ec_null_sign_init_ex(wpLibCtx);
}

return err;
}

#endif /* WP_HAVE_ECC */
49 changes: 49 additions & 0 deletions test/test_ecx.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,4 +626,53 @@ int test_ecx_misc(void *data)
return err;
}

static int test_ecx_null_sign_init_ex(OSSL_LIB_CTX *libCtx)
{
int err = 0;
EVP_MD_CTX *ctx = NULL;
EVP_MD *md = NULL;
EVP_PKEY *pkey = NULL;
#ifdef WP_HAVE_ED25519
const unsigned char *p = ed25519_key_der;
#endif

#ifndef WP_HAVE_ED25519
(void)libCtx;
(void)provider_name;
PRINT_MSG("Skipping test - WP_HAVE_ED25519 not defined");
return 0;
#endif

err = (ctx = EVP_MD_CTX_new()) == NULL;
if (err == 0) {
pkey = d2i_PrivateKey(EVP_PKEY_ED25519, NULL, &p, sizeof(ed25519_key_der));
err = pkey == NULL;
}
if (err == 0) {
err = EVP_DigestSignInit_ex(ctx, NULL, NULL, libCtx, NULL, pkey, NULL) != 1;
}
if (err == 0) {
err = EVP_DigestSignInit_ex(ctx, NULL, NULL, libCtx, NULL, NULL, NULL) != 1;
}

EVP_PKEY_free(pkey);
EVP_MD_free(md);
EVP_MD_CTX_free(ctx);

return err;
}

int test_ecx_null_init(void* data)
{
int err = 0;
(void)data;

err = test_ecx_null_sign_init_ex(osslLibCtx);
if (err == 0) {
err = test_ecx_null_sign_init_ex(wpLibCtx);
}

return err;
}

#endif /* defined(WP_HAVE_ED25519) || defined(WP_HAVE_ECD444) */
44 changes: 44 additions & 0 deletions test/test_rsa.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1615,4 +1615,48 @@ int test_rsa_decode(void* data)
return err;
}

static int test_rsa_null_sign_init_ex(OSSL_LIB_CTX *libCtx)
{
Comment thread
padelsbach marked this conversation as resolved.
int err = 0;
EVP_MD_CTX *ctx = NULL;
EVP_MD *md = NULL;
EVP_PKEY *pkey = NULL;
const unsigned char *p = rsa_key_der_2048;

err = (ctx = EVP_MD_CTX_new()) == NULL;
if (err == 0) {
md = EVP_MD_fetch(libCtx, "SHA256", NULL);
err = md == NULL;
}
if (err == 0) {
pkey = d2i_PrivateKey(EVP_PKEY_RSA, NULL, &p, sizeof(rsa_key_der_2048));
err = pkey == NULL;
}
if (err == 0) {
err = EVP_DigestSignInit_ex(ctx, NULL, "SHA256", libCtx, NULL, pkey, NULL) != 1;
}
if (err == 0) {
err = EVP_DigestSignInit_ex(ctx, NULL, "SHA256", libCtx, NULL, NULL, NULL) != 1;
}

EVP_PKEY_free(pkey);
EVP_MD_free(md);
EVP_MD_CTX_free(ctx);

return err;
}

int test_rsa_null_init(void* data)
{
int err = 0;
(void)data;

err = test_rsa_null_sign_init_ex(osslLibCtx);
if (err == 0) {
err = test_rsa_null_sign_init_ex(wpLibCtx);
}

return err;
}

#endif /* WP_HAVE_RSA */
3 changes: 3 additions & 0 deletions test/unit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ TEST_CASE test_case[] = {
TEST_DECL(test_rsa_load_cert, NULL),
TEST_DECL(test_rsa_fromdata, NULL),
TEST_DECL(test_rsa_decode, NULL),
TEST_DECL(test_rsa_null_init, NULL),
#endif /* WP_HAVE_RSA */
#ifdef WP_HAVE_EC_P192
#ifdef WP_HAVE_ECKEYGEN
Expand Down Expand Up @@ -217,6 +218,7 @@ TEST_CASE test_case[] = {
#endif
TEST_DECL(test_ec_decode, NULL),
TEST_DECL(test_ec_import, NULL),
TEST_DECL(test_ec_null_init, NULL),
#endif
#ifdef WP_HAVE_EC_P384
#ifdef WP_HAVE_ECKEYGEN
Expand Down Expand Up @@ -286,6 +288,7 @@ TEST_CASE test_case[] = {
TEST_DECL(test_ecx_sign_verify_raw_priv, NULL),
TEST_DECL(test_ecx_sign_verify_raw_pub, NULL),
TEST_DECL(test_ecx_misc, NULL),
TEST_DECL(test_ecx_null_init, NULL),
#endif
};
#define TEST_CASE_CNT (int)(sizeof(test_case) / sizeof(*test_case))
Expand Down
3 changes: 3 additions & 0 deletions test/unit.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ int test_rsa_load_key(void* data);
int test_rsa_load_cert(void* data);
int test_rsa_fromdata(void* data);
int test_rsa_decode(void* data);
int test_rsa_null_init(void* data);
#endif /* WP_HAVE_RSA */

#ifdef WP_HAVE_DH
Expand Down Expand Up @@ -375,6 +376,7 @@ int test_ec_load_cert(void* data);

int test_ec_decode(void* data);
int test_ec_import(void* data);
int test_ec_null_init(void* data);

#endif /* WP_HAVE_ECC */

Expand All @@ -387,6 +389,7 @@ int test_ecx_sign_verify(void *data);
int test_ecx_sign_verify_raw_priv(void *data);
int test_ecx_sign_verify_raw_pub(void *data);
int test_ecx_misc(void *data);
int test_ecx_null_init(void *data);
#endif

#endif /* UNIT_H */