client-key-perm-003.patch

text/plain

Filename: client-key-perm-003.patch
Type: text/plain
Part: 0
Message: Re: Allow root ownership of client certificate key

Patch

Format: unified
File+
src/backend/libpq/be-secure-common.c 15 17
src/interfaces/libpq/fe-secure-openssl.c 36 2
diff --git a/src/backend/libpq/be-secure-common.c b/src/backend/libpq/be-secure-common.c
index 7e9a64d08e..3f00040efe 100644
--- a/src/backend/libpq/be-secure-common.c
+++ b/src/backend/libpq/be-secure-common.c
@@ -143,6 +143,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,
@@ -153,10 +154,20 @@ 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.
-	 *
-	 * XXX surely we can check this on Windows somehow, too.
-	 */
+	* 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 our gid, or a supplementary gid that allows
+	* us to read system-wide certificates.
+	*
+	* 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)
 	{
@@ -166,20 +177,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 f6e563a2e5..59e3224f97 100644
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -1245,11 +1245,45 @@ initialize_SSL(PGconn *conn)
 								  fnbuf);
 			return -1;
 		}
+
+		/* Key file must be a regular file */
+		if (!S_ISREG(buf.st_mode))
+		{
+			appendPQExpBuffer(&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 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.
+		*/
 #ifndef WIN32
-		if (!S_ISREG(buf.st_mode) || buf.st_mode & (S_IRWXG | S_IRWXO))
+		if (buf.st_uid != geteuid() && buf.st_uid != 0)
+		{
+			appendPQExpBuffer(&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)))
 		{
 			appendPQExpBuffer(&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;
 		}