From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Joe Conway <mail@joeconway.com>
Cc: Joshua Brindle <joshua.brindle@crunchydata.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: use has_privs_of_role() for pg_hba.conf
Date: Fri, 7 Oct 2022 14:58:36 -0700
Message-ID: <20221007215836.GB814385@nathanxps13> (raw)
In-Reply-To: <3347532.1665173939@sss.pgh.pa.us>
References: <CAGB+Vh5epaDEu7aRokcuc_a7yG0B49u9ebpdViJHpyFN-exJtQ@mail.gmail.com>
<20220404142551.GB663209@nathanxps13>
<Yz6NRrfd7n2AXoHh@paquier.xyz>
<491dba16-55df-d5fd-1ae7-01b5140987f0@joeconway.com>
<20221006174343.GA444656@nathanxps13>
<Yz+Jt9537vjfO733@paquier.xyz>
<20221007032711.GA499771@nathanxps13>
<Yz/Ii6dr1CSu4Bnv@paquier.xyz>
<20221007194430.GA550749@nathanxps13>
<3347532.1665173939@sss.pgh.pa.us>
On Fri, Oct 07, 2022 at 04:18:59PM -0400, Tom Lane wrote:
> There's another problem there, which is that buildfarm animals
> using -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS will complain
> about role names that don't start with "regress_".
Huh, I hadn't noticed that one before. It looks like roles must start with
"regress_" and database names must include "regression", so I ended up
using "regress_regression_group" for the samegroup/samerole tests.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.comAttachments:v5-0001-Use-has_privs_of_role-for-samerole-samegroup-and-.patch (text/x-diff, 10.5K) download
From 6fe6da4ec1fe6792aa991721b77d6c2adb27697b Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandbossart@gmail.com>
Date: Fri, 1 Apr 2022 11:40:51 -0700
Subject: [PATCH v5 1/1] Use has_privs_of_role for samerole, samegroup, and +
in pg_hba.conf.
6198420 ensured that has_privs_of_role() is used for predefined
roles, which means that the role privilege inheritance hierarchy is
checked instead of mere role membership. However, inheritance is
still not respected for pg_hba.conf. This change alters the
authentication logic to consider role privileges instead of just
role membership.
Do not back-patch.
Author: Nathan Bossart
---
doc/src/sgml/client-auth.sgml | 18 ++++-----
src/backend/libpq/hba.c | 19 ++++-----
src/backend/utils/adt/acl.c | 23 +++++++++++
src/include/utils/acl.h | 1 +
src/test/authentication/t/001_password.pl | 48 +++++++++++++++++++++++
5 files changed, 91 insertions(+), 18 deletions(-)
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgmlindex c6f1b70fd3..b06b57f169 100644--- a/doc/src/sgml/client-auth.sgml+++ b/doc/src/sgml/client-auth.sgml@@ -221,12 +221,12 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceabl
The value <literal>sameuser</literal> specifies that the record
matches if the requested database has the same name as the
requested user. The value <literal>samerole</literal> specifies that
- the requested user must be a member of the role with the same+ the requested user must have privileges of the role with the same
name as the requested database. (<literal>samegroup</literal> is an
obsolete but still accepted spelling of <literal>samerole</literal>.)
- Superusers are not considered to be members of a role for the- purposes of <literal>samerole</literal> unless they are explicitly- members of the role, directly or indirectly, and not just by+ Superusers are not considered to have privileges of a role for the+ purposes of <literal>samerole</literal> unless they explicitly have+ privileges of the role, directly or indirectly, and not just by
virtue of being a superuser.
The value <literal>replication</literal> specifies that the record
matches if a physical replication connection is requested, however, it
@@ -252,10 +252,10 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceabl
database user, or a group name preceded by <literal>+</literal>.
(Recall that there is no real distinction between users and groups
in <productname>PostgreSQL</productname>; a <literal>+</literal> mark really means
- <quote>match any of the roles that are directly or indirectly members+ <quote>match any of the roles that directly or indirectly have privileges
of this role</quote>, while a name without a <literal>+</literal> mark matches
only that specific role.) For this purpose, a superuser is only
- considered to be a member of a role if they are explicitly a member+ considered to have privileges of a role if they explicitly have privileges
of the role, directly or indirectly, and not just by virtue of
being a superuser.
Multiple user names can be supplied by separating them with commas.
@@ -788,9 +788,9 @@ host all all 192.168.0.0/16 ident map=omicro
# If these are the only three lines for local connections, they will
# allow local users to connect only to their own databases (databases
# with the same name as their database user name) except for administrators
-# and members of role "support", who can connect to all databases. The file-# $PGDATA/admins contains a list of names of administrators. Passwords-# are required in all cases.+# and roles with privileges of role "support", who can connect to all+# databases. The file $PGDATA/admins contains a list of names of+# administrators. Passwords are required in all cases.
#
# TYPE DATABASE USER ADDRESS METHOD
local sameuser all md5
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.cindex 4637426d62..faa675f4af 100644--- a/src/backend/libpq/hba.c+++ b/src/backend/libpq/hba.c@@ -547,13 +547,13 @@ tokenize_auth_file(const char *filename, FILE *file, List **tok_lines,
/*
- * Does user belong to role?+ * Does user have privileges of role?
*
* userid is the OID of the role given as the attempted login identifier.
- * We check to see if it is a member of the specified role name.+ * We check to see if it has privileges of the specified role name.
*/
static bool
-is_member(Oid userid, const char *role)+has_privs(Oid userid, const char *role)
{
Oid roleid;
@@ -566,11 +566,12 @@ is_member(Oid userid, const char *role)
return false; /* if target role not exist, say "no" */
/*
- * See if user is directly or indirectly a member of role. For this- * purpose, a superuser is not considered to be automatically a member of- * the role, so group auth only applies to explicit membership.+ * See if user directly or indirectly has privileges of role. For this+ * purpose, a superuser is not considered to automatically have+ * privileges of the role, so group auth only applies to explicit+ * privileges.
*/
- return is_member_of_role_nosuper(userid, roleid);+ return has_privs_of_role_nosuper(userid, roleid);
}
/*
@@ -587,7 +588,7 @@ check_role(const char *role, Oid roleid, List *tokens)
tok = lfirst(cell);
if (!tok->quoted && tok->string[0] == '+')
{
- if (is_member(roleid, tok->string + 1))+ if (has_privs(roleid, tok->string + 1))
return true;
}
else if (token_matches(tok, role) ||
@@ -628,7 +629,7 @@ check_db(const char *dbname, const char *role, Oid roleid, List *tokens)
else if (token_is_keyword(tok, "samegroup") ||
token_is_keyword(tok, "samerole"))
{
- if (is_member(roleid, dbname))+ if (has_privs(roleid, dbname))
return true;
}
else if (token_is_keyword(tok, "replication"))
diff --git a/src/backend/utils/adt/acl.c b/src/backend/utils/adt/acl.cindex 4fac402e5b..cb5587d926 100644--- a/src/backend/utils/adt/acl.c+++ b/src/backend/utils/adt/acl.c@@ -4928,6 +4928,29 @@ has_privs_of_role(Oid member, Oid role)
}
+/*+ * Does member have the privileges of role, not considering superuserness?+ *+ * This is identical to has_privs_of_role except we ignore superuser+ * status.+ */+bool+has_privs_of_role_nosuper(Oid member, Oid role)+{+ /* Fast path for simple case */+ if (member == role)+ return true;++ /*+ * Find all the roles that member has the privileges of, including+ * multi-level recursion, then see if target role is any one of them.+ */+ return list_member_oid(roles_is_member_of(member, ROLERECURSE_PRIVS,+ InvalidOid, NULL),+ role);+}++
/*
* Is member a member of role (directly or indirectly)?
*
diff --git a/src/include/utils/acl.h b/src/include/utils/acl.hindex 9a4df3a5da..eded5e0f96 100644--- a/src/include/utils/acl.h+++ b/src/include/utils/acl.h@@ -209,6 +209,7 @@ extern AclMode aclmask(const Acl *acl, Oid roleid, Oid ownerId,
extern int aclmembers(const Acl *acl, Oid **roleids);
extern bool has_privs_of_role(Oid member, Oid role);
+extern bool has_privs_of_role_nosuper(Oid member, Oid role);
extern bool is_member_of_role(Oid member, Oid role);
extern bool is_member_of_role_nosuper(Oid member, Oid role);
extern bool is_admin_of_role(Oid member, Oid role);
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.plindex 93df77aa4e..abbf310d3e 100644--- a/src/test/authentication/t/001_password.pl+++ b/src/test/authentication/t/001_password.pl@@ -200,4 +200,52 @@ append_to_file(
test_conn($node, 'user=md5_role', 'password from pgpass', 0);
+unlink($pgpassfile);+delete $ENV{"PGPASSFILE"};++# Create database and roles for inheritance tests+reset_pg_hba($node, 'all', 'all', 'trust');+$node->safe_psql('postgres', "CREATE DATABASE regress_regression_group;");+$node->safe_psql('postgres', "CREATE ROLE regress_regression_group LOGIN PASSWORD 'pass';");+$node->safe_psql('postgres', "CREATE ROLE regress_member_inherit LOGIN SUPERUSER INHERIT IN ROLE regress_regression_group PASSWORD 'pass';");+$node->safe_psql('postgres', "CREATE ROLE regress_member_noinherit LOGIN SUPERUSER NOINHERIT IN ROLE regress_regression_group PASSWORD 'pass';");++# Test role inheritance is respected for ++$ENV{"PGPASSWORD"} = 'pass';+reset_pg_hba($node, 'all', '+regress_regression_group', 'scram-sha-256');+test_conn($node, 'user=regress_regression_group', 'scram-sha-256', 0,+ log_like =>+ [qr/connection authenticated: identity="regress_regression_group" method=scram-sha-256/]);+test_conn($node, 'user=regress_member_inherit', 'scram-sha-256', 0,+ log_like =>+ [qr/connection authenticated: identity="regress_member_inherit" method=scram-sha-256/]);+test_conn($node, 'user=regress_member_noinherit', 'scram-sha-256', 2,+ log_unlike =>+ [qr/connection authenticated: identity="regress_member_noinherit" method=scram-sha-256/]);++# Test role inheritance is respected for samerole+$ENV{"PGDATABASE"} = 'regress_regression_group';+reset_pg_hba($node, 'samerole', 'all', 'scram-sha-256');+test_conn($node, 'user=regress_regression_group', 'scram-sha-256', 0,+ log_like =>+ [qr/connection authenticated: identity="regress_regression_group" method=scram-sha-256/]);+test_conn($node, 'user=regress_member_inherit', 'scram-sha-256', 0,+ log_like =>+ [qr/connection authenticated: identity="regress_member_inherit" method=scram-sha-256/]);+test_conn($node, 'user=regress_member_noinherit', 'scram-sha-256', 2,+ log_unlike =>+ [qr/connection authenticated: identity="regress_member_noinherit" method=scram-sha-256/]);++# Test role inheritance is respected for samegroup+reset_pg_hba($node, 'samegroup', 'all', 'scram-sha-256');+test_conn($node, 'user=regress_regression_group', 'scram-sha-256', 0,+ log_like =>+ [qr/connection authenticated: identity="regress_regression_group" method=scram-sha-256/]);+test_conn($node, 'user=regress_member_inherit', 'scram-sha-256', 0,+ log_like =>+ [qr/connection authenticated: identity="regress_member_inherit" method=scram-sha-256/]);+test_conn($node, 'user=regress_member_noinherit', 'scram-sha-256', 2,+ log_unlike =>+ [qr/connection authenticated: identity="regress_member_noinherit" method=scram-sha-256/]);+
done_testing();
--
2.25.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, tgl@sss.pgh.pa.us, michael@paquier.xyz, mail@joeconway.com, joshua.brindle@crunchydata.com
Subject: Re: use has_privs_of_role() for pg_hba.conf
In-Reply-To: <20221007215836.GB814385@nathanxps13>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox