On 12/18/21 02:34, Alvaro Herrera wrote:
On 2021-Dec-17, Tomas Vondra wrote:
On 12/17/21 22:07, Alvaro Herrera wrote:
So I've been thinking about this as a "security" item (you can see my
comments to that effect sprinkled all over this thread), in the sense
that if a publication "hides" some column, then the replica just won't
get access to it. But in reality that's mistaken: the filtering that
this patch implements is done based on the queries that *the replica*
executes at its own volition; if the replica decides to ignore the list
of columns, it'll be able to get all columns. All it takes is an
uncooperative replica in order for the lot of data to be exposed anyway.
Interesting, I haven't really looked at this as a security feature. And in
my experience if something is not carefully designed to be secure from the
get go, it's really hard to add that bit later ...
I guess the way to really harden replication is to use the GRANT system
at the publisher's side to restrict access for the replication user.
This would provide actual security. So you're right that I seem to be
barking at the wrong tree ... maybe I need to give a careful look at
the documentation for logical replication to understand what is being
offered, and to make sure that we explicitly indicate that limiting the
column list does not provide any actual security.
You say it's the replica making the decisions, but my mental model is it's
the publisher decoding the data for a given list of publications (which
indeed is specified by the subscriber). But the subscriber can't tweak the
definition of publications, right? Or what do you mean by queries executed
by the replica? What are the gap?
I am thinking in somebody modifying the code that the replica runs, so
that it ignores the column list that the publication has been configured
to provide; instead of querying only those columns, it would query all
columns.
If the server has a *separate* security mechanism to hide the columns
(per-column privs), it is that feature that will protect the data, not
the logical-replication-feature to filter out columns.
Right. Although I haven't thought about how logical decoding interacts with
column privileges. I don't think logical decoding actually checks column
privileges - I certainly don't recall any ACL checks in
src/backend/replication ...
Well, in practice if you're confronted with a replica that's controlled
by a malicious user that can tweak its behavior, then replica-side
privilege checking won't do anything useful.
I don't follow. Surely the decoding happens on the primary node, right?
Which is where the ACL checks would happen, using the role the
replication connection is opened with.
This led me to realize that the replica-side code in tablesync.c is
totally oblivious to what's the publication through which a table is
being received from in the replica. So we're not aware of a replica
being exposed only a subset of columns through some specific
publication; and a lot more hacking is needed than this patch does, in
order to be aware of which publications are being used.
Does that mean we currently sync all the columns in the initial sync, and
only start filtering columns later while decoding transactions?
No, it does filter the list of columns in the initial sync. But the
current implementation is bogus, because it obtains the list of *all*
publications in which the table is published, not just the ones that the
subscription is configured to get data from. And the sync code doesn't
receive the list of publications. We need more thorough patching of the
sync code to close that hole.
Ah, got it. Thanks for the explanation. Yeah, that makes no sense.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
