Re: [17] Special search_path names "!pg_temp" and "!pg_catalog"
On Fri, Oct 27, 2023 at 12:58:47PM -0700, Jeff Davis wrote: > Do you, overall, find this feature useful? > > Most functions don't need pg_temp, so it feels cleaner to exclude it. > But pg_temp is ignored for function/op lookup anyway, so functions > won't be exposed to search_path risks related to pg_temp unless they > are accessing tables. > > If my proposal for the SEARCH clause got more support, I'd be more > excited about this feature because it could be set implicitly as part > of a safe search_path. Without the SEARCH clause, the only way to set > "!pg_temp" is by typing it out, and I'm not sure a lot of people will > actually do that. I thought it sounded generally useful, but if we're not going to proceed with the primary use-case for this feature, then perhaps it's not worth going through this particular one-way door at this time. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Re: [17] Special search_path names "!pg_temp" and "!pg_catalog"
On Thu, 2023-10-26 at 16:28 -0500, Nathan Bossart wrote: > On Fri, Aug 18, 2023 at 02:44:31PM -0700, Jeff Davis wrote: > > + SET search_path = admin, "!pg_temp"; > > I think it's unfortunate that these new identifiers must be quoted. > I > wonder if we could call these something like "no_pg_temp". *shrug* Do you, overall, find this feature useful? Most functions don't need pg_temp, so it feels cleaner to exclude it. But pg_temp is ignored for function/op lookup anyway, so functions won't be exposed to search_path risks related to pg_temp unless they are accessing tables. If my proposal for the SEARCH clause got more support, I'd be more excited about this feature because it could be set implicitly as part of a safe search_path. Without the SEARCH clause, the only way to set "!pg_temp" is by typing it out, and I'm not sure a lot of people will actually do that. Regards, Jeff Davis
Re: [17] Special search_path names "!pg_temp" and "!pg_catalog"
On Fri, Aug 18, 2023 at 02:44:31PM -0700, Jeff Davis wrote: > +SET search_path = admin, "!pg_temp"; I think it's unfortunate that these new identifiers must be quoted. I wonder if we could call these something like "no_pg_temp". *shrug* > + * Add any implicitly-searched namespaces to the list unless the markers > + * "!pg_catalog" or "!pg_temp" are present. Note these go on the front, > + * not the back; also notice that we do not check USAGE permissions for > + * these. >*/ > - if (!list_member_oid(oidlist, PG_CATALOG_NAMESPACE)) > + if (implicit_pg_catalog && > + !list_member_oid(oidlist, PG_CATALOG_NAMESPACE)) > oidlist = lcons_oid(PG_CATALOG_NAMESPACE, oidlist); > > - if (OidIsValid(myTempNamespace) && > + if (implicit_pg_temp && > + OidIsValid(myTempNamespace) && > !list_member_oid(oidlist, myTempNamespace)) > oidlist = lcons_oid(myTempNamespace, oidlist); Should we disallow including both !pg_temp and pg_temp at the same time? I worry that could be a source of confusion. IIUC your patches effectively ignore !pg_temp if pg_temp is explicitly listed elsewhere in the list. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
Re: [17] Special search_path names "!pg_temp" and "!pg_catalog"
On Sat, 2023-08-19 at 07:18 +0200, Pavel Stehule wrote: > cannot be better special syntax > > CREATE OR REPLACE FUNCTION xxx() > RETURNS yyy AS $$ ... $$$ > SET SEARCH_PATH DISABLE > > with possible next modification > > SET SEARCH_PATH CATALOG .. only for pg_catalog > SET SEARCH_PATH MINIMAL .. pg_catalog, pg_temp I agree that we should consider new syntax, and there's a related discussion here: https://www.postgresql.org/message-id/flat/2710f56add351a1ed553efb677408e51b060e67c.ca...@j-davis.com Regardless, even with syntax changes, we need something to print when someone does a "SHOW search_path", i.e. some representation that indicates pg_temp is excluded. That way it can also be saved and restored. > I question if we should block search path settings when this setting > is used. Although I set search_path, the search_path can be > overwritten in function of inside some nesting calls If so, that should be a separate feature. For the purposes of this thread, we just need a way to represent a search path that excludes pg_temp and/or pg_catalog. Regards, Jeff Davis
Re: [17] Special search_path names "!pg_temp" and "!pg_catalog"
Hi
pá 18. 8. 2023 v 23:44 odesílatel Jeff Davis napsal:
> The attached patch adds some special names to prevent pg_temp and/or
> pg_catalog from being included implicitly.
>
> This is a useful safety feature for functions that don't have any need
> to search pg_temp.
>
> The current (v16) recommendation is to include pg_temp last, which does
> add to the safety, but it's confusing to *include* a namespace when
> your intention is actually to *exclude* it, and it's also not
> completely excluding pg_temp.
>
> Although the syntax in the attached patch is not much friendlier, at
> least it's clear that the intent is to exclude pg_temp. Furthermore, it
> will be friendlier if we adopt the SEARCH SYSTEM syntax proposed in
> another thread[1].
>
> Additionally, this patch adds a WARNING when creating a schema that
> uses one of these special names. Previously, there was no warning when
> creating a schema with the name "$user", which could cause confusion.
>
> [1]
>
> https://www.postgresql.org/message-id/flat/2710f56add351a1ed553efb677408e51b060e67c.ca...@j-davis.com
cannot be better special syntax
CREATE OR REPLACE FUNCTION xxx()
RETURNS yyy AS $$ ... $$$
SET SEARCH_PATH DISABLE
with possible next modification
SET SEARCH_PATH CATALOG .. only for pg_catalog
SET SEARCH_PATH MINIMAL .. pg_catalog, pg_temp
I question if we should block search path settings when this setting is
used. Although I set search_path, the search_path can be overwritten in
function of inside some nesting calls
(2023-08-19 07:15:21) postgres=# create or replace function fx()
returns text as $$
begin
perform set_config('search_path', 'public', false);
return current_setting('search_path');
end;
$$ language plpgsql set search_path = 'pg_catalog';
CREATE FUNCTION
(2023-08-19 07:15:27) postgres=# select fx();
┌┐
│ fx │
╞╡
│ public │
└┘
(1 row)
>
>
>
> --
> Jeff Davis
> PostgreSQL Contributor Team - AWS
>
>
>
[17] Special search_path names "!pg_temp" and "!pg_catalog"
The attached patch adds some special names to prevent pg_temp and/or pg_catalog from being included implicitly. This is a useful safety feature for functions that don't have any need to search pg_temp. The current (v16) recommendation is to include pg_temp last, which does add to the safety, but it's confusing to *include* a namespace when your intention is actually to *exclude* it, and it's also not completely excluding pg_temp. Although the syntax in the attached patch is not much friendlier, at least it's clear that the intent is to exclude pg_temp. Furthermore, it will be friendlier if we adopt the SEARCH SYSTEM syntax proposed in another thread[1]. Additionally, this patch adds a WARNING when creating a schema that uses one of these special names. Previously, there was no warning when creating a schema with the name "$user", which could cause confusion. [1] https://www.postgresql.org/message-id/flat/2710f56add351a1ed553efb677408e51b060e67c.ca...@j-davis.com -- Jeff Davis PostgreSQL Contributor Team - AWS From 79cb94b857f51dec5cbc24b3d46d2e58de92b5c0 Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Fri, 18 Aug 2023 14:21:16 -0700 Subject: [PATCH] Special names "!pg_temp" and "!pg_catalog" for search_path. These names act as markers preventing the implicit namespaces from being added to the search path. Functions often don't need to look in pg_temp, and in those cases it's safer to exclude pg_temp entirely. --- doc/src/sgml/config.sgml| 25 ++ doc/src/sgml/ref/create_function.sgml | 8 +++--- src/backend/catalog/namespace.c | 23 src/backend/commands/schemacmds.c | 13 + src/test/regress/expected/namespace.out | 35 + src/test/regress/sql/namespace.sql | 18 + 6 files changed, 101 insertions(+), 21 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 11251fa05e..451f9c7e94 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -8631,10 +8631,11 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; The system catalog schema, pg_catalog, is always -searched, whether it is mentioned in the path or not. If it is -mentioned in the path then it will be searched in the specified -order. If pg_catalog is not in the path then it will -be searched before searching any of the path items. +searched, unless the special schema name +!pg_catalog is specified in the path. If it is +mentioned in the path then it will be searched in the specified order. +If pg_catalog is not in the path then it will be +searched before searching any of the path items. Likewise, the current session's temporary-table schema, -pg_temp_nnn, is always searched if it -exists. It can be explicitly listed in the path by using the -alias pg_temppg_temp. If it is not listed in the path then -it is searched first (even before pg_catalog). However, -the temporary schema is only searched for relation (table, view, -sequence, etc.) and data type names. It is never searched for -function or operator names. +pg_temp_nnn, is always +searched if it exists, unless the special schema name +!pg_temp is specified. It can be explicitly +listed in the path by using the alias +pg_temppg_temp. +If it is not listed in the path then it is searched first (even before +pg_catalog). However, the temporary schema is only +searched for relation (table, view, sequence, etc.) and data type +names. It is never searched for function or operator names. diff --git a/doc/src/sgml/ref/create_function.sgml b/doc/src/sgml/ref/create_function.sgml index 863d99d1fc..93aebdeb26 100644 --- a/doc/src/sgml/ref/create_function.sgml +++ b/doc/src/sgml/ref/create_function.sgml @@ -794,9 +794,8 @@ SELECT * FROM dup(42); Particularly important in this regard is the temporary-table schema, which is searched first by default, and is normally writable by anyone. A secure arrangement can be obtained -by forcing the temporary schema to be searched last. To do this, -write pg_temppg_tempsecuring functions as the last entry in search_path. -This function illustrates safe usage: +by specifying !pg_temp. This function illustrates +safe usage: CREATE FUNCTION check_password(uname TEXT, pass TEXT) @@ -811,8 +810,7 @@ BEGIN END; $$ LANGUAGE plpgsql SECURITY DEFINER --- Set a secure search_path: trusted schema(s), then 'pg_temp'. -SET search_path = admin, pg_temp; +SET search_path = admin, "!pg_temp"; This function's intention is to access a table admin.pwds. diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index 0c679fbf94..6d8a9cad02
