From 6fbff7fc8aabeee2254405f254220bbe8c05c67d Mon Sep 17 00:00:00 2001

From: Daiki Ueno <ueno@gnu.org>

Date: Fri, 5 Jun 2020 16:26:33 +0200

Subject: [PATCH] crypto-api: always allocate memory when serializing iovec_t

The AEAD iov interface falls back to serializing the input buffers if

the low-level cipher doesn't support scatter/gather encryption.

However, there was a bug in the functions used for the serialization,

which causes memory leaks under a certain condition (i.e. the number

of input buffers is 1).

This patch makes the logic of the functions simpler, by removing a

micro-optimization that tries to minimize the number of calls to

malloc/free.

The original problem was reported by Marius Steffen in:

https://bugzilla.samba.org/show_bug.cgi?id=14399

and the cause was investigated by Alexander Haase in:

https://gitlab.com/gnutls/gnutls/-/merge_requests/1277

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

---

 lib/crypto-api.c        | 36 +++++++++++-------------------------

 tests/aead-cipher-vec.c | 33 ++++++++++++++++++---------------

 2 files changed, 29 insertions(+), 40 deletions(-)

diff --git a/lib/crypto-api.c b/lib/crypto-api.c

index 45be64ed1..8524f5ed4 100644

--- a/lib/crypto-api.c

+++ b/lib/crypto-api.c

@@ -891,32 +891,23 @@ gnutls_aead_cipher_encrypt(gnutls_aead_cipher_hd_t handle,

 struct iov_store_st {

 	void *data;

 	size_t size;

-	unsigned allocated;

 };

 static void iov_store_free(struct iov_store_st *s)

 {

-	if (s->allocated) {

-		gnutls_free(s->data);

-		s->allocated = 0;

-	}

+	gnutls_free(s->data);

 }

 static int iov_store_grow(struct iov_store_st *s, size_t length)

 {

-	if (s->allocated || s->data == NULL) {

-		s->size += length;

-		s->data = gnutls_realloc(s->data, s->size);

-		if (s->data == NULL)

-			return gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);

-		s->allocated = 1;

-	} else {

-		void *data = s->data;

-		size_t size = s->size + length;

-		s->data = gnutls_malloc(size);

-		memcpy(s->data, data, s->size);

-		s->size += length;

-	}

+	void *data;

+

+	s->size += length;

+	data = gnutls_realloc(s->data, s->size);

+	if (data == NULL)

+		return gnutls_assert_val(GNUTLS_E_MEMORY_ERROR);

+

+	s->data = data;

 	return 0;

 }

@@ -926,11 +917,6 @@ copy_from_iov(struct iov_store_st *dst, const giovec_t *iov, int iovcnt)

 	memset(dst, 0, sizeof(*dst));

 	if (iovcnt == 0) {

 		return 0;

-	} else if (iovcnt == 1) {

-		dst->data = iov[0].iov_base;

-		dst->size = iov[0].iov_len;

-		/* implies: dst->allocated = 0; */

-		return 0;

 	} else {

 		int i;

 		uint8_t *p;

@@ -944,11 +930,11 @@ copy_from_iov(struct iov_store_st *dst, const giovec_t *iov, int iovcnt)

 		p = dst->data;

 		for (i=0;i

-			memcpy(p, iov[i].iov_base, iov[i].iov_len);

+			if (iov[i].iov_len > 0)

+				memcpy(p, iov[i].iov_base, iov[i].iov_len);

 			p += iov[i].iov_len;

 		}

-		dst->allocated = 1;

 		return 0;

 	}

 }

diff --git a/tests/aead-cipher-vec.c b/tests/aead-cipher-vec.c

index fba9010d9..6a30a35f7 100644

--- a/tests/aead-cipher-vec.c

+++ b/tests/aead-cipher-vec.c

@@ -49,6 +49,7 @@ static void start(const char *name, int algo)

 	giovec_t auth_iov[2];

 	uint8_t tag[64];

 	size_t tag_size = 0;

+	size_t i;

 	key.data = key16;

 	key.size = gnutls_cipher_get_key_size(algo);

@@ -82,21 +83,23 @@ static void start(const char *name, int algo)

 	if (ret < 0)

 		fail("gnutls_cipher_init: %s\n", gnutls_strerror(ret));

-	ret = gnutls_aead_cipher_encryptv2(ch,

-					   iv.data, iv.size,

-					   auth_iov, 2,

-					   iov, 3,

-					   tag, &tag_size);

-	if (ret < 0)

-		fail("could not encrypt data: %s\n", gnutls_strerror(ret));

-

-	ret = gnutls_aead_cipher_decryptv2(ch,

-					   iv.data, iv.size,

-					   auth_iov, 2,

-					   iov, 3,

-					   tag, tag_size);

-	if (ret < 0)

-		fail("could not decrypt data: %s\n", gnutls_strerror(ret));

+	for (i = 0; i < 2; i++) {

+		ret = gnutls_aead_cipher_encryptv2(ch,

+						   iv.data, iv.size,

+						   auth_iov, 2,

+						   iov, i + 1,

+						   tag, &tag_size);

+		if (ret < 0)

+			fail("could not encrypt data: %s\n", gnutls_strerror(ret));

+

+		ret = gnutls_aead_cipher_decryptv2(ch,

+						   iv.data, iv.size,

+						   auth_iov, 2,

+						   iov, i + 1,

+						   tag, tag_size);

+		if (ret < 0)

+			fail("could not decrypt data: %s\n", gnutls_strerror(ret));

+	}

 	gnutls_aead_cipher_deinit(ch);

 }

-- 

2.25.4