Re: running logical replication as the subscription owner

On Tue, Mar 28, 2023 at 1:38 AM Jeff Davis <pgsql@j-davis.com> wrote:
> >  If they do have those things then in
> > theory they might be able to protect themselves, but in practice they
> > are unlikely to be careful enough. I judge that practically every
> > installation where table owners use triggers would be easily
> > compromised. Only the most security-conscious of table owners are
> > going to get this right.
>
> This is the interesting part.
>
> Commit 11da97024ab set the subscription process's search path as empty.
> It seems it was done to protect the subscription owner against users
> writing into the public schema. But after your apply-as-table-owner
> patch, it seems to also offer some protection for table owners against
> a malicious subscription owner, too.
>
> But I must be missing something obvious here -- if the subscription
> process has an empty search_path, what can an attacker do to trick it?

Oh, interesting. I hadn't realized we were doing that, and I do think
that narrows the vulnerability quite a bit.

But I still feel pretty uncomfortable with the idea of requiring only
INSERT/UPDATE/DELETE permissions on the target table. Let's suppose
that you create a table and attach a trigger to it which is SECURITY
INVOKER. Absent logical replication, you don't really need to worry
about whether that function is written securely -- it will run with
privileges of the person performing the DML, and not impact your
account at all. They have reason to be scared of you, but not the
reverse. However, if the people doing DML on the table can arrange to
perform that DML as you, then any security vulnerabilities in your
function potentially allow them to compromise your account. Now, there
might not be any, but there also might be some, depending on exactly
what your function does. And if logical replication into a table
requires only I/U/D permission then it's basically a back-door way to
run functions that someone could otherwise execute only as themselves
as the table owner, and that's scary.

Now, I don't know how much to worry about that. As you just pointed
out to me in some out-of-band discussion, this is only going to affect
a table owner who writes a trigger, makes it insecure in some way
other than failure to sanitize the search_path, and sets it ENABLE
ALWAYS TRIGGER or ENABLE REPLICA TRIGGER. And maybe we could say that
if you do that last part, you kind of need to think about the
consequences for logical replication. If so, we could document that
problem away. It would also affect someone who uses a default
expression or other means of associating executable code with the
table, and something like a default expression doesn't require any
explicit setting to make it apply in case of replication, so maybe the
risk of someone messing up is a bit higher.

But this definitely makes it more of a judgment call than I thought
initially. Functions that are vulnerable to search_path exploits are a
dime a dozen; other kinds of exploits are going to be less common, and
more dependent on exactly what the function is doing.

I'm still inclined to leave the patch checking for SET ROLE, because
after all, we're thinking of switching user identities to the table
owner, and checking whether we can SET ROLE is the most natural way of
doing that, and definitely doesn't leave room to escalate to any
privilege you don't already have. However, there might be an argument
that we ought to do something else, like have a REPLICATE privilege on
the table that the owner can grant to users that they trust to
replicate into that table. Because time is short, I'd like to leave
that idea for a future release. What I would like to change in the
patch in this release is to add a new subscription property along the
lines of what I proposed to Jelte in my earlier email: let's provide
an escape hatch that turns off the user-switching behavior. If we do
that, then anyone who feels themselves worse off after this patch can
switch back to the old behavior. Most people will be better off, I
believe, and the opportunity to make things still better in the future
is not foreclosed.

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