Обсуждение: security_definer_search_path GUC

Glad you bring this problem up for discussion, something should be done to improve the situation.

Personally, as I really dislike search_path and consider using it an anti-pattern.

I would rather prefer a GUC to hard-code search_path to a constant default value of just ‘public’ that cannot be changed by anyone or any function. Trying to change it to a different value would raise an exception.

On Sat, May 29, 2021, at 22:10, Marko Tiikkaja wrote:

On Sat, May 29, 2021 at 11:06 PM Joel Jacobson <joel@compiler.org> wrote:

Glad you bring this problem up for discussion, something should be done to improve the situation.

Personally, as I really dislike search_path and consider using it an anti-pattern.

I would rather prefer a GUC to hard-code search_path to a constant default value of just ‘public’ that cannot be changed by anyone or any function. Trying to change it to a different value would raise an exception.

That would work, too!  I think it's a nice idea, perhaps even better than what I proposed.  I would be happy to see either one incorporated.

Another idea would be to create an extension that removes the search_path feature entirely,

not sure though if the current hooks would allow creating such an extension.

Maybe "extensions" that only removes unwanted core features could be by convention be prefixed with "no_"?

CREATE EXTENSION no_search_path;

That way, a company with a company-wide policy against using search_path,

could add this to all their company .control extension files:

requires = 'no_search_path'

If some employee would try to `DROP EXTENSION no_search_path` they would get an error:

# DROP EXTENSION no_search_path;

ERROR:  cannot drop extension no_search_path because other objects depend on it

DETAIL:  extension acme_inc depends on extension no_search_path

This would be especially useful when a company has a policy to use some extension,

instead of relying on the built-in functionality provided.

I'm not using "zson" myself, but perhaps it could be a good example to illustrate my point:

Let's say a company has decided to use zson instead of json/jsonb,

the company would then ensure nothing is using json/jsonb

via the top-level .control file for the company's own extension:

requires = 'no_json, no_jsonb, zson'

Or if not shipping the company's product as an extension,

they could instead add this to the company's install script:

CREATE EXTENSION no_json;

CREATE EXTENSION no_jsonb;

CREATE EXTENSION zson;

Maybe this is out of scope for extensions, since I guess extensions are supposed to add features?

If so, how about a new separate command `CREATE REDUCTION` specifically to remove unwanted core features,

which then wouldn't need the "no_" prefix since it would be implicit and in a different namespace:

E.g.

CREATE REDUCTION search_path;

and

CREATE REDUCTION json;

CREATE REDUCTION jsonb;

CREATE EXTENSION zson;

/Joel

On Sun, May 30, 2021, at 09:54, Pavel Stehule wrote:

Maybe inverted design can work better - there can be GUC - "qualified_names_required" with a list of schemas without enabled implicit access.

The one possible value can be "all".

The advantage of this design can be the possibility of work on current extensions.

I don't think so search_path can be disabled - but there can be checks that disallow non-qualified names.

I would prefer a pre-installed search_path-extension that can be uninstalled,

instead of yet another GUC, but if that's not an option, I'm happy with a GUC as well.

IMO, the current search_path default behaviour is a minefield.

For users like myself, who prefer a safer context-free name resolution behaviour, here is how I think it should work:

* The only schemas that don't require fully-qualified schemas are 'pg_catalog' and 'public'

* The $user schema feature is removed, i.e:

- $user is not part of the search_path

- objects are not created nor looked for in a $user schema if such a schema exists

- objects are always created in 'public' if a schema is not explicitly specified

* Temp objects always needs to be fully-qualified using 'pg_temp'

* 'pg_catalog' and 'public' are enforced to be completely disjoint.

That is, trying to create an object in 'public' that is in conflict with 'pg_catalog' would raise an error.

More ideas?

On Tue, Jun 1, 2021, at 10:44, Pavel Stehule wrote:

Operators use schemas too.  I cannot imagine any work with operators with the necessity of explicit schemas.

I thought operators are mostly installed in the public schema, in which case that wouldn't be a problem, or am I missing something here?

/Joel

On Tue, Jun 1, 2021, at 12:55, Pavel Stehule wrote:

út 1. 6. 2021 v 12:53 odesílatel Joel Jacobson <joel@compiler.org> napsal:

On Tue, Jun 1, 2021, at 10:44, Pavel Stehule wrote:

Operators use schemas too.  I cannot imagine any work with operators with the necessity of explicit schemas.

I thought operators are mostly installed in the public schema, in which case that wouldn't be a problem, or am I missing something here?

It is inconsistency - if I use schema for almost all, then can be strange to store operators just to public.

I don't agree. If an extension provides functionality that is supposed to be used by all parts of the system, then I think the 'public' schema is a good choice.

Using schemas only for the sake of separation, i.e. adding the schemas to the search_path, to make them implicitly available, is IMO an ugly hack, since if just wanting separation without fully-qualifying, then packaging the objects are separate extensions is much cleaner. That way you can easily see what objects are provided by each extension using \dx+.

/Joel

On Tue, Jun 1, 2021, at 14:41, Pavel Stehule wrote:

út 1. 6. 2021 v 13:13 odesílatel Joel Jacobson <joel@compiler.org> napsal:

I don't agree. If an extension provides functionality that is supposed to be used by all parts of the system, then I think the 'public' schema is a good choice.

I disagree

usual design of extensions (when schema is used) is

create schema ...

set schema ...

create table

create function

It is hard to say if it is good or it is bad.

Yes, it's hard, because it's a matter of taste.

Some prefer convenience, others clarity/safety.

Orafce using my own schema, and some things are in public (and some in pg_catalog), and people don't tell me, so it was a good choice.

I struggle to understand this last sentence.

So you orafce extension installs objects in both public and pg_catalog, right.

But what do you mean with "people don't tell me"?

And what "was a good choice"?

Thanks for explaining.

/Joel

On Tue, Jun 1, 2021, at 18:05, Pavel Stehule wrote:

I learned programming on Orafce, and I didn't expect any success, so I designed it quickly, and the placing of old Orafce's functions to schemas is messy.

I am sure, if I started again, I would never use pg_catalog or public schema. I think if somebody uses schema, then it is good to use schema for all without exceptions - but it expects usage of search_path. I am not sure if using  public schema or using search_path are two sides of one thing.

I think you're right they both try to provide solutions to the same problem, i.e. when wanting to avoid having to fully-qualify.

However, they are very different, and while I think the 'public' schema is a great idea, I think 'search_path' has some serious problems. I'll explain why:

'search_path' is a bit like a global variable in C, that can change the behaviour of the SQL commands executed.

It makes unqualified SQL code context-sensitive; you don't know by looking at a piece of code what objects are referred to, you also need to figure out what the active search_path is at this place in the code.

'public' schema if used (without ever changing the default 'search_path'), allows creating unqualified database objects, which I think can be useful in at least three situations:

1) when the application is a monolith inside a company, when there is only one version of the database, i.e. not having to worry about name collision with other objects in some other version, since the application is hidden in the company and the schema design is not exposed to the public

2) when installing a extension that uses schemas, when wanting the convenience of unqualified access to some functions frequently used, instead of adding its schema to the search_path for convenience, one can instead add wrapper-functions in the 'public' schema. This way, all internal functions in the extension, that are not meant to be executed by users, are still hidden in its schema and won't bother anyone (i.e. can't cause unexpected conflicts). Of course, access can also be controlled via REVOKE EXECUTE ... FROM PUBLIC for such internal functions, which is probably a good idea as well.

In a similar way, specific tables in the extension's schema can be made unqualified as well by adding simple views, installed in the public schema, if insisting on unqualified convenience.

In conclusion:

The main difference is 'public' makes it possible to make *specific* objects unqualified,

while 'search_path' makes *all* objects in such schema(s) unqualified.

/Joel

If a database object is to be accessed unqualified by all users, isn't the 'public' schema a perfect fit for it? How will it be helpful to create different database objects in different schemas, if also adding all such schemas to the search_path so they can be accessed unqualified? In such a scenario you risk unintentionally creating conflicting objects, and whatever schema happened to be first in the search_path will be resolved. Seems insecure and messy to me.

On Wed, Jun 2, 2021, at 09:07, Pavel Stehule wrote:

st 2. 6. 2021 v 8:45 odesílatel Joel Jacobson <joel@compiler.org> napsal:

'search_path' is a bit like a global variable in C, that can change the behaviour of the SQL commands executed.

It makes unqualified SQL code context-sensitive; you don't know by looking at a piece of code what objects are referred to, you also need to figure out what the active search_path is at this place in the code.

sometimes this is wanted feature - some sharding is based on this

set search_path = 'custormerx'

Oh, interesting, didn't know abou that one. Is that recommended best practise, or more of a hack?

I also think we can never get rid of search_path by default, since so much legacy depend on it.

But I think it would be good to provide a way to effectively uninstall the search_path for users who prefer to do so, in databases where it's possible, and where clarity and safety is desired.

'public' schema if used (without ever changing the default 'search_path'), allows creating unqualified database objects, which I think can be useful in at least three situations:

1) when the application is a monolith inside a company, when there is only one version of the database, i.e. not having to worry about name collision with other objects in some other version, since the application is hidden in the company and the schema design is not exposed to the public

2) when installing a extension that uses schemas, when wanting the convenience of unqualified access to some functions frequently used, instead of adding its schema to the search_path for convenience, one can instead add wrapper-functions in the 'public' schema. This way, all internal functions in the extension, that are not meant to be executed by users, are still hidden in its schema and won't bother anyone (i.e. can't cause unexpected conflicts). Of course, access can also be controlled via REVOKE EXECUTE ... FROM PUBLIC for such internal functions, which is probably a good idea as well.

In a similar way, specific tables in the extension's schema can be made unqualified as well by adding simple views, installed in the public schema, if insisting on unqualified convenience.

In conclusion:

The main difference is 'public' makes it possible to make *specific* objects unqualified,

while 'search_path' makes *all* objects in such schema(s) unqualified.

These arguments are valid, but I think so it is not all. If you remove search_path, then the "public" schema will be overused.

What makes you think that? If a database object is to be accessed unqualified by all users, isn't the 'public' schema a perfect fit for it? How will it be helpful to create different database objects in different schemas, if also adding all such schemas to the search_path so they can be accessed unqualified? In such a scenario you risk unintentionally creating conflicting objects, and whatever schema happened to be first in the search_path will be resolved. Seems insecure and messy to me.

Much safer to install objects that you want to access unqualified in 'public', and get an error if you try to create a new object with a conflicting name of an existing one.

I think we should ask - who can change the search path and how. Now, there are not any limits. I can imagine the situation when search_path can be changed by only some dedicated role - it can be implemented in a security definer function. Or another solution, we can fix the search path to one value, or only a few possibilities.

 

Maybe for your purpose is just enough to introduce syntax for defining all possibilities of search path:

search_path = "public" # now, just default

search_path = ["public"] # future - define vector of possible values of search path - in this case, only "public" is allowed - and if you want to change it, you should be database owner

or there can be hook for changing search_path, and it can be implemented dynamically in extension

Not bad ideas. I think they would improve the situation. Maybe it could even be a global immutable constant value, the same for all users, that could only be set upon initdb, similar to how encoding can only be set via initdb.

initdb --search_path "pg_catalog, public, pg_temp" foobar

But perhaps the search_path as an uninstallable extension is a less invasive idea.

Looking at the code, this seems to be the commit that introduced search_path back in 2002:

I'm not sure how difficult it would be to extract search_path into an extension.

Doesn't look to be that much code. Here is the initial commit that introduced the concept.

But perhaps it's more complex today due to new dependencies.

commit 838fe25a9532ab2e9b9b7517fec94e804cf3da81

Author: Tom Lane <tgl@sss.pgh.pa.us>

Date:   Mon Apr 1 03:34:27 2002 +0000

    Create a new GUC variable search_path to control the namespace search

    path.  The default behavior if no per-user schemas are created is that

    all users share a 'public' namespace, thus providing behavior backwards

    compatible with 7.2 and earlier releases.  Probably the semantics and

    default setting will need to be fine-tuned, but this is a start.

But search_path is not the only problem. I think it's also a problem objects with the same identifies can be created in both pg_catalog and public. Can we think of a valid reason why it is a good idea to continue to allow that? In what real-life scenario is it needed?

/Joel

On 2021-Jun-02, Marko Tiikkaja wrote:

> The use case is: version upgrades.  I want to be able to have a search_path
> of something like 'pg_catalog, compat, public'.  That way we can provide
> compatibility versions of newer functions in the "compat" schema, which get
> taken over by pg_catalog when running on a newer version.  That way all the
> compatibility crap is clearly separated from the stuff that should be in
> "public".

Can't you achieve that with "ALTER DATABASE .. SET search_path"?

On Wed, Jun 2, 2021, at 18:36, Marko Tiikkaja wrote:

The use case is: version upgrades.  I want to be able to have a search_path of something like 'pg_catalog, compat, public'.  That way we can provide compatibility versions of newer functions in the "compat" schema, which get taken over by pg_catalog when running on a newer version.  That way all the compatibility crap is clearly separated from the stuff that should be in "public".

That's a neat trick, probably the best solution in a really old PostgreSQL version, before we had extensions.

But if running a recent PostgreSQL version, with support for extensions, I think an even cleaner solution

would be to package such compatibility versions in a "compat" extension, that would just install them into the public schema.

And if you wonder what functions in public come from the compat extension, you can use use \dx+.

On Thu, Jun 3, 2021, at 00:55, Marko Tiikkaja wrote:

On Wed, Jun 2, 2021 at 11:32 PM Joel Jacobson <joel@compiler.org> wrote:

But if running a recent PostgreSQL version, with support for extensions, I think an even cleaner solution

would be to package such compatibility versions in a "compat" extension, that would just install them into the public schema.

Writing, verifying and shipping extension upgrade scripts is not pleasant. 

I agree. Thanks for acknowledging this problem.

I'm experimenting with an idea that I hope can simplify the "verifying" part of the problem.

hope to have something to show you all soon. 

I'd much prefer something that's integrated to the workflow I already have.

Fair point. I guess also the initial switching cost of changing workflow is quite high and difficult to motivate. So even if extensions ergonomics are improved, many existing users will not migrate their workflows anyway due to this.

 

And if you wonder what functions in public come from the compat extension, you can use use \dx+.

They still show up everywhere when looking at "public".  So this is only slightly better, and a maintenance burden.

Good point. I find this annoying as well sometimes.

It's easy to get a list of all objects for an extension, via \dx+

But it's hard to see what objects in a schema, that are provided by different extensions, via e.g. \df public.*

What about adding a new "Extension" column next to "Schema" to the relevant commands, such as \df?

/Joel

On Thu, Jun 3, 2021, at 00:55, Marko Tiikkaja wrote:

They still show up everywhere when looking at "public".  So this is only slightly better, and a maintenance burden.

Good point. I find this annoying as well sometimes.

It's easy to get a list of all objects for an extension, via \dx+

But it's hard to see what objects in a schema, that are provided by different extensions, via e.g. \df public.*

What about adding a new "Extension" column next to "Schema" to the relevant commands, such as \df? 

On Thu, Jun 3, 2021 at 9:14 AM Joel Jacobson <joel@compiler.org> wrote:

On Thu, Jun 3, 2021, at 00:55, Marko Tiikkaja wrote:

They still show up everywhere when looking at "public".  So this is only slightly better, and a maintenance burden.

Good point. I find this annoying as well sometimes.

It's easy to get a list of all objects for an extension, via \dx+

But it's hard to see what objects in a schema, that are provided by different extensions, via e.g. \df public.*

What about adding a new "Extension" column next to "Schema" to the relevant commands, such as \df? 

That's just one part of it.  The other part of my original proposal was to avoid having to  SET search_path  for all SECURITY DEFINER functions.  I still think either being able to lock search_path or the separate prosecdef search_path is the best option here.

.m

> On Jun 3, 2021, at 9:03 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> I agree so some possibility of locking search_path or possibility to control who and when can change it can increase security. This should be a core feature. It's maybe more generic issue - same functionality can be required for work_mem setting, maybe max_paralel_workers_per_gather, and other GUC

Chapman already suggested a mechanism in [1] to allow chaining together additional validators for GUCs.

When setting search_path, the check_search_path(char **newval, void **extra, GucSource source) function is invoked.  As I understand Chapman's proposal, additional validators could be added to any GUC.  You could implement search_path restrictions by defining additional validators that enforce whatever restriction you like.

Marko, does his idea sound workable for your needs?  I understood your original proposal as only restricting the value of search_path within security definer functions.  This idea would allow you to restrict it everywhere, and not tailored to just that context.

> On Jun 3, 2021, at 9:03 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> I agree so some possibility of locking search_path or possibility to control who and when can change it can increase security. This should be a core feature. It's maybe more generic issue - same functionality can be required for work_mem setting, maybe max_paralel_workers_per_gather, and other GUC

Chapman already suggested a mechanism in [1] to allow chaining together additional validators for GUCs.

When setting search_path, the check_search_path(char **newval, void **extra, GucSource source) function is invoked.  As I understand Chapman's proposal, additional validators could be added to any GUC.  You could implement search_path restrictions by defining additional validators that enforce whatever restriction you like.

Marko, does his idea sound workable for your needs?  I understood your original proposal as only restricting the value of search_path within security definer functions.  This idea would allow you to restrict it everywhere, and not tailored to just that context.

[1] https://www.postgresql.org/message-id/608C9A81.3020006@anastigmatix.net

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

> On Jun 3, 2021, at 9:38 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
> This design looks good for extensions, but I am not sure if it is good for users. Some declarative way without necessity to programming or install some extension can be nice.

I agree, though "some declarative way" is a bit vague.  I've had ideas that perhaps superusers should be able to further restrict the [min,max] fields of int and real GUCs.  Since -1 is sometimes used to mean "disabled", syntax to allow specifying a set might be necessary, something like [-1, 60..600].  For text and enum GUCs, perhaps a set of regexps would work, some being required to match and others being required not to match, such as:

        search_path !~ '\mcustomerx\M'
        search_path ~ '^pg_catalog,'

If we did something like this, we'd need it to play nicely with other filters provided by extensions, because I'm reasonably sure not all filters could be done merely using set notation and regular expression syntax.  In fact, I find it hard to convince myself that set notation and regular expression syntax would even be useful in a large enough number of cases to be worth implementing.  What are your thought on that?

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

> On Jun 3, 2021, at 12:06 PM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
>
>
>
> čt 3. 6. 2021 v 20:25 odesílatel Mark Dilger <mark.dilger@enterprisedb.com> napsal:
>
>
> > On Jun 3, 2021, at 9:38 AM, Pavel Stehule <pavel.stehule@gmail.com> wrote:
> >
> > This design looks good for extensions, but I am not sure if it is good for users. Some declarative way without necessity to programming or install some extension can be nice.
>
> I agree, though "some declarative way" is a bit vague.  I've had ideas that perhaps superusers should be able to further restrict the [min,max] fields of int and real GUCs.  Since -1 is sometimes used to mean "disabled", syntax to allow specifying a set might be necessary, something like [-1, 60..600].  For text and enum GUCs, perhaps a set of regexps would work, some being required to match and others being required not to match, such as:
>
>         search_path !~ '\mcustomerx\M'
>         search_path ~ '^pg_catalog,'
>
> If we did something like this, we'd need it to play nicely with other filters provided by extensions, because I'm reasonably sure not all filters could be done merely using set notation and regular expression syntax.  In fact, I find it hard to convince myself that set notation and regular expression syntax would even be useful in a large enough number of cases to be worth implementing.  What are your thought on that?
>
> I don't think so for immutable strings we need regular expressions.  Maybe use some special keyword
>
> search_path only "pg_catalog"

I think we're trying to solve different problems.  I'm trying to allow non-superusers to set GUCs while putting constraints on what values they choose.  You appear to be trying to revoke the ability to set a GUC by forcing it to only ever have a single value.

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

I realise "eliminate" is not really necessary, it would suffice to just allow setting a a sane default per database, and make that value immutable, then all data structures and code using wouldn't need to change, one would then only need to change the code that can mutate search_path, to prevent that from happening.

On Fri, Jun 4, 2021, at 08:58, Pavel Stehule wrote:

It is the same as using the command line without the possibility to customize the PATH variable. The advantages and disadvantages are exactly the same.

The reason why we even have PATH in the *nix world,

is not because they *wanted* to separate things (like we want with schemas or extensions),

but because they *needed* to, because /bin was overflowed:

"The UNIX shell gave up the Multics idea of a search path and looked for program names that weren’t

file names in just one place, /bin. Then in v3 /bin overflowed the small (256K), fast fixed-head drive.

Thus was /usr/bin born, and the idea of a search path reinstated." [1]

[1] https://www.cs.dartmouth.edu/~doug/reader.pdf

/Joel

Maybe this could work:
CREATE SCHEMA schema_name UNQUALIFIED;
Which would explicitly make all the objects created in the schema accessible unqualified, but also enforce there are no conflicts with other objects in existence in all unqualified schemas, upon the creation of new objects.

/Joel

pá 4. 6. 2021 v 17:43 odesílatel Joel Jacobson <joel@compiler.org> napsal:

Maybe this could work:
CREATE SCHEMA schema_name UNQUALIFIED;
Which would explicitly make all the objects created in the schema accessible unqualified, but also enforce there are no conflicts with other objects in existence in all unqualified schemas, upon the creation of new objects.

Yes, it can work. I am not sure if "unqualified" is the best keyword, but the idea is workable.

On Fri, Jun 4, 2021 at 9:03 AM Pavel Stehule <pavel.stehule@gmail.com> wrote:

pá 4. 6. 2021 v 17:43 odesílatel Joel Jacobson <joel@compiler.org> napsal:

Maybe this could work:
CREATE SCHEMA schema_name UNQUALIFIED;
Which would explicitly make all the objects created in the schema accessible unqualified, but also enforce there are no conflicts with other objects in existence in all unqualified schemas, upon the creation of new objects.

Yes, it can work. I am not sure if "unqualified" is the best keyword, but the idea is workable.

Sounds like a job for an event trigger listening to CREATE/ALTER schema.

If we don't like "UNQUALIFIED" as a keyword, maybe we could reuse "PUBLIC"?

Or will that be confusing since "PUBLIC" is also a role_specification?