Convert {DH,DSA}_new_method() to using calloc()

Due to OPENSSL_NO_ENGINE the engine member of dh and dsa is currently
uninitialized. As a consequence, {DH,DSA}_get0_engine() will return a
garbage pointer, which is particularly bad because the only reason we
kept them in the first place is that they are used by some software...

A side effect of freeing with {DH,DSA}_free() instead of a hand-rolled
version is that we may call ->meth->finish() before ->meth->init() was
called. We need a NULL check for ->meth to be on the safe side in case
we should need to bring ENGINE back.

with nits from djm
ok deraadt djm

2 files changed, 54 insertions, 83 deletions

diff --git a/src/lib/libcrypto/dh/dh_lib.c b/src/lib/libcrypto/dh/dh_lib.c
index 987f0b1f7a..6e53df9177 100644
--- a/src/lib/libcrypto/dh/dh_lib.c
+++ b/src/lib/libcrypto/dh/dh_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: dh_lib.c,v 1.39 2023/07/08 15:29:03 beck Exp $ */
+/* $OpenBSD: dh_lib.c,v 1.40 2023/08/12 06:14:36 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -122,61 +122,47 @@ LCRYPTO_ALIAS(DH_new);
 DH *
 DH_new_method(ENGINE *engine)
 {
-        DH *ret;
+        DH *dh;
 
-        ret = malloc(sizeof(DH));
-        if (ret == NULL) {
+        if ((dh = calloc(1, sizeof(*dh))) == NULL) {
                 DHerror(ERR_R_MALLOC_FAILURE);
-                return NULL;
+                goto err;
         }
 
-        ret->meth = DH_get_default_method();
+        dh->meth = DH_get_default_method();
+        dh->flags = dh->meth->flags & ~DH_FLAG_NON_FIPS_ALLOW;
+        dh->references = 1;
+
 #ifndef OPENSSL_NO_ENGINE
-        if (engine) {
+        if (engine != NULL) {
                 if (!ENGINE_init(engine)) {
                         DHerror(ERR_R_ENGINE_LIB);
-                        free(ret);
-                        return NULL;
+                        goto err;
                 }
-                ret->engine = engine;
+                dh->engine = engine;
         } else
-                ret->engine = ENGINE_get_default_DH();
-        if(ret->engine) {
-                ret->meth = ENGINE_get_DH(ret->engine);
-                if (ret->meth == NULL) {
+                dh->engine = ENGINE_get_default_DH();
+        if (dh->engine != NULL) {
+                if ((dh->meth = ENGINE_get_DH(dh->engine)) == NULL) {
                         DHerror(ERR_R_ENGINE_LIB);
-                        ENGINE_finish(ret->engine);
-                        free(ret);
-                        return NULL;
+                        goto err;
                 }
+                dh->flags = dh->meth->flags & ~DH_FLAG_NON_FIPS_ALLOW;
         }
 #endif
 
-        ret->pad = 0;
-        ret->version = 0;
-        ret->p = NULL;
-        ret->g = NULL;
-        ret->length = 0;
-        ret->pub_key = NULL;
-        ret->priv_key = NULL;
-        ret->q = NULL;
-        ret->j = NULL;
-        ret->seed = NULL;
-        ret->seedlen = 0;
-        ret->counter = NULL;
-        ret->method_mont_p=NULL;
-        ret->references = 1;
-        ret->flags = ret->meth->flags & ~DH_FLAG_NON_FIPS_ALLOW;
-        CRYPTO_new_ex_data(CRYPTO_EX_INDEX_DH, ret, &ret->ex_data);
-        if (ret->meth->init != NULL && !ret->meth->init(ret)) {
-#ifndef OPENSSL_NO_ENGINE
-                ENGINE_finish(ret->engine);
-#endif
-                CRYPTO_free_ex_data(CRYPTO_EX_INDEX_DH, ret, &ret->ex_data);
-                free(ret);
-                ret = NULL;
-        }
-        return ret;
+        if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_DH, dh, &dh->ex_data))
+                goto err;
+
+        if (dh->meth->init != NULL && !dh->meth->init(dh))
+                goto err;
+
+        return dh;
+
+ err:
+        DH_free(dh);
+
+        return NULL;
 }
 LCRYPTO_ALIAS(DH_new_method);
 
@@ -191,7 +177,7 @@ DH_free(DH *r)
         if (i > 0)
                 return;
 
-        if (r->meth->finish)
+        if (r->meth != NULL && r->meth->finish != NULL)
                 r->meth->finish(r);
 #ifndef OPENSSL_NO_ENGINE
         ENGINE_finish(r->engine);
diff --git a/src/lib/libcrypto/dsa/dsa_lib.c b/src/lib/libcrypto/dsa/dsa_lib.c
index 46a7dbcfbe..a9d2179aeb 100644
--- a/src/lib/libcrypto/dsa/dsa_lib.c
+++ b/src/lib/libcrypto/dsa/dsa_lib.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: dsa_lib.c,v 1.43 2023/07/08 14:28:15 beck Exp $ */
+/* $OpenBSD: dsa_lib.c,v 1.44 2023/08/12 06:14:36 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -127,61 +127,46 @@ LCRYPTO_ALIAS(DSA_set_method);
 DSA *
 DSA_new_method(ENGINE *engine)
 {
-        DSA *ret;
+        DSA *dsa;
 
-        ret = malloc(sizeof(DSA));
-        if (ret == NULL) {
+        if ((dsa = calloc(1, sizeof(DSA))) == NULL) {
                 DSAerror(ERR_R_MALLOC_FAILURE);
-                return NULL;
+                goto err;
         }
-        ret->meth = DSA_get_default_method();
+
+        dsa->meth = DSA_get_default_method();
+        dsa->flags = dsa->meth->flags & ~DSA_FLAG_NON_FIPS_ALLOW;
+        dsa->references = 1;
+
 #ifndef OPENSSL_NO_ENGINE
         if (engine) {
                 if (!ENGINE_init(engine)) {
                         DSAerror(ERR_R_ENGINE_LIB);
-                        free(ret);
-                        return NULL;
+                        goto err;
                 }
-                ret->engine = engine;
+                dsa->engine = engine;
         } else
-                ret->engine = ENGINE_get_default_DSA();
-        if (ret->engine) {
-                ret->meth = ENGINE_get_DSA(ret->engine);
-                if (ret->meth == NULL) {
+                dsa->engine = ENGINE_get_default_DSA();
+        if (dsa->engine != NULL) {
+                if ((dsa->meth = ENGINE_get_DSA(dsa->engine)) == NULL) {
                         DSAerror(ERR_R_ENGINE_LIB);
-                        ENGINE_finish(ret->engine);
-                        free(ret);
-                        return NULL;
+                        goto err;
                 }
+                dsa->flags = dsa->meth->flags & ~DSA_FLAG_NON_FIPS_ALLOW;
         }
 #endif
 
-        ret->pad = 0;
-        ret->version = 0;
-        ret->p = NULL;
-        ret->q = NULL;
-        ret->g = NULL;
-
-        ret->pub_key = NULL;
-        ret->priv_key = NULL;
+        if (!CRYPTO_new_ex_data(CRYPTO_EX_INDEX_DSA, dsa, &dsa->ex_data))
+                goto err;
+        if (dsa->meth->init != NULL && !dsa->meth->init(dsa))
+                goto err;
 
-        ret->kinv = NULL;
-        ret->r = NULL;
-        ret->method_mont_p = NULL;
+        return dsa;
 
-        ret->references = 1;
-        ret->flags = ret->meth->flags & ~DSA_FLAG_NON_FIPS_ALLOW;
-        CRYPTO_new_ex_data(CRYPTO_EX_INDEX_DSA, ret, &ret->ex_data);
-        if (ret->meth->init != NULL && !ret->meth->init(ret)) {
-#ifndef OPENSSL_NO_ENGINE
-                ENGINE_finish(ret->engine);
-#endif
-                CRYPTO_free_ex_data(CRYPTO_EX_INDEX_DSA, ret, &ret->ex_data);
-                free(ret);
-                ret = NULL;
-        }
+ err:
+        DSA_free(dsa);
 
-        return ret;
+        return NULL;
 }
 LCRYPTO_ALIAS(DSA_new_method);
 
@@ -197,7 +182,7 @@ DSA_free(DSA *r)
         if (i > 0)
                 return;
 
-        if (r->meth->finish)
+        if (r->meth != NULL && r->meth->finish != NULL)
                 r->meth->finish(r);
 #ifndef OPENSSL_NO_ENGINE
         ENGINE_finish(r->engine);