FFC cleanups

Discovered during coverage testing.

Remove unneccesary check when using ossl_dh_get0_params() and
ossl_dsa_get0_params(). These point to addresses and can not fail
for any existing calls.

Make dsa keygen tests only available in the FIPS module - as they are
not used in the default provider.

Change ossl_ffc_set_digest() to return void as it cannot fail.

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/20359)
This commit is contained in:
slontis 2023-02-22 14:16:05 +10:00 committed by Tomas Mraz
parent eb52450f51
commit a76ccb9d0d
9 changed files with 112 additions and 85 deletions

View File

@ -31,13 +31,7 @@
static int dh_ffc_params_fromdata(DH *dh, const OSSL_PARAM params[])
{
int ret;
FFC_PARAMS *ffc;
if (dh == NULL)
return 0;
ffc = ossl_dh_get0_params(dh);
if (ffc == NULL)
return 0;
FFC_PARAMS *ffc = ossl_dh_get0_params(dh);
ret = ossl_ffc_params_fromdata(ffc, params);
if (ret)

View File

@ -28,8 +28,7 @@
# define MIN_STRENGTH 80
#endif
static int dsa_keygen(DSA *dsa, int pairwise_test);
static int dsa_keygen_pairwise_test(DSA *dsa, OSSL_CALLBACK *cb, void *cbarg);
static int dsa_keygen(DSA *dsa);
int DSA_generate_key(DSA *dsa)
{
@ -37,7 +36,7 @@ int DSA_generate_key(DSA *dsa)
if (dsa->meth->dsa_keygen != NULL)
return dsa->meth->dsa_keygen(dsa);
#endif
return dsa_keygen(dsa, 0);
return dsa_keygen(dsa);
}
int ossl_dsa_generate_public_key(BN_CTX *ctx, const DSA *dsa,
@ -59,6 +58,7 @@ err:
return ret;
}
#ifdef FIPS_MODULE
/*
* Refer: FIPS 140-3 IG 10.3.A Additional Comment 1
* Perform a KAT by duplicating the public key generation.
@ -107,7 +107,44 @@ err:
return ret;
}
static int dsa_keygen(DSA *dsa, int pairwise_test)
/*
* FIPS 140-2 IG 9.9 AS09.33
* Perform a sign/verify operation.
*/
static int dsa_keygen_pairwise_test(DSA *dsa, OSSL_CALLBACK *cb, void *cbarg)
{
int ret = 0;
unsigned char dgst[16] = {0};
unsigned int dgst_len = (unsigned int)sizeof(dgst);
DSA_SIG *sig = NULL;
OSSL_SELF_TEST *st = NULL;
st = OSSL_SELF_TEST_new(cb, cbarg);
if (st == NULL)
goto err;
OSSL_SELF_TEST_onbegin(st, OSSL_SELF_TEST_TYPE_PCT,
OSSL_SELF_TEST_DESC_PCT_DSA);
sig = DSA_do_sign(dgst, (int)dgst_len, dsa);
if (sig == NULL)
goto err;
OSSL_SELF_TEST_oncorrupt_byte(st, dgst);
if (DSA_do_verify(dgst, dgst_len, sig, dsa) != 1)
goto err;
ret = 1;
err:
OSSL_SELF_TEST_onend(st, ret);
OSSL_SELF_TEST_free(st);
DSA_SIG_free(sig);
return ret;
}
#endif /* FIPS_MODULE */
static int dsa_keygen(DSA *dsa)
{
int ok = 0;
BN_CTX *ctx = NULL;
@ -151,12 +188,9 @@ static int dsa_keygen(DSA *dsa, int pairwise_test)
dsa->priv_key = priv_key;
dsa->pub_key = pub_key;
#ifdef FIPS_MODULE
pairwise_test = 1;
#endif /* FIPS_MODULE */
ok = 1;
if (pairwise_test) {
#ifdef FIPS_MODULE
{
OSSL_CALLBACK *cb = NULL;
void *cbarg = NULL;
@ -173,6 +207,7 @@ static int dsa_keygen(DSA *dsa, int pairwise_test)
return ok;
}
}
#endif
dsa->dirty_cnt++;
err:
@ -184,39 +219,3 @@ static int dsa_keygen(DSA *dsa, int pairwise_test)
return ok;
}
/*
* FIPS 140-2 IG 9.9 AS09.33
* Perform a sign/verify operation.
*/
static int dsa_keygen_pairwise_test(DSA *dsa, OSSL_CALLBACK *cb, void *cbarg)
{
int ret = 0;
unsigned char dgst[16] = {0};
unsigned int dgst_len = (unsigned int)sizeof(dgst);
DSA_SIG *sig = NULL;
OSSL_SELF_TEST *st = NULL;
st = OSSL_SELF_TEST_new(cb, cbarg);
if (st == NULL)
goto err;
OSSL_SELF_TEST_onbegin(st, OSSL_SELF_TEST_TYPE_PCT,
OSSL_SELF_TEST_DESC_PCT_DSA);
sig = DSA_do_sign(dgst, (int)dgst_len, dsa);
if (sig == NULL)
goto err;
OSSL_SELF_TEST_oncorrupt_byte(st, dgst);
if (DSA_do_verify(dgst, dgst_len, sig, dsa) != 1)
goto err;
ret = 1;
err:
OSSL_SELF_TEST_onend(st, ret);
OSSL_SELF_TEST_free(st);
DSA_SIG_free(sig);
return ret;
}

View File

@ -345,13 +345,7 @@ FFC_PARAMS *ossl_dsa_get0_params(DSA *dsa)
int ossl_dsa_ffc_params_fromdata(DSA *dsa, const OSSL_PARAM params[])
{
int ret;
FFC_PARAMS *ffc;
if (dsa == NULL)
return 0;
ffc = ossl_dsa_get0_params(dsa);
if (ffc == NULL)
return 0;
FFC_PARAMS *ffc = ossl_dsa_get0_params(dsa);
ret = ossl_ffc_params_fromdata(ffc, params);
if (ret)

View File

@ -24,9 +24,6 @@ int ossl_ffc_params_fromdata(FFC_PARAMS *ffc, const OSSL_PARAM params[])
BIGNUM *p = NULL, *q = NULL, *g = NULL, *j = NULL;
int i;
if (ffc == NULL)
return 0;
prm = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_GROUP_NAME);
if (prm != NULL) {
/*
@ -76,9 +73,8 @@ int ossl_ffc_params_fromdata(FFC_PARAMS *ffc, const OSSL_PARAM params[])
}
prm = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_FFC_SEED);
if (prm != NULL) {
if (prm->data_type != OSSL_PARAM_OCTET_STRING)
goto err;
if (!ossl_ffc_params_set_seed(ffc, prm->data, prm->data_size))
if (prm->data_type != OSSL_PARAM_OCTET_STRING
|| !ossl_ffc_params_set_seed(ffc, prm->data, prm->data_size))
goto err;
}
prm = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_FFC_VALIDATE_PQ);
@ -113,10 +109,8 @@ int ossl_ffc_params_fromdata(FFC_PARAMS *ffc, const OSSL_PARAM params[])
goto err;
props = p1->data;
}
if (!ossl_ffc_set_digest(ffc, prm->data, props))
goto err;
ossl_ffc_set_digest(ffc, prm->data, props);
}
ossl_ffc_params_set0_pqg(ffc, p, q, g);
ossl_ffc_params_set0_j(ffc, j);
return 1;

View File

@ -75,9 +75,6 @@ void ossl_ffc_params_set0_j(FFC_PARAMS *d, BIGNUM *j)
int ossl_ffc_params_set_seed(FFC_PARAMS *params,
const unsigned char *seed, size_t seedlen)
{
if (params == NULL)
return 0;
if (params->seed != NULL) {
if (params->seed == seed)
return 1;
@ -125,11 +122,10 @@ void ossl_ffc_params_enable_flags(FFC_PARAMS *params, unsigned int flags,
params->flags &= ~flags;
}
int ossl_ffc_set_digest(FFC_PARAMS *params, const char *alg, const char *props)
void ossl_ffc_set_digest(FFC_PARAMS *params, const char *alg, const char *props)
{
params->mdname = alg;
params->mdprops = props;
return 1;
}
int ossl_ffc_params_set_validate_params(FFC_PARAMS *params,
@ -214,9 +210,6 @@ int ossl_ffc_params_todata(const FFC_PARAMS *ffc, OSSL_PARAM_BLD *bld,
{
int test_flags;
if (ffc == NULL)
return 0;
if (ffc->p != NULL
&& !ossl_param_build_set_bn(bld, params, OSSL_PKEY_PARAM_FFC_P, ffc->p))
return 0;

View File

@ -132,7 +132,7 @@ void ossl_ffc_params_set_h(FFC_PARAMS *params, int index);
void ossl_ffc_params_set_flags(FFC_PARAMS *params, unsigned int flags);
void ossl_ffc_params_enable_flags(FFC_PARAMS *params, unsigned int flags,
int enable);
int ossl_ffc_set_digest(FFC_PARAMS *params, const char *alg, const char *props);
void ossl_ffc_set_digest(FFC_PARAMS *params, const char *alg, const char *props);
int ossl_ffc_params_set_validate_params(FFC_PARAMS *params,
const unsigned char *seed,

View File

@ -735,10 +735,8 @@ static void *dh_gen(void *genctx, OSSL_CALLBACK *osslcb, void *cbarg)
} else if (gctx->hindex != 0) {
ossl_ffc_params_set_h(ffc, gctx->hindex);
}
if (gctx->mdname != NULL) {
if (!ossl_ffc_set_digest(ffc, gctx->mdname, gctx->mdprops))
goto end;
}
if (gctx->mdname != NULL)
ossl_ffc_set_digest(ffc, gctx->mdname, gctx->mdprops);
gctx->cb = osslcb;
gctx->cbarg = cbarg;
gencb = BN_GENCB_new();

View File

@ -587,10 +587,9 @@ static void *dsa_gen(void *genctx, OSSL_CALLBACK *osslcb, void *cbarg)
} else if (gctx->hindex != 0) {
ossl_ffc_params_set_h(ffc, gctx->hindex);
}
if (gctx->mdname != NULL) {
if (!ossl_ffc_set_digest(ffc, gctx->mdname, gctx->mdprops))
goto end;
}
if (gctx->mdname != NULL)
ossl_ffc_set_digest(ffc, gctx->mdname, gctx->mdprops);
if ((gctx->selection & OSSL_KEYMGMT_SELECT_DOMAIN_PARAMETERS) != 0) {
if (ossl_dsa_generate_ffc_parameters(dsa, gctx->gen_type,

View File

@ -388,6 +388,61 @@ static int test_dh_paramgen(void)
return ret;
}
static int set_fromdata_string(EVP_PKEY_CTX *ctx, const char *name, char *value)
{
int ret;
OSSL_PARAM params[2];
EVP_PKEY *pkey = NULL;
if (EVP_PKEY_fromdata_init(ctx) != 1)
return -1;
params[0] = OSSL_PARAM_construct_utf8_string(name, value, 0);
params[1] = OSSL_PARAM_construct_end();
ret = EVP_PKEY_fromdata(ctx, &pkey, EVP_PKEY_KEY_PARAMETERS, params);
EVP_PKEY_free(pkey);
return ret;
}
static int set_fromdata_uint(EVP_PKEY_CTX *ctx, const char *name)
{
int ret;
unsigned int tmp = 0;
OSSL_PARAM params[2];
EVP_PKEY *pkey = NULL;
if (EVP_PKEY_fromdata_init(ctx) != 1)
return -1;
params[0] = OSSL_PARAM_construct_uint(name, &tmp);
params[1] = OSSL_PARAM_construct_end();
ret = EVP_PKEY_fromdata(ctx, &pkey, EVP_PKEY_KEY_PARAMETERS, params);
EVP_PKEY_free(pkey);
return ret;
}
static int test_dh_paramfromdata(void)
{
EVP_PKEY_CTX *ctx = NULL;
int ret = 0;
/* Test failure paths for FFC - mainly due to setting the wrong param type */
ret = TEST_ptr(ctx = EVP_PKEY_CTX_new_from_name(mainctx, "DH", NULL))
&& TEST_int_eq(set_fromdata_uint(ctx, OSSL_PKEY_PARAM_GROUP_NAME), 0)
&& TEST_int_eq(set_fromdata_string(ctx, OSSL_PKEY_PARAM_GROUP_NAME, "bad"), 0)
&& TEST_int_eq(set_fromdata_string(ctx, OSSL_PKEY_PARAM_FFC_P, "bad"), 0)
&& TEST_int_eq(set_fromdata_string(ctx, OSSL_PKEY_PARAM_FFC_GINDEX, "bad"), 0)
&& TEST_int_eq(set_fromdata_string(ctx, OSSL_PKEY_PARAM_FFC_PCOUNTER, "bad"), 0)
&& TEST_int_eq(set_fromdata_string(ctx, OSSL_PKEY_PARAM_FFC_COFACTOR, "bad"), 0)
&& TEST_int_eq(set_fromdata_string(ctx, OSSL_PKEY_PARAM_FFC_H, "bad"), 0)
&& TEST_int_eq(set_fromdata_uint(ctx, OSSL_PKEY_PARAM_FFC_SEED), 0)
&& TEST_int_eq(set_fromdata_string(ctx, OSSL_PKEY_PARAM_FFC_VALIDATE_PQ, "bad"), 0)
&& TEST_int_eq(set_fromdata_string(ctx, OSSL_PKEY_PARAM_FFC_VALIDATE_G, "bad"), 0)
&& TEST_int_eq(set_fromdata_string(ctx, OSSL_PKEY_PARAM_FFC_VALIDATE_LEGACY, "bad"), 0)
&& TEST_int_eq(set_fromdata_uint(ctx, OSSL_PKEY_PARAM_FFC_DIGEST), 0);
EVP_PKEY_CTX_free(ctx);
return ret;
}
#endif
#ifndef OPENSSL_NO_EC
@ -1247,6 +1302,7 @@ int setup_tests(void)
#ifndef OPENSSL_NO_DH
ADD_TEST(test_dh_tofrom_data_select);
ADD_TEST(test_dh_paramgen);
ADD_TEST(test_dh_paramfromdata);
#endif
ADD_TEST(test_rsa_tofrom_data_select);