fips: avoid memleak in (EC)DH internal APIs

There were some confusions of gnutls_pk_params_clear and
gnutls_pk_params_release, as well as the number of parameters to scan
in the gnutls_pk_params_st structure.

Flagged by address sanitizer:
  ==354688==ERROR: LeakSanitizer: detected memory leaks

  Direct leak of 192 byte(s) in 12 object(s) allocated from:
      #0 0x7f13506163cf in __interceptor_malloc (/lib64/libasan.so.6+0xab3cf)
      #1 0x7f13503b94de in wrap_nettle_mpi_init /home/ueno/devel/gnutls/lib/nettle/mpi.c:79
      #2 0x7ffcb8495f07  ([stack]+0x1ef07)

  Direct leak of 160 byte(s) in 10 object(s) allocated from:
      #0 0x7f13506163cf in __interceptor_malloc (/lib64/libasan.so.6+0xab3cf)
      #1 0x7f13503b94de in wrap_nettle_mpi_init /home/ueno/devel/gnutls/lib/nettle/mpi.c:79

Signed-off-by: Daiki Ueno <ueno@gnu.org>

diff --git a/lib/nettle/pk.c b/lib/nettle/pk.c
index 3475084f0f6c1d3ef57d097993b46c3bdb7e0337..432bcdd59db31de3a068e75b8bac18c422f9336b 100644 (file)
--- 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 217b23b762230b4ad622ca8feed2fe7c4ab6cc74..64eb2c5804a9fe7669745b6c82897f63eb129aff 100644 (file)
--- 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 d9f99a19cabb1a2f4f804659f5002110332fb7a8..2eac61c6c309e9a391b7650c75b66b8e8619e65f 100644 (file)
--- 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,