Thread: 'update' as action of 'insert' rule: permission denied

Gday All,

I'm surprised I can't find any descriptions of this in the archive
already, since I had imagined it would be a common action, but
apparently not!

Illustration:

As superuser:

create table1 ( test1 integer );
create table2 ( test2 integer );
create user pleb;
grant insert on table1 to pleb;
create rule test_rule as on insert to table1 do update table2 set test2
= 2 where test2 = 0;

As user pleb:

insert into table1 values (1);
ERROR:  table1: Permission denied.

However, just to check, if we remove the rule (using DROP RULE) the
INSERT query works fine.
I also wrote a query using 'insert' as the action instead of 'update'
and that worked fine too.
I also tried granting the user pleb update and select permission on
table test2, and that failed in the same way as above.

My impression from the doco is that the permission rules should be
applied _before_ any rule interpretation is applied, and then again as
the rule owner for the queries (extra queries only?  this is
unspecified in the doco) that the rule creates.  Since the rule owner
in this case is the superuser (correct?), the query should succeed
whether or not the rule is present.

Note that the query succeeds when run as the superuser...

Cheerio!,

Tim Burgess
Queens' College
University of Melbourne

Tim Burgess <tim@queens.unimelb.edu.au> writes:
> I'm surprised I can't find any descriptions of this in the archive
> already, since I had imagined it would be a common action, but
> apparently not!

I'm surprised too.  This bug seems to have escaped notice for quite awhile.
Attached is the patch against 7.3 (it would probably work in 7.2 or
earlier too, but I didn't try it).

            regards, tom lane


*** src/backend/rewrite/rewriteHandler.c.orig    Thu Jan 16 21:01:16 2003
--- src/backend/rewrite/rewriteHandler.c    Thu Feb 13 16:23:18 2003
***************
*** 62,70 ****
--- 62,72 ----
  {
      int            current_varno,
                  new_varno;
+     List       *main_rtable;
      int            rt_length;
      Query       *sub_action;
      Query      **sub_action_ptr;
+     List       *rt;

      /*
       * Make modifiable copies of rule action and qual (what we're passed
***************
*** 99,114 ****
       * Generate expanded rtable consisting of main parsetree's rtable plus
       * rule action's rtable; this becomes the complete rtable for the rule
       * action.    Some of the entries may be unused after we finish
!      * rewriting, but if we tried to clean those out we'd have a much
       * harder job to adjust RT indexes in the query's Vars.  It's OK to
       * have unused RT entries, since planner will ignore them.
       *
       * NOTE: because planner will destructively alter rtable, we must ensure
       * that rule action's rtable is separate and shares no substructure
       * with the main rtable.  Hence do a deep copy here.
       */
!     sub_action->rtable = nconc((List *) copyObject(parsetree->rtable),
!                                sub_action->rtable);

      /*
       * Each rule action's jointree should be the main parsetree's jointree
--- 101,131 ----
       * Generate expanded rtable consisting of main parsetree's rtable plus
       * rule action's rtable; this becomes the complete rtable for the rule
       * action.    Some of the entries may be unused after we finish
!      * rewriting, but if we tried to remove them we'd have a much
       * harder job to adjust RT indexes in the query's Vars.  It's OK to
       * have unused RT entries, since planner will ignore them.
       *
       * NOTE: because planner will destructively alter rtable, we must ensure
       * that rule action's rtable is separate and shares no substructure
       * with the main rtable.  Hence do a deep copy here.
+      *
+      * Also, we must disable write-access checking in all the RT entries
+      * copied from the main query.  This is safe since in fact the rule action
+      * won't write on them, and it's necessary because the rule action may
+      * have a different commandType than the main query, causing
+      * ExecCheckRTEPerms() to make an inappropriate check.  The read-access
+      * checks can be left enabled, although they're probably redundant.
       */
!     main_rtable = (List *) copyObject(parsetree->rtable);
!
!     foreach(rt, main_rtable)
!     {
!         RangeTblEntry *rte = (RangeTblEntry *) lfirst(rt);
!
!         rte->checkForWrite = false;
!     }
!
!     sub_action->rtable = nconc(main_rtable, sub_action->rtable);

      /*
       * Each rule action's jointree should be the main parsetree's jointree

----- Original Message -----=20
From: "Tom Lane" <tgl@sss.pgh.pa.us>
To: "Donald Fraser" <demolish@cwgsy.net>
Cc: "[ADMIN]" <pgsql-admin@postgresql.org>
Sent: Monday, February 24, 2003 2:42 PM
Subject: Re: [ADMIN] Bugs with rules on views/tables: permission denied=20


> "Donald Fraser" <demolish@cwgsy.net> writes:
> > In addition to this observation I note that the same bugs apply to view=
s.
>=20
> What bugs?  The original complaint was shown to be user error.  (If you
> try to duplicate the problem using the example quoted in your mail, it
> works fine.)
>=20
> regards, tom lane

Sorry I didn't see any follow up emails on that one...
May be I haven't understood the documentation correctly either. I interpret=
ed=20
rules on views as follows:

1) Permissions on views grant the said USER the ability to perform the gran=
ted=20
action on the view, for example SELECT, INSERT or UPDATE.
2) The rules of the view always run at the rule creator's permission access=
 level.

Here is a simple example that fails with views, both in an update and an in=
sert.

CREATE USER chkrule WITH  PASSWORD '' NOCREATEDB NOCREATEUSER;

CREATE OR REPLACE FUNCTION test_func(int4) RETURNS int4 AS '=20
DECLARE=20
    id ALIAS FOR $1;=20
    ndosomething int4;=20
BEGIN=20
    ndosomething :=3D id;=20
    RETURN ndosomething;=20
END; ' LANGUAGE 'plpgsql' VOLATILE SECURITY DEFINER;=20
REVOKE ALL ON FUNCTION test_func(int4) FROM PUBLIC;=20

CREATE TABLE public.tbl_test1 (id int4 NOT NULL, s_text text NOT NULL, PRIM=
ARY KEY (id)) WITHOUT OIDS;=20
REVOKE ALL ON TABLE public.tbl_test1 FROM PUBLIC;=20

CREATE TABLE public.tbl_test2 (id int4 NOT NULL, s_text text NOT NULL, PRIM=
ARY KEY (id)) WITHOUT OIDS;=20
REVOKE ALL ON TABLE public.tbl_test1 FROM PUBLIC;=20

INSERT INTO tbl_test2(id, s_text) VALUES('1', 'testtext');

CREATE VIEW vu_tbl_test AS SELECT id, s_text FROM tbl_test1;=20
REVOKE ALL ON TABLE vu_tbl_test FROM PUBLIC;=20
GRANT SELECT, INSERT ON TABLE vu_tbl_test TO chkrule;=20

CREATE RULE rul_vu_tbl_test_01 AS ON INSERT TO vu_tbl_test DO (UPDATE tbl_t=
est2 SET s_text =3D NEW.s_text WHERE id =3D NEW.id);
CREATE RULE rul_vu_tbl_test_02 AS ON INSERT TO vu_tbl_test DO INSTEAD (INSE=
RT INTO tbl_test1 (id, s_text) VALUES(test_func(NEW.id), NEW.s_text));

Scenario 1)
As USER chkrule do:
Bugs=3D> INSERT INTO vu_tbl_test (id, s_text) VALUES('1','sometext');
ERROR:  vu_tbl_test: permission denied

Scenario 2)
Now drop rule 1 as USER postgres
DROP RULE rul_vu_tbl_test_01 ON vu_tbl_test;

As USER chkrule do:
Bugs=3D> INSERT INTO vu_tbl_test (id, s_text) VALUES('1','sometext');
ERROR:  test_func: permission denied

If you give the USER chkrule UPDATE permissions on the view vu_tbl_test the=
n the error at Scenario 1 goes away.

Regards
Donald Fraser.

"Donald Fraser" <demolish@cwgsy.net> writes:
> As USER chkrule do:
> Bugs=3D> INSERT INTO vu_tbl_test (id, s_text) VALUES('1','sometext');
> ERROR:  vu_tbl_test: permission denied

What I get is
    ERROR:  test_func: permission denied
and the needed fix is to grant execute privilege on the function to the
calling user.

> If you give the USER chkrule UPDATE permissions on the view vu_tbl_test the=
> n the error at Scenario 1 goes away.

Not for me.

The reason it acts this way is that rule permissions apply to tables
mentioned in the rule, not to functions.  This is at least partly for
historical reasons: functions didn't have permissions to check, back
when the rule system was designed.  But I'm not sure that it's wrong.
In your example, the end user is controlling the arguments given to
test_func.  If the rule allowed him to call test_func without having
permission to do so, there'd be the potential for security breaches.

            regards, tom lane

----- Original Message -----=20
From: "Tom Lane" <tgl@sss.pgh.pa.us>
To: "Donald Fraser" <demolish@cwgsy.net>
Cc: "[BUGS]" <pgsql-bugs@postgresql.org>
Sent: Monday, February 24, 2003 8:29 PM
Subject: Re: [BUGS] Bugs with rules on views/tables: permission denied=20


> "Donald Fraser" <demolish@cwgsy.net> writes:
> > As USER chkrule do:
> > Bugs=3D3D> INSERT INTO vu_tbl_test (id, s_text) VALUES('1','sometext');
> > ERROR:  vu_tbl_test: permission denied
>=20
> What I get is=20
> ERROR:  test_func: permission denied

This is really weird... starting to pull my hair out!
I cut and paste everything straight out of my email=20
and the first error I got was definitely:
ERROR:  vu_tbl_test: permission denied

I noticed that I had made a typo on line 17 of SQL:
REVOKE ALL ON TABLE public.tbl_test1 FROM PUBLIC;=20
should have read:
REVOKE ALL ON TABLE public.tbl_test2 FROM PUBLIC;=20

So I changed this and tried again - this time I got
ERROR:  tbl_test2: permission denied

It doesn't matter what I do now, I tried putting line 17 back
how it was (shouldn't have mattered anyway), vacuuming,
restarting the server and the first error message I get now=20
is always:
ERROR:  tbl_test2: permission denied

I cannot get the original message:
ERROR:  vu_tbl_test: permission denied.

Not sure what is going on with that one?

I'm running PostgreSQL 7.3.1 on i686-pc-linux-gnu,=20
compiled by GCC 2.96
I take it you tested yours on at least 7.3.2 or greater?=20
Were there some changes between these versions that could effect
this scenario? I didn't see any in the readme...

> and the needed fix is to grant execute privilege on the function to the
> calling user.

In this case I have to draw an annalogy between functions and tables,
albeit that they are completely different.=20
Functions can perform more sophisticated logic on data input to=20
tables that the user wouldn't normally have access to. Just like you=20
wouldn't give them access to those tables in the first place.
Hence if you give them access to the function, you may as well give
them access to the tables which is not the goal of views.
It's probably a debatable point, of which I can see how Postgres
has arrived at the current point of logic given it's past history with
function permissions.

At the moment to get around this I only allow access to tables
through views (which is generally good database design) so
when it comes to functions that modify tables, it doesn't matter
too much because if a user calls a function directly they=20
still want have the permissions. They can only be given such
persmission via views.

It just seems a bit untidy that I can't stop a user calling the=20
functions directly in the first place.
I'm sure one day there will be the scenario where I will want
a function to do something other than just table modifications
and then I will be stuck with the "how do I stop them calling
it directly in an uncontrolled manner".

>=20
> > If you give the USER chkrule UPDATE permissions on the view vu_tbl_test=
 the=3D
> > n the error at Scenario 1 goes away.
>=20
> Not for me.
>=20
> The reason it acts this way is that rule permissions apply to tables
> mentioned in the rule, not to functions.  This is at least partly for
> historical reasons: functions didn't have permissions to check, back
> when the rule system was designed.  But I'm not sure that it's wrong.
> In your example, the end user is controlling the arguments given to
> test_func.  If the rule allowed him to call test_func without having
> permission to do so, there'd be the potential for security breaches.

In my opinion this is no more of a security breach than allowing the=20
modification of tables, via rules, that the user doesn't normally have=20
access to. The rule creator is granting them that permission by=20
putting it in the rule.

Regards
Donald Fraser.