Tom Lane wrote:
> I think we should change 9.3 to be restrictive about ownership/permissions
> on the stats_temp_directory (ie, require owner = postgres user,
> permissions = 0700, same as for the $PGDATA directory).
Not an easy thing to do, this. It should be done as a GUC check hook,
ISTM, but this doesn't work because the first time those are run we
haven't yet changed to the data directory, and so any relative path
(which the default value is) will cause the check to fail (I *assume*
setting an absolute path would work, but I haven't tried). We could
skip the check on the first run, and verify the directory separately in
PostmasterMain() after changing CWD, but I don't see any way to detect
that we're in the initial run of GUC processing. Any thoughts? Maybe
the idea of using a GUC check hook is flawed, but I don't think so
because we also need to verify a directory when the setting changes on
SIGHUP.
The implementation I chose for the actual check was to separate the
permission checks from checkDataDir() into src/port/pgcheckdir.c that
returns different error codes for each case; see first attachment.
This part seems pretty reasonable, except that the code should be in
src/common rather than src/port, but I believe the entire pgcheckdir.c
file should be moved.
> In addition to that, it might be a good idea to do what the comment in the
> code suggests, namely do more than zero checking on each file name to try
> to make sure it looks like a stats temp file name that we'd generate
> before we delete it. The ownership/permissions test wouldn't be enough
> to prevent you from pointing at, say, ~postgres and thereby losing some
> files you'd rather not.
This seems pretty simple to do; see second attachment. (It would delete
files named, "db_1234.tmpfoobar", that is, valid names with suffixes,
but I can't see that being a problem). (I haven't really tested this
part at all.)
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
