pg_upgrade and umask

Started by Bruce Momjianabout 14 years ago7 messageshackers
Jump to latest
#1Bruce Momjian
bruce@momjian.us

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 <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#1)
Re: pg_upgrade and umask

Bruce Momjian <bruce@momjian.us> 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

#3Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#1)
Re: 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.

#4Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#2)
Re: pg_upgrade and umask

On Fri, Mar 09, 2012 at 10:18:31AM -0500, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> 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 <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#4)
Re: pg_upgrade and umask

Bruce Momjian <bruce@momjian.us> 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

#6Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#5)
Re: pg_upgrade and umask

On Fri, Mar 09, 2012 at 10:41:53AM -0500, Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> 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 <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +

Attachments:

pg_upgrade.difftext/x-diff; charset=us-asciiDownload+349-338
#7Bruce Momjian
bruce@momjian.us
In reply to: Bruce Momjian (#6)
Re: 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 <bruce@momjian.us> 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 <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ It's impossible for everything to be true. +