[Pkg-postgresql-public] Bug#739276: postgresql: Put pgstat in /var/run

[Christian Tacke]

Hi,

(excuse my previous empty mail, hit the wrong button)


On Fri, Mar 07, 2014 at 13:40:53 +0100, Christoph Berg wrote:
> Re: Martin Pitt 2014-03-07 <20140307090805.GA23354 at piware.de>
[...]
> > > +
> > > +# Starting at version 8.4 one can move the pg_stat_tmp dir
> > > +stats_temp_directory = '/var/run/postgresql/%v-%c_stat_tmp'
> > 
> > Note that this is a little too simplistic. This directory is only
> > writable to clusters owned by postgres. While that is the default, you
> > can assign any other owner, and you can even call pg_createcluster as
> > a normal user and put everything into your $HOME; in those cases,
> > /tmp/ will be used for the Unix socket, and we can't use
> > /var/run/postgresql/.
> 
> pg_createcluster will still want to write to /etc/postgresql which is
> owned by postgres, so the stats_temp_directory doesn't make the
> situation worse.

/etc/postgresql is even owned by root on my system.
/etc/postgresql/VERSION is owned by postgres though.

Note that pg_ctlcluster creates the stats_temp_directory
and sets the owner of that directory to the cluster owner.
So for clusters that root/postgres created but hands over
to other people, this should actually work. (Not tested!)


> > One idea is to add a new "%s" macro to the evaluation of
> > createcluster.conf which expands to the used socket dir, and set
> > stats_temp_directory = '%s/%v-%c_stat_tmp'. That's a little more
> > complex, but always ought to work since we know that the socket dir is
> > writable.
> 
> For non-postgres clusters, the socket dir will be /tmp, which isn't a
> tmpfs at least on my machine (I forgot which default Debian really
> decided on in that flameware).

/tmp is possibly still better than the data directory. At
least for semantical purposes. For example, maybe someone
has /home on a snapshotting filesystem like nilfs2. You
wouldn't want to bloat the snapshot logs with stat_tmp
stuff.

SECURITY consideration: I have not reviewed pg_ctlcluster
for creating the stat_tmp directory in a safe way. The name
of the directory is very predictable, so this might be an
issue.


> > A simpler solution might be to use /var/run/lock/ instead, which is
> > world-writable (same permissions as /tmp/). It's just a bit of an
> > abuse as these are not really lock files.
> 
> /var/run/lock wouldn't work because it's too small (5120k here).

stats_temp_directory files are often quite small. My small
cluster has a 58 kB pgstat.stat. So this could work.

Instead of abusing /var/run/lock, I'd rather vote for
something else, really. Even /tmp feels better.


> > A third option would be to not put this into createcluster.conf, but
> > dynamically set it up in pg_createcluster; i. e. only postgres-owned
> > clusters would configure this (more specifically, add the option if
> > /var/run/postgresql is writable for the owner, so that admins could
> > put other users into the postgres group).
> 
> ... or mount a new tmpfs, but that would require root.
> 
> I think this, and the %s idea are too complex for the resulting
> effect. Imho what Christian proposed is just the right approach,
> people can still disable it in createcluster.conf.

Or disable it after creating the cluster (in the created
postgresql.conf). My proposed change only affects creation.
After that, people are free to change things.

I'm suggesting to use my patch for now/starters.

It shouldn't break anything for admin-created clusters that
wasn't broken before. (For those, the tmp-dir should be
created correctly by pg_ctlcluster).

It will likely break user-created clusters even more, yes.
Very simple mitigation: Add some more documentation to
createcluster.conf on the lines of
# Clusters created by normal users will need to change this
# option in postgresql.conf, because the directory will not
# be setup properly for them.

If this becomes a real issue, I'd vote for the %s (or %r
for (r)undir) option. This gives a more structured access
to the whole thing. And then we'd need to review the
security considerations above.

Cheers

Christian

-- 
www.mad-protection.de