Re: PATCH: Configurable file mode mask

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Chapman Flack <chap(at)anastigmatix(dot)net>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, David Steele <david(at)pgmasters(dot)net>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Adam Brightwell <adam(dot)brightwell(at)crunchydata(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: PATCH: Configurable file mode mask
Date: 2018-03-13 17:50:03
Message-ID: 20180313175003.GD2416@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Greetings Chapman,

* Chapman Flack (chap(at)anastigmatix(dot)net) wrote:
> On 03/13/2018 10:40 AM, Stephen Frost wrote:
> > ... Ultimately, the default which makes sense here isn't a
> > one-size-fits-all but is system dependent and the administrator should
> > be able to choose what permissions they want to have.
>
> Hear, hear. Returning for a moment again to
>
> https://www.postgresql.org/message-id/8b16cc30-0187-311e-04b5-1f32446d89dd%40anastigmatix.net
>
> we see that a stat() returning mode 0750 on a modern system may not
> even mean there is any group access at all. In that example, the
> datadir had these permissions:

Yes, that's true, but PG has never paid attention to POSIX ACLs or Linux
capabilities. Changing it to do so is quite a bit beyond the scope of
this particular patch and I don't see anything in what this patch is
doing which would preclude someone from putting in that effort in the
future.

> While PostgreSQL does its stat() and interprets the mode as if it is
> still on a Unix box from the '80s, two things have changed underneath
> it: POSIX ACLs and Linux capabilities. Capabilities take the place of
> the former specialness of root, who now needs to be granted r-x
> explicitly in the ACL to be able to read stuff there at all, and there
> is clearly no access to group and no access to other. It would be hard
> for anybody to call this an insecure configuration. But when you stat()
> an object with a POSIX ACL, you get the 'mask' value where the 'group'
> bits used to go, so postgres sees this as 0750, thinks the 5 represents
> group access, and refuses to start. Purely a mistake.

I'll point out that PG does run on quite a few other OS's beyond Linux.

> It's the kind of mistake that is inherent in this sort of check,
> which tries to draw conclusions from the semantics it assumes, while
> systems evolve and semantics march along. One hypothetical fix would
> be to add:
>
> #ifdef HAS_POSIX_ACLS
> ... check if there's really an ACL here, and the S_IRWXG bits are
> really just the mask, or even try to pass judgment on whether the
> admin's chosen ACL is adequately secure ...
> #endif
>
> but then sooner or later it will still end up making assumptions
> that aren't true under, say, SELinux, so there's another #ifdef,
> and where does it end?

I agree with this general concern.

> On 03/13/2018 11:00 AM, Tom Lane wrote:
> > In a larger sense, this fails to explain the operating principle,
> > namely what I stated above, that we allow the existing permissions
> > on PGDATA to decide what we allow as group permissions.
>
> I admit I've only skimmed the patches, trying to determine what
> that will mean in practice. In a case like the ACL example above,
> does this mean that postgres will stat PGDATA, conclude incorrectly
> that group access is granted, and then, based on that, actually go
> granting unwanted group access for real on newly-created files
> and directories?

PG will stat PGDATA and conclude that the system is saying that group
access has been granted on PGDATA and will do the same for subsequent
files created later on. This is new in PG, so there isn't any concern
about this causing problems in an existing environment- you couldn't
have had those ACLs on an existing PGDATA dir in the first place, as you
note above. The only issue that remains is that PG doesn't understand
or work with POSIX ACLs or Linux capabilities, but that's not anything
new.

> On 03/13/2018 10:45 AM, David Steele wrote:
> > As Stephen notes, this can be enforced by the user if they want to,
> > and without much effort (and with better tools).
>
> To me, that seems really the key. An --allow-group-access option is
> nice (but, as we see, misleading if its assumptions are outdated
> regarding how the filesystem works), but I would plug even harder for
> a --permission-transparency option, which would just assume that the
> admin is making security arrangements, through mechanisms that
> postgres may or may not even understand. The admin can create ACLs
> with default entries that propagate to newly created objects.
> SELinux contexts can work in similar ways. The admin controls the
> umask inherited by the postmaster. A --permission-transparency option
> would simply use open and mkdir in the traditional ways, allowing
> the chosen umask, ACL defaults, SELinux contexts, etc., to do their
> thing, and would avoid then stepping on the results with explicit
> chmods (and of course skip the I-refuse-to-start-because-I-
> misunderstand-your-setup check).
>
> It wouldn't be for every casual install, but it would be the best
> way to stay out of the way of an admin securing a system with modern
> facilities.
>
> A lot of the design effort put into debating what postgres itself
> should or shouldn't insist on could then, perhaps, go into writing
> postgres-specific configuration rule packages for some of those
> better configuration-checking tools, and there it might even be
> possible to write tests that incorporate knowledge of ACLs, SELinux,
> etc.

I'm a fan of this idea in general, but it's unclear how that
--permission-transparency option would work in practice. Are you
suggesting that it be a compile-time flag, which would certainly work
but also then have to be debated among packagers as to the right setting
and if there's any need to be concerned about users misunderstanding it,
or a flag for each program, which hardly seems ideal as some invokations
of programs might forget to specify that, leading to inconsistent
permissions, or something else..? We've pretty well established that
there's no particularly good way for PG to just "know" beyond looking at
the privileges on the data directory, but we'd have to build in complete
support for POSIX ACLs and Linux capabilities if we go down a route
where we want to enforce the same POSIX ACLs for every file that exists
in the data directory based on the data directory's ACLs. I'm not
entirely sure that would even be possible when it comes to capabilities,
or if it would make sense..

From what I've seen with SELinux, thankfully, these same issues don't
really crop up (probably because PG never tried to force anything
SELinux related, and SELinux is largely independent of the POSIX ACLs
and Linux capabilities, so it doesn't impact things in this way).

Thanks!

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jesper Pedersen 2018-03-13 17:54:45 Re: [HACKERS] path toward faster partition pruning
Previous Message Dr K.G Yadav 2018-03-13 17:48:40 Inquiry regarding the projects under GSOC