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

@@ -57,7 +57,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);

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
+#if OPENSSL_VERSION_NUMBER >= 0x30000000L
-    bignum priv_key = NULL;
-    bignum pub_key = NULL;
-#else
     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);

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();
+    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_P, bp);
-    if (param_bld == NULL)
+    if (rc != 1) {
-        goto err;
+        rc = SSH_ERROR;
-
+        goto fail;
-    pb = ssh_make_string_bn(p);
+    }
-    qb = ssh_make_string_bn(q);
+    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_Q, bq);
-    gb = ssh_make_string_bn(g);
+    if (rc != 1) {
-    pubb = ssh_make_string_bn(pubkey);
+        rc = SSH_ERROR;
-    privb = ssh_make_string_bn(privkey);
+        goto fail;
-
+    }
-    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_P, pb);
+    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_G, bg);
-    if (rc != 1)
+    if (rc != 1) {
-        goto err;
+        rc = SSH_ERROR;
-    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_Q, qb);
+        goto fail;
-    if (rc != 1)
+    }
-        goto err;
+    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_PUB_KEY, bpub_key);
-    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_G, gb);
+    if (rc != 1) {
-    if (rc != 1)
+        rc = SSH_ERROR;
-        goto err;
+        goto fail;
-    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_PUB_KEY, pubb);
+    }
-    if (rc != 1)
+    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_PRIV_KEY, bpriv_key);
-        goto err;
+    if (rc != 1) {
-    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_PRIV_KEY, privb);
+        rc = SSH_ERROR;
-    if (rc != 1)
+        goto fail;
-        goto err;
+    }
 
     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();
+    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_P, bp);
-    if (param_bld == NULL)
+    if (rc != 1) {
-        goto err;
+        rc = SSH_ERROR;
-
+        goto fail;
-    pb = ssh_make_string_bn(p);
+    }
-    qb = ssh_make_string_bn(q);
+    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_Q, bq);
-    gb = ssh_make_string_bn(g);
+    if (rc != 1) {
-    pubb = ssh_make_string_bn(pubkey);
+        rc = SSH_ERROR;
-
+        goto fail;
-    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_P, pb);
+    }
-    if (rc != 1)
+    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_G, bg);
-        goto err;
+    if (rc != 1) {
-    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_Q, qb);
+        rc = SSH_ERROR;
-    if (rc != 1)
+        goto fail;
-        goto err;
+    }
-    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_FFC_G, gb);
+    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_PUB_KEY, bpub_key);
-    if (rc != 1)
+    if (rc != 1) {
-        goto err;
+        rc = SSH_ERROR;
-    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_PUB_KEY, pubb);
+        goto fail;
-    if (rc != 1)
+    }
-        goto err;
 
     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();
+    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_N, bn);
-    if (param_bld == NULL)
+    if (rc != 1) {
-        goto err;
+        rc = SSH_ERROR;
-
+        goto fail;
-    nb = ssh_make_string_bn(n);
+    }
-    eb = ssh_make_string_bn(e);
+    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_E, be);
-    db = ssh_make_string_bn(d);
+    if (rc != 1) {
-    pb = ssh_make_string_bn(p);
+        rc = SSH_ERROR;
-    qb = ssh_make_string_bn(q);
+        goto fail;
-
+    }
-    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_N, nb);
+    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_D, bd);
-    if (rc != 1)
+    if (rc != 1) {
-        goto err;
+        rc = SSH_ERROR;
-    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_E, eb);
+        goto fail;
-    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 = evp_build_pkey("RSA", param_bld, &(key->key), EVP_PKEY_KEYPAIR);
+    if (rc != SSH_OK) {
+        rc = SSH_ERROR;
+        goto fail;
+    }
+
+    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, bq);
+    if (rc != 1) {
+        rc = SSH_ERROR;
+        goto fail;
+    }
+
+fail:
     OSSL_PARAM_BLD_free(param_bld);
-
+    bignum_safe_free(bn);
-    rc = EVP_PKEY_set_bn_param(key->key, OSSL_PKEY_PARAM_RSA_FACTOR1, pb);
+    bignum_safe_free(be);
-    if (rc != 1)
+    bignum_safe_free(bd);
-        goto err;
+    bignum_safe_free(bp);
-
+    bignum_safe_free(bq);
-    rc = EVP_PKEY_set_bn_param(key->key, OSSL_PKEY_PARAM_RSA_FACTOR2, qb);
-    if (rc != 1)
-        goto err;
 
     return rc;
-err:
-    OSSL_PARAM_BLD_free(param_bld);
-    return -1;
 #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);
+    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_N, bn);
-    eb = ssh_make_string_bn(e);
+    if (rc != 1) {
-
+        rc = SSH_ERROR;
-    param_bld = OSSL_PARAM_BLD_new();
+        goto fail;
-    if (param_bld == NULL)
+    }
-        goto err;
+    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_E, be);
-
+    if (rc != 1) {
-    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_N, nb);
+        rc = SSH_ERROR;
-    if (rc != 1)
+        goto fail;
-        goto err;
+    }
-    rc = OSSL_PARAM_BLD_push_BN(param_bld, OSSL_PKEY_PARAM_RSA_E, eb);
-    if (rc != 1)
-        goto err;
 
     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 */
 }