| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
| Cc: | Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Add a hook for handling logical decoding messages on subscribers. |
| Date: | 2026-10-16 20:49:23 |
| Message-ID: | CAD21AoC4TbKu8aAdSWR1a9udFULeC5-x3vxi24uPMf25BUkJWg@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sun, Sep 20, 2026 at 11:24 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Sep 18, 2026 at 6:13 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> >
> > Thanks for reviewing the patch!
> >
> > While I also think the patch is in good shape, I'd like to raise a
> > security risk this feature might introduce, particularly around
> > pg_logical_emit_message():
> >
>
> Review comments:
> ===============
> 1. Typo (/logcal/logical): <function>pg_logcal_emit_message()</function></link>.
Fixed.
>
> 2.
> + * The handler always runs with the privileges of the subscription owner.
> + * The payload is chosen by whoever called pg_logical_emit_message() on the
> + * publisher, which by default any role may do, so a handler that acts on the
> + * payload gives every role on the publisher the subscription owner's
> + * privileges. A handler that needs less than that can lower them itself
> + * with SwitchToUntrustedUser().
>
> One thing to consider about this is advice is that unlike DML there's
> no target role to derive, and SwitchToUntrustedUser() errors outright
> if the subscription owner can't SET ROLE to whatever the handler
> picks. So, it doesn't sound straight-forward to follow this advice.
Right, but I think the error isn't specific to the hoo. When the apply
worker applies a change, it also switches to the table owner, and
errors could happen if the subscription owner cannot SET ROLE to the
table owner. I think it would be an extension's responsibility that
the apply worker switch to a role, whereas it's a extension user's
responsibility that the subscription owner can SET ROLE to that role.
>
> 3.
> Restrict who can emit messages
> + on the publisher. Revoke the default privilege and grant it to a
> role created
> + for this purpose:
> +<programlisting>
> +REVOKE EXECUTE ON FUNCTION pg_logical_emit_message(boolean, text,
> text, boolean) FROM PUBLIC;
> +REVOKE EXECUTE ON FUNCTION pg_logical_emit_message(boolean, text,
> bytea, boolean) FROM PUBLIC;
> +GRANT EXECUTE ON FUNCTION pg_logical_emit_message(boolean, text,
> text, boolean) TO message_emitter;
> +GRANT EXECUTE ON FUNCTION pg_logical_emit_message(boolean, text,
> bytea, boolean) TO message_emitter;
> +</programlisting>
>
> That above security recipe sounds reasonable, but it asks the
> subscriber's security to be enforced by an action in a cluster the
> subscriber admin frequently does not control. The Security section
> should say something on the lines of: if you don't administer both the
> subscriber and publisher, don't install a message handler that does
> anything you wouldn't let every publisher role do. Also, I think the
> above REVOKE has to be performed in all databases separately.
Agreed on both, I'll add that.
>
> 4. Based on above two points, I think we can fill the hole with something like:
>
> CREATE SUBSCRIPTION ... WITH (messages = true, message_handler_role =
> 'msg_handler')
>
> and then run the hook under SwitchToUntrustedUser(that_role). That
> gives the subscriber admin a local, explicit privilege mechanism that
> does not depend on the publisher's ACLs, doesn't touch WAL or the
> protocol, and reuses machinery already in usercontext.c. If the option
> is unset, either don't call the hook or keep today's behavior with the
> documented warning.
I think it works for the cases where the handler doesn't or forget to
switch the role when applying the message. But I think the handler
would have to switch the role to the target role that is suitable for
doing its job in most cases. So I think the option gives us two SET
ROLE chains instead of one. Today the subscription owner needs to be
able to SET ROLE to each target role, which is the same requirement we
already document for table owners. With message_handler_role the owner
needs SET ROLE to the handler role, and the handler role needs SET
ROLE to every target role. That's noe more role and one more place for
the DBA to get it wrong, and either mistake shows up as an apply
error.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Zsolt Parragi | 2026-10-16 21:09:26 | [PG19] pg_upgrade transfers temporal keys using gist_inet_ops with the wrong opclass |
| Previous Message | Dmitry Dolgov | 2026-10-16 20:41:27 | Re: BUG #19736: `JSON_OBJECT ... ABSENT ON NULL` accepts invalid keys when the value is NULL |