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 <shane.lontis@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/10947)

diff --git a/include/openssl/core_names.h b/include/openssl/core_names.h
index c061902b8ca8bdb01176a4f45f83fe4edbfb345e..c17eee42a5b47193cb26d958c3f5c6db1b401e33 100644 (file)
--- 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 eaf6d4fe29bd96080e0d83a770f12aeae177fc04..b2309ef510e5ccd0e1c67f4ba5a5081962a22de1 100644 (file)
--- a/providers/implementations/signature/dsa.c
+++ b/providers/implementations/signature/dsa.c
@@ -16,10 +16,12 @@
 #include <openssl/dsa.h>
 #include <openssl/params.h>
 #include <openssl/evp.h>
+#include <openssl/err.h>
 #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;
 }