Hi, On 2023-01-19 10:45:35 -0500, Robert Haas wrote: > On Wed, Jan 18, 2023 at 3:58 PM Mark Dilger > <mark.dil...@enterprisedb.com> wrote: > > > On Jan 18, 2023, at 12:51 PM, Robert Haas <robertmh...@gmail.com> wrote: > > > > > > Unless I'm missing something, it seems like this could be a quite small > > > patch. > > > > I didn't like the idea of the create/alter subscription commands needing to > > parse the connection string and think about what it might do, because at > > some point in the future we might extend what things are allowed in that > > string, and we have to keep everything that contemplates that string in > > sync. I may have been overly hesitant to tackle that problem. Or maybe I > > just ran short of round tuits. > > I wouldn't be OK with writing our own connection string parser for > this purpose, but using PQconninfoParse seems OK. We still have to > embed knowledge of which connection string parameters can trigger > local file access, but that doesn't seem like a massive problem to me.
> 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(). As part of the fix for https://postgr.es/m/20220925232237.p6uskba2dw6fnwj2%40awork3.anarazel.de I am planning to introduce a bunch of server side helpers for dealing with libpq (for establishing a connection while accepting interrupts). We could try to centralize knowledge for those checks there. The approach of checking, after connection establishment (see dblink_security_check()), that we did in fact use the specified password, scares me somewhat. See also below. > The basic idea that by looking at which connection string properties are set > we can tell what kinds of things the connection string is going to do seems > sound to me. I don't think you *can* check it purely based on existing connection string properties, unfortunately. Think of e.g. a pg_hba.conf line of "local all user peer" (quite reasonable config) or "host all all 127.0.0.1/32 trust" (less so). Hence the hack with dblink_security_check(). I think there might be a discussion somewhere about adding an option to force libpq to not use certain auth methods, e.g. plaintext password/md5. It's possible this could be integrated. Greetings, Andres Freund
