Re: postgres_fdw super user checks

From: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
To: Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
Cc: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: postgres_fdw super user checks
Date: 2017-07-27 19:45:12
Message-ID: CAMkU=1zWsJLX9ExyU+y0PYdrXJZG93yzZ03pWT1=u=VcVohi3Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Thu, Dec 1, 2016 at 7:11 PM, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>
wrote:

> On Tue, Oct 18, 2016 at 10:38 AM, Michael Paquier <
> michael(dot)paquier(at)gmail(dot)com> wrote:
>
>> On Mon, Oct 17, 2016 at 10:51 PM, Robert Haas <robertmhaas(at)gmail(dot)com>
>> wrote:
>> > On Mon, Oct 17, 2016 at 2:18 AM, Michael Paquier
>> > <michael(dot)paquier(at)gmail(dot)com> wrote:
>> >> On Mon, Oct 17, 2016 at 3:33 AM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com>
>> wrote:
>> >>> postgres_fdw has some checks to enforce that non-superusers must
>> connect to
>> >>> the foreign server with a password-based method. The reason for this
>> is to
>> >>> prevent the authentication to the foreign server from happening on
>> the basis
>> >>> of the OS user who is running the non-foreign server.
>> >>>
>> >>> But I think these super user checks should be run against the userid
>> of the
>> >>> USER MAPPING being used for the connection, not the userid of
>> currently
>> >>> logged on user.
>> >>
>> >> So, if the user mapping user is a superuser locally, this would allow
>> >> any lambda user of the local server to attempt a connection to the
>> >> remote server. It looks dangerous rather dangerous to me to authorize
>> >> that, even if the current behavior is a bit inconsistent I agree.
>> >
>> > I don't know what "any lambda user" means. Did you mean to write "any
>> > random user"?
>>
>> Yes, in this context that would be "any non-superuser" or "any user
>> without superuser rights". Actually that's a French-ism. I just
>> translated it naturally to English to define a user that has no access
>> to advanced features :)
>>
>
>
> Thanks for the patch, but it breaking the existing functionality as per
> the other
> mails. Marked as "returned with feedback" in 2016-11 commitfest.
>

Here is an updated patch. This version allows you use the password-less
connection if you either are the super-user directly (which is the existing
committed behavior), or if you are using the super-user's mapping because
you are querying a super-user-owned view which you have been granted access
to.

It first I thought the currently committed behavior might be a security bug
as a directly logged in superuser can use another user's user-defined
mapping but without the password restriction when querying a view made by
someone else. But consulting with the security list nearly a year ago, the
conclusion was that it is never a good idea for a superuser to blindly
query from other users' views, and that the current situation is no worse
for postgres_fdw than it is for other features, and so nothing needs to be
done about it. So that is why I've decided to allow the passwordless
solution in either situation--a superuser using someone else mapping, or
someone else using a super user's mapping.

I didn't update any comments because the existing ones seem to apply
equally well to the new code as the old code.

The regression test passes with this version because I still allow the old
behavior. I didn't add a new test to also test the new behavior, because I
don't know how to do that with the existing make check framework, and a TAP
test seems like overkill.

Cheers,

Jeff

Attachment Content-Type Size
postgres_fdw_superuser_v2.patch application/octet-stream 3.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-07-27 19:52:13 Re: Change in "policy" on dump ordering?
Previous Message Mark Rofail 2017-07-27 19:15:28 Re: GSoC 2017: Foreign Key Arrays