mirror of https://github.com/openssl/openssl
Fix handling of NULL sig parameter in ECDSA_sign and similar
The problem is, that it almost works to pass sig=NULL to the ECDSA_sign, ECDSA_sign_ex and DSA_sign, to compute the necessary space for the resulting signature. But since the ECDSA signature is non-deterministic (except when ECDSA_sign_setup/ECDSA_sign_ex are used) the resulting length may be different when the API is called again. This can easily cause random memory corruption. Several internal APIs had the same issue, but since they are never called with sig=NULL, it is better to make them return an error in that case, instead of making the code more complex. Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from https://github.com/openssl/openssl/pull/23529)
This commit is contained in:
parent
387418893e
commit
1fa2bf9b18
|
@ -157,6 +157,11 @@ int ossl_dsa_sign_int(int type, const unsigned char *dgst, int dlen,
|
|||
{
|
||||
DSA_SIG *s;
|
||||
|
||||
if (sig == NULL) {
|
||||
*siglen = DSA_size(dsa);
|
||||
return 1;
|
||||
}
|
||||
|
||||
/* legacy case uses the method table */
|
||||
if (dsa->libctx == NULL || dsa->meth != DSA_get_default_method())
|
||||
s = DSA_do_sign(dgst, dlen, dsa);
|
||||
|
@ -167,7 +172,7 @@ int ossl_dsa_sign_int(int type, const unsigned char *dgst, int dlen,
|
|||
*siglen = 0;
|
||||
return 0;
|
||||
}
|
||||
*siglen = i2d_DSA_SIG(s, sig != NULL ? &sig : NULL);
|
||||
*siglen = i2d_DSA_SIG(s, &sig);
|
||||
DSA_SIG_free(s);
|
||||
return 1;
|
||||
}
|
||||
|
|
|
@ -77,6 +77,11 @@ int ossl_ecdsa_sign(int type, const unsigned char *dgst, int dlen,
|
|||
{
|
||||
ECDSA_SIG *s;
|
||||
|
||||
if (sig == NULL && (kinv == NULL || r == NULL)) {
|
||||
*siglen = ECDSA_size(eckey);
|
||||
return 1;
|
||||
}
|
||||
|
||||
s = ECDSA_do_sign_ex(dgst, dlen, kinv, r, eckey);
|
||||
if (s == NULL) {
|
||||
*siglen = 0;
|
||||
|
|
|
@ -450,6 +450,11 @@ int ossl_sm2_internal_sign(const unsigned char *dgst, int dgstlen,
|
|||
int sigleni;
|
||||
int ret = -1;
|
||||
|
||||
if (sig == NULL) {
|
||||
ERR_raise(ERR_LIB_SM2, ERR_R_PASSED_NULL_PARAMETER);
|
||||
goto done;
|
||||
}
|
||||
|
||||
e = BN_bin2bn(dgst, dgstlen, NULL);
|
||||
if (e == NULL) {
|
||||
ERR_raise(ERR_LIB_SM2, ERR_R_BN_LIB);
|
||||
|
@ -462,7 +467,7 @@ int ossl_sm2_internal_sign(const unsigned char *dgst, int dgstlen,
|
|||
goto done;
|
||||
}
|
||||
|
||||
sigleni = i2d_ECDSA_SIG(s, sig != NULL ? &sig : NULL);
|
||||
sigleni = i2d_ECDSA_SIG(s, &sig);
|
||||
if (sigleni < 0) {
|
||||
ERR_raise(ERR_LIB_SM2, ERR_R_INTERNAL_ERROR);
|
||||
goto done;
|
||||
|
|
|
@ -332,6 +332,7 @@ static int test_dsa_sig_infinite_loop(void)
|
|||
BIGNUM *p = NULL, *q = NULL, *g = NULL, *priv = NULL, *pub = NULL, *priv2 = NULL;
|
||||
BIGNUM *badq = NULL, *badpriv = NULL;
|
||||
const unsigned char msg[] = { 0x00 };
|
||||
unsigned int signature_len0;
|
||||
unsigned int signature_len;
|
||||
unsigned char signature[64];
|
||||
|
||||
|
@ -375,10 +376,13 @@ static int test_dsa_sig_infinite_loop(void)
|
|||
goto err;
|
||||
|
||||
/* Test passing signature as NULL */
|
||||
if (!TEST_true(DSA_sign(0, msg, sizeof(msg), NULL, &signature_len, dsa)))
|
||||
if (!TEST_true(DSA_sign(0, msg, sizeof(msg), NULL, &signature_len0, dsa))
|
||||
|| !TEST_int_gt(signature_len0, 0))
|
||||
goto err;
|
||||
|
||||
if (!TEST_true(DSA_sign(0, msg, sizeof(msg), signature, &signature_len, dsa)))
|
||||
if (!TEST_true(DSA_sign(0, msg, sizeof(msg), signature, &signature_len, dsa))
|
||||
|| !TEST_int_gt(signature_len, 0)
|
||||
|| !TEST_int_le(signature_len, signature_len0))
|
||||
goto err;
|
||||
|
||||
/* Test using a private key of zero fails - this causes an infinite loop without the retry test */
|
||||
|
|
|
@ -350,15 +350,39 @@ static int test_builtin_as_sm2(int n)
|
|||
static int test_ecdsa_sig_NULL(void)
|
||||
{
|
||||
int ret;
|
||||
unsigned int siglen0;
|
||||
unsigned int siglen;
|
||||
unsigned char dgst[128] = { 0 };
|
||||
EC_KEY *eckey = NULL;
|
||||
unsigned char *sig = NULL;
|
||||
BIGNUM *kinv = NULL, *rp = NULL;
|
||||
|
||||
ret = TEST_ptr(eckey = EC_KEY_new_by_curve_name(NID_X9_62_prime256v1))
|
||||
&& TEST_int_eq(EC_KEY_generate_key(eckey), 1)
|
||||
&& TEST_int_eq(ECDSA_sign(0, dgst, sizeof(dgst), NULL, &siglen, eckey), 1)
|
||||
&& TEST_int_gt(siglen, 0);
|
||||
&& TEST_int_eq(ECDSA_sign(0, dgst, sizeof(dgst), NULL, &siglen0,
|
||||
eckey), 1)
|
||||
&& TEST_int_gt(siglen0, 0)
|
||||
&& TEST_ptr(sig = OPENSSL_malloc(siglen0))
|
||||
&& TEST_int_eq(ECDSA_sign(0, dgst, sizeof(dgst), sig, &siglen,
|
||||
eckey), 1)
|
||||
&& TEST_int_gt(siglen, 0)
|
||||
&& TEST_int_le(siglen, siglen0)
|
||||
&& TEST_int_eq(ECDSA_verify(0, dgst, sizeof(dgst), sig, siglen,
|
||||
eckey), 1)
|
||||
&& TEST_int_eq(ECDSA_sign_setup(eckey, NULL, &kinv, &rp), 1)
|
||||
&& TEST_int_eq(ECDSA_sign_ex(0, dgst, sizeof(dgst), NULL, &siglen,
|
||||
kinv, rp, eckey), 1)
|
||||
&& TEST_int_gt(siglen, 0)
|
||||
&& TEST_int_le(siglen, siglen0)
|
||||
&& TEST_int_eq(ECDSA_sign_ex(0, dgst, sizeof(dgst), sig, &siglen0,
|
||||
kinv, rp, eckey), 1)
|
||||
&& TEST_int_eq(siglen, siglen0)
|
||||
&& TEST_int_eq(ECDSA_verify(0, dgst, sizeof(dgst), sig, siglen,
|
||||
eckey), 1);
|
||||
EC_KEY_free(eckey);
|
||||
OPENSSL_free(sig);
|
||||
BN_free(kinv);
|
||||
BN_free(rp);
|
||||
return ret;
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue