Enforce search_path=pg_catalog for non-replication connections (#2496)

There is a known [vector of attact](https://pganalyze.com/blog/5mins-postgres-security-patch-releases-pgspot-pghostile) by creating functions and/or operators in a public scheme with the same name and signature as corresponding objects in `pg_catalog`.

Since Patroni is heavily relying on superuser connections we want to mitigate it by enforcing `search_path=pg_catalog` for all connections created by Patroni (except replication connections). It is achieved by introducing a new function, that wraps psycopg.connect() and appends ` -c search_path=pg_catalog` to `options` parameter.

In addition to that, we set connection.autocommit to True before returning it.

patroni/api.py

@@ -629,7 +629,7 @@ class RestApiHandler(BaseHTTPRequestHandler):
             stmt = ("SELECT " + postgresql.POSTMASTER_START_TIME + ", " + postgresql.TL_LSN + ","
                     " pg_catalog.pg_last_xact_replay_timestamp(),"
                     " pg_catalog.array_to_json(pg_catalog.array_agg(pg_catalog.row_to_json(ri))) "
-                    "FROM (SELECT (SELECT rolname FROM pg_authid WHERE oid = usesysid) AS usename,"
+                    "FROM (SELECT (SELECT rolname FROM pg_catalog.pg_authid WHERE oid = usesysid) AS usename,"
                     " application_name, client_addr, w.state, sync_state, sync_priority"
                     " FROM pg_catalog.pg_stat_get_wal_senders() w, pg_catalog.pg_stat_get_activity(pid)) AS ri")
 

patroni/ctl.py

@@ -274,7 +274,6 @@ def get_cursor(cluster, connect_parameters, role='master', member=None):
 
     from . import psycopg
     conn = psycopg.connect(**params)
-    conn.autocommit = True
     cursor = conn.cursor()
     if role == 'any':
         return cursor

patroni/postgresql/connection.py

@@ -22,7 +22,6 @@ class Connection(object):
         with self._lock:
             if not self._connection or self._connection.closed != 0:
                 self._connection = psycopg.connect(**self._conn_kwargs)
-                self._connection.autocommit = True
                 self.server_version = self._connection.server_version
         return self._connection
 
@@ -42,7 +41,6 @@ class Connection(object):
 @contextmanager
 def get_connection_cursor(**kwargs):
     conn = psycopg.connect(**kwargs)
-    conn.autocommit = True
     with conn.cursor() as cur:
         yield cur
     conn.close()

patroni/psycopg.py

@@ -6,7 +6,7 @@ try:
     from . import MIN_PSYCOPG2, parse_version
     if parse_version(__version__) < MIN_PSYCOPG2:
         raise ImportError
-    from psycopg2 import connect, Error, DatabaseError, OperationalError, ProgrammingError
+    from psycopg2 import connect as _connect, Error, DatabaseError, OperationalError, ProgrammingError
     from psycopg2.extensions import adapt
 
     try:
@@ -20,10 +20,10 @@ try:
             value.prepare(conn)
         return value.getquoted().decode('utf-8')
 except ImportError:
-    from psycopg import connect as _connect, sql, Error, DatabaseError, OperationalError, ProgrammingError
+    from psycopg import connect as __connect, sql, Error, DatabaseError, OperationalError, ProgrammingError
 
-    def connect(*args, **kwargs):
-        ret = _connect(*args, **kwargs)
+    def _connect(*args, **kwargs):
+        ret = __connect(*args, **kwargs)
         ret.server_version = ret.pgconn.server_version  # compatibility with psycopg2
         return ret
 
@@ -34,6 +34,16 @@ except ImportError:
         return sql.Literal(value).as_string(conn)
 
 
+def connect(*args, **kwargs):
+    if kwargs and 'replication' not in kwargs and kwargs.get('fallback_application_name') != 'Patroni ctl':
+        options = [kwargs['options']] if 'options' in kwargs else []
+        options.append('-c search_path=pg_catalog')
+        kwargs['options'] = ' '.join(options)
+    ret = _connect(*args, **kwargs)
+    ret.autocommit = True
+    return ret
+
+
 def quote_ident(value, conn=None):
     if _legacy or conn is None:
         return '"{0}"'.format(value.replace('"', '""'))

patroni/scripts/wale_restore.py

@@ -214,26 +214,26 @@ class WALERestore(object):
         attempts_no = 0
         while True:
             if self.master_connection:
+                con = None
                 try:
                     # get the difference in bytes between the current WAL location and the backup start offset
-                    with psycopg.connect(self.master_connection) as con:
-                        if con.server_version >= 100000:
-                            wal_name = 'wal'
-                            lsn_name = 'lsn'
-                        else:
-                            wal_name = 'xlog'
-                            lsn_name = 'location'
-                        con.autocommit = True
-                        with con.cursor() as cur:
-                            cur.execute(("SELECT CASE WHEN pg_catalog.pg_is_in_recovery()"
-                                         " THEN GREATEST(pg_catalog.pg_{0}_{1}_diff(COALESCE("
-                                         "pg_last_{0}_receive_{1}(), '0/0'), %s)::bigint, "
-                                         "pg_catalog.pg_{0}_{1}_diff(pg_catalog.pg_last_{0}_replay_{1}(), %s)::bigint)"
-                                         " ELSE pg_catalog.pg_{0}_{1}_diff(pg_catalog.pg_current_{0}_{1}(), %s)::bigint"
-                                         " END").format(wal_name, lsn_name),
-                                        (backup_start_lsn, backup_start_lsn, backup_start_lsn))
+                    con = psycopg.connect(self.master_connection)
+                    if con.server_version >= 100000:
+                        wal_name = 'wal'
+                        lsn_name = 'lsn'
+                    else:
+                        wal_name = 'xlog'
+                        lsn_name = 'location'
+                    with con.cursor() as cur:
+                        cur.execute(("SELECT CASE WHEN pg_catalog.pg_is_in_recovery()"
+                                     " THEN GREATEST(pg_catalog.pg_{0}_{1}_diff(COALESCE("
+                                     "pg_last_{0}_receive_{1}(), '0/0'), %s)::bigint, "
+                                     "pg_catalog.pg_{0}_{1}_diff(pg_catalog.pg_last_{0}_replay_{1}(), %s)::bigint)"
+                                     " ELSE pg_catalog.pg_{0}_{1}_diff(pg_catalog.pg_current_{0}_{1}(), %s)::bigint"
+                                     " END").format(wal_name, lsn_name),
+                                    (backup_start_lsn, backup_start_lsn, backup_start_lsn))
 
-                            diff_in_bytes = int(cur.fetchone()[0])
+                        diff_in_bytes = int(cur.fetchone()[0])
                 except psycopg.Error:
                     logger.exception('could not determine difference with the master location')
                     if attempts_no < self.retries:  # retry in case of a temporarily connection issue
@@ -246,6 +246,9 @@ class WALERestore(object):
                         logger.info("continue with base backup from S3 since master is not available")
                         diff_in_bytes = 0
                         break
+                finally:
+                    if con:
+                        con.close()
             else:
                 # always try to use WAL-E if master connection string is not available
                 diff_in_bytes = 0