aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Clausecker <fuz@FreeBSD.org>2024-10-10 09:08:35 +0000
committerRobert Clausecker <fuz@FreeBSD.org>2024-10-29 15:45:31 +0000
commit1af027e583ec725ae772a4cc0b128652553fa7ec (patch)
tree1eb581df5c55c8980289b494de62b9f9314a16ae
parent1d271ba05fceb1ef6246e8079bc19e3b6416a833 (diff)
lib/libcrypt: use explicit_bzero() to clear sensitive buffers
Prevent a potentially sufficiently smart compiler from optimising away our attempts to clear sensitive buffers. A related change was discussed and rejected in D16059, but I don't believe the reasoning there applies: the code clearly documents its intent that the `memset` calls clear sensitive buffers so they don't hang around. `explicit_bzero` is the appropriate function for this purpose. A potential performance disadvantage seems less important: the functions in crypt are specifically designed to be slow, so a few extra calls to guarantee that sensitive buffers are cleared does not significantly affect runtime. See also: D16059 Reviewed by: delphij, kevans MFC after: 1 week Differential Revision: https://reviews.freebsd.org/D47037 (cherry picked from commit a2c0d2026fb422ade2171da4bc6d5d2773b268a6)
-rw-r--r--lib/libcrypt/crypt-md5.c5
-rw-r--r--lib/libcrypt/crypt-sha256.c7
-rw-r--r--lib/libcrypt/crypt-sha512.c7
3 files changed, 11 insertions, 8 deletions
diff --git a/lib/libcrypt/crypt-md5.c b/lib/libcrypt/crypt-md5.c
index 3fb80c1ba540..5313f59a4098 100644
--- a/lib/libcrypt/crypt-md5.c
+++ b/lib/libcrypt/crypt-md5.c
@@ -33,6 +33,7 @@
#include <md5.h>
#include <stdio.h>
#include <string.h>
+#include <strings.h>
#include <unistd.h>
#include "crypt.h"
@@ -85,7 +86,7 @@ crypt_md5(const char *pw, const char *salt, char *buffer)
(u_int)(pl > MD5_SIZE ? MD5_SIZE : pl));
/* Don't leave anything around in vm they could use. */
- memset(final, 0, sizeof(final));
+ explicit_bzero(final, sizeof(final));
/* Then something really weird... */
for (i = strlen(pw); i; i >>= 1)
@@ -141,7 +142,7 @@ crypt_md5(const char *pw, const char *salt, char *buffer)
*buffer = '\0';
/* Don't leave anything around in vm they could use. */
- memset(final, 0, sizeof(final));
+ explicit_bzero(final, sizeof(final));
return (0);
}
diff --git a/lib/libcrypt/crypt-sha256.c b/lib/libcrypt/crypt-sha256.c
index 35c36bf93f3d..6da1d518b12d 100644
--- a/lib/libcrypt/crypt-sha256.c
+++ b/lib/libcrypt/crypt-sha256.c
@@ -41,6 +41,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <strings.h>
#include "crypt.h"
@@ -234,9 +235,9 @@ crypt_sha256(const char *key, const char *salt, char *buffer)
* the SHA256 implementation as well. */
SHA256_Init(&ctx);
SHA256_Final(alt_result, &ctx);
- memset(temp_result, '\0', sizeof(temp_result));
- memset(p_bytes, '\0', key_len);
- memset(s_bytes, '\0', salt_len);
+ explicit_bzero(temp_result, sizeof(temp_result));
+ explicit_bzero(p_bytes, key_len);
+ explicit_bzero(s_bytes, salt_len);
return (0);
}
diff --git a/lib/libcrypt/crypt-sha512.c b/lib/libcrypt/crypt-sha512.c
index 640398afadc4..b760623b5d8d 100644
--- a/lib/libcrypt/crypt-sha512.c
+++ b/lib/libcrypt/crypt-sha512.c
@@ -41,6 +41,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <strings.h>
#include "crypt.h"
@@ -246,9 +247,9 @@ crypt_sha512(const char *key, const char *salt, char *buffer)
* the SHA512 implementation as well. */
SHA512_Init(&ctx);
SHA512_Final(alt_result, &ctx);
- memset(temp_result, '\0', sizeof(temp_result));
- memset(p_bytes, '\0', key_len);
- memset(s_bytes, '\0', salt_len);
+ explicit_bzero(temp_result, sizeof(temp_result));
+ explicit_bzero(p_bytes, key_len);
+ explicit_bzero(s_bytes, salt_len);
return (0);
}