Re: Non-superuser subscription owners

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>, Jeff Davis <pgsql(at)j-davis(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Non-superuser subscription owners
Date: 2023-01-20 16:08:54
Message-ID: CA+TgmoY=S3bHM7U=pzJjiQQ1ADQ4kcNV0zOBqDproHP1s-fFtg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jan 20, 2023 at 8:25 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> I still think you're talking about a different problem here. I'm
> talking about the problem of knowing whether local files are going to
> be accessed by the connection string.

So here's a dumb patch for this. At least in my mind, the connection
string sanitization/validation is the major design problem here, and
I'm not at all sure that what I did in the attached patch is right.
But let's talk about that. This approach is inspired by Jeff's
comments about local file access upthread, but as Andres pointed out,
that's a completely different set of things than we worry about in
other places. I'm not quite sure what the right model is here.

This patch incidentally allows ALTER SUBSCRIPTION .. SKIP for any
subscription owner, removing the existing check that limits that
operation to superusers and replacing it with nothing. I can't really
see why this needs to be any more restricted than that, and
regrettably neither the check in the existing code nor the commit that
added it have any comments explaining the logic behind that check. If,
for example, skipping a subscription could lead to a server crash,
that would be a reason to restrict the feature to superusers (or
revert it). If it's just a case of the operation being maybe not the
right thing to do, that's not a sufficient reason to restrict it to
superusers. This change is really independent of the rest of the patch
and, if we want to do this, I will separate it into its own patch. But
since this is just for discussion, I didn't worry about that right
now.

Aside from the above, I don't yet see a problem here that I would
consider to be serious enough that we couldn't proceed. I'll try to
avoid too much repetition of what's already been said on this topic,
but I do want to add that I think that creating subscriptions is
properly viewed as a *slightly* scary operation, not a *very* scary
operation. It lets you do two things that you couldn't otherwise. One
is get background processes running that take up process slots and
consume resources -- but note that your ability to consume resources
with however many normal database connections you can make is
virtually unlimited. The other thing it lets you do is poke around the
network, maybe figure out whether some ports are open or closed, and
try to replicate data from any accessible servers you can find, which
could include ports or servers that you can't access directly. I think
that the superuser will be in a good position to evaluate whether that
is a risk in a certain environment or not, and I think many superusers
will conclude that it isn't a big risk. I think that the main
motivation for NOT handing out pg_create_subscription will turn out to
be administrative rather than security-related i.e. they'll want to be
something that falls under their authority rather than someone else's.

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

Attachment Content-Type Size
v1-0001-Add-new-predefined-role-pg_create_subscriptions.patch application/octet-stream 10.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2023-01-20 16:42:33 Re: run pgindent on a regular basis / scripted manner
Previous Message Greg Stark 2023-01-20 16:08:32 Re: Experiments with Postgres and SSL