Allow root-owned SSL private keys in libpq, not only the backend.

This change makes libpq apply the same private-key-file ownership
and permissions checks that we have used in the backend since commit
9a83564c5.  Namely, that the private key can be owned by either the
current user or root (with different file permissions allowed in the
two cases).  This allows system-wide management of key files, which
is just as sensible on the client side as the server, particularly
when the client is itself some application daemon.

Sync the comments about this between libpq and the backend, too.

Back-patch of a59c79564 and 50f03473e into all supported branches.

David Steele

Discussion: https://postgr.es/m/f4b7bc55-97ac-9e69-7398-335e212f7743@pgmasters.net

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 1b76ef2db73118706c39991d69143fe13e1c7ee7..e452773df390e7925060679e1dbc793908ded1ed 100644 (file)
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -7837,23 +7837,35 @@ ldap://ldap.acme.com/cn=dbserver,cn=hosts?pgconnectinfo?base?(objectclass=*)
   <para>
    If the server attempts to verify the identity of the
    client by requesting the client's leaf certificate,
-   <application>libpq</application> will send the certificates stored in
+   <application>libpq</application> will send the certificate(s) stored in
    file <filename>~/.postgresql/postgresql.crt</filename> in the user's home
    directory.  The certificates must chain to the root certificate trusted
    by the server.  A matching
    private key file <filename>~/.postgresql/postgresql.key</filename> must also
-   be present. The private
-   key file must not allow any access to world or group; achieve this by the
-   command <command>chmod 0600 ~/.postgresql/postgresql.key</command>.
+   be present.
    On Microsoft Windows these files are named
    <filename>%APPDATA%\postgresql\postgresql.crt</filename> and
-   <filename>%APPDATA%\postgresql\postgresql.key</filename>, and there
-   is no special permissions check since the directory is presumed secure.
+   <filename>%APPDATA%\postgresql\postgresql.key</filename>.
    The location of the certificate and key files can be overridden by the
-   connection parameters <literal>sslcert</literal> and <literal>sslkey</literal> or the
+   connection parameters <literal>sslcert</literal>
+   and <literal>sslkey</literal>, or by the
    environment variables <envar>PGSSLCERT</envar> and <envar>PGSSLKEY</envar>.
   </para>
 
+  <para>
+   On Unix systems, the permissions on the private key file must disallow
+   any access to world or group; achieve this by a command such as
+   <command>chmod 0600 ~/.postgresql/postgresql.key</command>.
+   Alternatively, the file can be owned by root and have group read access
+   (that is, <literal>0640</literal> permissions).  That setup is intended
+   for installations where certificate and key files are managed by the
+   operating system.  The user of <application>libpq</application> should
+   then be made a member of the group that has access to those certificate
+   and key files.  (On Microsoft Windows, there is no file permissions
+   check, since the <filename>%APPDATA%\postgresql</filename> directory is
+   presumed secure.)
+  </para>
+
   <para>
    The first certificate in <filename>postgresql.crt</filename> must be the
    client's certificate because it must match the client's private key.

diff --git a/src/backend/libpq/be-secure-common.c b/src/backend/libpq/be-secure-common.c
index bee618abb931f17262a271c9a1393403132e4789..9af4ff15cd68b09da1c2d56f9ede1448a132d4a7 100644 (file)
--- a/src/backend/libpq/be-secure-common.c
+++ b/src/backend/libpq/be-secure-common.c
@@ -142,6 +142,7 @@ check_ssl_key_file_permissions(const char *ssl_key_file, bool isServerStart)
        return false;
    }
 
+   /* Key file must be a regular file */
    if (!S_ISREG(buf.st_mode))
    {
        ereport(loglevel,
@@ -152,9 +153,19 @@ check_ssl_key_file_permissions(const char *ssl_key_file, bool isServerStart)
    }
 
    /*
-    * Refuse to load key files owned by users other than us or root.
+    * Refuse to load key files owned by users other than us or root, and
+    * require no public access to the 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 either our gid or a supplementary gid that
+    * allows us to read system-wide certificates.
     *
-    * XXX surely we can check this on Windows somehow, too.
+    * Note that similar checks are performed in
+    * src/interfaces/libpq/fe-secure-openssl.c so any changes here may need
+    * to be made there as well.
+    *
+    * Ideally we would do similar permissions checks on Windows, but it is
+    * not clear how that would work since Unix-style permissions may not be
+    * available.
     */
 #if !defined(WIN32) && !defined(__CYGWIN__)
    if (buf.st_uid != geteuid() && buf.st_uid != 0)
@@ -165,20 +176,7 @@ check_ssl_key_file_permissions(const char *ssl_key_file, bool isServerStart)
                        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)))
    {

diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index 7aa7a06ad241c278abc813ac8619a0c9380cc82b..37b963a77ac0a64d02c9b1d59d35933beb802b5f 100644 (file)
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1121,11 +1121,45 @@ initialize_SSL(PGconn *conn)
                              fnbuf);
            return -1;
        }
-#ifndef WIN32
-       if (!S_ISREG(buf.st_mode) || buf.st_mode & (S_IRWXG | S_IRWXO))
+
+       /* Key file must be a regular file */
+       if (!S_ISREG(buf.st_mode))
+       {
+           printfPQExpBuffer(&conn->errorMessage,
+                             libpq_gettext("private key file \"%s\" is not a regular file"),
+                             fnbuf);
+           return -1;
+       }
+
+       /*
+        * Refuse to load key files owned by users other than us or root, and
+        * require no public access to the 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 either our gid or a supplementary
+        * gid that allows us to read system-wide certificates.
+        *
+        * Note that similar checks are performed in
+        * src/backend/libpq/be-secure-common.c so any changes here may need
+        * to be made there as well.
+        *
+        * Ideally we would do similar permissions checks on Windows, but it
+        * is not clear how that would work since Unix-style permissions may
+        * not be available.
+        */
+#if !defined(WIN32) && !defined(__CYGWIN__)
+       if (buf.st_uid != geteuid() && buf.st_uid != 0)
+       {
+           printfPQExpBuffer(&conn->errorMessage,
+                             libpq_gettext("private key file \"%s\" must be owned by the current user or root\n"),
+                             fnbuf);
+           return -1;
+       }
+
+       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)))
        {
            printfPQExpBuffer(&conn->errorMessage,
-                             libpq_gettext("private key file \"%s\" has group or world access; permissions should be u=rw (0600) or less\n"),
+                             libpq_gettext("private key file \"%s\" has group or world access; file must have permissions u=rw (0600) or less if owned by the current user, or permissions u=rw,g=r (0640) or less if owned by root\n"),
                              fnbuf);
            return -1;
        }