On 2/19/23 11:14 PM, Amit Kapila wrote:
On Sun, Feb 19, 2023 at 7:50 AM Jonathan S. Katz <jk...@postgresql.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
OpenPGP_signature
Description: OpenPGP digital signature
