diff options
| author | Robert Clausecker <fuz@FreeBSD.org> | 2024-10-10 09:08:35 +0000 |
|---|---|---|
| committer | Robert Clausecker <fuz@FreeBSD.org> | 2024-10-29 15:45:31 +0000 |
| commit | 1af027e583ec725ae772a4cc0b128652553fa7ec (patch) | |
| tree | 1eb581df5c55c8980289b494de62b9f9314a16ae | |
| parent | 1d271ba05fceb1ef6246e8079bc19e3b6416a833 (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.c | 5 | ||||
| -rw-r--r-- | lib/libcrypt/crypt-sha256.c | 7 | ||||
| -rw-r--r-- | 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); } |
