From 8bee651213c4755761ed5dd32d416d782cc5fc3c Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Sun, 2 Feb 2020 13:09:23 +0100 Subject: [PATCH] PROV: Fix the DSA SIGNATURE implementation for better digests handling Refactor the DSA SIGNATURE digest setup to be uniform, and to happen in two places: 1. when given through the digestsign and digestverify inits 2. when given through the set_ctx_params function. When setting up the digest, we also check that the digest is one of the officially accepted for DSA. Reviewed-by: Shane Lontis (Merged from https://github.com/openssl/openssl/pull/10947) --- include/openssl/core_names.h | 2 + providers/implementations/signature/dsa.c | 194 +++++++++++++--------- 2 files changed, 116 insertions(+), 80 deletions(-) diff --git a/include/openssl/core_names.h b/include/openssl/core_names.h index c061902b8c..c17eee42a5 100644 --- a/include/openssl/core_names.h +++ b/include/openssl/core_names.h @@ -168,6 +168,7 @@ extern "C" { #define OSSL_PKEY_PARAM_MAX_SIZE "max-size" /* integer */ #define OSSL_PKEY_PARAM_SECURITY_BITS "security-bits" /* integer */ #define OSSL_PKEY_PARAM_DIGEST OSSL_ALG_PARAM_DIGEST +#define OSSL_PKEY_PARAM_PROPERTIES OSSL_ALG_PARAM_PROPERTIES #define OSSL_PKEY_PARAM_DEFAULT_DIGEST "default-digest" /* utf8 string */ #define OSSL_PKEY_PARAM_MANDATORY_DIGEST "mandatory-digest" /* utf8 string */ @@ -212,6 +213,7 @@ extern "C" { /* Signature parameters */ #define OSSL_SIGNATURE_PARAM_ALGORITHM_ID "algorithm-id" #define OSSL_SIGNATURE_PARAM_DIGEST OSSL_PKEY_PARAM_DIGEST +#define OSSL_SIGNATURE_PARAM_PROPERTIES OSSL_PKEY_PARAM_PROPERTIES /* Asym cipher parameters */ #define OSSL_ASYM_CIPHER_PARAM_PAD_MODE "pad-mode" diff --git a/providers/implementations/signature/dsa.c b/providers/implementations/signature/dsa.c index eaf6d4fe29..b2309ef510 100644 --- a/providers/implementations/signature/dsa.c +++ b/providers/implementations/signature/dsa.c @@ -16,10 +16,12 @@ #include #include #include +#include #include "internal/nelem.h" #include "internal/sizes.h" #include "prov/providercommonerr.h" #include "prov/implementations.h" +#include "prov/providercommonerr.h" #include "prov/provider_ctx.h" #include "crypto/dsa.h" @@ -54,7 +56,15 @@ static OSSL_OP_signature_settable_ctx_md_params_fn dsa_settable_ctx_md_params; typedef struct { OPENSSL_CTX *libctx; DSA *dsa; - size_t mdsize; + + /* + * 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; + char mdname[OSSL_MAX_NAME_SIZE]; /* The Algorithm Identifier of the combined signature agorithm */ @@ -64,8 +74,54 @@ typedef struct { /* main digest */ EVP_MD *md; EVP_MD_CTX *mdctx; + size_t mdsize; } PROV_DSA_CTX; +static size_t dsa_get_md_size(const PROV_DSA_CTX *pdsactx) +{ + if (pdsactx->md != NULL) + return EVP_MD_size(pdsactx->md); + return 0; +} + +static int dsa_get_md_nid(const EVP_MD *md) +{ + /* + * Because the DSA library deals with NIDs, we need to translate. + * We do so using EVP_MD_is_a(), and therefore need a name to NID + * map. + */ + static const OSSL_ITEM name_to_nid[] = { + { NID_sha1, OSSL_DIGEST_NAME_SHA1 }, + { NID_sha224, OSSL_DIGEST_NAME_SHA2_224 }, + { NID_sha256, OSSL_DIGEST_NAME_SHA2_256 }, + { NID_sha384, OSSL_DIGEST_NAME_SHA2_384 }, + { NID_sha512, OSSL_DIGEST_NAME_SHA2_512 }, + { NID_sha3_224, OSSL_DIGEST_NAME_SHA3_224 }, + { NID_sha3_256, OSSL_DIGEST_NAME_SHA3_256 }, + { NID_sha3_384, OSSL_DIGEST_NAME_SHA3_384 }, + { NID_sha3_512, OSSL_DIGEST_NAME_SHA3_512 }, + }; + size_t i; + int mdnid = NID_undef; + + if (md == NULL) + goto end; + + for (i = 0; i < OSSL_NELEM(name_to_nid); i++) { + if (EVP_MD_is_a(md, name_to_nid[i].ptr)) { + mdnid = (int)name_to_nid[i].id; + break; + } + } + + if (mdnid == NID_undef) + ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_DIGEST); + + end: + return mdnid; +} + static void *dsa_newctx(void *provctx) { PROV_DSA_CTX *pdsactx = OPENSSL_zalloc(sizeof(PROV_DSA_CTX)); @@ -74,9 +130,37 @@ static void *dsa_newctx(void *provctx) return NULL; pdsactx->libctx = PROV_LIBRARY_CONTEXT_OF(provctx); + pdsactx->flag_allow_md = 1; return pdsactx; } +static int dsa_setup_md(PROV_DSA_CTX *ctx, + const char *mdname, const char *mdprops) +{ + if (mdname != NULL) { + EVP_MD *md = EVP_MD_fetch(ctx->libctx, mdname, mdprops); + int md_nid = dsa_get_md_nid(md); + size_t algorithmidentifier_len = 0; + const unsigned char *algorithmidentifier; + + EVP_MD_free(ctx->md); + ctx->md = NULL; + ctx->mdname[0] = '\0'; + + algorithmidentifier = + dsa_algorithmidentifier_encoding(md_nid, &algorithmidentifier_len); + + if (algorithmidentifier == NULL) { + EVP_MD_free(md); + return 0; + } + + ctx->md = md; + OPENSSL_strlcpy(ctx->mdname, mdname, sizeof(ctx->mdname)); + } + return 1; +} + static int dsa_signature_init(void *vpdsactx, void *vdsa) { PROV_DSA_CTX *pdsactx = (PROV_DSA_CTX *)vpdsactx; @@ -95,6 +179,7 @@ static int dsa_sign(void *vpdsactx, unsigned char *sig, size_t *siglen, int ret; unsigned int sltmp; size_t dsasize = DSA_size(pdsactx->dsa); + size_t mdsize = dsa_get_md_size(pdsactx); if (sig == NULL) { *siglen = dsasize; @@ -104,7 +189,7 @@ static int dsa_sign(void *vpdsactx, unsigned char *sig, size_t *siglen, if (sigsize < (size_t)dsasize) return 0; - if (pdsactx->mdsize != 0 && tbslen != pdsactx->mdsize) + if (mdsize != 0 && tbslen != mdsize) return 0; ret = dsa_sign_int(pdsactx->libctx, 0, tbs, tbslen, sig, &sltmp, @@ -120,83 +205,30 @@ static int dsa_verify(void *vpdsactx, const unsigned char *sig, size_t siglen, const unsigned char *tbs, size_t tbslen) { PROV_DSA_CTX *pdsactx = (PROV_DSA_CTX *)vpdsactx; + size_t mdsize = dsa_get_md_size(pdsactx); - if (pdsactx->mdsize != 0 && tbslen != pdsactx->mdsize) + if (mdsize != 0 && tbslen != mdsize) return 0; return DSA_verify(0, tbs, tbslen, sig, siglen, pdsactx->dsa); } -static int get_md_nid(const EVP_MD *md) -{ - /* - * Because the DSA library deals with NIDs, we need to translate. - * We do so using EVP_MD_is_a(), and therefore need a name to NID - * map. - */ - static const OSSL_ITEM name_to_nid[] = { - { NID_sha1, OSSL_DIGEST_NAME_SHA1 }, - { NID_sha224, OSSL_DIGEST_NAME_SHA2_224 }, - { NID_sha256, OSSL_DIGEST_NAME_SHA2_256 }, - { NID_sha384, OSSL_DIGEST_NAME_SHA2_384 }, - { NID_sha512, OSSL_DIGEST_NAME_SHA2_512 }, - { NID_sha3_224, OSSL_DIGEST_NAME_SHA3_224 }, - { NID_sha3_256, OSSL_DIGEST_NAME_SHA3_256 }, - { NID_sha3_384, OSSL_DIGEST_NAME_SHA3_384 }, - { NID_sha3_512, OSSL_DIGEST_NAME_SHA3_512 }, - }; - size_t i; - int mdnid = NID_undef; - - if (md == NULL) - goto end; - - for (i = 0; i < OSSL_NELEM(name_to_nid); i++) { - if (EVP_MD_is_a(md, name_to_nid[i].ptr)) { - mdnid = (int)name_to_nid[i].id; - break; - } - } - - if (mdnid == NID_undef) - ERR_raise(ERR_LIB_PROV, PROV_R_INVALID_DIGEST); - - end: - return mdnid; -} - static int dsa_digest_signverify_init(void *vpdsactx, const char *mdname, const char *props, void *vdsa) { PROV_DSA_CTX *pdsactx = (PROV_DSA_CTX *)vpdsactx; - size_t algorithmidentifier_len = 0; - const unsigned char *algorithmidentifier; - - EVP_MD_CTX_free(pdsactx->mdctx); - EVP_MD_free(pdsactx->md); - pdsactx->mdctx = NULL; - pdsactx->mdsize = 0; - pdsactx->md = NULL; + pdsactx->flag_allow_md = 0; if (!dsa_signature_init(vpdsactx, vdsa)) return 0; - pdsactx->md = EVP_MD_fetch(pdsactx->libctx, mdname, props); - algorithmidentifier = - dsa_algorithmidentifier_encoding(get_md_nid(pdsactx->md), - &algorithmidentifier_len); - - if (algorithmidentifier == NULL) - goto error; + if (!dsa_setup_md(pdsactx, mdname, props)) + return 0; - pdsactx->mdsize = EVP_MD_size(pdsactx->md); pdsactx->mdctx = EVP_MD_CTX_new(); if (pdsactx->mdctx == NULL) goto error; - memcpy(pdsactx->aid, algorithmidentifier, algorithmidentifier_len); - pdsactx->aid_len = algorithmidentifier_len; - if (!EVP_DigestInit_ex(pdsactx->mdctx, pdsactx->md, NULL)) goto error; @@ -245,6 +277,8 @@ int dsa_digest_sign_final(void *vpdsactx, unsigned char *sig, size_t *siglen, return 0; } + pdsactx->flag_allow_md = 1; + return dsa_sign(vpdsactx, sig, siglen, sigsize, digest, (size_t)dlen); } @@ -267,6 +301,8 @@ int dsa_digest_verify_final(void *vpdsactx, const unsigned char *sig, if (!EVP_DigestFinal_ex(pdsactx->mdctx, digest, &dlen)) return 0; + pdsactx->flag_allow_md = 1; + return dsa_verify(vpdsactx, sig, siglen, digest, (size_t)dlen); } @@ -330,9 +366,7 @@ static int dsa_get_ctx_params(void *vpdsactx, OSSL_PARAM *params) return 0; p = OSSL_PARAM_locate(params, OSSL_SIGNATURE_PARAM_DIGEST); - if (p != NULL && !OSSL_PARAM_set_utf8_string(p, pdsactx->md == NULL - ? pdsactx->mdname - : EVP_MD_name(pdsactx->md))) + if (p != NULL && !OSSL_PARAM_set_utf8_string(p, pdsactx->mdname)) return 0; return 1; @@ -353,29 +387,29 @@ static int dsa_set_ctx_params(void *vpdsactx, const OSSL_PARAM params[]) { PROV_DSA_CTX *pdsactx = (PROV_DSA_CTX *)vpdsactx; const OSSL_PARAM *p; - char *mdname; if (pdsactx == NULL || params == NULL) return 0; - if (pdsactx->md != NULL) { - /* - * You cannot set the digest name/size when doing a DigestSign or - * DigestVerify. - */ - return 1; - } - - /* - * 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 = pdsactx->mdname; - if (p != NULL - && !OSSL_PARAM_get_utf8_string(p, &mdname, sizeof(pdsactx->mdname))) + /* Not allowed during certain operations */ + if (p != NULL && !pdsactx->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 (!dsa_setup_md(pdsactx, mdname, mdprops)) + return 0; + } return 1; } -- 2.25.1