On Sat, Mar 25, 2023 at 3:16 PM Jeff Davis <pg...@j-davis.com> wrote: > On Fri, 2023-03-24 at 09:24 -0400, Robert Haas wrote: > > I certainly agree that the security model isn't in a reasonable place > > right now. However, I feel that: > > > > (1) adding an extra predefined 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.
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. > 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 trying to do non-superuser subscriptions as a documented feature: > it feels like we need to settle some of the underlying pieces first. I kind of agree with you about the feature itself. Even though the basic feature works quite well and does something people really want, there are a lot of loose ends to sort out, and not just about security. But I also want to make some progress. If there are problems with what I'm proposing that will make us regret committing things right before feature freeze, then we shouldn't. But waiting a whole additional year to see any kind of improvement is not free; these issues are serious. > I don't mean to say all of the above issues are blockers or that they > should all be resolved in my favor. But there are enough issues and > some of those issues are serious enough that I feel like it's premature > to just go ahead with the non-superuser subscriptions and the > predefined role. > > There are already users, which complicates things. And you make a good > point that some important users may be already working around the > flaws. But there's already a patch and discussion going on for some > security model improvements (thanks to you), so let's try to get that > one in first. If we can't, it's probably because we learned something > important. I think this patch is a lot better-baked and less speculative than that one. I think that patch is more important, so if they were equally mature, I'd favor getting that one committed first. But that's not the case. Also, I don't really understand how we could end up not wanting this patch. I mean there's a lot of things I don't understand that are still true anyway, so the mere fact that I don't understand how we could not end up wanting this patch doesn't mean that it couldn't happen. But like, the current state of play is that subscription owners are always going to be superusers at the time the subscription is created, and literally nobody thinks that's a good idea. Some people (like me) think that we ought to assume that subscription owners will be and need to be high-privilege users like superusers, but to my knowledge every such person thinks that it's OK for the subscription owner to be a non-superuser if they have adequate privileges. I just think that's a high amount of privileges, not that it has to be all the privileges i.e. superuser. Other people (like you, AIUI) think that we ought to try to set things up so that subscription owners can be low-privilege users, in which case we, once again, don't want the user who owns the subscription to start out a superuser. I actually can't imagine anyone defending the idea of having the subscription owner always be a superuser at the time they first own the subscription. That's a weird rule that can only serve to reduce security. Nor can I imagine anyone saying that forcing subscriptions to be created only by superusers improves security. I don't think anyone thinks that. If we're going to delay this patch, probably for a full year, because of other ongoing discussions, it should be because there is some outcome of those discussions that would involve deciding that this patch isn't needed or should be significantly redesigned. If this patch is going to end up being desirable no matter how those discussions turn out, and if it's not going to change significantly no matter how those discussions turn out, then those discussions aren't a reason not to get it into this release. -- Robert Haas EDB: http://www.enterprisedb.com
