On Fri, Sep 22, 2023 at 10:59 AM Benoit Lobréau <benoit.lobr...@dalibo.com> wrote: > You're right, it comes from the connection to drop the slot. > > But the code in for DropSubscription in > src/backend/commands/subscriptioncmds.c tries to connect before testing > if the slot is NONE / NULL. So it doesn't work to DISABLE the > subscription and set the slot to NONE.
So I'm seeing this: if (!slotname && rstates == NIL) { table_close(rel, NoLock); return; } load_file("libpqwalreceiver", false); wrconn = walrcv_connect(conninfo, true, must_use_password, subname, &err); That looks like it's intended to return if there's nothing to do, and the comments say as much. But that (!slotname && rstates == NIL) test looks sketchy. It seems like we should bail out early if *either* !slotname *or* rstates == NIL, or for that matter if all of the rstates have rstate->relid == 0 or rstate->state == SUBREL_STATE_SYNCDONE. Maybe we need to push setting up the connection inside the foreach(lc, rstates) loop and do it only once we're sure we want to do something. Or at least, I don't understand why we don't bail out immediately in all cases where slotname is NULL, regardless of rstates. Am I missing something here? > Reading the code, I think I understand why the postgres user cannot drop > the slot: > > * the owner is sub_owner (not a superuser) > * and form->subpasswordrequired is true > > Should there be a test to check if the user executing the query is > superuser? maybe it's handled differently? (I am not very familiar with > the code). I think that there normally shouldn't be any problem here, because if form->subpasswordrequired is true, we expect that the connection string should contain a password which the remote side actually uses, or we expect the subscription to be owned by the superuser. If neither of those things is the case, then either the superuser made a subscription that doesn't use a password owned by a non-superuser without fixing subpasswordrequired, or else the configuration on the remote side has changed so that it now doesn't use the password when formerly it did. In the first case, perhaps it would be fine to go ahead and drop the slot, but in the second case I don't think it's OK from a security point view, because the command is going to behave the same way no matter who executes the drop command, and a non-superuser who drops the slot shouldn't be permitted to rely on the postgres processes's identity to do anything on a remote node -- including dropping a relation slot. So I tentatively think that this behavior is correct. > I dont understand (yet?) why I can do ALTER SUBSCRIPTIONs after changing > the ownership to an unpriviledged user (must_use_password should be true > also in that case). Maybe you're altering it in a way that doesn't involve any connections or any changes to the connection string? There's no security issue if, say, you rename it. -- Robert Haas EDB: http://www.enterprisedb.com