rpms/libkcapi
3
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity
29c28aa0c9
libkcapi/libkcapi-1.1.1-Coverity_PR_follow-up.patch
Ondrej Mosnacek e55cfcddec Add more Coverity fixes from upstream 
This commit updates the last PR patch (one of the commits was not
accepted upstream) and adds new fixes that have been added in the
meantime (including an alternative version of the patch that had been
dropped).
2018-07-27 09:40:09 +02:00

273 lines
7.8 KiB
Diff
Raw Blame History

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)
{
Reference in New Issue View Git Blame Copy Permalink