Skip site navigation (1) Skip section navigation (2)

Re: Review for "Add permission check on SELECT INTO"

From: Kohei KaiGai <kaigai(at)kaigai(dot)gr(dot)jp>
To: Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Review for "Add permission check on SELECT INTO"
Date: 2011-11-19 19:01:39
Message-ID: CADyhKSUxtZ3a8vmk1q-6m2kvh1bFpOX6TnL-eoAw_odvfRo+Sw@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
Thanks for your reviewing.

The reason of this strange message was bug is the patch.

> CREATE TABLE public.copy1(a, b) AS SELECT * FROM public.test;
> ERROR:  whole-row update is not implemented

When it constructs a fake RangeTblEntry, it marked modifiedCols for
attribute 0 to (tupdesc->natts - 1), although it should be 1 to natts.
InvalidAttrNumber equals 0, so ExecCheckRTPerms got confused.

The attached patch is a revised version.
It fixed up this bug, and revised test cases to ensure permission
check error shall be raised due to the new table.

+SET SESSION AUTHORIZATION selinto_user;
+SELECT * INTO TABLE selinto_schema.tmp1
+	  FROM pg_class WHERE relname like '%a%';	-- Error
+ERROR:  permission denied for relation tmp1

Thanks,

2011/11/18 Albe Laurenz <laurenz(dot)albe(at)wien(dot)gv(dot)at>:
> The patch is in context diff format and applies cleanly.
>
> The functionality is needed because it keeps users from circumventing
> privilege restrictions, as they can currently do in this case.
>
> There is no documentation, which I think is OK since it changes
> behaviour to work as documented.
>
> The patch compiles without warning.
>
> It contains a test case, but that test case does not test
> the feature at all!
>
> The expected (and encountered) result is:
>
> CREATE SCHEMA selinto_schema;
> CREATE USER selinto_user;
> ALTER DEFAULT PRIVILEGES IN SCHEMA selinto_schema
>         REVOKE INSERT ON TABLES FROM selinto_user;
> SET SESSION AUTHORIZATION selinto_user;
> SELECT * INTO TABLE selinto_schema.tmp1
>         FROM onek WHERE onek.unique1 < 2;     -- Error
> ERROR:  permission denied for relation onek
> RESET SESSION AUTHORIZATION;
> DROP SCHEMA selinto_schema CASCADE;
> DROP USER selinto_user;
>
> This does not fail because "selinto_user" lacks INSERT permission
> on "selinto_schema.tmp1" (he doesn't!), but because he lacks
> SELECT permission on "onek" (as the error message indicates).
> Setting default privileges on a schema can never revoke
> default privileges.
>
>
> The patch works as advertised in that it causes SELECT ... INTO
> and CREATE TABLE ... AS to fail, however the error message is
> misleading. Here a (correct) test case:
>
> CREATE ROLE laurenz LOGIN;
> CREATE TABLE public.test(id, val) AS VALUES (1, 'eins'), (2, 'zwei');
> GRANT SELECT ON public.test TO laurenz;
> ALTER DEFAULT PRIVILEGES FOR ROLE laurenz REVOKE INSERT ON TABLES FROM
> laurenz;
> SET ROLE laurenz;
> CREATE TABLE public.copy1(a, b) AS SELECT * FROM public.test;
> ERROR:  whole-row update is not implemented
> CREATE TABLE public.copy1(a) AS SELECT id FROM public.test;
> ERROR:  whole-row update is not implemented
> SELECT * INTO public.copy2 FROM public.test;
> ERROR:  whole-row update is not implemented
> RESET ROLE;
> ALTER DEFAULT PRIVILEGES FOR ROLE laurenz GRANT ALL ON TABLES TO
> laurenz;
> DROP TABLE test;
> DROP ROLE laurenz;
>
> The correct error message would be:
> ERROR:  permission denied for relation copy1
>
> It seems like a wrong code path is taken in ExecCheckRTEPerms,
> maybe there's something wrong with rte->modifiedCols.
>
>
> I'll mark the patch as "Waiting on Author" until these problems are
> addressed.
>
> Yours,
> Laurenz Albe
>
-- 
KaiGai Kohei <kaigai(at)kaigai(dot)gr(dot)jp>

Attachment: pgsql-v9.2-add-select-into-checks.v2.patch
Description: application/octet-stream (4.5 KB)

In response to

Responses

pgsql-hackers by date

Next:From: Pavel StehuleDate: 2011-11-19 19:06:39
Subject: Re: Singleton range constructors versus functional coercion notation
Previous:From: Jeff DavisDate: 2011-11-19 18:57:27
Subject: Re: Singleton range constructors versus functional coercion notation

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group