Skip to content

P
Postgres FD Implementation
Commit 3d5cb31c authored by Stephen Frost's avatar Stephen Frost
Browse files
Options

Improve RLS handling in copy.c 

To avoid a race condition where the relation being COPY'd could be
changed into a view or otherwise modified, keep the original lock
on the relation.  Further, fully qualify the relation when building
the query up.

Also remove the poorly thought-out Assert() and check the entire
relationOids list as, post-RLS, there can certainly be multiple
relations involved and the planner does not guarantee their ordering.

Per discussion with Noah and Andres.

Back-patch to 9.5 where RLS was introduced.
parent 4c8f8ffa
Hide whitespace changes
Inline Side-by-side
Showing with 126 additions and 20 deletions
src/backend/commands/copy.c
View file @ 3d5cb31c
......@@ -896,8 +896,12 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed)
target->val = (Node *) cr;
target->location = 1;
/* Build FROM clause */
from = stmt->relation;
/*
* Build RangeVar for from clause, fully qualified based on the
* relation which we have opened and locked.
*/
from = makeRangeVar(get_namespace_name(RelationGetNamespace(rel)),
RelationGetRelationName(rel), -1);
/* Build query */
select = makeNode(SelectStmt);
......@@ -906,8 +910,13 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed)
query = (Node *) select;
/* Close the handle to the relation as it is no longer needed. */
heap_close(rel, (is_from ? RowExclusiveLock : AccessShareLock));
/*
* Close the relation for now, but keep the lock on it to prevent
* changes between now and when we start the query-based COPY.
*
* We'll reopen it later as part of the query-based COPY.
*/
heap_close(rel, NoLock);
rel = NULL;
}
}
......@@ -1407,25 +1416,25 @@ BeginCopy(bool is_from,
plan = planner(query, 0, NULL);
/*
* If we were passed in a relid, make sure we got the same one back
* after planning out the query. It's possible that it changed
* between when we checked the policies on the table and decided to
* use a query and now.
* With row level security and a user using "COPY relation TO", we
* have to convert the "COPY relation TO" to a query-based COPY (eg:
* "COPY (SELECT * FROM relation) TO"), to allow the rewriter to add
* in any RLS clauses.
*
* When this happens, we are passed in the relid of the originally
* found relation (which we have locked). As the planner will look
* up the relation again, we double-check here to make sure it found
* the same one that we have locked.
*/
if (queryRelId != InvalidOid)
{
Oid relid = linitial_oid(plan->relationOids);
/*
* There should only be one relationOid in this case, since we
* will only get here when we have changed the command for the
* user from a "COPY relation TO" to "COPY (SELECT * FROM
* relation) TO", to allow row level security policies to be
* applied.
* Note that with RLS involved there may be multiple relations,
* and while the one we need is almost certainly first, we don't
* make any guarantees of that in the planner, so check the whole
* list and make sure we find the original relation.
*/
Assert(list_length(plan->relationOids) == 1);
if (relid != queryRelId)
if (!list_member_oid(plan->relationOids, queryRelId))
ereport(ERROR,
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
errmsg("relation referenced by COPY statement has changed")));
......
src/test/regress/expected/rowsecurity.out
View file @ 3d5cb31c
......@@ -2672,7 +2672,7 @@ COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok
6,1679091c5a880faf6fb5e6087eb1b2dc
8,c9f0f895fb98ab9159f51fd0297e236d
10,d3d9446802a44259755d38e6d163e820
-- Check COPY TO as user without permissions.SET row_security TO OFF;
-- Check COPY TO as user without permissions. SET row_security TO OFF;
SET SESSION AUTHORIZATION rls_regress_user2;
SET row_security TO OFF;
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
SET row_security TO FORCE;
COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - permission denied
ERROR: permission denied for relation copy_t
-- Check COPY relation TO; keep it just one row to avoid reordering issues
RESET SESSION AUTHORIZATION;
SET row_security TO ON;
CREATE TABLE copy_rel_to (a integer, b text);
CREATE POLICY p1 ON copy_rel_to USING (a % 2 = 0);
ALTER TABLE copy_rel_to ENABLE ROW LEVEL SECURITY;
GRANT ALL ON copy_rel_to TO rls_regress_user1, rls_regress_exempt_user;
INSERT INTO copy_rel_to VALUES (1, md5('1'));
-- Check COPY TO as Superuser/owner.
RESET SESSION AUTHORIZATION;
SET row_security TO OFF;
COPY copy_rel_to TO STDOUT WITH DELIMITER ',';
1,c4ca4238a0b923820dcc509a6f75849b
SET row_security TO ON;
COPY copy_rel_to TO STDOUT WITH DELIMITER ',';
1,c4ca4238a0b923820dcc509a6f75849b
SET row_security TO FORCE;
COPY copy_rel_to TO STDOUT WITH DELIMITER ',';
-- Check COPY TO as user with permissions.
SET SESSION AUTHORIZATION rls_regress_user1;
SET row_security TO OFF;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls
ERROR: insufficient privilege to bypass row security.
SET row_security TO ON;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
SET row_security TO FORCE;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
-- Check COPY TO as user with permissions and BYPASSRLS
SET SESSION AUTHORIZATION rls_regress_exempt_user;
SET row_security TO OFF;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
1,c4ca4238a0b923820dcc509a6f75849b
SET row_security TO ON;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
SET row_security TO FORCE;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
-- Check COPY TO as user without permissions. SET row_security TO OFF;
SET SESSION AUTHORIZATION rls_regress_user2;
SET row_security TO OFF;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - permission denied
ERROR: permission denied for relation copy_rel_to
SET row_security TO ON;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - permission denied
ERROR: permission denied for relation copy_rel_to
SET row_security TO FORCE;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - permission denied
ERROR: permission denied for relation copy_rel_to
-- Check COPY FROM as Superuser/owner.
RESET SESSION AUTHORIZATION;
SET row_security TO OFF;
......@@ -2731,6 +2778,7 @@ COPY copy_t FROM STDIN; --fail - permission denied.
ERROR: permission denied for relation copy_t
RESET SESSION AUTHORIZATION;
DROP TABLE copy_t;
DROP TABLE copy_rel_to CASCADE;
-- Check WHERE CURRENT OF
SET SESSION AUTHORIZATION rls_regress_user0;
CREATE TABLE current_check (currentid int, payload text, rlsuser text);
......
src/test/regress/sql/rowsecurity.sql
View file @ 3d5cb31c
......@@ -1028,7 +1028,7 @@ COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok
SET row_security TO FORCE;
COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --ok
-- Check COPY TO as user without permissions.SET row_security TO OFF;
-- Check COPY TO as user without permissions. SET row_security TO OFF;
SET SESSION AUTHORIZATION rls_regress_user2;
SET row_security TO OFF;
COPY (SELECT * FROM copy_t ORDER BY a ASC) 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
SET row_security TO FORCE;
COPY (SELECT * FROM copy_t ORDER BY a ASC) TO STDOUT WITH DELIMITER ','; --fail - permission denied
-- Check COPY relation TO; keep it just one row to avoid reordering issues
RESET SESSION AUTHORIZATION;
SET row_security TO ON;
CREATE TABLE copy_rel_to (a integer, b text);
CREATE POLICY p1 ON copy_rel_to USING (a % 2 = 0);
ALTER TABLE copy_rel_to ENABLE ROW LEVEL SECURITY;
GRANT ALL ON copy_rel_to TO rls_regress_user1, rls_regress_exempt_user;
INSERT INTO copy_rel_to VALUES (1, md5('1'));
-- Check COPY TO as Superuser/owner.
RESET SESSION AUTHORIZATION;
SET row_security TO OFF;
COPY copy_rel_to TO STDOUT WITH DELIMITER ',';
SET row_security TO ON;
COPY copy_rel_to TO STDOUT WITH DELIMITER ',';
SET row_security TO FORCE;
COPY copy_rel_to TO STDOUT WITH DELIMITER ',';
-- Check COPY TO as user with permissions.
SET SESSION AUTHORIZATION rls_regress_user1;
SET row_security TO OFF;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - insufficient to bypass rls
SET row_security TO ON;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
SET row_security TO FORCE;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
-- Check COPY TO as user with permissions and BYPASSRLS
SET SESSION AUTHORIZATION rls_regress_exempt_user;
SET row_security TO OFF;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
SET row_security TO ON;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
SET row_security TO FORCE;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --ok
-- Check COPY TO as user without permissions. SET row_security TO OFF;
SET SESSION AUTHORIZATION rls_regress_user2;
SET row_security TO OFF;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - permission denied
SET row_security TO ON;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - permission denied
SET row_security TO FORCE;
COPY copy_rel_to TO STDOUT WITH DELIMITER ','; --fail - permission denied
-- Check COPY FROM as Superuser/owner.
RESET SESSION AUTHORIZATION;
SET row_security TO OFF;
......@@ -1090,6 +1138,7 @@ COPY copy_t FROM STDIN; --fail - permission denied.
RESET SESSION AUTHORIZATION;
DROP TABLE copy_t;
DROP TABLE copy_rel_to CASCADE;
-- Check WHERE CURRENT OF
SET SESSION AUTHORIZATION rls_regress_user0;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment