Re: [HACKERS] postgres_fdw super user checks

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Andreas Karlsson <andreas(at)proxel(dot)se>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] postgres_fdw super user checks
Date: 2017-11-28 23:26:38
Message-ID: 20171128232638.GY4628@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tom, Robert, all,

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Robert Haas <robertmhaas(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2017-11-28 23:26:59 Re: [HACKERS] [PATCH] Vacuum: Update FSM more frequently
Previous Message Mark Dilger 2017-11-28 23:16:18 Re: pgindent run?