Thread: Strange permission problem regarding pg_settings
Hi I installed a postgres-application (which was developed on debian woody) on red hat 9 today, using the postgres 7.3 rpms from redhad. One of my the triggers uses the pg_settings table (more precisely, it updates that table to change the search_path temporarily). With the postgres 7.3 (and 7.4 too) installed on my debian development system, this worked fine. On redhat 9, however, I get an "pg_settings: permission denied" error when my trigger is executed. The same thing happens when I try altering the pg_settings table from the commandline. (But of course works, when connected as superuser). I double-checked the permissions set on both the pg_settings view, and the set_config(text, text, bool)-function called from the update-rule for pg_settings, and both seem to be correct (and the same as on the debian machine). As I needed to get the thing running, I now solved the problem by making the user that my app connects as a superuser, but I'd like to get rid of this again... Are there any more permission I could check, or perhaps some config-option in postgres.conf that I could try? greetings, Florian Pflug PS: I also tried moving the postgres-data-dir away, and creating a fresh one with initdb - but with no success - still "pg_settings: permission denied"
Florian G. Pflug wrote: > I installed a postgres-application (which was developed on debian woody) > on red hat 9 today, using the postgres 7.3 rpms from redhad. > One of my the triggers uses the pg_settings table (more precisely, it > updates that table to change the search_path temporarily). With the > postgres 7.3 (and 7.4 too) installed on my debian development system, > this worked fine. On redhat 9, however, I get an "pg_settings: > permission denied" error when my trigger is executed. I've got Red Hat 9 here, but it is hard to guess what might be wrong without seeing some details. Can you post a self-contained example that recreates the problem? Joe
On Dec 10, 2003, at 8:19 AM, Joe Conway wrote: > Florian G. Pflug wrote: >> I installed a postgres-application (which was developed on debian >> woody) on red hat 9 today, using the postgres 7.3 rpms from redhad. >> One of my the triggers uses the pg_settings table (more precisely, it >> updates that table to change the search_path temporarily). With the >> postgres 7.3 (and 7.4 too) installed on my debian development system, >> this worked fine. On redhat 9, however, I get an "pg_settings: >> permission denied" error when my trigger is executed. > I've got Red Hat 9 here, but it is hard to guess what might be wrong > without seeing some details. Can you post a self-contained example > that recreates the problem? This is what I did: As user postgres (connected to template1) .) create user testuser password 'pw' nocreatedb nocreateuser .) create database testdb owner testuser encoding 'utf-8' As user testuser (connected to testdb) : .) update pg_settings set setting='public' where name='search_path' ; this gives "pg_settings: permission denied" .) select set_config('search_path', 'public', 'f') ; this works, and sets the search_path as expected to 'public' On debian(woody), with a woody-backport of postgresql-7.3 installed (the packages for sid recompiled for woody, and installed with dpkg), the "update pg_settings..." statement works. As soon as I remove the "nocreateuser" from the "create user testuser...." line (or alter the user afterwards), it works on redhat too (but the user is superuser then, of course...) If you need further information, or want me to test something, just say so ;-) greetings, Florian Pflug
"Florian G. Pflug" <fgp@phlo.org> writes: > As user testuser (connected to testdb) : > .) update pg_settings set setting='public' where name='search_path' ; > this gives "pg_settings: permission denied" Hm. Works fine here. What do you get from select relacl, relacl is null from pg_class where relname = 'pg_settings'; regards, tom lane
On Dec 10, 2003, at 5:35 PM, Tom Lane wrote: > "Florian G. Pflug" <fgp@phlo.org> writes: >> As user testuser (connected to testdb) : >> .) update pg_settings set setting='public' where name='search_path' ; >> this gives "pg_settings: permission denied" > Hm. Works fine here. What do you get from > select relacl, relacl is null from pg_class where relname = > 'pg_settings'; testdb=> select relacl, relacl is null from pg_class where relname = 'pg_settings' ; relacl | ?column? --------+---------- {=r} | f (1 row) mfg, Florian Pflug
Tom Lane wrote: > "Florian G. Pflug" <fgp@phlo.org> writes: >>As user testuser (connected to testdb) : >>.) update pg_settings set setting='public' where name='search_path' ; >>this gives "pg_settings: permission denied" > > Hm. Works fine here. What do you get from > > select relacl, relacl is null from pg_class where relname = 'pg_settings'; Works fine here too, on RH9: Welcome to psql 7.3.5, the PostgreSQL interactive terminal. Type: \copyright for distribution terms \h for help with SQL commands \? for help on internal slash commands \g or terminate with semicolon to execute query \q to quit regression=# \c template1 You are now connected to database template1. template1=# create user testuser password 'pw' nocreatedb nocreateuser; CREATE USER template1=# create database testdb owner testuser encoding 'utf-8'; CREATE DATABASE template1=# \c testdb testuser You are now connected to database testdb as user testuser. testdb=> update pg_settings set setting='public' where name='search_path' ; set_config ------------ public (1 row) testdb=> select relacl, relacl is null from pg_class where relname = 'pg_settings'; relacl | ?column? --------+---------- {=r} | f (1 row) Joe
Joe Conway <mail@joeconway.com> writes: > Works fine here too, on RH9: > testdb=> update pg_settings set setting='public' where name='search_path' ; > set_config > ------------ > public > (1 row) > testdb=> select relacl, relacl is null from pg_class where relname = > 'pg_settings'; > relacl | ?column? > --------+---------- > {=r} | f > (1 row) Hm. By rights it *should* fail, since the ACL is clearly not granting UPDATE permissions to anybody. The fact that it fails to fail seems to be because the rules on pg_settings rewrite the UPDATE into DO INSTEAD NOTHING (which does nothing, in particular makes no permission checks) and a SELECT, which only requires read-permission on pg_settings. This is probably bogus and we ought to see what we can do about fixing it. (And we'd better fix initdb to grant UPDATE on pg_settings to public, too.) Now, why does Florian see a permissions failure (which is really the *right* behavior) when we don't? He didn't say exactly which PG version he was running, but I see a likely-related bug fix between 7.3.2 and 7.3.3: 2003-02-13 16:40 tgl * src/backend/rewrite/rewriteHandler.c (REL7_3_STABLE): Repair rule permissions-checking bug reported by Tim Burgess 10-Feb-02: the table(s) modified by the original query would get checked for the type of write permission needed by a rule query. This fix may need to be rethought. I'm not sure though where is a clean place to plug in the UPDATE permissions check given that the rules for this case do not generate any UPDATE query. regards, tom lane
Tom Lane wrote: > Now, why does Florian see a permissions failure (which is really the > *right* behavior) when we don't? He didn't say exactly which PG version > he was running, but I see a likely-related bug fix between 7.3.2 and > 7.3.3: That seems to be it: # psql regression Welcome to psql 7.3.2, the PostgreSQL interactive terminal. Type: \copyright for distribution terms \h for help with SQL commands \? for help on internal slash commands \g or terminate with semicolon to execute query \q to quit regression=# \c template1 You are now connected to database template1. template1=# create user testuser password 'pw' nocreatedb nocreateuser; CREATE USER template1=# create database testdb owner testuser encoding 'utf-8'; CREATE DATABASE template1=# \c testdb testuser You are now connected to database testdb as user testuser. testdb=> update pg_settings set setting='public' where name='search_path' ; ERROR: pg_settings: permission denied > This fix may need to be rethought. I'm not sure though where is a clean > place to plug in the UPDATE permissions check given that the rules for > this case do not generate any UPDATE query. Do you want me to take a look at this, or are you planning to? Joe
Joe Conway <mail@joeconway.com> writes: > Tom Lane wrote: >> This fix may need to be rethought. I'm not sure though where is a clean >> place to plug in the UPDATE permissions check given that the rules for >> this case do not generate any UPDATE query. > Do you want me to take a look at this, or are you planning to? If you have any ideas, feel free to take a shot. I've not thought of anything I like. I suspect the fact that the pre-patch code made the "right" permissions check was really coincidental, and that the correct fix will not involve reversion of that patch but rather adding a facility somewhere to ensure that the original view gets properly permission-checked even if there's a DO INSTEAD NOTHING rule. However, before biting that bullet it'd probably be good to understand in detail what's happening in both the 7.3.2 and CVS-tip code. I have not looked at just why that patch changes this example's behavior. regards, tom lane
Tom Lane said: > Hm. By rights it *should* fail, since the ACL is clearly not granting UPDATE permissions to anybody. > > The fact that it fails to fail seems to be because the rules on > pg_settings rewrite the UPDATE into DO INSTEAD NOTHING (which does nothing, in particular makes no permission checks) and a SELECT, which only requires read-permission on pg_settings. This is probably bogus and we ought to see what we can do about fixing it. (And we'd better fix initdb to grant UPDATE on pg_settings to public, too.) > > Now, why does Florian see a permissions failure (which is really the *right* behavior) when we don't? He didn't say exactly which PG version he was running, but I see a likely-related bug fix between 7.3.2 and 7.3.3: Sorry for not specifing the exact postgres versions involved initially - I believed that the problem were different default on redhat and debian, or different compiling options... RedHat-9: postgres 7.3.2-3 debian: postgres 7.3.2r1-5 (sid backport) I tried setting the relacl for the pg_settings table to {=rw}, but I still get permission denied. To double-check, I then set it to {=}, and this lets not only the update fail, but also select now gives "permission denied" (The correct behaviour I believe). greetings, Florian Pflug
"Florian Pflug" <fgp@phlo.org> writes: > Sorry for not specifing the exact postgres versions involved initially - I > believed that the problem were different default on redhat and debian, or > different compiling options... > RedHat-9: postgres 7.3.2-3 > debian: postgres 7.3.2r1-5 (sid backport) Hm, could the debian version have that rewrite patch, even though it claims to be 7.3.2? I'd have expected the behavior to be the same if they're both straight 7.3.2. > I tried setting the relacl for the pg_settings table to {=rw}, but I still > get permission denied. This does not surprise me; the original code was just plain buggy. I suspect it is applying some completely inappropriate check (like checking some other permission bit than UPDATE), so that the apparently correct failure is really coincidental, and it still fails when it should succeed. Unfortunately I don't have a running copy of 7.3.2 to trace through ... Anyway, in the short run I'd suggest updating to 7.3.5, which will let you do the UPDATE even though it really shouldn't :-(. The cleanups I'm worried about making are for future development. regards, tom lane
On Thu, 2003-12-11 at 16:26, Tom Lane wrote: > "Florian Pflug" <fgp@phlo.org> writes: > > Sorry for not specifing the exact postgres versions involved initially - I > > believed that the problem were different default on redhat and debian, or > > different compiling options... > > > RedHat-9: postgres 7.3.2-3 > > debian: postgres 7.3.2r1-5 (sid backport) > > Hm, could the debian version have that rewrite patch, even though it > claims to be 7.3.2? I'd have expected the behavior to be the same > if they're both straight 7.3.2. Debian 7.3.2r1-5 included several patches that would later be released in 7.3.3. -- Oliver Elphick Oliver.Elphick@lfix.co.uk Isle of Wight, UK http://www.lfix.co.uk/oliver GPG: 1024D/3E1D0C1C: CA12 09E0 E8D5 8870 5839 932A 614D 4C34 3E1D 0C1C ======================================== "The spirit of the Lord GOD is upon me; because the LORD hath anointed me to preach good tidings unto the meek; he hath sent me to bind up the brokenhearted, to proclaim liberty to the captives, and the opening of the prison to them that are bound." Isaiah 61:1
Tom Lane wrote: > This does not surprise me; the original code was just plain buggy. > I suspect it is applying some completely inappropriate check (like > checking some other permission bit than UPDATE), so that the apparently > correct failure is really coincidental, and it still fails when it > should succeed. Unfortunately I don't have a running copy of 7.3.2 to > trace through ... I saw something similar the other day on my copy of 7.3.2. I'll try to work through this, but it may be the weekend before I have time to really dig in. Joe
On Thu, Dec 11, 2003 at 11:26:14AM -0500, Tom Lane wrote: > Anyway, in the short run I'd suggest updating to 7.3.5, which will let > you do the UPDATE even though it really shouldn't :-(. The cleanups > I'm worried about making are for future development. First, thanks to everyone involved in tracking down this problem - you have been of invaluable help to me - I surely would have spent many useless hours on debugging this without your help. Am I right to assume that updating to 7.4 will have the same effekt as updating to 7.3.5? I ask because I plan to update to 7.4 sometime anyway, and util then it doesn't really hurt to connect as superuser, since this isn't a production system for the moment. greetings, Florian Pflug
On Fri, Dec 12, 2003 at 04:24:32AM +0100, Florian G. Pflug wrote: > Am I right to assume that updating to 7.4 will have the same effekt as > updating to 7.3.5? I ask because I plan to update to 7.4 sometime anyway, > and util then it doesn't really hurt to connect as superuser, since this > isn't a production system for the moment. No, you're not. You can update to 7.3.5 by just dropping the new executables into place (after stopping the postmaster, of course). Going to 7.4 will require you to dump all your data and reload. (I'm assuming you are running 7.3.4 ...) -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "La experiencia nos dice que el hombre peló millones de veces las patatas, pero era forzoso admitir la posibilidad de que en un caso entre millones, las patatas pelarían al hombre" (Ijon Tichy)
Alvaro Herrera <alvherre@dcc.uchile.cl> writes: > On Fri, Dec 12, 2003 at 04:24:32AM +0100, Florian G. Pflug wrote: >> Am I right to assume that updating to 7.4 will have the same effekt as >> updating to 7.3.5? > No, you're not. You can update to 7.3.5 by just dropping the new > executables into place (after stopping the postmaster, of course). I think he was asking whether 7.4 will behave the same as 7.3.5 with respect to this particular permissions issue. Which it will. You're correct that getting to 7.3.5 should be less painful (which is why I suggested it) --- but if he wants to jump to 7.4, no reason why not. regards, tom lane
Tom Lane wrote: > I suspect the fact that the pre-patch code made the "right" permissions > check was really coincidental, and that the correct fix will not involve > reversion of that patch but rather adding a facility somewhere to ensure > that the original view gets properly permission-checked even if there's > a DO INSTEAD NOTHING rule. However, before biting that bullet it'd > probably be good to understand in detail what's happening in both the > 7.3.2 and CVS-tip code. I have not looked at just why that patch > changes this example's behavior. > I just started looking at this again. There is definitely an issue in cvs tip: create table t(f1 int, f2 text); insert into t values(1, 'abc'); create view v as select * from t; CREATE RULE v_upd AS ON UPDATE TO v DO INSTEAD UPDATE t SET f1 = NEW.f1, f2 = NEW.f2 WHERE f1 = OLD.f1; create user user1; -- this fails; as it should, I think \c - user1 update v set f2 = 'def' where f1 = 1; ERROR: permission denied for relation v -- so grant SELECT on the view \c - postgres grant select on v to public; -- this should fail, but doesn't \c - user1 update v set f2 = 'def' where f1 = 1; UPDATE 1 On 7.3.2 that last section of the above script gives: \c - user1 update v set f2 = 'def' where f1 = 1; ERROR: v: permission denied The comment associated with the change says this: * 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. */ So SELECT permissions get checked for user1, but write-access does not. The underlying table should be checked for permissions based on the rule owner per rewriteDefine.c around line 439 (line 387 in 7.3.2): /* * We want the rule's table references to be checked as though by the * rule owner, not the user referencing the rule. Therefore, scan * through the rule's rtables and set the checkAsUser field on all * rtable entries. */ Since the rule owner in this case is also the creator of the table, the UPDATE suceeds. ISTM that we want the relations in the un-rewritten query checked based on the basis of the user referencing the rule and for the modes used in the un-rewritten query -- suggesting the change need be reverted. Then we want the rule's table references checked based on rule owner and actual operations performed. It looks like this part should be what's happening. I went back to the original complaint -- here is the example on a 7.3.2 installation: regression=# create table table1 (test1 integer); grant insert on table1 to pleb; create rule test_rule as on insert to table1 do update table2 set test2 = 2 where test2 = 0; \c - pleb; insert into table1 values (1); CREATE TABLE regression=# create table table2 (test2 integer); CREATE TABLE regression=# create user pleb; ERROR: CREATE USER: user name "pleb" already exists regression=# grant insert on table1 to pleb; GRANT regression=# create rule test_rule as on insert to table1 do update table2 set test2 = 2 where test2 = 0; CREATE RULE regression=# \c - pleb; You are now connected as new user pleb. regression=> insert into table1 values (1); ERROR: table1: permission denied A few NOTICES placed in ExecCheckRTEPerms() reveals this: regression=> insert into table1 values (1); NOTICE: relOid = 1245674 NOTICE: userid = 101 NOTICE: operation = CMD_INSERT NOTICE: relOid = 1245674 NOTICE: userid = 101 NOTICE: operation = CMD_UPDATE ERROR: table1: permission denied regression=> select oid, relname from pg_class where relname like 'table%'; oid | relname ---------+--------- 1245674 | table1 1245676 | table2 (2 rows) It seems that second pass through ExecCheckRTEPerms() is not doing the right thing. It ought to be checking table2 (not table1) for UPDATE as userid == 1 (not 101), shouldn't it? Any thoughts on where to look next? Thanks, Joe
[ please respect moving of thread to pg-hackers ] Joe Conway <mail@joeconway.com> writes: > ISTM that we want the relations in the un-rewritten query checked based > on the basis of the user referencing the rule and for the modes used in > the un-rewritten query -- suggesting the change need be reverted. Reverting the change will bring back the bug for which it was created. It does seem though that we have an inadequate model of how to perform permission checks. In particular, the "write" flag bit in RTEs is context dependent: it can mean insert, update, or delete permission depending on the surrounding command. The problem the earlier bug report identified is really that when an RTE is copied from one query to another, the meaning of its "write" flag bit changes --- incorrectly --- if the new query is of a different type. I thought when making that patch that we could make an end-run around this problem by zeroing out the flag bit, but what we're now realizing is that that leaves us with no check at all in some scenarios (because the original query will be dropped completely when INSTEAD is specified). I begin to think that the only real solution is to change the RTE representation to identify the exact permission bits to be checked for each entry (say, replace the read and write booleans with a permission bitmask). Then a view reference specifying INSERT permission check could be copied into an UPDATE query without changing its permission semantics. This would be a fairly extensive change though. Does anyone see an easier way? Also, does anyone see a case where it would be correct for the checked permission to change when an RTE is copied to a query of a different type? regards, tom lane
I wrote > "Florian Pflug" <fgp@phlo.org> writes: >> I tried setting the relacl for the pg_settings table to {=rw}, but I still >> get permission denied. > This does not surprise me; the original code was just plain buggy. > I suspect it is applying some completely inappropriate check (like > checking some other permission bit than UPDATE), so that the apparently > correct failure is really coincidental, and it still fails when it > should succeed. I've finished tracing through this, and it turns out to be a combination of factors. ExecCheckRTEPerms sees pg_settings referenced with checkForWrite in a SELECT query, which it thinks means a SELECT FOR UPDATE. It happens that this requires the same UPDATE permission as a real UPDATE, so the aclcheck() call that gets issued is, more or less by chance, the correct thing: a check for UPDATE privilege on pg_settings. But that fails, and the reason it fails is the "usecatupd" defense against allowing anyone to update a system catalog. This latter defense obviously predates the existence of updatable views in the system schema. What I've done about it is to disable the check in the case of a view relation, which seems to be the minimum workable loosening of the check ... but maybe it deserves a more complete rethink. I have also reverted the broken change of 13-Feb-03 in rewriteHandler.c. Upshot is that: * loss of view permission checking is fixed in 7.3.*, 7.4.*, and HEAD; * 7.4.* and HEAD will correctly allow UPDATE on pg_settings, although this depends on a chance coincidence; * the bug Tim Burgess complained of here is back: http://archives.postgresql.org/pgsql-bugs/2003-02/msg00038.php Per previous discussion, the best way to fix Tim's problem and make the pg_settings behavior less coincidental seems to require a change in RTE representation, so it will only be fixable in 7.5. I will work on that next ... regards, tom lane