Re: [HACKERS] postgres_fdw super user checks - Mailing list pgsql-hackers

From Stephen Frost
Subject Re: [HACKERS] postgres_fdw super user checks
Date
Msg-id 20171128232638.GY4628@tamriel.snowman.net
Whole thread Raw
In response to Re: [HACKERS] postgres_fdw super user checks  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: [HACKERS] postgres_fdw super user checks  (Ashutosh Bapat <ashutosh.bapat@enterprisedb.com>)
List pgsql-hackers
Tom, Robert, all,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> > OK, let me try to summarize where we are with this.
>
> > Currently, postgres_fdw requires a password unless you are logged in
> > as a superuser.  Jeff proposes to change that so that it requires a
> > password if you are EITHER logged in as a superuser OR using a
> > superuser's credentials - e.g. by selecting from a view created by a
> > superuser.  Stephen and I propose that it should be one thing or the
> > other, and therefore that we should CHANGE the behavior to depend on
> > whose credentials you are using, not make it an either-or thing.  So
> > when selecting from a view, whether or not you need a password would
> > depend entirely on who owns the view, not who you are.  So that gives
> > us three options all of which are easy to implement: (1) leave it
> > alone [favored by nobody], (2) allow based on view owner OR current
> > user [favored by Jeff], (3) allowed based on view owner only [favored
> > by Stephen and me].
>
> FWIW, option (3) feels like the right thing to me.  It does not seem
> right that a view would behave differently depending on who calls it.

Just to make it clear, I continue to agree with (3) and agree with Tom
that we shouldn't be behaving differently depending on who is calling
the view.

> Now, the core reason why there's any restriction at all is that we
> do not want non-superusers to be able to piggyback on credentials
> belonging to the postgres OS user.  For example, without a user-
> supplied password, libpq might successfully make a connection
> because it got a password out of ~postgres/.pgpass, or because the
> "peer" auth method authenticated the connection as coming from a
> postgres-owned process, etc.  So there's a question as to which of
> these options squares with that concern.  But again it seems like
> (3) is a better choice.  As-is, a non-superuser-owned view might
> successfully scrape a password out of ~postgres/.pgpass if it's
> being called by a superuser, which doesn't seem especially desirable.
> If we change, then a superuser-owned view would successfully connect
> when being run by a non-superuser, which does seem desirable (given
> that the non-superuser must have been given privileges on the view).

Right, agreed.

> > Taken in complete isolation, this would, maybe, constitute a marginal
> > consensus for option (2).  However, Simon weighed in proposing a much
> > broader rethink in how foreign tables work -- letting them run with
> > either the owner's privileges or the accessor's privileges, rather
> > than always using the accessor's privileges as they do today.
>
> While I'm not objecting to the concept of a rethink, I don't think
> that "either-or" is what we want for security considerations.  What
> would make sense to me, if we're going to consider the foreign table
> owner as relevant, is always using the owner's privileges.  A view
> owner's privileges, for example, should determine whether the view
> is allowed to select from the foreign table --- but then the foreign
> table owner's identity should determine what happens while connecting
> to the remote server.

I agree that "either-or" is bad for security considerations.

> Anyway, one thing I'm not for is whacking this around at this time
> and then whacking it around some more later.  If there's any serious
> interest in a more global rethink, I think we ought to mark this
> Returned With Feedback pending someone doing that rethink.

The "global rethink" being contemplated seems to be more about
authentication forwarding than it is about this specific change.  If
there's some 'global rethink' which is actually applicable to this
specific deviation from the usual "use the view's owner for privilege
checks", then it's unclear to me what that is.

> > ... Unless we can get a clearer
> > consensus here, I think we should just mark this patch as Rejected.
>
> "Rejected" seems to me to imply "not only do we not want this patch,
> but we're uninterested in any likely successor either".  That seems
> overly strong, hence my suggestion of RWF.  Although maybe switching
> to owner privileges would be so different as to constitute an unrelated
> patch.

I tend to agree with RWF also in this case, though hopefully we can have
something done in the next CF which implements what seems to be the
consensus here from those looking specifically at this issue.

Thanks!

Stephen

pgsql-hackers by date:

Previous
From: Mark Dilger
Date:
Subject: Re: pgindent run?
Next
From: Michael Paquier
Date:
Subject: Re: [HACKERS] [PATCH] Vacuum: Update FSM more frequently