Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 29 Oct 2024 15:46:10 GMT
From:      Robert Clausecker <fuz@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 1af027e583ec - stable/14 - lib/libcrypt: use explicit_bzero() to clear sensitive buffers
Message-ID:  <202410291546.49TFkA8Y082551@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/14 has been updated by fuz:

URL: https://cgit.FreeBSD.org/src/commit/?id=1af027e583ec725ae772a4cc0b128652553fa7ec

commit 1af027e583ec725ae772a4cc0b128652553fa7ec
Author:     Robert Clausecker <fuz@FreeBSD.org>
AuthorDate: 2024-10-10 09:08:35 +0000
Commit:     Robert Clausecker <fuz@FreeBSD.org>
CommitDate: 2024-10-29 15:45:31 +0000

    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)
---
 lib/libcrypt/crypt-md5.c    | 5 +++--
 lib/libcrypt/crypt-sha256.c | 7 ++++---
 lib/libcrypt/crypt-sha512.c | 7 ++++---
 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);
 }



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202410291546.49TFkA8Y082551>