From f24f3435be39cab2aa54a49d31968a023ab6d1d5 Mon Sep 17 00:00:00 2001

From: Ondrej Mosnacek <omosnace@redhat.com>

Date: Thu, 26 Jul 2018 14:09:27 +0200

Subject: [PATCH 1/3] kcapi-kdf: Clear the whole out buffer on error

The KDF functions were decrementing the output length variable in the

loop, but on error they would clear the output buffer based on this

decremented value. This patch backs up the original length and uses it

when clearing the output buffer.

The kcapi_pbkdf() function also used an incremented output buffer

pointer. This one is now also backed-up and the original value is used

when clearing the output.

Signed-off-by: Stephan Mueller <smueller@chronox.de>

---

 lib/kcapi-kdf.c | 16 +++++++++++-----

 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/lib/kcapi-kdf.c b/lib/kcapi-kdf.c

index 78a7e0d..6eccbe1 100644

--- a/lib/kcapi-kdf.c

+++ b/lib/kcapi-kdf.c

@@ -99,6 +99,7 @@ int32_t kcapi_kdf_dpi(struct kcapi_handle *handle,

 	uint32_t h = kcapi_md_digestsize(handle);

 	int32_t err = 0;

 	uint8_t *dst_orig = dst;

+	uint32_t dlen_orig = dlen;

 	uint8_t Ai[h];

 	uint32_t i = 1;

@@ -161,7 +162,7 @@ int32_t kcapi_kdf_dpi(struct kcapi_handle *handle,

 	return 0;

 err:

-	kcapi_memset_secure(dst_orig, 0, dlen);

+	kcapi_memset_secure(dst_orig, 0, dlen_orig);

 	kcapi_memset_secure(Ai, 0, h);

 	return err;

 }

@@ -174,6 +175,7 @@ int32_t kcapi_kdf_fb(struct kcapi_handle *handle,

 	uint32_t h = kcapi_md_digestsize(handle);

 	int32_t err = 0;

 	uint8_t *dst_orig = dst;

+	uint32_t dlen_orig = dlen;

 	const uint8_t *label;

 	uint32_t labellen = 0;

 	uint32_t i = 1;

@@ -238,7 +240,7 @@ int32_t kcapi_kdf_fb(struct kcapi_handle *handle,

 	return 0;

 err:

-	kcapi_memset_secure(dst_orig, 0, dlen);

+	kcapi_memset_secure(dst_orig, 0, dlen_orig);

 	return err;

 }

@@ -250,6 +252,7 @@ int32_t kcapi_kdf_ctr(struct kcapi_handle *handle,

 	uint32_t h = kcapi_md_digestsize(handle);

 	int32_t err = 0;

 	uint8_t *dst_orig = dst;

+	uint32_t dlen_orig = dlen;

 	uint32_t i = 1;

 	if (dlen > INT_MAX)

@@ -295,7 +298,7 @@ int32_t kcapi_kdf_ctr(struct kcapi_handle *handle,

 	return 0;

 err:

-	kcapi_memset_secure(dst_orig, 0, dlen);

+	kcapi_memset_secure(dst_orig, 0, dlen_orig);

 	return err;

 }

@@ -316,6 +319,7 @@ int32_t kcapi_hkdf(const char *hashname,

 	uint8_t *prev = NULL;

 	int32_t err = 0;

 	uint8_t *dst_orig = dst;

+	uint32_t dlen_orig = dlen;

 	uint8_t ctr = 0x01;

 	struct kcapi_handle *handle = NULL;

@@ -415,7 +419,7 @@ int32_t kcapi_hkdf(const char *hashname,

 	goto out;

 err:

-	kcapi_memset_secure(dst_orig, 0, dlen);

+	kcapi_memset_secure(dst_orig, 0, dlen_orig);

 out:

 	kcapi_memset_secure(prk_tmp, 0, h);

 	kcapi_md_destroy(handle);

@@ -552,6 +556,8 @@ int32_t kcapi_pbkdf(const char *hashname,

 		    uint8_t *key, uint32_t keylen)

 {

 	struct kcapi_handle *handle;

+	uint8_t *key_orig = key;

+	uint32_t keylen_orig = keylen;

 	uint32_t h, i = 1;

 #define MAX_DIGESTSIZE 64

 	uint8_t u[MAX_DIGESTSIZE] __attribute__ ((aligned (sizeof(uint64_t))));

@@ -633,7 +639,7 @@ int32_t kcapi_pbkdf(const char *hashname,

 err:

 	kcapi_memset_secure(u, 0, h);

 	if (err)

-		kcapi_memset_secure(key, 0, keylen);

+		kcapi_memset_secure(key_orig, 0, keylen_orig);

 	kcapi_md_destroy(handle);

 	return err;

From eacb82b193a94d46d2ea70c621176d79a5486008 Mon Sep 17 00:00:00 2001

From: Ondrej Mosnacek <omosnace@redhat.com>

Date: Thu, 26 Jul 2018 14:12:51 +0200

Subject: [PATCH 2/3] kcapi-kdf: Simplify handling of final blocks

This patch avoids the use of temporary buffers when handling the last

block in the KDF functions, taking advantage of the fact that

kcapi_md_final() can be used to retrieve also a truncated hash directly.

The new code no longer produces a false-positive warning with CLang

static analysis, so the workaround (which Coverity identifies as

unreachable code) can be removed.

Signed-off-by: Stephan Mueller <smueller@chronox.de>

---

 lib/kcapi-kdf.c | 43 +++++++++----------------------------------

 1 file changed, 9 insertions(+), 34 deletions(-)

diff --git a/lib/kcapi-kdf.c b/lib/kcapi-kdf.c

index 6eccbe1..afa6eb3 100644

--- a/lib/kcapi-kdf.c

+++ b/lib/kcapi-kdf.c

@@ -140,13 +140,9 @@ int32_t kcapi_kdf_dpi(struct kcapi_handle *handle,

 		}

 		if (dlen < h) {

-			uint8_t tmpbuffer[h];

-

-			err = kcapi_md_final(handle, tmpbuffer, h);

+			err = kcapi_md_final(handle, dst, dlen);

 			if (err < 0)

 				goto err;

-			memcpy(dst, tmpbuffer, dlen);

-			kcapi_memset_secure(tmpbuffer, 0, h);

 			dlen = 0;

 		} else {

 			err = kcapi_md_final(handle, dst, h);

@@ -219,14 +215,10 @@ int32_t kcapi_kdf_fb(struct kcapi_handle *handle,

 		}

 		if (dlen < h) {

-			uint8_t tmpbuffer[h];

-

-			err = kcapi_md_final(handle, tmpbuffer, h);

+			err = kcapi_md_final(handle, dst, dlen);

 			if (err < 0)

 				goto err;

-			memcpy(dst, tmpbuffer, dlen);

-			kcapi_memset_secure(tmpbuffer, 0, h);

-			return 0;

+			dlen = 0;

 		} else {

 			err = kcapi_md_final(handle, dst, h);

 			if (err < 0)

@@ -276,14 +268,10 @@ int32_t kcapi_kdf_ctr(struct kcapi_handle *handle,

 		}

 		if (dlen < h) {

-			uint8_t tmpbuffer[h];

-

-			err = kcapi_md_final(handle, tmpbuffer, h);

+			err = kcapi_md_final(handle, dst, dlen);

 			if (err < 0)

 				goto err;

-			memcpy(dst, tmpbuffer, dlen);

-			kcapi_memset_secure(tmpbuffer, 0, h);

-			return 0;

+			dlen = 0;

 		} else {

 			err = kcapi_md_final(handle, dst, h);

 			if (err < 0)

@@ -392,16 +380,10 @@ int32_t kcapi_hkdf(const char *hashname,

 			goto err;

 		if (dlen < h) {

-			err = kcapi_md_final(handle, prk_tmp, h);

+			err = kcapi_md_final(handle, dst, dlen);

 			if (err < 0)

 				goto err;

-			/* Shut up Clang */

-			if (!dst) {

-				err = -EFAULT;

-				goto err;

-			}

-			memcpy(dst, prk_tmp, dlen);

 			dlen = 0;

 		} else {

 			err = kcapi_md_final(handle, dst, h);

@@ -561,8 +543,6 @@ int32_t kcapi_pbkdf(const char *hashname,

 	uint32_t h, i = 1;

 #define MAX_DIGESTSIZE 64

 	uint8_t u[MAX_DIGESTSIZE] __attribute__ ((aligned (sizeof(uint64_t))));

-	uint8_t T[MAX_DIGESTSIZE] __attribute__ ((aligned (sizeof(uint64_t)))) =

-									{ 0 };

 	int32_t err = 0;

 	if (keylen > INT_MAX)

@@ -617,17 +597,12 @@ int32_t kcapi_pbkdf(const char *hashname,

 			if (err < 0)

 				goto err;

-			if (keylen < h)

-				kcapi_xor_64_aligned(T, u, h);

-			else

-				kcapi_xor_64(key, u, h);

+			kcapi_xor_64(key, u, keylen < h ? keylen : h);

 		}

-		if (keylen < h) {

-			memcpy(key, T, keylen);

-			kcapi_memset_secure(T, 0, keylen);

+		if (keylen < h)

 			keylen = 0;

-		} else {

+		else {

 			keylen -= h;

 			key += h;

 			i++;

From c9ed6b2c07026e9bafd99e6c288cfbd175fd237f Mon Sep 17 00:00:00 2001

From: Ondrej Mosnacek <omosnace@redhat.com>

Date: Thu, 26 Jul 2018 14:28:53 +0200

Subject: [PATCH 3/3] kcapi-kdf: Fix unused function warning on 32-bit

The kcapi_xor_64_aligned() is now unused when compiling in 32-bit mode,

so we need to define it only in the 64-bit case, otherwise the build

fails under CLang due to an usnused function warning.

Signed-off-by: Stephan Mueller <smueller@chronox.de>

---

 lib/kcapi-kdf.c | 6 ++----

 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/kcapi-kdf.c b/lib/kcapi-kdf.c

index afa6eb3..a219d63 100644

--- a/lib/kcapi-kdf.c

+++ b/lib/kcapi-kdf.c

@@ -503,10 +503,10 @@ static inline void kcapi_xor_32(uint8_t *dst, const uint8_t *src, uint32_t size)

 		kcapi_xor_8(dst, src, size);

 }

+#ifdef __LP64__

 static inline void kcapi_xor_64_aligned(uint8_t *dst, const uint8_t *src,

 				        uint32_t size)

 {

-#ifdef __LP64__

 	uint64_t *dst_dword = (uint64_t *)dst;

 	uint64_t *src_dword = (uint64_t *)src;

@@ -514,10 +514,8 @@ static inline void kcapi_xor_64_aligned(uint8_t *dst, const uint8_t *src,

 		*dst_dword++ ^= *src_dword++;

 	kcapi_xor_32_aligned((uint8_t *)dst_dword, (uint8_t *)src_dword, size);

-#else

-	kcapi_xor_32_aligned(dst, src, size);

-#endif

 }

+#endif

 static inline void kcapi_xor_64(uint8_t *dst, const uint8_t *src, uint32_t size)

 {