Add a log_file_mode GUC that allows control of the file permissions set on

log files created by the syslogger process.

In passing, make unix_file_permissions display its value in octal, same
as log_file_mode now does.

Martin Pihlak

doc/src/sgml/config.sgml

@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/config.sgml,v 1.295 2010/07/16 11:20:23 heikki Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/config.sgml,v 1.296 2010/07/16 22:25:47 tgl Exp $ -->
 
 <chapter Id="runtime-config">
   <title>Server Configuration</title>
@@ -2844,6 +2844,39 @@ local0.*    /var/log/postgresql
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-log-file-mode" xreflabel="log_file_mode">
+      <term><varname>log_file_mode</varname> (<type>integer</type>)</term>
+      <indexterm>
+       <primary><varname>log_file_mode</> configuration parameter</primary>
+      </indexterm>
+      <listitem>
+       <para>
+        On Unix systems this parameter sets the permissions for log files
+        when <varname>logging_collector</varname> is enabled. (On Microsoft
+        Windows this parameter is ignored.)
+        The parameter value is expected to be a numeric mode
+        specified in the format accepted by the
+        <function>chmod</function> and <function>umask</function>
+        system calls.  (To use the customary octal format the number
+        must start with a <literal>0</literal> (zero).)
+       </para>
+       <para>
+        The default permissions are <literal>0600</>, meaning only the
+        server owner can read or write the log files.  The other commonly
+        useful setting is <literal>0640</>, allowing members of the owner's
+        group to read the files.  Note however that to make use of such a
+        setting, you'll need to alter <xref linkend="guc-log-directory"> to
+        store the files somewhere outside the cluster data directory.  In
+        any case, it's unwise to make the log files world-readable, since
+        they might contain sensitive data.
+       </para>
+       <para>
+        This parameter can only be set in the <filename>postgresql.conf</>
+        file or on the server command line.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-log-rotation-age" xreflabel="log_rotation_age">
       <term><varname>log_rotation_age</varname> (<type>integer</type>)</term>
       <indexterm>

src/backend/postmaster/syslogger.c

@@ -18,7 +18,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/postmaster/syslogger.c,v 1.58 2010/07/06 19:18:57 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/postmaster/syslogger.c,v 1.59 2010/07/16 22:25:50 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -73,6 +73,7 @@ int			Log_RotationSize = 10 * 1024;
 char	   *Log_directory = NULL;
 char	   *Log_filename = NULL;
 bool		Log_truncate_on_rotation = false;
+int			Log_file_mode = 0600;
 
 /*
  * Globally visible state (used by elog.c)
@@ -135,6 +136,8 @@ static void syslogger_parseArgs(int argc, char *argv[]);
 static void process_pipe_input(char *logbuffer, int *bytes_in_logbuffer);
 static void flush_pipe_input(char *logbuffer, int *bytes_in_logbuffer);
 static void open_csvlogfile(void);
+static FILE *logfile_open(const char *filename, const char *mode,
+						  bool allow_errors);
 
 #ifdef WIN32
 static unsigned int __stdcall pipeThread(void *arg);
@@ -516,15 +519,7 @@ SysLogger_Start(void)
 	 */
 	filename = logfile_getname(time(NULL), NULL);
 
-	syslogFile = fopen(filename, "a");
+	syslogFile = logfile_open(filename, "a", false);
-
-	if (!syslogFile)
-		ereport(FATAL,
-				(errcode_for_file_access(),
-				 (errmsg("could not create log file \"%s\": %m",
-						 filename))));
-
-	setvbuf(syslogFile, NULL, LBF_MODE, 0);
 
 	pfree(filename);
 
@@ -1000,28 +995,56 @@ static void
 open_csvlogfile(void)
 {
 	char	   *filename;
-	FILE	   *fh;
 
 	filename = logfile_getname(time(NULL), ".csv");
 
-	fh = fopen(filename, "a");
+	csvlogFile = logfile_open(filename, "a", false);
-
-	if (!fh)
-		ereport(FATAL,
-				(errcode_for_file_access(),
-				 (errmsg("could not create log file \"%s\": %m",
-						 filename))));
-
-	setvbuf(fh, NULL, LBF_MODE, 0);
-
-#ifdef WIN32
-	_setmode(_fileno(fh), _O_TEXT);		/* use CRLF line endings on Windows */
-#endif
-
-	csvlogFile = fh;
 
 	pfree(filename);
+}
 
+/*
+ * Open a new logfile with proper permissions and buffering options.
+ *
+ * If allow_errors is true, we just log any open failure and return NULL
+ * (with errno still correct for the fopen failure).
+ * Otherwise, errors are treated as fatal.
+ */
+static FILE *
+logfile_open(const char *filename, const char *mode, bool allow_errors)
+{
+	FILE	   *fh;
+	mode_t		oumask;
+
+	/*
+	 * Note we do not let Log_file_mode disable IWUSR, since we certainly
+	 * want to be able to write the files ourselves.
+	 */
+	oumask = umask((mode_t) ((~(Log_file_mode | S_IWUSR)) & 0777));
+	fh = fopen(filename, mode);
+	umask(oumask);
+
+	if (fh)
+	{
+		setvbuf(fh, NULL, LBF_MODE, 0);
+
+#ifdef WIN32
+		/* use CRLF line endings on Windows */
+		_setmode(_fileno(fh), _O_TEXT);
+#endif
+	}
+	else
+	{
+		int		save_errno = errno;
+
+		ereport(allow_errors ? LOG : FATAL,
+				(errcode_for_file_access(),
+				 errmsg("could not open log file \"%s\": %m",
+						filename)));
+		errno = save_errno;
+	}
+
+	return fh;
 }
 
 /*
@@ -1070,26 +1093,19 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 		if (Log_truncate_on_rotation && time_based_rotation &&
 			last_file_name != NULL &&
 			strcmp(filename, last_file_name) != 0)
-			fh = fopen(filename, "w");
+			fh = logfile_open(filename, "w", true);
 		else
-			fh = fopen(filename, "a");
+			fh = logfile_open(filename, "a", true);
 
 		if (!fh)
 		{
-			int			saveerrno = errno;
-
-			ereport(LOG,
-					(errcode_for_file_access(),
-					 errmsg("could not open new log file \"%s\": %m",
-							filename)));
-
 			/*
 			 * ENFILE/EMFILE are not too surprising on a busy system; just
 			 * keep using the old file till we manage to get a new one.
 			 * Otherwise, assume something's wrong with Log_directory and stop
 			 * trying to create files.
 			 */
-			if (saveerrno != ENFILE && saveerrno != EMFILE)
+			if (errno != ENFILE && errno != EMFILE)
 			{
 				ereport(LOG,
 						(errmsg("disabling automatic rotation (use SIGHUP to re-enable)")));
@@ -1104,12 +1120,6 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 			return;
 		}
 
-		setvbuf(fh, NULL, LBF_MODE, 0);
-
-#ifdef WIN32
-		_setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */
-#endif
-
 		fclose(syslogFile);
 		syslogFile = fh;
 
@@ -1128,26 +1138,19 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 		if (Log_truncate_on_rotation && time_based_rotation &&
 			last_csv_file_name != NULL &&
 			strcmp(csvfilename, last_csv_file_name) != 0)
-			fh = fopen(csvfilename, "w");
+			fh = logfile_open(csvfilename, "w", true);
 		else
-			fh = fopen(csvfilename, "a");
+			fh = logfile_open(csvfilename, "a", true);
 
 		if (!fh)
 		{
-			int			saveerrno = errno;
-
-			ereport(LOG,
-					(errcode_for_file_access(),
-					 errmsg("could not open new log file \"%s\": %m",
-							csvfilename)));
-
 			/*
 			 * ENFILE/EMFILE are not too surprising on a busy system; just
 			 * keep using the old file till we manage to get a new one.
 			 * Otherwise, assume something's wrong with Log_directory and stop
 			 * trying to create files.
 			 */
-			if (saveerrno != ENFILE && saveerrno != EMFILE)
+			if (errno != ENFILE && errno != EMFILE)
 			{
 				ereport(LOG,
 						(errmsg("disabling automatic rotation (use SIGHUP to re-enable)")));
@@ -1162,12 +1165,6 @@ logfile_rotate(bool time_based_rotation, int size_rotation_for)
 			return;
 		}
 
-		setvbuf(fh, NULL, LBF_MODE, 0);
-
-#ifdef WIN32
-		_setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */
-#endif
-
 		fclose(csvlogFile);
 		csvlogFile = fh;
 

src/backend/utils/misc/guc.c

@@ -10,7 +10,7 @@
  * Written by Peter Eisentraut <peter_e@gmx.net>.
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.561 2010/07/06 22:55:26 rhaas Exp $
+ *	  $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.562 2010/07/16 22:25:50 tgl Exp $
  *
  *--------------------------------------------------------------------
  */
@@ -174,6 +174,8 @@ static bool assign_autovacuum_max_workers(int newval, bool doit, GucSource sourc
 static bool assign_effective_io_concurrency(int newval, bool doit, GucSource source);
 static const char *assign_pgstat_temp_directory(const char *newval, bool doit, GucSource source);
 static const char *assign_application_name(const char *newval, bool doit, GucSource source);
+static const char *show_unix_socket_permissions(void);
+static const char *show_log_file_mode(void);
 
 static char *config_enum_get_options(struct config_enum * record,
 						const char *prefix, const char *suffix,
@@ -1454,13 +1456,27 @@ static struct config_int ConfigureNamesInt[] =
 		{"unix_socket_permissions", PGC_POSTMASTER, CONN_AUTH_SETTINGS,
 			gettext_noop("Sets the access permissions of the Unix-domain socket."),
 			gettext_noop("Unix-domain sockets use the usual Unix file system "
-						 "permission set. The parameter value is expected to be a numeric mode "
+						 "permission set. The parameter value is expected "
-						 "specification in the form accepted by the chmod and umask system "
+						 "to be a numeric mode specification in the form "
-						 "calls. (To use the customary octal format the number must start with "
+						 "accepted by the chmod and umask system calls. "
-						 "a 0 (zero).)")
+						 "(To use the customary octal format the number must "
+						 "start with a 0 (zero).)")
 		},
 		&Unix_socket_permissions,
-		0777, 0000, 0777, NULL, NULL
+		0777, 0000, 0777, NULL, show_unix_socket_permissions
+	},
+
+	{
+		{"log_file_mode", PGC_SIGHUP, LOGGING_WHERE,
+			gettext_noop("Sets the file permissions for log files."),
+			gettext_noop("The parameter value is expected "
+						 "to be a numeric mode specification in the form "
+						 "accepted by the chmod and umask system calls. "
+						 "(To use the customary octal format the number must "
+						 "start with a 0 (zero).)")
+		},
+		&Log_file_mode,
+		0600, 0000, 0777, NULL, show_log_file_mode
 	},
 
 	{
@@ -8084,4 +8100,22 @@ assign_application_name(const char *newval, bool doit, GucSource source)
 		return newval;
 }
 
+static const char *
+show_unix_socket_permissions(void)
+{
+	static char buf[8];
+
+	snprintf(buf, sizeof(buf), "%04o", Unix_socket_permissions);
+	return buf;
+}
+
+static const char *
+show_log_file_mode(void)
+{
+	static char buf[8];
+
+	snprintf(buf, sizeof(buf), "%04o", Log_file_mode);
+	return buf;
+}
+
 #include "guc-file.c"

src/backend/utils/misc/postgresql.conf.sample

@@ -269,6 +269,8 @@
 					# can be absolute or relative to PGDATA
 #log_filename = 'postgresql-%Y-%m-%d_%H%M%S.log'	# log file name pattern,
 					# can include strftime() escapes
+#log_file_mode = 0600			# creation mode for log files,
+					# begin with 0 to use octal notation
 #log_truncate_on_rotation = off		# If on, an existing log file of the
 					# same name as the new log file will be
 					# truncated rather than appended to.

src/include/postmaster/syslogger.h

@@ -5,7 +5,7 @@
  *
  * Copyright (c) 2004-2010, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/include/postmaster/syslogger.h,v 1.16 2010/01/02 16:58:08 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/postmaster/syslogger.h,v 1.17 2010/07/16 22:25:51 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -68,6 +68,7 @@ extern int	Log_RotationSize;
 extern PGDLLIMPORT char *Log_directory;
 extern PGDLLIMPORT char *Log_filename;
 extern bool Log_truncate_on_rotation;
+extern int	Log_file_mode;
 
 extern bool am_syslogger;