Re: Non-superuser subscription owners

2023-06-15 Thread Amit Kapila
On Thu, Jun 15, 2023 at 11:18 PM Alvaro Herrera wrote: > > On 2023-Jun-13, Amit Kapila wrote: > > > I'll push this tomorrow unless there are any suggestions or comments. > > Note the proposed commit message is wrong about which commit is to blame > for the original problem -- it mentions

Re: Non-superuser subscription owners

2023-06-15 Thread Alvaro Herrera
On 2023-Jun-13, Amit Kapila wrote: > I'll push this tomorrow unless there are any suggestions or comments. Note the proposed commit message is wrong about which commit is to blame for the original problem -- it mentions e7e7da2f8d57 twice, but one of them is actually c3afe8cf5a1e. -- Álvaro

RE: Non-superuser subscription owners

2023-06-13 Thread Zhijie Hou (Fujitsu)
On Wednesday, June 14, 2023 10:11 AM Amit Kapila wrote: > > On Tue, Jun 13, 2023 at 2:25 PM Amit Kapila wrote: > > > > On Fri, May 12, 2023 at 3:28 PM Zhijie Hou (Fujitsu) > > wrote: > > > > > > > > > I can reproduce this via gdb following similar steps in [1]. > > > > > > I think we need to

Re: Non-superuser subscription owners

2023-06-13 Thread Amit Kapila
On Tue, Jun 13, 2023 at 2:25 PM Amit Kapila wrote: > > On Fri, May 12, 2023 at 3:28 PM Zhijie Hou (Fujitsu) > wrote: > > > > > > I can reproduce this via gdb following similar steps in [1]. > > > > I think we need to move this call into a transaction as well and here is an > > attempt > > to do

Re: Non-superuser subscription owners

2023-06-13 Thread Amit Kapila
On Tue, Jun 13, 2023 at 2:25 PM Amit Kapila wrote: > > On Fri, May 12, 2023 at 3:28 PM Zhijie Hou (Fujitsu) > wrote: > > > > > > I can reproduce this via gdb following similar steps in [1]. > > > > I think we need to move this call into a transaction as well and here is an > > attempt > > to do

Re: Non-superuser subscription owners

2023-06-13 Thread Amit Kapila
On Fri, May 12, 2023 at 3:28 PM Zhijie Hou (Fujitsu) wrote: > > > I can reproduce this via gdb following similar steps in [1]. > > I think we need to move this call into a transaction as well and here is an > attempt > to do that. > I am able to reproduce this issue following the steps

RE: Non-superuser subscription owners

2023-05-12 Thread Zhijie Hou (Fujitsu)
On Tuesday, April 4, 2023 1:57 AM Robert Haas wrote: > > On Sat, Apr 1, 2023 at 12:00 PM Alexander Lakhin > wrote: > > I've managed to reproduce it using the following script: > > for ((i=1;i<=10;i++)); do > > echo "iteration $i" > > echo " > > CREATE ROLE sub_user; > > CREATE SUBSCRIPTION

Re: Non-superuser subscription owners

2023-04-23 Thread Amit Kapila
On Fri, Apr 21, 2023 at 6:21 PM Robert Haas wrote: > > On Fri, Apr 21, 2023 at 8:19 AM Amit Kapila wrote: > > LGTM. Let's see if Robert or others have any comments, otherwise, I'll > > push this early next week. > > LGTM too. > Pushed. -- With Regards, Amit Kapila.

Re: Non-superuser subscription owners

2023-04-21 Thread Robert Haas
On Fri, Apr 21, 2023 at 8:19 AM Amit Kapila wrote: > LGTM. Let's see if Robert or others have any comments, otherwise, I'll > push this early next week. LGTM too. -- Robert Haas EDB: http://www.enterprisedb.com

Re: Non-superuser subscription owners

2023-04-21 Thread Amit Kapila
On Fri, Apr 21, 2023 at 12:30 PM vignesh C wrote: > > On Fri, 21 Apr 2023 at 01:49, Robert Haas wrote: > > > > On Thu, Apr 20, 2023 at 1:08 AM Amit Kapila wrote: > > > Pushed. I noticed that we didn't display this new subscription option > > > 'password_required' in \dRs+: > > > > > >

Re: Non-superuser subscription owners

2023-04-21 Thread vignesh C
On Fri, 21 Apr 2023 at 01:49, Robert Haas wrote: > > On Thu, Apr 20, 2023 at 1:08 AM Amit Kapila wrote: > > Pushed. I noticed that we didn't display this new subscription option > > 'password_required' in \dRs+: > > > > postgres=# \dRs+ > > > > List of subscriptions > > Name | Owner |

Re: Non-superuser subscription owners

2023-04-20 Thread Robert Haas
On Thu, Apr 20, 2023 at 1:08 AM Amit Kapila wrote: > Pushed. I noticed that we didn't display this new subscription option > 'password_required' in \dRs+: > > postgres=# \dRs+ > > List of subscriptions > Name | Owner | Enabled | Publication | Binary | Streaming | > Two-phase commit |

Re: Non-superuser subscription owners

2023-04-19 Thread Amit Kapila
On Thu, Apr 13, 2023 at 8:02 AM Amit Kapila wrote: > > On Wed, Apr 12, 2023 at 5:50 PM Robert Haas wrote: > > > > On Tue, Apr 11, 2023 at 10:56 PM Amit Kapila > > wrote: > > > Anyway, I don't think as such there is any problem > > > with restarting the worker even when the subscription owner

Re: Non-superuser subscription owners

2023-04-12 Thread Amit Kapila
On Wed, Apr 12, 2023 at 5:50 PM Robert Haas wrote: > > On Tue, Apr 11, 2023 at 10:56 PM Amit Kapila wrote: > > Anyway, I don't think as such there is any problem > > with restarting the worker even when the subscription owner is a > > superuser, so adjusted the check accordingly. > > LGTM. >

Re: Non-superuser subscription owners

2023-04-12 Thread Robert Haas
On Tue, Apr 11, 2023 at 10:56 PM Amit Kapila wrote: > Anyway, I don't think as such there is any problem > with restarting the worker even when the subscription owner is a > superuser, so adjusted the check accordingly. LGTM. I realize we could do more sophisticated things here, but I think it's

Re: Non-superuser subscription owners

2023-04-11 Thread Amit Kapila
On Tue, Apr 11, 2023 at 8:21 PM Robert Haas wrote: > > On Tue, Apr 11, 2023 at 5:53 AM Amit Kapila wrote: > > I think additionally, we should check that the new owner of the > > subscription is not a superuser, otherwise, anyway, this parameter is > > ignored. Please find the attached to add

Re: Non-superuser subscription owners

2023-04-11 Thread Robert Haas
On Tue, Apr 11, 2023 at 5:53 AM Amit Kapila wrote: > I think additionally, we should check that the new owner of the > subscription is not a superuser, otherwise, anyway, this parameter is > ignored. Please find the attached to add this check. I don't see why we should check that. It makes this

Re: Non-superuser subscription owners

2023-04-11 Thread Amit Kapila
On Mon, Apr 10, 2023 at 9:15 PM Robert Haas wrote: > > On Sat, Apr 8, 2023 at 1:35 AM Amit Kapila wrote: > > Do we need to have a check for this new option "password_required" in > > maybe_reread_subscription() where we "Exit if any parameter that > > affects the remote connection was changed."?

Re: Non-superuser subscription owners

2023-04-10 Thread Robert Haas
On Sat, Apr 8, 2023 at 1:35 AM Amit Kapila wrote: > Do we need to have a check for this new option "password_required" in > maybe_reread_subscription() where we "Exit if any parameter that > affects the remote connection was changed."? This new option is > related to the remote connection so I

Re: Non-superuser subscription owners

2023-04-07 Thread Amit Kapila
On Thu, Mar 30, 2023 at 9:35 PM Robert Haas wrote: > > On Tue, Mar 28, 2023 at 1:52 PM Jeff Davis wrote: > > On Fri, 2023-03-24 at 00:17 -0700, Jeff Davis wrote: > > > The other patch you posted seems like it makes a lot of progress in > > > that direction, and I think that should go in first.

Re: Non-superuser subscription owners

2023-04-03 Thread Robert Haas
On Sat, Apr 1, 2023 at 12:00 PM Alexander Lakhin wrote: > I've managed to reproduce it using the following script: > for ((i=1;i<=10;i++)); do > echo "iteration $i" > echo " > CREATE ROLE sub_user; > CREATE SUBSCRIPTION testsub CONNECTION 'dbname=db' > PUBLICATION testpub WITH (connect =

Re: Non-superuser subscription owners

2023-04-01 Thread Andres Freund
Hi, On April 1, 2023 9:00:00 AM PDT, Alexander Lakhin wrote: >Hello Robert, > >31.03.2023 23:00, Robert Haas wrote: >> That looks like a reasonable fix but I can't reproduce the problem >> locally. I thought the reason why that machine sees the problem might >> be that it uses

Re: Non-superuser subscription owners

2023-04-01 Thread Alexander Lakhin
Hello Robert, 31.03.2023 23:00, Robert Haas wrote: That looks like a reasonable fix but I can't reproduce the problem locally. I thought the reason why that machine sees the problem might be that it uses -DRELCACHE_FORCE_RELEASE, but I tried that option here and the tests still pass. Anyone

RE: Non-superuser subscription owners

2023-03-31 Thread houzj.f...@fujitsu.com
On Saturday, April 1, 2023 4:00 AM Robert Haas Hi, > > On Thu, Mar 30, 2023 at 9:49 PM houzj.f...@fujitsu.com > wrote: > > It looks like the super user check is out of a transaction, I haven't > > checked why it only failed on one BF animal, but it seems we can put > > the check into the

Re: Non-superuser subscription owners

2023-03-31 Thread Robert Haas
On Thu, Mar 30, 2023 at 9:49 PM houzj.f...@fujitsu.com wrote: > It looks like the super user check is out of a transaction, I haven't checked > why > it only failed on one BF animal, but it seems we can put the check into the > transaction like the following: That looks like a reasonable fix

RE: Non-superuser subscription owners

2023-03-30 Thread houzj.f...@fujitsu.com
On Friday, March 31, 2023 12:05 AM Robert Haas wrote: Hi, > > On Tue, Mar 28, 2023 at 1:52 PM Jeff Davis wrote: > > On Fri, 2023-03-24 at 00:17 -0700, Jeff Davis wrote: > > > The other patch you posted seems like it makes a lot of progress in > > > that direction, and I think that should go

Re: Non-superuser subscription owners

2023-03-30 Thread Robert Haas
On Tue, Mar 28, 2023 at 1:52 PM Jeff Davis wrote: > On Fri, 2023-03-24 at 00:17 -0700, Jeff Davis wrote: > > The other patch you posted seems like it makes a lot of progress in > > that direction, and I think that should go in first. That was one of > > the items I suggested previously[2], so

Re: Non-superuser subscription owners

2023-03-28 Thread Jeff Davis
On Fri, 2023-03-24 at 00:17 -0700, Jeff Davis wrote: > The other patch you posted seems like it makes a lot of progress in > that direction, and I think that should go in first. That was one of > the items I suggested previously[2], so thank you for working on > that. The above is not a hard

Re: Non-superuser subscription owners

2023-03-27 Thread Jeff Davis
On Mon, 2023-03-27 at 14:06 -0400, Robert Haas wrote: > I thought you were asking for those changes to be made before this > patch got committed, so that's what I was responding to. If you're > asking for it not to be committed at all, that's a different > discussion. I separately had a complaint

Re: Non-superuser subscription owners

2023-03-27 Thread Jeff Davis
On Mon, 2023-03-27 at 10:46 -0700, Andres Freund wrote: > > There are some big issues, like the security model for replaying > > changes. > > That seems largely unrelated. They are self-evidently related in a fundamental way. The behavior of the non-superuser-subscription patch depends on the

Re: Non-superuser subscription owners

2023-03-27 Thread Robert Haas
> (2) even adding the connection string security stuff > > I don't see how these points are related to the question of whether you > should commit your non-superuser-subscription-owners patch or logical- > repl-as-table-owner patch first. I thought you were asking for those change

Re: Non-superuser subscription owners

2023-03-27 Thread Andres Freund
role > > > (2) even adding the connection string security stuff > > I don't see how these points are related to the question of whether you > should commit your non-superuser-subscription-owners patch or logical- > repl-as-table-owner patch first. > > > My perspec

Re: Non-superuser subscription owners

2023-03-25 Thread Jeff Davis
ow these points are related to the question of whether you should commit your non-superuser-subscription-owners patch or logical- repl-as-table-owner patch first. My perspective is that logical replication is an unfinished feature with an incomplete design. As I said earlier, that's why I backed away from t

Re: Non-superuser subscription owners

2023-03-24 Thread Robert Haas
On Fri, Mar 24, 2023 at 9:24 AM Robert Haas wrote: > > The other patch you posted seems like it makes a lot of progress in > > that direction, and I think that should go in first. That was one of > > the items I suggested previously[2], so thank you for working on that. > > Perhaps you could

Re: Non-superuser subscription owners

2023-03-24 Thread Robert Haas
On Fri, Mar 24, 2023 at 3:17 AM Jeff Davis wrote: > The current patch (non-superuser-subscriptions) is the most user-facing > aspect, and it seems wrong to commit it before we have the security > model in a reasonable place. As you pointed out[1], it's not in a > reasonable place now, so

Re: Non-superuser subscription owners

2023-03-24 Thread Jeff Davis
On Wed, 2023-03-22 at 12:16 -0400, Robert Haas wrote: > I've posted a > patch for that at > http://postgr.es/m/CA+TgmoaSCkg9ww9oppPqqs+9RVqCexYCE6Aq=usypfnoode...@mail.gmail.com > and AFAICT everyone agrees with the idea, even if the patch itself > hasn't yet attracted any code reviews. But

Re: Non-superuser subscription owners

2023-03-23 Thread Robert Haas
On Thu, Mar 23, 2023 at 1:41 PM Jeff Davis wrote: > Even if my changes don't happen, I would find it less confusing and > more likely that users understand what they're doing. I respectfully but firmly disagree. I think having two predefined roles that are both required to create a subscription

Re: Non-superuser subscription owners

2023-03-23 Thread Jeff Davis
On Thu, 2023-03-23 at 11:52 -0400, Robert Haas wrote: > What would this amount to concretely? Also adding a > pg_connection_string predefined role and requiring both that and > pg_create_subscription [to CREATE SUBSCRIPTION] Yes. > If so, I don't think that's a good idea. Maybe for some reason

Re: Non-superuser subscription owners

2023-03-23 Thread Robert Haas
On Wed, Mar 22, 2023 at 3:53 PM Jeff Davis wrote: > Is there any chance I can convince you to separate the privileges of > using a connection string and creating a subscription, as I > suggested[1] earlier? What would this amount to concretely? Also adding a pg_connection_string predefined role

Re: Non-superuser subscription owners

2023-03-22 Thread Jeff Davis
On Wed, 2023-03-22 at 12:16 -0400, Robert Haas wrote: > If nobody's too unhappy with the idea, I plan to commit this soon, > both because I think that the feature is useful, and also because I > think it's an important security improvement. Is there any chance I can convince you to separate the

Re: Non-superuser subscription owners

2023-03-22 Thread Robert Haas
On Wed, Mar 8, 2023 at 2:47 PM Andres Freund wrote: > Hm - it still feels wrong that we error out in case of failure, despite the > comment to the function saying: > * Returns NULL on error and fills the err with palloc'ed error message. I've amended the comment so that it explains why it's

Re: Non-superuser subscription owners

2023-03-08 Thread Andres Freund
Hi, On 2023-02-07 16:56:55 -0500, Robert Haas wrote: > On Wed, Feb 1, 2023 at 4:02 PM Andres Freund wrote: > > > + /* Is the use of a password mandatory? */ > > > + must_use_password = MySubscription->passwordrequired && > > > + !superuser_arg(MySubscription->owner); > > > >

Re: Non-superuser subscription owners

2023-03-03 Thread Robert Haas
On Wed, Mar 1, 2023 at 7:34 PM Andres Freund wrote: > ISTM that this would require annotating most functions in the system. There's > many problems besides accessing database contents. Just a few examples: > > - dblink functions to access another system / looping back > -

Re: Non-superuser subscription owners

2023-03-01 Thread Andres Freund
Hi, On 2023-02-28 08:37:02 -0500, Robert Haas wrote: > On Mon, Feb 27, 2023 at 7:37 PM Jeff Davis wrote: > > > Yeah. That's the idea I was floating, at least. > > > > Isn't that a hard problem; maybe impossible? > > It doesn't seem that hard to me; maybe I'm missing something. > > The existing

Re: Non-superuser subscription owners

2023-03-01 Thread Andres Freund
Hi, On 2023-02-28 12:36:38 -0800, Jeff Davis wrote: > On Tue, 2023-02-28 at 11:28 -0800, Andres Freund wrote: > > I can only repeat myself in stating that SECURITY DEFINER solves none > > of the > > relevant issues. I included several examples of why it doesn't in the > > recent > > thread about

Re: Non-superuser subscription owners

2023-03-01 Thread Jeff Davis
On Wed, 2023-03-01 at 16:06 -0500, Robert Haas wrote: > To be fair, it's possible that there's no solution to this class of > problems that *doesn't* suck, but I think we should look a lot harder > before coming to that conclusion. Fair enough. The situation is bad enough that I'm willing to

Re: Non-superuser subscription owners

2023-03-01 Thread Robert Haas
On Wed, Mar 1, 2023 at 1:13 PM Jeff Davis wrote: > I don't think it's magic, as I said above. But I assume that your more > general point is that if we take some responsibility away from the > invoker and place it on the definer, then it creates room for new kinds > of problems. And I agree. > >

Re: Non-superuser subscription owners

2023-03-01 Thread Jeff Davis
On Wed, 2023-03-01 at 10:05 -0500, Robert Haas wrote: > For this reason, these expressions are, by > default, restricted from doing . The hard part is defining without resorting to a bunch of special cases, and also in a way that doesn't break a bunch of stuff. > You earlier > mentioned that

Re: Non-superuser subscription owners

2023-03-01 Thread Robert Haas
On Wed, Mar 1, 2023 at 10:48 AM Stephen Frost wrote: > > Yeah, or any other expressions. Basically impose restrictions when the > > user running the code is not the same as the user who provided the > > code. > > Would this have carve-outs for things like "except if the user providing > the code

Re: Non-superuser subscription owners

2023-03-01 Thread Stephen Frost
Greetings, * Robert Haas (robertmh...@gmail.com) wrote: > On Tue, Feb 28, 2023 at 4:01 PM Jeff Davis wrote: > > Or default expressions, I presume. If we at least agree on this point, > > then I think we should try to find a way to treat these other hunks of > > code in a secure way (which I

Re: Non-superuser subscription owners

2023-03-01 Thread Robert Haas
On Tue, Feb 28, 2023 at 4:01 PM Jeff Davis wrote: > You're suggesting using the SECURITY_RESTRICTED_OPERATION flag, along > with the new security flags, but not switch to the table owner, right? Correct. > Or default expressions, I presume. If we at least agree on this point, > then I think we

Re: Non-superuser subscription owners

2023-02-28 Thread Stephen Frost
Greetings, * Jeff Davis (pg...@j-davis.com) wrote: > On Tue, 2023-02-28 at 08:37 -0500, Robert Haas wrote: > > The existing SECURITY_RESTRICTED_OPERATION flag basically prevents > > you > > from tinkering with the session state. > > Currently, every time we set that flag we also run all the code

Re: Non-superuser subscription owners

2023-02-28 Thread Jeff Davis
On Tue, 2023-02-28 at 08:37 -0500, Robert Haas wrote: > The existing SECURITY_RESTRICTED_OPERATION flag basically prevents > you > from tinkering with the session state. Currently, every time we set that flag we also run all the code as the table owner. You're suggesting using the

Re: Non-superuser subscription owners

2023-02-28 Thread Jeff Davis
On Tue, 2023-02-28 at 11:28 -0800, Andres Freund wrote: > I can only repeat myself in stating that SECURITY DEFINER solves none > of the > relevant issues. I included several examples of why it doesn't in the > recent > thread about "blocking SECURITY INVOKER". E.g. that default arguments > of >

Re: Non-superuser subscription owners

2023-02-28 Thread Andres Freund
Hi, On 2023-02-22 09:18:34 -0800, Jeff Davis wrote: > I can't resist mentioning that these are all SECURITY INVOKER problems. > SECURITY INVOKER is insecure unless the invoker absolutely trusts the > definer, and that only really makes sense if the definer is a superuser > (or something very

Re: Non-superuser subscription owners

2023-02-28 Thread Robert Haas
On Mon, Feb 27, 2023 at 7:37 PM Jeff Davis wrote: > > Yeah. That's the idea I was floating, at least. > > Isn't that a hard problem; maybe impossible? It doesn't seem that hard to me; maybe I'm missing something. The existing SECURITY_RESTRICTED_OPERATION flag basically prevents you from

Re: Non-superuser subscription owners

2023-02-27 Thread Stephen Frost
Greetings, * Jeff Davis (pg...@j-davis.com) wrote: > Not all steps would be breaking changes, and a lot of those steps are > things we should do anyway. We could make it easier to write safe > SECURITY DEFINER functions, provide more tools for users to opt-out of > executing SECURITY INVOKER

Re: Non-superuser subscription owners

2023-02-27 Thread Stephen Frost
Greetings, * Jeff Davis (pg...@j-davis.com) wrote: > On Mon, 2023-02-27 at 14:10 -0500, Stephen Frost wrote: > > I do think there are some use-cases for it, but agree that it'd be > > better to encourage more use of SECURITY DEFINER and one approach to > > that might be to have a way for users to

Re: Non-superuser subscription owners

2023-02-27 Thread Jeff Davis
On Mon, 2023-02-27 at 16:13 -0500, Robert Haas wrote: > On Mon, Feb 27, 2023 at 1:25 PM Jeff Davis wrote: > > I think you are saying that we should still run Alice's code with > > the > > privileges of Bob, but somehow make that safe(r) for Bob. Is that > > right? > > Yeah. That's the idea I was

Re: Non-superuser subscription owners

2023-02-27 Thread Jeff Davis
On Mon, 2023-02-27 at 14:10 -0500, Stephen Frost wrote: > I do think there are some use-cases for it, but agree that it'd be > better to encourage more use of SECURITY DEFINER and one approach to > that might be to have a way for users to explicitly say "don't run > code > that isn't mine or a

Re: Non-superuser subscription owners

2023-02-27 Thread Robert Haas
On Mon, Feb 27, 2023 at 1:25 PM Jeff Davis wrote: > I think you are saying that we should still run Alice's code with the > privileges of Bob, but somehow make that safe(r) for Bob. Is that > right? Yeah. That's the idea I was floating, at least. > That sounds hard, and I'm still stuck at the

Re: Non-superuser subscription owners

2023-02-27 Thread Stephen Frost
Greetings, * Jeff Davis (pg...@j-davis.com) wrote: > On Mon, 2023-02-27 at 10:45 -0500, Robert Haas wrote: > > Suppose Alice owns a table and attaches a trigger to it. If > > Bob inserts into that table, I think we have to run the trigger, > > because Alice is entitled to assume that, for

Re: Non-superuser subscription owners

2023-02-27 Thread Jeff Davis
On Mon, 2023-02-27 at 10:45 -0500, Robert Haas wrote: > Suppose Alice owns a table and attaches a trigger to it. If > Bob inserts into that table, I think we have to run the trigger, > because Alice is entitled to assume that, for example, any BEFORE > triggers she might have defined that block

Re: Non-superuser subscription owners

2023-02-27 Thread Robert Haas
On Wed, Feb 22, 2023 at 12:18 PM Jeff Davis wrote: > There are two questions: > > 1. Is the security situation with logical replication bad? Yes. You > nicely summarized just how bad. > > 2. Is it the same situation as accessing a table owned by a user you > don't absolutely trust? > > Regardless

Re: Non-superuser subscription owners

2023-02-22 Thread Joe Conway
On 2/22/23 14:12, Mark Dilger wrote: On Feb 22, 2023, at 10:49 AM, Jeff Davis wrote: On Wed, 2023-02-22 at 09:27 -0800, Mark Dilger wrote: Another option is to execute under the intersection of their privileges, where both the definer and the invoker need the privileges in order for the action

Re: Non-superuser subscription owners

2023-02-22 Thread Mark Dilger
> On Feb 22, 2023, at 10:49 AM, Jeff Davis wrote: > > On Wed, 2023-02-22 at 09:27 -0800, Mark Dilger wrote: >> Another option is to execute under the intersection of their >> privileges, where both the definer and the invoker need the >> privileges in order for the action to succeed. That

Re: Non-superuser subscription owners

2023-02-22 Thread Jeff Davis
On Wed, 2023-02-22 at 09:27 -0800, Mark Dilger wrote: > Another option is to execute under the intersection of their > privileges, where both the definer and the invoker need the > privileges in order for the action to succeed.  That would be more > permissive than the proposed SECURITY NONE,

Re: Non-superuser subscription owners

2023-02-22 Thread Mark Dilger
> On Feb 22, 2023, at 9:18 AM, Jeff Davis wrote: > > Another option is having some kind SECURITY NONE that would run the > code as a very limited-privilege user that can basically only access > the catalog. That would be useful for running default expressions and > the like without the

Re: Non-superuser subscription owners

2023-02-22 Thread Jeff Davis
On Mon, 2023-02-06 at 14:40 -0500, Robert Haas wrote: > On Mon, Feb 6, 2023 at 2:18 PM Andres Freund > wrote: > > It's decidedly not great, yes. I don't know if it's quite a CVE > > type issue, > > after all, the same is true for any other type of query the > > superuser > > executes. But at the

Re: Non-superuser subscription owners

2023-02-07 Thread Robert Haas
On Wed, Feb 1, 2023 at 4:02 PM Andres Freund wrote: > On 2023-01-30 15:32:34 -0500, Robert Haas wrote: > > I had a long think about what to do with ALTER SUBSCRIPTION ... OWNER > > TO in terms of permissions checks. > > As long as owner and run-as are the same, I think it's strongly > preferrable

Re: Non-superuser subscription owners

2023-02-06 Thread Robert Haas
On Mon, Feb 6, 2023 at 2:18 PM Andres Freund wrote: > It's decidedly not great, yes. I don't know if it's quite a CVE type issue, > after all, the same is true for any other type of query the superuser > executes. But at the very least the documentation needs to be better, with a > big red box

Re: Non-superuser subscription owners

2023-02-06 Thread Andres Freund
, including but not restricted to the replication context. Some potentially relevant discussion is in this thread: https://postgr.es/m/75b0dbb55e9febea54c441efff8012a6d2cb5bd7.camel%40j-davis.com I don't agree with Jeff's proposal, but I think there's some worthwhile ideas in the idea + followups.

Re: Non-superuser subscription owners

2023-02-06 Thread Robert Haas
ble at any table when somebody does a REINDEX. And then, in master, where there's some provision for non-superuser subscription owners, we maybe need to re-think the privileges required to replicate into a table in the first place. I don't think that having I/U/D permissions on a table is

Re: Non-superuser subscription owners

2023-02-03 Thread Andres Freund
Hi, On 2023-02-02 09:28:03 -0500, Robert Haas wrote: > I don't know what you mean by this. DML doesn't confer privileges. If > code gets executed and runs with the replication user's credentials, > that could lead to privilege escalation, but just moving rows around > doesn't, at least not in the

Re: Non-superuser subscription owners

2023-02-02 Thread Robert Haas
On Wed, Feb 1, 2023 at 3:37 PM Andres Freund wrote: > The general point is that IMO is that in many setups you should use a > user with fewer privileges than a superuser. It doesn't really matter > whether we have an ad-hoc restriction for system catalogs. More often > than not being able to

Re: Non-superuser subscription owners

2023-02-02 Thread Robert Haas
On Wed, Feb 1, 2023 at 1:09 PM Mark Dilger wrote: > > On Feb 1, 2023, at 6:43 AM, Robert Haas wrote: > > The thing I'm > > struggling to understand is: if you only want to replicate into tables > > that Alice can write, why not just make Alice own the subscription? > > For a run-as user to make

Re: Non-superuser subscription owners

2023-02-01 Thread Andres Freund
Hi, On 2023-01-30 15:32:34 -0500, Robert Haas wrote: > I had a long think about what to do with ALTER SUBSCRIPTION ... OWNER > TO in terms of permissions checks. The previous version required that > the new owner have permissions of pg_create_subscription, but there > seems to be no particular

Re: Non-superuser subscription owners

2023-02-01 Thread Andres Freund
Hi, On 2023-02-01 09:43:39 -0500, Robert Haas wrote: > On Tue, Jan 31, 2023 at 7:01 PM Andres Freund wrote: > > I don't really understand that - the run-as approach seems like a > > necessary piece of improving the security model. > > > > I think it's perfectly reasonable to want to replicate

Re: Non-superuser subscription owners

2023-02-01 Thread Mark Dilger
> On Feb 1, 2023, at 6:43 AM, Robert Haas wrote: > The thing I'm > struggling to understand is: if you only want to replicate into tables > that Alice can write, why not just make Alice own the subscription? > For a run-as user to make sense, you need a scenario where we want the >

Re: Non-superuser subscription owners

2023-02-01 Thread Robert Haas
On Tue, Jan 31, 2023 at 7:01 PM Andres Freund wrote: > I don't really understand that - the run-as approach seems like a > necessary piece of improving the security model. > > I think it's perfectly reasonable to want to replicate from one system > in another, but to not want to allow logical

Re: Non-superuser subscription owners

2023-01-31 Thread Andres Freund
Hi, On 2023-01-30 10:44:29 -0500, Robert Haas wrote: > On a technical level, I think that the idea of having a separate > objection for the connection string vs. the subscription itself is > perfectly sound, and to repeat what I said earlier, if someone wants > to implement that, cool. I also

Re: Non-superuser subscription owners

2023-01-31 Thread Mark Dilger
> On Jan 30, 2023, at 1:29 PM, Robert Haas wrote: > > I feel like you're accusing me of removing functionality that has > never existed. A subscription doesn't run as the subscription creator. > It runs as the subscription owner. If you or anyone else had added the > capability for it to run

Re: Non-superuser subscription owners

2023-01-30 Thread Robert Haas
On Mon, Jan 30, 2023 at 3:27 PM Mark Dilger wrote: > That was an aspirational example in which there's infinite daylight between > the publisher and subscriber. I, too, doubt that's ever going to be > possible. But I still think we should aspire to some extra daylight between > the two.

Re: Non-superuser subscription owners

2023-01-30 Thread Robert Haas
On Fri, Jan 27, 2023 at 5:00 PM Andres Freund wrote: > > Or, another thought, maybe this should be checking for CREATE on the > > current database + also pg_create_subscription. That seems like it > > might be the right idea, actually. > > Yes, that seems like a good idea. Done in this version.

Re: Non-superuser subscription owners

2023-01-30 Thread Mark Dilger
> On Jan 30, 2023, at 11:30 AM, Robert Haas wrote: > > CREATE SUBSCRIPTION > provides no tools at all for filtering the data that the subscriber > chooses to send. > > Now that can be changed, I suppose, and a run-as user would be one way > to make progress in that direction. But I'm not

Re: Non-superuser subscription owners

2023-01-30 Thread Robert Haas
On Mon, Jan 30, 2023 at 1:46 PM Mark Dilger wrote: > I have a grim view of the requirement that publishers and subscribers trust > each other. Even when they do trust each other, they can firewall attacks by > acting as if they do not. I think it's OK if the CREATE PUBLICATION user doesn't

Re: Non-superuser subscription owners

2023-01-30 Thread Mark Dilger
> On Jan 30, 2023, at 9:26 AM, Robert Haas wrote: > > So basically this doesn't really feel like a valid scenario to me. > We're supposed to believe that Alice is hostile to Bob, but the > superuser doesn't seem to have thought very carefully about how Bob is > supposed to defend himself

Re: Non-superuser subscription owners

2023-01-30 Thread Mark Dilger
> On Jan 30, 2023, at 9:26 AM, Robert Haas wrote: > > First, it doesn't seem to make a lot of sense to have one person > managing the publications and someone else managing the subscriptions, > and especially if those parties are mutually untrusting. I can't think > of any real reason to set

Re: Non-superuser subscription owners

2023-01-30 Thread Robert Haas
On Mon, Jan 30, 2023 at 11:11 AM Mark Dilger wrote: > > On Jan 30, 2023, at 7:44 AM, Robert Haas wrote: > > > > And if we suppose that > > that already works and is safe, well then what's the case where I do > > need a run-as user? > > A) Alice publishes tables, and occasionally adds new tables

Re: Non-superuser subscription owners

2023-01-30 Thread Mark Dilger
> On Jan 30, 2023, at 7:44 AM, Robert Haas wrote: > > And if we suppose that > that already works and is safe, well then what's the case where I do > need a run-as user? A) Alice publishes tables, and occasionally adds new tables to existing publications. B) Bob manages subscriptions, and

Re: Non-superuser subscription owners

2023-01-30 Thread Robert Haas
On Fri, Jan 27, 2023 at 5:56 PM Mark Dilger wrote: > If the owner cannot modify the subscription, then the owner degenerates into > a mere "run-as" user. Note that this isn't how things work now, and even if > we disallowed owners from modifying the connection string, there would still > be

Re: Non-superuser subscription owners

2023-01-27 Thread Mark Dilger
> On Jan 27, 2023, at 1:35 PM, Robert Haas wrote: > >> I started out asking what benefits it provides to own a subscription one >> cannot modify. But I think it is a good capability to have, to restrict the >> set of relations that replication could target. Although perhaps it'd be >> better

Re: Non-superuser subscription owners

2023-01-27 Thread Andres Freund
Hi, On 2023-01-27 16:35:11 -0500, Robert Haas wrote: > > Maybe a daft question: > > > > Have we considered using a "normal grant", e.g. on the database, instead of > > a > > role? Could it e.g. be useful to grant a user the permission to create a > > subscription in one database, but not in

Re: Non-superuser subscription owners

2023-01-27 Thread Robert Haas
On Fri, Jan 27, 2023 at 4:09 PM Andres Freund wrote: > Hm, compared to postgres_fdw, the user has far less control over what's > happening using that connection. Is there a way a subscription owner can > trigger evaluation of near-arbitrary SQL on the publisher side? I'm not aware of one, but

Re: Non-superuser subscription owners

2023-01-27 Thread Andres Freund
Hi, On 2023-01-27 14:42:01 -0500, Robert Haas wrote: > At first, I found it a bit tempting to see this as a further > indication that the force-a-password approach is not the right idea, > because the test case clearly memorializes a desire *not* to require a > password in this situation.

Re: Non-superuser subscription owners

2023-01-27 Thread Robert Haas
On Thu, Jan 19, 2023 at 8:46 PM Andres Freund wrote: > > If we already had (or have) that logic someplace else, it would > > probably make sense to reuse it > > We hve. See at least postgres_fdw's check_conn_params(), dblink's > dblink_connstr_check() and dblink_security_check(). In the patch I

Re: Non-superuser subscription owners

2023-01-26 Thread Robert Haas
On Thu, Jan 26, 2023 at 12:36 PM Jeff Davis wrote: > On Thu, 2023-01-26 at 09:43 -0500, Robert Haas wrote: > > I have no issue with that as a long-term plan. However, I think that > > for right now we should just introduce pg_create_subscription. It > > would make sense to add

Re: Non-superuser subscription owners

2023-01-26 Thread Jeff Davis
On Thu, 2023-01-26 at 09:43 -0500, Robert Haas wrote: > I have no issue with that as a long-term plan. However, I think that > for right now we should just introduce pg_create_subscription. It > would make sense to add pg_create_connection in the same patch that > adds a CREATE CONNECTION command

Re: Non-superuser subscription owners

2023-01-26 Thread Robert Haas
On Wed, Jan 25, 2023 at 10:45 PM Jeff Davis wrote: > I propose that we have two predefined roles: pg_create_subscription, > and pg_create_connection. If creating a subscription with a connection > string, you'd need to be a member of both roles. But to create a > subscription with a server

Re: Non-superuser subscription owners

2023-01-25 Thread Jeff Davis
On Tue, 2023-01-24 at 17:00 -0500, Robert Haas wrote: > It seems to me that the relevant > question isn't "are the servers tightly coupled?" but rather "could > some user make a mess if we let them use any arbitrary connection > string?". The split I created is much easier for an admin to answer:

  1   2   3   >