From bcba17939e1b1a568cd4a764534cde74d37078cc Mon Sep 17 00:00:00 2001 From: Christian Göttsche Date: Mon, 30 Jan 2023 17:56:58 +0100 Subject: modules: make use of secure memory erasure Use empty initialization of structs to minimize the memset() usage, to reduce the amount of calls which are not sensitive. Non trivial changes: - pam_env: * erase environment variables where possible - pam_exec: * erase responce on error * erase auth token - pam_pwhistory: * erase buffers containing old passwords - pam_selinux: skip overwriting data structure consisting of only pointers to insensitive data, which also gets free'd afterwards (so it currently does not protect against double-free or use-after-free on the member pointers) - pam_unix: erase cipher data in more places - pam_userdb: erase password hashes --- modules/pam_timestamp/hmac_openssl_wrapper.c | 8 ++++---- modules/pam_timestamp/hmacsha1.c | 16 ++++++++-------- modules/pam_timestamp/pam_timestamp.c | 3 +-- 3 files changed, 13 insertions(+), 14 deletions(-) (limited to 'modules/pam_timestamp') diff --git a/modules/pam_timestamp/hmac_openssl_wrapper.c b/modules/pam_timestamp/hmac_openssl_wrapper.c index 926c2fb9..df772d60 100644 --- a/modules/pam_timestamp/hmac_openssl_wrapper.c +++ b/modules/pam_timestamp/hmac_openssl_wrapper.c @@ -144,7 +144,7 @@ read_file(pam_handle_t *pamh, int fd, char **text, size_t *text_length) if (bytes_read < (size_t)st.st_size) { pam_syslog(pamh, LOG_ERR, "Short read on key file"); - memset(tmp, 0, st.st_size); + pam_overwrite_n(tmp, st.st_size); free(tmp); return PAM_AUTH_ERR; } @@ -167,14 +167,14 @@ write_file(pam_handle_t *pamh, const char *file_name, char *text, S_IRUSR | S_IWUSR); if (fd == -1) { pam_syslog(pamh, LOG_ERR, "Unable to open [%s]: %m", file_name); - memset(text, 0, text_length); + pam_overwrite_n(text, text_length); free(text); return PAM_AUTH_ERR; } if (fchown(fd, owner, group) == -1) { pam_syslog(pamh, LOG_ERR, "Unable to change ownership [%s]: %m", file_name); - memset(text, 0, text_length); + pam_overwrite_n(text, text_length); free(text); close(fd); return PAM_AUTH_ERR; @@ -294,7 +294,7 @@ done: free(hmac_message); } if (key != NULL) { - memset(key, 0, key_length); + pam_overwrite_n(key, key_length); free(key); } if (ctx != NULL) { diff --git a/modules/pam_timestamp/hmacsha1.c b/modules/pam_timestamp/hmacsha1.c index 45a3cac2..384ccde8 100644 --- a/modules/pam_timestamp/hmacsha1.c +++ b/modules/pam_timestamp/hmacsha1.c @@ -48,6 +48,7 @@ #include #include #include +#include "pam_inline.h" #include "hmacsha1.h" #include "sha1.h" @@ -107,7 +108,7 @@ hmac_key_create(pam_handle_t *pamh, const char *filename, size_t key_size, /* If we didn't get enough, stop here. */ if (count < key_size) { pam_syslog(pamh, LOG_ERR, "Short read on random device"); - memset(key, 0, key_size); + pam_overwrite_n(key, key_size); free(key); close(keyfd); return; @@ -122,7 +123,7 @@ hmac_key_create(pam_handle_t *pamh, const char *filename, size_t key_size, } count += i; } - memset(key, 0, key_size); + pam_overwrite_n(key, key_size); free(key); close(keyfd); } @@ -180,7 +181,7 @@ hmac_key_read(pam_handle_t *pamh, const char *filename, size_t default_key_size, /* Require that we got the expected amount of data. */ if (count < st.st_size) { - memset(tmp, 0, st.st_size); + pam_overwrite_n(tmp, st.st_size); free(tmp); return; } @@ -204,7 +205,7 @@ hmac_sha1_generate(void **mac, size_t *mac_length, const void *raw_key, size_t raw_key_size, const void *text, size_t text_length) { - unsigned char key[MAXIMUM_KEY_SIZE], tmp_key[MAXIMUM_KEY_SIZE]; + unsigned char key[MAXIMUM_KEY_SIZE] = {}, tmp_key[MAXIMUM_KEY_SIZE]; size_t maximum_key_size = SHA1_BLOCK_SIZE, minimum_key_size = SHA1_OUTPUT_SIZE; const unsigned char ipad = 0x36, opad = 0x5c; @@ -223,7 +224,6 @@ hmac_sha1_generate(void **mac, size_t *mac_length, /* If the key is too long, "compress" it, else copy it and pad it * out with zero bytes. */ - memset(key, 0, sizeof(key)); if (raw_key_size > maximum_key_size) { sha1_init(&sha1); sha1_update(&sha1, raw_key, raw_key_size); @@ -251,8 +251,8 @@ hmac_sha1_generate(void **mac, size_t *mac_length, sha1_output(&sha1, outer); /* We don't need any of the keys any more. */ - memset(key, 0, sizeof(key)); - memset(tmp_key, 0, sizeof(tmp_key)); + pam_overwrite_array(key); + pam_overwrite_array(tmp_key); /* Allocate space to store the output. */ *mac_length = sizeof(outer); @@ -284,7 +284,7 @@ hmac_sha1_generate_file(pam_handle_t *pamh, void **mac, size_t *mac_length, hmac_sha1_generate(mac, mac_length, key, key_length, text, text_length); - memset(key, 0, key_length); + pam_overwrite_n(key, key_length); free(key); } diff --git a/modules/pam_timestamp/pam_timestamp.c b/modules/pam_timestamp/pam_timestamp.c index 572d9ff2..c5fa6dfc 100644 --- a/modules/pam_timestamp/pam_timestamp.c +++ b/modules/pam_timestamp/pam_timestamp.c @@ -95,7 +95,7 @@ static int check_dir_perms(pam_handle_t *pamh, const char *tdir) { - char scratch[BUFLEN]; + char scratch[BUFLEN] = {}; struct stat st; int i; /* Check that the directory is "safe". */ @@ -103,7 +103,6 @@ check_dir_perms(pam_handle_t *pamh, const char *tdir) return PAM_AUTH_ERR; } /* Iterate over the path, checking intermediate directories. */ - memset(scratch, 0, sizeof(scratch)); for (i = 0; (tdir[i] != '\0') && (i < (int)sizeof(scratch)); i++) { scratch[i] = tdir[i]; if ((scratch[i] == '/') || (tdir[i + 1] == '\0')) { -- cgit v1.2.3