Re: Non-superuser subscription owners

On Sat, Mar 25, 2023 at 3:16 PM Jeff Davis <pgsql(at)j-davis(dot)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