On Tue, 2023-09-19 at 20:23 -0700, Maciek Sakrejda wrote:
> On Tue, Sep 19, 2023 at 5:56 PM Jeff Davis <pg...@j-davis.com> wrote:
> > ...
> > On Tue, 2023-09-19 at 11:41 -0400, Robert Haas wrote:
> > > That leads to a second idea, which is having it continue
> > > to be a GUC but only affect directly-entered SQL, with all
> > > indirectly-entered SQL either being stored as a node tree or
> > > having a
> > > search_path property attached somewhere.
> >
> > That's not too far from the proposed patch and I'd certainly be
> > interested to hear more and/or adapt my patch towards this idea.
>
> As an interested bystander, that's the same thing I was thinking when
> reading this. I reread your original e-mail, Jeff, and I still think
> that.
I have attached an updated patch. Changes:
* Syntax is now: SEARCH FROM { DEFAULT | TRUSTED | SESSION }
- added "FROM" to suggest that it's the source, and only a starting
place, rather than a specific and final setting. I don't feel strongly
about the FROM one way or another, so I can take it out if it's not
helpful.
- changed "SYSTEM" to "TRUSTED", which better reflects the purpose,
and doesn't suggest any connection to ALTER SYSTEM.
* Removed GUC -- we can reconsider this kind of thing later.
* ERROR if IMMUTABLE is combined with SEARCH FROM SESSION
* pg_dump support. Emits "SEARCH FROM SESSION" or "SEARCH FROM
TRUSTED" only if explicitly specified; otherwise emits no SEARCH
clause. Differentiating the unspecified cases may be useful for
migration purposes later.
* psql support.
* Updated docs to try to better present the concept, and document
CREATE PROCEDURE as well.
The SEARCH clause declares a new property that will be useful to both
enforce safety and also to guide users to migrate in a safe direction
over time.
For instance, the current patch prohibits the combination of IMMUTABLE
and SEARCH FROM SESSION; but allows IMMUTABLE if no SEARCH clause is
specified at all (to avoid breaking anything). We could extend that
slowly over several releases ratchet up the pressure (with warnings or
changing defaults) until all IMMUTABLE functions require SEARCH FROM
TRUSTED. Perhaps IMMUTABLE would even imply SEARCH FROM TRUSTED.
The search property is consistent with other properties, like
IMMUTABLE, which is both a marker and also enforces some restrictions
(e.g. you can't CREATE TABLE). It's also a lot nicer to use than a SET
clause, and provides a nice place to document certain behaviors.
(Aside: the concept of IMMUTABLE is basically broken today, due to
search_path problems.)
SEARCH FROM DEFAULT is just a way to get an object back to the
"unspecified search clause" state. It has the same behavior as SEARCH
FROM SESSION, except that the former will cause a hard error when
combined with IMMUTABLE. I think it's worth differentiating the
unspecified search clause from the explicit SEARCH FROM SESSION clause
for the purposes of migration.
There were three main complaints:
Comaplaint A: That it creates a new mechanism[1].
The patch doesn't create a new internal mechanism, it almost entirely
reuses the existing SET clause mechanism. I think complaint A is really
about the user-facing mechanics, which is essentially the same as the
complaint B.
Complaint B: That it's overlapping in functionality with the SET
clause[2][3]. In other words:
CREATE FUNCTION ... SEARCH FROM TRUSTED ...;
CREATE FUNCTION ... SET search_path = pg_catalog, pg_temp ...;
do similar things. But the latter is much worse:
* it's user-unfriendly (requiring pg_temp is highly unintuitive)
* it doesn't allow Postgres to warn if the function is being used in
an unsafe context
* there's no way to explicitly declare that you want the search path
to come from the session instead (either to be more clear about your
intentions, or to be forward-compatible)
In my opinion, the "SET search_path = ..." form should be used when you
actually want the search_path to contain some specific schema, not in
cases where you're just using built-in objects.
Complaint C: search_path is hopeless[4].
I think we can make meaningful improvements to the status quo, like
with the attached patch, that will reduce a lot of the surface area for
security risks. Right now our privilege model breaks down very quickly
even with trivial and typical use cases and we can do better.
--
Jeff Davis
PostgreSQL Contributor Team - AWS
[1]
https://www.postgresql.org/message-id/CA%2BTgmoaRPJJN%3DAOqC4b9t90vFQX81hKiXNPNhbxR0-Sm8F8nCA%40mail.gmail.com
[2]
https://www.postgresql.org/message-id/CA%2BTgmoah_bTjUFng-vZnivPQs0kQWUaSwAu49SU5M%2BzTxA%2B3Qw%40mail.gmail.com
[3]
https://www.postgresql.org/message-id/15464811-18fb-c7d4-4620-873366d367d6%40eisentraut.org
[4]
https://www.postgresql.org/message-id/20230812182559.d7plqwx3p65ys4i7%40awork3.anarazel.de
From 1cdb1e604535e7e0e1c3a1d8c9d38d99c9d42ba3 Mon Sep 17 00:00:00 2001
From: Jeff Davis <j...@j-davis.com>
Date: Fri, 11 Aug 2023 14:33:36 -0700
Subject: [PATCH v2] CREATE FUNCTION ... SEARCH FROM { DEFAULT | TRUSTED |
SESSION }.
Declare how search_path will be initialized before execution of a
function or procedure. SEARCH FROM TRUSTED causes the search_path to
be initialized to a safe value that includes built-in objects. SEARCH
FROM SESSION causes the search_path to be inherited from the current
setting in the session (or current setting in the calling function or
procedure).
The SEARCH clause specifies a new function property that may be used
in the future to check for safe usage of functions in, e.g., index
expressions.
Discussion: https://postgr.es/m/2710f56add351a1ed553efb677408e51b060e67c.camel%40j-davis.com
---
doc/src/sgml/ref/alter_function.sgml | 15 +++++
doc/src/sgml/ref/alter_procedure.sgml | 15 +++++
doc/src/sgml/ref/alter_routine.sgml | 1 +
doc/src/sgml/ref/create_function.sgml | 55 +++++++++++++++
doc/src/sgml/ref/create_procedure.sgml | 55 +++++++++++++++
src/backend/catalog/pg_aggregate.c | 1 +
src/backend/catalog/pg_proc.c | 2 +
src/backend/commands/functioncmds.c | 56 +++++++++++++++-
src/backend/commands/typecmds.c | 4 ++
src/backend/optimizer/util/clauses.c | 1 +
src/backend/parser/gram.y | 15 +++++
src/backend/utils/fmgr/fmgr.c | 23 +++++--
src/bin/pg_dump/pg_dump.c | 18 ++++-
src/bin/psql/describe.c | 23 ++++++-
src/include/catalog/namespace.h | 6 ++
src/include/catalog/pg_proc.h | 13 ++++
.../regress/expected/create_function_sql.out | 67 +++++++++++++++++++
src/test/regress/expected/psql.out | 12 ++--
src/test/regress/sql/create_function_sql.sql | 44 ++++++++++++
19 files changed, 407 insertions(+), 19 deletions(-)
diff --git a/doc/src/sgml/ref/alter_function.sgml b/doc/src/sgml/ref/alter_function.sgml
index 8193b17f25..b4103c0c90 100644
--- a/doc/src/sgml/ref/alter_function.sgml
+++ b/doc/src/sgml/ref/alter_function.sgml
@@ -37,6 +37,7 @@ ALTER FUNCTION <replaceable>name</replaceable> [ ( [ [ <replaceable class="param
CALLED ON NULL INPUT | RETURNS NULL ON NULL INPUT | STRICT
IMMUTABLE | STABLE | VOLATILE
[ NOT ] LEAKPROOF
+ SEARCH FROM { DEFAULT | TRUSTED | SESSION }
[ EXTERNAL ] SECURITY INVOKER | [ EXTERNAL ] SECURITY DEFINER
PARALLEL { UNSAFE | RESTRICTED | SAFE }
COST <replaceable class="parameter">execution_cost</replaceable>
@@ -198,6 +199,20 @@ ALTER FUNCTION <replaceable>name</replaceable> [ ( [ [ <replaceable class="param
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>SEARCH FROM DEFAULT</literal></term>
+ <term><literal>SEARCH FROM TRUSTED</literal></term>
+ <term><literal>SEARCH FROM SESSION</literal></term>
+
+ <listitem>
+ <para>
+ Change the <literal>SEARCH</literal> behavior of the function to the
+ specified setting. See <xref linkend="sql-createfunction"/> for
+ details.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal><optional> EXTERNAL </optional> SECURITY INVOKER</literal></term>
<term><literal><optional> EXTERNAL </optional> SECURITY DEFINER</literal></term>
diff --git a/doc/src/sgml/ref/alter_procedure.sgml b/doc/src/sgml/ref/alter_procedure.sgml
index a4737a3439..63bc6d695b 100644
--- a/doc/src/sgml/ref/alter_procedure.sgml
+++ b/doc/src/sgml/ref/alter_procedure.sgml
@@ -34,6 +34,7 @@ ALTER PROCEDURE <replaceable>name</replaceable> [ ( [ [ <replaceable class="para
<phrase>where <replaceable class="parameter">action</replaceable> is one of:</phrase>
+ SEARCH FROM { DEFAULT | TRUSTED | SESSION }
[ EXTERNAL ] SECURITY INVOKER | [ EXTERNAL ] SECURITY DEFINER
SET <replaceable class="parameter">configuration_parameter</replaceable> { TO | = } { <replaceable class="parameter">value</replaceable> | DEFAULT }
SET <replaceable class="parameter">configuration_parameter</replaceable> FROM CURRENT
@@ -158,6 +159,20 @@ ALTER PROCEDURE <replaceable>name</replaceable> [ ( [ [ <replaceable class="para
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>SEARCH FROM DEFAULT</literal></term>
+ <term><literal>SEARCH FROM TRUSTED</literal></term>
+ <term><literal>SEARCH FROM SESSION</literal></term>
+
+ <listitem>
+ <para>
+ Change the <literal>SEARCH</literal> behavior of the function to the
+ specified setting. See <xref linkend="sql-createprocedure"/> for
+ details.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal><optional> EXTERNAL </optional> SECURITY INVOKER</literal></term>
<term><literal><optional> EXTERNAL </optional> SECURITY DEFINER</literal></term>
diff --git a/doc/src/sgml/ref/alter_routine.sgml b/doc/src/sgml/ref/alter_routine.sgml
index d6c9dea2eb..f8575b6cd0 100644
--- a/doc/src/sgml/ref/alter_routine.sgml
+++ b/doc/src/sgml/ref/alter_routine.sgml
@@ -36,6 +36,7 @@ ALTER ROUTINE <replaceable>name</replaceable> [ ( [ [ <replaceable class="parame
IMMUTABLE | STABLE | VOLATILE
[ NOT ] LEAKPROOF
+ SEARCH FROM { DEFAULT | TRUSTED | SESSION }
[ EXTERNAL ] SECURITY INVOKER | [ EXTERNAL ] SECURITY DEFINER
PARALLEL { UNSAFE | RESTRICTED | SAFE }
COST <replaceable class="parameter">execution_cost</replaceable>
diff --git a/doc/src/sgml/ref/create_function.sgml b/doc/src/sgml/ref/create_function.sgml
index 863d99d1fc..0909bd9c06 100644
--- a/doc/src/sgml/ref/create_function.sgml
+++ b/doc/src/sgml/ref/create_function.sgml
@@ -31,6 +31,7 @@ CREATE [ OR REPLACE ] FUNCTION
| { IMMUTABLE | STABLE | VOLATILE }
| [ NOT ] LEAKPROOF
| { CALLED ON NULL INPUT | RETURNS NULL ON NULL INPUT | STRICT }
+ | SEARCH FROM { DEFAULT | TRUSTED | SESSION }
| { [ EXTERNAL ] SECURITY INVOKER | [ EXTERNAL ] SECURITY DEFINER }
| PARALLEL { UNSAFE | RESTRICTED | SAFE }
| COST <replaceable class="parameter">execution_cost</replaceable>
@@ -402,6 +403,60 @@ CREATE [ OR REPLACE ] FUNCTION
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>SEARCH FROM DEFAULT</literal></term>
+ <term><literal>SEARCH FROM TRUSTED</literal></term>
+ <term><literal>SEARCH FROM SESSION</literal></term>
+
+ <listitem>
+ <para>
+ Declare how <xref linkend="guc-search-path"/> will be initialized
+ before execution. <literal>SEARCH FROM TRUSTED</literal> causes the
+ search_path to be initialized to a safe value that includes built-in
+ objects. <literal>SEARCH FROM SESSION</literal> causes the search_path
+ to be inherited from the current setting in the session (or
+ current setting in the calling function or procedure).
+ </para>
+ <warning>
+ <para>
+ If <literal>search_path</literal> is inherited from the session, the
+ caller of the function or procedure may be able to manipulate its
+ behavior.
+ </para>
+ </warning>
+ <para>
+ Regardless of the <literal>SEARCH</literal> clause, the
+ <literal>search_path</literal> may be changed later just like any other
+ GUC, e.g. by using a <literal>SET</literal> clause in the declaration
+ or a <xref linkend="sql-set"/> command during execution.
+ </para>
+ <para>
+ If the function or procedure is specified with
+ <replaceable>sql_body</replaceable>, the <literal>SEARCH</literal>
+ clause does not affect the <literal>search_path</literal> used to find
+ objects referenced in the <replaceable>sql_body</replaceable> because
+ the it's parsed at definition time.
+ </para>
+ <para>
+ <literal>SEARCH FROM DEFAULT</literal> is the same as not specifying
+ any <literal>SEARCH</literal> clause at all, and the behavior is
+ equivalent to <literal>SEARCH FROM SESSION</literal>.
+ </para>
+ <note>
+ <para>
+ The <literal>search_path</literal> setting is important for functions
+ or procedures that parse statements at execution time (such as
+ PL/pgSQL, SQL specified with a <replaceable>definition</replaceable>,
+ or those that use the <link linkend="spi">Server Programming
+ Interface</link>), or those that call other functions or procedures
+ that parse statements at execution time.
+ </para>
+ <para>
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal><optional>EXTERNAL</optional> SECURITY INVOKER</literal></term>
<term><literal><optional>EXTERNAL</optional> SECURITY DEFINER</literal></term>
diff --git a/doc/src/sgml/ref/create_procedure.sgml b/doc/src/sgml/ref/create_procedure.sgml
index 03a14c8684..c1c63f0527 100644
--- a/doc/src/sgml/ref/create_procedure.sgml
+++ b/doc/src/sgml/ref/create_procedure.sgml
@@ -25,6 +25,7 @@ CREATE [ OR REPLACE ] PROCEDURE
<replaceable class="parameter">name</replaceable> ( [ [ <replaceable class="parameter">argmode</replaceable> ] [ <replaceable class="parameter">argname</replaceable> ] <replaceable class="parameter">argtype</replaceable> [ { DEFAULT | = } <replaceable class="parameter">default_expr</replaceable> ] [, ...] ] )
{ LANGUAGE <replaceable class="parameter">lang_name</replaceable>
| TRANSFORM { FOR TYPE <replaceable class="parameter">type_name</replaceable> } [, ... ]
+ | SEARCH FROM { DEFAULT | TRUSTED | SESSION }
| [ EXTERNAL ] SECURITY INVOKER | [ EXTERNAL ] SECURITY DEFINER
| SET <replaceable class="parameter">configuration_parameter</replaceable> { TO <replaceable class="parameter">value</replaceable> | = <replaceable class="parameter">value</replaceable> | FROM CURRENT }
| AS '<replaceable class="parameter">definition</replaceable>'
@@ -193,6 +194,60 @@ CREATE [ OR REPLACE ] PROCEDURE
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>SEARCH FROM DEFAULT</literal></term>
+ <term><literal>SEARCH FROM TRUSTED</literal></term>
+ <term><literal>SEARCH FROM SESSION</literal></term>
+
+ <listitem>
+ <para>
+ Declare how <xref linkend="guc-search-path"/> will be initialized
+ before execution. <literal>SEARCH FROM TRUSTED</literal> causes the
+ search_path to be initialized to a safe value that includes built-in
+ objects. <literal>SEARCH FROM SESSION</literal> causes the search_path
+ to be inherited from the current setting in the session (or
+ current setting in the calling function or procedure).
+ </para>
+ <warning>
+ <para>
+ If <literal>search_path</literal> is inherited from the session, the
+ caller of the function or procedure may be able to manipulate its
+ behavior.
+ </para>
+ </warning>
+ <para>
+ Regardless of the <literal>SEARCH</literal> clause, the
+ <literal>search_path</literal> may be changed later just like any other
+ GUC, e.g. by using a <literal>SET</literal> clause in the declaration
+ or a <xref linkend="sql-set"/> command during execution.
+ </para>
+ <para>
+ If the function or procedure is specified with
+ <replaceable>sql_body</replaceable>, the <literal>SEARCH</literal>
+ clause does not affect the <literal>search_path</literal> used to find
+ objects referenced in the <replaceable>sql_body</replaceable> because
+ the it's parsed at definition time.
+ </para>
+ <para>
+ <literal>SEARCH FROM DEFAULT</literal> is the same as not specifying
+ any <literal>SEARCH</literal> clause at all, and the behavior is
+ equivalent to <literal>SEARCH FROM SESSION</literal>.
+ </para>
+ <note>
+ <para>
+ The <literal>search_path</literal> setting is important for functions
+ or procedures that parse statements at execution time (such as
+ PL/pgSQL, SQL specified with a <replaceable>definition</replaceable>,
+ or those that use the <link linkend="spi">Server Programming
+ Interface</link>), or those that call other functions or procedures
+ that parse statements at execution time.
+ </para>
+ <para>
+ </para>
+ </note>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal><optional>EXTERNAL</optional> SECURITY INVOKER</literal></term>
<term><literal><optional>EXTERNAL</optional> SECURITY DEFINER</literal></term>
diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c
index ebc4454743..e4ec68ec43 100644
--- a/src/backend/catalog/pg_aggregate.c
+++ b/src/backend/catalog/pg_aggregate.c
@@ -624,6 +624,7 @@ AggregateCreate(const char *aggName,
NULL, /* probin */
NULL, /* prosqlbody */
PROKIND_AGGREGATE,
+ PROSEARCH_DEFAULT, /* no SEARCH clause for aggregates */
false, /* security invoker (currently not
* definable for agg) */
false, /* isLeakProof */
diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c
index b5fd364003..8a252acd99 100644
--- a/src/backend/catalog/pg_proc.c
+++ b/src/backend/catalog/pg_proc.c
@@ -81,6 +81,7 @@ ProcedureCreate(const char *procedureName,
const char *probin,
Node *prosqlbody,
char prokind,
+ char prosearch,
bool security_definer,
bool isLeakProof,
bool isStrict,
@@ -308,6 +309,7 @@ ProcedureCreate(const char *procedureName,
values[Anum_pg_proc_provariadic - 1] = ObjectIdGetDatum(variadicType);
values[Anum_pg_proc_prosupport - 1] = ObjectIdGetDatum(prosupport);
values[Anum_pg_proc_prokind - 1] = CharGetDatum(prokind);
+ values[Anum_pg_proc_prosearch - 1] = CharGetDatum(prosearch);
values[Anum_pg_proc_prosecdef - 1] = BoolGetDatum(security_definer);
values[Anum_pg_proc_proleakproof - 1] = BoolGetDatum(isLeakProof);
values[Anum_pg_proc_proisstrict - 1] = BoolGetDatum(isStrict);
diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c
index 7ba6a86ebe..1f086b84a5 100644
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -507,6 +507,7 @@ compute_common_attribute(ParseState *pstate,
DefElem *defel,
DefElem **volatility_item,
DefElem **strict_item,
+ DefElem **search_item,
DefElem **security_item,
DefElem **leakproof_item,
List **set_items,
@@ -533,6 +534,13 @@ compute_common_attribute(ParseState *pstate,
*strict_item = defel;
}
+ else if (strcmp(defel->defname, "search") == 0)
+ {
+ if (*search_item)
+ errorConflictingDefElem(defel, pstate);
+
+ *search_item = defel;
+ }
else if (strcmp(defel->defname, "security") == 0)
{
if (*security_item)
@@ -603,6 +611,24 @@ procedure_error:
return false;
}
+static char
+interpret_func_search(DefElem *defel)
+{
+ char *str = strVal(defel->arg);
+
+ if (strcmp(str, "default") == 0)
+ return PROSEARCH_DEFAULT;
+ else if (strcmp(str, "trusted") == 0)
+ return PROSEARCH_TRUSTED;
+ else if (strcmp(str, "session") == 0)
+ return PROSEARCH_SESSION;
+ else
+ {
+ elog(ERROR, "invalid search \"%s\"", str);
+ return 0; /* keep compiler quiet */
+ }
+}
+
static char
interpret_func_volatility(DefElem *defel)
{
@@ -725,6 +751,7 @@ compute_function_attributes(ParseState *pstate,
bool *windowfunc_p,
char *volatility_p,
bool *strict_p,
+ char *search_p,
bool *security_definer,
bool *leakproof_p,
ArrayType **proconfig,
@@ -740,6 +767,7 @@ compute_function_attributes(ParseState *pstate,
DefElem *windowfunc_item = NULL;
DefElem *volatility_item = NULL;
DefElem *strict_item = NULL;
+ DefElem *search_item = NULL;
DefElem *security_item = NULL;
DefElem *leakproof_item = NULL;
List *set_items = NIL;
@@ -786,6 +814,7 @@ compute_function_attributes(ParseState *pstate,
defel,
&volatility_item,
&strict_item,
+ &search_item,
&security_item,
&leakproof_item,
&set_items,
@@ -814,6 +843,8 @@ compute_function_attributes(ParseState *pstate,
*volatility_p = interpret_func_volatility(volatility_item);
if (strict_item)
*strict_p = boolVal(strict_item->arg);
+ if (search_item)
+ *search_p = interpret_func_search(search_item);
if (security_item)
*security_definer = boolVal(security_item->arg);
if (leakproof_item)
@@ -1042,7 +1073,8 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
isStrict,
security,
isLeakProof;
- char volatility;
+ char prosearch,
+ volatility;
ArrayType *proconfig;
float4 procost;
float4 prorows;
@@ -1067,6 +1099,7 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
language = NULL;
isWindowFunc = false;
isStrict = false;
+ prosearch = PROSEARCH_DEFAULT;
security = false;
isLeakProof = false;
volatility = PROVOLATILE_VOLATILE;
@@ -1082,10 +1115,16 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
stmt->options,
&as_clause, &language, &transformDefElem,
&isWindowFunc, &volatility,
- &isStrict, &security, &isLeakProof,
- &proconfig, &procost, &prorows,
+ &isStrict, &prosearch, &security,
+ &isLeakProof, &proconfig, &procost, &prorows,
&prosupport, ¶llel);
+ if (volatility == PROVOLATILE_IMMUTABLE && prosearch == PROSEARCH_SESSION)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("immutable functions cannot be specified with SEARCH FROM SESSION"),
+ errhint("Specify SEARCH FROM TRUSTED instead.")));
+
if (!language)
{
if (stmt->sql_body)
@@ -1271,6 +1310,7 @@ CreateFunction(ParseState *pstate, CreateFunctionStmt *stmt)
probin_str, /* converted to text later */
prosqlbody,
stmt->is_procedure ? PROKIND_PROCEDURE : (isWindowFunc ? PROKIND_WINDOW : PROKIND_FUNCTION),
+ prosearch,
security,
isLeakProof,
isStrict,
@@ -1355,6 +1395,7 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
ListCell *l;
DefElem *volatility_item = NULL;
DefElem *strict_item = NULL;
+ DefElem *search_item = NULL;
DefElem *security_def_item = NULL;
DefElem *leakproof_item = NULL;
List *set_items = NIL;
@@ -1399,6 +1440,7 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
defel,
&volatility_item,
&strict_item,
+ &search_item,
&security_def_item,
&leakproof_item,
&set_items,
@@ -1413,8 +1455,16 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt)
procForm->provolatile = interpret_func_volatility(volatility_item);
if (strict_item)
procForm->proisstrict = boolVal(strict_item->arg);
+ if (search_item)
+ procForm->prosearch = interpret_func_search(search_item);
if (security_def_item)
procForm->prosecdef = boolVal(security_def_item->arg);
+ if (procForm->provolatile == PROVOLATILE_IMMUTABLE &&
+ procForm->prosearch == PROSEARCH_SESSION)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("immutable functions cannot be specified with SEARCH FROM SESSION"),
+ errhint("Specify SEARCH FROM TRUSTED instead.")));
if (leakproof_item)
{
procForm->proleakproof = boolVal(leakproof_item->arg);
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 5e97606793..baa8dd706e 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -1763,6 +1763,7 @@ makeRangeConstructors(const char *name, Oid namespace,
NULL, /* probin */
NULL, /* prosqlbody */
PROKIND_FUNCTION,
+ PROSEARCH_DEFAULT,
false, /* security_definer */
false, /* leakproof */
false, /* isStrict */
@@ -1828,6 +1829,7 @@ makeMultirangeConstructors(const char *name, Oid namespace,
NULL, /* probin */
NULL, /* prosqlbody */
PROKIND_FUNCTION,
+ PROSEARCH_DEFAULT,
false, /* security_definer */
false, /* leakproof */
true, /* isStrict */
@@ -1872,6 +1874,7 @@ makeMultirangeConstructors(const char *name, Oid namespace,
NULL, /* probin */
NULL, /* prosqlbody */
PROKIND_FUNCTION,
+ PROSEARCH_DEFAULT,
false, /* security_definer */
false, /* leakproof */
true, /* isStrict */
@@ -1910,6 +1913,7 @@ makeMultirangeConstructors(const char *name, Oid namespace,
NULL, /* probin */
NULL, /* prosqlbody */
PROKIND_FUNCTION,
+ PROSEARCH_DEFAULT,
false, /* security_definer */
false, /* leakproof */
true, /* isStrict */
diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c
index da258968b8..5a843d71ae 100644
--- a/src/backend/optimizer/util/clauses.c
+++ b/src/backend/optimizer/util/clauses.c
@@ -4474,6 +4474,7 @@ inline_function(Oid funcid, Oid result_type, Oid result_collid,
*/
if (funcform->prolang != SQLlanguageId ||
funcform->prokind != PROKIND_FUNCTION ||
+ funcform->prosearch == PROSEARCH_TRUSTED ||
funcform->prosecdef ||
funcform->proretset ||
funcform->prorettype == RECORDOID ||
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 7d2032885e..ad6d09cc47 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -8545,6 +8545,21 @@ common_func_opt_item:
{
$$ = makeDefElem("support", (Node *) $2, @1);
}
+ | SEARCH FROM DEFAULT
+ {
+ /* we abuse the normal content of a DefElem here */
+ $$ = makeDefElem("search", (Node *) makeString("default"), @1);
+ }
+ | SEARCH FROM TRUSTED
+ {
+ /* we abuse the normal content of a DefElem here */
+ $$ = makeDefElem("search", (Node *) makeString("trusted"), @1);
+ }
+ | SEARCH FROM SESSION
+ {
+ /* we abuse the normal content of a DefElem here */
+ $$ = makeDefElem("search", (Node *) makeString("session"), @1);
+ }
| FunctionSetResetClause
{
/* we abuse the normal content of a DefElem here */
diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c
index 9dfdf890c5..d65d88fc31 100644
--- a/src/backend/utils/fmgr/fmgr.c
+++ b/src/backend/utils/fmgr/fmgr.c
@@ -16,6 +16,7 @@
#include "postgres.h"
#include "access/detoast.h"
+#include "catalog/namespace.h"
#include "catalog/pg_language.h"
#include "catalog/pg_proc.h"
#include "catalog/pg_type.h"
@@ -203,6 +204,7 @@ fmgr_info_cxt_security(Oid functionId, FmgrInfo *finfo, MemoryContext mcxt,
*/
if (!ignore_security &&
(procedureStruct->prosecdef ||
+ procedureStruct->prosearch == PROSEARCH_TRUSTED ||
!heap_attisnull(procedureTuple, Anum_pg_proc_proconfig, NULL) ||
FmgrHookIsNeeded(functionId)))
{
@@ -612,6 +614,7 @@ struct fmgr_security_definer_cache
{
FmgrInfo flinfo; /* lookup info for target function */
Oid userid; /* userid to set, or InvalidOid */
+ char *searchPath; /* from SEARCH clause, if specified */
List *configNames; /* GUC names to set, or NIL */
List *configValues; /* GUC values to set, or NIL */
Datum arg; /* passthrough argument for plugin modules */
@@ -630,6 +633,9 @@ struct fmgr_security_definer_cache
extern Datum
fmgr_security_definer(PG_FUNCTION_ARGS)
{
+ GucContext context = superuser() ? PGC_SUSET : PGC_USERSET;
+ GucSource source = PGC_S_SESSION;
+ GucAction action = GUC_ACTION_SAVE;
Datum result;
struct fmgr_security_definer_cache *volatile fcache;
FmgrInfo *save_flinfo;
@@ -662,6 +668,9 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
fcinfo->flinfo->fn_oid);
procedureStruct = (Form_pg_proc) GETSTRUCT(tuple);
+ if (procedureStruct->prosearch == PROSEARCH_TRUSTED)
+ fcache->searchPath = NAMESPACE_TRUSTED_SEARCH_PATH;
+
if (procedureStruct->prosecdef)
fcache->userid = procedureStruct->proowner;
@@ -687,20 +696,22 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
/* GetUserIdAndSecContext is cheap enough that no harm in a wasted call */
GetUserIdAndSecContext(&save_userid, &save_sec_context);
- if (fcache->configNames != NIL) /* Need a new GUC nesting level */
+ if (fcache->searchPath != NULL || fcache->configNames != NIL) /* Need a new GUC nesting level */
save_nestlevel = NewGUCNestLevel();
else
- save_nestlevel = 0; /* keep compiler quiet */
+ save_nestlevel = 0;
if (OidIsValid(fcache->userid))
SetUserIdAndSecContext(fcache->userid,
save_sec_context | SECURITY_LOCAL_USERID_CHANGE);
+ if (fcache->searchPath != NULL)
+ (void) set_config_option("search_path", fcache->searchPath,
+ context, source,
+ action, true, 0, false);
+
forboth(lc1, fcache->configNames, lc2, fcache->configValues)
{
- GucContext context = superuser() ? PGC_SUSET : PGC_USERSET;
- GucSource source = PGC_S_SESSION;
- GucAction action = GUC_ACTION_SAVE;
char *name = lfirst(lc1);
char *value = lfirst(lc2);
@@ -749,7 +760,7 @@ fmgr_security_definer(PG_FUNCTION_ARGS)
fcinfo->flinfo = save_flinfo;
- if (fcache->configNames != NIL)
+ if (save_nestlevel > 0)
AtEOXact_GUC(true, save_nestlevel);
if (OidIsValid(fcache->userid))
SetUserIdAndSecContext(save_userid, save_sec_context);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index f7b6176692..184508052f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -12005,6 +12005,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
char *prokind;
char *provolatile;
char *proisstrict;
+ char *prosearch;
char *prosecdef;
char *proleakproof;
char *proconfig;
@@ -12079,10 +12080,17 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
if (fout->remoteVersion >= 140000)
appendPQExpBufferStr(query,
- "pg_get_function_sqlbody(p.oid) AS prosqlbody\n");
+ "pg_get_function_sqlbody(p.oid) AS prosqlbody,\n");
else
appendPQExpBufferStr(query,
- "NULL AS prosqlbody\n");
+ "NULL AS prosqlbody,\n");
+
+ if (fout->remoteVersion >= 170000)
+ appendPQExpBufferStr(query,
+ "prosearch\n");
+ else
+ appendPQExpBufferStr(query,
+ "NULL AS prosearch\n");
appendPQExpBufferStr(query,
"FROM pg_catalog.pg_proc p, pg_catalog.pg_language l\n"
@@ -12120,6 +12128,7 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
prokind = PQgetvalue(res, 0, PQfnumber(res, "prokind"));
provolatile = PQgetvalue(res, 0, PQfnumber(res, "provolatile"));
proisstrict = PQgetvalue(res, 0, PQfnumber(res, "proisstrict"));
+ prosearch = PQgetvalue(res, 0, PQfnumber(res, "prosearch"));
prosecdef = PQgetvalue(res, 0, PQfnumber(res, "prosecdef"));
proleakproof = PQgetvalue(res, 0, PQfnumber(res, "proleakproof"));
proconfig = PQgetvalue(res, 0, PQfnumber(res, "proconfig"));
@@ -12245,6 +12254,11 @@ dumpFunc(Archive *fout, const FuncInfo *finfo)
if (proisstrict[0] == 't')
appendPQExpBufferStr(q, " STRICT");
+ if (prosearch[0] == 't')
+ appendPQExpBufferStr(q, " SEARCH FROM TRUSTED");
+ else if (prosearch[0] == 's')
+ appendPQExpBufferStr(q, " SEARCH FROM SESSION");
+
if (prosecdef[0] == 't')
appendPQExpBufferStr(q, " SECURITY DEFINER");
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index bac94a338c..a2b950382b 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -301,7 +301,10 @@ describeFunctions(const char *functypes, const char *func_pattern,
PQExpBufferData buf;
PGresult *res;
printQueryOpt myopt = pset.popt;
- static const bool translate_columns[] = {false, false, false, false, true, true, true, false, true, false, false, false, false};
+ static const bool translate_columns[] = {false, false, false, false, true, true, true, true, false, true, false, false, false, false};
+
+ /* No "Search" column before 17 */
+ static const bool translate_columns_pre_17[] = {false, false, false, false, true, true, true, false, true, false, false, false, false};
/* No "Parallel" column before 9.6 */
static const bool translate_columns_pre_96[] = {false, false, false, false, true, true, false, true, false, false, false, false};
@@ -377,6 +380,17 @@ describeFunctions(const char *functypes, const char *func_pattern,
if (verbose)
{
+ if (pset.sversion >= 170000)
+ appendPQExpBuffer(&buf,
+ ",\n CASE\n"
+ " WHEN p.prosearch = 'd' THEN '%s'\n"
+ " WHEN p.prosearch = 't' THEN '%s'\n"
+ " WHEN p.prosearch = 's' THEN '%s'\n"
+ " END as \"%s\"",
+ gettext_noop("default"),
+ gettext_noop("trusted"),
+ gettext_noop("session"),
+ gettext_noop("Search"));
appendPQExpBuffer(&buf,
",\n CASE\n"
" WHEN p.provolatile = 'i' THEN '%s'\n"
@@ -588,11 +602,16 @@ describeFunctions(const char *functypes, const char *func_pattern,
myopt.nullPrint = NULL;
myopt.title = _("List of functions");
myopt.translate_header = true;
- if (pset.sversion >= 90600)
+ if (pset.sversion >= 170000)
{
myopt.translate_columns = translate_columns;
myopt.n_translate_columns = lengthof(translate_columns);
}
+ else if (pset.sversion >= 90600)
+ {
+ myopt.translate_columns = translate_columns_pre_17;
+ myopt.n_translate_columns = lengthof(translate_columns_pre_17);
+ }
else
{
myopt.translate_columns = translate_columns_pre_96;
diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h
index e027940430..c1c313eec6 100644
--- a/src/include/catalog/namespace.h
+++ b/src/include/catalog/namespace.h
@@ -76,6 +76,12 @@ typedef enum RVROption
typedef void (*RangeVarGetRelidCallback) (const RangeVar *relation, Oid relId,
Oid oldRelId, void *callback_arg);
+/*
+ * Trusted search_path for cases where relying on the session search_path is
+ * unsafe (e.g. for SECURITY DEFINER functions).
+ */
+#define NAMESPACE_TRUSTED_SEARCH_PATH "pg_catalog, pg_temp"
+
#define RangeVarGetRelid(relation, lockmode, missing_ok) \
RangeVarGetRelidExtended(relation, lockmode, \
(missing_ok) ? RVR_MISSING_OK : 0, NULL, NULL)
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
index fdb39d4001..9b3763cc1b 100644
--- a/src/include/catalog/pg_proc.h
+++ b/src/include/catalog/pg_proc.h
@@ -58,6 +58,9 @@ CATALOG(pg_proc,1255,ProcedureRelationId) BKI_BOOTSTRAP BKI_ROWTYPE_OID(81,Proce
/* see PROKIND_ categories below */
char prokind BKI_DEFAULT(f);
+ /* security definer */
+ char prosearch BKI_DEFAULT(d);
+
/* security definer */
bool prosecdef BKI_DEFAULT(f);
@@ -150,6 +153,15 @@ DECLARE_UNIQUE_INDEX(pg_proc_proname_args_nsp_index, 2691, ProcedureNameArgsNspI
#define PROKIND_WINDOW 'w'
#define PROKIND_PROCEDURE 'p'
+/*
+ * Symbolic values for prosearch column: these determine how search_path is
+ * initialized when executing the function. PROSEARCH_DEFAULT means that no
+ * SEARCH clause was specified.
+ */
+#define PROSEARCH_DEFAULT 'd'
+#define PROSEARCH_TRUSTED 't'
+#define PROSEARCH_SESSION 's'
+
/*
* Symbolic values for provolatile column: these indicate whether the result
* of a function is dependent *only* on the values of its explicit arguments,
@@ -197,6 +209,7 @@ extern ObjectAddress ProcedureCreate(const char *procedureName,
const char *probin,
Node *prosqlbody,
char prokind,
+ char prosearch,
bool security_definer,
bool isLeakProof,
bool isStrict,
diff --git a/src/test/regress/expected/create_function_sql.out b/src/test/regress/expected/create_function_sql.out
index 50aca5940f..83c649f725 100644
--- a/src/test/regress/expected/create_function_sql.out
+++ b/src/test/regress/expected/create_function_sql.out
@@ -88,6 +88,73 @@ SELECT proname, provolatile FROM pg_proc
functest_b_4 | v
(4 rows)
+--
+-- SEARCH FROM DEFAULT | TRUSTED | SESSION
+--
+CREATE FUNCTION f_show_search_path() RETURNS TEXT
+ LANGUAGE plpgsql AS
+$$
+ BEGIN
+ RETURN current_setting('search_path');
+ END;
+$$;
+CREATE PROCEDURE p_show_search_path()
+ LANGUAGE plpgsql AS
+$$
+ BEGIN
+ RAISE NOTICE 'search_path: %', current_setting('search_path');
+ END;
+$$;
+SELECT f_show_search_path();
+ f_show_search_path
+------------------------
+ temp_func_test, public
+(1 row)
+
+CALL p_show_search_path();
+NOTICE: search_path: temp_func_test, public
+ALTER FUNCTION f_show_search_path() SEARCH FROM TRUSTED;
+SELECT f_show_search_path();
+ f_show_search_path
+---------------------
+ pg_catalog, pg_temp
+(1 row)
+
+ALTER FUNCTION f_show_search_path() SEARCH FROM DEFAULT;
+SELECT f_show_search_path();
+ f_show_search_path
+------------------------
+ temp_func_test, public
+(1 row)
+
+ALTER FUNCTION f_show_search_path() SEARCH FROM SESSION;
+SELECT f_show_search_path();
+ f_show_search_path
+------------------------
+ temp_func_test, public
+(1 row)
+
+ALTER FUNCTION f_show_search_path() IMMUTABLE; -- fail
+ERROR: immutable functions cannot be specified with SEARCH FROM SESSION
+HINT: Specify SEARCH FROM TRUSTED instead.
+ALTER FUNCTION f_show_search_path() SEARCH FROM DEFAULT;
+ALTER FUNCTION f_show_search_path() IMMUTABLE;
+ALTER FUNCTION f_show_search_path() SEARCH FROM SESSION; -- fail
+ERROR: immutable functions cannot be specified with SEARCH FROM SESSION
+HINT: Specify SEARCH FROM TRUSTED instead.
+ALTER FUNCTION f_show_search_path() VOLATILE;
+ALTER ROUTINE p_show_search_path() SEARCH FROM TRUSTED SET search_path = test1;
+CALL p_show_search_path();
+NOTICE: search_path: test1
+ALTER ROUTINE f_show_search_path() SEARCH FROM SESSION SET search_path = test1;
+SELECT f_show_search_path();
+ f_show_search_path
+--------------------
+ test1
+(1 row)
+
+DROP FUNCTION f_show_search_path();
+DROP PROCEDURE p_show_search_path();
--
-- SECURITY DEFINER | INVOKER
--
diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out
index 7cd0c27cca..d6293c4003 100644
--- a/src/test/regress/expected/psql.out
+++ b/src/test/regress/expected/psql.out
@@ -5277,12 +5277,12 @@ create function psql_df_plpgsql ()
as $$ begin return; end; $$;
comment on function psql_df_plpgsql () is 'some comment';
\df+ psql_df_*
- List of functions
- Schema | Name | Result data type | Argument data types | Type | Volatility | Parallel | Owner | Security | Access privileges | Language | Internal name | Description
---------+------------------+------------------+---------------------+------+------------+----------+-------------------+----------+-------------------+----------+---------------+--------------
- public | psql_df_internal | double precision | double precision | func | immutable | safe | regress_psql_user | invoker | | internal | dsin |
- public | psql_df_plpgsql | void | | func | volatile | unsafe | regress_psql_user | invoker | | plpgsql | | some comment
- public | psql_df_sql | integer | x integer | func | volatile | unsafe | regress_psql_user | definer | | sql | |
+ List of functions
+ Schema | Name | Result data type | Argument data types | Type | Search | Volatility | Parallel | Owner | Security | Access privileges | Language | Internal name | Description
+--------+------------------+------------------+---------------------+------+---------+------------+----------+-------------------+----------+-------------------+----------+---------------+--------------
+ public | psql_df_internal | double precision | double precision | func | default | immutable | safe | regress_psql_user | invoker | | internal | dsin |
+ public | psql_df_plpgsql | void | | func | default | volatile | unsafe | regress_psql_user | invoker | | plpgsql | | some comment
+ public | psql_df_sql | integer | x integer | func | default | volatile | unsafe | regress_psql_user | definer | | sql | |
(3 rows)
rollback;
diff --git a/src/test/regress/sql/create_function_sql.sql b/src/test/regress/sql/create_function_sql.sql
index 89e9af3a49..383633cec9 100644
--- a/src/test/regress/sql/create_function_sql.sql
+++ b/src/test/regress/sql/create_function_sql.sql
@@ -60,6 +60,50 @@ SELECT proname, provolatile FROM pg_proc
'functest_B_3'::regproc,
'functest_B_4'::regproc) ORDER BY proname;
+--
+-- SEARCH FROM DEFAULT | TRUSTED | SESSION
+--
+
+CREATE FUNCTION f_show_search_path() RETURNS TEXT
+ LANGUAGE plpgsql AS
+$$
+ BEGIN
+ RETURN current_setting('search_path');
+ END;
+$$;
+
+CREATE PROCEDURE p_show_search_path()
+ LANGUAGE plpgsql AS
+$$
+ BEGIN
+ RAISE NOTICE 'search_path: %', current_setting('search_path');
+ END;
+$$;
+
+SELECT f_show_search_path();
+CALL p_show_search_path();
+
+ALTER FUNCTION f_show_search_path() SEARCH FROM TRUSTED;
+SELECT f_show_search_path();
+ALTER FUNCTION f_show_search_path() SEARCH FROM DEFAULT;
+SELECT f_show_search_path();
+ALTER FUNCTION f_show_search_path() SEARCH FROM SESSION;
+SELECT f_show_search_path();
+
+ALTER FUNCTION f_show_search_path() IMMUTABLE; -- fail
+ALTER FUNCTION f_show_search_path() SEARCH FROM DEFAULT;
+ALTER FUNCTION f_show_search_path() IMMUTABLE;
+ALTER FUNCTION f_show_search_path() SEARCH FROM SESSION; -- fail
+ALTER FUNCTION f_show_search_path() VOLATILE;
+
+ALTER ROUTINE p_show_search_path() SEARCH FROM TRUSTED SET search_path = test1;
+CALL p_show_search_path();
+ALTER ROUTINE f_show_search_path() SEARCH FROM SESSION SET search_path = test1;
+SELECT f_show_search_path();
+
+DROP FUNCTION f_show_search_path();
+DROP PROCEDURE p_show_search_path();
+
--
-- SECURITY DEFINER | INVOKER
--
--
2.34.1
