Re: API change advice: Passing plan invalidation info from the rewriter into the planner?

From: Kevin Grittner <kgrittn(at)ymail(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Craig Ringer <craig(at)2ndquadrant(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Adam Brightwell <adam(dot)brightwell(at)crunchydatasolutions(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Yeb Havinga <yeb(dot)havinga(at)portavita(dot)nl>
Subject: Re: API change advice: Passing plan invalidation info from the rewriter into the planner?
Date: 2014-06-16 14:44:11
Message-ID: 1402929851.96221.YahooMailNeo@web122305.mail.ne1.yahoo.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Stephen Frost <sfrost(at)snowman(dot)net> wrote:
> Kevin Grittner (kgrittn(at)ymail(dot)com) wrote:

>> The proposed approach would leave the validity of any dump which
>> was not run as a superuser in doubt.  The last thing we need, in
>> terms of improving security, is another thing you can't do
>> without connecting as a superuser.
>
> Any dump not run by a superuser is already in doubt, imv.  That
> is a problem we already have which really needs to be addressed,
> but I view that as an independent issue.

I'm not seeing that.  If the user can't dump, you get an error and
pg_dump returns something other than SUCCESS.

test=# create user bob;
CREATE ROLE
test=# create user tom;
CREATE ROLE
test=# set role bob;
SET
test=> create table person(person_id int primary key, name text not null, ssn text);
CREATE TABLE
test=> insert into person values (1, 'Stephen Frost', '123-45-6789');
INSERT 0 1
test=> insert into person values (2, 'Kevin Grittner');
INSERT 0 1
test=> grant select (person_id, name) on person to tom;
GRANT
test=> \q
kgrittn(at)Kevin-Desktop:~/pg/master$ pg_dump -U bob test >bob-backup.sql
kgrittn(at)Kevin-Desktop:~/pg/master$ pg_dump -U tom test >tom-backup.sql
pg_dump: [archiver (db)] query failed: ERROR:  permission denied for relation person
pg_dump: [archiver (db)] query was: LOCK TABLE public.person IN ACCESS SHARE MODE
kgrittn(at)Kevin-Desktop:~/pg/master$ echo $?
1

> I agree with avoiding adding another superuser-only capability;
> see the other sub-thread about making this a per-user capability.

It should be possible to design something which does not have this
risk.  What I was saying was that what was being described at that
point wasn't it, and IMV was not acceptable.  I think that there
should never by any doubt that a pg_dump run which completes
without error copied all requested tables in their entirety, not a
subset of the rows in the tables.

A GUC which only caused an error on the attempt to actually read
specific rows which the user does not have permission to see would
leak too much information.  A GUC which caused a SELECT or COPY
from a table to throw an error if the user was not entitled to see
all rows in the table could work.  Another thing which could work,
if it can be coded, would be a GUC which would throw an error if
the there were not quals on the query to prohibit seeing rows which
the security conditions would prohibit, whether or not any matching
rows actually existed.  The latter would match the behavior of
column level security -- you get an error when trying to select a
prohibited column even if there are no rows in the table.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Abhijit Menon-Sen 2014-06-16 14:59:00 [REVIEW] Re: postgresql.auto.conf read from wrong directory
Previous Message Abhijit Menon-Sen 2014-06-16 14:30:53 [REVIEW] Re: Re: BUG #9578: Undocumented behaviour for temp tables created inside query language (SQL) functions