4
0
mirror of https://github.com/QuasarApp/openssl.git synced 2025-05-04 21:49:38 +00:00

Fix ecdsa digest setting code to match dsa.

Fixes 

ecdsa_set_ctx_params() was not setting the digest correctly. The side
effect noted was that the check for sha1 when signing was not being
done in fips mode.

Also fixed the dupctx() so that propq is deep copied.
The usage of the variable 'flag_allow_md' was also copied from the dsa code.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13520)
This commit is contained in:
Shane Lontis 2020-11-26 15:03:10 +10:00
parent ddfd7182cf
commit 283320281b
2 changed files with 133 additions and 83 deletions
providers/implementations/signature
test/recipes/30-test_evp_data

@ -66,6 +66,14 @@ typedef struct {
EC_KEY *ec;
char mdname[OSSL_MAX_NAME_SIZE];
/*
* Flag to determine if the hash function can be changed (1) or not (0)
* Because it's dangerous to change during a DigestSign or DigestVerify
* operation, this flag is cleared by their Init function, and set again
* by their Final function.
*/
unsigned int flag_allow_md : 1;
/* The Algorithm Identifier of the combined signature algorithm */
unsigned char aid_buf[OSSL_MAX_ALGORITHM_ID_SIZE];
unsigned char *aid;
@ -107,6 +115,7 @@ static void *ecdsa_newctx(void *provctx, const char *propq)
if (ctx == NULL)
return NULL;
ctx->flag_allow_md = 1;
ctx->libctx = PROV_LIBCTX_OF(provctx);
if (propq != NULL && (ctx->propq = OPENSSL_strdup(propq)) == NULL) {
OPENSSL_free(ctx);
@ -187,50 +196,43 @@ static int ecdsa_verify(void *vctx, const unsigned char *sig, size_t siglen,
return ECDSA_verify(0, tbs, tbslen, sig, siglen, ctx->ec);
}
static void free_md(PROV_ECDSA_CTX *ctx)
static int ecdsa_setup_md(PROV_ECDSA_CTX *ctx, const char *mdname,
const char *mdprops)
{
OPENSSL_free(ctx->propq);
EVP_MD *md = NULL;
size_t mdname_len;
int md_nid, sha1_allowed;
WPACKET pkt;
if (mdname == NULL)
return 1;
mdname_len = strlen(mdname);
if (mdname_len >= sizeof(ctx->mdname)) {
ERR_raise_data(ERR_LIB_PROV, PROV_R_INVALID_DIGEST,
"%s exceeds name buffer length", mdname);
return 0;
}
if (mdprops == NULL)
mdprops = ctx->propq;
md = EVP_MD_fetch(ctx->libctx, mdname, mdprops);
if (md == NULL) {
ERR_raise_data(ERR_LIB_PROV, PROV_R_INVALID_DIGEST,
"%s could not be fetched", mdname);
return 0;
}
sha1_allowed = (ctx->operation != EVP_PKEY_OP_SIGN);
md_nid = digest_get_approved_nid_with_sha1(md, sha1_allowed);
if (md_nid == NID_undef) {
ERR_raise_data(ERR_LIB_PROV, PROV_R_DIGEST_NOT_ALLOWED,
"digest=%s", mdname);
EVP_MD_free(md);
return 0;
}
EVP_MD_CTX_free(ctx->mdctx);
EVP_MD_free(ctx->md);
ctx->propq = NULL;
ctx->mdctx = NULL;
ctx->md = NULL;
ctx->mdsize = 0;
}
static int ecdsa_digest_signverify_init(void *vctx, const char *mdname,
void *ec, int operation)
{
PROV_ECDSA_CTX *ctx = (PROV_ECDSA_CTX *)vctx;
int md_nid = NID_undef;
WPACKET pkt;
int sha1_allowed = (ctx->operation != EVP_PKEY_OP_SIGN);
if (!ossl_prov_is_running())
return 0;
free_md(ctx);
if (!ecdsa_signverify_init(vctx, ec, operation))
return 0;
ctx->md = EVP_MD_fetch(ctx->libctx, mdname, ctx->propq);
md_nid = digest_get_approved_nid_with_sha1(ctx->md, sha1_allowed);
if (md_nid == NID_undef)
goto error;
ctx->mdsize = EVP_MD_size(ctx->md);
ctx->mdctx = EVP_MD_CTX_new();
if (ctx->mdctx == NULL)
goto error;
/*
* TODO(3.0) Should we care about DER writing errors?
* All it really means is that for some reason, there's no
* AlgorithmIdentifier to be had, but the operation itself is
* still valid, just as long as it's not used to construct
* anything that needs an AlgorithmIdentifier.
*/
ctx->aid_len = 0;
if (WPACKET_init_der(&pkt, ctx->aid_buf, sizeof(ctx->aid_buf))
&& ossl_DER_w_algorithmIdentifier_ECDSA_with_MD(&pkt, -1, ctx->ec,
@ -240,12 +242,39 @@ static int ecdsa_digest_signverify_init(void *vctx, const char *mdname,
ctx->aid = WPACKET_get_curr(&pkt);
}
WPACKET_cleanup(&pkt);
ctx->mdctx = NULL;
ctx->md = md;
ctx->mdsize = EVP_MD_size(ctx->md);
OPENSSL_strlcpy(ctx->mdname, mdname, sizeof(ctx->mdname));
return 1;
}
static int ecdsa_digest_signverify_init(void *vctx, const char *mdname,
void *ec, int operation)
{
PROV_ECDSA_CTX *ctx = (PROV_ECDSA_CTX *)vctx;
if (!ossl_prov_is_running())
return 0;
ctx->flag_allow_md = 0;
if (!ecdsa_signverify_init(vctx, ec, operation)
|| !ecdsa_setup_md(ctx, mdname, NULL))
return 0;
ctx->mdctx = EVP_MD_CTX_new();
if (ctx->mdctx == NULL)
goto error;
if (!EVP_DigestInit_ex(ctx->mdctx, ctx->md, NULL))
goto error;
return 1;
error:
free_md(ctx);
EVP_MD_CTX_free(ctx->mdctx);
EVP_MD_free(ctx->md);
ctx->mdctx = NULL;
ctx->md = NULL;
return 0;
}
@ -284,16 +313,10 @@ int ecdsa_digest_sign_final(void *vctx, unsigned char *sig, size_t *siglen,
* If sig is NULL then we're just finding out the sig size. Other fields
* are ignored. Defer to ecdsa_sign.
*/
if (sig != NULL) {
/*
* TODO(3.0): There is the possibility that some externally provided
* digests exceed EVP_MAX_MD_SIZE. We should probably handle that somehow -
* but that problem is much larger than just in DSA.
*/
if (!EVP_DigestFinal_ex(ctx->mdctx, digest, &dlen))
return 0;
}
if (sig != NULL
&& !EVP_DigestFinal_ex(ctx->mdctx, digest, &dlen))
return 0;
ctx->flag_allow_md = 1;
return ecdsa_sign(vctx, sig, siglen, sigsize, digest, (size_t)dlen);
}
@ -307,14 +330,9 @@ int ecdsa_digest_verify_final(void *vctx, const unsigned char *sig,
if (!ossl_prov_is_running() || ctx == NULL || ctx->mdctx == NULL)
return 0;
/*
* TODO(3.0): There is the possibility that some externally provided
* digests exceed EVP_MAX_MD_SIZE. We should probably handle that somehow -
* but that problem is much larger than just in DSA.
*/
if (!EVP_DigestFinal_ex(ctx->mdctx, digest, &dlen))
return 0;
ctx->flag_allow_md = 1;
return ecdsa_verify(ctx, sig, siglen, digest, (size_t)dlen);
}
@ -322,7 +340,13 @@ static void ecdsa_freectx(void *vctx)
{
PROV_ECDSA_CTX *ctx = (PROV_ECDSA_CTX *)vctx;
free_md(ctx);
OPENSSL_free(ctx->propq);
EVP_MD_CTX_free(ctx->mdctx);
EVP_MD_free(ctx->md);
ctx->propq = NULL;
ctx->mdctx = NULL;
ctx->md = NULL;
ctx->mdsize = 0;
EC_KEY_free(ctx->ec);
BN_clear_free(ctx->kinv);
BN_clear_free(ctx->r);
@ -345,6 +369,7 @@ static void *ecdsa_dupctx(void *vctx)
dstctx->ec = NULL;
dstctx->md = NULL;
dstctx->mdctx = NULL;
dstctx->propq = NULL;
if (srcctx->ec != NULL && !EC_KEY_up_ref(srcctx->ec))
goto err;
@ -364,6 +389,12 @@ static void *ecdsa_dupctx(void *vctx)
goto err;
}
if (srcctx->propq != NULL) {
dstctx->propq = OPENSSL_strdup(srcctx->propq);
if (dstctx->propq == NULL)
goto err;
}
return dstctx;
err:
ecdsa_freectx(dstctx);
@ -411,37 +442,40 @@ static int ecdsa_set_ctx_params(void *vctx, const OSSL_PARAM params[])
{
PROV_ECDSA_CTX *ctx = (PROV_ECDSA_CTX *)vctx;
const OSSL_PARAM *p;
char *mdname;
if (ctx == NULL || params == NULL)
return 0;
if (ctx->md != NULL) {
/*
* You cannot set the digest name/size when doing a DigestSign or
* DigestVerify.
*/
return 1;
}
#if !defined(OPENSSL_NO_ACVP_TESTS)
p = OSSL_PARAM_locate_const(params, OSSL_SIGNATURE_PARAM_KAT);
if (p != NULL && !OSSL_PARAM_get_uint(p, &ctx->kattest))
return 0;
#endif
p = OSSL_PARAM_locate_const(params, OSSL_SIGNATURE_PARAM_DIGEST_SIZE);
if (p != NULL && !OSSL_PARAM_get_size_t(p, &ctx->mdsize))
return 0;
/*
* We never actually use the mdname, but we do support getting it later.
* This can be useful for applications that want to know the MD that they
* previously set.
*/
p = OSSL_PARAM_locate_const(params, OSSL_SIGNATURE_PARAM_DIGEST);
mdname = ctx->mdname;
/* Not allowed during certain operations */
if (p != NULL && !ctx->flag_allow_md)
return 0;
if (p != NULL) {
char mdname[OSSL_MAX_NAME_SIZE] = "", *pmdname = mdname;
char mdprops[OSSL_MAX_PROPQUERY_SIZE] = "", *pmdprops = mdprops;
const OSSL_PARAM *propsp =
OSSL_PARAM_locate_const(params,
OSSL_SIGNATURE_PARAM_PROPERTIES);
if (!OSSL_PARAM_get_utf8_string(p, &pmdname, sizeof(mdname)))
return 0;
if (propsp != NULL
&& !OSSL_PARAM_get_utf8_string(propsp, &pmdprops, sizeof(mdprops)))
return 0;
if (!ecdsa_setup_md(ctx, mdname, mdprops))
return 0;
}
p = OSSL_PARAM_locate_const(params, OSSL_SIGNATURE_PARAM_DIGEST_SIZE);
if (p != NULL
&& !OSSL_PARAM_get_utf8_string(p, &mdname, sizeof(ctx->mdname)))
&& (!ctx->flag_allow_md
|| !OSSL_PARAM_get_size_t(p, &ctx->mdsize)))
return 0;
return 1;
@ -450,18 +484,13 @@ static int ecdsa_set_ctx_params(void *vctx, const OSSL_PARAM params[])
static const OSSL_PARAM known_settable_ctx_params[] = {
OSSL_PARAM_size_t(OSSL_SIGNATURE_PARAM_DIGEST_SIZE, NULL),
OSSL_PARAM_utf8_string(OSSL_SIGNATURE_PARAM_DIGEST, NULL, 0),
OSSL_PARAM_utf8_string(OSSL_SIGNATURE_PARAM_PROPERTIES, NULL, 0),
OSSL_PARAM_uint(OSSL_SIGNATURE_PARAM_KAT, NULL),
OSSL_PARAM_END
};
static const OSSL_PARAM *ecdsa_settable_ctx_params(ossl_unused void *provctx)
{
/*
* TODO(3.0): Should this function return a different set of settable ctx
* params if the ctx is being used for a DigestSign/DigestVerify? In that
* case it is not allowed to set the digest size/digest name because the
* digest is explicitly set as part of the init.
*/
return known_settable_ctx_params;
}

@ -108,6 +108,12 @@ Key = P-256-PUBLIC
Input = "Hello World"
Output = 3046022100e7515177ec3817b77a4a94066ab3070817b7aa9d44a8a09f040da250116e8972022100ba59b0f631258e59a9026be5d84f60685f4cf22b9165a0c2736d5c21c8ec1862
# Test that mdsize != tbssize fails
Sign = P-256
Ctrl = digest:SHA256
Input = "0123456789ABCDEF1234"
Result = KEYOP_ERROR
PrivateKey = P-256_NAMED_CURVE_EXPLICIT
-----BEGIN PRIVATE KEY-----
MIIBeQIBADCCAQMGByqGSM49AgEwgfcCAQEwLAYHKoZIzj0BAQIhAP////8AAAAB
@ -192,3 +198,18 @@ Securitycheck = 1
Key = secp256k1
Input = "Hello World"
Result = DIGESTSIGNINIT_ERROR
# Test that SHA1 is not allowed in fips mode for signing
Availablein = fips
DigestSign = SHA1
Securitycheck = 1
Key = B-163
Input = "Hello World"
Result = DIGESTSIGNINIT_ERROR
# Test that SHA1 is not allowed in fips mode for signing
Availablein = fips
Sign = P-256
Ctrl = digest:SHA1
Input = "0123456789ABCDEF1234"
Result = PKEY_CTRL_ERROR