Merge branch 'wip/dueno/fips-dh-memleak' into 'master'

fips: avoid memleak in (EC)DH internal APIs

See merge request gnutls/gnutls!1380

3 files changed, 28 insertions, 23 deletions

diff --git a/lib/nettle/pk.c b/lib/nettle/pk.c
index 3475084f0f..432bcdd59d 100644
--- a/lib/nettle/pk.c
+++ b/lib/nettle/pk.c
@@ -1824,7 +1824,7 @@ int _gnutls_dh_generate_key(gnutls_dh_params_t dh_params,
 	params.params[DH_P] = _gnutls_mpi_copy(dh_params->params[0]);
 	params.params[DH_G] = _gnutls_mpi_copy(dh_params->params[1]);
 
-	params.params_nr = 3; /* include empty q */
+	params.params_nr = 5;
 	params.algo = GNUTLS_PK_DH;
 
 	priv_key->data = NULL;
@@ -1856,6 +1856,7 @@ int _gnutls_dh_generate_key(gnutls_dh_params_t dh_params,
 	gnutls_free(priv_key->data);
  cleanup:
 	gnutls_pk_params_clear(&params);
+	gnutls_pk_params_release(&params);
 	return ret;
 }
 
@@ -1869,9 +1870,13 @@ int _gnutls_dh_compute_key(gnutls_dh_params_t dh_params,
 	int ret;
 
 	gnutls_pk_params_init(&pub);
-	gnutls_pk_params_init(&priv);
+	pub.params_nr = 5;
 	pub.algo = GNUTLS_PK_DH;
 
+	gnutls_pk_params_init(&priv);
+	priv.params_nr = 5;
+	priv.algo = GNUTLS_PK_DH;
+
 	if (_gnutls_mpi_init_scan_nz
 		    (&pub.params[DH_Y], peer_key->data,
 		     peer_key->size) != 0) {
@@ -1893,9 +1898,6 @@ int _gnutls_dh_compute_key(gnutls_dh_params_t dh_params,
 		goto cleanup;
 	}
 
-	priv.params_nr = 3; /* include, possibly empty, q */
-	priv.algo = GNUTLS_PK_DH;
-
 	Z->data = NULL;
 
 	ret = _gnutls_pk_derive(GNUTLS_PK_DH, Z, &priv, &pub);
@@ -1907,7 +1909,9 @@ int _gnutls_dh_compute_key(gnutls_dh_params_t dh_params,
 	ret = 0;
  cleanup:
 	gnutls_pk_params_clear(&pub);
+	gnutls_pk_params_release(&pub);
 	gnutls_pk_params_clear(&priv);
+	gnutls_pk_params_release(&priv);
 	return ret;
 }
 
@@ -1919,6 +1923,7 @@ int _gnutls_ecdh_generate_key(gnutls_ecc_curve_t curve,
 	int ret;
 
 	gnutls_pk_params_init(&params);
+	params.params_nr = 3;
 	params.curve = curve;
 	params.algo = GNUTLS_PK_ECDSA;
 
@@ -1960,6 +1965,7 @@ int _gnutls_ecdh_generate_key(gnutls_ecc_curve_t curve,
 	gnutls_free(k->data);
  cleanup:
 	gnutls_pk_params_clear(&params);
+	gnutls_pk_params_release(&params);
 	return ret;
 }
 
@@ -1973,11 +1979,15 @@ int _gnutls_ecdh_compute_key(gnutls_ecc_curve_t curve,
 	int ret;
 
 	gnutls_pk_params_init(&pub);
-	gnutls_pk_params_init(&priv);
-
+	pub.params_nr = 3;
 	pub.algo = GNUTLS_PK_ECDSA;
 	pub.curve = curve;
 
+	gnutls_pk_params_init(&priv);
+	priv.params_nr = 3;
+	priv.algo = GNUTLS_PK_ECDSA;
+	priv.curve = curve;
+
 	if (_gnutls_mpi_init_scan_nz
 		    (&pub.params[ECC_Y], peer_y->data,
 		     peer_y->size) != 0) {
@@ -1994,8 +2004,6 @@ int _gnutls_ecdh_compute_key(gnutls_ecc_curve_t curve,
 		goto cleanup;
 	}
 
-	pub.params_nr = 2;
-
 	if (_gnutls_mpi_init_scan_nz
 		    (&priv.params[ECC_Y], y->data,
 		     y->size) != 0) {
@@ -2020,11 +2028,6 @@ int _gnutls_ecdh_compute_key(gnutls_ecc_curve_t curve,
 		goto cleanup;
 	}
 
-
-	priv.params_nr = 3;
-	priv.algo = GNUTLS_PK_ECDSA;
-	priv.curve = curve;
-
 	Z->data = NULL;
 
 	ret = _gnutls_pk_derive(GNUTLS_PK_ECDSA, Z, &priv, &pub);
@@ -2036,7 +2039,9 @@ int _gnutls_ecdh_compute_key(gnutls_ecc_curve_t curve,
 	ret = 0;
  cleanup:
 	gnutls_pk_params_clear(&pub);
+	gnutls_pk_params_release(&pub);
 	gnutls_pk_params_clear(&priv);
+	gnutls_pk_params_release(&priv);
 	return ret;
 }
 
diff --git a/tests/dh-compute.c b/tests/dh-compute.c
index 217b23b762..64eb2c5804 100644
--- a/tests/dh-compute.c
+++ b/tests/dh-compute.c
@@ -55,18 +55,18 @@ static void params(gnutls_dh_params_t *dh_params, const gnutls_datum_t *p,
 		fail("error\n");
 }
 
-static void genkey(gnutls_dh_params_t *dh_params,
+static void genkey(const gnutls_dh_params_t dh_params,
 		   gnutls_datum_t *priv_key, gnutls_datum_t *pub_key)
 {
 	int ret;
 
-	ret = _gnutls_dh_generate_key(*dh_params, priv_key, pub_key);
+	ret = _gnutls_dh_generate_key(dh_params, priv_key, pub_key);
 	if (ret != 0)
 		fail("error\n");
 }
 
-static void compute_key(const char *name, gnutls_dh_params_t *dh_params,
-			gnutls_datum_t *priv_key, gnutls_datum_t *pub_key,
+static void compute_key(const char *name, const gnutls_dh_params_t dh_params,
+			const gnutls_datum_t *priv_key, const gnutls_datum_t *pub_key,
 			const gnutls_datum_t *peer_key, int expect_error,
 			gnutls_datum_t *result, bool expect_success)
 {
@@ -74,7 +74,7 @@ static void compute_key(const char *name, gnutls_dh_params_t *dh_params,
 	bool success;
 	int ret;
 
-	ret = _gnutls_dh_compute_key(*dh_params, priv_key, pub_key,
+	ret = _gnutls_dh_compute_key(dh_params, priv_key, pub_key,
 				     peer_key, &Z);
 	if (expect_error != ret)
 		fail("%s: error %d (expected %d)\n", name, ret, expect_error);
@@ -150,9 +150,9 @@ void doit(void)
 		params(&dh_params, &test_data[i].prime, &test_data[i].q,
 		       &test_data[i].generator);
 
-		genkey(&dh_params, &priv_key, &pub_key);
+		genkey(dh_params, &priv_key, &pub_key);
 
-		compute_key(test_data[i].name, &dh_params, &priv_key,
+		compute_key(test_data[i].name, dh_params, &priv_key,
 			    &pub_key, &test_data[i].peer_key,
 			    test_data[i].expected_error, NULL, 0);
 
diff --git a/tests/ecdh-compute.c b/tests/ecdh-compute.c
index d9f99a19ca..2eac61c6c3 100644
--- a/tests/ecdh-compute.c
+++ b/tests/ecdh-compute.c
@@ -53,8 +53,8 @@ static void genkey(gnutls_ecc_curve_t curve, gnutls_datum_t *x,
 		fail("error\n");
 }
 
-static void compute_key(gnutls_ecc_curve_t curve, gnutls_datum_t *x,
-			gnutls_datum_t *y, gnutls_datum_t *key,
+static void compute_key(gnutls_ecc_curve_t curve, const gnutls_datum_t *x,
+			const gnutls_datum_t *y, const gnutls_datum_t *key,
 			const gnutls_datum_t *peer_x,
 			const gnutls_datum_t *peer_y,
                         int expect_error,