On Thu, Jan 19, 2023 at 8:46 PM Andres Freund <and...@anarazel.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
v2-0001-Add-new-predefined-role-pg_create_subscriptions.patch
Description: Binary data
