Re: [HACKERS] PATCH: Configurable file mode mask - Mailing list pgsql-hackers

From David Steele
Subject Re: [HACKERS] PATCH: Configurable file mode mask
Date
Msg-id 23a04d33-2486-900f-0cb1-d52fb93fbe5b@pgmasters.net
Whole thread Raw
In response to Re: [HACKERS] PATCH: Configurable file mode mask  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [HACKERS] PATCH: Configurable file mode mask  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
Hi Tom,

On 3/13/17 1:03 PM, Tom Lane wrote:
> David Steele <david@pgmasters.net> writes:
>> At miscinit.c:893:
> 
>> /* We can treat the EPERM-error case as okay because that error implies
>> that the existing process has a different userid than we do, which means
>> it cannot be a competing postmaster.  A postmaster cannot successfully
>> attach to a data directory owned by a userid other than its own.  (This
>> is now checked directly in checkDataDir(), but has been true for a long
>> time because of the restriction that the data directory isn't group- or
>> world-accessible.)  Also, since we create the lockfiles mode 600, we'd
>> have failed above if the lockfile belonged to another userid --- which
>> means that whatever process kill() is reporting about isn't the one that
>> made the lockfile.  (NOTE: this last consideration is the only one that
>> keeps us from blowing away a Unix socket file belonging to an instance
>> of Postgres being run by someone else, at least on machines where /tmp
>> hasn't got a stickybit.) */
> 
> TBH, the fact that we're relying on 0600 mode for considerations such
> as these makes me tremendously afraid of this whole patch.  I think that
> the claimed advantages are not anywhere near worth the risk that somebody
> is going to destroy their database because we weakened some aspect of the
> protection against starting multiple postmasters in a database directory.

I don't see a risk if the user uses umask 0027 which is the example
given in the docs.  I'm happy to change this example to a strong
recommendation.

> At the very least, I'd want to see much closer analysis of the safety
> issues than I've seen so far in this thread.  

I think it's clear that there would be safety risks with unwise umask
choices.  I also think the example/recommended umask is safe.

Running external processes as the postgres user carries serious risks as
well.  Not only with regards to data access but the danger of corruption
due to bugs.  If a process does not require write access to do its job
then why take that risk?

To (hopefully) address your concerns, I'll perform an analysis of
starting multiple postmasters with a variety of umask choices and report
the outcomes here.

> And since the proposed
> patch falsifies the above-quoted comment (and probably others), why
> hasn't it updated it?

That was an oversight on my part.  I'll update it in the next patch.

Thanks,
-- 
-David
david@pgmasters.net



pgsql-hackers by date:

Previous
From: David Steele
Date:
Subject: Re: [HACKERS] PATCH: Configurable file mode mask
Next
From: Thomas Munro
Date:
Subject: [HACKERS] Bogus tab completion tweak for UPDATE ... SET ... =