From 8060a2fa82dd2d377465b06fab8daf44cc5b4e1d Mon Sep 17 00:00:00 2001 From: Colton Willey Date: Thu, 5 Jun 2025 11:29:07 -0700 Subject: [PATCH 1/2] Fix NULL reinit handling for signatures --- src/wp_ecdsa_sig.c | 13 +++++++++--- src/wp_ecx_sig.c | 12 ++++++++--- src/wp_rsa_sig.c | 9 ++++---- test/test_ecc.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++ test/test_ecx.c | 49 +++++++++++++++++++++++++++++++++++++++++++ test/test_rsa.c | 44 +++++++++++++++++++++++++++++++++++++++ test/unit.c | 3 +++ test/unit.h | 3 +++ 8 files changed, 174 insertions(+), 11 deletions(-) diff --git a/src/wp_ecdsa_sig.c b/src/wp_ecdsa_sig.c index 75752524..8fb72cbb 100644 --- a/src/wp_ecdsa_sig.c +++ b/src/wp_ecdsa_sig.c @@ -185,14 +185,21 @@ 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 (ecc == NULL && ctx->ecc == NULL) { + ok = 0; + } + + if (ok && 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)) { diff --git a/src/wp_ecx_sig.c b/src/wp_ecx_sig.c index 76ddc0c9..c784f411 100644 --- a/src/wp_ecx_sig.c +++ b/src/wp_ecx_sig.c @@ -188,14 +188,20 @@ static int wp_ecx_digest_signverify_init(wp_EcxSigCtx *ctx, ok = 0; } - if (ok && (ctx->ecx != ecx)) { - wp_ecx_free(ctx->ecx); + if (ctx->ecx == NULL && ecx == NULL) { + ok = 0; + } + + 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; } diff --git a/src/wp_rsa_sig.c b/src/wp_rsa_sig.c index 23dd96b7..e08c681d 100644 --- a/src/wp_rsa_sig.c +++ b/src/wp_rsa_sig.c @@ -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; + if (ok && (rsa != NULL)) { if (!wp_rsa_up_ref(rsa)) { ok = 0; } - else { + if (ok) { + wp_rsa_free(ctx->rsa); ctx->rsa = rsa; } } diff --git a/test/test_ecc.c b/test/test_ecc.c index e38e5b5d..9a87b24d 100644 --- a/test/test_ecc.c +++ b/test/test_ecc.c @@ -2005,5 +2005,57 @@ int test_ec_import(void* data) return err; } +static int test_ec_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_EC_P256 + const unsigned char *p = ecc_key_der_256; +#endif + +#ifndef WP_HAVE_EC_P256 + (void)libCtx; + (void)provider_name; + PRINT_MSG("Skipping test - WP_HAVE_EC_P256 not defined"); + return 0; +#endif + + 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)); + 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_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 */ diff --git a/test/test_ecx.c b/test/test_ecx.c index dd70b222..31773652 100644 --- a/test/test_ecx.c +++ b/test/test_ecx.c @@ -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) */ diff --git a/test/test_rsa.c b/test/test_rsa.c index 9c511976..79164d72 100644 --- a/test/test_rsa.c +++ b/test/test_rsa.c @@ -1615,4 +1615,48 @@ int test_rsa_decode(void* data) return err; } +static int test_rsa_null_sign_init_ex(OSSL_LIB_CTX *libCtx) +{ + 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 */ diff --git a/test/unit.c b/test/unit.c index fbd3bee2..bb64fafe 100644 --- a/test/unit.c +++ b/test/unit.c @@ -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 @@ -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 @@ -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)) diff --git a/test/unit.h b/test/unit.h index 1e1aad39..c836ed46 100644 --- a/test/unit.h +++ b/test/unit.h @@ -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 @@ -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 */ @@ -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 */ From c7d8fbc494b5e6513d49a0188805ae69cb3db15a Mon Sep 17 00:00:00 2001 From: Colton Willey Date: Thu, 5 Jun 2025 15:31:57 -0700 Subject: [PATCH 2/2] Cleanup to address review comments --- src/wp_ecdsa_sig.c | 5 ++--- src/wp_ecx_sig.c | 5 ++--- src/wp_rsa_sig.c | 2 +- test/test_ecc.c | 15 ++++++--------- 4 files changed, 11 insertions(+), 16 deletions(-) diff --git a/src/wp_ecdsa_sig.c b/src/wp_ecdsa_sig.c index 8fb72cbb..af56de4a 100644 --- a/src/wp_ecdsa_sig.c +++ b/src/wp_ecdsa_sig.c @@ -185,11 +185,10 @@ static int wp_ecdsa_signverify_init(wp_EcdsaSigCtx *ctx, wp_Ecc* ecc, { int ok = 1; - if (ecc == NULL && ctx->ecc == NULL) { + if (ctx == NULL || (ecc == NULL && ctx->ecc == NULL)) { ok = 0; } - - if (ok && ecc != NULL) { + else if (ecc != NULL) { if (!wp_ecc_up_ref(ecc)) { ok = 0; } diff --git a/src/wp_ecx_sig.c b/src/wp_ecx_sig.c index c784f411..15119532 100644 --- a/src/wp_ecx_sig.c +++ b/src/wp_ecx_sig.c @@ -188,11 +188,10 @@ static int wp_ecx_digest_signverify_init(wp_EcxSigCtx *ctx, ok = 0; } - if (ctx->ecx == NULL && ecx == NULL) { + if (ok && (ctx == NULL || (ctx->ecx == NULL && ecx == NULL))) { ok = 0; } - - if (ok && (ecx != NULL)) { + else if (ok && ecx != NULL) { if (!wp_ecx_up_ref(ecx)) { ok = 0; } diff --git a/src/wp_rsa_sig.c b/src/wp_rsa_sig.c index e08c681d..55d33ec0 100644 --- a/src/wp_rsa_sig.c +++ b/src/wp_rsa_sig.c @@ -458,7 +458,7 @@ static int wp_rsa_signverify_init(wp_RsaSigCtx* ctx, wp_Rsa* rsa, if ((ctx == NULL) || (ctx->rsa == NULL && rsa == NULL)) { ok = 0; } - if (ok && (rsa != NULL)) { + else if (rsa != NULL) { if (!wp_rsa_up_ref(rsa)) { ok = 0; } diff --git a/test/test_ecc.c b/test/test_ecc.c index 9a87b24d..ab67ec9a 100644 --- a/test/test_ecc.c +++ b/test/test_ecc.c @@ -2007,20 +2007,16 @@ int test_ec_import(void* data) 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; -#ifdef WP_HAVE_EC_P256 const unsigned char *p = ecc_key_der_256; -#endif - -#ifndef WP_HAVE_EC_P256 - (void)libCtx; - (void)provider_name; - PRINT_MSG("Skipping test - WP_HAVE_EC_P256 not defined"); - return 0; -#endif err = (ctx = EVP_MD_CTX_new()) == NULL; if (err == 0) { @@ -2043,6 +2039,7 @@ static int test_ec_null_sign_init_ex(OSSL_LIB_CTX *libCtx) EVP_MD_CTX_free(ctx); return err; +#endif } int test_ec_null_init(void* data)