Merge #142479

142479: sql: support multiple RLS policies on a table r=spilchen a=spilchen

Previously, row-level security (RLS) tables only ever enforced one policy. This change allows multiple policies to be applied.

When multiple policies exist, their type determines how they are combined:
- Permissive policies are combined using OR
- Restrictive policies are combined using AND
- At least one permissive policy is required; otherwise, the table is treated as deny-all (all rows are filtered out, or all new rows are rejected).

The change affects both the filtering of existing rows (USING expression) and constraints on new rows (WITH CHECK expression).

Epic: CRDB-45203
Release note: none
Informs #136742

Co-authored-by: Matt Spilchen <[email protected]>

Author: craig[bot]

pkg/sql/logictest/testdata/logic_test/row_level_security

@@ -1677,4 +1677,162 @@ DROP FUNCTION al_only;
 statement ok
 DROP SEQUENCE seq1;
 
+subtest multiple_policies
+
+statement ok
+CREATE TABLE multip (key INT NOT NULL, value TEXT, FAMILY (key,value));
+
+statement ok
+ALTER TABLE multip ENABLE ROW LEVEL SECURITY;
+
+# The policies will be combined as: (or1 || or2 || or3) && and1 && and2
+statement ok
+CREATE POLICY or1 ON multip AS PERMISSIVE USING (key = 1);
+
+statement ok
+CREATE POLICY or2 ON multip AS PERMISSIVE USING (key = 2);
+
+statement ok
+CREATE POLICY or3 ON multip AS PERMISSIVE USING (key = 3);
+
+statement ok
+CREATE POLICY and1 ON multip AS RESTRICTIVE USING (value not like '%sensitive%')
+
+statement ok
+CREATE POLICY and2 ON multip AS RESTRICTIVE USING (value not like '%confidential%')
+
+statement ok
+INSERT INTO multip VALUES
+ (0, 'key out of bounds'),
+ (1, 'okay'),
+ (1, 'sensitive - filtered out'),
+ (2, 'okay'),
+ (2, 'confidential - filtered out'),
+ (3, 'okay'),
+ (2, 'sensitive - filtered out'),
+ (4, 'key out of bounds'),
+ (4, 'confidential');
+
+statement ok
+CREATE USER multi_user;
+
+statement ok
+GRANT ALL ON multip TO multi_user;
+
+statement ok
+SET ROLE multi_user;
+
+query IT
+select * from multip ORDER BY key, value;
+----
+1  okay
+2  okay
+3  okay
+
+query IT
+select * from multip where key >= 0 ORDER BY key, value;
+----
+1  okay
+2  okay
+3  okay
+
+statement ok
+SET ROLE root
+
+# Ensure that if only a restrictive policy exists that all rows will be rejected
+# regardless of the policy expression.
+statement ok
+DROP POLICY or1 ON multip;
+
+statement ok
+DROP POLICY or2 ON multip;
+
+statement ok
+DROP POLICY or3 ON multip;
+
+statement ok
+SET ROLE multi_user;
+
+query TT
+SHOW CREATE multip;
+----
+multip  CREATE TABLE public.multip (
+          key INT8 NOT NULL,
+          value STRING NULL,
+          rowid INT8 NOT VISIBLE NOT NULL DEFAULT unique_rowid(),
+          CONSTRAINT multip_pkey PRIMARY KEY (rowid ASC),
+          FAMILY fam_0_key_value_rowid (key, value, rowid)
+        );
+        ALTER TABLE public.multip ENABLE ROW LEVEL SECURITY;
+        CREATE POLICY and1 ON public.multip AS RESTRICTIVE FOR ALL TO public USING (value NOT LIKE '%sensitive%':::STRING);
+        CREATE POLICY and2 ON public.multip AS RESTRICTIVE FOR ALL TO public USING (value NOT LIKE '%confidential%':::STRING)
+
+query IT
+select * from multip ORDER BY key, value;
+----
+
+statement ok
+SET ROLE root;
+
+# Setup to verify the write policies.
+statement ok
+TRUNCATE TABLE multip;
+
+statement ok
+CREATE POLICY or1 ON multip AS PERMISSIVE USING (key = 1);
+
+statement ok
+CREATE POLICY or2 ON multip AS PERMISSIVE USING (key = 2);
+
+statement ok
+CREATE POLICY or3 ON multip AS PERMISSIVE USING (key = 3);
+
+statement ok
+SET ROLE multi_user;
+
+statement error pq: new row violates row-level security policy for table "multip"
+INSERT INTO multip VALUES (0, 'key out of bounds');
+
+statement ok
+INSERT INTO multip VALUES (1, 'okay');
+
+statement error pq: new row violates row-level security policy for table "multip"
+INSERT INTO multip VALUES (1, 'sensitive - filtered out');
+
+statement ok
+INSERT INTO multip VALUES (2, 'okay')
+
+statement error pq: new row violates row-level security policy for table "multip"
+INSERT INTO multip VALUES (2, 'confidential - filtered out')
+
+statement ok
+INSERT INTO multip VALUES (3, 'okay')
+
+statement error pq: new row violates row-level security policy for table "multip"
+INSERT INTO multip VALUES (2, 'sensitive - filtered out')
+
+statement error pq: new row violates row-level security policy for table "multip"
+INSERT INTO multip VALUES (4, 'key out of bounds')
+
+statement error pq: new row violates row-level security policy for table "multip"
+INSERT INTO multip VALUES (4, 'confidential');
+
+statement ok
+SET ROLE root;
+
+query IT
+select * FROM multip ORDER BY key, value;
+----
+1  okay
+2  okay
+3  okay
+
+statement ok
+DROP TABLE multip;
+
+statement ok
+DROP USER multi_user;
+
+subtest multiple_insert_policies
+
 subtest end

pkg/sql/opt/exec/explain/emit.go

@@ -1404,14 +1404,14 @@ func (e *emitter) emitPolicies(ob *OutputBuilder, table cat.Table, n *Node) {
 	} else {
 		var sb strings.Builder
 		policies := table.Policies()
-		// TODO(136742): Add support for restrictive policies.
-		for i := range policies.Permissive {
-			policy := policies.Permissive[i]
-			if applied.Policies.Contains(policy.ID) {
-				if sb.Len() > 0 {
-					sb.WriteString(", ")
+		for _, grp := range [][]cat.Policy{policies.Permissive, policies.Restrictive} {
+			for _, policy := range grp {
+				if applied.Policies.Contains(policy.ID) {
+					if sb.Len() > 0 {
+						sb.WriteString(", ")
+					}
+					sb.WriteString(policy.Name.Normalize())
 				}
-				sb.WriteString(policy.Name.Normalize())
 			}
 		}
 		ob.AddField("policies", sb.String())

pkg/sql/opt/exec/explain/testdata/row_level_security

@@ -137,15 +137,13 @@ exec-ddl
 CREATE POLICY r2 on t1 AS RESTRICTIVE USING (true);
 ----
 
-# TODO(136742): We currently only support at most one permissive policy. Update
-# this when we support multiple and restrictive policies.
 plan
 select * from t1
 ----
 • scan
   table: t1@t1_pkey
   spans: FULL SCAN
-  policies: policy 1
+  policies: policy 1, p2, p3, r1, r2
 
 # Show policy information where an index scan is used for one table
 # ----------------------------------------------------------------------
@@ -162,7 +160,7 @@ select t1.c1 from t1, t2 where t1.c1 = t2.c1 and t1.c1 = 1;
 ├── • scan
 │     table: t1@t1_idx
 │     spans: 1+ spans
-│     policies: policy 1
+│     policies: policy 1, p2, p3, r1, r2
 │
 └── • filter
     │ filter: c1 = _
@@ -268,7 +266,7 @@ UPDATE writer SET key = key * 11;
 │ table: writer
 │ set: key
 │ auto commit
-│ policies: p_update1
+│ policies: p_update1, p_update2
 │
 └── • render
     │
@@ -285,7 +283,7 @@ UPDATE writer SET key = key * 15 WHERE value = 'some-val' or value = 'other-val'
 │ table: writer
 │ set: key
 │ auto commit
-│ policies: p_update1
+│ policies: p_update1, p_update2
 │
 └── • render
     │
@@ -304,7 +302,7 @@ UPDATE writer SET value = 'updated' WHERE key between 1 and 100;
 │ table: writer
 │ set: value
 │ auto commit
-│ policies: p_update1
+│ policies: p_update1, p_update2
 │
 └── • render
     │
@@ -321,7 +319,7 @@ UPDATE writer SET value = value WHERE key = 5;
 │ table: writer
 │ set: value
 │ auto commit
-│ policies: p_update1
+│ policies: p_update1, p_update2
 │
 └── • render
     │

pkg/sql/opt/optbuilder/row_level_security.go

@@ -17,6 +17,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/cat"
 	"github.com/cockroachdb/cockroach/pkg/sql/opt/memo"
 	"github.com/cockroachdb/cockroach/pkg/sql/parser"
+	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util/intsets"
 	"github.com/cockroachdb/errors"
@@ -54,32 +55,55 @@ func (b *Builder) buildRowLevelSecurityUsingExpression(
 	tabMeta *opt.TableMeta, tableScope *scope, cmdScope cat.PolicyCommandScope,
 ) opt.ScalarExpr {
 	var policiesUsed opt.PolicyIDSet
+	var combinedExpr tree.TypedExpr
 	policies := tabMeta.Table.Policies()
-	for _, policy := range policies.Permissive {
+
+	// Create a closure to handle building the expression for one policy.
+	buildForPolicy := func(policy cat.Policy, restrictive bool) {
 		if !policy.AppliesToRole(b.checkPrivilegeUser) || !b.policyAppliesToCommandScope(policy, cmdScope) {
-			continue
+			return
 		}
 		strExpr := policy.UsingExpr
 		if strExpr == "" {
-			continue
+			return
 		}
 		policiesUsed.Add(policy.ID)
 		parsedExpr, err := parser.ParseExpr(strExpr)
 		if err != nil {
 			panic(err)
 		}
 		typedExpr := tableScope.resolveType(parsedExpr, types.AnyElement)
-		scalar := b.buildScalar(typedExpr, tableScope, nil, nil, nil)
-		// TODO(136742): Apply multiple RLS policies.
-		b.factory.Metadata().GetRLSMeta().AddPoliciesUsed(tabMeta.MetaID, policiesUsed, true /* applyFilterExpr */)
-		return scalar
+		if combinedExpr != nil {
+			// Restrictive policies are combined using AND, while permissive
+			// policies are combined using OR.
+			if restrictive {
+				combinedExpr = tree.NewTypedAndExpr(combinedExpr, typedExpr)
+			} else {
+				combinedExpr = tree.NewTypedOrExpr(combinedExpr, typedExpr)
+			}
+		} else {
+			combinedExpr = typedExpr
+		}
 	}
 
-	// TODO(136742): Add support for restrictive policies.
+	for _, policy := range policies.Permissive {
+		buildForPolicy(policy, false /* restrictive */)
+	}
+	if combinedExpr == nil {
+		// If no permissive policies apply, filter out all rows by adding a "false" expression.
+		b.factory.Metadata().GetRLSMeta().NoPoliciesApplied = true
+		return memo.FalseSingleton
+	}
+	for _, policy := range policies.Restrictive {
+		buildForPolicy(policy, true /* restrictive */)
+	}
 
-	// If no permissive policies apply, filter out all rows by adding a "false" expression.
-	b.factory.Metadata().GetRLSMeta().NoPoliciesApplied = true
-	return memo.FalseSingleton
+	// We should have already exited early if there were no permissive policies.
+	if combinedExpr == nil {
+		panic(errors.AssertionFailedf("at least one applicable policy should have been found"))
+	}
+	b.factory.Metadata().GetRLSMeta().AddPoliciesUsed(tabMeta.MetaID, policiesUsed, true /* applyFilterExpr */)
+	return b.buildScalar(combinedExpr, tableScope, nil, nil, nil)
 }
 
 // policyAppliesToCommandScope checks whether a given PolicyCommandScope applies
@@ -155,20 +179,22 @@ func (r *optRLSConstraintBuilder) genExpression(ctx context.Context) (string, []
 	}
 
 	var policiesUsed opt.PolicyIDSet
-	for i := range r.tab.Policies().Permissive {
-		p := &r.tab.Policies().Permissive[i]
+	policies := r.tabMeta.Table.Policies()
 
-		if !p.AppliesToRole(r.user) || !r.policyAppliesToCommand(p, r.isUpdate) {
-			continue
+	// Create a closure to handle building the expression for one policy.
+	buildForPolicy := func(p cat.Policy, restrictive bool) {
+		if !p.AppliesToRole(r.user) || !r.policyAppliesToCommand(&p, r.isUpdate) {
+			return
 		}
 		policiesUsed.Add(p.ID)
+
 		var expr string
 		// If the WITH CHECK expression is missing, we default to the USING
 		// expression. If both are missing, then this policy doesn't apply and can
 		// be skipped.
 		if p.WithCheckExpr == "" {
 			if p.UsingExpr == "" {
-				continue
+				return
 			}
 			expr = p.UsingExpr
 			for _, id := range p.UsingColumnIDs {
@@ -181,25 +207,37 @@ func (r *optRLSConstraintBuilder) genExpression(ctx context.Context) (string, []
 			}
 		}
 		if sb.Len() != 0 {
-			sb.WriteString(" OR ")
+			if restrictive {
+				sb.WriteString(" AND ")
+			} else {
+				sb.WriteString(" OR ")
+			}
+		} else {
+			sb.WriteString("(") // Add the outer parenthesis that surrounds all permissive policies
 		}
 		sb.WriteString("(")
 		sb.WriteString(expr)
 		sb.WriteString(")")
-		// TODO(136742): Add support for multiple policies.
-		r.md.GetRLSMeta().AddPoliciesUsed(r.tabMeta.MetaID, policiesUsed, false /* applyFilterExpr */)
-		break
 	}
 
-	// TODO(136742): Add support for restrictive policies.
-
-	// If no policies apply, then we will add a false check as nothing is allowed
-	// to be written.
+	for _, policy := range policies.Permissive {
+		buildForPolicy(policy, false /* restrictive */)
+	}
+	// If no permissive policies apply, then we will add a false check as
+	// nothing is allowed to be written.
 	if sb.Len() == 0 {
 		r.md.GetRLSMeta().NoPoliciesApplied = true
 		return "false", nil
 	}
+	sb.WriteString(")") // Close the outer parenthesis that surrounds all permissive policies
+	for _, policy := range policies.Restrictive {
+		buildForPolicy(policy, true /* restrictive */)
+	}
 
+	if sb.Len() == 0 {
+		panic(errors.AssertionFailedf("at least one applicable policy should have been included"))
+	}
+	r.md.GetRLSMeta().AddPoliciesUsed(r.tabMeta.MetaID, policiesUsed, false /* applyFilterExpr */)
 	return sb.String(), colIDs.Ordered()
 }