From 44e7b9e5ba4449e10c0cd788534ac38f222fc68f Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter_e@gmx.net>
Date: Mon, 13 Feb 2017 16:28:36 -0500
Subject: [PATCH v2 4/5] 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 want to 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 4c8d454c9e..89b4335d00 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -322,7 +322,9 @@ <title>Security</title>
 
   <para>
    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.
   </para>
 
   <para>
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 c3e54af259..7534702df8 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);
@@ -607,6 +608,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);
@@ -716,6 +718,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.12.0

