src/backend/tcop/pquery.c | 28 +++++++++- src/include/utils/portal.h | 2 + src/test/regress/expected/portals.out | 92 +++++++++++++++++++++++++++++++++ src/test/regress/sql/portals.sql | 72 ++++++++++++++++++++++++++ 4 files changed, 193 insertions(+), 1 deletion(-) diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c index d0db7ad..6251a38 100644 --- a/src/backend/tcop/pquery.c +++ b/src/backend/tcop/pquery.c @@ -488,6 +488,11 @@ PortalStart(Portal portal, ParamListInfo params, portal->portalParams = params; /* + * Must track user-id and security context when the portal is started. + */ + GetUserIdAndSecContext(&portal->userId, &portal->secContext); + + /* * Determine the portal execution strategy */ portal->strategy = ChoosePortalStrategy(portal->stmts); @@ -715,6 +720,8 @@ PortalRun(Portal portal, long count, bool isTopLevel, ResourceOwner saveResourceOwner; MemoryContext savePortalContext; MemoryContext saveMemoryContext; + Oid saveUserId; + int saveSecContext; AssertArg(PortalIsValid(portal)); @@ -760,10 +767,15 @@ PortalRun(Portal portal, long count, bool isTopLevel, saveResourceOwner = CurrentResourceOwner; savePortalContext = PortalContext; saveMemoryContext = CurrentMemoryContext; + GetUserIdAndSecContext(&saveUserId, &saveSecContext); PG_TRY(); { ActivePortal = portal; CurrentResourceOwner = portal->resowner; + if (portal->userId != GetUserId()) + SetUserIdAndSecContext(portal->userId, portal->secContext); + else + saveUserId = InvalidOid; PortalContext = PortalGetHeapMemory(portal); MemoryContextSwitchTo(PortalContext); @@ -844,6 +856,8 @@ PortalRun(Portal portal, long count, bool isTopLevel, CurrentResourceOwner = TopTransactionResourceOwner; else CurrentResourceOwner = saveResourceOwner; + if (OidIsValid(saveUserId)) + SetUserIdAndSecContext(saveUserId, saveSecContext); PortalContext = savePortalContext; PG_RE_THROW(); @@ -859,6 +873,8 @@ PortalRun(Portal portal, long count, bool isTopLevel, CurrentResourceOwner = TopTransactionResourceOwner; else CurrentResourceOwner = saveResourceOwner; + if (OidIsValid(saveUserId)) + SetUserIdAndSecContext(saveUserId, saveSecContext); PortalContext = savePortalContext; if (log_executor_stats && portal->strategy != PORTAL_MULTI_QUERY) @@ -1391,6 +1407,8 @@ PortalRunFetch(Portal portal, ResourceOwner saveResourceOwner; MemoryContext savePortalContext; MemoryContext oldContext; + Oid saveUserId; + int saveSecContext; AssertArg(PortalIsValid(portal)); @@ -1409,12 +1427,16 @@ PortalRunFetch(Portal portal, saveActivePortal = ActivePortal; saveResourceOwner = CurrentResourceOwner; savePortalContext = PortalContext; + GetUserIdAndSecContext(&saveUserId, &saveSecContext); PG_TRY(); { ActivePortal = portal; CurrentResourceOwner = portal->resowner; PortalContext = PortalGetHeapMemory(portal); - + if (portal->userId != GetUserId()) + SetUserIdAndSecContext(portal->userId, portal->secContext); + else + saveUserId = InvalidOid; /* skip restore */ oldContext = MemoryContextSwitchTo(PortalContext); switch (portal->strategy) @@ -1454,6 +1476,8 @@ PortalRunFetch(Portal portal, /* Restore global vars and propagate error */ ActivePortal = saveActivePortal; CurrentResourceOwner = saveResourceOwner; + if (OidIsValid(saveUserId)) + SetUserIdAndSecContext(saveUserId, saveSecContext); PortalContext = savePortalContext; PG_RE_THROW(); @@ -1467,6 +1491,8 @@ PortalRunFetch(Portal portal, ActivePortal = saveActivePortal; CurrentResourceOwner = saveResourceOwner; + if (OidIsValid(saveUserId)) + SetUserIdAndSecContext(saveUserId, saveSecContext); PortalContext = savePortalContext; return result; diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h index 4833942..7a4af0b 100644 --- a/src/include/utils/portal.h +++ b/src/include/utils/portal.h @@ -117,6 +117,8 @@ typedef struct PortalData const char *prepStmtName; /* source prepared statement (NULL if none) */ MemoryContext heap; /* subsidiary memory for portal */ ResourceOwner resowner; /* resources owned by portal */ + Oid userId; /* user-id to perform with this portal */ + int secContext; /* security context with this portal */ void (*cleanup) (Portal portal); /* cleanup hook */ SubTransactionId createSubid; /* the ID of the creating subxact */ diff --git a/src/test/regress/expected/portals.out b/src/test/regress/expected/portals.out index 01152a9..defa558 100644 --- a/src/test/regress/expected/portals.out +++ b/src/test/regress/expected/portals.out @@ -1,3 +1,14 @@ +-- Clean up in case a prior regression run failed +-- Suppress NOTICE messages when users/groups don't exist +SET client_min_messages TO 'warning'; +DROP ROLE IF EXISTS regtestuser_portal1; +DROP ROLE IF EXISTS regtestuser_portal2; +DROP ROLE IF EXISTS regtestuser_portal3; +RESET client_min_messages; +-- Declaration +CREATE USER regtestuser_portal1; +CREATE USER regtestuser_portal2; +CREATE USER regtestuser_portal3; -- -- Cursor regression tests -- @@ -1257,3 +1268,84 @@ FETCH ALL FROM c1; COMMIT; DROP TABLE cursor; +-- Make sure the cursor should perform with user's privilege when +-- it was declared, even if it was switched later. +CREATE TABLE cursor_t2 (a int, b text); +GRANT ALL ON TABLE cursor_t2 TO regtestuser_portal2; +INSERT INTO cursor_t2 VALUES (1,'aaa'), (2,'bbb'), (3,'ccc'), (4,'ddd'); +CREATE TABLE cursor_t3 (x int, y text); +GRANT ALL ON TABLE cursor_t3 TO regtestuser_portal3; +INSERT INTO cursor_t3 VALUES (10, 'xxx'), (11,'yyy'), (12,'zzz'); +SET SESSION AUTHORIZATION regtestuser_portal2; +CREATE FUNCTION fcursor_scan2(refcursor) RETURNS refcursor + SECURITY DEFINER LANGUAGE plpgsql + AS 'BEGIN OPEN $1 FOR SELECT current_user,* FROM cursor_t2; + RETURN $1; END'; +SET SESSION AUTHORIZATION regtestuser_portal1; +SELECT * FROM cursor_t2; -- failed, due to permission checks +ERROR: permission denied for relation cursor_t2 +SELECT * FROM cursor_t3; -- failed, due to permission checks +ERROR: permission denied for relation cursor_t3 +BEGIN; +SELECT fcursor_scan2('abc'); + fcursor_scan2 +--------------- + abc +(1 row) + +FETCH abc; -- should perform with privilege of regtestuser_portal2 + current_user | a | b +---------------------+---+----- + regtestuser_portal2 | 1 | aaa +(1 row) + +SET SESSION AUTHORIZATION regtestuser_portal3; +DECLARE xyz CURSOR FOR SELECT current_user,* FROM cursor_t3; +FETCH abc; -- should perform with privilege of regtestuser_portal2 + current_user | a | b +---------------------+---+----- + regtestuser_portal2 | 2 | bbb +(1 row) + +FETCH xyz; -- should perform with privilege of regtestuser_portal3 + current_user | x | y +---------------------+----+----- + regtestuser_portal3 | 10 | xxx +(1 row) + +SET SESSION AUTHORIZATION regtestuser_portal1; +FETCH abc; -- should perform with privilege of regtestuser_portal2 + current_user | a | b +---------------------+---+----- + regtestuser_portal2 | 3 | ccc +(1 row) + +FETCH xyz; -- should perform with privilege of regtestuser_portal3 + current_user | x | y +---------------------+----+----- + regtestuser_portal3 | 11 | yyy +(1 row) + +RESET SESSION AUTHORIZATION; +FETCH abc; -- should perform with privilege of regtestuser_portal2 + current_user | a | b +---------------------+---+----- + regtestuser_portal2 | 4 | ddd +(1 row) + +FETCH xyz; -- should perform with privilege of regtestuser_portal3 + current_user | x | y +---------------------+----+----- + regtestuser_portal3 | 12 | zzz +(1 row) + +COMMIT; +DROP TABLE cursor_t2; +DROP TABLE cursor_t3; +DROP FUNCTION fcursor_scan2(refcursor); +-- +-- Clean-up +-- +DROP USER regtestuser_portal1; +DROP USER regtestuser_portal2; +DROP USER regtestuser_portal3; diff --git a/src/test/regress/sql/portals.sql b/src/test/regress/sql/portals.sql index 02286c4..5a70a80 100644 --- a/src/test/regress/sql/portals.sql +++ b/src/test/regress/sql/portals.sql @@ -1,3 +1,19 @@ +-- Clean up in case a prior regression run failed + +-- Suppress NOTICE messages when users/groups don't exist +SET client_min_messages TO 'warning'; + +DROP ROLE IF EXISTS regtestuser_portal1; +DROP ROLE IF EXISTS regtestuser_portal2; +DROP ROLE IF EXISTS regtestuser_portal3; + +RESET client_min_messages; + +-- Declaration +CREATE USER regtestuser_portal1; +CREATE USER regtestuser_portal2; +CREATE USER regtestuser_portal3; + -- -- Cursor regression tests -- @@ -470,3 +486,59 @@ UPDATE cursor SET a = 2; FETCH ALL FROM c1; COMMIT; DROP TABLE cursor; + +-- Make sure the cursor should perform with user's privilege when +-- it was declared, even if it was switched later. + +CREATE TABLE cursor_t2 (a int, b text); +GRANT ALL ON TABLE cursor_t2 TO regtestuser_portal2; +INSERT INTO cursor_t2 VALUES (1,'aaa'), (2,'bbb'), (3,'ccc'), (4,'ddd'); + +CREATE TABLE cursor_t3 (x int, y text); +GRANT ALL ON TABLE cursor_t3 TO regtestuser_portal3; +INSERT INTO cursor_t3 VALUES (10, 'xxx'), (11,'yyy'), (12,'zzz'); + +SET SESSION AUTHORIZATION regtestuser_portal2; +CREATE FUNCTION fcursor_scan2(refcursor) RETURNS refcursor + SECURITY DEFINER LANGUAGE plpgsql + AS 'BEGIN OPEN $1 FOR SELECT current_user,* FROM cursor_t2; + RETURN $1; END'; + +SET SESSION AUTHORIZATION regtestuser_portal1; + +SELECT * FROM cursor_t2; -- failed, due to permission checks +SELECT * FROM cursor_t3; -- failed, due to permission checks + +BEGIN; + +SELECT fcursor_scan2('abc'); +FETCH abc; -- should perform with privilege of regtestuser_portal2 + +SET SESSION AUTHORIZATION regtestuser_portal3; + +DECLARE xyz CURSOR FOR SELECT current_user,* FROM cursor_t3; + +FETCH abc; -- should perform with privilege of regtestuser_portal2 +FETCH xyz; -- should perform with privilege of regtestuser_portal3 + +SET SESSION AUTHORIZATION regtestuser_portal1; + +FETCH abc; -- should perform with privilege of regtestuser_portal2 +FETCH xyz; -- should perform with privilege of regtestuser_portal3 + +RESET SESSION AUTHORIZATION; + +FETCH abc; -- should perform with privilege of regtestuser_portal2 +FETCH xyz; -- should perform with privilege of regtestuser_portal3 + +COMMIT; +DROP TABLE cursor_t2; +DROP TABLE cursor_t3; +DROP FUNCTION fcursor_scan2(refcursor); + +-- +-- Clean-up +-- +DROP USER regtestuser_portal1; +DROP USER regtestuser_portal2; +DROP USER regtestuser_portal3;