database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-15 19:21:59 +03:00
Code Activity

Rename some variables related to ident files in hba.{c,h}

Browse Source 
The code that handles authentication for user maps was pretty confusing
with its choice of variable names.  It involves two types of users: a
system user and a Postgres user (well, role), and these were not named
consistently throughout the code that processes the user maps loaded
from pg_ident.conf at authentication.

This commit changes the following things to improve the situation:
- Rename "pg_role" to "pg_user" and "token" to "system_user" in
IndetLine.  These choices are more consistent with the pg_ident.conf
example in the docs, as well.  "token" has been introduced recently in
fc579e1, and it is way worse than the choice before that, "ident_user".
- Switch the order of the fields in IdentLine to map with the order of
the items in the ident files, as of map name, system user and PG user.
- In check_ident_usermap(), rename "regexp_pgrole" to "expanded_pg_user"
when processing a regexp for the system user entry in a user map.  This
variable does not store a regular expression at all: it would be either
a string or a substitution to \1 if the Postgres role is specified as
such.

Author: Jelte Fennema
Discussion: https://postgr.es/m/CAGECzQTkwELHUOAKhvdA+m3tWbUQySHHkExJV8GAZ1pwgbEgXg@mail.gmail.com
This commit is contained in:
Michael Paquier
2023-01-12 14:23:20 +09:00
parent bfd2542001
commit 8607630d74
3 changed files with 44 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

78
src/backend/libpq/hba.c
View File

@ -2792,7 +2792,7 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel)
token = linitial(tokens); token = linitial(tokens);
/* Copy the ident user token */ /* Copy the ident user token */
parsedline->token = copy_auth_token(token); parsedline->system_user = copy_auth_token(token);
/* Get the PG rolename token */ /* Get the PG rolename token */
field = lnext(tok_line->fields, field); field = lnext(tok_line->fields, field);
@ -2800,13 +2800,13 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel)
tokens = lfirst(field); tokens = lfirst(field);
IDENT_MULTI_VALUE(tokens); IDENT_MULTI_VALUE(tokens);
token = linitial(tokens); token = linitial(tokens);
parsedline->pg_role = pstrdup(token->string); parsedline->pg_user = pstrdup(token->string);
/* /*
* Now that the field validation is done, compile a regex from the user * Now that the field validation is done, compile a regex from the user
* token, if necessary. * token, if necessary.
*/ */
if (regcomp_auth_token(parsedline->token, file_name, line_num, if (regcomp_auth_token(parsedline->system_user, file_name, line_num,
err_msg, elevel)) err_msg, elevel))
{ {
/* err_msg includes the error to report */ /* err_msg includes the error to report */
@ -2819,12 +2819,12 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel)
/* /*
* Process one line from the parsed ident config lines. * Process one line from the parsed ident config lines.
* *
* Compare input parsed ident line to the needed map, pg_role and ident_user. * Compare input parsed ident line to the needed map, pg_user and system_user.
* *found_p and *error_p are set according to our results. * *found_p and *error_p are set according to our results.
*/ */
static void static void
check_ident_usermap(IdentLine *identLine, const char *usermap_name, check_ident_usermap(IdentLine *identLine, const char *usermap_name,
const char *pg_role, const char *ident_user, const char *pg_user, const char *system_user,
bool case_insensitive, bool *found_p, bool *error_p) bool case_insensitive, bool *found_p, bool *error_p)
{ {
*found_p = false; *found_p = false;
@ -2835,7 +2835,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
return; return;
/* Match? */ /* Match? */
if (token_has_regexp(identLine->token)) if (token_has_regexp(identLine->system_user))
{ {
/* /*
* Process the system username as a regular expression that returns * Process the system username as a regular expression that returns
@ -2845,9 +2845,9 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
int r; int r;
regmatch_t matches[2]; regmatch_t matches[2];
char *ofs; char *ofs;
char *regexp_pgrole; char *expanded_pg_user;
r = regexec_auth_token(ident_user, identLine->token, 2, matches); r = regexec_auth_token(system_user, identLine->system_user, 2, matches);
if (r) if (r)
{ {
char errstr[100]; char errstr[100];
@ -2855,17 +2855,17 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
if (r != REG_NOMATCH) if (r != REG_NOMATCH)
{ {
/* REG_NOMATCH is not an error, everything else is */ /* REG_NOMATCH is not an error, everything else is */
pg_regerror(r, identLine->token->regex, errstr, sizeof(errstr)); pg_regerror(r, identLine->system_user->regex, errstr, sizeof(errstr));
ereport(LOG, ereport(LOG,
(errcode(ERRCODE_INVALID_REGULAR_EXPRESSION), (errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),
errmsg("regular expression match for \"%s\" failed: %s", errmsg("regular expression match for \"%s\" failed: %s",
identLine->token->string + 1, errstr))); identLine->system_user->string + 1, errstr)));
*error_p = true; *error_p = true;
} }
return; return;
} }
if ((ofs = strstr(identLine->pg_role, "\\1")) != NULL) if ((ofs = strstr(identLine->pg_user, "\\1")) != NULL)
{ {
int offset; int offset;
@ -2875,7 +2875,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
ereport(LOG, ereport(LOG,
(errcode(ERRCODE_INVALID_REGULAR_EXPRESSION), (errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),
errmsg("regular expression \"%s\" has no subexpressions as requested by backreference in \"%s\"", errmsg("regular expression \"%s\" has no subexpressions as requested by backreference in \"%s\"",
identLine->token->string + 1, identLine->pg_role))); identLine->system_user->string + 1, identLine->pg_user)));
*error_p = true; *error_p = true;
return; return;
} }
@ -2884,18 +2884,18 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
* length: original length minus length of \1 plus length of match * length: original length minus length of \1 plus length of match
* plus null terminator * plus null terminator
*/ */
regexp_pgrole = palloc0(strlen(identLine->pg_role) - 2 + (matches[1].rm_eo - matches[1].rm_so) + 1); expanded_pg_user = palloc0(strlen(identLine->pg_user) - 2 + (matches[1].rm_eo - matches[1].rm_so) + 1);
offset = ofs - identLine->pg_role; offset = ofs - identLine->pg_user;
memcpy(regexp_pgrole, identLine->pg_role, offset); memcpy(expanded_pg_user, identLine->pg_user, offset);
memcpy(regexp_pgrole + offset, memcpy(expanded_pg_user + offset,
ident_user + matches[1].rm_so, system_user + matches[1].rm_so,
matches[1].rm_eo - matches[1].rm_so); matches[1].rm_eo - matches[1].rm_so);
strcat(regexp_pgrole, ofs + 2); strcat(expanded_pg_user, ofs + 2);
} }
else else
{ {
/* no substitution, so copy the match */ /* no substitution, so copy the match */
regexp_pgrole = pstrdup(identLine->pg_role); expanded_pg_user = pstrdup(identLine->pg_user);
} }
/* /*
@ -2904,15 +2904,15 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
*/ */
if (case_insensitive) if (case_insensitive)
{ {
if (pg_strcasecmp(regexp_pgrole, pg_role) == 0) if (pg_strcasecmp(expanded_pg_user, pg_user) == 0)
*found_p = true; *found_p = true;
} }
else else
{ {
if (strcmp(regexp_pgrole, pg_role) == 0) if (strcmp(expanded_pg_user, pg_user) == 0)
*found_p = true; *found_p = true;
} }
pfree(regexp_pgrole); pfree(expanded_pg_user);
return; return;
} }
@ -2921,14 +2921,14 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
/* Not regular expression, so make complete match */ /* Not regular expression, so make complete match */
if (case_insensitive) if (case_insensitive)
{ {
if (pg_strcasecmp(identLine->pg_role, pg_role) == 0 && if (pg_strcasecmp(identLine->pg_user, pg_user) == 0 &&
pg_strcasecmp(identLine->token->string, ident_user) == 0) pg_strcasecmp(identLine->system_user->string, system_user) == 0)
*found_p = true; *found_p = true;
} }
else else
{ {
if (strcmp(identLine->pg_role, pg_role) == 0 && if (strcmp(identLine->pg_user, pg_user) == 0 &&
strcmp(identLine->token->string, ident_user) == 0) strcmp(identLine->system_user->string, system_user) == 0)
*found_p = true; *found_p = true;
} }
} }
@ -2938,20 +2938,20 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
/* /*
* Scan the (pre-parsed) ident usermap file line by line, looking for a match * Scan the (pre-parsed) ident usermap file line by line, looking for a match
* *
* See if the user with ident username "auth_user" is allowed to act * See if the system user with ident username "system_user" is allowed to act as
* as Postgres user "pg_role" according to usermap "usermap_name". * Postgres user "pg_user" according to usermap "usermap_name".
* *
* Special case: Usermap NULL, equivalent to what was previously called * Special case: Usermap NULL, equivalent to what was previously called
* "sameuser" or "samerole", means don't look in the usermap file. * "sameuser" or "samerole", means don't look in the usermap file.
* That's an implied map wherein "pg_role" must be identical to * That's an implied map wherein "pg_user" must be identical to
* "auth_user" in order to be authorized. * "system_user" in order to be authorized.
* *
* Iff authorized, return STATUS_OK, otherwise return STATUS_ERROR. * Iff authorized, return STATUS_OK, otherwise return STATUS_ERROR.
*/ */
int int
check_usermap(const char *usermap_name, check_usermap(const char *usermap_name,
const char *pg_role, const char *pg_user,
const char *auth_user, const char *system_user,
bool case_insensitive) bool case_insensitive)
{ {
bool found_entry = false, bool found_entry = false,
@ -2961,17 +2961,17 @@ check_usermap(const char *usermap_name,
{ {
if (case_insensitive) if (case_insensitive)
{ {
if (pg_strcasecmp(pg_role, auth_user) == 0) if (pg_strcasecmp(pg_user, system_user) == 0)
return STATUS_OK; return STATUS_OK;
} }
else else
{ {
if (strcmp(pg_role, auth_user) == 0) if (strcmp(pg_user, system_user) == 0)
return STATUS_OK; return STATUS_OK;
} }
ereport(LOG, ereport(LOG,
(errmsg("provided user name (%s) and authenticated user name (%s) do not match", (errmsg("provided user name (%s) and authenticated user name (%s) do not match",
pg_role, auth_user))); pg_user, system_user)));
return STATUS_ERROR; return STATUS_ERROR;
} }
else else
@ -2981,7 +2981,7 @@ check_usermap(const char *usermap_name,
foreach(line_cell, parsed_ident_lines) foreach(line_cell, parsed_ident_lines)
{ {
check_ident_usermap(lfirst(line_cell), usermap_name, check_ident_usermap(lfirst(line_cell), usermap_name,
pg_role, auth_user, case_insensitive, pg_user, system_user, case_insensitive,
&found_entry, &error); &found_entry, &error);
if (found_entry || error) if (found_entry || error)
break; break;
@ -2991,7 +2991,7 @@ check_usermap(const char *usermap_name,
{ {
ereport(LOG, ereport(LOG,
(errmsg("no match in usermap \"%s\" for user \"%s\" authenticated as \"%s\"", (errmsg("no match in usermap \"%s\" for user \"%s\" authenticated as \"%s\"",
usermap_name, pg_role, auth_user))); usermap_name, pg_user, system_user)));
} }
return found_entry ? STATUS_OK : STATUS_ERROR; return found_entry ? STATUS_OK : STATUS_ERROR;
} }
@ -3073,7 +3073,7 @@ load_ident(void)
foreach(parsed_line_cell, new_parsed_lines) foreach(parsed_line_cell, new_parsed_lines)
{ {
newline = (IdentLine *) lfirst(parsed_line_cell); newline = (IdentLine *) lfirst(parsed_line_cell);
free_auth_token(newline->token); free_auth_token(newline->system_user);
} }
MemoryContextDelete(ident_context); MemoryContextDelete(ident_context);
return false; return false;
@ -3085,7 +3085,7 @@ load_ident(void)
foreach(parsed_line_cell, parsed_ident_lines) foreach(parsed_line_cell, parsed_ident_lines)
{ {
newline = (IdentLine *) lfirst(parsed_line_cell); newline = (IdentLine *) lfirst(parsed_line_cell);
free_auth_token(newline->token); free_auth_token(newline->system_user);
} }
} }
if (parsed_ident_context != NULL) if (parsed_ident_context != NULL)

4
src/backend/utils/adt/hbafuncs.c
View File

@ -492,8 +492,8 @@ fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
if (ident != NULL) if (ident != NULL)
{ {
values[index++] = CStringGetTextDatum(ident->usermap); values[index++] = CStringGetTextDatum(ident->usermap);
values[index++] = CStringGetTextDatum(ident->token->string); values[index++] = CStringGetTextDatum(ident->system_user->string);
values[index++] = CStringGetTextDatum(ident->pg_role); values[index++] = CStringGetTextDatum(ident->pg_user);
} }
else else
{ {

6
src/include/libpq/hba.h
View File

@ -142,8 +142,8 @@ typedef struct IdentLine
int linenumber; int linenumber;
char *usermap; char *usermap;
char *pg_role; AuthToken *system_user;
AuthToken *token; char *pg_user;
} IdentLine; } IdentLine;
/* /*
@ -172,7 +172,7 @@ extern bool load_ident(void);
extern const char *hba_authname(UserAuth auth_method); extern const char *hba_authname(UserAuth auth_method);
extern void hba_getauthmethod(hbaPort *port); extern void hba_getauthmethod(hbaPort *port);
extern int check_usermap(const char *usermap_name, extern int check_usermap(const char *usermap_name,
const char *pg_role, const char *auth_user, const char *pg_user, const char *system_user,
bool case_insensitive); bool case_insensitive);
extern HbaLine *parse_hba_line(TokenizedAuthLine *tok_line, int elevel); extern HbaLine *parse_hba_line(TokenizedAuthLine *tok_line, int elevel);
extern IdentLine *parse_ident_line(TokenizedAuthLine *tok_line, int elevel); extern IdentLine *parse_ident_line(TokenizedAuthLine *tok_line, int elevel);