On Wed, Jan 18, 2023 at 10:35:29AM +0100, Jelte Fennema wrote: > Anything I can do to help with this? Or will you do that yourself?
So, I have done a second lookup, and tweaked a few things: - Addition of a macro for pg_strcasecmp(), to match with token_matches(). - Fixed a bit the documentation. - Tweaked some comments and descriptions in the tests, I was rather fine with the role and group names. Jelte, do you like this version? -- Michael
From 0e42a5e9c2a532355df49346e47cc5612da1889d Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Thu, 19 Jan 2023 16:54:59 +0900
Subject: [PATCH v6] Support same user patterns in pg_ident.conf as in
pg_hba.conf
While pg_hba.conf has support for non-literal username matches,
pg_ident.conf doesn't have this same functionality. This changes
permission checking in pg_ident.conf to handle all the special cases for
username checking:
1. The "all" keyword
2. Membership checks using the + prefix
3. Using a regex to match against multiple roles
This allows matching certain system users against many different
postgres users with a single line in pg_ident.conf. Without this you
you would need a line for each of the postgres users that a system user
can log in as.
There is some risk of breaking existing pg_ident.conf configs that
contained such special strings and which were treated as literal user
names. But the advantages brought by the features added in this change
far outweigh the risk of breakage. And we only risk breaking when there
exist roles that are called "all", start with a literal + or start with
a literal /. Only "all" seems like a somewhat reasonable role name, but
such a role existing seems unlikely to me given all its special meaning
in pg_hba.conf.
**This compatibility change should be mentioned in the release notes.**
---
src/backend/libpq/hba.c | 98 +++++++++++-----
src/test/authentication/t/003_peer.pl | 160 ++++++++++++++++++++++++--
doc/src/sgml/client-auth.sgml | 27 ++++-
3 files changed, 246 insertions(+), 39 deletions(-)
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 029b8e4483..69f87667be 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -73,8 +73,10 @@ typedef struct
} tokenize_error_callback_arg;
#define token_has_regexp(t) (t->regex != NULL)
+#define token_is_member_check(t) (!t->quoted && t->string[0] == '+')
#define token_is_keyword(t, k) (!t->quoted && strcmp(t->string, k) == 0)
#define token_matches(t, k) (strcmp(t->string, k) == 0)
+#define token_matches_insensitive(t,k) (pg_strcasecmp(t->string, k) == 0)
/*
* Memory context holding the list of TokenizedAuthLines when parsing
@@ -995,10 +997,11 @@ is_member(Oid userid, const char *role)
*
* Each AuthToken listed is checked one-by-one. Keywords are processed
* first (these cannot have regular expressions), followed by regular
- * expressions (if any) and the exact match.
+ * expressions (if any), the case-insensitive match (if requested) and
+ * the exact match.
*/
static bool
-check_role(const char *role, Oid roleid, List *tokens)
+check_role(const char *role, Oid roleid, List *tokens, bool case_insensitive)
{
ListCell *cell;
AuthToken *tok;
@@ -1006,7 +1009,7 @@ check_role(const char *role, Oid roleid, List *tokens)
foreach(cell, tokens)
{
tok = lfirst(cell);
- if (!tok->quoted && tok->string[0] == '+')
+ if (token_is_member_check(tok))
{
if (is_member(roleid, tok->string + 1))
return true;
@@ -1018,6 +1021,11 @@ check_role(const char *role, Oid roleid, List *tokens)
if (regexec_auth_token(role, tok, 0, NULL) == REG_OKAY)
return true;
}
+ else if (case_insensitive)
+ {
+ if (token_matches_insensitive(tok, role))
+ return true;
+ }
else if (token_matches(tok, role))
return true;
}
@@ -2614,7 +2622,7 @@ check_hba(hbaPort *port)
hba->databases))
continue;
- if (!check_role(port->user_name, roleid, hba->roles))
+ if (!check_role(port->user_name, roleid, hba->roles, false))
continue;
/* Found a record that matched! */
@@ -2804,7 +2812,7 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel)
/*
* Now that the field validation is done, compile a regex from the user
- * token, if necessary.
+ * tokens, if necessary.
*/
if (regcomp_auth_token(parsedline->system_user, file_name, line_num,
err_msg, elevel))
@@ -2813,6 +2821,13 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel)
return NULL;
}
+ if (regcomp_auth_token(parsedline->pg_user, file_name, line_num,
+ err_msg, elevel))
+ {
+ /* err_msg includes the error to report */
+ return NULL;
+ }
+
return parsedline;
}
@@ -2827,6 +2842,8 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
const char *pg_user, const char *system_user,
bool case_insensitive, bool *found_p, bool *error_p)
{
+ Oid roleid;
+
*found_p = false;
*error_p = false;
@@ -2834,6 +2851,9 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
/* Line does not match the map name we're looking for, so just abort */
return;
+ /* Get the target role's OID. Note we do not error out for bad role */
+ roleid = get_role_oid(pg_user, true);
+
/* Match? */
if (token_has_regexp(identLine->system_user))
{
@@ -2845,7 +2865,8 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
int r;
regmatch_t matches[2];
char *ofs;
- char *expanded_pg_user;
+ AuthToken *expanded_pg_user_token;
+ bool created_temporary_token = false;
r = regexec_auth_token(system_user, identLine->system_user, 2, matches);
if (r)
@@ -2865,8 +2886,16 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
return;
}
- if ((ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
+ /*
+ * Replace \1 with the first captured group unless the field already
+ * has some special meaning, like a group membership or a regexp-based
+ * check.
+ */
+ if (!token_is_member_check(identLine->pg_user) &&
+ !token_has_regexp(identLine->pg_user) &&
+ (ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
{
+ char *expanded_pg_user;
int offset;
/* substitution of the first argument requested */
@@ -2891,46 +2920,53 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
system_user + matches[1].rm_so,
matches[1].rm_eo - matches[1].rm_so);
strcat(expanded_pg_user, ofs + 2);
+
+ /*
+ * Mark the token as quoted, so it will only be compared literally
+ * and not for some special meaning, such as "all" or a group
+ * membership checks.
+ */
+ expanded_pg_user_token = make_auth_token(expanded_pg_user, true);
+ created_temporary_token = true;
+ pfree(expanded_pg_user);
}
else
{
- /* no substitution, so copy the match */
- expanded_pg_user = pstrdup(identLine->pg_user->string);
+ expanded_pg_user_token = identLine->pg_user;
}
- /*
- * now check if the username actually matched what the user is trying
- * to connect as
- */
- if (case_insensitive)
- {
- if (pg_strcasecmp(expanded_pg_user, pg_user) == 0)
- *found_p = true;
- }
- else
- {
- if (strcmp(expanded_pg_user, pg_user) == 0)
- *found_p = true;
- }
- pfree(expanded_pg_user);
+ /* check the Postgres user */
+ *found_p = check_role(pg_user, roleid,
+ list_make1(expanded_pg_user_token),
+ case_insensitive);
+
+ if (created_temporary_token)
+ free_auth_token(expanded_pg_user_token);
return;
}
else
{
- /* Not regular expression, so make complete match */
+ /*
+ * Not a regular expression, so make a complete match. If the system
+ * user does not match, just leave.
+ */
if (case_insensitive)
{
- if (pg_strcasecmp(identLine->pg_user->string, pg_user) == 0 &&
- pg_strcasecmp(identLine->system_user->string, system_user) == 0)
- *found_p = true;
+ if (!token_matches_insensitive(identLine->system_user,
+ system_user))
+ return;
}
else
{
- if (strcmp(identLine->pg_user->string, pg_user) == 0 &&
- strcmp(identLine->system_user->string, system_user) == 0)
- *found_p = true;
+ if (!token_matches(identLine->system_user, system_user))
+ return;
}
+
+ /* check the Postgres user */
+ *found_p = check_role(pg_user, roleid,
+ list_make1(identLine->pg_user),
+ case_insensitive);
}
}
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index e6f5fdba16..a6be651ea7 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -98,8 +98,13 @@ if (find_in_log(
plan skip_all => 'peer authentication is not supported on this platform';
}
-# Add a database role, to use for the user name map.
+# Add a database role and a group, to use for the user name map.
$node->safe_psql('postgres', qq{CREATE ROLE testmapuser LOGIN});
+$node->safe_psql('postgres', "CREATE ROLE testmapgroup NOLOGIN");
+$node->safe_psql('postgres', "GRANT testmapgroup TO testmapuser");
+# Note the double quotes here.
+$node->safe_psql('postgres', 'CREATE ROLE "testmapgroupliteral\\1" LOGIN');
+$node->safe_psql('postgres', 'GRANT "testmapgroupliteral\\1" TO testmapuser');
# Extract as well the system user for the user name map.
my $system_user =
@@ -123,12 +128,56 @@ test_role($node, qq{testmapuser}, 'peer', 0, 'with user name map',
log_like =>
[qr/connection authenticated: identity="$system_user" method=peer/]);
+# Tests with the "all" keyword.
+reset_pg_ident($node, 'mypeermap', $system_user, 'all');
+
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 0,
+ 'with keyword "all" as database user in user name map',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+# Tests with the "all" keyword, but quoted (no effect here).
+reset_pg_ident($node, 'mypeermap', $system_user, '"all"');
+
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 2,
+ 'with quoted keyword "all" as database user in user name map',
+ log_like => [qr/no match in usermap "mypeermap" for user "testmapuser"/]);
+
+# Success as the regexp of the database user matches
+reset_pg_ident($node, 'mypeermap', $system_user, qq{/^testm.*\$});
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 0,
+ 'with regexp of database user in user name map',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+# Failure as the regexp of the database user does not match.
+reset_pg_ident($node, 'mypeermap', $system_user, qq{/^doesnotmatch.*\$});
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 2,
+ 'with bad regexp of database user in user name map',
+ log_like => [qr/no match in usermap "mypeermap" for user "testmapuser"/]);
+
# Test with regular expression in user name map.
# Extract the last 3 characters from the system_user
# or the entire system_user (if its length is <= -3).
my $regex_test_string = substr($system_user, -3);
-# Success as the regular expression matches.
+# Success as the system user regular expression matches.
reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string\$},
'testmapuser');
test_role(
@@ -136,10 +185,33 @@ test_role(
qq{testmapuser},
'peer',
0,
- 'with regular expression in user name map',
+ 'with regexp of system user in user name map',
log_like =>
[qr/connection authenticated: identity="$system_user" method=peer/]);
+# Success as both regular expressions match.
+reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string\$},
+ qq{/^testm.*\$});
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 0,
+ 'with regexps for both system and database user in user name map',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+# Success as the regular expression matches and database role is the "all"
+# keyword.
+reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string\$}, 'all');
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 0,
+ 'with regexp of system user and keyword "all" in user name map',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
# Success as the regular expression matches and \1 is replaced in the given
# subexpression.
@@ -179,17 +251,91 @@ test_role(
]);
# Concatenate system_user to system_user.
-$regex_test_string = $system_user . $system_user;
+my $bad_regex_test_string = $system_user . $system_user;
-# Failure as the regular expression does not match.
-reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string\$},
+# Failure as the regexp of system user does not match.
+reset_pg_ident($node, 'mypeermap', qq{/^.*$bad_regex_test_string\$},
'testmapuser');
test_role(
$node,
qq{testmapuser},
'peer',
2,
- 'with regular expression in user name map',
+ 'with regexp of system user in user name map',
log_like => [qr/no match in usermap "mypeermap" for user "testmapuser"/]);
+# Test using a group role match for the database user.
+reset_pg_ident($node, 'mypeermap', $system_user, '+testmapgroup');
+
+test_role($node, qq{testmapuser}, 'peer', 0, 'plain user with group',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+test_role(
+ $node, qq{testmapgroup}, 'peer', 2,
+ 'group user with group',
+ log_like => [qr/role "testmapgroup" is not permitted to log in/]);
+
+# Now apply quotes to the group match, nullifying its effect.
+reset_pg_ident($node, 'mypeermap', $system_user, '"+testmapgroup"');
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 2,
+ 'plain user with quoted group name',
+ log_like => [qr/no match in usermap "mypeermap" for user "testmapuser"/]);
+
+# Test using a regexp for the system user, with a group membership
+# check for the database user.
+reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string\$},
+ '+testmapgroup');
+
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 0,
+ 'regexp of system user as group member',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+test_role(
+ $node,
+ qq{testmapgroup},
+ 'peer',
+ 2,
+ 'regexp of system user as non-member of group',
+ log_like => [qr/role "testmapgroup" is not permitted to log in/]);
+
+# Test that membership checks and regexes will use literal \1 instead of
+# replacing it, as subexpression replacement is not allowed in this case.
+reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string(.*)\$},
+ '+testmapgroupliteral\\1');
+
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 0,
+ 'membership check with literal \1',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+# Do the same with a quoted regular expression for the database user this
+# time. No replacement of \1 is done.
+reset_pg_ident(
+ $node, 'mypeermap',
+ qq{/^.*$regex_test_string(.*)\$},
+ '"/^testmapgroupliteral\\\\1$"');
+
+test_role(
+ $node,
+ 'testmapgroupliteral\\\\1',
+ 'peer',
+ 0,
+ 'regexp of database user with literal \1',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
+
done_testing();
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index e4959663c4..b9d73deced 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -941,7 +941,22 @@ local db1,db2,@demodbs all md5
implying that they are equivalent. The connection will be allowed if
there is any map entry that pairs the user name obtained from the
external authentication system with the database user name that the
- user has requested to connect as.
+ user has requested to connect as. The value <literal>all</literal>
+ can be used as the <replaceable>database-username</replaceable> to specify
+ that if the <replaceable>system-user</replaceable> matches, then this user
+ is allowed to log in as any of the existing database users. Quoting
+ <literal>all</literal> makes the keyword lose its special meaning.
+ </para>
+ <para>
+ If the <replaceable>database-username</replaceable> begins with a
+ <literal>+</literal> character, then the operating system user can login as
+ any user belonging to that role, similarly to how user names beginning with
+ <literal>+</literal> are treated in <literal>pg_hba.conf</literal>.
+ Thus, a <literal>+</literal> mark means <quote>match any of the roles that
+ are directly or indirectly members of this role</quote>, while a name
+ without a <literal>+</literal> mark matches only that specific role. Quoting
+ a username starting with a <literal>+</literal> makes the
+ <literal>+</literal> lose its special meaning.
</para>
<para>
If the <replaceable>system-username</replaceable> field starts with a slash (<literal>/</literal>),
@@ -964,6 +979,16 @@ mymap /^(.*)@otherdomain\.com$ guest
<literal>\1</literal> <emphasis>does not</emphasis> make
<literal>\1</literal> lose its special meaning.
</para>
+ <para>
+ If the <replaceable>database-username</replaceable> field starts with
+ a slash (<literal>/</literal>), the remainder of the field is treated
+ as a regular expression (see <xref linkend="posix-syntax-details"/>
+ for details of <productname>PostgreSQL</productname>'s regular
+ expression syntax. It is not possible to use <literal>\1</literal>
+ to use a capture from regular expression on
+ <replaceable>system-username</replaceable> for a regular expression
+ on <replaceable>database-username</replaceable>.
+ </para>
<tip>
<para>
--
2.39.0
signature.asc
Description: PGP signature
