[openssl] master update
shane.lontis at oracle.com
shane.lontis at oracle.com
Thu Dec 3 22:37:14 UTC 2020
The branch master has been updated
via c2386b81feae22786502abb99b3b39f85e66d8a1 (commit)
via 283320281bda4e1b5cfaedb0db723a96325aabb9 (commit)
from ddfd7182cf2b7e69669cf4fd3471a37d09af4ea1 (commit)
- Log -----------------------------------------------------------------
commit c2386b81feae22786502abb99b3b39f85e66d8a1
Author: Shane Lontis <shane.lontis at oracle.com>
Date: Thu Nov 26 15:06:34 2020 +1000
Fix dsa & rsa signature dupctx() so that ctx->propq is strduped
Discovered when fixing up ecdsa code.
Reviewed-by: Matt Caswell <matt at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13520)
commit 283320281bda4e1b5cfaedb0db723a96325aabb9
Author: Shane Lontis <shane.lontis at oracle.com>
Date: Thu Nov 26 15:03:10 2020 +1000
Fix ecdsa digest setting code to match dsa.
Fixes #13422
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 at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13520)
-----------------------------------------------------------------------
Summary of changes:
providers/implementations/signature/dsa.c | 8 +-
providers/implementations/signature/ecdsa.c | 181 ++++++++++++++----------
providers/implementations/signature/rsa.c | 7 +
test/recipes/30-test_evp_data/evppkey_ecdsa.txt | 21 +++
4 files changed, 139 insertions(+), 78 deletions(-)
diff --git a/providers/implementations/signature/dsa.c b/providers/implementations/signature/dsa.c
index a1621acf62..515845c56c 100644
--- a/providers/implementations/signature/dsa.c
+++ b/providers/implementations/signature/dsa.c
@@ -85,7 +85,6 @@ typedef struct {
/* main digest */
EVP_MD *md;
EVP_MD_CTX *mdctx;
- size_t mdsize;
int operation;
} PROV_DSA_CTX;
@@ -361,7 +360,6 @@ static void dsa_freectx(void *vpdsactx)
ctx->propq = NULL;
ctx->mdctx = NULL;
ctx->md = NULL;
- ctx->mdsize = 0;
DSA_free(ctx->dsa);
OPENSSL_free(ctx);
}
@@ -382,6 +380,7 @@ static void *dsa_dupctx(void *vpdsactx)
dstctx->dsa = NULL;
dstctx->md = NULL;
dstctx->mdctx = NULL;
+ dstctx->propq = NULL;
if (srcctx->dsa != NULL && !DSA_up_ref(srcctx->dsa))
goto err;
@@ -397,6 +396,11 @@ static void *dsa_dupctx(void *vpdsactx)
|| !EVP_MD_CTX_copy_ex(dstctx->mdctx, srcctx->mdctx))
goto err;
}
+ if (srcctx->propq != NULL) {
+ dstctx->propq = OPENSSL_strdup(srcctx->propq);
+ if (dstctx->propq == NULL)
+ goto err;
+ }
return dstctx;
err:
diff --git a/providers/implementations/signature/ecdsa.c b/providers/implementations/signature/ecdsa.c
index b956917e49..28e8b46ac7 100644
--- a/providers/implementations/signature/ecdsa.c
+++ b/providers/implementations/signature/ecdsa.c
@@ -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,65 +196,85 @@ 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->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,
+ md_nid)
+ && WPACKET_finish(&pkt)) {
+ WPACKET_get_total_written(&pkt, &ctx->aid_len);
+ ctx->aid = WPACKET_get_curr(&pkt);
+ }
+ WPACKET_cleanup(&pkt);
ctx->mdctx = NULL;
- ctx->md = NULL;
- ctx->mdsize = 0;
+ 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;
- 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))
+ ctx->flag_allow_md = 0;
+ if (!ecdsa_signverify_init(vctx, ec, operation)
+ || !ecdsa_setup_md(ctx, mdname, NULL))
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,
- md_nid)
- && WPACKET_finish(&pkt)) {
- WPACKET_get_total_written(&pkt, &ctx->aid_len);
- ctx->aid = WPACKET_get_curr(&pkt);
- }
- WPACKET_cleanup(&pkt);
-
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))
+ p = OSSL_PARAM_locate_const(params, OSSL_SIGNATURE_PARAM_DIGEST);
+ /* 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;
+ }
- /*
- * 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;
+ 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;
}
diff --git a/providers/implementations/signature/rsa.c b/providers/implementations/signature/rsa.c
index b463f03d7f..98ebf6b243 100644
--- a/providers/implementations/signature/rsa.c
+++ b/providers/implementations/signature/rsa.c
@@ -870,6 +870,7 @@ static void *rsa_dupctx(void *vprsactx)
dstctx->md = NULL;
dstctx->mdctx = NULL;
dstctx->tbuf = NULL;
+ dstctx->propq = NULL;
if (srcctx->rsa != NULL && !RSA_up_ref(srcctx->rsa))
goto err;
@@ -890,6 +891,12 @@ static void *rsa_dupctx(void *vprsactx)
goto err;
}
+ if (srcctx->propq != NULL) {
+ dstctx->propq = OPENSSL_strdup(srcctx->propq);
+ if (dstctx->propq == NULL)
+ goto err;
+ }
+
return dstctx;
err:
rsa_freectx(dstctx);
diff --git a/test/recipes/30-test_evp_data/evppkey_ecdsa.txt b/test/recipes/30-test_evp_data/evppkey_ecdsa.txt
index 5bd68726ce..f09edd9032 100644
--- a/test/recipes/30-test_evp_data/evppkey_ecdsa.txt
+++ b/test/recipes/30-test_evp_data/evppkey_ecdsa.txt
@@ -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
More information about the openssl-commits
mailing list