sm2: Cleanup handling of DIGEST and DIGEST_SIZE parameters

Fixes #14873

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/15074)

diff --git a/providers/implementations/signature/sm2sig.c b/providers/implementations/signature/sm2sig.c
index 9016aefc029aad0968f7a36f657df18b90d561c8..8607a8b91162e44930f96147a9e774cf5d9ea65b 100644 (file)
--- a/providers/implementations/signature/sm2sig.c
+++ b/providers/implementations/signature/sm2sig.c
@@ -25,6 +25,7 @@
 #include "internal/nelem.h"
 #include "internal/sizes.h"
 #include "internal/cryptlib.h"
+#include "internal/sm3.h"
 #include "prov/implementations.h"
 #include "prov/provider_ctx.h"
 #include "crypto/ec.h"
@@ -63,13 +64,6 @@ typedef struct {
     char *propq;
     EC_KEY *ec;
 
-    /*
-     * 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;
     /*
      * Flag to termine if the 'z' digest needs to be computed and fed to the
      * hash function.
@@ -95,6 +89,21 @@ typedef struct {
     size_t id_len;
 } PROV_SM2_CTX;
 
+static int sm2sig_set_mdname(PROV_SM2_CTX *psm2ctx, const char *mdname)
+{
+    if (psm2ctx->md == NULL) /* We need an SM3 md to compare with */
+        psm2ctx->md = EVP_MD_fetch(psm2ctx->libctx, psm2ctx->mdname,
+                                   psm2ctx->propq);
+    if (psm2ctx->md == NULL
+        || strlen(mdname) >= sizeof(psm2ctx->mdname)
+        || !EVP_MD_is_a(psm2ctx->md, mdname)) {
+        return 0;
+    }
+
+    OPENSSL_strlcpy(psm2ctx->mdname, mdname, sizeof(psm2ctx->mdname));
+    return 1;
+}
+
 static void *sm2sig_newctx(void *provctx, const char *propq)
 {
     PROV_SM2_CTX *ctx = OPENSSL_zalloc(sizeof(PROV_SM2_CTX));
@@ -108,8 +117,8 @@ static void *sm2sig_newctx(void *provctx, const char *propq)
         ERR_raise(ERR_LIB_PROV, ERR_R_MALLOC_FAILURE);
         return NULL;
     }
-    /* don't allow to change MD, and in fact there is no such need */
-    ctx->flag_allow_md = 0;
+    ctx->mdsize = SM3_DIGEST_LENGTH;
+    strcpy(ctx->mdname, OSSL_DIGEST_NAME_SM3);
     return ctx;
 }
 
@@ -170,28 +179,27 @@ static void free_md(PROV_SM2_CTX *ctx)
     EVP_MD_free(ctx->md);
     ctx->mdctx = NULL;
     ctx->md = NULL;
-    ctx->mdsize = 0;
 }
 
 static int sm2sig_digest_signverify_init(void *vpsm2ctx, const char *mdname,
                                          void *ec, const OSSL_PARAM params[])
 {
     PROV_SM2_CTX *ctx = (PROV_SM2_CTX *)vpsm2ctx;
-    int md_nid = NID_sm3;
+    int md_nid;
     WPACKET pkt;
     int ret = 0;
 
-    free_md(ctx);
-
-    if (!sm2sig_signature_init(vpsm2ctx, ec, params))
+    if (!sm2sig_signature_init(vpsm2ctx, ec, params)
+        || !sm2sig_set_mdname(ctx, mdname))
         return ret;
 
-    ctx->md = EVP_MD_fetch(ctx->libctx, mdname, ctx->propq);
-    ctx->mdsize = EVP_MD_size(ctx->md);
+    EVP_MD_CTX_free(ctx->mdctx);
     ctx->mdctx = EVP_MD_CTX_new();
     if (ctx->mdctx == NULL)
         goto error;
 
+    md_nid = EVP_MD_type(ctx->md);
+
     /*
      * We do not care about DER writing errors.
      * All it really means is that for some reason, there's no
@@ -285,10 +293,11 @@ int sm2sig_digest_verify_final(void *vpsm2ctx, const unsigned char *sig,
     unsigned char digest[EVP_MAX_MD_SIZE];
     unsigned int dlen = 0;
 
-    if (psm2ctx == NULL || psm2ctx->mdctx == NULL)
+    if (psm2ctx == NULL
+        || psm2ctx->mdctx == NULL
+        || EVP_MD_size(psm2ctx->md) > (int)sizeof(digest))
         return 0;
 
-    /* SM2 always use SM3 so it's not possible to exceed the limit */
     if (!(sm2sig_compute_z_digest(psm2ctx)
           && EVP_DigestFinal_ex(psm2ctx->mdctx, digest, &dlen)))
         return 0;
@@ -392,7 +401,7 @@ static int sm2sig_set_ctx_params(void *vpsm2ctx, const OSSL_PARAM params[])
 {
     PROV_SM2_CTX *psm2ctx = (PROV_SM2_CTX *)vpsm2ctx;
     const OSSL_PARAM *p;
-    char *mdname;
+    size_t mdsize;
 
     if (psm2ctx == NULL)
         return 0;
@@ -417,28 +426,29 @@ static int sm2sig_set_ctx_params(void *vpsm2ctx, const OSSL_PARAM params[])
         psm2ctx->id_len = tmp_idlen;
     }
 
-    if (psm2ctx->md != NULL) {
-        /*
-         * You cannot set the digest name/size when doing a DigestSign or
-         * DigestVerify.
-         */
-        return 1;
-    }
-
+    /*
+     * The following code checks that the size is the same as the SM3 digest
+     * size returning an error otherwise.
+     * If there is ever any different digest algorithm allowed with SM2
+     * this needs to be adjusted accordingly.
+     */
     p = OSSL_PARAM_locate_const(params, OSSL_SIGNATURE_PARAM_DIGEST_SIZE);
-    if (p != NULL && !OSSL_PARAM_get_size_t(p, &psm2ctx->mdsize))
+    if (p != NULL && (!OSSL_PARAM_get_size_t(p, &mdsize)
+                      || mdsize != psm2ctx->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 = psm2ctx->mdname;
-    if (p != NULL
-            && !OSSL_PARAM_get_utf8_string(p, &mdname, sizeof(psm2ctx->mdname)))
-        return 0;
+    if (p != NULL) {
+        char *mdname = NULL;
+
+        if (!OSSL_PARAM_get_utf8_string(p, &mdname, 0))
+            return 0;
+        if (!sm2sig_set_mdname(psm2ctx, mdname)) {
+            OPENSSL_free(mdname);
+            return 0;
+        }
+        OPENSSL_free(mdname);
+    }
 
     return 1;
 }
@@ -453,12 +463,6 @@ static const OSSL_PARAM known_settable_ctx_params[] = {
 static const OSSL_PARAM *sm2sig_settable_ctx_params(ossl_unused void *vpsm2ctx,
                                                     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;
 }