From 4766aa28c9f95d2c65eaeb25303b410d6d76e5b4 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Mon, 13 Feb 2017 16:28:36 -0500 Subject: [PATCH 5/6] Add subscription apply worker privilege checks The subscription apply worker now checks INSERT/UPDATE/DELETE privileges on a table before writing to it. This was previously not checked, but was not necessary since a subscription owner had to be a superuser. But we now allow other users to own subscriptions. --- doc/src/sgml/logical-replication.sgml | 4 +++- src/backend/replication/logical/relation.c | 24 ++++++++++++++++++++++++ src/backend/replication/logical/worker.c | 3 +++ src/include/replication/logicalrelation.h | 5 +++++ 4 files changed, 35 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml index 98411af0e6..262008ee3b 100644 --- a/doc/src/sgml/logical-replication.sgml +++ b/doc/src/sgml/logical-replication.sgml @@ -326,7 +326,9 @@ Security The subscription apply process will run in the local database with the - privileges of a superuser. + privileges of the subscription owner. The subscription owner, if it is not + a superuser, therefore needs to have appropriate privileges on the tables + the subscription is operating on. diff --git a/src/backend/replication/logical/relation.c b/src/backend/replication/logical/relation.c index d8dc0c7194..4faee8323c 100644 --- a/src/backend/replication/logical/relation.c +++ b/src/backend/replication/logical/relation.c @@ -19,6 +19,7 @@ #include "access/heapam.h" #include "access/sysattr.h" #include "catalog/namespace.h" +#include "miscadmin.h" #include "nodes/makefuncs.h" #include "replication/logicalrelation.h" #include "replication/worker_internal.h" @@ -78,6 +79,18 @@ logicalrep_relmap_invalidate_cb(Datum arg, Oid reloid) } /* + * Syscache invalidation callback for our relation map cache. + * + * Same as the relcache callback, except that we just invalidate all cache + * entries all the time. + */ +static void +logicalrep_relmap_syscache_invalidate_cb(Datum arg, int cacheid, uint32 hashvalue) +{ + logicalrep_relmap_invalidate_cb(arg, InvalidOid); +} + +/* * Initialize the relation map cache. */ static void @@ -113,6 +126,8 @@ logicalrep_relmap_init() /* Watch for invalidation events. */ CacheRegisterRelcacheCallback(logicalrep_relmap_invalidate_cb, (Datum) 0); + CacheRegisterSyscacheCallback(AUTHOID, logicalrep_relmap_syscache_invalidate_cb, + (Datum) 0); CacheRegisterSyscacheCallback(TYPEOID, logicalrep_typmap_invalidate_cb, (Datum) 0); } @@ -277,6 +292,15 @@ logicalrep_rel_open(LogicalRepRelId remoteid, LOCKMODE lockmode) remoterel->nspname, remoterel->relname))); /* + * Cache ACL results. If they are changed while the worker is active, + * the relcache and syscache invalidation will ensure that we get here + * again to recompute this. + */ + entry->insert_aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_INSERT); + entry->update_aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_UPDATE); + entry->delete_aclresult = pg_class_aclcheck(relid, GetUserId(), ACL_DELETE); + + /* * Build the mapping of local attribute numbers to remote attribute * numbers and validate that we don't miss any replicated columns * as that would result in potentially unwanted data loss. diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c index 0b19feca40..69c4770bea 100644 --- a/src/backend/replication/logical/worker.c +++ b/src/backend/replication/logical/worker.c @@ -515,6 +515,7 @@ apply_handle_insert(StringInfo s) relid = logicalrep_read_insert(s, &newtup); rel = logicalrep_rel_open(relid, RowExclusiveLock); + aclcheck_error(rel->insert_aclresult, ACL_KIND_CLASS, get_rel_name(rel->localreloid)); /* Initialize the executor state. */ estate = create_estate_for_relation(rel); @@ -603,6 +604,7 @@ apply_handle_update(StringInfo s) relid = logicalrep_read_update(s, &has_oldtup, &oldtup, &newtup); rel = logicalrep_rel_open(relid, RowExclusiveLock); + aclcheck_error(rel->update_aclresult, ACL_KIND_CLASS, get_rel_name(rel->localreloid)); /* Check if we can do the update. */ check_relation_updatable(rel); @@ -708,6 +710,7 @@ apply_handle_delete(StringInfo s) relid = logicalrep_read_delete(s, &oldtup); rel = logicalrep_rel_open(relid, RowExclusiveLock); + aclcheck_error(rel->delete_aclresult, ACL_KIND_CLASS, get_rel_name(rel->localreloid)); /* Check if we can do the delete. */ check_relation_updatable(rel); diff --git a/src/include/replication/logicalrelation.h b/src/include/replication/logicalrelation.h index 7fb7fbfb4d..f9d5144f61 100644 --- a/src/include/replication/logicalrelation.h +++ b/src/include/replication/logicalrelation.h @@ -25,6 +25,11 @@ typedef struct LogicalRepRelMapEntry * remote ones */ bool updatable; /* Can apply updates/deletes? */ + /* Cache of ACL results */ + AclResult insert_aclresult; + AclResult update_aclresult; + AclResult delete_aclresult; + /* Sync state. */ char state; XLogRecPtr statelsn; -- 2.11.1