EVP_DigestSign/VerifyFinal: Duplicate the pctx to allow multiple calls
authorTomas Mraz <tomas@openssl.org>
Wed, 25 Aug 2021 11:50:40 +0000 (13:50 +0200)
committerTomas Mraz <tomas@openssl.org>
Thu, 26 Aug 2021 14:06:57 +0000 (16:06 +0200)
The legacy implementation duplicates the pctx before creating/verifying
the signature unless EVP_MD_CTX_FLAG_FINALISE is set. We have to do the
same with provided implementations.

Fixes #16321

Reviewed-by: Dmitry Belyavskiy <beldmit@gmail.com>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16422)

crypto/evp/m_sigver.c
test/evp_extra_test.c

index f21865a8c3c0a71d6533d1e3e8089ec15064cf66..806ef3224cd3acf6781e25011a2f12f276e2cc49 100644 (file)
@@ -400,7 +400,7 @@ int EVP_DigestSignFinal(EVP_MD_CTX *ctx, unsigned char *sigret,
                         size_t *siglen)
 {
     int sctx = 0, r = 0;
-    EVP_PKEY_CTX *pctx = ctx->pctx;
+    EVP_PKEY_CTX *dctx, *pctx = ctx->pctx;
 
     if (pctx == NULL
             || pctx->operation != EVP_PKEY_OP_SIGNCTX
@@ -408,8 +408,19 @@ int EVP_DigestSignFinal(EVP_MD_CTX *ctx, unsigned char *sigret,
             || pctx->op.sig.signature == NULL)
         goto legacy;
 
-    return pctx->op.sig.signature->digest_sign_final(pctx->op.sig.algctx,
-                                                     sigret, siglen, SIZE_MAX);
+    if (sigret == NULL || (ctx->flags & EVP_MD_CTX_FLAG_FINALISE) != 0)
+        return pctx->op.sig.signature->digest_sign_final(pctx->op.sig.algctx,
+                                                         sigret, siglen,
+                                                         SIZE_MAX);
+    dctx = EVP_PKEY_CTX_dup(pctx);
+    if (dctx == NULL)
+        return 0;
+
+    r = dctx->op.sig.signature->digest_sign_final(dctx->op.sig.algctx,
+                                                  sigret, siglen,
+                                                  SIZE_MAX);
+    EVP_PKEY_CTX_free(dctx);
+    return r;
 
  legacy:
     if (pctx == NULL || pctx->pmeth == NULL) {
@@ -429,8 +440,7 @@ int EVP_DigestSignFinal(EVP_MD_CTX *ctx, unsigned char *sigret,
         if (ctx->flags & EVP_MD_CTX_FLAG_FINALISE)
             r = pctx->pmeth->signctx(pctx, sigret, siglen, ctx);
         else {
-            EVP_PKEY_CTX *dctx = EVP_PKEY_CTX_dup(pctx);
-
+            dctx = EVP_PKEY_CTX_dup(pctx);
             if (dctx == NULL)
                 return 0;
             r = dctx->pmeth->signctx(dctx, sigret, siglen, ctx);
@@ -516,7 +526,7 @@ int EVP_DigestVerifyFinal(EVP_MD_CTX *ctx, const unsigned char *sig,
     int r = 0;
     unsigned int mdlen = 0;
     int vctx = 0;
-    EVP_PKEY_CTX *pctx = ctx->pctx;
+    EVP_PKEY_CTX *dctx, *pctx = ctx->pctx;
 
     if (pctx == NULL
             || pctx->operation != EVP_PKEY_OP_VERIFYCTX
@@ -524,8 +534,17 @@ int EVP_DigestVerifyFinal(EVP_MD_CTX *ctx, const unsigned char *sig,
             || pctx->op.sig.signature == NULL)
         goto legacy;
 
-    return pctx->op.sig.signature->digest_verify_final(pctx->op.sig.algctx,
-                                                       sig, siglen);
+    if ((ctx->flags & EVP_MD_CTX_FLAG_FINALISE) != 0)
+        return pctx->op.sig.signature->digest_verify_final(pctx->op.sig.algctx,
+                                                           sig, siglen);
+    dctx = EVP_PKEY_CTX_dup(pctx);
+    if (dctx == NULL)
+        return 0;
+
+    r = dctx->op.sig.signature->digest_verify_final(dctx->op.sig.algctx,
+                                                    sig, siglen);
+    EVP_PKEY_CTX_free(dctx);
+    return r;
 
  legacy:
     if (pctx == NULL || pctx->pmeth == NULL) {
index bc02cea95d70224dd43d38576410ffd0e57665e9..83f8902d2482d2415ea3ffde0e7ad41f028f5cff 100644 (file)
@@ -1051,8 +1051,8 @@ static int test_EVP_DigestSignInit(int tst)
 {
     int ret = 0;
     EVP_PKEY *pkey = NULL;
-    unsigned char *sig = NULL;
-    size_t sig_len = 0;
+    unsigned char *sig = NULL, *sig2 = NULL;
+    size_t sig_len = 0, sig2_len = 0;
     EVP_MD_CTX *md_ctx = NULL, *md_ctx_verify = NULL;
     EVP_MD_CTX *a_md_ctx = NULL, *a_md_ctx_verify = NULL;
     BIO *mdbio = NULL, *membio = NULL;
@@ -1115,17 +1115,17 @@ static int test_EVP_DigestSignInit(int tst)
             || !TEST_true(EVP_DigestSignFinal(md_ctx, sig, &sig_len)))
         goto out;
 
-    if (tst >= 6) {
-        if (!TEST_int_gt(BIO_reset(mdbio), 0)
-                || !TEST_int_gt(BIO_get_md_ctx(mdbio, &md_ctx_verify), 0))
-            goto out;
-    }
-
     /*
      * Ensure that the signature round-trips (Verification isn't supported for
      * HMAC via EVP_DigestVerify*)
      */
     if (tst != 2 && tst != 5 && tst != 8) {
+        if (tst >= 6) {
+            if (!TEST_int_gt(BIO_reset(mdbio), 0)
+                || !TEST_int_gt(BIO_get_md_ctx(mdbio, &md_ctx_verify), 0))
+                goto out;
+        }
+
         if (!TEST_true(EVP_DigestVerifyInit(md_ctx_verify, NULL, md,
                                             NULL, pkey)))
             goto out;
@@ -1140,6 +1140,22 @@ static int test_EVP_DigestSignInit(int tst)
         }
         if (!TEST_true(EVP_DigestVerifyFinal(md_ctx_verify, sig, sig_len)))
             goto out;
+
+        /* Multiple calls to EVP_DigestVerifyFinal should work */
+        if (!TEST_true(EVP_DigestVerifyFinal(md_ctx_verify, sig, sig_len)))
+            goto out;
+    } else {
+        /*
+         * For HMAC a doubled call to DigestSignFinal should produce the same
+         * value as finalization should not happen.
+         */
+        if (!TEST_true(EVP_DigestSignFinal(md_ctx, NULL, &sig2_len))
+            || !TEST_ptr(sig2 = OPENSSL_malloc(sig2_len))
+            || !TEST_true(EVP_DigestSignFinal(md_ctx, sig2, &sig2_len)))
+            goto out;
+
+        if (!TEST_mem_eq(sig, sig_len, sig2, sig2_len))
+            goto out;
     }
 
     ret = 1;
@@ -1151,6 +1167,7 @@ static int test_EVP_DigestSignInit(int tst)
     EVP_MD_CTX_free(a_md_ctx_verify);
     EVP_PKEY_free(pkey);
     OPENSSL_free(sig);
+    OPENSSL_free(sig2);
     EVP_MD_free(mdexp);
 
     return ret;