From 7a958edb5af210d3b9dfb3dc62c7ca419f0c8ca2 Mon Sep 17 00:00:00 2001
From: Matthew Blewitt <blewitt.matt@gmail.com>
Date: Tue, 15 Nov 2022 21:38:14 +0000
Subject: [PATCH] gatekeeper: disallow reading rolpassword in elevated contexts

It is possible to write a function that might extract password hashes
(or plaintext passwords, if enabled) from `pg_authid` that could be
executed in an elevated context. For the sake of example:

```
mblewitt=# \df+ steal_yer_passwords
List of functions
-[ RECORD 1 ]-------+-------------------------------------------------------------------------------------------------------------------------------------------
Schema              | public
Name                | steal_yer_passwords
Result data type    | void
Argument data types |
Type                | func
Volatility          | volatile
Parallel            | unsafe
Owner               | mblewitt
Security            | definer
Access privileges   |
Language            | plpgsql
Source code         |  BEGIN CREATE TABLE IF NOT EXISTS yer_passwords AS SELECT rolname, rolpassword FROM pg_authid; GRANT ALL ON yer_passwords TO foobar; END;
Description         |
```

Without this patch, it is possible to execute this func as an
unprivileged user and extract the information:

```
mblewitt=> select current_user;
 current_user
--------------
 foobar
(1 row)
```

```
mblewitt=> select * from pg_authid;
ERROR:  permission denied for table pg_authid
```

```
mblewitt=> select steal_yer_passwords();
 steal_yer_passwords
---------------------

(1 row)
```

```
mblewitt=> select * from yer_passwords where rolname = 'victim';
 rolname |             rolpassword
---------+-------------------------------------
 victim  | md5710772761ee5086c0a06cfef4cb4256d
(1 row)
```

With this patch, we prevent such access in an elevated context:

```
mblewitt=> select steal_yer_passwords();
ERROR:  Reading pg_authid sensitive columns is not allowed in elevated context
CONTEXT:  SQL statement "CREATE TABLE IF NOT EXISTS yer_passwords AS SELECT rolname, rolpassword FROM pg_authid"
PL/pgSQL function steal_yer_passwords() line 1 at SQL statement
```

This guards only against reading `rolpassword`, so other operations are
fine in an elevated context:

```
mblewitt=> \df+ baz_the_qux;
List of functions
-[ RECORD 1 ]-------+--------------------------------
Schema              | public
Name                | baz_the_qux
Result data type    | void
Argument data types |
Type                | func
Volatility          | volatile
Parallel            | unsafe
Owner               | mblewitt
Security            | definer
Access privileges   |
Language            | sql
Source code         |  SELECT rolname FROM pg_authid
Description         |
```

```
mblewitt=> select baz_the_qux();
-[ RECORD 1 ]-
baz_the_qux |
```
---
 .gitignore             |  2 ++
 src/aiven_gatekeeper.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/.gitignore b/.gitignore
index 1d74e21..a792d84 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1 +1,3 @@
 .vscode/
+*.o
+*.so
diff --git a/src/aiven_gatekeeper.c b/src/aiven_gatekeeper.c
index be375db..6101680 100644
--- a/src/aiven_gatekeeper.c
+++ b/src/aiven_gatekeeper.c
@@ -73,6 +73,10 @@ static int min_reserved_oid = 9000;
 static const char *reserved_col_names[] = {"proowner", "proacl", "prolang", "prosecdef"};
 static const int NUM_RESERVED_COLS = sizeof reserved_col_names / sizeof reserved_col_names[0];
 
+/* reserved columns in the pg_authid table that aren't permitted to be read */
+static const char *reserved_auth_col_names[] = {"rolpassword"};
+static const int NUM_RESERVED_AUTH_COLS = sizeof reserved_auth_col_names / sizeof reserved_auth_col_names[0];
+
 /* GUC Variables */
 static bool pg_security_agent_enabled = false;
 static bool pg_security_agent_strict = false;
@@ -580,8 +584,46 @@ pg_proc_guard_checks(QueryDesc *queryDesc, int eflags)
     {
         switch (queryDesc->operation)
         {
+        case CMD_SELECT:
+            foreach (resultRelations, queryDesc->plannedstmt->rtable)
+            {
+                rt = lfirst(resultRelations);
+                switch (rt->relid)
+                {
+                case 1260: // pg_authid
+                    colset = rt->selectedCols;
+                    index = -1;
+                    while ((index = bms_next_member(colset, index)) >= 0)
+                    {
+                        AttrNumber attno = index + FirstLowInvalidHeapAttributeNumber;
+                        char *attname;
+                        int i;
+
+                        /* get the column name, function definition changed with PG11 */
+#if PG11_GTE
+                        attname = get_attname(1260, attno, true);
+#else
+                        attname = get_attname(1260, attno);
+#endif
+                        /* check if column is reserved */
+                        for (i = 0; i < NUM_RESERVED_AUTH_COLS; i++)
+                        {
+                            if (strncmp(reserved_auth_col_names[i], attname, 10) == 0 && (pg_security_agent_strict || creating_extension || is_elevated() || is_security_restricted()))
+                            {
+                                elog(ERROR, "Reading pg_authid sensitive columns is not allowed in elevated context");
+                                return;
+                            }
+                        }
+                    }
+                    break;
+                default:
+                    break;
+                }
+            }
+            break;
         case CMD_INSERT:
         case CMD_UPDATE:
+        case CMD_DELETE:
             foreach (resultRelations, queryDesc->plannedstmt->rtable)
             {
                 rt = lfirst(resultRelations);