diff --git a/doc/src/sgml/plperl.sgml b/doc/src/sgml/plperl.sgml index 01f9870773d..ee02c01f106 100644 --- a/doc/src/sgml/plperl.sgml +++ b/doc/src/sgml/plperl.sgml @@ -1082,6 +1082,19 @@ $$ LANGUAGE plperl; be permitted to use this language. + + + Trusted PL/Perl relies on the Perl Opcode module to + preserve security. + Perl + documents + that the module is not effective for the trusted PL/Perl use case. If + your security needs are incompatible with the uncertainty in that warning, + consider executing REVOKE USAGE ON LANGUAGE plperl FROM + PUBLIC. + + + Here is an example of a function that will not work because file system operations are not allowed for security reasons: diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index 41da04f652b..f1b287d8ff4 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -2120,6 +2120,12 @@ check_sql_fn_retval(List *queryTreeLists, rtr->rtindex = 1; newquery->jointree = makeFromExpr(list_make1(rtr), NULL); + /* + * Make sure the new query is marked as having row security if the + * original one does. + */ + newquery->hasRowSecurity = parse->hasRowSecurity; + /* Replace original query in the correct element of the query list */ lfirst(parse_cell) = newquery; } diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c index 2938edf1c6a..9670eb5d34a 100644 --- a/src/backend/rewrite/rewriteHandler.c +++ b/src/backend/rewrite/rewriteHandler.c @@ -59,6 +59,12 @@ typedef struct acquireLocksOnSubLinks_context bool for_execute; /* AcquireRewriteLocks' forExecute param */ } acquireLocksOnSubLinks_context; +typedef struct fireRIRonSubLink_context +{ + List *activeRIRs; + bool hasRowSecurity; +} fireRIRonSubLink_context; + static bool acquireLocksOnSubLinks(Node *node, acquireLocksOnSubLinks_context *context); static Query *rewriteRuleAction(Query *parsetree, @@ -1910,6 +1916,12 @@ ApplyRetrieveRule(Query *parsetree, */ rule_action = fireRIRrules(rule_action, activeRIRs); + /* + * Make sure the query is marked as having row security if the view query + * does. + */ + parsetree->hasRowSecurity |= rule_action->hasRowSecurity; + /* * Now, plug the view query in as a subselect, converting the relation's * original RTE to a subquery RTE. @@ -2021,7 +2033,7 @@ markQueryForLocking(Query *qry, Node *jtnode, * the SubLink's subselect link with the possibly-rewritten subquery. */ static bool -fireRIRonSubLink(Node *node, List *activeRIRs) +fireRIRonSubLink(Node *node, fireRIRonSubLink_context *context) { if (node == NULL) return false; @@ -2031,7 +2043,13 @@ fireRIRonSubLink(Node *node, List *activeRIRs) /* Do what we came for */ sub->subselect = (Node *) fireRIRrules((Query *) sub->subselect, - activeRIRs); + context->activeRIRs); + + /* + * Remember if any of the sublinks have row security. + */ + context->hasRowSecurity |= ((Query *) sub->subselect)->hasRowSecurity; + /* Fall through to process lefthand args of SubLink */ } @@ -2040,7 +2058,7 @@ fireRIRonSubLink(Node *node, List *activeRIRs) * subselects of subselects for us. */ return expression_tree_walker(node, fireRIRonSubLink, - (void *) activeRIRs); + (void *) context); } @@ -2101,6 +2119,13 @@ fireRIRrules(Query *parsetree, List *activeRIRs) if (rte->rtekind == RTE_SUBQUERY || rte->rtekind == RTE_TABLEFUNCTION) { rte->subquery = fireRIRrules(rte->subquery, activeRIRs); + + /* + * While we are here, make sure the query is marked as having row + * security if any of its subqueries do. + */ + parsetree->hasRowSecurity |= rte->subquery->hasRowSecurity; + continue; } @@ -2218,6 +2243,12 @@ fireRIRrules(Query *parsetree, List *activeRIRs) cte->ctequery = (Node *) fireRIRrules((Query *) cte->ctequery, activeRIRs); + + /* + * While we are here, make sure the query is marked as having row + * security if any of its CTEs do. + */ + parsetree->hasRowSecurity |= ((Query *) cte->ctequery)->hasRowSecurity; } /* @@ -2225,9 +2256,22 @@ fireRIRrules(Query *parsetree, List *activeRIRs) * the rtable and cteList. */ if (parsetree->hasSubLinks) - query_tree_walker(parsetree, fireRIRonSubLink, (void *) activeRIRs, + { + fireRIRonSubLink_context context; + + context.activeRIRs = activeRIRs; + context.hasRowSecurity = false; + + query_tree_walker(parsetree, fireRIRonSubLink, (void *) &context, QTW_IGNORE_RC_SUBQUERIES); + /* + * Make sure the query is marked as having row security if any of its + * sublinks do. + */ + parsetree->hasRowSecurity |= context.hasRowSecurity; + } + /* * Apply any row-level security policies. We do this last because it * requires special recursion detection if the new quals have sublink @@ -2266,6 +2310,7 @@ fireRIRrules(Query *parsetree, List *activeRIRs) if (hasSubLinks) { acquireLocksOnSubLinks_context context; + fireRIRonSubLink_context fire_context; /* * Recursively process the new quals, checking for infinite @@ -2296,11 +2341,21 @@ fireRIRrules(Query *parsetree, List *activeRIRs) * Now that we have the locks on anything added by * get_row_security_policies, fire any RIR rules for them. */ + fire_context.activeRIRs = activeRIRs; + fire_context.hasRowSecurity = false; + expression_tree_walker((Node *) securityQuals, - fireRIRonSubLink, (void *) activeRIRs); + fireRIRonSubLink, (void *) &fire_context); expression_tree_walker((Node *) withCheckOptions, - fireRIRonSubLink, (void *) activeRIRs); + fireRIRonSubLink, (void *) &fire_context); + + /* + * We can ignore the value of fire_context.hasRowSecurity + * since we only reach this code in cases where hasRowSecurity + * is already true. + */ + Assert(hasRowSecurity); activeRIRs = list_delete_last(activeRIRs); } diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile index 06a6c1d57c1..671ac7b7bb7 100644 --- a/src/pl/plperl/GNUmakefile +++ b/src/pl/plperl/GNUmakefile @@ -55,11 +55,11 @@ endif # win32 SHLIB_LINK = $(perl_embed_ldflags) -REGRESS_OPTS = --dbname=$(PL_TESTDB) +REGRESS_OPTS = --dbname=$(PL_TESTDB) --dlpath=$(top_builddir)/src/test/regress REGRESS_OPTS += --init-file=init_file REGRESS = plperl_setup plperl plperl_lc plperl_trigger plperl_shared \ plperl_elog plperl_util plperl_init plperlu plperl_array \ - plperl_call plperl_transaction + plperl_call plperl_transaction plperl_env # if Perl can support two interpreters in one backend, # test plperl-and-plperlu cases ifneq ($(PERL),) @@ -130,6 +130,7 @@ submake: clean distclean maintainer-clean: clean-lib rm -f SPI.c Util.c $(OBJS) perlchunks.h plperl_opmask.h + rm -f sql/plperl_env.sql expected/plperl_env.out rm -rf $(pg_regress_clean_files) ifeq ($(PORTNAME), win32) rm -f $(perlwithver).def diff --git a/src/pl/plperl/expected/.gitignore b/src/pl/plperl/expected/.gitignore new file mode 100644 index 00000000000..0cee2ff3f88 --- /dev/null +++ b/src/pl/plperl/expected/.gitignore @@ -0,0 +1 @@ +/plperl_env.out diff --git a/src/pl/plperl/input/plperl_env.source b/src/pl/plperl/input/plperl_env.source new file mode 100644 index 00000000000..49f03acb1dc --- /dev/null +++ b/src/pl/plperl/input/plperl_env.source @@ -0,0 +1,55 @@ +-- +-- Test the environment setting +-- + +CREATE FUNCTION get_environ() + RETURNS text[] + AS '@libdir@/regress@DLSUFFIX@', 'get_environ' + LANGUAGE C STRICT; + +-- fetch the process environment + +CREATE FUNCTION process_env () RETURNS text[] +LANGUAGE plpgsql AS +$$ + +declare + res text[]; + tmp text[]; + f record; +begin + for f in select unnest(get_environ()) as t loop + tmp := regexp_split_to_array(f.t, '='); + if array_length(tmp, 1) = 2 then + res := res || tmp; + end if; + end loop; + return res; +end + +$$; + +-- plperl should not be able to affect the process environment + +DO +$$ + $ENV{TEST_PLPERL_ENV_FOO} = "shouldfail"; + untie %ENV; + $ENV{TEST_PLPERL_ENV_FOO} = "testval"; + my $penv = spi_exec_query("select unnest(process_env()) as pe"); + my %received; + for (my $f = 0; $f < $penv->{processed}; $f += 2) + { + my $k = $penv->{rows}[$f]->{pe}; + my $v = $penv->{rows}[$f+1]->{pe}; + $received{$k} = $v; + } + unless (exists $received{TEST_PLPERL_ENV_FOO}) + { + elog(NOTICE, "environ unaffected") + } + +$$ LANGUAGE plperl; + +-- clean up to simplify cross-version upgrade testing +DROP FUNCTION get_environ(); diff --git a/src/pl/plperl/output/plperl_env.source b/src/pl/plperl/output/plperl_env.source new file mode 100644 index 00000000000..ef75d5d5556 --- /dev/null +++ b/src/pl/plperl/output/plperl_env.source @@ -0,0 +1,51 @@ +-- +-- Test the environment setting +-- +CREATE FUNCTION get_environ() + RETURNS text[] + AS '@libdir@/regress@DLSUFFIX@', 'get_environ' + LANGUAGE C STRICT; +-- fetch the process environment +CREATE FUNCTION process_env () RETURNS text[] +LANGUAGE plpgsql AS +$$ + +declare + res text[]; + tmp text[]; + f record; +begin + for f in select unnest(get_environ()) as t loop + tmp := regexp_split_to_array(f.t, '='); + if array_length(tmp, 1) = 2 then + res := res || tmp; + end if; + end loop; + return res; +end + +$$; +-- plperl should not be able to affect the process environment +DO +$$ + $ENV{TEST_PLPERL_ENV_FOO} = "shouldfail"; + untie %ENV; + $ENV{TEST_PLPERL_ENV_FOO} = "testval"; + my $penv = spi_exec_query("select unnest(process_env()) as pe"); + my %received; + for (my $f = 0; $f < $penv->{processed}; $f += 2) + { + my $k = $penv->{rows}[$f]->{pe}; + my $v = $penv->{rows}[$f+1]->{pe}; + $received{$k} = $v; + } + unless (exists $received{TEST_PLPERL_ENV_FOO}) + { + elog(NOTICE, "environ unaffected") + } + +$$ LANGUAGE plperl; +WARNING: attempted alteration of $ENV{TEST_PLPERL_ENV_FOO} at line 12. +NOTICE: environ unaffected +-- clean up to simplify cross-version upgrade testing +DROP FUNCTION get_environ(); diff --git a/src/pl/plperl/plc_trusted.pl b/src/pl/plperl/plc_trusted.pl index 2ca71e6e12d..eba3877f31d 100644 --- a/src/pl/plperl/plc_trusted.pl +++ b/src/pl/plperl/plc_trusted.pl @@ -30,3 +30,27 @@ package PostgreSQL::InServer::safe; ## no critic (RequireFilenameMatchesPackage) require Carp::Heavy; require warnings; require feature if $] >= 5.010000; + +#<<< protect next line from perltidy so perlcritic annotation works +package PostgreSQL::InServer::WarnEnv; ## no critic (RequireFilenameMatchesPackage) +#>>> + +use strict; +use warnings; +use Tie::Hash; +our @ISA = qw(Tie::StdHash); + +sub STORE { warn "attempted alteration of \$ENV{$_[1]}"; } +sub DELETE { warn "attempted deletion of \$ENV{$_[1]}"; } +sub CLEAR { warn "attempted clearance of ENV hash"; } + +# Remove magic property of %ENV. Changes to this will now not be reflected in +# the process environment. +*main::ENV = {%ENV}; + +# Block %ENV changes from trusted PL/Perl, and warn. We changed %ENV to just a +# normal hash, yet the application may be expecting the usual Perl %ENV +# magic. Blocking and warning avoids silent application breakage. The user can +# untie or otherwise disable this, e.g. if the lost mutation is unimportant +# and modifying the code to stop that mutation would be onerous. +tie %main::ENV, 'PostgreSQL::InServer::WarnEnv', %ENV or die $!; diff --git a/src/pl/plperl/sql/.gitignore b/src/pl/plperl/sql/.gitignore new file mode 100644 index 00000000000..01795fdcd5a --- /dev/null +++ b/src/pl/plperl/sql/.gitignore @@ -0,0 +1 @@ +/plperl_env.sql diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index 2fc722f5c41..e57da544f2b 100644 --- a/src/test/regress/expected/rowsecurity.out +++ b/src/test/regress/expected/rowsecurity.out @@ -4194,6 +4194,84 @@ execute q; --------------+--- (0 rows) +-- make sure RLS dependencies in CTEs are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ with cte as (select * from rls_t) select * from cte $$; +prepare r as select current_user, * from rls_f(); +set role regress_rls_alice; +execute r; + current_user | c +-------------------+------------------ + regress_rls_alice | invisible to bob +(1 row) + +set role regress_rls_bob; +execute r; + current_user | c +--------------+--- +(0 rows) + +-- make sure RLS dependencies in subqueries are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select * from (select * from rls_t) _ $$; +prepare s as select current_user, * from rls_f(); +set role regress_rls_alice; +execute s; + current_user | c +-------------------+------------------ + regress_rls_alice | invisible to bob +(1 row) + +set role regress_rls_bob; +execute s; + current_user | c +--------------+--- +(0 rows) + +-- make sure RLS dependencies in sublinks are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select exists(select * from rls_t)::text $$; +prepare t as select current_user, * from rls_f(); +set role regress_rls_alice; +execute t; + current_user | c +-------------------+------ + regress_rls_alice | true +(1 row) + +set role regress_rls_bob; +execute t; + current_user | c +-----------------+------- + regress_rls_bob | false +(1 row) + +-- make sure RLS dependencies are handled when coercion projections are inserted +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select * from (select array_agg(c) as cs from rls_t) _ group by cs $$; +prepare u as select current_user, * from rls_f(); +set role regress_rls_alice; +execute u; + current_user | c +-------------------+---------------------- + regress_rls_alice | {"invisible to bob"} +(1 row) + +set role regress_rls_bob; +execute u; + current_user | c +-----------------+--- + regress_rls_bob | +(1 row) + RESET ROLE; DROP FUNCTION rls_f(); DROP TABLE rls_t; diff --git a/src/test/regress/expected/rowsecurity_optimizer.out b/src/test/regress/expected/rowsecurity_optimizer.out index 5717d5bf9a7..b6868c48786 100644 --- a/src/test/regress/expected/rowsecurity_optimizer.out +++ b/src/test/regress/expected/rowsecurity_optimizer.out @@ -4726,6 +4726,100 @@ DETAIL: Falling back to Postgres-based planner because GPORCA does not support --------------+--- (0 rows) +-- make sure RLS dependencies in CTEs are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ with cte as (select * from rls_t) select * from cte $$; +prepare r as select current_user, * from rls_f(); +set role regress_rls_alice; +execute r; +INFO: GPORCA failed to produce a plan, falling back to Postgres-based planner +DETAIL: Falling back to Postgres-based planner because GPORCA does not support the following feature: Non-default collation + current_user | c +-------------------+------------------ + regress_rls_alice | invisible to bob +(1 row) + +set role regress_rls_bob; +execute r; +INFO: GPORCA failed to produce a plan, falling back to Postgres-based planner +DETAIL: Falling back to Postgres-based planner because GPORCA does not support the following feature: Non-default collation + current_user | c +--------------+--- +(0 rows) + +-- make sure RLS dependencies in subqueries are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select * from (select * from rls_t) _ $$; +prepare s as select current_user, * from rls_f(); +set role regress_rls_alice; +execute s; +INFO: GPORCA failed to produce a plan, falling back to Postgres-based planner +DETAIL: Falling back to Postgres-based planner because GPORCA does not support the following feature: Non-default collation + current_user | c +-------------------+------------------ + regress_rls_alice | invisible to bob +(1 row) + +set role regress_rls_bob; +execute s; +INFO: GPORCA failed to produce a plan, falling back to Postgres-based planner +DETAIL: Falling back to Postgres-based planner because GPORCA does not support the following feature: Non-default collation + current_user | c +--------------+--- +(0 rows) + +-- make sure RLS dependencies in sublinks are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select exists(select * from rls_t)::text $$; +prepare t as select current_user, * from rls_f(); +set role regress_rls_alice; +execute t; +INFO: GPORCA failed to produce a plan, falling back to Postgres-based planner +DETAIL: Falling back to Postgres-based planner because GPORCA does not support the following feature: Non-default collation + current_user | c +-------------------+------ + regress_rls_alice | true +(1 row) + +set role regress_rls_bob; +execute t; +INFO: GPORCA failed to produce a plan, falling back to Postgres-based planner +DETAIL: Falling back to Postgres-based planner because GPORCA does not support the following feature: Non-default collation + current_user | c +-----------------+------- + regress_rls_bob | false +(1 row) + +-- make sure RLS dependencies are handled when coercion projections are inserted +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select * from (select array_agg(c) as cs from rls_t) _ group by cs $$; +prepare u as select current_user, * from rls_f(); +set role regress_rls_alice; +execute u; +INFO: GPORCA failed to produce a plan, falling back to Postgres-based planner +DETAIL: Falling back to Postgres-based planner because GPORCA does not support the following feature: Non-default collation + current_user | c +-------------------+---------------------- + regress_rls_alice | {"invisible to bob"} +(1 row) + +set role regress_rls_bob; +execute u; +INFO: GPORCA failed to produce a plan, falling back to Postgres-based planner +DETAIL: Falling back to Postgres-based planner because GPORCA does not support the following feature: Non-default collation + current_user | c +-----------------+--- + regress_rls_bob | +(1 row) + RESET ROLE; DROP FUNCTION rls_f(); DROP TABLE rls_t; diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c index d8fb52ebdd2..0a0153e5551 100644 --- a/src/test/regress/regress.c +++ b/src/test/regress/regress.c @@ -39,6 +39,7 @@ #include "parser/parse_coerce.h" #include "port/atomics.h" #include "storage/spin.h" +#include "utils/array.h" #include "utils/builtins.h" #include "utils/geo_decls.h" #include "utils/memutils.h" @@ -628,6 +629,29 @@ make_tuple_indirect(PG_FUNCTION_ARGS) PG_RETURN_POINTER(newtup->t_data); } +PG_FUNCTION_INFO_V1(get_environ); + +Datum +get_environ(PG_FUNCTION_ARGS) +{ + extern char **environ; + int nvals = 0; + ArrayType *result; + Datum *env; + + for (char **s = environ; *s; s++) + nvals++; + + env = palloc(nvals * sizeof(Datum)); + + for (int i = 0; i < nvals; i++) + env[i] = CStringGetTextDatum(environ[i]); + + result = construct_array(env, nvals, TEXTOID, -1, false, TYPALIGN_INT); + + PG_RETURN_POINTER(result); +} + PG_FUNCTION_INFO_V1(regress_setenv); Datum diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index e828368a4e5..225086be62e 100644 --- a/src/test/regress/sql/rowsecurity.sql +++ b/src/test/regress/sql/rowsecurity.sql @@ -1884,6 +1884,50 @@ execute q; set role regress_rls_bob; execute q; +-- make sure RLS dependencies in CTEs are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ with cte as (select * from rls_t) select * from cte $$; +prepare r as select current_user, * from rls_f(); +set role regress_rls_alice; +execute r; +set role regress_rls_bob; +execute r; + +-- make sure RLS dependencies in subqueries are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select * from (select * from rls_t) _ $$; +prepare s as select current_user, * from rls_f(); +set role regress_rls_alice; +execute s; +set role regress_rls_bob; +execute s; + +-- make sure RLS dependencies in sublinks are handled +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select exists(select * from rls_t)::text $$; +prepare t as select current_user, * from rls_f(); +set role regress_rls_alice; +execute t; +set role regress_rls_bob; +execute t; + +-- make sure RLS dependencies are handled when coercion projections are inserted +reset role; +create or replace function rls_f() returns setof rls_t + stable language sql + as $$ select * from (select array_agg(c) as cs from rls_t) _ group by cs $$; +prepare u as select current_user, * from rls_f(); +set role regress_rls_alice; +execute u; +set role regress_rls_bob; +execute u; + RESET ROLE; DROP FUNCTION rls_f(); DROP TABLE rls_t; diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl index d4e7d69df2e..f19b0a4f755 100644 --- a/src/tools/msvc/vcregress.pl +++ b/src/tools/msvc/vcregress.pl @@ -458,13 +458,15 @@ sub plcheck # Move on if no tests are listed. next if (scalar @tests == 0); + my @opts = fetchRegressOpts(); + print "============================================================\n"; print "Checking $lang\n"; my @args = ( "$topdir/$Config/pg_regress/pg_regress", "--bindir=$topdir/$Config/psql", - "--dbname=pl_regression", @lang_args, @tests); + "--dbname=pl_regression", @lang_args, @opts, @tests); system(@args); my $status = $? >> 8; exit $status if $status; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 2eb501b6e74..fe36db36936 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -3133,6 +3133,7 @@ fill_string_relopt finalize_primnode_context find_dependent_phvs_context find_expr_references_context +fireRIRonSubLink_context fix_join_expr_context fix_scan_expr_context fix_upper_expr_context