On Sat, Mar 25, 2023 at 5:24 AM Jelte Fennema <postg...@jeltef.nl> wrote:
> For my purposes I always trust the publisher, what I don't trust is
> the table owners. But I can indeed imagine scenarios where that's the
> other way around, and indeed you can protect against that currently,
> but not with your new patch. That seems fairly easily solvable though.
>
> + if (!member_can_set_role(context->save_userid, userid))
> + ereport(ERROR,
> + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> + errmsg("role \"%s\" cannot SET ROLE to \"%s\"",
> + GetUserNameFromId(context->save_userid, false),
> + GetUserNameFromId(userid, false))));
>
> If we don't throw an error here, but instead simply return, then the
> current behaviour is preserved and people can manually configure
> permissions to protect against an untrusted publisher. This would
> still mean that the table owners can escalate privileges to the
> subscription owner, but if that subscription owner actually has fewer
> privileges than the table owner then you don't have that issue.
I don't get it. If we just return, that would result in skipping
changes rather than erroring out on changes, but it wouldn't preserve
the current behavior, because we'd still care about the table owner's
permissions rather than, as now, the subscription owner's permissions.
--
Robert Haas
EDB: http://www.enterprisedb.com
