From 2764d5dcfa84d240c901c20ec6e194f72d82b78a Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Mon, 2 Apr 2018 11:34:52 -0400 Subject: [PATCH] Make be-secure-common.c more consistent for future SSL implementations Recent commit 8a3d9425 has introduced be-secure-common.c, which is aimed at including backend-side APIs that can be used by any SSL implementation. The purpose is similar to fe-secure-common.c for the frontend-side APIs. However, this has forgotten to include check_ssl_key_file_permissions() in the move, which causes a double dependency between be-secure.c and be-secure-openssl.c. Refactor the code in a more logical way. This also puts into light an API which is usable by future SSL implementations for permissions on SSL key files. Author: Michael Paquier --- src/backend/libpq/be-secure-common.c | 74 ++++++++++++++++++++++++++++ src/backend/libpq/be-secure.c | 69 -------------------------- src/include/libpq/libpq.h | 3 +- 3 files changed, 76 insertions(+), 70 deletions(-) diff --git a/src/backend/libpq/be-secure-common.c b/src/backend/libpq/be-secure-common.c index d1740967f1..46f0b5f4a3 100644 --- a/src/backend/libpq/be-secure-common.c +++ b/src/backend/libpq/be-secure-common.c @@ -19,6 +19,9 @@ #include "postgres.h" +#include +#include + #include "libpq/libpq.h" #include "storage/fd.h" @@ -118,3 +121,74 @@ error: pfree(command.data); return len; } + + +/* + * Check permissions for SSL key files. + */ +bool +check_ssl_key_file_permissions(const char *ssl_key_file, bool isServerStart) +{ + int loglevel = isServerStart ? FATAL : LOG; + struct stat buf; + + if (stat(ssl_key_file, &buf) != 0) + { + ereport(loglevel, + (errcode_for_file_access(), + errmsg("could not access private key file \"%s\": %m", + ssl_key_file))); + return false; + } + + if (!S_ISREG(buf.st_mode)) + { + ereport(loglevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("private key file \"%s\" is not a regular file", + ssl_key_file))); + return false; + } + + /* + * Refuse to load key files owned by users other than us or root. + * + * XXX surely we can check this on Windows somehow, too. + */ +#if !defined(WIN32) && !defined(__CYGWIN__) + if (buf.st_uid != geteuid() && buf.st_uid != 0) + { + ereport(loglevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("private key file \"%s\" must be owned by the database user or root", + ssl_key_file))); + return false; + } +#endif + + /* + * Require no public access to key file. If the file is owned by us, + * require mode 0600 or less. If owned by root, require 0640 or less to + * allow read access through our gid, or a supplementary gid that allows + * to read system-wide certificates. + * + * XXX temporarily suppress check when on Windows, because there may not + * be proper support for Unix-y file permissions. Need to think of a + * reasonable check to apply on Windows. (See also the data directory + * permission check in postmaster.c) + */ +#if !defined(WIN32) && !defined(__CYGWIN__) + if ((buf.st_uid == geteuid() && buf.st_mode & (S_IRWXG | S_IRWXO)) || + (buf.st_uid == 0 && buf.st_mode & (S_IWGRP | S_IXGRP | S_IRWXO))) + { + ereport(loglevel, + (errcode(ERRCODE_CONFIG_FILE_ERROR), + errmsg("private key file \"%s\" has group or world access", + ssl_key_file), + errdetail("File must have permissions u=rw (0600) or less if owned by the database user, or permissions u=rw,g=r (0640) or less if owned by root."))); + return false; + } +#endif + + return true; +} diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c index fb1f6b5bbe..edfe2c0751 100644 --- a/src/backend/libpq/be-secure.c +++ b/src/backend/libpq/be-secure.c @@ -18,12 +18,10 @@ #include "postgres.h" -#include #include #include #include #include -#include #include #include #ifdef HAVE_NETINET_TCP_H @@ -320,70 +318,3 @@ secure_raw_write(Port *port, const void *ptr, size_t len) return n; } - -bool -check_ssl_key_file_permissions(const char *ssl_key_file, bool isServerStart) -{ - int loglevel = isServerStart ? FATAL : LOG; - struct stat buf; - - if (stat(ssl_key_file, &buf) != 0) - { - ereport(loglevel, - (errcode_for_file_access(), - errmsg("could not access private key file \"%s\": %m", - ssl_key_file))); - return false; - } - - if (!S_ISREG(buf.st_mode)) - { - ereport(loglevel, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("private key file \"%s\" is not a regular file", - ssl_key_file))); - return false; - } - - /* - * Refuse to load key files owned by users other than us or root. - * - * XXX surely we can check this on Windows somehow, too. - */ -#if !defined(WIN32) && !defined(__CYGWIN__) - if (buf.st_uid != geteuid() && buf.st_uid != 0) - { - ereport(loglevel, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("private key file \"%s\" must be owned by the database user or root", - ssl_key_file))); - return false; - } -#endif - - /* - * Require no public access to key file. If the file is owned by us, - * require mode 0600 or less. If owned by root, require 0640 or less to - * allow read access through our gid, or a supplementary gid that allows - * to read system-wide certificates. - * - * XXX temporarily suppress check when on Windows, because there may not - * be proper support for Unix-y file permissions. Need to think of a - * reasonable check to apply on Windows. (See also the data directory - * permission check in postmaster.c) - */ -#if !defined(WIN32) && !defined(__CYGWIN__) - if ((buf.st_uid == geteuid() && buf.st_mode & (S_IRWXG | S_IRWXO)) || - (buf.st_uid == 0 && buf.st_mode & (S_IWGRP | S_IXGRP | S_IRWXO))) - { - ereport(loglevel, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("private key file \"%s\" has group or world access", - ssl_key_file), - errdetail("File must have permissions u=rw (0600) or less if owned by the database user, or permissions u=rw,g=r (0640) or less if owned by root."))); - return false; - } -#endif - - return true; -} diff --git a/src/include/libpq/libpq.h b/src/include/libpq/libpq.h index 997947b091..a74ad521b5 100644 --- a/src/include/libpq/libpq.h +++ b/src/include/libpq/libpq.h @@ -92,7 +92,6 @@ extern ssize_t secure_read(Port *port, void *ptr, size_t len); extern ssize_t secure_write(Port *port, void *ptr, size_t len); extern ssize_t secure_raw_read(Port *port, void *ptr, size_t len); extern ssize_t secure_raw_write(Port *port, const void *ptr, size_t len); -extern bool check_ssl_key_file_permissions(const char *ssl_key_file, bool isServerStart); extern bool ssl_loaded_verify_locations; @@ -108,5 +107,7 @@ extern bool SSLPreferServerCiphers; */ extern int run_ssl_passphrase_command(const char *prompt, bool is_server_start, char *buf, int size); +extern bool check_ssl_key_file_permissions(const char *ssl_key_file, + bool isServerStart); #endif /* LIBPQ_H */