Merge pull request #19 from aiven/allowed-superusers

allow defining a list of allowed superusers

.github/workflows/build.yml

@@ -16,12 +16,14 @@ jobs:
     strategy:
       max-parallel: 3
       matrix:
-        pg-version: [10, 11, 12, 13, 14]
+        pg-version: [11, 12, 13, 14, 15]
     steps:
       - id: install
         run: |
           # Remove preinstalled Postgres because this will conflict with the version we actually want.
           sudo apt-get remove -u postgresql\*
+          # Get the postgresql gpg key
+          sudo apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 7FCC7D46ACCC4CF8
           # Setup the Postgres repositories
           sudo sh -c 'echo "deb https://apt.postgresql.org/pub/repos/apt $(lsb_release -cs)-pgdg main" > /etc/apt/sources.list.d/pgdg.list'
           sudo apt-get update

src/aiven_gatekeeper.c

@@ -26,6 +26,7 @@
 #include "utils/guc.h"
 #include "utils/fmgrtab.h"
 #include "utils/lsyscache.h"
+#include "utils/varlena.h"
 #include "nodes/nodes.h"
 #include "access/sysattr.h"
 
@@ -80,12 +81,18 @@ static const int NUM_RESERVED_AUTH_COLS = sizeof reserved_auth_col_names / sizeo
 /* GUC Variables */
 static bool pg_security_agent_enabled = false;
 static bool pg_security_agent_strict = false;
+static char *allowed_superuser_roles = NULL;
 
 /* Saved hook values in case of unload */
 static ProcessUtility_hook_type prev_ProcessUtility = NULL;
 static object_access_hook_type next_object_access_hook = NULL;
 static ExecutorStart_hook_type prev_ExecutorStart_hook = NULL;
 
+/* bug that breaks some extension functionality due to nested queries inadvertently
+    reading, but not using, a reserved column name
+*/
+static bool BUG_01 = true;
+
 static bool
 allowed_guc_change_check_hook(bool *newval, void **extra, GucSource source)
 {
@@ -99,6 +106,14 @@ allowed_guc_change_check_hook(bool *newval, void **extra, GucSource source)
      */
     return !(pg_security_agent_strict || creating_extension || is_security_restricted() || is_elevated());
 }
+
+static bool
+allowed_guc_change_allowed_superusers(char **newval, void **extra, GucSource source)
+{
+    /* same as with the boolean version */
+    return !(pg_security_agent_strict || creating_extension || is_security_restricted() || is_elevated());
+}
+
 /* returns true if the session and current user ids are different */
 static bool
 is_elevated(void)
@@ -136,6 +151,31 @@ is_security_restricted(void)
     return InSecurityRestrictedOperation();
 }
 
+/* check if a target role is in the list of roles that are permitted to have superuser */
+static bool
+allow_superuser_role(const char *target_role)
+{
+    List *allowed_superuser_list;
+    ListCell *role;
+
+    if (allowed_superuser_roles)
+    {
+        SplitIdentifierString(pstrdup(allowed_superuser_roles), ',', &allowed_superuser_list);
+
+        foreach (role, allowed_superuser_list)
+        {
+            char *allowed_role = (char *)lfirst(role);
+            if (strcmp(target_role, allowed_role) == 0)
+            {
+                list_free(allowed_superuser_list);
+                return true;
+            }
+        }
+        list_free(allowed_superuser_list);
+    }
+    return false;
+}
+
 static char *
 allow_role_stmt(void)
 {
@@ -261,6 +301,10 @@ gatekeeper_checks(PROCESS_UTILITY_PARAMS)
             // superuser or nosuperuser is supplied (both are treated as defname superuser) and check that the arg is set to true
             if (strncmp(defel->defname, "superuser", 10) == 0 && defGetBoolean(defel))
             {
+                // regardless of context (elevated privilege or not), check if the target role is allowed to be superuser
+                if (!allow_superuser_role(alterRoleStmt->role->rolename))
+                    elog(ERROR, "Role %s not in permitted superuser list", alterRoleStmt->role->rolename);
+
                 result = allow_role_stmt();
                 if (result != NULL)
                     elog(ERROR, "%s", result);
@@ -277,6 +321,10 @@ gatekeeper_checks(PROCESS_UTILITY_PARAMS)
             // check if we are granting superuser
             if (strncmp(defel->defname, "superuser", 10) == 0 && defGetBoolean(defel))
             {
+                // regardless of context (elevated privilege or not), check if the target role is allowed to be superuser
+                if (!allow_superuser_role(createRoleStmt->role))
+                    elog(ERROR, "Role %s not in permitted superuser list", createRoleStmt->role);
+
                 result = allow_role_stmt();
                 if (result != NULL)
                     elog(ERROR, "%s", result);
@@ -580,7 +628,7 @@ pg_proc_guard_checks(QueryDesc *queryDesc, int eflags)
     int index;
 
     /* only check function if security agent is enabled */
-    if (pg_security_agent_enabled)
+    if (pg_security_agent_enabled && !BUG_01)
     {
         switch (queryDesc->operation)
         {
@@ -706,7 +754,21 @@ void _PG_init(void)
                              NULL,
                              NULL);
 
+    // comma-separated list of allowed superuser roles (can be assigned superuser)
+    DefineCustomStringVariable("aiven.pg_security_agent_reserved_roles",
+                               "Comma-separated list of roles that can be assigned superuser",
+                               NULL,
+                               &allowed_superuser_roles,
+                               "postgres",         // default to postgres
+                               PGC_POSTMASTER,     // only at postmaster startup
+                               GUC_SUPERUSER_ONLY, // only show to superuser
+                               allowed_guc_change_allowed_superusers,
+                               NULL,
+                               NULL);
+
     // allow toggling of the security agent
+    // this variable definition should always be last, otherwise further defines
+    // stop working because the agent has defaulted to strict = on
     DefineCustomBoolVariable("aiven.pg_security_agent_strict",
                              "Toggle the agent into strict mode. Reserved actions are blocked regardless of context",
                              NULL,
@@ -718,6 +780,7 @@ void _PG_init(void)
                              NULL,
                              NULL);
 
+
     if (set_reserved_oids())
     {
         /* Install Hooks */