public inbox for [email protected]
help / color / mirror / Atom feed[PATCH v3] Avoid errors in brin summarization..
15+ messages / 3 participants
[nested] [flat]
* [PATCH] Avoid errors in brin summarization..
@ 2020-11-13 16:39 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 15+ messages in thread
From: Alvaro Herrera @ 2020-11-13 16:39 UTC (permalink / raw)
..which can happen if an index is reindexed concurrently
---
src/backend/access/brin/brin.c | 34 ++++++++++++++++++++++++++++------
1 file changed, 28 insertions(+), 6 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 1f72562c60..a1089d5f9e 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -886,9 +886,9 @@ brin_summarize_range(PG_FUNCTION_ARGS)
/*
* We must lock table before index to avoid deadlocks. However, if the
- * passed indexoid isn't an index then IndexGetRelation() will fail.
- * Rather than emitting a not-very-helpful error message, postpone
- * complaining, expecting that the is-it-an-index test below will fail.
+ * passed indexoid isn't an index, then IndexGetRelation(true) would
+ * emit a not-very-helpful error message. Instead, postpone complaining
+ * until the is-it-an-index test, below.
*/
heapoid = IndexGetRelation(indexoid, true);
if (OidIsValid(heapoid))
@@ -896,11 +896,33 @@ brin_summarize_range(PG_FUNCTION_ARGS)
else
heapRel = NULL;
- indexRel = index_open(indexoid, ShareUpdateExclusiveLock);
+ indexRel = try_relation_open(indexoid, ShareUpdateExclusiveLock);
+
+ /* Silently skip autovacuum work-items if an index has disappeared. */
+ if (!indexRel)
+ {
+ if (heapRel)
+ table_close(heapRel, ShareUpdateExclusiveLock);
+
+ PG_RETURN_INT32(0);
+ }
+
+ /* If passed a non-index, fail noisily */
+ if (indexRel->rd_rel->relkind != RELKIND_INDEX)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not an index",
+ RelationGetRelationName(indexRel))));
+
+ /* If the table wasn't opened for some other reason, silently skip it */
+ if (!heapRel)
+ {
+ relation_close(indexRel, ShareUpdateExclusiveLock);
+ PG_RETURN_INT32(0);
+ }
/* Must be a BRIN index */
- if (indexRel->rd_rel->relkind != RELKIND_INDEX ||
- indexRel->rd_rel->relam != BRIN_AM_OID)
+ if (indexRel->rd_rel->relam != BRIN_AM_OID)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("\"%s\" is not a BRIN index",
--
2.17.0
--HuscSE0D68UGttcd--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v3] Avoid errors in brin summarization..
@ 2020-11-13 16:39 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 15+ messages in thread
From: Alvaro Herrera @ 2020-11-13 16:39 UTC (permalink / raw)
..which can happen if an index is reindexed concurrently
---
src/backend/access/brin/brin.c | 37 ++++++++++++++++++++++++----------
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 1f72562c60..4d012ebd76 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -33,6 +33,7 @@
#include "postmaster/autovacuum.h"
#include "storage/bufmgr.h"
#include "storage/freespace.h"
+#include "storage/lmgr.h"
#include "utils/acl.h"
#include "utils/builtins.h"
#include "utils/index_selfuncs.h"
@@ -886,21 +887,35 @@ brin_summarize_range(PG_FUNCTION_ARGS)
/*
* We must lock table before index to avoid deadlocks. However, if the
- * passed indexoid isn't an index then IndexGetRelation() will fail.
- * Rather than emitting a not-very-helpful error message, postpone
- * complaining, expecting that the is-it-an-index test below will fail.
+ * passed indexoid isn't an index, then IndexGetRelation(false) would
+ * emit a not-very-helpful error message. Instead, postpone complaining
+ * until the is-it-an-index test, below.
*/
heapoid = IndexGetRelation(indexoid, true);
- if (OidIsValid(heapoid))
- heapRel = table_open(heapoid, ShareUpdateExclusiveLock);
- else
- heapRel = NULL;
+ if (heapoid != InvalidOid)
+ LockRelationOid(heapoid, ShareUpdateExclusiveLock);
- indexRel = index_open(indexoid, ShareUpdateExclusiveLock);
+ /* Now we can open the index; silently skip if it's gone. */
+ indexRel = try_relation_open(indexoid, ShareUpdateExclusiveLock);
+ if (!indexRel)
+ {
+ if (heapoid != InvalidOid)
+ UnlockRelationOid(heapoid, ShareUpdateExclusiveLock);
+ PG_RETURN_INT32(0);
+ }
+
+ /* If passed a non-index, fail noisily */
+ if (indexRel->rd_rel->relkind != RELKIND_INDEX)
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("\"%s\" is not an index",
+ RelationGetRelationName(indexRel))));
+
+ /* Now we can open the table */
+ heapRel = table_open(heapoid, NoLock);
/* Must be a BRIN index */
- if (indexRel->rd_rel->relkind != RELKIND_INDEX ||
- indexRel->rd_rel->relam != BRIN_AM_OID)
+ if (indexRel->rd_rel->relam != BRIN_AM_OID)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("\"%s\" is not a BRIN index",
@@ -916,7 +931,7 @@ brin_summarize_range(PG_FUNCTION_ARGS)
* barely possible that a race against an index drop/recreation could have
* netted us the wrong table. Recheck.
*/
- if (heapRel == NULL || heapoid != IndexGetRelation(indexoid, false))
+ if (heapoid != IndexGetRelation(indexoid, false))
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_TABLE),
errmsg("could not open parent table of index %s",
--
2.20.1
--LQksG6bCIzRHxTLp
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="autovacuum-workitem.patch"
commit 2326df869ee8516b8b1f0e01930946e9a63800b5
Author: Alvaro Herrera <[email protected]>
AuthorDate: Mon Nov 23 15:56:23 2020 -0300
CommitDate: Mon Nov 23 15:57:28 2020 -0300
autovacuum-centered workitem fix
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index aa5b97fbac..77c1401bc9 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2561,6 +2561,7 @@ deleted:
for (i = 0; i < NUM_WORKITEMS; i++)
{
AutoVacuumWorkItem *workitem = &AutoVacuumShmem->av_workItems[i];
+ AutoVacuumWorkItem my_workitem;
if (!workitem->avw_used)
continue;
@@ -2569,11 +2570,14 @@ deleted:
if (workitem->avw_database != MyDatabaseId)
continue;
+ my_workitem = *workitem;
+ workitem->avw_used = false;
+
/* claim this one, and release lock while performing it */
workitem->avw_active = true;
LWLockRelease(AutovacuumLock);
- perform_work_item(workitem);
+ perform_work_item(&my_workitem);
/*
* Check for config changes before acquiring lock for further jobs.
--LQksG6bCIzRHxTLp--
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v3] Avoid errors in brin summarization..
@ 2020-11-13 16:39 Alvaro Herrera <[email protected]>
0 siblings, 0 replies; 15+ messages in thread
From: Alvaro Herrera @ 2020-11-13 16:39 UTC (permalink / raw)
..which can happen if an index is reindexed concurrently
---
src/backend/access/brin/brin.c | 22 +++++++++++++++-------
src/test/regress/expected/brin.out | 6 +++++-
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 1f72562c60..46e5269260 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -887,16 +887,24 @@ brin_summarize_range(PG_FUNCTION_ARGS)
/*
* We must lock table before index to avoid deadlocks. However, if the
* passed indexoid isn't an index then IndexGetRelation() will fail.
- * Rather than emitting a not-very-helpful error message, postpone
- * complaining, expecting that the is-it-an-index test below will fail.
+ * Rather than emitting a not-very-helpful error message, prepare to
+ * return without doing anything. This allows autovacuum work-items to be
+ * silently discarded rather than uselessly accumulating error messages in
+ * the server log.
*/
heapoid = IndexGetRelation(indexoid, true);
- if (OidIsValid(heapoid))
- heapRel = table_open(heapoid, ShareUpdateExclusiveLock);
- else
- heapRel = NULL;
+ heapRel = OidIsValid(heapoid) ?
+ table_open(heapoid, ShareUpdateExclusiveLock) : NULL;
- indexRel = index_open(indexoid, ShareUpdateExclusiveLock);
+ if (heapRel == NULL)
+ PG_RETURN_INT32(0);
+
+ indexRel = try_relation_open(indexoid, ShareUpdateExclusiveLock);
+ if (indexRel == NULL)
+ {
+ table_close(heapRel, ShareUpdateExclusiveLock);
+ PG_RETURN_INT32(0);
+ }
/* Must be a BRIN index */
if (indexRel->rd_rel->relkind != RELKIND_INDEX ||
diff --git a/src/test/regress/expected/brin.out b/src/test/regress/expected/brin.out
index e53d6e4885..51dff7295d 100644
--- a/src/test/regress/expected/brin.out
+++ b/src/test/regress/expected/brin.out
@@ -405,7 +405,11 @@ UPDATE brintest SET int8col = int8col * int4col;
UPDATE brintest SET textcol = '' WHERE textcol IS NOT NULL;
-- Tests for brin_summarize_new_values
SELECT brin_summarize_new_values('brintest'); -- error, not an index
-ERROR: "brintest" is not an index
+ brin_summarize_new_values
+---------------------------
+ 0
+(1 row)
+
SELECT brin_summarize_new_values('tenk1_unique1'); -- error, not a BRIN index
ERROR: "tenk1_unique1" is not a BRIN index
SELECT brin_summarize_new_values('brinidx'); -- ok, no change expected
--
2.17.0
--lQSB8Tqijvu1+4Ba--
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
@ 2023-01-11 11:05 Michael Paquier <[email protected]>
2023-01-11 14:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2023-01-11 11:05 UTC (permalink / raw)
To: Jelte Fennema <[email protected]>; +Cc: [email protected] <[email protected]>; [email protected] <[email protected]>
On Wed, Jan 11, 2023 at 09:04:56AM +0000, Jelte Fennema wrote:
> It's very different. I think easiest is to explain by example:
>
> If there exist three users on the postgres server: admin, jelte and michael
>
> Then this rule (your suggested rule):
> mapname /^(.*)$ \1
>
> Is equivalent to:
> mapname admin admin
> mapname jelte jelte
> mapname michael michael
>
> While with the "all" keyword you can create a rule like this:
> mapname admin all
>
> which is equivalent to:
> mapname admin admin
> mapname admin jelte
> mapname admin michael
Thanks for the explanation, I was missing your point. Hmm. On top
of my mind, couldn't we also use a regexp for the pg-role rather than
just a hardcoded keyword here then, so as it would be possible to
allow a mapping to pass for a group of role names? "all" is just a
pattern to allow everything, at the end.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
2023-01-11 11:05 Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
@ 2023-01-11 14:22 ` Jelte Fennema <[email protected]>
2023-01-12 05:32 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Jelte Fennema @ 2023-01-11 14:22 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Jelte Fennema <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Nathan Bossart <[email protected]>; Andrew Dunstan <[email protected]>
> couldn't we also use a regexp for the pg-role rather than
> just a hardcoded keyword here then, so as it would be possible to
> allow a mapping to pass for a group of role names? "all" is just a
> pattern to allow everything, at the end.
That's a good point. I hadn't realised that you added support for
regexes in pg_hba.conf in 8fea868. Attached is a patchset
where I reuse the pg_hba.conf code path to add support to
pg_ident.conf for: all, +group and regexes.
The main uncertainty I have is if the case insensitivity check is
actually needed in check_role. It seems like a case insensitive
check against the database user shouldn't actually be necessary.
I only understand the need for the case insensitive check against
the system user. But I have too little experience with LDAP/kerberos
to say for certain. So for now I kept the existing behaviour to
not regress.
The patchset also contains 3 preparatory patches with two refactoring
passes and one small bugfix + test additions.
> - renaming "systemuser" to "system_user_token" to outline that this is
> not a simple string but an AuthToken with potentially a regexp?
I decided against this, since now both system user and database user
are tokens. Furthermore, compiler warnings should avoid any confusion
against using this as a normal string. If you feel strongly about this
though, I'm happy to change this.
On Wed, 11 Jan 2023 at 14:34, Michael Paquier <[email protected]> wrote:
>
> On Wed, Jan 11, 2023 at 09:04:56AM +0000, Jelte Fennema wrote:
> > It's very different. I think easiest is to explain by example:
> >
> > If there exist three users on the postgres server: admin, jelte and michael
> >
> > Then this rule (your suggested rule):
> > mapname /^(.*)$ \1
> >
> > Is equivalent to:
> > mapname admin admin
> > mapname jelte jelte
> > mapname michael michael
> >
> > While with the "all" keyword you can create a rule like this:
> > mapname admin all
> >
> > which is equivalent to:
> > mapname admin admin
> > mapname admin jelte
> > mapname admin michael
>
> Thanks for the explanation, I was missing your point. Hmm. On top
> of my mind, couldn't we also use a regexp for the pg-role rather than
> just a hardcoded keyword here then, so as it would be possible to
> allow a mapping to pass for a group of role names? "all" is just a
> pattern to allow everything, at the end.
> --
> Michael
Attachments:
[application/octet-stream] v3-0004-Support-same-user-patterns-in-pg_ident.conf-as-in.patch (14.0K, ../../CAGECzQTkwELHUOAKhvdA+m3tWbUQySHHkExJV8GAZ1pwgbEgXg@mail.gmail.com/2-v3-0004-Support-same-user-patterns-in-pg_ident.conf-as-in.patch)
download | inline diff:
From c852e76a5866b3af70aef98d74132da51e40d88a Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Wed, 11 Jan 2023 12:10:19 +0100
Subject: [PATCH v3 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.
---
doc/src/sgml/client-auth.sgml | 26 +++++-
src/backend/libpq/hba.c | 76 ++++++++++------
src/test/authentication/t/003_peer.pl | 125 ++++++++++++++++++++++++--
3 files changed, 192 insertions(+), 35 deletions(-)
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 08a25a5e002..aefc9de0e45 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,15 @@ mymap /^(.*)@otherdomain\.com$ guest
<literal>\1</literal> makes the <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's
+ currently not possible to use the <literal>\1</literal> to use a capture
+ from a <replaceable>system-username</replaceable> regular expression in a
+ regular expression for a <replaceable>database-username</replaceable>.
+ </para>
<tip>
<para>
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 0c00580dff0..e3c90a7a5bf 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -998,7 +998,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;
@@ -1018,6 +1018,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 +2619,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 +2809,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 +2818,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 +2840,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 +2849,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 +2863,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,9 +2884,17 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
return;
}
+ /*
+ * Create a temporary token with \1 substituted if the string is not
+ * quoted and has no indicaters of other special meanings.
+ */
if (!identLine->pg_user->quoted &&
+ identLine->pg_user->string[0] != '+' &&
+ !token_is_keyword(identLine->pg_user, "all") &&
+ !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 */
@@ -2892,46 +2919,39 @@ 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);
+ 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);
+ *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 968f4a319d8..35a939bcc32 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -98,8 +98,10 @@ 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");
# Extract as well the system user for the user name map.
my $system_user =
@@ -123,12 +125,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 +174,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
reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$},
@@ -179,10 +241,10 @@ test_role(
log_like => [qr/regular expression "\^$system_user\$" has no subexpressions as requested by backreference in "\\1testmapuser"/]);
# 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,
@@ -192,4 +254,55 @@ 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/]);
+
done_testing();
--
2.34.1
[application/octet-stream] v3-0002-Store-pg_user-as-AuthToken-in-IdentLine.patch (4.7K, ../../CAGECzQTkwELHUOAKhvdA+m3tWbUQySHHkExJV8GAZ1pwgbEgXg@mail.gmail.com/3-v3-0002-Store-pg_user-as-AuthToken-in-IdentLine.patch)
download | inline diff:
From 18f669e70ffc2064728416ec43860be564c5cdda Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Wed, 11 Jan 2023 11:51:27 +0100
Subject: [PATCH v3 2/4] Store pg_user as AuthToken in IdentLine
While system_user was stored as an AuthToken in IdentLine, pg_user was
stored as a plain string. This starts storing pg_user as an AuthToken
too in preparation of follow-up commits that start checking if pg_user
was quoted or not.
---
src/backend/libpq/hba.c | 20 +++++++++++---------
src/backend/utils/adt/hbafuncs.c | 2 +-
src/include/libpq/hba.h | 2 +-
3 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 154b2857d2a..029b8e44838 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -2800,7 +2800,7 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel)
tokens = lfirst(field);
IDENT_MULTI_VALUE(tokens);
token = linitial(tokens);
- parsedline->pg_user = pstrdup(token->string);
+ parsedline->pg_user = copy_auth_token(token);
/*
* Now that the field validation is done, compile a regex from the user
@@ -2865,7 +2865,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
return;
}
- if ((ofs = strstr(identLine->pg_user, "\\1")) != NULL)
+ if ((ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
{
int offset;
@@ -2875,7 +2875,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
ereport(LOG,
(errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),
errmsg("regular expression \"%s\" has no subexpressions as requested by backreference in \"%s\"",
- identLine->system_user->string + 1, identLine->pg_user)));
+ identLine->system_user->string + 1, identLine->pg_user->string)));
*error_p = true;
return;
}
@@ -2884,9 +2884,9 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
* length: original length minus length of \1 plus length of match
* plus null terminator
*/
- expanded_pg_user = palloc0(strlen(identLine->pg_user) - 2 + (matches[1].rm_eo - matches[1].rm_so) + 1);
- offset = ofs - identLine->pg_user;
- memcpy(expanded_pg_user, identLine->pg_user, offset);
+ expanded_pg_user = palloc0(strlen(identLine->pg_user->string) - 2 + (matches[1].rm_eo - matches[1].rm_so) + 1);
+ offset = ofs - identLine->pg_user->string;
+ memcpy(expanded_pg_user, identLine->pg_user->string, offset);
memcpy(expanded_pg_user + offset,
system_user + matches[1].rm_so,
matches[1].rm_eo - matches[1].rm_so);
@@ -2895,7 +2895,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
else
{
/* no substitution, so copy the match */
- expanded_pg_user = pstrdup(identLine->pg_user);
+ expanded_pg_user = pstrdup(identLine->pg_user->string);
}
/*
@@ -2921,13 +2921,13 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
/* Not regular expression, so make complete match */
if (case_insensitive)
{
- if (pg_strcasecmp(identLine->pg_user, pg_user) == 0 &&
+ if (pg_strcasecmp(identLine->pg_user->string, pg_user) == 0 &&
pg_strcasecmp(identLine->system_user->string, system_user) == 0)
*found_p = true;
}
else
{
- if (strcmp(identLine->pg_user, pg_user) == 0 &&
+ if (strcmp(identLine->pg_user->string, pg_user) == 0 &&
strcmp(identLine->system_user->string, system_user) == 0)
*found_p = true;
}
@@ -3074,6 +3074,7 @@ load_ident(void)
{
newline = (IdentLine *) lfirst(parsed_line_cell);
free_auth_token(newline->system_user);
+ free_auth_token(newline->pg_user);
}
MemoryContextDelete(ident_context);
return false;
@@ -3086,6 +3087,7 @@ load_ident(void)
{
newline = (IdentLine *) lfirst(parsed_line_cell);
free_auth_token(newline->system_user);
+ free_auth_token(newline->pg_user);
}
}
if (parsed_ident_context != NULL)
diff --git a/src/backend/utils/adt/hbafuncs.c b/src/backend/utils/adt/hbafuncs.c
index 8a552ef8e9d..73d3ad1dadc 100644
--- a/src/backend/utils/adt/hbafuncs.c
+++ b/src/backend/utils/adt/hbafuncs.c
@@ -493,7 +493,7 @@ fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
{
values[index++] = CStringGetTextDatum(ident->usermap);
values[index++] = CStringGetTextDatum(ident->system_user->string);
- values[index++] = CStringGetTextDatum(ident->pg_user);
+ values[index++] = CStringGetTextDatum(ident->pg_user->string);
}
else
{
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index edc504c36e8..15aab5b49b0 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -143,7 +143,7 @@ typedef struct IdentLine
char *usermap;
AuthToken *system_user;
- char *pg_user;
+ AuthToken *pg_user;
} IdentLine;
/*
--
2.34.1
[application/octet-stream] v3-0003-Only-expand-1-in-pg_ident.conf-when-not-quoted.patch (3.4K, ../../CAGECzQTkwELHUOAKhvdA+m3tWbUQySHHkExJV8GAZ1pwgbEgXg@mail.gmail.com/4-v3-0003-Only-expand-1-in-pg_ident.conf-when-not-quoted.patch)
download | inline diff:
From fb3ca78b2d86567bb6572549c99f18fd61209390 Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Wed, 11 Jan 2023 11:55:08 +0100
Subject: [PATCH v3 3/4] Only expand \1 in pg_ident.conf when not quoted
While unlikely, it's possible for usernames to contain the literal
character sequence '\1'. This allows quoting the database-username in
a pg_ident.conf file such that it's possible to match against a literal
'\1' character sequence. This also adds tests for \1 substitution.
---
doc/src/sgml/client-auth.sgml | 3 +++
src/backend/libpq/hba.c | 3 ++-
src/test/authentication/t/003_peer.pl | 37 +++++++++++++++++++++++++++
3 files changed, 42 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index cc8c59206c9..08a25a5e002 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -960,6 +960,9 @@ mymap /^(.*)@otherdomain\.com$ guest
will remove the domain part for users with system user names that end with
<literal>@mydomain.com</literal>, and allow any user whose system name ends with
<literal>@otherdomain.com</literal> to log in as <literal>guest</literal>.
+ Quoting a <replaceable>database-username</replaceable> containing
+ <literal>\1</literal> makes the <literal>\1</literal>
+ lose its special meaning.
</para>
<tip>
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 029b8e44838..0c00580dff0 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -2865,7 +2865,8 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
return;
}
- if ((ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
+ if (!identLine->pg_user->quoted &&
+ (ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
{
int offset;
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 24cefd14e0a..968f4a319d8 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -141,6 +141,43 @@ test_role(
[qr/connection authenticated: identity="$system_user" method=peer/]);
+# Success as the regular expression matches and \1 is replaced
+reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$},
+ 'test\1mapuser');
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 0,
+ 'with regular expression in user name map with \1',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+# Failure since the \1 is part of a quoted string
+reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$},
+ '"test\1mapuser"');
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 2,
+ 'with regular expression in user name map with a quoted \1',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+
+# Failure as the regular expression doesn't contain a group, but the database
+# user contains \1
+reset_pg_ident($node, 'mypeermap', qq{/^$system_user\$},
+ '\1testmapuser');
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 2,
+ 'with regular expression in user name map with \1 without a matching group',
+ log_like => [qr/regular expression "\^$system_user\$" has no subexpressions as requested by backreference in "\\1testmapuser"/]);
+
# Concatenate system_user to system_user.
$regex_test_string = $system_user . $system_user;
--
2.34.1
[application/octet-stream] v3-0001-Make-naming-in-code-for-username-maps-consistent.patch (11.1K, ../../CAGECzQTkwELHUOAKhvdA+m3tWbUQySHHkExJV8GAZ1pwgbEgXg@mail.gmail.com/5-v3-0001-Make-naming-in-code-for-username-maps-consistent.patch)
download | inline diff:
From b5456d4159e6bb3a39a0e280b4eb542adba740de Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Wed, 28 Dec 2022 09:48:31 +0100
Subject: [PATCH v3 1/4] Make naming in code for username maps consistent
The code that handled authentication for username maps was pretty
confusing. It involves two types of users, but these two users were not
named consistently throughout the code. This changes the following
things to improve the situation:
1. Align on the names pg_user and system_user. These names are
consistent with the pg_ident.conf example in the docs.
2. Rename regexp_pgrole to expanded pg_user, because this variable does
not actually store a regular expression, but instead stores a string
where the \1 is substituted with a regex match
3. Order the fields in the IdentLine struct according to the column
order of pg_ident.conf
One reason for this change is to prepare for a future commit where
pg_user will become a token.
---
src/backend/libpq/hba.c | 78 ++++++++++++++++----------------
src/backend/utils/adt/hbafuncs.c | 4 +-
src/include/libpq/hba.h | 6 +--
3 files changed, 44 insertions(+), 44 deletions(-)
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index f5a2cc53be5..154b2857d2a 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -2792,7 +2792,7 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel)
token = linitial(tokens);
/* Copy the ident user token */
- parsedline->token = copy_auth_token(token);
+ parsedline->system_user = copy_auth_token(token);
/* Get the PG rolename token */
field = lnext(tok_line->fields, field);
@@ -2800,13 +2800,13 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel)
tokens = lfirst(field);
IDENT_MULTI_VALUE(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
* 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 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.
*
- * 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.
*/
static void
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)
{
*found_p = false;
@@ -2835,7 +2835,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
return;
/* Match? */
- if (token_has_regexp(identLine->token))
+ if (token_has_regexp(identLine->system_user))
{
/*
* 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;
regmatch_t matches[2];
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)
{
char errstr[100];
@@ -2855,17 +2855,17 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
if (r != REG_NOMATCH)
{
/* 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,
(errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),
errmsg("regular expression match for \"%s\" failed: %s",
- identLine->token->string + 1, errstr)));
+ identLine->system_user->string + 1, errstr)));
*error_p = true;
}
return;
}
- if ((ofs = strstr(identLine->pg_role, "\\1")) != NULL)
+ if ((ofs = strstr(identLine->pg_user, "\\1")) != NULL)
{
int offset;
@@ -2875,7 +2875,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
ereport(LOG,
(errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),
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;
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
* plus null terminator
*/
- regexp_pgrole = palloc0(strlen(identLine->pg_role) - 2 + (matches[1].rm_eo - matches[1].rm_so) + 1);
- offset = ofs - identLine->pg_role;
- memcpy(regexp_pgrole, identLine->pg_role, offset);
- memcpy(regexp_pgrole + offset,
- ident_user + matches[1].rm_so,
+ expanded_pg_user = palloc0(strlen(identLine->pg_user) - 2 + (matches[1].rm_eo - matches[1].rm_so) + 1);
+ offset = ofs - identLine->pg_user;
+ memcpy(expanded_pg_user, identLine->pg_user, offset);
+ memcpy(expanded_pg_user + offset,
+ system_user + matches[1].rm_so,
matches[1].rm_eo - matches[1].rm_so);
- strcat(regexp_pgrole, ofs + 2);
+ strcat(expanded_pg_user, ofs + 2);
}
else
{
/* 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 (pg_strcasecmp(regexp_pgrole, pg_role) == 0)
+ if (pg_strcasecmp(expanded_pg_user, pg_user) == 0)
*found_p = true;
}
else
{
- if (strcmp(regexp_pgrole, pg_role) == 0)
+ if (strcmp(expanded_pg_user, pg_user) == 0)
*found_p = true;
}
- pfree(regexp_pgrole);
+ pfree(expanded_pg_user);
return;
}
@@ -2921,14 +2921,14 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
/* Not regular expression, so make complete match */
if (case_insensitive)
{
- if (pg_strcasecmp(identLine->pg_role, pg_role) == 0 &&
- pg_strcasecmp(identLine->token->string, ident_user) == 0)
+ if (pg_strcasecmp(identLine->pg_user, pg_user) == 0 &&
+ pg_strcasecmp(identLine->system_user->string, system_user) == 0)
*found_p = true;
}
else
{
- if (strcmp(identLine->pg_role, pg_role) == 0 &&
- strcmp(identLine->token->string, ident_user) == 0)
+ if (strcmp(identLine->pg_user, pg_user) == 0 &&
+ strcmp(identLine->system_user->string, system_user) == 0)
*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
*
- * See if the user with ident username "auth_user" is allowed to act
- * as Postgres user "pg_role" according to usermap "usermap_name".
+ * See if the system user with ident username "system_user" is allowed to act as
+ * Postgres user "pg_user" according to usermap "usermap_name".
*
* Special case: Usermap NULL, equivalent to what was previously called
* "sameuser" or "samerole", means don't look in the usermap file.
- * That's an implied map wherein "pg_role" must be identical to
- * "auth_user" in order to be authorized.
+ * That's an implied map wherein "pg_user" must be identical to
+ * "system_user" in order to be authorized.
*
* Iff authorized, return STATUS_OK, otherwise return STATUS_ERROR.
*/
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 found_entry = false,
@@ -2961,17 +2961,17 @@ check_usermap(const char *usermap_name,
{
if (case_insensitive)
{
- if (pg_strcasecmp(pg_role, auth_user) == 0)
+ if (pg_strcasecmp(pg_user, system_user) == 0)
return STATUS_OK;
}
else
{
- if (strcmp(pg_role, auth_user) == 0)
+ if (strcmp(pg_user, system_user) == 0)
return STATUS_OK;
}
ereport(LOG,
(errmsg("provided user name (%s) and authenticated user name (%s) do not match",
- pg_role, auth_user)));
+ pg_user, system_user)));
return STATUS_ERROR;
}
else
@@ -2981,7 +2981,7 @@ check_usermap(const char *usermap_name,
foreach(line_cell, parsed_ident_lines)
{
check_ident_usermap(lfirst(line_cell), usermap_name,
- pg_role, auth_user, case_insensitive,
+ pg_user, system_user, case_insensitive,
&found_entry, &error);
if (found_entry || error)
break;
@@ -2991,7 +2991,7 @@ check_usermap(const char *usermap_name,
{
ereport(LOG,
(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;
}
@@ -3073,7 +3073,7 @@ load_ident(void)
foreach(parsed_line_cell, new_parsed_lines)
{
newline = (IdentLine *) lfirst(parsed_line_cell);
- free_auth_token(newline->token);
+ free_auth_token(newline->system_user);
}
MemoryContextDelete(ident_context);
return false;
@@ -3085,7 +3085,7 @@ load_ident(void)
foreach(parsed_line_cell, parsed_ident_lines)
{
newline = (IdentLine *) lfirst(parsed_line_cell);
- free_auth_token(newline->token);
+ free_auth_token(newline->system_user);
}
}
if (parsed_ident_context != NULL)
diff --git a/src/backend/utils/adt/hbafuncs.c b/src/backend/utils/adt/hbafuncs.c
index 3cd83c77f8f..8a552ef8e9d 100644
--- a/src/backend/utils/adt/hbafuncs.c
+++ b/src/backend/utils/adt/hbafuncs.c
@@ -492,8 +492,8 @@ fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
if (ident != NULL)
{
values[index++] = CStringGetTextDatum(ident->usermap);
- values[index++] = CStringGetTextDatum(ident->token->string);
- values[index++] = CStringGetTextDatum(ident->pg_role);
+ values[index++] = CStringGetTextDatum(ident->system_user->string);
+ values[index++] = CStringGetTextDatum(ident->pg_user);
}
else
{
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index 90c51ad6fa9..edc504c36e8 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -142,8 +142,8 @@ typedef struct IdentLine
int linenumber;
char *usermap;
- char *pg_role;
- AuthToken *token;
+ AuthToken *system_user;
+ char *pg_user;
} IdentLine;
/*
@@ -172,7 +172,7 @@ extern bool load_ident(void);
extern const char *hba_authname(UserAuth auth_method);
extern void hba_getauthmethod(hbaPort *port);
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);
extern HbaLine *parse_hba_line(TokenizedAuthLine *tok_line, int elevel);
extern IdentLine *parse_ident_line(TokenizedAuthLine *tok_line, int elevel);
--
2.34.1
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
2023-01-11 11:05 Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-11 14:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
@ 2023-01-12 05:32 ` Michael Paquier <[email protected]>
2023-01-12 09:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2023-01-12 05:32 UTC (permalink / raw)
To: Jelte Fennema <[email protected]>; +Cc: Jelte Fennema <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Nathan Bossart <[email protected]>; Andrew Dunstan <[email protected]>
On Wed, Jan 11, 2023 at 03:22:35PM +0100, Jelte Fennema wrote:
> The main uncertainty I have is if the case insensitivity check is
> actually needed in check_role. It seems like a case insensitive
> check against the database user shouldn't actually be necessary.
> I only understand the need for the case insensitive check against
> the system user. But I have too little experience with LDAP/kerberos
> to say for certain. So for now I kept the existing behaviour to
> not regress.
if (!identLine->pg_user->quoted &&
+ identLine->pg_user->string[0] != '+' &&
+ !token_is_keyword(identLine->pg_user, "all") &&
+ !token_has_regexp(identLine->pg_user) &&
If we finish by allowing a regexp for the PG user in an IdentLine, I
would choose to drop "all" entirely. Simpler is better when it comes
to authentication, though we are working on getting things more..
Complicated.
+ Quoting a <replaceable>database-username</replaceable> containing
+ <literal>\1</literal> makes the <literal>\1</literal>
+ lose its special meaning.
0002 and 0003 need careful thinking.
+# Success as the regular expression matches and \1 is replaced
+reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$},
+ 'test\1mapuser');
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 0,
+ 'with regular expression in user name map with \1',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
Relying on kerberos to check the substitution pattern is a bit
annoying.. I would be really tempted to extract and commit that
independently of the rest, actually, to provide some coverage of the
substitution case in the peer test.
> The patchset also contains 3 preparatory patches with two refactoring
> passes and one small bugfix + test additions.
Applied 0001, which looked fine and was an existing issue. At the
end, I had no issues with the names you suggested.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
2023-01-11 11:05 Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-11 14:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-12 05:32 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
@ 2023-01-12 09:10 ` Jelte Fennema <[email protected]>
2023-01-13 02:09 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Jelte Fennema @ 2023-01-12 09:10 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Jelte Fennema <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Nathan Bossart <[email protected]>; Andrew Dunstan <[email protected]>
> Simpler is better when it comes to authentication
I definitely agree with that, and if we didn't have existing
parsing logic for pg_hba.conf I would agree. But given the existing
logic for pg_hba.conf, I think the path of least surprises is to
support all of the same patterns that pg_hbac.conf supports.
It also makes the code simpler as we can simply reuse the
check_role function, since that. I removed the lines you quoted
since those are actually not strictly necessary. They only change
the detection logic a bit in case of a \1 existing in the string.
And I'm not sure what the desired behaviour is for those.
> I would be really tempted to extract and commit that
> independently of the rest, actually, to provide some coverage of the
> substitution case in the peer test.
I split up that patch in two parts now and added the tests in a new 0001
patch.
> 0002 and 0003 need careful thinking.
0002 should change no behaviour, since it simply stores the token in
the IdentLine struct, but doesn't start using the quoted or the regex field
yet. 0003 is debatable indeed. To me it makes sense conceptually, but
having a literal \1 in a username seems like an unlikely scenario and
there might be pg_ident.conf files in existence where the \1 is quoted
that would break because of this change. I haven't removed 0003 from
the patch set yet, but I kinda feel that the advantage is probably not
worth the risk of breakage.
0004 adds some breakage too. But there I think the advantages far outweigh
the risk of breakage. Both because added functionality is a much bigger
advantage, and because we only risk breaking when there exist users that
are called "all", start with a literal + or start with a literal /.
Only "all" seems
like a somewhat reasonable username, but such a user existing seems
unlikely to me given all its special meaning in pg_hba.conf. (I added this
consideration to the commit message)
> > The main uncertainty I have is if the case insensitivity check is
> > actually needed in check_role. It seems like a case insensitive
> > check against the database user shouldn't actually be necessary.
> > I only understand the need for the case insensitive check against
> > the system user. But I have too little experience with LDAP/kerberos
> > to say for certain. So for now I kept the existing behaviour to
> > not regress.
You didn't write a response about this, but you did quote it. Did you intend
to respond to it?
> Applied 0001
Awesome :)
Finally, one separate thing I noticed is that regcomp_auth_token only
checks the / prefix, but doesn't check if the token was quoted or not.
So even if it's quoted it will be interpreted as a regex. Maybe we should
change that? At least for the regex parsing that is not released yet.
Attachments:
[application/octet-stream] v4-0003-Only-expand-1-in-pg_ident.conf-when-not-quoted.patch (2.6K, ../../CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com/2-v4-0003-Only-expand-1-in-pg_ident.conf-when-not-quoted.patch)
download | inline diff:
From a789b47c401d263ff065cca1ca98b0e33deb0c7f Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Thu, 12 Jan 2023 09:23:45 +0100
Subject: [PATCH v4 3/4] Only expand \1 in pg_ident.conf when not quoted
While unlikely, it's possible for usernames to contain the literal
character sequence '\1'. This allows quoting the database-username in
a pg_ident.conf file such that it's possible to match against a literal
'\1' character sequence.
---
doc/src/sgml/client-auth.sgml | 3 +++
src/backend/libpq/hba.c | 3 ++-
src/test/authentication/t/003_peer.pl | 12 ++++++++++++
3 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index cc8c59206c9..08a25a5e002 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -960,6 +960,9 @@ mymap /^(.*)@otherdomain\.com$ guest
will remove the domain part for users with system user names that end with
<literal>@mydomain.com</literal>, and allow any user whose system name ends with
<literal>@otherdomain.com</literal> to log in as <literal>guest</literal>.
+ Quoting a <replaceable>database-username</replaceable> containing
+ <literal>\1</literal> makes the <literal>\1</literal>
+ lose its special meaning.
</para>
<tip>
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 029b8e44838..0c00580dff0 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -2865,7 +2865,8 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
return;
}
- if ((ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
+ if (!identLine->pg_user->quoted &&
+ (ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
{
int offset;
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index c41f146b35e..9f397ee220a 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -153,6 +153,18 @@ test_role(
log_like =>
[qr/connection authenticated: identity="$system_user" method=peer/]);
+# Failure since the \1 is part of a quoted string
+reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$},
+ '"test\1mapuser"');
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 2,
+ 'with regular expression in user name map with a quoted \1',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
+
# Failure as the regular expression doesn't contain a group, but the database
# user contains \1
reset_pg_ident($node, 'mypeermap', qq{/^$system_user\$},
--
2.34.1
[application/octet-stream] v4-0001-Add-tests-for-1-in-pg_ident.conf-to-0003_peer.pl.patch (2.0K, ../../CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com/3-v4-0001-Add-tests-for-1-in-pg_ident.conf-to-0003_peer.pl.patch)
download | inline diff:
From b8bd5bc0dfce59ddf32ba8510156350aed40ea31 Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Thu, 12 Jan 2023 09:18:19 +0100
Subject: [PATCH v4 1/4] Add tests for \1 in pg_ident.conf to 0003_peer.pl
There were no tests for \1 substitution in our main pg_ident.conf tests.
The reason was probably that it's not obvious how to get a consistent
capturing group given an arbitrary username. To get a consistent
capturing group value this patch always captures an empty string. While
it would be better to test a substitution where the group actually
captures some characters, the newly added test is better than having no
test at all.
---
src/test/authentication/t/003_peer.pl | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 24cefd14e0a..c41f146b35e 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -141,6 +141,30 @@ test_role(
[qr/connection authenticated: identity="$system_user" method=peer/]);
+# Success as the regular expression matches and \1 is replaced
+reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$},
+ 'test\1mapuser');
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 0,
+ 'with regular expression in user name map with \1',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
+
+# Failure as the regular expression doesn't contain a group, but the database
+# user contains \1
+reset_pg_ident($node, 'mypeermap', qq{/^$system_user\$},
+ '\1testmapuser');
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 2,
+ 'with regular expression in user name map with \1 without a matching group',
+ log_like => [qr/regular expression "\^$system_user\$" has no subexpressions as requested by backreference in "\\1testmapuser"/]);
+
# Concatenate system_user to system_user.
$regex_test_string = $system_user . $system_user;
--
2.34.1
[application/octet-stream] v4-0002-Store-pg_user-as-AuthToken-in-IdentLine.patch (4.7K, ../../CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com/4-v4-0002-Store-pg_user-as-AuthToken-in-IdentLine.patch)
download | inline diff:
From 150fbec895d59b7bffc48ecf0d84ba003867321c Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Wed, 11 Jan 2023 11:51:27 +0100
Subject: [PATCH v4 2/4] Store pg_user as AuthToken in IdentLine
While system_user was stored as an AuthToken in IdentLine, pg_user was
stored as a plain string. This starts storing pg_user as an AuthToken
too in preparation of follow-up commits that start checking if pg_user
was quoted or not.
---
src/backend/libpq/hba.c | 20 +++++++++++---------
src/backend/utils/adt/hbafuncs.c | 2 +-
src/include/libpq/hba.h | 2 +-
3 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 154b2857d2a..029b8e44838 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -2800,7 +2800,7 @@ parse_ident_line(TokenizedAuthLine *tok_line, int elevel)
tokens = lfirst(field);
IDENT_MULTI_VALUE(tokens);
token = linitial(tokens);
- parsedline->pg_user = pstrdup(token->string);
+ parsedline->pg_user = copy_auth_token(token);
/*
* Now that the field validation is done, compile a regex from the user
@@ -2865,7 +2865,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
return;
}
- if ((ofs = strstr(identLine->pg_user, "\\1")) != NULL)
+ if ((ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
{
int offset;
@@ -2875,7 +2875,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
ereport(LOG,
(errcode(ERRCODE_INVALID_REGULAR_EXPRESSION),
errmsg("regular expression \"%s\" has no subexpressions as requested by backreference in \"%s\"",
- identLine->system_user->string + 1, identLine->pg_user)));
+ identLine->system_user->string + 1, identLine->pg_user->string)));
*error_p = true;
return;
}
@@ -2884,9 +2884,9 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
* length: original length minus length of \1 plus length of match
* plus null terminator
*/
- expanded_pg_user = palloc0(strlen(identLine->pg_user) - 2 + (matches[1].rm_eo - matches[1].rm_so) + 1);
- offset = ofs - identLine->pg_user;
- memcpy(expanded_pg_user, identLine->pg_user, offset);
+ expanded_pg_user = palloc0(strlen(identLine->pg_user->string) - 2 + (matches[1].rm_eo - matches[1].rm_so) + 1);
+ offset = ofs - identLine->pg_user->string;
+ memcpy(expanded_pg_user, identLine->pg_user->string, offset);
memcpy(expanded_pg_user + offset,
system_user + matches[1].rm_so,
matches[1].rm_eo - matches[1].rm_so);
@@ -2895,7 +2895,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
else
{
/* no substitution, so copy the match */
- expanded_pg_user = pstrdup(identLine->pg_user);
+ expanded_pg_user = pstrdup(identLine->pg_user->string);
}
/*
@@ -2921,13 +2921,13 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
/* Not regular expression, so make complete match */
if (case_insensitive)
{
- if (pg_strcasecmp(identLine->pg_user, pg_user) == 0 &&
+ if (pg_strcasecmp(identLine->pg_user->string, pg_user) == 0 &&
pg_strcasecmp(identLine->system_user->string, system_user) == 0)
*found_p = true;
}
else
{
- if (strcmp(identLine->pg_user, pg_user) == 0 &&
+ if (strcmp(identLine->pg_user->string, pg_user) == 0 &&
strcmp(identLine->system_user->string, system_user) == 0)
*found_p = true;
}
@@ -3074,6 +3074,7 @@ load_ident(void)
{
newline = (IdentLine *) lfirst(parsed_line_cell);
free_auth_token(newline->system_user);
+ free_auth_token(newline->pg_user);
}
MemoryContextDelete(ident_context);
return false;
@@ -3086,6 +3087,7 @@ load_ident(void)
{
newline = (IdentLine *) lfirst(parsed_line_cell);
free_auth_token(newline->system_user);
+ free_auth_token(newline->pg_user);
}
}
if (parsed_ident_context != NULL)
diff --git a/src/backend/utils/adt/hbafuncs.c b/src/backend/utils/adt/hbafuncs.c
index 8a552ef8e9d..73d3ad1dadc 100644
--- a/src/backend/utils/adt/hbafuncs.c
+++ b/src/backend/utils/adt/hbafuncs.c
@@ -493,7 +493,7 @@ fill_ident_line(Tuplestorestate *tuple_store, TupleDesc tupdesc,
{
values[index++] = CStringGetTextDatum(ident->usermap);
values[index++] = CStringGetTextDatum(ident->system_user->string);
- values[index++] = CStringGetTextDatum(ident->pg_user);
+ values[index++] = CStringGetTextDatum(ident->pg_user->string);
}
else
{
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index ed4d5e7962c..189f6d0df24 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -143,7 +143,7 @@ typedef struct IdentLine
char *usermap;
AuthToken *system_user;
- char *pg_user;
+ AuthToken *pg_user;
} IdentLine;
/*
--
2.34.1
[application/octet-stream] v4-0004-Support-same-user-patterns-in-pg_ident.conf-as-in.patch (14.4K, ../../CAGECzQRNow4MwkBjgPxywXdJU_K3a9+Pm78JB7De3yQwwkTDew@mail.gmail.com/5-v4-0004-Support-same-user-patterns-in-pg_ident.conf-as-in.patch)
download | inline diff:
From 91cce919109fadd3fc60c5f041eb97542f692c5f Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Wed, 11 Jan 2023 12:10:19 +0100
Subject: [PATCH v4 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 | 74 +++++++++------
src/test/authentication/t/003_peer.pl | 125 ++++++++++++++++++++++++--
3 files changed, 190 insertions(+), 35 deletions(-)
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 08a25a5e002..aefc9de0e45 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,15 @@ mymap /^(.*)@otherdomain\.com$ guest
<literal>\1</literal> makes the <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's
+ currently not possible to use the <literal>\1</literal> to use a capture
+ from a <replaceable>system-username</replaceable> regular expression in a
+ regular expression for a <replaceable>database-username</replaceable>.
+ </para>
<tip>
<para>
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 0c00580dff0..73b92506304 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -998,7 +998,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;
@@ -1018,6 +1018,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 +2619,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 +2809,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 +2818,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 +2840,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 +2849,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 +2863,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)
@@ -2868,6 +2887,7 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
if (!identLine->pg_user->quoted &&
(ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
{
+ char *expanded_pg_user;
int offset;
/* substitution of the first argument requested */
@@ -2892,46 +2912,44 @@ 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 special meanings like, 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
{
- /* 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);
+ *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 9f397ee220a..64b16025433 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -98,8 +98,10 @@ 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");
# Extract as well the system user for the user name map.
my $system_user =
@@ -123,12 +125,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 +174,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
reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$},
@@ -178,10 +240,10 @@ test_role(
log_like => [qr/regular expression "\^$system_user\$" has no subexpressions as requested by backreference in "\\1testmapuser"/]);
# 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 +253,55 @@ 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/]);
+
done_testing();
--
2.34.1
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
2023-01-11 11:05 Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-11 14:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-12 05:32 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-12 09:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
@ 2023-01-13 02:09 ` Michael Paquier <[email protected]>
2023-01-13 08:19 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2023-01-13 02:09 UTC (permalink / raw)
To: Jelte Fennema <[email protected]>; +Cc: Jelte Fennema <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Nathan Bossart <[email protected]>; Andrew Dunstan <[email protected]>
On Thu, Jan 12, 2023 at 10:10:02AM +0100, Jelte Fennema wrote:
> It also makes the code simpler as we can simply reuse the
> check_role function, since that. I removed the lines you quoted
> since those are actually not strictly necessary. They only change
> the detection logic a bit in case of a \1 existing in the string.
> And I'm not sure what the desired behaviour is for those.
Hmm. This is a very good point. 0004 gets really easy to follow
now.
> I split up that patch in two parts now and added the tests in a new 0001
> patch.
Thanks, applied 0001.
> 0002 should change no behaviour, since it simply stores the token in
> the IdentLine struct, but doesn't start using the quoted or the regex field
> yet. 0003 is debatable indeed. To me it makes sense conceptually, but
> having a literal \1 in a username seems like an unlikely scenario and
> there might be pg_ident.conf files in existence where the \1 is quoted
> that would break because of this change. I haven't removed 0003 from
> the patch set yet, but I kinda feel that the advantage is probably not
> worth the risk of breakage.
0003 would allow folks to use \1 in a Postgres username if quoted. My
choice would be to agree with you here. Even if folks applying quotes
would not be able anymore to replace the pattern, the risk seems a bit
remote? I would suspect that basically everybody does not rely on
'\1' being in the middle of pg-username string, using it only as a
strict replacement of the result coming from system-username to keep a
simpler mapping between the PG roles and the krb5/gss system roles.
Even if they use a more complex schema that depends on strstr(),
things would break if they began the pg-username with quotes. Put it
simply, I'd agree with your 0003.
> 0004 adds some breakage too. But there I think the advantages far outweigh
> the risk of breakage. Both because added functionality is a much bigger
> advantage, and because we only risk breaking when there exist users that
> are called "all", start with a literal + or start with a literal /.
> Only "all" seems
> like a somewhat reasonable username, but such a user existing seems
> unlikely to me given all its special meaning in pg_hba.conf. (I added this
> consideration to the commit message)
I don't see how much that's different from the recent discussion with
regexps added for databases and users to pg_hba.conf. And consistency
sounds pretty good to me here.
> Finally, one separate thing I noticed is that regcomp_auth_token only
> checks the / prefix, but doesn't check if the token was quoted or not.
> So even if it's quoted it will be interpreted as a regex. Maybe we should
> change that? At least for the regex parsing that is not released yet.
regcomp_auth_token() should not decide to compile a regexp depending
on if an AuthToken is quoted or not. Regexps can have commas, and
this would impact the case of database or role lists in HBA entries.
And that could be an issue with spaces as well? See the docs for
patterns like:
db1,"/^db\d{2,4}$",db2
Point taken that we don't care about lists for pg_ident entries,
though.
> You didn't write a response about this, but you did quote it. Did you intend
> to respond to it?
Nah, I should have deleted it. I had no useful opinion on this
particular point.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
2023-01-11 11:05 Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-11 14:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-12 05:32 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-12 09:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-13 02:09 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
@ 2023-01-13 08:19 ` Jelte Fennema <[email protected]>
2023-01-16 05:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Jelte Fennema @ 2023-01-13 08:19 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Jelte Fennema <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Nathan Bossart <[email protected]>; Andrew Dunstan <[email protected]>
> Even if folks applying quotes
> would not be able anymore to replace the pattern, the risk seems a bit
> remote?
Yeah I agree the risk is remote. To be clear, the main pattern I'm
worried about breaking is simply "\1". Where people had put
quotes around \1 for no reason. All in all, I'm fine if 0003 gets
merged, but I'd also be fine with it if it doesn't. Both the risk
and the advantage seem fairly small.
> I don't see how much that's different from the recent discussion with
> regexps added for databases and users to pg_hba.conf. And consistency
> sounds pretty good to me here.
It's not much different, except that here also all and + change their meaning
(for pg_hba.conf those special cases already existed). Mainly I called it out
because I realised this discussion was called out in that commit too.
> Regexps can have commas
That's a really good reason to allow quoted regexes indeed. Even for pg_ident
entries, commas in unquoted regexes would cause the AuthToken parsing to fail.
Is there anything you still want to see changed about any of the patches?
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
2023-01-11 11:05 Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-11 14:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-12 05:32 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-12 09:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-13 02:09 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-13 08:19 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
@ 2023-01-16 05:22 ` Michael Paquier <[email protected]>
2023-01-16 10:53 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2023-01-16 05:22 UTC (permalink / raw)
To: Jelte Fennema <[email protected]>; +Cc: Jelte Fennema <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Nathan Bossart <[email protected]>; Andrew Dunstan <[email protected]>
On Fri, Jan 13, 2023 at 09:19:10AM +0100, Jelte Fennema wrote:
>> Even if folks applying quotes
>> would not be able anymore to replace the pattern, the risk seems a bit
>> remote?
>
> Yeah I agree the risk is remote. To be clear, the main pattern I'm
> worried about breaking is simply "\1". Where people had put
> quotes around \1 for no reason. All in all, I'm fine if 0003 gets
> merged, but I'd also be fine with it if it doesn't. Both the risk
> and the advantage seem fairly small.
Still, I am having a few second thoughts about 0003 after thinking
about it over the weekend. Except if I am missing something, there
are no issues with 0004 if we keep the current behavior of always
replacing \1 even if pg-user is quoted? I would certainly add a new
test case either way.
>> I don't see how much that's different from the recent discussion with
>> regexps added for databases and users to pg_hba.conf. And consistency
>> sounds pretty good to me here.
>
> It's not much different, except that here also all and + change their meaning
> (for pg_hba.conf those special cases already existed). Mainly I called it out
> because I realised this discussion was called out in that commit too.
>
>> Regexps can have commas
>
> That's a really good reason to allow quoted regexes indeed. Even for pg_ident
> entries, commas in unquoted regexes would cause the AuthToken parsing to fail.
>
> Is there anything you still want to see changed about any of the patches?
+ /*
+ * Mark the token as quoted, so it will only be compared literally
+ * and not for special meanings like, such as "all" and membership
+ * checks using the + prefix.
+ */
+ expanded_pg_user_token = make_auth_token(expanded_pg_user, true);
It is critical to quote this AuthToken after the replacement, indeed.
Or we are in big trouble.
- /* no substitution, so copy the match */
- expanded_pg_user = pstrdup(identLine->pg_user->string);
+ expanded_pg_user_token = identLine->pg_user;
Perhaps it would be simpler to use copy_auth_token() in this code path
and always free the resulting token?
In the code path where system-user is a regexp, could it be better
to skip the replacement of \1 in the new AuthToken if pg-user is
itself a regexp? The compiled regexp would be the same, but it could
be considered as a bit confusing, as it can be thought that the
compiled regexp of pg-user happened after the replacement?
No issues with 0002 after a second look, so applied to move on.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
2023-01-11 11:05 Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-11 14:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-12 05:32 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-12 09:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-13 02:09 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-13 08:19 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-16 05:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
@ 2023-01-16 10:53 ` Jelte Fennema <[email protected]>
2023-01-17 05:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Jelte Fennema @ 2023-01-16 10:53 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Jelte Fennema <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Nathan Bossart <[email protected]>; Andrew Dunstan <[email protected]>
> Still, I am having a few second thoughts about 0003 after thinking
> about it over the weekend. Except if I am missing something, there
> are no issues with 0004 if we keep the current behavior of always
> replacing \1 even if pg-user is quoted? I would certainly add a new
> test case either way.
Yes, 0004 is not dependent on 003 at all. I attached a new version
of 0003 where only a test and some documentation is added.
> Perhaps it would be simpler to use copy_auth_token() in this code path
> and always free the resulting token?
I initially tried that when working on the patch, but copy_auth_token
(surprisingly) doesn't copy the regex field into the new AuthToken.
So we'd have to regenerate it conditionally. Making the copy
conditional seemed just as simple code-wise, with the added
bonus that it's not doing a useless copy.
> In the code path where system-user is a regexp, could it be better
> to skip the replacement of \1 in the new AuthToken if pg-user is
> itself a regexp? The compiled regexp would be the same, but it could
> be considered as a bit confusing, as it can be thought that the
> compiled regexp of pg-user happened after the replacement?
I updated 0004 to prioritize membership checks and regexes over
substitution of \1. I also added tests for this. Prioritizing "all" over
substitution of \1 is not necessary, since by definition "all" does
not include \1.
Attachments:
[application/octet-stream] v5-0003-Only-expand-1-in-pg_ident.conf-when-not-quoted.patch (2.2K, ../../CAGECzQTuW=JwY6-eVRXMxP=_UeH7K_asAOOXXH8-1fHGm-Gbsg@mail.gmail.com/2-v5-0003-Only-expand-1-in-pg_ident.conf-when-not-quoted.patch)
download | inline diff:
From 053f4a0a63fb8e561faf66af94f6f8f16ff737ad Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
Date: Thu, 12 Jan 2023 09:23:45 +0100
Subject: [PATCH v5 3/4] Only expand \1 in pg_ident.conf when not quoted
While unlikely, it's possible for usernames to contain the literal
character sequence '\1'. This allows quoting the database-username in
a pg_ident.conf file such that it's possible to match against a literal
'\1' character sequence.
---
doc/src/sgml/client-auth.sgml | 3 +++
src/test/authentication/t/003_peer.pl | 12 ++++++++++++
2 files changed, 15 insertions(+)
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index cc8c59206c9..50af2bf03b8 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -960,6 +960,9 @@ mymap /^(.*)@otherdomain\.com$ guest
will remove the domain part for users with system user names that end with
<literal>@mydomain.com</literal>, and allow any user whose system name ends with
<literal>@otherdomain.com</literal> to log in as <literal>guest</literal>.
+ Quoting a <replaceable>database-username</replaceable> containing
+ <literal>\1</literal> <emphasis>does not</emphasis> make <literal>\1</literal>
+ lose its special meaning.
</para>
<tip>
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 966b2aa47ef..796fd059ccd 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -153,6 +153,18 @@ test_role(
log_like =>
[qr/connection authenticated: identity="$system_user" method=peer/]);
+# Success as the regular expression matches and \1 is replaced in the given
+# subexpression, even in a quoted string.
+reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$}, '"test\1mapuser"');
+test_role(
+ $node,
+ qq{testmapuser},
+ 'peer',
+ 0,
+ 'with regular expression in user name map replacing a quoted \1',
+ log_like =>
+ [qr/connection authenticated: identity="$system_user" method=peer/]);
+
# Failure as the regular expression does not include a subexpression, but
# the database user contains \1, requesting a replacement.
reset_pg_ident($node, 'mypeermap', qq{/^$system_user\$}, '\1testmapuser');
--
2.34.1
[application/octet-stream] v5-0004-Support-same-user-patterns-in-pg_ident.conf-as-in.patch (15.9K, ../../CAGECzQTuW=JwY6-eVRXMxP=_UeH7K_asAOOXXH8-1fHGm-Gbsg@mail.gmail.com/3-v5-0004-Support-same-user-patterns-in-pg_ident.conf-as-in.patch)
download | inline diff:
From 7f84fff3b8068c757b757387b50ed3c4d0a5f3a3 Mon Sep 17 00:00:00 2001
From: Jelte Fennema <[email protected]>
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 <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,15 @@ 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's
+ not possible to use the <literal>\1</literal> to use a capture from
+ a <replaceable>system-username</replaceable> regular expression in a
+ regular expression for a <replaceable>database-username</replaceable>.
+ </para>
<tip>
<para>
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
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
2023-01-11 11:05 Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-11 14:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-12 05:32 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-12 09:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-13 02:09 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-13 08:19 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-16 05:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-16 10:53 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
@ 2023-01-17 05:10 ` Michael Paquier <[email protected]>
2023-01-18 09:35 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2023-01-17 05:10 UTC (permalink / raw)
To: Jelte Fennema <[email protected]>; +Cc: Jelte Fennema <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Nathan Bossart <[email protected]>; Andrew Dunstan <[email protected]>
On Mon, Jan 16, 2023 at 11:53:57AM +0100, Jelte Fennema wrote:
>> Perhaps it would be simpler to use copy_auth_token() in this code path
>> and always free the resulting token?
>
> I initially tried that when working on the patch, but copy_auth_token
> (surprisingly) doesn't copy the regex field into the new AuthToken.
> So we'd have to regenerate it conditionally. Making the copy
> conditional seemed just as simple code-wise, with the added
> bonus that it's not doing a useless copy.
Okay, I can live with that.
>> In the code path where system-user is a regexp, could it be better
>> to skip the replacement of \1 in the new AuthToken if pg-user is
>> itself a regexp? The compiled regexp would be the same, but it could
>> be considered as a bit confusing, as it can be thought that the
>> compiled regexp of pg-user happened after the replacement?
>
> I updated 0004 to prioritize membership checks and regexes over
> substitution of \1. I also added tests for this. Prioritizing "all" over
> substitution of \1 is not necessary, since by definition "all" does
> not include \1.
Thanks, 0003 is OK, so applied now.
0004 looks fine as well, be it for the tests (I am hesitating to tweak
things a bit here actually for the role names), the code or the docs,
still I am planning a second lookup.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
2023-01-11 11:05 Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-11 14:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-12 05:32 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-12 09:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-13 02:09 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-13 08:19 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-16 05:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-16 10:53 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-17 05:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
@ 2023-01-18 09:35 ` Jelte Fennema <[email protected]>
2023-01-19 01:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-19 07:56 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
0 siblings, 2 replies; 15+ messages in thread
From: Jelte Fennema @ 2023-01-18 09:35 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Jelte Fennema <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Nathan Bossart <[email protected]>; Andrew Dunstan <[email protected]>
> 0004 looks fine as well, be it for the tests (I am hesitating to tweak
> things a bit here actually for the role names), the code or the docs,
Anything I can do to help with this? Or will you do that yourself?
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
2023-01-11 11:05 Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-11 14:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-12 05:32 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-12 09:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-13 02:09 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-13 08:19 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-16 05:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-16 10:53 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-17 05:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-18 09:35 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
@ 2023-01-19 01:10 ` Michael Paquier <[email protected]>
1 sibling, 0 replies; 15+ messages in thread
From: Michael Paquier @ 2023-01-19 01:10 UTC (permalink / raw)
To: Jelte Fennema <[email protected]>; +Cc: Jelte Fennema <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Nathan Bossart <[email protected]>; Andrew Dunstan <[email protected]>
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?
Nope. I just need some time to finish wrapping it, that's all.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../Y8iYom43ey8CpM%[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf
2023-01-11 11:05 Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-11 14:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-12 05:32 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-12 09:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-13 02:09 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-13 08:19 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-16 05:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-16 10:53 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-17 05:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-18 09:35 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
@ 2023-01-19 07:56 ` Michael Paquier <[email protected]>
1 sibling, 0 replies; 15+ messages in thread
From: Michael Paquier @ 2023-01-19 07:56 UTC (permalink / raw)
To: Jelte Fennema <[email protected]>; +Cc: Jelte Fennema <[email protected]>; [email protected] <[email protected]>; [email protected] <[email protected]>; Nathan Bossart <[email protected]>; Andrew Dunstan <[email protected]>
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
Attachments:
[text/x-diff] v6-0004-Support-same-user-patterns-in-pg_ident.conf-as-in.patch (16.8K, ../../[email protected]/2-v6-0004-Support-same-user-patterns-in-pg_ident.conf-as-in.patch)
download | inline diff:
From 0e42a5e9c2a532355df49346e47cc5612da1889d Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
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
[application/pgp-signature] signature.asc (833B, ../../[email protected]/3-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2023-01-19 07:56 UTC | newest]
Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-13 16:39 [PATCH] Avoid errors in brin summarization.. Alvaro Herrera <[email protected]>
2020-11-13 16:39 [PATCH v3] Avoid errors in brin summarization.. Alvaro Herrera <[email protected]>
2020-11-13 16:39 [PATCH v3] Avoid errors in brin summarization.. Alvaro Herrera <[email protected]>
2023-01-11 11:05 Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-11 14:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-12 05:32 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-12 09:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-13 02:09 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-13 08:19 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-16 05:22 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-16 10:53 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-17 05:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-18 09:35 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Jelte Fennema <[email protected]>
2023-01-19 01:10 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
2023-01-19 07:56 ` Re: [EXTERNAL] Re: [PATCH] Support using "all" for the db user in pg_ident.conf Michael Paquier <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox