From 8795d8912c8a83aaf900c0260e252a35f64eb200 Mon Sep 17 00:00:00 2001
From: Norbert Pocs <npocs@redhat.com>
Date: Fri, 18 Nov 2022 17:22:46 +0100
Subject: [PATCH] Fix memory leaks of bignums when openssl >= 3.0
The openssl 3.0 support has introduced some memory leaks at key build as
OSSL_PARAM_BLD_push_BN duplicates the bignum and does not save the pointer
itself.
Signed-off-by: Norbert Pocs <npocs@redhat.com>
Reviewed-by: Jakub Jelen <jjelen@redhat.com>
---
include/libssh/dh.h | 2 +-
src/dh_crypto.c | 28 ++---
src/pki_crypto.c | 262 ++++++++++++++++++++++++--------------------
3 files changed, 151 insertions(+), 141 deletions(-)
diff --git a/include/libssh/dh.h b/include/libssh/dh.h
index 353dc233..9b9bb472 100644
--- a/include/libssh/dh.h
+++ b/include/libssh/dh.h
@@ -53,7 +53,7 @@ int ssh_dh_keypair_get_keys(struct dh_ctx *ctx, int peer,
bignum *priv, bignum *pub);
#endif /* OPENSSL_VERSION_NUMBER */
int ssh_dh_keypair_set_keys(struct dh_ctx *ctx, int peer,
- const bignum priv, const bignum pub);
+ bignum priv, bignum pub);
int ssh_dh_compute_shared_secret(struct dh_ctx *ctx, int local, int remote,
bignum *dest);
diff --git a/src/dh_crypto.c b/src/dh_crypto.c
index a847c6a2..b578ddec 100644
--- a/src/dh_crypto.c
+++ b/src/dh_crypto.c
@@ -154,12 +154,9 @@ int ssh_dh_keypair_get_keys(struct dh_ctx *ctx, int peer,
#endif /* OPENSSL_VERSION_NUMBER */
int ssh_dh_keypair_set_keys(struct dh_ctx *ctx, int peer,
- const bignum priv, const bignum pub)
+ bignum priv, bignum pub)
{
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
- bignum priv_key = NULL;
- bignum pub_key = NULL;
-#else
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
int rc;
OSSL_PARAM *params = NULL, *out_params = NULL, *merged_params = NULL;
OSSL_PARAM_BLD *param_bld = NULL;
@@ -172,7 +169,11 @@ int ssh_dh_keypair_set_keys(struct dh_ctx *ctx, int peer,
return SSH_ERROR;
}
-#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
+ (void)DH_set0_key(ctx->keypair[peer], pub, priv);
+
+ return SSH_OK;
+#else
rc = EVP_PKEY_todata(ctx->keypair[peer], EVP_PKEY_KEYPAIR, &out_params);
if (rc != 1) {
return SSH_ERROR;
@@ -195,35 +196,22 @@ int ssh_dh_keypair_set_keys(struct dh_ctx *ctx, int peer,
rc = SSH_ERROR;
goto out;
}
-#endif /* OPENSSL_VERSION_NUMBER */
if (priv) {
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
- priv_key = priv;
-#else
rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_PRIV_KEY, priv);
if (rc != 1) {
rc = SSH_ERROR;
goto out;
}
-#endif /* OPENSSL_VERSION_NUMBER */
}
if (pub) {
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
- pub_key = pub;
-#else
rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_PUB_KEY, pub);
if (rc != 1) {
rc = SSH_ERROR;
goto out;
}
-#endif /* OPENSSL_VERSION_NUMBER */
}
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
- (void)DH_set0_key(ctx->keypair[peer], pub_key, priv_key);
- return SSH_OK;
-#else
params = OSSL_PARAM_BLD_to_param(param_bld);
if (params == NULL) {
rc = SSH_ERROR;
@@ -248,6 +236,8 @@ int ssh_dh_keypair_set_keys(struct dh_ctx *ctx, int peer,
rc = SSH_OK;
out:
+ bignum_safe_free(priv);
+ bignum_safe_free(pub);
EVP_PKEY_CTX_free(evp_ctx);
OSSL_PARAM_free(out_params);
OSSL_PARAM_free(params);
diff --git a/src/pki_crypto.c b/src/pki_crypto.c
index 0a5003da..d3359e2d 100644
--- a/src/pki_crypto.c
+++ b/src/pki_crypto.c
@@ -1492,18 +1492,18 @@ int pki_privkey_build_dss(ssh_key key,
ssh_string privkey)
{
int rc;
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
BIGNUM *bp, *bq, *bg, *bpub_key, *bpriv_key;
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ OSSL_PARAM_BLD *param_bld = OSSL_PARAM_BLD_new();
+ if (param_bld == NULL) {
+ return SSH_ERROR;
+ }
#else
- const BIGNUM *pb, *qb, *gb, *pubb, *privb;
- OSSL_PARAM_BLD *param_bld;
-#endif /* OPENSSL_VERSION_NUMBER */
-
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
key->dsa = DSA_new();
if (key->dsa == NULL) {
return SSH_ERROR;
}
+#endif /* OPENSSL_VERSION_NUMBER */
bp = ssh_make_string_bn(p);
bq = ssh_make_string_bn(q);
@@ -1512,9 +1512,11 @@ int pki_privkey_build_dss(ssh_key key,
bpriv_key = ssh_make_string_bn(privkey);
if (bp == NULL || bq == NULL ||
bg == NULL || bpub_key == NULL) {
+ rc = SSH_ERROR;
goto fail;
}
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
/* Memory management of bp, qq and bg is transferred to DSA object */
rc = DSA_set0_pqg(key->dsa, bp, bq, bg);
if (rc == 0) {
@@ -1532,39 +1534,43 @@ fail:
DSA_free(key->dsa);
return SSH_ERROR;
#else
- param_bld = OSSL_PARAM_BLD_new();
- if (param_bld == NULL)
- goto err;
-
- pb = ssh_make_string_bn(p);
- qb = ssh_make_string_bn(q);
- gb = ssh_make_string_bn(g);
- pubb = ssh_make_string_bn(pubkey);
- privb = ssh_make_string_bn(privkey);
-
- rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_P, pb);
- if (rc != 1)
- goto err;
- rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_Q, qb);
- if (rc != 1)
- goto err;
- rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_G, gb);
- if (rc != 1)
- goto err;
- rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_PUB_KEY, pubb);
- if (rc != 1)
- goto err;
- rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_PRIV_KEY, privb);
- if (rc != 1)
- goto err;
+ rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_P, bp);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
+ rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_Q, bq);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
+ rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_G, bg);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
+ rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_PUB_KEY, bpub_key);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
+ rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_PRIV_KEY, bpriv_key);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
rc = evp_build_pkey("DSA", param_bld, &(key->key), EVP_PKEY_KEYPAIR);
+
+fail:
OSSL_PARAM_BLD_free(param_bld);
+ bignum_safe_free(bp);
+ bignum_safe_free(bq);
+ bignum_safe_free(bg);
+ bignum_safe_free(bpub_key);
+ bignum_safe_free(bpriv_key);
return rc;
-err:
- OSSL_PARAM_BLD_free(param_bld);
- return -1;
#endif /* OPENSSL_VERSION_NUMBER */
}
@@ -1574,18 +1580,18 @@ int pki_pubkey_build_dss(ssh_key key,
ssh_string g,
ssh_string pubkey) {
int rc;
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
BIGNUM *bp = NULL, *bq = NULL, *bg = NULL, *bpub_key = NULL;
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ OSSL_PARAM_BLD *param_bld = OSSL_PARAM_BLD_new();
+ if (param_bld == NULL) {
+ return SSH_ERROR;
+ }
#else
- const BIGNUM *pb, *qb, *gb, *pubb;
- OSSL_PARAM_BLD *param_bld;
-#endif /* OPENSSL_VERSION_NUMBER */
-
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
key->dsa = DSA_new();
if (key->dsa == NULL) {
return SSH_ERROR;
}
+#endif /* OPENSSL_VERSION_NUMBER */
bp = ssh_make_string_bn(p);
bq = ssh_make_string_bn(q);
@@ -1593,9 +1599,11 @@ int pki_pubkey_build_dss(ssh_key key,
bpub_key = ssh_make_string_bn(pubkey);
if (bp == NULL || bq == NULL ||
bg == NULL || bpub_key == NULL) {
+ rc = SSH_ERROR;
goto fail;
}
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
/* Memory management of bp, bq and bg is transferred to DSA object */
rc = DSA_set0_pqg(key->dsa, bp, bq, bg);
if (rc == 0) {
@@ -1613,35 +1621,37 @@ fail:
DSA_free(key->dsa);
return SSH_ERROR;
#else
- param_bld = OSSL_PARAM_BLD_new();
- if (param_bld == NULL)
- goto err;
-
- pb = ssh_make_string_bn(p);
- qb = ssh_make_string_bn(q);
- gb = ssh_make_string_bn(g);
- pubb = ssh_make_string_bn(pubkey);
-
- rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_P, pb);
- if (rc != 1)
- goto err;
- rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_Q, qb);
- if (rc != 1)
- goto err;
- rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_G, gb);
- if (rc != 1)
- goto err;
- rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_PUB_KEY, pubb);
- if (rc != 1)
- goto err;
+ rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_P, bp);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
+ rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_Q, bq);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
+ rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_G, bg);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
+ rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_PUB_KEY, bpub_key);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
rc = evp_build_pkey("DSA", param_bld, &(key->key), EVP_PKEY_PUBLIC_KEY);
+
+fail:
OSSL_PARAM_BLD_free(param_bld);
+ bignum_safe_free(bp);
+ bignum_safe_free(bq);
+ bignum_safe_free(bg);
+ bignum_safe_free(bpub_key);
return rc;
-err:
- OSSL_PARAM_BLD_free(param_bld);
- return -1;
#endif /* OPENSSL_VERSION_NUMBER */
}
@@ -1654,18 +1664,18 @@ int pki_privkey_build_rsa(ssh_key key,
ssh_string q)
{
int rc;
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
BIGNUM *be, *bn, *bd/*, *biqmp*/, *bp, *bq;
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ OSSL_PARAM_BLD *param_bld = OSSL_PARAM_BLD_new();
+ if (param_bld == NULL) {
+ return SSH_ERROR;
+ }
#else
- const BIGNUM *nb, *eb, *db, *pb, *qb;
- OSSL_PARAM_BLD *param_bld;
-#endif /* OPENSSL_VERSION_NUMBER */
-
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
key->rsa = RSA_new();
if (key->rsa == NULL) {
return SSH_ERROR;
}
+#endif /* OPENSSL_VERSION_NUMBER */
bn = ssh_make_string_bn(n);
be = ssh_make_string_bn(e);
@@ -1675,9 +1685,11 @@ int pki_privkey_build_rsa(ssh_key key,
bq = ssh_make_string_bn(q);
if (be == NULL || bn == NULL || bd == NULL ||
/*biqmp == NULL ||*/ bp == NULL || bq == NULL) {
+ rc = SSH_ERROR;
goto fail;
}
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
/* Memory management of be, bn and bd is transferred to RSA object */
rc = RSA_set0_key(key->rsa, bn, be, bd);
if (rc == 0) {
@@ -1702,41 +1714,49 @@ fail:
RSA_free(key->rsa);
return SSH_ERROR;
#else
- param_bld = OSSL_PARAM_BLD_new();
- if (param_bld == NULL)
- goto err;
-
- nb = ssh_make_string_bn(n);
- eb = ssh_make_string_bn(e);
- db = ssh_make_string_bn(d);
- pb = ssh_make_string_bn(p);
- qb = ssh_make_string_bn(q);
-
- rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_N, nb);
- if (rc != 1)
- goto err;
- rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_E, eb);
- if (rc != 1)
- goto err;
- rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_D, db);
- if (rc != 1)
- goto err;
+ rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_N, bn);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
+ rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_E, be);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
+ rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_D, bd);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
rc = evp_build_pkey("RSA", param_bld, &(key->key), EVP_PKEY_KEYPAIR);
- OSSL_PARAM_BLD_free(param_bld);
+ if (rc != SSH_OK) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
- rc = EVP_PKEY_set_bn_param(key->key, OSSL_PKEY_PARAM_RSA_FACTOR1, pb);
- if (rc != 1)
- goto err;
+ rc = EVP_PKEY_set_bn_param(key->key, OSSL_PKEY_PARAM_RSA_FACTOR1, bp);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
- rc = EVP_PKEY_set_bn_param(key->key, OSSL_PKEY_PARAM_RSA_FACTOR2, qb);
- if (rc != 1)
- goto err;
+ rc = EVP_PKEY_set_bn_param(key->key, OSSL_PKEY_PARAM_RSA_FACTOR2, bq);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
- return rc;
-err:
+fail:
OSSL_PARAM_BLD_free(param_bld);
- return -1;
+ bignum_safe_free(bn);
+ bignum_safe_free(be);
+ bignum_safe_free(bd);
+ bignum_safe_free(bp);
+ bignum_safe_free(bq);
+
+ return rc;
#endif /* OPENSSL_VERSION_NUMBER */
}
@@ -1744,25 +1764,27 @@ int pki_pubkey_build_rsa(ssh_key key,
ssh_string e,
ssh_string n) {
int rc;
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
BIGNUM *be = NULL, *bn = NULL;
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
+ OSSL_PARAM_BLD *param_bld = OSSL_PARAM_BLD_new();
+ if (param_bld == NULL) {
+ return SSH_ERROR;
+ }
#else
- const BIGNUM *eb, *nb;
- OSSL_PARAM_BLD *param_bld;
-#endif /* OPENSSL_VERSION_NUMBER */
-
-#if OPENSSL_VERSION_NUMBER < 0x30000000L
key->rsa = RSA_new();
if (key->rsa == NULL) {
return SSH_ERROR;
}
+#endif /* OPENSSL_VERSION_NUMBER */
be = ssh_make_string_bn(e);
bn = ssh_make_string_bn(n);
if (be == NULL || bn == NULL) {
+ rc = SSH_ERROR;
goto fail;
}
+#if OPENSSL_VERSION_NUMBER < 0x30000000L
/* Memory management of bn and be is transferred to RSA object */
rc = RSA_set0_key(key->rsa, bn, be, NULL);
if (rc == 0) {
@@ -1774,27 +1796,25 @@ fail:
RSA_free(key->rsa);
return SSH_ERROR;
#else
- nb = ssh_make_string_bn(n);
- eb = ssh_make_string_bn(e);
-
- param_bld = OSSL_PARAM_BLD_new();
- if (param_bld == NULL)
- goto err;
-
- rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_N, nb);
- if (rc != 1)
- goto err;
- rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_E, eb);
- if (rc != 1)
- goto err;
+ rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_N, bn);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
+ rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_E, be);
+ if (rc != 1) {
+ rc = SSH_ERROR;
+ goto fail;
+ }
rc = evp_build_pkey("RSA", param_bld, &(key->key), EVP_PKEY_PUBLIC_KEY);
+
+fail:
OSSL_PARAM_BLD_free(param_bld);
+ bignum_safe_free(bn);
+ bignum_safe_free(be);
return rc;
-err:
- OSSL_PARAM_BLD_free(param_bld);
- return -1;
#endif /* OPENSSL_VERSION_NUMBER */
}
--
2.38.1