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

diff --git a/lib/libcrypto/dh/dh_lib.c b/lib/libcrypto/dh/dh_lib.c
index 987f0b1..6e53df9 100644 (file)
--- a/lib/libcrypto/dh/dh_lib.c
+++ b/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/lib/libcrypto/dsa/dsa_lib.c b/lib/libcrypto/dsa/dsa_lib.c
index 46a7dbc..a9d2179 100644 (file)
--- a/lib/libcrypto/dsa/dsa_lib.c
+++ b/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);