On Wed, Mar 8, 2023 at 2:47 PM Andres Freund <andres@anarazel.de> 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 done that way.
> Other than this, the change looks ready to me.
Well, it still needed documentation changes and pg_dump changes. I've
added those in the attached version.
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. Since replication is
currently run as the subscription owner, any table owner can
compromise the subscription owner's account, which is really bad, but
if the subscription owner can be a non-superuser, it's a little bit
less bad. From a security point of view, I think the right thing to do
and what would improve security a lot more is to run replication as
the table owner rather than the subscription owner. I've posted a
patch for that at
http://postgr.es/m/CA+TgmoaSCkg9ww9oppPqqs+9RVqCexYCE6Aq=UsYPfnOoDeFkw@mail.gmail.com
and AFAICT everyone agrees with the idea, even if the patch itself
hasn't yet attracted any code reviews. But although the two patches
are fairly closely related, this seems to be a good idea whether that
moves forward or not, and that seems to be a good idea whether this
moves forward or not. As this one has had more review and discussion,
my current thought is to try to get this one committed first.
--
Robert Haas
EDB: http://www.enterprisedb.com