Re: [HACKERS] pg_upgrade and umask
On Fri, Mar 09, 2012 at 11:33:36AM -0500, Bruce Momjian wrote: > On Fri, Mar 09, 2012 at 10:41:53AM -0500, Tom Lane wrote: > > Bruce Momjian writes: > > > The problem is that these files are being created often by shell > > > redirects, e.g. pg_dump -f out 2> log_file. There is no clean way to > > > control the file creation permissions in this case --- only umask gives > > > us a process-level setting. Actually, one crafty idea would be to do > > > the umask only when I exec something, and when I create the initial > > > files with the new banner you suggested. Let me look into that. > > > > You could create empty log files with the desired permissions, and then > > do the execs with >>log_file, and thereby not have to globally change > > umask. > > Yes, that is what I have done, with the attached patch. I basically > wrapped the fopen call with umask calls, and have the system() call > wrapped too. That takes care of all the files pg_upgrade creates. > > > > Frankly, the permissions are already being modified by the default > > > umask, e.g. 0022. Do we want a zero umask? > > > > I'm not so worried about default umask; nobody's complained yet about > > wrong permissions on pg_upgrade output files. But umask 077 would be > > likely to do things like get rid of group access to postgresql.conf, > > which some people intentionally set. > > Yes, that was my conclusion too, but I wanted to ask. FYI, this doesn't > affect the install itself, just what pg_upgrade changes, and it doesn't > touch postgresql.conf, but, as you, I am worried there might be > long-term problems with an aggressive umask that covered the entire > executable. I ended up creating fopen_priv to centralize the umask calls to a single function, and added an is_priv boolean to exec_prog for the same purpose. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and umask
On Fri, Mar 09, 2012 at 10:41:53AM -0500, Tom Lane wrote:
> Bruce Momjian writes:
> > The problem is that these files are being created often by shell
> > redirects, e.g. pg_dump -f out 2> log_file. There is no clean way to
> > control the file creation permissions in this case --- only umask gives
> > us a process-level setting. Actually, one crafty idea would be to do
> > the umask only when I exec something, and when I create the initial
> > files with the new banner you suggested. Let me look into that.
>
> You could create empty log files with the desired permissions, and then
> do the execs with >>log_file, and thereby not have to globally change
> umask.
Yes, that is what I have done, with the attached patch. I basically
wrapped the fopen call with umask calls, and have the system() call
wrapped too. That takes care of all the files pg_upgrade creates.
> > Frankly, the permissions are already being modified by the default
> > umask, e.g. 0022. Do we want a zero umask?
>
> I'm not so worried about default umask; nobody's complained yet about
> wrong permissions on pg_upgrade output files. But umask 077 would be
> likely to do things like get rid of group access to postgresql.conf,
> which some people intentionally set.
Yes, that was my conclusion too, but I wanted to ask. FYI, this doesn't
affect the install itself, just what pg_upgrade changes, and it doesn't
touch postgresql.conf, but, as you, I am worried there might be
long-term problems with an aggressive umask that covered the entire
executable.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/check.c b/contrib/pg_upgrade/check.c
new file mode 100644
index a5f63eb..7905534
*** a/contrib/pg_upgrade/check.c
--- b/contrib/pg_upgrade/check.c
*** issue_warnings(char *sequence_script_fil
*** 165,176
if (sequence_script_file_name)
{
prep_status("Adjusting sequences");
! exec_prog(true,
! SYSTEMQUOTE "\"%s/psql\" --set ON_ERROR_STOP=on "
"--no-psqlrc --port %d --username \"%s\" "
! "-f \"%s\" --dbname template1 >> \"%s\"" SYSTEMQUOTE,
new_cluster.bindir, new_cluster.port, os_info.user,
! sequence_script_file_name, log_opts.filename2);
unlink(sequence_script_file_name);
check_ok();
}
--- 165,177
if (sequence_script_file_name)
{
prep_status("Adjusting sequences");
! exec_prog(true, UTILITY_LOG_FILE,
! SYSTEMQUOTE "\"%s/psql\" --echo-queries "
! "--set ON_ERROR_STOP=on "
"--no-psqlrc --port %d --username \"%s\" "
! "-f \"%s\" --dbname template1 >> \"%s\" 2>&1" SYSTEMQUOTE,
new_cluster.bindir, new_cluster.port, os_info.user,
! sequence_script_file_name, UTILITY_LOG_FILE);
unlink(sequence_script_file_name);
check_ok();
}
diff --git a/contrib/pg_upgrade/controldata.c b/contrib/pg_upgrade/controldata.c
new file mode 100644
index 5239601..e01280d
*** a/contrib/pg_upgrade/controldata.c
--- b/contrib/pg_upgrade/controldata.c
*** get_control_data(ClusterInfo *cluster, b
*** 126,136
/* we have the result of cmd in "output". so parse it line by line now */
while (fgets(bufin, sizeof(bufin), output))
{
! if (log_opts.debug)
! fputs(bufin, log_opts.debug_fd);
#ifdef WIN32
-
/*
* Due to an installer bug, LANG=C doesn't work for PG 8.3.3, but does
* work 8.2.6 and 8.3.7, so check for non-ASCII output and suggest a
--- 126,134
/* we have the result of cmd in "output". so parse it line by line now */
while (fgets(bufin, sizeof(bufin), output))
{
! pg_log(PG_VERBOSE, "%s", bufin);
#ifdef WIN32
/*
* Due to an installer bug, LANG=C doesn't work for PG 8.3.3, but does
* work 8.2.6 and 8.3.7, so check for non-ASCII output and suggest a
diff --git a/contrib/pg_upgrade/dump.c b/contrib/pg_upgrade/dump.c
new file mode 100644
index 772ca37..b1d4034
*** a/contrib/pg_upgrade/dump.c
--- b/contrib/pg_upgrade/dump.c
***
*** 11,16
--- 11,17
#include "pg_upgrade.h"
+ #include
void
generate_old_dump(void)
*** generate_old_dump(void)
*** 22,31
* --binary-upgrade records the width of dropped columns in pg_class, and
* restores the frozenid's for databases and relations.
*/
! exec_prog(true,
SYSTEMQUOTE "\"%s/pg_dumpall\" --port %d --username \"%s\" "
! "--schema-only --binary-upgrade > \"%s/" ALL_DUMP_FILE "\""
! SYSTEMQUOTE, new_cluster.bindir, old_cluster.port, os_info.user, os_info.cwd);
check_ok();
}
--- 23,33
* --binary-upgrade records the width of dropped columns in pg_class, and
* restores the frozenid's for databases and relations.
*/
! exec_prog(true, UTILITY_LOG_FILE,
SYSTEMQUOTE "\"%s/pg_dumpall\" --port %d --username \"%s\" "
! "--schema-only --bina
Re: [HACKERS] pg_upgrade and umask
Bruce Momjian writes: > The problem is that these files are being created often by shell > redirects, e.g. pg_dump -f out 2> log_file. There is no clean way to > control the file creation permissions in this case --- only umask gives > us a process-level setting. Actually, one crafty idea would be to do > the umask only when I exec something, and when I create the initial > files with the new banner you suggested. Let me look into that. You could create empty log files with the desired permissions, and then do the execs with >>log_file, and thereby not have to globally change umask. > Frankly, the permissions are already being modified by the default > umask, e.g. 0022. Do we want a zero umask? I'm not so worried about default umask; nobody's complained yet about wrong permissions on pg_upgrade output files. But umask 077 would be likely to do things like get rid of group access to postgresql.conf, which some people intentionally set. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and umask
On Fri, Mar 09, 2012 at 10:18:31AM -0500, Tom Lane wrote: > Bruce Momjian writes: > > What do people think of pg_upgrade setting its umask to 0077 so the log > > and SQL files are only readable by the postgres user? > > +1 for restricting the log files, but I'm dubious that you should alter > the existing permissions on copied files in any way. > > IOW, umask seems like the wrong tool. I was afraid you would say that. :-( The problem is that these files are being created often by shell redirects, e.g. pg_dump -f out 2> log_file. There is no clean way to control the file creation permissions in this case --- only umask gives us a process-level setting. Actually, one crafty idea would be to do the umask only when I exec something, and when I create the initial files with the new banner you suggested. Let me look into that. Frankly, the permissions are already being modified by the default umask, e.g. 0022. Do we want a zero umask? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and umask
On fre, 2012-03-09 at 10:10 -0500, Bruce Momjian wrote: > What do people think of pg_upgrade setting its umask to 0077 so the > log and SQL files are only readable by the postgres user? That would be good to have. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_upgrade and umask
Bruce Momjian writes: > What do people think of pg_upgrade setting its umask to 0077 so the log > and SQL files are only readable by the postgres user? +1 for restricting the log files, but I'm dubious that you should alter the existing permissions on copied files in any way. IOW, umask seems like the wrong tool. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_upgrade and umask
What do people think of pg_upgrade setting its umask to 0077 so the log and SQL files are only readable by the postgres user? -rwx-- 1 postgres postgres 41 Mar 9 09:59 delete_old_cluster.sh* -rw--- 1 postgres postgres 6411 Mar 8 21:56 pg_upgrade_dump_all.sql -rw--- 1 postgres postgres 5651 Mar 8 21:56 pg_upgrade_dump_db.sql -rw--- 1 postgres postgres 738 Mar 8 21:56 pg_upgrade_dump_globals.sql -rw--- 1 postgres postgres 1669 Mar 8 21:56 pg_upgrade_internal.log -rw--- 1 postgres postgres 1667 Mar 8 21:56 pg_upgrade_restore.log -rw--- 1 postgres postgres 1397 Mar 8 21:56 pg_upgrade_server.log -rw--- 1 postgres postgres 385 Mar 8 21:56 pg_upgrade_utility.log The umask would also affect files it copies like clog and the data files, but those already have only postgres permissions. The downside is that users running pg_upgrade with 'su' or 'RUNAS' would need to use those to inspect the log files for errors. FYI, delete_old_cluster.sh probably has to be run as root, but root seems able to run an executable that it doesn't own. I am thinking it isn't worth the complexity of using umask and restricting those files, but wanted opinions. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
