Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit5d179a2

Browse files
committed
Improve RLS handling in copy.c
To avoid a race condition where the relation being COPY'd could bechanged into a view or otherwise modified, keep the original lockon the relation. Further, fully qualify the relation when buildingthe query up.Also remove the poorly thought-out Assert() and check the entirerelationOids list as, post-RLS, there can certainly be multiplerelations involved and the planner does not guarantee their ordering.Per discussion with Noah and Andres.Back-patch to 9.5 where RLS was introduced.
1 parentcb0bb53 commit5d179a2

File tree

3 files changed

+126
-20
lines changed

3 files changed

+126
-20
lines changed

‎src/backend/commands/copy.c

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -896,8 +896,12 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed)
896896
target->val= (Node*)cr;
897897
target->location=1;
898898

899-
/* Build FROM clause */
900-
from=stmt->relation;
899+
/*
900+
* Build RangeVar for from clause, fully qualified based on the
901+
* relation which we have opened and locked.
902+
*/
903+
from=makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
904+
RelationGetRelationName(rel),-1);
901905

902906
/* Build query */
903907
select=makeNode(SelectStmt);
@@ -906,8 +910,13 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed)
906910

907911
query= (Node*)select;
908912

909-
/* Close the handle to the relation as it is no longer needed. */
910-
heap_close(rel, (is_from ?RowExclusiveLock :AccessShareLock));
913+
/*
914+
* Close the relation for now, but keep the lock on it to prevent
915+
* changes between now and when we start the query-based COPY.
916+
*
917+
* We'll reopen it later as part of the query-based COPY.
918+
*/
919+
heap_close(rel,NoLock);
911920
rel=NULL;
912921
}
913922
}
@@ -1407,25 +1416,25 @@ BeginCopy(bool is_from,
14071416
plan=planner(query,0,NULL);
14081417

14091418
/*
1410-
* If we were passed in a relid, make sure we got the same one back
1411-
* after planning out the query. It's possible that it changed
1412-
* between when we checked the policies on the table and decided to
1413-
* use a query and now.
1419+
* With row level security and a user using "COPY relation TO", we
1420+
* have to convert the "COPY relation TO" to a query-based COPY (eg:
1421+
* "COPY (SELECT * FROM relation) TO"), to allow the rewriter to add
1422+
* in any RLS clauses.
1423+
*
1424+
* When this happens, we are passed in the relid of the originally
1425+
* found relation (which we have locked). As the planner will look
1426+
* up the relation again, we double-check here to make sure it found
1427+
* the same one that we have locked.
14141428
*/
14151429
if (queryRelId!=InvalidOid)
14161430
{
1417-
Oidrelid=linitial_oid(plan->relationOids);
1418-
14191431
/*
1420-
* There should only be one relationOid in this case, since we
1421-
* will only get here when we have changed the command for the
1422-
* user from a "COPY relation TO" to "COPY (SELECT * FROM
1423-
* relation) TO", to allow row level security policies to be
1424-
* applied.
1432+
* Note that with RLS involved there may be multiple relations,
1433+
* and while the one we need is almost certainly first, we don't
1434+
* make any guarantees of that in the planner, so check the whole
1435+
* list and make sure we find the original relation.
14251436
*/
1426-
Assert(list_length(plan->relationOids)==1);
1427-
1428-
if (relid!=queryRelId)
1437+
if (!list_member_oid(plan->relationOids,queryRelId))
14291438
ereport(ERROR,
14301439
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
14311440
errmsg("relation referenced by COPY statement has changed")));

‎src/test/regress/expected/rowsecurity.out

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2672,7 +2672,7 @@ COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok
26722672
6,1679091c5a880faf6fb5e6087eb1b2dc
26732673
8,c9f0f895fb98ab9159f51fd0297e236d
26742674
10,d3d9446802a44259755d38e6d163e820
2675-
-- Check COPY TO as user without permissions.SET row_security TO OFF;
2675+
-- Check COPY TO as user without permissions.SET row_security TO OFF;
26762676
SET SESSION AUTHORIZATION rls_regress_user2;
26772677
SET row_security TO OFF;
26782678
COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls
@@ -2683,6 +2683,53 @@ ERROR: permission denied for relation copy_t
26832683
SET row_security TO FORCE;
26842684
COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - permission denied
26852685
ERROR: permission denied for relation copy_t
2686+
-- Check COPY relation TO; keep it just one row to avoid reordering issues
2687+
RESET SESSION AUTHORIZATION;
2688+
SET row_security TO ON;
2689+
CREATE TABLE copy_rel_to (a integer, b text);
2690+
CREATE POLICY p1 ON copy_rel_to USING (a % 2 = 0);
2691+
ALTER TABLE copy_rel_to ENABLE ROW LEVEL SECURITY;
2692+
GRANT ALL ON copy_rel_to TO rls_regress_user1, rls_regress_exempt_user;
2693+
INSERT INTO copy_rel_to VALUES (1, md5('1'));
2694+
-- Check COPY TO as Superuser/owner.
2695+
RESET SESSION AUTHORIZATION;
2696+
SET row_security TO OFF;
2697+
COPY copy_rel_to TO STDOUT WITH DELIMITER ',';
2698+
1,c4ca4238a0b923820dcc509a6f75849b
2699+
SET row_security TO ON;
2700+
COPY copy_rel_to TO STDOUT WITH DELIMITER ',';
2701+
1,c4ca4238a0b923820dcc509a6f75849b
2702+
SET row_security TO FORCE;
2703+
COPY copy_rel_to TO STDOUT WITH DELIMITER ',';
2704+
-- Check COPY TO as user with permissions.
2705+
SET SESSION AUTHORIZATION rls_regress_user1;
2706+
SET row_security TO OFF;
2707+
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls
2708+
ERROR: insufficient privilege to bypass row security.
2709+
SET row_security TO ON;
2710+
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
2711+
SET row_security TO FORCE;
2712+
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
2713+
-- Check COPY TO as user with permissions and BYPASSRLS
2714+
SET SESSION AUTHORIZATION rls_regress_exempt_user;
2715+
SET row_security TO OFF;
2716+
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
2717+
1,c4ca4238a0b923820dcc509a6f75849b
2718+
SET row_security TO ON;
2719+
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
2720+
SET row_security TO FORCE;
2721+
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
2722+
-- Check COPY TO as user without permissions. SET row_security TO OFF;
2723+
SET SESSION AUTHORIZATION rls_regress_user2;
2724+
SET row_security TO OFF;
2725+
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - permission denied
2726+
ERROR: permission denied for relation copy_rel_to
2727+
SET row_security TO ON;
2728+
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - permission denied
2729+
ERROR: permission denied for relation copy_rel_to
2730+
SET row_security TO FORCE;
2731+
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - permission denied
2732+
ERROR: permission denied for relation copy_rel_to
26862733
-- Check COPY FROM as Superuser/owner.
26872734
RESET SESSION AUTHORIZATION;
26882735
SET row_security TO OFF;
@@ -2731,6 +2778,7 @@ COPY copy_t FROM STDIN; --fail - permission denied.
27312778
ERROR: permission denied for relation copy_t
27322779
RESET SESSION AUTHORIZATION;
27332780
DROP TABLE copy_t;
2781+
DROP TABLE copy_rel_to CASCADE;
27342782
-- Check WHERE CURRENT OF
27352783
SET SESSION AUTHORIZATION rls_regress_user0;
27362784
CREATE TABLE current_check (currentid int, payload text, rlsuser text);

‎src/test/regress/sql/rowsecurity.sql

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1028,7 +1028,7 @@ COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok
10281028
SET row_security TO FORCE;
10291029
COPY (SELECT*FROM copy_tORDER BY aASC) TO STDOUT WITH DELIMITER',';--ok
10301030

1031-
-- Check COPY TO as user without permissions.SET row_security TO OFF;
1031+
-- Check COPY TO as user without permissions.SET row_security TO OFF;
10321032
SET SESSION AUTHORIZATION rls_regress_user2;
10331033
SET row_security TO OFF;
10341034
COPY (SELECT*FROM copy_tORDER BY aASC) TO STDOUT WITH DELIMITER',';--fail - insufficient to bypass rls
@@ -1037,6 +1037,54 @@ COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail
10371037
SET row_security TO FORCE;
10381038
COPY (SELECT*FROM copy_tORDER BY aASC) TO STDOUT WITH DELIMITER',';--fail - permission denied
10391039

1040+
-- Check COPY relation TO; keep it just one row to avoid reordering issues
1041+
RESET SESSION AUTHORIZATION;
1042+
SET row_security TOON;
1043+
CREATETABLEcopy_rel_to (ainteger, btext);
1044+
CREATE POLICY p1ON copy_rel_to USING (a %2=0);
1045+
1046+
ALTERTABLE copy_rel_to ENABLE ROW LEVEL SECURITY;
1047+
1048+
GRANT ALLON copy_rel_to TO rls_regress_user1, rls_regress_exempt_user;
1049+
1050+
INSERT INTO copy_rel_toVALUES (1, md5('1'));
1051+
1052+
-- Check COPY TO as Superuser/owner.
1053+
RESET SESSION AUTHORIZATION;
1054+
SET row_security TO OFF;
1055+
COPY copy_rel_to TO STDOUT WITH DELIMITER',';
1056+
SET row_security TOON;
1057+
COPY copy_rel_to TO STDOUT WITH DELIMITER',';
1058+
SET row_security TO FORCE;
1059+
COPY copy_rel_to TO STDOUT WITH DELIMITER',';
1060+
1061+
-- Check COPY TO as user with permissions.
1062+
SET SESSION AUTHORIZATION rls_regress_user1;
1063+
SET row_security TO OFF;
1064+
COPY copy_rel_to TO STDOUT WITH DELIMITER',';--fail - insufficient to bypass rls
1065+
SET row_security TOON;
1066+
COPY copy_rel_to TO STDOUT WITH DELIMITER',';--ok
1067+
SET row_security TO FORCE;
1068+
COPY copy_rel_to TO STDOUT WITH DELIMITER',';--ok
1069+
1070+
-- Check COPY TO as user with permissions and BYPASSRLS
1071+
SET SESSION AUTHORIZATION rls_regress_exempt_user;
1072+
SET row_security TO OFF;
1073+
COPY copy_rel_to TO STDOUT WITH DELIMITER',';--ok
1074+
SET row_security TOON;
1075+
COPY copy_rel_to TO STDOUT WITH DELIMITER',';--ok
1076+
SET row_security TO FORCE;
1077+
COPY copy_rel_to TO STDOUT WITH DELIMITER',';--ok
1078+
1079+
-- Check COPY TO as user without permissions. SET row_security TO OFF;
1080+
SET SESSION AUTHORIZATION rls_regress_user2;
1081+
SET row_security TO OFF;
1082+
COPY copy_rel_to TO STDOUT WITH DELIMITER',';--fail - permission denied
1083+
SET row_security TOON;
1084+
COPY copy_rel_to TO STDOUT WITH DELIMITER',';--fail - permission denied
1085+
SET row_security TO FORCE;
1086+
COPY copy_rel_to TO STDOUT WITH DELIMITER',';--fail - permission denied
1087+
10401088
-- Check COPY FROM as Superuser/owner.
10411089
RESET SESSION AUTHORIZATION;
10421090
SET row_security TO OFF;
@@ -1090,6 +1138,7 @@ COPY copy_t FROM STDIN; --fail - permission denied.
10901138

10911139
RESET SESSION AUTHORIZATION;
10921140
DROPTABLE copy_t;
1141+
DROPTABLE copy_rel_to CASCADE;
10931142

10941143
-- Check WHERE CURRENT OF
10951144
SET SESSION AUTHORIZATION rls_regress_user0;

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp