From ff27fcfa0affe16405e801ed55fed10e7bc75216 Mon Sep 17 00:00:00 2001 From: Stephen Frost Date: Fri, 26 Sep 2014 12:46:26 -0400 Subject: [PATCH] Fix relcache for policies, and doc updates Andres pointed out that there was an extra ';' in equalPolicies, which made me realize that my prior testing with CLOBBER_CACHE_ALWAYS was insufficient (it didn't always catch the issue, just most of the time). Thanks to that, a different issue was discovered, specifically in equalRSDescs. This change corrects eqaulRSDescs to return 'true' once all policies have been confirmed logically identical. After stepping through both functions to ensure correct behavior, I ran this for about 12 hours of CLOBBER_CACHE_ALWAYS runs of the regression tests with no failures. In addition, correct a few typos in the documentation which were pointed out by Thom Brown (thanks!) and improve the policy documentation further by adding a flushed out usage example based on a unix passwd file. Lastly, clean up a few comments in the regression tests and pg_dump.h. --- doc/src/sgml/ddl.sgml | 111 +++++++++++++++++++++- doc/src/sgml/ref/alter_policy.sgml | 2 +- doc/src/sgml/ref/create_policy.sgml | 6 +- src/backend/utils/cache/relcache.c | 4 +- src/bin/pg_dump/pg_dump.h | 2 +- src/test/regress/expected/rowsecurity.out | 10 +- src/test/regress/sql/rowsecurity.sql | 10 +- 7 files changed, 127 insertions(+), 18 deletions(-) diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 98e897aeb6..f9dc151a0c 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -1615,7 +1615,7 @@ REVOKE ALL ON accounts FROM PUBLIC; and foreign key references, will bypass row security to ensure that data integrity is maintained. Care must be taken when developing schemas and row level policies to avoid a "covert channel" leak of - information through these referntial integrity checks. + information through these referential integrity checks. @@ -1674,6 +1674,115 @@ CREATE POLICY user_policy ON users normal privileges system. + + Below is a larger example of how this feature can be used in + production environments, based on a unix password file. + + + +-- Simple passwd-file based example +CREATE TABLE passwd ( + username text UNIQUE NOT NULL, + pwhash text, + uid int PRIMARY KEY, + gid int NOT NULL, + real_name text NOT NULL, + home_phone text, + extra_info text, + home_dir text NOT NULL, + shell text NOT NULL +); + +CREATE ROLE admin; -- Administrator +CREATE ROLE bob; -- Normal user +CREATE ROLE alice; -- Normal user + +-- Populate the table +INSERT INTO passwd VALUES + ('admin','xxx',0,0,'Admin','111-222-3333',null,'/root','/bin/dash'); +INSERT INTO passwd VALUES + ('bob','xxx',1,1,'Bob','123-456-7890',null,'/home/bob','/bin/zsh'); +INSERT INTO passwd VALUES + ('alice','xxx',2,1,'Alice','098-765-4321',null,'/home/alice','/bin/zsh'); + +-- Be sure to enable row level security on the table +ALTER TABLE passwd ENABLE ROW LEVEL SECURITY; + +-- Create policies +-- Administrator can see all rows and add any rows +CREATE POLICY admin_all ON passwd TO admin USING (true) WITH CHECK (true); +-- Normal users can view all rows +CREATE POLICY all_view ON passwd FOR SELECT USING (true); +-- Normal users can update their own records, but +-- limit which shells a normal user is allowed to set +CREATE POLICY user_mod ON passwd FOR UPDATE + USING (current_user = username) + WITH CHECK ( + current_user = username AND + shell IN ('/bin/bash','/bin/sh','/bin/dash','/bin/zsh','/bin/tcsh') + ); + +-- Allow admin all normal rights +GRANT SELECT, INSERT, UPDATE, DELETE ON passwd TO admin; +-- Users only get select access on public columns +GRANT SELECT + (username, uid, gid, real_name, home_phone, extra_info, home_dir, shell) + ON passwd TO public; +-- Allow users to update certain columns +GRANT UPDATE + (pwhash, real_name, home_phone, extra_info, shell) + ON passwd TO public; + + + + As with any security settings, it's important to test and ensure that + the system is behaving as expected. Using the example above, this + demonstrates that the permission system is working properly. + + + +-- admin can view all rows and fields +postgres=> set role admin; +SET +postgres=> table passwd; + username | pwhash | uid | gid | real_name | home_phone | extra_info | home_dir | shell +----------+--------+-----+-----+-----------+--------------+------------+-------------+----------- + admin | xxx | 0 | 0 | Admin | 111-222-3333 | | /root | /bin/dash + bob | xxx | 1 | 1 | Bob | 123-456-7890 | | /home/bob | /bin/zsh + alice | xxx | 2 | 1 | Alice | 098-765-4321 | | /home/alice | /bin/zsh +(3 rows) + +-- Test what Alice is able to do +postgres=> set role alice; +SET +postgres=> table passwd; +ERROR: permission denied for relation passwd +postgres=> select username,real_name,home_phone,extra_info,home_dir,shell from passwd; + username | real_name | home_phone | extra_info | home_dir | shell +----------+-----------+--------------+------------+-------------+----------- + admin | Admin | 111-222-3333 | | /root | /bin/dash + bob | Bob | 123-456-7890 | | /home/bob | /bin/zsh + alice | Alice | 098-765-4321 | | /home/alice | /bin/zsh +(3 rows) + +postgres=> update passwd set username = 'joe'; +ERROR: permission denied for relation passwd +-- Allowed to change her own real_name, but no others +postgres=> update passwd set real_name = 'Alice Doe'; +UPDATE 1 +postgres=> update passwd set real_name = 'John Doe' where username = 'admin'; +UPDATE 0 +postgres=> update passwd set shell = '/bin/xx'; +ERROR: new row violates WITH CHECK OPTION for "passwd" +postgres=> delete from passwd; +ERROR: permission denied for relation passwd +postgres=> insert into passwd (username) values ('xxx'); +ERROR: permission denied for relation passwd +-- Alice can change her own password +postgres=> update passwd set pwhash = 'abc'; +UPDATE 1 + + diff --git a/doc/src/sgml/ref/alter_policy.sgml b/doc/src/sgml/ref/alter_policy.sgml index 37615fcab5..ab717f31c5 100644 --- a/doc/src/sgml/ref/alter_policy.sgml +++ b/doc/src/sgml/ref/alter_policy.sgml @@ -94,7 +94,7 @@ ALTER POLICY name ON diff --git a/doc/src/sgml/ref/create_policy.sgml b/doc/src/sgml/ref/create_policy.sgml index 3c5bdc69cd..eff062c114 100644 --- a/doc/src/sgml/ref/create_policy.sgml +++ b/doc/src/sgml/ref/create_policy.sgml @@ -81,7 +81,7 @@ CREATE POLICY name ON @@ -218,7 +218,7 @@ CREATE POLICY name ON SELECT policy will be returned, even if other records exist in the relation. The SELECT policy only accepts the USING expression - as it only ever applies in cases where records are being retrived from + as it only ever applies in cases where records are being retrieved from the relation. @@ -272,7 +272,7 @@ CREATE POLICY name ON DELETE, and rows which are not visible through the SELECT policy may be deleted if they pass the DELETE USING policy. The - DELETE policy only accept the USING expression as + DELETE policy only accepts the USING expression as it only ever applies in cases where records are being extracted from the relation for deletion. diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index ec483a0e8e..c8137798f2 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -868,7 +868,7 @@ equalPolicy(RowSecurityPolicy *policy1, RowSecurityPolicy *policy2) return false; if (policy1->cmd != policy2->cmd) return false; - if (policy1->hassublinks != policy2->hassublinks); + if (policy1->hassublinks != policy2->hassublinks) return false; if (strcmp(policy1->policy_name,policy2->policy_name) != 0) return false; @@ -926,7 +926,7 @@ equalRSDesc(RowSecurityDesc *rsdesc1, RowSecurityDesc *rsdesc2) return false; } - return false; + return true; } /* diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index 646a2077a6..fd1184e8db 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -246,7 +246,7 @@ typedef struct _tableInfo bool hasindex; /* does it have any indexes? */ bool hasrules; /* does it have any rules? */ bool hastriggers; /* does it have any triggers? */ - bool rowsec; /* does it have any row-security policy? */ + bool rowsec; /* is row-security enabled? */ bool hasoids; /* does it have OIDs? */ uint32 frozenxid; /* for restore frozen xid */ uint32 minmxid; /* for restore min multi xid */ diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 007afc606b..3d793e2ff5 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -69,7 +69,7 @@ INSERT INTO document VALUES ( 7, 33, 2, 'rls_regress_user2', 'great technology book'), ( 8, 44, 1, 'rls_regress_user2', 'great manga'); ALTER TABLE document ENABLE ROW LEVEL SECURITY; --- user's security level must be higher that or equal to document's +-- user's security level must be higher than or equal to document's CREATE POLICY p1 ON document USING (dlevel <= (SELECT seclv FROM uaccount WHERE pguser = current_user)); -- viewpoint from rls_regress_user1 @@ -280,7 +280,7 @@ SELECT * FROM document d FULL OUTER JOIN category c on d.cid = c.cid; DELETE FROM category WHERE cid = 33; -- fails with FK violation ERROR: update or delete on table "category" violates foreign key constraint "document_cid_fkey" on table "document" DETAIL: Key (cid)=(33) is still referenced from table "document". --- cannot insert FK referencing invisible PK +-- can insert FK referencing invisible PK SET SESSION AUTHORIZATION rls_regress_user2; SELECT * FROM document d FULL OUTER JOIN category c on d.cid = c.cid; did | cid | dlevel | dauthor | dtitle | cid | cname @@ -301,7 +301,7 @@ SELECT * FROM document WHERE did = 8; -- and confirm we can't see it -----+-----+--------+---------+-------- (0 rows) --- database superuser cannot bypass RLS policy when enabled +-- database superuser does bypass RLS policy when enabled RESET SESSION AUTHORIZATION; SET row_security TO ON; SELECT * FROM document; @@ -327,7 +327,7 @@ SELECT * FROM category; 44 | manga (4 rows) --- database superuser cannot bypass RLS policy when FORCE enabled. +-- database superuser does not bypass RLS policy when FORCE enabled. RESET SESSION AUTHORIZATION; SET row_security TO FORCE; SELECT * FROM document; @@ -340,7 +340,7 @@ SELECT * FROM category; -----+------- (0 rows) --- database superuser can bypass RLS policy when disabled +-- database superuser does bypass RLS policy when disabled RESET SESSION AUTHORIZATION; SET row_security TO OFF; SELECT * FROM document; diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index 5409bb055a..b7969a8dab 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -84,7 +84,7 @@ INSERT INTO document VALUES ALTER TABLE document ENABLE ROW LEVEL SECURITY; --- user's security level must be higher that or equal to document's +-- user's security level must be higher than or equal to document's CREATE POLICY p1 ON document USING (dlevel <= (SELECT seclv FROM uaccount WHERE pguser = current_user)); @@ -136,7 +136,7 @@ SET SESSION AUTHORIZATION rls_regress_user1; SELECT * FROM document d FULL OUTER JOIN category c on d.cid = c.cid; DELETE FROM category WHERE cid = 33; -- fails with FK violation --- cannot insert FK referencing invisible PK +-- can insert FK referencing invisible PK SET SESSION AUTHORIZATION rls_regress_user2; SELECT * FROM document d FULL OUTER JOIN category c on d.cid = c.cid; INSERT INTO document VALUES (10, 33, 1, current_user, 'hoge'); @@ -146,19 +146,19 @@ SET SESSION AUTHORIZATION rls_regress_user1; INSERT INTO document VALUES (8, 44, 1, 'rls_regress_user1', 'my third manga'); -- Must fail with unique violation, revealing presence of did we can't see SELECT * FROM document WHERE did = 8; -- and confirm we can't see it --- database superuser cannot bypass RLS policy when enabled +-- database superuser does bypass RLS policy when enabled RESET SESSION AUTHORIZATION; SET row_security TO ON; SELECT * FROM document; SELECT * FROM category; --- database superuser cannot bypass RLS policy when FORCE enabled. +-- database superuser does not bypass RLS policy when FORCE enabled. RESET SESSION AUTHORIZATION; SET row_security TO FORCE; SELECT * FROM document; SELECT * FROM category; --- database superuser can bypass RLS policy when disabled +-- database superuser does bypass RLS policy when disabled RESET SESSION AUTHORIZATION; SET row_security TO OFF; SELECT * FROM document;