Обсуждение: [EXAMPLE] Overly zealous security of schemas...
Howdy. It looks as though the checks that allow for access to schemas
doesn't check the correct permissions of the running user in that if a
function is being run as the security definer, the schema checks still
check the session_user. Am I missing the work around someplace or is
this a bug? Here's the example code to demonstrate this problem:
/* BEGIN */
\c template1 pgsql
DROP DATABASE test;
CREATE DATABASE test WITH OWNER dba;
\c test pgsql
BEGIN;
-- In case pl/plpgsql isn't loaded
CREATE FUNCTION plpgsql_call_handler () RETURNS language_handler AS '$libdir/plpgsql', 'plpgsql_call_handler'
LANGUAGEc;
CREATE TRUSTED PROCEDURAL LANGUAGE plpgsql HANDLER plpgsql_call_handler;
COMMIT;
\c test dba
BEGIN;
CREATE SCHEMA s AUTHORIZATION dba;
SET search_path TO s;
CREATE TABLE t (i INT);
CREATE TABLE c (i INT, PRIMARY KEY(i));
ALTER TABLE s.t ADD FOREIGN KEY(i) REFERENCES s.c(i);
SET search_path TO public;
CREATE FUNCTION t_ins(INT) RETURNS INT EXTERNAL SECURITY DEFINER AS '
DECLARE v_usr TEXT;
BEGIN SELECT current_user INTO v_usr; RAISE NOTICE ''current_user: %'', v_usr; SELECT session_user INTO
v_usr; RAISE NOTICE ''session_user: %'', v_usr;
INSERT INTO s.t (i) VALUES ($1); RETURN $1;
END;
' LANGUAGE 'plpgsql';
SET search_path TO public;
REVOKE ALL ON SCHEMA public,s FROM PUBLIC;
GRANT EXECUTE ON FUNCTION t_ins(INT) TO PUBLIC;
-- Unnecessary grant, but example is explicit at least
GRANT USAGE ON SCHEMA s TO dba;
-- Usage has to be granted to the PUBLIC in order for this to work,
-- even though the function is being run as the dba user: this
-- seems broken in that session_user isn't being set correctly when a
-- function is run as the security definer, or that schema checks
-- aren't consulting the current_user and are consulting the
-- session_user. Anyway, for the sake of correctness, comment out the
-- GRANT command for now, but if you want to see this example work
-- from start to finish, uncomment the following line.
--
-- GRANT USAGE ON SCHEMA s TO PUBLIC;
INSERT INTO s.c VALUES (42);
COMMIT;
\c test dba
-- Works
SELECT t_ins(42);
\c test normal_user
-- Doesn't work unless you run: GRANT USAGE ON SCHEMA s TO PUBLIC;
SELECT t_ins(42);
/* END */
The place where this breaks down is in the foreign key constraints, it
can't select 1 from the s schema and returns a permission denied.
test=> SELECT t_ins(42);
NOTICE: current_user: dba
NOTICE: session_user: normal_user
ERROR: s: permission denied
-sc
--
Sean Chittenden
Sean Chittenden <sean@chittenden.org> writes:
> Howdy. It looks as though the checks that allow for access to schemas
> doesn't check the correct permissions of the running user in that if a
> function is being run as the security definer, the schema checks still
> check the session_user. Am I missing the work around someplace or is
> this a bug?
It looks to me like the bug is not related to the use of a SECURITY
DEFINER function at all, but just to the use of foreign keys. The
RI triggers know they should setuid to the table owner for execution
of their generated queries --- but they fail to do so for parsing the
queries. So parse-time security checks (such as USAGE on schemas)
will fail.
I think you can make the same problem happen without a SECURITY DEFINER
function --- what you need is user A inserting into table B, which has
an FK reference to table C, which is in a schema that B's owner has
USAGE rights on but A doesn't. Would you try it?
regards, tom lane
> > Howdy. It looks as though the checks that allow for access to > > schemas doesn't check the correct permissions of the running user > > in that if a function is being run as the security definer, the > > schema checks still check the session_user. Am I missing the work > > around someplace or is this a bug? > > It looks to me like the bug is not related to the use of a SECURITY > DEFINER function at all, but just to the use of foreign keys. The > RI triggers know they should setuid to the table owner for execution > of their generated queries --- but they fail to do so for parsing > the queries. So parse-time security checks (such as USAGE on > schemas) will fail. Ah, I had this backwards: I thought SECURITY DEFINER wasn't setting something that'd allow the foreign keys to run as the owner of the function. > I think you can make the same problem happen without a SECURITY > DEFINER function --- what you need is user A inserting into table B, > which has an FK reference to table C, which is in a schema that B's > owner has USAGE rights on but A doesn't. Would you try it? Yep, you're right. Here's the test script + logput: /* Begin */ \c template1 pgsql DROP DATABASE test; CREATE DATABASE test WITH OWNER dba; \c test dba BEGIN; CREATE SCHEMA s AUTHORIZATION dba; CREATE TABLE s.c (i INT, PRIMARY KEY(i)); CREATE TABLE public.t (i INT); ALTER TABLE public.t ADD FOREIGN KEY(i) REFERENCES s.c(i); REVOKE ALL ON SCHEMA s FROM PUBLIC; GRANT INSERT,SELECT ON TABLE t TO PUBLIC; INSERT INTO s.c VALUES (42); COMMIT; \c test normal_user INSERT INTO t VALUES (42); /* End */ And the bits from the log file: 2003-04-26 14:29:39 [1044] LOG: query: INSERT INTO t VALUES (42); 2003-04-26 14:29:39 [1044] LOG: query: SELECT 1 FROM ONLY "s"."c" x WHERE "i" = $1 FOR UPDATE OF x 2003-04-26 14:29:39 [1044] ERROR: s: permission denied -sc -- Sean Chittenden
> > > Howdy. It looks as though the checks that allow for access to > > > schemas doesn't check the correct permissions of the running user > > > in that if a function is being run as the security definer, the > > > schema checks still check the session_user. Am I missing the work > > > around someplace or is this a bug? > > > > It looks to me like the bug is not related to the use of a SECURITY > > DEFINER function at all, but just to the use of foreign keys. The > > RI triggers know they should setuid to the table owner for execution > > of their generated queries --- but they fail to do so for parsing > > the queries. So parse-time security checks (such as USAGE on > > schemas) will fail. > > Ah, I had this backwards: I thought SECURITY DEFINER wasn't setting > something that'd allow the foreign keys to run as the owner of the > function. > > > I think you can make the same problem happen without a SECURITY > > DEFINER function --- what you need is user A inserting into table B, > > which has an FK reference to table C, which is in a schema that B's > > owner has USAGE rights on but A doesn't. Would you try it? > > Yep, you're right. And actually, it looks like sequences have this same problem as well, only things are slightly worse there: you have to grant SELECT,UPDATE to a sequence to the public in order for those to work automagically. :-/ -sc -- Sean Chittenden
Sean Chittenden <sean@chittenden.org> writes:
> Ah, I had this backwards: I thought SECURITY DEFINER wasn't setting
> something that'd allow the foreign keys to run as the owner of the
> function.
Nah; by the time the RI triggers run, you're out of the function
entirely. So they have to fend for themselves.
Here's the 7.3 version of the patch (it's a bit ugly because I had to
back-port a couple of changes that are in CVS tip).
regards, tom lane
*** src/backend/utils/adt/ri_triggers.c.orig Thu Mar 27 14:25:52 2003
--- src/backend/utils/adt/ri_triggers.c Sat Apr 26 18:12:16 2003
***************
*** 58,74 **** #define RI_KEYS_SOME_NULL 1 #define RI_KEYS_NONE_NULL 2
! #define RI_PLAN_CHECK_LOOKUPPK_NOCOLS 1 #define RI_PLAN_CHECK_LOOKUPPK 2
! #define RI_PLAN_CASCADE_DEL_DODELETE 1
! #define RI_PLAN_CASCADE_UPD_DOUPDATE 1
! #define RI_PLAN_NOACTION_DEL_CHECKREF 1
! #define RI_PLAN_NOACTION_UPD_CHECKREF 1
! #define RI_PLAN_RESTRICT_DEL_CHECKREF 1
! #define RI_PLAN_RESTRICT_UPD_CHECKREF 1
! #define RI_PLAN_SETNULL_DEL_DOUPDATE 1
! #define RI_PLAN_SETNULL_UPD_DOUPDATE 1 #define MAX_QUOTED_NAME_LEN (NAMEDATALEN*2+3) #define
MAX_QUOTED_REL_NAME_LEN (MAX_QUOTED_NAME_LEN*2)
--- 58,75 ---- #define RI_KEYS_SOME_NULL 1 #define RI_KEYS_NONE_NULL 2
! /* queryno values must be distinct for the convenience of ri_PerformCheck */ #define RI_PLAN_CHECK_LOOKUPPK_NOCOLS
1#define RI_PLAN_CHECK_LOOKUPPK 2
! #define RI_PLAN_CASCADE_DEL_DODELETE 3
! #define RI_PLAN_CASCADE_UPD_DOUPDATE 4
! #define RI_PLAN_NOACTION_DEL_CHECKREF 5
! #define RI_PLAN_NOACTION_UPD_CHECKREF 6
! #define RI_PLAN_RESTRICT_DEL_CHECKREF 7
! #define RI_PLAN_RESTRICT_UPD_CHECKREF 8
! #define RI_PLAN_SETNULL_DEL_DOUPDATE 9
! #define RI_PLAN_SETNULL_UPD_DOUPDATE 10
! #define RI_PLAN_KEYEQUAL_UPD 11 #define MAX_QUOTED_NAME_LEN (NAMEDATALEN*2+3) #define
MAX_QUOTED_REL_NAME_LEN (MAX_QUOTED_NAME_LEN*2)
***************
*** 149,154 ****
--- 150,159 ---- static void ri_InitHashTables(void); static void *ri_FetchPreparedPlan(RI_QueryKey *key); static void
ri_HashPreparedPlan(RI_QueryKey*key, void *plan);
+ static void *ri_PlanCheck(char *querystr, int nargs, Oid *argtypes,
+ RI_QueryKey *qkey, Relation fk_rel, Relation pk_rel,
+ bool cache_plan);
+ /* ---------- * RI_FKey_check -
***************
*** 264,269 ****
--- 269,277 ---- fk_rel, pk_rel, tgnargs, tgargs);
+ if (SPI_connect() != SPI_OK_CONNECT)
+ elog(ERROR, "SPI_connect() failed in RI_FKey_check()");
+ if ((qplan = ri_FetchPreparedPlan(&qkey)) == NULL) { char
querystr[MAX_QUOTED_REL_NAME_LEN+ 100];
***************
*** 278,297 **** snprintf(querystr, sizeof(querystr), "SELECT 1 FROM ONLY %s x FOR UPDATE OF x",
pkrelname);
! /*
! * Prepare, save and remember the new plan.
! */
! qplan = SPI_prepare(querystr, 0, NULL);
! qplan = SPI_saveplan(qplan);
! ri_HashPreparedPlan(&qkey, qplan); } /* * Execute the plan */
- if (SPI_connect() != SPI_OK_CONNECT)
- elog(WARNING, "SPI_connect() failed in RI_FKey_check()");
- SetUserId(RelationGetForm(pk_rel)->relowner); if (SPI_execp(qplan, check_values, check_nulls, 1) !=
SPI_OK_SELECT)
--- 286,299 ---- snprintf(querystr, sizeof(querystr), "SELECT 1 FROM ONLY %s x FOR UPDATE OF x",
pkrelname);
! /* Prepare and save the plan */
! qplan = ri_PlanCheck(querystr, 0, NULL,
! &qkey, fk_rel, pk_rel, true); } /* * Execute the plan
*/ SetUserId(RelationGetForm(pk_rel)->relowner); if (SPI_execp(qplan, check_values, check_nulls, 1)
!=SPI_OK_SELECT)
***************
*** 420,426 **** * The query string built is * SELECT 1 FROM ONLY <pktable> WHERE pkatt1 = $1 [AND
...] * The type id's for the $ parameters are those of the
! * corresponding FK attributes. Thus, SPI_prepare could * eventually fail if the parser cannot
identifysome way * how to compare these two types by '='. * ----------
--- 422,428 ---- * The query string built is * SELECT 1 FROM ONLY <pktable> WHERE pkatt1 = $1 [AND
...] * The type id's for the $ parameters are those of the
! * corresponding FK attributes. Thus, ri_PlanCheck could * eventually fail if the parser cannot
identifysome way * how to compare these two types by '='. * ----------
***************
*** 440,451 **** } strcat(querystr, " FOR UPDATE OF x");
! /*
! * Prepare, save and remember the new plan.
! */
! qplan = SPI_prepare(querystr, qkey.nkeypairs, queryoids);
! qplan = SPI_saveplan(qplan);
! ri_HashPreparedPlan(&qkey, qplan); } /*
--- 442,450 ---- } strcat(querystr, " FOR UPDATE OF x");
! /* Prepare and save the plan */
! qplan = ri_PlanCheck(querystr, qkey.nkeypairs, queryoids,
! &qkey, fk_rel, pk_rel, true); } /*
***************
*** 625,631 **** * The query string built is * SELECT 1 FROM ONLY <pktable> WHERE pkatt1 = $1 [AND
...] * The type id's for the $ parameters are those of the
! * corresponding FK attributes. Thus, SPI_prepare could * eventually fail if the parser cannot
identifysome way * how to compare these two types by '='. * ----------
--- 624,630 ---- * The query string built is * SELECT 1 FROM ONLY <pktable> WHERE pkatt1 = $1 [AND
...] * The type id's for the $ parameters are those of the
! * corresponding FK attributes. Thus, ri_PlanCheck could * eventually fail if the parser cannot
identifysome way * how to compare these two types by '='. * ----------
***************
*** 645,656 **** } strcat(querystr, " FOR UPDATE OF x");
! /*
! * Prepare, save and remember the new plan.
! */
! qplan = SPI_prepare(querystr, qkey.nkeypairs, queryoids);
! qplan = SPI_saveplan(qplan);
! ri_HashPreparedPlan(&qkey, qplan); } /*
--- 644,652 ---- } strcat(querystr, " FOR UPDATE OF x");
! /* Prepare and save the plan */
! qplan = ri_PlanCheck(querystr, qkey.nkeypairs, queryoids,
! &qkey, pk_rel, pk_rel, true); } /*
***************
*** 834,840 **** * The query string built is * SELECT 1 FROM ONLY <fktable> WHERE
fkatt1= $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, SPI_prepare could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
--- 830,836 ---- * The query string built is * SELECT 1 FROM ONLY <fktable> WHERE
fkatt1= $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, ri_PlanCheck could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
***************
*** 854,865 **** } strcat(querystr, " FOR UPDATE OF x");
! /*
! * Prepare, save and remember the new plan.
! */
! qplan = SPI_prepare(querystr, qkey.nkeypairs, queryoids);
! qplan = SPI_saveplan(qplan);
! ri_HashPreparedPlan(&qkey, qplan); } /*
--- 850,858 ---- } strcat(querystr, " FOR UPDATE OF x");
! /* Prepare and save the plan */
! qplan = ri_PlanCheck(querystr, qkey.nkeypairs, queryoids,
! &qkey, fk_rel, pk_rel, true); } /*
***************
*** 1075,1081 **** * The query string built is * SELECT 1 FROM ONLY <fktable>
WHEREfkatt1 = $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, SPI_prepare could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
--- 1068,1074 ---- * The query string built is * SELECT 1 FROM ONLY <fktable>
WHEREfkatt1 = $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, ri_PlanCheck could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
***************
*** 1095,1106 **** } strcat(querystr, " FOR UPDATE OF x");
! /*
! * Prepare, save and remember the new plan.
! */
! qplan = SPI_prepare(querystr, qkey.nkeypairs, queryoids);
! qplan = SPI_saveplan(qplan);
! ri_HashPreparedPlan(&qkey, qplan); } /*
--- 1088,1096 ---- } strcat(querystr, " FOR UPDATE OF x");
! /* Prepare and save the plan */
! qplan = ri_PlanCheck(querystr, qkey.nkeypairs, queryoids,
! &qkey, fk_rel, pk_rel, true); } /*
***************
*** 1288,1294 **** * The query string built is * DELETE FROM ONLY <fktable> WHERE
fkatt1= $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, SPI_prepare could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
--- 1278,1284 ---- * The query string built is * DELETE FROM ONLY <fktable> WHERE
fkatt1= $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, ri_PlanCheck could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
***************
*** 1307,1318 **** qkey.keypair[i][RI_KEYPAIR_PK_IDX]); }
! /*
! * Prepare, save and remember the new plan.
! */
! qplan = SPI_prepare(querystr, qkey.nkeypairs, queryoids);
! qplan = SPI_saveplan(qplan);
! ri_HashPreparedPlan(&qkey, qplan); } /*
--- 1297,1305 ---- qkey.keypair[i][RI_KEYPAIR_PK_IDX]); }
! /* Prepare and save the plan */
! qplan = ri_PlanCheck(querystr, qkey.nkeypairs, queryoids,
! &qkey, fk_rel, pk_rel, true); } /*
***************
*** 1511,1517 **** * UPDATE ONLY <fktable> SET fkatt1 = $1 [, ...] *
WHEREfkatt1 = $n [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, SPI_prepare could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
--- 1498,1504 ---- * UPDATE ONLY <fktable> SET fkatt1 = $1 [, ...] *
WHEREfkatt1 = $n [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, ri_PlanCheck could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
***************
*** 1537,1548 **** } strcat(querystr, qualstr);
! /*
! * Prepare, save and remember the new plan.
! */
! qplan = SPI_prepare(querystr, qkey.nkeypairs * 2, queryoids);
! qplan = SPI_saveplan(qplan);
! ri_HashPreparedPlan(&qkey, qplan); } /*
--- 1524,1532 ---- } strcat(querystr, qualstr);
! /* Prepare and save the plan */
! qplan = ri_PlanCheck(querystr, qkey.nkeypairs * 2, queryoids,
! &qkey, fk_rel, pk_rel, true); } /*
***************
*** 1741,1747 **** * The query string built is * SELECT 1 FROM ONLY <fktable>
WHEREfkatt1 = $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, SPI_prepare could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
--- 1725,1731 ---- * The query string built is * SELECT 1 FROM ONLY <fktable>
WHEREfkatt1 = $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, ri_PlanCheck could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
***************
*** 1761,1772 **** } strcat(querystr, " FOR UPDATE OF x");
! /*
! * Prepare, save and remember the new plan.
! */
! qplan = SPI_prepare(querystr, qkey.nkeypairs, queryoids);
! qplan = SPI_saveplan(qplan);
! ri_HashPreparedPlan(&qkey, qplan); } /*
--- 1745,1753 ---- } strcat(querystr, " FOR UPDATE OF x");
! /* Prepare and save the plan */
! qplan = ri_PlanCheck(querystr, qkey.nkeypairs, queryoids,
! &qkey, fk_rel, pk_rel, true); } /*
***************
*** 1975,1981 **** * The query string built is * SELECT 1 FROM ONLY <fktable>
WHEREfkatt1 = $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, SPI_prepare could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
--- 1956,1962 ---- * The query string built is * SELECT 1 FROM ONLY <fktable>
WHEREfkatt1 = $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, ri_PlanCheck could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
***************
*** 1995,2006 **** } strcat(querystr, " FOR UPDATE OF x");
! /*
! * Prepare, save and remember the new plan.
! */
! qplan = SPI_prepare(querystr, qkey.nkeypairs, queryoids);
! qplan = SPI_saveplan(qplan);
! ri_HashPreparedPlan(&qkey, qplan); } /*
--- 1976,1984 ---- } strcat(querystr, " FOR UPDATE OF x");
! /* Prepare and save the plan */
! qplan = ri_PlanCheck(querystr, qkey.nkeypairs, queryoids,
! &qkey, fk_rel, pk_rel, true); } /*
***************
*** 2195,2201 **** * UPDATE ONLY <fktable> SET fkatt1 = NULL [, ...] *
WHEREfkatt1 = $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, SPI_prepare could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
--- 2173,2179 ---- * UPDATE ONLY <fktable> SET fkatt1 = NULL [, ...] *
WHEREfkatt1 = $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, ri_PlanCheck could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
***************
*** 2220,2231 **** } strcat(querystr, qualstr);
! /*
! * Prepare, save and remember the new plan.
! */
! qplan = SPI_prepare(querystr, qkey.nkeypairs, queryoids);
! qplan = SPI_saveplan(qplan);
! ri_HashPreparedPlan(&qkey, qplan); } /*
--- 2198,2206 ---- } strcat(querystr, qualstr);
! /* Prepare and save the plan */
! qplan = ri_PlanCheck(querystr, qkey.nkeypairs, queryoids,
! &qkey, fk_rel, pk_rel, true); } /*
***************
*** 2445,2451 **** * UPDATE ONLY <fktable> SET fkatt1 = NULL [, ...] *
WHEREfkatt1 = $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, SPI_prepare could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
--- 2420,2426 ---- * UPDATE ONLY <fktable> SET fkatt1 = NULL [, ...] *
WHEREfkatt1 = $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, ri_PlanCheck could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
***************
*** 2481,2499 **** strcat(querystr, qualstr); /*
! * Prepare the new plan.
! */
! qplan = SPI_prepare(querystr, qkey.nkeypairs, queryoids);
!
! /*
! * Save and remember the plan if we're building the * "standard" plan.
*/
! if (use_cached_query)
! {
! qplan = SPI_saveplan(qplan);
! ri_HashPreparedPlan(&qkey, qplan);
! } } /*
--- 2456,2467 ---- strcat(querystr, qualstr); /*
! * Prepare the plan. Save it only if we're building the * "standard" plan.
*/
! qplan = ri_PlanCheck(querystr, qkey.nkeypairs, queryoids,
! &qkey, fk_rel, pk_rel,
! use_cached_query); } /*
***************
*** 2682,2688 **** * UPDATE ONLY <fktable> SET fkatt1 = NULL [, ...] *
WHEREfkatt1 = $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, SPI_prepare could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
--- 2650,2656 ---- * UPDATE ONLY <fktable> SET fkatt1 = NULL [, ...] *
WHEREfkatt1 = $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, ri_PlanCheck could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
***************
*** 2707,2716 **** } strcat(querystr, qualstr);
! /*
! * Prepare the plan
! */
! qplan = SPI_prepare(querystr, qkey.nkeypairs, queryoids); /* * Scan
theplan's targetlist and replace the NULLs by
--- 2675,2683 ---- } strcat(querystr, qualstr);
! /* Prepare the plan, don't save it */
! qplan = ri_PlanCheck(querystr, qkey.nkeypairs, queryoids,
! &qkey, fk_rel, pk_rel, false); /* * Scan the
plan'stargetlist and replace the NULLs by
***************
*** 2942,2948 **** * UPDATE ONLY <fktable> SET fkatt1 = NULL [, ...] *
WHEREfkatt1 = $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, SPI_prepare could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
--- 2909,2915 ---- * UPDATE ONLY <fktable> SET fkatt1 = NULL [, ...] *
WHEREfkatt1 = $1 [AND ...] * The type id's for the $ parameters are those of the
! * corresponding PK attributes. Thus, ri_PlanCheck could * eventually fail if the
parsercannot identify some way * how to compare these two types by '='. * ----------
***************
*** 2977,2986 **** } strcat(querystr, qualstr);
! /*
! * Prepare the plan
! */
! qplan = SPI_prepare(querystr, qkey.nkeypairs, queryoids); /* * Scan
theplan's targetlist and replace the NULLs by
--- 2944,2952 ---- } strcat(querystr, qualstr);
! /* Prepare the plan, don't save it */
! qplan = ri_PlanCheck(querystr, qkey.nkeypairs, queryoids,
! &qkey, fk_rel, pk_rel, false); /* * Scan the
plan'stargetlist and replace the NULLs by
***************
*** 3124,3130 **** case RI_MATCH_TYPE_UNSPECIFIED: case RI_MATCH_TYPE_FULL:
ri_BuildQueryKeyFull(&qkey,trigdata->tg_trigger->tgoid,
! 0, fk_rel, pk_rel,
tgnargs,tgargs);
--- 3090,3096 ---- case RI_MATCH_TYPE_UNSPECIFIED: case RI_MATCH_TYPE_FULL:
ri_BuildQueryKeyFull(&qkey,trigdata->tg_trigger->tgoid,
! RI_PLAN_KEYEQUAL_UPD, fk_rel, pk_rel,
tgnargs, tgargs);
***************
*** 3160,3165 ****
--- 3126,3177 ---- * ---------- */
+
+ /*
+ * Prepare execution plan for a query to enforce an RI restriction
+ *
+ * If cache_plan is true, the plan is saved into our plan hashtable
+ * so that we don't need to plan it again.
+ */
+ static void *
+ ri_PlanCheck(char *querystr, int nargs, Oid *argtypes,
+ RI_QueryKey *qkey, Relation fk_rel, Relation pk_rel,
+ bool cache_plan)
+ {
+ void *qplan;
+ Relation query_rel;
+ Oid save_uid;
+
+ /*
+ * The query is always run against the FK table except
+ * when this is an update/insert trigger on the FK table itself -
+ * either RI_PLAN_CHECK_LOOKUPPK or RI_PLAN_CHECK_LOOKUPPK_NOCOLS
+ */
+ if (qkey->constr_queryno == RI_PLAN_CHECK_LOOKUPPK ||
+ qkey->constr_queryno == RI_PLAN_CHECK_LOOKUPPK_NOCOLS)
+ query_rel = pk_rel;
+ else
+ query_rel = fk_rel;
+
+ /* Switch to proper UID to perform check as */
+ save_uid = GetUserId();
+ SetUserId(RelationGetForm(query_rel)->relowner);
+
+ /* Create the plan */
+ qplan = SPI_prepare(querystr, nargs, argtypes);
+
+ /* Restore UID */
+ SetUserId(save_uid);
+
+ /* Save the plan if requested */
+ if (cache_plan)
+ {
+ qplan = SPI_saveplan(qplan);
+ ri_HashPreparedPlan(qkey, qplan);
+ }
+
+ return qplan;
+ } /* * quoteOneName --- safely quote a single SQL name
Sean Chittenden <sean@chittenden.org> writes:
> And actually, it looks like sequences have this same problem as well,
> only things are slightly worse there: you have to grant SELECT,UPDATE
> to a sequence to the public in order for those to work
> automagically. :-/
That's always been true though.
regards, tom lane
> > And actually, it looks like sequences have this same problem as > > well, only things are slightly worse there: you have to grant > > SELECT,UPDATE to a sequence to the public in order for those to > > work automagically. :-/ > > That's always been true though. True, but while we're on the topic, I figured I'd give things a shot in the, could this be fixed dept. Inserting into a view with a rule, the resulting query is run as the rule executor, not as the rule definer. If that were somehow possible, then it'd remove the need to have a rule rewrite the (insert|update|delete|select) into a function call running at the privs of its definer and writing the functions that run at an elevated user. -sc -- Sean Chittenden
> > > And actually, it looks like sequences have this same problem as
> > > well, only things are slightly worse there: you have to grant
> > > SELECT,UPDATE to a sequence to the public in order for those to
> > > work automagically. :-/
> >
> > That's always been true though.
>
> True, but while we're on the topic, I figured I'd give things a shot
> in the, could this be fixed dept. Inserting into a view with a
> rule, the resulting query is run as the rule executor, not as the
> rule definer. If that were somehow possible, then it'd remove the
> need to have a rule rewrite the (insert|update|delete|select) into a
> function call running at the privs of its definer and writing the
> functions that run at an elevated user.
Here's a little follow up on this post, here's an example of what I'm
trying to accomplish:
/* Begin example */
\c template1 pgsql
DROP DATABASE test;
CREATE DATABASE test WITH OWNER dba;
\c test dba
BEGIN;
CREATE SCHEMA s AUTHORIZATION dba;
CREATE TABLE s.f (i INT, PRIMARY KEY(i));
INSERT INTO s.f (i) VALUES (42);
CREATE TABLE s.t (i SERIAL, c INT, PRIMARY KEY(i));
ALTER TABLE s.t ADD FOREIGN KEY(c) REFERENCES s.f(i);
CREATE VIEW public.v AS SELECT c FROM s.t;
CREATE RULE t_ins AS ON INSERT TO public.v DO INSTEAD INSERT INTO s.t (c) VALUES (NEW.c);
REVOKE ALL ON SCHEMA s FROM PUBLIC;
GRANT SELECT ON public.v TO PUBLIC;
COMMIT;
\c test normal_user
INSERT INTO v VALUES (42);
/* End Example */
psql:test3.sql:30: ERROR: s: permission denied
:-/ If you grant access to s, you get further along in the process:
-- As dba
GRANT USAGE ON SCHEMA s TO PUBLIC;
-- As normal_user
INSERT INTO v VALUES (42);
ERROR: t_i_seq.nextval: you don't have permissions to set sequence t_i_seq
Still, :(. So, if you grant access to the schema, and allow
SELECT,UPDATE on the sequence, then you're good to go:
-- As dba
GRANT SELECT,UPDATE ON s.t_i_seq TO PUBLIC;
-- As normal_user
INSERT INTO v VALUES (42);
INSERT 2126593 1
Whew. Only problem is you have to know the name of all of the
sequences in use in the schema and open up access to the schema. If
there was a way of executing a query generated by a RULE as the
definer, all of this would magically disappear... unless I'm missing
something. It's possible to do the following to get around this
quirk, however it's really time consuming/error prone to do this with
tables with a large number of columns:
/* Begin */
\c test dba
CREATE FUNCTION public.t_ins(INT) RETURNS BOOL EXTERNAL SECURITY DEFINER AS 'BEGIN INSERT INTO s.t (c)
VALUES($1); RETURN TRUE;
END;' LANGUAGE 'plpgsql';
CREATE OR REPLACE RULE t_ins AS ON INSERT TO public.v DO INSTEAD SELECT public.v_ins(NEW.c);
GRANT EXECUTE ON FUNCTION public.t_ins(INT) TO PUBLIC;
/* End */
This is the only way that I've been able to get around giving access
to the public to schema s and sequence s.t_i_seq. Does this use case
make sense for why it'd be great to have:
CREATE OR REPLACE RULE t_ins EXTERNAL SECURITY DEFINER ON ...
Some food for thought I suppose given it changes the context of an
insert rather dramatically:
test=> INSERT INTO v VALUES (42);v_ins
-------t
(1 row)
Does this make sense? Do you think having a function + rule for every
view is the correct way to get around the perm barrier or would it be
more appropriate to have a rule run the resulting query at an elevated
priv? -sc
--
Sean Chittenden
> > Ah, I had this backwards: I thought SECURITY DEFINER wasn't > > setting something that'd allow the foreign keys to run as the > > owner of the function. > > Nah; by the time the RI triggers run, you're out of the function > entirely. So they have to fend for themselves. > > Here's the 7.3 version of the patch (it's a bit ugly because I had > to back-port a couple of changes that are in CVS tip). Ah, excellent! Thank you Tom. This patch works wonderfully for me! It doesn't apply cleanly to 7.2, but here's the diff in case anyone's interested (applies to postgresql7 and postgresql-devel, just drop it into files/ and recompile): http://people.FreeBSD.org/~seanc/patches/patch_postgresql-7.3.2::src::backend::utils::adt::ri_triggers.c -sc -- Sean Chittenden