Re: [HACKERS] flock patch breaks things here - Mailing list pgsql-hackers

From Bruce Momjian
Subject Re: [HACKERS] flock patch breaks things here
Date
Msg-id 199808311709.NAA23350@candle.pha.pa.us
Whole thread Raw
In response to Re: [HACKERS] flock patch breaks things here  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
> dg@informix.com (David Gould) writes:
> > If the traffic on bugtraq is any indication, writing in /tmp is a
> > security exposure for daemons. Typical attack is:
> >  ln -s /etc/passwd /tmp/dumbrootprog.tmpfile
>
> A partial answer to this is to unlink /tmp/pidfile before trying to
> create/write it.  There's still a window for the attack, but it's
> mighty tiny.  (You do have to check that the unlink either succeeds
> or fails with ENOENT --- in particular, an EPERM failure is trouble
> for obvious reasons.)

The real fix for this is to do an open(O_CREAT) to force creation at the
time of the open.  If it fails, try removing it and try again.

With our own directory, it is even easier.  If no pid locks,
unconditionally drop the directory, and recreate it with our
permissions, and if the create fails, try again.  We don't continue with
the postmaster until we drop and create the file or directory.  Can't
symlink hack around that because we force creation.


> 1. During installation, create a subdirectory of /tmp to hold Postgres'
> socket files and associated pid lockfiles.  This subdirectory should be
> owned by the Postgres superuser and have permissions 755
> (world-readable, writable only by Postgres superuser).  Maybe call it
> /tmp/.pgsql --- the name should start with a dot to keep it out of the

Problem is multiple postmasters with different users.

> way.  (Bruce points out that some systems clear /tmp during reboot, so
> it might be that a postmaster will have to be prepared to recreate this
> directory at startup --- anyone know if subdirectories of /tmp are
> zapped too?  My system doesn't do that...)

Get's wiped out.  Has to be done only by the postmaster.  Assuming
initdb is going to create something that is going to stay around in /tmp
is going to break.

>
> 2. When a postmaster fires up, it checks for/creates a pid lockfile in
> the subdirectory, and also creates the socket file in the subdirectory.

Forces creation.

>
> 3. For backwards compatibility with 1.0 clients, the socket file is also
> hard-linked to /tmp/.s.PGSQL.port# (use an unlink() and link()).

Good idea.

>
> This way, there's no security risk of overwriting someone else's file,
> since we never create or write on a file in a directory we don't own, we
> only try to link an already-existing socket file into /tmp.
>
> I'd suggest that the pid and socket files be given names in /tmp/.pgsql
> that don't start with dots --- no need to make them hard to see in that
> directory.

Yes, stuff in the directory doesn't need dots.

> We can change libpq to find the socket at /tmp/.pgsql/whatever, and
> eventually we could perhaps drop the backwards-compatibility feature
> of creating a link in /tmp.

OK

>
> > Also, before sprinkling files all over it is good to try to conform
> > to the FHS (File Hierarchy Standard) (see http://www.pathname.com/fhs/)
> > which is pretty easy to do and likely to make life easier later.
>
> I notice that on my system, the X11 socket files in /tmp/.X11-unix are
> actually symlinks to socket files in /usr/spool/sockets/X11.  I dunno if
> it's worth our trouble to get into putting our sockets under /usr/spool
> or /var/spool or whatever --- seems like another configuration choice to
> mess up.  It'd be nice if the socket directory lived somewhere where the
> parent dirs weren't world-writable, but this would mean one more thing
> that you have to have root permissions for in order to install pgsql.

Yes, too compilicated.

--
Bruce Momjian                          |  830 Blythe Avenue
maillist@candle.pha.pa.us              |  Drexel Hill, Pennsylvania 19026
  +  If your life is a hard drive,     |  (610) 353-9879(w)
  +  Christ can be your backup.        |  (610) 853-3000(h)

pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] flock patch breaks things here
Next
From: Andreas Zeugswetter
Date:
Subject: TPRINTF in trace.h