From: | "Jonathan S(dot) Katz" <jkatz(at)postgresql(dot)org> |
---|---|
To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
Cc: | Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Zheng Li <zhengli10(at)gmail(dot)com>, li jie <ggysxcq(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Japin Li <japinli(at)hotmail(dot)com>, rajesh singarapu <rajesh(dot)rs0541(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Support logical replication of DDLs |
Date: | 2023-02-20 23:44:58 |
Message-ID: | 5aa3d564-52bc-2dcd-b2d9-27ad7d4d45a9@postgresql.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-general pgsql-hackers |
On 2/19/23 11:14 PM, Amit Kapila wrote:
> On Sun, Feb 19, 2023 at 7:50 AM Jonathan S. Katz <jkatz(at)postgresql(dot)org> wrote:
>>
>> On 2/17/23 4:15 AM, Amit Kapila wrote:
>>
>>> This happens because of the function used in the index expression.
>>> Now, this is not the only thing, the replication can even fail during
>>> DDL replication when the function like above is IMMUTABLE and used as
>>> follows: ALTER TABLE tbl ADD COLUMN d int DEFAULT t(1);
>>>
>>> Normally, it is recommended that users can fix such errors by
>>> schema-qualifying affected names. See commits 11da97024a and
>>> 582edc369c.
>>
>> I'm very familiar with those CVEs, but even though these are our
>> recommended best practices, there is still a lot of code that does not
>> schema-qualify the names of functions (including many of our own examples ;)
>>
>> If we're going to say "You can use logical replication to replicate
>> functions, but you have to ensure you've schema-qualified any function
>> calls within them," I think that will prevent people from being able to
>> use this feature, particularly on existing applications.
>>
>
> I agree with this statement.
>
>> I guess there's a connection I'm missing here. For the failing examples
>> above, I look at the pg_proc entries on both the publisher and the
>> subscriber and they're identical. I'm not understanding why creating and
>> executing the functions works on the publisher, but it does not on the
>> subscriber.
>>
>
> It is because on the subscriber, in apply worker, we override the
> search path to "". See
>
> InitializeApplyWorker()
> {
> ...
> /*
> * Set always-secure search path, so malicious users can't redirect user
> * code (e.g. pg_index.indexprs).
> */
> SetConfigOption("search_path", "", PGC_SUSET, PGC_S_OVERRIDE);
> ...
>
> This has been done to ensure that apply worker doesn't execute
> arbitrary expressions as currently, it works with the privileges of
> the subscription owner which would be a superuser.
Got it, thanks!
>> What additional info would the subscriber need to be able to
>> successfully run these functions? Would we need to pass in some
>> additional context, e.g. what the search_path was at the time the
>> publisher created the function?
>>
>
> Yeah, I think search_path is required. I think we need some way to
> avoid breaking what we have done in commit 11da97024a and that also
> allows us to refer to objects without schema qualification in
> functions. Will it be sane to allow specifying search_path for a
> subscription via Alter Subscription? Can we think of any other way
> here?
Presumably CREATE SUBSCRIPTION as well -- and are you thinking on one of
the WITH options? Maybe it's also possible with a GUC on the subscriber
side that sets a default search path to use during apply. If not set, it
will use what 11da97024a specified. Regardless, one or both of these are
opt-in on the subscriber, so the subscriber can make the call what level
of permissiveness to have in the search_path.
We can then combine this with documentation, i.e. emphasize the
importance of the best-practice to schema qualify functions.
Additionally, we can also (strongly?) recommend for users to use SQL
standard function bodies (BEGIN ATOMIC) for SQL-based functions.
I want to be mindful of our security recommendations the work we've done
to harden the search_path and don't want to weaken anything there. At
the same time, I also want to ensure we don't make it a nonstarter to
use logical replication in the new set of use cases this and other work
is opening up.
Thanks,
Jonathan
From | Date | Subject | |
---|---|---|---|
Next Message | Bryn Llewellyn | 2023-02-21 01:54:14 | Re: Thanks! Re: Who adds the "start transaction" and "commit" to the intended SQL statement in "autocommit" mode? |
Previous Message | Adrian Klaver | 2023-02-20 23:24:23 | Re: pg_dump'ed file contains "DROP DATABASE" |
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2023-02-20 23:54:17 | meson and sslfiles.mk in src/test/ssl/ |
Previous Message | Stephen Frost | 2023-02-20 23:35:55 | Disable rdns for Kerberos tests |