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@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/13520)

diff --git a/providers/implementations/signature/ecdsa.c b/providers/implementations/signature/ecdsa.c
index b956917e494d11c129a74972645c32905eebb715..28e8b46ac7d003de6eb0532d00892f4957137661 100644 (file)
--- 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/test/recipes/30-test_evp_data/evppkey_ecdsa.txt b/test/recipes/30-test_evp_data/evppkey_ecdsa.txt
index 5bd68726cee579bb06309f82c4e3fdaeea87be72..f09edd90328b44bf1f5221ce69060848b595b47c 100644 (file)
--- 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