From 7f84fff3b8068c757b757387b50ed3c4d0a5f3a3 Mon Sep 17 00:00:00 2001 From: Jelte Fennema Date: Wed, 11 Jan 2023 12:10:19 +0100 Subject: [PATCH v5 4/4] 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.** --- doc/src/sgml/client-auth.sgml | 26 ++++- src/backend/libpq/hba.c | 86 ++++++++++----- src/test/authentication/t/003_peer.pl | 153 +++++++++++++++++++++++++- 3 files changed, 228 insertions(+), 37 deletions(-) diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml index 50af2bf03b8..0eb3bddd0e0 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 all + can be used as the database-username to specify + that if the system-user matches, then this user + is allowed to log in as any of the existing database users. Quoting + all makes the keyword lose its special meaning. + + + If the database-username begins with a + + character, then the operating system user can login as + any user belonging to that role, similarly to how user names beginning with + + are treated in pg_hba.conf. + Thus, a + mark means match any of the roles that + are directly or indirectly members of this role, while a name + without a + mark matches only that specific role. Quoting + a username starting with a + makes the + + lose its special meaning. If the system-username field starts with a slash (/), @@ -964,6 +979,15 @@ mymap /^(.*)@otherdomain\.com$ guest \1 does not make \1 lose its special meaning. + + If the database-username field starts with a slash (/), + the remainder of the field is treated as a regular expression. + (See for details of + PostgreSQL's regular expression syntax.) It's + not possible to use the \1 to use a capture from + a system-username regular expression in a + regular expression for a database-username. + diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 029b8e44838..1fc203117cb 100644 --- a/src/backend/libpq/hba.c +++ b/src/backend/libpq/hba.c @@ -73,6 +73,7 @@ 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) @@ -998,7 +999,7 @@ is_member(Oid userid, const char *role) * expressions (if any) 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 +1007,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 +1019,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 (pg_strcasecmp(tok->string, role) == 0) + return true; + } else if (token_matches(tok, role)) return true; } @@ -2614,7 +2620,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 +2810,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 +2819,14 @@ 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 +2841,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 +2850,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 +2864,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 +2885,15 @@ 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. + */ + 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 +2918,45 @@ 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); - } - else - { - /* no substitution, so copy the match */ - expanded_pg_user = pstrdup(identLine->pg_user->string); - } - /* - * 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; + /* + * Mark the token as quoted, so it will only be compared literally + * and not for special meanings, such as "all" and membership + * checks using the + prefix. + */ + expanded_pg_user_token = make_auth_token(expanded_pg_user, true); + created_temporary_token = true; + pfree(expanded_pg_user); } else { - if (strcmp(expanded_pg_user, pg_user) == 0) - *found_p = true; + expanded_pg_user_token = identLine->pg_user; } - pfree(expanded_pg_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 */ + /* + * If the systemuser does not match, there's no match + */ 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 (pg_strcasecmp(identLine->system_user->string, system_user) != 0) + return; } else { - if (strcmp(identLine->pg_user->string, pg_user) == 0 && - strcmp(identLine->system_user->string, system_user) == 0) - *found_p = true; + if (strcmp(identLine->system_user->string, system_user) != 0) + return; } + + *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 796fd059ccd..1251c97a274 100644 --- a/src/test/authentication/t/003_peer.pl +++ b/src/test/authentication/t/003_peer.pl @@ -98,8 +98,12 @@ 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 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"); +$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 +127,48 @@ 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'); + +# Success as the database role is the "all" keyword +test_role($node, qq{testmapuser}, 'peer', 0, 'with all keyword in user name map', + log_like => + [qr/connection authenticated: identity="$system_user" method=peer/]); + +# Tests with the the literal "all" user +reset_pg_ident($node, 'mypeermap', $system_user, '"all"'); + +# Failure as we're not logging is as the literal "all" user +test_role($node, qq{testmapuser}, 'peer', 2, 'with literal all user in user name map', + log_like => [qr/no match in usermap "mypeermap" for user "testmapuser"/]); + +# Success as the database user regular expression matches +reset_pg_ident($node, 'mypeermap', $system_user, qq{/^testm.*\$}); +test_role( + $node, + qq{testmapuser}, + 'peer', + 0, + 'with database user regular expression in user name map', + log_like => + [qr/connection authenticated: identity="$system_user" method=peer/]); + +# Failure as the database user regular expression does not match. +reset_pg_ident($node, 'mypeermap', $system_user, qq{/^doesnotmatch.*\$}); +test_role( + $node, + qq{testmapuser}, + 'peer', + 2, + 'with bad database user regular expression 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 +176,34 @@ test_role( qq{testmapuser}, 'peer', 0, - 'with regular expression in user name map', + 'with system user regular expression in user name map', log_like => [qr/connection authenticated: identity="$system_user" method=peer/]); +# Success as both regular expression match. +reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string\$}, + qq{/^testm.*\$}); +test_role( + $node, + qq{testmapuser}, + 'peer', + 0, + 'with system and database user regular expressions 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 system user regular expression and all keyword 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. @@ -178,10 +242,10 @@ 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 system user regular expression does not match. +reset_pg_ident($node, 'mypeermap', qq{/^.*$bad_regex_test_string\$}, 'testmapuser'); test_role( $node, @@ -191,4 +255,81 @@ test_role( 'with regular expression in user name map', log_like => [qr/no match in usermap "mypeermap" for user "testmapuser"/]); +# test using a group role match +# first with a plain 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/]); + +reset_pg_ident($node, 'mypeermap', $system_user, '"+testmapgroup"'); +test_role( + $node, + qq{testmapuser}, + 'peer', + 2, + 'plain user with literal + in front of a wrong user', + log_like => [qr/no match in usermap "mypeermap" for user "testmapuser"/]); + +# ... then with a regex user +reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string\$}, '+testmapgroup'); + +test_role( + $node, + qq{testmapuser}, + 'peer', + 0, + 'regex user with group', + log_like => + [qr/connection authenticated: identity="$system_user" method=peer/]); + +test_role( + $node, + qq{testmapgroup}, + 'peer', + 2, + 'regex group user with 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. +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/]); + +reset_pg_ident($node, 'mypeermap', qq{/^.*$regex_test_string(.*)\$}, + '"/^testmapgroupliteral\\\\1$"'); + +test_role( + $node, + 'testmapgroupliteral\\\\1', + 'peer', + 0, + 'regex database user with literal \1', + log_like => + [qr/connection authenticated: identity="$system_user" method=peer/]); + done_testing(); -- 2.34.1