log files and permissions
With logging_collector enabled, all the postgres log files are created with
mode 0600. This makes life complicated if users other than "postgres" need
to be able to examine the log files as well. Common example of this is when the
database runs under "postgres" user and DBA-s have named accounts. In order to
examine the log files the DBA then has to go through extra steps to sudo to
"postgres" or equivalent. Another example would be a monitoring script that
runs as an unprivileged user but needs to tail the log files.
It'd be convenient if the log files would have group read access. Then we could
make all the DBA or monitoring users members of the postgres group and they'd
have direct access to the logs. However, as the "group read" is not likely a
universally correct setting, the creation mode needs to be configurable.
Attached is a patch that adds a GUC "log_file_mode" which allows to specify
the creation mode for the log files. Presently it lacks documentation, which
I'll add if the idea is generally acceptable.
PS. I have no idea how all of this would work on Windows, maybe it's not
event relevant there?
regards,
Martin
Martin Pihlak wrote:
Attached is a patch that adds a GUC "log_file_mode" which allows to specify
the creation mode for the log files. Presently it lacks documentation, which
I'll add if the idea is generally acceptable.
Now it really is attached.
regards,
Martin
Attachments:
log-file-mode.patchtext/x-diff; name=log-file-mode.patchDownload
*** a/src/backend/commands/variable.c
--- b/src/backend/commands/variable.c
***************
*** 914,916 **** show_role(void)
--- 914,947 ----
return endptr + 1;
}
+
+
+ /*
+ * LOG_FILE_MODE
+ */
+
+ extern int Log_file_mode; /* in guc.c */
+
+ /*
+ * assign_log_file_mode: GUC assign_hook for log_file_mode
+ */
+ const char *
+ assign_log_file_mode(const char *value, bool doit, GucSource source)
+ {
+ int file_mode;
+
+ /* Parse the octal mode, complain if invalid */
+ if (sscanf(value, "%o", &file_mode) != 1 || file_mode > 0777)
+ {
+ ereport(GUC_complaint_elevel(source),
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for parameter \"log_file_mode\"")));
+ return NULL;
+ }
+
+ if (doit)
+ Log_file_mode = file_mode;
+
+ return value;
+ }
+
*** a/src/backend/postmaster/syslogger.c
--- b/src/backend/postmaster/syslogger.c
***************
*** 73,78 **** int Log_RotationSize = 10 * 1024;
--- 73,79 ----
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,140 **** static void syslogger_parseArgs(int argc, char *argv[]);
--- 136,142 ----
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 die_on_error);
#ifdef WIN32
static unsigned int __stdcall pipeThread(void *arg);
***************
*** 516,530 **** SysLogger_Start(void)
*/
filename = logfile_getname(time(NULL), NULL);
! syslogFile = fopen(filename, "a");
!
! 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);
--- 518,524 ----
*/
filename = logfile_getname(time(NULL), NULL);
! syslogFile = logfile_open(filename, "a", true);
pfree(filename);
***************
*** 1004,1018 **** open_csvlogfile(void)
filename = logfile_getname(time(NULL), ".csv");
! fh = fopen(filename, "a");
!
! 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 */
--- 998,1004 ----
filename = logfile_getname(time(NULL), ".csv");
! fh = logfile_open(filename, "a", true);
#ifdef WIN32
_setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */
***************
*** 1025,1030 **** open_csvlogfile(void)
--- 1011,1040 ----
}
/*
+ * Open the logfile, set permissions and buffering options.
+ */
+ static FILE *
+ logfile_open(const char *filename, const char *mode, bool die_on_error)
+ {
+ FILE *fh;
+
+ fh = fopen(filename, mode);
+
+ if (fh)
+ {
+ setvbuf(fh, NULL, LBF_MODE, 0);
+ fchmod(fileno(fh), Log_file_mode);
+ }
+ else
+ ereport(die_on_error ? FATAL : LOG,
+ (errcode_for_file_access(),
+ (errmsg("could not create log file \"%s\": %m",
+ filename))));
+
+ return fh;
+ }
+
+ /*
* perform logfile rotation
*/
static void
***************
*** 1070,1088 **** 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");
else
! fh = fopen(filename, "a");
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.
--- 1080,1093 ----
if (Log_truncate_on_rotation && time_based_rotation &&
last_file_name != NULL &&
strcmp(filename, last_file_name) != 0)
! fh = logfile_open(filename, "w", false);
else
! fh = logfile_open(filename, "a", false);
if (!fh)
{
int saveerrno = errno;
/*
* ENFILE/EMFILE are not too surprising on a busy system; just
* keep using the old file till we manage to get a new one.
***************
*** 1128,1146 **** 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");
else
! fh = fopen(csvfilename, "a");
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.
--- 1133,1146 ----
if (Log_truncate_on_rotation && time_based_rotation &&
last_csv_file_name != NULL &&
strcmp(csvfilename, last_csv_file_name) != 0)
! fh = logfile_open(csvfilename, "w", false);
else
! fh = logfile_open(csvfilename, "a", false);
if (!fh)
{
int saveerrno = errno;
/*
* ENFILE/EMFILE are not too surprising on a busy system; just
* keep using the old file till we manage to get a new one.
***************
*** 1162,1169 **** 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
--- 1162,1167 ----
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 412,417 **** static char *server_version_string;
--- 412,418 ----
static int server_version_num;
static char *timezone_string;
static char *log_timezone_string;
+ static char *log_file_mode_string;
static char *timezone_abbreviations_string;
static char *XactIsoLevel_string;
static char *data_directory;
***************
*** 2243,2248 **** static struct config_string ConfigureNamesString[] =
--- 2244,2258 ----
},
{
+ {"log_file_mode", PGC_SIGHUP, LOGGING_WHERE,
+ gettext_noop("Sets the file permissions for log files."),
+ NULL
+ },
+ &log_file_mode_string,
+ "0600", assign_log_file_mode
+ },
+
+ {
{"DateStyle", PGC_USERSET, CLIENT_CONN_LOCALE,
gettext_noop("Sets the display format for date and time values."),
gettext_noop("Also controls interpretation of ambiguous "
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 265,270 ****
--- 265,271 ----
# 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 (octal)
#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.
*** a/src/include/commands/variable.h
--- b/src/include/commands/variable.h
***************
*** 35,39 **** extern const char *show_role(void);
--- 35,42 ----
extern const char *assign_session_authorization(const char *value,
bool doit, GucSource source);
extern const char *show_session_authorization(void);
+ const char *assign_log_file_mode(const char *value,
+ bool doit, GucSource source);
+ const char *show_log_file_mode(void);
#endif /* VARIABLE_H */
*** a/src/include/postmaster/syslogger.h
--- b/src/include/postmaster/syslogger.h
***************
*** 68,73 **** extern int Log_RotationSize;
--- 68,74 ----
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;
Martin Pihlak <martin.pihlak@gmail.com> writes:
It'd be convenient if the log files would have group read access. Then we could
make all the DBA or monitoring users members of the postgres group and they'd
have direct access to the logs. However, as the "group read" is not likely a
universally correct setting, the creation mode needs to be configurable.
It doesn't appear to me that this helps unless you are willing to make
the containing director(ies) group-readable/executable as well, which is
something we've resisted doing.
regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> wrote:
Martin Pihlak <martin.pihlak@gmail.com> writes:
It'd be convenient if the log files would have group read access.
Then we could make all the DBA or monitoring users members of the
postgres group and they'd have direct access to the logs.
However, as the "group read" is not likely a universally correct
setting, the creation mode needs to be configurable.It doesn't appear to me that this helps unless you are willing to
make the containing director(ies) group-readable/executable as
well, which is something we've resisted doing.
I just tried creating a symbolic link to the pg_log directory and
flagging the existing logs within it to 640. As a member of the
group I was able to list and view the contents of log files through
the symbolic link, even though I didn't have any authority to the
PostgreSQL data directory.
That seems potentially useful to me.
-Kevin
Tom Lane wrote:
It doesn't appear to me that this helps unless you are willing to make
the containing director(ies) group-readable/executable as well, which is
something we've resisted doing.
The log can be moved outside of data directory by setting "log_directory"
to an absolute path. Then the permissions for the log directory can be arbitrary
as the postmaster is only strict about permissions on data directory.
regards,
Martin
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Martin Pihlak <martin.pihlak@gmail.com> writes:
It'd be convenient if the log files would have group read access. Then we could
make all the DBA or monitoring users members of the postgres group and they'd
have direct access to the logs. However, as the "group read" is not likely a
universally correct setting, the creation mode needs to be configurable.It doesn't appear to me that this helps unless you are willing to make
the containing director(ies) group-readable/executable as well, which is
something we've resisted doing.
Perhaps we should have a umask-like GUC instead of this?
In the end, I agree with and completely understand the OP's complaint.
I havn't run into this issue much since, on Debian systems, we use
logrotate to move log files around and use the copy/truncate method
there, so permissions end up being preserved once an admin has decided
to change them. Might be something to consider, but, really, we should
give the admin some flexibility here, even if the default is the same as
current behaviour.
I'll refrain from bringing up the fact that we're concerned about log
files having group permissions by default, but we ship with "trust" in
pg_hba.conf...
Thanks,
Stephen
Martin Pihlak <martin.pihlak@gmail.com> writes:
Tom Lane wrote:
It doesn't appear to me that this helps unless you are willing to make
the containing director(ies) group-readable/executable as well, which is
something we've resisted doing.
The log can be moved outside of data directory by setting "log_directory"
to an absolute path.
Oh, of course. We'd need to mention that in the documentation for the
log-file-permission GUC.
regards, tom lane
On 07/01/2010 12:56 PM, Kevin Grittner wrote:
I just tried creating a symbolic link to the pg_log directory and
flagging the existing logs within it to 640. As a member of the
group I was able to list and view the contents of log files through
the symbolic link, even though I didn't have any authority to the
PostgreSQL data directory.That seems potentially useful to me.
Symlinks are exactly equivalent to using the target of the link. Your
permissions are probably already arranged so that you (as a group
member) can access the files. Fedora's initscript seems to deliberately
revoke group permissions from PGDATA and pg_log so I'm guessing that at
some point some things were created with some group permissions.
That said, as Martin mentions one can easily place the log directory
outside of the data directory and set appropriate directory permissions.
-- m. tharp
On Thu, Jul 1, 2010 at 12:19 PM, Michael Tharp
<gxti@partiallystapled.com> wrote:
That said, as Martin mentions one can easily place the log directory outside
of the data directory and set appropriate directory permissions.
If I can offer my $0.02, I recently solved such a problem on SuSE
Linux with apache logs. I used the ACL support on ext3 to give a
specific group read-only access:
cd /var/log
# Add an ACL for the 'www' user
setfacl -m u:www:r-x apache2
setfacl -m u:www:r-- apache2/*
# Modify the default ACL so that new files get 'r' for user
setfacl -d -m u:www:r-- apache2
Just pointing out that this problem is solvable on systems that
support ACLs w/o patching postgres.
Martin Pihlak wrote:
Attached is a patch that adds a GUC "log_file_mode" which allows to specify
the creation mode for the log files. Presently it lacks documentation, which
I'll add if the idea is generally acceptable.
Updated patch attached.
regards,
Martin
Attachments:
log-file-mode.patchtext/x-diff; name=log-file-mode.patchDownload
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***************
*** 2789,2794 **** local0.* /var/log/postgresql
--- 2789,2813 ----
</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>
+ When <varname>logging_collector</varname> is enabled,
+ this parameter sets the permissions of the created log files.
+ The value is an octal number consisting of 3 digits signifying
+ the permissions for the user, group and others.
+ </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>
*** a/src/backend/commands/variable.c
--- b/src/backend/commands/variable.c
***************
*** 914,916 **** show_role(void)
--- 914,946 ----
return endptr + 1;
}
+
+
+ /*
+ * LOG_FILE_MODE
+ */
+
+ extern int Log_file_mode; /* in guc.c */
+
+ /*
+ * assign_log_file_mode: GUC assign_hook for log_file_mode
+ */
+ const char *
+ assign_log_file_mode(const char *value, bool doit, GucSource source)
+ {
+ char *endptr;
+ long file_mode = strtol(value, &endptr, 8);
+
+ if (!*value || *endptr || file_mode < 0 || file_mode > 0777)
+ {
+ ereport(GUC_complaint_elevel(source),
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for parameter \"log_file_mode\"")));
+ return NULL;
+ }
+
+ if (doit)
+ Log_file_mode = (int) file_mode;
+
+ return value;
+ }
*** a/src/backend/postmaster/syslogger.c
--- b/src/backend/postmaster/syslogger.c
***************
*** 73,78 **** int Log_RotationSize = 10 * 1024;
--- 73,79 ----
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,140 **** static void syslogger_parseArgs(int argc, char *argv[]);
--- 136,142 ----
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 am_rotating);
#ifdef WIN32
static unsigned int __stdcall pipeThread(void *arg);
***************
*** 516,530 **** SysLogger_Start(void)
*/
filename = logfile_getname(time(NULL), NULL);
! syslogFile = fopen(filename, "a");
!
! 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);
--- 518,524 ----
*/
filename = logfile_getname(time(NULL), NULL);
! syslogFile = logfile_open(filename, "a", false);
pfree(filename);
***************
*** 1004,1018 **** open_csvlogfile(void)
filename = logfile_getname(time(NULL), ".csv");
! fh = fopen(filename, "a");
!
! 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 */
--- 998,1004 ----
filename = logfile_getname(time(NULL), ".csv");
! fh = logfile_open(filename, "a", false);
#ifdef WIN32
_setmode(_fileno(fh), _O_TEXT); /* use CRLF line endings on Windows */
***************
*** 1025,1030 **** open_csvlogfile(void)
--- 1011,1040 ----
}
/*
+ * Open the logfile, set permissions and buffering options.
+ */
+ static FILE *
+ logfile_open(const char *filename, const char *mode, bool am_rotating)
+ {
+ FILE *fh;
+
+ fh = fopen(filename, mode);
+
+ if (fh)
+ {
+ setvbuf(fh, NULL, LBF_MODE, 0);
+ fchmod(fileno(fh), Log_file_mode);
+ }
+ else
+ ereport(am_rotating ? LOG : FATAL,
+ (errcode_for_file_access(),
+ (errmsg("could not create%slog file \"%s\": %m",
+ am_rotating ? " new " : " ", filename))));
+
+ return fh;
+ }
+
+ /*
* perform logfile rotation
*/
static void
***************
*** 1070,1088 **** 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");
else
! fh = fopen(filename, "a");
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.
--- 1080,1093 ----
if (Log_truncate_on_rotation && time_based_rotation &&
last_file_name != NULL &&
strcmp(filename, last_file_name) != 0)
! fh = logfile_open(filename, "w", true);
else
! fh = logfile_open(filename, "a", true);
if (!fh)
{
int saveerrno = errno;
/*
* ENFILE/EMFILE are not too surprising on a busy system; just
* keep using the old file till we manage to get a new one.
***************
*** 1128,1146 **** 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");
else
! fh = fopen(csvfilename, "a");
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.
--- 1133,1146 ----
if (Log_truncate_on_rotation && time_based_rotation &&
last_csv_file_name != NULL &&
strcmp(csvfilename, last_csv_file_name) != 0)
! fh = logfile_open(csvfilename, "w", true);
else
! fh = logfile_open(csvfilename, "a", true);
if (!fh)
{
int saveerrno = errno;
/*
* ENFILE/EMFILE are not too surprising on a busy system; just
* keep using the old file till we manage to get a new one.
***************
*** 1162,1169 **** 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
--- 1162,1167 ----
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***************
*** 412,417 **** static char *server_version_string;
--- 412,418 ----
static int server_version_num;
static char *timezone_string;
static char *log_timezone_string;
+ static char *log_file_mode_string;
static char *timezone_abbreviations_string;
static char *XactIsoLevel_string;
static char *data_directory;
***************
*** 2243,2248 **** static struct config_string ConfigureNamesString[] =
--- 2244,2258 ----
},
{
+ {"log_file_mode", PGC_SIGHUP, LOGGING_WHERE,
+ gettext_noop("Sets the file permissions for log files."),
+ NULL
+ },
+ &log_file_mode_string,
+ "0600", assign_log_file_mode
+ },
+
+ {
{"DateStyle", PGC_USERSET, CLIENT_CONN_LOCALE,
gettext_noop("Sets the display format for date and time values."),
gettext_noop("Also controls interpretation of ambiguous "
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***************
*** 265,270 ****
--- 265,271 ----
# 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, octal
#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.
*** a/src/include/commands/variable.h
--- b/src/include/commands/variable.h
***************
*** 35,39 **** extern const char *show_role(void);
--- 35,42 ----
extern const char *assign_session_authorization(const char *value,
bool doit, GucSource source);
extern const char *show_session_authorization(void);
+ const char *assign_log_file_mode(const char *value,
+ bool doit, GucSource source);
+ const char *show_log_file_mode(void);
#endif /* VARIABLE_H */
*** a/src/include/postmaster/syslogger.h
--- b/src/include/postmaster/syslogger.h
***************
*** 68,73 **** extern int Log_RotationSize;
--- 68,74 ----
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;