Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PostgreSQL grants #5130

Merged
merged 4 commits into from
Oct 27, 2023
+104 −91
Merged

Fix PostgreSQL grants #5130

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a99f685
Add missing pgsql `USAGE` privilege
yhabteab Sep 28, 2023
329fd9e
DbTool: Enhance issuing and verifying pgsql privileges
yhabteab Sep 28, 2023
bcdad8c
MigrationManager: Enhance how pgsql privileges are checked
yhabteab Sep 28, 2023
ae9575e
DbResourcePage: Don't fail validation if connecting to pgsql wasn't s…
yhabteab Sep 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions library/Icinga/Application/MigrationManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ public function yieldMigrations(bool $modules = false): Generator
*/
public function getRequiredDatabasePrivileges(): array
{
return ['CREATE','SELECT','INSERT','UPDATE','DELETE','DROP','ALTER','CREATE VIEW','INDEX','EXECUTE'];
return ['CREATE','SELECT','INSERT','UPDATE','DELETE','DROP','ALTER','CREATE VIEW','INDEX','EXECUTE','USAGE'];
}

/**
Expand Down Expand Up @@ -258,7 +258,11 @@ public function fixIcingaWebMysqlGrants(Sql\Connection $db, array $elevateConfig
$tool = $this->createDbTool($db);
$tool->connectToDb();

if ($tool->checkPrivileges(['SELECT'], [], $actualUsername)) {
$isPgsql = $db->getAdapter() instanceof Sql\Adapter\Pgsql;
// PgSQL doesn't have SELECT privilege on a database level and granting the CREATE,CONNECT, and TEMPORARY
// privileges on a database doesn't permit a user to read data from a table. Hence, we have to grant the
// required database,schema and table privileges simultaneously.
if (! $isPgsql && $tool->checkPrivileges(['SELECT'], [], $actualUsername)) {
// Checks only database level grants. If this succeeds, the grants were issued manually.
if (! $tool->checkPrivileges($privileges, [], $actualUsername) && $tool->isGrantable($privileges)) {
// Any missing grant is now granted on database level as well, not to mix things up
Expand Down Expand Up @@ -334,13 +338,20 @@ protected function checkRequiredPrivileges(

$dbTool = $this->createDbTool($conn);
$dbTool->connectToDb();
if (! $dbTool->checkPrivileges($this->getRequiredDatabasePrivileges())
&& ! $dbTool->checkPrivileges($this->getRequiredDatabasePrivileges(), $tables)
) {

$isPgsql = $conn->getAdapter() instanceof Sql\Adapter\Pgsql;
$privileges = $this->getRequiredDatabasePrivileges();
$dbPrivilegesGranted = $dbTool->checkPrivileges($privileges);
$tablePrivilegesGranted = $dbTool->checkPrivileges($privileges, $tables);
if (! $dbPrivilegesGranted && ($isPgsql || ! $tablePrivilegesGranted)) {
return false;
}

if ($isPgsql && ! $tablePrivilegesGranted) {
nilmerg marked this conversation as resolved.
Show resolved Hide resolved
return false;
}

if ($canIssueGrants && ! $dbTool->isGrantable($this->getRequiredDatabasePrivileges())) {
if ($canIssueGrants && ! $dbTool->isGrantable($privileges)) {
return false;
}

Expand Down
16 changes: 8 additions & 8 deletions modules/setup/application/forms/DbResourcePage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,14 @@ protected function validateConfiguration()

if ($this->getValue('db') === 'pgsql') {
if ($connectionError !== null) {
$this->warning(sprintf(
$this->translate('Unable to check the server\'s version. This is usually not a critical error'
. ' as there is probably only access to the database permitted which does not exist yet. If you are'
. ' absolutely sure you are running PostgreSQL in a version equal to or newer than 9.1,'
. ' you can skip the validation and safely proceed to the next step. The error was: %s'),
$connectionError->getMessage()
));
$state = false;
nilmerg marked this conversation as resolved.
Show resolved Hide resolved
// $this->warning(sprintf(
// $this->translate('Unable to check the server\'s version. This is usually not a critical error'
// . ' as there is probably only access to the database permitted which does not exist yet. If you are'
// . ' absolutely sure you are running PostgreSQL in a version equal to or newer than 9.1,'
// . ' you can skip the validation and safely proceed to the next step. The error was: %s'),
// $connectionError->getMessage()
// ));
// $state = false;
} else {
$version = $db->getServerVersion();
if (version_compare($version, '9.1', '<')) {
Expand Down
148 changes: 77 additions & 71 deletions modules/setup/library/Setup/Utils/DbTool.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,14 +99,7 @@ class DbTool
protected $pgsqlGrantContexts = array(
'ALL' => 63,
'ALL PRIVILEGES' => 63,
'SELECT' => 24,
'INSERT' => 24,
'UPDATE' => 24,
'DELETE' => 8,
'TRUNCATE' => 8,
'REFERENCES' => 24,
'TRIGGER' => 8,
'CREATE' => 12,
'CREATE' => 13,
'CONNECT' => 4,
'TEMPORARY' => 4,
'TEMP' => 4,
Expand Down Expand Up @@ -633,13 +626,21 @@ public function grantPrivileges(array $privileges, array $context, $username)
}
} elseif ($this->config['db'] === 'pgsql') {
$dbPrivileges = array();
$tablePrivileges = array();
$schemaPrivileges = [];
foreach (array_intersect($privileges, array_keys($this->pgsqlGrantContexts)) as $privilege) {
if (! empty($context) && $this->pgsqlGrantContexts[$privilege] & static::TABLE_LEVEL) {
$tablePrivileges[] = $privilege;
} elseif ($this->pgsqlGrantContexts[$privilege] & static::DATABASE_LEVEL) {
if ($this->pgsqlGrantContexts[$privilege] & static::DATABASE_LEVEL) {
$dbPrivileges[] = $privilege;
}

if ($this->pgsqlGrantContexts[$privilege] & static::GLOBAL_LEVEL) {
$schemaPrivileges[] = $privilege;
}
}

if (! empty($schemaPrivileges)) {
// Allow the user to create,alter and use all attribute types in schema public
// such as creating and dropping custom data types (boolenum)
$this->exec(sprintf('GRANT %s ON SCHEMA public TO %s', implode(',', $schemaPrivileges), $username));
}

if (! empty($dbPrivileges)) {
Expand All @@ -651,15 +652,10 @@ public function grantPrivileges(array $privileges, array $context, $username)
));
}

if (! empty($tablePrivileges)) {
foreach ($context as $table) {
$this->exec(sprintf(
'GRANT %s ON TABLE %s TO %s',
join(',', $tablePrivileges),
$table,
$username
));
}
foreach ($context as $table) {
// PostgreSQL documentation says "You must own the table to use ALTER TABLE.", hence it isn't
// sufficient to just issue grants, as the user is still not allowed to alter that table.
$this->exec(sprintf('ALTER TABLE %s OWNER TO %s', $table, $username));
}
}
}
Expand Down Expand Up @@ -854,57 +850,71 @@ public function checkPgsqlPrivileges(
$username = null
) {
$privilegesGranted = true;
$owner = $username ?: $this->config['username'];
$isSuperUser = $this->query('select rolsuper from pg_roles where rolname = :user', [':user' => $owner])
->fetchColumn();

if ($this->dbFromConfig) {
$schemaPrivileges = [];
$dbPrivileges = array();
$tablePrivileges = array();
foreach (array_intersect($privileges, array_keys($this->pgsqlGrantContexts)) as $privilege) {
if (! empty($context) && $this->pgsqlGrantContexts[$privilege] & static::TABLE_LEVEL) {
$tablePrivileges[] = $privilege;
}
if ($this->pgsqlGrantContexts[$privilege] & static::DATABASE_LEVEL) {
$dbPrivileges[] = $privilege;
if (! $isSuperUser) {
foreach (array_intersect($privileges, array_keys($this->pgsqlGrantContexts)) as $privilege) {
if ($this->pgsqlGrantContexts[$privilege] & static::DATABASE_LEVEL) {
$dbPrivileges[] = $privilege;
}
if ($this->pgsqlGrantContexts[$privilege] & static::GLOBAL_LEVEL) {
$schemaPrivileges[] = $privilege;
}
}
}

if (! empty($dbPrivileges)) {
$dbExclusivesGranted = true;
foreach ($dbPrivileges as $dbPrivilege) {
$query = $this->query(
'SELECT has_database_privilege(:user, :dbname, :privilege) AS db_privilege_granted',
array(
':user' => $username !== null ? $username : $this->config['username'],
':dbname' => $this->config['dbname'],
':privilege' => $dbPrivilege . ($requireGrants ? ' WITH GRANT OPTION' : '')
)
);
if (! $query->fetchObject()->db_privilege_granted) {
$privilegesGranted = false;
if (! in_array($dbPrivilege, $tablePrivileges)) {
$dbExclusivesGranted = false;
if (! empty($schemaPrivileges)) {
foreach ($schemaPrivileges as $schemaPrivilege) {
$query = $this->query(
'SELECT has_schema_privilege(:user, :schema, :privilege) AS db_privilege_granted',
[
':user' => $owner,
':schema' => 'public',
':privilege' => $schemaPrivilege . ($requireGrants ? ' WITH GRANT OPTION' : '')
]
);

if (! $query->fetchObject()->db_privilege_granted) {
// The user doesn't fully have the provided privileges.
$privilegesGranted = false;
break;
}
}
}

if ($privilegesGranted) {
// Do not check privileges twice if they are already granted at database level
$tablePrivileges = array_diff($tablePrivileges, $dbPrivileges);
} elseif ($dbExclusivesGranted) {
$privilegesGranted = true;
}
}

if ($privilegesGranted && !empty($tablePrivileges)) {
foreach (array_intersect($context, $this->listTables()) as $table) {
foreach ($tablePrivileges as $tablePrivilege) {
if ($privilegesGranted && ! empty($dbPrivileges)) {
foreach ($dbPrivileges as $dbPrivilege) {
$query = $this->query(
'SELECT has_table_privilege(:user, :table, :privilege) AS table_privilege_granted',
'SELECT has_database_privilege(:user, :dbname, :privilege) AS db_privilege_granted',
array(
':user' => $username !== null ? $username : $this->config['username'],
':table' => $table,
':privilege' => $tablePrivilege . ($requireGrants ? ' WITH GRANT OPTION' : '')
':user' => $owner,
':dbname' => $this->config['dbname'],
':privilege' => $dbPrivilege . ($requireGrants ? ' WITH GRANT OPTION' : '')
)
);
$privilegesGranted &= $query->fetchObject()->table_privilege_granted;
if (! $query->fetchObject()->db_privilege_granted) {
// The user doesn't fully have the provided privileges.
$privilegesGranted = false;
break;
}
}
}

if ($privilegesGranted && ! empty($context)) {
foreach (array_intersect($context, $this->listTables()) as $table) {
$query = $this->query(
'SELECT tableowner FROM pg_catalog.pg_tables WHERE tablename = :tablename',
[':tablename' => $table]
);

if ($query->fetchColumn() !== $owner) {
$privilegesGranted = false;
break;
}
}
}
}
Expand All @@ -914,31 +924,27 @@ public function checkPgsqlPrivileges(
// as the chances are very high that the database is created later causing the current user being
// the owner with ALL privileges. (Which in turn can be granted to others.)

if (array_search('CREATE', $privileges, true) !== false) {
if (in_array('CREATE', $privileges, true)) {
$query = $this->query(
'select rolcreatedb from pg_roles where rolname = :user',
array(':user' => $username !== null ? $username : $this->config['username'])
);
$privilegesGranted &= $query->fetchColumn() !== false;
$privilegesGranted = $query->fetchColumn() !== false;
}
}

if (array_search('CREATEROLE', $privileges, true) !== false) {
if ($privilegesGranted && in_array('CREATEROLE', $privileges, true)) {
$query = $this->query(
'select rolcreaterole from pg_roles where rolname = :user',
array(':user' => $username !== null ? $username : $this->config['username'])
);
$privilegesGranted &= $query->fetchColumn() !== false;
$privilegesGranted = $query->fetchColumn() !== false;
}

if (array_search('SUPER', $privileges, true) !== false) {
$query = $this->query(
'select rolsuper from pg_roles where rolname = :user',
array(':user' => $username !== null ? $username : $this->config['username'])
);
$privilegesGranted &= $query->fetchColumn() !== false;
if ($privilegesGranted && in_array('SUPER', $privileges, true)) {
$privilegesGranted = $isSuperUser === true;
}

return (bool) $privilegesGranted;
return $privilegesGranted;
}
}
1 change: 1 addition & 0 deletions modules/setup/library/Setup/WebWizard.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class WebWizard extends Wizard implements SetupWizard
'ALTER',
'DROP',
'INDEX',
'USAGE', // PostgreSQL
'TEMPORARY', // PostgreSql
'CREATE TEMPORARY TABLES' // MySQL
);
Expand Down
7 changes: 1 addition & 6 deletions phpstan-baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25527,7 +25527,7 @@ parameters:

-
message: "#^Cannot call method fetchColumn\\(\\) on mixed\\.$#"
count: 8
count: 9
path: modules/setup/library/Setup/Utils/DbTool.php

-
Expand Down Expand Up @@ -25660,11 +25660,6 @@ parameters:
count: 1
path: modules/setup/library/Setup/Utils/DbTool.php

-
message: "#^Parameter \\#1 \\$array of function array_intersect expects array, array\\|null given\\.$#"
count: 1
path: modules/setup/library/Setup/Utils/DbTool.php

-
message: "#^Parameter \\#1 \\$dbname of method Icinga\\\\Module\\\\Setup\\\\Utils\\\\DbTool\\:\\:pdoConnect\\(\\) expects string, string\\|null given\\.$#"
count: 1
Expand Down