From 3405f2b92532cb9559aed1316b3df2262aaadaef Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 31 Mar 2008 02:43:14 +0000 Subject: [PATCH] Use error message wordings for permissions checks on .pgpass and SSL private key files that are similar to the one for the postmaster's data directory permissions check. (I chose to standardize on that one since it's the most heavily used and presumably best-wordsmithed by now.) Also eliminate explicit tests on file ownership in these places, since the ensuing read attempt must fail anyway if it's wrong, and there seems no value in issuing the same error message for distinct problems. (But I left in the explicit ownership test in postmaster.c, since it had its own error message anyway.) Also be more specific in the documentation's descriptions of these checks. Per a gripe from Kevin Hunter. --- doc/src/sgml/libpq.sgml | 12 +++++++----- doc/src/sgml/runtime.sgml | 15 ++++++++++++--- src/backend/libpq/be-secure.c | 11 +++++------ src/backend/postmaster/postmaster.c | 9 ++++++++- src/interfaces/libpq/fe-connect.c | 7 +++---- src/interfaces/libpq/fe-secure.c | 9 ++++----- 6 files changed, 39 insertions(+), 24 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 9e63c91641..2f1a248ca7 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1,4 +1,4 @@ - + <application>libpq</application> - C Library @@ -5223,11 +5223,13 @@ defaultNoticeProcessor(void *arg, const char *message) authorities (CA) trusted by the server. A matching private key file ~/.postgresql/postgresql.key must also be present, unless the secret key for the certificate is stored in a - hardware token, as specified by PGSSLKEY. (On Microsoft - Windows these files are named + hardware token, as specified by PGSSLKEY. The private + key file must not allow any access to world or group; achieve this by the + command chmod 0600 ~/.postgresql/postgresql.key. + On Microsoft Windows these files are named %APPDATA%\postgresql\postgresql.crt and - %APPDATA%\postgresql\postgresql.key.) The private - key file must not be world-readable. + %APPDATA%\postgresql\postgresql.key, and there + is no special permissions check since the directory is presumed secure. diff --git a/doc/src/sgml/runtime.sgml b/doc/src/sgml/runtime.sgml index 7d89be32fb..1c799d8965 100644 --- a/doc/src/sgml/runtime.sgml +++ b/doc/src/sgml/runtime.sgml @@ -1,4 +1,4 @@ - + Operating System Environment @@ -1632,7 +1632,11 @@ $ kill -INT `head -1 /usr/local/pgsql/data/postmaster.pid`SSL mode, the files server.crt and server.key must exist in the server's data directory. These files should contain the server certificate and private key, - respectively. If the private key is protected with a passphrase, the + respectively. + On Unix systems, the permissions on server.key must + disallow any access to world or group; achieve this by the command + chmod 0600 server.key. + If the private key is protected with a passphrase, the server will prompt for the passphrase and will not start until it has been entered. @@ -1731,10 +1735,15 @@ rm privkey.pem Enter the old passphrase to unlock the existing key. Now do: openssl req -x509 -in server.req -text -key server.key -out server.crt -chmod og-rwx server.key to turn the certificate into a self-signed certificate and to copy the key and certificate to where the server will look for them. + Finally do + +chmod og-rwx server.key + + because the server will reject the file if its permissions are more + liberal than this. For more details on how to create your server private key and certificate, refer to the OpenSSL documentation. diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c index 923ad6bcb3..d036947176 100644 --- a/src/backend/libpq/be-secure.c +++ b/src/backend/libpq/be-secure.c @@ -11,7 +11,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.83 2008/01/01 19:45:49 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/libpq/be-secure.c,v 1.84 2008/03/31 02:43:14 tgl Exp $ * * Since the server static private key ($DataDir/server.key) * will normally be stored unencrypted so that the database @@ -735,7 +735,7 @@ initialize_SSL(void) errmsg("could not load server certificate file \"%s\": %s", SERVER_CERT_FILE, SSLerrmessage()))); - if (stat(SERVER_PRIVATE_KEY_FILE, &buf) == -1) + if (stat(SERVER_PRIVATE_KEY_FILE, &buf) != 0) ereport(FATAL, (errcode_for_file_access(), errmsg("could not access private key file \"%s\": %m", @@ -750,13 +750,12 @@ initialize_SSL(void) * directory permission check in postmaster.c) */ #if !defined(WIN32) && !defined(__CYGWIN__) - if (!S_ISREG(buf.st_mode) || (buf.st_mode & (S_IRWXG | S_IRWXO)) || - buf.st_uid != geteuid()) + if (!S_ISREG(buf.st_mode) || buf.st_mode & (S_IRWXG | S_IRWXO)) ereport(FATAL, (errcode(ERRCODE_CONFIG_FILE_ERROR), - errmsg("unsafe permissions on private key file \"%s\"", + errmsg("private key file \"%s\" has group or world access", SERVER_PRIVATE_KEY_FILE), - errdetail("File must be owned by the database user and must have no permissions for \"group\" or \"other\"."))); + errdetail("Permissions should be u=rw (0600) or less."))); #endif if (!SSL_CTX_use_PrivateKey_file(SSL_context, diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 7a83e075b1..7c6692b2a5 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -37,7 +37,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.553 2008/03/09 04:56:28 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.554 2008/03/31 02:43:14 tgl Exp $ * * NOTES * @@ -1053,6 +1053,13 @@ checkDataDir(void) DataDir))); } + /* eventual chdir would fail anyway, but let's test ... */ + if (!S_ISDIR(stat_buf.st_mode)) + ereport(FATAL, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("specified data directory \"%s\" is not a directory", + DataDir))); + /* * Check that the directory belongs to my userid; if not, reject. * diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 90f9d4334e..dd6c988790 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.356 2008/01/29 02:06:30 tgl Exp $ + * $PostgreSQL: pgsql/src/interfaces/libpq/fe-connect.c,v 1.357 2008/03/31 02:43:14 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -3718,11 +3718,10 @@ PasswordFromFile(char *hostname, char *port, char *dbname, char *username) } /* If password file cannot be opened, ignore it. */ - if (stat(pgpassfile, &stat_buf) == -1) + if (stat(pgpassfile, &stat_buf) != 0) return NULL; #ifndef WIN32 - if (!S_ISREG(stat_buf.st_mode)) { fprintf(stderr, @@ -3735,7 +3734,7 @@ PasswordFromFile(char *hostname, char *port, char *dbname, char *username) if (stat_buf.st_mode & (S_IRWXG | S_IRWXO)) { fprintf(stderr, - libpq_gettext("WARNING: password file \"%s\" has world or group read access; permission should be u=rw (0600)\n"), + libpq_gettext("WARNING: password file \"%s\" has group or world access; permissions should be u=rw (0600) or less\n"), pgpassfile); return NULL; } diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c index 3b1f4cee60..4414baba4a 100644 --- a/src/interfaces/libpq/fe-secure.c +++ b/src/interfaces/libpq/fe-secure.c @@ -11,7 +11,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.103 2008/02/16 21:03:30 momjian Exp $ + * $PostgreSQL: pgsql/src/interfaces/libpq/fe-secure.c,v 1.104 2008/03/31 02:43:14 tgl Exp $ * * NOTES * [ Most of these notes are wrong/obsolete, but perhaps not all ] @@ -703,7 +703,7 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey) { /* read the user key from file */ snprintf(fnbuf, sizeof(fnbuf), "%s/%s", homedir, USER_KEY_FILE); - if (stat(fnbuf, &buf) == -1) + if (stat(fnbuf, &buf) != 0) { printfPQExpBuffer(&conn->errorMessage, libpq_gettext("certificate present, but not private key file \"%s\"\n"), @@ -712,11 +712,10 @@ client_cert_cb(SSL *ssl, X509 **x509, EVP_PKEY **pkey) return 0; } #ifndef WIN32 - if (!S_ISREG(buf.st_mode) || (buf.st_mode & 0077) || - buf.st_uid != geteuid()) + if (!S_ISREG(buf.st_mode) || buf.st_mode & (S_IRWXG | S_IRWXO)) { printfPQExpBuffer(&conn->errorMessage, - libpq_gettext("private key file \"%s\" has wrong permissions\n"), + libpq_gettext("private key file \"%s\" has group or world access; permissions should be u=rw (0600) or less\n"), fnbuf); ERR_pop_to_mark(); return 0; -- 2.40.0