Re: running logical replication as the subscription owner - Mailing list pgsql-hackers

On Mon, Mar 27, 2023 at 04:47:22PM -0400, Robert Haas wrote:
> So that's a grey area, at least IMHO. The patch could be revised in
> some way, and the permissions requirements downgraded. However, if we
> do that, I think we're going to have to document that although in
> theory table owners can make themselves against subscription owners,
> in practice they probably won't be. That approach has some advantages,
> and I don't think it's insane. However, I am not convinced that it is
> the best idea, either, and I had the impression based on
> pgsql-security discussion that Andres and Noah thought this way was
> better. I might have misinterpreted their position, and they might
> have changed their minds, and they might have been wrong. But that's
> how we got here.

["this way" = requirement for SET ROLE]

On Wed, Mar 29, 2023 at 04:00:45PM -0400, Robert Haas wrote:
> > The dangerous cases seem to be something along the lines of a security-
> > invoker trigger function that builds and executes arbirary SQL based on
> > the row contents. And then the table owner would then still need to set
> > ENABLE ALWAYS TRIGGER.
> >
> > Do we really want to take that case on as our security responsibility?
> 
> That's something about which I would like to get more opinions.

The most-plausible-to-me attack involves an ENABLE ALWAYS trigger that logs
CURRENT_USER to an audit table.  The "SQL based on the row contents" scenario
feels remote.  Another remotely-possible attack involves a trigger that
internally queries some other table having RLS.  (Switching to the table owner
can change the rows visible to that query.)

If having INSERT/UPDATE privileges on the table were enough to make a
subscription that impersonates the table owner, then relatively-unprivileged
roles could make a subscription to bypass the aforementioned auditing.  Commit
c3afe8c has imposed weighty requirements beyond I/U privileges, namely holding
the pg_create_subscription role and database-level CREATE privilege.  Since
database-level CREATE is already powerful, it would be plausible to drop the
SET ROLE requirement and add this audit bypass to its powers.  The SET ROLE
requirement is nice for keeping the powers disentangled.  One drawback is
making people do GRANTs regardless of whether a relevant audit trigger exists.
Another drawback is the subscription role having more privileges than ideally
needed.  I do like keeping strong privileges orthogonal, so I lean toward
keeping the SET ROLE requirement.

On Thu, Mar 30, 2023 at 02:17:31PM -0400, Robert Haas wrote:
> --- a/doc/src/sgml/logical-replication.sgml
> +++ b/doc/src/sgml/logical-replication.sgml
> @@ -1774,6 +1774,23 @@ CONTEXT:  processing remote data for replication origin "pg_16395" during "INSER
>     <literal>SET ROLE</literal> to each role that owns a replicated table.
>    </para>
>  
> +  <para>
> +   If the subscription has been configued with

Typo.

> Subject: [PATCH v3 1/2] Perform logical replication actions as the table
>  owner.

> Since this involves switching the active user frequently within
> a session that is authenticated as the subscription user, also
> impose SECURITY_RESTRICTED_OPEATION restrictions on logical

s/OPEATION/OPERATION/

> replication code. As an exception, if the table owner can SET
> ROLE to the subscription owner, these restrictions have no
> security value, so don't impose them in that case.
> 
> Subscription owners are now required to have the ability to
> SET ROLE to every role that owns a table that the subscription
> is replicating. If they don't, replication will fail. Superusers,
> who normally own subscriptions, satisfy this property by default.
> Non-superusers users who own subscriptions will needed to be
> granted the roles that own relevant tables.

s/will needed/will need/

(I did not read the patches in their entirety.)