Skip to content

Commit

Permalink
gatekeeper: disallow reading rolpassword in elevated contexts
Browse files Browse the repository at this point in the history
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 |
```
  • Loading branch information
mble committed Nov 16, 2022
1 parent 65c5076 commit 7a958ed
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 0 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
.vscode/
*.o
*.so
42 changes: 42 additions & 0 deletions src/aiven_gatekeeper.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit 7a958ed

Please sign in to comment.