Re: Non-superuser subscription owners

On Thu, Jan 19, 2023 at 8:46 PM Andres Freund <andres(at)anarazel(dot)de> 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 posted previously, I had some other set of checks, more
or less along the lines suggested by Jeff. I looked into revising that
approach and making the behavior match exactly what we do in those
places instead. I find that it breaks 027_nosuperuser.pl.
Specifically, where without the patch I get "ok 6 - nosuperuser admin
with all table privileges can replicate into unpartitioned", with the
patch it goes boom, because the subscription can't connect any more
due to the password requirement.

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. However, the loopback-to-superuser attack
is just as viable for subscription as it in other cases, and my
previous patch would have done nothing to block it. So what I did
instead is add a password_required attribute, just like what
postgres_fdw has. As in the case of postgres_fdw, the actual rule is
that if the attribute is false, a password is not required, and if the
attribute is true, a password is required unless you are a superuser.
If you're a superuser, it still isn't. This is a slightly odd set of
semantics but it has precedent and practical advantages. Also, as in
the case of postgres_fdw, only a superuser can set
password_required=false, and a subscription that has that setting can
only be modified by a superuser, no matter who owns it.

Even though I hate the require-a-password stuff with the intensity of
a thousand suns, I think this is better than the previous patch,
because it's more consistent with what we do elsewhere and because it
blocks the loopback-connection-to-superuser attack. I think we
*really* need to develop a better system for restricting proxied
connections (no matter how proxied) and I hope that we do that soon.
But inventing something for this purpose that differs from what we do
elsewhere will make that task harder, not easier.

Thoughts?

--
Robert Haas
EDB: http://www.enterprisedb.com