In username-map substitution, cope with more than one \1.

If the system-name field of a pg_ident.conf line is a regex
containing capturing parentheses, you can write \1 in the
user-name field to represent the captured part of the system
name.  But what happens if you write \1 more than once?
The only reasonable expectation IMO is that each \1 gets
replaced, but presently our code replaces only the first.
Fix that.

Also, improve the tests for this feature to exercise cases
where a non-empty string needs to be substituted for \1.
The previous testing didn't inspire much faith that it
was verifying correct operation of the substitution code.

Given the lack of field complaints about this, I don't
feel a need to back-patch.

Reported-by: David G. Johnston <david.g.johnston@gmail.com>
Author: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CAKFQuwZu6kZ8ZPvJ3pWXig+6UX4nTVK-hdL_ZS3fSdps=RJQQQ@mail.gmail.com

diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 332fad278351c62b0dcbb3ea4e7fff246a87a742..fecee8224d075887d795fbc7536121b4dfb534dc 100644 (file)
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -2873,8 +2873,11 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
            !token_has_regexp(identLine->pg_user) &&
            (ofs = strstr(identLine->pg_user->string, "\\1")) != NULL)
        {
+           const char *repl_str;
+           size_t      repl_len;
+           char       *old_pg_user;
            char       *expanded_pg_user;
-           int         offset;
+           size_t      offset;
 
            /* substitution of the first argument requested */
            if (matches[1].rm_so < 0)
@@ -2886,18 +2889,33 @@ check_ident_usermap(IdentLine *identLine, const char *usermap_name,
                *error_p = true;
                return;
            }
+           repl_str = system_user + matches[1].rm_so;
+           repl_len = matches[1].rm_eo - matches[1].rm_so;
 
            /*
-            * length: original length minus length of \1 plus length of match
-            * plus null terminator
+            * It's allowed to have more than one \1 in the string, and we'll
+            * replace them all.  But that's pretty unusual so we optimize on
+            * the assumption of only one occurrence, which motivates doing
+            * repeated replacements instead of making two passes over the
+            * string to determine the final length right away.
             */
-           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);
-           strcat(expanded_pg_user, ofs + 2);
+           old_pg_user = identLine->pg_user->string;
+           do
+           {
+               /*
+                * length: current length minus length of \1 plus length of
+                * replacement plus null terminator
+                */
+               expanded_pg_user = palloc(strlen(old_pg_user) - 2 + repl_len + 1);
+               /* ofs points into the old_pg_user string at this point */
+               offset = ofs - old_pg_user;
+               memcpy(expanded_pg_user, old_pg_user, offset);
+               memcpy(expanded_pg_user + offset, repl_str, repl_len);
+               strcpy(expanded_pg_user + offset + repl_len, ofs + 2);
+               if (old_pg_user != identLine->pg_user->string)
+                   pfree(old_pg_user);
+               old_pg_user = expanded_pg_user;
+           } while ((ofs = strstr(old_pg_user + offset + repl_len, "\\1")) != NULL);
 
            /*
             * Mark the token as quoted, so it will only be compared literally

diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index f2320b62c872185a509800d81dd09786df8cbde9..c751fbdbaa5ece5c28b6d6c62b34204cdc4ac21d 100644 (file)
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -171,7 +171,8 @@ test_role(
 
 # 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).
+# or the entire system_user name (if its length is <= 3).
+# We trust this will not include any regex metacharacters.
 my $regex_test_string = substr($system_user, -3);
 
 # Success as the system user regular expression matches.
@@ -210,12 +211,17 @@ test_role(
    log_like =>
      [qr/connection authenticated: identity="$system_user" method=peer/]);
 
+# Create target role for \1 tests.
+my $mapped_name = "test${regex_test_string}map${regex_test_string}user";
+$node->safe_psql('postgres', "CREATE ROLE $mapped_name LOGIN");
+
 # Success as the regular expression matches and \1 is replaced in the given
 # subexpression.
-reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$}, 'test\1mapuser');
+reset_pg_ident($node, 'mypeermap', qq{/^.*($regex_test_string)\$},
+   'test\1map\1user');
 test_role(
    $node,
-   qq{testmapuser},
+   $mapped_name,
    'peer',
    0,
    'with regular expression in user name map with \1 replaced',
@@ -224,11 +230,11 @@ test_role(
 
 # Success as the regular expression matches and \1 is replaced in the given
 # subexpression, even if quoted.
-reset_pg_ident($node, 'mypeermap', qq{/^$system_user(.*)\$},
-   '"test\1mapuser"');
+reset_pg_ident($node, 'mypeermap', qq{/^.*($regex_test_string)\$},
+   '"test\1map\1user"');
 test_role(
    $node,
-   qq{testmapuser},
+   $mapped_name,
    'peer',
    0,
    'with regular expression in user name map with quoted \1 replaced',