Re: PATCH: Configurable file mode mask

Hi Tom,

On 3/13/17 1:03 PM, Tom Lane wrote:
> David Steele <david(at)pgmasters(dot)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(at)pgmasters(dot)net