From 4ce82a167b3a7bc4b0f00f8794441d4d7bcb9edd Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 19 Jan 2018 10:17:56 -0500 Subject: [PATCH 5/5] Extract common bits from OpenSSL implementation Some things in be-secure-openssl.c and fe-secure-openssl.c were not actually specific to OpenSSL but could also be used by other implementations. In order to avoid copy-and-pasting, move some of that code to common files. --- src/backend/libpq/be-secure-openssl.c | 62 +--------------------------- src/backend/libpq/be-secure.c | 71 ++++++++++++++++++++++++++++++++ src/include/libpq/libpq.h | 1 + src/interfaces/libpq/fe-secure-openssl.c | 8 ---- src/interfaces/libpq/fe-secure.c | 14 ++++--- 5 files changed, 81 insertions(+), 75 deletions(-) diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index f550ec82a9..02601da6c8 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -75,7 +75,6 @@ be_tls_init(bool isServerStart) { STACK_OF(X509_NAME) *root_cert_list = NULL; SSL_CTX *context; - struct stat buf; /* This stuff need be done only once. */ if (!SSL_initialized) @@ -133,63 +132,8 @@ be_tls_init(bool isServerStart) goto error; } - if (stat(ssl_key_file, &buf) != 0) - { - ereport(isServerStart ? FATAL : LOG, - (errcode_for_file_access(), - errmsg("could not access private key file \"%s\": %m", - ssl_key_file))); + if (!check_ssl_key_file_permissions(ssl_key_file, isServerStart)) goto error; - } - - if (!S_ISREG(buf.st_mode)) - { - ereport(isServerStart ? FATAL : LOG, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("private key file \"%s\" is not a regular file", - ssl_key_file))); - goto error; - } - - /* - * 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(isServerStart ? FATAL : LOG, - (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("private key file \"%s\" must be owned by the database user or root", - ssl_key_file))); - goto error; - } -#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(isServerStart ? FATAL : LOG, - (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."))); - goto error; - } -#endif /* * OK, try to load the private key file. @@ -516,10 +460,6 @@ be_tls_open_server(Port *port) port->peer_cert_valid = true; } - ereport(DEBUG2, - (errmsg("SSL connection from \"%s\"", - port->peer_cn ? port->peer_cn : "(anonymous)"))); - /* set up debugging/info callback */ SSL_CTX_set_info_callback(SSL_context, info_cb); diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c index eb42ea1a1e..76c0a9e39b 100644 --- a/src/backend/libpq/be-secure.c +++ b/src/backend/libpq/be-secure.c @@ -114,6 +114,10 @@ secure_open_server(Port *port) #ifdef USE_SSL r = be_tls_open_server(port); + + ereport(DEBUG2, + (errmsg("SSL connection from \"%s\"", + port->peer_cn ? port->peer_cn : "(anonymous)"))); #endif return r; @@ -314,3 +318,70 @@ 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 2e7725db21..255222acd7 100644 --- a/src/include/libpq/libpq.h +++ b/src/include/libpq/libpq.h @@ -90,6 +90,7 @@ 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; diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index eb13120941..9ab317320a 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -1547,14 +1547,6 @@ SSLerrfree(char *buf) /* SSL information functions */ /* ------------------------------------------------------------ */ -int -PQsslInUse(PGconn *conn) -{ - if (!conn) - return 0; - return conn->ssl_in_use; -} - /* * Return pointer to OpenSSL object. */ diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c index ec6c65a4b4..cfb77f6d85 100644 --- a/src/interfaces/libpq/fe-secure.c +++ b/src/interfaces/libpq/fe-secure.c @@ -129,6 +129,14 @@ struct sigpipe_info /* ------------------------------------------------------------ */ +int +PQsslInUse(PGconn *conn) +{ + if (!conn) + return 0; + return conn->ssl_in_use; +} + /* * Exported function to allow application to tell us it's already * initialized OpenSSL. @@ -384,12 +392,6 @@ pqsecure_raw_write(PGconn *conn, const void *ptr, size_t len) /* Dummy versions of SSL info functions, when built without SSL support */ #ifndef USE_SSL -int -PQsslInUse(PGconn *conn) -{ - return 0; -} - void * PQgetssl(PGconn *conn) { -- 2.15.1