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
