-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 06/01/2015 02:21 AM, Dean Rasheed wrote: > While going through this, I spotted another issue --- in a DML > query with additional non-target relations, such as UPDATE t1 .. > FROM t2 .., the old code was checking the UPDATE policies of both > t1 and t2, but really I think it ought to be checking the SELECT > policies of t2 (in the same way as this query requires SELECT table > permissions on t2, not UPDATE permissions). I've changed that and > added new regression tests to test that change.
I assume the entire refactoring patch needs a fair bit of work to rebase against current HEAD, but I picked out the attached to address just the above issue. Does this look correct, and if so does it make sense to apply at least this part right now? Thanks, Joe -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJVuXFuAAoJEDfy90M199hlOMkP/RZoBU0MJF64GjHfuLVa3T5w PnDrnIoLBMOgOzkXrI+Ov0w0ESXltNvYjyIxkuyaB5PuoeDOdyYnnzbpPe7hH4pv zDjSinJnfNmFEcm0UjBzuBiSH0dv52PEIToexTKu6SnjvH3Co/Hk4Ar2uZ0r7bRF krl+Dd4kZX1uuRIigsSFqy873C79m3o11Szs5aW8c5od9adGxSvRHqZNf/UIDIbZ CxAo0E3Tlw0DmGl1cw1tdN8gWMzmvx5dQ0ih3+0/hkVUqN9p3Pg8BGajSvxxFlb2 l4+6RZGUw5ZTpJxRZf/zPc4updhq0zf/Z5g7GUYddrhO29eLS6al2ySlb7HL5G9M VPMjEzXuhFwhxSMdgHfz8UQh82KuNENMTKp81BvtIgZ7w86A9lWrKxCLaVMGhi8m MnfCQ4cdOmnE2vEHi0Ue3Dg/rvYO8QpVW0JKDOdzoqPErC7to9vorXI5X3vcfLbF fYfiJe6wUwbDEdxh/z0oKVFxWlf2NRk4pd+jZ7C+ibraLIwgEgZe7G4GEje5LxSP h4zTfx0sj3IyrvqziurO/aZqIBXBZEsm3Gv6OQs26C5Xvkx/QmgROB42lHwcDYri BTk6+uzNYKbX+kW56vEY0f0WMTLYZzc6jZRIVr3s+aLeyG0P9acY/n3uY1xBFCZZ Xb7gmepAN3rY1CF7By9o =e5hz -----END PGP SIGNATURE-----
diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c index 2386cf0..562dbc9 100644 *** a/src/backend/rewrite/rowsecurity.c --- b/src/backend/rewrite/rowsecurity.c *************** get_row_security_policies(Query *root, C *** 147,154 **** return; } ! /* Grab the built-in policies which should be applied to this relation. */ rel = heap_open(rte->relid, NoLock); rowsec_policies = pull_row_security_policies(commandType, rel, user_id); --- 147,164 ---- return; } ! /* ! * RLS is enabled for this relation. ! * ! * Get the security policies that should be applied, based on the command ! * type. Note that if this isn't the target relation, we actually want ! * the relation's SELECT policies, regardless of the query command type, ! * for example in UPDATE t1 ... FROM t2 we need to apply t1's UPDATE ! * policies and t2's SELECT policies. ! */ rel = heap_open(rte->relid, NoLock); + if (rt_index != root->resultRelation) + commandType = CMD_SELECT; rowsec_policies = pull_row_security_policies(commandType, rel, user_id); diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index b0556c2..6fc80af 100644 *** a/src/test/regress/expected/rowsecurity.out --- b/src/test/regress/expected/rowsecurity.out *************** CREATE POLICY p ON t USING (max(c)); -- *** 3033,3038 **** --- 3033,3121 ---- ERROR: aggregate functions are not allowed in policy expressions ROLLBACK; -- + -- Non-target relations are only subject to SELECT policies + -- + SET SESSION AUTHORIZATION rls_regress_user0; + CREATE TABLE r1 (a int); + CREATE TABLE r2 (a int); + INSERT INTO r1 VALUES (10), (20); + INSERT INTO r2 VALUES (10), (20); + GRANT ALL ON r1, r2 TO rls_regress_user1; + CREATE POLICY p1 ON r1 USING (true); + ALTER TABLE r1 ENABLE ROW LEVEL SECURITY; + CREATE POLICY p1 ON r2 FOR SELECT USING (true); + CREATE POLICY p2 ON r2 FOR INSERT WITH CHECK (false); + CREATE POLICY p3 ON r2 FOR UPDATE USING (false); + CREATE POLICY p4 ON r2 FOR DELETE USING (false); + ALTER TABLE r2 ENABLE ROW LEVEL SECURITY; + SET SESSION AUTHORIZATION rls_regress_user1; + SELECT * FROM r1; + a + ---- + 10 + 20 + (2 rows) + + SELECT * FROM r2; + a + ---- + 10 + 20 + (2 rows) + + -- r2 is read-only + INSERT INTO r2 VALUES (2); -- Not allowed + ERROR: new row violates row level security policy for "r2" + UPDATE r2 SET a = 2 RETURNING *; -- Updates nothing + a + --- + (0 rows) + + DELETE FROM r2 RETURNING *; -- Deletes nothing + a + --- + (0 rows) + + -- r2 can be used as a non-target relation in DML + INSERT INTO r1 SELECT a + 1 FROM r2 RETURNING *; -- OK + a + ---- + 11 + 21 + (2 rows) + + UPDATE r1 SET a = r2.a + 2 FROM r2 WHERE r1.a = r2.a RETURNING *; -- OK + a | a + ----+---- + 12 | 10 + 22 | 20 + (2 rows) + + DELETE FROM r1 USING r2 WHERE r1.a = r2.a + 2 RETURNING *; -- OK + a | a + ----+---- + 12 | 10 + 22 | 20 + (2 rows) + + SELECT * FROM r1; + a + ---- + 11 + 21 + (2 rows) + + SELECT * FROM r2; + a + ---- + 10 + 20 + (2 rows) + + SET SESSION AUTHORIZATION rls_regress_user0; + DROP TABLE r1; + DROP TABLE r2; + -- -- Clean up objects -- RESET SESSION AUTHORIZATION; diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index 300f34a..e8c09e9 100644 *** a/src/test/regress/sql/rowsecurity.sql --- b/src/test/regress/sql/rowsecurity.sql *************** CREATE POLICY p ON t USING (max(c)); -- *** 1299,1304 **** --- 1299,1344 ---- ROLLBACK; -- + -- Non-target relations are only subject to SELECT policies + -- + SET SESSION AUTHORIZATION rls_regress_user0; + CREATE TABLE r1 (a int); + CREATE TABLE r2 (a int); + INSERT INTO r1 VALUES (10), (20); + INSERT INTO r2 VALUES (10), (20); + + GRANT ALL ON r1, r2 TO rls_regress_user1; + + CREATE POLICY p1 ON r1 USING (true); + ALTER TABLE r1 ENABLE ROW LEVEL SECURITY; + + CREATE POLICY p1 ON r2 FOR SELECT USING (true); + CREATE POLICY p2 ON r2 FOR INSERT WITH CHECK (false); + CREATE POLICY p3 ON r2 FOR UPDATE USING (false); + CREATE POLICY p4 ON r2 FOR DELETE USING (false); + ALTER TABLE r2 ENABLE ROW LEVEL SECURITY; + + SET SESSION AUTHORIZATION rls_regress_user1; + SELECT * FROM r1; + SELECT * FROM r2; + + -- r2 is read-only + INSERT INTO r2 VALUES (2); -- Not allowed + UPDATE r2 SET a = 2 RETURNING *; -- Updates nothing + DELETE FROM r2 RETURNING *; -- Deletes nothing + + -- r2 can be used as a non-target relation in DML + INSERT INTO r1 SELECT a + 1 FROM r2 RETURNING *; -- OK + UPDATE r1 SET a = r2.a + 2 FROM r2 WHERE r1.a = r2.a RETURNING *; -- OK + DELETE FROM r1 USING r2 WHERE r1.a = r2.a + 2 RETURNING *; -- OK + SELECT * FROM r1; + SELECT * FROM r2; + + SET SESSION AUTHORIZATION rls_regress_user0; + DROP TABLE r1; + DROP TABLE r2; + + -- -- Clean up objects -- RESET SESSION AUTHORIZATION;
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
