In a recent thread[0], the existence of explicit_bzero() was mentioned. I went to look where we could use that to clear sensitive information from memory and found a few candidates:
- In be-secure-common.c, clear the entered SSL passphrase in the error path. (In the non-error path, the buffer belongs to OpenSSL.) - In libpq, clean up after reading .pgpass. Otherwise, the entire file including all passwords potentially remains in memory. - In libpq, clear the password after a connection is closed (freePGconn/part of PQfinish). - pg_hba.conf could potentially contain passwords for LDAP, so that should maybe also be cleared, but the structure of that code would make that more involved, so I skipped that for now. Efforts are probably better directed at providing facilities to avoid having to do that.[1] Any other ones? A patch that implements the first three is attached. [0]: https://www.postgresql.org/message-id/043403c2-f04d-3a69-aa8a-9bb7b9ce8...@iki.fi [1]: https://www.postgresql.org/message-id/flat/CA%2BhUKGJ44ssWhcKP1KYK2Dm9_XXk1_b629_qSDUhH1fWfuAvXg%40mail.gmail.com -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 5cd9ed8e0cc5fae18657acb033bbdb6fcba90f1b Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Wed, 12 Jun 2019 08:58:22 +0000 Subject: [PATCH] Use explicit_bzero --- configure | 2 +- configure.in | 1 + src/backend/libpq/be-secure-common.c | 1 + src/include/pg_config.h.in | 3 +++ src/include/port.h | 4 ++++ src/interfaces/libpq/fe-connect.c | 8 ++++++++ 6 files changed, 18 insertions(+), 1 deletion(-) diff --git a/configure b/configure index fd61bf6472..3fa8ec0603 100755 --- a/configure +++ b/configure @@ -15176,7 +15176,7 @@ fi LIBS_including_readline="$LIBS" LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'` -for ac_func in cbrt clock_gettime copyfile fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink sync_file_range uselocale utime utimes wcstombs_l +for ac_func in cbrt clock_gettime copyfile explicit_bzero fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate ppoll pstat pthread_is_threaded_np readlink setproctitle setproctitle_fast setsid shm_open strchrnul strsignal symlink sync_file_range uselocale utime utimes wcstombs_l do : as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var" diff --git a/configure.in b/configure.in index 4586a1716c..7488f4519f 100644 --- a/configure.in +++ b/configure.in @@ -1615,6 +1615,7 @@ AC_CHECK_FUNCS(m4_normalize([ cbrt clock_gettime copyfile + explicit_bzero fdatasync getifaddrs getpeerucred diff --git a/src/backend/libpq/be-secure-common.c b/src/backend/libpq/be-secure-common.c index 877226d377..4c1c6cb3c4 100644 --- a/src/backend/libpq/be-secure-common.c +++ b/src/backend/libpq/be-secure-common.c @@ -118,6 +118,7 @@ run_ssl_passphrase_command(const char *prompt, bool is_server_start, char *buf, buf[--len] = '\0'; error: + explicit_bzero(buf, size); pfree(command.data); return len; } diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 6cd4cfed0a..0062a4a426 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -201,6 +201,9 @@ /* Define to 1 if you have the <editline/readline.h> header file. */ #undef HAVE_EDITLINE_READLINE_H +/* Define to 1 if you have the `explicit_bzero' function. */ +#undef HAVE_EXPLICIT_BZERO + /* Define to 1 if you have the `fdatasync' function. */ #undef HAVE_FDATASYNC diff --git a/src/include/port.h b/src/include/port.h index b5c03d912b..af754b261c 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -381,6 +381,10 @@ extern int isinf(double x); #endif /* __clang__ && !__cplusplus */ #endif /* !HAVE_ISINF */ +#ifndef HAVE_EXPLICIT_BZERO +#define explicit_bzero(b, len) bzero(b, len) +#endif + #ifndef HAVE_STRTOF extern float strtof(const char *nptr, char **endptr); #endif diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index e58fa6742a..67ea169397 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -3886,7 +3886,10 @@ freePGconn(PGconn *conn) if (conn->connhost[i].port != NULL) free(conn->connhost[i].port); if (conn->connhost[i].password != NULL) + { + explicit_bzero(conn->connhost[i].password, strlen(conn->connhost[i].password)); free(conn->connhost[i].password); + } } free(conn->connhost); } @@ -3920,7 +3923,10 @@ freePGconn(PGconn *conn) if (conn->pguser) free(conn->pguser); if (conn->pgpass) + { + explicit_bzero(conn->pgpass, strlen(conn->pgpass)); free(conn->pgpass); + } if (conn->pgpassfile) free(conn->pgpassfile); if (conn->keepalives) @@ -6939,6 +6945,7 @@ passwordFromFile(const char *hostname, const char *port, const char *dbname, if (!ret) { /* Out of memory. XXX: an error message would be nice. */ + explicit_bzero(buf, sizeof(buf)); return NULL; } @@ -6955,6 +6962,7 @@ passwordFromFile(const char *hostname, const char *port, const char *dbname, } fclose(fp); + explicit_bzero(buf, sizeof(buf)); return NULL; #undef LINELEN -- 2.22.0